-----Original Message-----
From: Marcel Ziswiler <redacted>
Sent: Tuesday, November 2, 2021 7:53 AM
To: tharvey@gateworks.com; Richard Zhu <hongxing.zhu@nxp.com>
Cc: kishon@ti.com; vkoul@kernel.org; kernel@pengutronix.de;
dl-linux-imx [off-list ref]; devicetree@vger.kernel.org;
robh@kernel.org; l.stach@pengutronix.de; shawnguo@kernel.org;
linux-arm-kernel@lists.infradead.org; linux-phy@lists.infradead.org;
galak@kernel.crashing.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 5/8] phy: freescale: pcie: Initialize the imx8 pcie
standalone phy driver

Hi Tim and Richard

On Mon, 2021-11-01 at 10:13 -0700, Tim Harvey wrote:

quoted

On Mon, Nov 1, 2021 at 1:19 AM Richard Zhu [off-list ref]

wrote:

quoted

quoted

quoted

-----Original Message-----
From: Tim Harvey <tharvey@gateworks.com>
Sent: Saturday, October 30, 2021 1:45 AM
To: Richard Zhu <hongxing.zhu@nxp.com>; Marcel Ziswiler
[off-list ref]
Cc: kishon@ti.com; vkoul@kernel.org; robh@kernel.org;
l.stach@pengutronix.de; shawnguo@kernel.org;
galak@kernel.crashing.org; linux-phy@lists.infradead.org;
linux-arm-kernel@lists.infradead.org; kernel@pengutronix.de;
devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
dl-linux-imx [off-list ref]
Subject: Re: [PATCH v4 5/8] phy: freescale: pcie: Initialize the
imx8 pcie standalone phy driver

On Fri, Oct 29, 2021 at 1:45 AM Richard Zhu

[off-list ref]

quoted

quoted

quoted

wrote:

quoted

quoted

-----Original Message-----
From: Marcel Ziswiler <redacted>
Sent: Friday, October 29, 2021 4:13 PM
To: kishon@ti.com; vkoul@kernel.org; robh@kernel.org;
l.stach@pengutronix.de; shawnguo@kernel.org;

tharvey@gateworks.com;

quoted

quoted

galak@kernel.crashing.org; Richard Zhu

[off-list ref]

quoted

quoted

quoted

quoted

quoted

Cc: linux-phy@lists.infradead.org;
linux-arm-kernel@lists.infradead.org;
kernel@pengutronix.de; devicetree@vger.kernel.org;
linux-kernel@vger.kernel.org; dl-linux-imx

[off-list ref]

quoted

quoted

quoted

quoted

quoted

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

On Thu, 2021-10-28 at 15:27 +0800, Richard Zhu wrote:

quoted

Add the standalone i.MX8 PCIe PHY driver.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Tested-by: Marcel Ziswiler <redacted>

Unfortunately, this version no longer works for our
IMX8_PCIE_REFCLK_PAD_OUTPUT use-case. Further comments

in-

quoted

quoted

quoted

lined

quoted

quoted

below.

[Richard Zhu] Sorry to hear about that. Then, it seems that this
bit should be  set anyway.
It's hard to understand this bit refer to the RM document. Sigh

☹.

quoted

quoted

quoted

quoted

Would set the AUX_EN bit later.

quoted

quoted

---
 drivers/phy/freescale/Kconfig              |   9 +
 drivers/phy/freescale/Makefile             |   1 +
 drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 234
+++++++++++++++++++++
 3 files changed, 244 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..de9ee7020f76
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

quoted

quoted

quoted

quoted

quoted

quoted

as

found

quoted

          on NXP's i.MX8 family of SOCs.
+
+config PHY_FSL_IMX8M_PCIE
+       tristate "Freescale i.MX8 PCIE PHY"

Above description is missing the M as in i.MX 8M.

[Richard Zhu] Okay, would be added later.
Thanks.

quoted

quoted

+       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

quoted

quoted

quoted

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

quoted

 obj-$(CONFIG_PHY_MIXEL_MIPI_DPHY)      +=

phy-fsl-imx8-mipi-dphy.o

quoted

+obj-$(CONFIG_PHY_FSL_IMX8M_PCIE)       +=

phy-fsl-imx8m-pcie.o

quoted

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..4b4402eaddcc

--- /dev/null
+++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c

@@ -0,0 +1,234 @@
+// 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

quoted

quoted

quoted

quoted

quoted

quoted

+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

quoted

+BIT(7)

#define  ANA_AUX_RX_TERM_GND_EN                BIT(3)
#define

quoted

quoted

quoted

quoted

quoted

quoted

+ANA_AUX_TX_TERM               BIT(2) #define
+IMX8MM_PCIE_PHY_CMN_REG065     0x194

#define  ANA_AUX_RX_TERM

quoted

+(BIT(7) | BIT(4))

#define  ANA_AUX_TX_LVL

quoted

+GENMASK(3, 0) #define

IMX8MM_PCIE_PHY_CMN_REG75      0x1D4 #define

quoted

+PCIE_PHY_CMN_REG75_PLL_DONE   0x3 #define

PCIE_PHY_TRSV_REG5

quoted

+0x414 #define  PCIE_PHY_TRSV_REG5_GEN1_DEEMP 0x2D

#define

quoted

quoted

quoted

+PCIE_PHY_TRSV_REG6             0x418 #define
+PCIE_PHY_TRSV_REG6_GEN2_DEEMP 0xF
+
+#define

IMX8MM_GPR_PCIE_REF_CLK_SEL    GENMASK(25, 24)

quoted

quoted

quoted

quoted

quoted

#define

quoted

+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)

quoted

quoted

quoted

quoted

quoted

#define

quoted

+IMX8MM_GPR_PCIE_POWER_OFF      BIT(17) #define

IMX8MM_GPR_PCIE_SSC_EN

quoted

+BIT(16) #define

IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE        BIT(9)

quoted

+
+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;

quoted

quoted

quoted

quoted

quoted

quoted

+       u32                     tx_deemph_gen1;

quoted

quoted

quoted

quoted

quoted

quoted

+       u32                     tx_deemph_gen2;

quoted

quoted

quoted

quoted

quoted

quoted

+       bool                    clkreq_unused; };

quoted

quoted

quoted

quoted

quoted

quoted

+
+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

quoted

quoted

quoted

quoted

quoted

hooked */

quoted

+       regmap_update_bits(imx8_phy->iomuxc_gpr,

IOMUXC_GPR14,

quoted

+                          IMX8MM_GPR_PCIE_AUX_EN_

quoted

quoted

quoted

quoted

quoted

OVERRIDE,

quoted

+                          imx8_phy->clkreq_unused ?

quoted

quoted

quoted

quoted

quoted

quoted

+                          0 :

IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE);

quoted

+       regmap_update_bits(imx8_phy->iomuxc_gpr,

IOMUXC_GPR14,

quoted

+                          IMX8MM_GPR_PCIE_AUX_EN,

quoted

quoted

quoted

quoted

quoted

quoted

+                          pad_mode ==

IMX8_PCIE_REFCLK_PAD_INPUT ?

quoted

+                          IMX8MM_GPR_PCIE_AUX_EN :

quoted

quoted

quoted

quoted

quoted

0);

V3 had IMX8MM_GPR_PCIE_AUX_EN always enabled. Turns out

V4

quoted

quoted

quoted

stopped

quoted

quoted

working for our output use-case as it only enables it for the
input use-case. If I enable this one always it starts working

again.

quoted

quoted

quoted

quoted

[Richard Zhu] See my comment above.
Sorry to bring the regression on your board.

Marcel,

Your board does not use an external clk, but does it hook up
CLKREQ# from the socket to either I2C4_SCL or UART4_RXD and pin
muxed as such?

For my board that uses an external clk and does not connect
CLKREQ# to the IMX8MM I need to disable that bit. As Richard says
we have invalid documentation for these bits unfortunately which is

not helping.

quoted

quoted

quoted

Richard, when we do figure out proper documentation for these bits
I suggest you also add a comment block right above their #defines
in the phy driver with the correct documentation to avoid future

confusion.

quoted

quoted

quoted

NXP has had so many mistakes in the various IMX8M RM's and I

fear

quoted

quoted

quoted

they will never get fixed.

[Richard Zhu] Hi Tim:
I took look at the validation codes, and found that the AUX_EN is
always
 set to be 1b'1. Whatever the reference clock mode is selected.
I'm sending a query email to design team, but I'm not sure I can get
response in time.
Can you help to take a double tests at your board when AUX_EN(bit19
of GPR14) is set to be 1b'1 firstly?
Thanks in advanced.

Richard and Marcel,

I apologize, my mistake I was referring to GPR14 bit 9 and not bit19
in my testing above.

I tested leaving bit 9 set and this still works on my boards with ext
clk and no CLKREQ# as well as imx8mm-evk with ext clk and CLKREQ#.

So the change on top of your v4 would be:

diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c

b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
index 4b4402eaddcc..003f575b36f0 100644

--- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
+++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c

@@ -73,8 +73,7 @@ static int imx8_pcie_phy_init(struct phy *phy)

                           0 :

IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE);

quoted

        regmap_update_bits(imx8_phy->iomuxc_gpr,

IOMUXC_GPR14,

quoted

                           IMX8MM_GPR_PCIE_AUX_E

N,

quoted

-                          pad_mode ==

IMX8_PCIE_REFCLK_PAD_INPUT ?

quoted

-                          IMX8MM_GPR_PCIE_AUX_EN :

0);

quoted

+                          IMX8MM_GPR_PCIE_AUX_EN);

quoted

        regmap_update_bits(imx8_phy->iomuxc_gpr,

IOMUXC_GPR14,

quoted

                           IMX8MM_GPR_PCIE_POWER

_OFF, 0);

quoted

        regmap_update_bits(imx8_phy->iomuxc_gpr,

IOMUXC_GPR14,

quoted

Marcel, does this look right for your board?

Yes, that is exactly how I run it now. Thanks!

[Richard Zhu] That's great.
Thanks a lot. I will send v5 set a moment later.
BR
Richard

quoted

Best regards,

Tim

Cheers

Marcel

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy