Re: [PATCH v4 3/6] PCI: brcmstb: Add control of slot0 device voltage regulators

From: Mark Brown
Date: Tue Apr 06 2021 - 13:23:45 EST


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@xxxxxxxxxx> 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.

Attachment: signature.asc
Description: PGP signature