linux-kernel - Re: [PATCH v4 3/6] PCI: brcmstb: Add control of slot0 device 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]

Message-ID: <20210406172320.GO6443@sirena.org.uk>
Date:   Tue, 6 Apr 2021 18:23:21 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Jim Quinlan <jim2101024@...il.com>
Cc:     linux-pci <linux-pci@...r.kernel.org>,
        Nicolas Saenz Julienne <nsaenzjulienne@...e.de>,
        Rob Herring <robh@...nel.org>,
        bcm-kernel-feedback-list <bcm-kernel-feedback-list@...adcom.com>,
        Jim Quinlan <james.quinlan@...adcom.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "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 v4 3/6] PCI: brcmstb: Add control of slot0 device voltage
 regulators

On Tue, Apr 06, 2021 at 12:59:16PM -0400, Jim Quinlan wrote:
> On Tue, Apr 6, 2021 at 12:34 PM Mark Brown <broonie@...nel.org> wrote:
> > On Thu, Apr 01, 2021 at 05:21:43PM -0400, Jim Quinlan wrote:

> > This is broken, the driver knows which supplies are expected, the device
> > can't function without these supplies so the driver should just
> > unconditionally request them like any other supply.

> Some boards require the regulators, some do not.  So the driver is

No, some boards have the supplies described in firmware and some do not.

> only  sure what the names may be if they are present.  If  I put these
> names in my struct regulator_bulk_data array and do a
> devm_regulator_bulk_get(), I will get the following for the boards
> that do not need the regulators (e.g. the RPi SOC):
> 
> [    6.823820] brcm-pcie xxx.pcie: supply vpcie12v-supply not found,
> using dummy regulator
> [    6.832265] brcm-pcie xxx.pcie: supply vpcie3v3-supply not found,
> using dummy regulator

Sure, those are just warnings.

> IIRC you consider this a debug feature?  Be that as it may, these
> lines will confuse our customers and I'd like that they not be printed
> if possible.

You can stop the warnings by updating your firmware to more completely
describe the system - ideally all the supplies in the system would be
described for future proofing.  Or if this is a custom software stack
just delete whatever error checking and warnings you like.  The warnings
are there in case we've not got something mapped properly (eg, if there
were a typo in a property name) and things stop working, it's not great
to just ignore errors.

> So I ask you to allow the code as is.  If you still insist, I will
> change and resubmit.

Remove it, conditional code like this is just as bad in this driver as
it is in every other one.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)