linux-kernel - Re: [PATCH v1 3/4] PCI: brcmstb: Add control of subdevice voltage regulators

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]

Date:   Fri, 8 Jul 2022 14:09:11 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Jim Quinlan <james.quinlan@...adcom.com>
Cc:     Jim Quinlan <jim2101024@...il.com>,
        "open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS" 
        <linux-pci@...r.kernel.org>,
        Nicolas Saenz Julienne <nsaenz@...nel.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Cyril Brulebois <kibi@...ian.org>,
        "maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE" 
        <bcm-kernel-feedback-list@...adcom.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Lorenzo Pieralisi <lpieralisi@...nel.org>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Wilczyński <kw@...ux.com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" 
        <linux-rpi-kernel@...ts.infradead.org>,
        "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" 
        <linux-arm-kernel@...ts.infradead.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 3/4] PCI: brcmstb: Add control of subdevice voltage
 regulators

On Fri, Jul 08, 2022 at 11:16:08AM -0400, Jim Quinlan wrote:
> On Wed, Jul 6, 2022 at 7:13 PM Bjorn Helgaas <helgaas@...nel.org> wrote:
> > On Fri, Jul 01, 2022 at 12:27:24PM -0400, Jim Quinlan wrote:
> > > This Broadcom STB PCIe RC driver has one port and connects directly to one
> > > device, be it a switch or an endpoint.  We want to be able to leverage the
> > > recently added mechanism that allocates and turns on/off subdevice
> > > regulators.
> > > ...

> > > +      * If we have failed linkup there is no point to return an error as
> > > +      * currently it will cause a WARNING() from pci_alloc_child_bus().
> > > +      * We return 0 and turn on the "refusal_mode" so that any further
> > > +      * accesses to the pci_dev just get 0xffffffff
> > > +      */
> > > +     if (brcm_pcie_linkup(pcie) != 0)
> > > +             pcie->refusal_mode = true;
> >
> > Is there a bisection hole between the previous patch and this one?
> > The previous patch sets .add_bus() to pci_subdev_regulators_add_bus(),
> > so we'll turn on the regulators, but we don't know whether the link
> > came up.  If it didn't come up, it looks like we might get a CPU abort
> > when enumerating?
> 
> I don't think so.  At this commit, attempting the link-up is still
> done before the call
> to pci_host_probe().  Since there is no power there will be no link,
> the probe will
> fail and pci_host_probe() will never be invoked.

Ah, OK, thanks for the explanation.

> > I think we should split out the refusal_mode patch:
> >
> >   - Add refusal mode
> >   - Add subdev regulator mechanism
> >   - This patch (which would then be clearly about managing regulators
> >     in suspend/resume, IIUC)
> 
> Will do.

If it's not too hard to do, I think splitting it will make the patches
easier to read.

Bjorn