Message-ID: <20251003172156.GA357448@bhelgaas>
Date: Fri, 3 Oct 2025 12:21:56 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Frank Li <Frank.li@....com>
Cc: Hongxing Zhu <hongxing.zhu@....com>,
	"jingoohan1@...il.com" <jingoohan1@...il.com>,
	"l.stach@...gutronix.de" <l.stach@...gutronix.de>,
	"lpieralisi@...nel.org" <lpieralisi@...nel.org>,
	"kwilczynski@...nel.org" <kwilczynski@...nel.org>,
	"mani@...nel.org" <mani@...nel.org>,
	"robh@...nel.org" <robh@...nel.org>,
	"bhelgaas@...gle.com" <bhelgaas@...gle.com>,
	"shawnguo@...nel.org" <shawnguo@...nel.org>,
	"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
	"kernel@...gutronix.de" <kernel@...gutronix.de>,
	"festevam@...il.com" <festevam@...il.com>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
	"imx@...ts.linux.dev" <imx@...ts.linux.dev>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 2/2] PCI: imx6: Add a method to handle CLKREQ#
 override active low

On Fri, Sep 26, 2025 at 06:46:28PM -0400, Frank Li wrote:
> On Fri, Sep 26, 2025 at 03:25:21PM -0500, Bjorn Helgaas wrote:
> > On Fri, Sep 26, 2025 at 03:08:30AM +0000, Hongxing Zhu wrote:
> > > > -----Original Message-----
> > > > From: Bjorn Helgaas <helgaas@...nel.org>
> >
> > > > On Fri, Sep 26, 2025 at 02:19:37AM +0000, Hongxing Zhu wrote:
> > > > > > -----Original Message-----
> > > > > > From: Bjorn Helgaas <helgaas@...nel.org> On Tue, Sep 23, 2025 at
> > > > > > 03:39:13PM +0800, Richard Zhu wrote:
> > > > > > > The CLKREQ# is an open drain, active low signal that is
> > > > > > > driven low by the card to request reference clock. It's an
> > > > > > > optional signal added in PCIe CEM r4.0, sec 2. Thus, this
> > > > > > > signal wouldn't be driven low if it's reserved.
> > > > > > >
> > > > > > > Since the reference clock controlled by CLKREQ# may be
> > > > > > > required by i.MX PCIe host too. To make sure this clock is
> > > > > > > ready even when the CLKREQ# isn't driven low by the card(e.x
> > > > > > > the scenario described above), force CLKREQ# override active
> > > > > > > low for i.MX PCIe host during initialization.
> > > > > > >
> > > > > > > The CLKREQ# override can be cleared safely when
> > > > > > > supports-clkreq is present and PCIe link is up later.
> > > > > > > Because the CLKREQ# would be driven low by the card at this
> > > > > > > time.
> > > > > >
> > > > > > What happens if we clear the CLKREQ# override (so the host
> > > > > > doesn't assert it), and the link is up but the card never
> > > > > > asserts CLKREQ# (since it's an optional signal)?
> > > > > >
> > > > > > Does the i.MX host still work?
> > > > >
> > > > > The CLKREQ# override active low only be cleared when link is up
> > > > > and supports-clkreq is present. In the other words, there is a
> > > > > remote endpoint  device, and the CLKREQ# would be driven active
> > > > > low by this endpoint device.
> > > >
> > > > Assume an endpoint designed to CEM r2.0.  CLKREQ# doesn't exist in
> > > > CEM r2.0, so even if the endpoint is present and the link is up,
> > > > the endpoint will not assert CLKREQ#.
> > > >
> > > > Will the i.MX host still work?
> >
> > > Yes, i.MX host still work.
> > > If the endpoint designed to CEM r2.0, and CLKREQ# is reserved. The
> > > property suppots-clkreq wouldn't present in this scenario. Thus, the
> > > CLKREQ# override active low set by host driver wouldn't be cleared
> > > later, although the link is up and an endpoint is present.
> >
> > Do you mean 'supports-clkreq' describes the *endpoint*, and you need
> > to change the devicetree depending on which endpoint is connected?
> 
> It is NOT descript *endpoint*. supports-clkreq descript the board design,
> which connect CLKREQ# signal. Because standard slot's CLKREQ# (PIN12) is
> reserved in beggin, so some old PCIe card have not pull down this signal as
> latest spec requirement.
> 
> PCIe Standard slot with INTEL E2000 1G ethernet card, which is producted
> around 10 year ago, PIN12 is reserved.
> 
> So we don't set supports-clkreq for stardard PCI slot, only set it for
> M.2 slot. So stardard PCI slot in imx95 evk can support most cards. We have
> not vendor card lists, which already connect/not connect CLKREQ#, so we
> have to fallback to disconnect CLKREQ# situation by clarm our evk board
> have not connect CLKREQ# to make all card works, eventhough it lost power
> save feature. work is more impantant then power saving.
> 
> > The schema says 'supports-clkreq' tells us whether CLKREQ# signal
> > routing exists, not whether the downstream device actually supports
> > CLKREQ#:
> >
> >   https://github.com/devicetree-org/dt-schema/blob/4b28bc79fdc552f3e0b870ef1362bb711925f4f3/dtschema/schemas/pci/pci-bus-common.yaml#L155
> >
> > I don't see 'supports-clkreq' in any devicetree related to imx6, so
> > I'm not sure this patch is needed yet.  Does it fix an existing
> > problem?
> 
> The patch adding 'supports-clkreq' in dts is on going. No funtional broken
> because it just impact l1ss power saving features.
> 
> > If it enables some future functionality, maybe we should defer it
> > until we're actually ready to enable that functionality?
> 
> Actually, it fixes i.MX95 19x19 EVK second slot problem. At least
> INTEL E2000 1G ethernet card can't work at i.MX95 EVK boards at main
> stream kernel without this patch.

I deferred these two patches so we have time to tidy these up:

  - Coordinate with adding 'supports-clkreq' in devicetrees.

  - Fix the imx95 refclk enable that was missed in the v5 series.

  - Consider making imx95 refclk enable parallel to the other
    versions, e.g., by using .enable_ref_clk() instead of doing it in
    imx95_pcie_init_phy().

  - Describe the "i.MX95 19x19 EVK second slot problem" in the commit
    log.  Possibly split that into a second patch if it can be
    separated from the CLKREQ# override.  It sounds like this part
    doesn't depend on 'supports-clkreq' in a devicetree?

Maybe we can also figure out how to explain why CLKREQ# override is an
issue for imx6 but not for other DWC-based drivers.

Bjorn

Powered by blists - more mailing lists