Thread (23 messages) 23 messages, 5 authors, 2026-03-25

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

From: Mark Brown <broonie@kernel.org>
Date: 2026-03-06 19:20:46
Also in: linux-devicetree, lkml

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:
quoted
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.
quoted
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?

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help