Re: [PATCH net-next 2/2] net: sfp: manage receiver and transmitter regulators

[Mark Brown]

On Tue, Mar 03, 2026 at 06:32:52PM +0000, Russell King (Oracle) wrote:
> On Tue, Mar 03, 2026 at 05:31:36PM +0000, Mark Brown wrote:

> > supplies?  The logs you posted looked like it was controllers requesting
> > the supplies which does look like the bindings and associated requests
> > aren't what I'd expect for something describing the hardware.

> The setup here as far as can be derived from the SoC documentation is:

> The AHCI SATA controller and COMPHY are integrated into the SoC.
> Internally, these components would be provided power by the internal
> SoC design.

> So, let's go through the supplies for the AHCI controller. The binding
> documentation states:

>   ahci-supply:
>     description: Power regulator for AHCI controller

> First, this is not listed as a required property. In this case, the
> AHCI controller is internal to the SoC, and no details are given in
> the documentation what that supply is, where it is derived from, or
> whether it's controllable.

This just shouldn't be there as a generic property for all AHCI
controllers, if there are controllers that have such a supply it should
be part of the controller specific bindings not forced on every single
controller.

>   target-supply:
>     description: Power regulator for SATA target device

> The board does not provide power to the target device, that is derived
> off-board and completely up to the user. Thus, this can't be specified.

Right, and also even within the board this is a property of the target
device not the controller.  Arguably you could say that your separate
ATX supply ends up being part of the same system for practical purposes
but it's a stretch.

>   phy-supply:
>     description: Power regulator for SATA PHY

> The PHY is an entirely separate device (the COMPHY) from the AHCI
> controller. Thus, specifying a PHY supply for this AHCI hardware is
> not relevant at the AHCI controller layer. Sure, other AHCI designs
> where the PHY is not generic may require a separate PHY supply which
> may be controllable, but here is one example where this just isn't
> relevant.

Yeah, same here.  This is a property of the PHY, and possibly of a
specific PHY.

> So, as a result of commit 962399bb7fbf ("ata: libahci_platform: Fix
> regulator_get_optional() misuse") we have now ended up with kernel
> boots issuing messages at warning severity (which means something is
> wrong and requires attention) for supplies that are not relevant to
> this SoC hardware design.

The underlying issue here being that the generic AHCI code's bindings
and hence regulator usage just don't reflect general AHCI hardware at
all, that's then causing knock on effects since as you say there's no
way for systems to provide sensible information corresponding to what
the drivers are asking for.  

> We're now left with the annoying warning messages that - as I've
> stated above - are certainly in one case just not relevant to the
> SoC design.

I agree that we should do something here, I was working on the basis
that the bindings reflected the hardware but that seems rather far from
reality.  I'm wondering if we should have an API that lets drivers say
they're requesting a regulator for a "legacy" binding, that way we can
keep all the checking of usage and avoid fragility with mishandling of
errors but avoid complaining at users.

> > For SFP my understanding is that SFP has a physical specification which
> > includes power inputs and that these supplies are being requested by the
> > devices that consume them.  If some part of that is not the case then it
> > sounds like the bindings aren't describing the hardware (or at least are
> > a bit unclear about how they're doing so) and should be revised.  The
> > series doesn't seem to do anything at all with the supply side either,
> > I'm guessing there are some SFP controllers with integrated power
> > provisioning.

> First, please scrap the idea of "SFP controller". There's no such
> device. There's a SFP cage, which is a physical connector and a
> specification of the signals on the pins. How those signals are
> provided, or even whether they are provided is outside the scope
> of the "specification".

I see.  That's sounding like rather than having something that applies
to all SFP devices anything we do end up having should be something that
represents a SFP cage specifically, that's the thing that has the
physical specification with a known set of supplies.  With soldered down
stuff I suspect there's enough room for complication that it should be
dealt with on a device by device basis if needed.

> Given (1), it is unwise for a board with a cage to present two
> controllable supplies, as turning one supply off can result in
> power being back-fed via a module tying both together. Thus,
> specifying separate supplies could be dangerous.

I guess even if they're not controlled separately it's possible that
someone might decide that having two LDOs is part of their filtering
strategy...  It is generally easier to cope with the situation where one
supply does two consumers that are controlled together than it is to
deal with the situation where two supplies that are normally the same
are delivered by two separate regulators.  It all feels very edge casey
thugh.

> Now, if you're going to say "ah, so it has power pins, you need to
> describe them using a regulator" then I would say to you, when are
> we introducing regulators for every device we describe in DT such
> as LEDs, switches, GPIO pins, RAM, etc? Every device needs to have a
> source of power after all.

It sounds from the cover letter for this series like there's some demand
for power control of SFP cages, the cover letter isn't terribly specific
about what circumstances though.  Possibly there's some UI for this on
the system, or the hardware has some mechanism for detecting physical
insertion to the SFP cage (hopefully well in advance of the electrical
contacts being made)?  Romain, are you able to share more specifics on
the use case here?

Attachment: signature.asc
Description: PGP signature