On Mon, 8 Sep 2025 09:39:58 -0700 Gregory Fuchedgi <gfuchedgi@xxxxxxxxx> wrote: > On Sun, Sep 7, 2025 at 9:51 PM Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> 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
