linux-kernel - Re: [PATCH v1 2/2] PCI: imx6: Add a method to handle CLKREQ# override active low

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ruhcvw3oqalrspkbl4ay5vebomatww6wbirwzowxyqxq7sdjou@yba5ri45j24w>
Date: Mon, 8 Sep 2025 21:19:40 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Frank Li <Frank.li@....com>
Cc: Richard Zhu <hongxing.zhu@....com>, l.stach@...gutronix.de, 
	lpieralisi@...nel.org, kwilczynski@...nel.org, robh@...nel.org, bhelgaas@...gle.com, 
	shawnguo@...nel.org, s.hauer@...gutronix.de, kernel@...gutronix.de, 
	festevam@...il.com, linux-pci@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	imx@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 2/2] PCI: imx6: Add a method to handle CLKREQ#
 override active low

On Mon, Sep 08, 2025 at 11:26:11AM GMT, Frank Li wrote:
> On Mon, Sep 08, 2025 at 11:36:02AM +0530, Manivannan Sadhasivam wrote:
> > On Wed, Aug 20, 2025 at 04:10:48PM GMT, Richard Zhu wrote:
> > > The CLKREQ# is an open drain, active low signal that is driven low by
> > > the card to request reference clock.
> > >
> > > Since the reference clock may be required by i.MX PCIe host too.
> >
> > Add some info on why the refclk is needed by the host.
> >
> > > To make
> > > sure this clock is available even when the CLKREQ# isn't driven low by
> > > the card(e.x no card connected), force CLKREQ# override active low for
> > > i.MX PCIe host during initialization.
> > >
> >
> > CLKREQ# override is not a spec defined feature. So you need to explain what it
> > does first.
> >
> > > 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 in this case.
> > >
> >
> > Why do you need to depend on 'supports-clkreq' property? Don't you already know
> > if your platform supports CLKREQ# or not? None of the upstream DTS has the
> > 'supports-clkreq' property set and the NXP binding also doesn't enable this
> > property.
> 
> It is history reason. Supposed all the boards which supports L1SS need set
> 'supports-clkreq' in dts. L1SS require board design use open drain connect
> RC's clk-req and EP's clk-req together, which come from one ECN of PCI
> spec.
> 
> But most M.2 slot now, which support L1SS, so most platform default enable
> L1SS or default 'supports-clkreq' on.
> 
> Ideally, 'supports-clkreq' should use revert logic like 'clk-req-broken'.
> but 'supports-clkreq' already come into stardard PCIe binding now.
> 
> One of i.MX95 boards use standard PCIe slot, PIN 12
> 12	CLKREQ#	Ground	Clock Request Signal[26]
> which is reserved at old PCIe standard, so some old PCIe card float this
> pin.
> 

Ok. IIUC, i.MX platforms doesn't always support CLKREQ#, as the pin might not be
wired on some connectors. So if the driver turns off the override, CLKREQ# will
be driven high, but the endpoint wouldn't get a chance to drive it low and it
won't receive the refclk.

Is my understanding correct?

I'm wondering in those cases, why can't you keep the CLKREQ# pin to be in
active low state by defining the initial pinctrl state in DT? Can't you change
the pinctrl state of CLKREQ#?

> So I think most dts in kernel tree should add 'supports-clkreq' property
> if they use M.2 and connect CLK_REQ# as below [1]
> ============================================
>               VCC
>               ---
>                |
>                R (10K)
>                |
> CLK_REQ# (RC)------ CLK_REQ#(EP)
> 
> NOT add supports-clkreq if connect as below [2]
> ==========================================
> 
> CLK_REQ# (RC)  ---> |---------|
>                     | OR GATE | ---> control ref clock
> CLK_REQ#(EP)   ---> |-------- |
> 
> 
> >
> > So I'm wondering how you are suddenly using this property. The property implies
> > that when not set to true, CLKREQ# is not supported by the platform. So when the
> > driver starts using this property, all the old DTS based platforms are not going
> > to release CLKREQ# from driving low, so L1SS will not be entered for them. Do
> > you really want it to happen?
> 
> Actually, some old board use [2]. we will add supports-clkreq if board
> design use [1], so correct reflect board design.
> 

Ok, thanks for clarifying.

- Mani

-- 
மணிவண்ணன் சதாசிவம்