Hi Frank: I'm very appreciated that you kindly help to add the explanations. Best Regards Richard Zhu > -----Original Message----- > From: Frank Li <frank.li@xxxxxxx> > Sent: 2025年9月9日 0:33 > To: Manivannan Sadhasivam <mani@xxxxxxxxxx> > Cc: Hongxing Zhu <hongxing.zhu@xxxxxxx>; l.stach@xxxxxxxxxxxxxx; > lpieralisi@xxxxxxxxxx; kwilczynski@xxxxxxxxxx; robh@xxxxxxxxxx; > bhelgaas@xxxxxxxxxx; shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; > kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; imx@xxxxxxxxxxxxxxx; > linux-kernel@xxxxxxxxxxxxxxx > Subject: Re: [PATCH v1 2/2] PCI: imx6: Add a method to handle CLKREQ# > override active low > > On Mon, Sep 08, 2025 at 09:19:40PM +0530, Manivannan Sadhasivam wrote: > > 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 > > CLKREQ# will be float and pull up by pull up resistor. The old endpoint (PCIe > standard slot) float this pin also because it is reversed at old PCIe standard. > So ref clock is off at that case. > > > won't receive the refclk. > > > > Is my understanding correct? > > Basic is correct with some small problem. > > It is caused by two common PCIe problem. > - stadarnd PCIe slot define change PIN12 from reserved to CLKREQ#, which is > ECN, before ECN, all EP device float this pin. after ECN, EP device pull this > pin down. > - use logic [2] to design boards, which just impact L1SS, the basic function > should work. > > Frank > > > > 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 > > > > -- > > மணிவண்ணன் சதாசிவம்
