linux-kernel - Re: [PATCH v2] hwmon: Add driver for MPS MPQ8785 Synchronous Step-Down Converter

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <ee2f4758-0b50-449a-b74b-492814a9cc8a@roeck-us.net>
Date: Mon, 29 Jan 2024 07:16:52 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Charles Hsu <ythsu0511@...il.com>, robh+dt@...nel.org,
 krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org, jdelvare@...e.com,
 corbet@....net, Delphine_CC_Chiu@...ynn.com, naresh.solanki@...ements.com,
 peteryin.openbmc@...il.com, patrick.rudolph@...ements.com,
 alexander.stein@...tq-group.com, lakshmiy@...ibm.com, bhelgaas@...gle.com,
 michal.simek@....com, lukas@...ner.de, festevam@...x.de
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-hwmon@...r.kernel.org, linux-doc@...r.kernel.org,
 linux-i2c@...r.kernel.org, ytshu0511@...il.com
Subject: Re: [PATCH v2] hwmon: Add driver for MPS MPQ8785 Synchronous
 Step-Down Converter

On 1/29/24 00:31, Charles Hsu wrote:
> Add support for mpq8785 device from Monolithic Power Systems, Inc.
> (MPS) vendor. This is synchronous step-down controller.
> 
> Signed-off-by: Charles Hsu <ythsu0511@...il.com>
> 
> ---
> Change in v1:
>      Initial patchset.
> Change in v2:
>      1.Add pmbus support status registers.
>      2.Add mpq8785 in trivial-devices.yaml.
>      3.Remove format[PSC_VOLTAGE_OUT]

This is insufficient, sorry. Problem is that pmbus_identify_common()
compares the more reported by PMBUS_VOUT_MODE with the configured mode.
Not setting format[PSC_VOLTAGE_OUT] means that the mode is the default (0),
which is linear mode. If the chip is configured for direct or vid mode
this again will result in probe failure.

I can see two options:

- Add a chip configuration flag such as PMBUS_DETECT_VOUT_MODE (for lack
   of a better idea) and use it in pmbus_identify_common() to actually
   _set_ the mode from the VOUT_MODE command instead of expecting it
   to be already set. This would require providing the coefficients for
   output voltage direct mode in mpq8785_info. This would be my preferred
   solution (but require a separate patch to add the code to pmbus_core.c).
- Select a fixed mode and write code to convert affected register readings
   to that mode if needed. See mp5990.c for an example.

>      4.Fix MODULE_DESCRIPTION
> ---
>   .../devicetree/bindings/trivial-devices.yaml  |  2 +
>   Documentation/hwmon/index.rst                 |  1 +
>   Documentation/hwmon/mpq8785.rst               | 94 +++++++++++++++++++
>   drivers/hwmon/pmbus/Kconfig                   |  9 ++
>   drivers/hwmon/pmbus/Makefile                  |  1 +
>   drivers/hwmon/pmbus/mpq8785.c                 | 62 ++++++++++++
>   6 files changed, 169 insertions(+)
>   create mode 100644 Documentation/hwmon/mpq8785.rst
>   create mode 100644 drivers/hwmon/pmbus/mpq8785.c
> 
> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> index 79dcd92c4a43..088b23ed2ae6 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -129,6 +129,8 @@ properties:
>             - mps,mp2975
>               # Monolithic Power Systems Inc. multi-phase hot-swap controller mp5990
>             - mps,mp5990
> +            # Monolithic Power Systems Inc. synchronous step-down converter mpq8785
> +          - mps,mpq8785
>               # Honeywell Humidicon HIH-6130 humidity/temperature sensor
>             - honeywell,hi6130
>               # IBM Common Form Factor Power Supply Versions (all versions)

Sorry, devicetree changes require a separate patch.

Thanks,
Guenter