| Message ID | 20240708-pci2_upstream-v7-4-ac00b8174f89@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PCI: imx6: Fix\rename\clean up and add lut information for imx95 | expand |
On Mon, Jul 08, 2024 at 01:08:08PM -0400, Frank Li wrote: > Instead of using the switch case statement to enable/disable the reference > clock handled by this driver itself, let's introduce a new callback > enable_ref_clk() and define it for platforms that require it. This > simplifies the code. > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > drivers/pci/controller/dwc/pci-imx6.c | 111 ++++++++++++++++------------------ > 1 file changed, 51 insertions(+), 60 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > index 47134e2dfecf2..dbcb70186036e 100644 > --- a/drivers/pci/controller/dwc/pci-imx6.c > +++ b/drivers/pci/controller/dwc/pci-imx6.c > @@ -103,6 +103,7 @@ struct imx_pcie_drvdata { > const u32 mode_mask[IMX_PCIE_MAX_INSTANCES]; > const struct pci_epc_features *epc_features; > int (*init_phy)(struct imx_pcie *pcie); > + int (*enable_ref_clk)(struct imx_pcie *pcie, bool enable); > }; > > struct imx_pcie { > @@ -585,21 +586,20 @@ static int imx_pcie_attach_pd(struct device *dev) > return 0; > } > > -static int imx_pcie_enable_ref_clk(struct imx_pcie *imx_pcie) > +static int imx6sx_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable) > { > - unsigned int offset; > - int ret = 0; > + if (enable) > + regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR12, > + IMX6SX_GPR12_PCIE_TEST_POWERDOWN); Since all SoCs except IMX6Q/6QP doesn't have both enable/disable controls (which is very weird btw), you can have separate enable/disable callbacks and just populate the ones that require. This way it becomes clear which SoC is supporting what. If you have a common helper and just toggle based on a bool, then it becomes hard to follow. - Mani > > - switch (imx_pcie->drvdata->variant) { > - case IMX6SX: > - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR12, > - IMX6SX_GPR12_PCIE_TEST_POWERDOWN, 0); > - break; > - case IMX6QP: > - case IMX6Q: > + return 0; > +} > + > +static int imx6q_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable) > +{ > + if (enable) { > /* power up core phy and enable ref clock */ > - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, > - IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18); > + regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_TEST_PD); > /* > * the async reset input need ref clock to sync internally, > * when the ref clock comes after reset, internal synced > @@ -607,55 +607,33 @@ static int imx_pcie_enable_ref_clk(struct imx_pcie *imx_pcie) > * add one ~10us delay here. > */ > usleep_range(10, 100); > - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, > - IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16); > - break; > - case IMX7D: > - case IMX95: > - case IMX95_EP: > - break; > - case IMX8MM: > - case IMX8MM_EP: > - case IMX8MQ: > - case IMX8MQ_EP: > - case IMX8MP: > - case IMX8MP_EP: > - offset = imx_pcie_grp_offset(imx_pcie); > - /* > - * Set the over ride low and enabled > - * make sure that REF_CLK is turned on. > - */ > - regmap_update_bits(imx_pcie->iomuxc_gpr, offset, > - IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE, > - 0); > - regmap_update_bits(imx_pcie->iomuxc_gpr, offset, > - IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN, > - IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN); > - break; > + regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_REF_CLK_EN); > + } else { > + regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_REF_CLK_EN); > + regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_TEST_PD); > } > > - return ret; > + return 0; > } > > -static void imx_pcie_disable_ref_clk(struct imx_pcie *imx_pcie) > +static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable) > { > - switch (imx_pcie->drvdata->variant) { > - case IMX6QP: > - case IMX6Q: > - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, > - IMX6Q_GPR1_PCIE_REF_CLK_EN, 0); > - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, > - IMX6Q_GPR1_PCIE_TEST_PD, > - IMX6Q_GPR1_PCIE_TEST_PD); > - break; > - case IMX7D: > - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR12, > - IMX7D_GPR12_PCIE_PHY_REFCLK_SEL, > - IMX7D_GPR12_PCIE_PHY_REFCLK_SEL); > - break; > - default: > - break; > + int offset = imx_pcie_grp_offset(imx_pcie); > + > + if (enable) { > + regmap_clear_bits(imx_pcie->iomuxc_gpr, offset, IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE); > + regmap_set_bits(imx_pcie->iomuxc_gpr, offset, IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN); > } > + > + return 0; > +} > + > +static int imx7d_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable) > +{ > + if (!enable) > + regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR12, > + IMX7D_GPR12_PCIE_PHY_REFCLK_SEL); > + return 0; > } > > static int imx_pcie_clk_enable(struct imx_pcie *imx_pcie) > @@ -668,10 +646,12 @@ static int imx_pcie_clk_enable(struct imx_pcie *imx_pcie) > if (ret) > return ret; > > - ret = imx_pcie_enable_ref_clk(imx_pcie); > - if (ret) { > - dev_err(dev, "unable to enable pcie ref clock\n"); > - goto err_ref_clk; > + if (imx_pcie->drvdata->enable_ref_clk) { > + ret = imx_pcie->drvdata->enable_ref_clk(imx_pcie, true); > + if (ret) { > + dev_err(dev, "Failed to enable PCIe REFCLK\n"); > + goto err_ref_clk; > + } > } > > /* allow the clocks to stabilize */ > @@ -686,7 +666,8 @@ static int imx_pcie_clk_enable(struct imx_pcie *imx_pcie) > > static void imx_pcie_clk_disable(struct imx_pcie *imx_pcie) > { > - imx_pcie_disable_ref_clk(imx_pcie); > + if (imx_pcie->drvdata->enable_ref_clk) > + imx_pcie->drvdata->enable_ref_clk(imx_pcie, false); > clk_bulk_disable_unprepare(imx_pcie->drvdata->clks_cnt, imx_pcie->clks); > } > > @@ -1475,6 +1456,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > .mode_off[0] = IOMUXC_GPR12, > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > .init_phy = imx_pcie_init_phy, > + .enable_ref_clk = imx6q_pcie_enable_ref_clk, > }, > [IMX6SX] = { > .variant = IMX6SX, > @@ -1489,6 +1471,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > .mode_off[0] = IOMUXC_GPR12, > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > .init_phy = imx6sx_pcie_init_phy, > + .enable_ref_clk = imx6sx_pcie_enable_ref_clk, > }, > [IMX6QP] = { > .variant = IMX6QP, > @@ -1504,6 +1487,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > .mode_off[0] = IOMUXC_GPR12, > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > .init_phy = imx_pcie_init_phy, > + .enable_ref_clk = imx6q_pcie_enable_ref_clk, > }, > [IMX7D] = { > .variant = IMX7D, > @@ -1516,6 +1500,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > .mode_off[0] = IOMUXC_GPR12, > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > .init_phy = imx7d_pcie_init_phy, > + .enable_ref_clk = imx7d_pcie_enable_ref_clk, > }, > [IMX8MQ] = { > .variant = IMX8MQ, > @@ -1529,6 +1514,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > .mode_off[1] = IOMUXC_GPR12, > .mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE, > .init_phy = imx8mq_pcie_init_phy, > + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, > }, > [IMX8MM] = { > .variant = IMX8MM, > @@ -1540,6 +1526,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > .clks_cnt = ARRAY_SIZE(imx8mm_clks), > .mode_off[0] = IOMUXC_GPR12, > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, > }, > [IMX8MP] = { > .variant = IMX8MP, > @@ -1551,6 +1538,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > .clks_cnt = ARRAY_SIZE(imx8mm_clks), > .mode_off[0] = IOMUXC_GPR12, > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, > }, > [IMX95] = { > .variant = IMX95, > @@ -1577,6 +1565,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > .mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE, > .epc_features = &imx8m_pcie_epc_features, > .init_phy = imx8mq_pcie_init_phy, > + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, > }, > [IMX8MM_EP] = { > .variant = IMX8MM_EP, > @@ -1589,6 +1578,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > .mode_off[0] = IOMUXC_GPR12, > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > .epc_features = &imx8m_pcie_epc_features, > + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, > }, > [IMX8MP_EP] = { > .variant = IMX8MP_EP, > @@ -1601,6 +1591,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > .mode_off[0] = IOMUXC_GPR12, > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > .epc_features = &imx8m_pcie_epc_features, > + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, > }, > [IMX95_EP] = { > .variant = IMX95_EP, > > -- > 2.34.1 >
On Sun, Jul 21, 2024 at 01:26:34PM +0530, Manivannan Sadhasivam wrote: > On Mon, Jul 08, 2024 at 01:08:08PM -0400, Frank Li wrote: > > Instead of using the switch case statement to enable/disable the reference > > clock handled by this driver itself, let's introduce a new callback > > enable_ref_clk() and define it for platforms that require it. This > > simplifies the code. > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > drivers/pci/controller/dwc/pci-imx6.c | 111 ++++++++++++++++------------------ > > 1 file changed, 51 insertions(+), 60 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > > index 47134e2dfecf2..dbcb70186036e 100644 > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > @@ -103,6 +103,7 @@ struct imx_pcie_drvdata { > > const u32 mode_mask[IMX_PCIE_MAX_INSTANCES]; > > const struct pci_epc_features *epc_features; > > int (*init_phy)(struct imx_pcie *pcie); > > + int (*enable_ref_clk)(struct imx_pcie *pcie, bool enable); > > }; > > > > struct imx_pcie { > > @@ -585,21 +586,20 @@ static int imx_pcie_attach_pd(struct device *dev) > > return 0; > > } > > > > -static int imx_pcie_enable_ref_clk(struct imx_pcie *imx_pcie) > > +static int imx6sx_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable) > > { > > - unsigned int offset; > > - int ret = 0; > > + if (enable) > > + regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR12, > > + IMX6SX_GPR12_PCIE_TEST_POWERDOWN); > > Since all SoCs except IMX6Q/6QP doesn't have both enable/disable controls (which > is very weird btw), you can have separate enable/disable callbacks and just > populate the ones that require. I think old code is wrong, which depended on hardware reset value. It should paired between enable/disable. I just want to keep the same logic here as old code. I have another patches to improve these. This patch series were already big, I want to do it after these patch merged. Is it okay? Frank > > This way it becomes clear which SoC is supporting what. If you have a common > helper and just toggle based on a bool, then it becomes hard to follow. > > - Mani > > > > > - switch (imx_pcie->drvdata->variant) { > > - case IMX6SX: > > - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR12, > > - IMX6SX_GPR12_PCIE_TEST_POWERDOWN, 0); > > - break; > > - case IMX6QP: > > - case IMX6Q: > > + return 0; > > +} > > + > > +static int imx6q_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable) > > +{ > > + if (enable) { > > /* power up core phy and enable ref clock */ > > - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, > > - IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18); > > + regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_TEST_PD); > > /* > > * the async reset input need ref clock to sync internally, > > * when the ref clock comes after reset, internal synced > > @@ -607,55 +607,33 @@ static int imx_pcie_enable_ref_clk(struct imx_pcie *imx_pcie) > > * add one ~10us delay here. > > */ > > usleep_range(10, 100); > > - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, > > - IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16); > > - break; > > - case IMX7D: > > - case IMX95: > > - case IMX95_EP: > > - break; > > - case IMX8MM: > > - case IMX8MM_EP: > > - case IMX8MQ: > > - case IMX8MQ_EP: > > - case IMX8MP: > > - case IMX8MP_EP: > > - offset = imx_pcie_grp_offset(imx_pcie); > > - /* > > - * Set the over ride low and enabled > > - * make sure that REF_CLK is turned on. > > - */ > > - regmap_update_bits(imx_pcie->iomuxc_gpr, offset, > > - IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE, > > - 0); > > - regmap_update_bits(imx_pcie->iomuxc_gpr, offset, > > - IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN, > > - IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN); > > - break; > > + regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_REF_CLK_EN); > > + } else { > > + regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_REF_CLK_EN); > > + regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_TEST_PD); > > } > > > > - return ret; > > + return 0; > > } > > > > -static void imx_pcie_disable_ref_clk(struct imx_pcie *imx_pcie) > > +static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable) > > { > > - switch (imx_pcie->drvdata->variant) { > > - case IMX6QP: > > - case IMX6Q: > > - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, > > - IMX6Q_GPR1_PCIE_REF_CLK_EN, 0); > > - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, > > - IMX6Q_GPR1_PCIE_TEST_PD, > > - IMX6Q_GPR1_PCIE_TEST_PD); > > - break; > > - case IMX7D: > > - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR12, > > - IMX7D_GPR12_PCIE_PHY_REFCLK_SEL, > > - IMX7D_GPR12_PCIE_PHY_REFCLK_SEL); > > - break; > > - default: > > - break; > > + int offset = imx_pcie_grp_offset(imx_pcie); > > + > > + if (enable) { > > + regmap_clear_bits(imx_pcie->iomuxc_gpr, offset, IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE); > > + regmap_set_bits(imx_pcie->iomuxc_gpr, offset, IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN); > > } > > + > > + return 0; > > +} > > + > > +static int imx7d_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable) > > +{ > > + if (!enable) > > + regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR12, > > + IMX7D_GPR12_PCIE_PHY_REFCLK_SEL); > > + return 0; > > } > > > > static int imx_pcie_clk_enable(struct imx_pcie *imx_pcie) > > @@ -668,10 +646,12 @@ static int imx_pcie_clk_enable(struct imx_pcie *imx_pcie) > > if (ret) > > return ret; > > > > - ret = imx_pcie_enable_ref_clk(imx_pcie); > > - if (ret) { > > - dev_err(dev, "unable to enable pcie ref clock\n"); > > - goto err_ref_clk; > > + if (imx_pcie->drvdata->enable_ref_clk) { > > + ret = imx_pcie->drvdata->enable_ref_clk(imx_pcie, true); > > + if (ret) { > > + dev_err(dev, "Failed to enable PCIe REFCLK\n"); > > + goto err_ref_clk; > > + } > > } > > > > /* allow the clocks to stabilize */ > > @@ -686,7 +666,8 @@ static int imx_pcie_clk_enable(struct imx_pcie *imx_pcie) > > > > static void imx_pcie_clk_disable(struct imx_pcie *imx_pcie) > > { > > - imx_pcie_disable_ref_clk(imx_pcie); > > + if (imx_pcie->drvdata->enable_ref_clk) > > + imx_pcie->drvdata->enable_ref_clk(imx_pcie, false); > > clk_bulk_disable_unprepare(imx_pcie->drvdata->clks_cnt, imx_pcie->clks); > > } > > > > @@ -1475,6 +1456,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > > .mode_off[0] = IOMUXC_GPR12, > > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > > .init_phy = imx_pcie_init_phy, > > + .enable_ref_clk = imx6q_pcie_enable_ref_clk, > > }, > > [IMX6SX] = { > > .variant = IMX6SX, > > @@ -1489,6 +1471,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > > .mode_off[0] = IOMUXC_GPR12, > > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > > .init_phy = imx6sx_pcie_init_phy, > > + .enable_ref_clk = imx6sx_pcie_enable_ref_clk, > > }, > > [IMX6QP] = { > > .variant = IMX6QP, > > @@ -1504,6 +1487,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > > .mode_off[0] = IOMUXC_GPR12, > > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > > .init_phy = imx_pcie_init_phy, > > + .enable_ref_clk = imx6q_pcie_enable_ref_clk, > > }, > > [IMX7D] = { > > .variant = IMX7D, > > @@ -1516,6 +1500,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > > .mode_off[0] = IOMUXC_GPR12, > > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > > .init_phy = imx7d_pcie_init_phy, > > + .enable_ref_clk = imx7d_pcie_enable_ref_clk, > > }, > > [IMX8MQ] = { > > .variant = IMX8MQ, > > @@ -1529,6 +1514,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > > .mode_off[1] = IOMUXC_GPR12, > > .mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE, > > .init_phy = imx8mq_pcie_init_phy, > > + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, > > }, > > [IMX8MM] = { > > .variant = IMX8MM, > > @@ -1540,6 +1526,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > > .clks_cnt = ARRAY_SIZE(imx8mm_clks), > > .mode_off[0] = IOMUXC_GPR12, > > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > > + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, > > }, > > [IMX8MP] = { > > .variant = IMX8MP, > > @@ -1551,6 +1538,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > > .clks_cnt = ARRAY_SIZE(imx8mm_clks), > > .mode_off[0] = IOMUXC_GPR12, > > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > > + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, > > }, > > [IMX95] = { > > .variant = IMX95, > > @@ -1577,6 +1565,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > > .mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE, > > .epc_features = &imx8m_pcie_epc_features, > > .init_phy = imx8mq_pcie_init_phy, > > + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, > > }, > > [IMX8MM_EP] = { > > .variant = IMX8MM_EP, > > @@ -1589,6 +1578,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > > .mode_off[0] = IOMUXC_GPR12, > > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > > .epc_features = &imx8m_pcie_epc_features, > > + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, > > }, > > [IMX8MP_EP] = { > > .variant = IMX8MP_EP, > > @@ -1601,6 +1591,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > > .mode_off[0] = IOMUXC_GPR12, > > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > > .epc_features = &imx8m_pcie_epc_features, > > + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, > > }, > > [IMX95_EP] = { > > .variant = IMX95_EP, > > > > -- > > 2.34.1 > > > > -- > மணிவண்ணன் சதாசிவம்
On Mon, Jul 22, 2024 at 11:16:25AM -0400, Frank Li wrote: > On Sun, Jul 21, 2024 at 01:26:34PM +0530, Manivannan Sadhasivam wrote: > > On Mon, Jul 08, 2024 at 01:08:08PM -0400, Frank Li wrote: > > > Instead of using the switch case statement to enable/disable the reference > > > clock handled by this driver itself, let's introduce a new callback > > > enable_ref_clk() and define it for platforms that require it. This > > > simplifies the code. > > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > --- > > > drivers/pci/controller/dwc/pci-imx6.c | 111 ++++++++++++++++------------------ > > > 1 file changed, 51 insertions(+), 60 deletions(-) > > > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > > > index 47134e2dfecf2..dbcb70186036e 100644 > > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > > @@ -103,6 +103,7 @@ struct imx_pcie_drvdata { > > > const u32 mode_mask[IMX_PCIE_MAX_INSTANCES]; > > > const struct pci_epc_features *epc_features; > > > int (*init_phy)(struct imx_pcie *pcie); > > > + int (*enable_ref_clk)(struct imx_pcie *pcie, bool enable); > > > }; > > > > > > struct imx_pcie { > > > @@ -585,21 +586,20 @@ static int imx_pcie_attach_pd(struct device *dev) > > > return 0; > > > } > > > > > > -static int imx_pcie_enable_ref_clk(struct imx_pcie *imx_pcie) > > > +static int imx6sx_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable) > > > { > > > - unsigned int offset; > > > - int ret = 0; > > > + if (enable) > > > + regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR12, > > > + IMX6SX_GPR12_PCIE_TEST_POWERDOWN); > > > > Since all SoCs except IMX6Q/6QP doesn't have both enable/disable controls (which > > is very weird btw), you can have separate enable/disable callbacks and just > > populate the ones that require. > > I think old code is wrong, which depended on hardware reset value. It > should paired between enable/disable. I just want to keep the same logic > here as old code. I have another patches to improve these. This patch > series were already big, I want to do it after these patch merged. > > Is it okay? > Fine with me. - Mani > Frank > > > > > This way it becomes clear which SoC is supporting what. If you have a common > > helper and just toggle based on a bool, then it becomes hard to follow. > > > > - Mani > > > > > > > > - switch (imx_pcie->drvdata->variant) { > > > - case IMX6SX: > > > - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR12, > > > - IMX6SX_GPR12_PCIE_TEST_POWERDOWN, 0); > > > - break; > > > - case IMX6QP: > > > - case IMX6Q: > > > + return 0; > > > +} > > > + > > > +static int imx6q_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable) > > > +{ > > > + if (enable) { > > > /* power up core phy and enable ref clock */ > > > - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, > > > - IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18); > > > + regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_TEST_PD); > > > /* > > > * the async reset input need ref clock to sync internally, > > > * when the ref clock comes after reset, internal synced > > > @@ -607,55 +607,33 @@ static int imx_pcie_enable_ref_clk(struct imx_pcie *imx_pcie) > > > * add one ~10us delay here. > > > */ > > > usleep_range(10, 100); > > > - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, > > > - IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16); > > > - break; > > > - case IMX7D: > > > - case IMX95: > > > - case IMX95_EP: > > > - break; > > > - case IMX8MM: > > > - case IMX8MM_EP: > > > - case IMX8MQ: > > > - case IMX8MQ_EP: > > > - case IMX8MP: > > > - case IMX8MP_EP: > > > - offset = imx_pcie_grp_offset(imx_pcie); > > > - /* > > > - * Set the over ride low and enabled > > > - * make sure that REF_CLK is turned on. > > > - */ > > > - regmap_update_bits(imx_pcie->iomuxc_gpr, offset, > > > - IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE, > > > - 0); > > > - regmap_update_bits(imx_pcie->iomuxc_gpr, offset, > > > - IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN, > > > - IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN); > > > - break; > > > + regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_REF_CLK_EN); > > > + } else { > > > + regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_REF_CLK_EN); > > > + regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_TEST_PD); > > > } > > > > > > - return ret; > > > + return 0; > > > } > > > > > > -static void imx_pcie_disable_ref_clk(struct imx_pcie *imx_pcie) > > > +static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable) > > > { > > > - switch (imx_pcie->drvdata->variant) { > > > - case IMX6QP: > > > - case IMX6Q: > > > - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, > > > - IMX6Q_GPR1_PCIE_REF_CLK_EN, 0); > > > - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, > > > - IMX6Q_GPR1_PCIE_TEST_PD, > > > - IMX6Q_GPR1_PCIE_TEST_PD); > > > - break; > > > - case IMX7D: > > > - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR12, > > > - IMX7D_GPR12_PCIE_PHY_REFCLK_SEL, > > > - IMX7D_GPR12_PCIE_PHY_REFCLK_SEL); > > > - break; > > > - default: > > > - break; > > > + int offset = imx_pcie_grp_offset(imx_pcie); > > > + > > > + if (enable) { > > > + regmap_clear_bits(imx_pcie->iomuxc_gpr, offset, IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE); > > > + regmap_set_bits(imx_pcie->iomuxc_gpr, offset, IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN); > > > } > > > + > > > + return 0; > > > +} > > > + > > > +static int imx7d_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable) > > > +{ > > > + if (!enable) > > > + regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR12, > > > + IMX7D_GPR12_PCIE_PHY_REFCLK_SEL); > > > + return 0; > > > } > > > > > > static int imx_pcie_clk_enable(struct imx_pcie *imx_pcie) > > > @@ -668,10 +646,12 @@ static int imx_pcie_clk_enable(struct imx_pcie *imx_pcie) > > > if (ret) > > > return ret; > > > > > > - ret = imx_pcie_enable_ref_clk(imx_pcie); > > > - if (ret) { > > > - dev_err(dev, "unable to enable pcie ref clock\n"); > > > - goto err_ref_clk; > > > + if (imx_pcie->drvdata->enable_ref_clk) { > > > + ret = imx_pcie->drvdata->enable_ref_clk(imx_pcie, true); > > > + if (ret) { > > > + dev_err(dev, "Failed to enable PCIe REFCLK\n"); > > > + goto err_ref_clk; > > > + } > > > } > > > > > > /* allow the clocks to stabilize */ > > > @@ -686,7 +666,8 @@ static int imx_pcie_clk_enable(struct imx_pcie *imx_pcie) > > > > > > static void imx_pcie_clk_disable(struct imx_pcie *imx_pcie) > > > { > > > - imx_pcie_disable_ref_clk(imx_pcie); > > > + if (imx_pcie->drvdata->enable_ref_clk) > > > + imx_pcie->drvdata->enable_ref_clk(imx_pcie, false); > > > clk_bulk_disable_unprepare(imx_pcie->drvdata->clks_cnt, imx_pcie->clks); > > > } > > > > > > @@ -1475,6 +1456,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > > > .mode_off[0] = IOMUXC_GPR12, > > > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > > > .init_phy = imx_pcie_init_phy, > > > + .enable_ref_clk = imx6q_pcie_enable_ref_clk, > > > }, > > > [IMX6SX] = { > > > .variant = IMX6SX, > > > @@ -1489,6 +1471,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > > > .mode_off[0] = IOMUXC_GPR12, > > > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > > > .init_phy = imx6sx_pcie_init_phy, > > > + .enable_ref_clk = imx6sx_pcie_enable_ref_clk, > > > }, > > > [IMX6QP] = { > > > .variant = IMX6QP, > > > @@ -1504,6 +1487,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > > > .mode_off[0] = IOMUXC_GPR12, > > > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > > > .init_phy = imx_pcie_init_phy, > > > + .enable_ref_clk = imx6q_pcie_enable_ref_clk, > > > }, > > > [IMX7D] = { > > > .variant = IMX7D, > > > @@ -1516,6 +1500,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > > > .mode_off[0] = IOMUXC_GPR12, > > > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > > > .init_phy = imx7d_pcie_init_phy, > > > + .enable_ref_clk = imx7d_pcie_enable_ref_clk, > > > }, > > > [IMX8MQ] = { > > > .variant = IMX8MQ, > > > @@ -1529,6 +1514,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > > > .mode_off[1] = IOMUXC_GPR12, > > > .mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE, > > > .init_phy = imx8mq_pcie_init_phy, > > > + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, > > > }, > > > [IMX8MM] = { > > > .variant = IMX8MM, > > > @@ -1540,6 +1526,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > > > .clks_cnt = ARRAY_SIZE(imx8mm_clks), > > > .mode_off[0] = IOMUXC_GPR12, > > > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > > > + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, > > > }, > > > [IMX8MP] = { > > > .variant = IMX8MP, > > > @@ -1551,6 +1538,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > > > .clks_cnt = ARRAY_SIZE(imx8mm_clks), > > > .mode_off[0] = IOMUXC_GPR12, > > > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > > > + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, > > > }, > > > [IMX95] = { > > > .variant = IMX95, > > > @@ -1577,6 +1565,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > > > .mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE, > > > .epc_features = &imx8m_pcie_epc_features, > > > .init_phy = imx8mq_pcie_init_phy, > > > + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, > > > }, > > > [IMX8MM_EP] = { > > > .variant = IMX8MM_EP, > > > @@ -1589,6 +1578,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > > > .mode_off[0] = IOMUXC_GPR12, > > > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > > > .epc_features = &imx8m_pcie_epc_features, > > > + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, > > > }, > > > [IMX8MP_EP] = { > > > .variant = IMX8MP_EP, > > > @@ -1601,6 +1591,7 @@ static const struct imx_pcie_drvdata drvdata[] = { > > > .mode_off[0] = IOMUXC_GPR12, > > > .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, > > > .epc_features = &imx8m_pcie_epc_features, > > > + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, > > > }, > > > [IMX95_EP] = { > > > .variant = IMX95_EP, > > > > > > -- > > > 2.34.1 > > > > > > > -- > > மணிவண்ணன் சதாசிவம் >
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c index 47134e2dfecf2..dbcb70186036e 100644 --- a/drivers/pci/controller/dwc/pci-imx6.c +++ b/drivers/pci/controller/dwc/pci-imx6.c @@ -103,6 +103,7 @@ struct imx_pcie_drvdata { const u32 mode_mask[IMX_PCIE_MAX_INSTANCES]; const struct pci_epc_features *epc_features; int (*init_phy)(struct imx_pcie *pcie); + int (*enable_ref_clk)(struct imx_pcie *pcie, bool enable); }; struct imx_pcie { @@ -585,21 +586,20 @@ static int imx_pcie_attach_pd(struct device *dev) return 0; } -static int imx_pcie_enable_ref_clk(struct imx_pcie *imx_pcie) +static int imx6sx_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable) { - unsigned int offset; - int ret = 0; + if (enable) + regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR12, + IMX6SX_GPR12_PCIE_TEST_POWERDOWN); - switch (imx_pcie->drvdata->variant) { - case IMX6SX: - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR12, - IMX6SX_GPR12_PCIE_TEST_POWERDOWN, 0); - break; - case IMX6QP: - case IMX6Q: + return 0; +} + +static int imx6q_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable) +{ + if (enable) { /* power up core phy and enable ref clock */ - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, - IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18); + regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_TEST_PD); /* * the async reset input need ref clock to sync internally, * when the ref clock comes after reset, internal synced @@ -607,55 +607,33 @@ static int imx_pcie_enable_ref_clk(struct imx_pcie *imx_pcie) * add one ~10us delay here. */ usleep_range(10, 100); - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, - IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16); - break; - case IMX7D: - case IMX95: - case IMX95_EP: - break; - case IMX8MM: - case IMX8MM_EP: - case IMX8MQ: - case IMX8MQ_EP: - case IMX8MP: - case IMX8MP_EP: - offset = imx_pcie_grp_offset(imx_pcie); - /* - * Set the over ride low and enabled - * make sure that REF_CLK is turned on. - */ - regmap_update_bits(imx_pcie->iomuxc_gpr, offset, - IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE, - 0); - regmap_update_bits(imx_pcie->iomuxc_gpr, offset, - IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN, - IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN); - break; + regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_REF_CLK_EN); + } else { + regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_REF_CLK_EN); + regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_TEST_PD); } - return ret; + return 0; } -static void imx_pcie_disable_ref_clk(struct imx_pcie *imx_pcie) +static int imx8mm_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable) { - switch (imx_pcie->drvdata->variant) { - case IMX6QP: - case IMX6Q: - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, - IMX6Q_GPR1_PCIE_REF_CLK_EN, 0); - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, - IMX6Q_GPR1_PCIE_TEST_PD, - IMX6Q_GPR1_PCIE_TEST_PD); - break; - case IMX7D: - regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR12, - IMX7D_GPR12_PCIE_PHY_REFCLK_SEL, - IMX7D_GPR12_PCIE_PHY_REFCLK_SEL); - break; - default: - break; + int offset = imx_pcie_grp_offset(imx_pcie); + + if (enable) { + regmap_clear_bits(imx_pcie->iomuxc_gpr, offset, IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE); + regmap_set_bits(imx_pcie->iomuxc_gpr, offset, IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE_EN); } + + return 0; +} + +static int imx7d_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable) +{ + if (!enable) + regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR12, + IMX7D_GPR12_PCIE_PHY_REFCLK_SEL); + return 0; } static int imx_pcie_clk_enable(struct imx_pcie *imx_pcie) @@ -668,10 +646,12 @@ static int imx_pcie_clk_enable(struct imx_pcie *imx_pcie) if (ret) return ret; - ret = imx_pcie_enable_ref_clk(imx_pcie); - if (ret) { - dev_err(dev, "unable to enable pcie ref clock\n"); - goto err_ref_clk; + if (imx_pcie->drvdata->enable_ref_clk) { + ret = imx_pcie->drvdata->enable_ref_clk(imx_pcie, true); + if (ret) { + dev_err(dev, "Failed to enable PCIe REFCLK\n"); + goto err_ref_clk; + } } /* allow the clocks to stabilize */ @@ -686,7 +666,8 @@ static int imx_pcie_clk_enable(struct imx_pcie *imx_pcie) static void imx_pcie_clk_disable(struct imx_pcie *imx_pcie) { - imx_pcie_disable_ref_clk(imx_pcie); + if (imx_pcie->drvdata->enable_ref_clk) + imx_pcie->drvdata->enable_ref_clk(imx_pcie, false); clk_bulk_disable_unprepare(imx_pcie->drvdata->clks_cnt, imx_pcie->clks); } @@ -1475,6 +1456,7 @@ static const struct imx_pcie_drvdata drvdata[] = { .mode_off[0] = IOMUXC_GPR12, .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, .init_phy = imx_pcie_init_phy, + .enable_ref_clk = imx6q_pcie_enable_ref_clk, }, [IMX6SX] = { .variant = IMX6SX, @@ -1489,6 +1471,7 @@ static const struct imx_pcie_drvdata drvdata[] = { .mode_off[0] = IOMUXC_GPR12, .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, .init_phy = imx6sx_pcie_init_phy, + .enable_ref_clk = imx6sx_pcie_enable_ref_clk, }, [IMX6QP] = { .variant = IMX6QP, @@ -1504,6 +1487,7 @@ static const struct imx_pcie_drvdata drvdata[] = { .mode_off[0] = IOMUXC_GPR12, .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, .init_phy = imx_pcie_init_phy, + .enable_ref_clk = imx6q_pcie_enable_ref_clk, }, [IMX7D] = { .variant = IMX7D, @@ -1516,6 +1500,7 @@ static const struct imx_pcie_drvdata drvdata[] = { .mode_off[0] = IOMUXC_GPR12, .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, .init_phy = imx7d_pcie_init_phy, + .enable_ref_clk = imx7d_pcie_enable_ref_clk, }, [IMX8MQ] = { .variant = IMX8MQ, @@ -1529,6 +1514,7 @@ static const struct imx_pcie_drvdata drvdata[] = { .mode_off[1] = IOMUXC_GPR12, .mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE, .init_phy = imx8mq_pcie_init_phy, + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, }, [IMX8MM] = { .variant = IMX8MM, @@ -1540,6 +1526,7 @@ static const struct imx_pcie_drvdata drvdata[] = { .clks_cnt = ARRAY_SIZE(imx8mm_clks), .mode_off[0] = IOMUXC_GPR12, .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, }, [IMX8MP] = { .variant = IMX8MP, @@ -1551,6 +1538,7 @@ static const struct imx_pcie_drvdata drvdata[] = { .clks_cnt = ARRAY_SIZE(imx8mm_clks), .mode_off[0] = IOMUXC_GPR12, .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, }, [IMX95] = { .variant = IMX95, @@ -1577,6 +1565,7 @@ static const struct imx_pcie_drvdata drvdata[] = { .mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE, .epc_features = &imx8m_pcie_epc_features, .init_phy = imx8mq_pcie_init_phy, + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, }, [IMX8MM_EP] = { .variant = IMX8MM_EP, @@ -1589,6 +1578,7 @@ static const struct imx_pcie_drvdata drvdata[] = { .mode_off[0] = IOMUXC_GPR12, .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, .epc_features = &imx8m_pcie_epc_features, + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, }, [IMX8MP_EP] = { .variant = IMX8MP_EP, @@ -1601,6 +1591,7 @@ static const struct imx_pcie_drvdata drvdata[] = { .mode_off[0] = IOMUXC_GPR12, .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE, .epc_features = &imx8m_pcie_epc_features, + .enable_ref_clk = imx8mm_pcie_enable_ref_clk, }, [IMX95_EP] = { .variant = IMX95_EP,
Instead of using the switch case statement to enable/disable the reference clock handled by this driver itself, let's introduce a new callback enable_ref_clk() and define it for platforms that require it. This simplifies the code. Signed-off-by: Frank Li <Frank.Li@nxp.com> --- drivers/pci/controller/dwc/pci-imx6.c | 111 ++++++++++++++++------------------ 1 file changed, 51 insertions(+), 60 deletions(-)