[PATCH v2 0/5] hwmon: pmbus: Add support for MPM82504 and MPM3695 family

[PATCH v2 0/5] hwmon: pmbus: Add support for MPM82504 and MPM3695 family

[Pawel Dembicki]

This series extends the hwmon PMBus driver for the MPS MPQ8785 to support
two additional Monolithic Power Systems devices: the MPM82504 and
MPM3695 family.

The driver is restructured to support multiple devices using device tree
matching. It also introduces an optional "mps,vout-fb-divider-ratio-permille"
property to configure the VOUT_SCALE_LOOP PMBus register, which adjusts
reported output voltages depending on the external feedback divider.

Device tree bindings are updated accordingly.

Changes have been tested on hardware with device-tree based matching
using the MPM82504 and MPM3695-10.

Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>

Pawel Dembicki (5):
  hwmon: pmbus: mpq8785: Prepare driver for multiple device support
  hwmon: pmbus: mpq8785: Add support for MPM82504
  hwmon: pmbus: mpq8785: Add support for MPM3695 family
  hwmon: pmbus: mpq8785: Implement VOUT feedback resistor divider ratio
    configuration
  dt-bindings: hwmon: Add bindings for mpq8785 driver

 .../bindings/hwmon/pmbus/mps,mpq8785.yaml     | 88 ++++++++++++++++++
 .../devicetree/bindings/trivial-devices.yaml  |  2 -
 Documentation/hwmon/mpq8785.rst               | 27 ++++--
 drivers/hwmon/pmbus/mpq8785.c                 | 93 +++++++++++++++++--
 4 files changed, 194 insertions(+), 16 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml

-- 
2.43.0

[PATCH v2 1/5] hwmon: pmbus: mpq8785: Prepare driver for multiple device support

[Pawel Dembicki]

Refactor the driver to support multiple Monolithic Power Systems devices.
Introduce chip ID handling based on device tree matching.

No functional changes intended.

Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>

---
v2:
 - no changes done
---
 drivers/hwmon/pmbus/mpq8785.c | 38 +++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/drivers/hwmon/pmbus/mpq8785.c b/drivers/hwmon/pmbus/mpq8785.c
index 331c274ca892..00ec21b081cb 100644
--- a/drivers/hwmon/pmbus/mpq8785.c
+++ b/drivers/hwmon/pmbus/mpq8785.c
@@ -8,6 +8,8 @@
 #include <linux/of_device.h>
 #include "pmbus.h"
 
+enum chips { mpq8785 };
+
 static int mpq8785_identify(struct i2c_client *client,
 			    struct pmbus_driver_info *info)
 {
@@ -53,26 +55,46 @@ static struct pmbus_driver_info mpq8785_info = {
 		PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
 		PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
 		PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
-	.identify = mpq8785_identify,
-};
-
-static int mpq8785_probe(struct i2c_client *client)
-{
-	return pmbus_do_probe(client, &mpq8785_info);
 };
 
 static const struct i2c_device_id mpq8785_id[] = {
-	{ "mpq8785" },
+	{ "mpq8785", mpq8785 },
 	{ },
 };
 MODULE_DEVICE_TABLE(i2c, mpq8785_id);
 
 static const struct of_device_id __maybe_unused mpq8785_of_match[] = {
-	{ .compatible = "mps,mpq8785" },
+	{ .compatible = "mps,mpq8785", .data = (void *)mpq8785 },
 	{}
 };
 MODULE_DEVICE_TABLE(of, mpq8785_of_match);
 
+static int mpq8785_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct pmbus_driver_info *info;
+	enum chips chip_id;
+
+	info = devm_kmemdup(dev, &mpq8785_info, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	if (dev->of_node)
+		chip_id = (uintptr_t)of_device_get_match_data(dev);
+	else
+		chip_id = i2c_match_id(mpq8785_id, client)->driver_data;
+
+	switch (chip_id) {
+	case mpq8785:
+		info->identify = mpq8785_identify;
+		break;
+	default:
+		return -ENODEV;
+	}
+
+	return pmbus_do_probe(client, info);
+};
+
 static struct i2c_driver mpq8785_driver = {
 	.driver = {
 		   .name = "mpq8785",
-- 
2.43.0

[PATCH v2 2/5] hwmon: pmbus: mpq8785: Add support for MPM82504

[Pawel Dembicki]

Add support for the Monolithic Power Systems MPM82504 digital voltage
regulator. MPM82504 uses PMBus direct format for voltage output.

Tested with device tree based matching.

Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>

---
v2:
  - fixed signedess for temperatures < 0 deg C
  - remove empty lines
---
 Documentation/hwmon/mpq8785.rst | 20 +++++++++++++++-----
 drivers/hwmon/pmbus/mpq8785.c   | 30 +++++++++++++++++++++++++++++-
 2 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/Documentation/hwmon/mpq8785.rst b/Documentation/hwmon/mpq8785.rst
index bf8176b87086..be228ee58ce2 100644
--- a/Documentation/hwmon/mpq8785.rst
+++ b/Documentation/hwmon/mpq8785.rst
@@ -6,6 +6,7 @@ Kernel driver mpq8785
 Supported chips:
 
   * MPS MPQ8785
+  * MPS MPM82504
 
     Prefix: 'mpq8785'
 
@@ -20,21 +21,30 @@ buck converter. The MPQ8785 offers a very compact solution that achieves up to
 wide input supply range. The MPQ8785 operates at high efficiency over a wide
 output current load range.
 
+The MPM82504 is a quad 25A, scalable, fully integrated power module with a PMBus
+interface. The device offers a complete power solution that achieves up to 25A
+per output channel. The MPM82504 has four output channels that can be paralleled
+to provide 50A, 75A, or 100A of output current for flexible configurations.
+The device can also operate in parallel with the MPM3695-100 and additional
+MPM82504 devices to provide a higher output current. The MPM82504 operates
+at high efficiency across a wide 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)
+The devices 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
+The MCOT scheme also allows multiple devices or chennels 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.
+All supported modules require a minimal number of readily available, standard
+external components. The MPQ8785 is available in a TLGA (5mmx6mm) package
+and the MPM82504 is available in a BGA (15mmx30mmx5.18mm) package.
 
 Device compliant with:
 
diff --git a/drivers/hwmon/pmbus/mpq8785.c b/drivers/hwmon/pmbus/mpq8785.c
index 00ec21b081cb..9a4a211b2aeb 100644
--- a/drivers/hwmon/pmbus/mpq8785.c
+++ b/drivers/hwmon/pmbus/mpq8785.c
@@ -8,7 +8,9 @@
 #include <linux/of_device.h>
 #include "pmbus.h"
 
-enum chips { mpq8785 };
+#define PMBUS_READ_TEMPERATURE_1_SIGN	BIT(9)
+
+enum chips { mpq8785, mpm82504 };
 
 static int mpq8785_identify(struct i2c_client *client,
 			    struct pmbus_driver_info *info)
@@ -36,6 +38,23 @@ static int mpq8785_identify(struct i2c_client *client,
 	return 0;
 };
 
+static int mpm82504_read_word_data(struct i2c_client *client, int page,
+				   int phase, int reg)
+{
+	int ret;
+
+	ret = pmbus_read_word_data(client, page, phase, reg);
+
+	if (ret < 0 || reg != PMBUS_READ_TEMPERATURE_1)
+		return ret;
+
+	/* Fix PMBUS_READ_TEMPERATURE_1 signedness */
+	if (ret & PMBUS_READ_TEMPERATURE_1_SIGN)
+		ret |= GENMASK(15, 10);
+
+	return ret;
+}
+
 static struct pmbus_driver_info mpq8785_info = {
 	.pages = 1,
 	.format[PSC_VOLTAGE_IN] = direct,
@@ -59,12 +78,14 @@ static struct pmbus_driver_info mpq8785_info = {
 
 static const struct i2c_device_id mpq8785_id[] = {
 	{ "mpq8785", mpq8785 },
+	{ "mpm82504", mpm82504 },
 	{ },
 };
 MODULE_DEVICE_TABLE(i2c, mpq8785_id);
 
 static const struct of_device_id __maybe_unused mpq8785_of_match[] = {
 	{ .compatible = "mps,mpq8785", .data = (void *)mpq8785 },
+	{ .compatible = "mps,mpm82504", .data = (void *)mpm82504 },
 	{}
 };
 MODULE_DEVICE_TABLE(of, mpq8785_of_match);
@@ -88,6 +109,13 @@ static int mpq8785_probe(struct i2c_client *client)
 	case mpq8785:
 		info->identify = mpq8785_identify;
 		break;
+	case mpm82504:
+		info->format[PSC_VOLTAGE_OUT] = direct;
+		info->m[PSC_VOLTAGE_OUT] = 8;
+		info->b[PSC_VOLTAGE_OUT] = 0;
+		info->R[PSC_VOLTAGE_OUT] = 2;
+		info->read_word_data = mpm82504_read_word_data;
+		break;
 	default:
 		return -ENODEV;
 	}
-- 
2.43.0

[PATCH v2 3/5] hwmon: pmbus: mpq8785: Add support for MPM3695 family

[Pawel Dembicki]

Add support for the Monolithic Power Systems MPM3695 family.
It contains four devices with suffixes: -10, -20, -25 and -100.
The device is PMBus compliant and shares characteristics with the
MPM82504.

MPM3695-25 has different VOLTAGE_SCALE_LOOP register size [11:0]
and it needs to be separated because it will be configured in the next
commit.

Tested with device tree based matching (MPM3695-10).

Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>

---
v2:
  - Added whole MPM3695 family
---
 Documentation/hwmon/mpq8785.rst | 13 +++++++++----
 drivers/hwmon/pmbus/mpq8785.c   |  8 +++++++-
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/Documentation/hwmon/mpq8785.rst b/Documentation/hwmon/mpq8785.rst
index be228ee58ce2..7093e4db4f55 100644
--- a/Documentation/hwmon/mpq8785.rst
+++ b/Documentation/hwmon/mpq8785.rst
@@ -7,6 +7,7 @@ Supported chips:
 
   * MPS MPQ8785
   * MPS MPM82504
+  * MPS MPM3695 family
 
     Prefix: 'mpq8785'
 
@@ -29,6 +30,14 @@ The device can also operate in parallel with the MPM3695-100 and additional
 MPM82504 devices to provide a higher output current. The MPM82504 operates
 at high efficiency across a wide load range.
 
+The MPM3695 family is a scalable, ultra-thin, fully integrated power module with
+a PMBus interface. It offers a complete power solution that achieves up to
+10A (-10 variant), 20A (-25 variant), 25A (-20 variant), 100A (-100 variant)
+of output current with excellent load and line regulation across a wide input
+voltage range. It operates at high efficiency over a wide load range, and can
+be parallled to deliver higher current. Variants -10,-20 and -100 have different
+voltage scale configuration register range (10 bits) than -25 version (11 bits).
+
 The PMBus interface provides converter configurations and key parameters
 monitoring.
 
@@ -42,10 +51,6 @@ Fully integrated protection features include over-current protection (OCP),
 over-voltage protection (OVP), under-voltage protection (UVP), and
 over-temperature protection (OTP).
 
-All supported modules require a minimal number of readily available, standard
-external components. The MPQ8785 is available in a TLGA (5mmx6mm) package
-and the MPM82504 is available in a BGA (15mmx30mmx5.18mm) package.
-
 Device compliant with:
 
 - PMBus rev 1.3 interface.
diff --git a/drivers/hwmon/pmbus/mpq8785.c b/drivers/hwmon/pmbus/mpq8785.c
index 9a4a211b2aeb..34245d0d2125 100644
--- a/drivers/hwmon/pmbus/mpq8785.c
+++ b/drivers/hwmon/pmbus/mpq8785.c
@@ -10,7 +10,7 @@
 
 #define PMBUS_READ_TEMPERATURE_1_SIGN	BIT(9)
 
-enum chips { mpq8785, mpm82504 };
+enum chips { mpq8785, mpm82504, mpm3695, mpm3695_25 };
 
 static int mpq8785_identify(struct i2c_client *client,
 			    struct pmbus_driver_info *info)
@@ -79,6 +79,8 @@ static struct pmbus_driver_info mpq8785_info = {
 static const struct i2c_device_id mpq8785_id[] = {
 	{ "mpq8785", mpq8785 },
 	{ "mpm82504", mpm82504 },
+	{ "mpm3695", mpm3695 },
+	{ "mpm3695-25", mpm3695_25 },
 	{ },
 };
 MODULE_DEVICE_TABLE(i2c, mpq8785_id);
@@ -86,6 +88,8 @@ MODULE_DEVICE_TABLE(i2c, mpq8785_id);
 static const struct of_device_id __maybe_unused mpq8785_of_match[] = {
 	{ .compatible = "mps,mpq8785", .data = (void *)mpq8785 },
 	{ .compatible = "mps,mpm82504", .data = (void *)mpm82504 },
+	{ .compatible = "mps,mpm3695", .data = (void *)mpm3695 },
+	{ .compatible = "mps,mpm3695-25", .data = (void *)mpm3695_25 },
 	{}
 };
 MODULE_DEVICE_TABLE(of, mpq8785_of_match);
@@ -110,6 +114,8 @@ static int mpq8785_probe(struct i2c_client *client)
 		info->identify = mpq8785_identify;
 		break;
 	case mpm82504:
+	case mpm3695:
+	case mpm3695_25:
 		info->format[PSC_VOLTAGE_OUT] = direct;
 		info->m[PSC_VOLTAGE_OUT] = 8;
 		info->b[PSC_VOLTAGE_OUT] = 0;
-- 
2.43.0

[PATCH v2 4/5] hwmon: pmbus: mpq8785: Implement VOUT feedback resistor divider ratio configuration

[Pawel Dembicki]

Implement support for setting the VOUT_SCALE_LOOP PMBus register
based on an optional device tree property
"mps,vout-fb-divider-ratio-permille".

This allows the driver to provide the correct VOUT value depending
on the feedback voltage divider configuration for chips where the
bootloader does not configure the VOUT_SCALE_LOOP register.

Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>

---
v2:
  - rename property to mps,vout-fb-divider-ratio-permille
  - add register value range checking
---
 drivers/hwmon/pmbus/mpq8785.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/hwmon/pmbus/mpq8785.c b/drivers/hwmon/pmbus/mpq8785.c
index 34245d0d2125..1d0e7ac9daf4 100644
--- a/drivers/hwmon/pmbus/mpq8785.c
+++ b/drivers/hwmon/pmbus/mpq8785.c
@@ -12,6 +12,13 @@
 
 enum chips { mpq8785, mpm82504, mpm3695, mpm3695_25 };
 
+static u16 voltage_scale_loop_max_val[] = {
+	GENMASK(10, 0), /* mpq8785 */
+	GENMASK(9, 0), /* mpm82504 */
+	GENMASK(9, 0), /* mpm3695 */
+	GENMASK(11, 0), /* mpm3695_25 */
+};
+
 static int mpq8785_identify(struct i2c_client *client,
 			    struct pmbus_driver_info *info)
 {
@@ -99,6 +106,8 @@ static int mpq8785_probe(struct i2c_client *client)
 	struct device *dev = &client->dev;
 	struct pmbus_driver_info *info;
 	enum chips chip_id;
+	u32 voltage_scale;
+	int ret;
 
 	info = devm_kmemdup(dev, &mpq8785_info, sizeof(*info), GFP_KERNEL);
 	if (!info)
@@ -126,6 +135,18 @@ static int mpq8785_probe(struct i2c_client *client)
 		return -ENODEV;
 	}
 
+	if (!of_property_read_u32(dev->of_node,
+				  "mps,vout-fb-divider-ratio-permille",
+				  &voltage_scale)) {
+		if (voltage_scale > voltage_scale_loop_max_val[chip_id])
+			return -EINVAL;
+
+		ret = i2c_smbus_write_word_data(client, PMBUS_VOUT_SCALE_LOOP,
+						voltage_scale);
+		if (ret)
+			return ret;
+	}
+
 	return pmbus_do_probe(client, info);
 };
 
-- 
2.43.0

[PATCH v2 5/5] dt-bindings: hwmon: Add bindings for mpq8785 driver

[Pawel Dembicki]

Add device tree bindings for Monolithic Power Systems MPQ8785, MPM82504
and MPM3695 PMBus-compliant voltage regulators.

These bindings also documents the optional
"mps,vout-fb-divider-ratio-permille" property.

---
v2:
  - remove mps,mpq8785 from trivial-devices.yaml
  - fix alphabetical order
  - rename voltage-scale-loop to mps,vout-fb-divider-ratio-permille
  - add mps,vout-fb-divider-ratio-permille min and max values
  - rewrite mps,vout-fb-divider-ratio-permille description

Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
 .../bindings/hwmon/pmbus/mps,mpq8785.yaml     | 88 +++++++++++++++++++
 .../devicetree/bindings/trivial-devices.yaml  |  2 -
 2 files changed, 88 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml
new file mode 100644
index 000000000000..3c61f5484326
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml
@@ -0,0 +1,88 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/pmbus/mps,mpq8785.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Monolithic Power Systems Multiphase Voltage Regulators with PMBus
+
+maintainers:
+  - Charles Hsu <ythsu0511@gmail.com>
+
+description:
+  Monolithic Power Systems digital multiphase voltage regulators with PMBus.
+
+properties:
+  compatible:
+    enum:
+      - mps,mpm3695
+      - mps,mpm3695-25
+      - mps,mpm82504
+      - mps,mpq8785
+
+  reg:
+    maxItems: 1
+
+  mps,vout-fb-divider-ratio-permille:
+    description:
+      The feedback resistor divider ratio, expressed in permille
+      (Vfb / Vout * 1000). This value is written to the PMBUS_VOUT_SCALE_LOOP
+      register and is required for correct output voltage presentation.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          const: mps,mpq8785
+    then:
+      properties:
+        mps,vout-fb-divider-ratio-permille:
+          maximum: 2047
+
+  - if:
+      properties:
+        compatible:
+          const: mps,mpm82504
+    then:
+      properties:
+        mps,vout-fb-divider-ratio-permille:
+          maximum: 1023
+
+  - if:
+      properties:
+        compatible:
+          const: mps,mpm3695
+    then:
+      properties:
+        mps,vout-fb-divider-ratio-permille:
+          maximum: 1023
+
+  - if:
+      properties:
+        compatible:
+          const: mps,mpm3695-25
+    then:
+      properties:
+        mps,vout-fb-divider-ratio-permille:
+          maximum: 4095
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      pmic@30 {
+        compatible = "mps,mpm82504";
+        reg = <0x30>;
+        mps,vout-fb-divider-ratio-permille = <600>;
+      };
+    };
diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index 8da408107e55..7c1c0cc29655 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -293,8 +293,6 @@ properties:
           - mps,mp5990
             # Monolithic Power Systems Inc. digital step-down converter mp9941
           - mps,mp9941
-            # Monolithic Power Systems Inc. synchronous step-down converter mpq8785
-          - mps,mpq8785
             # Temperature sensor with integrated fan control
           - national,lm63
             # Serial Interface ACPI-Compatible Microprocessor System Hardware Monitor
-- 
2.43.0

Re: [PATCH v2 5/5] dt-bindings: hwmon: Add bindings for mpq8785 driver

[Krzysztof Kozlowski]

On 09/05/2025 08:51, Pawel Dembicki wrote:
> Add device tree bindings for Monolithic Power Systems MPQ8785, MPM82504
> and MPM3695 PMBus-compliant voltage regulators.
> 
> These bindings also documents the optional
> "mps,vout-fb-divider-ratio-permille" property.
> 
> ---
> v2:
>   - remove mps,mpq8785 from trivial-devices.yaml
>   - fix alphabetical order
>   - rename voltage-scale-loop to mps,vout-fb-divider-ratio-permille
>   - add mps,vout-fb-divider-ratio-permille min and max values
>   - rewrite mps,vout-fb-divider-ratio-permille description

If you are going to send a new version, then reorder the patches so the
bindings are before the user (see submitting patches in DT doc dir).

> 
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> ---
>  .../bindings/hwmon/pmbus/mps,mpq8785.yaml     | 88 +++++++++++++++++++
>  .../devicetree/bindings/trivial-devices.yaml  |  2 -
>  2 files changed, 88 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml
> new file mode 100644
> index 000000000000..3c61f5484326
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml
> @@ -0,0 +1,88 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/pmbus/mps,mpq8785.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Monolithic Power Systems Multiphase Voltage Regulators with PMBus
> +
> +maintainers:
> +  - Charles Hsu <ythsu0511@gmail.com>
> +
> +description:
> +  Monolithic Power Systems digital multiphase voltage regulators with PMBus.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - mps,mpm3695
> +      - mps,mpm3695-25
> +      - mps,mpm82504
> +      - mps,mpq8785
> +
> +  reg:
> +    maxItems: 1
> +
> +  mps,vout-fb-divider-ratio-permille:
> +    description:
> +      The feedback resistor divider ratio, expressed in permille
> +      (Vfb / Vout * 1000). This value is written to the PMBUS_VOUT_SCALE_LOOP
> +      register and is required for correct output voltage presentation.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1

maximum: 4095
default: X

required: block goes here, before allOf: block.

> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          const: mps,mpq8785
> +    then:
> +      properties:
> +        mps,vout-fb-divider-ratio-permille:
> +          maximum: 2047
> +
> +  - if:
> +      properties:
> +        compatible:
> +          const: mps,mpm82504

That's enum with mpm3695

> +    then:
> +      properties:
> +        mps,vout-fb-divider-ratio-permille:
> +          maximum: 1023
> +
Best regards,
Krzysztof

Re: [PATCH v2 1/5] hwmon: pmbus: mpq8785: Prepare driver for multiple device support

[Krzysztof Kozlowski]

On 09/05/2025 08:51, Pawel Dembicki wrote:
> Refactor the driver to support multiple Monolithic Power Systems devices.
> Introduce chip ID handling based on device tree matching.
> 
> No functional changes intended.
> 
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> 
> ---
> v2:
>  - no changes done
> ---
>  drivers/hwmon/pmbus/mpq8785.c | 38 +++++++++++++++++++++++++++--------
>  1 file changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/mpq8785.c b/drivers/hwmon/pmbus/mpq8785.c
> index 331c274ca892..00ec21b081cb 100644
> --- a/drivers/hwmon/pmbus/mpq8785.c
> +++ b/drivers/hwmon/pmbus/mpq8785.c
> @@ -8,6 +8,8 @@
>  #include <linux/of_device.h>
>  #include "pmbus.h"
>  
> +enum chips { mpq8785 };

Use Linux coding style, so:
1. missing wrapping after/before each {}
2. missing descriptive name for the type (mpq8785_chips)
3. CAPITALICS see Linux coding style - there is a chapter exactly about
this.


> +
>  static int mpq8785_identify(struct i2c_client *client,
>  			    struct pmbus_driver_info *info)
>  {
> @@ -53,26 +55,46 @@ static struct pmbus_driver_info mpq8785_info = {
>  		PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
>  		PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
>  		PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
> -	.identify = mpq8785_identify,
> -};
> -
> -static int mpq8785_probe(struct i2c_client *client)
> -{
> -	return pmbus_do_probe(client, &mpq8785_info);
>  };
>  
>  static const struct i2c_device_id mpq8785_id[] = {
> -	{ "mpq8785" },
> +	{ "mpq8785", mpq8785 },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(i2c, mpq8785_id);
>  
>  static const struct of_device_id __maybe_unused mpq8785_of_match[] = {
> -	{ .compatible = "mps,mpq8785" },
> +	{ .compatible = "mps,mpq8785", .data = (void *)mpq8785 },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, mpq8785_of_match);
>  
> +static int mpq8785_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct pmbus_driver_info *info;
> +	enum chips chip_id;
> +
> +	info = devm_kmemdup(dev, &mpq8785_info, sizeof(*info), GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	if (dev->of_node)
> +		chip_id = (uintptr_t)of_device_get_match_data(dev);

(kernel_ulong_t) instead

> +	else
> +		chip_id = i2c_match_id(mpq8785_id, client)->driver_data;

Do not open-code i2c_get_match_data().


Best regards,
Krzysztof

Re: [PATCH v2 3/5] hwmon: pmbus: mpq8785: Add support for MPM3695 family

[Krzysztof Kozlowski]

On 09/05/2025 08:51, Pawel Dembicki wrote:
> Add support for the Monolithic Power Systems MPM3695 family.
> It contains four devices with suffixes: -10, -20, -25 and -100.
> The device is PMBus compliant and shares characteristics with the
> MPM82504.
> 
> MPM3695-25 has different VOLTAGE_SCALE_LOOP register size [11:0]
> and it needs to be separated because it will be configured in the next
> commit.

This should be *this* commit. Add proper support for a device in one
commit. Not half-broken commit, being fixed later.

> 
> Tested with device tree based matching (MPM3695-10).
> 
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> 
> ---
> v2:
>   - Added whole MPM3695 family
> ---
>  Documentation/hwmon/mpq8785.rst | 13 +++++++++----
>  drivers/hwmon/pmbus/mpq8785.c   |  8 +++++++-
>  2 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/hwmon/mpq8785.rst b/Documentation/hwmon/mpq8785.rst
> index be228ee58ce2..7093e4db4f55 100644
> --- a/Documentation/hwmon/mpq8785.rst
> +++ b/Documentation/hwmon/mpq8785.rst
> @@ -7,6 +7,7 @@ Supported chips:
>  
>    * MPS MPQ8785
>    * MPS MPM82504
> +  * MPS MPM3695 family
>  
>      Prefix: 'mpq8785'
>  
> @@ -29,6 +30,14 @@ The device can also operate in parallel with the MPM3695-100 and additional
>  MPM82504 devices to provide a higher output current. The MPM82504 operates
>  at high efficiency across a wide load range.
>  
> +The MPM3695 family is a scalable, ultra-thin, fully integrated power module with
> +a PMBus interface. It offers a complete power solution that achieves up to
> +10A (-10 variant), 20A (-25 variant), 25A (-20 variant), 100A (-100 variant)
> +of output current with excellent load and line regulation across a wide input
> +voltage range. It operates at high efficiency over a wide load range, and can
> +be parallled to deliver higher current. Variants -10,-20 and -100 have different
> +voltage scale configuration register range (10 bits) than -25 version (11 bits).
> +
>  The PMBus interface provides converter configurations and key parameters
>  monitoring.
>  
> @@ -42,10 +51,6 @@ Fully integrated protection features include over-current protection (OCP),
>  over-voltage protection (OVP), under-voltage protection (UVP), and
>  over-temperature protection (OTP).
>  
> -All supported modules require a minimal number of readily available, standard
> -external components. The MPQ8785 is available in a TLGA (5mmx6mm) package
> -and the MPM82504 is available in a BGA (15mmx30mmx5.18mm) package.
> -
>  Device compliant with:
>  
>  - PMBus rev 1.3 interface.
> diff --git a/drivers/hwmon/pmbus/mpq8785.c b/drivers/hwmon/pmbus/mpq8785.c
> index 9a4a211b2aeb..34245d0d2125 100644
> --- a/drivers/hwmon/pmbus/mpq8785.c
> +++ b/drivers/hwmon/pmbus/mpq8785.c
> @@ -10,7 +10,7 @@
>  
>  #define PMBUS_READ_TEMPERATURE_1_SIGN	BIT(9)
>  
> -enum chips { mpq8785, mpm82504 };
> +enum chips { mpq8785, mpm82504, mpm3695, mpm3695_25 };
>  
>  static int mpq8785_identify(struct i2c_client *client,
>  			    struct pmbus_driver_info *info)
> @@ -79,6 +79,8 @@ static struct pmbus_driver_info mpq8785_info = {
>  static const struct i2c_device_id mpq8785_id[] = {
>  	{ "mpq8785", mpq8785 },
>  	{ "mpm82504", mpm82504 },
> +	{ "mpm3695", mpm3695 },
> +	{ "mpm3695-25", mpm3695_25 },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(i2c, mpq8785_id);
> @@ -86,6 +88,8 @@ MODULE_DEVICE_TABLE(i2c, mpq8785_id);
>  static const struct of_device_id __maybe_unused mpq8785_of_match[] = {
>  	{ .compatible = "mps,mpq8785", .data = (void *)mpq8785 },
>  	{ .compatible = "mps,mpm82504", .data = (void *)mpm82504 },
> +	{ .compatible = "mps,mpm3695", .data = (void *)mpm3695 },
> +	{ .compatible = "mps,mpm3695-25", .data = (void *)mpm3695_25 },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, mpq8785_of_match);
> @@ -110,6 +114,8 @@ static int mpq8785_probe(struct i2c_client *client)
>  		info->identify = mpq8785_identify;
>  		break;
>  	case mpm82504:
> +	case mpm3695:
> +	case mpm3695_25:

So are these fully compatible? Looks like, so why aren't you expressing
it in the binding and here? No need for these redundant ID table entries.

>  		info->format[PSC_VOLTAGE_OUT] = direct;
>  		info->m[PSC_VOLTAGE_OUT] = 8;
>  		info->b[PSC_VOLTAGE_OUT] = 0;


Best regards,
Krzysztof

Re: [PATCH v2 4/5] hwmon: pmbus: mpq8785: Implement VOUT feedback resistor divider ratio configuration

[Krzysztof Kozlowski]

On 09/05/2025 08:51, Pawel Dembicki wrote:
> Implement support for setting the VOUT_SCALE_LOOP PMBus register
> based on an optional device tree property
> "mps,vout-fb-divider-ratio-permille".
> 
> This allows the driver to provide the correct VOUT value depending
> on the feedback voltage divider configuration for chips where the
> bootloader does not configure the VOUT_SCALE_LOOP register.
> 
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> 
> ---
> v2:
>   - rename property to mps,vout-fb-divider-ratio-permille
>   - add register value range checking
> ---
>  drivers/hwmon/pmbus/mpq8785.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/mpq8785.c b/drivers/hwmon/pmbus/mpq8785.c
> index 34245d0d2125..1d0e7ac9daf4 100644
> --- a/drivers/hwmon/pmbus/mpq8785.c
> +++ b/drivers/hwmon/pmbus/mpq8785.c
> @@ -12,6 +12,13 @@
>  
>  enum chips { mpq8785, mpm82504, mpm3695, mpm3695_25 };
>  
> +static u16 voltage_scale_loop_max_val[] = {
> +	GENMASK(10, 0), /* mpq8785 */

Drop comments and index the table with enums instead. It makes clear and
obvious code. Code should be readable and self-documenting instead of
adding comments as an substitute of non-obvious code.

> +	GENMASK(9, 0), /* mpm82504 */
> +	GENMASK(9, 0), /* mpm3695 */
> +	GENMASK(11, 0), /* mpm3695_25 */
> +};
> +
Best regards,
Krzysztof

Re: [PATCH v2 1/5] hwmon: pmbus: mpq8785: Prepare driver for multiple device support

[Paweł Dembicki]

pt., 9 maj 2025 o 09:03 Krzysztof Kozlowski <krzk@kernel.org> napisał(a):
>
> On 09/05/2025 08:51, Pawel Dembicki wrote:
> > Refactor the driver to support multiple Monolithic Power Systems devices.
> > Introduce chip ID handling based on device tree matching.
> >
> > No functional changes intended.
> >
> > Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> >
> > ---
> > v2:
> >  - no changes done
> > ---
> >  drivers/hwmon/pmbus/mpq8785.c | 38 +++++++++++++++++++++++++++--------
> >  1 file changed, 30 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/hwmon/pmbus/mpq8785.c b/drivers/hwmon/pmbus/mpq8785.c
> > index 331c274ca892..00ec21b081cb 100644
> > --- a/drivers/hwmon/pmbus/mpq8785.c
> > +++ b/drivers/hwmon/pmbus/mpq8785.c
> > @@ -8,6 +8,8 @@
> >  #include <linux/of_device.h>
> >  #include "pmbus.h"
> >
> > +enum chips { mpq8785 };
>
> Use Linux coding style, so:
> 1. missing wrapping after/before each {}
> 2. missing descriptive name for the type (mpq8785_chips)
> 3. CAPITALICS see Linux coding style - there is a chapter exactly about
> this.
>
>

Sorry, I was thinking that it is a local pmbus tradition.
Many drivers have the same enum without capitalics :

grep -r "enum chips {" .
./isl68137.c:enum chips {
./bel-pfe.c:enum chips {pfe1100, pfe3000};
./mp2975.c:enum chips {
./ucd9200.c:enum chips { ucd9200, ucd9220, ucd9222, ucd9224, ucd9240,
ucd9244, ucd9246,
./zl6100.c:enum chips { zl2004, zl2005, zl2006, zl2008, zl2105,
zl2106, zl6100, zl6105,
./ucd9000.c:enum chips { ucd9000, ucd90120, ucd90124, ucd90160,
ucd90320, ucd9090,
./max16601.c:enum chips { max16508, max16600, max16601, max16602 };
./q54sj108a2.c:enum chips {
./bpa-rs600.c:enum chips { bpa_rs600, bpd_rs600 };
./adm1275.c:enum chips { adm1075, adm1272, adm1273, adm1275, adm1276,
adm1278, adm1281, adm1293, adm1294 };
./max20730.c:enum chips {
./mp2856.c:enum chips { mp2856, mp2857 };
./tps53679.c:enum chips {
./ltc2978.c:enum chips {
./max34440.c:enum chips {
./pim4328.c:enum chips { pim4006, pim4328, pim4820 };
./fsp-3y.c:enum chips {
./lm25066.c:enum chips { lm25056, lm25066, lm5064, lm5066, lm5066i };

> > +
> >  static int mpq8785_identify(struct i2c_client *client,
> >                           struct pmbus_driver_info *info)
> >  {
> > @@ -53,26 +55,46 @@ static struct pmbus_driver_info mpq8785_info = {
> >               PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
> >               PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
> >               PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP,
> > -     .identify = mpq8785_identify,
> > -};
> > -
> > -static int mpq8785_probe(struct i2c_client *client)
> > -{
> > -     return pmbus_do_probe(client, &mpq8785_info);
> >  };
> >
> >  static const struct i2c_device_id mpq8785_id[] = {
> > -     { "mpq8785" },
> > +     { "mpq8785", mpq8785 },
> >       { },
> >  };
> >  MODULE_DEVICE_TABLE(i2c, mpq8785_id);
> >
> >  static const struct of_device_id __maybe_unused mpq8785_of_match[] = {
> > -     { .compatible = "mps,mpq8785" },
> > +     { .compatible = "mps,mpq8785", .data = (void *)mpq8785 },
> >       {}
> >  };
> >  MODULE_DEVICE_TABLE(of, mpq8785_of_match);
> >
> > +static int mpq8785_probe(struct i2c_client *client)
> > +{
> > +     struct device *dev = &client->dev;
> > +     struct pmbus_driver_info *info;
> > +     enum chips chip_id;
> > +
> > +     info = devm_kmemdup(dev, &mpq8785_info, sizeof(*info), GFP_KERNEL);
> > +     if (!info)
> > +             return -ENOMEM;
> > +
> > +     if (dev->of_node)
> > +             chip_id = (uintptr_t)of_device_get_match_data(dev);
>
> (kernel_ulong_t) instead
>
> > +     else
> > +             chip_id = i2c_match_id(mpq8785_id, client)->driver_data;
>
> Do not open-code i2c_get_match_data().
>
>
> Best regards,
> Krzysztof



Best regards,
Paweł Dembicki

Re: [PATCH v2 1/5] hwmon: pmbus: mpq8785: Prepare driver for multiple device support

[Krzysztof Kozlowski]

On 09/05/2025 09:41, Paweł Dembicki wrote:
> pt., 9 maj 2025 o 09:03 Krzysztof Kozlowski <krzk@kernel.org> napisał(a):
>>
>> On 09/05/2025 08:51, Pawel Dembicki wrote:
>>> Refactor the driver to support multiple Monolithic Power Systems devices.
>>> Introduce chip ID handling based on device tree matching.
>>>
>>> No functional changes intended.
>>>
>>> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
>>>
>>> ---
>>> v2:
>>>  - no changes done
>>> ---
>>>  drivers/hwmon/pmbus/mpq8785.c | 38 +++++++++++++++++++++++++++--------
>>>  1 file changed, 30 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/hwmon/pmbus/mpq8785.c b/drivers/hwmon/pmbus/mpq8785.c
>>> index 331c274ca892..00ec21b081cb 100644
>>> --- a/drivers/hwmon/pmbus/mpq8785.c
>>> +++ b/drivers/hwmon/pmbus/mpq8785.c
>>> @@ -8,6 +8,8 @@
>>>  #include <linux/of_device.h>
>>>  #include "pmbus.h"
>>>
>>> +enum chips { mpq8785 };
>>
>> Use Linux coding style, so:
>> 1. missing wrapping after/before each {}
>> 2. missing descriptive name for the type (mpq8785_chips)
>> 3. CAPITALICS see Linux coding style - there is a chapter exactly about
>> this.
>>
>>
> 
> Sorry, I was thinking that it is a local pmbus tradition.
> Many drivers have the same enum without capitalics :
> 
> grep -r "enum chips {" .
> ./isl68137.c:enum chips {
> ./bel-pfe.c:enum chips {pfe1100, pfe3000};
> ./mp2975.c:enum chips {
> ./ucd9200.c:enum chips { ucd9200, ucd9220, ucd9222, ucd9224, ucd9240,
> ucd9244, ucd9246,
> ./zl6100.c:enum chips { zl2004, zl2005, zl2006, zl2008, zl2105,
> zl2106, zl6100, zl6105,
> ./ucd9000.c:enum chips { ucd9000, ucd90120, ucd90124, ucd90160,
> ucd90320, ucd9090,
> ./max16601.c:enum chips { max16508, max16600, max16601, max16602 };
> ./q54sj108a2.c:enum chips {
> ./bpa-rs600.c:enum chips { bpa_rs600, bpd_rs600 };
> ./adm1275.c:enum chips { adm1075, adm1272, adm1273, adm1275, adm1276,
> adm1278, adm1281, adm1293, adm1294 };
> ./max20730.c:enum chips {
> ./mp2856.c:enum chips { mp2856, mp2857 };
> ./tps53679.c:enum chips {
> ./ltc2978.c:enum chips {
> ./max34440.c:enum chips {
> ./pim4328.c:enum chips { pim4006, pim4328, pim4820 };
> ./fsp-3y.c:enum chips {
> ./lm25066.c:enum chips { lm25056, lm25066, lm5064, lm5066, lm5066i };

If that's standard for this subsystem, then it's fine, although to me it
feels odd to see all over the code lower case constant.


Best regards,
Krzysztof

Re: [PATCH v2 4/5] hwmon: pmbus: mpq8785: Implement VOUT feedback resistor divider ratio configuration

[Guenter Roeck]

On 5/8/25 23:51, Pawel Dembicki wrote:
> Implement support for setting the VOUT_SCALE_LOOP PMBus register
> based on an optional device tree property
> "mps,vout-fb-divider-ratio-permille".
> 
> This allows the driver to provide the correct VOUT value depending
> on the feedback voltage divider configuration for chips where the
> bootloader does not configure the VOUT_SCALE_LOOP register.
> 
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> 
> ---
> v2:
>    - rename property to mps,vout-fb-divider-ratio-permille
>    - add register value range checking
> ---
>   drivers/hwmon/pmbus/mpq8785.c | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/mpq8785.c b/drivers/hwmon/pmbus/mpq8785.c
> index 34245d0d2125..1d0e7ac9daf4 100644
> --- a/drivers/hwmon/pmbus/mpq8785.c
> +++ b/drivers/hwmon/pmbus/mpq8785.c
> @@ -12,6 +12,13 @@
>   
>   enum chips { mpq8785, mpm82504, mpm3695, mpm3695_25 };
>   
> +static u16 voltage_scale_loop_max_val[] = {
> +	GENMASK(10, 0), /* mpq8785 */
> +	GENMASK(9, 0), /* mpm82504 */
> +	GENMASK(9, 0), /* mpm3695 */
> +	GENMASK(11, 0), /* mpm3695_25 */

Use
	[... ] = GENMASK()
as suggested.

> +};
> +
>   static int mpq8785_identify(struct i2c_client *client,
>   			    struct pmbus_driver_info *info)
>   {
> @@ -99,6 +106,8 @@ static int mpq8785_probe(struct i2c_client *client)
>   	struct device *dev = &client->dev;
>   	struct pmbus_driver_info *info;
>   	enum chips chip_id;
> +	u32 voltage_scale;
> +	int ret;
>   
>   	info = devm_kmemdup(dev, &mpq8785_info, sizeof(*info), GFP_KERNEL);
>   	if (!info)
> @@ -126,6 +135,18 @@ static int mpq8785_probe(struct i2c_client *client)
>   		return -ENODEV;
>   	}
>   
> +	if (!of_property_read_u32(dev->of_node,

s/of_property/device_property/ (and include linux/property.h) to make this
usable from non-devicetree systems.

Also, please swap this patch with the previous patch to address the concern
about patch order (i.e., introduce the property first and then add support
for the new chips).

Thanks,
Guenter

> +				  "mps,vout-fb-divider-ratio-permille",
> +				  &voltage_scale)) {
> +		if (voltage_scale > voltage_scale_loop_max_val[chip_id])
> +			return -EINVAL;
> +
> +		ret = i2c_smbus_write_word_data(client, PMBUS_VOUT_SCALE_LOOP,
> +						voltage_scale);
> +		if (ret)
> +			return ret;
> +	}
> +
>   	return pmbus_do_probe(client, info);
>   };
>   

Re: [PATCH v2 2/5] hwmon: pmbus: mpq8785: Add support for MPM82504

[Guenter Roeck]

On 5/8/25 23:51, Pawel Dembicki wrote:
> Add support for the Monolithic Power Systems MPM82504 digital voltage
> regulator. MPM82504 uses PMBus direct format for voltage output.
> 
> Tested with device tree based matching.
> 
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> 
> ---
> v2:
>    - fixed signedess for temperatures < 0 deg C
>    - remove empty lines
> ---
>   Documentation/hwmon/mpq8785.rst | 20 +++++++++++++++-----
>   drivers/hwmon/pmbus/mpq8785.c   | 30 +++++++++++++++++++++++++++++-
>   2 files changed, 44 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/hwmon/mpq8785.rst b/Documentation/hwmon/mpq8785.rst
> index bf8176b87086..be228ee58ce2 100644
> --- a/Documentation/hwmon/mpq8785.rst
> +++ b/Documentation/hwmon/mpq8785.rst
> @@ -6,6 +6,7 @@ Kernel driver mpq8785
>   Supported chips:
>   
>     * MPS MPQ8785
> +  * MPS MPM82504
>   
>       Prefix: 'mpq8785'
>   
> @@ -20,21 +21,30 @@ buck converter. The MPQ8785 offers a very compact solution that achieves up to
>   wide input supply range. The MPQ8785 operates at high efficiency over a wide
>   output current load range.
>   
> +The MPM82504 is a quad 25A, scalable, fully integrated power module with a PMBus
> +interface. The device offers a complete power solution that achieves up to 25A
> +per output channel. The MPM82504 has four output channels that can be paralleled
> +to provide 50A, 75A, or 100A of output current for flexible configurations.
> +The device can also operate in parallel with the MPM3695-100 and additional
> +MPM82504 devices to provide a higher output current. The MPM82504 operates
> +at high efficiency across a wide 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)
> +The devices 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
> +The MCOT scheme also allows multiple devices or chennels to be connected in

channels

> +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.
> +All supported modules require a minimal number of readily available, standard
> +external components. The MPQ8785 is available in a TLGA (5mmx6mm) package
> +and the MPM82504 is available in a BGA (15mmx30mmx5.18mm) package.
>   
>   Device compliant with:
>   
> diff --git a/drivers/hwmon/pmbus/mpq8785.c b/drivers/hwmon/pmbus/mpq8785.c
> index 00ec21b081cb..9a4a211b2aeb 100644
> --- a/drivers/hwmon/pmbus/mpq8785.c
> +++ b/drivers/hwmon/pmbus/mpq8785.c
> @@ -8,7 +8,9 @@
>   #include <linux/of_device.h>
>   #include "pmbus.h"
>   
> -enum chips { mpq8785 };
> +#define PMBUS_READ_TEMPERATURE_1_SIGN	BIT(9)

s/PMBUS/MPM82504/

because this is not a standard bit. Needs to include linux/bits.h
or better linux/bitops.h (see below).

> +
> +enum chips { mpq8785, mpm82504 };

Please keep those in alphabetic order.
>   
>   static int mpq8785_identify(struct i2c_client *client,
>   			    struct pmbus_driver_info *info)
> @@ -36,6 +38,23 @@ static int mpq8785_identify(struct i2c_client *client,
>   	return 0;
>   };
>   
> +static int mpm82504_read_word_data(struct i2c_client *client, int page,
> +				   int phase, int reg)
> +{
> +	int ret;
> +
> +	ret = pmbus_read_word_data(client, page, phase, reg);
> +
> +	if (ret < 0 || reg != PMBUS_READ_TEMPERATURE_1)
> +		return ret;
> +
> +	/* Fix PMBUS_READ_TEMPERATURE_1 signedness */
> +	if (ret & PMBUS_READ_TEMPERATURE_1_SIGN)
> +		ret |= GENMASK(15, 10);

Better use sign_extend32(ret, PMBUS_READ_TEMPERATURE_1_SIGN) & 0xffff.
The mask is a bit odd but there is no sign_extend16(), but this makes it
more obvious what is done here.

Also, this needs to include linux/bitops.h.

> +
> +	return ret;
> +}
> +
>   static struct pmbus_driver_info mpq8785_info = {
>   	.pages = 1,
>   	.format[PSC_VOLTAGE_IN] = direct,
> @@ -59,12 +78,14 @@ static struct pmbus_driver_info mpq8785_info = {
>   
>   static const struct i2c_device_id mpq8785_id[] = {
>   	{ "mpq8785", mpq8785 },
> +	{ "mpm82504", mpm82504 },
>   	{ },
>   };
>   MODULE_DEVICE_TABLE(i2c, mpq8785_id);
>   
>   static const struct of_device_id __maybe_unused mpq8785_of_match[] = {
>   	{ .compatible = "mps,mpq8785", .data = (void *)mpq8785 },
> +	{ .compatible = "mps,mpm82504", .data = (void *)mpm82504 },

Alphabetic order.

>   	{}
>   };
>   MODULE_DEVICE_TABLE(of, mpq8785_of_match);
> @@ -88,6 +109,13 @@ static int mpq8785_probe(struct i2c_client *client)
>   	case mpq8785:
>   		info->identify = mpq8785_identify;
>   		break;
> +	case mpm82504:
> +		info->format[PSC_VOLTAGE_OUT] = direct;
> +		info->m[PSC_VOLTAGE_OUT] = 8;
> +		info->b[PSC_VOLTAGE_OUT] = 0;
> +		info->R[PSC_VOLTAGE_OUT] = 2;
> +		info->read_word_data = mpm82504_read_word_data;
> +		break;

Same here, with case statements. Sorry, I should have noticed in v1.

Thanks,
Guenter

>   	default:
>   		return -ENODEV;
>   	}

Re: [PATCH v2 1/5] hwmon: pmbus: mpq8785: Prepare driver for multiple device support

[Guenter Roeck]

On 5/9/25 02:22, Krzysztof Kozlowski wrote:
> On 09/05/2025 09:41, Paweł Dembicki wrote:
>> pt., 9 maj 2025 o 09:03 Krzysztof Kozlowski <krzk@kernel.org> napisał(a):
>>>
>>> On 09/05/2025 08:51, Pawel Dembicki wrote:
>>>> Refactor the driver to support multiple Monolithic Power Systems devices.
>>>> Introduce chip ID handling based on device tree matching.
>>>>
>>>> No functional changes intended.
>>>>
>>>> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
>>>>
>>>> ---
>>>> v2:
>>>>   - no changes done
>>>> ---
>>>>   drivers/hwmon/pmbus/mpq8785.c | 38 +++++++++++++++++++++++++++--------
>>>>   1 file changed, 30 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/hwmon/pmbus/mpq8785.c b/drivers/hwmon/pmbus/mpq8785.c
>>>> index 331c274ca892..00ec21b081cb 100644
>>>> --- a/drivers/hwmon/pmbus/mpq8785.c
>>>> +++ b/drivers/hwmon/pmbus/mpq8785.c
>>>> @@ -8,6 +8,8 @@
>>>>   #include <linux/of_device.h>
>>>>   #include "pmbus.h"
>>>>
>>>> +enum chips { mpq8785 };
>>>
>>> Use Linux coding style, so:
>>> 1. missing wrapping after/before each {}
>>> 2. missing descriptive name for the type (mpq8785_chips)
>>> 3. CAPITALICS see Linux coding style - there is a chapter exactly about
>>> this.
>>>
>>>
>>
>> Sorry, I was thinking that it is a local pmbus tradition.
>> Many drivers have the same enum without capitalics :
>>

hwmon, really, not just pmbus.

>> grep -r "enum chips {" .
>> ./isl68137.c:enum chips {
>> ./bel-pfe.c:enum chips {pfe1100, pfe3000};
>> ./mp2975.c:enum chips {
>> ./ucd9200.c:enum chips { ucd9200, ucd9220, ucd9222, ucd9224, ucd9240,
>> ucd9244, ucd9246,
>> ./zl6100.c:enum chips { zl2004, zl2005, zl2006, zl2008, zl2105,
>> zl2106, zl6100, zl6105,
>> ./ucd9000.c:enum chips { ucd9000, ucd90120, ucd90124, ucd90160,
>> ucd90320, ucd9090,
>> ./max16601.c:enum chips { max16508, max16600, max16601, max16602 };
>> ./q54sj108a2.c:enum chips {
>> ./bpa-rs600.c:enum chips { bpa_rs600, bpd_rs600 };
>> ./adm1275.c:enum chips { adm1075, adm1272, adm1273, adm1275, adm1276,
>> adm1278, adm1281, adm1293, adm1294 };
>> ./max20730.c:enum chips {
>> ./mp2856.c:enum chips { mp2856, mp2857 };
>> ./tps53679.c:enum chips {
>> ./ltc2978.c:enum chips {
>> ./max34440.c:enum chips {
>> ./pim4328.c:enum chips { pim4006, pim4328, pim4820 };
>> ./fsp-3y.c:enum chips {
>> ./lm25066.c:enum chips { lm25056, lm25066, lm5064, lm5066, lm5066i };
> 
> If that's standard for this subsystem, then it's fine, although to me it
> feels odd to see all over the code lower case constant.
> 

hwmon traditionally (as in: from the very beginning) uses lowercase enums
for chip types. It would be even more odd to change that now.

Thanks,
Guenter