Re: [PATCH v2 3/4] hwmon: (pmbus/mpq7932) Add a support for mpq7932 Power Management IC

[Guenter Roeck]

On 12/6/22 19:51, Saravanan Sekar wrote:
> On 01/12/22 16:34, Guenter Roeck wrote:
> > On 11/30/22 20:46, Saravanan Sekar wrote:
> > > The MPQ7932 is a power management IC designed to operate from 5V buses to
> > > power a variety of Advanced driver-assistance system SOCs. Six integrated
> > > buck converters with hardware monitoring capability powers a variety of
> > > target rails configurable over PMBus interface.
> > >
> > > Signed-off-by: Saravanan Sekar <saravanan@xxxxxxxxxxx>
> > > ---
> > >   drivers/hwmon/pmbus/Kconfig   |  10 +++
> > >   drivers/hwmon/pmbus/Makefile  |   1 +
> > >   drivers/hwmon/pmbus/mpq7932.c | 144 ++++++++++++++++++++++++++++++++++
> > >   3 files changed, 155 insertions(+)
> > >   create mode 100644 drivers/hwmon/pmbus/mpq7932.c
> > >
> > > diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> > > index 89668af67206..4a1538949a73 100644
> > > --- a/drivers/hwmon/pmbus/Kconfig
> > > +++ b/drivers/hwmon/pmbus/Kconfig
> > > @@ -317,6 +317,16 @@ config SENSORS_MP5023
> > >         This driver can also be built as a module. If so, the module will
> > >         be called mp5023.
> > > +config SENSORS_MPQ7932
> > > +    tristate "MPS MPQ7932"
> >
> > As written, a dependency on REGULATOR is missing. However, we want the driver
> > enabled even if CONFIG_REGULATOR is not enabled. I would suggest to follow the
> > approach used by other drivers: add a second configuration option
> > SENSORS_MPQ7932_REGULATOR which depends on SENSORS_MPQ7932 and REGULATOR
> > and enables regulator functionality, and use that in the driver to
> > make regulator support optional.
>
> Hello Guenter,
>
> I need clarification or confirmation from you before V3.
>
> Here is my view, communication to MPQ7932 PMIC chip is based on pmbus specification.
>
> Now the conflict is that we have pmbus directory under hwmon subsystem, if pmbus spec implementation would have been separate like i2c-smbus then we can implement chip driver in regulator subsystem which access pmbus.
>
> pmbus_core does supports regulator framework PMBUS_REGUALTOR and I believe it is valid MPQ7932 driver implantation under pmbus directory.
>
> Kindly suggest to remove pmbus dependency on hwmon and proceed further.

Every chip supporting PMBus has hwmon functionality. The PMBus core
as implemented primarily supports hardware monitoring. Some can
act as regulators; that is why regulator support was added to
the PMBus core. Trying to extract it would make no sense.

If you want to implement a driver without hardware monitoring
support for this chip, you won't need the PMBus core. I would not
agree with such an approach, but there is nothing that prevents you
from implementing a regulator-only driver for this chip in the
regulator subsystem.

Guenter