[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
<IA1PR12MB614051F6617C327B6306CC1CCD53A@IA1PR12MB6140.namprd12.prod.outlook.com>
Date: Sat, 19 Jul 2025 02:59:41 +0000
From: "Musham, Sai Krishna" <sai.krishna.musham@....com>
To: Bjorn Helgaas <helgaas@...nel.org>
CC: "bhelgaas@...gle.com" <bhelgaas@...gle.com>, "lpieralisi@...nel.org"
<lpieralisi@...nel.org>, "kw@...ux.com" <kw@...ux.com>, "mani@...nel.org"
<mani@...nel.org>, "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: [PATCH v5 2/2] PCI: amd-mdb: Add support for PCIe RP PERST#
signal handling
[AMD Official Use Only - AMD Internal Distribution Only]
Hi Bjorn,
> -----Original Message-----
> From: Bjorn Helgaas <helgaas@...nel.org>
> Sent: Friday, July 18, 2025 10:02 PM
> To: Musham, Sai Krishna <sai.krishna.musham@....com>
> Cc: bhelgaas@...gle.com; lpieralisi@...nel.org; kw@...ux.com; mani@...nel.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: [PATCH v5 2/2] PCI: amd-mdb: Add support for PCIe RP PERST#
> signal handling
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Fri, Jul 18, 2025 at 04:30:32AM +0000, Musham, Sai Krishna wrote:
> > [AMD Official Use Only - AMD Internal Distribution Only]
> >
> > Hi Bjorn,
> >
> > > -----Original Message-----
> > > From: Bjorn Helgaas <helgaas@...nel.org>
> > > Sent: Saturday, July 12, 2025 4:49 AM
> > > To: Musham, Sai Krishna <sai.krishna.musham@....com>
> > > Cc: bhelgaas@...gle.com; lpieralisi@...nel.org; kw@...ux.com;
> mani@...nel.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: [PATCH v5 2/2] PCI: amd-mdb: Add support for PCIe RP PERST#
> > > signal handling
> > >
> > > Caution: This message originated from an External Source. Use proper caution
> > > when opening attachments, clicking links, or responding.
> > >
> > >
> > > On Fri, Jul 11, 2025 at 10:53:57AM +0530, Sai Krishna Musham wrote:
> > > > Add support for handling the AMD Versal Gen 2 MDB PCIe Root Port PERST#
> > > > signal via a GPIO by parsing the new PCIe bridge node to acquire the
> > > > reset GPIO. If the bridge node is not found, fall back to acquiring it
> > > > from the PCIe node.
> > > >
> > > > As part of this, update the interrupt controller node parsing to use
> > > > of_get_child_by_name() instead of of_get_next_child(), since the PCIe
> > > > node now has multiple children. This ensures the correct node is
> > > > selected during initialization.
>
> > > > + * If amd_mdb_parse_pcie_port returns -ENODEV, it indicates that the
> > > > + * PCIe Bridge node was not found in the device tree. This is not
> > > > + * considered a fatal error and will trigger a fallback where the
> > > > + * reset GPIO is acquired directly from the PCIe node.
> > > > + */
> > > > + if (ret && ret != -ENODEV) {
> > > > + return ret;
> > > > + } else if (ret == -ENODEV) {
> > >
> > > The "ret" checking seems unnecessarily complicated.
> > >
> > > > + dev_info(dev, "Falling back to acquire reset GPIO from PCIe
> node\n");
> > >
> > > I don't think this is worthy of a message. If there are DTs in the
> > > field that were valid once, they continue to be valid forever, and
> > > there's no point in complaining about them.
> > >
> > > https://lore.kernel.org/all/20250702-perst-v5-2-
> 920b3d1f6ee1@....qualcomm.com/
> > > has a good example of how to this fallback nicely.
> > >
> > > Otherwise looks good to me.
> >
> > Thanks for the feedback. I've removed the fallback message and simplified the
> "ret"
> > checking. Could you please confirm if this looks good for v6?
> >
> > if (ret == -ENODEV) {
> >
> > /* Request the GPIO for PCIe reset signal and assert */
> > pcie->perst_gpio = devm_gpiod_get_optional(dev, "reset",
> > GPIOD_OUT_HIGH);
> > if (IS_ERR(pcie->perst_gpio))
> > return dev_err_probe(dev, PTR_ERR(pcie->perst_gpio),
> > "Failed to request reset GPIO\n");
> > } else if (ret) {
> > return ret;
> > }
>
> Looks good to me. It's important to note that this -ENODEV fallback
> uses the PERST# GPIO described in the host bridge, not in a Root Port,
> but I think your comment above includes this.
>
Thanks for the confirmation. Yes, you're correct - the fallback uses the
PERST# GPIO described in the host bridge.
I will submit v6 with the simplified error handling.
> Bjorn
Regards,
Sai Krishna
Powered by blists - more mailing lists