Re: [PATCH 2/2] hwmon: (pmbus) Add driver for Analog Devices MAX20912 and MAX20916

[Guenter Roeck]

On 7/9/26 09:09, Nuno Sá wrote:
> On Thu, Jul 09, 2026 at 08:54:09AM -0700, Guenter Roeck wrote:
> > On Thu, Jul 09, 2026 at 09:54:22AM +0100, Nuno Sá wrote:
> > > > Based on the MAX20912/16 specs on my hand, these chips do not support
> > > > PMBUS_PHASE (0x04). Furthermore, the spec only indicates support for VID mode
> > > > and does not provide m/b/r. Therefore, some of the features you mentioned might
> > > > be specific to the MAX20826 series.
> > >
> > > I see, phases are not supported using standard PMBUS.
> >
> > As mentioned in my other e-mail, it can still be supported by the driver.
> > That is what the chip drivers are for, after all.
> >
> > > > Regarding enabling VOUT via GPIO, our platform handles this via the CPLD as
> > > > part of the hardware power sequencing. Managing this pin through the driver is
> > > > not a requirement for our system.
> > >
> > > But we cannot assume all systems will behave like the above. But now i
> > > do wonder about controlling the GPIOs in the driver. In your system you
> > > clearly did not need to do it. In mine (testing with a rpi) I had to
> > > use a GPIO (well I could have used hogs or pinctrl). But if you control the pin
> > > you do gain the ability to turn off the regulator. If you don't it's always on
> > > (which might be indeed the bulk of the real usecases for these systems).
> > Agreed. I don't really like it, but if the chip and some specific hardware
> > mandate it, it should be supported. However, that code also needs to be
> > tested - an untested implementation would be worse than no implementation.
>
> That's how I tested it :). So from what I understand the preferred way
> is to support these pins as optional? If they are not there we just
> assume the pins are always enabled by some other means? In contrast to
> what I have today which makes these pins mandatory!

I had to go back a bit in mental history .... actually, the regulator code
in the PMBus core doesn't touch the ON_OFF_CONFIG register. It uses the
OPERATION command to enable/disable outputs. The reason is that the enable
pin and ON_OFF_CONFIG typically affect the entire chip, while the OPERATION
command only affects a single channel.

So far PMBus drivers typically don't touch ON_OFF_CONFIG. The ibm-cffps
driver supports writing it, but only through a debugfs file. The tda38640
driver supports writing it, but only because it does not (or not correctly)
support the OPERATION command. ON_OFF_CONFIG (if it is modified at all)
is normally set by the firmware or even in production (if the chip supports
non-volatile configuration). It is one of the "hairy" PMBus commands
which should be left alone if at all possible.

You can not make the property mandatory since most systems will neither
support nor need it since it is handled by hardware/firmware.
In many if not almost all cases there won't even _be_ a GPIO pin that
can be set by software.

Guenter