On Thu, 2021-12-30 at 04:58 +0000, Hongxing Zhu wrote: > > -----Original Message----- > > From: Philip Molloy <philip@xxxxxxxxxxxxx> > > Sent: Wednesday, December 29, 2021 8:40 PM > > To: Hongxing Zhu <hongxing.zhu@xxxxxxx> > > Cc: l.stach@xxxxxxxxxxxxxx; bhelgaas@xxxxxxxxxx; > > lorenzo.pieralisi@xxxxxxx; tharvey@xxxxxxxxxxxxx; Marcel Ziswiler > > <marcel.ziswiler@xxxxxxxxxxx>; kishon@xxxxxx; vkoul@xxxxxxxxxx; > > robh@xxxxxxxxxx; galak@xxxxxxxxxxxxxxxxxxx; shawnguo@xxxxxxxxxx; > > linux-phy@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; > > linux-pci@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > > linux-kernel@xxxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; dl-linux-imx > > <linux-imx@xxxxxxx> > > 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); > > > + }