Re: [PATCH 2/3] lm25066: export sysfs attribute for SAMPLES_FOR_AVG

[Nicolin Chen <nicoleotsuka@gmail.com>]

On Wed, Apr 10, 2019 at 05:55:19PM -0700, Guenter Roeck wrote:

> > +static ssize_t samples_for_avg_show(struct device *dev,
> > +				    struct device_attribute *attr, char *buf)
> > +{
> > +	struct i2c_client *client = to_i2c_client(dev->parent);
> > +	int ret;
> > +
> > +	ret = pmbus_read_byte_data(client, 0, LM25066_SAMPLES_FOR_AVG);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return sprintf(buf, "%d\n", 1 << ret);
> > +}
> > +
> > +static ssize_t samples_for_avg_store(struct device *dev,
> > +				     struct device_attribute *attr,
> > +				     const char *buf, size_t count)
> > +{
> > +	struct i2c_client *client = to_i2c_client(dev->parent);
> > +	int ret, val;
> > +
> > +	ret = kstrtoint(buf, 0, &val);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = pmbus_write_byte_data(client, 0, LM25066_SAMPLES_FOR_AVG,
> > +				    ilog2(val));
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return count;
> > +}
> > +
> > +static DEVICE_ATTR_RW(samples_for_avg);
> > +
> > +static struct attribute *lm25056_attrs[] = {
> > +	&dev_attr_samples_for_avg.attr,
> > +	NULL,
> > +};
> > +ATTRIBUTE_GROUPS(lm25056); // should we set a name of this group to put all
> > +			   // those attributes in subdirectory? Like "custom" ?
> > +
> We don't add subdirectories for other chips, and we won't start it here.
> 
> I don't mind the attribute itself, but I do mind its name. We'll have
> to find something more generic, such as 'num_samples' or just 'samples'.
> I am open to suggestions. We'll also have to decide if the attribute(s)
> should be limited to one per chip, or if there can be multiple attributes.
> For example, MAX34462 has separate values for iout_samples and adc_average.
> Do we want samples, {curr,in,power,...}_samples, or both ? Or even
> currX_samples ?

For my use case -- TI's INA chips, there is only one "samples"
configuration being used for all currX_inputs and inX_inputs.
So having a "samples" node would certainly fit better than an
in0_samples. So I vote for having both.

Thank you
Nicolin