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
