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

[Nuno Sá]

On Thu, Jul 09, 2026 at 04:39:36PM -0700, Guenter Roeck wrote:
> 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.
> 

Yes. The only thing I do is to read the ON_OFF_CONFIG register at probe
to check if I need to check the enable bit in OPERATION. So that
vout state becomes gpio && OPERATION. 

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

Understood.

- Nuno Sá
> 
> Guenter
>