2024-01-26 09:05:45

by 徐永謄

[permalink] [raw]
Subject: [PATCH v1] hwmon: Add driver for MPS MPQ8785 Synchronous Step-Down Converter

Add support for mpq8785 device from Monolithic Power Systems, Inc.
(MPS) vendor. This is synchronous step-down controller.

Signed-off-by: Charles Hsu <[email protected]>

---
Change in v1:
Initial patchset.
---
Documentation/hwmon/index.rst | 1 +
Documentation/hwmon/mpq8785.rst | 86 +++++++++++++++++++++++++++++++++
drivers/hwmon/pmbus/Kconfig | 9 ++++
drivers/hwmon/pmbus/Makefile | 1 +
drivers/hwmon/pmbus/mpq8785.c | 55 +++++++++++++++++++++
5 files changed, 152 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..d8afdf875518
--- /dev/null
+++ b/Documentation/hwmon/mpq8785.rst
@@ -0,0 +1,86 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+Kernel driver mpq8785
+=======================
+
+Supported chips:
+
+ * MPS MPQ8785
+
+ Prefix: 'mpq8785'
+
+Author: Charles Hsu <[email protected]>
+
+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**
+
+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_alarm**
+
+**curr1_max**
+
+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..a0ebdb1b48b2
--- /dev/null
+++ b/drivers/hwmon/pmbus/mpq8785.c
@@ -0,0 +1,55 @@
+// 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 struct pmbus_driver_info mpq8785_info = {
+ .pages = 1,
+ .format[PSC_VOLTAGE_IN] = direct,
+ .format[PSC_CURRENT_OUT] = direct,
+ .format[PSC_VOLTAGE_OUT] = linear,
+ .format[PSC_TEMPERATURE] = direct,
+ .m[PSC_VOLTAGE_IN] = 4,
+ .b[PSC_VOLTAGE_IN] = 0,
+ .R[PSC_VOLTAGE_IN] = 1,
+ .m[PSC_CURRENT_OUT] = 16,
+ .b[PSC_CURRENT_OUT] = 0,
+ .R[PSC_CURRENT_OUT] = 0,
+ .m[PSC_TEMPERATURE] = 1,
+ .b[PSC_TEMPERATURE] = 0,
+ .R[PSC_TEMPERATURE] = 0,
+ .func[0] =
+ PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_TEMP | PMBUS_HAVE_IOUT |
+ PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP,
+};
+
+static int mpq8785_probe(struct i2c_client *client)
+{
+ return pmbus_do_probe(client, &mpq8785_info);
+}
+
+static const struct of_device_id __maybe_unused mpq8785_of_match[] = {
+ { .compatible = "mps,mpq8785", },
+ {}
+};
+
+MODULE_DEVICE_TABLE(of, mpq8785_of_match);
+
+static struct i2c_driver mpq8785_driver = {
+ .driver = {
+ .name = "mpq8785",
+ .of_match_table = of_match_ptr(mpq8785_of_match),
+ },
+ .probe_new = mpq8785_probe,
+};
+
+module_i2c_driver(mpq8785_driver);
+
+MODULE_AUTHOR("Charles Hsu <[email protected]>");
+MODULE_DESCRIPTION("MPQ8785 PMIC regulator driver");
+MODULE_LICENSE("GPL");
--
2.34.1



2024-01-28 00:00:51

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v1] hwmon: Add driver for MPS MPQ8785 Synchronous Step-Down Converter

On Fri, Jan 26, 2024 at 03:52:13PM +0800, Charles Hsu wrote:
> Add support for mpq8785 device from Monolithic Power Systems, Inc.
> (MPS) vendor. This is synchronous step-down controller.
>

"(MPS) vendor" above has no value.

I find no reference that this chip actually exists. Sorry, but I can not
apply such patches without confirmation that the chip actually exists.

> Signed-off-by: Charles Hsu <[email protected]>
> ---
> Change in v1:
> Initial patchset.

A change log or v1 tag is not needed for the first version of a patch
or patch series.

> ---
..
> + PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_TEMP | PMBUS_HAVE_IOUT |
> + PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP,

I am not too happy that all those drivers claim to have no output status
registers. It always makes me wonder if the definitions are just copied
from one driver to the next.

> +static const struct of_device_id __maybe_unused mpq8785_of_match[] = {
> + { .compatible = "mps,mpq8785", },
> + {}

checkpatch says:

WARNING: DT compatible string "mps,mpq8785" appears un-documented -- check ./Documentation/devicetree/bindings/
#293: FILE: drivers/hwmon/pmbus/mpq8785.c:37:
+ { .compatible = "mps,mpq8785", },

I don't care about the also missing entry in MAINTAINERS,
but the device needs to be be documented in
Documentation/devicetree/bindings/trivial-devices.yaml.

> +};
> +
> +MODULE_DEVICE_TABLE(of, mpq8785_of_match);
> +
> +static struct i2c_driver mpq8785_driver = {
> + .driver = {
> + .name = "mpq8785",
> + .of_match_table = of_match_ptr(mpq8785_of_match),
> + },
> + .probe_new = mpq8785_probe,
> +};
> +
> +module_i2c_driver(mpq8785_driver);
> +
> +MODULE_AUTHOR("Charles Hsu <[email protected]>");
> +MODULE_DESCRIPTION("MPQ8785 PMIC regulator driver");

Is this copied from the mpq7932 driver ? This driver does not include a
regulator component, so it is a bit misleading to label it as regulator
driver.

Guenter

2024-01-28 17:13:26

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v1] hwmon: Add driver for MPS MPQ8785 Synchronous Step-Down Converter

On 1/27/24 16:00, Guenter Roeck wrote:
> On Fri, Jan 26, 2024 at 03:52:13PM +0800, Charles Hsu wrote:
>> Add support for mpq8785 device from Monolithic Power Systems, Inc.
>> (MPS) vendor. This is synchronous step-down controller.
>>
>
> "(MPS) vendor" above has no value.
>
> I find no reference that this chip actually exists. Sorry, but I can not
> apply such patches without confirmation that the chip actually exists.
>

I since learned that the chip does exist, so this is no longer a concern.

>> Signed-off-by: Charles Hsu <[email protected]>
>> ---
>> Change in v1:
>> Initial patchset.
>
> A change log or v1 tag is not needed for the first version of a patch
> or patch series.
>
>> ---
> ...
>> + PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_TEMP | PMBUS_HAVE_IOUT |
>> + PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP,
>
> I am not too happy that all those drivers claim to have no output status
> registers. It always makes me wonder if the definitions are just copied
> from one driver to the next.

I also learned that the chip supports additional status registers. Please
list all supported status registers.

I also learned that the chips voltage output mode is configurable. As such,

+ .format[PSC_CURRENT_OUT] = direct,

won't do because it cause instantiation to fail due to mode mismatch
in pmbus_identify_common() if the mode is configured to linear or vid mode.
This will need to be addressed.

Thanks,
Guenter