> -----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?
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);
> > + }
