On Tue, 20 Feb 2024 at 21:02, Marek Vasut <ma...@denx.de> wrote: > > 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. >
How about the following? Add support for GPCv2 power domains and clock handling for PCIe and PCIe PHY. It is required to enable PCIe support on the iMX8MP SoC. > > Signed-off-by: Sumit Garg <sumit.g...@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 ? Let me gate them behind corresponding power domain IDs. > > > 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 ? > Although I can't find their reference in the TRM, as per Linux commit [1], setting the reset bit is actually deassertion of PCIe PHY reset. You can think of it like an active low signal. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5506018d3dec41e6678efb92b836586e9ee1d628 -Sumit