Re: [PATCH 1/3] hwmon: (pmbus/ibm-cffps) Add new manufacturer debugfs entries

[Eddie James <eajames@xxxxxxxxxxxxx>]

On 1/4/20 10:26 AM, Guenter Roeck wrote:
> On Thu, Dec 19, 2019 at 02:50:05PM -0600, Eddie James wrote:
> > Add support for a number of manufacturer-specific registers in the
> > debugfs entries, as well as support to read and write the
> > PMBUS_ON_OFF_CONFIG register through debugfs.
> >
> > Signed-off-by: Eddie James <eajames@xxxxxxxxxxxxx>
> I assume you know what you are doing, letting the user write anything
> into on_off_config. Applied to -next.


Thanks for merging the series! Yep, that's the requirement I was given...


Cheers,

Eddie


> Guenter
>
> > ---
> >   drivers/hwmon/pmbus/ibm-cffps.c | 74 ++++++++++++++++++++++++++++++++++++++---
> >   1 file changed, 70 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/hwmon/pmbus/ibm-cffps.c b/drivers/hwmon/pmbus/ibm-cffps.c
> > index d359b76..a564be9 100644
> > --- a/drivers/hwmon/pmbus/ibm-cffps.c
> > +++ b/drivers/hwmon/pmbus/ibm-cffps.c
> > @@ -20,7 +20,9 @@
> >   
> >   #define CFFPS_FRU_CMD				0x9A
> >   #define CFFPS_PN_CMD				0x9B
> > +#define CFFPS_HEADER_CMD			0x9C
> >   #define CFFPS_SN_CMD				0x9E
> > +#define CFFPS_MAX_POWER_OUT_CMD			0xA7
> >   #define CFFPS_CCIN_CMD				0xBD
> >   #define CFFPS_FW_CMD				0xFA
> >   #define CFFPS1_FW_NUM_BYTES			4
> > @@ -57,9 +59,12 @@ enum {
> >   	CFFPS_DEBUGFS_INPUT_HISTORY = 0,
> >   	CFFPS_DEBUGFS_FRU,
> >   	CFFPS_DEBUGFS_PN,
> > +	CFFPS_DEBUGFS_HEADER,
> >   	CFFPS_DEBUGFS_SN,
> > +	CFFPS_DEBUGFS_MAX_POWER_OUT,
> >   	CFFPS_DEBUGFS_CCIN,
> >   	CFFPS_DEBUGFS_FW,
> > +	CFFPS_DEBUGFS_ON_OFF_CONFIG,
> >   	CFFPS_DEBUGFS_NUM_ENTRIES
> >   };
> >   
> > @@ -136,15 +141,15 @@ static ssize_t ibm_cffps_read_input_history(struct ibm_cffps *psu,
> >   				       psu->input_history.byte_count);
> >   }
> >   
> > -static ssize_t ibm_cffps_debugfs_op(struct file *file, char __user *buf,
> > -				    size_t count, loff_t *ppos)
> > +static ssize_t ibm_cffps_debugfs_read(struct file *file, char __user *buf,
> > +				      size_t count, loff_t *ppos)
> >   {
> >   	u8 cmd;
> >   	int i, rc;
> >   	int *idxp = file->private_data;
> >   	int idx = *idxp;
> >   	struct ibm_cffps *psu = to_psu(idxp, idx);
> > -	char data[I2C_SMBUS_BLOCK_MAX] = { 0 };
> > +	char data[I2C_SMBUS_BLOCK_MAX + 2] = { 0 };
> >   
> >   	pmbus_set_page(psu->client, 0);
> >   
> > @@ -157,9 +162,20 @@ static ssize_t ibm_cffps_debugfs_op(struct file *file, char __user *buf,
> >   	case CFFPS_DEBUGFS_PN:
> >   		cmd = CFFPS_PN_CMD;
> >   		break;
> > +	case CFFPS_DEBUGFS_HEADER:
> > +		cmd = CFFPS_HEADER_CMD;
> > +		break;
> >   	case CFFPS_DEBUGFS_SN:
> >   		cmd = CFFPS_SN_CMD;
> >   		break;
> > +	case CFFPS_DEBUGFS_MAX_POWER_OUT:
> > +		rc = i2c_smbus_read_word_swapped(psu->client,
> > +						 CFFPS_MAX_POWER_OUT_CMD);
> > +		if (rc < 0)
> > +			return rc;
> > +
> > +		rc = snprintf(data, I2C_SMBUS_BLOCK_MAX, "%d", rc);
> > +		goto done;
> >   	case CFFPS_DEBUGFS_CCIN:
> >   		rc = i2c_smbus_read_word_swapped(psu->client, CFFPS_CCIN_CMD);
> >   		if (rc < 0)
> > @@ -199,6 +215,14 @@ static ssize_t ibm_cffps_debugfs_op(struct file *file, char __user *buf,
> >   			return -EOPNOTSUPP;
> >   		}
> >   		goto done;
> > +	case CFFPS_DEBUGFS_ON_OFF_CONFIG:
> > +		rc = i2c_smbus_read_byte_data(psu->client,
> > +					      PMBUS_ON_OFF_CONFIG);
> > +		if (rc < 0)
> > +			return rc;
> > +
> > +		rc = snprintf(data, 3, "%02x", rc);
> > +		goto done;
> >   	default:
> >   		return -EINVAL;
> >   	}
> > @@ -214,9 +238,42 @@ static ssize_t ibm_cffps_debugfs_op(struct file *file, char __user *buf,
> >   	return simple_read_from_buffer(buf, count, ppos, data, rc);
> >   }
> >   
> > +static ssize_t ibm_cffps_debugfs_write(struct file *file,
> > +				       const char __user *buf, size_t count,
> > +				       loff_t *ppos)
> > +{
> > +	u8 data;
> > +	ssize_t rc;
> > +	int *idxp = file->private_data;
> > +	int idx = *idxp;
> > +	struct ibm_cffps *psu = to_psu(idxp, idx);
> > +
> > +	switch (idx) {
> > +	case CFFPS_DEBUGFS_ON_OFF_CONFIG:
> > +		pmbus_set_page(psu->client, 0);
> > +
> > +		rc = simple_write_to_buffer(&data, 1, ppos, buf, count);
> > +		if (rc < 0)
> > +			return rc;
> > +
> > +		rc = i2c_smbus_write_byte_data(psu->client,
> > +					       PMBUS_ON_OFF_CONFIG, data);
> > +		if (rc)
> > +			return rc;
> > +
> > +		rc = 1;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	return rc;
> > +}
> > +
> >   static const struct file_operations ibm_cffps_fops = {
> >   	.llseek = noop_llseek,
> > -	.read = ibm_cffps_debugfs_op,
> > +	.read = ibm_cffps_debugfs_read,
> > +	.write = ibm_cffps_debugfs_write,
> >   	.open = simple_open,
> >   };
> >   
> > @@ -486,15 +543,24 @@ static int ibm_cffps_probe(struct i2c_client *client,
> >   	debugfs_create_file("part_number", 0444, ibm_cffps_dir,
> >   			    &psu->debugfs_entries[CFFPS_DEBUGFS_PN],
> >   			    &ibm_cffps_fops);
> > +	debugfs_create_file("header", 0444, ibm_cffps_dir,
> > +			    &psu->debugfs_entries[CFFPS_DEBUGFS_HEADER],
> > +			    &ibm_cffps_fops);
> >   	debugfs_create_file("serial_number", 0444, ibm_cffps_dir,
> >   			    &psu->debugfs_entries[CFFPS_DEBUGFS_SN],
> >   			    &ibm_cffps_fops);
> > +	debugfs_create_file("max_power_out", 0444, ibm_cffps_dir,
> > +			    &psu->debugfs_entries[CFFPS_DEBUGFS_MAX_POWER_OUT],
> > +			    &ibm_cffps_fops);
> >   	debugfs_create_file("ccin", 0444, ibm_cffps_dir,
> >   			    &psu->debugfs_entries[CFFPS_DEBUGFS_CCIN],
> >   			    &ibm_cffps_fops);
> >   	debugfs_create_file("fw_version", 0444, ibm_cffps_dir,
> >   			    &psu->debugfs_entries[CFFPS_DEBUGFS_FW],
> >   			    &ibm_cffps_fops);
> > +	debugfs_create_file("on_off_config", 0644, ibm_cffps_dir,
> > +			    &psu->debugfs_entries[CFFPS_DEBUGFS_ON_OFF_CONFIG],
> > +			    &ibm_cffps_fops);
> >   
> >   	return 0;
> >   }