diff mbox

[v3,2/3] hwmon: ibmpowernv: Add attributes to enable/disable sensor groups

Message ID 1530798689-27742-3-git-send-email-shilpa.bhat@linux.vnet.ibm.com (mailing list archive)
State Superseded
Headers show

Commit Message

Shilpasri G Bhat July 5, 2018, 1:51 p.m. UTC
On-Chip-Controller(OCC) is an embedded micro-processor in POWER9 chip
which measures various system and chip level sensors. These sensors
comprises of environmental sensors (like power, temperature, current
and voltage) and performance sensors (like utilization, frequency).
All these sensors are copied to main memory at a regular interval of
100ms. OCC provides a way to select a group of sensors that is copied
to the main memory to increase the update frequency of selected sensor
groups. When a sensor-group is disabled, OCC will not copy it to main
memory and those sensors read 0 values.

This patch provides support for enabling/disabling the sensor groups
like power, temperature, current and voltage. This patch adds new
per-senor sysfs attribute to disable and enable them.

Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
---
Changes from v2:
- Writes to first 'enable' attribute of the sensor group will affect all the
  sensors in the group
- Removed global mutex and made it per sensor-group

 drivers/hwmon/ibmpowernv.c | 184 ++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 155 insertions(+), 29 deletions(-)

Comments

Guenter Roeck July 5, 2018, 3:37 p.m. UTC | #1
On 07/05/2018 06:51 AM, Shilpasri G Bhat wrote:
> On-Chip-Controller(OCC) is an embedded micro-processor in POWER9 chip
> which measures various system and chip level sensors. These sensors
> comprises of environmental sensors (like power, temperature, current
> and voltage) and performance sensors (like utilization, frequency).
> All these sensors are copied to main memory at a regular interval of
> 100ms. OCC provides a way to select a group of sensors that is copied
> to the main memory to increase the update frequency of selected sensor
> groups. When a sensor-group is disabled, OCC will not copy it to main
> memory and those sensors read 0 values.
> 
> This patch provides support for enabling/disabling the sensor groups
> like power, temperature, current and voltage. This patch adds new
> per-senor sysfs attribute to disable and enable them.
> 
> Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
> ---
> Changes from v2:
> - Writes to first 'enable' attribute of the sensor group will affect all the
>    sensors in the group
> - Removed global mutex and made it per sensor-group
> 
>   drivers/hwmon/ibmpowernv.c | 184 ++++++++++++++++++++++++++++++++++++++-------
>   1 file changed, 155 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
> index f829dad..9c6adee 100644
> --- a/drivers/hwmon/ibmpowernv.c
> +++ b/drivers/hwmon/ibmpowernv.c
> @@ -73,6 +73,10 @@ enum sensors {
>   	struct attribute_group group;
>   	u32 attr_count;
>   	u32 hwmon_index;
> +	struct mutex mutex;
> +	u32 *gid;
> +	u32 nr_gid;
> +	bool enable;
>   } sensor_groups[] = {
>   	{ "fan"   },
>   	{ "temp"  },
> @@ -105,6 +109,9 @@ static ssize_t show_sensor(struct device *dev, struct device_attribute *devattr,
>   	ssize_t ret;
>   	u64 x;
>   
> +	if (!sensor_groups[sdata->type].enable)
> +		return -ENODATA;
> +
>   	ret =  opal_get_sensor_data_u64(sdata->id, &x);
>   
>   	if (ret)
> @@ -120,6 +127,46 @@ static ssize_t show_sensor(struct device *dev, struct device_attribute *devattr,
>   	return sprintf(buf, "%llu\n", x);
>   }
>   
> +static ssize_t show_enable(struct device *dev,
> +			   struct device_attribute *devattr, char *buf)
> +{
> +	struct sensor_data *sdata = container_of(devattr, struct sensor_data,
> +						 dev_attr);
> +
> +	return sprintf(buf, "%u\n", sensor_groups[sdata->type].enable);
> +}
> +
> +static ssize_t store_enable(struct device *dev,
> +			    struct device_attribute *devattr,
> +			    const char *buf, size_t count)
> +{
> +	struct sensor_data *sdata = container_of(devattr, struct sensor_data,
> +						 dev_attr);
> +	struct sensor_group *sg = &sensor_groups[sdata->type];
> +	int ret, i;
> +	bool data;
> +
> +	ret = kstrtobool(buf, &data);
> +	if (ret)
> +		return ret;
> +
> +	ret = mutex_lock_interruptible(&sg->mutex);
> +	if (ret)
> +		return ret;
> +
> +	if (data != sg->enable)
> +		for (i = 0; i < sg->nr_gid && !ret; i++)
> +			ret =  sensor_group_enable(sg->gid[i], data);
> +

Wouldn't it be better to have a separate attribute for each of the
affected groups if there can be more than one ? Just wondering.

The idea was to widen the scope to a point where there is a 1:1 match
between the hardware capabilities and attributes. Clearly having
a separate attribute for all sensors was inappropriate, but the code
above now suggests that a single attribute for all sensors may have
widened the scope too much (because the hardware can do better than
this).

Thanks,
Guenter

> +	if (!ret) {
> +		sg->enable = data;
> +		ret = count;
> +	}
> +
> +	mutex_unlock(&sg->mutex);
> +	return ret;
> +}
> +
>   static ssize_t show_label(struct device *dev, struct device_attribute *devattr,
>   			  char *buf)
>   {
> @@ -292,13 +339,68 @@ static u32 get_sensor_hwmon_index(struct sensor_data *sdata,
>   	return ++sensor_groups[sdata->type].hwmon_index;
>   }
>   
> +static int init_sensor_group_data(struct platform_device *pdev)
> +{
> +	struct device_node *groups, *sg;
> +	enum sensors type;
> +	int ret = 0, i;
> +
> +	for (i = 0; i < MAX_SENSOR_TYPE; i++) {
> +		sensor_groups[i].nr_gid = 0;
> +		sensor_groups[i].enable = true;
> +	}
> +
> +	groups = of_find_node_by_path("/ibm,opal/sensor-groups");
> +	if (!groups)
> +		return ret;
> +
> +	for (i = 0; i < MAX_SENSOR_TYPE; i++) {
> +		u32 gid[256];
> +		u32 id, size;
> +
> +		for_each_child_of_node(groups, sg) {
> +			type = get_sensor_type(sg);
> +			if (type != i)
> +				continue;
> +
> +			if (of_property_read_u32(sg, "sensor-group-id", &id))
> +				continue;
> +
> +			gid[sensor_groups[i].nr_gid++] = id;
> +		}
> +
> +		if (!sensor_groups[i].nr_gid)
> +			continue;
> +
> +		size = sensor_groups[i].nr_gid * sizeof(u32);
> +		sensor_groups[i].gid = devm_kzalloc(&pdev->dev, size,
> +						    GFP_KERNEL);
> +		if (!sensor_groups[i].gid) {
> +			ret = -ENOMEM;
> +			break;
> +		}
> +
> +		memcpy(sensor_groups[i].gid, gid, size);
> +		sensor_groups[i].enable = false;
> +		mutex_init(&sensor_groups[i].mutex);
> +	}
> +
> +	of_node_put(groups);
> +	return ret;
> +}
> +
>   static int populate_attr_groups(struct platform_device *pdev)
>   {
>   	struct platform_data *pdata = platform_get_drvdata(pdev);
>   	const struct attribute_group **pgroups = pdata->attr_groups;
>   	struct device_node *opal, *np;
> +	int ret;
>   	enum sensors type;
>   
> +	ret = init_sensor_group_data(pdev);
> +	if (ret)
> +		return ret;
> +
>   	opal = of_find_node_by_path("/ibm,opal/sensors");
>   	for_each_child_of_node(opal, np) {
>   		const char *label;
> @@ -313,7 +415,7 @@ static int populate_attr_groups(struct platform_device *pdev)
>   		sensor_groups[type].attr_count++;
>   
>   		/*
> -		 * add attributes for labels, min and max
> +		 * add attributes for labels, min, max and enable
>   		 */
>   		if (!of_property_read_string(np, "label", &label))
>   			sensor_groups[type].attr_count++;
> @@ -321,6 +423,8 @@ static int populate_attr_groups(struct platform_device *pdev)
>   			sensor_groups[type].attr_count++;
>   		if (of_find_property(np, "sensor-data-max", NULL))
>   			sensor_groups[type].attr_count++;
> +		if (sensor_groups[type].nr_gid)
> +			sensor_groups[type].attr_count++;
>   	}
>   
>   	of_node_put(opal);
> @@ -344,7 +448,10 @@ static int populate_attr_groups(struct platform_device *pdev)
>   static void create_hwmon_attr(struct sensor_data *sdata, const char *attr_name,
>   			      ssize_t (*show)(struct device *dev,
>   					      struct device_attribute *attr,
> -					      char *buf))
> +					      char *buf),
> +			    ssize_t (*store)(struct device *dev,
> +					     struct device_attribute *attr,
> +					     const char *buf, size_t count))
>   {
>   	snprintf(sdata->name, MAX_ATTR_LEN, "%s%d_%s",
>   		 sensor_groups[sdata->type].name, sdata->hwmon_index,
> @@ -352,8 +459,13 @@ static void create_hwmon_attr(struct sensor_data *sdata, const char *attr_name,
>   
>   	sysfs_attr_init(&sdata->dev_attr.attr);
>   	sdata->dev_attr.attr.name = sdata->name;
> -	sdata->dev_attr.attr.mode = S_IRUGO;
>   	sdata->dev_attr.show = show;
> +	if (store) {
> +		sdata->dev_attr.store = store;
> +		sdata->dev_attr.attr.mode = 0664;
> +	} else {
> +		sdata->dev_attr.attr.mode = 0444;
> +	}
>   }
>   
>   static void populate_sensor(struct sensor_data *sdata, int od, int hd, int sid,
> @@ -361,13 +473,16 @@ static void populate_sensor(struct sensor_data *sdata, int od, int hd, int sid,
>   			    const struct attribute_group *pgroup,
>   			    ssize_t (*show)(struct device *dev,
>   					    struct device_attribute *attr,
> -					    char *buf))
> +					    char *buf),
> +			    ssize_t (*store)(struct device *dev,
> +					     struct device_attribute *attr,
> +					     const char *buf, size_t count))
>   {
>   	sdata->id = sid;
>   	sdata->type = type;
>   	sdata->opal_index = od;
>   	sdata->hwmon_index = hd;
> -	create_hwmon_attr(sdata, attr_name, show);
> +	create_hwmon_attr(sdata, attr_name, show, store);
>   	pgroup->attrs[sensor_groups[type].attr_count++] = &sdata->dev_attr.attr;
>   }
>   
> @@ -408,18 +523,16 @@ static int create_device_attrs(struct platform_device *pdev)
>   	u32 count = 0;
>   	int err = 0;
>   
> -	opal = of_find_node_by_path("/ibm,opal/sensors");
>   	sdata = devm_kcalloc(&pdev->dev,
>   			     pdata->sensors_count, sizeof(*sdata),
>   			     GFP_KERNEL);
> -	if (!sdata) {
> -		err = -ENOMEM;
> -		goto exit_put_node;
> -	}
> +	if (!sdata)
> +		return -ENOMEM;
>   
> +	opal = of_find_node_by_path("/ibm,opal/sensors");
>   	for_each_child_of_node(opal, np) {
>   		const char *attr_name;
> -		u32 opal_index;
> +		u32 opal_index, hw_id;
>   		const char *label;
>   
>   		if (np->name == NULL)
> @@ -456,14 +569,11 @@ static int create_device_attrs(struct platform_device *pdev)
>   			opal_index = INVALID_INDEX;
>   		}
>   
> -		sdata[count].opal_index = opal_index;
> -		sdata[count].hwmon_index =
> -			get_sensor_hwmon_index(&sdata[count], sdata, count);
> -
> -		create_hwmon_attr(&sdata[count], attr_name, show_sensor);
> -
> -		pgroups[type]->attrs[sensor_groups[type].attr_count++] =
> -				&sdata[count++].dev_attr.attr;
> +		hw_id = get_sensor_hwmon_index(&sdata[count], sdata, count);
> +		populate_sensor(&sdata[count], opal_index, hw_id, sensor_id,
> +				attr_name, type, pgroups[type], show_sensor,
> +				NULL);
> +		count++;
>   
>   		if (!of_property_read_string(np, "label", &label)) {
>   			/*
> @@ -474,33 +584,49 @@ static int create_device_attrs(struct platform_device *pdev)
>   			 */
>   
>   			make_sensor_label(np, &sdata[count], label);
> -			populate_sensor(&sdata[count], opal_index,
> -					sdata[count - 1].hwmon_index,
> +			populate_sensor(&sdata[count], opal_index, hw_id,
>   					sensor_id, "label", type, pgroups[type],
> -					show_label);
> +					show_label, NULL);
>   			count++;
>   		}
>   
>   		if (!of_property_read_u32(np, "sensor-data-max", &sensor_id)) {
>   			attr_name = get_max_attr(type);
> -			populate_sensor(&sdata[count], opal_index,
> -					sdata[count - 1].hwmon_index,
> +			populate_sensor(&sdata[count], opal_index, hw_id,
>   					sensor_id, attr_name, type,
> -					pgroups[type], show_sensor);
> +					pgroups[type], show_sensor, NULL);
>   			count++;
>   		}
>   
>   		if (!of_property_read_u32(np, "sensor-data-min", &sensor_id)) {
>   			attr_name = get_min_attr(type);
> -			populate_sensor(&sdata[count], opal_index,
> -					sdata[count - 1].hwmon_index,
> +			populate_sensor(&sdata[count], opal_index, hw_id,
>   					sensor_id, attr_name, type,
> -					pgroups[type], show_sensor);
> +					pgroups[type], show_sensor, NULL);
>   			count++;
>   		}
> +
> +		if (sensor_groups[type].nr_gid) {
> +			ssize_t (*store)(struct device *dev,
> +					 struct device_attribute *attr,
> +					 const char *buf, size_t count);
> +
> +			if (!sensor_groups[type].enable) {
> +				sensor_groups[type].enable = true;
> +				store = store_enable;
> +			} else {
> +				store = NULL;
> +			}
> +
> +			sensor_groups[type].enable = true;
> +			populate_sensor(&sdata[count], opal_index, hw_id,
> +					sensor_id, "enable", type,
> +					pgroups[type], show_enable, store);
> +			count++;
> +		}
> +
>   	}
>   
> -exit_put_node:
>   	of_node_put(opal);
>   	return err;
>   }
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shilpasri G Bhat July 5, 2018, 5:35 p.m. UTC | #2
Hi,

On 07/05/2018 09:07 PM, Guenter Roeck wrote:
> On 07/05/2018 06:51 AM, Shilpasri G Bhat wrote:
>> On-Chip-Controller(OCC) is an embedded micro-processor in POWER9 chip
>> which measures various system and chip level sensors. These sensors
>> comprises of environmental sensors (like power, temperature, current
>> and voltage) and performance sensors (like utilization, frequency).
>> All these sensors are copied to main memory at a regular interval of
>> 100ms. OCC provides a way to select a group of sensors that is copied
>> to the main memory to increase the update frequency of selected sensor
>> groups. When a sensor-group is disabled, OCC will not copy it to main
>> memory and those sensors read 0 values.
>>
>> This patch provides support for enabling/disabling the sensor groups
>> like power, temperature, current and voltage. This patch adds new
>> per-senor sysfs attribute to disable and enable them.
>>
>> Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
>> ---
>> Changes from v2:
>> - Writes to first 'enable' attribute of the sensor group will affect all the
>>    sensors in the group
>> - Removed global mutex and made it per sensor-group
>>
>>   drivers/hwmon/ibmpowernv.c | 184 ++++++++++++++++++++++++++++++++++++++-------
>>   1 file changed, 155 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
>> index f829dad..9c6adee 100644
>> --- a/drivers/hwmon/ibmpowernv.c
>> +++ b/drivers/hwmon/ibmpowernv.c
>> @@ -73,6 +73,10 @@ enum sensors {
>>       struct attribute_group group;
>>       u32 attr_count;
>>       u32 hwmon_index;
>> +    struct mutex mutex;
>> +    u32 *gid;
>> +    u32 nr_gid;
>> +    bool enable;
>>   } sensor_groups[] = {
>>       { "fan"   },
>>       { "temp"  },
>> @@ -105,6 +109,9 @@ static ssize_t show_sensor(struct device *dev, struct
>> device_attribute *devattr,
>>       ssize_t ret;
>>       u64 x;
>>   +    if (!sensor_groups[sdata->type].enable)
>> +        return -ENODATA;
>> +
>>       ret =  opal_get_sensor_data_u64(sdata->id, &x);
>>         if (ret)
>> @@ -120,6 +127,46 @@ static ssize_t show_sensor(struct device *dev, struct
>> device_attribute *devattr,
>>       return sprintf(buf, "%llu\n", x);
>>   }
>>   +static ssize_t show_enable(struct device *dev,
>> +               struct device_attribute *devattr, char *buf)
>> +{
>> +    struct sensor_data *sdata = container_of(devattr, struct sensor_data,
>> +                         dev_attr);
>> +
>> +    return sprintf(buf, "%u\n", sensor_groups[sdata->type].enable);
>> +}
>> +
>> +static ssize_t store_enable(struct device *dev,
>> +                struct device_attribute *devattr,
>> +                const char *buf, size_t count)
>> +{
>> +    struct sensor_data *sdata = container_of(devattr, struct sensor_data,
>> +                         dev_attr);
>> +    struct sensor_group *sg = &sensor_groups[sdata->type];
>> +    int ret, i;
>> +    bool data;
>> +
>> +    ret = kstrtobool(buf, &data);
>> +    if (ret)
>> +        return ret;
>> +
>> +    ret = mutex_lock_interruptible(&sg->mutex);
>> +    if (ret)
>> +        return ret;
>> +
>> +    if (data != sg->enable)
>> +        for (i = 0; i < sg->nr_gid && !ret; i++)
>> +            ret =  sensor_group_enable(sg->gid[i], data);
>> +
> 
> Wouldn't it be better to have a separate attribute for each of the
> affected groups if there can be more than one ? Just wondering.
> 
> The idea was to widen the scope to a point where there is a 1:1 match
> between the hardware capabilities and attributes. Clearly having
> a separate attribute for all sensors was inappropriate, but the code
> above now suggests that a single attribute for all sensors may have
> widened the scope too much (because the hardware can do better than
> this).
> 

Yup it would be better to have 'enable' attribute for each sub-group.

Thanks and Regards,
Shilpa

> Thanks,
> Guenter
> 
>> +    if (!ret) {
>> +        sg->enable = data;
>> +        ret = count;
>> +    }
>> +
>> +    mutex_unlock(&sg->mutex);
>> +    return ret;
>> +}
>> +
>>   static ssize_t show_label(struct device *dev, struct device_attribute *devattr,
>>                 char *buf)
>>   {
>> @@ -292,13 +339,68 @@ static u32 get_sensor_hwmon_index(struct sensor_data
>> *sdata,
>>       return ++sensor_groups[sdata->type].hwmon_index;
>>   }
>>   +static int init_sensor_group_data(struct platform_device *pdev)
>> +{
>> +    struct device_node *groups, *sg;
>> +    enum sensors type;
>> +    int ret = 0, i;
>> +
>> +    for (i = 0; i < MAX_SENSOR_TYPE; i++) {
>> +        sensor_groups[i].nr_gid = 0;
>> +        sensor_groups[i].enable = true;
>> +    }
>> +
>> +    groups = of_find_node_by_path("/ibm,opal/sensor-groups");
>> +    if (!groups)
>> +        return ret;
>> +
>> +    for (i = 0; i < MAX_SENSOR_TYPE; i++) {
>> +        u32 gid[256];
>> +        u32 id, size;
>> +
>> +        for_each_child_of_node(groups, sg) {
>> +            type = get_sensor_type(sg);
>> +            if (type != i)
>> +                continue;
>> +
>> +            if (of_property_read_u32(sg, "sensor-group-id", &id))
>> +                continue;
>> +
>> +            gid[sensor_groups[i].nr_gid++] = id;
>> +        }
>> +
>> +        if (!sensor_groups[i].nr_gid)
>> +            continue;
>> +
>> +        size = sensor_groups[i].nr_gid * sizeof(u32);
>> +        sensor_groups[i].gid = devm_kzalloc(&pdev->dev, size,
>> +                            GFP_KERNEL);
>> +        if (!sensor_groups[i].gid) {
>> +            ret = -ENOMEM;
>> +            break;
>> +        }
>> +
>> +        memcpy(sensor_groups[i].gid, gid, size);
>> +        sensor_groups[i].enable = false;
>> +        mutex_init(&sensor_groups[i].mutex);
>> +    }
>> +
>> +    of_node_put(groups);
>> +    return ret;
>> +}
>> +
>>   static int populate_attr_groups(struct platform_device *pdev)
>>   {
>>       struct platform_data *pdata = platform_get_drvdata(pdev);
>>       const struct attribute_group **pgroups = pdata->attr_groups;
>>       struct device_node *opal, *np;
>> +    int ret;
>>       enum sensors type;
>>   +    ret = init_sensor_group_data(pdev);
>> +    if (ret)
>> +        return ret;
>> +
>>       opal = of_find_node_by_path("/ibm,opal/sensors");
>>       for_each_child_of_node(opal, np) {
>>           const char *label;
>> @@ -313,7 +415,7 @@ static int populate_attr_groups(struct platform_device *pdev)
>>           sensor_groups[type].attr_count++;
>>             /*
>> -         * add attributes for labels, min and max
>> +         * add attributes for labels, min, max and enable
>>            */
>>           if (!of_property_read_string(np, "label", &label))
>>               sensor_groups[type].attr_count++;
>> @@ -321,6 +423,8 @@ static int populate_attr_groups(struct platform_device *pdev)
>>               sensor_groups[type].attr_count++;
>>           if (of_find_property(np, "sensor-data-max", NULL))
>>               sensor_groups[type].attr_count++;
>> +        if (sensor_groups[type].nr_gid)
>> +            sensor_groups[type].attr_count++;
>>       }
>>         of_node_put(opal);
>> @@ -344,7 +448,10 @@ static int populate_attr_groups(struct platform_device
>> *pdev)
>>   static void create_hwmon_attr(struct sensor_data *sdata, const char *attr_name,
>>                     ssize_t (*show)(struct device *dev,
>>                             struct device_attribute *attr,
>> -                          char *buf))
>> +                          char *buf),
>> +                ssize_t (*store)(struct device *dev,
>> +                         struct device_attribute *attr,
>> +                         const char *buf, size_t count))
>>   {
>>       snprintf(sdata->name, MAX_ATTR_LEN, "%s%d_%s",
>>            sensor_groups[sdata->type].name, sdata->hwmon_index,
>> @@ -352,8 +459,13 @@ static void create_hwmon_attr(struct sensor_data *sdata,
>> const char *attr_name,
>>         sysfs_attr_init(&sdata->dev_attr.attr);
>>       sdata->dev_attr.attr.name = sdata->name;
>> -    sdata->dev_attr.attr.mode = S_IRUGO;
>>       sdata->dev_attr.show = show;
>> +    if (store) {
>> +        sdata->dev_attr.store = store;
>> +        sdata->dev_attr.attr.mode = 0664;
>> +    } else {
>> +        sdata->dev_attr.attr.mode = 0444;
>> +    }
>>   }
>>     static void populate_sensor(struct sensor_data *sdata, int od, int hd, int
>> sid,
>> @@ -361,13 +473,16 @@ static void populate_sensor(struct sensor_data *sdata,
>> int od, int hd, int sid,
>>                   const struct attribute_group *pgroup,
>>                   ssize_t (*show)(struct device *dev,
>>                           struct device_attribute *attr,
>> -                        char *buf))
>> +                        char *buf),
>> +                ssize_t (*store)(struct device *dev,
>> +                         struct device_attribute *attr,
>> +                         const char *buf, size_t count))
>>   {
>>       sdata->id = sid;
>>       sdata->type = type;
>>       sdata->opal_index = od;
>>       sdata->hwmon_index = hd;
>> -    create_hwmon_attr(sdata, attr_name, show);
>> +    create_hwmon_attr(sdata, attr_name, show, store);
>>       pgroup->attrs[sensor_groups[type].attr_count++] = &sdata->dev_attr.attr;
>>   }
>>   @@ -408,18 +523,16 @@ static int create_device_attrs(struct platform_device
>> *pdev)
>>       u32 count = 0;
>>       int err = 0;
>>   -    opal = of_find_node_by_path("/ibm,opal/sensors");
>>       sdata = devm_kcalloc(&pdev->dev,
>>                    pdata->sensors_count, sizeof(*sdata),
>>                    GFP_KERNEL);
>> -    if (!sdata) {
>> -        err = -ENOMEM;
>> -        goto exit_put_node;
>> -    }
>> +    if (!sdata)
>> +        return -ENOMEM;
>>   +    opal = of_find_node_by_path("/ibm,opal/sensors");
>>       for_each_child_of_node(opal, np) {
>>           const char *attr_name;
>> -        u32 opal_index;
>> +        u32 opal_index, hw_id;
>>           const char *label;
>>             if (np->name == NULL)
>> @@ -456,14 +569,11 @@ static int create_device_attrs(struct platform_device
>> *pdev)
>>               opal_index = INVALID_INDEX;
>>           }
>>   -        sdata[count].opal_index = opal_index;
>> -        sdata[count].hwmon_index =
>> -            get_sensor_hwmon_index(&sdata[count], sdata, count);
>> -
>> -        create_hwmon_attr(&sdata[count], attr_name, show_sensor);
>> -
>> -        pgroups[type]->attrs[sensor_groups[type].attr_count++] =
>> -                &sdata[count++].dev_attr.attr;
>> +        hw_id = get_sensor_hwmon_index(&sdata[count], sdata, count);
>> +        populate_sensor(&sdata[count], opal_index, hw_id, sensor_id,
>> +                attr_name, type, pgroups[type], show_sensor,
>> +                NULL);
>> +        count++;
>>             if (!of_property_read_string(np, "label", &label)) {
>>               /*
>> @@ -474,33 +584,49 @@ static int create_device_attrs(struct platform_device
>> *pdev)
>>                */
>>                 make_sensor_label(np, &sdata[count], label);
>> -            populate_sensor(&sdata[count], opal_index,
>> -                    sdata[count - 1].hwmon_index,
>> +            populate_sensor(&sdata[count], opal_index, hw_id,
>>                       sensor_id, "label", type, pgroups[type],
>> -                    show_label);
>> +                    show_label, NULL);
>>               count++;
>>           }
>>             if (!of_property_read_u32(np, "sensor-data-max", &sensor_id)) {
>>               attr_name = get_max_attr(type);
>> -            populate_sensor(&sdata[count], opal_index,
>> -                    sdata[count - 1].hwmon_index,
>> +            populate_sensor(&sdata[count], opal_index, hw_id,
>>                       sensor_id, attr_name, type,
>> -                    pgroups[type], show_sensor);
>> +                    pgroups[type], show_sensor, NULL);
>>               count++;
>>           }
>>             if (!of_property_read_u32(np, "sensor-data-min", &sensor_id)) {
>>               attr_name = get_min_attr(type);
>> -            populate_sensor(&sdata[count], opal_index,
>> -                    sdata[count - 1].hwmon_index,
>> +            populate_sensor(&sdata[count], opal_index, hw_id,
>>                       sensor_id, attr_name, type,
>> -                    pgroups[type], show_sensor);
>> +                    pgroups[type], show_sensor, NULL);
>>               count++;
>>           }
>> +
>> +        if (sensor_groups[type].nr_gid) {
>> +            ssize_t (*store)(struct device *dev,
>> +                     struct device_attribute *attr,
>> +                     const char *buf, size_t count);
>> +
>> +            if (!sensor_groups[type].enable) {
>> +                sensor_groups[type].enable = true;
>> +                store = store_enable;
>> +            } else {
>> +                store = NULL;
>> +            }
>> +
>> +            sensor_groups[type].enable = true;
>> +            populate_sensor(&sdata[count], opal_index, hw_id,
>> +                    sensor_id, "enable", type,
>> +                    pgroups[type], show_enable, store);
>> +            count++;
>> +        }
>> +
>>       }
>>   -exit_put_node:
>>       of_node_put(opal);
>>       return err;
>>   }
>>
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
index f829dad..9c6adee 100644
--- a/drivers/hwmon/ibmpowernv.c
+++ b/drivers/hwmon/ibmpowernv.c
@@ -73,6 +73,10 @@  enum sensors {
 	struct attribute_group group;
 	u32 attr_count;
 	u32 hwmon_index;
+	struct mutex mutex;
+	u32 *gid;
+	u32 nr_gid;
+	bool enable;
 } sensor_groups[] = {
 	{ "fan"   },
 	{ "temp"  },
@@ -105,6 +109,9 @@  static ssize_t show_sensor(struct device *dev, struct device_attribute *devattr,
 	ssize_t ret;
 	u64 x;
 
+	if (!sensor_groups[sdata->type].enable)
+		return -ENODATA;
+
 	ret =  opal_get_sensor_data_u64(sdata->id, &x);
 
 	if (ret)
@@ -120,6 +127,46 @@  static ssize_t show_sensor(struct device *dev, struct device_attribute *devattr,
 	return sprintf(buf, "%llu\n", x);
 }
 
+static ssize_t show_enable(struct device *dev,
+			   struct device_attribute *devattr, char *buf)
+{
+	struct sensor_data *sdata = container_of(devattr, struct sensor_data,
+						 dev_attr);
+
+	return sprintf(buf, "%u\n", sensor_groups[sdata->type].enable);
+}
+
+static ssize_t store_enable(struct device *dev,
+			    struct device_attribute *devattr,
+			    const char *buf, size_t count)
+{
+	struct sensor_data *sdata = container_of(devattr, struct sensor_data,
+						 dev_attr);
+	struct sensor_group *sg = &sensor_groups[sdata->type];
+	int ret, i;
+	bool data;
+
+	ret = kstrtobool(buf, &data);
+	if (ret)
+		return ret;
+
+	ret = mutex_lock_interruptible(&sg->mutex);
+	if (ret)
+		return ret;
+
+	if (data != sg->enable)
+		for (i = 0; i < sg->nr_gid && !ret; i++)
+			ret =  sensor_group_enable(sg->gid[i], data);
+
+	if (!ret) {
+		sg->enable = data;
+		ret = count;
+	}
+
+	mutex_unlock(&sg->mutex);
+	return ret;
+}
+
 static ssize_t show_label(struct device *dev, struct device_attribute *devattr,
 			  char *buf)
 {
@@ -292,13 +339,68 @@  static u32 get_sensor_hwmon_index(struct sensor_data *sdata,
 	return ++sensor_groups[sdata->type].hwmon_index;
 }
 
+static int init_sensor_group_data(struct platform_device *pdev)
+{
+	struct device_node *groups, *sg;
+	enum sensors type;
+	int ret = 0, i;
+
+	for (i = 0; i < MAX_SENSOR_TYPE; i++) {
+		sensor_groups[i].nr_gid = 0;
+		sensor_groups[i].enable = true;
+	}
+
+	groups = of_find_node_by_path("/ibm,opal/sensor-groups");
+	if (!groups)
+		return ret;
+
+	for (i = 0; i < MAX_SENSOR_TYPE; i++) {
+		u32 gid[256];
+		u32 id, size;
+
+		for_each_child_of_node(groups, sg) {
+			type = get_sensor_type(sg);
+			if (type != i)
+				continue;
+
+			if (of_property_read_u32(sg, "sensor-group-id", &id))
+				continue;
+
+			gid[sensor_groups[i].nr_gid++] = id;
+		}
+
+		if (!sensor_groups[i].nr_gid)
+			continue;
+
+		size = sensor_groups[i].nr_gid * sizeof(u32);
+		sensor_groups[i].gid = devm_kzalloc(&pdev->dev, size,
+						    GFP_KERNEL);
+		if (!sensor_groups[i].gid) {
+			ret = -ENOMEM;
+			break;
+		}
+
+		memcpy(sensor_groups[i].gid, gid, size);
+		sensor_groups[i].enable = false;
+		mutex_init(&sensor_groups[i].mutex);
+	}
+
+	of_node_put(groups);
+	return ret;
+}
+
 static int populate_attr_groups(struct platform_device *pdev)
 {
 	struct platform_data *pdata = platform_get_drvdata(pdev);
 	const struct attribute_group **pgroups = pdata->attr_groups;
 	struct device_node *opal, *np;
+	int ret;
 	enum sensors type;
 
+	ret = init_sensor_group_data(pdev);
+	if (ret)
+		return ret;
+
 	opal = of_find_node_by_path("/ibm,opal/sensors");
 	for_each_child_of_node(opal, np) {
 		const char *label;
@@ -313,7 +415,7 @@  static int populate_attr_groups(struct platform_device *pdev)
 		sensor_groups[type].attr_count++;
 
 		/*
-		 * add attributes for labels, min and max
+		 * add attributes for labels, min, max and enable
 		 */
 		if (!of_property_read_string(np, "label", &label))
 			sensor_groups[type].attr_count++;
@@ -321,6 +423,8 @@  static int populate_attr_groups(struct platform_device *pdev)
 			sensor_groups[type].attr_count++;
 		if (of_find_property(np, "sensor-data-max", NULL))
 			sensor_groups[type].attr_count++;
+		if (sensor_groups[type].nr_gid)
+			sensor_groups[type].attr_count++;
 	}
 
 	of_node_put(opal);
@@ -344,7 +448,10 @@  static int populate_attr_groups(struct platform_device *pdev)
 static void create_hwmon_attr(struct sensor_data *sdata, const char *attr_name,
 			      ssize_t (*show)(struct device *dev,
 					      struct device_attribute *attr,
-					      char *buf))
+					      char *buf),
+			    ssize_t (*store)(struct device *dev,
+					     struct device_attribute *attr,
+					     const char *buf, size_t count))
 {
 	snprintf(sdata->name, MAX_ATTR_LEN, "%s%d_%s",
 		 sensor_groups[sdata->type].name, sdata->hwmon_index,
@@ -352,8 +459,13 @@  static void create_hwmon_attr(struct sensor_data *sdata, const char *attr_name,
 
 	sysfs_attr_init(&sdata->dev_attr.attr);
 	sdata->dev_attr.attr.name = sdata->name;
-	sdata->dev_attr.attr.mode = S_IRUGO;
 	sdata->dev_attr.show = show;
+	if (store) {
+		sdata->dev_attr.store = store;
+		sdata->dev_attr.attr.mode = 0664;
+	} else {
+		sdata->dev_attr.attr.mode = 0444;
+	}
 }
 
 static void populate_sensor(struct sensor_data *sdata, int od, int hd, int sid,
@@ -361,13 +473,16 @@  static void populate_sensor(struct sensor_data *sdata, int od, int hd, int sid,
 			    const struct attribute_group *pgroup,
 			    ssize_t (*show)(struct device *dev,
 					    struct device_attribute *attr,
-					    char *buf))
+					    char *buf),
+			    ssize_t (*store)(struct device *dev,
+					     struct device_attribute *attr,
+					     const char *buf, size_t count))
 {
 	sdata->id = sid;
 	sdata->type = type;
 	sdata->opal_index = od;
 	sdata->hwmon_index = hd;
-	create_hwmon_attr(sdata, attr_name, show);
+	create_hwmon_attr(sdata, attr_name, show, store);
 	pgroup->attrs[sensor_groups[type].attr_count++] = &sdata->dev_attr.attr;
 }
 
@@ -408,18 +523,16 @@  static int create_device_attrs(struct platform_device *pdev)
 	u32 count = 0;
 	int err = 0;
 
-	opal = of_find_node_by_path("/ibm,opal/sensors");
 	sdata = devm_kcalloc(&pdev->dev,
 			     pdata->sensors_count, sizeof(*sdata),
 			     GFP_KERNEL);
-	if (!sdata) {
-		err = -ENOMEM;
-		goto exit_put_node;
-	}
+	if (!sdata)
+		return -ENOMEM;
 
+	opal = of_find_node_by_path("/ibm,opal/sensors");
 	for_each_child_of_node(opal, np) {
 		const char *attr_name;
-		u32 opal_index;
+		u32 opal_index, hw_id;
 		const char *label;
 
 		if (np->name == NULL)
@@ -456,14 +569,11 @@  static int create_device_attrs(struct platform_device *pdev)
 			opal_index = INVALID_INDEX;
 		}
 
-		sdata[count].opal_index = opal_index;
-		sdata[count].hwmon_index =
-			get_sensor_hwmon_index(&sdata[count], sdata, count);
-
-		create_hwmon_attr(&sdata[count], attr_name, show_sensor);
-
-		pgroups[type]->attrs[sensor_groups[type].attr_count++] =
-				&sdata[count++].dev_attr.attr;
+		hw_id = get_sensor_hwmon_index(&sdata[count], sdata, count);
+		populate_sensor(&sdata[count], opal_index, hw_id, sensor_id,
+				attr_name, type, pgroups[type], show_sensor,
+				NULL);
+		count++;
 
 		if (!of_property_read_string(np, "label", &label)) {
 			/*
@@ -474,33 +584,49 @@  static int create_device_attrs(struct platform_device *pdev)
 			 */
 
 			make_sensor_label(np, &sdata[count], label);
-			populate_sensor(&sdata[count], opal_index,
-					sdata[count - 1].hwmon_index,
+			populate_sensor(&sdata[count], opal_index, hw_id,
 					sensor_id, "label", type, pgroups[type],
-					show_label);
+					show_label, NULL);
 			count++;
 		}
 
 		if (!of_property_read_u32(np, "sensor-data-max", &sensor_id)) {
 			attr_name = get_max_attr(type);
-			populate_sensor(&sdata[count], opal_index,
-					sdata[count - 1].hwmon_index,
+			populate_sensor(&sdata[count], opal_index, hw_id,
 					sensor_id, attr_name, type,
-					pgroups[type], show_sensor);
+					pgroups[type], show_sensor, NULL);
 			count++;
 		}
 
 		if (!of_property_read_u32(np, "sensor-data-min", &sensor_id)) {
 			attr_name = get_min_attr(type);
-			populate_sensor(&sdata[count], opal_index,
-					sdata[count - 1].hwmon_index,
+			populate_sensor(&sdata[count], opal_index, hw_id,
 					sensor_id, attr_name, type,
-					pgroups[type], show_sensor);
+					pgroups[type], show_sensor, NULL);
 			count++;
 		}
+
+		if (sensor_groups[type].nr_gid) {
+			ssize_t (*store)(struct device *dev,
+					 struct device_attribute *attr,
+					 const char *buf, size_t count);
+
+			if (!sensor_groups[type].enable) {
+				sensor_groups[type].enable = true;
+				store = store_enable;
+			} else {
+				store = NULL;
+			}
+
+			sensor_groups[type].enable = true;
+			populate_sensor(&sdata[count], opal_index, hw_id,
+					sensor_id, "enable", type,
+					pgroups[type], show_enable, store);
+			count++;
+		}
+
 	}
 
-exit_put_node:
 	of_node_put(opal);
 	return err;
 }