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

Romain Gantois posted 2 patches 1 month, 1 week ago
[PATCH net-next 2/2] net: sfp: manage receiver and transmitter regulators
Posted by Romain Gantois 1 month, 1 week ago
If phandles to receiver and/or transmitter regulators for an SFP device are
found, enable them at probe time.

Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
 drivers/net/phy/sfp.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index f4bf53da3d4fd..602c166f60ddf 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -12,6 +12,7 @@
 #include <linux/of.h>
 #include <linux/phy.h>
 #include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
 #include <linux/rtnetlink.h>
 #include <linux/slab.h>
 #include <linux/workqueue.h>
@@ -3095,6 +3096,14 @@ static int sfp_probe(struct platform_device *pdev)
 	struct sfp *sfp;
 	int err, i;
 
+	err = devm_regulator_get_enable_optional(&pdev->dev, "vccr");
+	if (err && err != -ENODEV)
+		return err;
+
+	err = devm_regulator_get_enable_optional(&pdev->dev, "vcct");
+	if (err && err != -ENODEV)
+		return err;
+
 	sfp = sfp_alloc(&pdev->dev);
 	if (IS_ERR(sfp))
 		return PTR_ERR(sfp);

-- 
2.52.0
Re: [PATCH net-next 2/2] net: sfp: manage receiver and transmitter regulators
Posted by Mark Brown 1 month, 1 week ago
On Tue, Mar 03, 2026 at 02:54:27PM +0100, Romain Gantois wrote:
> If phandles to receiver and/or transmitter regulators for an SFP device are
> found, enable them at probe time.

The driver should unconditionally request whatever power the device
needs.
Re: [PATCH net-next 2/2] net: sfp: manage receiver and transmitter regulators
Posted by Russell King (Oracle) 1 month, 1 week ago
On Tue, Mar 03, 2026 at 02:22:40PM +0000, Mark Brown wrote:
> On Tue, Mar 03, 2026 at 02:54:27PM +0100, Romain Gantois wrote:
> > If phandles to receiver and/or transmitter regulators for an SFP device are
> > found, enable them at probe time.
> 
> The driver should unconditionally request whatever power the device
> needs.

... and then we break everyone, just like you broke SATA, and I've
never forgiven you for taking a principled line on this rather than a
pragmatic approach. You're making the same mistake here.

As we disagree, we're at a stalemate, the result of which will be a
NACK to this patch set.

Sorry.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next 2/2] net: sfp: manage receiver and transmitter regulators
Posted by Mark Brown 1 month, 1 week ago
On Tue, Mar 03, 2026 at 03:12:14PM +0000, Russell King (Oracle) wrote:
> On Tue, Mar 03, 2026 at 02:22:40PM +0000, Mark Brown wrote:
> > On Tue, Mar 03, 2026 at 02:54:27PM +0100, Romain Gantois wrote:

> > > If phandles to receiver and/or transmitter regulators for an SFP device are
> > > found, enable them at probe time.

> > The driver should unconditionally request whatever power the device
> > needs.

> ... and then we break everyone, just like you broke SATA, and I've
> never forgiven you for taking a principled line on this rather than a
> pragmatic approach. You're making the same mistake here.

Sorry, what's the breakage here?  The log messages, or something else?
Re: [PATCH net-next 2/2] net: sfp: manage receiver and transmitter regulators
Posted by Russell King (Oracle) 1 month, 1 week ago
On Tue, Mar 03, 2026 at 03:14:02PM +0000, Mark Brown wrote:
> On Tue, Mar 03, 2026 at 03:12:14PM +0000, Russell King (Oracle) wrote:
> > On Tue, Mar 03, 2026 at 02:22:40PM +0000, Mark Brown wrote:
> > > On Tue, Mar 03, 2026 at 02:54:27PM +0100, Romain Gantois wrote:
> 
> > > > If phandles to receiver and/or transmitter regulators for an SFP device are
> > > > found, enable them at probe time.
> 
> > > The driver should unconditionally request whatever power the device
> > > needs.
> 
> > ... and then we break everyone, just like you broke SATA, and I've
> > never forgiven you for taking a principled line on this rather than a
> > pragmatic approach. You're making the same mistake here.
> 
> 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.

Yet you demand that a SATA PHY supply must be provided, and so we're
stuck with:

[    1.207484] ahci f2540000.sata: supply ahci not found, using dummy regulator
[    1.213524] ahci f2540000.sata: supply phy not found, using dummy regulator
[    1.219630] platform f2540000.sata:sata-port@1: supply target not found, using dummy regulator
[    1.227800] ahci f4540000.sata: supply ahci not found, using dummy regulator
[    1.233757] ahci f4540000.sata: supply phy not found, using dummy regulator
[    1.239805] platform f4540000.sata:sata-port@0: supply target not found, using dummy regulator

on every boot with no way to shut them up. These supplies *ARE*
optional in terms of whether they can be described in DT.

As I've told you before, we have no information by which to describe
these supplies in DT on these platforms (not even with the data for
the chip) so you are effectively forcing people to "make stuff up" in
DT to shut up your warnings - blowing the whole idea that DT should
describe the hardware out of the water. How can we describe this
hardware if we don't have the internal design details of the chip?

The only way is to make something up, and all because you've decided
SATA supplies are no longer optional.

As SFP cages have not had to describe these supplies, requiring them
*now* will cause *regressions* because no one is going to be specifying
them, which will lead to stuff breaking. Again.

So, for the good of users of my SFP code, I will refuse any
introduction of regulators into my code for as long as you're take
the principled stance that regulators shall not be optional, rather
than applying a sensible pragmatic approach. You give me no option
here.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next 2/2] net: sfp: manage receiver and transmitter regulators
Posted by Mark Brown 1 month, 1 week ago
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.

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.
Re: [PATCH net-next 2/2] net: sfp: manage receiver and transmitter regulators
Posted by Andrew Lunn 1 month ago
> 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.

There is not really an SFP controller.

SFPs really break up into two parts, because they are
hot-pluggable. There is a cage, which is mounted on the board, and a
module which is inserted into the cage. The cage is passive.

https://en.wikipedia.org/wiki/Small_Form-factor_Pluggable gives a
reasonable overview.

The cage provides the module with power, 3.3v, max 300mA for Rx and
the same for TX. Something must supply the cage, and most designs just
connect the cage to the board power rails. The example give in
Multisource Agreement does exactly that, with some capacitors and
inducters to limit surge on hot plug.

This is the first board since 2017, when support for SFPs was added,
which can actually control the power supplies. We cannot make
regulators mandatory without breaking backwards compatibility.

So for me, the patch is good as it is now, the regulators are
optional.

   Andrew
Re: [PATCH net-next 2/2] net: sfp: manage receiver and transmitter regulators
Posted by Mark Brown 1 month ago
On Wed, Mar 04, 2026 at 10:38:54PM +0100, Andrew Lunn wrote:

> This is the first board since 2017, when support for SFPs was added,
> which can actually control the power supplies. We cannot make
> regulators mandatory without breaking backwards compatibility.

> So for me, the patch is good as it is now, the regulators are
> optional.

To be clear the actual effect of the regular API is that no regulator
has been specified by the firmware then we assume that there's something
we don't know about and substitute in a dummy regulator, otherwise
adding regulator support to anything would be excessively complex and
fragile.  We do currently warn when we do this since we're generally
very conservative about things like this as messing up power can crash
the system.

The issues Russell is seeing with AHCI are a combination of people just
bodging those warnings and the AHCI regulator bindings being very
confused, I'll follow up to his mail separately.

Optional regulators are for the case where the supply may be physically
absent and the driver needs to do something about it, for example the
original use case was for some of the MMC/SD specs where higher
performance modes add an additional supply which needs to be turned on
only after negotating with the device, if it's absent then the system
should handle this by not enabling those higher performance modes.

If a supply absolutely has to be there the driver should use a normal
regulator and write code on the basis that it will actually have power.
Re: [PATCH net-next 2/2] net: sfp: manage receiver and transmitter regulators
Posted by Russell King (Oracle) 1 month ago
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!
Re: [PATCH net-next 2/2] net: sfp: manage receiver and transmitter regulators
Posted by Mark Brown 1 month ago
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?
Re: [PATCH net-next 2/2] net: sfp: manage receiver and transmitter regulators
Posted by Romain Gantois 2 weeks, 6 days ago
Hello Mark,

On Friday, 6 March 2026 20:20:41 CET Mark Brown wrote:
> 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:
...
> 
> > 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?

It seems like my upstreaming strategy was incorrect. The series that I sent is 
a subset of my original use case, but in hindsight I should've just presented 
the original one in the first place, that would've been clearer, sorry about 
that.

Originally, I implemented a runtime PM support in the SFP core. This allowed 
to cut power to the cages when the attached network interface was down, 
thereby saving power. This is interesting since I'm dealing with a battery-
powered system which has SFP cages. However, an upstream version of this would 
require some kind of new userspace interface to signal indifference to module 
detection when the upstream network interface is down. Otherwise it could 
break existing userspace applications which expect to detect and interact with 
SFP modules (e.g. read EEPROMs, read temperature sensors) even when their 
upper network interfaces are down.

Aside from this, what Russell told me in this message:

https://lore.kernel.org/all/aacYGTBobbfJgZpp@shell.armlinux.org.uk/

suggests that cutting power to SFP cages could lead to unspecified behavior 
with some modules, so for example unloading the SFP core kernel module while 
an SFP module was inserted could have unintended consequences... This problem 
requires some more investigation on my side before I can submit a proper 
runtime PM solution.

Thanks,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH net-next 2/2] net: sfp: manage receiver and transmitter regulators
Posted by Romain Gantois 2 weeks, 6 days ago
Hello Mark,

On Friday, 6 March 2026 20:20:41 CET Mark Brown wrote:
> 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:
...
> 
> > 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?

It seems like my upstreaming strategy was incorrect. The series that I sent is 
a subset of my original use case, but in hindsight I should've just presented 
the original one in the first place, that would've been clearer, sorry about 
that.

Originally, I implemented a runtime PM support in the SFP core. This allowed 
to cut power to the cages when the attached network interface was down, 
thereby saving power. This is interesting since I'm dealing with a battery-
powered system which has SFP cages. However, an upstream version of this would 
require some kind of new userspace interface to signal indifference to module 
detection when the upstream network interface is down. Otherwise it could 
break existing userspace applications which expect to detect and interact with 
SFP modules (e.g. read EEPROMs, read temperature sensors) even when their 
upper network interfaces are down.

Aside from this, what Russell told me in this message:

https://lore.kernel.org/all/aacYGTBobbfJgZpp@shell.armlinux.org.uk/

suggests that cutting power to SFP cages could lead to unspecified behavior 
with some modules, so for example unloading the SFP core kernel module while 
an SFP module was inserted could have unintended consequences... This problem 
requires some more investigation on my side before I can submit a proper 
runtime PM solution.

Thanks,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH net-next 2/2] net: sfp: manage receiver and transmitter regulators
Posted by Russell King (Oracle) 2 weeks, 6 days ago
On Fri, Mar 20, 2026 at 10:39:10AM +0100, Romain Gantois wrote:
> Originally, I implemented a runtime PM support in the SFP core. This allowed 
> to cut power to the cages when the attached network interface was down, 
> thereby saving power.

Have you measured how much power is saved by this?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next 2/2] net: sfp: manage receiver and transmitter regulators
Posted by Romain Gantois 2 weeks, 1 day ago
On Friday, 20 March 2026 15:45:31 CET Russell King (Oracle) wrote:
> On Fri, Mar 20, 2026 at 10:39:10AM +0100, Romain Gantois wrote:
> > Originally, I implemented a runtime PM support in the SFP core. This
> > allowed to cut power to the cages when the attached network interface was
> > down, thereby saving power.
> 
> Have you measured how much power is saved by this?

Sure, here are a few measures of saved power with several different modules 
I've tested. The measures were done with a shunt resistor on the regulator 
line feeding all power pins on both SFP cages. Only one SFP cage was occupied 
during each measure and the upper network interface was down.

Module           | Power saved (uW)
SFP-10G-T-I    | 80
SFP-10G-T-30 | 359
SFP-GB-GE-T  | 191
CHAMPION ONE 1000SFPT | 188
SFP-H10GB-CU1M | 0
ES8512-3LCD05 | 248
SFP100BFXST | 351

So with both SFP cages occupied, we could see about 0.7mW of saved power by 
cutting the regulators to the cages.

Thanks,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH net-next 2/2] net: sfp: manage receiver and transmitter regulators
Posted by Romain Gantois 1 month, 1 week ago
Hello Mark,

On Tuesday, 3 March 2026 15:22:40 CET Mark Brown wrote:
> On Tue, Mar 03, 2026 at 02:54:27PM +0100, Romain Gantois wrote:
> > If phandles to receiver and/or transmitter regulators for an SFP device
> > are
> > found, enable them at probe time.
> 
> The driver should unconditionally request whatever power the device
> needs.

Ok, I'll use devm_regulator_get_enable() instead then.

Thanks,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH net-next 2/2] net: sfp: manage receiver and transmitter regulators
Posted by Russell King (Oracle) 1 month, 1 week ago
On Tue, Mar 03, 2026 at 03:40:54PM +0100, Romain Gantois wrote:
> Hello Mark,
> 
> On Tuesday, 3 March 2026 15:22:40 CET Mark Brown wrote:
> > On Tue, Mar 03, 2026 at 02:54:27PM +0100, Romain Gantois wrote:
> > > If phandles to receiver and/or transmitter regulators for an SFP device
> > > are
> > > found, enable them at probe time.
> > 
> > The driver should unconditionally request whatever power the device
> > needs.
> 
> Ok, I'll use devm_regulator_get_enable() instead then.

What Mark is suggesting will break existing users. So, I'm afraid we
can't proceed with this proposal. Sorry.

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