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

Marcel Ziswiler
2022-01-02T00:26:00Z

On Thu, 2021-12-30 at 04:58 +0000, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Philip Molloy <philip@linutronix.de>
> > Sent: Wednesday, December 29, 2021 8:40 PM
> > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> > lorenzo.pieralisi@arm.com; tharvey@gateworks.com; Marcel Ziswiler
> > <marcel.ziswiler@toradex.com>; kishon@ti.com; vkoul@kernel.org;
> > robh@kernel.org; galak@kernel.crashing.org; shawnguo@kernel.org;
> > linux-phy@lists.infradead.org; devicetree@vger.kernel.org;
> > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > linux-kernel@vger.kernel.org; kernel@pengutronix.de; dl-linux-imx
> > <linux-imx@nxp.com>
> > Subject: Re: [PATCH v7 5/8] phy: freescale: pcie: Initialize the imx8 pcie
> > standalone phy driver
> >
> > Hi Richard,
> >
> > I've run into an issue that appears to indicate a functional difference
> > between the existing integrated pci-imx6.c implementation and this new
> > implementation with the separate phy driver.
> >
> > I'm working with a SOM and baseboard from Phytec that is based on the
> > IMX8MM. The board does not have an external PCIe clock and has a
> > ethernet controller hanging off the PCIe bus.
> >
> > When booting from a 5.4 NXP-based kernel from Phytec the ethernet
> > controller is probed and functions as expected.
> >
> > A co-worker backported a slightly earlier version of this patchset to
> > 5.10.[1] With our kernel both the controller driver and new PHY driver are
> > probed, but a timeout occurs in dw_pcie_wait_for_link() which indicates
> > that the "Phy link never came up".
> >
> > After reproducing this issue I configured pcie_phy with
> > IMX8_PCIE_REFCLK_PAD_OUTPUT. With that configured, phy register
> > CMN_REG062/0x188 matches the 5.4 NXP/Phytec kernel. I then
> > compared the controller and PHY registers between the two kernels and
> > noticed that CMN_REG063/0x18c is set to AUX_IN/0x0 in the 5.4
> > NXP/Phytec kernel, but the new PHY driver writes
> > I_PLL_REFCLK_FROM_SYSPLL/0xc0 to that register.
> >
> > If I modify the phy driver to not write I_PLL_REFCLK_FROM_SYSPLL/0xc0
> > then the system behaves as expected.
> [Richard Zhu]
> Hi Philip:
> The address 0x018C is a register to control the output mode of Refclk IO.
> When internal syspll is used as PHY REF clock.
> Regarding my understand, this register should select the syspll, and route
> it out of SOC from CLK N/P pads.
> Then the remote EP device can use the clock from CLK N/P pads.
>
> If the bit7-6 is set to 2b'00, there wouldn't clock output from CLK N/P pads.
> What's the hardware design of the CLK N/P pads in your project?
> Can you monitor the situation of the CLK N/P if it is possible?

I can confirm that for me similar hardware which also "does not have an external PCIe clock" works just fine
with the following device tree sniped:

&pcie0 {
assigned-clocks = <&clk IMX8MM_CLK_PCIE1_AUX>,
<&clk IMX8MM_CLK_PCIE1_CTRL>;
assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_50M>,
<&clk IMX8MM_SYS_PLL2_250M>;
assigned-clock-rates = <10000000>, <250000000>;
clocks = <&clk IMX8MM_CLK_PCIE1_ROOT>, <&clk IMX8MM_CLK_PCIE1_AUX>,
<&clk IMX8MM_CLK_PCIE1_PHY>;
clock-names = "pcie", "pcie_aux", "pcie_bus";
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pcie0>;
/* PCIE_1_RESET# (SODIMM 244) */
reset-gpio = <&gpio3 19 GPIO_ACTIVE_LOW>;
status = "okay";
};

&pcie_phy {
clocks = <&clk IMX8MM_CLK_PCIE1_PHY>;
fsl,clkreq-unsupported;
fsl,refclk-pad-mode = <IMX8_PCIE_REFCLK_PAD_OUTPUT>;
fsl,tx-deemph-gen1 = <0x2d>;
fsl,tx-deemph-gen2 = <0xf>;
status = "okay";
};

During boot that looks as follows:

[ 1.858312] imx6q-pcie 33800000.pcie: host bridge /soc@0/pcie@33800000 ranges:

[ 1.865630] imx6q-pcie 33800000.pcie: IO 0x001ff80000..0x001ff8ffff -> 0x0000000000
[ 1.877833] imx6q-pcie 33800000.pcie: MEM 0x0018000000..0x001fefffff -> 0x0018000000

[ 1.992010] imx6q-pcie 33800000.pcie: iATU unroll: enabled
[ 1.997523] imx6q-pcie 33800000.pcie: Detected iATU regions: 4 outbound, 4 inbound

[ 2.103140] imx6q-pcie 33800000.pcie: Link up
[ 2.107527] imx6q-pcie 33800000.pcie: Link up
[ 2.111895] imx6q-pcie 33800000.pcie: Link up, Gen1
[ 2.116786] imx6q-pcie 33800000.pcie: Link up
[ 2.121298] imx6q-pcie 33800000.pcie: PCI host bridge to bus 0000:00
[ 2.127671] pci_bus 0000:00: root bus resource [bus 00-ff]
[ 2.133171] pci_bus 0000:00: root bus resource [io 0x0000-0xffff]
[ 2.139363] pci_bus 0000:00: root bus resource [mem 0x18000000-0x1fefffff]
[ 2.146282] pci 0000:00:00.0: [16c3:abcd] type 01 class 0x060400
[ 2.152322] pci 0000:00:00.0: reg 0x10: [mem 0x00000000-0x000fffff]
[ 2.158612] pci 0000:00:00.0: reg 0x38: [mem 0x00000000-0x0000ffff pref]
[ 2.165373] pci 0000:00:00.0: supports D1
[ 2.169395] pci 0000:00:00.0: PME# supported from D0 D1 D3hot D3cold
[ 2.178101] pci 0000:01:00.0: [10ec:8168] type 00 class 0x020000
[ 2.184196] pci 0000:01:00.0: reg 0x10: [io 0x0000-0x00ff]
[ 2.189864] pci 0000:01:00.0: reg 0x18: [mem 0x00000000-0x00000fff 64bit pref]
[ 2.197152] pci 0000:01:00.0: reg 0x20: [mem 0x00000000-0x00003fff 64bit pref]
[ 2.204777] pci 0000:01:00.0: supports D1 D2
[ 2.209059] pci 0000:01:00.0: PME# supported from D0 D1 D2 D3hot D3cold
[ 2.229114] pci 0000:00:00.0: BAR 0: assigned [mem 0x18000000-0x180fffff]
[ 2.235934] pci 0000:00:00.0: BAR 15: assigned [mem 0x18100000-0x181fffff pref]
[ 2.243260] pci 0000:00:00.0: BAR 6: assigned [mem 0x18200000-0x1820ffff pref]
[ 2.250501] pci 0000:00:00.0: BAR 13: assigned [io 0x1000-0x1fff]
[ 2.256734] pci 0000:01:00.0: BAR 4: assigned [mem 0x18100000-0x18103fff 64bit pref]
[ 2.264534] pci 0000:01:00.0: BAR 2: assigned [mem 0x18104000-0x18104fff 64bit pref]
[ 2.272338] pci 0000:01:00.0: BAR 0: assigned [io 0x1000-0x10ff]
[ 2.278460] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
[ 2.283698] pci 0000:00:00.0: bridge window [io 0x1000-0x1fff]
[ 2.289808] pci 0000:00:00.0: bridge window [mem 0x18100000-0x181fffff pref]
[ 2.297444] pcieport 0000:00:00.0: PME: Signaling with IRQ 225

And my mini-PCIe Ethernet card gets detected and is fully functional:

[ 4.433899] r8169 0000:01:00.0: enabling device (0000 -> 0003)
[ 4.466731] libphy: r8169: probed
[ 4.476288] r8169 0000:01:00.0 eth1: RTL8168e/8111e, 00:e0:4c:80:f4:0d, XID 2c2, IRQ 229
[ 4.484485] r8169 0000:01:00.0 eth1: jumbo features [frames: 9194 bytes, tx checksumming: ko]

[ 5.023616] r8169 0000:01:00.0 enp1s0: renamed from eth1

[ 6.713601] r8169 0000:01:00.0: Direct firmware load for rtl_nic/rtl8168e-2.fw failed with error -2
[ 6.722786] r8169 0000:01:00.0: Unable to load firmware rtl_nic/rtl8168e-2.fw (-2)
[ 6.731391] RTL8211DN Gigabit Ethernet r8169-0-100:00: attached PHY driver (mii_bus:phy_addr=r8169-0-100:00,
irq=MAC)
[ 6.808798] r8169 0000:01:00.0 enp1s0: Link is Down

root@verdin-imx8mm-06760554:~# lspci
00:00.0 PCI bridge: Synopsys, Inc. DWC_usb3 / PCIe bridge (rev 01)
01:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet
Controller (rev 06)

Cheers

Marcel

> Best Regards
> Richard Zhu
>
> >
> > Best,
> > Philip
> >
> > [1]: I plan on rebasing our branch with the latest patches that have been
> > applied upstream. Note that I did not see any difference in the following
> > code with what we have applied.
> >
> > On 12/2/21 09:02, Richard Zhu wrote:
> > > +#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)
> > ...
> > > +       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);
> >
> > If I comment out this writel() then the register defaults to 0x0/AUX_IN
> > and then the system behaves as expected.
> >
> > > +               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);
> > > +       }