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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <61eb8715-958a-f81d-17ad-7086f9ea81f5@roeck-us.net>
Date:   Tue, 6 Dec 2022 20:46:39 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     Saravanan Sekar <saravanan@...umiz.com>
Cc:     linux-hwmon@...r.kernel.org, jdelvare@...e.com,
        marten.lindahl@...s.com, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/4] hwmon: (pmbus/mpq7932) Add a support for mpq7932
 Power Management IC

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@...umiz.com>
>>> ---
>>>   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