[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <698500ed-5e90-4436-a90d-d153eb9f73c3@roeck-us.net>
Date: Tue, 30 Jan 2024 22:12:07 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Charles Hsu <ythsu0511@...il.com>, jdelvare@...e.com, corbet@....net,
Delphine_CC_Chiu@...ynn.com, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
linux-hwmon@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/2] hwmon: Add driver for MPS MPQ8785 Synchronous
Step-Down Converter
On 1/30/24 21:55, 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].
> 4.Fix MODULE_DESCRIPTION.
> Change in v3:
> 1.Identify vout_mode.
> 2.Separate dt-binding.
> ---
> 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 | 91 +++++++++++++++++++++++++++++++
> 5 files changed, 196 insertions(+)
> create mode 100644 Documentation/hwmon/mpq8785.rst
> create mode 100644 drivers/hwmon/pmbus/mpq8785.c
>
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index c7ed1f73ac06..085ad6ca9b05 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -163,6 +163,7 @@ Hardware Monitoring Kernel Drivers
> mp2975
> mp5023
> mp5990
> + mpq8785
> nct6683
> nct6775
> nct7802
> diff --git a/Documentation/hwmon/mpq8785.rst b/Documentation/hwmon/mpq8785.rst
> new file mode 100644
> index 000000000000..bf8176b87086
> --- /dev/null
> +++ b/Documentation/hwmon/mpq8785.rst
> @@ -0,0 +1,94 @@
> +.. SPDX-License-Identifier: GPL-2.0-only
> +
> +Kernel driver mpq8785
> +=======================
> +
> +Supported chips:
> +
> + * MPS MPQ8785
> +
> + Prefix: 'mpq8785'
> +
> +Author: Charles Hsu <ythsu0511@...il.com>
> +
> +Description
> +-----------
> +
> +The MPQ8785 is a fully integrated, PMBus-compatible, high-frequency, synchronous
> +buck converter. The MPQ8785 offers a very compact solution that achieves up to
> +40A output current per phase, with excellent load and line regulation over a
> +wide input supply range. The MPQ8785 operates at high efficiency over a wide
> +output current load range.
> +
> +The PMBus interface provides converter configurations and key parameters
> +monitoring.
> +
> +The MPQ8785 adopts MPS's proprietary multi-phase digital constant-on-time (MCOT)
> +control, which provides fast transient response and eases loop stabilization.
> +The MCOT scheme also allows multiple MPQ8785 devices to be connected in parallel
> +with excellent current sharing and phase interleaving for high-current
> +applications.
> +
> +Fully integrated protection features include over-current protection (OCP),
> +over-voltage protection (OVP), under-voltage protection (UVP), and
> +over-temperature protection (OTP).
> +
> +The MPQ8785 requires a minimal number of readily available, standard external
> +components, and is available in a TLGA (5mmx6mm) package.
> +
> +Device compliant with:
> +
> +- PMBus rev 1.3 interface.
> +
> +The driver exports the following attributes via the 'sysfs' files
> +for input voltage:
> +
> +**in1_input**
> +
> +**in1_label**
> +
> +**in1_max**
> +
> +**in1_max_alarm**
> +
> +**in1_min**
> +
> +**in1_min_alarm**
> +
> +**in1_crit**
> +
> +**in1_crit_alarm**
> +
> +The driver provides the following attributes for output voltage:
> +
> +**in2_input**
> +
> +**in2_label**
> +
> +**in2_alarm**
> +
> +The driver provides the following attributes for output current:
> +
> +**curr1_input**
> +
> +**curr1_label**
> +
> +**curr1_max**
> +
> +**curr1_max_alarm**
> +
> +**curr1_crit**
> +
> +**curr1_crit_alarm**
> +
> +The driver provides the following attributes for temperature:
> +
> +**temp1_input**
> +
> +**temp1_max**
> +
> +**temp1_max_alarm**
> +
> +**temp1_crit**
> +
> +**temp1_crit_alarm**
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 294808f5240a..557ae0c414b0 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -377,6 +377,15 @@ config SENSORS_MPQ7932
> This driver can also be built as a module. If so, the module will
> be called mpq7932.
>
> +config SENSORS_MPQ8785
> + tristate "MPS MPQ8785"
> + help
> + If you say yes here you get hardware monitoring functionality support
> + for power management IC MPS MPQ8785.
> +
> + This driver can also be built as a module. If so, the module will
> + be called mpq8785.
> +
> config SENSORS_PIM4328
> tristate "Flex PIM4328 and compatibles"
> help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index cf8a76744545..f14ecf03ad77 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -39,6 +39,7 @@ obj-$(CONFIG_SENSORS_MP2975) += mp2975.o
> obj-$(CONFIG_SENSORS_MP5023) += mp5023.o
> obj-$(CONFIG_SENSORS_MP5990) += mp5990.o
> obj-$(CONFIG_SENSORS_MPQ7932) += mpq7932.o
> +obj-$(CONFIG_SENSORS_MPQ8785) += mpq8785.o
> obj-$(CONFIG_SENSORS_PLI1209BC) += pli1209bc.o
> obj-$(CONFIG_SENSORS_PM6764TR) += pm6764tr.o
> obj-$(CONFIG_SENSORS_PXE1610) += pxe1610.o
> diff --git a/drivers/hwmon/pmbus/mpq8785.c b/drivers/hwmon/pmbus/mpq8785.c
> new file mode 100644
> index 000000000000..b5bfc5d8a96b
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/mpq8785.c
> @@ -0,0 +1,91 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Driver for MPS MPQ8785 Step-Down Converter
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include "pmbus.h"
> +
> +static int mpq8785_identify(struct i2c_client *client,
> + struct pmbus_driver_info *info)
> +{
> + int vout_mode;
> +
> + vout_mode = pmbus_read_byte_data(client, 0, PMBUS_VOUT_MODE);
Excellent solution, but it will have to return an error if reading
VOUT_MODE fails. Something like:
vout_mode = ...;
if (vout_mode < 0 || vout_mode == 0xff)
return vout_mode < 0 ? vout_mode : -ENODEV;
switch (vout_mode >> 5) {
...
The output mode _has_ to be valid.
Thanks,
Guenter
Powered by blists - more mailing lists