[PATCH 3/7] imx8mp: power-domain: Add PCIe support

[Marek Vasut]

On 2/20/24 14:10, Sumit Garg wrote:
> Pre-requisite to enable PCIe support on iMX8MP SoC.

This commit message is useless, write a proper one.

> Signed-off-by: Sumit Garg <sumit.garg at linaro.org>
> ---
>   drivers/power/domain/imx8mp-hsiomix.c | 50 +++++++++++++++++++++++++--
>   1 file changed, 48 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/power/domain/imx8mp-hsiomix.c b/drivers/power/domain/imx8mp-hsiomix.c
> index e2d772c5ec7..62145e0261b 100644
> --- a/drivers/power/domain/imx8mp-hsiomix.c
> +++ b/drivers/power/domain/imx8mp-hsiomix.c
> @@ -16,14 +16,19 @@
>   #define GPR_REG0		0x0
>   #define  PCIE_CLOCK_MODULE_EN	BIT(0)
>   #define  USB_CLOCK_MODULE_EN	BIT(1)
> +#define  PCIE_PHY_APB_RST	BIT(4)
> +#define  PCIE_PHY_INIT_RST	BIT(5)
>   
>   struct imx8mp_hsiomix_priv {
>   	void __iomem *base;
>   	struct clk clk_usb;
> +	struct clk clk_pcie;
>   	struct power_domain pd_bus;
>   	struct power_domain pd_usb;
> +	struct power_domain pd_pcie;
>   	struct power_domain pd_usb_phy1;
>   	struct power_domain pd_usb_phy2;
> +	struct power_domain pd_pcie_phy;
>   };
>   
>   static int imx8mp_hsiomix_on(struct power_domain *power_domain)
> @@ -43,6 +48,10 @@ static int imx8mp_hsiomix_on(struct power_domain *power_domain)
>   		domain = &priv->pd_usb_phy1;
>   	} else if (power_domain->id == IMX8MP_HSIOBLK_PD_USB_PHY2) {
>   		domain = &priv->pd_usb_phy2;
> +	} else if (power_domain->id == IMX8MP_HSIOBLK_PD_PCIE) {
> +		domain = &priv->pd_pcie;
> +	} else if (power_domain->id == IMX8MP_HSIOBLK_PD_PCIE_PHY) {
> +		domain = &priv->pd_pcie_phy;
>   	} else {
>   		ret = -EINVAL;
>   		goto err_pd;
> @@ -54,14 +63,25 @@ static int imx8mp_hsiomix_on(struct power_domain *power_domain)
>   
>   	ret = clk_enable(&priv->clk_usb);
>   	if (ret)
> -		goto err_clk;
> +		goto err_clk_usb;
> +
> +	ret = clk_enable(&priv->clk_pcie);
> +	if (ret)
> +		goto err_clk_pcie;

Does this mean that when USB power domains get enabled, PCIe clock are 
also enabled ? Why ?

What if the PCIe clock enable fails, do USB clock remain enabled ?

>   	if (power_domain->id == IMX8MP_HSIOBLK_PD_USB)
>   		setbits_le32(priv->base + GPR_REG0, USB_CLOCK_MODULE_EN);
> +	else if (power_domain->id == IMX8MP_HSIOBLK_PD_PCIE)
> +		setbits_le32(priv->base + GPR_REG0, PCIE_CLOCK_MODULE_EN);
> +	else if (power_domain->id == IMX8MP_HSIOBLK_PD_PCIE_PHY)
> +		setbits_le32(priv->base + GPR_REG0, PCIE_PHY_APB_RST |
> +						    PCIE_PHY_INIT_RST);

Shouldn't the reset bits be cleared here ?

[...]