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.
@richard and #shenwei
This just precode, please review it as draft ideas.
Frank Li (8):
PCI: imx6: Simplify clock handling by using HAS_CLK_* bitmask
PCI: imx6: Simplify phy handling by using by using
IMX6_PCIE_FLAG_HAS_PHY
PCI: imx6: Simplify reset handling by using by using
*_FLAG_HAS_*_RESET
PCI: imx6: Using "linux,pci-domain" as slot ID
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
PCI: imx6: Add iMX95 PCIe support
Richard Zhu (1):
dt-bindings: imx6q-pcie: Add imx95 pcie compatible string
.../bindings/pci/fsl,imx6q-pcie.yaml | 1 +
drivers/pci/controller/dwc/pci-imx6.c | 513 ++++++++++--------
2 files changed, 302 insertions(+), 212 deletions(-)
--
2.34.1
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.
Signed-off-by: Frank Li <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 115 +++++++++++---------------
1 file changed, 47 insertions(+), 68 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index bcf52aa86462..62d77fabd82a 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -63,6 +63,8 @@ enum imx6_pcie_variants {
#define IMX6_PCIE_FLAG_HAS_CLK_INBOUND_AXI BIT(3)
#define IMX6_PCIE_FLAG_HAS_CLK_AUX BIT(4)
#define IMX6_PCIE_FLAG_HAS_PHY BIT(5)
+#define IMX6_PCIE_FLAG_HAS_APP_RESET BIT(6)
+#define IMX6_PCIE_FLAG_HAS_PHY_RESET BIT(7)
#define imx6_check_flag(pci, val) (pci->drvdata->flags & val)
@@ -696,18 +698,13 @@ static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
{
- switch (imx6_pcie->drvdata->variant) {
- case IMX7D:
- case IMX8MQ:
- case IMX8MQ_EP:
+ if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_PHY_RESET))
reset_control_assert(imx6_pcie->pciephy_reset);
- fallthrough;
- case IMX8MM:
- case IMX8MM_EP:
- case IMX8MP:
- case IMX8MP_EP:
+
+ if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_APP_RESET))
reset_control_assert(imx6_pcie->apps_reset);
- break;
+
+ switch (imx6_pcie->drvdata->variant) {
case IMX6SX:
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
@@ -728,6 +725,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. */
@@ -741,14 +740,11 @@ static int imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
struct dw_pcie *pci = imx6_pcie->pci;
struct device *dev = pci->dev;
- switch (imx6_pcie->drvdata->variant) {
- case IMX8MQ:
- case IMX8MQ_EP:
- reset_control_deassert(imx6_pcie->pciephy_reset);
- break;
- case IMX7D:
+ if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_PHY_RESET))
reset_control_deassert(imx6_pcie->pciephy_reset);
+ switch (imx6_pcie->drvdata->variant) {
+ case IMX7D:
/* 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.
@@ -780,11 +776,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;
}
@@ -831,16 +823,12 @@ 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;
}
+
+ if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_APP_RESET))
+ reset_control_deassert(imx6_pcie->apps_reset);
}
static void imx6_pcie_ltssm_disable(struct device *dev)
@@ -854,16 +842,12 @@ 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;
}
+
+ if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_APP_RESET))
+ reset_control_assert(imx6_pcie->apps_reset);
}
static int imx6_pcie_start_link(struct dw_pcie *pci)
@@ -1335,36 +1319,24 @@ static int imx6_pcie_probe(struct platform_device *pdev)
"failed to get pcie phy\n");
}
- switch (imx6_pcie->drvdata->variant) {
- 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 (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");
+ }
- break;
+ 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 IMX7D:
+ if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
+ imx6_pcie->controller_id = 1;
default:
break;
}
@@ -1492,32 +1464,39 @@ 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",
},
[IMX8MQ] = {
.variant = IMX8MQ,
- .flags = IMX6_PCIE_FLAG_HAS_CLK_AUX,
+ .flags = IMX6_PCIE_FLAG_HAS_CLK_AUX |
+ IMX6_PCIE_FLAG_HAS_APP_RESET |
+ IMX6_PCIE_FLAG_HAS_PHY_RESET,
.gpr = "fsl,imx8mq-iomuxc-gpr",
},
[IMX8MM] = {
.variant = IMX8MM,
.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND |
IMX6_PCIE_FLAG_HAS_CLK_AUX |
- IMX6_PCIE_FLAG_HAS_PHY,
+ IMX6_PCIE_FLAG_HAS_PHY |
+ IMX6_PCIE_FLAG_HAS_APP_RESET,
.gpr = "fsl,imx8mm-iomuxc-gpr",
},
[IMX8MP] = {
.variant = IMX8MP,
.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND |
IMX6_PCIE_FLAG_HAS_CLK_AUX |
- IMX6_PCIE_FLAG_HAS_PHY,
+ IMX6_PCIE_FLAG_HAS_PHY |
+ IMX6_PCIE_FLAG_HAS_APP_RESET,
.gpr = "fsl,imx8mp-iomuxc-gpr",
},
[IMX8MQ_EP] = {
.variant = IMX8MQ_EP,
.flags = IMX6_PCIE_FLAG_HAS_CLK_AUX |
- IMX6_PCIE_FLAG_HAS_PHY,
+ IMX6_PCIE_FLAG_HAS_PHY |
+ IMX6_PCIE_FLAG_HAS_PHY_RESET,
.mode = DW_PCIE_EP_TYPE,
.gpr = "fsl,imx8mq-iomuxc-gpr",
},
--
2.34.1
Add drvdata::ltssm_off and drvdata::ltssm_mask to simple
imx6_pcie_ltssm_enable(disable)() logic.
Signed-off-by: Frank Li <[email protected]>
---
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 239ef439ba70..334e714a7e96 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -75,6 +75,8 @@ struct imx6_pcie_drvdata {
u32 flags;
int dbi_length;
const char *gpr;
+ const u32 ltssm_off;
+ const u32 ltssm_mask;
};
struct imx6_pcie {
@@ -815,18 +817,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);
if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_APP_RESET))
reset_control_deassert(imx6_pcie->apps_reset);
@@ -835,17 +830,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);
if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_APP_RESET))
reset_control_assert(imx6_pcie->apps_reset);
@@ -1451,6 +1440,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE,
.dbi_length = 0x200,
.gpr = "fsl,imx6q-iomuxc-gpr",
+ .ltssm_off = IOMUXC_GPR12,
+ .ltssm_mask = IMX6Q_GPR12_PCIE_CTL_2,
},
[IMX6SX] = {
.variant = IMX6SX,
@@ -1459,6 +1450,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
IMX6_PCIE_FLAG_SUPPORTS_SUSPEND |
IMX6_PCIE_FLAG_HAS_CLK_INBOUND_AXI,
.gpr = "fsl,imx6q-iomuxc-gpr",
+ .ltssm_off = IOMUXC_GPR12,
+ .ltssm_mask = IMX6Q_GPR12_PCIE_CTL_2,
},
[IMX6QP] = {
.variant = IMX6QP,
@@ -1467,6 +1460,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
.dbi_length = 0x200,
.gpr = "fsl,imx6q-iomuxc-gpr",
+ .ltssm_off = IOMUXC_GPR12,
+ .ltssm_mask = IMX6Q_GPR12_PCIE_CTL_2,
},
[IMX7D] = {
.variant = IMX7D,
--
2.34.1
Add drvdata::mode_off and drvdata::mode_mask to simple
imx6_pcie_configure_type() logic.
Signed-off-by: Frank Li <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 60 ++++++++++++++++++---------
1 file changed, 40 insertions(+), 20 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 334e714a7e96..a2a3496fe8a0 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -69,6 +69,7 @@ enum imx6_pcie_variants {
#define imx6_check_flag(pci, val) (pci->drvdata->flags & val)
+#define IMX6_PCIE_MAX_INSTANCES 2
struct imx6_pcie_drvdata {
enum imx6_pcie_variants variant;
enum dw_pcie_device_mode mode;
@@ -77,6 +78,8 @@ struct imx6_pcie_drvdata {
const char *gpr;
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 {
@@ -177,32 +180,25 @@ 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];
+ /* FIELD_PREP mask have been constant */
+ val = mode << __bf_shf(mask);
- 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)
@@ -1442,6 +1438,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
.gpr = "fsl,imx6q-iomuxc-gpr",
.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,
@@ -1452,6 +1450,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
.gpr = "fsl,imx6q-iomuxc-gpr",
.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,
@@ -1462,6 +1462,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
.gpr = "fsl,imx6q-iomuxc-gpr",
.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,
@@ -1469,6 +1471,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
IMX6_PCIE_FLAG_HAS_APP_RESET |
IMX6_PCIE_FLAG_HAS_PHY_RESET,
.gpr = "fsl,imx7d-iomuxc-gpr",
+ .mode_off[0] = IOMUXC_GPR12,
+ .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
},
[IMX8MQ] = {
.variant = IMX8MQ,
@@ -1476,6 +1480,10 @@ static const struct imx6_pcie_drvdata drvdata[] = {
IMX6_PCIE_FLAG_HAS_APP_RESET |
IMX6_PCIE_FLAG_HAS_PHY_RESET,
.gpr = "fsl,imx8mq-iomuxc-gpr",
+ .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,
@@ -1484,6 +1492,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
IMX6_PCIE_FLAG_HAS_PHY |
IMX6_PCIE_FLAG_HAS_APP_RESET,
.gpr = "fsl,imx8mm-iomuxc-gpr",
+ .mode_off[0] = IOMUXC_GPR12,
+ .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
},
[IMX8MP] = {
.variant = IMX8MP,
@@ -1492,6 +1502,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
IMX6_PCIE_FLAG_HAS_PHY |
IMX6_PCIE_FLAG_HAS_APP_RESET,
.gpr = "fsl,imx8mp-iomuxc-gpr",
+ .mode_off[0] = IOMUXC_GPR12,
+ .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
},
[IMX8MQ_EP] = {
.variant = IMX8MQ_EP,
@@ -1500,6 +1512,10 @@ static const struct imx6_pcie_drvdata drvdata[] = {
IMX6_PCIE_FLAG_HAS_PHY_RESET,
.mode = DW_PCIE_EP_TYPE,
.gpr = "fsl,imx8mq-iomuxc-gpr",
+ .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,
@@ -1507,12 +1523,16 @@ static const struct imx6_pcie_drvdata drvdata[] = {
IMX6_PCIE_FLAG_HAS_PHY,
.mode = DW_PCIE_EP_TYPE,
.gpr = "fsl,imx8mm-iomuxc-gpr",
+ .mode_off[0] = IOMUXC_GPR12,
+ .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
},
[IMX8MP_EP] = {
.variant = IMX8MP_EP,
.flags = IMX6_PCIE_FLAG_HAS_CLK_AUX,
.mode = DW_PCIE_EP_TYPE,
.gpr = "fsl,imx8mp-iomuxc-gpr",
+ .mode_off[0] = IOMUXC_GPR12,
+ .mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
},
};
--
2.34.1
Refactors the phy handling logic in the imx6 PCI driver by adding
IMX6_PCIE_FLAG_HAS_PHY bitmask define for drvdata::flags.
The drvdata::flags and a bitmask ensures a cleaner and more scalable
switch-case structure for handling phy.
Signed-off-by: Frank Li <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 8a9b527934f8..bcf52aa86462 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -62,6 +62,7 @@ enum imx6_pcie_variants {
#define IMX6_PCIE_FLAG_SUPPORTS_SUSPEND BIT(2)
#define IMX6_PCIE_FLAG_HAS_CLK_INBOUND_AXI BIT(3)
#define IMX6_PCIE_FLAG_HAS_CLK_AUX BIT(4)
+#define IMX6_PCIE_FLAG_HAS_PHY BIT(5)
#define imx6_check_flag(pci, val) (pci->drvdata->flags & val)
@@ -1327,6 +1328,13 @@ static int imx6_pcie_probe(struct platform_device *pdev)
"pcie_aux clock source missing or invalid\n");
}
+ if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_PHY)) {
+ 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 IMX7D:
if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
@@ -1356,11 +1364,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;
@@ -1500,24 +1503,28 @@ static const struct imx6_pcie_drvdata drvdata[] = {
[IMX8MM] = {
.variant = IMX8MM,
.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND |
- IMX6_PCIE_FLAG_HAS_CLK_AUX,
+ IMX6_PCIE_FLAG_HAS_CLK_AUX |
+ IMX6_PCIE_FLAG_HAS_PHY,
.gpr = "fsl,imx8mm-iomuxc-gpr",
},
[IMX8MP] = {
.variant = IMX8MP,
.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND |
- IMX6_PCIE_FLAG_HAS_CLK_AUX,
+ IMX6_PCIE_FLAG_HAS_CLK_AUX |
+ IMX6_PCIE_FLAG_HAS_PHY,
.gpr = "fsl,imx8mp-iomuxc-gpr",
},
[IMX8MQ_EP] = {
.variant = IMX8MQ_EP,
- .flags = IMX6_PCIE_FLAG_HAS_CLK_AUX,
+ .flags = IMX6_PCIE_FLAG_HAS_CLK_AUX |
+ IMX6_PCIE_FLAG_HAS_PHY,
.mode = DW_PCIE_EP_TYPE,
.gpr = "fsl,imx8mq-iomuxc-gpr",
},
[IMX8MM_EP] = {
.variant = IMX8MM_EP,
- .flags = IMX6_PCIE_FLAG_HAS_CLK_AUX,
+ .flags = IMX6_PCIE_FLAG_HAS_CLK_AUX |
+ IMX6_PCIE_FLAG_HAS_PHY,
.mode = DW_PCIE_EP_TYPE,
.gpr = "fsl,imx8mm-iomuxc-gpr",
},
--
2.34.1
Avoid use get slot id by compared with register physical address. If there
are more than 2 slots, compared logic will become complex.
Signed-off-by: Frank Li <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 62d77fabd82a..239ef439ba70 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -33,6 +33,7 @@
#include <linux/pm_domain.h>
#include <linux/pm_runtime.h>
+#include "../../pci.h"
#include "pcie-designware.h"
#define IMX8MQ_GPR_PCIE_REF_USE_PAD BIT(9)
@@ -1333,6 +1334,11 @@ static int imx6_pcie_probe(struct platform_device *pdev)
"Failed to get PCIEPHY reset control\n");
}
+ /* Using linux,pci-domain as PCI slot id */
+ imx6_pcie->controller_id = of_get_pci_domain_nr(node);
+ if (imx6_pcie->controller_id)
+ imx6_pcie->controller_id = 0;
+
switch (imx6_pcie->drvdata->variant) {
case IMX7D:
if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
--
2.34.1
Add iMX95 PCIe basic root complex function support.
Signed-off-by: Frank Li <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 85 +++++++++++++++++++++++++--
1 file changed, 80 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 990b91534ea9..7e51aef97baa 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -43,6 +43,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 {
@@ -53,6 +72,7 @@ enum imx6_pcie_variants {
IMX8MQ,
IMX8MM,
IMX8MP,
+ IMX95,
IMX8MQ_EP,
IMX8MM_EP,
IMX8MP_EP,
@@ -79,6 +99,7 @@ struct imx6_pcie_drvdata {
u32 flags;
int dbi_length;
const char *gpr;
+ u32 pf_lut_offset;
const u32 ltssm_off;
const u32 ltssm_mask;
const u32 mode_off[IMX6_PCIE_MAX_INSTANCES];
@@ -182,6 +203,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;
@@ -589,6 +628,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:
@@ -735,11 +775,20 @@ static int imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
{
struct dw_pcie *pci = imx6_pcie->pci;
struct device *dev = pci->dev;
+ u32 val;
if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_PHY_RESET))
reset_control_deassert(imx6_pcie->pciephy_reset);
switch (imx6_pcie->drvdata->variant) {
+ case IMX95:
+ /* Polling the MPLL_STATE */
+ if (regmap_read_poll_timeout(imx6_pcie->iomuxc_gpr,
+ IMX95_PCIE_PHY_MPLLA_CTRL, val,
+ val & IMX95_PCIE_PHY_MPLL_STATE,
+ 10, 10000))
+ dev_err(dev, "PCIe PLL lock timeout\n");
+ break;
case IMX7D:
/* Workaround for ERR010728, failure of PCI-e PLL VCO to
* oscillate, especially when cold. This turns off "Duty-cycle
@@ -1348,12 +1397,27 @@ 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_pcie->drvdata->pf_lut_offset) {
+ void __iomem *off = pci->dbi_base + imx6_pcie->drvdata->pf_lut_offset;
+ static struct regmap_config regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ };
+
+ imx6_pcie->iomuxc_gpr = devm_regmap_init_mmio(dev, off, ®map_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 */
@@ -1511,6 +1575,16 @@ static const struct imx6_pcie_drvdata drvdata[] = {
.mode_off[0] = IOMUXC_GPR12,
.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
},
+ [IMX95] = {
+ .variant = IMX8MP,
+ .flags = IMX6_PCIE_FLAG_HAS_CLK_AUX,
+ .pf_lut_offset = 0x40000,
+ .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_CLK_AUX |
@@ -1551,6 +1625,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
Refactors the clock handling logic in the imx6 PCI driver by adding
HAS_CLK_* bitmask define for drvdata::flags . Simplifies the code and makes
it more maintainable, as future additions of SOC support will only require
straightforward changes. The drvdata::flags and a bitmask ensures a cleaner
and more scalable switch-case structure for handling clocks.
Signed-off-by: Frank Li <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 84 +++++++++++++--------------
1 file changed, 42 insertions(+), 42 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 74703362aeec..8a9b527934f8 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -60,6 +60,10 @@ 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_CLK_INBOUND_AXI BIT(3)
+#define IMX6_PCIE_FLAG_HAS_CLK_AUX BIT(4)
+
+#define imx6_check_flag(pci, val) (pci->drvdata->flags & val)
struct imx6_pcie_drvdata {
enum imx6_pcie_variants variant;
@@ -550,19 +554,23 @@ 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:
+ if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_CLK_INBOUND_AXI)) {
ret = clk_prepare_enable(imx6_pcie->pcie_inbound_axi);
- if (ret) {
- dev_err(dev, "unable to enable pcie_axi clock\n");
- break;
- }
+ if (ret)
+ return ret;
+ }
+ if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_CLK_AUX)) {
+ ret = clk_prepare_enable(imx6_pcie->pcie_aux);
+ if (ret)
+ return ret;
+ }
+
+ switch (imx6_pcie->drvdata->variant) {
+ case IMX6SX:
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
IMX6SX_GPR12_PCIE_TEST_POWERDOWN, 0);
break;
@@ -589,12 +597,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
@@ -614,10 +616,13 @@ 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:
+ if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_CLK_INBOUND_AXI))
clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
- break;
+
+ if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_CLK_AUX))
+ clk_disable_unprepare(imx6_pcie->pcie_aux);
+
+ switch (imx6_pcie->drvdata->variant) {
case IMX6QP:
case IMX6Q:
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
@@ -631,14 +636,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;
}
@@ -1316,21 +1313,21 @@ static int imx6_pcie_probe(struct platform_device *pdev)
return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie),
"pcie clock source missing or invalid\n");
- switch (imx6_pcie->drvdata->variant) {
- case IMX6SX:
- imx6_pcie->pcie_inbound_axi = devm_clk_get(dev,
- "pcie_inbound_axi");
+ if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_CLK_INBOUND_AXI)) {
+ 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:
+ dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_inbound_axi),
+ "pcie_inbound_axi clock missing or invalid\n");
+ }
+
+ if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_CLK_AUX)) {
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;
+ }
+
+ switch (imx6_pcie->drvdata->variant) {
case IMX7D:
if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
imx6_pcie->controller_id = 1;
@@ -1353,10 +1350,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))
@@ -1482,7 +1475,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
.variant = IMX6SX,
.flags = IMX6_PCIE_FLAG_IMX6_PHY |
IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE |
- IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
+ IMX6_PCIE_FLAG_SUPPORTS_SUSPEND |
+ IMX6_PCIE_FLAG_HAS_CLK_INBOUND_AXI,
.gpr = "fsl,imx6q-iomuxc-gpr",
},
[IMX6QP] = {
@@ -1500,30 +1494,36 @@ static const struct imx6_pcie_drvdata drvdata[] = {
},
[IMX8MQ] = {
.variant = IMX8MQ,
+ .flags = IMX6_PCIE_FLAG_HAS_CLK_AUX,
.gpr = "fsl,imx8mq-iomuxc-gpr",
},
[IMX8MM] = {
.variant = IMX8MM,
- .flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
+ .flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND |
+ IMX6_PCIE_FLAG_HAS_CLK_AUX,
.gpr = "fsl,imx8mm-iomuxc-gpr",
},
[IMX8MP] = {
.variant = IMX8MP,
- .flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
+ .flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND |
+ IMX6_PCIE_FLAG_HAS_CLK_AUX,
.gpr = "fsl,imx8mp-iomuxc-gpr",
},
[IMX8MQ_EP] = {
.variant = IMX8MQ_EP,
+ .flags = IMX6_PCIE_FLAG_HAS_CLK_AUX,
.mode = DW_PCIE_EP_TYPE,
.gpr = "fsl,imx8mq-iomuxc-gpr",
},
[IMX8MM_EP] = {
.variant = IMX8MM_EP,
+ .flags = IMX6_PCIE_FLAG_HAS_CLK_AUX,
.mode = DW_PCIE_EP_TYPE,
.gpr = "fsl,imx8mm-iomuxc-gpr",
},
[IMX8MP_EP] = {
.variant = IMX8MP_EP,
+ .flags = IMX6_PCIE_FLAG_HAS_CLK_AUX,
.mode = DW_PCIE_EP_TYPE,
.gpr = "fsl,imx8mp-iomuxc-gpr",
},
--
2.34.1
From: Richard Zhu <[email protected]>
Add i.MX95 PCIe "fsl,imx95-pcie" compatible string.
Signed-off-by: Richard Zhu <[email protected]>
---
Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
index 81bbb8728f0f..723335767843 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
reg:
items:
--
2.34.1
Add drvdata::init_phy() callback function, so difference SOC choose
difference callback function to simple switch-case logic.
Signed-off-by: Frank Li <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 135 ++++++++++++++------------
1 file changed, 71 insertions(+), 64 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index a2a3496fe8a0..990b91534ea9 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -70,6 +70,9 @@ enum imx6_pcie_variants {
#define imx6_check_flag(pci, val) (pci->drvdata->flags & val)
#define IMX6_PCIE_MAX_INSTANCES 2
+
+struct imx6_pcie;
+
struct imx6_pcie_drvdata {
enum imx6_pcie_variants variant;
enum dw_pcie_device_mode mode;
@@ -80,6 +83,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 {
@@ -326,76 +330,69 @@ 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,
+ IMX8MQ_GPR_PCIE_VREG_BYPASS,
+ 0);
+
+ return 0;
+}
+
+static int imx7d_pcie_init_phy(struct imx6_pcie *imx6_pcie)
+{
+ return 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,
+}
+
+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)
@@ -945,7 +942,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) {
@@ -1440,6 +1441,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,
@@ -1452,6 +1454,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,
@@ -1464,6 +1467,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,
@@ -1473,6 +1477,7 @@ static const struct imx6_pcie_drvdata drvdata[] = {
.gpr = "fsl,imx7d-iomuxc-gpr",
.mode_off[0] = IOMUXC_GPR12,
.mode_mask[0] = IMX6Q_GPR12_DEVICE_TYPE,
+ .init_phy = imx7d_pcie_init_phy,
},
[IMX8MQ] = {
.variant = IMX8MQ,
@@ -1484,6 +1489,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,
@@ -1516,6 +1522,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
In subject, maybe you mean "Use 'linux,pci-domain' as slot ID"?
"Using" is the wrong verb form here.
On Wed, Dec 06, 2023 at 10:58:58AM -0500, Frank Li wrote:
> Avoid use get slot id by compared with register physical address. If there
> are more than 2 slots, compared logic will become complex.
But this doesn't say anything about "linux,pci-domain", and I don't
see anything about a register physical address in the patch.
Maybe this commit log was meant for a different patch? I'm confused.
> Signed-off-by: Frank Li <[email protected]>
> ---
> drivers/pci/controller/dwc/pci-imx6.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 62d77fabd82a..239ef439ba70 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -33,6 +33,7 @@
> #include <linux/pm_domain.h>
> #include <linux/pm_runtime.h>
>
> +#include "../../pci.h"
> #include "pcie-designware.h"
>
> #define IMX8MQ_GPR_PCIE_REF_USE_PAD BIT(9)
> @@ -1333,6 +1334,11 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> "Failed to get PCIEPHY reset control\n");
> }
>
> + /* Using linux,pci-domain as PCI slot id */
> + imx6_pcie->controller_id = of_get_pci_domain_nr(node);
> + if (imx6_pcie->controller_id)
> + imx6_pcie->controller_id = 0;
I don't understand what this is doing. It looks the same as just:
imx6_pcie->controller_id = 0;
Maybe this is a typo? As written, it doesn't look like there's any
point in calling of_get_pci_domain_nr().
> switch (imx6_pcie->drvdata->variant) {
> case IMX7D:
> if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
> --
> 2.34.1
>
On Wed, Dec 06, 2023 at 10:36:56AM -0600, Bjorn Helgaas wrote:
> In subject, maybe you mean "Use 'linux,pci-domain' as slot ID"?
> "Using" is the wrong verb form here.
>
> On Wed, Dec 06, 2023 at 10:58:58AM -0500, Frank Li wrote:
> > Avoid use get slot id by compared with register physical address. If there
> > are more than 2 slots, compared logic will become complex.
>
> But this doesn't say anything about "linux,pci-domain", and I don't
> see anything about a register physical address in the patch.
>
> Maybe this commit log was meant for a different patch? I'm confused.
>
> > Signed-off-by: Frank Li <[email protected]>
> > ---
> > drivers/pci/controller/dwc/pci-imx6.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > index 62d77fabd82a..239ef439ba70 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -33,6 +33,7 @@
> > #include <linux/pm_domain.h>
> > #include <linux/pm_runtime.h>
> >
> > +#include "../../pci.h"
> > #include "pcie-designware.h"
> >
> > #define IMX8MQ_GPR_PCIE_REF_USE_PAD BIT(9)
> > @@ -1333,6 +1334,11 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> > "Failed to get PCIEPHY reset control\n");
> > }
> >
> > + /* Using linux,pci-domain as PCI slot id */
> > + imx6_pcie->controller_id = of_get_pci_domain_nr(node);
> > + if (imx6_pcie->controller_id)
> > + imx6_pcie->controller_id = 0;
>
> I don't understand what this is doing. It looks the same as just:
Good capture. It should be
if (imx6_pcie->controller_id < 0)
imx6_pcie->controller_id = 0;
for only one PCI controller case. I just tested first one slot before send
patch, so not met problem.
Previously, we use below logic
if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
imx6_pcie->controller_id = 1;
It is not good to depend on register's base address. If there are 3
controllers, check logic will becomoe ugly.
Frank
>
> imx6_pcie->controller_id = 0;
>
> Maybe this is a typo? As written, it doesn't look like there's any
> point in calling of_get_pci_domain_nr().
>
> > switch (imx6_pcie->drvdata->variant) {
> > case IMX7D:
> > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
> > --
> > 2.34.1
> >
Hi Frank,
On Mi, 2023-12-06 at 10:58 -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.
>
> Signed-off-by: Frank Li <[email protected]>
> ---
> drivers/pci/controller/dwc/pci-imx6.c | 115 +++++++++++---------------
> 1 file changed, 47 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index bcf52aa86462..62d77fabd82a 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
[...]
> @@ -696,18 +698,13 @@ static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
>
> static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> {
> - switch (imx6_pcie->drvdata->variant) {
> - case IMX7D:
> - case IMX8MQ:
> - case IMX8MQ_EP:
> + if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_PHY_RESET))
This check is not strictly necessary. If the flag is not set,
imx6_pcie->pciephy_reset is guaranteed to be NULL, and then
reset_control_assert() is a no-op. Same for the other (de)assert
calls below.
[...]
> @@ -1335,36 +1319,24 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> "failed to get pcie phy\n");
> }
>
> - switch (imx6_pcie->drvdata->variant) {
> - 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 (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_APP_RESET)) {
> + imx6_pcie->apps_reset = devm_reset_control_get_exclusive(dev, "apps");
[...]
I wonder whether we should just defer the check whether apps/pciephy
resets should be used to the device tree validation, and make this an
unconditional call to get an optional reset control:
imx6_pcie->apps_reset = devm_reset_control_get_optional_exclusive(dev, "apps");
regards
Philipp
On Wed, Dec 06, 2023 at 11:50:16AM -0500, Frank Li wrote:
> On Wed, Dec 06, 2023 at 10:36:56AM -0600, Bjorn Helgaas wrote:
> > In subject, maybe you mean "Use 'linux,pci-domain' as slot ID"?
> > "Using" is the wrong verb form here.
> >
> > On Wed, Dec 06, 2023 at 10:58:58AM -0500, Frank Li wrote:
> > > Avoid use get slot id by compared with register physical address. If there
> > > are more than 2 slots, compared logic will become complex.
> >
> > But this doesn't say anything about "linux,pci-domain", and I don't
> > see anything about a register physical address in the patch.
> >
> > Maybe this commit log was meant for a different patch? I'm confused.
> >
> > > Signed-off-by: Frank Li <[email protected]>
> > > ---
> > > drivers/pci/controller/dwc/pci-imx6.c | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > > index 62d77fabd82a..239ef439ba70 100644
> > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > @@ -33,6 +33,7 @@
> > > #include <linux/pm_domain.h>
> > > #include <linux/pm_runtime.h>
> > >
> > > +#include "../../pci.h"
> > > #include "pcie-designware.h"
> > >
> > > #define IMX8MQ_GPR_PCIE_REF_USE_PAD BIT(9)
> > > @@ -1333,6 +1334,11 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> > > "Failed to get PCIEPHY reset control\n");
> > > }
> > >
> > > + /* Using linux,pci-domain as PCI slot id */
> > > + imx6_pcie->controller_id = of_get_pci_domain_nr(node);
> > > + if (imx6_pcie->controller_id)
> > > + imx6_pcie->controller_id = 0;
> >
> > I don't understand what this is doing. It looks the same as just:
>
> Good capture. It should be
> if (imx6_pcie->controller_id < 0)
> imx6_pcie->controller_id = 0;
>
> for only one PCI controller case. I just tested first one slot before send
> patch, so not met problem.
>
> Previously, we use below logic
> if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
> imx6_pcie->controller_id = 1;
>
> It is not good to depend on register's base address. If there are 3
> controllers, check logic will becomoe ugly.
Makes sense. If the previous code depended on the base address, this
patch would make more sense if it contained both the addition of the
of_get_pci_domain_nr() call and the removal of the base address code.
> > Maybe this is a typo? As written, it doesn't look like there's any
> > point in calling of_get_pci_domain_nr().
> >
> > > switch (imx6_pcie->drvdata->variant) {
> > > case IMX7D:
> > > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
> > > --
> > > 2.34.1
> > >
On Wed, Dec 06, 2023 at 10:59:53AM -0600, Bjorn Helgaas wrote:
> On Wed, Dec 06, 2023 at 11:50:16AM -0500, Frank Li wrote:
> > On Wed, Dec 06, 2023 at 10:36:56AM -0600, Bjorn Helgaas wrote:
> > > In subject, maybe you mean "Use 'linux,pci-domain' as slot ID"?
> > > "Using" is the wrong verb form here.
> > >
> > > On Wed, Dec 06, 2023 at 10:58:58AM -0500, Frank Li wrote:
> > > > Avoid use get slot id by compared with register physical address. If there
> > > > are more than 2 slots, compared logic will become complex.
> > >
> > > But this doesn't say anything about "linux,pci-domain", and I don't
> > > see anything about a register physical address in the patch.
> > >
> > > Maybe this commit log was meant for a different patch? I'm confused.
> > >
> > > > Signed-off-by: Frank Li <[email protected]>
> > > > ---
> > > > drivers/pci/controller/dwc/pci-imx6.c | 6 ++++++
> > > > 1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > > > index 62d77fabd82a..239ef439ba70 100644
> > > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > > @@ -33,6 +33,7 @@
> > > > #include <linux/pm_domain.h>
> > > > #include <linux/pm_runtime.h>
> > > >
> > > > +#include "../../pci.h"
> > > > #include "pcie-designware.h"
> > > >
> > > > #define IMX8MQ_GPR_PCIE_REF_USE_PAD BIT(9)
> > > > @@ -1333,6 +1334,11 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> > > > "Failed to get PCIEPHY reset control\n");
> > > > }
> > > >
> > > > + /* Using linux,pci-domain as PCI slot id */
> > > > + imx6_pcie->controller_id = of_get_pci_domain_nr(node);
> > > > + if (imx6_pcie->controller_id)
> > > > + imx6_pcie->controller_id = 0;
> > >
> > > I don't understand what this is doing. It looks the same as just:
> >
> > Good capture. It should be
> > if (imx6_pcie->controller_id < 0)
> > imx6_pcie->controller_id = 0;
> >
> > for only one PCI controller case. I just tested first one slot before send
> > patch, so not met problem.
> >
> > Previously, we use below logic
> > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
> > imx6_pcie->controller_id = 1;
> >
> > It is not good to depend on register's base address. If there are 3
> > controllers, check logic will becomoe ugly.
>
> Makes sense. If the previous code depended on the base address, this
> patch would make more sense if it contained both the addition of the
> of_get_pci_domain_nr() call and the removal of the base address code.
Remove base address code will block existed functions. My plan is
1. this patch upstreamed
2. related dts add linux,pci-domain.
3. removed base address compare code.
Frank
>
> > > Maybe this is a typo? As written, it doesn't look like there's any
> > > point in calling of_get_pci_domain_nr().
> > >
> > > > switch (imx6_pcie->drvdata->variant) {
> > > > case IMX7D:
> > > > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
> > > > --
> > > > 2.34.1
> > > >
On 06/12/2023 16:59, Frank Li wrote:
> From: Richard Zhu <[email protected]>
>
> Add i.MX95 PCIe "fsl,imx95-pcie" compatible string.
>
Acked-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
Hi Frank,
kernel test robot noticed the following build errors:
[auto build test ERROR on pci/next]
[also build test ERROR on pci/for-linus linus/master v6.7-rc4 next-20231207]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Frank-Li/PCI-imx6-Simplify-clock-handling-by-using-HAS_CLK_-bitmask/20231207-000209
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link: https://lore.kernel.org/r/20231206155903.566194-8-Frank.Li%40nxp.com
patch subject: [PATCH 7/9] PCI: imx6: Simplify switch-case logic by involve init_phy callback
config: powerpc-randconfig-002-20231207 (https://download.01.org/0day-ci/archive/20231207/[email protected]/config)
compiler: powerpc-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231207/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
powerpc-linux-ld: drivers/pci/controller/dwc/pci-imx6.o: in function `imx6_pcie_host_init':
>> pci-imx6.c:(.text.imx6_pcie_host_init+0x120): undefined reference to `__ffsdi2'
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Frank,
kernel test robot noticed the following build errors:
[auto build test ERROR on pci/next]
[also build test ERROR on pci/for-linus linus/master v6.7-rc4 next-20231207]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Frank-Li/PCI-imx6-Simplify-clock-handling-by-using-HAS_CLK_-bitmask/20231207-000209
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link: https://lore.kernel.org/r/20231206155903.566194-7-Frank.Li%40nxp.com
patch subject: [PATCH 6/9] PCI: imx6: Simplify configure_type() by using mode_off and mode_mask
config: arm-randconfig-002-20231207 (https://download.01.org/0day-ci/archive/20231207/[email protected]/config)
compiler: arm-linux-gnueabi-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231207/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
arm-linux-gnueabi-ld: drivers/pci/controller/dwc/pci-imx6.o: in function `imx6_pcie_init_phy':
>> pci-imx6.c:(.text+0xc40): undefined reference to `__ffsdi2'
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Wed, Dec 06, 2023 at 05:52:23PM +0100, Philipp Zabel wrote:
> Hi Frank,
>
> On Mi, 2023-12-06 at 10:58 -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.
> >
> > Signed-off-by: Frank Li <[email protected]>
> > ---
> > drivers/pci/controller/dwc/pci-imx6.c | 115 +++++++++++---------------
> > 1 file changed, 47 insertions(+), 68 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > index bcf52aa86462..62d77fabd82a 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> [...]
> > @@ -696,18 +698,13 @@ static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
> >
> > static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> > {
> > - switch (imx6_pcie->drvdata->variant) {
> > - case IMX7D:
> > - case IMX8MQ:
> > - case IMX8MQ_EP:
> > + if (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_PHY_RESET))
>
> This check is not strictly necessary. If the flag is not set,
> imx6_pcie->pciephy_reset is guaranteed to be NULL, and then
> reset_control_assert() is a no-op. Same for the other (de)assert
> calls below.
>
> [...]
>
> > @@ -1335,36 +1319,24 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> > "failed to get pcie phy\n");
> > }
> >
> > - switch (imx6_pcie->drvdata->variant) {
> > - 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 (imx6_check_flag(imx6_pcie, IMX6_PCIE_FLAG_HAS_APP_RESET)) {
> > + imx6_pcie->apps_reset = devm_reset_control_get_exclusive(dev, "apps");
> [...]
>
> I wonder whether we should just defer the check whether apps/pciephy
> resets should be used to the device tree validation, and make this an
> unconditional call to get an optional reset control:
>
> imx6_pcie->apps_reset = devm_reset_control_get_optional_exclusive(dev, "apps");
I think double check here is neccesary. No sure if dts file version
binding yaml was exactly matched driver. Sometime user use old version dts.
Double check here will help identify the dts problem.
Frank
>
> regards
> Philipp
On Do, 2023-12-07 at 12:10 -0500, Frank Li wrote:
> On Wed, Dec 06, 2023 at 05:52:23PM +0100, Philipp Zabel wrote:
[...]
> > I wonder whether we should just defer the check whether apps/pciephy
> > resets should be used to the device tree validation, and make this an
> > unconditional call to get an optional reset control:
> >
> > imx6_pcie->apps_reset = devm_reset_control_get_optional_exclusive(dev, "apps");
>
> I think double check here is neccesary. No sure if dts file version
> binding yaml was exactly matched driver. Sometime user use old version dts.
> Double check here will help identify the dts problem.
Makes sense to me,
Reviewed-by: Philipp Zabel <[email protected]>
regards
Philipp