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

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Tue, Mar 03, 2026 at 05:31:36PM +0000, Mark Brown wrote:
> On Tue, Mar 03, 2026 at 03:25:35PM +0000, Russell King (Oracle) wrote:
> > On Tue, Mar 03, 2026 at 03:14:02PM +0000, Mark Brown wrote:
> 
> > > Sorry, what's the breakage here?  The log messages, or something else?
> 
> > ... which then caused someone to "fix" DT by disabling devices to shut
> > up those log messages, including for platforms where those devices were
> > being used, which ultimately caused a boot failure.
> 
> > ... and your argument that SATA PHYs need these supplies, which is false
> > when the SATA PHY is integrated into the SoC and there's no details on
> > what those supplies are or where they come from, or even if they are
> > controllable.
> 
> The supplies don't need to be controllable or have any other information
> to be specified, it sounds like this hardware has a fixed voltage
> regulator that's supplying the PHY which is representable without any
> changes, though I do agree it's annoying.
> 
> Though having said that with your description above I'm really not clear
> that the regulator support is in the right place in the SATA framework
> at all, it sounds like the supplies are being requested by the SATA
> controller but the expectation is that the SATA controller is the thing
> that is supplying power rather than consuming it.  I think that's where
> things are going wrong here?  There are some SATA implementations that
> don't include the power delivery part of SATA and only those require the
> 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:


----------------------------------------------------------+ External
 SoC +--------------------------------+                   | World
     |             .---- SATA port 0 --- COMPHY SerDes <-----> port 2
<-bus-> AHCI SATA controller          |                   |
     |             ^---- SATA port 1 --- COMPHY SerDes <-----> port 1
     +--------------------------------+                   |
----------------------------------------------------------+

The COMPHY SerDes is a multi-protocol SerDes, which supports PCIe, SATA,
and Ethernet, and is driven optionally by firmware via the generic PHY
driver. This is entirely separate from the SATA ports in the AHCI
controller. The external world are the two pairs of data pin
connections to the SATA drive per port. Nothing more - no power pins.

Drive power is up to the user. In my case, I've plugged in a 12V ATX
format power module that provides the +12V and +5V through normal PC
style wiring to the drives - none of that has any business being
described in DT because it's out of scope of the board itself.

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.

  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.

  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.

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 side effect of that commit was commit 30023876aef4 ("arm64: dts:
marvell: only enable complete sata nodes") which disabled the ports
to shut up these warning messages, but the hunk for
arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi was incomplete,
only enabling one SATA port, despite the board having three SATA
ports. This caused boot failure as SATA drives that were previously
accessible became inaccessible due to the SATA ports missing until
I fixed that in 2aeadea47f5b ("ARM64: dts: mcbin: fix SATA ports on
Macchiatobin").

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.

> 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".

This includes the high-speed SerDes pairs for transmit and receive
data. There are also low-speed pins - I2C and state pins. Finally,
there are power pins. State pins end up being connected to GPIOs, or
appropriately tied to their inactive levels or left open depending
whether they are an input to the module or output.

There is no specification that power pins will be controllable.
The only requirement is the voltage on the pins and the stability
of that voltage.

SFP modules have two sets of power pins, one set for the transmitter
and one set for the receiver.

1. Modules are allowed to tie these two sets of power pins together.
2. The recommendation in the SFF documentation is that these are
   derived from a common supply via filtering, and that common supply
   is also used for the signalling pin pull-ups.
3. It is unspecified which of these supplies is used for the "low-
   speed" signalling interfaces.

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.

Removing power from a module while plugged in, while keeping the
low-speed signals at logic high levels may result in damage due to
backfeeding power through the ESD protection diodes in the silicon
devices. If the I2C bus is shared, then powering down a module may
corrupt communication to other devices on that I2C bus.

So, given all that, every design I've seen so far connects the SFP
cage power pins to the board's 3.3V supply that is active when the
board is powered.

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.

- A LED may be wired between a GPIO and a supply.
- A switch that pulls a GPIO up to logic 1 state needs to be connected
  to some sort of supply to do that.
- A GPIO controller requires a source of power in order to signal a
  logic high.

We tend not to describe those because they aren't relevant to the
OS, because they tend to be "always on" supplies that the OS can't
control, and have no effect at all on the kernel itself. DT is not
about translating every single wire and component on a schematic for
a board into a firmware description.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!