2021-11-18 02:21:56

by Richard Zhu

Subject: [PATCH v6 0/8] Add the imx8m pcie phy driver and imx8mm pcie support

Refer to the discussion [1] when try to enable i.MX8MM PCIe support,
one standalone PCIe PHY driver should be seperated from i.MX PCIe
driver when enable i.MX8MM PCIe support.

This patch-set adds the standalone PCIe PHY driver suport[1-5], and i.MX8MM
PCIe support[6-8] to have whole view to review this patch-set.

The PCIe works on i.MX8MM EVK board based the the blkctrl power driver
[2] and this patch-set. And tested by Tim and Marcel on the different
reference clock modes boards.

[1] https://patchwork.ozlabs.org/project/linux-pci/patch/[email protected]/
[2] https://patchwork.kernel.org/project/linux-arm-kernel/cover/[email protected]/

Main changes v5 --> v6:
- Add "Reviewed-by: Rob Herring <[email protected]>" into #1 and #3 patches.
- Merge Rob's review comments to the #2 patch.

Main changes v4 --> v5:
- Set the AUX_EN always 1b'1, thus it can fix the regression introduced in v4
series on Marcel's board.
- Use the lower-case letter in the devicetreee refer to Marcel's comments.
_ Since the default value of the deemphasis parameters are zero, only set
the deemphasis registers when the input paramters are none zero.

Main changes v3 --> v4:
- Update the yaml to fix syntax error, add maxitems and drop description of phy
- Correct the clock name in PHY DT node.
- Squash the EVK board relalted dts changes into one patch, and drop the
useless dummy clock and gpio suffix in DT nodes.
- Add board specific de-emphasis parameters as DT properties. Thus each board
can specify its actual de-emphasis values.
- Update the commit log of PHY driver.
- Remove the useless codes from PCIe driver, since they are moved to PHY driver
- After the discussion and verification of the CLKREQ# configurations with Tim,
agree to add an optional boolean property "fsl,clkreq-unsupported", indicates
the CLKREQ# signal is hooked or not in HW designs.
- Add "Tested-by: Marcel Ziswiler <[email protected]>" tag, since
Marcel help to test the v3 patch-set.

Main changes v2 --> v3:
- Regarding Lucas' comments.
- to have a whole view to review the patches, send out the i.MX8MM PCIe support too.
- move the PHY related bits manipulations of the GPR/SRC to standalone PHY driver.
- split the dts changes to SOC and board DT, and use the enum instead of raw value.
- update the license of the dt-binding header file.

Changes v1 --> v2:
- Update the license of the dt-binding header file to make the license
compatible with dts files.
- Fix the dt_binding_check errors.

Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 6 +++
Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml | 92 +++++++++++++++++++++++++++++++
arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi | 55 +++++++++++++++++++
arch/arm64/boot/dts/freescale/imx8mm.dtsi | 46 +++++++++++++++-
drivers/pci/controller/dwc/pci-imx6.c | 73 ++++++++++++++++++++++---
drivers/phy/freescale/Kconfig | 9 ++++
drivers/phy/freescale/Makefile | 1 +
drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 237 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
include/dt-bindings/phy/phy-imx8-pcie.h | 14 +++++
9 files changed, 525 insertions(+), 8 deletions(-)

[PATCH v6 1/8] dt-bindings: phy: phy-imx8-pcie: Add binding for the
[PATCH v6 2/8] dt-bindings: phy: Add imx8 pcie phy driver support
[PATCH v6 3/8] dt-bindings: imx6q-pcie: Add PHY phandles and name
[PATCH v6 4/8] arm64: dts: imx8mm: Add the pcie phy support
[PATCH v6 5/8] phy: freescale: pcie: Initialize the imx8 pcie
[PATCH v6 6/8] arm64: dts: imx8mm: Add the pcie support
[PATCH v6 7/8] arm64: dts: imx8mm-evk: Add the pcie support on imx8mm
[PATCH v6 8/8] PCI: imx: Add the imx8mm pcie support

2021-11-18 02:21:56

by Richard Zhu

Subject: [PATCH v6 1/8] dt-bindings: phy: phy-imx8-pcie: Add binding for the pad modes of imx8 pcie phy

Add binding for reference clock PAD modes of the i.MX8 PCIe PHY.

Signed-off-by: Richard Zhu <[email protected]>
Tested-by: Marcel Ziswiler <[email protected]>
Reviewed-by: Tim Harvey <[email protected]>
Tested-by: Tim Harvey <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
include/dt-bindings/phy/phy-imx8-pcie.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
create mode 100644 include/dt-bindings/phy/phy-imx8-pcie.h

diff --git a/include/dt-bindings/phy/phy-imx8-pcie.h b/include/dt-bindings/phy/phy-imx8-pcie.h
new file mode 100644
index 000000000000..8bbe2d6538d8
--- /dev/null
+++ b/include/dt-bindings/phy/phy-imx8-pcie.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+/*
+ * This header provides constants for i.MX8 PCIe.
+ */
+
+#ifndef _DT_BINDINGS_IMX8_PCIE_H
+#define _DT_BINDINGS_IMX8_PCIE_H
+
+/* Reference clock PAD mode */
+#define IMX8_PCIE_REFCLK_PAD_UNUSED 0
+#define IMX8_PCIE_REFCLK_PAD_INPUT 1
+#define IMX8_PCIE_REFCLK_PAD_OUTPUT 2
+
+#endif /* _DT_BINDINGS_IMX8_PCIE_H */
--
2.25.1

2021-11-18 02:21:56

by Richard Zhu

Subject: [PATCH v6 2/8] dt-bindings: phy: Add imx8 pcie phy driver support

Add dt-binding for the standalone i.MX8 PCIe PHY driver.

Signed-off-by: Richard Zhu <[email protected]>
Tested-by: Marcel Ziswiler <[email protected]>
Reviewed-by: Tim Harvey <[email protected]>
Tested-by: Tim Harvey <[email protected]>
---
.../bindings/phy/fsl,imx8-pcie-phy.yaml | 92 +++++++++++++++++++
1 file changed, 92 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml
new file mode 100644
index 000000000000..b6421eedece3
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml
@@ -0,0 +1,92 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/fsl,imx8-pcie-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale i.MX8 SoC series PCIe PHY Device Tree Bindings
+
+maintainers:
+ - Richard Zhu <[email protected]>
+
+properties:
+ "#phy-cells":
+ const: 0
+
+ compatible:
+ enum:
+ - fsl,imx8mm-pcie-phy
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ items:
+ - const: ref
+
+ resets:
+ maxItems: 1
+
+ reset-names:
+ items:
+ - const: pciephy
+
+ fsl,refclk-pad-mode:
+ description: |
+ Specifies the mode of the refclk pad used. It can be UNUSED(PHY
+ refclock is derived from SoC internal source), INPUT(PHY refclock
+ is provided externally via the refclk pad) or OUTPUT(PHY refclock
+ is derived from SoC internal source and provided on the refclk pad).
+ Refer include/dt-bindings/phy/phy-imx8-pcie.h for the constants
+ to be used.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [ 0, 1, 2 ]
+
+ fsl,tx-deemph-gen1:
+ description: Gen1 De-emphasis value (optional).
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 0
+
+ fsl,tx-deemph-gen2:
+ description: Gen2 De-emphasis value (optional).
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 0
+
+ fsl,clkreq-unsupported:
+ type: boolean
+ description: A boolean property indicating the CLKREQ# signal is
+ not supported in the board design (optional)
+
+required:
+ - "#phy-cells"
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - fsl,refclk-pad-mode
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/imx8mm-clock.h>
+ #include <dt-bindings/phy/phy-imx8-pcie.h>
+ #include <dt-bindings/reset/imx8mq-reset.h>
+
+ pcie_phy: pcie-phy@32f00000 {
+ compatible = "fsl,imx8mm-pcie-phy";
+ reg = <0x32f00000 0x10000>;
+ clocks = <&clk IMX8MM_CLK_PCIE1_PHY>;
+ clock-names = "ref";
+ assigned-clocks = <&clk IMX8MM_CLK_PCIE1_PHY>;
+ assigned-clock-rates = <100000000>;
+ assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_100M>;
+ resets = <&src IMX8MQ_RESET_PCIEPHY>;
+ reset-names = "pciephy";
+ fsl,refclk-pad-mode = <IMX8_PCIE_REFCLK_PAD_INPUT>;
+ #phy-cells = <0>;
+ };
+...
--
2.25.1

2021-11-18 02:22:02

by Richard Zhu

Subject: [PATCH v6 4/8] arm64: dts: imx8mm: Add the pcie phy support

Add the PCIe PHY support on iMX8MM platforms.

Signed-off-by: Richard Zhu <[email protected]>
Tested-by: Marcel Ziswiler <[email protected]>
Reviewed-by: Tim Harvey <[email protected]>
Tested-by: Tim Harvey <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8mm.dtsi | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index c2f3f118f82e..0844f3144887 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -1135,6 +1135,19 @@ usbmisc2: usbmisc@32e50200 {
reg = <0x32e50200 0x200>;
};

+ pcie_phy: pcie-phy@32f00000 {
+ compatible = "fsl,imx8mm-pcie-phy";
+ reg = <0x32f00000 0x10000>;
+ clocks = <&clk IMX8MM_CLK_PCIE1_PHY>;
+ clock-names = "ref";
+ assigned-clocks = <&clk IMX8MM_CLK_PCIE1_PHY>;
+ assigned-clock-rates = <100000000>;
+ assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_100M>;
+ resets = <&src IMX8MQ_RESET_PCIEPHY>;
+ reset-names = "pciephy";
+ #phy-cells = <0>;
+ status = "disabled";
+ };
};

dma_apbh: dma-controller@33000000 {
--
2.25.1

2021-11-18 02:22:09

by Richard Zhu

Subject: [PATCH v6 3/8] dt-bindings: imx6q-pcie: Add PHY phandles and name properties

i.MX8MM PCIe has the PHY. Add a PHY phandle and name properties
in the binding document.

Signed-off-by: Richard Zhu <[email protected]>
Tested-by: Marcel Ziswiler <[email protected]>
Reviewed-by: Tim Harvey <[email protected]>
Tested-by: Tim Harvey <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
index acea1cd444fd..643a6333b07b 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
@@ -127,6 +127,12 @@ properties:
enum: [1, 2, 3, 4]
default: 1

+ phys:
+ maxItems: 1
+
+ phy-names:
+ const: pcie-phy
+
reset-gpio:
description: Should specify the GPIO for controlling the PCI bus device
reset signal. It's not polarity aware and defaults to active-low reset
--
2.25.1

2021-11-18 02:22:12

by Richard Zhu

Subject: [PATCH v6 5/8] phy: freescale: pcie: Initialize the imx8 pcie standalone phy driver

Add the standalone i.MX8 PCIe PHY driver.

Signed-off-by: Richard Zhu <[email protected]>
Tested-by: Marcel Ziswiler <[email protected]>
Reviewed-by: Tim Harvey <[email protected]>
Tested-by: Tim Harvey <[email protected]>
---
drivers/phy/freescale/Kconfig | 9 +
drivers/phy/freescale/Makefile | 1 +
drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 237 +++++++++++++++++++++
3 files changed, 247 insertions(+)
create mode 100644 drivers/phy/freescale/phy-fsl-imx8m-pcie.c

diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig
index 320630ffe3cd..e821498b1f7f 100644
--- a/drivers/phy/freescale/Kconfig
+++ b/drivers/phy/freescale/Kconfig
@@ -14,3 +14,12 @@ config PHY_MIXEL_MIPI_DPHY
help
Enable this to add support for the Mixel DSI PHY as found
on NXP's i.MX8 family of SOCs.
+
+config PHY_FSL_IMX8M_PCIE
+ tristate "Freescale i.MX8M PCIE PHY"
+ depends on OF && HAS_IOMEM
+ select GENERIC_PHY
+ default ARCH_MXC && ARM64
+ help
+ Enable this to add support for the PCIE PHY as found on
+ i.MX8M family of SOCs.
diff --git a/drivers/phy/freescale/Makefile b/drivers/phy/freescale/Makefile
index 1d02e3869b45..55d07c742ab0 100644
--- a/drivers/phy/freescale/Makefile
+++ b/drivers/phy/freescale/Makefile
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-$(CONFIG_PHY_FSL_IMX8MQ_USB) += phy-fsl-imx8mq-usb.o
obj-$(CONFIG_PHY_MIXEL_MIPI_DPHY) += phy-fsl-imx8-mipi-dphy.o
+obj-$(CONFIG_PHY_FSL_IMX8M_PCIE) += phy-fsl-imx8m-pcie.o
diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
new file mode 100644
index 000000000000..357483a7efeb
--- /dev/null
+++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
@@ -0,0 +1,237 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2021 NXP
+ */
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/delay.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
+#include <linux/module.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <dt-bindings/phy/phy-imx8-pcie.h>
+
+#define IMX8MM_PCIE_PHY_CMN_REG061 0x184
+#define ANA_PLL_CLK_OUT_TO_EXT_IO_EN BIT(0)
+#define IMX8MM_PCIE_PHY_CMN_REG062 0x188
+#define ANA_PLL_CLK_OUT_TO_EXT_IO_SEL BIT(3)
+#define IMX8MM_PCIE_PHY_CMN_REG063 0x18C
+#define AUX_PLL_REFCLK_SEL_SYS_PLL GENMASK(7, 6)
+#define IMX8MM_PCIE_PHY_CMN_REG064 0x190
+#define ANA_AUX_RX_TX_SEL_TX BIT(7)
+#define ANA_AUX_RX_TERM_GND_EN BIT(3)
+#define ANA_AUX_TX_TERM BIT(2)
+#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 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)
+#define IMX8MM_GPR_PCIE_REF_CLK_EXT FIELD_PREP(IMX8MM_GPR_PCIE_REF_CLK_SEL, 0x2)
+#define IMX8MM_GPR_PCIE_AUX_EN BIT(19)
+#define IMX8MM_GPR_PCIE_CMN_RST BIT(18)
+#define IMX8MM_GPR_PCIE_POWER_OFF BIT(17)
+#define IMX8MM_GPR_PCIE_SSC_EN BIT(16)
+#define IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE BIT(9)
+
+struct imx8_pcie_phy {
+ void __iomem *base;
+ struct clk *clk;
+ struct phy *phy;
+ struct regmap *iomuxc_gpr;
+ struct reset_control *reset;
+ u32 refclk_pad_mode;
+ u32 tx_deemph_gen1;
+ u32 tx_deemph_gen2;
+ bool clkreq_unused;
+};
+
+static int imx8_pcie_phy_init(struct phy *phy)
+{
+ int ret;
+ u32 val, pad_mode;
+ struct imx8_pcie_phy *imx8_phy = phy_get_drvdata(phy);
+
+ reset_control_assert(imx8_phy->reset);
+
+ pad_mode = imx8_phy->refclk_pad_mode;
+ /* 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,
+ imx8_phy->clkreq_unused ?
+ 0 : IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE);
+ regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
+ IMX8MM_GPR_PCIE_AUX_EN,
+ IMX8MM_GPR_PCIE_AUX_EN);
+ regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
+ IMX8MM_GPR_PCIE_POWER_OFF, 0);
+ regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
+ IMX8MM_GPR_PCIE_SSC_EN, 0);
+
+ regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
+ IMX8MM_GPR_PCIE_REF_CLK_SEL,
+ pad_mode == IMX8_PCIE_REFCLK_PAD_INPUT ?
+ IMX8MM_GPR_PCIE_REF_CLK_EXT :
+ IMX8MM_GPR_PCIE_REF_CLK_PLL);
+ usleep_range(100, 200);
+
+ /* Do the PHY common block reset */
+ 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);
+ }
+
+ /* 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);
+ return ret;
+}
+
+static int imx8_pcie_phy_power_on(struct phy *phy)
+{
+ struct imx8_pcie_phy *imx8_phy = phy_get_drvdata(phy);
+
+ return clk_prepare_enable(imx8_phy->clk);
+}
+
+static int imx8_pcie_phy_power_off(struct phy *phy)
+{
+ struct imx8_pcie_phy *imx8_phy = phy_get_drvdata(phy);
+
+ clk_disable_unprepare(imx8_phy->clk);
+
+ return 0;
+}
+
+static const struct phy_ops imx8_pcie_phy_ops = {
+ .init = imx8_pcie_phy_init,
+ .power_on = imx8_pcie_phy_power_on,
+ .power_off = imx8_pcie_phy_power_off,
+ .owner = THIS_MODULE,
+};
+
+static int imx8_pcie_phy_probe(struct platform_device *pdev)
+{
+ struct phy_provider *phy_provider;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ struct imx8_pcie_phy *imx8_phy;
+ struct resource *res;
+
+ imx8_phy = devm_kzalloc(dev, sizeof(*imx8_phy), GFP_KERNEL);
+ if (!imx8_phy)
+ return -ENOMEM;
+
+ /* get PHY refclk pad mode */
+ of_property_read_u32(np, "fsl,refclk-pad-mode",
+ &imx8_phy->refclk_pad_mode);
+
+ if (of_property_read_u32(np, "fsl,tx-deemph-gen1",
+ &imx8_phy->tx_deemph_gen1))
+ imx8_phy->tx_deemph_gen1 = 0;
+
+ if (of_property_read_u32(np, "fsl,tx-deemph-gen2",
+ &imx8_phy->tx_deemph_gen2))
+ imx8_phy->tx_deemph_gen2 = 0;
+
+ if (of_property_read_bool(np, "fsl,clkreq-unsupported"))
+ imx8_phy->clkreq_unused = true;
+ else
+ imx8_phy->clkreq_unused = false;
+
+ imx8_phy->clk = devm_clk_get(dev, "ref");
+ if (IS_ERR(imx8_phy->clk)) {
+ dev_err(dev, "failed to get imx pcie phy clock\n");
+ return PTR_ERR(imx8_phy->clk);
+ }
+
+ /* Grab GPR config register range */
+ imx8_phy->iomuxc_gpr =
+ syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+ if (IS_ERR(imx8_phy->iomuxc_gpr)) {
+ dev_err(dev, "unable to find iomuxc registers\n");
+ return PTR_ERR(imx8_phy->iomuxc_gpr);
+ }
+
+ imx8_phy->reset = devm_reset_control_get_exclusive(dev, "pciephy");
+ if (IS_ERR(imx8_phy->reset)) {
+ dev_err(dev, "Failed to get PCIEPHY reset control\n");
+ return PTR_ERR(imx8_phy->reset);
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ imx8_phy->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(imx8_phy->base))
+ return PTR_ERR(imx8_phy->base);
+
+ imx8_phy->phy = devm_phy_create(dev, NULL, &imx8_pcie_phy_ops);
+ if (IS_ERR(imx8_phy->phy))
+ return PTR_ERR(imx8_phy->phy);
+
+ phy_set_drvdata(imx8_phy->phy, imx8_phy);
+
+ phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+
+ 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 = {
+ .name = "imx8-pcie-phy",
+ .of_match_table = imx8_pcie_phy_of_match,
+ }
+};
+module_platform_driver(imx8_pcie_phy_driver);
+
+MODULE_DESCRIPTION("FSL IMX8 PCIE PHY driver");
+MODULE_LICENSE("GPL");
--
2.25.1

2021-11-18 02:22:14

by Richard Zhu

Subject: [PATCH v6 6/8] arm64: dts: imx8mm: Add the pcie support

Add the PCIe support on i.MX8MM platforms.

Signed-off-by: Richard Zhu <[email protected]>
Tested-by: Marcel Ziswiler <[email protected]>
Reviewed-by: Tim Harvey <[email protected]>
Tested-by: Tim Harvey <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8mm.dtsi | 33 ++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 0844f3144887..75f4317215ac 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -520,7 +520,7 @@ iomuxc: pinctrl@30330000 {
};

gpr: iomuxc-gpr@30340000 {
- compatible = "fsl,imx8mm-iomuxc-gpr", "syscon";
+ compatible = "fsl,imx8mm-iomuxc-gpr", "fsl,imx6q-iomuxc-gpr", "syscon";
reg = <0x30340000 0x10000>;
};

@@ -1179,6 +1179,37 @@ gpmi: nand-controller@33002000{
status = "disabled";
};

+ pcie0: pcie@33800000 {
+ compatible = "fsl,imx8mm-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 122 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 125 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 2 &gic GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 3 &gic GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 4 &gic GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>;
+ fsl,max-link-speed = <2>;
+ linux,pci-domain = <0>;
+ power-domains = <&pgc_pcie>;
+ resets = <&src IMX8MQ_RESET_PCIE_CTRL_APPS_EN>,
+ <&src IMX8MQ_RESET_PCIE_CTRL_APPS_TURNOFF>;
+ reset-names = "apps", "turnoff";
+ phys = <&pcie_phy>;
+ phy-names = "pcie-phy";
+ status = "disabled";
+ };
+
gpu_3d: gpu@38000000 {
compatible = "vivante,gc";
reg = <0x38000000 0x8000>;
--
2.25.1

2021-11-18 02:22:16

by Richard Zhu

Subject: [PATCH v6 7/8] arm64: dts: imx8mm-evk: Add the pcie support on imx8mm evk board

Add the PCIe support on iMX8MM EVK boards.
And set the default reference clock mode.

Signed-off-by: Richard Zhu <[email protected]>
Tested-by: Marcel Ziswiler <[email protected]>
Reviewed-by: Tim Harvey <[email protected]>
Tested-by: Tim Harvey <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi | 55 +++++++++++++++++++
1 file changed, 55 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
index e033d0257b5a..87a30daf0b3c 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
@@ -5,6 +5,7 @@

/dts-v1/;

+#include <dt-bindings/phy/phy-imx8-pcie.h>
#include <dt-bindings/usb/pd.h>
#include "imx8mm.dtsi"

@@ -30,6 +31,23 @@ status {
};
};

+ pcie0_refclk: pcie0-refclk {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <100000000>;
+ };
+
+ 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 = <&gpio1 5 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
+
reg_usdhc2_vmmc: regulator-usdhc2 {
compatible = "regulator-fixed";
pinctrl-names = "default";
@@ -289,6 +307,30 @@ pca6416: gpio@20 {
};
};

+&pcie_phy {
+ fsl,refclk-pad-mode = <IMX8_PCIE_REFCLK_PAD_INPUT>;
+ fsl,tx-deemph-gen1 = <0x2d>;
+ fsl,tx-deemph-gen2 = <0xf>;
+ clocks = <&pcie0_refclk>;
+ status = "okay";
+};
+
+&pcie0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pcie0>;
+ reset-gpio = <&gpio4 21 GPIO_ACTIVE_LOW>;
+ clocks = <&clk IMX8MM_CLK_PCIE1_ROOT>, <&clk IMX8MM_CLK_PCIE1_AUX>,
+ <&pcie0_refclk>;
+ clock-names = "pcie", "pcie_aux", "pcie_bus";
+ assigned-clocks = <&clk IMX8MM_CLK_PCIE1_AUX>,
+ <&clk IMX8MM_CLK_PCIE1_CTRL>;
+ assigned-clock-rates = <10000000>, <250000000>;
+ assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_50M>,
+ <&clk IMX8MM_SYS_PLL2_250M>;
+ vpcie-supply = <&reg_pcie0>;
+ status = "okay";
+};
+
&sai3 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_sai3>;
@@ -406,6 +448,19 @@ MX8MM_IOMUXC_I2C3_SDA_I2C3_SDA 0x400001c3
>;
};

+ pinctrl_pcie0: pcie0grp {
+ fsl,pins = <
+ MX8MM_IOMUXC_I2C4_SCL_PCIE1_CLKREQ_B 0x61
+ MX8MM_IOMUXC_SAI2_RXFS_GPIO4_IO21 0x41
+ >;
+ };
+
+ pinctrl_pcie0_reg: pcie0reggrp {
+ fsl,pins = <
+ MX8MM_IOMUXC_GPIO1_IO05_GPIO1_IO5 0x41
+ >;
+ };
+
pinctrl_pmic: pmicirqgrp {
fsl,pins = <
MX8MM_IOMUXC_GPIO1_IO03_GPIO1_IO3 0x141
--
2.25.1

2021-11-18 02:22:26

by Richard Zhu

Subject: [PATCH v6 8/8] PCI: imx: Add the imx8mm pcie support

i.MX8MM PCIe works mostly like the i.MX8MQ one, but has a different PHY
and allows to output the internal PHY reference clock via the refclk pad.
Add the i.MX8MM PCIe support based on the standalone PHY driver.

Signed-off-by: Richard Zhu <[email protected]>
Tested-by: Marcel Ziswiler <[email protected]>
Reviewed-by: Tim Harvey <[email protected]>
Tested-by: Tim Harvey <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 73 ++++++++++++++++++++++++---
1 file changed, 66 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 26f49f797b0f..d8c587b4d54f 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -29,6 +29,7 @@
#include <linux/types.h>
#include <linux/interrupt.h>
#include <linux/reset.h>
+#include <linux/phy/phy.h>
#include <linux/pm_domain.h>
#include <linux/pm_runtime.h>

@@ -49,6 +50,7 @@ enum imx6_pcie_variants {
IMX6QP,
IMX7D,
IMX8MQ,
+ IMX8MM,
};

#define IMX6_PCIE_FLAG_IMX6_PHY BIT(0)
@@ -88,6 +90,7 @@ struct imx6_pcie {
struct device *pd_pcie;
/* power domain for pcie phy */
struct device *pd_pcie_phy;
+ struct phy *phy;
const struct imx6_pcie_drvdata *drvdata;
};

@@ -372,6 +375,8 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
case IMX7D:
case IMX8MQ:
reset_control_assert(imx6_pcie->pciephy_reset);
+ fallthrough;
+ case IMX8MM:
reset_control_assert(imx6_pcie->apps_reset);
break;
case IMX6SX:
@@ -407,7 +412,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);
+ WARN_ON(imx6_pcie->drvdata->variant != IMX8MQ &&
+ imx6_pcie->drvdata->variant != IMX8MM);
return imx6_pcie->controller_id == 1 ? IOMUXC_GPR16 : IOMUXC_GPR14;
}

@@ -446,6 +452,13 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
break;
case IMX7D:
break;
+ case IMX8MM:
+ ret = clk_prepare_enable(imx6_pcie->pcie_aux);
+ if (ret) {
+ dev_err(dev, "unable to enable pcie_aux clock\n");
+ break;
+ }
+ break;
case IMX8MQ:
ret = clk_prepare_enable(imx6_pcie->pcie_aux);
if (ret) {
@@ -522,6 +535,14 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
goto err_ref_clk;
}

+ switch (imx6_pcie->drvdata->variant) {
+ case IMX8MM:
+ if (phy_power_on(imx6_pcie->phy))
+ dev_err(dev, "unable to power on PHY\n");
+ break;
+ default:
+ break;
+ }
/* allow the clocks to stabilize */
usleep_range(200, 500);

@@ -538,6 +559,10 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
case IMX8MQ:
reset_control_deassert(imx6_pcie->pciephy_reset);
break;
+ case IMX8MM:
+ if (phy_init(imx6_pcie->phy) != 0)
+ dev_err(dev, "Waiting for PHY ready timeout!\n");
+ break;
case IMX7D:
reset_control_deassert(imx6_pcie->pciephy_reset);

@@ -614,6 +639,8 @@ static void imx6_pcie_configure_type(struct imx6_pcie *imx6_pcie)
static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
{
switch (imx6_pcie->drvdata->variant) {
+ case IMX8MM:
+ break;
case IMX8MQ:
/*
* TODO: Currently this code assumes external
@@ -753,6 +780,7 @@ static void imx6_pcie_ltssm_enable(struct device *dev)
break;
case IMX7D:
case IMX8MQ:
+ case IMX8MM:
reset_control_deassert(imx6_pcie->apps_reset);
break;
}
@@ -871,6 +899,7 @@ static void imx6_pcie_ltssm_disable(struct device *dev)
IMX6Q_GPR12_PCIE_CTL_2, 0);
break;
case IMX7D:
+ case IMX8MM:
reset_control_assert(imx6_pcie->apps_reset);
break;
default:
@@ -930,6 +959,7 @@ static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
break;
case IMX8MQ:
+ case IMX8MM:
clk_disable_unprepare(imx6_pcie->pcie_aux);
break;
default:
@@ -1043,11 +1073,6 @@ static int imx6_pcie_probe(struct platform_device *pdev)
}

/* Fetch clocks */
- imx6_pcie->pcie_phy = devm_clk_get(dev, "pcie_phy");
- if (IS_ERR(imx6_pcie->pcie_phy))
- return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_phy),
- "pcie_phy clock source missing or invalid\n");
-
imx6_pcie->pcie_bus = devm_clk_get(dev, "pcie_bus");
if (IS_ERR(imx6_pcie->pcie_bus))
return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_bus),
@@ -1089,10 +1114,39 @@ static int imx6_pcie_probe(struct platform_device *pdev)
dev_err(dev, "Failed to get PCIE APPS reset control\n");
return PTR_ERR(imx6_pcie->apps_reset);
}
+ break;
+ case IMX8MM:
+ imx6_pcie->pcie_aux = devm_clk_get(dev, "pcie_aux");
+ if (IS_ERR(imx6_pcie->pcie_aux))
+ return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_aux),
+ "pcie_aux clock source missing or invalid\n");
+ imx6_pcie->apps_reset = devm_reset_control_get_exclusive(dev,
+ "apps");
+ if (IS_ERR(imx6_pcie->apps_reset)) {
+ dev_err(dev, "Failed to get PCIE APPS reset control\n");
+ return PTR_ERR(imx6_pcie->apps_reset);
+ }
+
+ imx6_pcie->phy = devm_phy_get(dev, "pcie-phy");
+ if (IS_ERR(imx6_pcie->phy)) {
+ if (PTR_ERR(imx6_pcie->phy) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ dev_err(dev, "Failed to get PCIE PHY\n");
+ return PTR_ERR(imx6_pcie->phy);
+ }
+
break;
default:
break;
}
+ /* Don't fetch the pcie_phy clock, if it has abstract PHY driver */
+ if (imx6_pcie->phy == NULL) {
+ imx6_pcie->pcie_phy = devm_clk_get(dev, "pcie_phy");
+ if (IS_ERR(imx6_pcie->pcie_phy))
+ return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_phy),
+ "pcie_phy clock source missing or invalid\n");
+ }
+

/* Grab turnoff reset */
imx6_pcie->turnoff_reset = devm_reset_control_get_optional_exclusive(dev, "turnoff");
@@ -1202,6 +1256,10 @@ static const struct imx6_pcie_drvdata drvdata[] = {
[IMX8MQ] = {
.variant = IMX8MQ,
},
+ [IMX8MM] = {
+ .variant = IMX8MM,
+ .flags = IMX6_PCIE_FLAG_SUPPORTS_SUSPEND,
+ },
};

static const struct of_device_id imx6_pcie_of_match[] = {
@@ -1209,7 +1267,8 @@ static const struct of_device_id imx6_pcie_of_match[] = {
{ .compatible = "fsl,imx6sx-pcie", .data = &drvdata[IMX6SX], },
{ .compatible = "fsl,imx6qp-pcie", .data = &drvdata[IMX6QP], },
{ .compatible = "fsl,imx7d-pcie", .data = &drvdata[IMX7D], },
- { .compatible = "fsl,imx8mq-pcie", .data = &drvdata[IMX8MQ], } ,
+ { .compatible = "fsl,imx8mq-pcie", .data = &drvdata[IMX8MQ], },
+ { .compatible = "fsl,imx8mm-pcie", .data = &drvdata[IMX8MM], },
{},
};

--
2.25.1

2021-11-18 23:50:39

by Rob Herring (Arm)

Subject: Re: [PATCH v6 2/8] dt-bindings: phy: Add imx8 pcie phy driver support

On Thu, 18 Nov 2021 09:54:43 +0800, Richard Zhu wrote:
> Add dt-binding for the standalone i.MX8 PCIe PHY driver.
>
> Signed-off-by: Richard Zhu <[email protected]>
> Tested-by: Marcel Ziswiler <[email protected]>
> Reviewed-by: Tim Harvey <[email protected]>
> Tested-by: Tim Harvey <[email protected]>
> ---
> .../bindings/phy/fsl,imx8-pcie-phy.yaml | 92 +++++++++++++++++++
> 1 file changed, 92 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml
>

Reviewed-by: Rob Herring <[email protected]>

2021-11-19 01:18:00

by Richard Zhu

Subject: RE: [PATCH v6 2/8] dt-bindings: phy: Add imx8 pcie phy driver support

> -----Original Message-----
> From: Rob Herring <[email protected]>
> Sent: Friday, November 19, 2021 7:51 AM
> To: Hongxing Zhu <[email protected]>
> Cc: [email protected]; [email protected]; Marcel Ziswiler
> <[email protected]>; [email protected];
> [email protected]; [email protected];
> dl-linux-imx <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH v6 2/8] dt-bindings: phy: Add imx8 pcie phy driver
> support
>
> On Thu, 18 Nov 2021 09:54:43 +0800, Richard Zhu wrote:
> > Add dt-binding for the standalone i.MX8 PCIe PHY driver.
> >
> > Signed-off-by: Richard Zhu <[email protected]>
> > Tested-by: Marcel Ziswiler <[email protected]>
> > Reviewed-by: Tim Harvey <[email protected]>
> > Tested-by: Tim Harvey <[email protected]>
> > ---
> > .../bindings/phy/fsl,imx8-pcie-phy.yaml | 92
> +++++++++++++++++++
> > 1 file changed, 92 insertions(+)
> > create mode 100644
> Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml
> >
>
> Reviewed-by: Rob Herring <[email protected]>
[Richard Zhu] Thanks a lot.

Best Regards
Richard Zhu

2021-12-01 12:44:35

by Lorenzo Pieralisi

Subject: Re: [PATCH v6 0/8] Add the imx8m pcie phy driver and imx8mm pcie support

On Thu, Nov 18, 2021 at 09:54:41AM +0800, Richard Zhu wrote:
> Refer to the discussion [1] when try to enable i.MX8MM PCIe support,
> one standalone PCIe PHY driver should be seperated from i.MX PCIe
> driver when enable i.MX8MM PCIe support.
>
> This patch-set adds the standalone PCIe PHY driver suport[1-5], and i.MX8MM
> PCIe support[6-8] to have whole view to review this patch-set.
>
> The PCIe works on i.MX8MM EVK board based the the blkctrl power driver
> [2] and this patch-set. And tested by Tim and Marcel on the different
> reference clock modes boards.
>
> [1] https://patchwork.ozlabs.org/project/linux-pci/patch/[email protected]/
> [2] https://patchwork.kernel.org/project/linux-arm-kernel/cover/[email protected]/
>
> Main changes v5 --> v6:
> - Add "Reviewed-by: Rob Herring <[email protected]>" into #1 and #3 patches.
> - Merge Rob's review comments to the #2 patch.
>
> Main changes v4 --> v5:
> - Set the AUX_EN always 1b'1, thus it can fix the regression introduced in v4
> series on Marcel's board.
> - Use the lower-case letter in the devicetreee refer to Marcel's comments.
> _ Since the default value of the deemphasis parameters are zero, only set
> the deemphasis registers when the input paramters are none zero.
>
> Main changes v3 --> v4:
> - Update the yaml to fix syntax error, add maxitems and drop description of phy
> - Correct the clock name in PHY DT node.
> - Squash the EVK board relalted dts changes into one patch, and drop the
> useless dummy clock and gpio suffix in DT nodes.
> - Add board specific de-emphasis parameters as DT properties. Thus each board
> can specify its actual de-emphasis values.
> - Update the commit log of PHY driver.
> - Remove the useless codes from PCIe driver, since they are moved to PHY driver
> - After the discussion and verification of the CLKREQ# configurations with Tim,
> agree to add an optional boolean property "fsl,clkreq-unsupported", indicates
> the CLKREQ# signal is hooked or not in HW designs.
> - Add "Tested-by: Marcel Ziswiler <[email protected]>" tag, since
> Marcel help to test the v3 patch-set.
>
> Main changes v2 --> v3:
> - Regarding Lucas' comments.
> - to have a whole view to review the patches, send out the i.MX8MM PCIe support too.
> - move the PHY related bits manipulations of the GPR/SRC to standalone PHY driver.
> - split the dts changes to SOC and board DT, and use the enum instead of raw value.
> - update the license of the dt-binding header file.
>
> Changes v1 --> v2:
> - Update the license of the dt-binding header file to make the license
> compatible with dts files.
> - Fix the dt_binding_check errors.
>
> Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 6 +++
> Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml | 92 +++++++++++++++++++++++++++++++
> arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi | 55 +++++++++++++++++++
> arch/arm64/boot/dts/freescale/imx8mm.dtsi | 46 +++++++++++++++-
> drivers/pci/controller/dwc/pci-imx6.c | 73 ++++++++++++++++++++++---
> drivers/phy/freescale/Kconfig | 9 ++++
> drivers/phy/freescale/Makefile | 1 +
> drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 237 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> include/dt-bindings/phy/phy-imx8-pcie.h | 14 +++++
> 9 files changed, 525 insertions(+), 8 deletions(-)

Hi Richard,

I can pull this series into the PCI tree (but not the dts changes
that should be routed elsewhere) or give an ACK for patch 8, please
let me know what's the best option.

Thanks,
Lorenzo

> [PATCH v6 1/8] dt-bindings: phy: phy-imx8-pcie: Add binding for the
> [PATCH v6 2/8] dt-bindings: phy: Add imx8 pcie phy driver support
> [PATCH v6 3/8] dt-bindings: imx6q-pcie: Add PHY phandles and name
> [PATCH v6 4/8] arm64: dts: imx8mm: Add the pcie phy support
> [PATCH v6 5/8] phy: freescale: pcie: Initialize the imx8 pcie
> [PATCH v6 6/8] arm64: dts: imx8mm: Add the pcie support
> [PATCH v6 7/8] arm64: dts: imx8mm-evk: Add the pcie support on imx8mm
> [PATCH v6 8/8] PCI: imx: Add the imx8mm pcie support

2021-12-02 04:30:29

by Vinod Koul

Subject: Re: [PATCH v6 5/8] phy: freescale: pcie: Initialize the imx8 pcie standalone phy driver

On 18-11-21, 09:54, Richard Zhu wrote:
> Add the standalone i.MX8 PCIe PHY driver.
>
> Signed-off-by: Richard Zhu <[email protected]>
> Tested-by: Marcel Ziswiler <[email protected]>
> Reviewed-by: Tim Harvey <[email protected]>
> Tested-by: Tim Harvey <[email protected]>
> ---
> drivers/phy/freescale/Kconfig | 9 +
> drivers/phy/freescale/Makefile | 1 +
> drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 237 +++++++++++++++++++++
> 3 files changed, 247 insertions(+)
> create mode 100644 drivers/phy/freescale/phy-fsl-imx8m-pcie.c
>
> diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig
> index 320630ffe3cd..e821498b1f7f 100644
> --- a/drivers/phy/freescale/Kconfig
> +++ b/drivers/phy/freescale/Kconfig
> @@ -14,3 +14,12 @@ config PHY_MIXEL_MIPI_DPHY
> help
> Enable this to add support for the Mixel DSI PHY as found
> on NXP's i.MX8 family of SOCs.
> +
> +config PHY_FSL_IMX8M_PCIE
> + tristate "Freescale i.MX8M PCIE PHY"
> + depends on OF && HAS_IOMEM
> + select GENERIC_PHY
> + default ARCH_MXC && ARM64

Why should this be default ? We dont do that for new drivers.. You may
add this to respective config file though...

> +static int imx8_pcie_phy_init(struct phy *phy)
> +{
> + int ret;
> + u32 val, pad_mode;
> + struct imx8_pcie_phy *imx8_phy = phy_get_drvdata(phy);
> +
> + reset_control_assert(imx8_phy->reset);
> +
> + pad_mode = imx8_phy->refclk_pad_mode;
> + /* 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,
> + imx8_phy->clkreq_unused ?
> + 0 : IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE);
> + regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> + IMX8MM_GPR_PCIE_AUX_EN,
> + IMX8MM_GPR_PCIE_AUX_EN);
> + regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> + IMX8MM_GPR_PCIE_POWER_OFF, 0);
> + regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> + IMX8MM_GPR_PCIE_SSC_EN, 0);
> +
> + regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> + IMX8MM_GPR_PCIE_REF_CLK_SEL,
> + pad_mode == IMX8_PCIE_REFCLK_PAD_INPUT ?
> + IMX8MM_GPR_PCIE_REF_CLK_EXT :
> + IMX8MM_GPR_PCIE_REF_CLK_PLL);
> + usleep_range(100, 200);
> +
> + /* Do the PHY common block reset */
> + regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> + IMX8MM_GPR_PCIE_CMN_RST,
> + IMX8MM_GPR_PCIE_CMN_RST);
> + usleep_range(200, 500);
> +
> +

No multi blank line please

> +static struct platform_driver imx8_pcie_phy_driver = {
> + .probe = imx8_pcie_phy_probe,
> + .driver = {
> + .name = "imx8-pcie-phy",
> + .of_match_table = imx8_pcie_phy_of_match,
> + }
> +};
> +module_platform_driver(imx8_pcie_phy_driver);
> +
> +MODULE_DESCRIPTION("FSL IMX8 PCIE PHY driver");
> +MODULE_LICENSE("GPL");

This does not match the SPDX tag you have given

--
~Vinod

2021-12-02 05:43:45

by Richard Zhu

Subject: RE: [PATCH v6 0/8] Add the imx8m pcie phy driver and imx8mm pcie support

> -----Original Message-----
> From: Lorenzo Pieralisi <[email protected]>
> Sent: Wednesday, December 1, 2021 8:44 PM
> To: Hongxing Zhu <[email protected]>
> Cc: [email protected]; [email protected]; Marcel Ziswiler
> <[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 v6 0/8] Add the imx8m pcie phy driver and imx8mm
> pcie support
>
> On Thu, Nov 18, 2021 at 09:54:41AM +0800, Richard Zhu wrote:
> > Refer to the discussion [1] when try to enable i.MX8MM PCIe support,
> > one standalone PCIe PHY driver should be seperated from i.MX PCIe
> > driver when enable i.MX8MM PCIe support.
> >
> > This patch-set adds the standalone PCIe PHY driver suport[1-5], and
> > i.MX8MM PCIe support[6-8] to have whole view to review this
> patch-set.
> >
> > The PCIe works on i.MX8MM EVK board based the the blkctrl power
> driver
> > [2] and this patch-set. And tested by Tim and Marcel on the different
> > reference clock modes boards.
> >
> > [1]
> >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa
> tc
> >
> hwork.ozlabs.org%2Fproject%2Flinux-pci%2Fpatch%2F20210510141509.
> 929120
> >
> -3-l.stach%40pengutronix.de%2F&amp;data=04%7C01%7Chongxing.zhu
> %40nxp.c
> >
> om%7C3edb11e040e6412cf91108d9b4c85052%7C686ea1d3bc2b4c6fa9
> 2cd99c5c3016
> >
> 35%7C0%7C1%7C637739594698843569%7CUnknown%7CTWFpbGZsb3d
> 8eyJWIjoiMC4wLj
> >
> AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&
> amp;sdata=
> >
> 5xlZSnBYs1SIIbMnmlQwi0qtfLDgKbueLNjPWIPD1pw%3D&amp;reserved=
> 0
> > [2]
> >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa
> tc
> >
> hwork.kernel.org%2Fproject%2Flinux-arm-kernel%2Fcover%2F20210910
> 202640
> > .980366-1-l.stach%40pengutronix.de%2F&amp;data=04%7C01%7Chon
> gxing.zhu%
> >
> 40nxp.com%7C3edb11e040e6412cf91108d9b4c85052%7C686ea1d3bc2
> b4c6fa92cd99
> >
> c5c301635%7C0%7C1%7C637739594698843569%7CUnknown%7CTWFp
> bGZsb3d8eyJWIjo
> >
> iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> 7C3000&amp
> > ;sdata=1388J8dLuKUc6KEUnWj5pLpkaPDC4kTIZFF%2BPTspHZY%3D&a
> mp;reserved=0
> >
> > Main changes v5 --> v6:
> > - Add "Reviewed-by: Rob Herring <[email protected]>" into #1 and #3
> patches.
> > - Merge Rob's review comments to the #2 patch.
> >
> > Main changes v4 --> v5:
> > - Set the AUX_EN always 1b'1, thus it can fix the regression introduced
> in v4
> > series on Marcel's board.
> > - Use the lower-case letter in the devicetreee refer to Marcel's
> comments.
> > _ Since the default value of the deemphasis parameters are zero, only
> set
> > the deemphasis registers when the input paramters are none zero.
> >
> > Main changes v3 --> v4:
> > - Update the yaml to fix syntax error, add maxitems and drop
> > description of phy
> > - Correct the clock name in PHY DT node.
> > - Squash the EVK board relalted dts changes into one patch, and drop
> the
> > useless dummy clock and gpio suffix in DT nodes.
> > - Add board specific de-emphasis parameters as DT properties. Thus
> each board
> > can specify its actual de-emphasis values.
> > - Update the commit log of PHY driver.
> > - Remove the useless codes from PCIe driver, since they are moved to
> > PHY driver
> > - After the discussion and verification of the CLKREQ# configurations
> with Tim,
> > agree to add an optional boolean property "fsl,clkreq-unsupported",
> indicates
> > the CLKREQ# signal is hooked or not in HW designs.
> > - Add "Tested-by: Marcel Ziswiler <[email protected]>" tag,
> since
> > Marcel help to test the v3 patch-set.
> >
> > Main changes v2 --> v3:
> > - Regarding Lucas' comments.
> > - to have a whole view to review the patches, send out the i.MX8MM
> PCIe support too.
> > - move the PHY related bits manipulations of the GPR/SRC to
> standalone PHY driver.
> > - split the dts changes to SOC and board DT, and use the enum instead
> of raw value.
> > - update the license of the dt-binding header file.
> >
> > Changes v1 --> v2:
> > - Update the license of the dt-binding header file to make the license
> > compatible with dts files.
> > - Fix the dt_binding_check errors.
> >
> > Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 6
> +++
> > Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml | 92
> +++++++++++++++++++++++++++++++
> > arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi |
> 55 +++++++++++++++++++
> > arch/arm64/boot/dts/freescale/imx8mm.dtsi |
> 46 +++++++++++++++-
> > drivers/pci/controller/dwc/pci-imx6.c |
> 73 ++++++++++++++++++++++---
> > drivers/phy/freescale/Kconfig |
> 9 ++++
> > drivers/phy/freescale/Makefile |
> 1 +
> > drivers/phy/freescale/phy-fsl-imx8m-pcie.c |
> 237
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++
> > include/dt-bindings/phy/phy-imx8-pcie.h |
> 14 +++++
> > 9 files changed, 525 insertions(+), 8 deletions(-)
>
> Hi Richard,
>
> I can pull this series into the PCI tree (but not the dts changes that should
> be routed elsewhere) or give an ACK for patch 8, please let me know
> what's the best option.
[Richard Zhu] Hi Lorenzo:
First of all, thanks a lot for your kindly help.
To my original understand, #1-3 patch had been reviewed by Rob, might be
merged into Rob's dt-binding git repos.
Shawn takes dts changes ( #4, #6 and #7). And PHY driver merged by vkoul
or Kishon.
In the end, the PCIe changes(#8) are merged into PCIe git repos.

I'm appreciated if you pull this whole series although I'm not sure that
you can do it or not.
Today, I'm glad to receive vkoul's comments about the PHY driver part.
Let me continue refine the PHY driver, send anther version of this patch-set.
Then, let's figure out what's the best option to merge this series.

BTW, I'm prefer to let the following one patch-set merged firstly.
But I can't get contact and response from Lucas for a while.
Lucas might be on his vocation and limited to access the email.
I will ping him a few days later.
https://patchwork.ozlabs.org/project/linux-pci/cover/[email protected]/

Best Regards
Richard
>
> Thanks,
> Lorenzo
>
> > [PATCH v6 1/8] dt-bindings: phy: phy-imx8-pcie: Add binding for the
> > [PATCH v6 2/8] dt-bindings: phy: Add imx8 pcie phy driver support
> > [PATCH v6 3/8] dt-bindings: imx6q-pcie: Add PHY phandles and name
> > [PATCH v6 4/8] arm64: dts: imx8mm: Add the pcie phy support [PATCH
> v6
> > 5/8] phy: freescale: pcie: Initialize the imx8 pcie [PATCH v6 6/8]
> > arm64: dts: imx8mm: Add the pcie support [PATCH v6 7/8] arm64: dts:
> > imx8mm-evk: Add the pcie support on imx8mm [PATCH v6 8/8] PCI:
> imx:
> > Add the imx8mm pcie support

2021-12-02 05:57:37

by Richard Zhu

Subject: RE: [PATCH v6 5/8] phy: freescale: pcie: Initialize the imx8 pcie standalone phy driver

> -----Original Message-----
> From: Vinod Koul <[email protected]>
> Sent: Thursday, December 2, 2021 12:30 PM
> To: Hongxing Zhu <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; Marcel Ziswiler
> <[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 v6 5/8] phy: freescale: pcie: Initialize the imx8 pcie
> standalone phy driver
>
> On 18-11-21, 09:54, Richard Zhu wrote:
> > Add the standalone i.MX8 PCIe PHY driver.
> >
> > Signed-off-by: Richard Zhu <[email protected]>
> > Tested-by: Marcel Ziswiler <[email protected]>
> > Reviewed-by: Tim Harvey <[email protected]>
> > Tested-by: Tim Harvey <[email protected]>
> > ---
> > drivers/phy/freescale/Kconfig | 9 +
> > drivers/phy/freescale/Makefile | 1 +
> > drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 237
> > +++++++++++++++++++++
> > 3 files changed, 247 insertions(+)
> > create mode 100644 drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> >
> > diff --git a/drivers/phy/freescale/Kconfig
> > b/drivers/phy/freescale/Kconfig index 320630ffe3cd..e821498b1f7f
> > 100644
> > --- a/drivers/phy/freescale/Kconfig
> > +++ b/drivers/phy/freescale/Kconfig
> > @@ -14,3 +14,12 @@ config PHY_MIXEL_MIPI_DPHY
> > help
> > Enable this to add support for the Mixel DSI PHY as found
> > on NXP's i.MX8 family of SOCs.
> > +
> > +config PHY_FSL_IMX8M_PCIE
> > + tristate "Freescale i.MX8M PCIE PHY"
> > + depends on OF && HAS_IOMEM
> > + select GENERIC_PHY
> > + default ARCH_MXC && ARM64
>
> Why should this be default ? We dont do that for new drivers.. You may
> add this to respective config file though...
>
[Richard Zhu] First of all, thanks a lot for your review comments.
I see, and would remove the default in the kconfig.

> > +static int imx8_pcie_phy_init(struct phy *phy) {
> > + int ret;
> > + u32 val, pad_mode;
> > + struct imx8_pcie_phy *imx8_phy = phy_get_drvdata(phy);
> > +
> > + reset_control_assert(imx8_phy->reset);
> > +
> > + pad_mode = imx8_phy->refclk_pad_mode;
> > + /* 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,
> > + imx8_phy->clkreq_unused ?
> > + 0 : IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE);
> > + regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> > + IMX8MM_GPR_PCIE_AUX_EN,
> > + IMX8MM_GPR_PCIE_AUX_EN);
> > + regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> > + IMX8MM_GPR_PCIE_POWER_OFF, 0);
> > + regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> > + IMX8MM_GPR_PCIE_SSC_EN, 0);
> > +
> > + regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> > + IMX8MM_GPR_PCIE_REF_CLK_SEL,
> > + pad_mode == IMX8_PCIE_REFCLK_PAD_INPUT ?
> > + IMX8MM_GPR_PCIE_REF_CLK_EXT :
> > + IMX8MM_GPR_PCIE_REF_CLK_PLL);
> > + usleep_range(100, 200);
> > +
> > + /* Do the PHY common block reset */
> > + regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> > + IMX8MM_GPR_PCIE_CMN_RST,
> > + IMX8MM_GPR_PCIE_CMN_RST);
> > + usleep_range(200, 500);
> > +
> > +
>
> No multi blank line please
[Richard Zhu] Ok, got that.
>
> > +static struct platform_driver imx8_pcie_phy_driver = {
> > + .probe = imx8_pcie_phy_probe,
> > + .driver = {
> > + .name = "imx8-pcie-phy",
> > + .of_match_table = imx8_pcie_phy_of_match,
> > + }
> > +};
> > +module_platform_driver(imx8_pcie_phy_driver);
> > +
> > +MODULE_DESCRIPTION("FSL IMX8 PCIE PHY driver");
> > +MODULE_LICENSE("GPL");
>
> This does not match the SPDX tag you have given
[Richard Zhu] Should the "GPL v2" correct one?
If yes, I would change it this way later.
Thanks.

Best Regards
Richard
>
> --
> ~Vinod

2021-12-02 06:03:23

by Vinod Koul

Subject: Re: [PATCH v6 0/8] Add the imx8m pcie phy driver and imx8mm pcie support

On 02-12-21, 05:43, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Lorenzo Pieralisi <[email protected]>
> > Sent: Wednesday, December 1, 2021 8:44 PM
> > To: Hongxing Zhu <[email protected]>
> > Cc: [email protected]; [email protected]; Marcel Ziswiler
> > <[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 v6 0/8] Add the imx8m pcie phy driver and imx8mm
> > pcie support
> >
> > On Thu, Nov 18, 2021 at 09:54:41AM +0800, Richard Zhu wrote:
> > > Refer to the discussion [1] when try to enable i.MX8MM PCIe support,
> > > one standalone PCIe PHY driver should be seperated from i.MX PCIe
> > > driver when enable i.MX8MM PCIe support.
> > >
> > > This patch-set adds the standalone PCIe PHY driver suport[1-5], and
> > > i.MX8MM PCIe support[6-8] to have whole view to review this
> > patch-set.
> > >
> > > The PCIe works on i.MX8MM EVK board based the the blkctrl power
> > driver
> > > [2] and this patch-set. And tested by Tim and Marcel on the different
> > > reference clock modes boards.
> > >
> > > [1]
> > >
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa
> > tc
> > >
> > hwork.ozlabs.org%2Fproject%2Flinux-pci%2Fpatch%2F20210510141509.
> > 929120
> > >
> > -3-l.stach%40pengutronix.de%2F&amp;data=04%7C01%7Chongxing.zhu
> > %40nxp.c
> > >
> > om%7C3edb11e040e6412cf91108d9b4c85052%7C686ea1d3bc2b4c6fa9
> > 2cd99c5c3016
> > >
> > 35%7C0%7C1%7C637739594698843569%7CUnknown%7CTWFpbGZsb3d
> > 8eyJWIjoiMC4wLj
> > >
> > AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&
> > amp;sdata=
> > >
> > 5xlZSnBYs1SIIbMnmlQwi0qtfLDgKbueLNjPWIPD1pw%3D&amp;reserved=
> > 0
> > > [2]
> > >
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa
> > tc
> > >
> > hwork.kernel.org%2Fproject%2Flinux-arm-kernel%2Fcover%2F20210910
> > 202640
> > > .980366-1-l.stach%40pengutronix.de%2F&amp;data=04%7C01%7Chon
> > gxing.zhu%
> > >
> > 40nxp.com%7C3edb11e040e6412cf91108d9b4c85052%7C686ea1d3bc2
> > b4c6fa92cd99
> > >
> > c5c301635%7C0%7C1%7C637739594698843569%7CUnknown%7CTWFp
> > bGZsb3d8eyJWIjo
> > >
> > iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> > 7C3000&amp
> > > ;sdata=1388J8dLuKUc6KEUnWj5pLpkaPDC4kTIZFF%2BPTspHZY%3D&a
> > mp;reserved=0
> > >
> > > Main changes v5 --> v6:
> > > - Add "Reviewed-by: Rob Herring <[email protected]>" into #1 and #3
> > patches.
> > > - Merge Rob's review comments to the #2 patch.
> > >
> > > Main changes v4 --> v5:
> > > - Set the AUX_EN always 1b'1, thus it can fix the regression introduced
> > in v4
> > > series on Marcel's board.
> > > - Use the lower-case letter in the devicetreee refer to Marcel's
> > comments.
> > > _ Since the default value of the deemphasis parameters are zero, only
> > set
> > > the deemphasis registers when the input paramters are none zero.
> > >
> > > Main changes v3 --> v4:
> > > - Update the yaml to fix syntax error, add maxitems and drop
> > > description of phy
> > > - Correct the clock name in PHY DT node.
> > > - Squash the EVK board relalted dts changes into one patch, and drop
> > the
> > > useless dummy clock and gpio suffix in DT nodes.
> > > - Add board specific de-emphasis parameters as DT properties. Thus
> > each board
> > > can specify its actual de-emphasis values.
> > > - Update the commit log of PHY driver.
> > > - Remove the useless codes from PCIe driver, since they are moved to
> > > PHY driver
> > > - After the discussion and verification of the CLKREQ# configurations
> > with Tim,
> > > agree to add an optional boolean property "fsl,clkreq-unsupported",
> > indicates
> > > the CLKREQ# signal is hooked or not in HW designs.
> > > - Add "Tested-by: Marcel Ziswiler <[email protected]>" tag,
> > since
> > > Marcel help to test the v3 patch-set.
> > >
> > > Main changes v2 --> v3:
> > > - Regarding Lucas' comments.
> > > - to have a whole view to review the patches, send out the i.MX8MM
> > PCIe support too.
> > > - move the PHY related bits manipulations of the GPR/SRC to
> > standalone PHY driver.
> > > - split the dts changes to SOC and board DT, and use the enum instead
> > of raw value.
> > > - update the license of the dt-binding header file.
> > >
> > > Changes v1 --> v2:
> > > - Update the license of the dt-binding header file to make the license
> > > compatible with dts files.
> > > - Fix the dt_binding_check errors.
> > >
> > > Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 6
> > +++
> > > Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml | 92
> > +++++++++++++++++++++++++++++++
> > > arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi |
> > 55 +++++++++++++++++++
> > > arch/arm64/boot/dts/freescale/imx8mm.dtsi |
> > 46 +++++++++++++++-
> > > drivers/pci/controller/dwc/pci-imx6.c |
> > 73 ++++++++++++++++++++++---
> > > drivers/phy/freescale/Kconfig |
> > 9 ++++
> > > drivers/phy/freescale/Makefile |
> > 1 +
> > > drivers/phy/freescale/phy-fsl-imx8m-pcie.c |
> > 237
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > +++++++++++++++++++++
> > > include/dt-bindings/phy/phy-imx8-pcie.h |
> > 14 +++++
> > > 9 files changed, 525 insertions(+), 8 deletions(-)
> >
> > Hi Richard,
> >
> > I can pull this series into the PCI tree (but not the dts changes that should
> > be routed elsewhere) or give an ACK for patch 8, please let me know
> > what's the best option.
> [Richard Zhu] Hi Lorenzo:
> First of all, thanks a lot for your kindly help.
> To my original understand, #1-3 patch had been reviewed by Rob, might be
> merged into Rob's dt-binding git repos.
> Shawn takes dts changes ( #4, #6 and #7). And PHY driver merged by vkoul
> or Kishon.
> In the end, the PCIe changes(#8) are merged into PCIe git repos.
>
> I'm appreciated if you pull this whole series although I'm not sure that
> you can do it or not.
> Today, I'm glad to receive vkoul's comments about the PHY driver part.
> Let me continue refine the PHY driver, send anther version of this patch-set.
> Then, let's figure out what's the best option to merge this series.

I think phy binding and driver changes should go thru phy tree and pcie
binding and driver changes thru pcie tree, dt should be picked by
respective arch tree

--
~Vinod

2021-12-02 06:04:09

by Vinod Koul

Subject: Re: [PATCH v6 5/8] phy: freescale: pcie: Initialize the imx8 pcie standalone phy driver

On 02-12-21, 05:57, Hongxing Zhu wrote:

> > > +static struct platform_driver imx8_pcie_phy_driver = {
> > > + .probe = imx8_pcie_phy_probe,
> > > + .driver = {
> > > + .name = "imx8-pcie-phy",
> > > + .of_match_table = imx8_pcie_phy_of_match,
> > > + }
> > > +};
> > > +module_platform_driver(imx8_pcie_phy_driver);
> > > +
> > > +MODULE_DESCRIPTION("FSL IMX8 PCIE PHY driver");
> > > +MODULE_LICENSE("GPL");
> >
> > This does not match the SPDX tag you have given
> [Richard Zhu] Should the "GPL v2" correct one?
> If yes, I would change it this way later.

That would match spdx tag

--
~Vinod

2021-12-03 02:12:46

by Richard Zhu

Subject: RE: [PATCH v6 0/8] Add the imx8m pcie phy driver and imx8mm pcie support

> -----Original Message-----
> From: Vinod Koul <[email protected]>
> Sent: Thursday, December 2, 2021 2:03 PM
> To: Hongxing Zhu <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>;
> [email protected]; [email protected]; Marcel Ziswiler
> <[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 v6 0/8] Add the imx8m pcie phy driver and imx8mm
> pcie support
>
> On 02-12-21, 05:43, Hongxing Zhu wrote:
> > > -----Original Message-----
> > > From: Lorenzo Pieralisi <[email protected]>
> > > Sent: Wednesday, December 1, 2021 8:44 PM
> > > To: Hongxing Zhu <[email protected]>
> > > Cc: [email protected]; [email protected]; Marcel Ziswiler
> > > <[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 v6 0/8] Add the imx8m pcie phy driver and
> imx8mm
> > > pcie support
> > >
> > > On Thu, Nov 18, 2021 at 09:54:41AM +0800, Richard Zhu wrote:
> > > > Refer to the discussion [1] when try to enable i.MX8MM PCIe
> > > > support, one standalone PCIe PHY driver should be seperated from
> > > > i.MX PCIe driver when enable i.MX8MM PCIe support.
> > > >
> > > > This patch-set adds the standalone PCIe PHY driver suport[1-5],
> > > > and i.MX8MM PCIe support[6-8] to have whole view to review this
> > > patch-set.
> > > >
> > > > The PCIe works on i.MX8MM EVK board based the the blkctrl power
> > > driver
> > > > [2] and this patch-set. And tested by Tim and Marcel on the
> > > > different reference clock modes boards.
> > > >
> > > > [1]
> > > >
> > >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa
> > > tc
> > > >
> > >
> hwork.ozlabs.org%2Fproject%2Flinux-pci%2Fpatch%2F20210510141509.
> > > 929120
> > > >
> > >
> -3-l.stach%40pengutronix.de%2F&amp;data=04%7C01%7Chongxing.zhu
> > > %40nxp.c
> > > >
> > >
> om%7C3edb11e040e6412cf91108d9b4c85052%7C686ea1d3bc2b4c6fa9
> > > 2cd99c5c3016
> > > >
> > >
> 35%7C0%7C1%7C637739594698843569%7CUnknown%7CTWFpbGZsb3d
> > > 8eyJWIjoiMC4wLj
> > > >
> > >
> AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&
> > > amp;sdata=
> > > >
> > >
> 5xlZSnBYs1SIIbMnmlQwi0qtfLDgKbueLNjPWIPD1pw%3D&amp;reserved=
> > > 0
> > > > [2]
> > > >
> > >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa
> > > tc
> > > >
> > >
> hwork.kernel.org%2Fproject%2Flinux-arm-kernel%2Fcover%2F20210910
> > > 202640
> > > > .980366-1-l.stach%40pengutronix.de%2F&amp;data=04%7C01%7C
> hon
> > > gxing.zhu%
> > > >
> > >
> 40nxp.com%7C3edb11e040e6412cf91108d9b4c85052%7C686ea1d3bc2
> > > b4c6fa92cd99
> > > >
> > >
> c5c301635%7C0%7C1%7C637739594698843569%7CUnknown%7CTWFp
> > > bGZsb3d8eyJWIjo
> > > >
> > >
> iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> > > 7C3000&amp
> > > > ;sdata=1388J8dLuKUc6KEUnWj5pLpkaPDC4kTIZFF%2BPTspHZY%3D
> &a
> > > mp;reserved=0
> > > >
> > > > Main changes v5 --> v6:
> > > > - Add "Reviewed-by: Rob Herring <[email protected]>" into #1 and
> #3
> > > patches.
> > > > - Merge Rob's review comments to the #2 patch.
> > > >
> > > > Main changes v4 --> v5:
> > > > - Set the AUX_EN always 1b'1, thus it can fix the regression
> > > > introduced
> > > in v4
> > > > series on Marcel's board.
> > > > - Use the lower-case letter in the devicetreee refer to Marcel's
> > > comments.
> > > > _ Since the default value of the deemphasis parameters are zero,
> > > > only
> > > set
> > > > the deemphasis registers when the input paramters are none
> zero.
> > > >
> > > > Main changes v3 --> v4:
> > > > - Update the yaml to fix syntax error, add maxitems and drop
> > > > description of phy
> > > > - Correct the clock name in PHY DT node.
> > > > - Squash the EVK board relalted dts changes into one patch, and
> > > > drop
> > > the
> > > > useless dummy clock and gpio suffix in DT nodes.
> > > > - Add board specific de-emphasis parameters as DT properties. Thus
> > > each board
> > > > can specify its actual de-emphasis values.
> > > > - Update the commit log of PHY driver.
> > > > - Remove the useless codes from PCIe driver, since they are moved
> > > > to PHY driver
> > > > - After the discussion and verification of the CLKREQ#
> > > > configurations
> > > with Tim,
> > > > agree to add an optional boolean property
> > > > "fsl,clkreq-unsupported",
> > > indicates
> > > > the CLKREQ# signal is hooked or not in HW designs.
> > > > - Add "Tested-by: Marcel Ziswiler <[email protected]>"
> > > > tag,
> > > since
> > > > Marcel help to test the v3 patch-set.
> > > >
> > > > Main changes v2 --> v3:
> > > > - Regarding Lucas' comments.
> > > > - to have a whole view to review the patches, send out the
> > > > i.MX8MM
> > > PCIe support too.
> > > > - move the PHY related bits manipulations of the GPR/SRC to
> > > standalone PHY driver.
> > > > - split the dts changes to SOC and board DT, and use the enum
> > > > instead
> > > of raw value.
> > > > - update the license of the dt-binding header file.
> > > >
> > > > Changes v1 --> v2:
> > > > - Update the license of the dt-binding header file to make the
> license
> > > > compatible with dts files.
> > > > - Fix the dt_binding_check errors.
> > > >
> > > > Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml |
> 6
> > > +++
> > > > Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml |
> 92
> > > +++++++++++++++++++++++++++++++
> > > > arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
> |
> > > 55 +++++++++++++++++++
> > > > arch/arm64/boot/dts/freescale/imx8mm.dtsi
> |
> > > 46 +++++++++++++++-
> > > > drivers/pci/controller/dwc/pci-imx6.c
> |
> > > 73 ++++++++++++++++++++++---
> > > > drivers/phy/freescale/Kconfig
> |
> > > 9 ++++
> > > > drivers/phy/freescale/Makefile
> |
> > > 1 +
> > > > drivers/phy/freescale/phy-fsl-imx8m-pcie.c |
> > > 237
> > >
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > +++++++++++++++++++++
> > > > include/dt-bindings/phy/phy-imx8-pcie.h
> |
> > > 14 +++++
> > > > 9 files changed, 525 insertions(+), 8 deletions(-)
> > >
> > > Hi Richard,
> > >
> > > I can pull this series into the PCI tree (but not the dts changes
> > > that should be routed elsewhere) or give an ACK for patch 8, please
> > > let me know what's the best option.
> > [Richard Zhu] Hi Lorenzo:
> > First of all, thanks a lot for your kindly help.
> > To my original understand, #1-3 patch had been reviewed by Rob, might
> > be merged into Rob's dt-binding git repos.
> > Shawn takes dts changes ( #4, #6 and #7). And PHY driver merged by
> > vkoul or Kishon.
> > In the end, the PCIe changes(#8) are merged into PCIe git repos.
> >
> > I'm appreciated if you pull this whole series although I'm not sure
> > that you can do it or not.
> > Today, I'm glad to receive vkoul's comments about the PHY driver part.
> > Let me continue refine the PHY driver, send anther version of this
> patch-set.
> > Then, let's figure out what's the best option to merge this series.
>
> I think phy binding and driver changes should go thru phy tree and pcie
> binding and driver changes thru pcie tree, dt should be picked by
> respective arch tree
[Richard Zhu] Hi Vinod: I'm fine with this option.
Please help to take the phy binding and driver after the review cycle.
Thanks in advanced.

Best regards
Richard
>
> --
> ~Vinod