.../devicetree/bindings/hwmon/ti,tps23861.yaml | 93 +++++++- Documentation/hwmon/tps23861.rst | 6 +- drivers/hwmon/tps23861.c | 249 ++++++++++++++++++++- 3 files changed, 341 insertions(+), 7 deletions(-)
This patch series introduces per-port device tree configuration with poe
class restrictions. Also adds optional reset/shutdown gpios.
Tested with hw poe tester:
- Auto mode tested with no per-port DT settings as well as explicit port
DT ti,class=4. Tested that no IRQ is required in this case.
- Semi-Auto mode with class restricted to 0, 1, 2 or 3. IRQ required.
- Tested current cut-offs in Semi-Auto mode.
- On/off by default setting tested for both Auto and Semi-Auto modes.
- Tested fully disabling the ports in DT.
- Tested with both reset and ti,ports-shutdown gpios defined, as well as
with reset only, as well as with neither reset nor shutdown.
Signed-off-by: Gregory Fuchedgi <gfuchedgi@gmail.com>
---
Changes in v3:
- cleaned up dt bindings in response to v2 review
- Link to v2: https://lore.kernel.org/r/20250811-hwmon-tps23861-add-class-restrictions-v2-0-ebd122ec5e3b@gmail.com
Changes in v2:
- code cleanup
- split bindings into separate patch
- use patternProperties
- use labels instead of DT node names
- add few comments for clarity
---
Gregory Fuchedgi (2):
dt-bindings: hwmon: update TI TPS23861 with per-port schema
hwmon: (tps23861) add class restrictions and semi-auto mode support
.../devicetree/bindings/hwmon/ti,tps23861.yaml | 93 +++++++-
Documentation/hwmon/tps23861.rst | 6 +-
drivers/hwmon/tps23861.c | 249 ++++++++++++++++++++-
3 files changed, 341 insertions(+), 7 deletions(-)
---
base-commit: 3db46a82d467bd23d9ebc473d872a865785299d8
change-id: 20250808-hwmon-tps23861-add-class-restrictions-83ce3c02d885
Best regards,
--
Gregory Fuchedgi <gfuchedgi@gmail.com>
+Cc: pse-pd maintainers and netdev mailing list On 9/4/25 10:33, Gregory Fuchedgi via B4 Relay wrote: > This patch series introduces per-port device tree configuration with poe > class restrictions. Also adds optional reset/shutdown gpios. > > Tested with hw poe tester: > - Auto mode tested with no per-port DT settings as well as explicit port > DT ti,class=4. Tested that no IRQ is required in this case. > - Semi-Auto mode with class restricted to 0, 1, 2 or 3. IRQ required. > - Tested current cut-offs in Semi-Auto mode. > - On/off by default setting tested for both Auto and Semi-Auto modes. > - Tested fully disabling the ports in DT. > - Tested with both reset and ti,ports-shutdown gpios defined, as well as > with reset only, as well as with neither reset nor shutdown. > > Signed-off-by: Gregory Fuchedgi <gfuchedgi@gmail.com> This entire series makes me more and more unhappy. It is not the responsibility of the hardware monitoring subsystem to control power. The hardware monitoring subsystem is for monitoring, not for control. Please consider adding a driver for this chip to the pse-pd subsystem (drivers/net/pse-pd). As it turns out, that subsystem already supports tps23881. This is a similar chip which even has a similar register set. This driver could then be modified to be an auxiliary driver of that driver. Alternatively, we could drop this driver entirely since the pse-pd subsystem registers the chips it supports as regulator which has its own means to handle telemetry. Thanks, Guenter
On Sun, Sep 07, 2025 at 09:06:25AM -0700, Guenter Roeck wrote: > +Cc: pse-pd maintainers and netdev mailing list > > On 9/4/25 10:33, Gregory Fuchedgi via B4 Relay wrote: > > This patch series introduces per-port device tree configuration with poe > > class restrictions. Also adds optional reset/shutdown gpios. > > > > Tested with hw poe tester: > > - Auto mode tested with no per-port DT settings as well as explicit port > > DT ti,class=4. Tested that no IRQ is required in this case. > > - Semi-Auto mode with class restricted to 0, 1, 2 or 3. IRQ required. > > - Tested current cut-offs in Semi-Auto mode. > > - On/off by default setting tested for both Auto and Semi-Auto modes. > > - Tested fully disabling the ports in DT. > > - Tested with both reset and ti,ports-shutdown gpios defined, as well as > > with reset only, as well as with neither reset nor shutdown. > > > > Signed-off-by: Gregory Fuchedgi <gfuchedgi@gmail.com> > > This entire series makes me more and more unhappy. It is not the responsibility > of the hardware monitoring subsystem to control power. The hardware monitoring > subsystem is for monitoring, not for control. > > Please consider adding a driver for this chip to the pse-pd subsystem > (drivers/net/pse-pd). As it turns out, that subsystem already supports > tps23881. This is a similar chip which even has a similar register set. > > This driver could then be modified to be an auxiliary driver of that driver. > Alternatively, we could drop this driver entirely since the pse-pd subsystem > registers the chips it supports as regulator which has its own means to handle > telemetry. > > Thanks, > Guenter Yes, Guenter is right. This driver belongs to the pse-pd framework. Best Regards, Oleksik -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Sun, Sep 7, 2025 at 9:51 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > On Sun, Sep 07, 2025 at 09:06:25AM -0700, Guenter Roeck wrote: > > +Cc: pse-pd maintainers and netdev mailing list > > > > On 9/4/25 10:33, Gregory Fuchedgi via B4 Relay wrote: > > > This patch series introduces per-port device tree configuration with poe > > > class restrictions. Also adds optional reset/shutdown gpios. > > > > > > Tested with hw poe tester: > > > - Auto mode tested with no per-port DT settings as well as explicit port > > > DT ti,class=4. Tested that no IRQ is required in this case. > > > - Semi-Auto mode with class restricted to 0, 1, 2 or 3. IRQ required. > > > - Tested current cut-offs in Semi-Auto mode. > > > - On/off by default setting tested for both Auto and Semi-Auto modes. > > > - Tested fully disabling the ports in DT. > > > - Tested with both reset and ti,ports-shutdown gpios defined, as well as > > > with reset only, as well as with neither reset nor shutdown. > > > > > > Signed-off-by: Gregory Fuchedgi <gfuchedgi@gmail.com> > > > > This entire series makes me more and more unhappy. It is not the responsibility > > of the hardware monitoring subsystem to control power. The hardware monitoring > > subsystem is for monitoring, not for control. > > > > Please consider adding a driver for this chip to the pse-pd subsystem > > (drivers/net/pse-pd). As it turns out, that subsystem already supports > > tps23881. This is a similar chip which even has a similar register set. > > > > This driver could then be modified to be an auxiliary driver of that driver. > > Alternatively, we could drop this driver entirely since the pse-pd subsystem > > registers the chips it supports as regulator which has its own means to handle > > telemetry. > Yes, Guenter is right. This driver belongs to the pse-pd framework. No disagreement here in principle. However, the current hwmon driver already implements power control and exposes it via in*_enable sysfs files. I found this a bit odd, but I don't write drivers often. My understanding of Guenter's suggestion is that it would require breaking this userspace API? From a quick look at the tps23881 datasheet I can see that it is similar, however, it is quite different in the context of this patch. tps23881 (unlike tps23861) has Port Power Allocation register that can limit poe power class. This register can be set prior to detection/classification. So the extra complexity of an interrupt handler that decides whether to enable the power may not be required. Perhaps it still makes sense to merge these drivers, but I don't have time or hardware to do it at the moment.
On 9/8/25 09:39, Gregory Fuchedgi wrote: > On Sun, Sep 7, 2025 at 9:51 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote: >> >> On Sun, Sep 07, 2025 at 09:06:25AM -0700, Guenter Roeck wrote: >>> +Cc: pse-pd maintainers and netdev mailing list >>> >>> On 9/4/25 10:33, Gregory Fuchedgi via B4 Relay wrote: >>>> This patch series introduces per-port device tree configuration with poe >>>> class restrictions. Also adds optional reset/shutdown gpios. >>>> >>>> Tested with hw poe tester: >>>> - Auto mode tested with no per-port DT settings as well as explicit port >>>> DT ti,class=4. Tested that no IRQ is required in this case. >>>> - Semi-Auto mode with class restricted to 0, 1, 2 or 3. IRQ required. >>>> - Tested current cut-offs in Semi-Auto mode. >>>> - On/off by default setting tested for both Auto and Semi-Auto modes. >>>> - Tested fully disabling the ports in DT. >>>> - Tested with both reset and ti,ports-shutdown gpios defined, as well as >>>> with reset only, as well as with neither reset nor shutdown. >>>> >>>> Signed-off-by: Gregory Fuchedgi <gfuchedgi@gmail.com> >>> >>> This entire series makes me more and more unhappy. It is not the responsibility >>> of the hardware monitoring subsystem to control power. The hardware monitoring >>> subsystem is for monitoring, not for control. >>> >>> Please consider adding a driver for this chip to the pse-pd subsystem >>> (drivers/net/pse-pd). As it turns out, that subsystem already supports >>> tps23881. This is a similar chip which even has a similar register set. >>> >>> This driver could then be modified to be an auxiliary driver of that driver. >>> Alternatively, we could drop this driver entirely since the pse-pd subsystem >>> registers the chips it supports as regulator which has its own means to handle >>> telemetry. >> Yes, Guenter is right. This driver belongs to the pse-pd framework. > No disagreement here in principle. However, the current hwmon driver > already implements power control and exposes it via in*_enable sysfs > files. I found this a bit odd, but I don't write drivers often. > My understanding of Guenter's suggestion is that it would require breaking > this userspace API? > If the enable attributes enable power to the ports, that code and functionality is simply wrong. It should only enable (or have enabled) power _monitoring_. As such, changing that would from my perspective be a bug fix. And, yes, that slipped my attention when reviewing the original code. Sorry to have to say that, but I am not perfect. > From a quick look at the tps23881 datasheet I can see that it is > similar, however, it is quite different in the context of this patch. > tps23881 (unlike tps23861) has Port Power Allocation register that can > limit poe power class. This register can be set prior to > detection/classification. So the extra complexity of an interrupt > handler that decides whether to enable the power may not be required. > > Perhaps it still makes sense to merge these drivers, but I don't have > time or hardware to do it at the moment. I didn't suggest to merge the tps23881 and tps23861 drivers; I just pointed out that they have a similar register set. The point here is that a hardware monitoring driver should limit itself to hardware monitoring. Actual control should, for example, be implemented through the regulator or thermal subsystems. Guenter
On Mon, Sep 8, 2025 at 11:02 AM Guenter Roeck <linux@roeck-us.net> wrote: > > On 9/8/25 09:39, Gregory Fuchedgi wrote: > > On Sun, Sep 7, 2025 at 9:51 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote: > >> > >> On Sun, Sep 07, 2025 at 09:06:25AM -0700, Guenter Roeck wrote: > >>> +Cc: pse-pd maintainers and netdev mailing list > >>> > >>> On 9/4/25 10:33, Gregory Fuchedgi via B4 Relay wrote: > >>>> This patch series introduces per-port device tree configuration with poe > >>>> class restrictions. Also adds optional reset/shutdown gpios. > >>>> > >>>> Tested with hw poe tester: > >>>> - Auto mode tested with no per-port DT settings as well as explicit port > >>>> DT ti,class=4. Tested that no IRQ is required in this case. > >>>> - Semi-Auto mode with class restricted to 0, 1, 2 or 3. IRQ required. > >>>> - Tested current cut-offs in Semi-Auto mode. > >>>> - On/off by default setting tested for both Auto and Semi-Auto modes. > >>>> - Tested fully disabling the ports in DT. > >>>> - Tested with both reset and ti,ports-shutdown gpios defined, as well as > >>>> with reset only, as well as with neither reset nor shutdown. > >>>> > >>>> Signed-off-by: Gregory Fuchedgi <gfuchedgi@gmail.com> > >>> > >>> This entire series makes me more and more unhappy. It is not the responsibility > >>> of the hardware monitoring subsystem to control power. The hardware monitoring > >>> subsystem is for monitoring, not for control. > >>> > >>> Please consider adding a driver for this chip to the pse-pd subsystem > >>> (drivers/net/pse-pd). As it turns out, that subsystem already supports > >>> tps23881. This is a similar chip which even has a similar register set. > >>> > >>> This driver could then be modified to be an auxiliary driver of that driver. > >>> Alternatively, we could drop this driver entirely since the pse-pd subsystem > >>> registers the chips it supports as regulator which has its own means to handle > >>> telemetry. > >> Yes, Guenter is right. This driver belongs to the pse-pd framework. > > No disagreement here in principle. However, the current hwmon driver > > already implements power control and exposes it via in*_enable sysfs > > files. I found this a bit odd, but I don't write drivers often. > > My understanding of Guenter's suggestion is that it would require breaking > > this userspace API? > > > > If the enable attributes enable power to the ports, that code and functionality > is simply wrong. It should only enable (or have enabled) power _monitoring_. > As such, changing that would from my perspective be a bug fix. Alright, then. I'll try to find some time in the next few months to port this over to a separate driver in pse-pd. And then remove the in*_enable from hwmon one. Even if it's there by mistake, probably shouldn't fix it until there's an alternative in place. > > And, yes, that slipped my attention when reviewing the original code. > Sorry to have to say that, but I am not perfect. > > > From a quick look at the tps23881 datasheet I can see that it is > > similar, however, it is quite different in the context of this patch. > > tps23881 (unlike tps23861) has Port Power Allocation register that can > > limit poe power class. This register can be set prior to > > detection/classification. So the extra complexity of an interrupt > > handler that decides whether to enable the power may not be required. > > > > Perhaps it still makes sense to merge these drivers, but I don't have > > time or hardware to do it at the moment. > > I didn't suggest to merge the tps23881 and tps23861 drivers; I just pointed out > that they have a similar register set. > > The point here is that a hardware monitoring driver should limit itself > to hardware monitoring. Actual control should, for example, be implemented > through the regulator or thermal subsystems.
On Mon, 8 Sep 2025 09:39:58 -0700 Gregory Fuchedgi <gfuchedgi@gmail.com> wrote: > On Sun, Sep 7, 2025 at 9:51 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > > > On Sun, Sep 07, 2025 at 09:06:25AM -0700, Guenter Roeck wrote: > > > +Cc: pse-pd maintainers and netdev mailing list > > > > > > On 9/4/25 10:33, Gregory Fuchedgi via B4 Relay wrote: > [...] > > > > > > This entire series makes me more and more unhappy. It is not the > > > responsibility of the hardware monitoring subsystem to control power. The > > > hardware monitoring subsystem is for monitoring, not for control. > > > > > > Please consider adding a driver for this chip to the pse-pd subsystem > > > (drivers/net/pse-pd). As it turns out, that subsystem already supports > > > tps23881. This is a similar chip which even has a similar register set. > > > > > > This driver could then be modified to be an auxiliary driver of that > > > driver. Alternatively, we could drop this driver entirely since the > > > pse-pd subsystem registers the chips it supports as regulator which has > > > its own means to handle telemetry. > > Yes, Guenter is right. This driver belongs to the pse-pd framework. > No disagreement here in principle. However, the current hwmon driver > already implements power control and exposes it via in*_enable sysfs > files. I found this a bit odd, but I don't write drivers often. > My understanding of Guenter's suggestion is that it would require breaking > this userspace API? > > From a quick look at the tps23881 datasheet I can see that it is > similar, however, it is quite different in the context of this patch. > tps23881 (unlike tps23861) has Port Power Allocation register that can > limit poe power class. This register can be set prior to > detection/classification. So the extra complexity of an interrupt > handler that decides whether to enable the power may not be required. > > Perhaps it still makes sense to merge these drivers, but I don't have > time or hardware to do it at the moment. In either way the tps23861 is a PoE controller therefore the driver should land into the pse-pd framework. Then tweaking tps23881 driver to support tps2361 or using a standalone driver is another question. From a quick look the register map is really similar so indeed using tps23881 driver seems relevant. Regards, -- Köry Maincent, Bootlin Embedded Linux and kernel engineering https://bootlin.com
On Thu, Sep 04, 2025 at 10:33:43AM -0700, Gregory Fuchedgi wrote: > This patch series introduces per-port device tree configuration with poe > class restrictions. Also adds optional reset/shutdown gpios. > > Tested with hw poe tester: > - Auto mode tested with no per-port DT settings as well as explicit port > DT ti,class=4. Tested that no IRQ is required in this case. > - Semi-Auto mode with class restricted to 0, 1, 2 or 3. IRQ required. > - Tested current cut-offs in Semi-Auto mode. > - On/off by default setting tested for both Auto and Semi-Auto modes. > - Tested fully disabling the ports in DT. > - Tested with both reset and ti,ports-shutdown gpios defined, as well as > with reset only, as well as with neither reset nor shutdown. > > Signed-off-by: Gregory Fuchedgi <gfuchedgi@gmail.com> > --- > Changes in v3: > - cleaned up dt bindings in response to v2 review This is very vague. Everything is a change or clean up. I requested specific things to be changed and you should list them. Best regards, Krzysztof
© 2016 - 2025 Red Hat, Inc.