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

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);
>   };
>

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