[PATCH v10 00/14] PCI: imx6: Clean up and add imx95 pci support

[PATCH v10 00/14] PCI: imx6: Clean up and add imx95 pci support

[Frank Li]

first 6 patches use drvdata: flags to simplify some switch-case code.
Improve maintaince and easy to read code.

Then add imx95 basic pci host function.

follow two patch do endpoint code clean up.
Then add imx95 basic endpont function.

Compared with v2, added EP function support and some fixes,  please change
notes at each patches.

Change from v9 to v10
- remove two patches:
>   dt-bindings: imx6q-pcie: Add linux,pci-domain as required for iMX8MQ
>   PCI: imx6: Using "linux,pci-domain" as slot ID
it is not good solution to fixed hardcode check to get controller id.
Will see better solution later.

dt-binding pass pcie node:

pcie0: pcie@4c300000 {
                        compatible = "fsl,imx95-pcie";
                        reg = <0 0x4c300000 0 0x40000>,
                                <0 0x4c360000 0 0x10000>,
                                <0 0x4c340000 0 0x20000>,
                                <0 0x60100000 0 0xfe00000>;
                        reg-names = "dbi", "atu", "app", "config";
                        #address-cells = <3>;
                        #size-cells = <2>;
                        device_type = "pci";
                        linux,pci-domain = <0>;
                        bus-range = <0x00 0xff>;
                        ranges = <0x81000000 0x0 0x00000000 0x0 0x6ff00000 0 0x00100000>,
                                 <0x82000000 0x0 0x10000000 0x9 0x10000000 0 0x10000000>;
                        num-lanes = <1>;
                        num-viewport = <8>;
                        interrupts = <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>;
                        interrupt-names = "msi";
                        #interrupt-cells = <1>;
                        interrupt-map-mask = <0 0 0 0x7>;
                        interrupt-map = <0 0 0 1 &gic 0 0 GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
                                        <0 0 0 2 &gic 0 0 GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>,
                                        <0 0 0 3 &gic 0 0 GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
                                        <0 0 0 4 &gic 0 0 GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>;
                        fsl,max-link-speed = <3>;
                        clocks = <&scmi_clk IMX95_CLK_HSIO>,
                                 <&scmi_clk IMX95_CLK_HSIOPLL>,
                                 <&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
                                 <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
                        clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux";
                        assigned-clocks =<&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
                                         <&scmi_clk IMX95_CLK_HSIOPLL>,
                                         <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
                        assigned-clock-rates = <3600000000>, <100000000>, <10000000>;
                        assigned-clock-parents = <0>, <0>,
                                                 <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>;
                        power-domains = <&scmi_devpd IMX95_PD_HSIO_TOP>;
                        /* 0x30~0x37 stream id for pci0 */
                        /*
                         * iommu-map = <0x000 &apps_smmu 0x30 0x1>,
                         * <0x100 &apps_smmu 0x31 0x1>;
                         */
                        status = "disabled";
                };

pcie1: pcie-ep@4c380000 {
                        compatible = "fsl,imx95-pcie-ep";
                        reg = <0 0x4c380000 0 0x20000>,
                              <0 0x4c3e0000 0 0x1000>,
                              <0 0x4c3a0000 0 0x1000>,
                              <0 0x4c3c0000 0 0x10000>,
                              <0 0x4c3f0000 0 0x10000>,
                              <0xa 0 1 0>;
                        reg-names = "dbi", "atu", "dbi2", "app", "dma", "addr_space";
                        interrupts = <GIC_SPI 317 IRQ_TYPE_LEVEL_HIGH>;
                        interrupt-names = "dma";
                        fsl,max-link-speed = <3>;
                        clocks = <&scmi_clk IMX95_CLK_HSIO>,
                                 <&scmi_clk IMX95_CLK_HSIOPLL>,
                                 <&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
                                 <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
                        clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux";
                        assigned-clocks =<&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
                                         <&scmi_clk IMX95_CLK_HSIOPLL>,
                                         <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
                        assigned-clock-rates = <3600000000>, <100000000>, <10000000>;
                        assigned-clock-parents = <0>, <0>,
                                                 <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>;
                        power-domains = <&scmi_devpd IMX95_PD_HSIO_TOP>;
                        status = "disabled";
                };

Frank Li (13):
  PCI: imx6: Simplify clock handling by using clk_bulk*() function
  PCI: imx6: Simplify phy handling by using IMX6_PCIE_FLAG_HAS_PHYDRV
  PCI: imx6: Simplify reset handling by using by using
    *_FLAG_HAS_*_RESET
  PCI: imx6: Simplify ltssm_enable() by using ltssm_off and ltssm_mask
  PCI: imx6: Simplify configure_type() by using mode_off and mode_mask
  PCI: imx6: Simplify switch-case logic by involve init_phy callback
  dt-bindings: imx6q-pcie: Clean up irrationality clocks check
  dt-bindings: imx6q-pcie: Restruct reg and reg-name
  PCI: imx6: Add iMX95 PCIe Root Complex support
  PCI: imx6: Clean up get addr_space code
  PCI: imx6: Add epc_features in imx6_pcie_drvdata
  dt-bindings: imx6q-pcie: Add iMX95 pcie endpoint compatible string
  PCI: imx6: Add iMX95 Endpoint (EP) support

Richard Zhu (1):
  dt-bindings: imx6q-pcie: Add imx95 pcie compatible string

 .../bindings/pci/fsl,imx6q-pcie-common.yaml   |  17 +-
 .../bindings/pci/fsl,imx6q-pcie-ep.yaml       |  46 +-
 .../bindings/pci/fsl,imx6q-pcie.yaml          |  49 +-
 drivers/pci/controller/dwc/pci-imx6.c         | 634 ++++++++++--------
 4 files changed, 436 insertions(+), 310 deletions(-)

-- 
2.34.1

[PATCH v10 01/14] PCI: imx6: Simplify clock handling by using clk_bulk*() function

[Frank Li]

Refector the clock handling logic. Add 'clk_names' define in drvdata. Use
clk_bulk*() api simplify the code.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Notes:
    Change from v9 to v10
    - fixed missed delete a case, which need failthrough to next one.
    Change from v8 to v9
    - change clks names
    - Add Manivannan's review tag
    
    Change from v7 to v8
    - update comment message
    - using ARRAY_SIZE to count clk_names.
    Change from v6 to v7
    - none
    Change from v4 to v5
    - update commit message
    - direct using clk name list, instead of macro
    - still keep caculate clk list count because sizeof return pre allocated
    array size.
    
    Change from v3 to v4
    - using clk_bulk_*() API
    Change from v1 to v3
    - none
    
    Change from v4 to v5
    - update commit message
    - direct using clk name list, instead of macro
    - still keep caculate clk list count because sizeof return pre allocated
    array size.
    
    Change from v3 to v4
    - using clk_bulk_*() API
    Change from v1 to v3
    - none
    
    Change from v8 to v9
    - change clks names
    - Add Manivannan's review tag
    
    Change from v7 to v8
    - update comment message
    - using ARRAY_SIZE to count clk_names.
    Change from v6 to v7
    - none
    Change from v4 to v5
    - update commit message
    - direct using clk name list, instead of macro
    - still keep caculate clk list count because sizeof return pre allocated
    array size.
    
    Change from v3 to v4
    - using clk_bulk_*() API
    Change from v1 to v3
    - none
    
    Change from v4 to v5
    - update commit message
    - direct using clk name list, instead of macro
    - still keep caculate clk list count because sizeof return pre allocated
    array size.
    
    Change from v3 to v4
    - using clk_bulk_*() API
    Change from v1 to v3
    - none

 drivers/pci/controller/dwc/pci-imx6.c | 138 ++++++++++----------------
 1 file changed, 50 insertions(+), 88 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 74703362aeec7..82854e94c5621 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -61,12 +61,16 @@ enum imx6_pcie_variants {
 #define IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE	BIT(1)
 #define IMX6_PCIE_FLAG_SUPPORTS_SUSPEND		BIT(2)
 
+#define IMX6_PCIE_MAX_CLKS       6
+
 struct imx6_pcie_drvdata {
 	enum imx6_pcie_variants variant;
 	enum dw_pcie_device_mode mode;
 	u32 flags;
 	int dbi_length;
 	const char *gpr;
+	const char * const *clk_names;
+	const u32 clks_cnt;
 };
 
 struct imx6_pcie {
@@ -74,11 +78,7 @@ struct imx6_pcie {
 	int			reset_gpio;
 	bool			gpio_active_high;
 	bool			link_is_up;
-	struct clk		*pcie_bus;
-	struct clk		*pcie_phy;
-	struct clk		*pcie_inbound_axi;
-	struct clk		*pcie;
-	struct clk		*pcie_aux;
+	struct clk_bulk_data	clks[IMX6_PCIE_MAX_CLKS];
 	struct regmap		*iomuxc_gpr;
 	u16			msi_ctrl;
 	u32			controller_id;
@@ -407,13 +407,18 @@ static void imx7d_pcie_wait_for_phy_pll_lock(struct imx6_pcie *imx6_pcie)
 
 static int imx6_setup_phy_mpll(struct imx6_pcie *imx6_pcie)
 {
-	unsigned long phy_rate = clk_get_rate(imx6_pcie->pcie_phy);
+	unsigned long phy_rate = 0;
 	int mult, div;
 	u16 val;
+	int i;
 
 	if (!(imx6_pcie->drvdata->flags & IMX6_PCIE_FLAG_IMX6_PHY))
 		return 0;
 
+	for (i = 0; i < imx6_pcie->drvdata->clks_cnt; i++)
+		if (strncmp(imx6_pcie->clks[i].id, "pcie_phy", 8) == 0)
+			phy_rate = clk_get_rate(imx6_pcie->clks[i].clk);
+
 	switch (phy_rate) {
 	case 125000000:
 		/*
@@ -550,19 +555,11 @@ static int imx6_pcie_attach_pd(struct device *dev)
 
 static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
 {
-	struct dw_pcie *pci = imx6_pcie->pci;
-	struct device *dev = pci->dev;
 	unsigned int offset;
 	int ret = 0;
 
 	switch (imx6_pcie->drvdata->variant) {
 	case IMX6SX:
-		ret = clk_prepare_enable(imx6_pcie->pcie_inbound_axi);
-		if (ret) {
-			dev_err(dev, "unable to enable pcie_axi clock\n");
-			break;
-		}
-
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
 				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN, 0);
 		break;
@@ -589,12 +586,6 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
 	case IMX8MQ_EP:
 	case IMX8MP:
 	case IMX8MP_EP:
-		ret = clk_prepare_enable(imx6_pcie->pcie_aux);
-		if (ret) {
-			dev_err(dev, "unable to enable pcie_aux clock\n");
-			break;
-		}
-
 		offset = imx6_pcie_grp_offset(imx6_pcie);
 		/*
 		 * Set the over ride low and enabled
@@ -615,9 +606,6 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
 static void imx6_pcie_disable_ref_clk(struct imx6_pcie *imx6_pcie)
 {
 	switch (imx6_pcie->drvdata->variant) {
-	case IMX6SX:
-		clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
-		break;
 	case IMX6QP:
 	case IMX6Q:
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
@@ -631,14 +619,6 @@ static void imx6_pcie_disable_ref_clk(struct imx6_pcie *imx6_pcie)
 				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
 				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
 		break;
-	case IMX8MM:
-	case IMX8MM_EP:
-	case IMX8MQ:
-	case IMX8MQ_EP:
-	case IMX8MP:
-	case IMX8MP_EP:
-		clk_disable_unprepare(imx6_pcie->pcie_aux);
-		break;
 	default:
 		break;
 	}
@@ -650,23 +630,9 @@ static int imx6_pcie_clk_enable(struct imx6_pcie *imx6_pcie)
 	struct device *dev = pci->dev;
 	int ret;
 
-	ret = clk_prepare_enable(imx6_pcie->pcie_phy);
-	if (ret) {
-		dev_err(dev, "unable to enable pcie_phy clock\n");
+	ret = clk_bulk_prepare_enable(imx6_pcie->drvdata->clks_cnt, imx6_pcie->clks);
+	if (ret)
 		return ret;
-	}
-
-	ret = clk_prepare_enable(imx6_pcie->pcie_bus);
-	if (ret) {
-		dev_err(dev, "unable to enable pcie_bus clock\n");
-		goto err_pcie_bus;
-	}
-
-	ret = clk_prepare_enable(imx6_pcie->pcie);
-	if (ret) {
-		dev_err(dev, "unable to enable pcie clock\n");
-		goto err_pcie;
-	}
 
 	ret = imx6_pcie_enable_ref_clk(imx6_pcie);
 	if (ret) {
@@ -679,11 +645,7 @@ static int imx6_pcie_clk_enable(struct imx6_pcie *imx6_pcie)
 	return 0;
 
 err_ref_clk:
-	clk_disable_unprepare(imx6_pcie->pcie);
-err_pcie:
-	clk_disable_unprepare(imx6_pcie->pcie_bus);
-err_pcie_bus:
-	clk_disable_unprepare(imx6_pcie->pcie_phy);
+	clk_bulk_disable_unprepare(imx6_pcie->drvdata->clks_cnt, imx6_pcie->clks);
 
 	return ret;
 }
@@ -691,9 +653,7 @@ static int imx6_pcie_clk_enable(struct imx6_pcie *imx6_pcie)
 static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
 {
 	imx6_pcie_disable_ref_clk(imx6_pcie);
-	clk_disable_unprepare(imx6_pcie->pcie);
-	clk_disable_unprepare(imx6_pcie->pcie_bus);
-	clk_disable_unprepare(imx6_pcie->pcie_phy);
+	clk_bulk_disable_unprepare(imx6_pcie->drvdata->clks_cnt, imx6_pcie->clks);
 }
 
 static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
@@ -1252,6 +1212,7 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 	struct device_node *node = dev->of_node;
 	int ret;
 	u16 val;
+	int i;
 
 	imx6_pcie = devm_kzalloc(dev, sizeof(*imx6_pcie), GFP_KERNEL);
 	if (!imx6_pcie)
@@ -1305,32 +1266,20 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 		return imx6_pcie->reset_gpio;
 	}
 
-	/* Fetch clocks */
-	imx6_pcie->pcie_bus = devm_clk_get(dev, "pcie_bus");
-	if (IS_ERR(imx6_pcie->pcie_bus))
-		return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_bus),
-				     "pcie_bus clock source missing or invalid\n");
+	if (imx6_pcie->drvdata->clks_cnt >= IMX6_PCIE_MAX_CLKS)
+		return dev_err_probe(dev, -ENOMEM, "clks_cnt is too big\n");
 
-	imx6_pcie->pcie = devm_clk_get(dev, "pcie");
-	if (IS_ERR(imx6_pcie->pcie))
-		return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie),
-				     "pcie clock source missing or invalid\n");
+	for (i = 0; i < imx6_pcie->drvdata->clks_cnt; i++)
+		imx6_pcie->clks[i].id = imx6_pcie->drvdata->clk_names[i];
+
+	/* Fetch clocks */
+	ret = devm_clk_bulk_get(dev, imx6_pcie->drvdata->clks_cnt, imx6_pcie->clks);
+	if (ret)
+		return ret;
 
 	switch (imx6_pcie->drvdata->variant) {
-	case IMX6SX:
-		imx6_pcie->pcie_inbound_axi = devm_clk_get(dev,
-							   "pcie_inbound_axi");
-		if (IS_ERR(imx6_pcie->pcie_inbound_axi))
-			return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_inbound_axi),
-					     "pcie_inbound_axi clock missing or invalid\n");
-		break;
 	case IMX8MQ:
 	case IMX8MQ_EP:
-		imx6_pcie->pcie_aux = devm_clk_get(dev, "pcie_aux");
-		if (IS_ERR(imx6_pcie->pcie_aux))
-			return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_aux),
-					     "pcie_aux clock source missing or invalid\n");
-		fallthrough;
 	case IMX7D:
 		if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
 			imx6_pcie->controller_id = 1;
@@ -1353,10 +1302,6 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 	case IMX8MM_EP:
 	case IMX8MP:
 	case IMX8MP_EP:
-		imx6_pcie->pcie_aux = devm_clk_get(dev, "pcie_aux");
-		if (IS_ERR(imx6_pcie->pcie_aux))
-			return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_aux),
-					     "pcie_aux clock source missing or invalid\n");
 		imx6_pcie->apps_reset = devm_reset_control_get_exclusive(dev,
 									 "apps");
 		if (IS_ERR(imx6_pcie->apps_reset))
@@ -1372,14 +1317,6 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 	default:
 		break;
 	}
-	/* Don't fetch the pcie_phy clock, if it has abstract PHY driver */
-	if (imx6_pcie->phy == NULL) {
-		imx6_pcie->pcie_phy = devm_clk_get(dev, "pcie_phy");
-		if (IS_ERR(imx6_pcie->pcie_phy))
-			return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_phy),
-					     "pcie_phy clock source missing or invalid\n");
-	}
-
 
 	/* Grab turnoff reset */
 	imx6_pcie->turnoff_reset = devm_reset_control_get_optional_exclusive(dev, "turnoff");
@@ -1470,6 +1407,11 @@ static void imx6_pcie_shutdown(struct platform_device *pdev)
 	imx6_pcie_assert_core_reset(imx6_pcie);
 }
 
+static const char * const imx6q_clks[] = {"pcie_bus", "pcie", "pcie_phy"};
+static const char * const imx8mm_clks[] = {"pcie_bus", "pcie", "pcie_aux"};
+static const char * const imx8mq_clks[] = {"pcie_bus", "pcie", "pcie_phy", "pcie_aux"};
+static const char * const imx6sx_clks[] = {"pcie_bus", "pcie", "pcie_phy", "pcie_inbound_axi"};
+
 static const struct imx6_pcie_drvdata drvdata[] = {
 	[IMX6Q] = {
 		.variant = IMX6Q,
@@ -1477,6 +1419,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 			 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE,
 		.dbi_length = 0x200,
 		.gpr = "fsl,imx6q-iomuxc-gpr",
+		.clk_names = imx6q_clks,
+		.clks_cnt = ARRAY_SIZE(imx6q_clks),
 	},
 	[IMX6SX] = {
 		.variant = IMX6SX,
@@ -1484,6 +1428,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 			 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE |
 			 IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
 		.gpr = "fsl,imx6q-iomuxc-gpr",
+		.clk_names = imx6sx_clks,
+		.clks_cnt = ARRAY_SIZE(imx6sx_clks),
 	},
 	[IMX6QP] = {
 		.variant = IMX6QP,
@@ -1492,40 +1438,56 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 			 IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
 		.dbi_length = 0x200,
 		.gpr = "fsl,imx6q-iomuxc-gpr",
+		.clk_names = imx6q_clks,
+		.clks_cnt = ARRAY_SIZE(imx6q_clks),
 	},
 	[IMX7D] = {
 		.variant = IMX7D,
 		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
 		.gpr = "fsl,imx7d-iomuxc-gpr",
+		.clk_names = imx6q_clks,
+		.clks_cnt = ARRAY_SIZE(imx6q_clks),
 	},
 	[IMX8MQ] = {
 		.variant = IMX8MQ,
 		.gpr = "fsl,imx8mq-iomuxc-gpr",
+		.clk_names = imx8mq_clks,
+		.clks_cnt = ARRAY_SIZE(imx8mq_clks),
 	},
 	[IMX8MM] = {
 		.variant = IMX8MM,
 		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
 		.gpr = "fsl,imx8mm-iomuxc-gpr",
+		.clk_names = imx8mm_clks,
+		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
 	},
 	[IMX8MP] = {
 		.variant = IMX8MP,
 		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
 		.gpr = "fsl,imx8mp-iomuxc-gpr",
+		.clk_names = imx8mm_clks,
+		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
 	},
 	[IMX8MQ_EP] = {
 		.variant = IMX8MQ_EP,
 		.mode = DW_PCIE_EP_TYPE,
 		.gpr = "fsl,imx8mq-iomuxc-gpr",
+		.clk_names = imx8mq_clks,
+		.clks_cnt = ARRAY_SIZE(imx8mq_clks),
 	},
 	[IMX8MM_EP] = {
 		.variant = IMX8MM_EP,
 		.mode = DW_PCIE_EP_TYPE,
 		.gpr = "fsl,imx8mm-iomuxc-gpr",
+		.clk_names = imx8mm_clks,
+		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
 	},
 	[IMX8MP_EP] = {
 		.variant = IMX8MP_EP,
 		.mode = DW_PCIE_EP_TYPE,
 		.gpr = "fsl,imx8mp-iomuxc-gpr",
+		.clk_names = imx8mm_clks,
+		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
 	},
 };
 
-- 
2.34.1

Re: [PATCH v10 01/14] PCI: imx6: Simplify clock handling by using clk_bulk*() function

[Lorenzo Pieralisi]

On Mon, Feb 05, 2024 at 12:33:22PM -0500, Frank Li wrote:
> Refector the clock handling logic. Add 'clk_names' define in drvdata. Use
> clk_bulk*() api simplify the code.
> 
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> 
> Notes:
>     Change from v9 to v10
>     - fixed missed delete a case, which need failthrough to next one.
>     Change from v8 to v9
>     - change clks names
>     - Add Manivannan's review tag
>     
>     Change from v7 to v8
>     - update comment message
>     - using ARRAY_SIZE to count clk_names.
>     Change from v6 to v7
>     - none
>     Change from v4 to v5
>     - update commit message
>     - direct using clk name list, instead of macro
>     - still keep caculate clk list count because sizeof return pre allocated
>     array size.
>     
>     Change from v3 to v4
>     - using clk_bulk_*() API
>     Change from v1 to v3
>     - none
>     
>     Change from v4 to v5
>     - update commit message
>     - direct using clk name list, instead of macro
>     - still keep caculate clk list count because sizeof return pre allocated
>     array size.
>     
>     Change from v3 to v4
>     - using clk_bulk_*() API
>     Change from v1 to v3
>     - none
>     
>     Change from v8 to v9
>     - change clks names
>     - Add Manivannan's review tag
>     
>     Change from v7 to v8
>     - update comment message
>     - using ARRAY_SIZE to count clk_names.
>     Change from v6 to v7
>     - none
>     Change from v4 to v5
>     - update commit message
>     - direct using clk name list, instead of macro
>     - still keep caculate clk list count because sizeof return pre allocated
>     array size.
>     
>     Change from v3 to v4
>     - using clk_bulk_*() API
>     Change from v1 to v3
>     - none
>     
>     Change from v4 to v5
>     - update commit message
>     - direct using clk name list, instead of macro
>     - still keep caculate clk list count because sizeof return pre allocated
>     array size.
>     
>     Change from v3 to v4
>     - using clk_bulk_*() API
>     Change from v1 to v3
>     - none
> 
>  drivers/pci/controller/dwc/pci-imx6.c | 138 ++++++++++----------------
>  1 file changed, 50 insertions(+), 88 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 74703362aeec7..82854e94c5621 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -61,12 +61,16 @@ enum imx6_pcie_variants {
>  #define IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE	BIT(1)
>  #define IMX6_PCIE_FLAG_SUPPORTS_SUSPEND		BIT(2)
>  
> +#define IMX6_PCIE_MAX_CLKS       6
> +
>  struct imx6_pcie_drvdata {
>  	enum imx6_pcie_variants variant;
>  	enum dw_pcie_device_mode mode;
>  	u32 flags;
>  	int dbi_length;
>  	const char *gpr;
> +	const char * const *clk_names;
> +	const u32 clks_cnt;
>  };
>  
>  struct imx6_pcie {
> @@ -74,11 +78,7 @@ struct imx6_pcie {
>  	int			reset_gpio;
>  	bool			gpio_active_high;
>  	bool			link_is_up;
> -	struct clk		*pcie_bus;
> -	struct clk		*pcie_phy;
> -	struct clk		*pcie_inbound_axi;
> -	struct clk		*pcie;
> -	struct clk		*pcie_aux;
> +	struct clk_bulk_data	clks[IMX6_PCIE_MAX_CLKS];

Why can't you allocate this dynamically ?

>  	struct regmap		*iomuxc_gpr;
>  	u16			msi_ctrl;
>  	u32			controller_id;
> @@ -407,13 +407,18 @@ static void imx7d_pcie_wait_for_phy_pll_lock(struct imx6_pcie *imx6_pcie)
>  
>  static int imx6_setup_phy_mpll(struct imx6_pcie *imx6_pcie)
>  {
> -	unsigned long phy_rate = clk_get_rate(imx6_pcie->pcie_phy);
> +	unsigned long phy_rate = 0;
>  	int mult, div;
>  	u16 val;
> +	int i;
>  
>  	if (!(imx6_pcie->drvdata->flags & IMX6_PCIE_FLAG_IMX6_PHY))
>  		return 0;
>  
> +	for (i = 0; i < imx6_pcie->drvdata->clks_cnt; i++)
> +		if (strncmp(imx6_pcie->clks[i].id, "pcie_phy", 8) == 0)
> +			phy_rate = clk_get_rate(imx6_pcie->clks[i].clk);
> +
>  	switch (phy_rate) {
>  	case 125000000:
>  		/*
> @@ -550,19 +555,11 @@ static int imx6_pcie_attach_pd(struct device *dev)
>  
>  static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
>  {
> -	struct dw_pcie *pci = imx6_pcie->pci;
> -	struct device *dev = pci->dev;
>  	unsigned int offset;
>  	int ret = 0;
>  
>  	switch (imx6_pcie->drvdata->variant) {
>  	case IMX6SX:
> -		ret = clk_prepare_enable(imx6_pcie->pcie_inbound_axi);
> -		if (ret) {
> -			dev_err(dev, "unable to enable pcie_axi clock\n");
> -			break;
> -		}
> -
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN, 0);
>  		break;
> @@ -589,12 +586,6 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
>  	case IMX8MQ_EP:
>  	case IMX8MP:
>  	case IMX8MP_EP:
> -		ret = clk_prepare_enable(imx6_pcie->pcie_aux);
> -		if (ret) {
> -			dev_err(dev, "unable to enable pcie_aux clock\n");
> -			break;
> -		}
> -
>  		offset = imx6_pcie_grp_offset(imx6_pcie);
>  		/*
>  		 * Set the over ride low and enabled
> @@ -615,9 +606,6 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
>  static void imx6_pcie_disable_ref_clk(struct imx6_pcie *imx6_pcie)
>  {
>  	switch (imx6_pcie->drvdata->variant) {
> -	case IMX6SX:
> -		clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
> -		break;
>  	case IMX6QP:
>  	case IMX6Q:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> @@ -631,14 +619,6 @@ static void imx6_pcie_disable_ref_clk(struct imx6_pcie *imx6_pcie)
>  				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
>  				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
>  		break;
> -	case IMX8MM:
> -	case IMX8MM_EP:
> -	case IMX8MQ:
> -	case IMX8MQ_EP:
> -	case IMX8MP:
> -	case IMX8MP_EP:
> -		clk_disable_unprepare(imx6_pcie->pcie_aux);
> -		break;
>  	default:
>  		break;
>  	}
> @@ -650,23 +630,9 @@ static int imx6_pcie_clk_enable(struct imx6_pcie *imx6_pcie)
>  	struct device *dev = pci->dev;
>  	int ret;
>  
> -	ret = clk_prepare_enable(imx6_pcie->pcie_phy);
> -	if (ret) {
> -		dev_err(dev, "unable to enable pcie_phy clock\n");
> +	ret = clk_bulk_prepare_enable(imx6_pcie->drvdata->clks_cnt, imx6_pcie->clks);
> +	if (ret)
>  		return ret;
> -	}
> -
> -	ret = clk_prepare_enable(imx6_pcie->pcie_bus);
> -	if (ret) {
> -		dev_err(dev, "unable to enable pcie_bus clock\n");
> -		goto err_pcie_bus;
> -	}
> -
> -	ret = clk_prepare_enable(imx6_pcie->pcie);
> -	if (ret) {
> -		dev_err(dev, "unable to enable pcie clock\n");
> -		goto err_pcie;
> -	}
>  
>  	ret = imx6_pcie_enable_ref_clk(imx6_pcie);
>  	if (ret) {
> @@ -679,11 +645,7 @@ static int imx6_pcie_clk_enable(struct imx6_pcie *imx6_pcie)
>  	return 0;
>  
>  err_ref_clk:
> -	clk_disable_unprepare(imx6_pcie->pcie);
> -err_pcie:
> -	clk_disable_unprepare(imx6_pcie->pcie_bus);
> -err_pcie_bus:
> -	clk_disable_unprepare(imx6_pcie->pcie_phy);
> +	clk_bulk_disable_unprepare(imx6_pcie->drvdata->clks_cnt, imx6_pcie->clks);
>  
>  	return ret;
>  }
> @@ -691,9 +653,7 @@ static int imx6_pcie_clk_enable(struct imx6_pcie *imx6_pcie)
>  static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
>  {
>  	imx6_pcie_disable_ref_clk(imx6_pcie);
> -	clk_disable_unprepare(imx6_pcie->pcie);
> -	clk_disable_unprepare(imx6_pcie->pcie_bus);
> -	clk_disable_unprepare(imx6_pcie->pcie_phy);
> +	clk_bulk_disable_unprepare(imx6_pcie->drvdata->clks_cnt, imx6_pcie->clks);
>  }
>  
>  static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> @@ -1252,6 +1212,7 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>  	struct device_node *node = dev->of_node;
>  	int ret;
>  	u16 val;
> +	int i;
>  
>  	imx6_pcie = devm_kzalloc(dev, sizeof(*imx6_pcie), GFP_KERNEL);
>  	if (!imx6_pcie)
> @@ -1305,32 +1266,20 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>  		return imx6_pcie->reset_gpio;
>  	}
>  
> -	/* Fetch clocks */
> -	imx6_pcie->pcie_bus = devm_clk_get(dev, "pcie_bus");
> -	if (IS_ERR(imx6_pcie->pcie_bus))
> -		return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_bus),
> -				     "pcie_bus clock source missing or invalid\n");
> +	if (imx6_pcie->drvdata->clks_cnt >= IMX6_PCIE_MAX_CLKS)
> +		return dev_err_probe(dev, -ENOMEM, "clks_cnt is too big\n");

Same question as above, this should not fail if the clks array is
dynamically allocated according to imx6_pcie->drvdata->clks_cnt.

Lorenzo

>  
> -	imx6_pcie->pcie = devm_clk_get(dev, "pcie");
> -	if (IS_ERR(imx6_pcie->pcie))
> -		return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie),
> -				     "pcie clock source missing or invalid\n");
> +	for (i = 0; i < imx6_pcie->drvdata->clks_cnt; i++)
> +		imx6_pcie->clks[i].id = imx6_pcie->drvdata->clk_names[i];
> +
> +	/* Fetch clocks */
> +	ret = devm_clk_bulk_get(dev, imx6_pcie->drvdata->clks_cnt, imx6_pcie->clks);
> +	if (ret)
> +		return ret;
>  
>  	switch (imx6_pcie->drvdata->variant) {
> -	case IMX6SX:
> -		imx6_pcie->pcie_inbound_axi = devm_clk_get(dev,
> -							   "pcie_inbound_axi");
> -		if (IS_ERR(imx6_pcie->pcie_inbound_axi))
> -			return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_inbound_axi),
> -					     "pcie_inbound_axi clock missing or invalid\n");
> -		break;
>  	case IMX8MQ:
>  	case IMX8MQ_EP:
> -		imx6_pcie->pcie_aux = devm_clk_get(dev, "pcie_aux");
> -		if (IS_ERR(imx6_pcie->pcie_aux))
> -			return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_aux),
> -					     "pcie_aux clock source missing or invalid\n");
> -		fallthrough;
>  	case IMX7D:
>  		if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
>  			imx6_pcie->controller_id = 1;
> @@ -1353,10 +1302,6 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>  	case IMX8MM_EP:
>  	case IMX8MP:
>  	case IMX8MP_EP:
> -		imx6_pcie->pcie_aux = devm_clk_get(dev, "pcie_aux");
> -		if (IS_ERR(imx6_pcie->pcie_aux))
> -			return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_aux),
> -					     "pcie_aux clock source missing or invalid\n");
>  		imx6_pcie->apps_reset = devm_reset_control_get_exclusive(dev,
>  									 "apps");
>  		if (IS_ERR(imx6_pcie->apps_reset))
> @@ -1372,14 +1317,6 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>  	default:
>  		break;
>  	}
> -	/* Don't fetch the pcie_phy clock, if it has abstract PHY driver */
> -	if (imx6_pcie->phy == NULL) {
> -		imx6_pcie->pcie_phy = devm_clk_get(dev, "pcie_phy");
> -		if (IS_ERR(imx6_pcie->pcie_phy))
> -			return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_phy),
> -					     "pcie_phy clock source missing or invalid\n");
> -	}
> -
>  
>  	/* Grab turnoff reset */
>  	imx6_pcie->turnoff_reset = devm_reset_control_get_optional_exclusive(dev, "turnoff");
> @@ -1470,6 +1407,11 @@ static void imx6_pcie_shutdown(struct platform_device *pdev)
>  	imx6_pcie_assert_core_reset(imx6_pcie);
>  }
>  
> +static const char * const imx6q_clks[] = {"pcie_bus", "pcie", "pcie_phy"};
> +static const char * const imx8mm_clks[] = {"pcie_bus", "pcie", "pcie_aux"};
> +static const char * const imx8mq_clks[] = {"pcie_bus", "pcie", "pcie_phy", "pcie_aux"};
> +static const char * const imx6sx_clks[] = {"pcie_bus", "pcie", "pcie_phy", "pcie_inbound_axi"};
> +
>  static const struct imx6_pcie_drvdata drvdata[] = {
>  	[IMX6Q] = {
>  		.variant = IMX6Q,
> @@ -1477,6 +1419,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
>  			 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE,
>  		.dbi_length = 0x200,
>  		.gpr = "fsl,imx6q-iomuxc-gpr",
> +		.clk_names = imx6q_clks,
> +		.clks_cnt = ARRAY_SIZE(imx6q_clks),
>  	},
>  	[IMX6SX] = {
>  		.variant = IMX6SX,
> @@ -1484,6 +1428,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
>  			 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE |
>  			 IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
>  		.gpr = "fsl,imx6q-iomuxc-gpr",
> +		.clk_names = imx6sx_clks,
> +		.clks_cnt = ARRAY_SIZE(imx6sx_clks),
>  	},
>  	[IMX6QP] = {
>  		.variant = IMX6QP,
> @@ -1492,40 +1438,56 @@ static const struct imx6_pcie_drvdata drvdata[] = {
>  			 IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
>  		.dbi_length = 0x200,
>  		.gpr = "fsl,imx6q-iomuxc-gpr",
> +		.clk_names = imx6q_clks,
> +		.clks_cnt = ARRAY_SIZE(imx6q_clks),
>  	},
>  	[IMX7D] = {
>  		.variant = IMX7D,
>  		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
>  		.gpr = "fsl,imx7d-iomuxc-gpr",
> +		.clk_names = imx6q_clks,
> +		.clks_cnt = ARRAY_SIZE(imx6q_clks),
>  	},
>  	[IMX8MQ] = {
>  		.variant = IMX8MQ,
>  		.gpr = "fsl,imx8mq-iomuxc-gpr",
> +		.clk_names = imx8mq_clks,
> +		.clks_cnt = ARRAY_SIZE(imx8mq_clks),
>  	},
>  	[IMX8MM] = {
>  		.variant = IMX8MM,
>  		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
>  		.gpr = "fsl,imx8mm-iomuxc-gpr",
> +		.clk_names = imx8mm_clks,
> +		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
>  	},
>  	[IMX8MP] = {
>  		.variant = IMX8MP,
>  		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
>  		.gpr = "fsl,imx8mp-iomuxc-gpr",
> +		.clk_names = imx8mm_clks,
> +		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
>  	},
>  	[IMX8MQ_EP] = {
>  		.variant = IMX8MQ_EP,
>  		.mode = DW_PCIE_EP_TYPE,
>  		.gpr = "fsl,imx8mq-iomuxc-gpr",
> +		.clk_names = imx8mq_clks,
> +		.clks_cnt = ARRAY_SIZE(imx8mq_clks),
>  	},
>  	[IMX8MM_EP] = {
>  		.variant = IMX8MM_EP,
>  		.mode = DW_PCIE_EP_TYPE,
>  		.gpr = "fsl,imx8mm-iomuxc-gpr",
> +		.clk_names = imx8mm_clks,
> +		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
>  	},
>  	[IMX8MP_EP] = {
>  		.variant = IMX8MP_EP,
>  		.mode = DW_PCIE_EP_TYPE,
>  		.gpr = "fsl,imx8mp-iomuxc-gpr",
> +		.clk_names = imx8mm_clks,
> +		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
>  	},
>  };
>  
> -- 
> 2.34.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v10 01/14] PCI: imx6: Simplify clock handling by using clk_bulk*() function

[Frank Li]

On Tue, Feb 13, 2024 at 04:31:22PM +0100, Lorenzo Pieralisi wrote:
> On Mon, Feb 05, 2024 at 12:33:22PM -0500, Frank Li wrote:
> > Refector the clock handling logic. Add 'clk_names' define in drvdata. Use
> > clk_bulk*() api simplify the code.
> > 
> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > 
> > Notes:
> >     Change from v9 to v10
> >     - fixed missed delete a case, which need failthrough to next one.
> >     Change from v8 to v9
> >     - change clks names
> >     - Add Manivannan's review tag
> >     
> >     Change from v7 to v8
> >     - update comment message
> >     - using ARRAY_SIZE to count clk_names.
> >     Change from v6 to v7
> >     - none
> >     Change from v4 to v5
> >     - update commit message
> >     - direct using clk name list, instead of macro
> >     - still keep caculate clk list count because sizeof return pre allocated
> >     array size.
> >     
> >     Change from v3 to v4
> >     - using clk_bulk_*() API
> >     Change from v1 to v3
> >     - none
> >     
> >     Change from v4 to v5
> >     - update commit message
> >     - direct using clk name list, instead of macro
> >     - still keep caculate clk list count because sizeof return pre allocated
> >     array size.
> >     
> >     Change from v3 to v4
> >     - using clk_bulk_*() API
> >     Change from v1 to v3
> >     - none
> >     
> >     Change from v8 to v9
> >     - change clks names
> >     - Add Manivannan's review tag
> >     
> >     Change from v7 to v8
> >     - update comment message
> >     - using ARRAY_SIZE to count clk_names.
> >     Change from v6 to v7
> >     - none
> >     Change from v4 to v5
> >     - update commit message
> >     - direct using clk name list, instead of macro
> >     - still keep caculate clk list count because sizeof return pre allocated
> >     array size.
> >     
> >     Change from v3 to v4
> >     - using clk_bulk_*() API
> >     Change from v1 to v3
> >     - none
> >     
> >     Change from v4 to v5
> >     - update commit message
> >     - direct using clk name list, instead of macro
> >     - still keep caculate clk list count because sizeof return pre allocated
> >     array size.
> >     
> >     Change from v3 to v4
> >     - using clk_bulk_*() API
> >     Change from v1 to v3
> >     - none
> > 
> >  drivers/pci/controller/dwc/pci-imx6.c | 138 ++++++++++----------------
> >  1 file changed, 50 insertions(+), 88 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > index 74703362aeec7..82854e94c5621 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -61,12 +61,16 @@ enum imx6_pcie_variants {
> >  #define IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE	BIT(1)
> >  #define IMX6_PCIE_FLAG_SUPPORTS_SUSPEND		BIT(2)
> >  
> > +#define IMX6_PCIE_MAX_CLKS       6
> > +
> >  struct imx6_pcie_drvdata {
> >  	enum imx6_pcie_variants variant;
> >  	enum dw_pcie_device_mode mode;
> >  	u32 flags;
> >  	int dbi_length;
> >  	const char *gpr;
> > +	const char * const *clk_names;
> > +	const u32 clks_cnt;
> >  };
> >  
> >  struct imx6_pcie {
> > @@ -74,11 +78,7 @@ struct imx6_pcie {
> >  	int			reset_gpio;
> >  	bool			gpio_active_high;
> >  	bool			link_is_up;
> > -	struct clk		*pcie_bus;
> > -	struct clk		*pcie_phy;
> > -	struct clk		*pcie_inbound_axi;
> > -	struct clk		*pcie;
> > -	struct clk		*pcie_aux;
> > +	struct clk_bulk_data	clks[IMX6_PCIE_MAX_CLKS];
> 
> Why can't you allocate this dynamically ?

Engineer choose. clk_bulk_data is small data struct, two pointers (16byte
in 64bit system). clks in imx is 3-4, Over half already used(compared 6).
waste case only wast 48byte.
 
Dynamically allocate can't save much memory, there are some extra manage
meta data for dynamical memory, which may bigger than 48byte.

Frank

> 
> >  	struct regmap		*iomuxc_gpr;
> >  	u16			msi_ctrl;
> >  	u32			controller_id;
> > @@ -407,13 +407,18 @@ static void imx7d_pcie_wait_for_phy_pll_lock(struct imx6_pcie *imx6_pcie)
> >  
> >  static int imx6_setup_phy_mpll(struct imx6_pcie *imx6_pcie)
> >  {
> > -	unsigned long phy_rate = clk_get_rate(imx6_pcie->pcie_phy);
> > +	unsigned long phy_rate = 0;
> >  	int mult, div;
> >  	u16 val;
> > +	int i;
> >  
> >  	if (!(imx6_pcie->drvdata->flags & IMX6_PCIE_FLAG_IMX6_PHY))
> >  		return 0;
> >  
> > +	for (i = 0; i < imx6_pcie->drvdata->clks_cnt; i++)
> > +		if (strncmp(imx6_pcie->clks[i].id, "pcie_phy", 8) == 0)
> > +			phy_rate = clk_get_rate(imx6_pcie->clks[i].clk);
> > +
> >  	switch (phy_rate) {
> >  	case 125000000:
> >  		/*
> > @@ -550,19 +555,11 @@ static int imx6_pcie_attach_pd(struct device *dev)
> >  
> >  static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
> >  {
> > -	struct dw_pcie *pci = imx6_pcie->pci;
> > -	struct device *dev = pci->dev;
> >  	unsigned int offset;
> >  	int ret = 0;
> >  
> >  	switch (imx6_pcie->drvdata->variant) {
> >  	case IMX6SX:
> > -		ret = clk_prepare_enable(imx6_pcie->pcie_inbound_axi);
> > -		if (ret) {
> > -			dev_err(dev, "unable to enable pcie_axi clock\n");
> > -			break;
> > -		}
> > -
> >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> >  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN, 0);
> >  		break;
> > @@ -589,12 +586,6 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
> >  	case IMX8MQ_EP:
> >  	case IMX8MP:
> >  	case IMX8MP_EP:
> > -		ret = clk_prepare_enable(imx6_pcie->pcie_aux);
> > -		if (ret) {
> > -			dev_err(dev, "unable to enable pcie_aux clock\n");
> > -			break;
> > -		}
> > -
> >  		offset = imx6_pcie_grp_offset(imx6_pcie);
> >  		/*
> >  		 * Set the over ride low and enabled
> > @@ -615,9 +606,6 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
> >  static void imx6_pcie_disable_ref_clk(struct imx6_pcie *imx6_pcie)
> >  {
> >  	switch (imx6_pcie->drvdata->variant) {
> > -	case IMX6SX:
> > -		clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
> > -		break;
> >  	case IMX6QP:
> >  	case IMX6Q:
> >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> > @@ -631,14 +619,6 @@ static void imx6_pcie_disable_ref_clk(struct imx6_pcie *imx6_pcie)
> >  				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
> >  				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
> >  		break;
> > -	case IMX8MM:
> > -	case IMX8MM_EP:
> > -	case IMX8MQ:
> > -	case IMX8MQ_EP:
> > -	case IMX8MP:
> > -	case IMX8MP_EP:
> > -		clk_disable_unprepare(imx6_pcie->pcie_aux);
> > -		break;
> >  	default:
> >  		break;
> >  	}
> > @@ -650,23 +630,9 @@ static int imx6_pcie_clk_enable(struct imx6_pcie *imx6_pcie)
> >  	struct device *dev = pci->dev;
> >  	int ret;
> >  
> > -	ret = clk_prepare_enable(imx6_pcie->pcie_phy);
> > -	if (ret) {
> > -		dev_err(dev, "unable to enable pcie_phy clock\n");
> > +	ret = clk_bulk_prepare_enable(imx6_pcie->drvdata->clks_cnt, imx6_pcie->clks);
> > +	if (ret)
> >  		return ret;
> > -	}
> > -
> > -	ret = clk_prepare_enable(imx6_pcie->pcie_bus);
> > -	if (ret) {
> > -		dev_err(dev, "unable to enable pcie_bus clock\n");
> > -		goto err_pcie_bus;
> > -	}
> > -
> > -	ret = clk_prepare_enable(imx6_pcie->pcie);
> > -	if (ret) {
> > -		dev_err(dev, "unable to enable pcie clock\n");
> > -		goto err_pcie;
> > -	}
> >  
> >  	ret = imx6_pcie_enable_ref_clk(imx6_pcie);
> >  	if (ret) {
> > @@ -679,11 +645,7 @@ static int imx6_pcie_clk_enable(struct imx6_pcie *imx6_pcie)
> >  	return 0;
> >  
> >  err_ref_clk:
> > -	clk_disable_unprepare(imx6_pcie->pcie);
> > -err_pcie:
> > -	clk_disable_unprepare(imx6_pcie->pcie_bus);
> > -err_pcie_bus:
> > -	clk_disable_unprepare(imx6_pcie->pcie_phy);
> > +	clk_bulk_disable_unprepare(imx6_pcie->drvdata->clks_cnt, imx6_pcie->clks);
> >  
> >  	return ret;
> >  }
> > @@ -691,9 +653,7 @@ static int imx6_pcie_clk_enable(struct imx6_pcie *imx6_pcie)
> >  static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
> >  {
> >  	imx6_pcie_disable_ref_clk(imx6_pcie);
> > -	clk_disable_unprepare(imx6_pcie->pcie);
> > -	clk_disable_unprepare(imx6_pcie->pcie_bus);
> > -	clk_disable_unprepare(imx6_pcie->pcie_phy);
> > +	clk_bulk_disable_unprepare(imx6_pcie->drvdata->clks_cnt, imx6_pcie->clks);
> >  }
> >  
> >  static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> > @@ -1252,6 +1212,7 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> >  	struct device_node *node = dev->of_node;
> >  	int ret;
> >  	u16 val;
> > +	int i;
> >  
> >  	imx6_pcie = devm_kzalloc(dev, sizeof(*imx6_pcie), GFP_KERNEL);
> >  	if (!imx6_pcie)
> > @@ -1305,32 +1266,20 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> >  		return imx6_pcie->reset_gpio;
> >  	}
> >  
> > -	/* Fetch clocks */
> > -	imx6_pcie->pcie_bus = devm_clk_get(dev, "pcie_bus");
> > -	if (IS_ERR(imx6_pcie->pcie_bus))
> > -		return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_bus),
> > -				     "pcie_bus clock source missing or invalid\n");
> > +	if (imx6_pcie->drvdata->clks_cnt >= IMX6_PCIE_MAX_CLKS)
> > +		return dev_err_probe(dev, -ENOMEM, "clks_cnt is too big\n");
> 
> Same question as above, this should not fail if the clks array is
> dynamically allocated according to imx6_pcie->drvdata->clks_cnt.

Devm_kzalloc also may return NULL. Still need check. This safe check only
meanful when add new platform. imx6_pcie->drvdata->clks_cnt is static data.

It should never failure at running time. But, devm_kzalloc may failure
at run time.

It is not big deal. It's most likely personal tast. For small static data,
I perfer use static memory.

Frank

> 
> Lorenzo
> 
> >  
> > -	imx6_pcie->pcie = devm_clk_get(dev, "pcie");
> > -	if (IS_ERR(imx6_pcie->pcie))
> > -		return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie),
> > -				     "pcie clock source missing or invalid\n");
> > +	for (i = 0; i < imx6_pcie->drvdata->clks_cnt; i++)
> > +		imx6_pcie->clks[i].id = imx6_pcie->drvdata->clk_names[i];
> > +
> > +	/* Fetch clocks */
> > +	ret = devm_clk_bulk_get(dev, imx6_pcie->drvdata->clks_cnt, imx6_pcie->clks);
> > +	if (ret)
> > +		return ret;
> >  
> >  	switch (imx6_pcie->drvdata->variant) {
> > -	case IMX6SX:
> > -		imx6_pcie->pcie_inbound_axi = devm_clk_get(dev,
> > -							   "pcie_inbound_axi");
> > -		if (IS_ERR(imx6_pcie->pcie_inbound_axi))
> > -			return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_inbound_axi),
> > -					     "pcie_inbound_axi clock missing or invalid\n");
> > -		break;
> >  	case IMX8MQ:
> >  	case IMX8MQ_EP:
> > -		imx6_pcie->pcie_aux = devm_clk_get(dev, "pcie_aux");
> > -		if (IS_ERR(imx6_pcie->pcie_aux))
> > -			return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_aux),
> > -					     "pcie_aux clock source missing or invalid\n");
> > -		fallthrough;
> >  	case IMX7D:
> >  		if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
> >  			imx6_pcie->controller_id = 1;
> > @@ -1353,10 +1302,6 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> >  	case IMX8MM_EP:
> >  	case IMX8MP:
> >  	case IMX8MP_EP:
> > -		imx6_pcie->pcie_aux = devm_clk_get(dev, "pcie_aux");
> > -		if (IS_ERR(imx6_pcie->pcie_aux))
> > -			return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_aux),
> > -					     "pcie_aux clock source missing or invalid\n");
> >  		imx6_pcie->apps_reset = devm_reset_control_get_exclusive(dev,
> >  									 "apps");
> >  		if (IS_ERR(imx6_pcie->apps_reset))
> > @@ -1372,14 +1317,6 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> >  	default:
> >  		break;
> >  	}
> > -	/* Don't fetch the pcie_phy clock, if it has abstract PHY driver */
> > -	if (imx6_pcie->phy == NULL) {
> > -		imx6_pcie->pcie_phy = devm_clk_get(dev, "pcie_phy");
> > -		if (IS_ERR(imx6_pcie->pcie_phy))
> > -			return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_phy),
> > -					     "pcie_phy clock source missing or invalid\n");
> > -	}
> > -
> >  
> >  	/* Grab turnoff reset */
> >  	imx6_pcie->turnoff_reset = devm_reset_control_get_optional_exclusive(dev, "turnoff");
> > @@ -1470,6 +1407,11 @@ static void imx6_pcie_shutdown(struct platform_device *pdev)
> >  	imx6_pcie_assert_core_reset(imx6_pcie);
> >  }
> >  
> > +static const char * const imx6q_clks[] = {"pcie_bus", "pcie", "pcie_phy"};
> > +static const char * const imx8mm_clks[] = {"pcie_bus", "pcie", "pcie_aux"};
> > +static const char * const imx8mq_clks[] = {"pcie_bus", "pcie", "pcie_phy", "pcie_aux"};
> > +static const char * const imx6sx_clks[] = {"pcie_bus", "pcie", "pcie_phy", "pcie_inbound_axi"};
> > +
> >  static const struct imx6_pcie_drvdata drvdata[] = {
> >  	[IMX6Q] = {
> >  		.variant = IMX6Q,
> > @@ -1477,6 +1419,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
> >  			 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE,
> >  		.dbi_length = 0x200,
> >  		.gpr = "fsl,imx6q-iomuxc-gpr",
> > +		.clk_names = imx6q_clks,
> > +		.clks_cnt = ARRAY_SIZE(imx6q_clks),
> >  	},
> >  	[IMX6SX] = {
> >  		.variant = IMX6SX,
> > @@ -1484,6 +1428,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
> >  			 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE |
> >  			 IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
> >  		.gpr = "fsl,imx6q-iomuxc-gpr",
> > +		.clk_names = imx6sx_clks,
> > +		.clks_cnt = ARRAY_SIZE(imx6sx_clks),
> >  	},
> >  	[IMX6QP] = {
> >  		.variant = IMX6QP,
> > @@ -1492,40 +1438,56 @@ static const struct imx6_pcie_drvdata drvdata[] = {
> >  			 IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
> >  		.dbi_length = 0x200,
> >  		.gpr = "fsl,imx6q-iomuxc-gpr",
> > +		.clk_names = imx6q_clks,
> > +		.clks_cnt = ARRAY_SIZE(imx6q_clks),
> >  	},
> >  	[IMX7D] = {
> >  		.variant = IMX7D,
> >  		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
> >  		.gpr = "fsl,imx7d-iomuxc-gpr",
> > +		.clk_names = imx6q_clks,
> > +		.clks_cnt = ARRAY_SIZE(imx6q_clks),
> >  	},
> >  	[IMX8MQ] = {
> >  		.variant = IMX8MQ,
> >  		.gpr = "fsl,imx8mq-iomuxc-gpr",
> > +		.clk_names = imx8mq_clks,
> > +		.clks_cnt = ARRAY_SIZE(imx8mq_clks),
> >  	},
> >  	[IMX8MM] = {
> >  		.variant = IMX8MM,
> >  		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
> >  		.gpr = "fsl,imx8mm-iomuxc-gpr",
> > +		.clk_names = imx8mm_clks,
> > +		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
> >  	},
> >  	[IMX8MP] = {
> >  		.variant = IMX8MP,
> >  		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
> >  		.gpr = "fsl,imx8mp-iomuxc-gpr",
> > +		.clk_names = imx8mm_clks,
> > +		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
> >  	},
> >  	[IMX8MQ_EP] = {
> >  		.variant = IMX8MQ_EP,
> >  		.mode = DW_PCIE_EP_TYPE,
> >  		.gpr = "fsl,imx8mq-iomuxc-gpr",
> > +		.clk_names = imx8mq_clks,
> > +		.clks_cnt = ARRAY_SIZE(imx8mq_clks),
> >  	},
> >  	[IMX8MM_EP] = {
> >  		.variant = IMX8MM_EP,
> >  		.mode = DW_PCIE_EP_TYPE,
> >  		.gpr = "fsl,imx8mm-iomuxc-gpr",
> > +		.clk_names = imx8mm_clks,
> > +		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
> >  	},
> >  	[IMX8MP_EP] = {
> >  		.variant = IMX8MP_EP,
> >  		.mode = DW_PCIE_EP_TYPE,
> >  		.gpr = "fsl,imx8mp-iomuxc-gpr",
> > +		.clk_names = imx8mm_clks,
> > +		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
> >  	},
> >  };
> >  
> > -- 
> > 2.34.1
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v10 01/14] PCI: imx6: Simplify clock handling by using clk_bulk*() function

[Lorenzo Pieralisi]

On Tue, Feb 13, 2024 at 10:58:01AM -0500, Frank Li wrote:
> On Tue, Feb 13, 2024 at 04:31:22PM +0100, Lorenzo Pieralisi wrote:
> > On Mon, Feb 05, 2024 at 12:33:22PM -0500, Frank Li wrote:
> > > Refector the clock handling logic. Add 'clk_names' define in drvdata. Use
> > > clk_bulk*() api simplify the code.
> > > 
> > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > > 
> > > Notes:
> > >     Change from v9 to v10
> > >     - fixed missed delete a case, which need failthrough to next one.
> > >     Change from v8 to v9
> > >     - change clks names
> > >     - Add Manivannan's review tag
> > >     
> > >     Change from v7 to v8
> > >     - update comment message
> > >     - using ARRAY_SIZE to count clk_names.
> > >     Change from v6 to v7
> > >     - none
> > >     Change from v4 to v5
> > >     - update commit message
> > >     - direct using clk name list, instead of macro
> > >     - still keep caculate clk list count because sizeof return pre allocated
> > >     array size.
> > >     
> > >     Change from v3 to v4
> > >     - using clk_bulk_*() API
> > >     Change from v1 to v3
> > >     - none
> > >     
> > >     Change from v4 to v5
> > >     - update commit message
> > >     - direct using clk name list, instead of macro
> > >     - still keep caculate clk list count because sizeof return pre allocated
> > >     array size.
> > >     
> > >     Change from v3 to v4
> > >     - using clk_bulk_*() API
> > >     Change from v1 to v3
> > >     - none
> > >     
> > >     Change from v8 to v9
> > >     - change clks names
> > >     - Add Manivannan's review tag
> > >     
> > >     Change from v7 to v8
> > >     - update comment message
> > >     - using ARRAY_SIZE to count clk_names.
> > >     Change from v6 to v7
> > >     - none
> > >     Change from v4 to v5
> > >     - update commit message
> > >     - direct using clk name list, instead of macro
> > >     - still keep caculate clk list count because sizeof return pre allocated
> > >     array size.
> > >     
> > >     Change from v3 to v4
> > >     - using clk_bulk_*() API
> > >     Change from v1 to v3
> > >     - none
> > >     
> > >     Change from v4 to v5
> > >     - update commit message
> > >     - direct using clk name list, instead of macro
> > >     - still keep caculate clk list count because sizeof return pre allocated
> > >     array size.
> > >     
> > >     Change from v3 to v4
> > >     - using clk_bulk_*() API
> > >     Change from v1 to v3
> > >     - none
> > > 
> > >  drivers/pci/controller/dwc/pci-imx6.c | 138 ++++++++++----------------
> > >  1 file changed, 50 insertions(+), 88 deletions(-)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > > index 74703362aeec7..82854e94c5621 100644
> > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > @@ -61,12 +61,16 @@ enum imx6_pcie_variants {
> > >  #define IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE	BIT(1)
> > >  #define IMX6_PCIE_FLAG_SUPPORTS_SUSPEND		BIT(2)
> > >  
> > > +#define IMX6_PCIE_MAX_CLKS       6
> > > +
> > >  struct imx6_pcie_drvdata {
> > >  	enum imx6_pcie_variants variant;
> > >  	enum dw_pcie_device_mode mode;
> > >  	u32 flags;
> > >  	int dbi_length;
> > >  	const char *gpr;
> > > +	const char * const *clk_names;
> > > +	const u32 clks_cnt;
> > >  };
> > >  
> > >  struct imx6_pcie {
> > > @@ -74,11 +78,7 @@ struct imx6_pcie {
> > >  	int			reset_gpio;
> > >  	bool			gpio_active_high;
> > >  	bool			link_is_up;
> > > -	struct clk		*pcie_bus;
> > > -	struct clk		*pcie_phy;
> > > -	struct clk		*pcie_inbound_axi;
> > > -	struct clk		*pcie;
> > > -	struct clk		*pcie_aux;
> > > +	struct clk_bulk_data	clks[IMX6_PCIE_MAX_CLKS];
> > 
> > Why can't you allocate this dynamically ?
> 
> Engineer choose. clk_bulk_data is small data struct, two pointers (16byte
> in 64bit system). clks in imx is 3-4, Over half already used(compared 6).
> waste case only wast 48byte.
>  
> Dynamically allocate can't save much memory, there are some extra manage
> meta data for dynamical memory, which may bigger than 48byte.
> 
> Frank
> 
> > 
> > >  	struct regmap		*iomuxc_gpr;
> > >  	u16			msi_ctrl;
> > >  	u32			controller_id;
> > > @@ -407,13 +407,18 @@ static void imx7d_pcie_wait_for_phy_pll_lock(struct imx6_pcie *imx6_pcie)
> > >  
> > >  static int imx6_setup_phy_mpll(struct imx6_pcie *imx6_pcie)
> > >  {
> > > -	unsigned long phy_rate = clk_get_rate(imx6_pcie->pcie_phy);
> > > +	unsigned long phy_rate = 0;
> > >  	int mult, div;
> > >  	u16 val;
> > > +	int i;
> > >  
> > >  	if (!(imx6_pcie->drvdata->flags & IMX6_PCIE_FLAG_IMX6_PHY))
> > >  		return 0;
> > >  
> > > +	for (i = 0; i < imx6_pcie->drvdata->clks_cnt; i++)
> > > +		if (strncmp(imx6_pcie->clks[i].id, "pcie_phy", 8) == 0)
> > > +			phy_rate = clk_get_rate(imx6_pcie->clks[i].clk);
> > > +
> > >  	switch (phy_rate) {
> > >  	case 125000000:
> > >  		/*
> > > @@ -550,19 +555,11 @@ static int imx6_pcie_attach_pd(struct device *dev)
> > >  
> > >  static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
> > >  {
> > > -	struct dw_pcie *pci = imx6_pcie->pci;
> > > -	struct device *dev = pci->dev;
> > >  	unsigned int offset;
> > >  	int ret = 0;
> > >  
> > >  	switch (imx6_pcie->drvdata->variant) {
> > >  	case IMX6SX:
> > > -		ret = clk_prepare_enable(imx6_pcie->pcie_inbound_axi);
> > > -		if (ret) {
> > > -			dev_err(dev, "unable to enable pcie_axi clock\n");
> > > -			break;
> > > -		}
> > > -
> > >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > >  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN, 0);
> > >  		break;
> > > @@ -589,12 +586,6 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
> > >  	case IMX8MQ_EP:
> > >  	case IMX8MP:
> > >  	case IMX8MP_EP:
> > > -		ret = clk_prepare_enable(imx6_pcie->pcie_aux);
> > > -		if (ret) {
> > > -			dev_err(dev, "unable to enable pcie_aux clock\n");
> > > -			break;
> > > -		}
> > > -
> > >  		offset = imx6_pcie_grp_offset(imx6_pcie);
> > >  		/*
> > >  		 * Set the over ride low and enabled
> > > @@ -615,9 +606,6 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
> > >  static void imx6_pcie_disable_ref_clk(struct imx6_pcie *imx6_pcie)
> > >  {
> > >  	switch (imx6_pcie->drvdata->variant) {
> > > -	case IMX6SX:
> > > -		clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
> > > -		break;
> > >  	case IMX6QP:
> > >  	case IMX6Q:
> > >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> > > @@ -631,14 +619,6 @@ static void imx6_pcie_disable_ref_clk(struct imx6_pcie *imx6_pcie)
> > >  				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
> > >  				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
> > >  		break;
> > > -	case IMX8MM:
> > > -	case IMX8MM_EP:
> > > -	case IMX8MQ:
> > > -	case IMX8MQ_EP:
> > > -	case IMX8MP:
> > > -	case IMX8MP_EP:
> > > -		clk_disable_unprepare(imx6_pcie->pcie_aux);
> > > -		break;
> > >  	default:
> > >  		break;
> > >  	}
> > > @@ -650,23 +630,9 @@ static int imx6_pcie_clk_enable(struct imx6_pcie *imx6_pcie)
> > >  	struct device *dev = pci->dev;
> > >  	int ret;
> > >  
> > > -	ret = clk_prepare_enable(imx6_pcie->pcie_phy);
> > > -	if (ret) {
> > > -		dev_err(dev, "unable to enable pcie_phy clock\n");
> > > +	ret = clk_bulk_prepare_enable(imx6_pcie->drvdata->clks_cnt, imx6_pcie->clks);
> > > +	if (ret)
> > >  		return ret;
> > > -	}
> > > -
> > > -	ret = clk_prepare_enable(imx6_pcie->pcie_bus);
> > > -	if (ret) {
> > > -		dev_err(dev, "unable to enable pcie_bus clock\n");
> > > -		goto err_pcie_bus;
> > > -	}
> > > -
> > > -	ret = clk_prepare_enable(imx6_pcie->pcie);
> > > -	if (ret) {
> > > -		dev_err(dev, "unable to enable pcie clock\n");
> > > -		goto err_pcie;
> > > -	}
> > >  
> > >  	ret = imx6_pcie_enable_ref_clk(imx6_pcie);
> > >  	if (ret) {
> > > @@ -679,11 +645,7 @@ static int imx6_pcie_clk_enable(struct imx6_pcie *imx6_pcie)
> > >  	return 0;
> > >  
> > >  err_ref_clk:
> > > -	clk_disable_unprepare(imx6_pcie->pcie);
> > > -err_pcie:
> > > -	clk_disable_unprepare(imx6_pcie->pcie_bus);
> > > -err_pcie_bus:
> > > -	clk_disable_unprepare(imx6_pcie->pcie_phy);
> > > +	clk_bulk_disable_unprepare(imx6_pcie->drvdata->clks_cnt, imx6_pcie->clks);
> > >  
> > >  	return ret;
> > >  }
> > > @@ -691,9 +653,7 @@ static int imx6_pcie_clk_enable(struct imx6_pcie *imx6_pcie)
> > >  static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
> > >  {
> > >  	imx6_pcie_disable_ref_clk(imx6_pcie);
> > > -	clk_disable_unprepare(imx6_pcie->pcie);
> > > -	clk_disable_unprepare(imx6_pcie->pcie_bus);
> > > -	clk_disable_unprepare(imx6_pcie->pcie_phy);
> > > +	clk_bulk_disable_unprepare(imx6_pcie->drvdata->clks_cnt, imx6_pcie->clks);
> > >  }
> > >  
> > >  static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> > > @@ -1252,6 +1212,7 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> > >  	struct device_node *node = dev->of_node;
> > >  	int ret;
> > >  	u16 val;
> > > +	int i;
> > >  
> > >  	imx6_pcie = devm_kzalloc(dev, sizeof(*imx6_pcie), GFP_KERNEL);
> > >  	if (!imx6_pcie)
> > > @@ -1305,32 +1266,20 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> > >  		return imx6_pcie->reset_gpio;
> > >  	}
> > >  
> > > -	/* Fetch clocks */
> > > -	imx6_pcie->pcie_bus = devm_clk_get(dev, "pcie_bus");
> > > -	if (IS_ERR(imx6_pcie->pcie_bus))
> > > -		return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_bus),
> > > -				     "pcie_bus clock source missing or invalid\n");
> > > +	if (imx6_pcie->drvdata->clks_cnt >= IMX6_PCIE_MAX_CLKS)
> > > +		return dev_err_probe(dev, -ENOMEM, "clks_cnt is too big\n");
> > 
> > Same question as above, this should not fail if the clks array is
> > dynamically allocated according to imx6_pcie->drvdata->clks_cnt.
> 
> Devm_kzalloc also may return NULL. Still need check. This safe check only
> meanful when add new platform. imx6_pcie->drvdata->clks_cnt is static data.
> 
> It should never failure at running time. But, devm_kzalloc may failure
> at run time.
> 
> It is not big deal. It's most likely personal tast. For small static data,
> I perfer use static memory.

It is fine to keep the static data (but meanful, tast and perfer aren't
English words, a spell checker would fix them, if I may suggest).

Thanks,
Lorenzo

[PATCH v10 02/14] PCI: imx6: Simplify phy handling by using IMX6_PCIE_FLAG_HAS_PHYDRV

[Frank Li]

Since some i.MX platforms make use of the separate PHY driver, use
IMX6_PCIE_FLAG_HAS_PHYDRV flag to identify them and get the reference to
PHY from DT. This simplifies the code.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Notes:
    Change from v8 to v9:
    - change commit message according Manivannan's suggestion
    - Add Manivannan's review tag
    Change from v7 to v8:
    - renmae IMX6_PCIE_FLAG_HAS_PHY to IMX6_PCIE_FLAG_HAS_PHYDRV
    Change from v6 to v7:
    - none
    Change from v4 to v5:
    - none, Keep IMX6_PCIE_FLAG_HAS_PHY to indicate dts mismatch when platform
    require phy suppport.
    
    Change from v1 to v3:
    - none

 drivers/pci/controller/dwc/pci-imx6.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 82854e94c5621..59f117f855c26 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -60,6 +60,9 @@ enum imx6_pcie_variants {
 #define IMX6_PCIE_FLAG_IMX6_PHY			BIT(0)
 #define IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE	BIT(1)
 #define IMX6_PCIE_FLAG_SUPPORTS_SUSPEND		BIT(2)
+#define IMX6_PCIE_FLAG_HAS_PHYDRV			BIT(3)
+
+#define imx6_check_flag(pci, val)     (pci->drvdata->flags & val)
 
 #define IMX6_PCIE_MAX_CLKS       6
 
@@ -1277,6 +1280,13 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_PHYDRV)) {
+		imx6_pcie->phy = devm_phy_get(dev, "pcie-phy");
+		if (IS_ERR(imx6_pcie->phy))
+			return dev_err_probe(dev, PTR_ERR(imx6_pcie->phy),
+					     "failed to get pcie phy\n");
+	}
+
 	switch (imx6_pcie->drvdata->variant) {
 	case IMX8MQ:
 	case IMX8MQ_EP:
@@ -1308,11 +1318,6 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 			return dev_err_probe(dev, PTR_ERR(imx6_pcie->apps_reset),
 					     "failed to get pcie apps reset control\n");
 
-		imx6_pcie->phy = devm_phy_get(dev, "pcie-phy");
-		if (IS_ERR(imx6_pcie->phy))
-			return dev_err_probe(dev, PTR_ERR(imx6_pcie->phy),
-					     "failed to get pcie phy\n");
-
 		break;
 	default:
 		break;
@@ -1456,14 +1461,17 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 	},
 	[IMX8MM] = {
 		.variant = IMX8MM,
-		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
+		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND |
+			 IMX6_PCIE_FLAG_HAS_PHYDRV |
+			 IMX6_PCIE_FLAG_HAS_APP_RESET,
 		.gpr = "fsl,imx8mm-iomuxc-gpr",
 		.clk_names = imx8mm_clks,
 		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
 	},
 	[IMX8MP] = {
 		.variant = IMX8MP,
-		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
+		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND |
+			 IMX6_PCIE_FLAG_HAS_PHYDRV,
 		.gpr = "fsl,imx8mp-iomuxc-gpr",
 		.clk_names = imx8mm_clks,
 		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
@@ -1477,6 +1485,7 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 	},
 	[IMX8MM_EP] = {
 		.variant = IMX8MM_EP,
+		.flags = IMX6_PCIE_FLAG_HAS_PHYDRV,
 		.mode = DW_PCIE_EP_TYPE,
 		.gpr = "fsl,imx8mm-iomuxc-gpr",
 		.clk_names = imx8mm_clks,
@@ -1484,6 +1493,7 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 	},
 	[IMX8MP_EP] = {
 		.variant = IMX8MP_EP,
+		.flags = IMX6_PCIE_FLAG_HAS_PHYDRV,
 		.mode = DW_PCIE_EP_TYPE,
 		.gpr = "fsl,imx8mp-iomuxc-gpr",
 		.clk_names = imx8mm_clks,
-- 
2.34.1

[PATCH v10 03/14] PCI: imx6: Simplify reset handling by using by using *_FLAG_HAS_*_RESET

[Frank Li]

Refactors the reset handling logic in the imx6 PCI driver by adding
IMX6_PCIE_FLAG_HAS_*_RESET bitmask define for drvdata::flags.

The drvdata::flags and a bitmask ensures a cleaner and more scalable
switch-case structure for handling reset.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Notes:
    Change from v4 to v5:
    - Add Mani's Reviewed-by tag
    - Fixed MQ_EP's flags
    
    Chagne from v3 to v4:
    - none
    Change from v2 to v3:
    - add Philipp's Reviewed-by tag
    Change from v1 to v2:
    - remove condition check before reset_control_(de)assert() because it is
      none ops if a NULL pointer pass down.
    - still keep condition check at probe to help identify dts file mismatch
      problem.
    
    Change from v1 to v2:
    - remove condition check before reset_control_(de)assert() because it is
      none ops if a NULL pointer pass down.
    - still keep condition check at probe to help identify dts file mismatch
      problem.

 drivers/pci/controller/dwc/pci-imx6.c | 105 ++++++++++----------------
 1 file changed, 39 insertions(+), 66 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 59f117f855c26..a1653b58051b7 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -61,6 +61,8 @@ enum imx6_pcie_variants {
 #define IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE	BIT(1)
 #define IMX6_PCIE_FLAG_SUPPORTS_SUSPEND		BIT(2)
 #define IMX6_PCIE_FLAG_HAS_PHYDRV			BIT(3)
+#define IMX6_PCIE_FLAG_HAS_APP_RESET		BIT(4)
+#define IMX6_PCIE_FLAG_HAS_PHY_RESET		BIT(5)
 
 #define imx6_check_flag(pci, val)     (pci->drvdata->flags & val)
 
@@ -661,18 +663,10 @@ static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
 
 static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
 {
+	reset_control_assert(imx6_pcie->pciephy_reset);
+	reset_control_assert(imx6_pcie->apps_reset);
+
 	switch (imx6_pcie->drvdata->variant) {
-	case IMX7D:
-	case IMX8MQ:
-	case IMX8MQ_EP:
-		reset_control_assert(imx6_pcie->pciephy_reset);
-		fallthrough;
-	case IMX8MM:
-	case IMX8MM_EP:
-	case IMX8MP:
-	case IMX8MP_EP:
-		reset_control_assert(imx6_pcie->apps_reset);
-		break;
 	case IMX6SX:
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
 				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
@@ -693,6 +687,8 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
 				   IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);
 		break;
+	default:
+		break;
 	}
 
 	/* Some boards don't have PCIe reset GPIO. */
@@ -706,14 +702,10 @@ static int imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
 	struct dw_pcie *pci = imx6_pcie->pci;
 	struct device *dev = pci->dev;
 
+	reset_control_deassert(imx6_pcie->pciephy_reset);
+
 	switch (imx6_pcie->drvdata->variant) {
-	case IMX8MQ:
-	case IMX8MQ_EP:
-		reset_control_deassert(imx6_pcie->pciephy_reset);
-		break;
 	case IMX7D:
-		reset_control_deassert(imx6_pcie->pciephy_reset);
-
 		/* Workaround for ERR010728, failure of PCI-e PLL VCO to
 		 * oscillate, especially when cold.  This turns off "Duty-cycle
 		 * Corrector" and other mysterious undocumented things.
@@ -745,11 +737,7 @@ static int imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
 
 		usleep_range(200, 500);
 		break;
-	case IMX6Q:		/* Nothing to do */
-	case IMX8MM:
-	case IMX8MM_EP:
-	case IMX8MP:
-	case IMX8MP_EP:
+	default:
 		break;
 	}
 
@@ -796,16 +784,11 @@ static void imx6_pcie_ltssm_enable(struct device *dev)
 				   IMX6Q_GPR12_PCIE_CTL_2,
 				   IMX6Q_GPR12_PCIE_CTL_2);
 		break;
-	case IMX7D:
-	case IMX8MQ:
-	case IMX8MQ_EP:
-	case IMX8MM:
-	case IMX8MM_EP:
-	case IMX8MP:
-	case IMX8MP_EP:
-		reset_control_deassert(imx6_pcie->apps_reset);
+	default:
 		break;
 	}
+
+	reset_control_deassert(imx6_pcie->apps_reset);
 }
 
 static void imx6_pcie_ltssm_disable(struct device *dev)
@@ -819,16 +802,11 @@ static void imx6_pcie_ltssm_disable(struct device *dev)
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
 				   IMX6Q_GPR12_PCIE_CTL_2, 0);
 		break;
-	case IMX7D:
-	case IMX8MQ:
-	case IMX8MQ_EP:
-	case IMX8MM:
-	case IMX8MM_EP:
-	case IMX8MP:
-	case IMX8MP_EP:
-		reset_control_assert(imx6_pcie->apps_reset);
+	default:
 		break;
 	}
+
+	reset_control_assert(imx6_pcie->apps_reset);
 }
 
 static int imx6_pcie_start_link(struct dw_pcie *pci)
@@ -1287,38 +1265,26 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 					     "failed to get pcie phy\n");
 	}
 
+	if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_APP_RESET)) {
+		imx6_pcie->apps_reset = devm_reset_control_get_exclusive(dev, "apps");
+		if (IS_ERR(imx6_pcie->apps_reset))
+			return dev_err_probe(dev, PTR_ERR(imx6_pcie->apps_reset),
+					     "failed to get pcie apps reset control\n");
+	}
+
+	if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_PHY_RESET)) {
+		imx6_pcie->pciephy_reset = devm_reset_control_get_exclusive(dev, "pciephy");
+		if (IS_ERR(imx6_pcie->pciephy_reset))
+			return dev_err_probe(dev, PTR_ERR(imx6_pcie->pciephy_reset),
+					     "Failed to get PCIEPHY reset control\n");
+	}
+
 	switch (imx6_pcie->drvdata->variant) {
 	case IMX8MQ:
 	case IMX8MQ_EP:
 	case IMX7D:
 		if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
 			imx6_pcie->controller_id = 1;
-
-		imx6_pcie->pciephy_reset = devm_reset_control_get_exclusive(dev,
-									    "pciephy");
-		if (IS_ERR(imx6_pcie->pciephy_reset)) {
-			dev_err(dev, "Failed to get PCIEPHY reset control\n");
-			return PTR_ERR(imx6_pcie->pciephy_reset);
-		}
-
-		imx6_pcie->apps_reset = devm_reset_control_get_exclusive(dev,
-									 "apps");
-		if (IS_ERR(imx6_pcie->apps_reset)) {
-			dev_err(dev, "Failed to get PCIE APPS reset control\n");
-			return PTR_ERR(imx6_pcie->apps_reset);
-		}
-		break;
-	case IMX8MM:
-	case IMX8MM_EP:
-	case IMX8MP:
-	case IMX8MP_EP:
-		imx6_pcie->apps_reset = devm_reset_control_get_exclusive(dev,
-									 "apps");
-		if (IS_ERR(imx6_pcie->apps_reset))
-			return dev_err_probe(dev, PTR_ERR(imx6_pcie->apps_reset),
-					     "failed to get pcie apps reset control\n");
-
-		break;
 	default:
 		break;
 	}
@@ -1448,13 +1414,17 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 	},
 	[IMX7D] = {
 		.variant = IMX7D,
-		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
+		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND |
+			 IMX6_PCIE_FLAG_HAS_APP_RESET |
+			 IMX6_PCIE_FLAG_HAS_PHY_RESET,
 		.gpr = "fsl,imx7d-iomuxc-gpr",
 		.clk_names = imx6q_clks,
 		.clks_cnt = ARRAY_SIZE(imx6q_clks),
 	},
 	[IMX8MQ] = {
 		.variant = IMX8MQ,
+		.flags = IMX6_PCIE_FLAG_HAS_APP_RESET |
+			 IMX6_PCIE_FLAG_HAS_PHY_RESET,
 		.gpr = "fsl,imx8mq-iomuxc-gpr",
 		.clk_names = imx8mq_clks,
 		.clks_cnt = ARRAY_SIZE(imx8mq_clks),
@@ -1471,13 +1441,16 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 	[IMX8MP] = {
 		.variant = IMX8MP,
 		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND |
-			 IMX6_PCIE_FLAG_HAS_PHYDRV,
+			 IMX6_PCIE_FLAG_HAS_PHYDRV |
+			 IMX6_PCIE_FLAG_HAS_APP_RESET,
 		.gpr = "fsl,imx8mp-iomuxc-gpr",
 		.clk_names = imx8mm_clks,
 		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
 	},
 	[IMX8MQ_EP] = {
 		.variant = IMX8MQ_EP,
+		.flags = IMX6_PCIE_FLAG_HAS_APP_RESET |
+			 IMX6_PCIE_FLAG_HAS_PHY_RESET,
 		.mode = DW_PCIE_EP_TYPE,
 		.gpr = "fsl,imx8mq-iomuxc-gpr",
 		.clk_names = imx8mq_clks,
-- 
2.34.1

Re: [PATCH v10 03/14] PCI: imx6: Simplify reset handling by using by using *_FLAG_HAS_*_RESET

[Lorenzo Pieralisi]

On Mon, Feb 05, 2024 at 12:33:24PM -0500, Frank Li wrote:
> Refactors the reset handling logic in the imx6 PCI driver by adding
> IMX6_PCIE_FLAG_HAS_*_RESET bitmask define for drvdata::flags.
> 
> The drvdata::flags and a bitmask ensures a cleaner and more scalable
> switch-case structure for handling reset.
> 
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> 
> Notes:
>     Change from v4 to v5:
>     - Add Mani's Reviewed-by tag
>     - Fixed MQ_EP's flags
>     
>     Chagne from v3 to v4:
>     - none
>     Change from v2 to v3:
>     - add Philipp's Reviewed-by tag
>     Change from v1 to v2:
>     - remove condition check before reset_control_(de)assert() because it is
>       none ops if a NULL pointer pass down.
>     - still keep condition check at probe to help identify dts file mismatch
>       problem.
>     
>     Change from v1 to v2:
>     - remove condition check before reset_control_(de)assert() because it is
>       none ops if a NULL pointer pass down.
>     - still keep condition check at probe to help identify dts file mismatch
>       problem.
> 
>  drivers/pci/controller/dwc/pci-imx6.c | 105 ++++++++++----------------
>  1 file changed, 39 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 59f117f855c26..a1653b58051b7 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -61,6 +61,8 @@ enum imx6_pcie_variants {
>  #define IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE	BIT(1)
>  #define IMX6_PCIE_FLAG_SUPPORTS_SUSPEND		BIT(2)
>  #define IMX6_PCIE_FLAG_HAS_PHYDRV			BIT(3)
> +#define IMX6_PCIE_FLAG_HAS_APP_RESET		BIT(4)
> +#define IMX6_PCIE_FLAG_HAS_PHY_RESET		BIT(5)
>  
>  #define imx6_check_flag(pci, val)     (pci->drvdata->flags & val)
>  
> @@ -661,18 +663,10 @@ static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
>  
>  static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>  {
> +	reset_control_assert(imx6_pcie->pciephy_reset);
> +	reset_control_assert(imx6_pcie->apps_reset);
> +
>  	switch (imx6_pcie->drvdata->variant) {
> -	case IMX7D:
> -	case IMX8MQ:
> -	case IMX8MQ_EP:
> -		reset_control_assert(imx6_pcie->pciephy_reset);
> -		fallthrough;
> -	case IMX8MM:
> -	case IMX8MM_EP:
> -	case IMX8MP:
> -	case IMX8MP_EP:
> -		reset_control_assert(imx6_pcie->apps_reset);
> -		break;
>  	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> @@ -693,6 +687,8 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
>  				   IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);
>  		break;
> +	default:
> +		break;
>  	}
>  
>  	/* Some boards don't have PCIe reset GPIO. */
> @@ -706,14 +702,10 @@ static int imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  	struct dw_pcie *pci = imx6_pcie->pci;
>  	struct device *dev = pci->dev;
>  
> +	reset_control_deassert(imx6_pcie->pciephy_reset);
> +
>  	switch (imx6_pcie->drvdata->variant) {
> -	case IMX8MQ:
> -	case IMX8MQ_EP:
> -		reset_control_deassert(imx6_pcie->pciephy_reset);
> -		break;
>  	case IMX7D:
> -		reset_control_deassert(imx6_pcie->pciephy_reset);
> -
>  		/* Workaround for ERR010728, failure of PCI-e PLL VCO to
>  		 * oscillate, especially when cold.  This turns off "Duty-cycle
>  		 * Corrector" and other mysterious undocumented things.
> @@ -745,11 +737,7 @@ static int imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  
>  		usleep_range(200, 500);
>  		break;
> -	case IMX6Q:		/* Nothing to do */
> -	case IMX8MM:
> -	case IMX8MM_EP:
> -	case IMX8MP:
> -	case IMX8MP_EP:
> +	default:
>  		break;
>  	}
>  
> @@ -796,16 +784,11 @@ static void imx6_pcie_ltssm_enable(struct device *dev)
>  				   IMX6Q_GPR12_PCIE_CTL_2,
>  				   IMX6Q_GPR12_PCIE_CTL_2);
>  		break;
> -	case IMX7D:
> -	case IMX8MQ:
> -	case IMX8MQ_EP:
> -	case IMX8MM:
> -	case IMX8MM_EP:
> -	case IMX8MP:
> -	case IMX8MP_EP:
> -		reset_control_deassert(imx6_pcie->apps_reset);
> +	default:
>  		break;
>  	}
> +
> +	reset_control_deassert(imx6_pcie->apps_reset);

You rely on the fact that passing NULL is a no-op on platforms where
this wasn't called before (valid question for other hunks in this commit),
correct ?

Just checking, it does not make things much cleaner but I am not opposed to
this change.

Thanks,
Lorenzo

>  }
>  
>  static void imx6_pcie_ltssm_disable(struct device *dev)
> @@ -819,16 +802,11 @@ static void imx6_pcie_ltssm_disable(struct device *dev)
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6Q_GPR12_PCIE_CTL_2, 0);
>  		break;
> -	case IMX7D:
> -	case IMX8MQ:
> -	case IMX8MQ_EP:
> -	case IMX8MM:
> -	case IMX8MM_EP:
> -	case IMX8MP:
> -	case IMX8MP_EP:
> -		reset_control_assert(imx6_pcie->apps_reset);
> +	default:
>  		break;
>  	}
> +
> +	reset_control_assert(imx6_pcie->apps_reset);
>  }
>  
>  static int imx6_pcie_start_link(struct dw_pcie *pci)
> @@ -1287,38 +1265,26 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>  					     "failed to get pcie phy\n");
>  	}
>  
> +	if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_APP_RESET)) {
> +		imx6_pcie->apps_reset = devm_reset_control_get_exclusive(dev, "apps");
> +		if (IS_ERR(imx6_pcie->apps_reset))
> +			return dev_err_probe(dev, PTR_ERR(imx6_pcie->apps_reset),
> +					     "failed to get pcie apps reset control\n");
> +	}
> +
> +	if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_PHY_RESET)) {
> +		imx6_pcie->pciephy_reset = devm_reset_control_get_exclusive(dev, "pciephy");
> +		if (IS_ERR(imx6_pcie->pciephy_reset))
> +			return dev_err_probe(dev, PTR_ERR(imx6_pcie->pciephy_reset),
> +					     "Failed to get PCIEPHY reset control\n");
> +	}
> +
>  	switch (imx6_pcie->drvdata->variant) {
>  	case IMX8MQ:
>  	case IMX8MQ_EP:
>  	case IMX7D:
>  		if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
>  			imx6_pcie->controller_id = 1;
> -
> -		imx6_pcie->pciephy_reset = devm_reset_control_get_exclusive(dev,
> -									    "pciephy");
> -		if (IS_ERR(imx6_pcie->pciephy_reset)) {
> -			dev_err(dev, "Failed to get PCIEPHY reset control\n");
> -			return PTR_ERR(imx6_pcie->pciephy_reset);
> -		}
> -
> -		imx6_pcie->apps_reset = devm_reset_control_get_exclusive(dev,
> -									 "apps");
> -		if (IS_ERR(imx6_pcie->apps_reset)) {
> -			dev_err(dev, "Failed to get PCIE APPS reset control\n");
> -			return PTR_ERR(imx6_pcie->apps_reset);
> -		}
> -		break;
> -	case IMX8MM:
> -	case IMX8MM_EP:
> -	case IMX8MP:
> -	case IMX8MP_EP:
> -		imx6_pcie->apps_reset = devm_reset_control_get_exclusive(dev,
> -									 "apps");
> -		if (IS_ERR(imx6_pcie->apps_reset))
> -			return dev_err_probe(dev, PTR_ERR(imx6_pcie->apps_reset),
> -					     "failed to get pcie apps reset control\n");
> -
> -		break;
>  	default:
>  		break;
>  	}
> @@ -1448,13 +1414,17 @@ static const struct imx6_pcie_drvdata drvdata[] = {
>  	},
>  	[IMX7D] = {
>  		.variant = IMX7D,
> -		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
> +		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND |
> +			 IMX6_PCIE_FLAG_HAS_APP_RESET |
> +			 IMX6_PCIE_FLAG_HAS_PHY_RESET,
>  		.gpr = "fsl,imx7d-iomuxc-gpr",
>  		.clk_names = imx6q_clks,
>  		.clks_cnt = ARRAY_SIZE(imx6q_clks),
>  	},
>  	[IMX8MQ] = {
>  		.variant = IMX8MQ,
> +		.flags = IMX6_PCIE_FLAG_HAS_APP_RESET |
> +			 IMX6_PCIE_FLAG_HAS_PHY_RESET,
>  		.gpr = "fsl,imx8mq-iomuxc-gpr",
>  		.clk_names = imx8mq_clks,
>  		.clks_cnt = ARRAY_SIZE(imx8mq_clks),
> @@ -1471,13 +1441,16 @@ static const struct imx6_pcie_drvdata drvdata[] = {
>  	[IMX8MP] = {
>  		.variant = IMX8MP,
>  		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND |
> -			 IMX6_PCIE_FLAG_HAS_PHYDRV,
> +			 IMX6_PCIE_FLAG_HAS_PHYDRV |
> +			 IMX6_PCIE_FLAG_HAS_APP_RESET,
>  		.gpr = "fsl,imx8mp-iomuxc-gpr",
>  		.clk_names = imx8mm_clks,
>  		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
>  	},
>  	[IMX8MQ_EP] = {
>  		.variant = IMX8MQ_EP,
> +		.flags = IMX6_PCIE_FLAG_HAS_APP_RESET |
> +			 IMX6_PCIE_FLAG_HAS_PHY_RESET,
>  		.mode = DW_PCIE_EP_TYPE,
>  		.gpr = "fsl,imx8mq-iomuxc-gpr",
>  		.clk_names = imx8mq_clks,
> -- 
> 2.34.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v10 03/14] PCI: imx6: Simplify reset handling by using by using *_FLAG_HAS_*_RESET

[Frank Li]

On Tue, Feb 13, 2024 at 12:41:10PM +0100, Lorenzo Pieralisi wrote:
> On Mon, Feb 05, 2024 at 12:33:24PM -0500, Frank Li wrote:
> > Refactors the reset handling logic in the imx6 PCI driver by adding
> > IMX6_PCIE_FLAG_HAS_*_RESET bitmask define for drvdata::flags.
> > 
> > The drvdata::flags and a bitmask ensures a cleaner and more scalable
> > switch-case structure for handling reset.
> > 
> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > 
> > Notes:
> >     Change from v4 to v5:
> >     - Add Mani's Reviewed-by tag
> >     - Fixed MQ_EP's flags
> >     
> >     Chagne from v3 to v4:
> >     - none
> >     Change from v2 to v3:
> >     - add Philipp's Reviewed-by tag
> >     Change from v1 to v2:
> >     - remove condition check before reset_control_(de)assert() because it is
> >       none ops if a NULL pointer pass down.
> >     - still keep condition check at probe to help identify dts file mismatch
> >       problem.
> >     
> >     Change from v1 to v2:
> >     - remove condition check before reset_control_(de)assert() because it is
> >       none ops if a NULL pointer pass down.
> >     - still keep condition check at probe to help identify dts file mismatch
> >       problem.
> > 
> >  drivers/pci/controller/dwc/pci-imx6.c | 105 ++++++++++----------------
> >  1 file changed, 39 insertions(+), 66 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > index 59f117f855c26..a1653b58051b7 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -61,6 +61,8 @@ enum imx6_pcie_variants {
> >  #define IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE	BIT(1)
> >  #define IMX6_PCIE_FLAG_SUPPORTS_SUSPEND		BIT(2)
> >  #define IMX6_PCIE_FLAG_HAS_PHYDRV			BIT(3)
> > +#define IMX6_PCIE_FLAG_HAS_APP_RESET		BIT(4)
> > +#define IMX6_PCIE_FLAG_HAS_PHY_RESET		BIT(5)
> >  
> >  #define imx6_check_flag(pci, val)     (pci->drvdata->flags & val)
> >  
> > @@ -661,18 +663,10 @@ static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
> >  
> >  static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> >  {
> > +	reset_control_assert(imx6_pcie->pciephy_reset);
> > +	reset_control_assert(imx6_pcie->apps_reset);
> > +
> >  	switch (imx6_pcie->drvdata->variant) {
> > -	case IMX7D:
> > -	case IMX8MQ:
> > -	case IMX8MQ_EP:
> > -		reset_control_assert(imx6_pcie->pciephy_reset);
> > -		fallthrough;
> > -	case IMX8MM:
> > -	case IMX8MM_EP:
> > -	case IMX8MP:
> > -	case IMX8MP_EP:
> > -		reset_control_assert(imx6_pcie->apps_reset);
> > -		break;
> >  	case IMX6SX:
> >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> >  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> > @@ -693,6 +687,8 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> >  				   IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);
> >  		break;
> > +	default:
> > +		break;
> >  	}
> >  
> >  	/* Some boards don't have PCIe reset GPIO. */
> > @@ -706,14 +702,10 @@ static int imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
> >  	struct dw_pcie *pci = imx6_pcie->pci;
> >  	struct device *dev = pci->dev;
> >  
> > +	reset_control_deassert(imx6_pcie->pciephy_reset);
> > +
> >  	switch (imx6_pcie->drvdata->variant) {
> > -	case IMX8MQ:
> > -	case IMX8MQ_EP:
> > -		reset_control_deassert(imx6_pcie->pciephy_reset);
> > -		break;
> >  	case IMX7D:
> > -		reset_control_deassert(imx6_pcie->pciephy_reset);
> > -
> >  		/* Workaround for ERR010728, failure of PCI-e PLL VCO to
> >  		 * oscillate, especially when cold.  This turns off "Duty-cycle
> >  		 * Corrector" and other mysterious undocumented things.
> > @@ -745,11 +737,7 @@ static int imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
> >  
> >  		usleep_range(200, 500);
> >  		break;
> > -	case IMX6Q:		/* Nothing to do */
> > -	case IMX8MM:
> > -	case IMX8MM_EP:
> > -	case IMX8MP:
> > -	case IMX8MP_EP:
> > +	default:
> >  		break;
> >  	}
> >  
> > @@ -796,16 +784,11 @@ static void imx6_pcie_ltssm_enable(struct device *dev)
> >  				   IMX6Q_GPR12_PCIE_CTL_2,
> >  				   IMX6Q_GPR12_PCIE_CTL_2);
> >  		break;
> > -	case IMX7D:
> > -	case IMX8MQ:
> > -	case IMX8MQ_EP:
> > -	case IMX8MM:
> > -	case IMX8MM_EP:
> > -	case IMX8MP:
> > -	case IMX8MP_EP:
> > -		reset_control_deassert(imx6_pcie->apps_reset);
> > +	default:
> >  		break;
> >  	}
> > +
> > +	reset_control_deassert(imx6_pcie->apps_reset);
> 
> You rely on the fact that passing NULL is a no-op on platforms where
> this wasn't called before (valid question for other hunks in this commit),
> correct ?
> 

Thank you take time to review it. 
Yes, it follow Philipp's suggestion. Remove conditional check at v2.

Frank

> Just checking, it does not make things much cleaner but I am not opposed to
> this change.
> 
> Thanks,
> Lorenzo
> 
> >  }
> >  
> >  static void imx6_pcie_ltssm_disable(struct device *dev)
> > @@ -819,16 +802,11 @@ static void imx6_pcie_ltssm_disable(struct device *dev)
> >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> >  				   IMX6Q_GPR12_PCIE_CTL_2, 0);
> >  		break;
> > -	case IMX7D:
> > -	case IMX8MQ:
> > -	case IMX8MQ_EP:
> > -	case IMX8MM:
> > -	case IMX8MM_EP:
> > -	case IMX8MP:
> > -	case IMX8MP_EP:
> > -		reset_control_assert(imx6_pcie->apps_reset);
> > +	default:
> >  		break;
> >  	}
> > +
> > +	reset_control_assert(imx6_pcie->apps_reset);
> >  }
> >  
> >  static int imx6_pcie_start_link(struct dw_pcie *pci)
> > @@ -1287,38 +1265,26 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> >  					     "failed to get pcie phy\n");
> >  	}
> >  
> > +	if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_APP_RESET)) {
> > +		imx6_pcie->apps_reset = devm_reset_control_get_exclusive(dev, "apps");
> > +		if (IS_ERR(imx6_pcie->apps_reset))
> > +			return dev_err_probe(dev, PTR_ERR(imx6_pcie->apps_reset),
> > +					     "failed to get pcie apps reset control\n");
> > +	}
> > +
> > +	if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_PHY_RESET)) {
> > +		imx6_pcie->pciephy_reset = devm_reset_control_get_exclusive(dev, "pciephy");
> > +		if (IS_ERR(imx6_pcie->pciephy_reset))
> > +			return dev_err_probe(dev, PTR_ERR(imx6_pcie->pciephy_reset),
> > +					     "Failed to get PCIEPHY reset control\n");
> > +	}
> > +
> >  	switch (imx6_pcie->drvdata->variant) {
> >  	case IMX8MQ:
> >  	case IMX8MQ_EP:
> >  	case IMX7D:
> >  		if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
> >  			imx6_pcie->controller_id = 1;
> > -
> > -		imx6_pcie->pciephy_reset = devm_reset_control_get_exclusive(dev,
> > -									    "pciephy");
> > -		if (IS_ERR(imx6_pcie->pciephy_reset)) {
> > -			dev_err(dev, "Failed to get PCIEPHY reset control\n");
> > -			return PTR_ERR(imx6_pcie->pciephy_reset);
> > -		}
> > -
> > -		imx6_pcie->apps_reset = devm_reset_control_get_exclusive(dev,
> > -									 "apps");
> > -		if (IS_ERR(imx6_pcie->apps_reset)) {
> > -			dev_err(dev, "Failed to get PCIE APPS reset control\n");
> > -			return PTR_ERR(imx6_pcie->apps_reset);
> > -		}
> > -		break;
> > -	case IMX8MM:
> > -	case IMX8MM_EP:
> > -	case IMX8MP:
> > -	case IMX8MP_EP:
> > -		imx6_pcie->apps_reset = devm_reset_control_get_exclusive(dev,
> > -									 "apps");
> > -		if (IS_ERR(imx6_pcie->apps_reset))
> > -			return dev_err_probe(dev, PTR_ERR(imx6_pcie->apps_reset),
> > -					     "failed to get pcie apps reset control\n");
> > -
> > -		break;
> >  	default:
> >  		break;
> >  	}
> > @@ -1448,13 +1414,17 @@ static const struct imx6_pcie_drvdata drvdata[] = {
> >  	},
> >  	[IMX7D] = {
> >  		.variant = IMX7D,
> > -		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
> > +		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND |
> > +			 IMX6_PCIE_FLAG_HAS_APP_RESET |
> > +			 IMX6_PCIE_FLAG_HAS_PHY_RESET,
> >  		.gpr = "fsl,imx7d-iomuxc-gpr",
> >  		.clk_names = imx6q_clks,
> >  		.clks_cnt = ARRAY_SIZE(imx6q_clks),
> >  	},
> >  	[IMX8MQ] = {
> >  		.variant = IMX8MQ,
> > +		.flags = IMX6_PCIE_FLAG_HAS_APP_RESET |
> > +			 IMX6_PCIE_FLAG_HAS_PHY_RESET,
> >  		.gpr = "fsl,imx8mq-iomuxc-gpr",
> >  		.clk_names = imx8mq_clks,
> >  		.clks_cnt = ARRAY_SIZE(imx8mq_clks),
> > @@ -1471,13 +1441,16 @@ static const struct imx6_pcie_drvdata drvdata[] = {
> >  	[IMX8MP] = {
> >  		.variant = IMX8MP,
> >  		.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND |
> > -			 IMX6_PCIE_FLAG_HAS_PHYDRV,
> > +			 IMX6_PCIE_FLAG_HAS_PHYDRV |
> > +			 IMX6_PCIE_FLAG_HAS_APP_RESET,
> >  		.gpr = "fsl,imx8mp-iomuxc-gpr",
> >  		.clk_names = imx8mm_clks,
> >  		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
> >  	},
> >  	[IMX8MQ_EP] = {
> >  		.variant = IMX8MQ_EP,
> > +		.flags = IMX6_PCIE_FLAG_HAS_APP_RESET |
> > +			 IMX6_PCIE_FLAG_HAS_PHY_RESET,
> >  		.mode = DW_PCIE_EP_TYPE,
> >  		.gpr = "fsl,imx8mq-iomuxc-gpr",
> >  		.clk_names = imx8mq_clks,
> > -- 
> > 2.34.1
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v10 03/14] PCI: imx6: Simplify reset handling by using by using *_FLAG_HAS_*_RESET

[Bjorn Helgaas]

[+cc Nathan]

On Mon, Feb 05, 2024 at 12:33:24PM -0500, Frank Li wrote:
> Refactors the reset handling logic in the imx6 PCI driver by adding
> IMX6_PCIE_FLAG_HAS_*_RESET bitmask define for drvdata::flags.
> 
> The drvdata::flags and a bitmask ensures a cleaner and more scalable
> switch-case structure for handling reset.
> 
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Lorenzo, would you mind squashing in Nathan's fix from
https://lore.kernel.org/r/20240301-pci-imx6-fix-clang-implicit-fallthrough-v1-1-db78c7cbb384@kernel.org?

Also, the subject line has a repeated "by using by using".

Bjorn

Re: [PATCH v10 03/14] PCI: imx6: Simplify reset handling by using by using *_FLAG_HAS_*_RESET

[Frank Li]

On Fri, Mar 01, 2024 at 01:09:31PM -0600, Bjorn Helgaas wrote:
> [+cc Nathan]
> 
> On Mon, Feb 05, 2024 at 12:33:24PM -0500, Frank Li wrote:
> > Refactors the reset handling logic in the imx6 PCI driver by adding
> > IMX6_PCIE_FLAG_HAS_*_RESET bitmask define for drvdata::flags.
> > 
> > The drvdata::flags and a bitmask ensures a cleaner and more scalable
> > switch-case structure for handling reset.
> > 
> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> 
> Lorenzo, would you mind squashing in Nathan's fix from
> https://lore.kernel.org/r/20240301-pci-imx6-fix-clang-implicit-fallthrough-v1-1-db78c7cbb384@kernel.org?
> 
> Also, the subject line has a repeated "by using by using".

I think that it is good to squashing it.

Frank Li

> 
> Bjorn

Re: [PATCH v10 03/14] PCI: imx6: Simplify reset handling by using by using *_FLAG_HAS_*_RESET

[Lorenzo Pieralisi]

On Fri, Mar 01, 2024 at 01:09:31PM -0600, Bjorn Helgaas wrote:
> [+cc Nathan]
> 
> On Mon, Feb 05, 2024 at 12:33:24PM -0500, Frank Li wrote:
> > Refactors the reset handling logic in the imx6 PCI driver by adding
> > IMX6_PCIE_FLAG_HAS_*_RESET bitmask define for drvdata::flags.
> > 
> > The drvdata::flags and a bitmask ensures a cleaner and more scalable
> > switch-case structure for handling reset.
> > 
> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> 
> Lorenzo, would you mind squashing in Nathan's fix from
> https://lore.kernel.org/r/20240301-pci-imx6-fix-clang-implicit-fallthrough-v1-1-db78c7cbb384@kernel.org?
> 
> Also, the subject line has a repeated "by using by using".

I will fix it up.

Thanks,
Lorenzo

[PATCH v10 04/14] PCI: imx6: Simplify ltssm_enable() by using ltssm_off and ltssm_mask

[Frank Li]

Add drvdata::ltssm_off and drvdata::ltssm_mask to simple
imx6_pcie_ltssm_enable(disable)() logic.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Notes:
    Change from v7 to v8
    - Add Manivannan Sadhasivam's review tag
    Change from v1 to v7
    - none

 drivers/pci/controller/dwc/pci-imx6.c | 37 ++++++++++++---------------
 1 file changed, 16 insertions(+), 21 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index a1653b58051b7..8c816ff159115 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -76,6 +76,8 @@ struct imx6_pcie_drvdata {
 	const char *gpr;
 	const char * const *clk_names;
 	const u32 clks_cnt;
+	const u32 ltssm_off;
+	const u32 ltssm_mask;
 };
 
 struct imx6_pcie {
@@ -775,18 +777,11 @@ static int imx6_pcie_wait_for_speed_change(struct imx6_pcie *imx6_pcie)
 static void imx6_pcie_ltssm_enable(struct device *dev)
 {
 	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+	const struct imx6_pcie_drvdata *drvdata = imx6_pcie->drvdata;
 
-	switch (imx6_pcie->drvdata->variant) {
-	case IMX6Q:
-	case IMX6SX:
-	case IMX6QP:
-		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
-				   IMX6Q_GPR12_PCIE_CTL_2,
-				   IMX6Q_GPR12_PCIE_CTL_2);
-		break;
-	default:
-		break;
-	}
+	if (drvdata->ltssm_mask)
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, drvdata->ltssm_off, drvdata->ltssm_mask,
+				   drvdata->ltssm_mask);
 
 	reset_control_deassert(imx6_pcie->apps_reset);
 }
@@ -794,17 +789,11 @@ static void imx6_pcie_ltssm_enable(struct device *dev)
 static void imx6_pcie_ltssm_disable(struct device *dev)
 {
 	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+	const struct imx6_pcie_drvdata *drvdata = imx6_pcie->drvdata;
 
-	switch (imx6_pcie->drvdata->variant) {
-	case IMX6Q:
-	case IMX6SX:
-	case IMX6QP:
-		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
-				   IMX6Q_GPR12_PCIE_CTL_2, 0);
-		break;
-	default:
-		break;
-	}
+	if (drvdata->ltssm_mask)
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, drvdata->ltssm_off,
+				   drvdata->ltssm_mask, 0);
 
 	reset_control_assert(imx6_pcie->apps_reset);
 }
@@ -1392,6 +1381,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.gpr = "fsl,imx6q-iomuxc-gpr",
 		.clk_names = imx6q_clks,
 		.clks_cnt = ARRAY_SIZE(imx6q_clks),
+		.ltssm_off = IOMUXC_GPR12,
+		.ltssm_mask = IMX6Q_GPR12_PCIE_CTL_2,
 	},
 	[IMX6SX] = {
 		.variant = IMX6SX,
@@ -1401,6 +1392,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.gpr = "fsl,imx6q-iomuxc-gpr",
 		.clk_names = imx6sx_clks,
 		.clks_cnt = ARRAY_SIZE(imx6sx_clks),
+		.ltssm_off = IOMUXC_GPR12,
+		.ltssm_mask = IMX6Q_GPR12_PCIE_CTL_2,
 	},
 	[IMX6QP] = {
 		.variant = IMX6QP,
@@ -1411,6 +1404,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.gpr = "fsl,imx6q-iomuxc-gpr",
 		.clk_names = imx6q_clks,
 		.clks_cnt = ARRAY_SIZE(imx6q_clks),
+		.ltssm_off = IOMUXC_GPR12,
+		.ltssm_mask = IMX6Q_GPR12_PCIE_CTL_2,
 	},
 	[IMX7D] = {
 		.variant = IMX7D,
-- 
2.34.1

[PATCH v10 05/14] PCI: imx6: Simplify configure_type() by using mode_off and mode_mask

[Frank Li]

Add drvdata::mode_off and drvdata::mode_mask to simplify
imx6_pcie_configure_type() logic.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Notes:
    Chagne from v8 to v9
    - add Manivannan's review tag
    Change from v7 to v8
    - replace simple with simplify
    - remove reduntant comments about FILED_PREP
    Change from v3 to v7
    - none
    Change from v2 to v3
    - none
    Change from v1 to v2
    - use ffs() to fixe build error.
    
    Change from v2 to v3
    - none
    Change from v1 to v2
    - use ffs() to fixe build error.

 drivers/pci/controller/dwc/pci-imx6.c | 59 ++++++++++++++++++---------
 1 file changed, 39 insertions(+), 20 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 8c816ff159115..4eeaf54709afd 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -68,6 +68,7 @@ enum imx6_pcie_variants {
 
 #define IMX6_PCIE_MAX_CLKS       6
 
+#define IMX6_PCIE_MAX_INSTANCES			2
 struct imx6_pcie_drvdata {
 	enum imx6_pcie_variants variant;
 	enum dw_pcie_device_mode mode;
@@ -78,6 +79,8 @@ struct imx6_pcie_drvdata {
 	const u32 clks_cnt;
 	const u32 ltssm_off;
 	const u32 ltssm_mask;
+	const u32 mode_off[IMX6_PCIE_MAX_INSTANCES];
+	const u32 mode_mask[IMX6_PCIE_MAX_INSTANCES];
 };
 
 struct imx6_pcie {
@@ -174,32 +177,24 @@ static unsigned int imx6_pcie_grp_offset(const struct imx6_pcie *imx6_pcie)
 
 static void imx6_pcie_configure_type(struct imx6_pcie *imx6_pcie)
 {
-	unsigned int mask, val, mode;
+	const struct imx6_pcie_drvdata *drvdata = imx6_pcie->drvdata;
+	unsigned int mask, val, mode, id;
 
-	if (imx6_pcie->drvdata->mode == DW_PCIE_EP_TYPE)
+	if (drvdata->mode == DW_PCIE_EP_TYPE)
 		mode = PCI_EXP_TYPE_ENDPOINT;
 	else
 		mode = PCI_EXP_TYPE_ROOT_PORT;
 
-	switch (imx6_pcie->drvdata->variant) {
-	case IMX8MQ:
-	case IMX8MQ_EP:
-		if (imx6_pcie->controller_id == 1) {
-			mask = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE;
-			val  = FIELD_PREP(IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE,
-					  mode);
-		} else {
-			mask = IMX6Q_GPR12_DEVICE_TYPE;
-			val  = FIELD_PREP(IMX6Q_GPR12_DEVICE_TYPE, mode);
-		}
-		break;
-	default:
-		mask = IMX6Q_GPR12_DEVICE_TYPE;
-		val  = FIELD_PREP(IMX6Q_GPR12_DEVICE_TYPE, mode);
-		break;
-	}
+	id = imx6_pcie->controller_id;
+
+	/* If mode_mask[id] is zero, means each controller have its individual gpr */
+	if (!drvdata->mode_mask[id])
+		id = 0;
+
+	mask = drvdata->mode_mask[id];
+	val = mode << (ffs(mask) - 1);
 
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12, mask, val);
+	regmap_update_bits(imx6_pcie->iomuxc_gpr, drvdata->mode_off[id], mask, val);
 }
 
 static int pcie_phy_poll_ack(struct imx6_pcie *imx6_pcie, bool exp_val)
@@ -1383,6 +1378,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.clks_cnt = ARRAY_SIZE(imx6q_clks),
 		.ltssm_off = IOMUXC_GPR12,
 		.ltssm_mask = IMX6Q_GPR12_PCIE_CTL_2,
+		.mode_off[0] = IOMUXC_GPR12,
+		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
 	},
 	[IMX6SX] = {
 		.variant = IMX6SX,
@@ -1394,6 +1391,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.clks_cnt = ARRAY_SIZE(imx6sx_clks),
 		.ltssm_off = IOMUXC_GPR12,
 		.ltssm_mask = IMX6Q_GPR12_PCIE_CTL_2,
+		.mode_off[0] = IOMUXC_GPR12,
+		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
 	},
 	[IMX6QP] = {
 		.variant = IMX6QP,
@@ -1406,6 +1405,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.clks_cnt = ARRAY_SIZE(imx6q_clks),
 		.ltssm_off = IOMUXC_GPR12,
 		.ltssm_mask = IMX6Q_GPR12_PCIE_CTL_2,
+		.mode_off[0] = IOMUXC_GPR12,
+		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
 	},
 	[IMX7D] = {
 		.variant = IMX7D,
@@ -1415,6 +1416,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.gpr = "fsl,imx7d-iomuxc-gpr",
 		.clk_names = imx6q_clks,
 		.clks_cnt = ARRAY_SIZE(imx6q_clks),
+		.mode_off[0] = IOMUXC_GPR12,
+		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
 	},
 	[IMX8MQ] = {
 		.variant = IMX8MQ,
@@ -1423,6 +1426,10 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.gpr = "fsl,imx8mq-iomuxc-gpr",
 		.clk_names = imx8mq_clks,
 		.clks_cnt = ARRAY_SIZE(imx8mq_clks),
+		.mode_off[0] = IOMUXC_GPR12,
+		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
+		.mode_off[1] = IOMUXC_GPR12,
+		.mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE,
 	},
 	[IMX8MM] = {
 		.variant = IMX8MM,
@@ -1432,6 +1439,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.gpr = "fsl,imx8mm-iomuxc-gpr",
 		.clk_names = imx8mm_clks,
 		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
+		.mode_off[0] = IOMUXC_GPR12,
+		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
 	},
 	[IMX8MP] = {
 		.variant = IMX8MP,
@@ -1441,6 +1450,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.gpr = "fsl,imx8mp-iomuxc-gpr",
 		.clk_names = imx8mm_clks,
 		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
+		.mode_off[0] = IOMUXC_GPR12,
+		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
 	},
 	[IMX8MQ_EP] = {
 		.variant = IMX8MQ_EP,
@@ -1450,6 +1461,10 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.gpr = "fsl,imx8mq-iomuxc-gpr",
 		.clk_names = imx8mq_clks,
 		.clks_cnt = ARRAY_SIZE(imx8mq_clks),
+		.mode_off[0] = IOMUXC_GPR12,
+		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
+		.mode_off[1] = IOMUXC_GPR12,
+		.mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE,
 	},
 	[IMX8MM_EP] = {
 		.variant = IMX8MM_EP,
@@ -1458,6 +1473,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.gpr = "fsl,imx8mm-iomuxc-gpr",
 		.clk_names = imx8mm_clks,
 		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
+		.mode_off[0] = IOMUXC_GPR12,
+		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
 	},
 	[IMX8MP_EP] = {
 		.variant = IMX8MP_EP,
@@ -1466,6 +1483,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.gpr = "fsl,imx8mp-iomuxc-gpr",
 		.clk_names = imx8mm_clks,
 		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
+		.mode_off[0] = IOMUXC_GPR12,
+		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
 	},
 };
 
-- 
2.34.1

[PATCH v10 06/14] PCI: imx6: Simplify switch-case logic by involve init_phy callback

[Frank Li]

Instead of using the switch case statement to initialize the PHY handled by
this driver itself, let's introduce a new callback init_phy() and define it
for platforms that require it. This simplifies the code.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Notes:
    Change from v7 to v8:
    - rework commit message
    - wrap comments to 100 chars
    - return 0 at imx7d_pcie_init_phy()
    
    change from v1 to v4:
    - none

 drivers/pci/controller/dwc/pci-imx6.c | 134 +++++++++++++-------------
 1 file changed, 69 insertions(+), 65 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 4eeaf54709afd..c266b9f098a5b 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -69,6 +69,9 @@ enum imx6_pcie_variants {
 #define IMX6_PCIE_MAX_CLKS       6
 
 #define IMX6_PCIE_MAX_INSTANCES			2
+
+struct imx6_pcie;
+
 struct imx6_pcie_drvdata {
 	enum imx6_pcie_variants variant;
 	enum dw_pcie_device_mode mode;
@@ -81,6 +84,7 @@ struct imx6_pcie_drvdata {
 	const u32 ltssm_mask;
 	const u32 mode_off[IMX6_PCIE_MAX_INSTANCES];
 	const u32 mode_mask[IMX6_PCIE_MAX_INSTANCES];
+	int (*init_phy)(struct imx6_pcie *pcie);
 };
 
 struct imx6_pcie {
@@ -322,76 +326,66 @@ static int pcie_phy_write(struct imx6_pcie *imx6_pcie, int addr, u16 data)
 	return 0;
 }
 
-static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
+static int imx8mq_pcie_init_phy(struct imx6_pcie *imx6_pcie)
 {
-	switch (imx6_pcie->drvdata->variant) {
-	case IMX8MM:
-	case IMX8MM_EP:
-	case IMX8MP:
-	case IMX8MP_EP:
-		/*
-		 * The PHY initialization had been done in the PHY
-		 * driver, break here directly.
-		 */
-		break;
-	case IMX8MQ:
-	case IMX8MQ_EP:
-		/*
-		 * TODO: Currently this code assumes external
-		 * oscillator is being used
-		 */
+	/* TODO: Currently this code assumes external oscillator is being used */
+	regmap_update_bits(imx6_pcie->iomuxc_gpr,
+			   imx6_pcie_grp_offset(imx6_pcie),
+			   IMX8MQ_GPR_PCIE_REF_USE_PAD,
+			   IMX8MQ_GPR_PCIE_REF_USE_PAD);
+	/*
+	 * Regarding the datasheet, the PCIE_VPH is suggested to be 1.8V. If the PCIE_VPH is
+	 * supplied by 3.3V, the VREG_BYPASS should be cleared to zero.
+	 */
+	if (imx6_pcie->vph && regulator_get_voltage(imx6_pcie->vph) > 3000000)
 		regmap_update_bits(imx6_pcie->iomuxc_gpr,
 				   imx6_pcie_grp_offset(imx6_pcie),
-				   IMX8MQ_GPR_PCIE_REF_USE_PAD,
-				   IMX8MQ_GPR_PCIE_REF_USE_PAD);
-		/*
-		 * Regarding the datasheet, the PCIE_VPH is suggested
-		 * to be 1.8V. If the PCIE_VPH is supplied by 3.3V, the
-		 * VREG_BYPASS should be cleared to zero.
-		 */
-		if (imx6_pcie->vph &&
-		    regulator_get_voltage(imx6_pcie->vph) > 3000000)
-			regmap_update_bits(imx6_pcie->iomuxc_gpr,
-					   imx6_pcie_grp_offset(imx6_pcie),
-					   IMX8MQ_GPR_PCIE_VREG_BYPASS,
-					   0);
-		break;
-	case IMX7D:
-		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
-				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL, 0);
-		break;
-	case IMX6SX:
-		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
-				   IMX6SX_GPR12_PCIE_RX_EQ_MASK,
-				   IMX6SX_GPR12_PCIE_RX_EQ_2);
-		fallthrough;
-	default:
-		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+				   IMX8MQ_GPR_PCIE_VREG_BYPASS,
+				   0);
+
+	return 0;
+}
+
+static int imx7d_pcie_init_phy(struct imx6_pcie *imx6_pcie)
+{
+	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12, IMX7D_GPR12_PCIE_PHY_REFCLK_SEL, 0);
+
+	return 0;
+}
+
+static int imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
+{
+	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
 				   IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
 
-		/* configure constant input signal to the pcie ctrl and phy */
-		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
-				   IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
-
-		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
-				   IMX6Q_GPR8_TX_DEEMPH_GEN1,
-				   imx6_pcie->tx_deemph_gen1 << 0);
-		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
-				   IMX6Q_GPR8_TX_DEEMPH_GEN2_3P5DB,
-				   imx6_pcie->tx_deemph_gen2_3p5db << 6);
-		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
-				   IMX6Q_GPR8_TX_DEEMPH_GEN2_6DB,
-				   imx6_pcie->tx_deemph_gen2_6db << 12);
-		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
-				   IMX6Q_GPR8_TX_SWING_FULL,
-				   imx6_pcie->tx_swing_full << 18);
-		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
-				   IMX6Q_GPR8_TX_SWING_LOW,
-				   imx6_pcie->tx_swing_low << 25);
-		break;
-	}
+	/* configure constant input signal to the pcie ctrl and phy */
+	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+			   IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
+
+	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
+			   IMX6Q_GPR8_TX_DEEMPH_GEN1,
+			   imx6_pcie->tx_deemph_gen1 << 0);
+	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
+			   IMX6Q_GPR8_TX_DEEMPH_GEN2_3P5DB,
+			   imx6_pcie->tx_deemph_gen2_3p5db << 6);
+	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
+			   IMX6Q_GPR8_TX_DEEMPH_GEN2_6DB,
+			   imx6_pcie->tx_deemph_gen2_6db << 12);
+	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
+			   IMX6Q_GPR8_TX_SWING_FULL,
+			   imx6_pcie->tx_swing_full << 18);
+	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
+			   IMX6Q_GPR8_TX_SWING_LOW,
+			   imx6_pcie->tx_swing_low << 25);
+	return 0;
+}
 
-	imx6_pcie_configure_type(imx6_pcie);
+static int imx6sx_pcie_init_phy(struct imx6_pcie *imx6_pcie)
+{
+	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+			   IMX6SX_GPR12_PCIE_RX_EQ_MASK, IMX6SX_GPR12_PCIE_RX_EQ_2);
+
+	return imx6_pcie_init_phy(imx6_pcie);
 }
 
 static void imx7d_pcie_wait_for_phy_pll_lock(struct imx6_pcie *imx6_pcie)
@@ -902,7 +896,11 @@ static int imx6_pcie_host_init(struct dw_pcie_rp *pp)
 	}
 
 	imx6_pcie_assert_core_reset(imx6_pcie);
-	imx6_pcie_init_phy(imx6_pcie);
+
+	if (imx6_pcie->drvdata->init_phy)
+		imx6_pcie->drvdata->init_phy(imx6_pcie);
+
+	imx6_pcie_configure_type(imx6_pcie);
 
 	ret = imx6_pcie_clk_enable(imx6_pcie);
 	if (ret) {
@@ -1380,6 +1378,7 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.ltssm_mask = IMX6Q_GPR12_PCIE_CTL_2,
 		.mode_off[0] = IOMUXC_GPR12,
 		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
+		.init_phy = imx6_pcie_init_phy,
 	},
 	[IMX6SX] = {
 		.variant = IMX6SX,
@@ -1393,6 +1392,7 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.ltssm_mask = IMX6Q_GPR12_PCIE_CTL_2,
 		.mode_off[0] = IOMUXC_GPR12,
 		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
+		.init_phy = imx6sx_pcie_init_phy,
 	},
 	[IMX6QP] = {
 		.variant = IMX6QP,
@@ -1407,6 +1407,7 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.ltssm_mask = IMX6Q_GPR12_PCIE_CTL_2,
 		.mode_off[0] = IOMUXC_GPR12,
 		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
+		.init_phy = imx6_pcie_init_phy,
 	},
 	[IMX7D] = {
 		.variant = IMX7D,
@@ -1418,6 +1419,7 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.clks_cnt = ARRAY_SIZE(imx6q_clks),
 		.mode_off[0] = IOMUXC_GPR12,
 		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
+		.init_phy = imx7d_pcie_init_phy,
 	},
 	[IMX8MQ] = {
 		.variant = IMX8MQ,
@@ -1430,6 +1432,7 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
 		.mode_off[1] = IOMUXC_GPR12,
 		.mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE,
+		.init_phy = imx8mq_pcie_init_phy,
 	},
 	[IMX8MM] = {
 		.variant = IMX8MM,
@@ -1465,6 +1468,7 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
 		.mode_off[1] = IOMUXC_GPR12,
 		.mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE,
+		.init_phy = imx8mq_pcie_init_phy,
 	},
 	[IMX8MM_EP] = {
 		.variant = IMX8MM_EP,
-- 
2.34.1

[PATCH v10 07/14] dt-bindings: imx6q-pcie: Clean up irrationality clocks check

[Frank Li]

The bindings referencing this file already define these constraints for
each of the variants, so the if not: then: is redundant.

Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Notes:
    Change from v7 to v8
    - add Manivannan Sadhasiva's Ack tag
    Change from v6 to v7
    - rewrite git commit message by using simple words
    Change from v5 to v6
    - rewrite git commit message and explain why remove it safely.
    - Add Rob's Ack
    Change from v1 to v4
    - new patch at v4

 .../bindings/pci/fsl,imx6q-pcie-common.yaml      | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml
index d91b639ae7ae7..0c50487a3866d 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml
@@ -150,22 +150,6 @@ allOf:
             - {}
             - const: pcie_phy
             - const: pcie_aux
-  - if:
-      properties:
-        compatible:
-          not:
-            contains:
-              enum:
-                - fsl,imx6sx-pcie
-                - fsl,imx8mq-pcie
-                - fsl,imx6sx-pcie-ep
-                - fsl,imx8mq-pcie-ep
-    then:
-      properties:
-        clocks:
-          maxItems: 3
-        clock-names:
-          maxItems: 3
 
   - if:
       properties:
-- 
2.34.1

[PATCH v10 08/14] dt-bindings: imx6q-pcie: Restruct reg and reg-name

[Frank Li]

snps,dw-pcie.yaml already have reg and reg-name information. Needn't
duplciate here.

Add 'if' check for existed compatible string to restrict reg and reg-names.
This prepare to add new compatible string with difference reg-names sets.

Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Notes:
    Change from v7 to v8
    - Add Manivannan Sadhasivam's ACK tag
    Change from v6 to v7
    - add Krzysztof's review tag
    Change from v5 to v6
    - Add if check for existed compatible string
    Change from v4 to v5
    - add Rob's Acked
    Change from v1 to v4:
    - new patch at v4

 .../bindings/pci/fsl,imx6q-pcie.yaml          | 30 ++++++++++++-------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
index 81bbb8728f0f9..eeca6b7b540f9 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
@@ -30,16 +30,6 @@ properties:
       - fsl,imx8mm-pcie
       - fsl,imx8mp-pcie
 
-  reg:
-    items:
-      - description: Data Bus Interface (DBI) registers.
-      - description: PCIe configuration space region.
-
-  reg-names:
-    items:
-      - const: dbi
-      - const: config
-
   clocks:
     minItems: 3
     items:
@@ -90,6 +80,26 @@ required:
 allOf:
   - $ref: /schemas/pci/snps,dw-pcie.yaml#
   - $ref: /schemas/pci/fsl,imx6q-pcie-common.yaml#
+  - if:
+      properties:
+        compatible:
+          enum:
+            - fsl,imx6q-pcie
+            - fsl,imx6sx-pcie
+            - fsl,imx6qp-pcie
+            - fsl,imx7d-pcie
+            - fsl,imx8mq-pcie
+            - fsl,imx8mm-pcie
+            - fsl,imx8mp-pcie
+    then:
+      properties:
+        reg:
+          maxItems: 2
+        reg-names:
+          items:
+            - const: dbi
+            - const: config
+
   - if:
       properties:
         compatible:
-- 
2.34.1

[PATCH v10 09/14] dt-bindings: imx6q-pcie: Add imx95 pcie compatible string

[Frank Li]

From: Richard Zhu <hongxing.zhu@nxp.com>

Add i.MX95 PCIe "fsl,imx95-pcie" compatible string.
Add "atu" and "app" to reg-names.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Notes:
    Change from v8 to v9
    - Added rob's review tag
    Change from v7 to v8
    -none
    Change from v6 to v7
    - Added my sign off
    
    Change from v5 to v6
    - move atu and app after config
    
    Change from v2 to v3
    - Remove krzy's ACK tag
    - Add condition check for imx95, which required more reg-names then old
    platform, so need Krzy review again,
    
    Change from v1 to v2
    - add Krzy's ACK tag

 .../bindings/pci/fsl,imx6q-pcie-common.yaml   |  1 +
 .../bindings/pci/fsl,imx6q-pcie.yaml          | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml
index 0c50487a3866d..a8b34f58f8f49 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml
@@ -207,6 +207,7 @@ allOf:
                 - fsl,imx6sx-pcie
                 - fsl,imx6q-pcie
                 - fsl,imx6qp-pcie
+                - fsl,imx95-pcie
                 - fsl,imx6sx-pcie-ep
                 - fsl,imx6q-pcie-ep
                 - fsl,imx6qp-pcie-ep
diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
index eeca6b7b540f9..8b8d77b1154b5 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
@@ -29,6 +29,7 @@ properties:
       - fsl,imx8mq-pcie
       - fsl,imx8mm-pcie
       - fsl,imx8mp-pcie
+      - fsl,imx95-pcie
 
   clocks:
     minItems: 3
@@ -100,6 +101,23 @@ allOf:
             - const: dbi
             - const: config
 
+  - if:
+      properties:
+        compatible:
+          enum:
+            - fsl,imx95-pcie
+    then:
+      properties:
+        reg:
+          minItems: 4
+          maxItems: 4
+        reg-names:
+          items:
+            - const: dbi
+            - const: config
+            - const: atu
+            - const: app
+
   - if:
       properties:
         compatible:
@@ -121,6 +139,7 @@ allOf:
         compatible:
           enum:
             - fsl,imx8mq-pcie
+            - fsl,imx95-pcie
     then:
       properties:
         clocks:
-- 
2.34.1

[PATCH v10 10/14] PCI: imx6: Add iMX95 PCIe Root Complex support

[Frank Li]

Add iMX95 PCIe Root Complex support.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Notes:
    Change from v8 to v9
    - Add mani's review tag
    
    Change from v7 to v8
    - Update commit subject
    - add const from regmap
    - remove unnessary logic in imx6_pcie_deassert_core_reset()
    
    Change from v4 to v7
    - none
    Change from v1 to v3
    - none

 drivers/pci/controller/dwc/pci-imx6.c | 82 +++++++++++++++++++++++++--
 1 file changed, 77 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index c266b9f098a5b..183bb3b31cf16 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -42,6 +42,25 @@
 #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE	GENMASK(11, 8)
 #define IMX8MQ_PCIE2_BASE_ADDR			0x33c00000
 
+#define IMX95_PCIE_PHY_GEN_CTRL			0x0
+#define IMX95_PCIE_REF_USE_PAD			BIT(17)
+
+#define IMX95_PCIE_PHY_MPLLA_CTRL		0x10
+#define IMX95_PCIE_PHY_MPLL_STATE		BIT(30)
+
+#define IMX95_PCIE_SS_RW_REG_0			0xf0
+#define IMX95_PCIE_REF_CLKEN			BIT(23)
+#define IMX95_PCIE_PHY_CR_PARA_SEL		BIT(9)
+
+#define IMX95_PE0_GEN_CTRL_1			0x1050
+#define IMX95_PCIE_DEVICE_TYPE			GENMASK(3, 0)
+
+#define IMX95_PE0_GEN_CTRL_3			0x1058
+#define IMX95_PCIE_LTSSM_EN			BIT(0)
+
+#define IMX95_PE0_PM_STS			0x1064
+#define IMX95_PCIE_PM_LINKST_IN_L2		BIT(14)
+
 #define to_imx6_pcie(x)	dev_get_drvdata((x)->dev)
 
 enum imx6_pcie_variants {
@@ -52,6 +71,7 @@ enum imx6_pcie_variants {
 	IMX8MQ,
 	IMX8MM,
 	IMX8MP,
+	IMX95,
 	IMX8MQ_EP,
 	IMX8MM_EP,
 	IMX8MP_EP,
@@ -63,6 +83,7 @@ enum imx6_pcie_variants {
 #define IMX6_PCIE_FLAG_HAS_PHYDRV			BIT(3)
 #define IMX6_PCIE_FLAG_HAS_APP_RESET		BIT(4)
 #define IMX6_PCIE_FLAG_HAS_PHY_RESET		BIT(5)
+#define IMX6_PCIE_FLAG_HAS_SERDES		BIT(6)
 
 #define imx6_check_flag(pci, val)     (pci->drvdata->flags & val)
 
@@ -179,6 +200,24 @@ static unsigned int imx6_pcie_grp_offset(const struct imx6_pcie *imx6_pcie)
 	return imx6_pcie->controller_id == 1 ? IOMUXC_GPR16 : IOMUXC_GPR14;
 }
 
+static int imx95_pcie_init_phy(struct imx6_pcie *imx6_pcie)
+{
+	regmap_update_bits(imx6_pcie->iomuxc_gpr,
+			IMX95_PCIE_SS_RW_REG_0,
+			IMX95_PCIE_PHY_CR_PARA_SEL,
+			IMX95_PCIE_PHY_CR_PARA_SEL);
+
+	regmap_update_bits(imx6_pcie->iomuxc_gpr,
+			   IMX95_PCIE_PHY_GEN_CTRL,
+			   IMX95_PCIE_REF_USE_PAD, 0);
+	regmap_update_bits(imx6_pcie->iomuxc_gpr,
+			   IMX95_PCIE_SS_RW_REG_0,
+			   IMX95_PCIE_REF_CLKEN,
+			   IMX95_PCIE_REF_CLKEN);
+
+	return 0;
+}
+
 static void imx6_pcie_configure_type(struct imx6_pcie *imx6_pcie)
 {
 	const struct imx6_pcie_drvdata *drvdata = imx6_pcie->drvdata;
@@ -575,6 +614,7 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
 				   IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
 		break;
 	case IMX7D:
+	case IMX95:
 		break;
 	case IMX8MM:
 	case IMX8MM_EP:
@@ -1278,12 +1318,32 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(imx6_pcie->turnoff_reset);
 	}
 
+	if (imx6_pcie->drvdata->gpr) {
 	/* Grab GPR config register range */
-	imx6_pcie->iomuxc_gpr =
-		 syscon_regmap_lookup_by_compatible(imx6_pcie->drvdata->gpr);
-	if (IS_ERR(imx6_pcie->iomuxc_gpr)) {
-		dev_err(dev, "unable to find iomuxc registers\n");
-		return PTR_ERR(imx6_pcie->iomuxc_gpr);
+		imx6_pcie->iomuxc_gpr =
+			 syscon_regmap_lookup_by_compatible(imx6_pcie->drvdata->gpr);
+		if (IS_ERR(imx6_pcie->iomuxc_gpr))
+			return dev_err_probe(dev, PTR_ERR(imx6_pcie->iomuxc_gpr),
+					     "unable to find iomuxc registers\n");
+	}
+
+	if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_SERDES)) {
+		void __iomem *off = devm_platform_ioremap_resource_byname(pdev, "app");
+
+		if (IS_ERR(off))
+			return dev_err_probe(dev, PTR_ERR(off),
+					     "unable to find serdes registers\n");
+
+		static const struct regmap_config regmap_config = {
+			.reg_bits = 32,
+			.val_bits = 32,
+			.reg_stride = 4,
+		};
+
+		imx6_pcie->iomuxc_gpr = devm_regmap_init_mmio(dev, off, &regmap_config);
+		if (IS_ERR(imx6_pcie->iomuxc_gpr))
+			return dev_err_probe(dev, PTR_ERR(imx6_pcie->iomuxc_gpr),
+					     "unable to find iomuxc registers\n");
 	}
 
 	/* Grab PCIe PHY Tx Settings */
@@ -1456,6 +1516,17 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.mode_off[0] = IOMUXC_GPR12,
 		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
 	},
+	[IMX95] = {
+		.variant = IMX95,
+		.flags = IMX6_PCIE_FLAG_HAS_SERDES,
+		.clk_names = imx8mq_clks,
+		.clks_cnt = ARRAY_SIZE(imx8mq_clks),
+		.ltssm_off = IMX95_PE0_GEN_CTRL_3,
+		.ltssm_mask = IMX95_PCIE_LTSSM_EN,
+		.mode_off[0]  = IMX95_PE0_GEN_CTRL_1,
+		.mode_mask[0] = IMX95_PCIE_DEVICE_TYPE,
+		.init_phy = imx95_pcie_init_phy,
+	},
 	[IMX8MQ_EP] = {
 		.variant = IMX8MQ_EP,
 		.flags = IMX6_PCIE_FLAG_HAS_APP_RESET |
@@ -1500,6 +1571,7 @@ static const struct of_device_id imx6_pcie_of_match[] = {
 	{ .compatible = "fsl,imx8mq-pcie", .data = &drvdata[IMX8MQ], },
 	{ .compatible = "fsl,imx8mm-pcie", .data = &drvdata[IMX8MM], },
 	{ .compatible = "fsl,imx8mp-pcie", .data = &drvdata[IMX8MP], },
+	{ .compatible = "fsl,imx95-pcie", .data = &drvdata[IMX95], },
 	{ .compatible = "fsl,imx8mq-pcie-ep", .data = &drvdata[IMX8MQ_EP], },
 	{ .compatible = "fsl,imx8mm-pcie-ep", .data = &drvdata[IMX8MM_EP], },
 	{ .compatible = "fsl,imx8mp-pcie-ep", .data = &drvdata[IMX8MP_EP], },
-- 
2.34.1

[PATCH v10 11/14] PCI: imx6: Clean up get addr_space code

[Frank Li]

Since the dw_pcie_ep_init() function is already fetching the 'addr_space'
region, no need to do the same in this driver.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Notes:
    Change from v7 to v8
    - update commit message
    - Add Manivannan Sadhasivam's review tag
    Change from v4 to v7
    - none
    Change from v1 to v3
    - new patches

 drivers/pci/controller/dwc/pci-imx6.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 183bb3b31cf16..9cc6802e93641 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1067,7 +1067,6 @@ static int imx6_add_pcie_ep(struct imx6_pcie *imx6_pcie,
 	int ret;
 	unsigned int pcie_dbi2_offset;
 	struct dw_pcie_ep *ep;
-	struct resource *res;
 	struct dw_pcie *pci = imx6_pcie->pci;
 	struct dw_pcie_rp *pp = &pci->pp;
 	struct device *dev = pci->dev;
@@ -1086,14 +1085,8 @@ static int imx6_add_pcie_ep(struct imx6_pcie *imx6_pcie,
 		pcie_dbi2_offset = SZ_4K;
 		break;
 	}
-	pci->dbi_base2 = pci->dbi_base + pcie_dbi2_offset;
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");
-	if (!res)
-		return -EINVAL;
 
-	ep->phys_base = res->start;
-	ep->addr_size = resource_size(res);
-	ep->page_size = SZ_64K;
+	pci->dbi_base2 = pci->dbi_base + pcie_dbi2_offset;
 
 	ret = dw_pcie_ep_init(ep);
 	if (ret) {
-- 
2.34.1

[PATCH v10 12/14] PCI: imx6: Add epc_features in imx6_pcie_drvdata

[Frank Li]

The i.MX EP exhibits variations in epc_features among different EP
configurations. This introduces the addition of epc_features in
imx6_pcie_drvdata to accommodate these differences. It's important to note
that there are no functional changes in this commit; instead, it lays the
groundwork for supporting i.MX95 EP functions.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Notes:
    Change from v7 to v8
    - Added  Manivannan Sadhasivam's review tag
    Change from v6 to v7
    - none
    Change from v5 to v6
    - add missed maxitems.
    - add comments about reuse linux,pci-domain as controller id.
    linux,pci-domain have not defined at PCI endpoint side.
    
    Change from v1 to v3
    - new patch at v3

 drivers/pci/controller/dwc/pci-imx6.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 9cc6802e93641..c2098e59fde1e 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -105,6 +105,7 @@ struct imx6_pcie_drvdata {
 	const u32 ltssm_mask;
 	const u32 mode_off[IMX6_PCIE_MAX_INSTANCES];
 	const u32 mode_mask[IMX6_PCIE_MAX_INSTANCES];
+	const struct pci_epc_features *epc_features;
 	int (*init_phy)(struct imx6_pcie *pcie);
 };
 
@@ -1052,7 +1053,10 @@ static const struct pci_epc_features imx8m_pcie_epc_features = {
 static const struct pci_epc_features*
 imx6_pcie_ep_get_features(struct dw_pcie_ep *ep)
 {
-	return &imx8m_pcie_epc_features;
+	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+	struct imx6_pcie *imx6_pcie = to_imx6_pcie(pci);
+
+	return imx6_pcie->drvdata->epc_features;
 }
 
 static const struct dw_pcie_ep_ops pcie_ep_ops = {
@@ -1532,6 +1536,7 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
 		.mode_off[1] = IOMUXC_GPR12,
 		.mode_mask[1] = IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE,
+		.epc_features = &imx8m_pcie_epc_features,
 		.init_phy = imx8mq_pcie_init_phy,
 	},
 	[IMX8MM_EP] = {
@@ -1543,6 +1548,7 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
 		.mode_off[0] = IOMUXC_GPR12,
 		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
+		.epc_features = &imx8m_pcie_epc_features,
 	},
 	[IMX8MP_EP] = {
 		.variant = IMX8MP_EP,
@@ -1553,6 +1559,7 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.clks_cnt = ARRAY_SIZE(imx8mm_clks),
 		.mode_off[0] = IOMUXC_GPR12,
 		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
+		.epc_features = &imx8m_pcie_epc_features,
 	},
 };
 
-- 
2.34.1

[PATCH v10 13/14] dt-bindings: imx6q-pcie: Add iMX95 pcie endpoint compatible string

[Frank Li]

Add i.MX95 PCIe "fsl,imx95-pcie-ep" compatible string.
Add reg-name: "atu", "dbi2", "dma" and "app".

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Notes:
    Change from v9 to v10
    - remove "pci-domain" part
    Change from v8 to v9
    - add rob's review tag
    Change from v3 to v8
    - none
    Change from v1 to v3
    - new patches at v3

 .../bindings/pci/fsl,imx6q-pcie-ep.yaml       | 46 +++++++++++++++----
 1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
index ee155ed5f1811..a06f75df8458a 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
@@ -22,14 +22,7 @@ properties:
       - fsl,imx8mm-pcie-ep
       - fsl,imx8mq-pcie-ep
       - fsl,imx8mp-pcie-ep
-
-  reg:
-    minItems: 2
-
-  reg-names:
-    items:
-      - const: dbi
-      - const: addr_space
+      - fsl,imx95-pcie-ep
 
   clocks:
     minItems: 3
@@ -62,11 +55,48 @@ required:
 allOf:
   - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
   - $ref: /schemas/pci/fsl,imx6q-pcie-common.yaml#
+  - if:
+      properties:
+        compatible:
+          enum:
+            - fsl,imx8mm-pcie-ep
+            - fsl,imx8mq-pcie-ep
+            - fsl,imx8mp-pcie-ep
+    then:
+      properties:
+        reg:
+          minItems: 2
+          maxItems: 2
+        reg-names:
+          items:
+            - const: dbi
+            - const: addr_space
+
+  - if:
+      properties:
+        compatible:
+          enum:
+            - fsl,imx95-pcie-ep
+    then:
+      properties:
+        reg:
+          minItems: 6
+          maxItems: 6
+        reg-names:
+          items:
+            - const: dbi
+            - const: atu
+            - const: dbi2
+            - const: app
+            - const: dma
+            - const: addr_space
+
   - if:
       properties:
         compatible:
           enum:
             - fsl,imx8mq-pcie-ep
+            - fsl,imx95-pcie-ep
     then:
       properties:
         clocks:
-- 
2.34.1

[PATCH v10 14/14] PCI: imx6: Add iMX95 Endpoint (EP) support

[Frank Li]

Add iMX95 EP support and add 64bit address support. Internal bus bridge for
PCI support 64bit dma address in iMX95. Hence, call
dma_set_mask_and_coherent() to set 64 bit DMA mask.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Notes:
    Change from v9 to v10
    - using if (device_property_match_string(dev, "reg-names", "dbi2")) instead of
    	if (imx6_pcie->drvdata->variant == IMX95_EP)
    - so other platform can stat use "dbi2" in dts.
    
    @Mani: I change to check device_property_match_string(), so I can update
    other platform dts file, It should be better than
    	imx6_pcie->drvdata->variant == IMX95_EP
    
    Change from v8 to v9
    - update fixme comments
    - update BAR1 comments
    - Add mani's review tag
    Change from v7 to v8
    - Update commit message
    - Using Fixme
    - Update clks_cnts by ARRAY_SIZE
    
    Change from v4 to v7
    - none
    Change from v3 to v4
    - change align to 4k for imx95
    Change from v1 to v3
    - new patches at v3

 drivers/pci/controller/dwc/pci-imx6.c | 47 +++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index c2098e59fde1e..472ff1cc17d2f 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -75,6 +75,7 @@ enum imx6_pcie_variants {
 	IMX8MQ_EP,
 	IMX8MM_EP,
 	IMX8MP_EP,
+	IMX95_EP,
 };
 
 #define IMX6_PCIE_FLAG_IMX6_PHY			BIT(0)
@@ -84,6 +85,7 @@ enum imx6_pcie_variants {
 #define IMX6_PCIE_FLAG_HAS_APP_RESET		BIT(4)
 #define IMX6_PCIE_FLAG_HAS_PHY_RESET		BIT(5)
 #define IMX6_PCIE_FLAG_HAS_SERDES		BIT(6)
+#define IMX6_PCIE_FLAG_SUPPORT_64BIT		BIT(7)
 
 #define imx6_check_flag(pci, val)     (pci->drvdata->flags & val)
 
@@ -616,6 +618,7 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
 		break;
 	case IMX7D:
 	case IMX95:
+	case IMX95_EP:
 		break;
 	case IMX8MM:
 	case IMX8MM_EP:
@@ -1050,6 +1053,23 @@ static const struct pci_epc_features imx8m_pcie_epc_features = {
 	.align = SZ_64K,
 };
 
+/*
+ * BAR#	| Default BAR enable	| Default BAR Type	| Default BAR Size	| BAR Sizing Scheme
+ * ================================================================================================
+ * BAR0	| Enable		| 64-bit		| 1 MB			| Programmable Size
+ * BAR1	| Disable		| 32-bit		| 64 KB			| Fixed Size
+ *        BAR1 should be disabled if BAR0 is 64bit.
+ * BAR2	| Enable		| 32-bit		| 1 MB			| Programmable Size
+ * BAR3	| Enable		| 32-bit		| 64 KB			| Programmable Size
+ * BAR4	| Enable		| 32-bit		| 1M			| Programmable Size
+ * BAR5	| Enable		| 32-bit		| 64 KB			| Programmable Size
+ */
+static const struct pci_epc_features imx95_pcie_epc_features = {
+	.msi_capable = true,
+	.bar_fixed_size[1] = SZ_64K,
+	.align = SZ_4K,
+};
+
 static const struct pci_epc_features*
 imx6_pcie_ep_get_features(struct dw_pcie_ep *ep)
 {
@@ -1092,6 +1112,15 @@ static int imx6_add_pcie_ep(struct imx6_pcie *imx6_pcie,
 
 	pci->dbi_base2 = pci->dbi_base + pcie_dbi2_offset;
 
+	/*
+	 * FIXME: Ideally, dbi2 base address should come from DT. But since only IMX95 is defining
+	 * "dbi2" in DT, "dbi_base2" is set to NULL here for that platform alone so that the DWC
+	 * core code can fetch that from DT. But once all platform DTs were fixed, this and the
+	 * above "dbi_base2" setting should be removed.
+	 */
+	if (device_property_match_string(dev, "reg-names", "dbi2") >= 0)
+		pci->dbi_base2 = NULL;
+
 	ret = dw_pcie_ep_init(ep);
 	if (ret) {
 		dev_err(dev, "failed to initialize endpoint\n");
@@ -1343,6 +1372,9 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 					     "unable to find iomuxc registers\n");
 	}
 
+	if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_SUPPORT_64BIT))
+		dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+
 	/* Grab PCIe PHY Tx Settings */
 	if (of_property_read_u32(node, "fsl,tx-deemph-gen1",
 				 &imx6_pcie->tx_deemph_gen1))
@@ -1561,6 +1593,20 @@ static const struct imx6_pcie_drvdata drvdata[] = {
 		.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
 		.epc_features = &imx8m_pcie_epc_features,
 	},
+	[IMX95_EP] = {
+		.variant = IMX95_EP,
+		.flags = IMX6_PCIE_FLAG_HAS_SERDES |
+			 IMX6_PCIE_FLAG_SUPPORT_64BIT,
+		.clk_names = imx8mq_clks,
+		.clks_cnt = ARRAY_SIZE(imx8mq_clks),
+		.ltssm_off = IMX95_PE0_GEN_CTRL_3,
+		.ltssm_mask = IMX95_PCIE_LTSSM_EN,
+		.mode_off[0]  = IMX95_PE0_GEN_CTRL_1,
+		.mode_mask[0] = IMX95_PCIE_DEVICE_TYPE,
+		.init_phy = imx95_pcie_init_phy,
+		.epc_features = &imx95_pcie_epc_features,
+		.mode = DW_PCIE_EP_TYPE,
+	},
 };
 
 static const struct of_device_id imx6_pcie_of_match[] = {
@@ -1575,6 +1621,7 @@ static const struct of_device_id imx6_pcie_of_match[] = {
 	{ .compatible = "fsl,imx8mq-pcie-ep", .data = &drvdata[IMX8MQ_EP], },
 	{ .compatible = "fsl,imx8mm-pcie-ep", .data = &drvdata[IMX8MM_EP], },
 	{ .compatible = "fsl,imx8mp-pcie-ep", .data = &drvdata[IMX8MP_EP], },
+	{ .compatible = "fsl,imx95-pcie-ep", .data = &drvdata[IMX95_EP], },
 	{},
 };
 
-- 
2.34.1

Re: [PATCH v10 00/14] PCI: imx6: Clean up and add imx95 pci support

[Frank Li]

On Mon, Feb 05, 2024 at 12:33:21PM -0500, Frank Li wrote:
> first 6 patches use drvdata: flags to simplify some switch-case code.
> Improve maintaince and easy to read code.
> 

@Lorenzo Pieralisi:

	Do you have chance to look other patches?
	Mani's apply EP side change. 
	'PCI: imx6: Add iMX95 Endpoint (EP) support' need be rebased. 

Frank

> Then add imx95 basic pci host function.
> 
> follow two patch do endpoint code clean up.
> Then add imx95 basic endpont function.
> 
> Compared with v2, added EP function support and some fixes,  please change
> notes at each patches.
> 
> Change from v9 to v10
> - remove two patches:
> >   dt-bindings: imx6q-pcie: Add linux,pci-domain as required for iMX8MQ
> >   PCI: imx6: Using "linux,pci-domain" as slot ID
> it is not good solution to fixed hardcode check to get controller id.
> Will see better solution later.
> 
> dt-binding pass pcie node:
> 
> pcie0: pcie@4c300000 {
>                         compatible = "fsl,imx95-pcie";
>                         reg = <0 0x4c300000 0 0x40000>,
>                                 <0 0x4c360000 0 0x10000>,
>                                 <0 0x4c340000 0 0x20000>,
>                                 <0 0x60100000 0 0xfe00000>;
>                         reg-names = "dbi", "atu", "app", "config";
>                         #address-cells = <3>;
>                         #size-cells = <2>;
>                         device_type = "pci";
>                         linux,pci-domain = <0>;
>                         bus-range = <0x00 0xff>;
>                         ranges = <0x81000000 0x0 0x00000000 0x0 0x6ff00000 0 0x00100000>,
>                                  <0x82000000 0x0 0x10000000 0x9 0x10000000 0 0x10000000>;
>                         num-lanes = <1>;
>                         num-viewport = <8>;
>                         interrupts = <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>;
>                         interrupt-names = "msi";
>                         #interrupt-cells = <1>;
>                         interrupt-map-mask = <0 0 0 0x7>;
>                         interrupt-map = <0 0 0 1 &gic 0 0 GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
>                                         <0 0 0 2 &gic 0 0 GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>,
>                                         <0 0 0 3 &gic 0 0 GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
>                                         <0 0 0 4 &gic 0 0 GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>;
>                         fsl,max-link-speed = <3>;
>                         clocks = <&scmi_clk IMX95_CLK_HSIO>,
>                                  <&scmi_clk IMX95_CLK_HSIOPLL>,
>                                  <&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
>                                  <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
>                         clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux";
>                         assigned-clocks =<&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
>                                          <&scmi_clk IMX95_CLK_HSIOPLL>,
>                                          <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
>                         assigned-clock-rates = <3600000000>, <100000000>, <10000000>;
>                         assigned-clock-parents = <0>, <0>,
>                                                  <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>;
>                         power-domains = <&scmi_devpd IMX95_PD_HSIO_TOP>;
>                         /* 0x30~0x37 stream id for pci0 */
>                         /*
>                          * iommu-map = <0x000 &apps_smmu 0x30 0x1>,
>                          * <0x100 &apps_smmu 0x31 0x1>;
>                          */
>                         status = "disabled";
>                 };
> 
> pcie1: pcie-ep@4c380000 {
>                         compatible = "fsl,imx95-pcie-ep";
>                         reg = <0 0x4c380000 0 0x20000>,
>                               <0 0x4c3e0000 0 0x1000>,
>                               <0 0x4c3a0000 0 0x1000>,
>                               <0 0x4c3c0000 0 0x10000>,
>                               <0 0x4c3f0000 0 0x10000>,
>                               <0xa 0 1 0>;
>                         reg-names = "dbi", "atu", "dbi2", "app", "dma", "addr_space";
>                         interrupts = <GIC_SPI 317 IRQ_TYPE_LEVEL_HIGH>;
>                         interrupt-names = "dma";
>                         fsl,max-link-speed = <3>;
>                         clocks = <&scmi_clk IMX95_CLK_HSIO>,
>                                  <&scmi_clk IMX95_CLK_HSIOPLL>,
>                                  <&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
>                                  <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
>                         clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux";
>                         assigned-clocks =<&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
>                                          <&scmi_clk IMX95_CLK_HSIOPLL>,
>                                          <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
>                         assigned-clock-rates = <3600000000>, <100000000>, <10000000>;
>                         assigned-clock-parents = <0>, <0>,
>                                                  <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>;
>                         power-domains = <&scmi_devpd IMX95_PD_HSIO_TOP>;
>                         status = "disabled";
>                 };
> 
> Frank Li (13):
>   PCI: imx6: Simplify clock handling by using clk_bulk*() function
>   PCI: imx6: Simplify phy handling by using IMX6_PCIE_FLAG_HAS_PHYDRV
>   PCI: imx6: Simplify reset handling by using by using
>     *_FLAG_HAS_*_RESET
>   PCI: imx6: Simplify ltssm_enable() by using ltssm_off and ltssm_mask
>   PCI: imx6: Simplify configure_type() by using mode_off and mode_mask
>   PCI: imx6: Simplify switch-case logic by involve init_phy callback
>   dt-bindings: imx6q-pcie: Clean up irrationality clocks check
>   dt-bindings: imx6q-pcie: Restruct reg and reg-name
>   PCI: imx6: Add iMX95 PCIe Root Complex support
>   PCI: imx6: Clean up get addr_space code
>   PCI: imx6: Add epc_features in imx6_pcie_drvdata
>   dt-bindings: imx6q-pcie: Add iMX95 pcie endpoint compatible string
>   PCI: imx6: Add iMX95 Endpoint (EP) support
> 
> Richard Zhu (1):
>   dt-bindings: imx6q-pcie: Add imx95 pcie compatible string
> 
>  .../bindings/pci/fsl,imx6q-pcie-common.yaml   |  17 +-
>  .../bindings/pci/fsl,imx6q-pcie-ep.yaml       |  46 +-
>  .../bindings/pci/fsl,imx6q-pcie.yaml          |  49 +-
>  drivers/pci/controller/dwc/pci-imx6.c         | 634 ++++++++++--------
>  4 files changed, 436 insertions(+), 310 deletions(-)
> 
> -- 
> 2.34.1
> 

Re: [PATCH v10 00/14] PCI: imx6: Clean up and add imx95 pci support

[Lorenzo Pieralisi]

On Mon, Feb 19, 2024 at 10:11:45AM -0500, Frank Li wrote:
> On Mon, Feb 05, 2024 at 12:33:21PM -0500, Frank Li wrote:
> > first 6 patches use drvdata: flags to simplify some switch-case code.
> > Improve maintaince and easy to read code.
> > 
> 
> @Lorenzo Pieralisi:
> 
> 	Do you have chance to look other patches?

Yes, they are fine.

> 	Mani's apply EP side change. 
> 	'PCI: imx6: Add iMX95 Endpoint (EP) support' need be rebased. 

What does that mean ? I think it is best to pull the series in a single
branch if there are not any dependencies on other branches.

Thanks,
Lorenzo

> Frank
> 
> > Then add imx95 basic pci host function.
> > 
> > follow two patch do endpoint code clean up.
> > Then add imx95 basic endpont function.
> > 
> > Compared with v2, added EP function support and some fixes,  please change
> > notes at each patches.
> > 
> > Change from v9 to v10
> > - remove two patches:
> > >   dt-bindings: imx6q-pcie: Add linux,pci-domain as required for iMX8MQ
> > >   PCI: imx6: Using "linux,pci-domain" as slot ID
> > it is not good solution to fixed hardcode check to get controller id.
> > Will see better solution later.
> > 
> > dt-binding pass pcie node:
> > 
> > pcie0: pcie@4c300000 {
> >                         compatible = "fsl,imx95-pcie";
> >                         reg = <0 0x4c300000 0 0x40000>,
> >                                 <0 0x4c360000 0 0x10000>,
> >                                 <0 0x4c340000 0 0x20000>,
> >                                 <0 0x60100000 0 0xfe00000>;
> >                         reg-names = "dbi", "atu", "app", "config";
> >                         #address-cells = <3>;
> >                         #size-cells = <2>;
> >                         device_type = "pci";
> >                         linux,pci-domain = <0>;
> >                         bus-range = <0x00 0xff>;
> >                         ranges = <0x81000000 0x0 0x00000000 0x0 0x6ff00000 0 0x00100000>,
> >                                  <0x82000000 0x0 0x10000000 0x9 0x10000000 0 0x10000000>;
> >                         num-lanes = <1>;
> >                         num-viewport = <8>;
> >                         interrupts = <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>;
> >                         interrupt-names = "msi";
> >                         #interrupt-cells = <1>;
> >                         interrupt-map-mask = <0 0 0 0x7>;
> >                         interrupt-map = <0 0 0 1 &gic 0 0 GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
> >                                         <0 0 0 2 &gic 0 0 GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>,
> >                                         <0 0 0 3 &gic 0 0 GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
> >                                         <0 0 0 4 &gic 0 0 GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>;
> >                         fsl,max-link-speed = <3>;
> >                         clocks = <&scmi_clk IMX95_CLK_HSIO>,
> >                                  <&scmi_clk IMX95_CLK_HSIOPLL>,
> >                                  <&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
> >                                  <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
> >                         clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux";
> >                         assigned-clocks =<&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
> >                                          <&scmi_clk IMX95_CLK_HSIOPLL>,
> >                                          <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
> >                         assigned-clock-rates = <3600000000>, <100000000>, <10000000>;
> >                         assigned-clock-parents = <0>, <0>,
> >                                                  <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>;
> >                         power-domains = <&scmi_devpd IMX95_PD_HSIO_TOP>;
> >                         /* 0x30~0x37 stream id for pci0 */
> >                         /*
> >                          * iommu-map = <0x000 &apps_smmu 0x30 0x1>,
> >                          * <0x100 &apps_smmu 0x31 0x1>;
> >                          */
> >                         status = "disabled";
> >                 };
> > 
> > pcie1: pcie-ep@4c380000 {
> >                         compatible = "fsl,imx95-pcie-ep";
> >                         reg = <0 0x4c380000 0 0x20000>,
> >                               <0 0x4c3e0000 0 0x1000>,
> >                               <0 0x4c3a0000 0 0x1000>,
> >                               <0 0x4c3c0000 0 0x10000>,
> >                               <0 0x4c3f0000 0 0x10000>,
> >                               <0xa 0 1 0>;
> >                         reg-names = "dbi", "atu", "dbi2", "app", "dma", "addr_space";
> >                         interrupts = <GIC_SPI 317 IRQ_TYPE_LEVEL_HIGH>;
> >                         interrupt-names = "dma";
> >                         fsl,max-link-speed = <3>;
> >                         clocks = <&scmi_clk IMX95_CLK_HSIO>,
> >                                  <&scmi_clk IMX95_CLK_HSIOPLL>,
> >                                  <&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
> >                                  <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
> >                         clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux";
> >                         assigned-clocks =<&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
> >                                          <&scmi_clk IMX95_CLK_HSIOPLL>,
> >                                          <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
> >                         assigned-clock-rates = <3600000000>, <100000000>, <10000000>;
> >                         assigned-clock-parents = <0>, <0>,
> >                                                  <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>;
> >                         power-domains = <&scmi_devpd IMX95_PD_HSIO_TOP>;
> >                         status = "disabled";
> >                 };
> > 
> > Frank Li (13):
> >   PCI: imx6: Simplify clock handling by using clk_bulk*() function
> >   PCI: imx6: Simplify phy handling by using IMX6_PCIE_FLAG_HAS_PHYDRV
> >   PCI: imx6: Simplify reset handling by using by using
> >     *_FLAG_HAS_*_RESET
> >   PCI: imx6: Simplify ltssm_enable() by using ltssm_off and ltssm_mask
> >   PCI: imx6: Simplify configure_type() by using mode_off and mode_mask
> >   PCI: imx6: Simplify switch-case logic by involve init_phy callback
> >   dt-bindings: imx6q-pcie: Clean up irrationality clocks check
> >   dt-bindings: imx6q-pcie: Restruct reg and reg-name
> >   PCI: imx6: Add iMX95 PCIe Root Complex support
> >   PCI: imx6: Clean up get addr_space code
> >   PCI: imx6: Add epc_features in imx6_pcie_drvdata
> >   dt-bindings: imx6q-pcie: Add iMX95 pcie endpoint compatible string
> >   PCI: imx6: Add iMX95 Endpoint (EP) support
> > 
> > Richard Zhu (1):
> >   dt-bindings: imx6q-pcie: Add imx95 pcie compatible string
> > 
> >  .../bindings/pci/fsl,imx6q-pcie-common.yaml   |  17 +-
> >  .../bindings/pci/fsl,imx6q-pcie-ep.yaml       |  46 +-
> >  .../bindings/pci/fsl,imx6q-pcie.yaml          |  49 +-
> >  drivers/pci/controller/dwc/pci-imx6.c         | 634 ++++++++++--------
> >  4 files changed, 436 insertions(+), 310 deletions(-)
> > 
> > -- 
> > 2.34.1
> > 

Re: [PATCH v10 00/14] PCI: imx6: Clean up and add imx95 pci support

[Frank Li]

On Mon, Feb 19, 2024 at 04:21:26PM +0100, Lorenzo Pieralisi wrote:
> On Mon, Feb 19, 2024 at 10:11:45AM -0500, Frank Li wrote:
> > On Mon, Feb 05, 2024 at 12:33:21PM -0500, Frank Li wrote:
> > > first 6 patches use drvdata: flags to simplify some switch-case code.
> > > Improve maintaince and easy to read code.
> > > 
> > 
> > @Lorenzo Pieralisi:
> > 
> > 	Do you have chance to look other patches?
> 
> Yes, they are fine.
> 
> > 	Mani's apply EP side change. 
> > 	'PCI: imx6: Add iMX95 Endpoint (EP) support' need be rebased. 
> 
> What does that mean ? I think it is best to pull the series in a single
> branch if there are not any dependencies on other branches.

Two options:

Options 1:
	Merge 1-13. Left 'PCI: imx6: Add iMX95 Endpoint (EP) support'.
I will send out later with other clean up patches.

Options 2:
	I rebase to https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=endpoint
	After Bjorn Helgaas merge to next, you can pick all.

I perfer opitons 1.

Frank

> 
> Thanks,
> Lorenzo
> 
> > Frank
> > 
> > > Then add imx95 basic pci host function.
> > > 
> > > follow two patch do endpoint code clean up.
> > > Then add imx95 basic endpont function.
> > > 
> > > Compared with v2, added EP function support and some fixes,  please change
> > > notes at each patches.
> > > 
> > > Change from v9 to v10
> > > - remove two patches:
> > > >   dt-bindings: imx6q-pcie: Add linux,pci-domain as required for iMX8MQ
> > > >   PCI: imx6: Using "linux,pci-domain" as slot ID
> > > it is not good solution to fixed hardcode check to get controller id.
> > > Will see better solution later.
> > > 
> > > dt-binding pass pcie node:
> > > 
> > > pcie0: pcie@4c300000 {
> > >                         compatible = "fsl,imx95-pcie";
> > >                         reg = <0 0x4c300000 0 0x40000>,
> > >                                 <0 0x4c360000 0 0x10000>,
> > >                                 <0 0x4c340000 0 0x20000>,
> > >                                 <0 0x60100000 0 0xfe00000>;
> > >                         reg-names = "dbi", "atu", "app", "config";
> > >                         #address-cells = <3>;
> > >                         #size-cells = <2>;
> > >                         device_type = "pci";
> > >                         linux,pci-domain = <0>;
> > >                         bus-range = <0x00 0xff>;
> > >                         ranges = <0x81000000 0x0 0x00000000 0x0 0x6ff00000 0 0x00100000>,
> > >                                  <0x82000000 0x0 0x10000000 0x9 0x10000000 0 0x10000000>;
> > >                         num-lanes = <1>;
> > >                         num-viewport = <8>;
> > >                         interrupts = <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>;
> > >                         interrupt-names = "msi";
> > >                         #interrupt-cells = <1>;
> > >                         interrupt-map-mask = <0 0 0 0x7>;
> > >                         interrupt-map = <0 0 0 1 &gic 0 0 GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
> > >                                         <0 0 0 2 &gic 0 0 GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>,
> > >                                         <0 0 0 3 &gic 0 0 GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
> > >                                         <0 0 0 4 &gic 0 0 GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>;
> > >                         fsl,max-link-speed = <3>;
> > >                         clocks = <&scmi_clk IMX95_CLK_HSIO>,
> > >                                  <&scmi_clk IMX95_CLK_HSIOPLL>,
> > >                                  <&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
> > >                                  <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
> > >                         clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux";
> > >                         assigned-clocks =<&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
> > >                                          <&scmi_clk IMX95_CLK_HSIOPLL>,
> > >                                          <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
> > >                         assigned-clock-rates = <3600000000>, <100000000>, <10000000>;
> > >                         assigned-clock-parents = <0>, <0>,
> > >                                                  <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>;
> > >                         power-domains = <&scmi_devpd IMX95_PD_HSIO_TOP>;
> > >                         /* 0x30~0x37 stream id for pci0 */
> > >                         /*
> > >                          * iommu-map = <0x000 &apps_smmu 0x30 0x1>,
> > >                          * <0x100 &apps_smmu 0x31 0x1>;
> > >                          */
> > >                         status = "disabled";
> > >                 };
> > > 
> > > pcie1: pcie-ep@4c380000 {
> > >                         compatible = "fsl,imx95-pcie-ep";
> > >                         reg = <0 0x4c380000 0 0x20000>,
> > >                               <0 0x4c3e0000 0 0x1000>,
> > >                               <0 0x4c3a0000 0 0x1000>,
> > >                               <0 0x4c3c0000 0 0x10000>,
> > >                               <0 0x4c3f0000 0 0x10000>,
> > >                               <0xa 0 1 0>;
> > >                         reg-names = "dbi", "atu", "dbi2", "app", "dma", "addr_space";
> > >                         interrupts = <GIC_SPI 317 IRQ_TYPE_LEVEL_HIGH>;
> > >                         interrupt-names = "dma";
> > >                         fsl,max-link-speed = <3>;
> > >                         clocks = <&scmi_clk IMX95_CLK_HSIO>,
> > >                                  <&scmi_clk IMX95_CLK_HSIOPLL>,
> > >                                  <&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
> > >                                  <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
> > >                         clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux";
> > >                         assigned-clocks =<&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
> > >                                          <&scmi_clk IMX95_CLK_HSIOPLL>,
> > >                                          <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
> > >                         assigned-clock-rates = <3600000000>, <100000000>, <10000000>;
> > >                         assigned-clock-parents = <0>, <0>,
> > >                                                  <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>;
> > >                         power-domains = <&scmi_devpd IMX95_PD_HSIO_TOP>;
> > >                         status = "disabled";
> > >                 };
> > > 
> > > Frank Li (13):
> > >   PCI: imx6: Simplify clock handling by using clk_bulk*() function
> > >   PCI: imx6: Simplify phy handling by using IMX6_PCIE_FLAG_HAS_PHYDRV
> > >   PCI: imx6: Simplify reset handling by using by using
> > >     *_FLAG_HAS_*_RESET
> > >   PCI: imx6: Simplify ltssm_enable() by using ltssm_off and ltssm_mask
> > >   PCI: imx6: Simplify configure_type() by using mode_off and mode_mask
> > >   PCI: imx6: Simplify switch-case logic by involve init_phy callback
> > >   dt-bindings: imx6q-pcie: Clean up irrationality clocks check
> > >   dt-bindings: imx6q-pcie: Restruct reg and reg-name
> > >   PCI: imx6: Add iMX95 PCIe Root Complex support
> > >   PCI: imx6: Clean up get addr_space code
> > >   PCI: imx6: Add epc_features in imx6_pcie_drvdata
> > >   dt-bindings: imx6q-pcie: Add iMX95 pcie endpoint compatible string
> > >   PCI: imx6: Add iMX95 Endpoint (EP) support
> > > 
> > > Richard Zhu (1):
> > >   dt-bindings: imx6q-pcie: Add imx95 pcie compatible string
> > > 
> > >  .../bindings/pci/fsl,imx6q-pcie-common.yaml   |  17 +-
> > >  .../bindings/pci/fsl,imx6q-pcie-ep.yaml       |  46 +-
> > >  .../bindings/pci/fsl,imx6q-pcie.yaml          |  49 +-
> > >  drivers/pci/controller/dwc/pci-imx6.c         | 634 ++++++++++--------
> > >  4 files changed, 436 insertions(+), 310 deletions(-)
> > > 
> > > -- 
> > > 2.34.1
> > > 

Re: [PATCH v10 00/14] PCI: imx6: Clean up and add imx95 pci support

[Manivannan Sadhasivam]

On Mon, Feb 19, 2024 at 10:11:45AM -0500, Frank Li wrote:
> On Mon, Feb 05, 2024 at 12:33:21PM -0500, Frank Li wrote:
> > first 6 patches use drvdata: flags to simplify some switch-case code.
> > Improve maintaince and easy to read code.
> > 
> 
> @Lorenzo Pieralisi:
> 
> 	Do you have chance to look other patches?
> 	Mani's apply EP side change. 

Even though the controller is for the endpoint, it is still a controller
driver. So all the patches should go through Lorenzo.

I only merge patches under drivers/pci/endpoint. Hope this clarifies.

- Mani

> 	'PCI: imx6: Add iMX95 Endpoint (EP) support' need be rebased. 
> 
> Frank
> 
> > Then add imx95 basic pci host function.
> > 
> > follow two patch do endpoint code clean up.
> > Then add imx95 basic endpont function.
> > 
> > Compared with v2, added EP function support and some fixes,  please change
> > notes at each patches.
> > 
> > Change from v9 to v10
> > - remove two patches:
> > >   dt-bindings: imx6q-pcie: Add linux,pci-domain as required for iMX8MQ
> > >   PCI: imx6: Using "linux,pci-domain" as slot ID
> > it is not good solution to fixed hardcode check to get controller id.
> > Will see better solution later.
> > 
> > dt-binding pass pcie node:
> > 
> > pcie0: pcie@4c300000 {
> >                         compatible = "fsl,imx95-pcie";
> >                         reg = <0 0x4c300000 0 0x40000>,
> >                                 <0 0x4c360000 0 0x10000>,
> >                                 <0 0x4c340000 0 0x20000>,
> >                                 <0 0x60100000 0 0xfe00000>;
> >                         reg-names = "dbi", "atu", "app", "config";
> >                         #address-cells = <3>;
> >                         #size-cells = <2>;
> >                         device_type = "pci";
> >                         linux,pci-domain = <0>;
> >                         bus-range = <0x00 0xff>;
> >                         ranges = <0x81000000 0x0 0x00000000 0x0 0x6ff00000 0 0x00100000>,
> >                                  <0x82000000 0x0 0x10000000 0x9 0x10000000 0 0x10000000>;
> >                         num-lanes = <1>;
> >                         num-viewport = <8>;
> >                         interrupts = <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>;
> >                         interrupt-names = "msi";
> >                         #interrupt-cells = <1>;
> >                         interrupt-map-mask = <0 0 0 0x7>;
> >                         interrupt-map = <0 0 0 1 &gic 0 0 GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
> >                                         <0 0 0 2 &gic 0 0 GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>,
> >                                         <0 0 0 3 &gic 0 0 GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
> >                                         <0 0 0 4 &gic 0 0 GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>;
> >                         fsl,max-link-speed = <3>;
> >                         clocks = <&scmi_clk IMX95_CLK_HSIO>,
> >                                  <&scmi_clk IMX95_CLK_HSIOPLL>,
> >                                  <&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
> >                                  <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
> >                         clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux";
> >                         assigned-clocks =<&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
> >                                          <&scmi_clk IMX95_CLK_HSIOPLL>,
> >                                          <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
> >                         assigned-clock-rates = <3600000000>, <100000000>, <10000000>;
> >                         assigned-clock-parents = <0>, <0>,
> >                                                  <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>;
> >                         power-domains = <&scmi_devpd IMX95_PD_HSIO_TOP>;
> >                         /* 0x30~0x37 stream id for pci0 */
> >                         /*
> >                          * iommu-map = <0x000 &apps_smmu 0x30 0x1>,
> >                          * <0x100 &apps_smmu 0x31 0x1>;
> >                          */
> >                         status = "disabled";
> >                 };
> > 
> > pcie1: pcie-ep@4c380000 {
> >                         compatible = "fsl,imx95-pcie-ep";
> >                         reg = <0 0x4c380000 0 0x20000>,
> >                               <0 0x4c3e0000 0 0x1000>,
> >                               <0 0x4c3a0000 0 0x1000>,
> >                               <0 0x4c3c0000 0 0x10000>,
> >                               <0 0x4c3f0000 0 0x10000>,
> >                               <0xa 0 1 0>;
> >                         reg-names = "dbi", "atu", "dbi2", "app", "dma", "addr_space";
> >                         interrupts = <GIC_SPI 317 IRQ_TYPE_LEVEL_HIGH>;
> >                         interrupt-names = "dma";
> >                         fsl,max-link-speed = <3>;
> >                         clocks = <&scmi_clk IMX95_CLK_HSIO>,
> >                                  <&scmi_clk IMX95_CLK_HSIOPLL>,
> >                                  <&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
> >                                  <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
> >                         clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux";
> >                         assigned-clocks =<&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
> >                                          <&scmi_clk IMX95_CLK_HSIOPLL>,
> >                                          <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
> >                         assigned-clock-rates = <3600000000>, <100000000>, <10000000>;
> >                         assigned-clock-parents = <0>, <0>,
> >                                                  <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>;
> >                         power-domains = <&scmi_devpd IMX95_PD_HSIO_TOP>;
> >                         status = "disabled";
> >                 };
> > 
> > Frank Li (13):
> >   PCI: imx6: Simplify clock handling by using clk_bulk*() function
> >   PCI: imx6: Simplify phy handling by using IMX6_PCIE_FLAG_HAS_PHYDRV
> >   PCI: imx6: Simplify reset handling by using by using
> >     *_FLAG_HAS_*_RESET
> >   PCI: imx6: Simplify ltssm_enable() by using ltssm_off and ltssm_mask
> >   PCI: imx6: Simplify configure_type() by using mode_off and mode_mask
> >   PCI: imx6: Simplify switch-case logic by involve init_phy callback
> >   dt-bindings: imx6q-pcie: Clean up irrationality clocks check
> >   dt-bindings: imx6q-pcie: Restruct reg and reg-name
> >   PCI: imx6: Add iMX95 PCIe Root Complex support
> >   PCI: imx6: Clean up get addr_space code
> >   PCI: imx6: Add epc_features in imx6_pcie_drvdata
> >   dt-bindings: imx6q-pcie: Add iMX95 pcie endpoint compatible string
> >   PCI: imx6: Add iMX95 Endpoint (EP) support
> > 
> > Richard Zhu (1):
> >   dt-bindings: imx6q-pcie: Add imx95 pcie compatible string
> > 
> >  .../bindings/pci/fsl,imx6q-pcie-common.yaml   |  17 +-
> >  .../bindings/pci/fsl,imx6q-pcie-ep.yaml       |  46 +-
> >  .../bindings/pci/fsl,imx6q-pcie.yaml          |  49 +-
> >  drivers/pci/controller/dwc/pci-imx6.c         | 634 ++++++++++--------
> >  4 files changed, 436 insertions(+), 310 deletions(-)
> > 
> > -- 
> > 2.34.1
> > 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v10 00/14] PCI: imx6: Clean up and add imx95 pci support

[Frank Li]

On Mon, Feb 19, 2024 at 09:42:08PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Feb 19, 2024 at 10:11:45AM -0500, Frank Li wrote:
> > On Mon, Feb 05, 2024 at 12:33:21PM -0500, Frank Li wrote:
> > > first 6 patches use drvdata: flags to simplify some switch-case code.
> > > Improve maintaince and easy to read code.
> > > 
> > 
> > @Lorenzo Pieralisi:
> > 
> > 	Do you have chance to look other patches?
> > 	Mani's apply EP side change. 
> 
> Even though the controller is for the endpoint, it is still a controller
> driver. So all the patches should go through Lorenzo.
> 
> I only merge patches under drivers/pci/endpoint. Hope this clarifies.

Sorry. It confused everyone. My means was that Mani applied Niklas Cassel's
patches, which cause my 14th patch build failure.

I asked if I need update my 14th patch or applied 1-13 only. 

Frank Li

> 
> - Mani
> 
> > 	'PCI: imx6: Add iMX95 Endpoint (EP) support' need be rebased. 
> > 
> > Frank
> > 
> > > Then add imx95 basic pci host function.
> > > 
> > > follow two patch do endpoint code clean up.
> > > Then add imx95 basic endpont function.
> > > 
> > > Compared with v2, added EP function support and some fixes,  please change
> > > notes at each patches.
> > > 
> > > Change from v9 to v10
> > > - remove two patches:
> > > >   dt-bindings: imx6q-pcie: Add linux,pci-domain as required for iMX8MQ
> > > >   PCI: imx6: Using "linux,pci-domain" as slot ID
> > > it is not good solution to fixed hardcode check to get controller id.
> > > Will see better solution later.
> > > 
> > > dt-binding pass pcie node:
> > > 
> > > pcie0: pcie@4c300000 {
> > >                         compatible = "fsl,imx95-pcie";
> > >                         reg = <0 0x4c300000 0 0x40000>,
> > >                                 <0 0x4c360000 0 0x10000>,
> > >                                 <0 0x4c340000 0 0x20000>,
> > >                                 <0 0x60100000 0 0xfe00000>;
> > >                         reg-names = "dbi", "atu", "app", "config";
> > >                         #address-cells = <3>;
> > >                         #size-cells = <2>;
> > >                         device_type = "pci";
> > >                         linux,pci-domain = <0>;
> > >                         bus-range = <0x00 0xff>;
> > >                         ranges = <0x81000000 0x0 0x00000000 0x0 0x6ff00000 0 0x00100000>,
> > >                                  <0x82000000 0x0 0x10000000 0x9 0x10000000 0 0x10000000>;
> > >                         num-lanes = <1>;
> > >                         num-viewport = <8>;
> > >                         interrupts = <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>;
> > >                         interrupt-names = "msi";
> > >                         #interrupt-cells = <1>;
> > >                         interrupt-map-mask = <0 0 0 0x7>;
> > >                         interrupt-map = <0 0 0 1 &gic 0 0 GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
> > >                                         <0 0 0 2 &gic 0 0 GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>,
> > >                                         <0 0 0 3 &gic 0 0 GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
> > >                                         <0 0 0 4 &gic 0 0 GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>;
> > >                         fsl,max-link-speed = <3>;
> > >                         clocks = <&scmi_clk IMX95_CLK_HSIO>,
> > >                                  <&scmi_clk IMX95_CLK_HSIOPLL>,
> > >                                  <&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
> > >                                  <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
> > >                         clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux";
> > >                         assigned-clocks =<&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
> > >                                          <&scmi_clk IMX95_CLK_HSIOPLL>,
> > >                                          <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
> > >                         assigned-clock-rates = <3600000000>, <100000000>, <10000000>;
> > >                         assigned-clock-parents = <0>, <0>,
> > >                                                  <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>;
> > >                         power-domains = <&scmi_devpd IMX95_PD_HSIO_TOP>;
> > >                         /* 0x30~0x37 stream id for pci0 */
> > >                         /*
> > >                          * iommu-map = <0x000 &apps_smmu 0x30 0x1>,
> > >                          * <0x100 &apps_smmu 0x31 0x1>;
> > >                          */
> > >                         status = "disabled";
> > >                 };
> > > 
> > > pcie1: pcie-ep@4c380000 {
> > >                         compatible = "fsl,imx95-pcie-ep";
> > >                         reg = <0 0x4c380000 0 0x20000>,
> > >                               <0 0x4c3e0000 0 0x1000>,
> > >                               <0 0x4c3a0000 0 0x1000>,
> > >                               <0 0x4c3c0000 0 0x10000>,
> > >                               <0 0x4c3f0000 0 0x10000>,
> > >                               <0xa 0 1 0>;
> > >                         reg-names = "dbi", "atu", "dbi2", "app", "dma", "addr_space";
> > >                         interrupts = <GIC_SPI 317 IRQ_TYPE_LEVEL_HIGH>;
> > >                         interrupt-names = "dma";
> > >                         fsl,max-link-speed = <3>;
> > >                         clocks = <&scmi_clk IMX95_CLK_HSIO>,
> > >                                  <&scmi_clk IMX95_CLK_HSIOPLL>,
> > >                                  <&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
> > >                                  <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
> > >                         clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux";
> > >                         assigned-clocks =<&scmi_clk IMX95_CLK_HSIOPLL_VCO>,
> > >                                          <&scmi_clk IMX95_CLK_HSIOPLL>,
> > >                                          <&scmi_clk IMX95_CLK_HSIOPCIEAUX>;
> > >                         assigned-clock-rates = <3600000000>, <100000000>, <10000000>;
> > >                         assigned-clock-parents = <0>, <0>,
> > >                                                  <&scmi_clk IMX95_CLK_SYSPLL1_PFD1_DIV2>;
> > >                         power-domains = <&scmi_devpd IMX95_PD_HSIO_TOP>;
> > >                         status = "disabled";
> > >                 };
> > > 
> > > Frank Li (13):
> > >   PCI: imx6: Simplify clock handling by using clk_bulk*() function
> > >   PCI: imx6: Simplify phy handling by using IMX6_PCIE_FLAG_HAS_PHYDRV
> > >   PCI: imx6: Simplify reset handling by using by using
> > >     *_FLAG_HAS_*_RESET
> > >   PCI: imx6: Simplify ltssm_enable() by using ltssm_off and ltssm_mask
> > >   PCI: imx6: Simplify configure_type() by using mode_off and mode_mask
> > >   PCI: imx6: Simplify switch-case logic by involve init_phy callback
> > >   dt-bindings: imx6q-pcie: Clean up irrationality clocks check
> > >   dt-bindings: imx6q-pcie: Restruct reg and reg-name
> > >   PCI: imx6: Add iMX95 PCIe Root Complex support
> > >   PCI: imx6: Clean up get addr_space code
> > >   PCI: imx6: Add epc_features in imx6_pcie_drvdata
> > >   dt-bindings: imx6q-pcie: Add iMX95 pcie endpoint compatible string
> > >   PCI: imx6: Add iMX95 Endpoint (EP) support
> > > 
> > > Richard Zhu (1):
> > >   dt-bindings: imx6q-pcie: Add imx95 pcie compatible string
> > > 
> > >  .../bindings/pci/fsl,imx6q-pcie-common.yaml   |  17 +-
> > >  .../bindings/pci/fsl,imx6q-pcie-ep.yaml       |  46 +-
> > >  .../bindings/pci/fsl,imx6q-pcie.yaml          |  49 +-
> > >  drivers/pci/controller/dwc/pci-imx6.c         | 634 ++++++++++--------
> > >  4 files changed, 436 insertions(+), 310 deletions(-)
> > > 
> > > -- 
> > > 2.34.1
> > > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்

Re: [PATCH v10 00/14] PCI: imx6: Clean up and add imx95 pci support

[Niklas Cassel]

On Mon, Feb 19, 2024 at 11:18:03AM -0500, Frank Li wrote:
> On Mon, Feb 19, 2024 at 09:42:08PM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Feb 19, 2024 at 10:11:45AM -0500, Frank Li wrote:
> > > On Mon, Feb 05, 2024 at 12:33:21PM -0500, Frank Li wrote:
> > > > first 6 patches use drvdata: flags to simplify some switch-case code.
> > > > Improve maintaince and easy to read code.
> > > > 
> > > 
> > > @Lorenzo Pieralisi:
> > > 
> > > 	Do you have chance to look other patches?
> > > 	Mani's apply EP side change. 
> > 
> > Even though the controller is for the endpoint, it is still a controller
> > driver. So all the patches should go through Lorenzo.
> > 
> > I only merge patches under drivers/pci/endpoint. Hope this clarifies.
> 
> Sorry. It confused everyone. My means was that Mani applied Niklas Cassel's
> patches, which cause my 14th patch build failure.

Hello Frank,

Patch 14, which adds this:

+static const struct pci_epc_features imx95_pcie_epc_features = {
+       .msi_capable = true,
+       .bar_fixed_size[1] = SZ_64K,
+       .align = SZ_4K,
+};


Should, after rebasing on Mani's pci/endpoint branch, instead look like this:

+static const struct pci_epc_features imx95_pcie_epc_features = {
+       .msi_capable = true,
+       .bar[BAR_1] = { .type = BAR_FIXED, .fixed_size = SZ_64K, },
+       .align = SZ_4K,
+};


> 
> I asked if I need update my 14th patch or applied 1-13 only. 

I see, you want the maintainers to apply 1-13, and simply drop patch 14
instead of you sending out a rebased series.

I assume that the maintainers will be fine with your suggested approach.


Kind regards,
Niklas

Re: [PATCH v10 00/14] PCI: imx6: Clean up and add imx95 pci support

[Lorenzo Pieralisi]

On Tue, Feb 20, 2024 at 10:51:17AM +0100, Niklas Cassel wrote:
> On Mon, Feb 19, 2024 at 11:18:03AM -0500, Frank Li wrote:
> > On Mon, Feb 19, 2024 at 09:42:08PM +0530, Manivannan Sadhasivam wrote:
> > > On Mon, Feb 19, 2024 at 10:11:45AM -0500, Frank Li wrote:
> > > > On Mon, Feb 05, 2024 at 12:33:21PM -0500, Frank Li wrote:
> > > > > first 6 patches use drvdata: flags to simplify some switch-case code.
> > > > > Improve maintaince and easy to read code.
> > > > > 
> > > > 
> > > > @Lorenzo Pieralisi:
> > > > 
> > > > 	Do you have chance to look other patches?
> > > > 	Mani's apply EP side change. 
> > > 
> > > Even though the controller is for the endpoint, it is still a controller
> > > driver. So all the patches should go through Lorenzo.
> > > 
> > > I only merge patches under drivers/pci/endpoint. Hope this clarifies.
> > 
> > Sorry. It confused everyone. My means was that Mani applied Niklas Cassel's
> > patches, which cause my 14th patch build failure.
> 
> Hello Frank,
> 
> Patch 14, which adds this:
> 
> +static const struct pci_epc_features imx95_pcie_epc_features = {
> +       .msi_capable = true,
> +       .bar_fixed_size[1] = SZ_64K,
> +       .align = SZ_4K,
> +};
> 
> 
> Should, after rebasing on Mani's pci/endpoint branch, instead look like this:
> 
> +static const struct pci_epc_features imx95_pcie_epc_features = {
> +       .msi_capable = true,
> +       .bar[BAR_1] = { .type = BAR_FIXED, .fixed_size = SZ_64K, },
> +       .align = SZ_4K,
> +};
> 
> 
> > 
> > I asked if I need update my 14th patch or applied 1-13 only. 
> 
> I see, you want the maintainers to apply 1-13, and simply drop patch 14
> instead of you sending out a rebased series.
> 
> I assume that the maintainers will be fine with your suggested approach.

If patch 14 has no dependencies on 1-13 yes; if it does we need to
coordinate the merge between branches in the PCI tree.

Lorenzo

Re: [PATCH v10 00/14] PCI: imx6: Clean up and add imx95 pci support

[Frank Li]

On Tue, Feb 20, 2024 at 11:08:17AM +0100, Lorenzo Pieralisi wrote:
> On Tue, Feb 20, 2024 at 10:51:17AM +0100, Niklas Cassel wrote:
> > On Mon, Feb 19, 2024 at 11:18:03AM -0500, Frank Li wrote:
> > > On Mon, Feb 19, 2024 at 09:42:08PM +0530, Manivannan Sadhasivam wrote:
> > > > On Mon, Feb 19, 2024 at 10:11:45AM -0500, Frank Li wrote:
> > > > > On Mon, Feb 05, 2024 at 12:33:21PM -0500, Frank Li wrote:
> > > > > > first 6 patches use drvdata: flags to simplify some switch-case code.
> > > > > > Improve maintaince and easy to read code.
> > > > > > 
> > > > > 
> > > > > @Lorenzo Pieralisi:
> > > > > 
> > > > > 	Do you have chance to look other patches?
> > > > > 	Mani's apply EP side change. 
> > > > 
> > > > Even though the controller is for the endpoint, it is still a controller
> > > > driver. So all the patches should go through Lorenzo.
> > > > 
> > > > I only merge patches under drivers/pci/endpoint. Hope this clarifies.
> > > 
> > > Sorry. It confused everyone. My means was that Mani applied Niklas Cassel's
> > > patches, which cause my 14th patch build failure.
> > 
> > Hello Frank,
> > 
> > Patch 14, which adds this:
> > 
> > +static const struct pci_epc_features imx95_pcie_epc_features = {
> > +       .msi_capable = true,
> > +       .bar_fixed_size[1] = SZ_64K,
> > +       .align = SZ_4K,
> > +};
> > 
> > 
> > Should, after rebasing on Mani's pci/endpoint branch, instead look like this:
> > 
> > +static const struct pci_epc_features imx95_pcie_epc_features = {
> > +       .msi_capable = true,
> > +       .bar[BAR_1] = { .type = BAR_FIXED, .fixed_size = SZ_64K, },
> > +       .align = SZ_4K,
> > +};
> > 
> > 
> > > 
> > > I asked if I need update my 14th patch or applied 1-13 only. 
> > 
> > I see, you want the maintainers to apply 1-13, and simply drop patch 14
> > instead of you sending out a rebased series.
> > 
> > I assume that the maintainers will be fine with your suggested approach.
> 
> If patch 14 has no dependencies on 1-13 yes; if it does we need to
> coordinate the merge between branches in the PCI tree.

Keep it easy. I rebase to linux-pci/endpoint and v11 patch sent out.
https://lore.kernel.org/imx/20240220161924.3871774-1-Frank.Li@nxp.com/T/#t

Frank

> 
> Lorenzo

Re: [PATCH v10 00/14] PCI: imx6: Clean up and add imx95 pci support

[Frank Li]

On Tue, Feb 20, 2024 at 11:21:28AM -0500, Frank Li wrote:
> On Tue, Feb 20, 2024 at 11:08:17AM +0100, Lorenzo Pieralisi wrote:
> > On Tue, Feb 20, 2024 at 10:51:17AM +0100, Niklas Cassel wrote:
> > > On Mon, Feb 19, 2024 at 11:18:03AM -0500, Frank Li wrote:
> > > > On Mon, Feb 19, 2024 at 09:42:08PM +0530, Manivannan Sadhasivam wrote:
> > > > > On Mon, Feb 19, 2024 at 10:11:45AM -0500, Frank Li wrote:
> > > > > > On Mon, Feb 05, 2024 at 12:33:21PM -0500, Frank Li wrote:
> > > > > > > first 6 patches use drvdata: flags to simplify some switch-case code.
> > > > > > > Improve maintaince and easy to read code.
> > > > > > > 
> > > > > > 
> > > > > > @Lorenzo Pieralisi:
> > > > > > 
> > > > > > 	Do you have chance to look other patches?
> > > > > > 	Mani's apply EP side change. 
> > > > > 
> > > > > Even though the controller is for the endpoint, it is still a controller
> > > > > driver. So all the patches should go through Lorenzo.
> > > > > 
> > > > > I only merge patches under drivers/pci/endpoint. Hope this clarifies.
> > > > 
> > > > Sorry. It confused everyone. My means was that Mani applied Niklas Cassel's
> > > > patches, which cause my 14th patch build failure.
> > > 
> > > Hello Frank,
> > > 
> > > Patch 14, which adds this:
> > > 
> > > +static const struct pci_epc_features imx95_pcie_epc_features = {
> > > +       .msi_capable = true,
> > > +       .bar_fixed_size[1] = SZ_64K,
> > > +       .align = SZ_4K,
> > > +};
> > > 
> > > 
> > > Should, after rebasing on Mani's pci/endpoint branch, instead look like this:
> > > 
> > > +static const struct pci_epc_features imx95_pcie_epc_features = {
> > > +       .msi_capable = true,
> > > +       .bar[BAR_1] = { .type = BAR_FIXED, .fixed_size = SZ_64K, },
> > > +       .align = SZ_4K,
> > > +};
> > > 
> > > 
> > > > 
> > > > I asked if I need update my 14th patch or applied 1-13 only. 
> > > 
> > > I see, you want the maintainers to apply 1-13, and simply drop patch 14
> > > instead of you sending out a rebased series.
> > > 
> > > I assume that the maintainers will be fine with your suggested approach.
> > 
> > If patch 14 has no dependencies on 1-13 yes; if it does we need to
> > coordinate the merge between branches in the PCI tree.
> 
> Keep it easy. I rebase to linux-pci/endpoint and v11 patch sent out.
> https://lore.kernel.org/imx/20240220161924.3871774-1-Frank.Li@nxp.com/T/#t

Bjorn just merge endpoint to next. v11 will work with linux-pci/next.

Frank

> 
> Frank
> 
> > 
> > Lorenzo