Re: [PATCH 3/3] pmbus: export coefficients via sysfs

Guenter Roeck <linux@xxxxxxxxxxxx> · Wed, 10 Apr 2019 17:30:08 -0700

On 4/10/19 3:39 PM, Adamski, Krzysztof (Nokia - PL/Wroclaw) wrote:
In order to get best accuracy, in case of some devices it is required to
tweak coefficient values. This is especially true for devices using
some external shunt resistor or being operated in non-usual environment.
Those values may be measured during device production and stored as
calibration values in some place (like EEPROM or even a file).

To support wide range of possible use cases, lets export those to
user space via sysfs attributes so that it can implement its own
policies about them. All those attributes are put into separate
attribute_group struct so that we can set its name to group all of them
in a "coefficients" subdirectory in sysfs.


Coefficients are hardcoded into the chip, and the hwmon ABI reports raw
values. Any correction should be done using sensors3.conf.

Guenter

Signed-off-by: Krzysztof Adamski <krzysztof.adamski@xxxxxxxxx> > ---
  drivers/hwmon/pmbus/pmbus_core.c | 100 ++++++++++++++++++++++++++++++-
  1 file changed, 97 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 7a7dcdc8acc9..8cb61fc977db 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -57,6 +57,8 @@
  
  #define PMBUS_NAME_SIZE		24
  
+static const char * const coeff_name[] = {"m", "b", "R"};
+
  struct pmbus_sensor {
  	struct pmbus_sensor *next;
  	char name[PMBUS_NAME_SIZE];	/* sysfs sensor name */
@@ -98,11 +100,12 @@ struct pmbus_data {
  	int exponent[PMBUS_PAGES];
  				/* linear mode: exponent for output voltages */
  
-	const struct pmbus_driver_info *info;
+	struct pmbus_driver_info *info;
  
  	int max_attributes;
  	int num_attributes;
  	struct attribute_group group;
+	struct attribute_group group_coeff;
  	const struct attribute_group **groups;
  	struct dentry *debugfs;		/* debugfs device directory */
  
@@ -1901,6 +1904,88 @@ static int pmbus_add_fan_attributes(struct i2c_client *client,
  	return 0;
  }
  
+static struct attribute *pmbus_init_coeff_attr(struct pmbus_data *data,
+					       const char *prefix,
+					       const char *coeff, int *target)
+{
+	struct dev_ext_attribute *ext_attr;
+	char *name;
+
+	ext_attr = devm_kzalloc(data->dev, sizeof(ext_attr), GFP_KERNEL);
+	if (!ext_attr)
+		return NULL;
+
+	name = devm_kasprintf(data->dev, GFP_KERNEL, "%s_%s", prefix, coeff);
+	if (!name)
+		return NULL;
+
+	pmbus_dev_attr_init(&ext_attr->attr, name, (S_IWUSR | S_IRUGO),
+			    device_show_int, device_store_int);
+	ext_attr->var = target;
+
+	return &ext_attr->attr.attr;
+}
+
+static int pmbus_add_coeff_attributes_class(struct pmbus_data *data,
+					    const char *prefix,
+					    enum pmbus_sensor_classes class,
+					    struct attribute **attrs)
+{
+	int *coeff_val[] = {data->info->m, data->info->b, data->info->R};
+	struct attribute *ret;
+	int i, count = 0;
+
+	for (i = 0; i < ARRAY_SIZE(coeff_name); i++) {
+		ret = pmbus_init_coeff_attr(data, prefix, coeff_name[i],
+					    coeff_val[i]);
+		if (!ret)
+			return -ENOMEM;
+		attrs[count++] = ret;
+	}
+
+	return count;
+}
+
+static const char * const classes_names[] = {
+	[PSC_VOLTAGE_IN] = "vin",
+	[PSC_VOLTAGE_OUT] = "vout",
+	[PSC_CURRENT_IN] = "iin",
+	[PSC_CURRENT_OUT] = "iout",
+	[PSC_POWER] = "p",
+	[PSC_TEMPERATURE] = "temp",
+};
+
+static int pmbus_add_coeff_attributes(struct i2c_client *client,
+				      struct pmbus_data *data)
+{
+	struct attribute **attrs;
+	int i, n = 0, ret = 0;
+
+	attrs = kcalloc(ARRAY_SIZE(coeff_name) * (PSC_NUM_CLASSES + 1),
+			sizeof(void *), GFP_KERNEL);
+	if (!attrs)
+		return -ENOMEM;
+
+	for (i = 0; i < ARRAY_SIZE(classes_names); i++) {
+		if (classes_names[i] == NULL)
+			continue;
+
+		ret = pmbus_add_coeff_attributes_class(data, classes_names[i],
+						       i, &attrs[n]);
+		if (ret < 0) {
+			kfree(attrs);
+			return ret;
+		}
+
+		n += ret;
+	}
+
+	data->group_coeff.name = "coefficients";
+	data->group_coeff.attrs =  attrs;
+
+	return 0;
+}
+
  static int pmbus_find_attributes(struct i2c_client *client,
  				 struct pmbus_data *data)
  {
@@ -1932,6 +2017,11 @@ static int pmbus_find_attributes(struct i2c_client *client,
  
  	/* Fans */
  	ret = pmbus_add_fan_attributes(client, data);
+	if (ret)
+		return ret;
+
+	/* Coefficients */
+	ret = pmbus_add_coeff_attributes(client, data);
  	return ret;
  }
  
@@ -2324,7 +2414,8 @@ int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,
  		while (info->groups[groups_num])
  			groups_num++;
  
-	data->groups = devm_kcalloc(dev, groups_num + 2, sizeof(void *),
+	/* Two default groups + ending NULL makes 3 groups minimum */
+	data->groups = devm_kcalloc(dev, groups_num + 3, sizeof(void *),
  				    GFP_KERNEL);
  	if (!data->groups)
  		return -ENOMEM;
@@ -2356,7 +2447,8 @@ int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,
  	}
  
  	data->groups[0] = &data->group;
-	memcpy(data->groups + 1, info->groups, sizeof(void *) * groups_num);
+	data->groups[1] = &data->group_coeff;
+	memcpy(data->groups + 2, info->groups, sizeof(void *) * groups_num);
  	data->hwmon_dev = hwmon_device_register_with_groups(dev, client->name,
  							    data, data->groups);
  	if (IS_ERR(data->hwmon_dev)) {
@@ -2379,6 +2471,7 @@ int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,
  	hwmon_device_unregister(data->hwmon_dev);
  out_kfree:
  	kfree(data->group.attrs);
+	kfree(data->group_coeff.attrs);
  	return ret;
  }
  EXPORT_SYMBOL_GPL(pmbus_do_probe);
@@ -2391,6 +2484,7 @@ int pmbus_do_remove(struct i2c_client *client)
  
  	hwmon_device_unregister(data->hwmon_dev);
  	kfree(data->group.attrs);
+	kfree(data->group_coeff.attrs);
  	return 0;
  }
  EXPORT_SYMBOL_GPL(pmbus_do_remove);








