| 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
| ||
|
Message-ID:
<AS8PR04MB88333851444D5A10A4A8F6668C32A@AS8PR04MB8833.eurprd04.prod.outlook.com>
Date: Thu, 21 Aug 2025 06:28:30 +0000
From: Hongxing Zhu <hongxing.zhu@....com>
To: Bjorn Helgaas <helgaas@...nel.org>, Frank Li <frank.li@....com>
CC: "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 v1 2/2] PCI: imx6: Add a method to handle CLKREQ# override
active low
> -----Original Message-----
> From: Bjorn Helgaas <helgaas@...nel.org>
> Sent: 2025年8月21日 5:40
> To: Frank Li <frank.li@....com>
> Cc: Hongxing Zhu <hongxing.zhu@....com>; l.stach@...gutronix.de;
> lpieralisi@...nel.org; kwilczynski@...nel.org; mani@...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 Wed, Aug 20, 2025 at 05:02:28PM -0400, Frank Li wrote:
> > On Wed, Aug 20, 2025 at 03:35:36PM -0500, Bjorn Helgaas wrote:
> > > On Wed, Aug 20, 2025 at 04:10:48PM +0800, 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.
> > > > 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.
>
> What is the effect of refclk not being available when no card is connected?
> I guess something is wrong with the i.MX PCIe host? Maybe register
> accesses don't work? Something else?
If the refclk is not available, but mandatory required by RC host when no
card is connected to driven active low. There would be PCIe RC driver probe
failure because of the pll lock error return or the failure of PHY power on.
>
> How would a user know that something is wrong when the slot is empty?
> Presumably this patch fixes whatever is wrong in that case.
The error message would be dumped to inform users.
Best Regards
Richard Zhu
>
> > > > 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.
> > > >
> > > > Signed-off-by: Richard Zhu <hongxing.zhu@....com>
> > > > ---
> > > > drivers/pci/controller/dwc/pci-imx6.c | 35
> > > > +++++++++++++++++++++++++++
> > > > 1 file changed, 35 insertions(+)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > > > b/drivers/pci/controller/dwc/pci-imx6.c
> > > > index 80e48746bbaf6..a73632b47e2d3 100644
> > > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > > @@ -52,6 +52,8 @@
> > > > #define IMX95_PCIE_REF_CLKEN BIT(23)
> > > > #define IMX95_PCIE_PHY_CR_PARA_SEL BIT(9)
> > > > #define IMX95_PCIE_SS_RW_REG_1 0xf4
> > > > +#define IMX95_PCIE_CLKREQ_OVERRIDE_EN BIT(8)
> > > > +#define IMX95_PCIE_CLKREQ_OVERRIDE_VAL BIT(9)
> > > > #define IMX95_PCIE_SYS_AUX_PWR_DET BIT(31)
> > > >
> > > > #define IMX95_PE0_GEN_CTRL_1 0x1050
> > > > @@ -136,6 +138,7 @@ struct imx_pcie_drvdata {
> > > > int (*enable_ref_clk)(struct imx_pcie *pcie, bool enable);
> > > > int (*core_reset)(struct imx_pcie *pcie, bool assert);
> > > > int (*wait_pll_lock)(struct imx_pcie *pcie);
> > > > + void (*clr_clkreq_override)(struct imx_pcie *pcie);
> > > > const struct dw_pcie_host_ops *ops; };
> > > >
> > > > @@ -149,6 +152,7 @@ struct imx_pcie {
> > > > struct gpio_desc *reset_gpiod;
> > > > struct clk_bulk_data *clks;
> > > > int num_clks;
> > > > + bool supports_clkreq;
> > > > struct regmap *iomuxc_gpr;
> > > > u16 msi_ctrl;
> > > > u32 controller_id;
> > > > @@ -267,6 +271,13 @@ static int imx95_pcie_init_phy(struct imx_pcie
> *imx_pcie)
> > > > IMX95_PCIE_REF_CLKEN,
> > > > IMX95_PCIE_REF_CLKEN);
> > > >
> > > > + /* Force CLKREQ# low by override */
> > > > + regmap_update_bits(imx_pcie->iomuxc_gpr,
> > > > + IMX95_PCIE_SS_RW_REG_1,
> > > > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > > > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL,
> > > > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > > > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL);
> > > > return 0;
> > > > }
> > > >
> > > > @@ -1298,6 +1309,18 @@ static void imx_pcie_host_exit(struct
> dw_pcie_rp *pp)
> > > > regulator_disable(imx_pcie->vpcie);
> > > > }
> > > >
> > > > +static void imx8mm_pcie_clr_clkreq_override(struct imx_pcie
> > > > +*imx_pcie) {
> > > > + imx8mm_pcie_enable_ref_clk(imx_pcie, false); }
> > > > +
> > > > +static void imx95_pcie_clr_clkreq_override(struct imx_pcie
> > > > +*imx_pcie) {
> > > > + regmap_update_bits(imx_pcie->iomuxc_gpr,
> IMX95_PCIE_SS_RW_REG_1,
> > > > + IMX95_PCIE_CLKREQ_OVERRIDE_EN |
> > > > + IMX95_PCIE_CLKREQ_OVERRIDE_VAL, 0); }
> > > > +
> > > > static void imx_pcie_host_post_init(struct dw_pcie_rp *pp) {
> > > > struct dw_pcie *pci = to_dw_pcie_from_pp(pp); @@ -1322,6
> > > > +1345,12 @@ static void imx_pcie_host_post_init(struct dw_pcie_rp
> *pp)
> > > > dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
> > > > dw_pcie_dbi_ro_wr_dis(pci);
> > > > }
> > > > +
> > > > + /* Clear CLKREQ# override if supports_clkreq is true and link is up */
> > > > + if (dw_pcie_link_up(pci) && imx_pcie->supports_clkreq) {
> > > > + if (imx_pcie->drvdata->clr_clkreq_override)
> > > > + imx_pcie->drvdata->clr_clkreq_override(imx_pcie);
> > >
> > > It seems racy to clear the override when the link is up.
> > >
> > > Since it sounds like the i.MX host requires refclock all the time,
> > > why not keep the override permanently?
> >
> > It will Lost l1ss support if enable override permanetly. I think other
> > platform have similar issues (at least dwc controller). Most platform
> > use
> > m.2 socket, which pull down clk_req by EP side devices.
> >
> > Only standard PCIe slots, which clkreq have not connect by EP devices
> > because stardard PCIe slots add #clkreq signal happen recent years
> > (maybe after miniPCIe slot spec). Some old PCIe devices have not connect
> it.
> >
> > Even latest PCIe devices, I saw have jumper in PCIe card to controller
> > #clkreq.
> >
> > > Obviously it must be ok to keep the override for a while, because
> > > there is some interval between the link coming up and the call of
> > > .clr_clkreq_override().
> > >
> > > Would something bad happen if we *never* called
> > > .clr_clkreq_override()?
> >
> > clock will always running, lost L1SS power save feature.
>
> Thanks! Let's include a little of this back story in the next rev of the
> commit log and the comment near the .clr_clkreq_override() call.
>
> Bjorn
Powered by blists - more mailing lists