diff mbox series

[v1] hwmon: Add driver for MPS MPQ8785 Synchronous Step-Down Converter

Message ID 20240126075213.1707572-1-ythsu0511@gmail.com
State New
Headers
Series [v1] hwmon: Add driver for MPS MPQ8785 Synchronous Step-Down Converter |

Commit Message

徐永謄 Jan. 26, 2024, 7:52 a.m. UTC 
  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.
---
 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

Comments

Guenter Roeck Jan. 28, 2024, midnight UTC | #1 
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 <ythsu0511@gmail.com>
> ---
> 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 <ythsu0511@gmail.com>");
> +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
Guenter Roeck Jan. 28, 2024, 5:13 p.m. UTC | #2 
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 <ythsu0511@gmail.com>
>> ---
>> 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
diff mbox series

Patch

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 <ythsu0511@gmail.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**
+
+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 <ythsu0511@gmail.com>");
+MODULE_DESCRIPTION("MPQ8785 PMIC regulator driver");
+MODULE_LICENSE("GPL");