[PATCH v3 0/3] hwmon: (pmbus/ltc2978) Set voltage resolution

[PATCH v3 0/3] hwmon: (pmbus/ltc2978) Set voltage resolution

[Mårten Lindahl]

Hi!

When checking if a regulator supports a voltage range, the regulator
needs to have support for listing the range or else -EINVAL will be
returned.

This support does not exist for the LTC2977 regulator, so this change
adds support for list voltage to the pmbus regulators by adding
regulator_list_voltage_linear to the pmbus_regulator_ops. It also
defines the voltage resolution for regulators LTC2972/LTC2974/LTC2975/
LTC2977/LTC2978/LTC2979/LTC2980/LTM2987 based on that they all have the
same stepwise 122.07uV resolution, and scales the resolution to a 1mV
resolution which is easier to handle.

These patches have been tested on an ARTPEC-8 developer board with a group
of LTC2977 power regulators.

Kind regards
Mårten Lindahl

Changes in v3:
 - Move read of low/high margins into local functions
 - Add check for invalid selector value
 - Introduce new macro PMBUS_REGULATOR_STEP(_name, _id, _voltages, _step)

Changes in v2:
 - Correct #define format
 - Change dev_err to dev_warn
 - Add new pmbus_regulator_list_voltage function
 - Cache low/high vout margins

Mårten Lindahl (3):
  hwmon: (pmbus) Introduce and use cached vout margins
  hwmon: (pmbus) Add list_voltage to pmbus ops
  hwmon: (pmbus/ltc2978) Set voltage resolution

 drivers/hwmon/pmbus/ltc2978.c    |  44 +++++++++++--
 drivers/hwmon/pmbus/pmbus.h      |   8 ++-
 drivers/hwmon/pmbus/pmbus_core.c | 108 ++++++++++++++++++++++++++-----
 3 files changed, 137 insertions(+), 23 deletions(-)

-- 
2.30.2

[PATCH v3 1/3] hwmon: (pmbus) Introduce and use cached vout margins

[Mårten Lindahl]

When setting a new voltage the voltage boundaries are read every time to
check that the new voltage is within the proper range. Checking these
voltage boundaries consists of reading one of PMBUS_MFR_VOUT_MIN/
PMBUS_VOUT_MARGIN_LOW registers and then PMBUS_MFR_VOUT_MAX/
PMBUS_VOUT_MARGIN_HIGH together with writing the PMBUS_CLEAR_FAULTS
register.

Since these boundaries are never being changed, it can be cached and
thus saving unnecessary smbus transmissions.

Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>
---
 drivers/hwmon/pmbus/pmbus_core.c | 78 +++++++++++++++++++++++++-------
 1 file changed, 61 insertions(+), 17 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 02912022853d..5e0d16512fa6 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -104,6 +104,9 @@ struct pmbus_data {
 
 	s16 currpage;	/* current page, -1 for unknown/unset */
 	s16 currphase;	/* current phase, 0xff for all, -1 for unknown/unset */
+
+	int vout_low[PMBUS_PAGES];	/* voltage low margin */
+	int vout_high[PMBUS_PAGES];	/* voltage high margin */
 };
 
 struct pmbus_debugfs_entry {
@@ -2636,6 +2639,58 @@ static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned
 	return 0;
 }
 
+static int pmbus_regulator_get_low_margin(struct i2c_client *client, int page)
+{
+	struct pmbus_data *data = i2c_get_clientdata(client);
+	struct pmbus_sensor s = {
+		.page = page,
+		.class = PSC_VOLTAGE_OUT,
+		.convert = true,
+		.data = -1,
+	};
+
+	if (!data->vout_low[page]) {
+		if (pmbus_check_word_register(client, page, PMBUS_MFR_VOUT_MIN))
+			s.data = _pmbus_read_word_data(client, page, 0xff,
+						       PMBUS_MFR_VOUT_MIN);
+		if (s.data < 0) {
+			s.data = _pmbus_read_word_data(client, page, 0xff,
+						       PMBUS_VOUT_MARGIN_LOW);
+			if (s.data < 0)
+				return s.data;
+		}
+		data->vout_low[page] = pmbus_reg2data(data, &s);
+	}
+
+	return data->vout_low[page];
+}
+
+static int pmbus_regulator_get_high_margin(struct i2c_client *client, int page)
+{
+	struct pmbus_data *data = i2c_get_clientdata(client);
+	struct pmbus_sensor s = {
+		.page = page,
+		.class = PSC_VOLTAGE_OUT,
+		.convert = true,
+		.data = -1,
+	};
+
+	if (!data->vout_high[page]) {
+		if (pmbus_check_word_register(client, page, PMBUS_MFR_VOUT_MAX))
+			s.data = _pmbus_read_word_data(client, page, 0xff,
+						       PMBUS_MFR_VOUT_MAX);
+		if (s.data < 0) {
+			s.data = _pmbus_read_word_data(client, page, 0xff,
+						       PMBUS_VOUT_MARGIN_HIGH);
+			if (s.data < 0)
+				return s.data;
+		}
+		data->vout_high[page] = pmbus_reg2data(data, &s);
+	}
+
+	return data->vout_high[page];
+}
+
 static int pmbus_regulator_get_voltage(struct regulator_dev *rdev)
 {
 	struct device *dev = rdev_get_dev(rdev);
@@ -2671,24 +2726,13 @@ static int pmbus_regulator_set_voltage(struct regulator_dev *rdev, int min_uv,
 
 	*selector = 0;
 
-	if (pmbus_check_word_register(client, s.page, PMBUS_MFR_VOUT_MIN))
-		s.data = _pmbus_read_word_data(client, s.page, 0xff, PMBUS_MFR_VOUT_MIN);
-	if (s.data < 0) {
-		s.data = _pmbus_read_word_data(client, s.page, 0xff, PMBUS_VOUT_MARGIN_LOW);
-		if (s.data < 0)
-			return s.data;
-	}
-	low = pmbus_reg2data(data, &s);
+	low = pmbus_regulator_get_low_margin(client, s.page);
+	if (low < 0)
+		return low;
 
-	s.data = -1;
-	if (pmbus_check_word_register(client, s.page, PMBUS_MFR_VOUT_MAX))
-		s.data = _pmbus_read_word_data(client, s.page, 0xff, PMBUS_MFR_VOUT_MAX);
-	if (s.data < 0) {
-		s.data = _pmbus_read_word_data(client, s.page, 0xff, PMBUS_VOUT_MARGIN_HIGH);
-		if (s.data < 0)
-			return s.data;
-	}
-	high = pmbus_reg2data(data, &s);
+	high = pmbus_regulator_get_high_margin(client, s.page);
+	if (high < 0)
+		return high;
 
 	/* Make sure we are within margins */
 	if (low > val)
-- 
2.30.2

[PATCH v3 2/3] hwmon: (pmbus) Add list_voltage to pmbus ops

[Mårten Lindahl]

When checking if a regulator supports a voltage range, the regulator
needs to have a list_voltage callback set to the regulator_ops or else
-EINVAL will be returned. This support does not exist for the pmbus
regulators, so this patch adds pmbus_regulator_list_voltage to the
pmbus_regulator_ops.

Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>
---
 drivers/hwmon/pmbus/pmbus_core.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 5e0d16512fa6..f303c2598fb5 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -2745,6 +2745,35 @@ static int pmbus_regulator_set_voltage(struct regulator_dev *rdev, int min_uv,
 	return _pmbus_write_word_data(client, s.page, PMBUS_VOUT_COMMAND, (u16)val);
 }
 
+static int pmbus_regulator_list_voltage(struct regulator_dev *rdev,
+					 unsigned int selector)
+{
+	struct device *dev = rdev_get_dev(rdev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
+	int val, low, high;
+
+	if (selector >= rdev->desc->n_voltages ||
+	    selector < rdev->desc->linear_min_sel)
+		return -EINVAL;
+
+	selector -= rdev->desc->linear_min_sel;
+	val = DIV_ROUND_CLOSEST(rdev->desc->min_uV +
+				(rdev->desc->uV_step * selector), 1000); /* convert to mV */
+
+	low = pmbus_regulator_get_low_margin(client, rdev_get_id(rdev));
+	if (low < 0)
+		return low;
+
+	high = pmbus_regulator_get_high_margin(client, rdev_get_id(rdev));
+	if (high < 0)
+		return high;
+
+	if (val >= low && val <= high)
+		return val * 1000; /* unit is uV */
+
+	return 0;
+}
+
 const struct regulator_ops pmbus_regulator_ops = {
 	.enable = pmbus_regulator_enable,
 	.disable = pmbus_regulator_disable,
@@ -2752,6 +2781,7 @@ const struct regulator_ops pmbus_regulator_ops = {
 	.get_error_flags = pmbus_regulator_get_error_flags,
 	.get_voltage = pmbus_regulator_get_voltage,
 	.set_voltage = pmbus_regulator_set_voltage,
+	.list_voltage = pmbus_regulator_list_voltage,
 };
 EXPORT_SYMBOL_NS_GPL(pmbus_regulator_ops, PMBUS);
 
-- 
2.30.2

[PATCH v3 3/3] hwmon: (pmbus/ltc2978) Set voltage resolution

[Mårten Lindahl]

The LTC2977 regulator does not set the regulator_desc .n_voltages value
which is needed in order to let the regulator core list the regulator
voltage range.

This patch defines a regulator_desc with a voltage range, and uses it
for defining voltage resolution for regulators LTC2972/LTC2974/LTC2975/
LTC2977/LTC2978/LTC2979/LTC2980/LTM2987 based on that they all have a 16
bit ADC with the same stepwise 122.07uV resolution. It also scales the
resolution to a 1mV resolution which is easier to handle.

Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>
---
 drivers/hwmon/pmbus/ltc2978.c | 44 +++++++++++++++++++++++++++++++----
 drivers/hwmon/pmbus/pmbus.h   |  8 +++++--
 2 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/drivers/hwmon/pmbus/ltc2978.c b/drivers/hwmon/pmbus/ltc2978.c
index 531aa674a928..6d2592731ba3 100644
--- a/drivers/hwmon/pmbus/ltc2978.c
+++ b/drivers/hwmon/pmbus/ltc2978.c
@@ -562,7 +562,24 @@ static const struct i2c_device_id ltc2978_id[] = {
 MODULE_DEVICE_TABLE(i2c, ltc2978_id);
 
 #if IS_ENABLED(CONFIG_SENSORS_LTC2978_REGULATOR)
+#define LTC2978_ADC_RES	0xFFFF
+#define LTC2978_N_ADC	122
+#define LTC2978_MAX_UV	(LTC2978_ADC_RES * LTC2978_N_ADC)
+#define LTC2978_UV_STEP	1000
+#define LTC2978_N_VOLTAGES	((LTC2978_MAX_UV / LTC2978_UV_STEP) + 1)
+
 static const struct regulator_desc ltc2978_reg_desc[] = {
+	PMBUS_REGULATOR_STEP("vout", 0, LTC2978_N_VOLTAGES, LTC2978_UV_STEP),
+	PMBUS_REGULATOR_STEP("vout", 1, LTC2978_N_VOLTAGES, LTC2978_UV_STEP),
+	PMBUS_REGULATOR_STEP("vout", 2, LTC2978_N_VOLTAGES, LTC2978_UV_STEP),
+	PMBUS_REGULATOR_STEP("vout", 3, LTC2978_N_VOLTAGES, LTC2978_UV_STEP),
+	PMBUS_REGULATOR_STEP("vout", 4, LTC2978_N_VOLTAGES, LTC2978_UV_STEP),
+	PMBUS_REGULATOR_STEP("vout", 5, LTC2978_N_VOLTAGES, LTC2978_UV_STEP),
+	PMBUS_REGULATOR_STEP("vout", 6, LTC2978_N_VOLTAGES, LTC2978_UV_STEP),
+	PMBUS_REGULATOR_STEP("vout", 7, LTC2978_N_VOLTAGES, LTC2978_UV_STEP),
+};
+
+static const struct regulator_desc ltc2978_reg_desc_default[] = {
 	PMBUS_REGULATOR("vout", 0),
 	PMBUS_REGULATOR("vout", 1),
 	PMBUS_REGULATOR("vout", 2),
@@ -839,10 +856,29 @@ static int ltc2978_probe(struct i2c_client *client)
 
 #if IS_ENABLED(CONFIG_SENSORS_LTC2978_REGULATOR)
 	info->num_regulators = info->pages;
-	info->reg_desc = ltc2978_reg_desc;
-	if (info->num_regulators > ARRAY_SIZE(ltc2978_reg_desc)) {
-		dev_err(&client->dev, "num_regulators too large!");
-		info->num_regulators = ARRAY_SIZE(ltc2978_reg_desc);
+	switch (data->id) {
+	case ltc2972:
+	case ltc2974:
+	case ltc2975:
+	case ltc2977:
+	case ltc2978:
+	case ltc2979:
+	case ltc2980:
+	case ltm2987:
+		info->reg_desc = ltc2978_reg_desc;
+		if (info->num_regulators > ARRAY_SIZE(ltc2978_reg_desc)) {
+			dev_warn(&client->dev, "num_regulators too large!");
+			info->num_regulators = ARRAY_SIZE(ltc2978_reg_desc);
+		}
+		break;
+	default:
+		info->reg_desc = ltc2978_reg_desc_default;
+		if (info->num_regulators > ARRAY_SIZE(ltc2978_reg_desc_default)) {
+			dev_warn(&client->dev, "num_regulators too large!");
+			info->num_regulators =
+			    ARRAY_SIZE(ltc2978_reg_desc_default);
+		}
+		break;
 	}
 #endif
 
diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index c031a9700ace..4a4689c4ab7f 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -463,8 +463,8 @@ struct pmbus_driver_info {
 
 extern const struct regulator_ops pmbus_regulator_ops;
 
-/* Macro for filling in array of struct regulator_desc */
-#define PMBUS_REGULATOR(_name, _id)				\
+/* Macros for filling in array of struct regulator_desc */
+#define PMBUS_REGULATOR_STEP(_name, _id, _voltages, _step)  \
 	[_id] = {						\
 		.name = (_name # _id),				\
 		.supply_name = "vin",				\
@@ -474,8 +474,12 @@ extern const struct regulator_ops pmbus_regulator_ops;
 		.ops = &pmbus_regulator_ops,			\
 		.type = REGULATOR_VOLTAGE,			\
 		.owner = THIS_MODULE,				\
+		.n_voltages = _voltages,			\
+		.uV_step = _step,				\
 	}
 
+#define PMBUS_REGULATOR(_name, _id)	PMBUS_REGULATOR_STEP(_name, _id, 0, 0)
+
 /* Function declarations */
 
 void pmbus_clear_cache(struct i2c_client *client);
-- 
2.30.2

Re: [PATCH v3 1/3] hwmon: (pmbus) Introduce and use cached vout margins

[Guenter Roeck]

On Tue, Jun 14, 2022 at 11:38:54AM +0200, Mårten Lindahl wrote:
> When setting a new voltage the voltage boundaries are read every time to
> check that the new voltage is within the proper range. Checking these
> voltage boundaries consists of reading one of PMBUS_MFR_VOUT_MIN/
> PMBUS_VOUT_MARGIN_LOW registers and then PMBUS_MFR_VOUT_MAX/
> PMBUS_VOUT_MARGIN_HIGH together with writing the PMBUS_CLEAR_FAULTS
> register.
> 
> Since these boundaries are never being changed, it can be cached and
> thus saving unnecessary smbus transmissions.
> 
> Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>

Applied to hwmon-next.

Thanks,
Guenter

> ---
>  drivers/hwmon/pmbus/pmbus_core.c | 78 +++++++++++++++++++++++++-------
>  1 file changed, 61 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 02912022853d..5e0d16512fa6 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -104,6 +104,9 @@ struct pmbus_data {
>  
>  	s16 currpage;	/* current page, -1 for unknown/unset */
>  	s16 currphase;	/* current phase, 0xff for all, -1 for unknown/unset */
> +
> +	int vout_low[PMBUS_PAGES];	/* voltage low margin */
> +	int vout_high[PMBUS_PAGES];	/* voltage high margin */
>  };
>  
>  struct pmbus_debugfs_entry {
> @@ -2636,6 +2639,58 @@ static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned
>  	return 0;
>  }
>  
> +static int pmbus_regulator_get_low_margin(struct i2c_client *client, int page)
> +{
> +	struct pmbus_data *data = i2c_get_clientdata(client);
> +	struct pmbus_sensor s = {
> +		.page = page,
> +		.class = PSC_VOLTAGE_OUT,
> +		.convert = true,
> +		.data = -1,
> +	};
> +
> +	if (!data->vout_low[page]) {
> +		if (pmbus_check_word_register(client, page, PMBUS_MFR_VOUT_MIN))
> +			s.data = _pmbus_read_word_data(client, page, 0xff,
> +						       PMBUS_MFR_VOUT_MIN);
> +		if (s.data < 0) {
> +			s.data = _pmbus_read_word_data(client, page, 0xff,
> +						       PMBUS_VOUT_MARGIN_LOW);
> +			if (s.data < 0)
> +				return s.data;
> +		}
> +		data->vout_low[page] = pmbus_reg2data(data, &s);
> +	}
> +
> +	return data->vout_low[page];
> +}
> +
> +static int pmbus_regulator_get_high_margin(struct i2c_client *client, int page)
> +{
> +	struct pmbus_data *data = i2c_get_clientdata(client);
> +	struct pmbus_sensor s = {
> +		.page = page,
> +		.class = PSC_VOLTAGE_OUT,
> +		.convert = true,
> +		.data = -1,
> +	};
> +
> +	if (!data->vout_high[page]) {
> +		if (pmbus_check_word_register(client, page, PMBUS_MFR_VOUT_MAX))
> +			s.data = _pmbus_read_word_data(client, page, 0xff,
> +						       PMBUS_MFR_VOUT_MAX);
> +		if (s.data < 0) {
> +			s.data = _pmbus_read_word_data(client, page, 0xff,
> +						       PMBUS_VOUT_MARGIN_HIGH);
> +			if (s.data < 0)
> +				return s.data;
> +		}
> +		data->vout_high[page] = pmbus_reg2data(data, &s);
> +	}
> +
> +	return data->vout_high[page];
> +}
> +
>  static int pmbus_regulator_get_voltage(struct regulator_dev *rdev)
>  {
>  	struct device *dev = rdev_get_dev(rdev);
> @@ -2671,24 +2726,13 @@ static int pmbus_regulator_set_voltage(struct regulator_dev *rdev, int min_uv,
>  
>  	*selector = 0;
>  
> -	if (pmbus_check_word_register(client, s.page, PMBUS_MFR_VOUT_MIN))
> -		s.data = _pmbus_read_word_data(client, s.page, 0xff, PMBUS_MFR_VOUT_MIN);
> -	if (s.data < 0) {
> -		s.data = _pmbus_read_word_data(client, s.page, 0xff, PMBUS_VOUT_MARGIN_LOW);
> -		if (s.data < 0)
> -			return s.data;
> -	}
> -	low = pmbus_reg2data(data, &s);
> +	low = pmbus_regulator_get_low_margin(client, s.page);
> +	if (low < 0)
> +		return low;
>  
> -	s.data = -1;
> -	if (pmbus_check_word_register(client, s.page, PMBUS_MFR_VOUT_MAX))
> -		s.data = _pmbus_read_word_data(client, s.page, 0xff, PMBUS_MFR_VOUT_MAX);
> -	if (s.data < 0) {
> -		s.data = _pmbus_read_word_data(client, s.page, 0xff, PMBUS_VOUT_MARGIN_HIGH);
> -		if (s.data < 0)
> -			return s.data;
> -	}
> -	high = pmbus_reg2data(data, &s);
> +	high = pmbus_regulator_get_high_margin(client, s.page);
> +	if (high < 0)
> +		return high;
>  
>  	/* Make sure we are within margins */
>  	if (low > val)

Re: [PATCH v3 2/3] hwmon: (pmbus) Add list_voltage to pmbus ops

[Guenter Roeck]

On Tue, Jun 14, 2022 at 11:38:55AM +0200, Mårten Lindahl wrote:
> When checking if a regulator supports a voltage range, the regulator
> needs to have a list_voltage callback set to the regulator_ops or else
> -EINVAL will be returned. This support does not exist for the pmbus
> regulators, so this patch adds pmbus_regulator_list_voltage to the
> pmbus_regulator_ops.
> 
> Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>

Applied to hwmon-next.

Thanks,
Guenter

> ---
>  drivers/hwmon/pmbus/pmbus_core.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 5e0d16512fa6..f303c2598fb5 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -2745,6 +2745,35 @@ static int pmbus_regulator_set_voltage(struct regulator_dev *rdev, int min_uv,
>  	return _pmbus_write_word_data(client, s.page, PMBUS_VOUT_COMMAND, (u16)val);
>  }
>  
> +static int pmbus_regulator_list_voltage(struct regulator_dev *rdev,
> +					 unsigned int selector)
> +{
> +	struct device *dev = rdev_get_dev(rdev);
> +	struct i2c_client *client = to_i2c_client(dev->parent);
> +	int val, low, high;
> +
> +	if (selector >= rdev->desc->n_voltages ||
> +	    selector < rdev->desc->linear_min_sel)
> +		return -EINVAL;
> +
> +	selector -= rdev->desc->linear_min_sel;
> +	val = DIV_ROUND_CLOSEST(rdev->desc->min_uV +
> +				(rdev->desc->uV_step * selector), 1000); /* convert to mV */
> +
> +	low = pmbus_regulator_get_low_margin(client, rdev_get_id(rdev));
> +	if (low < 0)
> +		return low;
> +
> +	high = pmbus_regulator_get_high_margin(client, rdev_get_id(rdev));
> +	if (high < 0)
> +		return high;
> +
> +	if (val >= low && val <= high)
> +		return val * 1000; /* unit is uV */
> +
> +	return 0;
> +}
> +
>  const struct regulator_ops pmbus_regulator_ops = {
>  	.enable = pmbus_regulator_enable,
>  	.disable = pmbus_regulator_disable,
> @@ -2752,6 +2781,7 @@ const struct regulator_ops pmbus_regulator_ops = {
>  	.get_error_flags = pmbus_regulator_get_error_flags,
>  	.get_voltage = pmbus_regulator_get_voltage,
>  	.set_voltage = pmbus_regulator_set_voltage,
> +	.list_voltage = pmbus_regulator_list_voltage,
>  };
>  EXPORT_SYMBOL_NS_GPL(pmbus_regulator_ops, PMBUS);
>