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

Guenter Roeck
2024-01-29T15:16:56Z

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