linux-kernel - RE: [RESEND PATCH v7 2/2] PCI: xilinx-cpm: Add support for PCIe RP PERST# signal

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] [day] [month] [year] [list]
Message-ID:
 <DM4PR12MB6158A154D1186EFDBFD92BC1CD73A@DM4PR12MB6158.namprd12.prod.outlook.com>
Date: Tue, 17 Jun 2025 15:49:41 +0000
From: "Musham, Sai Krishna" <sai.krishna.musham@....com>
To: Bjorn Helgaas <helgaas@...nel.org>
CC: Manivannan Sadhasivam <mani@...nel.org>, "bhelgaas@...gle.com"
	<bhelgaas@...gle.com>, "lpieralisi@...nel.org" <lpieralisi@...nel.org>,
	"kw@...ux.com" <kw@...ux.com>, "robh@...nel.org" <robh@...nel.org>,
	"krzk+dt@...nel.org" <krzk+dt@...nel.org>, "conor+dt@...nel.org"
	<conor+dt@...nel.org>, "cassel@...nel.org" <cassel@...nel.org>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "Simek,
 Michal" <michal.simek@....com>, "Gogada, Bharat Kumar"
	<bharat.kumar.gogada@....com>, "Havalige, Thippeswamy"
	<thippeswamy.havalige@....com>
Subject: RE: [RESEND PATCH v7 2/2] PCI: xilinx-cpm: Add support for PCIe RP
 PERST# signal

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Bjorn,

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@...nel.org>
> Sent: Tuesday, June 17, 2025 8:17 PM
> To: Musham, Sai Krishna <sai.krishna.musham@....com>
> Cc: Manivannan Sadhasivam <mani@...nel.org>; bhelgaas@...gle.com;
> lpieralisi@...nel.org; kw@...ux.com; manivannan.sadhasivam@...aro.org;
> robh@...nel.org; krzk+dt@...nel.org; conor+dt@...nel.org; cassel@...nel.org;
> linux-pci@...r.kernel.org; devicetree@...r.kernel.org; linux-
> kernel@...r.kernel.org; Simek, Michal <michal.simek@....com>; Gogada, Bharat
> Kumar <bharat.kumar.gogada@....com>; Havalige, Thippeswamy
> <thippeswamy.havalige@....com>
> Subject: Re: [RESEND PATCH v7 2/2] PCI: xilinx-cpm: Add support for PCIe RP
> PERST# signal
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Tue, Jun 17, 2025 at 04:14:37AM +0000, Musham, Sai Krishna wrote:
> > [AMD Official Use Only - AMD Internal Distribution Only]
> >
> > Hi Manivannan,
> >
> > > -----Original Message-----
> > > From: Manivannan Sadhasivam <mani@...nel.org>
> > > Sent: Thursday, June 12, 2025 10:49 PM
> > > To: Musham, Sai Krishna <sai.krishna.musham@....com>
> > > Cc: bhelgaas@...gle.com; lpieralisi@...nel.org; kw@...ux.com;
> > > manivannan.sadhasivam@...aro.org; robh@...nel.org; krzk+dt@...nel.org;
> > > conor+dt@...nel.org; cassel@...nel.org; linux-pci@...r.kernel.org;
> > > devicetree@...r.kernel.org; linux-kernel@...r.kernel.org; Simek, Michal
> > > <michal.simek@....com>; Gogada, Bharat Kumar
> > > <bharat.kumar.gogada@....com>; Havalige, Thippeswamy
> > > <thippeswamy.havalige@....com>
> > > Subject: Re: [RESEND PATCH v7 2/2] PCI: xilinx-cpm: Add support for PCIe RP
> > > PERST# signal
> > >
> > > Caution: This message originated from an External Source. Use proper caution
> > > when opening attachments, clicking links, or responding.
> > >
> > >
> > > On Mon, Apr 14, 2025 at 08:53:04AM +0530, Sai Krishna Musham wrote:
> > > > Add support for handling the PCIe Root Port (RP) PERST# signal using
> > > > the GPIO framework, along with the PCIe IP reset. This reset is
> > > > managed by the driver and occurs after the Initial Power Up sequence
> > > > (PCIe CEM r6.0, 2.2.1) is handled in hardware before the driver's probe
> > > > function is called.
>
> > > > +     if (do_reset) {
> > > > +             /* Assert the PCIe IP reset */
> > > > +             writel_relaxed(0x1, port->crx_base + variant->cpm_pcie_rst);
> > > > +
> > > > +             /*
> > > > +              * "PERST# active time", as per Table 2-10: Power Sequencing
> > > > +              * and Reset Signal Timings of the PCIe Electromechanical
> > > > +              * Specification, Revision 6.0, symbol "T_PERST".
> > > > +              */
> > > > +             udelay(100);
> > >
> > > Are you sure that you need T_PERST here and not T_PVPERL? T_PERST
> > > is only valid while resuming from D3Cold i.e., after power up,
> > > while T_PVPERL is valid during the power up, which is usually the
> > > case when a controller driver probes. Is your driver relying on
> > > power being enabled by the bootloader and the driver just toggling
> > > PERST# to perform conventional reset of the endpoint?
> >
> > Thanks for pointing that out. Yes, the power-up sequence is handled
> > by the hardware, and the driver relies on power being enabled by it.
> > We're only toggling the PERST# signal in the driver to perform a
> > conventional reset of the endpoint. So, I'm confident that T_PERST
> > is the appropriate timing reference here, not T_PVPERL.
> >
> > Additionally, this delay was recommended by our hardware team, who
> > confirmed that the power-up sequence is managed in hardware logic,
> > and that T_PERST is the appropriate timing to apply in this context.
> >
> > I also checked pci.h but couldn't find a predefined macro for
> > T_PERST, so I used 100.  Please let me know if there's a preferred
> > macro I should be using instead.
>
> If we need a new macro, please add it.  Include a citation to the
> relevant section of the spec ("PCIe CEM r6.0, sec 2.11.2"; table
> numbers don't appear in the table of contents so they're hard to
> find), and include the units ("_US", I guess) in the macro name.
>
> Given a comment at the macro definition, you don't need to repeat it
> at all the uses.
>

Thanks for the review, sure I will add new macro and include a citation
to the relevant section of the PCIe spec.

> Bjorn

Thanks,
Sai Krishna