On Wed, Apr 20, 2016 at 08:45:29AM -0700, Andrey Smirnov wrote: > Use enumerated type instead of a boolean flag to specify the variant of > the PCIe IP block (6Q, 6SX, etc). This patch has zero functional impact, > however it makes the code easier to extend for the case of more than 2 > possible variants of an IP block (of which there are). > > Signed-off-by: Andrey Smirnov <andrew.smirnov@xxxxxxxxx> Applied with Fabio's Reviewed-by and Gary's Tested-by to pci/host-imx6 for v4.7, thanks, Andrey! Also, thank you Fabio, Gary, and Lucas for reviewing and testing. > --- > > Changes since v2: > - Removed default clause in all introduced switch statements > - Switched to using of_device_get_match_data instead of > explicit of_device_is_compatible checks > - Added a mention of the new DT compatibility string in the documentation > > Changes since v1: > > - Patchset is rebased against > https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-imx6 > > - DTS files changes moved into a separate patch > > drivers/pci/host/pci-imx6.c | 124 ++++++++++++++++++++++++-------------------- > 1 file changed, 67 insertions(+), 57 deletions(-) > > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c > index 0f6d630..a77e7fd 100644 > --- a/drivers/pci/host/pci-imx6.c > +++ b/drivers/pci/host/pci-imx6.c > @@ -19,6 +19,7 @@ > #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h> > #include <linux/module.h> > #include <linux/of_gpio.h> > +#include <linux/of_device.h> > #include <linux/pci.h> > #include <linux/platform_device.h> > #include <linux/regmap.h> > @@ -31,6 +32,11 @@ > > #define to_imx6_pcie(x) container_of(x, struct imx6_pcie, pp) > > +enum imx6_pcie_variants { > + IMX6Q, > + IMX6SX > +}; > + > struct imx6_pcie { > struct gpio_desc *reset_gpio; > struct clk *pcie_bus; > @@ -39,7 +45,7 @@ struct imx6_pcie { > struct clk *pcie; > struct pcie_port pp; > struct regmap *iomuxc_gpr; > - bool is_imx6sx; > + enum imx6_pcie_variants variant; > void __iomem *mem_base; > u32 tx_deemph_gen1; > u32 tx_deemph_gen2_3p5db; > @@ -238,7 +244,8 @@ static int imx6_pcie_assert_core_reset(struct pcie_port *pp) > struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp); > u32 val, gpr1, gpr12; > > - if (imx6_pcie->is_imx6sx) { > + switch (imx6_pcie->variant) { > + case IMX6SX: > regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12, > IMX6SX_GPR12_PCIE_TEST_POWERDOWN, > IMX6SX_GPR12_PCIE_TEST_POWERDOWN); > @@ -246,72 +253,76 @@ static int imx6_pcie_assert_core_reset(struct pcie_port *pp) > regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5, > IMX6SX_GPR5_PCIE_BTNRST_RESET, > IMX6SX_GPR5_PCIE_BTNRST_RESET); > - return 0; > - } > - > - /* > - * If the bootloader already enabled the link we need some special > - * handling to get the core back into a state where it is safe to > - * touch it for configuration. As there is no dedicated reset signal > - * wired up for MX6QDL, we need to manually force LTSSM into "detect" > - * state before completely disabling LTSSM, which is a prerequisite > - * for core configuration. > - * > - * If both LTSSM_ENABLE and REF_SSP_ENABLE are active we have a strong > - * indication that the bootloader activated the link. > - */ > - regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1, &gpr1); > - regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12, &gpr12); > - > - if ((gpr1 & IMX6Q_GPR1_PCIE_REF_CLK_EN) && > - (gpr12 & IMX6Q_GPR12_PCIE_CTL_2)) { > - val = readl(pp->dbi_base + PCIE_PL_PFLR); > - val &= ~PCIE_PL_PFLR_LINK_STATE_MASK; > - val |= PCIE_PL_PFLR_FORCE_LINK; > - writel(val, pp->dbi_base + PCIE_PL_PFLR); > + break; > + case IMX6Q: > + /* > + * If the bootloader already enabled the link we need some special > + * handling to get the core back into a state where it is safe to > + * touch it for configuration. As there is no dedicated reset signal > + * wired up for MX6QDL, we need to manually force LTSSM into "detect" > + * state before completely disabling LTSSM, which is a prerequisite > + * for core configuration. > + * > + * If both LTSSM_ENABLE and REF_SSP_ENABLE are active we have a strong > + * indication that the bootloader activated the link. > + */ > + regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1, &gpr1); > + regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12, &gpr12); > + > + if ((gpr1 & IMX6Q_GPR1_PCIE_REF_CLK_EN) && > + (gpr12 & IMX6Q_GPR12_PCIE_CTL_2)) { > + val = readl(pp->dbi_base + PCIE_PL_PFLR); > + val &= ~PCIE_PL_PFLR_LINK_STATE_MASK; > + val |= PCIE_PL_PFLR_FORCE_LINK; > + writel(val, pp->dbi_base + PCIE_PL_PFLR); > + > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12, > + IMX6Q_GPR12_PCIE_CTL_2, 0 << 10); > + } > > - regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12, > - IMX6Q_GPR12_PCIE_CTL_2, 0 << 10); > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1, > + IMX6Q_GPR1_PCIE_TEST_PD, 1 << 18); > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1, > + IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16); > + break; > } > > - regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1, > - IMX6Q_GPR1_PCIE_TEST_PD, 1 << 18); > - regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1, > - IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16); > - > return 0; > } > > static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie) > { > struct pcie_port *pp = &imx6_pcie->pp; > - int ret; > + int ret = 0; > > - if (imx6_pcie->is_imx6sx) { > + switch (imx6_pcie->variant) { > + case IMX6SX: > ret = clk_prepare_enable(imx6_pcie->pcie_inbound_axi); > if (ret) { > dev_err(pp->dev, "unable to enable pcie_axi clock\n"); > - return ret; > + break; > } > > regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12, > IMX6SX_GPR12_PCIE_TEST_POWERDOWN, 0); > - return ret; > + break; > + case IMX6Q: > + /* power up core phy and enable ref clock */ > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1, > + IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18); > + /* > + * the async reset input need ref clock to sync internally, > + * when the ref clock comes after reset, internal synced > + * reset time is too short, cannot meet the requirement. > + * add one ~10us delay here. > + */ > + udelay(10); > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1, > + IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16); > + break; > } > > - /* power up core phy and enable ref clock */ > - regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1, > - IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18); > - /* > - * the async reset input need ref clock to sync internally, > - * when the ref clock comes after reset, internal synced > - * reset time is too short, cannot meet the requirement. > - * add one ~10us delay here. > - */ > - udelay(10); > - regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1, > - IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16); > - return 0; > + return ret; > } > > static int imx6_pcie_deassert_core_reset(struct pcie_port *pp) > @@ -353,7 +364,7 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp) > gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1); > } > > - if (imx6_pcie->is_imx6sx) > + if (imx6_pcie->variant == IMX6SX) > regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5, > IMX6SX_GPR5_PCIE_BTNRST_RESET, 0); > > @@ -374,11 +385,10 @@ static void imx6_pcie_init_phy(struct pcie_port *pp) > { > struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp); > > - if (imx6_pcie->is_imx6sx) { > + if (imx6_pcie->variant == IMX6SX) > regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12, > IMX6SX_GPR12_PCIE_RX_EQ_MASK, > IMX6SX_GPR12_PCIE_RX_EQ_2); > - } > > regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12, > IMX6Q_GPR12_PCIE_CTL_2, 0 << 10); > @@ -585,8 +595,8 @@ static int __init imx6_pcie_probe(struct platform_device *pdev) > pp = &imx6_pcie->pp; > pp->dev = &pdev->dev; > > - imx6_pcie->is_imx6sx = of_device_is_compatible(pp->dev->of_node, > - "fsl,imx6sx-pcie"); > + imx6_pcie->variant = > + (enum imx6_pcie_variants)of_device_get_match_data(&pdev->dev); > > /* Added for PCI abort handling */ > hook_fault_code(16 + 6, imx6q_pcie_abort_handler, SIGBUS, 0, > @@ -623,7 +633,7 @@ static int __init imx6_pcie_probe(struct platform_device *pdev) > return PTR_ERR(imx6_pcie->pcie); > } > > - if (imx6_pcie->is_imx6sx) { > + if (imx6_pcie->variant == IMX6SX) { > imx6_pcie->pcie_inbound_axi = devm_clk_get(&pdev->dev, > "pcie_inbound_axi"); > if (IS_ERR(imx6_pcie->pcie_inbound_axi)) { > @@ -679,8 +689,8 @@ static void imx6_pcie_shutdown(struct platform_device *pdev) > } > > static const struct of_device_id imx6_pcie_of_match[] = { > - { .compatible = "fsl,imx6q-pcie", }, > - { .compatible = "fsl,imx6sx-pcie", }, > + { .compatible = "fsl,imx6q-pcie", .data = (void *)IMX6Q, }, > + { .compatible = "fsl,imx6sx-pcie", .data = (void *)IMX6SX, }, > {}, > }; > MODULE_DEVICE_TABLE(of, imx6_pcie_of_match); > -- > 2.5.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
