* [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support
@ 2024-01-19 17:11 Frank Li
2024-01-19 17:11 ` [PATCH v9 01/16] PCI: imx6: Simplify clock handling by using clk_bulk*() function Frank Li
` (16 more replies)
0 siblings, 17 replies; 32+ messages in thread
From: Frank Li @ 2024-01-19 17:11 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Frank.Li, bhelgaas, conor+dt, devicetree, festevam, helgaas,
hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, lpieralisi, robh, s.hauer, shawnguo
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.
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 (15):
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
dt-bindings: imx6q-pcie: Add linux,pci-domain as required for iMX8MQ
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
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 | 28 +-
.../bindings/pci/fsl,imx6q-pcie-ep.yaml | 57 +-
.../bindings/pci/fsl,imx6q-pcie.yaml | 49 +-
drivers/pci/controller/dwc/pci-imx6.c | 640 ++++++++++--------
4 files changed, 462 insertions(+), 312 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v9 01/16] PCI: imx6: Simplify clock handling by using clk_bulk*() function
2024-01-19 17:11 [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
@ 2024-01-19 17:11 ` Frank Li
2024-01-19 17:11 ` [PATCH v9 02/16] PCI: imx6: Simplify phy handling by using IMX6_PCIE_FLAG_HAS_PHYDRV Frank Li
` (15 subsequent siblings)
16 siblings, 0 replies; 32+ messages in thread
From: Frank Li @ 2024-01-19 17:11 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Frank.Li, bhelgaas, conor+dt, devicetree, festevam, helgaas,
hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, lpieralisi, robh, s.hauer, shawnguo
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 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 | 140 +++++++++-----------------
1 file changed, 50 insertions(+), 90 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 74703362aeec7..0997dd7d901a4 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,18 @@ 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 +1300,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 +1315,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 +1405,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 +1417,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 +1426,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 +1436,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
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v9 02/16] PCI: imx6: Simplify phy handling by using IMX6_PCIE_FLAG_HAS_PHYDRV
2024-01-19 17:11 [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
2024-01-19 17:11 ` [PATCH v9 01/16] PCI: imx6: Simplify clock handling by using clk_bulk*() function Frank Li
@ 2024-01-19 17:11 ` Frank Li
2024-01-19 17:11 ` [PATCH v9 03/16] PCI: imx6: Simplify reset handling by using by using *_FLAG_HAS_*_RESET Frank Li
` (14 subsequent siblings)
16 siblings, 0 replies; 32+ messages in thread
From: Frank Li @ 2024-01-19 17:11 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Frank.Li, bhelgaas, conor+dt, devicetree, festevam, helgaas,
hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, lpieralisi, robh, s.hauer, shawnguo
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 0997dd7d901a4..a33ec006660c8 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 IMX7D:
if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
@@ -1306,11 +1316,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;
@@ -1454,14 +1459,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),
@@ -1475,6 +1483,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,
@@ -1482,6 +1491,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
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v9 03/16] PCI: imx6: Simplify reset handling by using by using *_FLAG_HAS_*_RESET
2024-01-19 17:11 [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
2024-01-19 17:11 ` [PATCH v9 01/16] PCI: imx6: Simplify clock handling by using clk_bulk*() function Frank Li
2024-01-19 17:11 ` [PATCH v9 02/16] PCI: imx6: Simplify phy handling by using IMX6_PCIE_FLAG_HAS_PHYDRV Frank Li
@ 2024-01-19 17:11 ` Frank Li
2024-01-19 17:11 ` [PATCH v9 04/16] dt-bindings: imx6q-pcie: Add linux,pci-domain as required for iMX8MQ Frank Li
` (13 subsequent siblings)
16 siblings, 0 replies; 32+ messages in thread
From: Frank Li @ 2024-01-19 17:11 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Frank.Li, bhelgaas, conor+dt, devicetree, festevam, helgaas,
hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, lpieralisi, robh, s.hauer, shawnguo
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 a33ec006660c8..eda6bc6ef80ee 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,36 +1265,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;
}
@@ -1446,13 +1412,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),
@@ -1469,13 +1439,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
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v9 04/16] dt-bindings: imx6q-pcie: Add linux,pci-domain as required for iMX8MQ
2024-01-19 17:11 [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
` (2 preceding siblings ...)
2024-01-19 17:11 ` [PATCH v9 03/16] PCI: imx6: Simplify reset handling by using by using *_FLAG_HAS_*_RESET Frank Li
@ 2024-01-19 17:11 ` Frank Li
2024-01-19 17:11 ` [PATCH v9 05/16] PCI: imx6: Using "linux,pci-domain" as slot ID Frank Li
` (12 subsequent siblings)
16 siblings, 0 replies; 32+ messages in thread
From: Frank Li @ 2024-01-19 17:11 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Frank.Li, bhelgaas, conor+dt, devicetree, festevam, helgaas,
hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, lpieralisi, robh, s.hauer, shawnguo
iMX8MQ have two pci controllers. Adds "linux,pci-domain" as required
property for iMX8MQ to indicate pci controller index.
This adjustment paves the way for eliminating the hardcoded check on the
base register for acquiring the controller_id.
...
if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
imx6_pcie->controller_id = 1;
...
The controller_id is crucial and utilized for certain register bit
positions. It must align precisely with the controller index in the SoC.
An auto-incremented ID don't fit this case. The DTS or fuse configurations
may deactivate specific PCI controllers.
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Notes:
Change from v8 to v9
- Add rob's review tag
Change from v6 to v8
-none
Change from v5 to v6
- rework commit message to explain why need required and why auto increase
id not work
Change from v4 to v5
- new patch at v5
.../bindings/pci/fsl,imx6q-pcie-common.yaml | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml
index d91b639ae7ae7..8f39b4e6e8491 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-common.yaml
@@ -265,6 +265,17 @@ allOf:
- const: apps
- const: turnoff
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - fsl,imx8mq-pcie
+ - fsl,imx8mq-pcie-ep
+ then:
+ required:
+ - linux,pci-domain
+
additionalProperties: true
...
--
2.34.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v9 05/16] PCI: imx6: Using "linux,pci-domain" as slot ID
2024-01-19 17:11 [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
` (3 preceding siblings ...)
2024-01-19 17:11 ` [PATCH v9 04/16] dt-bindings: imx6q-pcie: Add linux,pci-domain as required for iMX8MQ Frank Li
@ 2024-01-19 17:11 ` Frank Li
2024-02-02 12:12 ` Lorenzo Pieralisi
2024-02-02 21:54 ` Bjorn Helgaas
2024-01-19 17:11 ` [PATCH v9 06/16] PCI: imx6: Simplify ltssm_enable() by using ltssm_off and ltssm_mask Frank Li
` (11 subsequent siblings)
16 siblings, 2 replies; 32+ messages in thread
From: Frank Li @ 2024-01-19 17:11 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Frank.Li, bhelgaas, conor+dt, devicetree, festevam, helgaas,
hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, lpieralisi, robh, s.hauer, shawnguo
Avoid use get slot id by compared with register physical address. If there
are more than 2 slots, compared logic will become complex.
"linux,pci-domain" already exist at dts since commit:
commit (c0b70f05c87f3b arm64: dts: imx8mq: use_dt_domains for pci node).
So it is safe to remove compare basic address code:
...
if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
imx6_pcie->controller_id = 1;
...
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Notes:
Change from v7 to v8
- fixed comments
- Added Manivannan Sadhasivam's review tag
Change from v5 to v7
- none
Change from v3 to v4
- remove compare basic address logic
Change from v2 to v3
- none
Change from v1 to v2
- fix of_get_pci_domain_nr return value check logic
drivers/pci/controller/dwc/pci-imx6.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index eda6bc6ef80ee..773411d20329f 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)
@@ -40,7 +41,6 @@
#define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11)
#define IMX8MQ_GPR_PCIE_VREG_BYPASS BIT(12)
#define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE GENMASK(11, 8)
-#define IMX8MQ_PCIE2_BASE_ADDR 0x33c00000
#define to_imx6_pcie(x) dev_get_drvdata((x)->dev)
@@ -1279,13 +1279,17 @@ static int imx6_pcie_probe(struct platform_device *pdev)
"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;
- }
+ /* Using linux,pci-domain as PCI slot id */
+ imx6_pcie->controller_id = of_get_pci_domain_nr(node);
+ /*
+ * If there are no "linux,pci-domain" property specified in DT, then assume only one
+ * controller is available.
+ */
+ if (imx6_pcie->controller_id == -EINVAL)
+ imx6_pcie->controller_id = 0;
+ else if (imx6_pcie->controller_id < 0)
+ return dev_err_probe(dev, imx6_pcie->controller_id,
+ "linux,pci-domain have wrong value\n");
/* Grab turnoff reset */
imx6_pcie->turnoff_reset = devm_reset_control_get_optional_exclusive(dev, "turnoff");
--
2.34.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v9 06/16] PCI: imx6: Simplify ltssm_enable() by using ltssm_off and ltssm_mask
2024-01-19 17:11 [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
` (4 preceding siblings ...)
2024-01-19 17:11 ` [PATCH v9 05/16] PCI: imx6: Using "linux,pci-domain" as slot ID Frank Li
@ 2024-01-19 17:11 ` Frank Li
2024-01-19 17:11 ` [PATCH v9 07/16] PCI: imx6: Simplify configure_type() by using mode_off and mode_mask Frank Li
` (10 subsequent siblings)
16 siblings, 0 replies; 32+ messages in thread
From: Frank Li @ 2024-01-19 17:11 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Frank.Li, bhelgaas, conor+dt, devicetree, festevam, helgaas,
hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, lpieralisi, robh, s.hauer, shawnguo
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 773411d20329f..d19fcb54fde0d 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);
}
@@ -1394,6 +1383,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,
@@ -1403,6 +1394,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,
@@ -1413,6 +1406,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
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v9 07/16] PCI: imx6: Simplify configure_type() by using mode_off and mode_mask
2024-01-19 17:11 [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
` (5 preceding siblings ...)
2024-01-19 17:11 ` [PATCH v9 06/16] PCI: imx6: Simplify ltssm_enable() by using ltssm_off and ltssm_mask Frank Li
@ 2024-01-19 17:11 ` Frank Li
2024-02-13 10:57 ` Lorenzo Pieralisi
2024-01-19 17:11 ` [PATCH v9 08/16] PCI: imx6: Simplify switch-case logic by involve init_phy callback Frank Li
` (9 subsequent siblings)
16 siblings, 1 reply; 32+ messages in thread
From: Frank Li @ 2024-01-19 17:11 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Frank.Li, bhelgaas, conor+dt, devicetree, festevam, helgaas,
hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, lpieralisi, robh, s.hauer, shawnguo
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 d19fcb54fde0d..8df07b71c93e5 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)
@@ -1385,6 +1380,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,
@@ -1396,6 +1393,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,
@@ -1408,6 +1407,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,
@@ -1417,6 +1418,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,
@@ -1425,6 +1428,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,
@@ -1434,6 +1441,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,
@@ -1443,6 +1452,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,
@@ -1452,6 +1463,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,
@@ -1460,6 +1475,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,
@@ -1468,6 +1485,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
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v9 08/16] PCI: imx6: Simplify switch-case logic by involve init_phy callback
2024-01-19 17:11 [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
` (6 preceding siblings ...)
2024-01-19 17:11 ` [PATCH v9 07/16] PCI: imx6: Simplify configure_type() by using mode_off and mode_mask Frank Li
@ 2024-01-19 17:11 ` Frank Li
2024-01-19 17:11 ` [PATCH v9 09/16] dt-bindings: imx6q-pcie: Clean up irrationality clocks check Frank Li
` (8 subsequent siblings)
16 siblings, 0 replies; 32+ messages in thread
From: Frank Li @ 2024-01-19 17:11 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Frank.Li, bhelgaas, conor+dt, devicetree, festevam, helgaas,
hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, lpieralisi, robh, s.hauer, shawnguo
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 8df07b71c93e5..dad192f38e702 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) {
@@ -1382,6 +1380,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,
@@ -1395,6 +1394,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,
@@ -1409,6 +1409,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,
@@ -1420,6 +1421,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,
@@ -1432,6 +1434,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,
@@ -1467,6 +1470,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
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v9 09/16] dt-bindings: imx6q-pcie: Clean up irrationality clocks check
2024-01-19 17:11 [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
` (7 preceding siblings ...)
2024-01-19 17:11 ` [PATCH v9 08/16] PCI: imx6: Simplify switch-case logic by involve init_phy callback Frank Li
@ 2024-01-19 17:11 ` Frank Li
2024-01-19 17:11 ` [PATCH v9 10/16] dt-bindings: imx6q-pcie: Restruct reg and reg-name Frank Li
` (7 subsequent siblings)
16 siblings, 0 replies; 32+ messages in thread
From: Frank Li @ 2024-01-19 17:11 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Frank.Li, bhelgaas, conor+dt, devicetree, festevam, helgaas,
hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, lpieralisi, robh, s.hauer, shawnguo
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 8f39b4e6e8491..a284a27c5e873 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
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v9 10/16] dt-bindings: imx6q-pcie: Restruct reg and reg-name
2024-01-19 17:11 [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
` (8 preceding siblings ...)
2024-01-19 17:11 ` [PATCH v9 09/16] dt-bindings: imx6q-pcie: Clean up irrationality clocks check Frank Li
@ 2024-01-19 17:11 ` Frank Li
2024-01-23 20:36 ` Rob Herring
2024-01-19 17:11 ` [PATCH v9 11/16] dt-bindings: imx6q-pcie: Add imx95 pcie compatible string Frank Li
` (6 subsequent siblings)
16 siblings, 1 reply; 32+ messages in thread
From: Frank Li @ 2024-01-19 17:11 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Frank.Li, bhelgaas, conor+dt, devicetree, festevam, helgaas,
hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, lpieralisi, robh, s.hauer, shawnguo
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.
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
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v9 11/16] dt-bindings: imx6q-pcie: Add imx95 pcie compatible string
2024-01-19 17:11 [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
` (9 preceding siblings ...)
2024-01-19 17:11 ` [PATCH v9 10/16] dt-bindings: imx6q-pcie: Restruct reg and reg-name Frank Li
@ 2024-01-19 17:11 ` Frank Li
2024-01-19 17:11 ` [PATCH v9 12/16] PCI: imx6: Add iMX95 PCIe Root Complex support Frank Li
` (5 subsequent siblings)
16 siblings, 0 replies; 32+ messages in thread
From: Frank Li @ 2024-01-19 17:11 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Frank.Li, bhelgaas, conor+dt, devicetree, festevam, helgaas,
hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, lpieralisi, robh, s.hauer, shawnguo
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 a284a27c5e873..1b63089ff0aee 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
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v9 12/16] PCI: imx6: Add iMX95 PCIe Root Complex support
2024-01-19 17:11 [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
` (10 preceding siblings ...)
2024-01-19 17:11 ` [PATCH v9 11/16] dt-bindings: imx6q-pcie: Add imx95 pcie compatible string Frank Li
@ 2024-01-19 17:11 ` Frank Li
2024-01-19 17:11 ` [PATCH v9 13/16] PCI: imx6: Clean up get addr_space code Frank Li
` (4 subsequent siblings)
16 siblings, 0 replies; 32+ messages in thread
From: Frank Li @ 2024-01-19 17:11 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Frank.Li, bhelgaas, conor+dt, devicetree, festevam, helgaas,
hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, lpieralisi, robh, s.hauer, shawnguo
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 dad192f38e702..1cfa5f14f18f3 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -42,6 +42,25 @@
#define IMX8MQ_GPR_PCIE_VREG_BYPASS BIT(12)
#define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE GENMASK(11, 8)
+#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:
@@ -1280,12 +1320,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, ®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 */
@@ -1458,6 +1518,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 |
@@ -1502,6 +1573,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
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v9 13/16] PCI: imx6: Clean up get addr_space code
2024-01-19 17:11 [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
` (11 preceding siblings ...)
2024-01-19 17:11 ` [PATCH v9 12/16] PCI: imx6: Add iMX95 PCIe Root Complex support Frank Li
@ 2024-01-19 17:11 ` Frank Li
2024-01-19 17:11 ` [PATCH v9 14/16] PCI: imx6: Add epc_features in imx6_pcie_drvdata Frank Li
` (3 subsequent siblings)
16 siblings, 0 replies; 32+ messages in thread
From: Frank Li @ 2024-01-19 17:11 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Frank.Li, bhelgaas, conor+dt, devicetree, festevam, helgaas,
hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, lpieralisi, robh, s.hauer, shawnguo
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 1cfa5f14f18f3..139ed618bfccc 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
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v9 14/16] PCI: imx6: Add epc_features in imx6_pcie_drvdata
2024-01-19 17:11 [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
` (12 preceding siblings ...)
2024-01-19 17:11 ` [PATCH v9 13/16] PCI: imx6: Clean up get addr_space code Frank Li
@ 2024-01-19 17:11 ` Frank Li
2024-01-19 17:11 ` [PATCH v9 15/16] dt-bindings: imx6q-pcie: Add iMX95 pcie endpoint compatible string Frank Li
` (2 subsequent siblings)
16 siblings, 0 replies; 32+ messages in thread
From: Frank Li @ 2024-01-19 17:11 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Frank.Li, bhelgaas, conor+dt, devicetree, festevam, helgaas,
hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, lpieralisi, robh, s.hauer, shawnguo
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 139ed618bfccc..bbaa45c08323b 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 = {
@@ -1534,6 +1538,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] = {
@@ -1545,6 +1550,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,
@@ -1555,6 +1561,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
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v9 15/16] dt-bindings: imx6q-pcie: Add iMX95 pcie endpoint compatible string
2024-01-19 17:11 [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
` (13 preceding siblings ...)
2024-01-19 17:11 ` [PATCH v9 14/16] PCI: imx6: Add epc_features in imx6_pcie_drvdata Frank Li
@ 2024-01-19 17:11 ` Frank Li
2024-01-19 17:11 ` [PATCH v9 16/16] PCI: imx6: Add iMX95 Endpoint (EP) support Frank Li
2024-01-25 15:31 ` [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
16 siblings, 0 replies; 32+ messages in thread
From: Frank Li @ 2024-01-19 17:11 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Frank.Li, bhelgaas, conor+dt, devicetree, festevam, helgaas,
hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, lpieralisi, robh, s.hauer, shawnguo
Add i.MX95 PCIe "fsl,imx95-pcie-ep" compatible string.
Add reg-name: "atu", "dbi2", "dma" and "app".
Reuse PCI linux,pci-domain as controller id at endpoint.
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Notes:
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 | 57 ++++++++++++++++---
1 file changed, 49 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..f4d6ae5dab785 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:
@@ -87,6 +117,17 @@ allOf:
- const: pcie_bus
- const: pcie_aux
+# reuse PCI linux,pci-domain as controller id at Endpoint
+ - if:
+ properties:
+ compatible:
+ enum:
+ - fsl,imx95-pcie-ep
+ then:
+ properties:
+ linux,pci-domain: true
+ required:
+ - linux,pci-domain
unevaluatedProperties: false
--
2.34.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v9 16/16] PCI: imx6: Add iMX95 Endpoint (EP) support
2024-01-19 17:11 [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
` (14 preceding siblings ...)
2024-01-19 17:11 ` [PATCH v9 15/16] dt-bindings: imx6q-pcie: Add iMX95 pcie endpoint compatible string Frank Li
@ 2024-01-19 17:11 ` Frank Li
2024-02-13 10:38 ` Lorenzo Pieralisi
2024-01-25 15:31 ` [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
16 siblings, 1 reply; 32+ messages in thread
From: Frank Li @ 2024-01-19 17:11 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Frank.Li, bhelgaas, conor+dt, devicetree, festevam, helgaas,
hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, lpieralisi, robh, s.hauer, shawnguo
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 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 bbaa45c08323b..7889318f6555a 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 (imx6_pcie->drvdata->variant == IMX95_EP)
+ pci->dbi_base2 = NULL;
+
ret = dw_pcie_ep_init(ep);
if (ret) {
dev_err(dev, "failed to initialize endpoint\n");
@@ -1345,6 +1374,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))
@@ -1563,6 +1595,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[] = {
@@ -1577,6 +1623,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
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v9 10/16] dt-bindings: imx6q-pcie: Restruct reg and reg-name
2024-01-19 17:11 ` [PATCH v9 10/16] dt-bindings: imx6q-pcie: Restruct reg and reg-name Frank Li
@ 2024-01-23 20:36 ` Rob Herring
0 siblings, 0 replies; 32+ messages in thread
From: Rob Herring @ 2024-01-23 20:36 UTC (permalink / raw)
To: Frank Li
Cc: linux-arm-kernel, linux-kernel, kernel, s.hauer, linux-pci,
linux-imx, conor+dt, l.stach, hongxing.zhu, devicetree, festevam,
manivannan.sadhasivam, imx, shawnguo, lpieralisi, kw,
krzysztof.kozlowski, bhelgaas, krzysztof.kozlowski+dt, helgaas
On Fri, 19 Jan 2024 12:11:16 -0500, Frank Li wrote:
> 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.
>
> 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(-)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support
2024-01-19 17:11 [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
` (15 preceding siblings ...)
2024-01-19 17:11 ` [PATCH v9 16/16] PCI: imx6: Add iMX95 Endpoint (EP) support Frank Li
@ 2024-01-25 15:31 ` Frank Li
2024-02-01 19:34 ` Frank Li
16 siblings, 1 reply; 32+ messages in thread
From: Frank Li @ 2024-01-25 15:31 UTC (permalink / raw)
To: lpieralisi
Cc: bhelgaas, conor+dt, devicetree, festevam, helgaas, hongxing.zhu,
imx, kernel, krzysztof.kozlowski+dt, krzysztof.kozlowski, kw,
l.stach, linux-arm-kernel, linux-imx, linux-kernel, linux-pci,
lpieralisi, robh, s.hauer, shawnguo
On Fri, Jan 19, 2024 at 12:11:06PM -0500, Frank Li wrote:
> first 6 patches use drvdata: flags to simplify some switch-case code.
> Improve maintaince and easy to read code.
@lpieralisi:
Could you please pick up these patches? All already reviewed by
Mani. dt-binding part acked by rob/krzysztof. Add it only impact freecale
imx platform.
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.
>
> 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 (15):
> 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
> dt-bindings: imx6q-pcie: Add linux,pci-domain as required for iMX8MQ
> 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
> 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 | 28 +-
> .../bindings/pci/fsl,imx6q-pcie-ep.yaml | 57 +-
> .../bindings/pci/fsl,imx6q-pcie.yaml | 49 +-
> drivers/pci/controller/dwc/pci-imx6.c | 640 ++++++++++--------
> 4 files changed, 462 insertions(+), 312 deletions(-)
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support
2024-01-25 15:31 ` [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
@ 2024-02-01 19:34 ` Frank Li
0 siblings, 0 replies; 32+ messages in thread
From: Frank Li @ 2024-02-01 19:34 UTC (permalink / raw)
To: lpieralisi
Cc: bhelgaas, conor+dt, devicetree, festevam, helgaas, hongxing.zhu,
imx, kernel, krzysztof.kozlowski+dt, krzysztof.kozlowski, kw,
l.stach, linux-arm-kernel, linux-imx, linux-kernel, linux-pci,
robh, s.hauer, shawnguo
On Thu, Jan 25, 2024 at 10:31:00AM -0500, Frank Li wrote:
> On Fri, Jan 19, 2024 at 12:11:06PM -0500, Frank Li wrote:
> > first 6 patches use drvdata: flags to simplify some switch-case code.
> > Improve maintaince and easy to read code.
>
> @lpieralisi:
>
> Could you please pick up these patches? All already reviewed by
> Mani. dt-binding part acked by rob/krzysztof. Add it only impact freecale
> imx platform.
>
> Frank
@lpieralisi@kernel.org
Ping!
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.
> >
> > 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 (15):
> > 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
> > dt-bindings: imx6q-pcie: Add linux,pci-domain as required for iMX8MQ
> > 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
> > 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 | 28 +-
> > .../bindings/pci/fsl,imx6q-pcie-ep.yaml | 57 +-
> > .../bindings/pci/fsl,imx6q-pcie.yaml | 49 +-
> > drivers/pci/controller/dwc/pci-imx6.c | 640 ++++++++++--------
> > 4 files changed, 462 insertions(+), 312 deletions(-)
> >
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v9 05/16] PCI: imx6: Using "linux,pci-domain" as slot ID
2024-01-19 17:11 ` [PATCH v9 05/16] PCI: imx6: Using "linux,pci-domain" as slot ID Frank Li
@ 2024-02-02 12:12 ` Lorenzo Pieralisi
2024-02-02 13:25 ` Frank Li
2024-02-02 21:54 ` Bjorn Helgaas
1 sibling, 1 reply; 32+ messages in thread
From: Lorenzo Pieralisi @ 2024-02-02 12:12 UTC (permalink / raw)
To: Frank Li
Cc: manivannan.sadhasivam, bhelgaas, conor+dt, devicetree, festevam,
helgaas, hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, robh, s.hauer, shawnguo
"PCI: imx6: Use "linux,pci-domain" as slot ID"
On Fri, Jan 19, 2024 at 12:11:11PM -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.
>
> "linux,pci-domain" already exist at dts since commit:
> commit (c0b70f05c87f3b arm64: dts: imx8mq: use_dt_domains for pci node).
>
> So it is safe to remove compare basic address code:
> ...
> if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
> imx6_pcie->controller_id = 1;
No it is not unless you magically update all firmware out
there in the field.
> ...
>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>
> Notes:
> Change from v7 to v8
> - fixed comments
> - Added Manivannan Sadhasivam's review tag
> Change from v5 to v7
> - none
> Change from v3 to v4
> - remove compare basic address logic
> Change from v2 to v3
> - none
> Change from v1 to v2
> - fix of_get_pci_domain_nr return value check logic
>
> drivers/pci/controller/dwc/pci-imx6.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index eda6bc6ef80ee..773411d20329f 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)
> @@ -40,7 +41,6 @@
> #define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11)
> #define IMX8MQ_GPR_PCIE_VREG_BYPASS BIT(12)
> #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE GENMASK(11, 8)
> -#define IMX8MQ_PCIE2_BASE_ADDR 0x33c00000
>
> #define to_imx6_pcie(x) dev_get_drvdata((x)->dev)
>
> @@ -1279,13 +1279,17 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> "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;
> - }
> + /* Using linux,pci-domain as PCI slot id */
> + imx6_pcie->controller_id = of_get_pci_domain_nr(node);
> + /*
> + * If there are no "linux,pci-domain" property specified in DT, then assume only one
> + * controller is available.
> + */
> + if (imx6_pcie->controller_id == -EINVAL)
> + imx6_pcie->controller_id = 0;
See above, this breaks compatibility with old DTs (and -EINVAL is not
the only error code you should handle).
Lorenzo
> + else if (imx6_pcie->controller_id < 0)
> + return dev_err_probe(dev, imx6_pcie->controller_id,
> + "linux,pci-domain have wrong value\n");
>
> /* Grab turnoff reset */
> imx6_pcie->turnoff_reset = devm_reset_control_get_optional_exclusive(dev, "turnoff");
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v9 05/16] PCI: imx6: Using "linux,pci-domain" as slot ID
2024-02-02 12:12 ` Lorenzo Pieralisi
@ 2024-02-02 13:25 ` Frank Li
2024-02-02 18:53 ` Lorenzo Pieralisi
0 siblings, 1 reply; 32+ messages in thread
From: Frank Li @ 2024-02-02 13:25 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: manivannan.sadhasivam, bhelgaas, conor+dt, devicetree, festevam,
helgaas, hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, robh, s.hauer, shawnguo
On Fri, Feb 02, 2024 at 01:12:41PM +0100, Lorenzo Pieralisi wrote:
> "PCI: imx6: Use "linux,pci-domain" as slot ID"
>
> On Fri, Jan 19, 2024 at 12:11:11PM -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.
> >
> > "linux,pci-domain" already exist at dts since commit:
> > commit (c0b70f05c87f3b arm64: dts: imx8mq: use_dt_domains for pci node).
> >
> > So it is safe to remove compare basic address code:
> > ...
> > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
> > imx6_pcie->controller_id = 1;
>
> No it is not unless you magically update all firmware out
> there in the field.
commit c0b70f05c87f3b09b391027c6f056d0facf331ef
Author: Peng Fan <peng.fan@nxp.com>
AuthorDate: Fri Jan 15 11:26:57 2021 +0800
Commit: Shawn Guo <shawnguo@kernel.org>
CommitDate: Fri Jan 29 14:46:28 2021 +0800
I am not sure if it is neccesary to compatible with 2 years ago's dts.
I think it may not boot at all with using 2 year agao dtb file directly.
Do you have other comments? I can remove it from this big patch series,
Frank
>
> > ...
> >
> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >
> > Notes:
> > Change from v7 to v8
> > - fixed comments
> > - Added Manivannan Sadhasivam's review tag
> > Change from v5 to v7
> > - none
> > Change from v3 to v4
> > - remove compare basic address logic
> > Change from v2 to v3
> > - none
> > Change from v1 to v2
> > - fix of_get_pci_domain_nr return value check logic
> >
> > drivers/pci/controller/dwc/pci-imx6.c | 20 ++++++++++++--------
> > 1 file changed, 12 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > index eda6bc6ef80ee..773411d20329f 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)
> > @@ -40,7 +41,6 @@
> > #define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11)
> > #define IMX8MQ_GPR_PCIE_VREG_BYPASS BIT(12)
> > #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE GENMASK(11, 8)
> > -#define IMX8MQ_PCIE2_BASE_ADDR 0x33c00000
> >
> > #define to_imx6_pcie(x) dev_get_drvdata((x)->dev)
> >
> > @@ -1279,13 +1279,17 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> > "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;
> > - }
> > + /* Using linux,pci-domain as PCI slot id */
> > + imx6_pcie->controller_id = of_get_pci_domain_nr(node);
> > + /*
> > + * If there are no "linux,pci-domain" property specified in DT, then assume only one
> > + * controller is available.
> > + */
> > + if (imx6_pcie->controller_id == -EINVAL)
> > + imx6_pcie->controller_id = 0;
>
> See above, this breaks compatibility with old DTs (and -EINVAL is not
> the only error code you should handle).
>
> Lorenzo
>
> > + else if (imx6_pcie->controller_id < 0)
> > + return dev_err_probe(dev, imx6_pcie->controller_id,
> > + "linux,pci-domain have wrong value\n");
> >
> > /* Grab turnoff reset */
> > imx6_pcie->turnoff_reset = devm_reset_control_get_optional_exclusive(dev, "turnoff");
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v9 05/16] PCI: imx6: Using "linux,pci-domain" as slot ID
2024-02-02 13:25 ` Frank Li
@ 2024-02-02 18:53 ` Lorenzo Pieralisi
2024-02-05 17:41 ` Frank Li
0 siblings, 1 reply; 32+ messages in thread
From: Lorenzo Pieralisi @ 2024-02-02 18:53 UTC (permalink / raw)
To: Frank Li
Cc: manivannan.sadhasivam, bhelgaas, conor+dt, devicetree, festevam,
helgaas, hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, robh, s.hauer, shawnguo
On Fri, Feb 02, 2024 at 08:25:58AM -0500, Frank Li wrote:
> On Fri, Feb 02, 2024 at 01:12:41PM +0100, Lorenzo Pieralisi wrote:
> > "PCI: imx6: Use "linux,pci-domain" as slot ID"
> >
> > On Fri, Jan 19, 2024 at 12:11:11PM -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.
> > >
> > > "linux,pci-domain" already exist at dts since commit:
> > > commit (c0b70f05c87f3b arm64: dts: imx8mq: use_dt_domains for pci node).
> > >
> > > So it is safe to remove compare basic address code:
> > > ...
> > > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
> > > imx6_pcie->controller_id = 1;
> >
> > No it is not unless you magically update all firmware out
> > there in the field.
>
> commit c0b70f05c87f3b09b391027c6f056d0facf331ef
> Author: Peng Fan <peng.fan@nxp.com>
> AuthorDate: Fri Jan 15 11:26:57 2021 +0800
> Commit: Shawn Guo <shawnguo@kernel.org>
> CommitDate: Fri Jan 29 14:46:28 2021 +0800
>
> I am not sure if it is neccesary to compatible with 2 years ago's dts.
> I think it may not boot at all with using 2 year agao dtb file directly.
>
> Do you have other comments? I can remove it from this big patch series,
I will have a look at the full series first we can decide whether
to drop it later.
I am travelling so I am not sure I can review it before February
12th, FYI.
Lorenzo
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v9 05/16] PCI: imx6: Using "linux,pci-domain" as slot ID
2024-01-19 17:11 ` [PATCH v9 05/16] PCI: imx6: Using "linux,pci-domain" as slot ID Frank Li
2024-02-02 12:12 ` Lorenzo Pieralisi
@ 2024-02-02 21:54 ` Bjorn Helgaas
2024-02-02 22:22 ` Frank Li
1 sibling, 1 reply; 32+ messages in thread
From: Bjorn Helgaas @ 2024-02-02 21:54 UTC (permalink / raw)
To: Frank Li
Cc: manivannan.sadhasivam, bhelgaas, conor+dt, devicetree, festevam,
hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, lpieralisi, robh, s.hauer, shawnguo
On Fri, Jan 19, 2024 at 12:11:11PM -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.
>
> "linux,pci-domain" already exist at dts since commit:
> commit (c0b70f05c87f3b arm64: dts: imx8mq: use_dt_domains for pci node).
>
> So it is safe to remove compare basic address code:
> ...
> if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
> imx6_pcie->controller_id = 1;
> ...
I have no idea what this is telling me. I guess you don't want to use
IMX8MQ_PCIE2_BASE_ADDR to decide something? That much sounds good:
the *address* of some MMIO space doesn't tell us anything about the
function of that space.
I expect the "compatible" string to tell the driver what the
programming model of the device is.
> + /* Using linux,pci-domain as PCI slot id */
> + imx6_pcie->controller_id = of_get_pci_domain_nr(node);
> + /*
> + * If there are no "linux,pci-domain" property specified in DT, then assume only one
> + * controller is available.
> + */
> + if (imx6_pcie->controller_id == -EINVAL)
> + imx6_pcie->controller_id = 0;
> + else if (imx6_pcie->controller_id < 0)
> + return dev_err_probe(dev, imx6_pcie->controller_id,
> + "linux,pci-domain have wrong value\n");
Maybe I'm missing something here. It looks like this driver uses
controller_id to distinguish between hardware variants or maybe
between two Root Ports (slots?) in the same SoC?
imx6_pcie_grp_offset
return imx6_pcie->controller_id == 1 ? IOMUXC_GPR16 : IOMUXC_GPR14;
imx6_pcie_configure_type
id = imx6_pcie->controller_id
if (!drvdata->mode_mask[id]) # <-- looks unsafe
id = 0;
regmap_update_bits(drvdata->mode_off[id], ...)
(This "mode_mask[id]" looks like it will reference garbage if the DT
supplies "linux,pci-domain = <2>". A bogus DT shouldn't be able to
cause a driver to misbehave like that.)
That doesn't seem related to "linux,pci-domain" at all.
Bjorn
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v9 05/16] PCI: imx6: Using "linux,pci-domain" as slot ID
2024-02-02 21:54 ` Bjorn Helgaas
@ 2024-02-02 22:22 ` Frank Li
2024-02-02 22:51 ` Bjorn Helgaas
0 siblings, 1 reply; 32+ messages in thread
From: Frank Li @ 2024-02-02 22:22 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: manivannan.sadhasivam, bhelgaas, conor+dt, devicetree, festevam,
hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, lpieralisi, robh, s.hauer, shawnguo
On Fri, Feb 02, 2024 at 03:54:31PM -0600, Bjorn Helgaas wrote:
> On Fri, Jan 19, 2024 at 12:11:11PM -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.
> >
> > "linux,pci-domain" already exist at dts since commit:
> > commit (c0b70f05c87f3b arm64: dts: imx8mq: use_dt_domains for pci node).
> >
> > So it is safe to remove compare basic address code:
> > ...
> > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
> > imx6_pcie->controller_id = 1;
> > ...
>
> I have no idea what this is telling me. I guess you don't want to use
> IMX8MQ_PCIE2_BASE_ADDR to decide something? That much sounds good:
> the *address* of some MMIO space doesn't tell us anything about the
> function of that space.
You are right. If there are more than two controller. The check logic
will be extremely complex.
There are some discussin at below thread about linux,pci-domain
https://lore.kernel.org/imx/20231206165953.GA717921@bhelgaas/
https://lore.kernel.org/imx/20231217175158.GF6748@thinkpad/
>
> I expect the "compatible" string to tell the driver what the
> programming model of the device is.
>
> > + /* Using linux,pci-domain as PCI slot id */
> > + imx6_pcie->controller_id = of_get_pci_domain_nr(node);
> > + /*
> > + * If there are no "linux,pci-domain" property specified in DT, then assume only one
> > + * controller is available.
> > + */
> > + if (imx6_pcie->controller_id == -EINVAL)
> > + imx6_pcie->controller_id = 0;
> > + else if (imx6_pcie->controller_id < 0)
> > + return dev_err_probe(dev, imx6_pcie->controller_id,
> > + "linux,pci-domain have wrong value\n");
>
> Maybe I'm missing something here. It looks like this driver uses
> controller_id to distinguish between hardware variants or maybe
> between two Root Ports (slots?) in the same SoC?
Yes!
>
> imx6_pcie_grp_offset
> return imx6_pcie->controller_id == 1 ? IOMUXC_GPR16 : IOMUXC_GPR14;
>
> imx6_pcie_configure_type
> id = imx6_pcie->controller_id
> if (!drvdata->mode_mask[id]) # <-- looks unsafe
I can add safe check here.
> id = 0;
> regmap_update_bits(drvdata->mode_off[id], ...)
>
> (This "mode_mask[id]" looks like it will reference garbage if the DT
> supplies "linux,pci-domain = <2>". A bogus DT shouldn't be able to
> cause a driver to misbehave like that.)
Suppose I can use dt-bind doc to force to 0,1 and safe check here.
>
> That doesn't seem related to "linux,pci-domain" at all.
I added comments about
/* Using linux,pci-domain as PCI slot id */
We may add new property about controller-id, but there already have common
one "linux,pci-domain", which value in upstreamed dts exactly match our
expection, I also found other platform use it as slot id in kernel tree.
Any way, we can continue discuss the better solution here. But I hope
it was not block whole 16 patches. we can skip this one firstly.
I still have more than 10 clean up patches my local tree.
>
> Bjorn
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v9 05/16] PCI: imx6: Using "linux,pci-domain" as slot ID
2024-02-02 22:22 ` Frank Li
@ 2024-02-02 22:51 ` Bjorn Helgaas
2024-02-03 1:01 ` Frank Li
0 siblings, 1 reply; 32+ messages in thread
From: Bjorn Helgaas @ 2024-02-02 22:51 UTC (permalink / raw)
To: Frank Li, Rob Herring
Cc: manivannan.sadhasivam, bhelgaas, conor+dt, devicetree, festevam,
hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, lpieralisi, s.hauer, shawnguo
[Rob to to: line]
On Fri, Feb 02, 2024 at 05:22:07PM -0500, Frank Li wrote:
> On Fri, Feb 02, 2024 at 03:54:31PM -0600, Bjorn Helgaas wrote:
> > On Fri, Jan 19, 2024 at 12:11:11PM -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.
> > >
> > > "linux,pci-domain" already exist at dts since commit:
> > > commit (c0b70f05c87f3b arm64: dts: imx8mq: use_dt_domains for pci node).
> > >
> > > So it is safe to remove compare basic address code:
> > > ...
> > > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
> > > imx6_pcie->controller_id = 1;
> > > ...
> >
> > I have no idea what this is telling me. I guess you don't want to use
> > IMX8MQ_PCIE2_BASE_ADDR to decide something? That much sounds good:
> > the *address* of some MMIO space doesn't tell us anything about the
> > function of that space.
>
> You are right. If there are more than two controller. The check logic
> will be extremely complex.
>
> There are some discussin at below thread about linux,pci-domain
> https://lore.kernel.org/imx/20231206165953.GA717921@bhelgaas/
My response here was too low level, just about trivial syntactic and
style issues. I should have seen the larger issue at the time; sorry
about that.
> https://lore.kernel.org/imx/20231217175158.GF6748@thinkpad/
That's a good response from Mani, but again not relevant to my point.
My point here is that "compatible" should tell the driver how to
operate the device, i.e., the driver knows what registers are present
and how they work.
If you have two variant devices that both implement a register that
can be used to distinguish them, a single "compatible" string might be
enough because the driver can use that register to tell the
difference.
If the driver can't tell the difference by looking at the hardware
itself, I think you need a separate "compatible" string for it. Of
course I'm far from a DT expert, so please correct this if necessary,
Rob, et al.
> > I expect the "compatible" string to tell the driver what the
> > programming model of the device is.
> >
> > > + /* Using linux,pci-domain as PCI slot id */
> > > + imx6_pcie->controller_id = of_get_pci_domain_nr(node);
> > > + /*
> > > + * If there are no "linux,pci-domain" property specified in DT, then assume only one
> > > + * controller is available.
> > > + */
> > > + if (imx6_pcie->controller_id == -EINVAL)
> > > + imx6_pcie->controller_id = 0;
> > > + else if (imx6_pcie->controller_id < 0)
> > > + return dev_err_probe(dev, imx6_pcie->controller_id,
> > > + "linux,pci-domain have wrong value\n");
> >
> > Maybe I'm missing something here. It looks like this driver uses
> > controller_id to distinguish between hardware variants or maybe
> > between two Root Ports (slots?) in the same SoC?
>
> Yes!
>
> > imx6_pcie_grp_offset
> > return imx6_pcie->controller_id == 1 ? IOMUXC_GPR16 : IOMUXC_GPR14;
> >
> > imx6_pcie_configure_type
> > id = imx6_pcie->controller_id
> > if (!drvdata->mode_mask[id]) # <-- looks unsafe
>
> I can add safe check here.
>
> > id = 0;
> > regmap_update_bits(drvdata->mode_off[id], ...)
> >
> > (This "mode_mask[id]" looks like it will reference garbage if the DT
> > supplies "linux,pci-domain = <2>". A bogus DT shouldn't be able to
> > cause a driver to misbehave like that.)
>
> Suppose I can use dt-bind doc to force to 0,1 and safe check here.
Nope. The driver must protect itself from garbage in the DT.
> > That doesn't seem related to "linux,pci-domain" at all.
>
> I added comments about
> /* Using linux,pci-domain as PCI slot id */
That doesn't make it related :)
> We may add new property about controller-id, but there already have common
> one "linux,pci-domain", which value in upstreamed dts exactly match our
> expection, I also found other platform use it as slot id in kernel tree.
>
> Any way, we can continue discuss the better solution here. But I hope
> it was not block whole 16 patches. we can skip this one firstly.
>
> I still have more than 10 clean up patches my local tree.
>
> >
> > Bjorn
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v9 05/16] PCI: imx6: Using "linux,pci-domain" as slot ID
2024-02-02 22:51 ` Bjorn Helgaas
@ 2024-02-03 1:01 ` Frank Li
0 siblings, 0 replies; 32+ messages in thread
From: Frank Li @ 2024-02-03 1:01 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Rob Herring, manivannan.sadhasivam, bhelgaas, conor+dt,
devicetree, festevam, hongxing.zhu, imx, kernel,
krzysztof.kozlowski+dt, krzysztof.kozlowski, kw, l.stach,
linux-arm-kernel, linux-imx, linux-kernel, linux-pci, lpieralisi,
s.hauer, shawnguo
On Fri, Feb 02, 2024 at 04:51:16PM -0600, Bjorn Helgaas wrote:
> [Rob to to: line]
>
> On Fri, Feb 02, 2024 at 05:22:07PM -0500, Frank Li wrote:
> > On Fri, Feb 02, 2024 at 03:54:31PM -0600, Bjorn Helgaas wrote:
> > > On Fri, Jan 19, 2024 at 12:11:11PM -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.
> > > >
> > > > "linux,pci-domain" already exist at dts since commit:
> > > > commit (c0b70f05c87f3b arm64: dts: imx8mq: use_dt_domains for pci node).
> > > >
> > > > So it is safe to remove compare basic address code:
> > > > ...
> > > > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
> > > > imx6_pcie->controller_id = 1;
> > > > ...
> > >
> > > I have no idea what this is telling me. I guess you don't want to use
> > > IMX8MQ_PCIE2_BASE_ADDR to decide something? That much sounds good:
> > > the *address* of some MMIO space doesn't tell us anything about the
> > > function of that space.
> >
> > You are right. If there are more than two controller. The check logic
> > will be extremely complex.
> >
> > There are some discussin at below thread about linux,pci-domain
> > https://lore.kernel.org/imx/20231206165953.GA717921@bhelgaas/
>
> My response here was too low level, just about trivial syntactic and
> style issues. I should have seen the larger issue at the time; sorry
> about that.
>
> > https://lore.kernel.org/imx/20231217175158.GF6748@thinkpad/
>
> That's a good response from Mani, but again not relevant to my point.
>
> My point here is that "compatible" should tell the driver how to
> operate the device, i.e., the driver knows what registers are present
> and how they work.
>
> If you have two variant devices that both implement a register that
> can be used to distinguish them, a single "compatible" string might be
> enough because the driver can use that register to tell the
> difference.
>
> If the driver can't tell the difference by looking at the hardware
> itself, I think you need a separate "compatible" string for it. Of
> course I'm far from a DT expert, so please correct this if necessary,
> Rob, et al.
>
> > > I expect the "compatible" string to tell the driver what the
> > > programming model of the device is.
> > >
> > > > + /* Using linux,pci-domain as PCI slot id */
> > > > + imx6_pcie->controller_id = of_get_pci_domain_nr(node);
> > > > + /*
> > > > + * If there are no "linux,pci-domain" property specified in DT, then assume only one
> > > > + * controller is available.
> > > > + */
> > > > + if (imx6_pcie->controller_id == -EINVAL)
> > > > + imx6_pcie->controller_id = 0;
> > > > + else if (imx6_pcie->controller_id < 0)
> > > > + return dev_err_probe(dev, imx6_pcie->controller_id,
> > > > + "linux,pci-domain have wrong value\n");
> > >
> > > Maybe I'm missing something here. It looks like this driver uses
> > > controller_id to distinguish between hardware variants or maybe
> > > between two Root Ports (slots?) in the same SoC?
> >
> > Yes!
> >
> > > imx6_pcie_grp_offset
> > > return imx6_pcie->controller_id == 1 ? IOMUXC_GPR16 : IOMUXC_GPR14;
> > >
> > > imx6_pcie_configure_type
> > > id = imx6_pcie->controller_id
> > > if (!drvdata->mode_mask[id]) # <-- looks unsafe
> >
> > I can add safe check here.
> >
> > > id = 0;
> > > regmap_update_bits(drvdata->mode_off[id], ...)
> > >
> > > (This "mode_mask[id]" looks like it will reference garbage if the DT
> > > supplies "linux,pci-domain = <2>". A bogus DT shouldn't be able to
> > > cause a driver to misbehave like that.)
> >
> > Suppose I can use dt-bind doc to force to 0,1 and safe check here.
>
> Nope. The driver must protect itself from garbage in the DT.
>
> > > That doesn't seem related to "linux,pci-domain" at all.
> >
> > I added comments about
> > /* Using linux,pci-domain as PCI slot id */
>
> That doesn't make it related :)
Okay, linux,pci-domain is not good method for this. Anyways, previous
implement is wrong.
Let me skip it and think a better method to fix this problem later.
Frank
>
> > We may add new property about controller-id, but there already have common
> > one "linux,pci-domain", which value in upstreamed dts exactly match our
> > expection, I also found other platform use it as slot id in kernel tree.
> >
> > Any way, we can continue discuss the better solution here. But I hope
> > it was not block whole 16 patches. we can skip this one firstly.
> >
> > I still have more than 10 clean up patches my local tree.
> >
> > >
> > > Bjorn
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v9 05/16] PCI: imx6: Using "linux,pci-domain" as slot ID
2024-02-02 18:53 ` Lorenzo Pieralisi
@ 2024-02-05 17:41 ` Frank Li
0 siblings, 0 replies; 32+ messages in thread
From: Frank Li @ 2024-02-05 17:41 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: manivannan.sadhasivam, bhelgaas, conor+dt, devicetree, festevam,
helgaas, hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, robh, s.hauer, shawnguo
On Fri, Feb 02, 2024 at 07:53:24PM +0100, Lorenzo Pieralisi wrote:
> On Fri, Feb 02, 2024 at 08:25:58AM -0500, Frank Li wrote:
> > On Fri, Feb 02, 2024 at 01:12:41PM +0100, Lorenzo Pieralisi wrote:
> > > "PCI: imx6: Use "linux,pci-domain" as slot ID"
> > >
> > > On Fri, Jan 19, 2024 at 12:11:11PM -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.
> > > >
> > > > "linux,pci-domain" already exist at dts since commit:
> > > > commit (c0b70f05c87f3b arm64: dts: imx8mq: use_dt_domains for pci node).
> > > >
> > > > So it is safe to remove compare basic address code:
> > > > ...
> > > > if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR)
> > > > imx6_pcie->controller_id = 1;
> > >
> > > No it is not unless you magically update all firmware out
> > > there in the field.
> >
> > commit c0b70f05c87f3b09b391027c6f056d0facf331ef
> > Author: Peng Fan <peng.fan@nxp.com>
> > AuthorDate: Fri Jan 15 11:26:57 2021 +0800
> > Commit: Shawn Guo <shawnguo@kernel.org>
> > CommitDate: Fri Jan 29 14:46:28 2021 +0800
> >
> > I am not sure if it is neccesary to compatible with 2 years ago's dts.
> > I think it may not boot at all with using 2 year agao dtb file directly.
> >
> > Do you have other comments? I can remove it from this big patch series,
>
> I will have a look at the full series first we can decide whether
> to drop it later.
>
> I am travelling so I am not sure I can review it before February
> 12th, FYI.
Okay, I send v10 at
https://lore.kernel.org/imx/Zb06JL5X8EiHEyFD@lpieralisi/raw
which removed "linux,pci-domain" patches.
If you have more comments, please comment at v10 version.
Frank
>
> Lorenzo
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v9 16/16] PCI: imx6: Add iMX95 Endpoint (EP) support
2024-01-19 17:11 ` [PATCH v9 16/16] PCI: imx6: Add iMX95 Endpoint (EP) support Frank Li
@ 2024-02-13 10:38 ` Lorenzo Pieralisi
2024-02-13 13:23 ` Robin Murphy
0 siblings, 1 reply; 32+ messages in thread
From: Lorenzo Pieralisi @ 2024-02-13 10:38 UTC (permalink / raw)
To: Frank Li
Cc: manivannan.sadhasivam, bhelgaas, conor+dt, devicetree, festevam,
helgaas, hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, robh, s.hauer, shawnguo, hch,
robin.murphy
[+Christoph, Robin - just checking with you if the DMA mask handling is
what you expect from drivers, don't want to merge code that breaks your
expectations]
On Fri, Jan 19, 2024 at 12:11:22PM -0500, Frank Li wrote:
> 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 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 bbaa45c08323b..7889318f6555a 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 (imx6_pcie->drvdata->variant == IMX95_EP)
> + pci->dbi_base2 = NULL;
> +
> ret = dw_pcie_ep_init(ep);
> if (ret) {
> dev_err(dev, "failed to initialize endpoint\n");
> @@ -1345,6 +1374,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))
> @@ -1563,6 +1595,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[] = {
> @@ -1577,6 +1623,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
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v9 07/16] PCI: imx6: Simplify configure_type() by using mode_off and mode_mask
2024-01-19 17:11 ` [PATCH v9 07/16] PCI: imx6: Simplify configure_type() by using mode_off and mode_mask Frank Li
@ 2024-02-13 10:57 ` Lorenzo Pieralisi
0 siblings, 0 replies; 32+ messages in thread
From: Lorenzo Pieralisi @ 2024-02-13 10:57 UTC (permalink / raw)
To: Frank Li
Cc: manivannan.sadhasivam, bhelgaas, conor+dt, devicetree, festevam,
helgaas, hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, robh, s.hauer, shawnguo
On Fri, Jan 19, 2024 at 12:11:13PM -0500, Frank Li wrote:
> 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 d19fcb54fde0d..8df07b71c93e5 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;
I don't understand what this means. If the mode mask for id is == 0, we
are falling back to mode_mask and mode_off for controller ID 0 ? Is that
what this code is supposed to do ? If so the comment makes no sense to
me.
Lorenzo
> +
> + 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)
> @@ -1385,6 +1380,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,
> @@ -1396,6 +1393,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,
> @@ -1408,6 +1407,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,
> @@ -1417,6 +1418,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,
> @@ -1425,6 +1428,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,
> @@ -1434,6 +1441,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,
> @@ -1443,6 +1452,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,
> @@ -1452,6 +1463,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,
> @@ -1460,6 +1475,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,
> @@ -1468,6 +1485,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
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v9 16/16] PCI: imx6: Add iMX95 Endpoint (EP) support
2024-02-13 10:38 ` Lorenzo Pieralisi
@ 2024-02-13 13:23 ` Robin Murphy
2024-02-13 15:35 ` Frank Li
0 siblings, 1 reply; 32+ messages in thread
From: Robin Murphy @ 2024-02-13 13:23 UTC (permalink / raw)
To: Lorenzo Pieralisi, Frank Li
Cc: manivannan.sadhasivam, bhelgaas, conor+dt, devicetree, festevam,
helgaas, hongxing.zhu, imx, kernel, krzysztof.kozlowski+dt,
krzysztof.kozlowski, kw, l.stach, linux-arm-kernel, linux-imx,
linux-kernel, linux-pci, robh, s.hauer, shawnguo, hch
On 13/02/2024 10:38 am, Lorenzo Pieralisi wrote:
> [+Christoph, Robin - just checking with you if the DMA mask handling is
> what you expect from drivers, don't want to merge code that breaks your
> expectations]
I don't really understand how all the endpoint stuff fits together, but
my hunch would be that setting the DMA masks in imx6_add_pcie_ep() might
be more sensible, unless perhaps there's an implied intent to account
for eDMA channels in root complex mode as well (and so assuming that
eDMA and endpoint mode play nicely together sharing the same platform
device) - as we've discussed before across various threads, setting DMA
masks for a host bridge itself doesn't really make sense (and hence it
leaves the gap where we can get away with co-opting them for the MSI
address hack); it's any additional DMA-initiating functionality within a
root complex which should own that responsibility.
FWIW it looks like the equivalent change for Layerscape *is* specific to
endpoint mode, but I don't know how relevant that is to i.MX given that
they're unrelated hardware configurations.
Thanks,
Robin.
> On Fri, Jan 19, 2024 at 12:11:22PM -0500, Frank Li wrote:
>> 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 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 bbaa45c08323b..7889318f6555a 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 (imx6_pcie->drvdata->variant == IMX95_EP)
>> + pci->dbi_base2 = NULL;
>> +
>> ret = dw_pcie_ep_init(ep);
>> if (ret) {
>> dev_err(dev, "failed to initialize endpoint\n");
>> @@ -1345,6 +1374,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))
>> @@ -1563,6 +1595,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[] = {
>> @@ -1577,6 +1623,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
>>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v9 16/16] PCI: imx6: Add iMX95 Endpoint (EP) support
2024-02-13 13:23 ` Robin Murphy
@ 2024-02-13 15:35 ` Frank Li
0 siblings, 0 replies; 32+ messages in thread
From: Frank Li @ 2024-02-13 15:35 UTC (permalink / raw)
To: Robin Murphy
Cc: Lorenzo Pieralisi, manivannan.sadhasivam, bhelgaas, conor+dt,
devicetree, festevam, helgaas, hongxing.zhu, imx, kernel,
krzysztof.kozlowski+dt, krzysztof.kozlowski, kw, l.stach,
linux-arm-kernel, linux-imx, linux-kernel, linux-pci, robh,
s.hauer, shawnguo, hch
On Tue, Feb 13, 2024 at 01:23:45PM +0000, Robin Murphy wrote:
> On 13/02/2024 10:38 am, Lorenzo Pieralisi wrote:
> > [+Christoph, Robin - just checking with you if the DMA mask handling is
> > what you expect from drivers, don't want to merge code that breaks your
> > expectations]
>
> I don't really understand how all the endpoint stuff fits together, but my
> hunch would be that setting the DMA masks in imx6_add_pcie_ep() might be
> more sensible, unless perhaps there's an implied intent to account for eDMA
pci_epc_mem_alloc_addr() ioremap PCIe EP side outbound memory. Without it
it will be mapped to lower 4G space / SWIOTLB. Generally, outbound is quite
big in some chips, such imx95, (4GB). So it need set dmamask to make sure
it can be mapped to > 4G space.
> channels in root complex mode as well (and so assuming that eDMA and
> endpoint mode play nicely together sharing the same platform device) - as
There are eDMA in host side, but still not find a usecase yet. So it is not
enabled yet. I just need add flags IMX6_PCIE_FLAG_SUPPORT_64BIT in imx95
host if needs.
Anyways, if you like, I can move it into imx6_add_pcie_ep() at next
version.
Frank
> we've discussed before across various threads, setting DMA masks for a host
> bridge itself doesn't really make sense (and hence it leaves the gap where
> we can get away with co-opting them for the MSI address hack); it's any
> additional DMA-initiating functionality within a root complex which should
> own that responsibility.
>
> FWIW it looks like the equivalent change for Layerscape *is* specific to
> endpoint mode, but I don't know how relevant that is to i.MX given that
> they're unrelated hardware configurations.
Previous i.MX PCI outbound space < 4G space. And only have 32bit address
access in system bus. So needn't set DMA mask. i.MX95 change it and PCIe
part is inheriate from layerscape.
Frank
>
> Thanks,
> Robin.
>
> > On Fri, Jan 19, 2024 at 12:11:22PM -0500, Frank Li wrote:
> > > 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 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 bbaa45c08323b..7889318f6555a 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 (imx6_pcie->drvdata->variant == IMX95_EP)
> > > + pci->dbi_base2 = NULL;
> > > +
> > > ret = dw_pcie_ep_init(ep);
> > > if (ret) {
> > > dev_err(dev, "failed to initialize endpoint\n");
> > > @@ -1345,6 +1374,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))
> > > @@ -1563,6 +1595,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[] = {
> > > @@ -1577,6 +1623,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
> > >
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2024-02-13 15:36 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-19 17:11 [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
2024-01-19 17:11 ` [PATCH v9 01/16] PCI: imx6: Simplify clock handling by using clk_bulk*() function Frank Li
2024-01-19 17:11 ` [PATCH v9 02/16] PCI: imx6: Simplify phy handling by using IMX6_PCIE_FLAG_HAS_PHYDRV Frank Li
2024-01-19 17:11 ` [PATCH v9 03/16] PCI: imx6: Simplify reset handling by using by using *_FLAG_HAS_*_RESET Frank Li
2024-01-19 17:11 ` [PATCH v9 04/16] dt-bindings: imx6q-pcie: Add linux,pci-domain as required for iMX8MQ Frank Li
2024-01-19 17:11 ` [PATCH v9 05/16] PCI: imx6: Using "linux,pci-domain" as slot ID Frank Li
2024-02-02 12:12 ` Lorenzo Pieralisi
2024-02-02 13:25 ` Frank Li
2024-02-02 18:53 ` Lorenzo Pieralisi
2024-02-05 17:41 ` Frank Li
2024-02-02 21:54 ` Bjorn Helgaas
2024-02-02 22:22 ` Frank Li
2024-02-02 22:51 ` Bjorn Helgaas
2024-02-03 1:01 ` Frank Li
2024-01-19 17:11 ` [PATCH v9 06/16] PCI: imx6: Simplify ltssm_enable() by using ltssm_off and ltssm_mask Frank Li
2024-01-19 17:11 ` [PATCH v9 07/16] PCI: imx6: Simplify configure_type() by using mode_off and mode_mask Frank Li
2024-02-13 10:57 ` Lorenzo Pieralisi
2024-01-19 17:11 ` [PATCH v9 08/16] PCI: imx6: Simplify switch-case logic by involve init_phy callback Frank Li
2024-01-19 17:11 ` [PATCH v9 09/16] dt-bindings: imx6q-pcie: Clean up irrationality clocks check Frank Li
2024-01-19 17:11 ` [PATCH v9 10/16] dt-bindings: imx6q-pcie: Restruct reg and reg-name Frank Li
2024-01-23 20:36 ` Rob Herring
2024-01-19 17:11 ` [PATCH v9 11/16] dt-bindings: imx6q-pcie: Add imx95 pcie compatible string Frank Li
2024-01-19 17:11 ` [PATCH v9 12/16] PCI: imx6: Add iMX95 PCIe Root Complex support Frank Li
2024-01-19 17:11 ` [PATCH v9 13/16] PCI: imx6: Clean up get addr_space code Frank Li
2024-01-19 17:11 ` [PATCH v9 14/16] PCI: imx6: Add epc_features in imx6_pcie_drvdata Frank Li
2024-01-19 17:11 ` [PATCH v9 15/16] dt-bindings: imx6q-pcie: Add iMX95 pcie endpoint compatible string Frank Li
2024-01-19 17:11 ` [PATCH v9 16/16] PCI: imx6: Add iMX95 Endpoint (EP) support Frank Li
2024-02-13 10:38 ` Lorenzo Pieralisi
2024-02-13 13:23 ` Robin Murphy
2024-02-13 15:35 ` Frank Li
2024-01-25 15:31 ` [PATCH v9 00/16] PCI: imx6: Clean up and add imx95 pci support Frank Li
2024-02-01 19:34 ` Frank Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).