[PATCH linux dev-4.10 v3 2/5] drivers/hwmon/pmbus: ibmps: Add non-hwmon attributes

[Eddie James]

On 08/03/2017 07:33 PM, Andrew Jeffery wrote:
> On Wed, 2017-08-02 at 15:25 -0500, Eddie James wrote:
>>> From: "Edward A. James" <eajames at us.ibm.com>
>> Add sysfs entries to dump out PS registers
> Why are we working against what the pmbus subsystem already gives us?
> Why not augment support in pmbus core to give us the information we
> need in a manner acceptable to upstream?

Yea, that would be great. I feel like we're a little short on time for 
that, as we need this functionality basically now. But we can give it a 
shot.

>
> Flipping to the other end of the spectrum: The device only has one
> page. We can just use `i2cget -f -y ...` without risk of corruption.

Yea, I just get a lot of requests to have it all in the device driver. I 
guess we need to just push back and ask people to use i2c tools. But we 
will have to solve this for the UCD driver.

>
>>   and clear faults.
> Can you describe the motivation for this? I feel like it would be worth
> having this as a separate patch, and to the core rather than in an
> arbitrary driver.

Users need to be able to clear faults, that was just on my list of 
requirements :) They could probably use i2cset though.

>
> Cheers,
>
> Andrew
>
>>> Signed-off-by: Edward A. James <eajames at us.ibm.com>
>> ---
>>   drivers/hwmon/pmbus/ibmps.c | 78 +++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 78 insertions(+)
>>
>> diff --git a/drivers/hwmon/pmbus/ibmps.c b/drivers/hwmon/pmbus/ibmps.c
>> index 1928dd9..f55d2bf 100644
>> --- a/drivers/hwmon/pmbus/ibmps.c
>> +++ b/drivers/hwmon/pmbus/ibmps.c
>> @@ -9,8 +9,10 @@
>>   
>>   #include <linux/device.h>
>>   #include <linux/i2c.h>
>> +#include <linux/hwmon-sysfs.h>
>>   #include <linux/jiffies.h>
>>   #include <linux/module.h>
>> +#include <linux/sysfs.h>
>>   
>>   #include "pmbus.h"
>>   
>> @@ -122,14 +124,90 @@ static int ibmps_read_word_data(struct i2c_client *client, int page, int reg)
>>>   	.read_word_data = ibmps_read_word_data,
>>   };
>>   
>> +static ssize_t ibmps_clear_faults(struct device *dev,
>>> +				  struct device_attribute *dev_attr,
>>> +				  const char *buf, size_t count)
>> +{
>>> +	struct i2c_client *client = to_i2c_client(dev);
>> +
>>> +	pmbus_clear_faults(client);
>> +
>>> +	return count;
>> +}
>> +static DEVICE_ATTR(clear_faults, 0200, NULL, ibmps_clear_faults);
>> +
>> +static ssize_t ibmps_show_status_word(struct device *dev,
>>> +				      struct device_attribute *dev_attr,
>>> +				      char *buf)
>> +{
>>> +	int rc;
>>> +	struct i2c_client *client = to_i2c_client(dev);
>> +
>>> +	rc = pmbus_read_word_data(client, 0, PMBUS_STATUS_WORD);
>>> +	if (rc < 0)
>>> +		return rc;
>> +
>>> +	return snprintf(buf, PAGE_SIZE - 1, "%04x\n", rc);
>> +}
>> +static DEVICE_ATTR(status_word, 0444, ibmps_show_status_word, NULL);
>> +
>> +static ssize_t ibmps_show_status_byte(struct device *dev,
>>> +				      struct device_attribute *dev_attr,
>>> +				      char *buf)
>> +{
>>> +	int rc;
>>> +	struct i2c_client *client = to_i2c_client(dev);
>>> +	struct sensor_device_attribute *sattr = to_sensor_dev_attr(dev_attr);
>> +
>>> +	rc = pmbus_read_byte_data(client, 0, PMBUS_STATUS_VOUT + sattr->index);
>>> +	if (rc < 0)
>>> +		return rc;
>> +
>>> +	return snprintf(buf, PAGE_SIZE - 1, "%02x\n", rc);
>> +}
>> +
>> +static SENSOR_DEVICE_ATTR(status_vout, 0444, ibmps_show_status_byte, NULL, 0);
>> +static SENSOR_DEVICE_ATTR(status_iout, 0444, ibmps_show_status_byte, NULL, 1);
>> +static SENSOR_DEVICE_ATTR(status_input, 0444, ibmps_show_status_byte, NULL, 2);
>> +static SENSOR_DEVICE_ATTR(status_temp, 0444, ibmps_show_status_byte, NULL, 3);
>> +static SENSOR_DEVICE_ATTR(status_cml, 0444, ibmps_show_status_byte, NULL, 4);
>> +static SENSOR_DEVICE_ATTR(status_other, 0444, ibmps_show_status_byte, NULL, 5);
>> +static SENSOR_DEVICE_ATTR(status_mfr, 0444, ibmps_show_status_byte, NULL, 6);
>> +static SENSOR_DEVICE_ATTR(status_fan, 0444, ibmps_show_status_byte, NULL, 7);
>> +
>> +static struct attribute *ibmps_attributes[] = {
>>> +	&dev_attr_clear_faults.attr,
>>> +	&dev_attr_status_word.attr,
>>> +	&sensor_dev_attr_status_vout.dev_attr.attr,
>>> +	&sensor_dev_attr_status_iout.dev_attr.attr,
>>> +	&sensor_dev_attr_status_input.dev_attr.attr,
>>> +	&sensor_dev_attr_status_temp.dev_attr.attr,
>>> +	&sensor_dev_attr_status_cml.dev_attr.attr,
>>> +	&sensor_dev_attr_status_other.dev_attr.attr,
>>> +	&sensor_dev_attr_status_mfr.dev_attr.attr,
>>> +	&sensor_dev_attr_status_fan.dev_attr.attr,
>>> +	NULL
>> +};
>> +
>> +static const struct attribute_group ibmps_attribute_group = {
>>> +	.attrs = ibmps_attributes,
>> +};
>> +
>>   static int ibmps_probe(struct i2c_client *client,
>>>   		       const struct i2c_device_id *id)
>>   {
>>> +	int rc = sysfs_create_group(&client->dev.kobj,
>>> +				    &ibmps_attribute_group);
>>> +	if (rc)
>>> +		return rc;
>> +
>>>   	return pmbus_do_probe(client, id, &ibmps_info);
>>   }
>>   
>>   static int ibmps_remove(struct i2c_client *client)
>>   {
>>> +	sysfs_remove_group(&client->dev.kobj, &ibmps_attribute_group);
>> +
>>>   	return pmbus_do_remove(client);
>>   }
>>