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

[Andrew Jeffery]

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?

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.

>  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.

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);
>  }
>  
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20170804/cc6cdaa5/attachment.sig>