Based on the i.MX8MP GPC and blk-ctrl patch-set[1] issued by Lucas and the
following commits.
- one codes refine patch-set[5].
- two Fixes[2],[3].
- one binding commit[4].
- some dts changes in Shawn's git if you want to test PCIe on i.MX8MM EVK.
b4d36c10bf17 arm64: dts: imx8mm-evk: Add the pcie support on imx8mm evk board
aaeba6a8e226 arm64: dts: imx8mm: Add the pcie support
cfc5078432ca arm64: dts: imx8mm: Add the pcie phy support
Sorry about that there may be some conflictions when do the codes merge.
I'm waiting for the ack now, and will re-base them in a proper sequence later.
This series patches add the i.MX8MP PCIe support and tested on i.MX8MM EVK and
i.MX8MP EVk boards. The PCIe NVME works fine on both boards.
- i.MX8MP PCIe PHY has two resets refer to the i.MX8MM PCIe PHY.
Add one more PHY reset for i.MX8MP PCIe PHY accordingly.
- Add the i.MX8MP PCIe PHY support in the i.MX8M PCIe PHY driver.
And share as much as possible codes with i.MX8MM PCIe PHY.
- Add the i.MX8MP PCIe support in binding document, DTS files, and PCIe
driver.
Main changes v1-->v2:
- It's my fault forget including Vinod, re-send v2 after include Vinod
and [email protected].
- List the basements of this patch-set. The branch, codes changes and so on.
- Clean up some useless register and bit definitions in #3 patch.
[1]https://patchwork.kernel.org/project/linux-arm-kernel/cover/[email protected]/
[2]https://patchwork.ozlabs.org/project/linux-pci/patch/[email protected]/
[3]https://patchwork.ozlabs.org/project/linux-pci/patch/[email protected]/
[4]https://patchwork.ozlabs.org/project/linux-pci/patch/[email protected]/
[5]https://patchwork.ozlabs.org/project/linux-pci/cover/[email protected]/
NOTE:
Based git <git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git>
Based branch <pci/imx6>
Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 1 +
Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml | 4 +-
arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 55 ++++++++++++++++++++++
arch/arm64/boot/dts/freescale/imx8mp.dtsi | 46 ++++++++++++++++++-
drivers/pci/controller/dwc/pci-imx6.c | 19 +++++++-
drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 205 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------
drivers/reset/reset-imx7.c | 1 +
7 files changed, 286 insertions(+), 45 deletions(-)
[PATCH v2 1/7] reset: imx7: Add the iMX8MP PCIe PHY PERST support
[PATCH v2 2/7] dt-binding: phy: Add iMX8MP PCIe PHY binding
[PATCH v2 3/7] phy: freescale: imx8m-pcie: Add iMX8MP PCIe PHY
[PATCH v2 4/7] dt-bindings: imx6q-pcie: Add iMX8MP PCIe compatible
[PATCH v2 5/7] arm64: dts: imx8mp: add the iMX8MP PCIe support
[PATCH v2 6/7] arm64: dts: imx8mp-evk: Add PCIe support
[PATCH v2 7/7] PCI: imx6: Add the iMX8MP PCIe support
Add the i.MX8MP PCIe PHY PERST support.
Signed-off-by: Richard Zhu <[email protected]>
---
drivers/reset/reset-imx7.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
index 185a333df66c..d2408725eb2c 100644
--- a/drivers/reset/reset-imx7.c
+++ b/drivers/reset/reset-imx7.c
@@ -329,6 +329,7 @@ static int imx8mp_reset_set(struct reset_controller_dev *rcdev,
break;
case IMX8MP_RESET_PCIE_CTRL_APPS_EN:
+ case IMX8MP_RESET_PCIEPHY_PERST:
value = assert ? 0 : bit;
break;
}
--
2.25.1
Add i.MX8MP PCIe PHY binding.
Signed-off-by: Richard Zhu <[email protected]>
---
Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml
index b6421eedece3..3646b3ed4375 100644
--- a/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml
@@ -16,6 +16,7 @@ properties:
compatible:
enum:
- fsl,imx8mm-pcie-phy
+ - fsl,imx8mp-pcie-phy
reg:
maxItems: 1
@@ -28,11 +29,12 @@ properties:
- const: ref
resets:
- maxItems: 1
+ maxItems: 2
reset-names:
items:
- const: pciephy
+ - const: perst
fsl,refclk-pad-mode:
description: |
--
2.25.1
Add i.MX8MP PCIe compatible string.
Signed-off-by: Richard Zhu <[email protected]>
---
Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
index 36c8a06d17a0..252e5b72aee0 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
@@ -26,6 +26,7 @@ properties:
- fsl,imx7d-pcie
- fsl,imx8mq-pcie
- fsl,imx8mm-pcie
+ - fsl,imx8mp-pcie
reg:
items:
--
2.25.1
Add the i.MX8MP PCIe PHY support
Signed-off-by: Richard Zhu <[email protected]>
---
drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 205 ++++++++++++++++-----
1 file changed, 163 insertions(+), 42 deletions(-)
diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
index 04b1aafb29f4..3d01da4323a6 100644
--- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
+++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
@@ -11,6 +11,8 @@
#include <linux/mfd/syscon.h>
#include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>
@@ -30,12 +32,10 @@
#define IMX8MM_PCIE_PHY_CMN_REG065 0x194
#define ANA_AUX_RX_TERM (BIT(7) | BIT(4))
#define ANA_AUX_TX_LVL GENMASK(3, 0)
-#define IMX8MM_PCIE_PHY_CMN_REG75 0x1D4
-#define PCIE_PHY_CMN_REG75_PLL_DONE 0x3
+#define IMX8MM_PCIE_PHY_CMN_REG075 0x1D4
+#define ANA_PLL_DONE 0x3
#define PCIE_PHY_TRSV_REG5 0x414
-#define PCIE_PHY_TRSV_REG5_GEN1_DEEMP 0x2D
#define PCIE_PHY_TRSV_REG6 0x418
-#define PCIE_PHY_TRSV_REG6_GEN2_DEEMP 0xF
#define IMX8MM_GPR_PCIE_REF_CLK_SEL GENMASK(25, 24)
#define IMX8MM_GPR_PCIE_REF_CLK_PLL FIELD_PREP(IMX8MM_GPR_PCIE_REF_CLK_SEL, 0x3)
@@ -46,16 +46,43 @@
#define IMX8MM_GPR_PCIE_SSC_EN BIT(16)
#define IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE BIT(9)
+#define IMX8MP_GPR_REG0 0x0
+#define IMX8MP_GPR_CLK_MOD_EN BIT(0)
+#define IMX8MP_GPR_PHY_APB_RST BIT(4)
+#define IMX8MP_GPR_PHY_INIT_RST BIT(5)
+#define IMX8MP_GPR_REG1 0x4
+#define IMX8MP_GPR_PM_EN_CORE_CLK BIT(0)
+#define IMX8MP_GPR_PLL_LOCK BIT(13)
+#define IMX8MP_GPR_REG2 0x8
+#define IMX8MP_GPR_P_PLL_MASK GENMASK(5, 0)
+#define IMX8MP_GPR_M_PLL_MASK GENMASK(15, 6)
+#define IMX8MP_GPR_S_PLL_MASK GENMASK(18, 16)
+#define IMX8MP_GPR_P_PLL (0xc << 0)
+#define IMX8MP_GPR_M_PLL (0x320 << 6)
+#define IMX8MP_GPR_S_PLL (0x4 << 16)
+#define IMX8MP_GPR_REG3 0xc
+#define IMX8MP_GPR_PLL_CKE BIT(17)
+#define IMX8MP_GPR_PLL_RST BIT(31)
+
+enum imx8_pcie_phy_type {
+ IMX8MM,
+ IMX8MP,
+};
+
struct imx8_pcie_phy {
void __iomem *base;
+ struct device *dev;
struct clk *clk;
struct phy *phy;
+ struct regmap *hsio_blk_ctrl;
struct regmap *iomuxc_gpr;
struct reset_control *reset;
+ struct reset_control *perst;
u32 refclk_pad_mode;
u32 tx_deemph_gen1;
u32 tx_deemph_gen2;
bool clkreq_unused;
+ enum imx8_pcie_phy_type variant;
};
static int imx8_pcie_phy_init(struct phy *phy)
@@ -67,6 +94,87 @@ static int imx8_pcie_phy_init(struct phy *phy)
reset_control_assert(imx8_phy->reset);
pad_mode = imx8_phy->refclk_pad_mode;
+ switch (imx8_phy->variant) {
+ case IMX8MM:
+ /* Tune PHY de-emphasis setting to pass PCIe compliance. */
+ if (imx8_phy->tx_deemph_gen1)
+ writel(imx8_phy->tx_deemph_gen1,
+ imx8_phy->base + PCIE_PHY_TRSV_REG5);
+ if (imx8_phy->tx_deemph_gen2)
+ writel(imx8_phy->tx_deemph_gen2,
+ imx8_phy->base + PCIE_PHY_TRSV_REG6);
+ break;
+ case IMX8MP:
+ reset_control_assert(imx8_phy->perst);
+ /* Set P=12,M=800,S=4 and must set ICP=2'b01. */
+ regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG2,
+ IMX8MP_GPR_P_PLL_MASK |
+ IMX8MP_GPR_M_PLL_MASK |
+ IMX8MP_GPR_S_PLL_MASK,
+ IMX8MP_GPR_P_PLL |
+ IMX8MP_GPR_M_PLL |
+ IMX8MP_GPR_S_PLL);
+ /* wait greater than 1/F_FREF =1/2MHZ=0.5us */
+ udelay(1);
+
+ regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG3,
+ IMX8MP_GPR_PLL_RST,
+ IMX8MP_GPR_PLL_RST);
+ udelay(10);
+
+ /* Set 1 to pll_cke of GPR_REG3 */
+ regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG3,
+ IMX8MP_GPR_PLL_CKE,
+ IMX8MP_GPR_PLL_CKE);
+
+ /* Lock time should be greater than 300cycle=300*0.5us=150us */
+ ret = regmap_read_poll_timeout(imx8_phy->hsio_blk_ctrl,
+ IMX8MP_GPR_REG1, val,
+ val & IMX8MP_GPR_PLL_LOCK,
+ 10, 1000);
+ if (ret) {
+ dev_err(imx8_phy->dev, "PCIe PLL lock timeout\n");
+ return ret;
+ }
+
+ /* pcie_clock_module_en */
+ regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG0,
+ IMX8MP_GPR_CLK_MOD_EN,
+ IMX8MP_GPR_CLK_MOD_EN);
+ udelay(10);
+
+ reset_control_deassert(imx8_phy->reset);
+ reset_control_deassert(imx8_phy->perst);
+
+ /* release pcie_phy_apb_reset and pcie_phy_init_resetn */
+ regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG0,
+ IMX8MP_GPR_PHY_APB_RST |
+ IMX8MP_GPR_PHY_INIT_RST,
+ IMX8MP_GPR_PHY_APB_RST |
+ IMX8MP_GPR_PHY_INIT_RST);
+ break;
+ }
+
+ if (pad_mode == IMX8_PCIE_REFCLK_PAD_INPUT) {
+ /* Configure the pad as input */
+ val = readl(imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
+ writel(val & ~ANA_PLL_CLK_OUT_TO_EXT_IO_EN,
+ imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
+ } else if (pad_mode == IMX8_PCIE_REFCLK_PAD_OUTPUT) {
+ /* Configure the PHY to output the refclock via pad */
+ writel(ANA_PLL_CLK_OUT_TO_EXT_IO_EN,
+ imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
+ writel(ANA_PLL_CLK_OUT_TO_EXT_IO_SEL,
+ imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG062);
+ writel(AUX_PLL_REFCLK_SEL_SYS_PLL,
+ imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG063);
+ val = ANA_AUX_RX_TX_SEL_TX | ANA_AUX_TX_TERM;
+ writel(val | ANA_AUX_RX_TERM_GND_EN,
+ imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG064);
+ writel(ANA_AUX_RX_TERM | ANA_AUX_TX_LVL,
+ imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG065);
+ }
+
/* Set AUX_EN_OVERRIDE 1'b0, when the CLKREQ# isn't hooked */
regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE,
@@ -91,42 +199,30 @@ static int imx8_pcie_phy_init(struct phy *phy)
regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
IMX8MM_GPR_PCIE_CMN_RST,
IMX8MM_GPR_PCIE_CMN_RST);
- usleep_range(200, 500);
- if (pad_mode == IMX8_PCIE_REFCLK_PAD_INPUT) {
- /* Configure the pad as input */
- val = readl(imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
- writel(val & ~ANA_PLL_CLK_OUT_TO_EXT_IO_EN,
- imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
- } else if (pad_mode == IMX8_PCIE_REFCLK_PAD_OUTPUT) {
- /* Configure the PHY to output the refclock via pad */
- writel(ANA_PLL_CLK_OUT_TO_EXT_IO_EN,
- imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
- writel(ANA_PLL_CLK_OUT_TO_EXT_IO_SEL,
- imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG062);
- writel(AUX_PLL_REFCLK_SEL_SYS_PLL,
- imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG063);
- val = ANA_AUX_RX_TX_SEL_TX | ANA_AUX_TX_TERM;
- writel(val | ANA_AUX_RX_TERM_GND_EN,
- imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG064);
- writel(ANA_AUX_RX_TERM | ANA_AUX_TX_LVL,
- imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG065);
+ switch (imx8_phy->variant) {
+ case IMX8MM:
+ reset_control_deassert(imx8_phy->reset);
+ usleep_range(200, 500);
+ break;
+
+ case IMX8MP:
+ /* wait for core_clk enabled */
+ ret = regmap_read_poll_timeout(imx8_phy->hsio_blk_ctrl,
+ IMX8MP_GPR_REG1, val,
+ val & IMX8MP_GPR_PM_EN_CORE_CLK,
+ 10, 20000);
+ if (ret) {
+ dev_err(imx8_phy->dev, "PCIe CORE CLK enable failed\n");
+ return ret;
+ }
+
+ break;
}
- /* Tune PHY de-emphasis setting to pass PCIe compliance. */
- if (imx8_phy->tx_deemph_gen1)
- writel(imx8_phy->tx_deemph_gen1,
- imx8_phy->base + PCIE_PHY_TRSV_REG5);
- if (imx8_phy->tx_deemph_gen2)
- writel(imx8_phy->tx_deemph_gen2,
- imx8_phy->base + PCIE_PHY_TRSV_REG6);
-
- reset_control_deassert(imx8_phy->reset);
-
/* Polling to check the phy is ready or not. */
- ret = readl_poll_timeout(imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG75,
- val, val == PCIE_PHY_CMN_REG75_PLL_DONE,
- 10, 20000);
+ ret = readl_poll_timeout(imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG075,
+ val, val == ANA_PLL_DONE, 10, 20000);
return ret;
}
@@ -153,18 +249,33 @@ static const struct phy_ops imx8_pcie_phy_ops = {
.owner = THIS_MODULE,
};
+static const struct of_device_id imx8_pcie_phy_of_match[] = {
+ {.compatible = "fsl,imx8mm-pcie-phy", .data = (void *)IMX8MM},
+ {.compatible = "fsl,imx8mp-pcie-phy", .data = (void *)IMX8MP},
+ { },
+};
+MODULE_DEVICE_TABLE(of, imx8_pcie_phy_of_match);
+
static int imx8_pcie_phy_probe(struct platform_device *pdev)
{
struct phy_provider *phy_provider;
struct device *dev = &pdev->dev;
+ const struct of_device_id *of_id;
struct device_node *np = dev->of_node;
struct imx8_pcie_phy *imx8_phy;
struct resource *res;
+ of_id = of_match_device(imx8_pcie_phy_of_match, dev);
+ if (!of_id)
+ return -EINVAL;
+
imx8_phy = devm_kzalloc(dev, sizeof(*imx8_phy), GFP_KERNEL);
if (!imx8_phy)
return -ENOMEM;
+ imx8_phy->dev = dev;
+ imx8_phy->variant = (enum imx8_pcie_phy_type)of_id->data;
+
/* get PHY refclk pad mode */
of_property_read_u32(np, "fsl,refclk-pad-mode",
&imx8_phy->refclk_pad_mode);
@@ -201,6 +312,22 @@ static int imx8_pcie_phy_probe(struct platform_device *pdev)
dev_err(dev, "Failed to get PCIEPHY reset control\n");
return PTR_ERR(imx8_phy->reset);
}
+ if (imx8_phy->variant == IMX8MP) {
+ /* Grab HSIO MIX config register range */
+ imx8_phy->hsio_blk_ctrl =
+ syscon_regmap_lookup_by_compatible("fsl,imx8mp-hsio-blk-ctrl");
+ if (IS_ERR(imx8_phy->hsio_blk_ctrl)) {
+ dev_err(dev, "unable to find hsio mix registers\n");
+ return PTR_ERR(imx8_phy->hsio_blk_ctrl);
+ }
+
+ imx8_phy->perst =
+ devm_reset_control_get_exclusive(dev, "perst");
+ if (IS_ERR(imx8_phy->perst)) {
+ dev_err(dev, "Failed to get PCIEPHY perst control\n");
+ return PTR_ERR(imx8_phy->perst);
+ }
+ }
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
imx8_phy->base = devm_ioremap_resource(dev, res);
@@ -218,12 +345,6 @@ static int imx8_pcie_phy_probe(struct platform_device *pdev)
return PTR_ERR_OR_ZERO(phy_provider);
}
-static const struct of_device_id imx8_pcie_phy_of_match[] = {
- {.compatible = "fsl,imx8mm-pcie-phy",},
- { },
-};
-MODULE_DEVICE_TABLE(of, imx8_pcie_phy_of_match);
-
static struct platform_driver imx8_pcie_phy_driver = {
.probe = imx8_pcie_phy_probe,
.driver = {
--
2.25.1
Add the i.MX8MP PCIe support.
Signed-off-by: Richard Zhu <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index bb662f90d4f3..4d34f0c88550 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -51,6 +51,7 @@ enum imx6_pcie_variants {
IMX7D,
IMX8MQ,
IMX8MM,
+ IMX8MP,
};
#define IMX6_PCIE_FLAG_IMX6_PHY BIT(0)
@@ -379,6 +380,7 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
reset_control_assert(imx6_pcie->pciephy_reset);
fallthrough;
case IMX8MM:
+ case IMX8MP:
reset_control_assert(imx6_pcie->apps_reset);
break;
case IMX6SX:
@@ -407,7 +409,8 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
static unsigned int imx6_pcie_grp_offset(const struct imx6_pcie *imx6_pcie)
{
WARN_ON(imx6_pcie->drvdata->variant != IMX8MQ &&
- imx6_pcie->drvdata->variant != IMX8MM);
+ imx6_pcie->drvdata->variant != IMX8MM &&
+ imx6_pcie->drvdata->variant != IMX8MP);
return imx6_pcie->controller_id == 1 ? IOMUXC_GPR16 : IOMUXC_GPR14;
}
@@ -448,6 +451,7 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
break;
case IMX8MM:
case IMX8MQ:
+ case IMX8MP:
ret = clk_prepare_enable(imx6_pcie->pcie_aux);
if (ret) {
dev_err(dev, "unable to enable pcie_aux clock\n");
@@ -503,6 +507,7 @@ static int imx6_pcie_clk_enable(struct imx6_pcie *imx6_pcie)
switch (imx6_pcie->drvdata->variant) {
case IMX8MM:
+ case IMX8MP:
if (phy_power_on(imx6_pcie->phy))
dev_err(dev, "unable to power on PHY\n");
break;
@@ -603,6 +608,7 @@ static int imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
reset_control_deassert(imx6_pcie->pciephy_reset);
break;
case IMX8MM:
+ case IMX8MP:
if (phy_init(imx6_pcie->phy))
dev_err(dev, "waiting for phy ready timeout!\n");
break;
@@ -678,6 +684,7 @@ static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
{
switch (imx6_pcie->drvdata->variant) {
case IMX8MM:
+ case IMX8MP:
/*
* The PHY initialization had been done in the PHY
* driver, break here directly.
@@ -823,6 +830,7 @@ static void imx6_pcie_ltssm_enable(struct device *dev)
case IMX7D:
case IMX8MQ:
case IMX8MM:
+ case IMX8MP:
reset_control_deassert(imx6_pcie->apps_reset);
break;
}
@@ -938,6 +946,7 @@ static void imx6_pcie_host_exit(struct pcie_port *pp)
imx6_pcie_clk_disable(imx6_pcie);
switch (imx6_pcie->drvdata->variant) {
case IMX8MM:
+ case IMX8MP:
if (phy_power_off(imx6_pcie->phy))
dev_err(dev, "unable to power off phy\n");
phy_exit(imx6_pcie->phy);
@@ -972,6 +981,7 @@ static void imx6_pcie_ltssm_disable(struct device *dev)
break;
case IMX7D:
case IMX8MM:
+ case IMX8MP:
reset_control_assert(imx6_pcie->apps_reset);
break;
default:
@@ -1028,6 +1038,7 @@ static int imx6_pcie_suspend_noirq(struct device *dev)
imx6_pcie_clk_disable(imx6_pcie);
switch (imx6_pcie->drvdata->variant) {
case IMX8MM:
+ case IMX8MP:
if (phy_power_off(imx6_pcie->phy))
dev_err(dev, "unable to power off PHY\n");
phy_exit(imx6_pcie->phy);
@@ -1177,6 +1188,7 @@ static int imx6_pcie_probe(struct platform_device *pdev)
}
break;
case IMX8MM:
+ case IMX8MP:
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),
@@ -1327,6 +1339,10 @@ static const struct imx6_pcie_drvdata drvdata[] = {
.variant = IMX8MM,
.flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
},
+ [IMX8MP] = {
+ .variant = IMX8MP,
+ .flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
+ },
};
static const struct of_device_id imx6_pcie_of_match[] = {
@@ -1336,6 +1352,7 @@ static const struct of_device_id imx6_pcie_of_match[] = {
{ .compatible = "fsl,imx7d-pcie", .data = &drvdata[IMX7D], },
{ .compatible = "fsl,imx8mq-pcie", .data = &drvdata[IMX8MQ], },
{ .compatible = "fsl,imx8mm-pcie", .data = &drvdata[IMX8MM], },
+ { .compatible = "fsl,imx8mp-pcie", .data = &drvdata[IMX8MP], },
{},
};
--
2.25.1
Add PCIe support on i.MX8MP EVK board.
Signed-off-by: Richard Zhu <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 55 ++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
index 2eb943210678..ed77455a3f73 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
@@ -5,6 +5,7 @@
/dts-v1/;
+#include <dt-bindings/phy/phy-imx8-pcie.h>
#include "imx8mp.dtsi"
/ {
@@ -33,6 +34,12 @@ memory@40000000 {
<0x1 0x00000000 0 0xc0000000>;
};
+ pcie0_refclk: pcie0-refclk {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <100000000>;
+ };
+
reg_can1_stby: regulator-can1-stby {
compatible = "regulator-fixed";
regulator-name = "can1-stby";
@@ -55,6 +62,17 @@ reg_can2_stby: regulator-can2-stby {
enable-active-high;
};
+ reg_pcie0: regulator-pcie {
+ compatible = "regulator-fixed";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pcie0_reg>;
+ regulator-name = "MPCIE_3V3";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ gpio = <&gpio2 6 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
+
reg_usdhc2_vmmc: regulator-usdhc2 {
compatible = "regulator-fixed";
pinctrl-names = "default";
@@ -297,6 +315,30 @@ pca6416: gpio@20 {
};
};
+&pcie_phy {
+ fsl,refclk-pad-mode = <IMX8_PCIE_REFCLK_PAD_INPUT>;
+ clocks = <&pcie0_refclk>;
+ clock-names = "ref";
+ status = "okay";
+};
+
+&pcie{
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pcie0>;
+ reset-gpio = <&gpio2 7 GPIO_ACTIVE_LOW>;
+ clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
+ <&clk IMX8MP_CLK_PCIE_ROOT>,
+ <&clk IMX8MP_CLK_HSIO_AXI>;
+ clock-names = "pcie", "pcie_aux", "pcie_bus";
+ assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>,
+ <&clk IMX8MP_CLK_PCIE_AUX>;
+ assigned-clock-rates = <500000000>, <10000000>;
+ assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>,
+ <&clk IMX8MP_SYS_PLL2_50M>;
+ vpcie-supply = <®_pcie0>;
+ status = "okay";
+};
+
&snvs_pwrkey {
status = "okay";
};
@@ -442,6 +484,19 @@ MX8MP_IOMUXC_I2C3_SDA__I2C3_SDA 0x400001c3
>;
};
+ pinctrl_pcie0: pcie0grp {
+ fsl,pins = <
+ MX8MP_IOMUXC_I2C4_SCL__PCIE_CLKREQ_B 0x61 /* open drain, pull up */
+ MX8MP_IOMUXC_SD1_DATA5__GPIO2_IO07 0x41
+ >;
+ };
+
+ pinctrl_pcie0_reg: pcie0reggrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_SD1_DATA4__GPIO2_IO06 0x41
+ >;
+ };
+
pinctrl_pmic: pmicgrp {
fsl,pins = <
MX8MP_IOMUXC_GPIO1_IO03__GPIO1_IO03 0x000001c0
--
2.25.1
Add the i.MX8MP PCIe support.
Signed-off-by: Richard Zhu <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8mp.dtsi | 46 ++++++++++++++++++++++-
1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index b40a5646f205..e7b3d8029e34 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -5,6 +5,7 @@
#include <dt-bindings/clock/imx8mp-clock.h>
#include <dt-bindings/power/imx8mp-power.h>
+#include <dt-bindings/reset/imx8mp-reset.h>
#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/input/input.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
@@ -375,7 +376,8 @@ iomuxc: pinctrl@30330000 {
};
gpr: iomuxc-gpr@30340000 {
- compatible = "fsl,imx8mp-iomuxc-gpr", "syscon";
+ compatible = "fsl,imx8mp-iomuxc-gpr",
+ "fsl,imx6q-iomuxc-gpr", "syscon";
reg = <0x30340000 0x10000>;
};
@@ -965,6 +967,17 @@ aips4: bus@32c00000 {
#size-cells = <1>;
ranges;
+ pcie_phy: pcie-phy@32f00000 {
+ compatible = "fsl,imx8mp-pcie-phy";
+ reg = <0x32f00000 0x10000>;
+ resets = <&src IMX8MP_RESET_PCIEPHY>,
+ <&src IMX8MP_RESET_PCIEPHY_PERST>;
+ reset-names = "pciephy", "perst";
+ power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_PCIE_PHY>;
+ #phy-cells = <0>;
+ status = "disabled";
+ };
+
hsio_blk_ctrl: blk-ctrl@32f10000 {
compatible = "fsl,imx8mp-hsio-blk-ctrl", "syscon";
reg = <0x32f10000 0x24>;
@@ -980,6 +993,37 @@ hsio_blk_ctrl: blk-ctrl@32f10000 {
};
};
+ pcie: pcie@33800000 {
+ compatible = "fsl,imx8mp-pcie";
+ reg = <0x33800000 0x400000>, <0x1ff00000 0x80000>;
+ reg-names = "dbi", "config";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ device_type = "pci";
+ bus-range = <0x00 0xff>;
+ ranges = <0x81000000 0 0x00000000 0x1ff80000 0 0x00010000 /* downstream I/O 64KB */
+ 0x82000000 0 0x18000000 0x18000000 0 0x07f00000>; /* non-prefetchable memory */
+ num-lanes = <1>;
+ num-viewport = <4>;
+ interrupts = <GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "msi";
+ #interrupt-cells = <1>;
+ interrupt-map-mask = <0 0 0 0x7>;
+ interrupt-map = <0 0 0 1 &gic GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 2 &gic GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 3 &gic GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 4 &gic GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
+ fsl,max-link-speed = <3>;
+ linux,pci-domain = <0>;
+ power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_PCIE>;
+ resets = <&src IMX8MP_RESET_PCIE_CTRL_APPS_EN>,
+ <&src IMX8MP_RESET_PCIE_CTRL_APPS_TURNOFF>;
+ reset-names = "apps", "turnoff";
+ phys = <&pcie_phy>;
+ phy-names = "pcie-phy";
+ status = "disabled";
+ };
+
gpu3d: gpu@38000000 {
compatible = "vivante,gc";
reg = <0x38000000 0x8000>;
--
2.25.1
On Mon, 07 Mar 2022 17:07:29 +0800, Richard Zhu wrote:
> Add i.MX8MP PCIe PHY binding.
>
> Signed-off-by: Richard Zhu <[email protected]>
> ---
> Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.example.dt.yaml: pcie-phy@32f00000: resets: [[4294967295, 26]] is too short
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.example.dt.yaml: pcie-phy@32f00000: reset-names: ['pciephy'] is too short
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1601963
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
Hi Richard,
Am Montag, dem 07.03.2022 um 17:07 +0800 schrieb Richard Zhu:
> Add the i.MX8MP PCIe PHY support
>
> Signed-off-by: Richard Zhu <[email protected]>
> ---
> drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 205 ++++++++++++++++-----
> 1 file changed, 163 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> index 04b1aafb29f4..3d01da4323a6 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> @@ -11,6 +11,8 @@
> #include <linux/mfd/syscon.h>
> #include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
> #include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> #include <linux/phy/phy.h>
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
> @@ -30,12 +32,10 @@
> #define IMX8MM_PCIE_PHY_CMN_REG065 0x194
> #define ANA_AUX_RX_TERM (BIT(7) | BIT(4))
> #define ANA_AUX_TX_LVL GENMASK(3, 0)
> -#define IMX8MM_PCIE_PHY_CMN_REG75 0x1D4
> -#define PCIE_PHY_CMN_REG75_PLL_DONE 0x3
> +#define IMX8MM_PCIE_PHY_CMN_REG075 0x1D4
> +#define ANA_PLL_DONE 0x3
> #define PCIE_PHY_TRSV_REG5 0x414
> -#define PCIE_PHY_TRSV_REG5_GEN1_DEEMP 0x2D
> #define PCIE_PHY_TRSV_REG6 0x418
> -#define PCIE_PHY_TRSV_REG6_GEN2_DEEMP 0xF
>
> #define IMX8MM_GPR_PCIE_REF_CLK_SEL GENMASK(25, 24)
> #define IMX8MM_GPR_PCIE_REF_CLK_PLL FIELD_PREP(IMX8MM_GPR_PCIE_REF_CLK_SEL, 0x3)
> @@ -46,16 +46,43 @@
> #define IMX8MM_GPR_PCIE_SSC_EN BIT(16)
> #define IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE BIT(9)
>
> +#define IMX8MP_GPR_REG0 0x0
> +#define IMX8MP_GPR_CLK_MOD_EN BIT(0)
> +#define IMX8MP_GPR_PHY_APB_RST BIT(4)
> +#define IMX8MP_GPR_PHY_INIT_RST BIT(5)
> +#define IMX8MP_GPR_REG1 0x4
> +#define IMX8MP_GPR_PM_EN_CORE_CLK BIT(0)
> +#define IMX8MP_GPR_PLL_LOCK BIT(13)
> +#define IMX8MP_GPR_REG2 0x8
> +#define IMX8MP_GPR_P_PLL_MASK GENMASK(5, 0)
> +#define IMX8MP_GPR_M_PLL_MASK GENMASK(15, 6)
> +#define IMX8MP_GPR_S_PLL_MASK GENMASK(18, 16)
> +#define IMX8MP_GPR_P_PLL (0xc << 0)
> +#define IMX8MP_GPR_M_PLL (0x320 << 6)
> +#define IMX8MP_GPR_S_PLL (0x4 << 16)
> +#define IMX8MP_GPR_REG3 0xc
> +#define IMX8MP_GPR_PLL_CKE BIT(17)
> +#define IMX8MP_GPR_PLL_RST BIT(31)
> +
> +enum imx8_pcie_phy_type {
> + IMX8MM,
> + IMX8MP,
> +};
> +
> struct imx8_pcie_phy {
> void __iomem *base;
> + struct device *dev;
> struct clk *clk;
> struct phy *phy;
> + struct regmap *hsio_blk_ctrl;
> struct regmap *iomuxc_gpr;
> struct reset_control *reset;
> + struct reset_control *perst;
> u32 refclk_pad_mode;
> u32 tx_deemph_gen1;
> u32 tx_deemph_gen2;
> bool clkreq_unused;
> + enum imx8_pcie_phy_type variant;
> };
>
> static int imx8_pcie_phy_init(struct phy *phy)
> @@ -67,6 +94,87 @@ static int imx8_pcie_phy_init(struct phy *phy)
> reset_control_assert(imx8_phy->reset);
>
> pad_mode = imx8_phy->refclk_pad_mode;
> + switch (imx8_phy->variant) {
> + case IMX8MM:
> + /* Tune PHY de-emphasis setting to pass PCIe compliance. */
> + if (imx8_phy->tx_deemph_gen1)
> + writel(imx8_phy->tx_deemph_gen1,
> + imx8_phy->base + PCIE_PHY_TRSV_REG5);
> + if (imx8_phy->tx_deemph_gen2)
> + writel(imx8_phy->tx_deemph_gen2,
> + imx8_phy->base + PCIE_PHY_TRSV_REG6);
> + break;
> + case IMX8MP:
> + reset_control_assert(imx8_phy->perst);
> + /* Set P=12,M=800,S=4 and must set ICP=2'b01. */
> + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG2,
> + IMX8MP_GPR_P_PLL_MASK |
> + IMX8MP_GPR_M_PLL_MASK |
> + IMX8MP_GPR_S_PLL_MASK,
> + IMX8MP_GPR_P_PLL |
> + IMX8MP_GPR_M_PLL |
> + IMX8MP_GPR_S_PLL);
> + /* wait greater than 1/F_FREF =1/2MHZ=0.5us */
> + udelay(1);
> +
> + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG3,
> + IMX8MP_GPR_PLL_RST,
> + IMX8MP_GPR_PLL_RST);
> + udelay(10);
> +
> + /* Set 1 to pll_cke of GPR_REG3 */
> + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG3,
> + IMX8MP_GPR_PLL_CKE,
> + IMX8MP_GPR_PLL_CKE);
> +
> + /* Lock time should be greater than 300cycle=300*0.5us=150us */
> + ret = regmap_read_poll_timeout(imx8_phy->hsio_blk_ctrl,
> + IMX8MP_GPR_REG1, val,
> + val & IMX8MP_GPR_PLL_LOCK,
> + 10, 1000);
> + if (ret) {
> + dev_err(imx8_phy->dev, "PCIe PLL lock timeout\n");
> + return ret;
> + }
As far as I understand the reference manual, this PLL is not exclusive
to the PCIe core, but can be used as a alternate reference clock for
the USB PHYs. I think we should not poke the PLL registers from the
PCIe PHY driver, but should make this PLL a real clock provided by the
HSIO blk-ctrl.
It's probably a good prove of concept for other clocks that need to be
provided by the blk-ctrls, as the audio blk-ctrl has much more of this
secondary clock controller functionality.
Do you want to give it a shot at integrating this into the blk-ctrl
driver, or should I take a look?
Regards,
Lucas
> +
> + /* pcie_clock_module_en */
> + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG0,
> + IMX8MP_GPR_CLK_MOD_EN,
> + IMX8MP_GPR_CLK_MOD_EN);
> + udelay(10);
> +
> + reset_control_deassert(imx8_phy->reset);
> + reset_control_deassert(imx8_phy->perst);
> +
> + /* release pcie_phy_apb_reset and pcie_phy_init_resetn */
> + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG0,
> + IMX8MP_GPR_PHY_APB_RST |
> + IMX8MP_GPR_PHY_INIT_RST,
> + IMX8MP_GPR_PHY_APB_RST |
> + IMX8MP_GPR_PHY_INIT_RST);
> + break;
> + }
> +
> + if (pad_mode == IMX8_PCIE_REFCLK_PAD_INPUT) {
> + /* Configure the pad as input */
> + val = readl(imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> + writel(val & ~ANA_PLL_CLK_OUT_TO_EXT_IO_EN,
> + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> + } else if (pad_mode == IMX8_PCIE_REFCLK_PAD_OUTPUT) {
> + /* Configure the PHY to output the refclock via pad */
> + writel(ANA_PLL_CLK_OUT_TO_EXT_IO_EN,
> + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> + writel(ANA_PLL_CLK_OUT_TO_EXT_IO_SEL,
> + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG062);
> + writel(AUX_PLL_REFCLK_SEL_SYS_PLL,
> + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG063);
> + val = ANA_AUX_RX_TX_SEL_TX | ANA_AUX_TX_TERM;
> + writel(val | ANA_AUX_RX_TERM_GND_EN,
> + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG064);
> + writel(ANA_AUX_RX_TERM | ANA_AUX_TX_LVL,
> + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG065);
> + }
> +
> /* Set AUX_EN_OVERRIDE 1'b0, when the CLKREQ# isn't hooked */
> regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE,
> @@ -91,42 +199,30 @@ static int imx8_pcie_phy_init(struct phy *phy)
> regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> IMX8MM_GPR_PCIE_CMN_RST,
> IMX8MM_GPR_PCIE_CMN_RST);
> - usleep_range(200, 500);
>
> - if (pad_mode == IMX8_PCIE_REFCLK_PAD_INPUT) {
> - /* Configure the pad as input */
> - val = readl(imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> - writel(val & ~ANA_PLL_CLK_OUT_TO_EXT_IO_EN,
> - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> - } else if (pad_mode == IMX8_PCIE_REFCLK_PAD_OUTPUT) {
> - /* Configure the PHY to output the refclock via pad */
> - writel(ANA_PLL_CLK_OUT_TO_EXT_IO_EN,
> - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> - writel(ANA_PLL_CLK_OUT_TO_EXT_IO_SEL,
> - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG062);
> - writel(AUX_PLL_REFCLK_SEL_SYS_PLL,
> - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG063);
> - val = ANA_AUX_RX_TX_SEL_TX | ANA_AUX_TX_TERM;
> - writel(val | ANA_AUX_RX_TERM_GND_EN,
> - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG064);
> - writel(ANA_AUX_RX_TERM | ANA_AUX_TX_LVL,
> - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG065);
> + switch (imx8_phy->variant) {
> + case IMX8MM:
> + reset_control_deassert(imx8_phy->reset);
> + usleep_range(200, 500);
> + break;
> +
> + case IMX8MP:
> + /* wait for core_clk enabled */
> + ret = regmap_read_poll_timeout(imx8_phy->hsio_blk_ctrl,
> + IMX8MP_GPR_REG1, val,
> + val & IMX8MP_GPR_PM_EN_CORE_CLK,
> + 10, 20000);
> + if (ret) {
> + dev_err(imx8_phy->dev, "PCIe CORE CLK enable failed\n");
> + return ret;
> + }
> +
> + break;
> }
>
> - /* Tune PHY de-emphasis setting to pass PCIe compliance. */
> - if (imx8_phy->tx_deemph_gen1)
> - writel(imx8_phy->tx_deemph_gen1,
> - imx8_phy->base + PCIE_PHY_TRSV_REG5);
> - if (imx8_phy->tx_deemph_gen2)
> - writel(imx8_phy->tx_deemph_gen2,
> - imx8_phy->base + PCIE_PHY_TRSV_REG6);
> -
> - reset_control_deassert(imx8_phy->reset);
> -
> /* Polling to check the phy is ready or not. */
> - ret = readl_poll_timeout(imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG75,
> - val, val == PCIE_PHY_CMN_REG75_PLL_DONE,
> - 10, 20000);
> + ret = readl_poll_timeout(imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG075,
> + val, val == ANA_PLL_DONE, 10, 20000);
> return ret;
> }
>
> @@ -153,18 +249,33 @@ static const struct phy_ops imx8_pcie_phy_ops = {
> .owner = THIS_MODULE,
> };
>
> +static const struct of_device_id imx8_pcie_phy_of_match[] = {
> + {.compatible = "fsl,imx8mm-pcie-phy", .data = (void *)IMX8MM},
> + {.compatible = "fsl,imx8mp-pcie-phy", .data = (void *)IMX8MP},
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, imx8_pcie_phy_of_match);
> +
> static int imx8_pcie_phy_probe(struct platform_device *pdev)
> {
> struct phy_provider *phy_provider;
> struct device *dev = &pdev->dev;
> + const struct of_device_id *of_id;
> struct device_node *np = dev->of_node;
> struct imx8_pcie_phy *imx8_phy;
> struct resource *res;
>
> + of_id = of_match_device(imx8_pcie_phy_of_match, dev);
> + if (!of_id)
> + return -EINVAL;
> +
> imx8_phy = devm_kzalloc(dev, sizeof(*imx8_phy), GFP_KERNEL);
> if (!imx8_phy)
> return -ENOMEM;
>
> + imx8_phy->dev = dev;
> + imx8_phy->variant = (enum imx8_pcie_phy_type)of_id->data;
> +
> /* get PHY refclk pad mode */
> of_property_read_u32(np, "fsl,refclk-pad-mode",
> &imx8_phy->refclk_pad_mode);
> @@ -201,6 +312,22 @@ static int imx8_pcie_phy_probe(struct platform_device *pdev)
> dev_err(dev, "Failed to get PCIEPHY reset control\n");
> return PTR_ERR(imx8_phy->reset);
> }
> + if (imx8_phy->variant == IMX8MP) {
> + /* Grab HSIO MIX config register range */
> + imx8_phy->hsio_blk_ctrl =
> + syscon_regmap_lookup_by_compatible("fsl,imx8mp-hsio-blk-ctrl");
> + if (IS_ERR(imx8_phy->hsio_blk_ctrl)) {
> + dev_err(dev, "unable to find hsio mix registers\n");
> + return PTR_ERR(imx8_phy->hsio_blk_ctrl);
> + }
> +
> + imx8_phy->perst =
> + devm_reset_control_get_exclusive(dev, "perst");
> + if (IS_ERR(imx8_phy->perst)) {
> + dev_err(dev, "Failed to get PCIEPHY perst control\n");
> + return PTR_ERR(imx8_phy->perst);
> + }
> + }
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> imx8_phy->base = devm_ioremap_resource(dev, res);
> @@ -218,12 +345,6 @@ static int imx8_pcie_phy_probe(struct platform_device *pdev)
> return PTR_ERR_OR_ZERO(phy_provider);
> }
>
> -static const struct of_device_id imx8_pcie_phy_of_match[] = {
> - {.compatible = "fsl,imx8mm-pcie-phy",},
> - { },
> -};
> -MODULE_DEVICE_TABLE(of, imx8_pcie_phy_of_match);
> -
> static struct platform_driver imx8_pcie_phy_driver = {
> .probe = imx8_pcie_phy_probe,
> .driver = {
> -----Original Message-----
> From: Lucas Stach <[email protected]>
> Sent: 2022年3月8日 18:05
> To: Hongxing Zhu <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>
> Subject: Re: [PATCH v2 3/7] phy: freescale: imx8m-pcie: Add iMX8MP PCIe PHY
> support
>
> Hi Richard,
>
> Am Montag, dem 07.03.2022 um 17:07 +0800 schrieb Richard Zhu:
> > Add the i.MX8MP PCIe PHY support
> >
> > Signed-off-by: Richard Zhu <[email protected]>
> > ---
> > drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 205
> > ++++++++++++++++-----
> > 1 file changed, 163 insertions(+), 42 deletions(-)
> >
> > diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > index 04b1aafb29f4..3d01da4323a6 100644
> > --- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > +++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > @@ -11,6 +11,8 @@
> > #include <linux/mfd/syscon.h>
> > #include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
> > #include <linux/module.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_device.h>
> > #include <linux/phy/phy.h>
> > #include <linux/platform_device.h>
> > #include <linux/regmap.h>
> > @@ -30,12 +32,10 @@
> > #define IMX8MM_PCIE_PHY_CMN_REG065 0x194
> > #define ANA_AUX_RX_TERM (BIT(7) | BIT(4))
> > #define ANA_AUX_TX_LVL GENMASK(3, 0)
> > -#define IMX8MM_PCIE_PHY_CMN_REG75 0x1D4
> > -#define PCIE_PHY_CMN_REG75_PLL_DONE 0x3
> > +#define IMX8MM_PCIE_PHY_CMN_REG075 0x1D4
> > +#define ANA_PLL_DONE 0x3
> > #define PCIE_PHY_TRSV_REG5 0x414
> > -#define PCIE_PHY_TRSV_REG5_GEN1_DEEMP 0x2D
> > #define PCIE_PHY_TRSV_REG6 0x418
> > -#define PCIE_PHY_TRSV_REG6_GEN2_DEEMP 0xF
> >
> > #define IMX8MM_GPR_PCIE_REF_CLK_SEL GENMASK(25, 24)
> > #define IMX8MM_GPR_PCIE_REF_CLK_PLL
> FIELD_PREP(IMX8MM_GPR_PCIE_REF_CLK_SEL, 0x3)
> > @@ -46,16 +46,43 @@
> > #define IMX8MM_GPR_PCIE_SSC_EN BIT(16)
> > #define IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE BIT(9)
> >
> > +#define IMX8MP_GPR_REG0 0x0
> > +#define IMX8MP_GPR_CLK_MOD_EN BIT(0)
> > +#define IMX8MP_GPR_PHY_APB_RST BIT(4)
> > +#define IMX8MP_GPR_PHY_INIT_RST BIT(5)
> > +#define IMX8MP_GPR_REG1 0x4
> > +#define IMX8MP_GPR_PM_EN_CORE_CLK BIT(0)
> > +#define IMX8MP_GPR_PLL_LOCK BIT(13)
> > +#define IMX8MP_GPR_REG2 0x8
> > +#define IMX8MP_GPR_P_PLL_MASK GENMASK(5, 0)
> > +#define IMX8MP_GPR_M_PLL_MASK GENMASK(15, 6)
> > +#define IMX8MP_GPR_S_PLL_MASK GENMASK(18, 16)
> > +#define IMX8MP_GPR_P_PLL (0xc << 0)
> > +#define IMX8MP_GPR_M_PLL (0x320 << 6)
> > +#define IMX8MP_GPR_S_PLL (0x4 << 16)
> > +#define IMX8MP_GPR_REG3 0xc
> > +#define IMX8MP_GPR_PLL_CKE BIT(17)
> > +#define IMX8MP_GPR_PLL_RST BIT(31)
> > +
> > +enum imx8_pcie_phy_type {
> > + IMX8MM,
> > + IMX8MP,
> > +};
> > +
> > struct imx8_pcie_phy {
> > void __iomem *base;
> > + struct device *dev;
> > struct clk *clk;
> > struct phy *phy;
> > + struct regmap *hsio_blk_ctrl;
> > struct regmap *iomuxc_gpr;
> > struct reset_control *reset;
> > + struct reset_control *perst;
> > u32 refclk_pad_mode;
> > u32 tx_deemph_gen1;
> > u32 tx_deemph_gen2;
> > bool clkreq_unused;
> > + enum imx8_pcie_phy_type variant;
> > };
> >
> > static int imx8_pcie_phy_init(struct phy *phy) @@ -67,6 +94,87 @@
> > static int imx8_pcie_phy_init(struct phy *phy)
> > reset_control_assert(imx8_phy->reset);
> >
> > pad_mode = imx8_phy->refclk_pad_mode;
> > + switch (imx8_phy->variant) {
> > + case IMX8MM:
> > + /* Tune PHY de-emphasis setting to pass PCIe compliance. */
> > + if (imx8_phy->tx_deemph_gen1)
> > + writel(imx8_phy->tx_deemph_gen1,
> > + imx8_phy->base + PCIE_PHY_TRSV_REG5);
> > + if (imx8_phy->tx_deemph_gen2)
> > + writel(imx8_phy->tx_deemph_gen2,
> > + imx8_phy->base + PCIE_PHY_TRSV_REG6);
> > + break;
> > + case IMX8MP:
> > + reset_control_assert(imx8_phy->perst);
> > + /* Set P=12,M=800,S=4 and must set ICP=2'b01. */
> > + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG2,
> > + IMX8MP_GPR_P_PLL_MASK |
> > + IMX8MP_GPR_M_PLL_MASK |
> > + IMX8MP_GPR_S_PLL_MASK,
> > + IMX8MP_GPR_P_PLL |
> > + IMX8MP_GPR_M_PLL |
> > + IMX8MP_GPR_S_PLL);
> > + /* wait greater than 1/F_FREF =1/2MHZ=0.5us */
> > + udelay(1);
> > +
> > + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG3,
> > + IMX8MP_GPR_PLL_RST,
> > + IMX8MP_GPR_PLL_RST);
> > + udelay(10);
> > +
> > + /* Set 1 to pll_cke of GPR_REG3 */
> > + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG3,
> > + IMX8MP_GPR_PLL_CKE,
> > + IMX8MP_GPR_PLL_CKE);
> > +
> > + /* Lock time should be greater than 300cycle=300*0.5us=150us */
> > + ret = regmap_read_poll_timeout(imx8_phy->hsio_blk_ctrl,
> > + IMX8MP_GPR_REG1, val,
> > + val & IMX8MP_GPR_PLL_LOCK,
> > + 10, 1000);
> > + if (ret) {
> > + dev_err(imx8_phy->dev, "PCIe PLL lock timeout\n");
> > + return ret;
> > + }
>
> As far as I understand the reference manual, this PLL is not exclusive to the
> PCIe core, but can be used as a alternate reference clock for the USB PHYs. I
> think we should not poke the PLL registers from the PCIe PHY driver, but
> should make this PLL a real clock provided by the HSIO blk-ctrl.
>
> It's probably a good prove of concept for other clocks that need to be provided
> by the blk-ctrls, as the audio blk-ctrl has much more of this secondary clock
> controller functionality.
>
> Do you want to give it a shot at integrating this into the blk-ctrl driver, or
> should I take a look?
>
Hi Lucas:
Thanks for your comments.
Yes, refer to the reference manual, most bits of the hsio-blk registers are
defined for this PLL clock. But there are still two PCIe PHY reset bits,
BIT(4) and BIT(5) of GPR_REG0.
Can all this bits and manipulations be encapsulated into one clock provided
by blk-ctrls?
It's my pleasure that if you can take a look at it, and let blk-ctrls provide
one clock for PCIe or USB modules.
Best Regards
Richard Zhu
> Regards,
> Lucas
>
> > +
> > + /* pcie_clock_module_en */
> > + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG0,
> > + IMX8MP_GPR_CLK_MOD_EN,
> > + IMX8MP_GPR_CLK_MOD_EN);
> > + udelay(10);
> > +
> > + reset_control_deassert(imx8_phy->reset);
> > + reset_control_deassert(imx8_phy->perst);
> > +
> > + /* release pcie_phy_apb_reset and pcie_phy_init_resetn */
> > + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG0,
> > + IMX8MP_GPR_PHY_APB_RST |
> > + IMX8MP_GPR_PHY_INIT_RST,
> > + IMX8MP_GPR_PHY_APB_RST |
> > + IMX8MP_GPR_PHY_INIT_RST);
> > + break;
> > + }
> > +
> > + if (pad_mode == IMX8_PCIE_REFCLK_PAD_INPUT) {
> > + /* Configure the pad as input */
> > + val = readl(imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> > + writel(val & ~ANA_PLL_CLK_OUT_TO_EXT_IO_EN,
> > + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> > + } else if (pad_mode == IMX8_PCIE_REFCLK_PAD_OUTPUT) {
> > + /* Configure the PHY to output the refclock via pad */
> > + writel(ANA_PLL_CLK_OUT_TO_EXT_IO_EN,
> > + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> > + writel(ANA_PLL_CLK_OUT_TO_EXT_IO_SEL,
> > + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG062);
> > + writel(AUX_PLL_REFCLK_SEL_SYS_PLL,
> > + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG063);
> > + val = ANA_AUX_RX_TX_SEL_TX | ANA_AUX_TX_TERM;
> > + writel(val | ANA_AUX_RX_TERM_GND_EN,
> > + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG064);
> > + writel(ANA_AUX_RX_TERM | ANA_AUX_TX_LVL,
> > + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG065);
> > + }
> > +
> > /* Set AUX_EN_OVERRIDE 1'b0, when the CLKREQ# isn't hooked */
> > regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> > IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE, @@ -91,42
> +199,30 @@ static
> > int imx8_pcie_phy_init(struct phy *phy)
> > regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> > IMX8MM_GPR_PCIE_CMN_RST,
> > IMX8MM_GPR_PCIE_CMN_RST);
> > - usleep_range(200, 500);
> >
> > - if (pad_mode == IMX8_PCIE_REFCLK_PAD_INPUT) {
> > - /* Configure the pad as input */
> > - val = readl(imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> > - writel(val & ~ANA_PLL_CLK_OUT_TO_EXT_IO_EN,
> > - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> > - } else if (pad_mode == IMX8_PCIE_REFCLK_PAD_OUTPUT) {
> > - /* Configure the PHY to output the refclock via pad */
> > - writel(ANA_PLL_CLK_OUT_TO_EXT_IO_EN,
> > - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> > - writel(ANA_PLL_CLK_OUT_TO_EXT_IO_SEL,
> > - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG062);
> > - writel(AUX_PLL_REFCLK_SEL_SYS_PLL,
> > - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG063);
> > - val = ANA_AUX_RX_TX_SEL_TX | ANA_AUX_TX_TERM;
> > - writel(val | ANA_AUX_RX_TERM_GND_EN,
> > - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG064);
> > - writel(ANA_AUX_RX_TERM | ANA_AUX_TX_LVL,
> > - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG065);
> > + switch (imx8_phy->variant) {
> > + case IMX8MM:
> > + reset_control_deassert(imx8_phy->reset);
> > + usleep_range(200, 500);
> > + break;
> > +
> > + case IMX8MP:
> > + /* wait for core_clk enabled */
> > + ret = regmap_read_poll_timeout(imx8_phy->hsio_blk_ctrl,
> > + IMX8MP_GPR_REG1, val,
> > + val & IMX8MP_GPR_PM_EN_CORE_CLK,
> > + 10, 20000);
> > + if (ret) {
> > + dev_err(imx8_phy->dev, "PCIe CORE CLK enable failed\n");
> > + return ret;
> > + }
> > +
> > + break;
> > }
> >
> > - /* Tune PHY de-emphasis setting to pass PCIe compliance. */
> > - if (imx8_phy->tx_deemph_gen1)
> > - writel(imx8_phy->tx_deemph_gen1,
> > - imx8_phy->base + PCIE_PHY_TRSV_REG5);
> > - if (imx8_phy->tx_deemph_gen2)
> > - writel(imx8_phy->tx_deemph_gen2,
> > - imx8_phy->base + PCIE_PHY_TRSV_REG6);
> > -
> > - reset_control_deassert(imx8_phy->reset);
> > -
> > /* Polling to check the phy is ready or not. */
> > - ret = readl_poll_timeout(imx8_phy->base +
> IMX8MM_PCIE_PHY_CMN_REG75,
> > - val, val == PCIE_PHY_CMN_REG75_PLL_DONE,
> > - 10, 20000);
> > + ret = readl_poll_timeout(imx8_phy->base +
> IMX8MM_PCIE_PHY_CMN_REG075,
> > + val, val == ANA_PLL_DONE, 10, 20000);
> > return ret;
> > }
> >
> > @@ -153,18 +249,33 @@ static const struct phy_ops imx8_pcie_phy_ops =
> {
> > .owner = THIS_MODULE,
> > };
> >
> > +static const struct of_device_id imx8_pcie_phy_of_match[] = {
> > + {.compatible = "fsl,imx8mm-pcie-phy", .data = (void *)IMX8MM},
> > + {.compatible = "fsl,imx8mp-pcie-phy", .data = (void *)IMX8MP},
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(of, imx8_pcie_phy_of_match);
> > +
> > static int imx8_pcie_phy_probe(struct platform_device *pdev) {
> > struct phy_provider *phy_provider;
> > struct device *dev = &pdev->dev;
> > + const struct of_device_id *of_id;
> > struct device_node *np = dev->of_node;
> > struct imx8_pcie_phy *imx8_phy;
> > struct resource *res;
> >
> > + of_id = of_match_device(imx8_pcie_phy_of_match, dev);
> > + if (!of_id)
> > + return -EINVAL;
> > +
> > imx8_phy = devm_kzalloc(dev, sizeof(*imx8_phy), GFP_KERNEL);
> > if (!imx8_phy)
> > return -ENOMEM;
> >
> > + imx8_phy->dev = dev;
> > + imx8_phy->variant = (enum imx8_pcie_phy_type)of_id->data;
> > +
> > /* get PHY refclk pad mode */
> > of_property_read_u32(np, "fsl,refclk-pad-mode",
> > &imx8_phy->refclk_pad_mode);
> > @@ -201,6 +312,22 @@ static int imx8_pcie_phy_probe(struct
> platform_device *pdev)
> > dev_err(dev, "Failed to get PCIEPHY reset control\n");
> > return PTR_ERR(imx8_phy->reset);
> > }
> > + if (imx8_phy->variant == IMX8MP) {
> > + /* Grab HSIO MIX config register range */
> > + imx8_phy->hsio_blk_ctrl =
> > +
> syscon_regmap_lookup_by_compatible("fsl,imx8mp-hsio-blk-ctrl");
> > + if (IS_ERR(imx8_phy->hsio_blk_ctrl)) {
> > + dev_err(dev, "unable to find hsio mix registers\n");
> > + return PTR_ERR(imx8_phy->hsio_blk_ctrl);
> > + }
> > +
> > + imx8_phy->perst =
> > + devm_reset_control_get_exclusive(dev, "perst");
> > + if (IS_ERR(imx8_phy->perst)) {
> > + dev_err(dev, "Failed to get PCIEPHY perst control\n");
> > + return PTR_ERR(imx8_phy->perst);
> > + }
> > + }
> >
> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > imx8_phy->base = devm_ioremap_resource(dev, res); @@ -218,12 +345,6
> > @@ static int imx8_pcie_phy_probe(struct platform_device *pdev)
> > return PTR_ERR_OR_ZERO(phy_provider); }
> >
> > -static const struct of_device_id imx8_pcie_phy_of_match[] = {
> > - {.compatible = "fsl,imx8mm-pcie-phy",},
> > - { },
> > -};
> > -MODULE_DEVICE_TABLE(of, imx8_pcie_phy_of_match);
> > -
> > static struct platform_driver imx8_pcie_phy_driver = {
> > .probe = imx8_pcie_phy_probe,
> > .driver = {
>
Hello Richard,
Am Montag, 7. M?rz 2022, 10:07:27 CET schrieb Richard Zhu:
> Based on the i.MX8MP GPC and blk-ctrl patch-set[1] issued by Lucas and the
> following commits.
> - one codes refine patch-set[5].
> - two Fixes[2],[3].
> - one binding commit[4].
> - some dts changes in Shawn's git if you want to test PCIe on i.MX8MM EVK.
> b4d36c10bf17 arm64: dts: imx8mm-evk: Add the pcie support on imx8mm evk
> board aaeba6a8e226 arm64: dts: imx8mm: Add the pcie support
> cfc5078432ca arm64: dts: imx8mm: Add the pcie phy support
>
> Sorry about that there may be some conflictions when do the codes merge.
> I'm waiting for the ack now, and will re-base them in a proper sequence
> later.
Thanks for providing the dependency list. Unfortunately they did not apply
without error on my local tree, but this is caused by other patches I track.
I managed to fix the conflicts, I think.
Eventually I was able to get a PCIe M.2 ethernet interface working on my
TQMa8MPxL based board. iperf showed >900MBit/s Tx and > 700 MBit/s Rx.
Thanks for your effort. Once the depenencies and reviews are settled, you'll
get my tested-by.
Thanks again and regards
Alexander
> This series patches add the i.MX8MP PCIe support and tested on i.MX8MM EVK
> and i.MX8MP EVk boards. The PCIe NVME works fine on both boards.
>
> - i.MX8MP PCIe PHY has two resets refer to the i.MX8MM PCIe PHY.
> Add one more PHY reset for i.MX8MP PCIe PHY accordingly.
> - Add the i.MX8MP PCIe PHY support in the i.MX8M PCIe PHY driver.
> And share as much as possible codes with i.MX8MM PCIe PHY.
> - Add the i.MX8MP PCIe support in binding document, DTS files, and PCIe
> driver.
>
> Main changes v1-->v2:
> - It's my fault forget including Vinod, re-send v2 after include Vinod
> and [email protected].
> - List the basements of this patch-set. The branch, codes changes and so on.
> - Clean up some useless register and bit definitions in #3 patch.
>
> [1]https://patchwork.kernel.org/project/linux-arm-kernel/cover/2022022820173
> [email protected]/
> [2]https://patchwork.ozlabs.org/project/linux-pci/patch/1646289275-17813-1-> [email protected]/
> [3]https://patchwork.ozlabs.org/project/linux-pci/patch/1645672013-8949-1-g
> [email protected]/
> [4]https://patchwork.ozlabs.org/project/linux-pci/patch/1646293805-18248-1-> [email protected]/
> [5]https://patchwork.ozlabs.org/project/linux-pci/cover/1645760667-10510-1-> [email protected]/
>
> NOTE:
> Based git <git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git>
> Based branch <pci/imx6>
>
> Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 1 +
> Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml | 4 +-
> arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 55
> ++++++++++++++++++++++ arch/arm64/boot/dts/freescale/imx8mp.dtsi
> | 46 ++++++++++++++++++- drivers/pci/controller/dwc/pci-imx6.c
> | 19 +++++++-
> drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 205
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------
> ------- drivers/reset/reset-imx7.c | 1 +
> 7 files changed, 286 insertions(+), 45 deletions(-)
>
> [PATCH v2 1/7] reset: imx7: Add the iMX8MP PCIe PHY PERST support
> [PATCH v2 2/7] dt-binding: phy: Add iMX8MP PCIe PHY binding
> [PATCH v2 3/7] phy: freescale: imx8m-pcie: Add iMX8MP PCIe PHY
> [PATCH v2 4/7] dt-bindings: imx6q-pcie: Add iMX8MP PCIe compatible
> [PATCH v2 5/7] arm64: dts: imx8mp: add the iMX8MP PCIe support
> [PATCH v2 6/7] arm64: dts: imx8mp-evk: Add PCIe support
> [PATCH v2 7/7] PCI: imx6: Add the iMX8MP PCIe support
> -----Original Message-----
> From: Rob Herring <[email protected]>
> Sent: 2022??3??8?? 9:08
> To: Hongxing Zhu <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; dl-linux-imx <[email protected]>;
> [email protected]
> Subject: Re: [PATCH v2 2/7] dt-binding: phy: Add iMX8MP PCIe PHY binding
>
> On Mon, 07 Mar 2022 17:07:29 +0800, Richard Zhu wrote:
> > Add i.MX8MP PCIe PHY binding.
> >
> > Signed-off-by: Richard Zhu <[email protected]>
> > ---
> > Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
Hi Rob:
Thanks for your help. Would be fixed in next version.
Best Regards
Richard Zhu
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/fs
> l,imx8-pcie-phy.example.dt.yaml: pcie-phy@32f00000: resets: [[4294967295,
> 26]] is too short
> From schema:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/fs
> l,imx8-pcie-phy.yaml
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/fs
> l,imx8-pcie-phy.example.dt.yaml: pcie-phy@32f00000: reset-names: ['pciephy']
> is too short
> From schema:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/fs
> l,imx8-pcie-phy.yaml
>
> doc reference errors (make refcheckdocs):
>
> See
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchw
> ork.ozlabs.org%2Fpatch%2F1601963&data=04%7C01%7Chongxing.zhu%
> 40nxp.com%7Cd1f3b562533e4c666e8508da00a01379%7C686ea1d3bc2b4c6
> fa92cd99c5c301635%7C0%7C1%7C637822984762155977%7CUnknown%7CT
> WFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJ
> XVCI6Mn0%3D%7C3000&sdata=bONN5kLwoBPe3D0KwURR1FapNFcGI3js
> K7XLPzZeZnQ%3D&reserved=0
>
> This check can fail if there are any dependencies. The base for a patch series is
> generally the most recent rc1.
>
> If you already ran 'make dt_binding_check' and didn't see the above error(s),
> then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit.
> -----Original Message-----
> From: Alexander Stein <[email protected]>
> Sent: 2022年3月9日 15:57
> To: Hongxing Zhu <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>
> Subject: Re: (EXT) [PATCH v2 0/7] Add the iMX8MP PCIe support
>
> Hello Richard,
>
> Am Montag, 7. März 2022, 10:07:27 CET schrieb Richard Zhu:
> > Based on the i.MX8MP GPC and blk-ctrl patch-set[1] issued by Lucas and
> > the following commits.
> > - one codes refine patch-set[5].
> > - two Fixes[2],[3].
> > - one binding commit[4].
> > - some dts changes in Shawn's git if you want to test PCIe on i.MX8MM
> EVK.
> > b4d36c10bf17 arm64: dts: imx8mm-evk: Add the pcie support on imx8mm
> > evk board aaeba6a8e226 arm64: dts: imx8mm: Add the pcie support
> > cfc5078432ca arm64: dts: imx8mm: Add the pcie phy support
> >
> > Sorry about that there may be some conflictions when do the codes merge.
> > I'm waiting for the ack now, and will re-base them in a proper
> > sequence later.
>
> Thanks for providing the dependency list. Unfortunately they did not apply
> without error on my local tree, but this is caused by other patches I track.
> I managed to fix the conflicts, I think.
> Eventually I was able to get a PCIe M.2 ethernet interface working on my
> TQMa8MPxL based board. iperf showed >900MBit/s Tx and > 700 MBit/s Rx.
> Thanks for your effort. Once the depenencies and reviews are settled, you'll get
> my tested-by.
Hi Alexander
It's great that this patch-set works on your board.
Thanks for your help to test it.
Best Regards
Richard Zhu
>
> Thanks again and regards
> Alexander
>
> > This series patches add the i.MX8MP PCIe support and tested on i.MX8MM
> > EVK and i.MX8MP EVk boards. The PCIe NVME works fine on both boards.
> >
> > - i.MX8MP PCIe PHY has two resets refer to the i.MX8MM PCIe PHY.
> > Add one more PHY reset for i.MX8MP PCIe PHY accordingly.
> > - Add the i.MX8MP PCIe PHY support in the i.MX8M PCIe PHY driver.
> > And share as much as possible codes with i.MX8MM PCIe PHY.
> > - Add the i.MX8MP PCIe support in binding document, DTS files, and PCIe
> > driver.
> >
> > Main changes v1-->v2:
> > - It's my fault forget including Vinod, re-send v2 after include Vinod
> > and [email protected].
> > - List the basements of this patch-set. The branch, codes changes and so on.
> > - Clean up some useless register and bit definitions in #3 patch.
> >
> > [1]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> >
> atchwork.kernel.org%2Fproject%2Flinux-arm-kernel%2Fcover%2F2022022820
> 1
> >
> 73&data=04%7C01%7Chongxing.zhu%40nxp.com%7C5a7b5c3d050242c
> b9aa808d
> >
> a01a26c70%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637824
> 094369636
> >
> 537%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMz
> IiLCJBTi
> >
> I6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=ZXo1zTOOSxOsGe8IMDr1
> axWcqCnf
> > uL8KAaOdIGk1nxI%3D&reserved=0
> > [email protected]/
> > [2]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> >
> atchwork.ozlabs.org%2Fproject%2Flinux-pci%2Fpatch%2F1646289275-17813-
> 1
> >
> -&data=04%7C01%7Chongxing.zhu%40nxp.com%7C5a7b5c3d050242cb9
> aa808da
> >
> 01a26c70%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6378240
> 943696365
> >
> 37%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIi
> LCJBTiI
> >
> 6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=K786yDfjsjIV9QpT6Hyawl
> EkQv%2F
> > hQNVBuXk2UU2p%2BXs%3D&reserved=0>
> > [email protected]/
> > [3]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> >
> atchwork.ozlabs.org%2Fproject%2Flinux-pci%2Fpatch%2F1645672013-8949-1
> -
> >
> g&data=04%7C01%7Chongxing.zhu%40nxp.com%7C5a7b5c3d050242cb
> 9aa808da
> >
> 01a26c70%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6378240
> 943696365
> >
> 37%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIi
> LCJBTiI
> >
> 6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=Dp2GU3iDuTR91Y6Awqbn
> 0AxPvbBe4
> > %2BtPVvylgzFeWOU%3D&reserved=0
> > [email protected]/
> > [4]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> >
> atchwork.ozlabs.org%2Fproject%2Flinux-pci%2Fpatch%2F1646293805-18248-
> 1
> >
> -&data=04%7C01%7Chongxing.zhu%40nxp.com%7C5a7b5c3d050242cb9
> aa808da
> >
> 01a26c70%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6378240
> 943696365
> >
> 37%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIi
> LCJBTiI
> >
> 6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=hsbURaSevBxxRfus%2Bjre
> Ppo96Oi
> > %2Fel%2BSceueYdIay%2B8%3D&reserved=0>
> > [email protected]/
> > [5]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> >
> atchwork.ozlabs.org%2Fproject%2Flinux-pci%2Fcover%2F1645760667-10510-
> 1
> >
> -&data=04%7C01%7Chongxing.zhu%40nxp.com%7C5a7b5c3d050242cb9
> aa808da
> >
> 01a26c70%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6378240
> 943696365
> >
> 37%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIi
> LCJBTiI
> >
> 6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=uYwnISUWBSP1yBxe5qfRT
> a%2F9I7y
> > 8YqfyEZXmts5D9Ys%3D&reserved=0>
> > [email protected]/
> >
> > NOTE:
> > Based git
> > <git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git>
> > Based branch <pci/imx6>
> >
> > Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 1 +
> > Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml | 4 +-
> > arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 55
> > ++++++++++++++++++++++ arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > | 46 ++++++++++++++++++-
> drivers/pci/controller/dwc/pci-imx6.c
> > | 19 +++++++-
> > drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 205
> >
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++-----
> >
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++-----
> > ------- drivers/reset/reset-imx7.c |
> 1 +
> > 7 files changed, 286 insertions(+), 45 deletions(-)
> >
> > [PATCH v2 1/7] reset: imx7: Add the iMX8MP PCIe PHY PERST support
> > [PATCH v2 2/7] dt-binding: phy: Add iMX8MP PCIe PHY binding [PATCH v2
> > 3/7] phy: freescale: imx8m-pcie: Add iMX8MP PCIe PHY [PATCH v2 4/7]
> > dt-bindings: imx6q-pcie: Add iMX8MP PCIe compatible [PATCH v2 5/7]
> > arm64: dts: imx8mp: add the iMX8MP PCIe support [PATCH v2 6/7] arm64:
> > dts: imx8mp-evk: Add PCIe support [PATCH v2 7/7] PCI: imx6: Add the
> > iMX8MP PCIe support
>
>
>
On Mon, 07 Mar 2022 17:07:31 +0800, Richard Zhu wrote:
> Add i.MX8MP PCIe compatible string.
>
> Signed-off-by: Richard Zhu <[email protected]>
> ---
> Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Applied, thanks!
Hello Richard,
thanks for providing PCIe support for iMX8MP.
Am Montag, 7. M?rz 2022, 10:07:33 CET schrieb Richard Zhu:
> Add PCIe support on i.MX8MP EVK board.
>
> Signed-off-by: Richard Zhu <[email protected]>
> ---
> arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 55 ++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts index
> 2eb943210678..ed77455a3f73 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> @@ -5,6 +5,7 @@
>
> /dts-v1/;
>
> +#include <dt-bindings/phy/phy-imx8-pcie.h>
> #include "imx8mp.dtsi"
>
> / {
> @@ -33,6 +34,12 @@ memory@40000000 {
> <0x1 0x00000000 0 0xc0000000>;
> };
>
> + pcie0_refclk: pcie0-refclk {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <100000000>;
> + };
> +
> reg_can1_stby: regulator-can1-stby {
> compatible = "regulator-fixed";
> regulator-name = "can1-stby";
> @@ -55,6 +62,17 @@ reg_can2_stby: regulator-can2-stby {
> enable-active-high;
> };
>
> + reg_pcie0: regulator-pcie {
> + compatible = "regulator-fixed";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_pcie0_reg>;
> + regulator-name = "MPCIE_3V3";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + gpio = <&gpio2 6 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + };
> +
> reg_usdhc2_vmmc: regulator-usdhc2 {
> compatible = "regulator-fixed";
> pinctrl-names = "default";
> @@ -297,6 +315,30 @@ pca6416: gpio@20 {
> };
> };
>
> +&pcie_phy {
> + fsl,refclk-pad-mode = <IMX8_PCIE_REFCLK_PAD_INPUT>;
> + clocks = <&pcie0_refclk>;
> + clock-names = "ref";
> + status = "okay";
> +};
> +
> +&pcie{
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_pcie0>;
> + reset-gpio = <&gpio2 7 GPIO_ACTIVE_LOW>;
> + clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
> + <&clk IMX8MP_CLK_PCIE_ROOT>,
> + <&clk IMX8MP_CLK_HSIO_AXI>;
> + clock-names = "pcie", "pcie_aux", "pcie_bus";
This causes the following warnings in dtbs_check (paths stripped):
imx8mp-evk.dtb: pcie@33800000: clock-names:1: 'pcie_bus' was expected
From schema: Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
imx8mp-evk.dtb: pcie@33800000: clock-names:2: 'pcie_phy' was expected
From schema: Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
The bindings want 4 clocks for imx8mq (and imx8mp which seems similar):
* pcie
* pcie_bus
* pcie_phy
* pcie_aux
Ignoring the order there is no pcie_phy clock anymore, it was removed in
commit 1840518ae7de ("clk: imx8mp: Remove the none exist pcie clocks"). I was
wondering why, because the PCIE_PHY_CLK_ROOT at register 0xa380 inside CCM is
listed in RM.
So there is a clock missing for 'pcie_phy' or the binding needs some update
for imx8mp, no?
Regards,
Alexander
> + assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>,
> + <&clk IMX8MP_CLK_PCIE_AUX>;
> + assigned-clock-rates = <500000000>, <10000000>;
> + assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>,
> + <&clk IMX8MP_SYS_PLL2_50M>;
> + vpcie-supply = <®_pcie0>;
> + status = "okay";
> +};
> +
> &snvs_pwrkey {
> status = "okay";
> };
> @@ -442,6 +484,19 @@ MX8MP_IOMUXC_I2C3_SDA__I2C3_SDA 0x400001c3
>
> >;
>
> };
>
> + pinctrl_pcie0: pcie0grp {
> + fsl,pins = <
> + MX8MP_IOMUXC_I2C4_SCL__PCIE_CLKREQ_B
0x61 /* open drain, pull up */
> + MX8MP_IOMUXC_SD1_DATA5__GPIO2_IO07
0x41
> + >;
> + };
> +
> + pinctrl_pcie0_reg: pcie0reggrp {
> + fsl,pins = <
> + MX8MP_IOMUXC_SD1_DATA4__GPIO2_IO06
0x41
> + >;
> + };
> +
> pinctrl_pmic: pmicgrp {
> fsl,pins = <
> MX8MP_IOMUXC_GPIO1_IO03__GPIO1_IO03
0x000001c0
> -----Original Message-----
> From: Alexander Stein <[email protected]>
> Sent: 2022年3月24日 18:05
> To: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; Hongxing Zhu <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>; Hongxing Zhu <[email protected]>
> Subject: Re: (EXT) [PATCH v2 6/7] arm64: dts: imx8mp-evk: Add PCIe support
>
> Hello Richard,
>
> thanks for providing PCIe support for iMX8MP.
>
> Am Montag, 7. März 2022, 10:07:33 CET schrieb Richard Zhu:
> > Add PCIe support on i.MX8MP EVK board.
> >
> > Signed-off-by: Richard Zhu <[email protected]>
> > ---
> > arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 55
> > ++++++++++++++++++++
> > 1 file changed, 55 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts index
> > 2eb943210678..ed77455a3f73 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > @@ -5,6 +5,7 @@
> >
> > /dts-v1/;
> >
> > +#include <dt-bindings/phy/phy-imx8-pcie.h>
> > #include "imx8mp.dtsi"
> >
> > / {
> > @@ -33,6 +34,12 @@ memory@40000000 {
> > <0x1 0x00000000 0 0xc0000000>;
> > };
> >
> > + pcie0_refclk: pcie0-refclk {
> > + compatible = "fixed-clock";
> > + #clock-cells = <0>;
> > + clock-frequency = <100000000>;
> > + };
> > +
> > reg_can1_stby: regulator-can1-stby {
> > compatible = "regulator-fixed";
> > regulator-name = "can1-stby";
> > @@ -55,6 +62,17 @@ reg_can2_stby: regulator-can2-stby {
> > enable-active-high;
> > };
> >
> > + reg_pcie0: regulator-pcie {
> > + compatible = "regulator-fixed";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_pcie0_reg>;
> > + regulator-name = "MPCIE_3V3";
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > + gpio = <&gpio2 6 GPIO_ACTIVE_HIGH>;
> > + enable-active-high;
> > + };
> > +
> > reg_usdhc2_vmmc: regulator-usdhc2 {
> > compatible = "regulator-fixed";
> > pinctrl-names = "default";
> > @@ -297,6 +315,30 @@ pca6416: gpio@20 {
> > };
> > };
> >
> > +&pcie_phy {
> > + fsl,refclk-pad-mode = <IMX8_PCIE_REFCLK_PAD_INPUT>;
> > + clocks = <&pcie0_refclk>;
> > + clock-names = "ref";
> > + status = "okay";
> > +};
> > +
> > +&pcie{
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_pcie0>;
> > + reset-gpio = <&gpio2 7 GPIO_ACTIVE_LOW>;
> > + clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
> > + <&clk IMX8MP_CLK_PCIE_ROOT>,
> > + <&clk IMX8MP_CLK_HSIO_AXI>;
> > + clock-names = "pcie", "pcie_aux", "pcie_bus";
>
> This causes the following warnings in dtbs_check (paths stripped):
> imx8mp-evk.dtb: pcie@33800000: clock-names:1: 'pcie_bus' was expected
> From schema:
> Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> imx8mp-evk.dtb: pcie@33800000: clock-names:2: 'pcie_phy' was expected
> From schema:
> Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
>
> The bindings want 4 clocks for imx8mq (and imx8mp which seems similar):
> * pcie
> * pcie_bus
> * pcie_phy
> * pcie_aux
>
> Ignoring the order there is no pcie_phy clock anymore, it was removed in
> commit 1840518ae7de ("clk: imx8mp: Remove the none exist pcie clocks"). I
> was wondering why, because the PCIE_PHY_CLK_ROOT at register 0xa380
> inside CCM is listed in RM.
> So there is a clock missing for 'pcie_phy' or the binding needs some update for
> imx8mp, no?
Hi Alexander:
Thanks for your comments.
I had confirmed with design team, the PCIE_PHY_CLK_ROOT at register 0xa380 is
not used anymore on i.MX8MP PCIe. So I used to issue one patch to remove the
useless code from clock driver.
About the dt-binding of the i.MX8MP clocks, I would update the yaml later.
BTW, i.MX8MP PCIe is similar to the i.MX8MM PCIe's.
It's better to make reference to the i.MX8MM PCIe clocks definitions.
Best Regards
Richard Zhu
>
> Regards,
> Alexander
>
> > + assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>,
> > + <&clk IMX8MP_CLK_PCIE_AUX>;
> > + assigned-clock-rates = <500000000>, <10000000>;
> > + assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>,
> > + <&clk IMX8MP_SYS_PLL2_50M>;
> > + vpcie-supply = <®_pcie0>;
> > + status = "okay";
> > +};
> > +
> > &snvs_pwrkey {
> > status = "okay";
> > };
> > @@ -442,6 +484,19 @@ MX8MP_IOMUXC_I2C3_SDA__I2C3_SDA
> 0x400001c3
> >
> > >;
> >
> > };
> >
> > + pinctrl_pcie0: pcie0grp {
> > + fsl,pins = <
> > + MX8MP_IOMUXC_I2C4_SCL__PCIE_CLKREQ_B
> 0x61 /* open drain, pull up */
> > + MX8MP_IOMUXC_SD1_DATA5__GPIO2_IO07
> 0x41
> > + >;
> > + };
> > +
> > + pinctrl_pcie0_reg: pcie0reggrp {
> > + fsl,pins = <
> > + MX8MP_IOMUXC_SD1_DATA4__GPIO2_IO06
> 0x41
> > + >;
> > + };
> > +
> > pinctrl_pmic: pmicgrp {
> > fsl,pins = <
> > MX8MP_IOMUXC_GPIO1_IO03__GPIO1_IO03
> 0x000001c0
>
>
>
Hi Richard,
On Mo, 2022-03-07 at 17:07 +0800, Richard Zhu wrote:
> Add the i.MX8MP PCIe PHY PERST support.
>
> Signed-off-by: Richard Zhu <[email protected]>
> ---
> drivers/reset/reset-imx7.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
> index 185a333df66c..d2408725eb2c 100644
> --- a/drivers/reset/reset-imx7.c
> +++ b/drivers/reset/reset-imx7.c
> @@ -329,6 +329,7 @@ static int imx8mp_reset_set(struct
> reset_controller_dev *rcdev,
> break;
>
> case IMX8MP_RESET_PCIE_CTRL_APPS_EN:
> + case IMX8MP_RESET_PCIEPHY_PERST:
> value = assert ? 0 : bit;
> break;
> }
This doesn't do what the commit description says.
The PCIEPHY_PERST bit is already supported by the driver (albeit
incorrectly?) - this patch just inverts the bit.
Since this bit is not inverted on the other platforms, and the i.MX8MP
reference manual says nothing about this, please explicitly state why
this needs to be inverted and call it a fix in the commit description.
regards
Philipp
On Mon, Mar 7, 2022 at 1:18 AM Richard Zhu <[email protected]> wrote:
>
> Based on the i.MX8MP GPC and blk-ctrl patch-set[1] issued by Lucas and the
> following commits.
> - one codes refine patch-set[5].
> - two Fixes[2],[3].
> - one binding commit[4].
> - some dts changes in Shawn's git if you want to test PCIe on i.MX8MM EVK.
> b4d36c10bf17 arm64: dts: imx8mm-evk: Add the pcie support on imx8mm evk board
> aaeba6a8e226 arm64: dts: imx8mm: Add the pcie support
> cfc5078432ca arm64: dts: imx8mm: Add the pcie phy support
>
> Sorry about that there may be some conflictions when do the codes merge.
> I'm waiting for the ack now, and will re-base them in a proper sequence later.
>
> This series patches add the i.MX8MP PCIe support and tested on i.MX8MM EVK and
> i.MX8MP EVk boards. The PCIe NVME works fine on both boards.
>
> - i.MX8MP PCIe PHY has two resets refer to the i.MX8MM PCIe PHY.
> Add one more PHY reset for i.MX8MP PCIe PHY accordingly.
> - Add the i.MX8MP PCIe PHY support in the i.MX8M PCIe PHY driver.
> And share as much as possible codes with i.MX8MM PCIe PHY.
> - Add the i.MX8MP PCIe support in binding document, DTS files, and PCIe
> driver.
>
> Main changes v1-->v2:
> - It's my fault forget including Vinod, re-send v2 after include Vinod
> and [email protected].
> - List the basements of this patch-set. The branch, codes changes and so on.
> - Clean up some useless register and bit definitions in #3 patch.
>
> [1]https://patchwork.kernel.org/project/linux-arm-kernel/cover/[email protected]/
> [2]https://patchwork.ozlabs.org/project/linux-pci/patch/[email protected]/
> [3]https://patchwork.ozlabs.org/project/linux-pci/patch/[email protected]/
> [4]https://patchwork.ozlabs.org/project/linux-pci/patch/[email protected]/
> [5]https://patchwork.ozlabs.org/project/linux-pci/cover/[email protected]/
>
> NOTE:
> Based git <git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git>
> Based branch <pci/imx6>
>
> Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 1 +
> Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml | 4 +-
> arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 55 ++++++++++++++++++++++
> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 46 ++++++++++++++++++-
> drivers/pci/controller/dwc/pci-imx6.c | 19 +++++++-
> drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 205 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------
> drivers/reset/reset-imx7.c | 1 +
> 7 files changed, 286 insertions(+), 45 deletions(-)
>
> [PATCH v2 1/7] reset: imx7: Add the iMX8MP PCIe PHY PERST support
> [PATCH v2 2/7] dt-binding: phy: Add iMX8MP PCIe PHY binding
> [PATCH v2 3/7] phy: freescale: imx8m-pcie: Add iMX8MP PCIe PHY
> [PATCH v2 4/7] dt-bindings: imx6q-pcie: Add iMX8MP PCIe compatible
> [PATCH v2 5/7] arm64: dts: imx8mp: add the iMX8MP PCIe support
> [PATCH v2 6/7] arm64: dts: imx8mp-evk: Add PCIe support
> [PATCH v2 7/7] PCI: imx6: Add the iMX8MP PCIe support
>
Richard,
Thanks for working on this!
Do you plan on submitting another version soon? I've tried to test
this with an imx8mp board I'm bringing up and while the host
controller enumerates I fail to get a link to a device. It's very
likely I am missing something as this series depends on the IMX8MP
blk-ctrl and gpc series which I also can't cleanly apply. Lucas just
submitted a 'consolidated i.MX8MP HSIO/MEDIA/HDMI blk-ctrl series' [1]
yet I can't find a repo/branch that applies to either.
Perhaps you have a git repo somewhere I can look at while we wait for
imx8mp blk-ctl/gpc to settle and you to submit a v3?
Best Regards,
Tim
[1] https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=629586
> -----Original Message-----
> From: Tim Harvey <[email protected]>
> Sent: 2022??4??8?? 4:42
> To: Hongxing Zhu <[email protected]>; Lucas Stach
> <[email protected]>
> Cc: Philipp Zabel <[email protected]>; [email protected]; Lorenzo
> Pieralisi <[email protected]>; Rob Herring <[email protected]>; Shawn
> Guo <[email protected]>; Vinod Koul <[email protected]>; Alexander Stein
> <[email protected]>; [email protected]; Device
> Tree Mailing List <[email protected]>; [email protected];
> Linux ARM Mailing List <[email protected]>; open list
> <[email protected]>; Sascha Hauer <[email protected]>;
> dl-linux-imx <[email protected]>
> Subject: Re: [PATCH v2 0/7] Add the iMX8MP PCIe support
>
> On Mon, Mar 7, 2022 at 1:18 AM Richard Zhu <[email protected]>
> wrote:
> >
> > Based on the i.MX8MP GPC and blk-ctrl patch-set[1] issued by Lucas and
> > the following commits.
> > - one codes refine patch-set[5].
> > - two Fixes[2],[3].
> > - one binding commit[4].
> > - some dts changes in Shawn's git if you want to test PCIe on i.MX8MM
> EVK.
> > b4d36c10bf17 arm64: dts: imx8mm-evk: Add the pcie support on
> imx8mm evk board
> > aaeba6a8e226 arm64: dts: imx8mm: Add the pcie support
> > cfc5078432ca arm64: dts: imx8mm: Add the pcie phy support
> >
> > Sorry about that there may be some conflictions when do the codes merge.
> > I'm waiting for the ack now, and will re-base them in a proper sequence later.
> >
> > This series patches add the i.MX8MP PCIe support and tested on i.MX8MM
> > EVK and i.MX8MP EVk boards. The PCIe NVME works fine on both boards.
> >
> > - i.MX8MP PCIe PHY has two resets refer to the i.MX8MM PCIe PHY.
> > Add one more PHY reset for i.MX8MP PCIe PHY accordingly.
> > - Add the i.MX8MP PCIe PHY support in the i.MX8M PCIe PHY driver.
> > And share as much as possible codes with i.MX8MM PCIe PHY.
> > - Add the i.MX8MP PCIe support in binding document, DTS files, and PCIe
> > driver.
> >
> > Main changes v1-->v2:
> > - It's my fault forget including Vinod, re-send v2 after include Vinod
> > and [email protected].
> > - List the basements of this patch-set. The branch, codes changes and so on.
> > - Clean up some useless register and bit definitions in #3 patch.
> >
> > [1]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> >
> atchwork.kernel.org%2Fproject%2Flinux-arm-kernel%2Fcover%2F2022022820
> 1
> >
> 731.3330192-1-l.stach%40pengutronix.de%2F&data=04%7C01%7Chongx
> ing.
> >
> zhu%40nxp.com%7C19e85ae119bc47d3397e08da18d71007%7C686ea1d3bc
> 2b4c6fa92
> >
> cd99c5c301635%7C0%7C1%7C637849609225124527%7CUnknown%7CTWF
> pbGZsb3d8eyJ
> >
> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> C3000
> >
> &sdata=namjBp1ZpawS9s25%2FwS8aOnd2A7rHTK2rQRwG4V0Dt8%3D&
> amp;reserv
> > ed=0
> > [2]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> >
> atchwork.ozlabs.org%2Fproject%2Flinux-pci%2Fpatch%2F1646289275-17813-
> 1
> >
> -git-send-email-hongxing.zhu%40nxp.com%2F&data=04%7C01%7Chongxi
> ng.
> >
> zhu%40nxp.com%7C19e85ae119bc47d3397e08da18d71007%7C686ea1d3bc
> 2b4c6fa92
> >
> cd99c5c301635%7C0%7C1%7C637849609225124527%7CUnknown%7CTWF
> pbGZsb3d8eyJ
> >
> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> C3000
> >
> &sdata=dWr1ui7eIc92iWzvo8VKPXTkNel3NR9yNxD5CyHIuV0%3D&r
> eserved
> > =0
> > [3]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> >
> atchwork.ozlabs.org%2Fproject%2Flinux-pci%2Fpatch%2F1645672013-8949-1
> -
> >
> git-send-email-hongxing.zhu%40nxp.com%2F&data=04%7C01%7Chongxi
> ng.z
> >
> hu%40nxp.com%7C19e85ae119bc47d3397e08da18d71007%7C686ea1d3bc2
> b4c6fa92c
> >
> d99c5c301635%7C0%7C1%7C637849609225124527%7CUnknown%7CTWFp
> bGZsb3d8eyJW
> >
> IjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3
> 000&
> >
> amp;sdata=FCis4KE9KZqS8Ou6I0KTQu%2FayWSm%2Ftj%2Bcrd68EThsNs%3D
> &res
> > erved=0
> > [4]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> >
> atchwork.ozlabs.org%2Fproject%2Flinux-pci%2Fpatch%2F1646293805-18248-
> 1
> >
> -git-send-email-hongxing.zhu%40nxp.com%2F&data=04%7C01%7Chongxi
> ng.
> >
> zhu%40nxp.com%7C19e85ae119bc47d3397e08da18d71007%7C686ea1d3bc
> 2b4c6fa92
> >
> cd99c5c301635%7C0%7C1%7C637849609225124527%7CUnknown%7CTWF
> pbGZsb3d8eyJ
> >
> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> C3000
> >
> &sdata=sbYuLpfBFUImVi7YLe%2FCYvQNxleK2tnHKfr%2FByoAJsA%3D&am
> p;rese
> > rved=0
> > [5]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> >
> atchwork.ozlabs.org%2Fproject%2Flinux-pci%2Fcover%2F1645760667-10510-
> 1
> >
> -git-send-email-hongxing.zhu%40nxp.com%2F&data=04%7C01%7Chongxi
> ng.
> >
> zhu%40nxp.com%7C19e85ae119bc47d3397e08da18d71007%7C686ea1d3bc
> 2b4c6fa92
> >
> cd99c5c301635%7C0%7C1%7C637849609225124527%7CUnknown%7CTWF
> pbGZsb3d8eyJ
> >
> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> C3000
> >
> &sdata=tRZQBUN4CleGFFbxqNn4W1kUwCgATERggfa8qEQyc9E%3D&am
> p;reserved
> > =0
> >
> > NOTE:
> > Based git
> > <git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git>
> > Based branch <pci/imx6>
> >
> > Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 1 +
> > Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml | 4 +-
> > arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 55
> ++++++++++++++++++++++
> > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 46
> ++++++++++++++++++-
> > drivers/pci/controller/dwc/pci-imx6.c | 19
> +++++++-
> > drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 205
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++-----------------
> > drivers/reset/reset-imx7.c | 1 +
> > 7 files changed, 286 insertions(+), 45 deletions(-)
> >
> > [PATCH v2 1/7] reset: imx7: Add the iMX8MP PCIe PHY PERST support
> > [PATCH v2 2/7] dt-binding: phy: Add iMX8MP PCIe PHY binding [PATCH v2
> > 3/7] phy: freescale: imx8m-pcie: Add iMX8MP PCIe PHY [PATCH v2 4/7]
> > dt-bindings: imx6q-pcie: Add iMX8MP PCIe compatible [PATCH v2 5/7]
> > arm64: dts: imx8mp: add the iMX8MP PCIe support [PATCH v2 6/7] arm64:
> > dts: imx8mp-evk: Add PCIe support [PATCH v2 7/7] PCI: imx6: Add the
> > iMX8MP PCIe support
> >
>
> Richard,
>
> Thanks for working on this!
>
> Do you plan on submitting another version soon? I've tried to test this with an
> imx8mp board I'm bringing up and while the host controller enumerates I fail
> to get a link to a device. It's very likely I am missing something as this series
> depends on the IMX8MP blk-ctrl and gpc series which I also can't cleanly apply.
> Lucas just submitted a 'consolidated i.MX8MP HSIO/MEDIA/HDMI blk-ctrl
> series' [1] yet I can't find a repo/branch that applies to either.
>
> Perhaps you have a git repo somewhere I can look at while we wait for
> imx8mp blk-ctl/gpc to settle and you to submit a v3?
Hi Tim:
Thanks for your kindly help to do the tests.
I had listed the dependencies in the cover-letter log.
Alexander and I used to test this series commits based on the V5.17 kernel.
Lucas had provided some review comments and suggestions about the PLL bits
manipulations of HSIOMIX in i.MX8MP PCIe PHY driver #3 of this series.
And he suggested to let the HSIOMIX blk-ctrl make this PLL as a real clock,
and used by i.MX8MP PCIe PHY driver later.
Although I have some confusions, it's better let's wating for the blk-ctrl
settle down and get clear discussion with Lucas later.
How do you think about that?
Best Regards
Richard Zhu
>
> Best Regards,
>
> Tim
> [1]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchw
> ork.kernel.org%2Fproject%2Flinux-arm-kernel%2Flist%2F%3Fseries%3D62958
> 6&data=04%7C01%7Chongxing.zhu%40nxp.com%7C19e85ae119bc47d3
> 397e08da18d71007%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7
> C637849609225124527%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sda
> ta=SUCCWtnCtTSCONfSoixOPgpMO4dnsBTW20x9qRdw4Fw%3D&reserve
> d=0
Am Freitag, dem 08.04.2022 um 03:14 +0000 schrieb Hongxing Zhu:
> >
[...]
> > Richard,
> >
> > Thanks for working on this!
> >
> > Do you plan on submitting another version soon? I've tried to test this with an
> > imx8mp board I'm bringing up and while the host controller enumerates I fail
> > to get a link to a device. It's very likely I am missing something as this series
> > depends on the IMX8MP blk-ctrl and gpc series which I also can't cleanly apply.
> > Lucas just submitted a 'consolidated i.MX8MP HSIO/MEDIA/HDMI blk-ctrl
> > series' [1] yet I can't find a repo/branch that applies to either.
> >
> > Perhaps you have a git repo somewhere I can look at while we wait for
> > imx8mp blk-ctl/gpc to settle and you to submit a v3?
> Hi Tim:
> Thanks for your kindly help to do the tests.
> I had listed the dependencies in the cover-letter log.
> Alexander and I used to test this series commits based on the V5.17 kernel.
>
> Lucas had provided some review comments and suggestions about the PLL bits
> manipulations of HSIOMIX in i.MX8MP PCIe PHY driver #3 of this series.
> And he suggested to let the HSIOMIX blk-ctrl make this PLL as a real clock,
> and used by i.MX8MP PCIe PHY driver later.
>
> Although I have some confusions, it's better let's wating for the blk-ctrl
> settle down and get clear discussion with Lucas later.
> How do you think about that?
Just to let you know my plans: I was quite busy with getting the
i.MX8MP HDMI part to work. Now that this is at least in a state where
it can collect some feedback from upstream I have some time to circle
back to this topic. I can't commit to do it immediately, but I'll get
around to looking at the PCIe series a bit more in-depth and apply my
HSIO PLL suggestion to the blk-ctrl driver during the next week.
Regards,
Lucas
> -----Original Message-----
> From: Lucas Stach <[email protected]>
> Sent: 2022年4月8日 16:13
> To: Hongxing Zhu <[email protected]>; [email protected];
> Alexander Stein <[email protected]>
> Cc: Philipp Zabel <[email protected]>; [email protected]; Lorenzo
> Pieralisi <[email protected]>; Rob Herring <[email protected]>; Shawn
> Guo <[email protected]>; Vinod Koul <[email protected]>;
> [email protected]; Device Tree Mailing List
> <[email protected]>; [email protected]; Linux ARM Mailing
> List <[email protected]>; open list
> <[email protected]>; Sascha Hauer <[email protected]>;
> dl-linux-imx <[email protected]>
> Subject: Re: [PATCH v2 0/7] Add the iMX8MP PCIe support
>
> Am Freitag, dem 08.04.2022 um 03:14 +0000 schrieb Hongxing Zhu:
> > >
> [...]
> > > Richard,
> > >
> > > Thanks for working on this!
> > >
> > > Do you plan on submitting another version soon? I've tried to test
> > > this with an imx8mp board I'm bringing up and while the host
> > > controller enumerates I fail to get a link to a device. It's very
> > > likely I am missing something as this series depends on the IMX8MP blk-ctrl
> and gpc series which I also can't cleanly apply.
> > > Lucas just submitted a 'consolidated i.MX8MP HSIO/MEDIA/HDMI
> > > blk-ctrl series' [1] yet I can't find a repo/branch that applies to either.
> > >
> > > Perhaps you have a git repo somewhere I can look at while we wait
> > > for imx8mp blk-ctl/gpc to settle and you to submit a v3?
> > Hi Tim:
> > Thanks for your kindly help to do the tests.
> > I had listed the dependencies in the cover-letter log.
> > Alexander and I used to test this series commits based on the V5.17 kernel.
> >
> > Lucas had provided some review comments and suggestions about the PLL
> > bits
> > manipulations of HSIOMIX in i.MX8MP PCIe PHY driver #3 of this series.
> > And he suggested to let the HSIOMIX blk-ctrl make this PLL as a real
> > clock,
> > and used by i.MX8MP PCIe PHY driver later.
> >
> > Although I have some confusions, it's better let's wating for the
> > blk-ctrl settle down and get clear discussion with Lucas later.
> > How do you think about that?
>
> Just to let you know my plans: I was quite busy with getting the i.MX8MP
> HDMI part to work. Now that this is at least in a state where it can collect
> some feedback from upstream I have some time to circle back to this topic. I
> can't commit to do it immediately, but I'll get around to looking at the PCIe
> series a bit more in-depth and apply my HSIO PLL suggestion to the blk-ctrl
> driver during the next week.
Hi Lucas:
Thanks for your helpful reply.
I know that you're busy with the blk-ctrl settle down now, and had explained to
Tim.
Best Regards
Richard Zhu
>
> Regards,
> Lucas
On Thu, Apr 7, 2022 at 8:14 PM Hongxing Zhu <[email protected]> wrote:
>
>
> > -----Original Message-----
> > From: Tim Harvey <[email protected]>
> > Sent: 2022年4月8日 4:42
> > To: Hongxing Zhu <[email protected]>; Lucas Stach
> > <[email protected]>
> > Cc: Philipp Zabel <[email protected]>; [email protected]; Lorenzo
> > Pieralisi <[email protected]>; Rob Herring <[email protected]>; Shawn
> > Guo <[email protected]>; Vinod Koul <[email protected]>; Alexander Stein
> > <[email protected]>; [email protected]; Device
> > Tree Mailing List <[email protected]>; [email protected];
> > Linux ARM Mailing List <[email protected]>; open list
> > <[email protected]>; Sascha Hauer <[email protected]>;
> > dl-linux-imx <[email protected]>
> > Subject: Re: [PATCH v2 0/7] Add the iMX8MP PCIe support
> >
> > On Mon, Mar 7, 2022 at 1:18 AM Richard Zhu <[email protected]>
> > wrote:
> > >
> > > Based on the i.MX8MP GPC and blk-ctrl patch-set[1] issued by Lucas and
> > > the following commits.
> > > - one codes refine patch-set[5].
> > > - two Fixes[2],[3].
> > > - one binding commit[4].
> > > - some dts changes in Shawn's git if you want to test PCIe on i.MX8MM
> > EVK.
> > > b4d36c10bf17 arm64: dts: imx8mm-evk: Add the pcie support on
> > imx8mm evk board
> > > aaeba6a8e226 arm64: dts: imx8mm: Add the pcie support
> > > cfc5078432ca arm64: dts: imx8mm: Add the pcie phy support
> > >
> > > Sorry about that there may be some conflictions when do the codes merge.
> > > I'm waiting for the ack now, and will re-base them in a proper sequence later.
> > >
> > > This series patches add the i.MX8MP PCIe support and tested on i.MX8MM
> > > EVK and i.MX8MP EVk boards. The PCIe NVME works fine on both boards.
> > >
> > > - i.MX8MP PCIe PHY has two resets refer to the i.MX8MM PCIe PHY.
> > > Add one more PHY reset for i.MX8MP PCIe PHY accordingly.
> > > - Add the i.MX8MP PCIe PHY support in the i.MX8M PCIe PHY driver.
> > > And share as much as possible codes with i.MX8MM PCIe PHY.
> > > - Add the i.MX8MP PCIe support in binding document, DTS files, and PCIe
> > > driver.
> > >
> > > Main changes v1-->v2:
> > > - It's my fault forget including Vinod, re-send v2 after include Vinod
> > > and [email protected].
> > > - List the basements of this patch-set. The branch, codes changes and so on.
> > > - Clean up some useless register and bit definitions in #3 patch.
> > >
> > > [1]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> > >
> > atchwork.kernel.org%2Fproject%2Flinux-arm-kernel%2Fcover%2F2022022820
> > 1
> > >
> > 731.3330192-1-l.stach%40pengutronix.de%2F&data=04%7C01%7Chongx
> > ing.
> > >
> > zhu%40nxp.com%7C19e85ae119bc47d3397e08da18d71007%7C686ea1d3bc
> > 2b4c6fa92
> > >
> > cd99c5c301635%7C0%7C1%7C637849609225124527%7CUnknown%7CTWF
> > pbGZsb3d8eyJ
> > >
> > WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> > C3000
> > >
> > &sdata=namjBp1ZpawS9s25%2FwS8aOnd2A7rHTK2rQRwG4V0Dt8%3D&
> > amp;reserv
> > > ed=0
> > > [2]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> > >
> > atchwork.ozlabs.org%2Fproject%2Flinux-pci%2Fpatch%2F1646289275-17813-
> > 1
> > >
> > -git-send-email-hongxing.zhu%40nxp.com%2F&data=04%7C01%7Chongxi
> > ng.
> > >
> > zhu%40nxp.com%7C19e85ae119bc47d3397e08da18d71007%7C686ea1d3bc
> > 2b4c6fa92
> > >
> > cd99c5c301635%7C0%7C1%7C637849609225124527%7CUnknown%7CTWF
> > pbGZsb3d8eyJ
> > >
> > WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> > C3000
> > >
> > &sdata=dWr1ui7eIc92iWzvo8VKPXTkNel3NR9yNxD5CyHIuV0%3D&r
> > eserved
> > > =0
> > > [3]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> > >
> > atchwork.ozlabs.org%2Fproject%2Flinux-pci%2Fpatch%2F1645672013-8949-1
> > -
> > >
> > git-send-email-hongxing.zhu%40nxp.com%2F&data=04%7C01%7Chongxi
> > ng.z
> > >
> > hu%40nxp.com%7C19e85ae119bc47d3397e08da18d71007%7C686ea1d3bc2
> > b4c6fa92c
> > >
> > d99c5c301635%7C0%7C1%7C637849609225124527%7CUnknown%7CTWFp
> > bGZsb3d8eyJW
> > >
> > IjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3
> > 000&
> > >
> > amp;sdata=FCis4KE9KZqS8Ou6I0KTQu%2FayWSm%2Ftj%2Bcrd68EThsNs%3D
> > &res
> > > erved=0
> > > [4]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> > >
> > atchwork.ozlabs.org%2Fproject%2Flinux-pci%2Fpatch%2F1646293805-18248-
> > 1
> > >
> > -git-send-email-hongxing.zhu%40nxp.com%2F&data=04%7C01%7Chongxi
> > ng.
> > >
> > zhu%40nxp.com%7C19e85ae119bc47d3397e08da18d71007%7C686ea1d3bc
> > 2b4c6fa92
> > >
> > cd99c5c301635%7C0%7C1%7C637849609225124527%7CUnknown%7CTWF
> > pbGZsb3d8eyJ
> > >
> > WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> > C3000
> > >
> > &sdata=sbYuLpfBFUImVi7YLe%2FCYvQNxleK2tnHKfr%2FByoAJsA%3D&am
> > p;rese
> > > rved=0
> > > [5]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> > >
> > atchwork.ozlabs.org%2Fproject%2Flinux-pci%2Fcover%2F1645760667-10510-
> > 1
> > >
> > -git-send-email-hongxing.zhu%40nxp.com%2F&data=04%7C01%7Chongxi
> > ng.
> > >
> > zhu%40nxp.com%7C19e85ae119bc47d3397e08da18d71007%7C686ea1d3bc
> > 2b4c6fa92
> > >
> > cd99c5c301635%7C0%7C1%7C637849609225124527%7CUnknown%7CTWF
> > pbGZsb3d8eyJ
> > >
> > WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> > C3000
> > >
> > &sdata=tRZQBUN4CleGFFbxqNn4W1kUwCgATERggfa8qEQyc9E%3D&am
> > p;reserved
> > > =0
> > >
> > > NOTE:
> > > Based git
> > > <git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git>
> > > Based branch <pci/imx6>
> > >
> > > Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 1 +
> > > Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml | 4 +-
> > > arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 55
> > ++++++++++++++++++++++
> > > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 46
> > ++++++++++++++++++-
> > > drivers/pci/controller/dwc/pci-imx6.c | 19
> > +++++++-
> > > drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 205
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > ++++-----------------
> > > drivers/reset/reset-imx7.c | 1 +
> > > 7 files changed, 286 insertions(+), 45 deletions(-)
> > >
> > > [PATCH v2 1/7] reset: imx7: Add the iMX8MP PCIe PHY PERST support
> > > [PATCH v2 2/7] dt-binding: phy: Add iMX8MP PCIe PHY binding [PATCH v2
> > > 3/7] phy: freescale: imx8m-pcie: Add iMX8MP PCIe PHY [PATCH v2 4/7]
> > > dt-bindings: imx6q-pcie: Add iMX8MP PCIe compatible [PATCH v2 5/7]
> > > arm64: dts: imx8mp: add the iMX8MP PCIe support [PATCH v2 6/7] arm64:
> > > dts: imx8mp-evk: Add PCIe support [PATCH v2 7/7] PCI: imx6: Add the
> > > iMX8MP PCIe support
> > >
> >
> > Richard,
> >
> > Thanks for working on this!
> >
> > Do you plan on submitting another version soon? I've tried to test this with an
> > imx8mp board I'm bringing up and while the host controller enumerates I fail
> > to get a link to a device. It's very likely I am missing something as this series
> > depends on the IMX8MP blk-ctrl and gpc series which I also can't cleanly apply.
> > Lucas just submitted a 'consolidated i.MX8MP HSIO/MEDIA/HDMI blk-ctrl
> > series' [1] yet I can't find a repo/branch that applies to either.
> >
Richard,
I found that I had an issue with PERST# on my board which was causing
the link failure so I was able to get this series to work after
figuring out which patches were needed.
> > Perhaps you have a git repo somewhere I can look at while we wait for
> > imx8mp blk-ctl/gpc to settle and you to submit a v3?
> Hi Tim:
> Thanks for your kindly help to do the tests.
> I had listed the dependencies in the cover-letter log.
> Alexander and I used to test this series commits based on the V5.17 kernel.
>
> Lucas had provided some review comments and suggestions about the PLL bits
> manipulations of HSIOMIX in i.MX8MP PCIe PHY driver #3 of this series.
> And he suggested to let the HSIOMIX blk-ctrl make this PLL as a real clock,
> and used by i.MX8MP PCIe PHY driver later.
>
> Although I have some confusions, it's better let's wating for the blk-ctrl
> settle down and get clear discussion with Lucas later.
> How do you think about that?
>
Yes, I agree.
Please Cc me on your next submission and I can test with the
imx8mp-venice-gw74xx board which uses an external REFCLK and does not
support CLKREQ.
Best Regards,
Tim
> Best Regards
> Richard Zhu
> >
> > Best Regards,
> >
> > Tim
> > [1]
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchw
> > ork.kernel.org%2Fproject%2Flinux-arm-kernel%2Flist%2F%3Fseries%3D62958
> > 6&data=04%7C01%7Chongxing.zhu%40nxp.com%7C19e85ae119bc47d3
> > 397e08da18d71007%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7
> > C637849609225124527%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> > MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sda
> > ta=SUCCWtnCtTSCONfSoixOPgpMO4dnsBTW20x9qRdw4Fw%3D&reserve
> > d=0
Hi Richard,
Am Montag, dem 11.04.2022 um 03:32 +0000 schrieb Hongxing Zhu:
> > -----Original Message-----
> > From: Lucas Stach <[email protected]>
> > Sent: 2022年4月8日 16:13
> > To: Hongxing Zhu <[email protected]>; [email protected];
> > Alexander Stein <[email protected]>
> > Cc: Philipp Zabel <[email protected]>; [email protected];
> > Lorenzo
> > Pieralisi <[email protected]>; Rob Herring
> > <[email protected]>; Shawn
> > Guo <[email protected]>; Vinod Koul <[email protected]>;
> > [email protected]; Device Tree Mailing List
> > <[email protected]>; [email protected]; Linux ARM
> > Mailing
> > List <[email protected]>; open list
> > <[email protected]>; Sascha Hauer
> > <[email protected]>;
> > dl-linux-imx <[email protected]>
> > Subject: Re: [PATCH v2 0/7] Add the iMX8MP PCIe support
> >
> > Am Freitag, dem 08.04.2022 um 03:14 +0000 schrieb Hongxing Zhu:
> > > >
> > [...]
> > > > Richard,
> > > >
> > > > Thanks for working on this!
> > > >
> > > > Do you plan on submitting another version soon? I've tried to
> > > > test
> > > > this with an imx8mp board I'm bringing up and while the host
> > > > controller enumerates I fail to get a link to a device. It's
> > > > very
> > > > likely I am missing something as this series depends on the
> > > > IMX8MP blk-ctrl
> > and gpc series which I also can't cleanly apply.
> > > > Lucas just submitted a 'consolidated i.MX8MP HSIO/MEDIA/HDMI
> > > > blk-ctrl series' [1] yet I can't find a repo/branch that
> > > > applies to either.
> > > >
> > > > Perhaps you have a git repo somewhere I can look at while we
> > > > wait
> > > > for imx8mp blk-ctl/gpc to settle and you to submit a v3?
> > > Hi Tim:
> > > Thanks for your kindly help to do the tests.
> > > I had listed the dependencies in the cover-letter log.
> > > Alexander and I used to test this series commits based on the
> > > V5.17 kernel.
> > >
> > > Lucas had provided some review comments and suggestions about the
> > > PLL
> > > bits
> > > manipulations of HSIOMIX in i.MX8MP PCIe PHY driver #3 of this
> > > series.
> > > And he suggested to let the HSIOMIX blk-ctrl make this PLL as a
> > > real
> > > clock,
> > > and used by i.MX8MP PCIe PHY driver later.
> > >
> > > Although I have some confusions, it's better let's wating for the
> > > blk-ctrl settle down and get clear discussion with Lucas later.
> > > How do you think about that?
> >
> > Just to let you know my plans: I was quite busy with getting the
> > i.MX8MP
> > HDMI part to work. Now that this is at least in a state where it
> > can collect
> > some feedback from upstream I have some time to circle back to this
> > topic. I
> > can't commit to do it immediately, but I'll get around to looking
> > at the PCIe
> > series a bit more in-depth and apply my HSIO PLL suggestion to the
> > blk-ctrl
> > driver during the next week.
>
> Hi Lucas:
> Thanks for your helpful reply.
> I know that you're busy with the blk-ctrl settle down now, and had
> explained to
> Tim.
Yesterday I tried to get this series working on my 8MP-EVK board, but
was unsuccessful for now. The PCIe link does not come up. PCIe REF_CLK
is present and PERST de-asserted. I'm not sure if this is due to a
faulty EP device (I don't have another M.2 KeyE device around to check)
or if this may be due to the old revision of my 8MP-EVK, as I'm still
working with a RevA board. The schematic revision log doesn't show
anything that would obviously influence PCIe operation, but I have also
not found any schematics for RevA.
Do you know if there have been any PCIe related changes/fixes in the
later EVK board revisions?
For now I'll try to get this working on a custom i.MX8MP board, where I
can use a different EP device. Maybe I'll have a bit more luck there.
Regards,
Lucas
> -----Original Message-----
> From: Lucas Stach <[email protected]>
> Sent: 2022年4月13日 15:22
> To: Hongxing Zhu <[email protected]>; [email protected];
> Alexander Stein <[email protected]>
> Cc: Philipp Zabel <[email protected]>; [email protected]; Lorenzo
> Pieralisi <[email protected]>; Rob Herring <[email protected]>; Shawn
> Guo <[email protected]>; Vinod Koul <[email protected]>;
> [email protected]; Device Tree Mailing List
> <[email protected]>; [email protected]; Linux ARM Mailing
> List <[email protected]>; open list
> <[email protected]>; Sascha Hauer <[email protected]>;
> dl-linux-imx <[email protected]>
> Subject: Re: [PATCH v2 0/7] Add the iMX8MP PCIe support
>
> Hi Richard,
>
> Am Montag, dem 11.04.2022 um 03:32 +0000 schrieb Hongxing Zhu:
> > > -----Original Message-----
> > > From: Lucas Stach <[email protected]>
> > > Sent: 2022年4月8日 16:13
> > > To: Hongxing Zhu <[email protected]>; [email protected];
> > > Alexander Stein <[email protected]>
> > > Cc: Philipp Zabel <[email protected]>; [email protected];
> > > Lorenzo Pieralisi <[email protected]>; Rob Herring
> > > <[email protected]>; Shawn Guo <[email protected]>; Vinod Koul
> > > <[email protected]>; [email protected]; Device Tree
> > > Mailing List <[email protected]>;
> > > [email protected]; Linux ARM Mailing List
> > > <[email protected]>; open list
> > > <[email protected]>; Sascha Hauer
> > > <[email protected]>; dl-linux-imx <[email protected]>
> > > Subject: Re: [PATCH v2 0/7] Add the iMX8MP PCIe support
> > >
> > > Am Freitag, dem 08.04.2022 um 03:14 +0000 schrieb Hongxing Zhu:
> > > > >
> > > [...]
> > > > > Richard,
> > > > >
> > > > > Thanks for working on this!
> > > > >
> > > > > Do you plan on submitting another version soon? I've tried to
> > > > > test this with an imx8mp board I'm bringing up and while the
> > > > > host controller enumerates I fail to get a link to a device.
> > > > > It's very likely I am missing something as this series depends
> > > > > on the IMX8MP blk-ctrl
> > > and gpc series which I also can't cleanly apply.
> > > > > Lucas just submitted a 'consolidated i.MX8MP HSIO/MEDIA/HDMI
> > > > > blk-ctrl series' [1] yet I can't find a repo/branch that applies
> > > > > to either.
> > > > >
> > > > > Perhaps you have a git repo somewhere I can look at while we
> > > > > wait for imx8mp blk-ctl/gpc to settle and you to submit a v3?
> > > > Hi Tim:
> > > > Thanks for your kindly help to do the tests.
> > > > I had listed the dependencies in the cover-letter log.
> > > > Alexander and I used to test this series commits based on the
> > > > V5.17 kernel.
> > > >
> > > > Lucas had provided some review comments and suggestions about the
> > > > PLL bits
> > > > manipulations of HSIOMIX in i.MX8MP PCIe PHY driver #3 of this
> > > > series.
> > > > And he suggested to let the HSIOMIX blk-ctrl make this PLL as a
> > > > real clock,
> > > > and used by i.MX8MP PCIe PHY driver later.
> > > >
> > > > Although I have some confusions, it's better let's wating for the
> > > > blk-ctrl settle down and get clear discussion with Lucas later.
> > > > How do you think about that?
> > >
> > > Just to let you know my plans: I was quite busy with getting the
> > > i.MX8MP HDMI part to work. Now that this is at least in a state
> > > where it can collect some feedback from upstream I have some time to
> > > circle back to this topic. I can't commit to do it immediately, but
> > > I'll get around to looking at the PCIe series a bit more in-depth
> > > and apply my HSIO PLL suggestion to the blk-ctrl driver during the
> > > next week.
> >
> > Hi Lucas:
> > Thanks for your helpful reply.
> > I know that you're busy with the blk-ctrl settle down now, and had
> > explained to Tim.
>
> Yesterday I tried to get this series working on my 8MP-EVK board, but was
> unsuccessful for now. The PCIe link does not come up. PCIe REF_CLK is present
> and PERST de-asserted. I'm not sure if this is due to a faulty EP device (I don't
> have another M.2 KeyE device around to check) or if this may be due to the old
> revision of my 8MP-EVK, as I'm still working with a RevA board. The schematic
> revision log doesn't show anything that would obviously influence PCIe
> operation, but I have also not found any schematics for RevA.
>
> Do you know if there have been any PCIe related changes/fixes in the later EVK
> board revisions?
>
Hi Lucas:
First of all, thanks a lot for your kindly help on this series.
I remember that there is one PCIe related HW rework used to support the NVME
device when Rev A board is used.
Here is description of this rework and hope it's helpful.
Support High power consumption NVMe SSD
Current EVK has limitation to support high power consumption NVMe SSD, transient
voltage drop is high, need to change R452 to 0ohm to solve this issue.
Rework Steps:
Change R452 to 470-75394 0ohm, 0402
BTW, R452 is placed just at the right side of the M.2 slot.
Best Regards
Richard Zhu
> For now I'll try to get this working on a custom i.MX8MP board, where I can
> use a different EP device. Maybe I'll have a bit more luck there.
>
> Regards,
> Lucas
Am Montag, dem 07.03.2022 um 17:07 +0800 schrieb Richard Zhu:
> Add the i.MX8MP PCIe PHY PERST support.
As Philipp said: please add some more description on why this is
necessary. As far as I can see the reset is already present on 8MQ, and
is low-active, like this patch claims. We just didn't handle this reset
at all on other SoCs as the power on de-asserted state was okay to get
things working.
Regards,
Lucas
>
> Signed-off-by: Richard Zhu <[email protected]>
> ---
> drivers/reset/reset-imx7.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
> index 185a333df66c..d2408725eb2c 100644
> --- a/drivers/reset/reset-imx7.c
> +++ b/drivers/reset/reset-imx7.c
> @@ -329,6 +329,7 @@ static int imx8mp_reset_set(struct reset_controller_dev *rcdev,
> break;
>
> case IMX8MP_RESET_PCIE_CTRL_APPS_EN:
> + case IMX8MP_RESET_PCIEPHY_PERST:
> value = assert ? 0 : bit;
> break;
> }
Am Montag, dem 07.03.2022 um 17:07 +0800 schrieb Richard Zhu:
> Add the i.MX8MP PCIe support.
>
> Signed-off-by: Richard Zhu <[email protected]>
> ---
> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 46 ++++++++++++++++++++++-
> 1 file changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index b40a5646f205..e7b3d8029e34 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -5,6 +5,7 @@
>
> #include <dt-bindings/clock/imx8mp-clock.h>
> #include <dt-bindings/power/imx8mp-power.h>
> +#include <dt-bindings/reset/imx8mp-reset.h>
> #include <dt-bindings/gpio/gpio.h>
> #include <dt-bindings/input/input.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> @@ -375,7 +376,8 @@ iomuxc: pinctrl@30330000 {
> };
>
> gpr: iomuxc-gpr@30340000 {
> - compatible = "fsl,imx8mp-iomuxc-gpr", "syscon";
> + compatible = "fsl,imx8mp-iomuxc-gpr",
> + "fsl,imx6q-iomuxc-gpr", "syscon";
> reg = <0x30340000 0x10000>;
> };
>
> @@ -965,6 +967,17 @@ aips4: bus@32c00000 {
> #size-cells = <1>;
> ranges;
>
> + pcie_phy: pcie-phy@32f00000 {
> + compatible = "fsl,imx8mp-pcie-phy";
> + reg = <0x32f00000 0x10000>;
> + resets = <&src IMX8MP_RESET_PCIEPHY>,
> + <&src IMX8MP_RESET_PCIEPHY_PERST>;
> + reset-names = "pciephy", "perst";
> + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_PCIE_PHY>;
> + #phy-cells = <0>;
> + status = "disabled";
> + };
> +
> hsio_blk_ctrl: blk-ctrl@32f10000 {
> compatible = "fsl,imx8mp-hsio-blk-ctrl", "syscon";
> reg = <0x32f10000 0x24>;
> @@ -980,6 +993,37 @@ hsio_blk_ctrl: blk-ctrl@32f10000 {
> };
> };
>
> + pcie: pcie@33800000 {
> + compatible = "fsl,imx8mp-pcie";
> + reg = <0x33800000 0x400000>, <0x1ff00000 0x80000>;
> + reg-names = "dbi", "config";
> + #address-cells = <3>;
> + #size-cells = <2>;
> + device_type = "pci";
> + bus-range = <0x00 0xff>;
> + ranges = <0x81000000 0 0x00000000 0x1ff80000 0 0x00010000 /* downstream I/O 64KB */
> + 0x82000000 0 0x18000000 0x18000000 0 0x07f00000>; /* non-prefetchable memory */
> + num-lanes = <1>;
> + num-viewport = <4>;
> + interrupts = <GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "msi";
> + #interrupt-cells = <1>;
> + interrupt-map-mask = <0 0 0 0x7>;
> + interrupt-map = <0 0 0 1 &gic GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>,
> + <0 0 0 2 &gic GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
> + <0 0 0 3 &gic GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>,
> + <0 0 0 4 &gic GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
> + fsl,max-link-speed = <3>;
I believe that imx6_pcie_start_link does not properly handle Gen3
speeds.
Regards,
Lucas
> + linux,pci-domain = <0>;
> + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_PCIE>;
> + resets = <&src IMX8MP_RESET_PCIE_CTRL_APPS_EN>,
> + <&src IMX8MP_RESET_PCIE_CTRL_APPS_TURNOFF>;
> + reset-names = "apps", "turnoff";
> + phys = <&pcie_phy>;
> + phy-names = "pcie-phy";
> + status = "disabled";
> + };
> +
> gpu3d: gpu@38000000 {
> compatible = "vivante,gc";
> reg = <0x38000000 0x8000>;
Am Montag, dem 07.03.2022 um 17:07 +0800 schrieb Richard Zhu:
> Add PCIe support on i.MX8MP EVK board.
>
> Signed-off-by: Richard Zhu <[email protected]>
> ---
> arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 55 ++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> index 2eb943210678..ed77455a3f73 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> @@ -5,6 +5,7 @@
>
> /dts-v1/;
>
> +#include <dt-bindings/phy/phy-imx8-pcie.h>
> #include "imx8mp.dtsi"
>
> / {
> @@ -33,6 +34,12 @@ memory@40000000 {
> <0x1 0x00000000 0 0xc0000000>;
> };
>
> + pcie0_refclk: pcie0-refclk {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <100000000>;
> + };
> +
> reg_can1_stby: regulator-can1-stby {
> compatible = "regulator-fixed";
> regulator-name = "can1-stby";
> @@ -55,6 +62,17 @@ reg_can2_stby: regulator-can2-stby {
> enable-active-high;
> };
>
> + reg_pcie0: regulator-pcie {
> + compatible = "regulator-fixed";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_pcie0_reg>;
> + regulator-name = "MPCIE_3V3";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + gpio = <&gpio2 6 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + };
> +
> reg_usdhc2_vmmc: regulator-usdhc2 {
> compatible = "regulator-fixed";
> pinctrl-names = "default";
> @@ -297,6 +315,30 @@ pca6416: gpio@20 {
> };
> };
>
> +&pcie_phy {
> + fsl,refclk-pad-mode = <IMX8_PCIE_REFCLK_PAD_INPUT>;
> + clocks = <&pcie0_refclk>;
> + clock-names = "ref";
> + status = "okay";
> +};
> +
> +&pcie{
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_pcie0>;
> + reset-gpio = <&gpio2 7 GPIO_ACTIVE_LOW>;
> + clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
> + <&clk IMX8MP_CLK_PCIE_ROOT>,
> + <&clk IMX8MP_CLK_HSIO_AXI>;
> + clock-names = "pcie", "pcie_aux", "pcie_bus";
> + assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>,
> + <&clk IMX8MP_CLK_PCIE_AUX>;
> + assigned-clock-rates = <500000000>, <10000000>;
> + assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>,
You don't need to set the IMX8MP_CLK_HSIO_AXI clock rate here, that's
already don't by the power-domain, as it is keeping this bus clock
active. Only need to set the PCIE_AUX rate here.
Regards,
Lucas
> + <&clk IMX8MP_SYS_PLL2_50M>;
> + vpcie-supply = <®_pcie0>;
> + status = "okay";
> +};
> +
> &snvs_pwrkey {
> status = "okay";
> };
> @@ -442,6 +484,19 @@ MX8MP_IOMUXC_I2C3_SDA__I2C3_SDA 0x400001c3
> >;
> };
>
> + pinctrl_pcie0: pcie0grp {
> + fsl,pins = <
> + MX8MP_IOMUXC_I2C4_SCL__PCIE_CLKREQ_B 0x61 /* open drain, pull up */
> + MX8MP_IOMUXC_SD1_DATA5__GPIO2_IO07 0x41
> + >;
> + };
> +
> + pinctrl_pcie0_reg: pcie0reggrp {
> + fsl,pins = <
> + MX8MP_IOMUXC_SD1_DATA4__GPIO2_IO06 0x41
> + >;
> + };
> +
> pinctrl_pmic: pmicgrp {
> fsl,pins = <
> MX8MP_IOMUXC_GPIO1_IO03__GPIO1_IO03 0x000001c0
Am Montag, dem 07.03.2022 um 17:07 +0800 schrieb Richard Zhu:
> Add the i.MX8MP PCIe PHY support
>
> Signed-off-by: Richard Zhu <[email protected]>
> ---
> drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 205 ++++++++++++++++-----
> 1 file changed, 163 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> index 04b1aafb29f4..3d01da4323a6 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> @@ -11,6 +11,8 @@
> #include <linux/mfd/syscon.h>
> #include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
> #include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> #include <linux/phy/phy.h>
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
> @@ -30,12 +32,10 @@
> #define IMX8MM_PCIE_PHY_CMN_REG065 0x194
> #define ANA_AUX_RX_TERM (BIT(7) | BIT(4))
> #define ANA_AUX_TX_LVL GENMASK(3, 0)
> -#define IMX8MM_PCIE_PHY_CMN_REG75 0x1D4
> -#define PCIE_PHY_CMN_REG75_PLL_DONE 0x3
> +#define IMX8MM_PCIE_PHY_CMN_REG075 0x1D4
> +#define ANA_PLL_DONE 0x3
Why do you drop the register prefix from the name here?
> #define PCIE_PHY_TRSV_REG5 0x414
> -#define PCIE_PHY_TRSV_REG5_GEN1_DEEMP 0x2D
> #define PCIE_PHY_TRSV_REG6 0x418
> -#define PCIE_PHY_TRSV_REG6_GEN2_DEEMP 0xF
>
> #define IMX8MM_GPR_PCIE_REF_CLK_SEL GENMASK(25, 24)
> #define IMX8MM_GPR_PCIE_REF_CLK_PLL FIELD_PREP(IMX8MM_GPR_PCIE_REF_CLK_SEL, 0x3)
> @@ -46,16 +46,43 @@
> #define IMX8MM_GPR_PCIE_SSC_EN BIT(16)
> #define IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE BIT(9)
>
> +#define IMX8MP_GPR_REG0 0x0
> +#define IMX8MP_GPR_CLK_MOD_EN BIT(0)
> +#define IMX8MP_GPR_PHY_APB_RST BIT(4)
> +#define IMX8MP_GPR_PHY_INIT_RST BIT(5)
> +#define IMX8MP_GPR_REG1 0x4
> +#define IMX8MP_GPR_PM_EN_CORE_CLK BIT(0)
> +#define IMX8MP_GPR_PLL_LOCK BIT(13)
> +#define IMX8MP_GPR_REG2 0x8
> +#define IMX8MP_GPR_P_PLL_MASK GENMASK(5, 0)
> +#define IMX8MP_GPR_M_PLL_MASK GENMASK(15, 6)
> +#define IMX8MP_GPR_S_PLL_MASK GENMASK(18, 16)
> +#define IMX8MP_GPR_P_PLL (0xc << 0)
> +#define IMX8MP_GPR_M_PLL (0x320 << 6)
> +#define IMX8MP_GPR_S_PLL (0x4 << 16)
> +#define IMX8MP_GPR_REG3 0xc
> +#define IMX8MP_GPR_PLL_CKE BIT(17)
> +#define IMX8MP_GPR_PLL_RST BIT(31)
> +
> +enum imx8_pcie_phy_type {
> + IMX8MM,
> + IMX8MP,
> +};
> +
> struct imx8_pcie_phy {
> void __iomem *base;
> + struct device *dev;
> struct clk *clk;
> struct phy *phy;
> + struct regmap *hsio_blk_ctrl;
> struct regmap *iomuxc_gpr;
> struct reset_control *reset;
> + struct reset_control *perst;
> u32 refclk_pad_mode;
> u32 tx_deemph_gen1;
> u32 tx_deemph_gen2;
> bool clkreq_unused;
> + enum imx8_pcie_phy_type variant;
> };
>
> static int imx8_pcie_phy_init(struct phy *phy)
> @@ -67,6 +94,87 @@ static int imx8_pcie_phy_init(struct phy *phy)
> reset_control_assert(imx8_phy->reset);
>
> pad_mode = imx8_phy->refclk_pad_mode;
> + switch (imx8_phy->variant) {
> + case IMX8MM:
> + /* Tune PHY de-emphasis setting to pass PCIe compliance. */
> + if (imx8_phy->tx_deemph_gen1)
> + writel(imx8_phy->tx_deemph_gen1,
> + imx8_phy->base + PCIE_PHY_TRSV_REG5);
> + if (imx8_phy->tx_deemph_gen2)
> + writel(imx8_phy->tx_deemph_gen2,
> + imx8_phy->base + PCIE_PHY_TRSV_REG6);
> + break;
> + case IMX8MP:
> + reset_control_assert(imx8_phy->perst);
Could you tell us something more about this reset. What exactly is it
resetting. Do we really need to assert it before starting the HSIO PLL?
AFAICS the PLL should be pretty much independent of the PHY.
Do we need to enable this PLL when the PHY gets an external refclock? I
couldn't test it yet, but I suspect that the HSIO PLL is only needed as
an internal reference, when the i.MX8MP is the refclock source, either
through the PHY pads or via a clkout from the PLL.
> + /* Set P=12,M=800,S=4 and must set ICP=2'b01. */
> + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG2,
> + IMX8MP_GPR_P_PLL_MASK |
> + IMX8MP_GPR_M_PLL_MASK |
> + IMX8MP_GPR_S_PLL_MASK,
> + IMX8MP_GPR_P_PLL |
> + IMX8MP_GPR_M_PLL |
> + IMX8MP_GPR_S_PLL);
> + /* wait greater than 1/F_FREF =1/2MHZ=0.5us */
> + udelay(1);
> +
> + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG3,
> + IMX8MP_GPR_PLL_RST,
> + IMX8MP_GPR_PLL_RST);
> + udelay(10);
> +
> + /* Set 1 to pll_cke of GPR_REG3 */
> + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG3,
> + IMX8MP_GPR_PLL_CKE,
> + IMX8MP_GPR_PLL_CKE);
> +
> + /* Lock time should be greater than 300cycle=300*0.5us=150us */
> + ret = regmap_read_poll_timeout(imx8_phy->hsio_blk_ctrl,
> + IMX8MP_GPR_REG1, val,
> + val & IMX8MP_GPR_PLL_LOCK,
> + 10, 1000);
> + if (ret) {
> + dev_err(imx8_phy->dev, "PCIe PLL lock timeout\n");
> + return ret;
> + }
> +
> + /* pcie_clock_module_en */
> + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG0,
> + IMX8MP_GPR_CLK_MOD_EN,
> + IMX8MP_GPR_CLK_MOD_EN);
You shouldn't need to touch this bit. The HSIO blk-ctrl already enables
this bit when the PCIe power-domain is powered up.
> + udelay(10);
> +
> + reset_control_deassert(imx8_phy->reset);
> + reset_control_deassert(imx8_phy->perst);
> +
> + /* release pcie_phy_apb_reset and pcie_phy_init_resetn */
> + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG0,
> + IMX8MP_GPR_PHY_APB_RST |
> + IMX8MP_GPR_PHY_INIT_RST,
> + IMX8MP_GPR_PHY_APB_RST |
> + IMX8MP_GPR_PHY_INIT_RST);
Not sure about those yet. We might want to toggle them via a virtual PD
in the HSIO blk-ctrl.
> + break;
> + }
> +
> + if (pad_mode == IMX8_PCIE_REFCLK_PAD_INPUT) {
> + /* Configure the pad as input */
> + val = readl(imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> + writel(val & ~ANA_PLL_CLK_OUT_TO_EXT_IO_EN,
> + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> + } else if (pad_mode == IMX8_PCIE_REFCLK_PAD_OUTPUT) {
> + /* Configure the PHY to output the refclock via pad */
> + writel(ANA_PLL_CLK_OUT_TO_EXT_IO_EN,
> + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> + writel(ANA_PLL_CLK_OUT_TO_EXT_IO_SEL,
> + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG062);
> + writel(AUX_PLL_REFCLK_SEL_SYS_PLL,
> + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG063);
> + val = ANA_AUX_RX_TX_SEL_TX | ANA_AUX_TX_TERM;
> + writel(val | ANA_AUX_RX_TERM_GND_EN,
> + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG064);
> + writel(ANA_AUX_RX_TERM | ANA_AUX_TX_LVL,
> + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG065);
> + }
> +
> /* Set AUX_EN_OVERRIDE 1'b0, when the CLKREQ# isn't hooked */
> regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE,
> @@ -91,42 +199,30 @@ static int imx8_pcie_phy_init(struct phy *phy)
> regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> IMX8MM_GPR_PCIE_CMN_RST,
> IMX8MM_GPR_PCIE_CMN_RST);
> - usleep_range(200, 500);
>
> - if (pad_mode == IMX8_PCIE_REFCLK_PAD_INPUT) {
> - /* Configure the pad as input */
> - val = readl(imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> - writel(val & ~ANA_PLL_CLK_OUT_TO_EXT_IO_EN,
> - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> - } else if (pad_mode == IMX8_PCIE_REFCLK_PAD_OUTPUT) {
> - /* Configure the PHY to output the refclock via pad */
> - writel(ANA_PLL_CLK_OUT_TO_EXT_IO_EN,
> - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> - writel(ANA_PLL_CLK_OUT_TO_EXT_IO_SEL,
> - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG062);
> - writel(AUX_PLL_REFCLK_SEL_SYS_PLL,
> - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG063);
> - val = ANA_AUX_RX_TX_SEL_TX | ANA_AUX_TX_TERM;
> - writel(val | ANA_AUX_RX_TERM_GND_EN,
> - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG064);
> - writel(ANA_AUX_RX_TERM | ANA_AUX_TX_LVL,
> - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG065);
> + switch (imx8_phy->variant) {
> + case IMX8MM:
> + reset_control_deassert(imx8_phy->reset);
> + usleep_range(200, 500);
> + break;
> +
> + case IMX8MP:
> + /* wait for core_clk enabled */
> + ret = regmap_read_poll_timeout(imx8_phy->hsio_blk_ctrl,
> + IMX8MP_GPR_REG1, val,
> + val & IMX8MP_GPR_PM_EN_CORE_CLK,
> + 10, 20000);
> + if (ret) {
> + dev_err(imx8_phy->dev, "PCIe CORE CLK enable failed\n");
> + return ret;
> + }
> +
> + break;
> }
>
> - /* Tune PHY de-emphasis setting to pass PCIe compliance. */
> - if (imx8_phy->tx_deemph_gen1)
> - writel(imx8_phy->tx_deemph_gen1,
> - imx8_phy->base + PCIE_PHY_TRSV_REG5);
> - if (imx8_phy->tx_deemph_gen2)
> - writel(imx8_phy->tx_deemph_gen2,
> - imx8_phy->base + PCIE_PHY_TRSV_REG6);
> -
> - reset_control_deassert(imx8_phy->reset);
> -
> /* Polling to check the phy is ready or not. */
> - ret = readl_poll_timeout(imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG75,
> - val, val == PCIE_PHY_CMN_REG75_PLL_DONE,
> - 10, 20000);
> + ret = readl_poll_timeout(imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG075,
> + val, val == ANA_PLL_DONE, 10, 20000);
> return ret;
> }
>
> @@ -153,18 +249,33 @@ static const struct phy_ops imx8_pcie_phy_ops = {
> .owner = THIS_MODULE,
> };
>
> +static const struct of_device_id imx8_pcie_phy_of_match[] = {
> + {.compatible = "fsl,imx8mm-pcie-phy", .data = (void *)IMX8MM},
> + {.compatible = "fsl,imx8mp-pcie-phy", .data = (void *)IMX8MP},
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, imx8_pcie_phy_of_match);
> +
> static int imx8_pcie_phy_probe(struct platform_device *pdev)
> {
> struct phy_provider *phy_provider;
> struct device *dev = &pdev->dev;
> + const struct of_device_id *of_id;
> struct device_node *np = dev->of_node;
> struct imx8_pcie_phy *imx8_phy;
> struct resource *res;
>
> + of_id = of_match_device(imx8_pcie_phy_of_match, dev);
> + if (!of_id)
> + return -EINVAL;
> +
> imx8_phy = devm_kzalloc(dev, sizeof(*imx8_phy), GFP_KERNEL);
> if (!imx8_phy)
> return -ENOMEM;
>
> + imx8_phy->dev = dev;
> + imx8_phy->variant = (enum imx8_pcie_phy_type)of_id->data;
> +
> /* get PHY refclk pad mode */
> of_property_read_u32(np, "fsl,refclk-pad-mode",
> &imx8_phy->refclk_pad_mode);
> @@ -201,6 +312,22 @@ static int imx8_pcie_phy_probe(struct platform_device *pdev)
> dev_err(dev, "Failed to get PCIEPHY reset control\n");
> return PTR_ERR(imx8_phy->reset);
> }
> + if (imx8_phy->variant == IMX8MP) {
> + /* Grab HSIO MIX config register range */
> + imx8_phy->hsio_blk_ctrl =
> + syscon_regmap_lookup_by_compatible("fsl,imx8mp-hsio-blk-ctrl");
> + if (IS_ERR(imx8_phy->hsio_blk_ctrl)) {
> + dev_err(dev, "unable to find hsio mix registers\n");
> + return PTR_ERR(imx8_phy->hsio_blk_ctrl);
> + }
> +
> + imx8_phy->perst =
> + devm_reset_control_get_exclusive(dev, "perst");
> + if (IS_ERR(imx8_phy->perst)) {
> + dev_err(dev, "Failed to get PCIEPHY perst control\n");
> + return PTR_ERR(imx8_phy->perst);
> + }
> + }
I still hope that we can push all the HSIO blk-ctrl register access
into the blk-ctrl driver, by adding the PLL as a clock there and maybe
abstracting the PHY reset bits a virtual PD for the PHY, so we don't
need this direct access in the PHY driver. Depends a bit on weather we
are able to get the sequencing right when splitting things across
multiple drivers.
Regards,
Lucas
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> imx8_phy->base = devm_ioremap_resource(dev, res);
> @@ -218,12 +345,6 @@ static int imx8_pcie_phy_probe(struct platform_device *pdev)
> return PTR_ERR_OR_ZERO(phy_provider);
> }
>
> -static const struct of_device_id imx8_pcie_phy_of_match[] = {
> - {.compatible = "fsl,imx8mm-pcie-phy",},
> - { },
> -};
> -MODULE_DEVICE_TABLE(of, imx8_pcie_phy_of_match);
> -
> static struct platform_driver imx8_pcie_phy_driver = {
> .probe = imx8_pcie_phy_probe,
> .driver = {
Hi Philipp:
> -----Original Message-----
> From: Philipp Zabel <[email protected]>
> Sent: 2022年4月4日 17:34
> To: Hongxing Zhu <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>
> Subject: Re: [PATCH v2 1/7] reset: imx7: Add the iMX8MP PCIe PHY PERST
> support
>
> Hi Richard,
>
> On Mo, 2022-03-07 at 17:07 +0800, Richard Zhu wrote:
> > Add the i.MX8MP PCIe PHY PERST support.
> >
> > Signed-off-by: Richard Zhu <[email protected]>
> > ---
> > drivers/reset/reset-imx7.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
> > index 185a333df66c..d2408725eb2c 100644
> > --- a/drivers/reset/reset-imx7.c
> > +++ b/drivers/reset/reset-imx7.c
> > @@ -329,6 +329,7 @@ static int imx8mp_reset_set(struct
> > reset_controller_dev *rcdev,
> > break;
> >
> > case IMX8MP_RESET_PCIE_CTRL_APPS_EN:
> > + case IMX8MP_RESET_PCIEPHY_PERST:
> > value = assert ? 0 : bit;
> > break;
> > }
>
> This doesn't do what the commit description says.
>
> The PCIEPHY_PERST bit is already supported by the driver (albeit
> incorrectly?) - this patch just inverts the bit.
>
> Since this bit is not inverted on the other platforms, and the i.MX8MP
> reference manual says nothing about this, please explicitly state why this
> needs to be inverted and call it a fix in the commit description.
Thanks for your comments, and sorry for replying late.
I didn't get more details about this bit difference between i.MX8MP and others.
Let me consult with design team again, and back to you later.
Best Regards
Richard Zhu
>
> regards
> Philipp
Hi Richard,
I didn't get around to see what's wrong with the PCIe on my EVK, so I
wasn't able to do any reworks to the series. As I will be on vacation
from tomorrow until Apr. 25th, I'll just leave some more comments on
this series for your consideration.
Regards,
Lucas
Am Montag, dem 07.03.2022 um 17:07 +0800 schrieb Richard Zhu:
> Based on the i.MX8MP GPC and blk-ctrl patch-set[1] issued by Lucas and the
> following commits.
> - one codes refine patch-set[5].
> - two Fixes[2],[3].
> - one binding commit[4].
> - some dts changes in Shawn's git if you want to test PCIe on i.MX8MM EVK.
> b4d36c10bf17 arm64: dts: imx8mm-evk: Add the pcie support on imx8mm evk board
> aaeba6a8e226 arm64: dts: imx8mm: Add the pcie support
> cfc5078432ca arm64: dts: imx8mm: Add the pcie phy support
>
> Sorry about that there may be some conflictions when do the codes merge.
> I'm waiting for the ack now, and will re-base them in a proper sequence later.
>
> This series patches add the i.MX8MP PCIe support and tested on i.MX8MM EVK and
> i.MX8MP EVk boards. The PCIe NVME works fine on both boards.
>
> - i.MX8MP PCIe PHY has two resets refer to the i.MX8MM PCIe PHY.
> Add one more PHY reset for i.MX8MP PCIe PHY accordingly.
> - Add the i.MX8MP PCIe PHY support in the i.MX8M PCIe PHY driver.
> And share as much as possible codes with i.MX8MM PCIe PHY.
> - Add the i.MX8MP PCIe support in binding document, DTS files, and PCIe
> driver.
>
> Main changes v1-->v2:
> - It's my fault forget including Vinod, re-send v2 after include Vinod
> and [email protected].
> - List the basements of this patch-set. The branch, codes changes and so on.
> - Clean up some useless register and bit definitions in #3 patch.
>
> [1]https://patchwork.kernel.org/project/linux-arm-kernel/cover/[email protected]/
> [2]https://patchwork.ozlabs.org/project/linux-pci/patch/[email protected]/
> [3]https://patchwork.ozlabs.org/project/linux-pci/patch/[email protected]/
> [4]https://patchwork.ozlabs.org/project/linux-pci/patch/[email protected]/
> [5]https://patchwork.ozlabs.org/project/linux-pci/cover/[email protected]/
>
> NOTE:
> Based git <git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git>
> Based branch <pci/imx6>
>
> Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 1 +
> Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml | 4 +-
> arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 55 ++++++++++++++++++++++
> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 46 ++++++++++++++++++-
> drivers/pci/controller/dwc/pci-imx6.c | 19 +++++++-
> drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 205 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------
> drivers/reset/reset-imx7.c | 1 +
> 7 files changed, 286 insertions(+), 45 deletions(-)
>
> [PATCH v2 1/7] reset: imx7: Add the iMX8MP PCIe PHY PERST support
> [PATCH v2 2/7] dt-binding: phy: Add iMX8MP PCIe PHY binding
> [PATCH v2 3/7] phy: freescale: imx8m-pcie: Add iMX8MP PCIe PHY
> [PATCH v2 4/7] dt-bindings: imx6q-pcie: Add iMX8MP PCIe compatible
> [PATCH v2 5/7] arm64: dts: imx8mp: add the iMX8MP PCIe support
> [PATCH v2 6/7] arm64: dts: imx8mp-evk: Add PCIe support
> [PATCH v2 7/7] PCI: imx6: Add the iMX8MP PCIe support
> -----Original Message-----
> From: Lucas Stach <[email protected]>
> Sent: 2022年4月15日 5:03
> To: Hongxing Zhu <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>
> Subject: Re: [PATCH v2 5/7] arm64: dts: imx8mp: add the iMX8MP PCIe
> support
>
> Am Montag, dem 07.03.2022 um 17:07 +0800 schrieb Richard Zhu:
> > Add the i.MX8MP PCIe support.
> >
> > Signed-off-by: Richard Zhu <[email protected]>
> > ---
> > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 46
> > ++++++++++++++++++++++-
> > 1 file changed, 45 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > index b40a5646f205..e7b3d8029e34 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > @@ -5,6 +5,7 @@
> >
> > #include <dt-bindings/clock/imx8mp-clock.h>
> > #include <dt-bindings/power/imx8mp-power.h>
> > +#include <dt-bindings/reset/imx8mp-reset.h>
> > #include <dt-bindings/gpio/gpio.h>
> > #include <dt-bindings/input/input.h>
> > #include <dt-bindings/interrupt-controller/arm-gic.h>
> > @@ -375,7 +376,8 @@ iomuxc: pinctrl@30330000 {
> > };
> >
> > gpr: iomuxc-gpr@30340000 {
> > - compatible = "fsl,imx8mp-iomuxc-gpr", "syscon";
> > + compatible = "fsl,imx8mp-iomuxc-gpr",
> > + "fsl,imx6q-iomuxc-gpr", "syscon";
> > reg = <0x30340000 0x10000>;
> > };
> >
> > @@ -965,6 +967,17 @@ aips4: bus@32c00000 {
> > #size-cells = <1>;
> > ranges;
> >
> > + pcie_phy: pcie-phy@32f00000 {
> > + compatible = "fsl,imx8mp-pcie-phy";
> > + reg = <0x32f00000 0x10000>;
> > + resets = <&src IMX8MP_RESET_PCIEPHY>,
> > + <&src IMX8MP_RESET_PCIEPHY_PERST>;
> > + reset-names = "pciephy", "perst";
> > + power-domains = <&hsio_blk_ctrl
> IMX8MP_HSIOBLK_PD_PCIE_PHY>;
> > + #phy-cells = <0>;
> > + status = "disabled";
> > + };
> > +
> > hsio_blk_ctrl: blk-ctrl@32f10000 {
> > compatible = "fsl,imx8mp-hsio-blk-ctrl", "syscon";
> > reg = <0x32f10000 0x24>;
> > @@ -980,6 +993,37 @@ hsio_blk_ctrl: blk-ctrl@32f10000 {
> > };
> > };
> >
> > + pcie: pcie@33800000 {
> > + compatible = "fsl,imx8mp-pcie";
> > + reg = <0x33800000 0x400000>, <0x1ff00000 0x80000>;
> > + reg-names = "dbi", "config";
> > + #address-cells = <3>;
> > + #size-cells = <2>;
> > + device_type = "pci";
> > + bus-range = <0x00 0xff>;
> > + ranges = <0x81000000 0 0x00000000 0x1ff80000 0
> 0x00010000 /* downstream I/O 64KB */
> > + 0x82000000 0 0x18000000 0x18000000 0
> 0x07f00000>; /* non-prefetchable memory */
> > + num-lanes = <1>;
> > + num-viewport = <4>;
> > + interrupts = <GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>;
> > + interrupt-names = "msi";
> > + #interrupt-cells = <1>;
> > + interrupt-map-mask = <0 0 0 0x7>;
> > + interrupt-map = <0 0 0 1 &gic GIC_SPI 126
> IRQ_TYPE_LEVEL_HIGH>,
> > + <0 0 0 2 &gic GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
> > + <0 0 0 3 &gic GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>,
> > + <0 0 0 4 &gic GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
> > + fsl,max-link-speed = <3>;
>
> I believe that imx6_pcie_start_link does not properly handle Gen3 speeds.
Good caught.
The according link_gen condition should be changed in driver too.
Would be changed in next version.
Thanks.
Best Regards
Richard Zhu
>
> Regards,
> Lucas
>
> > + linux,pci-domain = <0>;
> > + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_PCIE>;
> > + resets = <&src IMX8MP_RESET_PCIE_CTRL_APPS_EN>,
> > + <&src IMX8MP_RESET_PCIE_CTRL_APPS_TURNOFF>;
> > + reset-names = "apps", "turnoff";
> > + phys = <&pcie_phy>;
> > + phy-names = "pcie-phy";
> > + status = "disabled";
> > + };
> > +
> > gpu3d: gpu@38000000 {
> > compatible = "vivante,gc";
> > reg = <0x38000000 0x8000>;
>
> -----Original Message-----
> From: Lucas Stach <[email protected]>
> Sent: 2022年4月15日 4:48
> To: Hongxing Zhu <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>
> Subject: Re: [PATCH v2 1/7] reset: imx7: Add the iMX8MP PCIe PHY PERST
> support
>
> Am Montag, dem 07.03.2022 um 17:07 +0800 schrieb Richard Zhu:
> > Add the i.MX8MP PCIe PHY PERST support.
>
> As Philipp said: please add some more description on why this is necessary. As
> far as I can see the reset is already present on 8MQ, and is low-active, like this
> patch claims. We just didn't handle this reset at all on other SoCs as the power
> on de-asserted state was okay to get things working.
>
Yes, it is.
I had asking the details from design team.
Would update the description after I get the more information.
Best Regards
Richard Zhu
> Regards,
> Lucas
>
> >
> > Signed-off-by: Richard Zhu <[email protected]>
> > ---
> > drivers/reset/reset-imx7.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
> > index 185a333df66c..d2408725eb2c 100644
> > --- a/drivers/reset/reset-imx7.c
> > +++ b/drivers/reset/reset-imx7.c
> > @@ -329,6 +329,7 @@ static int imx8mp_reset_set(struct
> reset_controller_dev *rcdev,
> > break;
> >
> > case IMX8MP_RESET_PCIE_CTRL_APPS_EN:
> > + case IMX8MP_RESET_PCIEPHY_PERST:
> > value = assert ? 0 : bit;
> > break;
> > }
>
> -----Original Message-----
> From: Lucas Stach <[email protected]>
> Sent: 2022??4??15?? 4:45
> To: Hongxing Zhu <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>
> Subject: Re: [PATCH v2 0/7] Add the iMX8MP PCIe support
>
> Hi Richard,
>
> I didn't get around to see what's wrong with the PCIe on my EVK, so I wasn't
> able to do any reworks to the series. As I will be on vacation from tomorrow
> until Apr. 25th, I'll just leave some more comments on this series for your
> consideration.
Hi Lucas:
Got that, thanks.
Have a good holiday.
Best Regards
Richard Zhu
>
> Regards,
> Lucas
>
> Am Montag, dem 07.03.2022 um 17:07 +0800 schrieb Richard Zhu:
> > Based on the i.MX8MP GPC and blk-ctrl patch-set[1] issued by Lucas and
> > the following commits.
> > - one codes refine patch-set[5].
> > - two Fixes[2],[3].
> > - one binding commit[4].
> > - some dts changes in Shawn's git if you want to test PCIe on i.MX8MM
> EVK.
> > b4d36c10bf17 arm64: dts: imx8mm-evk: Add the pcie support on
> imx8mm evk board
> > aaeba6a8e226 arm64: dts: imx8mm: Add the pcie support
> > cfc5078432ca arm64: dts: imx8mm: Add the pcie phy support
> >
> > Sorry about that there may be some conflictions when do the codes merge.
> > I'm waiting for the ack now, and will re-base them in a proper sequence later.
> >
> > This series patches add the i.MX8MP PCIe support and tested on i.MX8MM
> > EVK and i.MX8MP EVk boards. The PCIe NVME works fine on both boards.
> >
> > - i.MX8MP PCIe PHY has two resets refer to the i.MX8MM PCIe PHY.
> > Add one more PHY reset for i.MX8MP PCIe PHY accordingly.
> > - Add the i.MX8MP PCIe PHY support in the i.MX8M PCIe PHY driver.
> > And share as much as possible codes with i.MX8MM PCIe PHY.
> > - Add the i.MX8MP PCIe support in binding document, DTS files, and PCIe
> > driver.
> >
> > Main changes v1-->v2:
> > - It's my fault forget including Vinod, re-send v2 after include Vinod
> > and [email protected].
> > - List the basements of this patch-set. The branch, codes changes and so on.
> > - Clean up some useless register and bit definitions in #3 patch.
> >
> > [1]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> >
> atchwork.kernel.org%2Fproject%2Flinux-arm-kernel%2Fcover%2F2022022820
> 1
> >
> 731.3330192-1-l.stach%40pengutronix.de%2F&data=04%7C01%7Chongx
> ing.
> >
> zhu%40nxp.com%7C3e8710b45fcd4516145908da1e57ad9b%7C686ea1d3bc2
> b4c6fa92
> >
> cd99c5c301635%7C0%7C1%7C637855659162145906%7CUnknown%7CTWF
> pbGZsb3d8eyJ
> >
> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> C3000
> >
> &sdata=p758ZvzD9wSh23Bcz29e%2BbvVi6BfmvDqIgh4yzR3ACs%3D&am
> p;reserv
> > ed=0
> > [2]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> >
> atchwork.ozlabs.org%2Fproject%2Flinux-pci%2Fpatch%2F1646289275-17813-
> 1
> >
> -git-send-email-hongxing.zhu%40nxp.com%2F&data=04%7C01%7Chongxi
> ng.
> >
> zhu%40nxp.com%7C3e8710b45fcd4516145908da1e57ad9b%7C686ea1d3bc2
> b4c6fa92
> >
> cd99c5c301635%7C0%7C1%7C637855659162145906%7CUnknown%7CTWF
> pbGZsb3d8eyJ
> >
> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> C3000
> >
> &sdata=PKa0bhjfgg%2B74KtKYEtMLtar%2BMkw6od3IZkkjd5U6fM%3D&a
> mp;rese
> > rved=0
> > [3]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> >
> atchwork.ozlabs.org%2Fproject%2Flinux-pci%2Fpatch%2F1645672013-8949-1
> -
> >
> git-send-email-hongxing.zhu%40nxp.com%2F&data=04%7C01%7Chongxi
> ng.z
> >
> hu%40nxp.com%7C3e8710b45fcd4516145908da1e57ad9b%7C686ea1d3bc2
> b4c6fa92c
> >
> d99c5c301635%7C0%7C1%7C637855659162145906%7CUnknown%7CTWFp
> bGZsb3d8eyJW
> >
> IjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3
> 000&
> >
> amp;sdata=wCnAZqH22CWRBc%2BNJOTdfFUEcGcb9JtziIVJsUMSXVc%3D&
> ;reserve
> > d=0
> > [4]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> >
> atchwork.ozlabs.org%2Fproject%2Flinux-pci%2Fpatch%2F1646293805-18248-
> 1
> >
> -git-send-email-hongxing.zhu%40nxp.com%2F&data=04%7C01%7Chongxi
> ng.
> >
> zhu%40nxp.com%7C3e8710b45fcd4516145908da1e57ad9b%7C686ea1d3bc2
> b4c6fa92
> >
> cd99c5c301635%7C0%7C1%7C637855659162145906%7CUnknown%7CTWF
> pbGZsb3d8eyJ
> >
> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> C3000
> >
> &sdata=U221NuIhee%2FF9Yf%2B28pFwVlg8fi3m694qbdfPZs%2BHEw%3
> D&re
> > served=0
> > [5]https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> >
> atchwork.ozlabs.org%2Fproject%2Flinux-pci%2Fcover%2F1645760667-10510-
> 1
> >
> -git-send-email-hongxing.zhu%40nxp.com%2F&data=04%7C01%7Chongxi
> ng.
> >
> zhu%40nxp.com%7C3e8710b45fcd4516145908da1e57ad9b%7C686ea1d3bc2
> b4c6fa92
> >
> cd99c5c301635%7C0%7C1%7C637855659162145906%7CUnknown%7CTWF
> pbGZsb3d8eyJ
> >
> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> C3000
> >
> &sdata=9KHzoq6kMFAL%2FVnG6GMvY%2FdbfzaOlZf9y4Y%2FuVrEKV4%3
> D&re
> > served=0
> >
> > NOTE:
> > Based git
> > <git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git>
> > Based branch <pci/imx6>
> >
> > Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 1 +
> > Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml | 4 +-
> > arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 55
> ++++++++++++++++++++++
> > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 46
> ++++++++++++++++++-
> > drivers/pci/controller/dwc/pci-imx6.c | 19
> +++++++-
> > drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 205
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++-----------------
> > drivers/reset/reset-imx7.c | 1 +
> > 7 files changed, 286 insertions(+), 45 deletions(-)
> >
> > [PATCH v2 1/7] reset: imx7: Add the iMX8MP PCIe PHY PERST support
> > [PATCH v2 2/7] dt-binding: phy: Add iMX8MP PCIe PHY binding [PATCH v2
> > 3/7] phy: freescale: imx8m-pcie: Add iMX8MP PCIe PHY [PATCH v2 4/7]
> > dt-bindings: imx6q-pcie: Add iMX8MP PCIe compatible [PATCH v2 5/7]
> > arm64: dts: imx8mp: add the iMX8MP PCIe support [PATCH v2 6/7] arm64:
> > dts: imx8mp-evk: Add PCIe support [PATCH v2 7/7] PCI: imx6: Add the
> > iMX8MP PCIe support
>
> -----Original Message-----
> From: Lucas Stach <[email protected]>
> Sent: 2022年4月15日 4:59
> To: Hongxing Zhu <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>
> Subject: Re: [PATCH v2 3/7] phy: freescale: imx8m-pcie: Add iMX8MP PCIe PHY
> support
>
> Am Montag, dem 07.03.2022 um 17:07 +0800 schrieb Richard Zhu:
> > Add the i.MX8MP PCIe PHY support
> >
> > Signed-off-by: Richard Zhu <[email protected]>
> > ---
> > drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 205
> > ++++++++++++++++-----
> > 1 file changed, 163 insertions(+), 42 deletions(-)
> >
> > diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > index 04b1aafb29f4..3d01da4323a6 100644
> > --- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > +++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > @@ -11,6 +11,8 @@
> > #include <linux/mfd/syscon.h>
> > #include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
> > #include <linux/module.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_device.h>
> > #include <linux/phy/phy.h>
> > #include <linux/platform_device.h>
> > #include <linux/regmap.h>
> > @@ -30,12 +32,10 @@
> > #define IMX8MM_PCIE_PHY_CMN_REG065 0x194
> > #define ANA_AUX_RX_TERM (BIT(7) | BIT(4))
> > #define ANA_AUX_TX_LVL GENMASK(3, 0)
> > -#define IMX8MM_PCIE_PHY_CMN_REG75 0x1D4
> > -#define PCIE_PHY_CMN_REG75_PLL_DONE 0x3
> > +#define IMX8MM_PCIE_PHY_CMN_REG075 0x1D4
> > +#define ANA_PLL_DONE 0x3
>
> Why do you drop the register prefix from the name here?
To prevent the codes from exceeding the 80 columns and align with the other
bit definitions, drop the prefix and keep the bit definitions as short as
possible.
>
> > #define PCIE_PHY_TRSV_REG5 0x414
> > -#define PCIE_PHY_TRSV_REG5_GEN1_DEEMP 0x2D
> > #define PCIE_PHY_TRSV_REG6 0x418
> > -#define PCIE_PHY_TRSV_REG6_GEN2_DEEMP 0xF
> >
> > #define IMX8MM_GPR_PCIE_REF_CLK_SEL GENMASK(25, 24)
> > #define IMX8MM_GPR_PCIE_REF_CLK_PLL
> FIELD_PREP(IMX8MM_GPR_PCIE_REF_CLK_SEL, 0x3)
> > @@ -46,16 +46,43 @@
> > #define IMX8MM_GPR_PCIE_SSC_EN BIT(16)
> > #define IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE BIT(9)
> >
> > +#define IMX8MP_GPR_REG0 0x0
> > +#define IMX8MP_GPR_CLK_MOD_EN BIT(0)
> > +#define IMX8MP_GPR_PHY_APB_RST BIT(4)
> > +#define IMX8MP_GPR_PHY_INIT_RST BIT(5)
> > +#define IMX8MP_GPR_REG1 0x4
> > +#define IMX8MP_GPR_PM_EN_CORE_CLK BIT(0)
> > +#define IMX8MP_GPR_PLL_LOCK BIT(13)
> > +#define IMX8MP_GPR_REG2 0x8
> > +#define IMX8MP_GPR_P_PLL_MASK GENMASK(5, 0)
> > +#define IMX8MP_GPR_M_PLL_MASK GENMASK(15, 6)
> > +#define IMX8MP_GPR_S_PLL_MASK GENMASK(18, 16)
> > +#define IMX8MP_GPR_P_PLL (0xc << 0)
> > +#define IMX8MP_GPR_M_PLL (0x320 << 6)
> > +#define IMX8MP_GPR_S_PLL (0x4 << 16)
> > +#define IMX8MP_GPR_REG3 0xc
> > +#define IMX8MP_GPR_PLL_CKE BIT(17)
> > +#define IMX8MP_GPR_PLL_RST BIT(31)
> > +
> > +enum imx8_pcie_phy_type {
> > + IMX8MM,
> > + IMX8MP,
> > +};
> > +
> > struct imx8_pcie_phy {
> > void __iomem *base;
> > + struct device *dev;
> > struct clk *clk;
> > struct phy *phy;
> > + struct regmap *hsio_blk_ctrl;
> > struct regmap *iomuxc_gpr;
> > struct reset_control *reset;
> > + struct reset_control *perst;
> > u32 refclk_pad_mode;
> > u32 tx_deemph_gen1;
> > u32 tx_deemph_gen2;
> > bool clkreq_unused;
> > + enum imx8_pcie_phy_type variant;
> > };
> >
> > static int imx8_pcie_phy_init(struct phy *phy) @@ -67,6 +94,87 @@
> > static int imx8_pcie_phy_init(struct phy *phy)
> > reset_control_assert(imx8_phy->reset);
> >
> > pad_mode = imx8_phy->refclk_pad_mode;
> > + switch (imx8_phy->variant) {
> > + case IMX8MM:
> > + /* Tune PHY de-emphasis setting to pass PCIe compliance. */
> > + if (imx8_phy->tx_deemph_gen1)
> > + writel(imx8_phy->tx_deemph_gen1,
> > + imx8_phy->base + PCIE_PHY_TRSV_REG5);
> > + if (imx8_phy->tx_deemph_gen2)
> > + writel(imx8_phy->tx_deemph_gen2,
> > + imx8_phy->base + PCIE_PHY_TRSV_REG6);
> > + break;
> > + case IMX8MP:
> > + reset_control_assert(imx8_phy->perst);
>
> Could you tell us something more about this reset. What exactly is it resetting.
> Do we really need to assert it before starting the HSIO PLL?
Yes, this reset should be asserted, otherwise, the PCIe wouldn't work.
I'm asking more details of this reset bit from design team, and would update
later after I get the response.
> AFAICS the PLL should be pretty much independent of the PHY.
Agree.
>
> Do we need to enable this PLL when the PHY gets an external refclock? I
> couldn't test it yet, but I suspect that the HSIO PLL is only needed as an
> internal reference, when the i.MX8MP is the refclock source, either through
> the PHY pads or via a clkout from the PLL.
>
Refer to my experience, the HSIO PLL should be enabled firstly.
> > + /* Set P=12,M=800,S=4 and must set ICP=2'b01. */
> > + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG2,
> > + IMX8MP_GPR_P_PLL_MASK |
> > + IMX8MP_GPR_M_PLL_MASK |
> > + IMX8MP_GPR_S_PLL_MASK,
> > + IMX8MP_GPR_P_PLL |
> > + IMX8MP_GPR_M_PLL |
> > + IMX8MP_GPR_S_PLL);
> > + /* wait greater than 1/F_FREF =1/2MHZ=0.5us */
> > + udelay(1);
> > +
> > + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG3,
> > + IMX8MP_GPR_PLL_RST,
> > + IMX8MP_GPR_PLL_RST);
> > + udelay(10);
> > +
> > + /* Set 1 to pll_cke of GPR_REG3 */
> > + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG3,
> > + IMX8MP_GPR_PLL_CKE,
> > + IMX8MP_GPR_PLL_CKE);
> > +
> > + /* Lock time should be greater than 300cycle=300*0.5us=150us */
> > + ret = regmap_read_poll_timeout(imx8_phy->hsio_blk_ctrl,
> > + IMX8MP_GPR_REG1, val,
> > + val & IMX8MP_GPR_PLL_LOCK,
> > + 10, 1000);
> > + if (ret) {
> > + dev_err(imx8_phy->dev, "PCIe PLL lock timeout\n");
> > + return ret;
> > + }
> > +
> > + /* pcie_clock_module_en */
> > + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG0,
> > + IMX8MP_GPR_CLK_MOD_EN,
> > + IMX8MP_GPR_CLK_MOD_EN);
>
> You shouldn't need to touch this bit. The HSIO blk-ctrl already enables this bit
> when the PCIe power-domain is powered up.
Okay, got that.
>
> > + udelay(10);
> > +
> > + reset_control_deassert(imx8_phy->reset);
> > + reset_control_deassert(imx8_phy->perst);
> > +
> > + /* release pcie_phy_apb_reset and pcie_phy_init_resetn */
> > + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG0,
> > + IMX8MP_GPR_PHY_APB_RST |
> > + IMX8MP_GPR_PHY_INIT_RST,
> > + IMX8MP_GPR_PHY_APB_RST |
> > + IMX8MP_GPR_PHY_INIT_RST);
>
> Not sure about those yet. We might want to toggle them via a virtual PD in the
> HSIO blk-ctrl.
Refer to my understand, these reset should be a part of power-up sequence of
the PHY. It's reasonable to toggle them via a PD.
>
> > + break;
> > + }
> > +
> > + if (pad_mode == IMX8_PCIE_REFCLK_PAD_INPUT) {
> > + /* Configure the pad as input */
> > + val = readl(imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> > + writel(val & ~ANA_PLL_CLK_OUT_TO_EXT_IO_EN,
> > + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> > + } else if (pad_mode == IMX8_PCIE_REFCLK_PAD_OUTPUT) {
> > + /* Configure the PHY to output the refclock via pad */
> > + writel(ANA_PLL_CLK_OUT_TO_EXT_IO_EN,
> > + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> > + writel(ANA_PLL_CLK_OUT_TO_EXT_IO_SEL,
> > + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG062);
> > + writel(AUX_PLL_REFCLK_SEL_SYS_PLL,
> > + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG063);
> > + val = ANA_AUX_RX_TX_SEL_TX | ANA_AUX_TX_TERM;
> > + writel(val | ANA_AUX_RX_TERM_GND_EN,
> > + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG064);
> > + writel(ANA_AUX_RX_TERM | ANA_AUX_TX_LVL,
> > + imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG065);
> > + }
> > +
> > /* Set AUX_EN_OVERRIDE 1'b0, when the CLKREQ# isn't hooked */
> > regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> > IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE, @@ -91,42
> +199,30 @@ static
> > int imx8_pcie_phy_init(struct phy *phy)
> > regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> > IMX8MM_GPR_PCIE_CMN_RST,
> > IMX8MM_GPR_PCIE_CMN_RST);
> > - usleep_range(200, 500);
> >
> > - if (pad_mode == IMX8_PCIE_REFCLK_PAD_INPUT) {
> > - /* Configure the pad as input */
> > - val = readl(imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> > - writel(val & ~ANA_PLL_CLK_OUT_TO_EXT_IO_EN,
> > - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> > - } else if (pad_mode == IMX8_PCIE_REFCLK_PAD_OUTPUT) {
> > - /* Configure the PHY to output the refclock via pad */
> > - writel(ANA_PLL_CLK_OUT_TO_EXT_IO_EN,
> > - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG061);
> > - writel(ANA_PLL_CLK_OUT_TO_EXT_IO_SEL,
> > - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG062);
> > - writel(AUX_PLL_REFCLK_SEL_SYS_PLL,
> > - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG063);
> > - val = ANA_AUX_RX_TX_SEL_TX | ANA_AUX_TX_TERM;
> > - writel(val | ANA_AUX_RX_TERM_GND_EN,
> > - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG064);
> > - writel(ANA_AUX_RX_TERM | ANA_AUX_TX_LVL,
> > - imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG065);
> > + switch (imx8_phy->variant) {
> > + case IMX8MM:
> > + reset_control_deassert(imx8_phy->reset);
> > + usleep_range(200, 500);
> > + break;
> > +
> > + case IMX8MP:
> > + /* wait for core_clk enabled */
> > + ret = regmap_read_poll_timeout(imx8_phy->hsio_blk_ctrl,
> > + IMX8MP_GPR_REG1, val,
> > + val & IMX8MP_GPR_PM_EN_CORE_CLK,
> > + 10, 20000);
> > + if (ret) {
> > + dev_err(imx8_phy->dev, "PCIe CORE CLK enable failed\n");
> > + return ret;
> > + }
> > +
> > + break;
> > }
> >
> > - /* Tune PHY de-emphasis setting to pass PCIe compliance. */
> > - if (imx8_phy->tx_deemph_gen1)
> > - writel(imx8_phy->tx_deemph_gen1,
> > - imx8_phy->base + PCIE_PHY_TRSV_REG5);
> > - if (imx8_phy->tx_deemph_gen2)
> > - writel(imx8_phy->tx_deemph_gen2,
> > - imx8_phy->base + PCIE_PHY_TRSV_REG6);
> > -
> > - reset_control_deassert(imx8_phy->reset);
> > -
> > /* Polling to check the phy is ready or not. */
> > - ret = readl_poll_timeout(imx8_phy->base +
> IMX8MM_PCIE_PHY_CMN_REG75,
> > - val, val == PCIE_PHY_CMN_REG75_PLL_DONE,
> > - 10, 20000);
> > + ret = readl_poll_timeout(imx8_phy->base +
> IMX8MM_PCIE_PHY_CMN_REG075,
> > + val, val == ANA_PLL_DONE, 10, 20000);
> > return ret;
> > }
> >
> > @@ -153,18 +249,33 @@ static const struct phy_ops imx8_pcie_phy_ops =
> {
> > .owner = THIS_MODULE,
> > };
> >
> > +static const struct of_device_id imx8_pcie_phy_of_match[] = {
> > + {.compatible = "fsl,imx8mm-pcie-phy", .data = (void *)IMX8MM},
> > + {.compatible = "fsl,imx8mp-pcie-phy", .data = (void *)IMX8MP},
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(of, imx8_pcie_phy_of_match);
> > +
> > static int imx8_pcie_phy_probe(struct platform_device *pdev) {
> > struct phy_provider *phy_provider;
> > struct device *dev = &pdev->dev;
> > + const struct of_device_id *of_id;
> > struct device_node *np = dev->of_node;
> > struct imx8_pcie_phy *imx8_phy;
> > struct resource *res;
> >
> > + of_id = of_match_device(imx8_pcie_phy_of_match, dev);
> > + if (!of_id)
> > + return -EINVAL;
> > +
> > imx8_phy = devm_kzalloc(dev, sizeof(*imx8_phy), GFP_KERNEL);
> > if (!imx8_phy)
> > return -ENOMEM;
> >
> > + imx8_phy->dev = dev;
> > + imx8_phy->variant = (enum imx8_pcie_phy_type)of_id->data;
> > +
> > /* get PHY refclk pad mode */
> > of_property_read_u32(np, "fsl,refclk-pad-mode",
> > &imx8_phy->refclk_pad_mode);
> > @@ -201,6 +312,22 @@ static int imx8_pcie_phy_probe(struct
> platform_device *pdev)
> > dev_err(dev, "Failed to get PCIEPHY reset control\n");
> > return PTR_ERR(imx8_phy->reset);
> > }
> > + if (imx8_phy->variant == IMX8MP) {
> > + /* Grab HSIO MIX config register range */
> > + imx8_phy->hsio_blk_ctrl =
> > +
> syscon_regmap_lookup_by_compatible("fsl,imx8mp-hsio-blk-ctrl");
> > + if (IS_ERR(imx8_phy->hsio_blk_ctrl)) {
> > + dev_err(dev, "unable to find hsio mix registers\n");
> > + return PTR_ERR(imx8_phy->hsio_blk_ctrl);
> > + }
> > +
> > + imx8_phy->perst =
> > + devm_reset_control_get_exclusive(dev, "perst");
> > + if (IS_ERR(imx8_phy->perst)) {
> > + dev_err(dev, "Failed to get PCIEPHY perst control\n");
> > + return PTR_ERR(imx8_phy->perst);
> > + }
> > + }
>
> I still hope that we can push all the HSIO blk-ctrl register access into the
> blk-ctrl driver, by adding the PLL as a clock there and maybe abstracting the
> PHY reset bits a virtual PD for the PHY, so we don't need this direct access in
> the PHY driver. Depends a bit on weather we are able to get the sequencing
> right when splitting things across multiple drivers.
Agree, thanks for your considerations.
Best Regards
Richard Zhu
>
> Regards,
> Lucas
>
> >
> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > imx8_phy->base = devm_ioremap_resource(dev, res); @@ -218,12 +345,6
> > @@ static int imx8_pcie_phy_probe(struct platform_device *pdev)
> > return PTR_ERR_OR_ZERO(phy_provider); }
> >
> > -static const struct of_device_id imx8_pcie_phy_of_match[] = {
> > - {.compatible = "fsl,imx8mm-pcie-phy",},
> > - { },
> > -};
> > -MODULE_DEVICE_TABLE(of, imx8_pcie_phy_of_match);
> > -
> > static struct platform_driver imx8_pcie_phy_driver = {
> > .probe = imx8_pcie_phy_probe,
> > .driver = {
>
> -----Original Message-----
> From: Lucas Stach <[email protected]>
> Sent: 2022年4月15日 5:05
> To: Hongxing Zhu <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>
> Subject: Re: [PATCH v2 6/7] arm64: dts: imx8mp-evk: Add PCIe support
>
> Am Montag, dem 07.03.2022 um 17:07 +0800 schrieb Richard Zhu:
> > Add PCIe support on i.MX8MP EVK board.
> >
> > Signed-off-by: Richard Zhu <[email protected]>
> > ---
> > arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 55
> > ++++++++++++++++++++
> > 1 file changed, 55 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > index 2eb943210678..ed77455a3f73 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > @@ -5,6 +5,7 @@
> >
> > /dts-v1/;
> >
> > +#include <dt-bindings/phy/phy-imx8-pcie.h>
> > #include "imx8mp.dtsi"
> >
> > / {
> > @@ -33,6 +34,12 @@ memory@40000000 {
> > <0x1 0x00000000 0 0xc0000000>;
> > };
> >
> > + pcie0_refclk: pcie0-refclk {
> > + compatible = "fixed-clock";
> > + #clock-cells = <0>;
> > + clock-frequency = <100000000>;
> > + };
> > +
> > reg_can1_stby: regulator-can1-stby {
> > compatible = "regulator-fixed";
> > regulator-name = "can1-stby";
> > @@ -55,6 +62,17 @@ reg_can2_stby: regulator-can2-stby {
> > enable-active-high;
> > };
> >
> > + reg_pcie0: regulator-pcie {
> > + compatible = "regulator-fixed";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_pcie0_reg>;
> > + regulator-name = "MPCIE_3V3";
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > + gpio = <&gpio2 6 GPIO_ACTIVE_HIGH>;
> > + enable-active-high;
> > + };
> > +
> > reg_usdhc2_vmmc: regulator-usdhc2 {
> > compatible = "regulator-fixed";
> > pinctrl-names = "default";
> > @@ -297,6 +315,30 @@ pca6416: gpio@20 {
> > };
> > };
> >
> > +&pcie_phy {
> > + fsl,refclk-pad-mode = <IMX8_PCIE_REFCLK_PAD_INPUT>;
> > + clocks = <&pcie0_refclk>;
> > + clock-names = "ref";
> > + status = "okay";
> > +};
> > +
> > +&pcie{
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_pcie0>;
> > + reset-gpio = <&gpio2 7 GPIO_ACTIVE_LOW>;
> > + clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
> > + <&clk IMX8MP_CLK_PCIE_ROOT>,
> > + <&clk IMX8MP_CLK_HSIO_AXI>;
> > + clock-names = "pcie", "pcie_aux", "pcie_bus";
> > + assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>,
> > + <&clk IMX8MP_CLK_PCIE_AUX>;
> > + assigned-clock-rates = <500000000>, <10000000>;
> > + assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>,
>
> You don't need to set the IMX8MP_CLK_HSIO_AXI clock rate here, that's
> already don't by the power-domain, as it is keeping this bus clock active. Only
> need to set the PCIE_AUX rate here.
Thanks, for your review comments.
Would be changed in next version.
Best Regards
Richard Zhu
>
> Regards,
> Lucas
>
> > + <&clk IMX8MP_SYS_PLL2_50M>;
> > + vpcie-supply = <®_pcie0>;
> > + status = "okay";
> > +};
> > +
> > &snvs_pwrkey {
> > status = "okay";
> > };
> > @@ -442,6 +484,19 @@ MX8MP_IOMUXC_I2C3_SDA__I2C3_SDA
> 0x400001c3
> > >;
> > };
> >
> > + pinctrl_pcie0: pcie0grp {
> > + fsl,pins = <
> > + MX8MP_IOMUXC_I2C4_SCL__PCIE_CLKREQ_B 0x61 /* open
> drain, pull up */
> > + MX8MP_IOMUXC_SD1_DATA5__GPIO2_IO07 0x41
> > + >;
> > + };
> > +
> > + pinctrl_pcie0_reg: pcie0reggrp {
> > + fsl,pins = <
> > + MX8MP_IOMUXC_SD1_DATA4__GPIO2_IO06 0x41
> > + >;
> > + };
> > +
> > pinctrl_pmic: pmicgrp {
> > fsl,pins = <
> > MX8MP_IOMUXC_GPIO1_IO03__GPIO1_IO03 0x000001c0
>
Hi Philipp:
> -----Original Message-----
> From: Hongxing Zhu
> Sent: 2022年4月15日 15:33
> To: Philipp Zabel <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>
> Subject: RE: [PATCH v2 1/7] reset: imx7: Add the iMX8MP PCIe PHY PERST
> support
>
> Hi Philipp:
>
> > -----Original Message-----
> > From: Philipp Zabel <[email protected]>
> > Sent: 2022年4月4日 17:34
> > To: Hongxing Zhu <[email protected]>; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected]
> > Cc: [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected]; dl-linux-imx
> > <[email protected]>
> > Subject: Re: [PATCH v2 1/7] reset: imx7: Add the iMX8MP PCIe PHY PERST
> > support
> >
> > Hi Richard,
> >
> > On Mo, 2022-03-07 at 17:07 +0800, Richard Zhu wrote:
> > > Add the i.MX8MP PCIe PHY PERST support.
> > >
> > > Signed-off-by: Richard Zhu <[email protected]>
> > > ---
> > > drivers/reset/reset-imx7.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
> > > index 185a333df66c..d2408725eb2c 100644
> > > --- a/drivers/reset/reset-imx7.c
> > > +++ b/drivers/reset/reset-imx7.c
> > > @@ -329,6 +329,7 @@ static int imx8mp_reset_set(struct
> > > reset_controller_dev *rcdev,
> > > break;
> > >
> > > case IMX8MP_RESET_PCIE_CTRL_APPS_EN:
> > > + case IMX8MP_RESET_PCIEPHY_PERST:
> > > value = assert ? 0 : bit;
> > > break;
> > > }
> >
> > This doesn't do what the commit description says.
> >
> > The PCIEPHY_PERST bit is already supported by the driver (albeit
> > incorrectly?) - this patch just inverts the bit.
> >
> > Since this bit is not inverted on the other platforms, and the i.MX8MP
> > reference manual says nothing about this, please explicitly state why
> > this needs to be inverted and call it a fix in the commit description.
> Thanks for your comments, and sorry for replying late.
> I didn't get more details about this bit difference between i.MX8MP and
> others.
> Let me consult with design team again, and back to you later.
I got some details of this PERST bit(BIT3) of SRC_PCIEPHY_RCR.
The initialized default value of this bit is 1b'1 on i.MX7/iMX8MM/iMX8MQ.
But unfortunately, the i.MX8MP has one inverted default value 1b'0 of this bit.
And this bit should be kept 1b'1 after power and clocks are stable.
So, I assert/de-assert this bit on i.MX8MP only.
Best Regards
Richard Zhu
>
> Best Regards
> Richard Zhu
>
> >
> > regards
> > Philipp
Hi Richard,
Am Montag, dem 18.04.2022 um 04:55 +0000 schrieb Hongxing Zhu:
> > -----Original Message-----
> > From: Lucas Stach <[email protected]>
> > Sent: 2022年4月15日 4:59
> > To: Hongxing Zhu <[email protected]>; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected]
> > Cc: [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected]; dl-linux-imx
> > <[email protected]>
> > Subject: Re: [PATCH v2 3/7] phy: freescale: imx8m-pcie: Add iMX8MP PCIe PHY
> > support
> >
> > Am Montag, dem 07.03.2022 um 17:07 +0800 schrieb Richard Zhu:
> > > Add the i.MX8MP PCIe PHY support
> > >
> > > Signed-off-by: Richard Zhu <[email protected]>
> > > ---
> > > drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 205
> > > ++++++++++++++++-----
> > > 1 file changed, 163 insertions(+), 42 deletions(-)
> > >
> > > diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > index 04b1aafb29f4..3d01da4323a6 100644
> > > --- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > +++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > @@ -11,6 +11,8 @@
> > > #include <linux/mfd/syscon.h>
> > > #include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
> > > #include <linux/module.h>
> > > +#include <linux/of_address.h>
> > > +#include <linux/of_device.h>
> > > #include <linux/phy/phy.h>
> > > #include <linux/platform_device.h>
> > > #include <linux/regmap.h>
> > > @@ -30,12 +32,10 @@
> > > #define IMX8MM_PCIE_PHY_CMN_REG065 0x194
> > > #define ANA_AUX_RX_TERM (BIT(7) | BIT(4))
> > > #define ANA_AUX_TX_LVL GENMASK(3, 0)
> > > -#define IMX8MM_PCIE_PHY_CMN_REG75 0x1D4
> > > -#define PCIE_PHY_CMN_REG75_PLL_DONE 0x3
> > > +#define IMX8MM_PCIE_PHY_CMN_REG075 0x1D4
> > > +#define ANA_PLL_DONE 0x3
> >
> > Why do you drop the register prefix from the name here?
> To prevent the codes from exceeding the 80 columns and align with the other
> bit definitions, drop the prefix and keep the bit definitions as short as
> possible.
>
> >
> > > #define PCIE_PHY_TRSV_REG5 0x414
> > > -#define PCIE_PHY_TRSV_REG5_GEN1_DEEMP 0x2D
> > > #define PCIE_PHY_TRSV_REG6 0x418
> > > -#define PCIE_PHY_TRSV_REG6_GEN2_DEEMP 0xF
> > >
> > > #define IMX8MM_GPR_PCIE_REF_CLK_SEL GENMASK(25, 24)
> > > #define IMX8MM_GPR_PCIE_REF_CLK_PLL
> > FIELD_PREP(IMX8MM_GPR_PCIE_REF_CLK_SEL, 0x3)
> > > @@ -46,16 +46,43 @@
> > > #define IMX8MM_GPR_PCIE_SSC_EN BIT(16)
> > > #define IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE BIT(9)
> > >
> > > +#define IMX8MP_GPR_REG0 0x0
> > > +#define IMX8MP_GPR_CLK_MOD_EN BIT(0)
> > > +#define IMX8MP_GPR_PHY_APB_RST BIT(4)
> > > +#define IMX8MP_GPR_PHY_INIT_RST BIT(5)
> > > +#define IMX8MP_GPR_REG1 0x4
> > > +#define IMX8MP_GPR_PM_EN_CORE_CLK BIT(0)
> > > +#define IMX8MP_GPR_PLL_LOCK BIT(13)
> > > +#define IMX8MP_GPR_REG2 0x8
> > > +#define IMX8MP_GPR_P_PLL_MASK GENMASK(5, 0)
> > > +#define IMX8MP_GPR_M_PLL_MASK GENMASK(15, 6)
> > > +#define IMX8MP_GPR_S_PLL_MASK GENMASK(18, 16)
> > > +#define IMX8MP_GPR_P_PLL (0xc << 0)
> > > +#define IMX8MP_GPR_M_PLL (0x320 << 6)
> > > +#define IMX8MP_GPR_S_PLL (0x4 << 16)
> > > +#define IMX8MP_GPR_REG3 0xc
> > > +#define IMX8MP_GPR_PLL_CKE BIT(17)
> > > +#define IMX8MP_GPR_PLL_RST BIT(31)
> > > +
> > > +enum imx8_pcie_phy_type {
> > > + IMX8MM,
> > > + IMX8MP,
> > > +};
> > > +
> > > struct imx8_pcie_phy {
> > > void __iomem *base;
> > > + struct device *dev;
> > > struct clk *clk;
> > > struct phy *phy;
> > > + struct regmap *hsio_blk_ctrl;
> > > struct regmap *iomuxc_gpr;
> > > struct reset_control *reset;
> > > + struct reset_control *perst;
> > > u32 refclk_pad_mode;
> > > u32 tx_deemph_gen1;
> > > u32 tx_deemph_gen2;
> > > bool clkreq_unused;
> > > + enum imx8_pcie_phy_type variant;
> > > };
> > >
> > > static int imx8_pcie_phy_init(struct phy *phy) @@ -67,6 +94,87 @@
> > > static int imx8_pcie_phy_init(struct phy *phy)
> > > reset_control_assert(imx8_phy->reset);
> > >
> > > pad_mode = imx8_phy->refclk_pad_mode;
> > > + switch (imx8_phy->variant) {
> > > + case IMX8MM:
> > > + /* Tune PHY de-emphasis setting to pass PCIe compliance. */
> > > + if (imx8_phy->tx_deemph_gen1)
> > > + writel(imx8_phy->tx_deemph_gen1,
> > > + imx8_phy->base + PCIE_PHY_TRSV_REG5);
> > > + if (imx8_phy->tx_deemph_gen2)
> > > + writel(imx8_phy->tx_deemph_gen2,
> > > + imx8_phy->base + PCIE_PHY_TRSV_REG6);
> > > + break;
> > > + case IMX8MP:
> > > + reset_control_assert(imx8_phy->perst);
> >
> > Could you tell us something more about this reset. What exactly is it resetting.
> > Do we really need to assert it before starting the HSIO PLL?
> Yes, this reset should be asserted, otherwise, the PCIe wouldn't work.
> I'm asking more details of this reset bit from design team, and would update
> later after I get the response.
>
> > AFAICS the PLL should be pretty much independent of the PHY.
> Agree.
>
> >
> > Do we need to enable this PLL when the PHY gets an external refclock? I
> > couldn't test it yet, but I suspect that the HSIO PLL is only needed as an
> > internal reference, when the i.MX8MP is the refclock source, either through
> > the PHY pads or via a clkout from the PLL.
> >
> Refer to my experience, the HSIO PLL should be enabled firstly.
>
> > > + /* Set P=12,M=800,S=4 and must set ICP=2'b01. */
> > > + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG2,
> > > + IMX8MP_GPR_P_PLL_MASK |
> > > + IMX8MP_GPR_M_PLL_MASK |
> > > + IMX8MP_GPR_S_PLL_MASK,
> > > + IMX8MP_GPR_P_PLL |
> > > + IMX8MP_GPR_M_PLL |
> > > + IMX8MP_GPR_S_PLL);
> > > + /* wait greater than 1/F_FREF =1/2MHZ=0.5us */
> > > + udelay(1);
> > > +
> > > + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG3,
> > > + IMX8MP_GPR_PLL_RST,
> > > + IMX8MP_GPR_PLL_RST);
> > > + udelay(10);
> > > +
> > > + /* Set 1 to pll_cke of GPR_REG3 */
> > > + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG3,
> > > + IMX8MP_GPR_PLL_CKE,
> > > + IMX8MP_GPR_PLL_CKE);
> > > +
> > > + /* Lock time should be greater than 300cycle=300*0.5us=150us */
> > > + ret = regmap_read_poll_timeout(imx8_phy->hsio_blk_ctrl,
> > > + IMX8MP_GPR_REG1, val,
> > > + val & IMX8MP_GPR_PLL_LOCK,
> > > + 10, 1000);
> > > + if (ret) {
> > > + dev_err(imx8_phy->dev, "PCIe PLL lock timeout\n");
> > > + return ret;
> > > + }
> > > +
> > > + /* pcie_clock_module_en */
> > > + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG0,
> > > + IMX8MP_GPR_CLK_MOD_EN,
> > > + IMX8MP_GPR_CLK_MOD_EN);
> >
> > You shouldn't need to touch this bit. The HSIO blk-ctrl already enables this bit
> > when the PCIe power-domain is powered up.
> Okay, got that.
>
> >
> > > + udelay(10);
> > > +
> > > + reset_control_deassert(imx8_phy->reset);
> > > + reset_control_deassert(imx8_phy->perst);
> > > +
> > > + /* release pcie_phy_apb_reset and pcie_phy_init_resetn */
> > > + regmap_update_bits(imx8_phy->hsio_blk_ctrl, IMX8MP_GPR_REG0,
> > > + IMX8MP_GPR_PHY_APB_RST |
> > > + IMX8MP_GPR_PHY_INIT_RST,
> > > + IMX8MP_GPR_PHY_APB_RST |
> > > + IMX8MP_GPR_PHY_INIT_RST);
> >
> > Not sure about those yet. We might want to toggle them via a virtual PD in the
> > HSIO blk-ctrl.
> Refer to my understand, these reset should be a part of power-up sequence of
> the PHY. It's reasonable to toggle them via a PD.
So I had a chance to look into why this series isn't working for me
some more.
It seems the full PHY initialization fails, as the complete PHY MMIO
region reads back as 0xff. This hints at either a missing clock, or
(more likely) the register interface of the PHY being held in reset.
Note that I'm running upstream TF-A and the Barebox bootloader, so this
might be a missing initialization somewhere, that is done by downstream
TF-A or U-Boot.
Sadly the above bits are also not documented in the RM, but are marked
as reserved. By chance, do you know about any other secondary
clocks/resets that may have an impact on PCIe?
Regards,
Lucas
> -----Original Message-----
> From: Lucas Stach <[email protected]>
> Sent: 2022年4月27日 23:19
> To: Hongxing Zhu <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>
> Subject: Re: [PATCH v2 3/7] phy: freescale: imx8m-pcie: Add iMX8MP PCIe PHY
> support
>
> Hi Richard,
>
> Am Montag, dem 18.04.2022 um 04:55 +0000 schrieb Hongxing Zhu:
> > > -----Original Message-----
> > > From: Lucas Stach <[email protected]>
> > > Sent: 2022年4月15日 4:59
> > > To: Hongxing Zhu <[email protected]>; [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]
> > > Cc: [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected]; dl-linux-imx
> > > <[email protected]>
> > > Subject: Re: [PATCH v2 3/7] phy: freescale: imx8m-pcie: Add iMX8MP
> > > PCIe PHY support
> > >
> > > Am Montag, dem 07.03.2022 um 17:07 +0800 schrieb Richard Zhu:
> > > > Add the i.MX8MP PCIe PHY support
> > > >
> > > > Signed-off-by: Richard Zhu <[email protected]>
> > > > ---
> > > > drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 205
> > > > ++++++++++++++++-----
> > > > 1 file changed, 163 insertions(+), 42 deletions(-)
> > > >
> > > > diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > > b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > > index 04b1aafb29f4..3d01da4323a6 100644
> > > > --- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > > +++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > > @@ -11,6 +11,8 @@
> > > > #include <linux/mfd/syscon.h>
> > > > #include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
> > > > #include <linux/module.h>
> > > > +#include <linux/of_address.h>
> > > > +#include <linux/of_device.h>
> > > > #include <linux/phy/phy.h>
> > > > #include <linux/platform_device.h>
> > > > #include <linux/regmap.h>
> > > > @@ -30,12 +32,10 @@
> > > > #define IMX8MM_PCIE_PHY_CMN_REG065 0x194
> > > > #define ANA_AUX_RX_TERM (BIT(7) | BIT(4))
> > > > #define ANA_AUX_TX_LVL GENMASK(3, 0)
> > > > -#define IMX8MM_PCIE_PHY_CMN_REG75 0x1D4
> > > > -#define PCIE_PHY_CMN_REG75_PLL_DONE 0x3
> > > > +#define IMX8MM_PCIE_PHY_CMN_REG075 0x1D4
> > > > +#define ANA_PLL_DONE 0x3
> > >
> > > Why do you drop the register prefix from the name here?
> > To prevent the codes from exceeding the 80 columns and align with the
> > other
> > bit definitions, drop the prefix and keep the bit definitions as
> > short as
> > possible.
> >
> > >
> > > > #define PCIE_PHY_TRSV_REG5 0x414
> > > > -#define PCIE_PHY_TRSV_REG5_GEN1_DEEMP 0x2D
> > > > #define PCIE_PHY_TRSV_REG6 0x418
> > > > -#define PCIE_PHY_TRSV_REG6_GEN2_DEEMP 0xF
> > > >
> > > > #define IMX8MM_GPR_PCIE_REF_CLK_SEL GENMASK(25, 24)
> > > > #define IMX8MM_GPR_PCIE_REF_CLK_PLL
> > > FIELD_PREP(IMX8MM_GPR_PCIE_REF_CLK_SEL, 0x3)
> > > > @@ -46,16 +46,43 @@
> > > > #define IMX8MM_GPR_PCIE_SSC_EN BIT(16)
> > > > #define IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE BIT(9)
> > > >
> > > > +#define IMX8MP_GPR_REG0 0x0
> > > > +#define IMX8MP_GPR_CLK_MOD_EN BIT(0)
> > > > +#define IMX8MP_GPR_PHY_APB_RST BIT(4)
> > > > +#define IMX8MP_GPR_PHY_INIT_RST BIT(5)
> > > > +#define IMX8MP_GPR_REG1 0x4
> > > > +#define IMX8MP_GPR_PM_EN_CORE_CLK BIT(0)
> > > > +#define IMX8MP_GPR_PLL_LOCK BIT(13)
> > > > +#define IMX8MP_GPR_REG2 0x8
> > > > +#define IMX8MP_GPR_P_PLL_MASK GENMASK(5, 0)
> > > > +#define IMX8MP_GPR_M_PLL_MASK GENMASK(15, 6)
> > > > +#define IMX8MP_GPR_S_PLL_MASK GENMASK(18, 16)
> > > > +#define IMX8MP_GPR_P_PLL (0xc << 0)
> > > > +#define IMX8MP_GPR_M_PLL (0x320 << 6)
> > > > +#define IMX8MP_GPR_S_PLL (0x4 << 16)
> > > > +#define IMX8MP_GPR_REG3 0xc
> > > > +#define IMX8MP_GPR_PLL_CKE BIT(17)
> > > > +#define IMX8MP_GPR_PLL_RST BIT(31)
> > > > +
> > > > +enum imx8_pcie_phy_type {
> > > > + IMX8MM,
> > > > + IMX8MP,
> > > > +};
> > > > +
> > > > struct imx8_pcie_phy {
> > > > void __iomem *base;
> > > > + struct device *dev;
> > > > struct clk *clk;
> > > > struct phy *phy;
> > > > + struct regmap *hsio_blk_ctrl;
> > > > struct regmap *iomuxc_gpr;
> > > > struct reset_control *reset;
> > > > + struct reset_control *perst;
> > > > u32 refclk_pad_mode;
> > > > u32 tx_deemph_gen1;
> > > > u32 tx_deemph_gen2;
> > > > bool clkreq_unused;
> > > > + enum imx8_pcie_phy_type variant;
> > > > };
> > > >
> > > > static int imx8_pcie_phy_init(struct phy *phy) @@ -67,6 +94,87 @@
> > > > static int imx8_pcie_phy_init(struct phy *phy)
> > > > reset_control_assert(imx8_phy->reset);
> > > >
> > > > pad_mode = imx8_phy->refclk_pad_mode;
> > > > + switch (imx8_phy->variant) {
> > > > + case IMX8MM:
> > > > + /* Tune PHY de-emphasis setting to pass PCIe compliance. */
> > > > + if (imx8_phy->tx_deemph_gen1)
> > > > + writel(imx8_phy->tx_deemph_gen1,
> > > > + imx8_phy->base + PCIE_PHY_TRSV_REG5);
> > > > + if (imx8_phy->tx_deemph_gen2)
> > > > + writel(imx8_phy->tx_deemph_gen2,
> > > > + imx8_phy->base + PCIE_PHY_TRSV_REG6);
> > > > + break;
> > > > + case IMX8MP:
> > > > + reset_control_assert(imx8_phy->perst);
> > >
> > > Could you tell us something more about this reset. What exactly is it
> resetting.
> > > Do we really need to assert it before starting the HSIO PLL?
> > Yes, this reset should be asserted, otherwise, the PCIe wouldn't work.
> > I'm asking more details of this reset bit from design team, and would
> > update
> > later after I get the response.
> >
> > > AFAICS the PLL should be pretty much independent of the PHY.
> > Agree.
> >
> > >
> > > Do we need to enable this PLL when the PHY gets an external
> > > refclock? I couldn't test it yet, but I suspect that the HSIO PLL is
> > > only needed as an internal reference, when the i.MX8MP is the
> > > refclock source, either through the PHY pads or via a clkout from the PLL.
> > >
> > Refer to my experience, the HSIO PLL should be enabled firstly.
> >
> > > > + /* Set P=12,M=800,S=4 and must set ICP=2'b01. */
> > > > + regmap_update_bits(imx8_phy->hsio_blk_ctrl,
> IMX8MP_GPR_REG2,
> > > > + IMX8MP_GPR_P_PLL_MASK |
> > > > + IMX8MP_GPR_M_PLL_MASK |
> > > > + IMX8MP_GPR_S_PLL_MASK,
> > > > + IMX8MP_GPR_P_PLL |
> > > > + IMX8MP_GPR_M_PLL |
> > > > + IMX8MP_GPR_S_PLL);
> > > > + /* wait greater than 1/F_FREF =1/2MHZ=0.5us */
> > > > + udelay(1);
> > > > +
> > > > + regmap_update_bits(imx8_phy->hsio_blk_ctrl,
> IMX8MP_GPR_REG3,
> > > > + IMX8MP_GPR_PLL_RST,
> > > > + IMX8MP_GPR_PLL_RST);
> > > > + udelay(10);
> > > > +
> > > > + /* Set 1 to pll_cke of GPR_REG3 */
> > > > + regmap_update_bits(imx8_phy->hsio_blk_ctrl,
> IMX8MP_GPR_REG3,
> > > > + IMX8MP_GPR_PLL_CKE,
> > > > + IMX8MP_GPR_PLL_CKE);
> > > > +
> > > > + /* Lock time should be greater than 300cycle=300*0.5us=150us
> */
> > > > + ret = regmap_read_poll_timeout(imx8_phy->hsio_blk_ctrl,
> > > > + IMX8MP_GPR_REG1, val,
> > > > + val & IMX8MP_GPR_PLL_LOCK,
> > > > + 10, 1000);
> > > > + if (ret) {
> > > > + dev_err(imx8_phy->dev, "PCIe PLL lock timeout\n");
> > > > + return ret;
> > > > + }
> > > > +
> > > > + /* pcie_clock_module_en */
> > > > + regmap_update_bits(imx8_phy->hsio_blk_ctrl,
> IMX8MP_GPR_REG0,
> > > > + IMX8MP_GPR_CLK_MOD_EN,
> > > > + IMX8MP_GPR_CLK_MOD_EN);
> > >
> > > You shouldn't need to touch this bit. The HSIO blk-ctrl already
> > > enables this bit when the PCIe power-domain is powered up.
> > Okay, got that.
> >
> > >
> > > > + udelay(10);
> > > > +
> > > > + reset_control_deassert(imx8_phy->reset);
> > > > + reset_control_deassert(imx8_phy->perst);
> > > > +
> > > > + /* release pcie_phy_apb_reset and pcie_phy_init_resetn */
> > > > + regmap_update_bits(imx8_phy->hsio_blk_ctrl,
> IMX8MP_GPR_REG0,
> > > > + IMX8MP_GPR_PHY_APB_RST |
> > > > + IMX8MP_GPR_PHY_INIT_RST,
> > > > + IMX8MP_GPR_PHY_APB_RST |
> > > > + IMX8MP_GPR_PHY_INIT_RST);
> > >
> > > Not sure about those yet. We might want to toggle them via a virtual
> > > PD in the HSIO blk-ctrl.
> > Refer to my understand, these reset should be a part of power-up
> > sequence of
> > the PHY. It's reasonable to toggle them via a PD.
>
> So I had a chance to look into why this series isn't working for me some more.
>
> It seems the full PHY initialization fails, as the complete PHY MMIO region
> reads back as 0xff. This hints at either a missing clock, or (more likely) the
> register interface of the PHY being held in reset.
> Note that I'm running upstream TF-A and the Barebox bootloader, so this
> might be a missing initialization somewhere, that is done by downstream TF-A
> or U-Boot.
>
> Sadly the above bits are also not documented in the RM, but are marked as
> reserved. By chance, do you know about any other secondary clocks/resets
> that may have an impact on PCIe?
>
Hi Lucas:
Thanks for your help to look at this series.
Refer to your descriptions, it seems that one initial version i.MX865 chip
is used at your side.
There is a design bug in the initial version of i.MX865 PCIe. All the PHY MMIO
region reads back as 0xff.
It is fixed by the later chips. Find another newer board is a quick method to
fix it. Or, apply one SW workaround to let it work only in Gen1/Gen2 modes.
Best Regards
Richard Zhu
> Regards,
> Lucas
On Mon, Mar 07, 2022 at 05:07:34PM +0800, Richard Zhu wrote:
> Add the i.MX8MP PCIe support.
>
> Signed-off-by: Richard Zhu <[email protected]>
> ---
> drivers/pci/controller/dwc/pci-imx6.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
I expect this series will eventually go via the imx6 platform tree.
To avoid you waiting for me when this series is deemed acceptable:
Acked-by: Lorenzo Pieralisi <[email protected]>
I will mark it as "handled elsewhere" in the PCI tree patchwork.
Lorenzo
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index bb662f90d4f3..4d34f0c88550 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -51,6 +51,7 @@ enum imx6_pcie_variants {
> IMX7D,
> IMX8MQ,
> IMX8MM,
> + IMX8MP,
> };
>
> #define IMX6_PCIE_FLAG_IMX6_PHY BIT(0)
> @@ -379,6 +380,7 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> reset_control_assert(imx6_pcie->pciephy_reset);
> fallthrough;
> case IMX8MM:
> + case IMX8MP:
> reset_control_assert(imx6_pcie->apps_reset);
> break;
> case IMX6SX:
> @@ -407,7 +409,8 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> static unsigned int imx6_pcie_grp_offset(const struct imx6_pcie *imx6_pcie)
> {
> WARN_ON(imx6_pcie->drvdata->variant != IMX8MQ &&
> - imx6_pcie->drvdata->variant != IMX8MM);
> + imx6_pcie->drvdata->variant != IMX8MM &&
> + imx6_pcie->drvdata->variant != IMX8MP);
> return imx6_pcie->controller_id == 1 ? IOMUXC_GPR16 : IOMUXC_GPR14;
> }
>
> @@ -448,6 +451,7 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
> break;
> case IMX8MM:
> case IMX8MQ:
> + case IMX8MP:
> ret = clk_prepare_enable(imx6_pcie->pcie_aux);
> if (ret) {
> dev_err(dev, "unable to enable pcie_aux clock\n");
> @@ -503,6 +507,7 @@ static int imx6_pcie_clk_enable(struct imx6_pcie *imx6_pcie)
>
> switch (imx6_pcie->drvdata->variant) {
> case IMX8MM:
> + case IMX8MP:
> if (phy_power_on(imx6_pcie->phy))
> dev_err(dev, "unable to power on PHY\n");
> break;
> @@ -603,6 +608,7 @@ static int imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
> reset_control_deassert(imx6_pcie->pciephy_reset);
> break;
> case IMX8MM:
> + case IMX8MP:
> if (phy_init(imx6_pcie->phy))
> dev_err(dev, "waiting for phy ready timeout!\n");
> break;
> @@ -678,6 +684,7 @@ static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
> {
> switch (imx6_pcie->drvdata->variant) {
> case IMX8MM:
> + case IMX8MP:
> /*
> * The PHY initialization had been done in the PHY
> * driver, break here directly.
> @@ -823,6 +830,7 @@ static void imx6_pcie_ltssm_enable(struct device *dev)
> case IMX7D:
> case IMX8MQ:
> case IMX8MM:
> + case IMX8MP:
> reset_control_deassert(imx6_pcie->apps_reset);
> break;
> }
> @@ -938,6 +946,7 @@ static void imx6_pcie_host_exit(struct pcie_port *pp)
> imx6_pcie_clk_disable(imx6_pcie);
> switch (imx6_pcie->drvdata->variant) {
> case IMX8MM:
> + case IMX8MP:
> if (phy_power_off(imx6_pcie->phy))
> dev_err(dev, "unable to power off phy\n");
> phy_exit(imx6_pcie->phy);
> @@ -972,6 +981,7 @@ static void imx6_pcie_ltssm_disable(struct device *dev)
> break;
> case IMX7D:
> case IMX8MM:
> + case IMX8MP:
> reset_control_assert(imx6_pcie->apps_reset);
> break;
> default:
> @@ -1028,6 +1038,7 @@ static int imx6_pcie_suspend_noirq(struct device *dev)
> imx6_pcie_clk_disable(imx6_pcie);
> switch (imx6_pcie->drvdata->variant) {
> case IMX8MM:
> + case IMX8MP:
> if (phy_power_off(imx6_pcie->phy))
> dev_err(dev, "unable to power off PHY\n");
> phy_exit(imx6_pcie->phy);
> @@ -1177,6 +1188,7 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> }
> break;
> case IMX8MM:
> + case IMX8MP:
> 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),
> @@ -1327,6 +1339,10 @@ static const struct imx6_pcie_drvdata drvdata[] = {
> .variant = IMX8MM,
> .flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
> },
> + [IMX8MP] = {
> + .variant = IMX8MP,
> + .flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
> + },
> };
>
> static const struct of_device_id imx6_pcie_of_match[] = {
> @@ -1336,6 +1352,7 @@ static const struct of_device_id imx6_pcie_of_match[] = {
> { .compatible = "fsl,imx7d-pcie", .data = &drvdata[IMX7D], },
> { .compatible = "fsl,imx8mq-pcie", .data = &drvdata[IMX8MQ], },
> { .compatible = "fsl,imx8mm-pcie", .data = &drvdata[IMX8MM], },
> + { .compatible = "fsl,imx8mp-pcie", .data = &drvdata[IMX8MP], },
> {},
> };
>
> --
> 2.25.1
>
Hi Lorenzo:
> -----Original Message-----
> From: Lorenzo Pieralisi <[email protected]>
> Sent: 2022??5??13?? 0:09
> To: Hongxing Zhu <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; dl-linux-imx <[email protected]>
> Subject: Re: [PATCH v2 7/7] PCI: imx6: Add the iMX8MP PCIe support
>
> On Mon, Mar 07, 2022 at 05:07:34PM +0800, Richard Zhu wrote:
> > Add the i.MX8MP PCIe support.
> >
> > Signed-off-by: Richard Zhu <[email protected]>
> > ---
> > drivers/pci/controller/dwc/pci-imx6.c | 19 ++++++++++++++++++-
> > 1 file changed, 18 insertions(+), 1 deletion(-)
>
> I expect this series will eventually go via the imx6 platform tree.
>
> To avoid you waiting for me when this series is deemed acceptable:
>
> Acked-by: Lorenzo Pieralisi <[email protected]>
>
> I will mark it as "handled elsewhere" in the PCI tree patchwork.
Thanks for your kindly help.
Lucas has some suggestions and advices about the HSIOMIX bits manipulations
in the PHY driver of this series.
Would issue the next version, after co-operate with Lucas and settle-down
that part.
Best Regards
Richard
>
> Lorenzo
>
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index bb662f90d4f3..4d34f0c88550 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -51,6 +51,7 @@ enum imx6_pcie_variants {
> > IMX7D,
> > IMX8MQ,
> > IMX8MM,
> > + IMX8MP,
> > };
> >
> > #define IMX6_PCIE_FLAG_IMX6_PHY BIT(0)
> > @@ -379,6 +380,7 @@ static void imx6_pcie_assert_core_reset(struct
> imx6_pcie *imx6_pcie)
> > reset_control_assert(imx6_pcie->pciephy_reset);
> > fallthrough;
> > case IMX8MM:
> > + case IMX8MP:
> > reset_control_assert(imx6_pcie->apps_reset);
> > break;
> > case IMX6SX:
> > @@ -407,7 +409,8 @@ static void imx6_pcie_assert_core_reset(struct
> > imx6_pcie *imx6_pcie) static unsigned int imx6_pcie_grp_offset(const
> > struct imx6_pcie *imx6_pcie) {
> > WARN_ON(imx6_pcie->drvdata->variant != IMX8MQ &&
> > - imx6_pcie->drvdata->variant != IMX8MM);
> > + imx6_pcie->drvdata->variant != IMX8MM &&
> > + imx6_pcie->drvdata->variant != IMX8MP);
> > return imx6_pcie->controller_id == 1 ? IOMUXC_GPR16 : IOMUXC_GPR14;
> > }
> >
> > @@ -448,6 +451,7 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie
> *imx6_pcie)
> > break;
> > case IMX8MM:
> > case IMX8MQ:
> > + case IMX8MP:
> > ret = clk_prepare_enable(imx6_pcie->pcie_aux);
> > if (ret) {
> > dev_err(dev, "unable to enable pcie_aux clock\n"); @@ -503,6
> > +507,7 @@ static int imx6_pcie_clk_enable(struct imx6_pcie *imx6_pcie)
> >
> > switch (imx6_pcie->drvdata->variant) {
> > case IMX8MM:
> > + case IMX8MP:
> > if (phy_power_on(imx6_pcie->phy))
> > dev_err(dev, "unable to power on PHY\n");
> > break;
> > @@ -603,6 +608,7 @@ static int imx6_pcie_deassert_core_reset(struct
> imx6_pcie *imx6_pcie)
> > reset_control_deassert(imx6_pcie->pciephy_reset);
> > break;
> > case IMX8MM:
> > + case IMX8MP:
> > if (phy_init(imx6_pcie->phy))
> > dev_err(dev, "waiting for phy ready timeout!\n");
> > break;
> > @@ -678,6 +684,7 @@ static void imx6_pcie_init_phy(struct imx6_pcie
> > *imx6_pcie) {
> > switch (imx6_pcie->drvdata->variant) {
> > case IMX8MM:
> > + case IMX8MP:
> > /*
> > * The PHY initialization had been done in the PHY
> > * driver, break here directly.
> > @@ -823,6 +830,7 @@ static void imx6_pcie_ltssm_enable(struct device
> *dev)
> > case IMX7D:
> > case IMX8MQ:
> > case IMX8MM:
> > + case IMX8MP:
> > reset_control_deassert(imx6_pcie->apps_reset);
> > break;
> > }
> > @@ -938,6 +946,7 @@ static void imx6_pcie_host_exit(struct pcie_port
> *pp)
> > imx6_pcie_clk_disable(imx6_pcie);
> > switch (imx6_pcie->drvdata->variant) {
> > case IMX8MM:
> > + case IMX8MP:
> > if (phy_power_off(imx6_pcie->phy))
> > dev_err(dev, "unable to power off phy\n");
> > phy_exit(imx6_pcie->phy);
> > @@ -972,6 +981,7 @@ static void imx6_pcie_ltssm_disable(struct device
> *dev)
> > break;
> > case IMX7D:
> > case IMX8MM:
> > + case IMX8MP:
> > reset_control_assert(imx6_pcie->apps_reset);
> > break;
> > default:
> > @@ -1028,6 +1038,7 @@ static int imx6_pcie_suspend_noirq(struct device
> *dev)
> > imx6_pcie_clk_disable(imx6_pcie);
> > switch (imx6_pcie->drvdata->variant) {
> > case IMX8MM:
> > + case IMX8MP:
> > if (phy_power_off(imx6_pcie->phy))
> > dev_err(dev, "unable to power off PHY\n");
> > phy_exit(imx6_pcie->phy);
> > @@ -1177,6 +1188,7 @@ static int imx6_pcie_probe(struct platform_device
> *pdev)
> > }
> > break;
> > case IMX8MM:
> > + case IMX8MP:
> > 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), @@
> -1327,6
> > +1339,10 @@ static const struct imx6_pcie_drvdata drvdata[] = {
> > .variant = IMX8MM,
> > .flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
> > },
> > + [IMX8MP] = {
> > + .variant = IMX8MP,
> > + .flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
> > + },
> > };
> >
> > static const struct of_device_id imx6_pcie_of_match[] = { @@ -1336,6
> > +1352,7 @@ static const struct of_device_id imx6_pcie_of_match[] = {
> > { .compatible = "fsl,imx7d-pcie", .data = &drvdata[IMX7D], },
> > { .compatible = "fsl,imx8mq-pcie", .data = &drvdata[IMX8MQ], },
> > { .compatible = "fsl,imx8mm-pcie", .data = &drvdata[IMX8MM], },
> > + { .compatible = "fsl,imx8mp-pcie", .data = &drvdata[IMX8MP], },
> > {},
> > };
> >
> > --
> > 2.25.1
> >
On Sun, Apr 17, 2022 at 10:00 PM Hongxing Zhu <[email protected]> wrote:
>
> > -----Original Message-----
> > From: Lucas Stach <[email protected]>
> > Sent: 2022年4月15日 5:03
> > To: Hongxing Zhu <[email protected]>; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected]
> > Cc: [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected]; dl-linux-imx
> > <[email protected]>
> > Subject: Re: [PATCH v2 5/7] arm64: dts: imx8mp: add the iMX8MP PCIe
> > support
> >
> > Am Montag, dem 07.03.2022 um 17:07 +0800 schrieb Richard Zhu:
> > > Add the i.MX8MP PCIe support.
> > >
> > > Signed-off-by: Richard Zhu <[email protected]>
> > > ---
> > > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 46
> > > ++++++++++++++++++++++-
> > > 1 file changed, 45 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > index b40a5646f205..e7b3d8029e34 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > @@ -5,6 +5,7 @@
> > >
> > > #include <dt-bindings/clock/imx8mp-clock.h>
> > > #include <dt-bindings/power/imx8mp-power.h>
> > > +#include <dt-bindings/reset/imx8mp-reset.h>
> > > #include <dt-bindings/gpio/gpio.h>
> > > #include <dt-bindings/input/input.h>
> > > #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > @@ -375,7 +376,8 @@ iomuxc: pinctrl@30330000 {
> > > };
> > >
> > > gpr: iomuxc-gpr@30340000 {
> > > - compatible = "fsl,imx8mp-iomuxc-gpr", "syscon";
> > > + compatible = "fsl,imx8mp-iomuxc-gpr",
> > > + "fsl,imx6q-iomuxc-gpr", "syscon";
> > > reg = <0x30340000 0x10000>;
> > > };
> > >
> > > @@ -965,6 +967,17 @@ aips4: bus@32c00000 {
> > > #size-cells = <1>;
> > > ranges;
> > >
> > > + pcie_phy: pcie-phy@32f00000 {
> > > + compatible = "fsl,imx8mp-pcie-phy";
> > > + reg = <0x32f00000 0x10000>;
> > > + resets = <&src IMX8MP_RESET_PCIEPHY>,
> > > + <&src IMX8MP_RESET_PCIEPHY_PERST>;
> > > + reset-names = "pciephy", "perst";
> > > + power-domains = <&hsio_blk_ctrl
> > IMX8MP_HSIOBLK_PD_PCIE_PHY>;
> > > + #phy-cells = <0>;
> > > + status = "disabled";
> > > + };
> > > +
> > > hsio_blk_ctrl: blk-ctrl@32f10000 {
> > > compatible = "fsl,imx8mp-hsio-blk-ctrl", "syscon";
> > > reg = <0x32f10000 0x24>;
> > > @@ -980,6 +993,37 @@ hsio_blk_ctrl: blk-ctrl@32f10000 {
> > > };
> > > };
> > >
> > > + pcie: pcie@33800000 {
> > > + compatible = "fsl,imx8mp-pcie";
> > > + reg = <0x33800000 0x400000>, <0x1ff00000 0x80000>;
> > > + reg-names = "dbi", "config";
> > > + #address-cells = <3>;
> > > + #size-cells = <2>;
> > > + device_type = "pci";
> > > + bus-range = <0x00 0xff>;
> > > + ranges = <0x81000000 0 0x00000000 0x1ff80000 0
> > 0x00010000 /* downstream I/O 64KB */
> > > + 0x82000000 0 0x18000000 0x18000000 0
> > 0x07f00000>; /* non-prefetchable memory */
> > > + num-lanes = <1>;
> > > + num-viewport = <4>;
> > > + interrupts = <GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>;
> > > + interrupt-names = "msi";
> > > + #interrupt-cells = <1>;
> > > + interrupt-map-mask = <0 0 0 0x7>;
> > > + interrupt-map = <0 0 0 1 &gic GIC_SPI 126
> > IRQ_TYPE_LEVEL_HIGH>,
> > > + <0 0 0 2 &gic GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
> > > + <0 0 0 3 &gic GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>,
> > > + <0 0 0 4 &gic GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
> > > + fsl,max-link-speed = <3>;
> >
> > I believe that imx6_pcie_start_link does not properly handle Gen3 speeds.
> Good caught.
> The according link_gen condition should be changed in driver too.
> Would be changed in next version.
> Thanks.
>
> Best Regards
> Richard Zhu
> >
> > Regards,
> > Lucas
> >
> > > + linux,pci-domain = <0>;
> > > + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_PCIE>;
> > > + resets = <&src IMX8MP_RESET_PCIE_CTRL_APPS_EN>,
> > > + <&src IMX8MP_RESET_PCIE_CTRL_APPS_TURNOFF>;
> > > + reset-names = "apps", "turnoff";
> > > + phys = <&pcie_phy>;
> > > + phy-names = "pcie-phy";
> > > + status = "disabled";
> > > + };
> > > +
> > > gpu3d: gpu@38000000 {
> > > compatible = "vivante,gc";
> > > reg = <0x38000000 0x8000>;
> >
>
Richard,
Do you have an updated series for IMX8MP PCIe yet? I believe
everything you were waiting on is now merged (blk-ctrl and
power-domain).
Best Regards,
Tim
> -----Original Message-----
> From: Tim Harvey <[email protected]>
> Sent: 2022年5月24日 2:48
> To: Hongxing Zhu <[email protected]>
> Cc: Lucas Stach <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>
> Subject: Re: [PATCH v2 5/7] arm64: dts: imx8mp: add the iMX8MP PCIe
> support
>
> On Sun, Apr 17, 2022 at 10:00 PM Hongxing Zhu <[email protected]>
> wrote:
> >
> > > -----Original Message-----
> > > From: Lucas Stach <[email protected]>
> > > Sent: 2022年4月15日 5:03
> > > To: Hongxing Zhu <[email protected]>; [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]
> > > Cc: [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected]; dl-linux-imx
> > > <[email protected]>
> > > Subject: Re: [PATCH v2 5/7] arm64: dts: imx8mp: add the iMX8MP PCIe
> > > support
> > >
> > > Am Montag, dem 07.03.2022 um 17:07 +0800 schrieb Richard Zhu:
> > > > Add the i.MX8MP PCIe support.
> > > >
> > > > Signed-off-by: Richard Zhu <[email protected]>
> > > > ---
> > > > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 46
> > > > ++++++++++++++++++++++-
> > > > 1 file changed, 45 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > > index b40a5646f205..e7b3d8029e34 100644
> > > > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > > @@ -5,6 +5,7 @@
> > > >
> > > > #include <dt-bindings/clock/imx8mp-clock.h>
> > > > #include <dt-bindings/power/imx8mp-power.h>
> > > > +#include <dt-bindings/reset/imx8mp-reset.h>
> > > > #include <dt-bindings/gpio/gpio.h> #include
> > > > <dt-bindings/input/input.h> #include
> > > > <dt-bindings/interrupt-controller/arm-gic.h>
> > > > @@ -375,7 +376,8 @@ iomuxc: pinctrl@30330000 {
> > > > };
> > > >
> > > > gpr: iomuxc-gpr@30340000 {
> > > > - compatible = "fsl,imx8mp-iomuxc-gpr",
> "syscon";
> > > > + compatible = "fsl,imx8mp-iomuxc-gpr",
> > > > + "fsl,imx6q-iomuxc-gpr",
> > > > + "syscon";
> > > > reg = <0x30340000 0x10000>;
> > > > };
> > > >
> > > > @@ -965,6 +967,17 @@ aips4: bus@32c00000 {
> > > > #size-cells = <1>;
> > > > ranges;
> > > >
> > > > + pcie_phy: pcie-phy@32f00000 {
> > > > + compatible = "fsl,imx8mp-pcie-phy";
> > > > + reg = <0x32f00000 0x10000>;
> > > > + resets = <&src
> IMX8MP_RESET_PCIEPHY>,
> > > > + <&src
> IMX8MP_RESET_PCIEPHY_PERST>;
> > > > + reset-names = "pciephy", "perst";
> > > > + power-domains = <&hsio_blk_ctrl
> > > IMX8MP_HSIOBLK_PD_PCIE_PHY>;
> > > > + #phy-cells = <0>;
> > > > + status = "disabled";
> > > > + };
> > > > +
> > > > hsio_blk_ctrl: blk-ctrl@32f10000 {
> > > > compatible = "fsl,imx8mp-hsio-blk-ctrl",
> "syscon";
> > > > reg = <0x32f10000 0x24>; @@ -980,6
> > > > +993,37 @@ hsio_blk_ctrl: blk-ctrl@32f10000 {
> > > > };
> > > > };
> > > >
> > > > + pcie: pcie@33800000 {
> > > > + compatible = "fsl,imx8mp-pcie";
> > > > + reg = <0x33800000 0x400000>, <0x1ff00000
> 0x80000>;
> > > > + reg-names = "dbi", "config";
> > > > + #address-cells = <3>;
> > > > + #size-cells = <2>;
> > > > + device_type = "pci";
> > > > + bus-range = <0x00 0xff>;
> > > > + ranges = <0x81000000 0 0x00000000
> 0x1ff80000
> > > > + 0
> > > 0x00010000 /* downstream I/O 64KB */
> > > > + 0x82000000 0 0x18000000
> 0x18000000
> > > > + 0
> > > 0x07f00000>; /* non-prefetchable memory */
> > > > + num-lanes = <1>;
> > > > + num-viewport = <4>;
> > > > + interrupts = <GIC_SPI 140
> IRQ_TYPE_LEVEL_HIGH>;
> > > > + interrupt-names = "msi";
> > > > + #interrupt-cells = <1>;
> > > > + interrupt-map-mask = <0 0 0 0x7>;
> > > > + interrupt-map = <0 0 0 1 &gic GIC_SPI 126
> > > IRQ_TYPE_LEVEL_HIGH>,
> > > > + <0 0 0 2 &gic GIC_SPI 125
> IRQ_TYPE_LEVEL_HIGH>,
> > > > + <0 0 0 3 &gic GIC_SPI 124
> IRQ_TYPE_LEVEL_HIGH>,
> > > > + <0 0 0 4 &gic GIC_SPI 123
> IRQ_TYPE_LEVEL_HIGH>;
> > > > + fsl,max-link-speed = <3>;
> > >
> > > I believe that imx6_pcie_start_link does not properly handle Gen3 speeds.
> > Good caught.
> > The according link_gen condition should be changed in driver too.
> > Would be changed in next version.
> > Thanks.
> >
> > Best Regards
> > Richard Zhu
> > >
> > > Regards,
> > > Lucas
> > >
> > > > + linux,pci-domain = <0>;
> > > > + power-domains = <&hsio_blk_ctrl
> IMX8MP_HSIOBLK_PD_PCIE>;
> > > > + resets = <&src
> IMX8MP_RESET_PCIE_CTRL_APPS_EN>,
> > > > + <&src
> IMX8MP_RESET_PCIE_CTRL_APPS_TURNOFF>;
> > > > + reset-names = "apps", "turnoff";
> > > > + phys = <&pcie_phy>;
> > > > + phy-names = "pcie-phy";
> > > > + status = "disabled";
> > > > + };
> > > > +
> > > > gpu3d: gpu@38000000 {
> > > > compatible = "vivante,gc";
> > > > reg = <0x38000000 0x8000>;
> > >
> >
>
> Richard,
>
> Do you have an updated series for IMX8MP PCIe yet? I believe everything you
> were waiting on is now merged (blk-ctrl and power-domain).
Hi Tim:
Thanks for your kindly help.
Lucas has some suggestions and advices about the HSIOMIX bits manipulations
in the PHY driver of this series(#3 patch).
Would issue the next version, after co-operate with Lucas and settle-down
that part.
Best Regards
Richard Zhu
>
> Best Regards,
>
> Tim
Hi Lucas:
> -----Original Message-----
> From: Hongxing Zhu
> Sent: 2022年4月28日 9:30
> To: Lucas Stach <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>
> Subject: RE: [PATCH v2 3/7] phy: freescale: imx8m-pcie: Add iMX8MP PCIe PHY
> support
>
> > -----Original Message-----
> > From: Lucas Stach <[email protected]>
> > Sent: 2022年4月27日 23:19
> > To: Hongxing Zhu <[email protected]>; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected]
> > Cc: [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected]; dl-linux-imx
> > <[email protected]>
> > Subject: Re: [PATCH v2 3/7] phy: freescale: imx8m-pcie: Add iMX8MP
> > PCIe PHY support
> >
> > Hi Richard,
> >
> > Am Montag, dem 18.04.2022 um 04:55 +0000 schrieb Hongxing Zhu:
> > > > -----Original Message-----
> > > > From: Lucas Stach <[email protected]>
> > > > Sent: 2022年4月15日 4:59
> > > > To: Hongxing Zhu <[email protected]>; [email protected];
> > > > [email protected]; [email protected]; [email protected];
> > > > [email protected]; [email protected];
> > > > [email protected]
> > > > Cc: [email protected]; [email protected];
> > > > [email protected]; [email protected];
> > > > [email protected]; [email protected]; dl-linux-imx
> > > > <[email protected]>
> > > > Subject: Re: [PATCH v2 3/7] phy: freescale: imx8m-pcie: Add iMX8MP
> > > > PCIe PHY support
> > > >
> > > > Am Montag, dem 07.03.2022 um 17:07 +0800 schrieb Richard Zhu:
> > > > > Add the i.MX8MP PCIe PHY support
> > > > >
> > > > > Signed-off-by: Richard Zhu <[email protected]>
> > > > > ---
> > > > > drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 205
> > > > > ++++++++++++++++-----
> > > > > 1 file changed, 163 insertions(+), 42 deletions(-)
> > > > >
> > > > > diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > > > b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > > > index 04b1aafb29f4..3d01da4323a6 100644
> > > > > --- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > > > +++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > > > @@ -11,6 +11,8 @@
> > > > > #include <linux/mfd/syscon.h>
> > > > > #include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
> > > > > #include <linux/module.h>
> > > > > +#include <linux/of_address.h>
> > > > > +#include <linux/of_device.h>
> > > > > #include <linux/phy/phy.h>
> > > > > #include <linux/platform_device.h>
> > > > > #include <linux/regmap.h>
> > > > > @@ -30,12 +32,10 @@
> > > > > #define IMX8MM_PCIE_PHY_CMN_REG065 0x194
> > > > > #define ANA_AUX_RX_TERM (BIT(7) | BIT(4))
> > > > > #define ANA_AUX_TX_LVL GENMASK(3, 0)
> > > > > -#define IMX8MM_PCIE_PHY_CMN_REG75 0x1D4
> > > > > -#define PCIE_PHY_CMN_REG75_PLL_DONE 0x3
> > > > > +#define IMX8MM_PCIE_PHY_CMN_REG075 0x1D4
> > > > > +#define ANA_PLL_DONE 0x3
> > > >
> > > > Why do you drop the register prefix from the name here?
> > > To prevent the codes from exceeding the 80 columns and align with
> > > the other
> > > bit definitions, drop the prefix and keep the bit definitions as
> > > short as
> > > possible.
> > >
> > > >
> > > > > #define PCIE_PHY_TRSV_REG5 0x414
> > > > > -#define PCIE_PHY_TRSV_REG5_GEN1_DEEMP 0x2D
> > > > > #define PCIE_PHY_TRSV_REG6 0x418
> > > > > -#define PCIE_PHY_TRSV_REG6_GEN2_DEEMP 0xF
> > > > >
> > > > > #define IMX8MM_GPR_PCIE_REF_CLK_SEL GENMASK(25, 24)
> > > > > #define IMX8MM_GPR_PCIE_REF_CLK_PLL
> > > > FIELD_PREP(IMX8MM_GPR_PCIE_REF_CLK_SEL, 0x3)
> > > > > @@ -46,16 +46,43 @@
> > > > > #define IMX8MM_GPR_PCIE_SSC_EN BIT(16)
> > > > > #define IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE BIT(9)
> > > > >
> > > > > +#define IMX8MP_GPR_REG0 0x0
> > > > > +#define IMX8MP_GPR_CLK_MOD_EN BIT(0)
> > > > > +#define IMX8MP_GPR_PHY_APB_RST BIT(4)
> > > > > +#define IMX8MP_GPR_PHY_INIT_RST BIT(5)
> > > > > +#define IMX8MP_GPR_REG1 0x4
> > > > > +#define IMX8MP_GPR_PM_EN_CORE_CLK BIT(0)
> > > > > +#define IMX8MP_GPR_PLL_LOCK BIT(13)
> > > > > +#define IMX8MP_GPR_REG2 0x8
> > > > > +#define IMX8MP_GPR_P_PLL_MASK GENMASK(5, 0)
> > > > > +#define IMX8MP_GPR_M_PLL_MASK GENMASK(15, 6)
> > > > > +#define IMX8MP_GPR_S_PLL_MASK GENMASK(18, 16)
> > > > > +#define IMX8MP_GPR_P_PLL (0xc << 0)
> > > > > +#define IMX8MP_GPR_M_PLL (0x320 << 6)
> > > > > +#define IMX8MP_GPR_S_PLL (0x4 << 16)
> > > > > +#define IMX8MP_GPR_REG3 0xc
> > > > > +#define IMX8MP_GPR_PLL_CKE BIT(17)
> > > > > +#define IMX8MP_GPR_PLL_RST BIT(31)
> > > > > +
> > > > > +enum imx8_pcie_phy_type {
> > > > > + IMX8MM,
> > > > > + IMX8MP,
> > > > > +};
> > > > > +
> > > > > struct imx8_pcie_phy {
> > > > > void __iomem *base;
> > > > > + struct device *dev;
> > > > > struct clk *clk;
> > > > > struct phy *phy;
> > > > > + struct regmap *hsio_blk_ctrl;
> > > > > struct regmap *iomuxc_gpr;
> > > > > struct reset_control *reset;
> > > > > + struct reset_control *perst;
> > > > > u32 refclk_pad_mode;
> > > > > u32 tx_deemph_gen1;
> > > > > u32 tx_deemph_gen2;
> > > > > bool clkreq_unused;
> > > > > + enum imx8_pcie_phy_type variant;
> > > > > };
> > > > >
> > > > > static int imx8_pcie_phy_init(struct phy *phy) @@ -67,6 +94,87
> > > > > @@ static int imx8_pcie_phy_init(struct phy *phy)
> > > > > reset_control_assert(imx8_phy->reset);
> > > > >
> > > > > pad_mode = imx8_phy->refclk_pad_mode;
> > > > > + switch (imx8_phy->variant) {
> > > > > + case IMX8MM:
> > > > > + /* Tune PHY de-emphasis setting to pass PCIe compliance. */
> > > > > + if (imx8_phy->tx_deemph_gen1)
> > > > > + writel(imx8_phy->tx_deemph_gen1,
> > > > > + imx8_phy->base + PCIE_PHY_TRSV_REG5);
> > > > > + if (imx8_phy->tx_deemph_gen2)
> > > > > + writel(imx8_phy->tx_deemph_gen2,
> > > > > + imx8_phy->base + PCIE_PHY_TRSV_REG6);
> > > > > + break;
> > > > > + case IMX8MP:
> > > > > + reset_control_assert(imx8_phy->perst);
> > > >
> > > > Could you tell us something more about this reset. What exactly is
> > > > it
> > resetting.
> > > > Do we really need to assert it before starting the HSIO PLL?
> > > Yes, this reset should be asserted, otherwise, the PCIe wouldn't work.
> > > I'm asking more details of this reset bit from design team, and
> > > would update
> > > later after I get the response.
> > >
> > > > AFAICS the PLL should be pretty much independent of the PHY.
> > > Agree.
> > >
> > > >
> > > > Do we need to enable this PLL when the PHY gets an external
> > > > refclock? I couldn't test it yet, but I suspect that the HSIO PLL
> > > > is only needed as an internal reference, when the i.MX8MP is the
> > > > refclock source, either through the PHY pads or via a clkout from the PLL.
> > > >
> > > Refer to my experience, the HSIO PLL should be enabled firstly.
> > >
> > > > > + /* Set P=12,M=800,S=4 and must set ICP=2'b01. */
> > > > > + regmap_update_bits(imx8_phy->hsio_blk_ctrl,
> > IMX8MP_GPR_REG2,
> > > > > + IMX8MP_GPR_P_PLL_MASK |
> > > > > + IMX8MP_GPR_M_PLL_MASK |
> > > > > + IMX8MP_GPR_S_PLL_MASK,
> > > > > + IMX8MP_GPR_P_PLL |
> > > > > + IMX8MP_GPR_M_PLL |
> > > > > + IMX8MP_GPR_S_PLL);
> > > > > + /* wait greater than 1/F_FREF =1/2MHZ=0.5us */
> > > > > + udelay(1);
> > > > > +
> > > > > + regmap_update_bits(imx8_phy->hsio_blk_ctrl,
> > IMX8MP_GPR_REG3,
> > > > > + IMX8MP_GPR_PLL_RST,
> > > > > + IMX8MP_GPR_PLL_RST);
> > > > > + udelay(10);
> > > > > +
> > > > > + /* Set 1 to pll_cke of GPR_REG3 */
> > > > > + regmap_update_bits(imx8_phy->hsio_blk_ctrl,
> > IMX8MP_GPR_REG3,
> > > > > + IMX8MP_GPR_PLL_CKE,
> > > > > + IMX8MP_GPR_PLL_CKE);
> > > > > +
> > > > > + /* Lock time should be greater than 300cycle=300*0.5us=150us
> > */
> > > > > + ret = regmap_read_poll_timeout(imx8_phy->hsio_blk_ctrl,
> > > > > + IMX8MP_GPR_REG1, val,
> > > > > + val & IMX8MP_GPR_PLL_LOCK,
> > > > > + 10, 1000);
> > > > > + if (ret) {
> > > > > + dev_err(imx8_phy->dev, "PCIe PLL lock timeout\n");
> > > > > + return ret;
> > > > > + }
> > > > > +
> > > > > + /* pcie_clock_module_en */
> > > > > + regmap_update_bits(imx8_phy->hsio_blk_ctrl,
> > IMX8MP_GPR_REG0,
> > > > > + IMX8MP_GPR_CLK_MOD_EN,
> > > > > + IMX8MP_GPR_CLK_MOD_EN);
> > > >
> > > > You shouldn't need to touch this bit. The HSIO blk-ctrl already
> > > > enables this bit when the PCIe power-domain is powered up.
> > > Okay, got that.
> > >
> > > >
> > > > > + udelay(10);
> > > > > +
> > > > > + reset_control_deassert(imx8_phy->reset);
> > > > > + reset_control_deassert(imx8_phy->perst);
> > > > > +
> > > > > + /* release pcie_phy_apb_reset and pcie_phy_init_resetn */
> > > > > + regmap_update_bits(imx8_phy->hsio_blk_ctrl,
> > IMX8MP_GPR_REG0,
> > > > > + IMX8MP_GPR_PHY_APB_RST |
> > > > > + IMX8MP_GPR_PHY_INIT_RST,
> > > > > + IMX8MP_GPR_PHY_APB_RST |
> > > > > + IMX8MP_GPR_PHY_INIT_RST);
> > > >
> > > > Not sure about those yet. We might want to toggle them via a
> > > > virtual PD in the HSIO blk-ctrl.
> > > Refer to my understand, these reset should be a part of power-up
> > > sequence of
> > > the PHY. It's reasonable to toggle them via a PD.
> >
> > So I had a chance to look into why this series isn't working for me some
> more.
> >
> > It seems the full PHY initialization fails, as the complete PHY MMIO
> > region reads back as 0xff. This hints at either a missing clock, or
> > (more likely) the register interface of the PHY being held in reset.
> > Note that I'm running upstream TF-A and the Barebox bootloader, so
> > this might be a missing initialization somewhere, that is done by
> > downstream TF-A or U-Boot.
> >
> > Sadly the above bits are also not documented in the RM, but are marked
> > as reserved. By chance, do you know about any other secondary
> > clocks/resets that may have an impact on PCIe?
> >
> Hi Lucas:
> Thanks for your help to look at this series.
> Refer to your descriptions, it seems that one initial version i.MX865 chip is
> used at your side.
> There is a design bug in the initial version of i.MX865 PCIe. All the PHY MMIO
> region reads back as 0xff.
> It is fixed by the later chips. Find another newer board is a quick method to
> fix it. Or, apply one SW workaround to let it work only in Gen1/Gen2 modes.
>
How does this going on? Do you find a solution that can make i.MX8MP PCIe
works?
Please let me know if you need any information or help from me, thanks.
Best Regards
Richard Zhu
> Best Regards
> Richard Zhu
>
> > Regards,
> > Lucas