Re: [Uclinux-dist-devel] [PATCH] ad525x_dpot: new driver for AD525x digital potentiometers

[Mike Frysinger <vapier.adi@gmail.com>]

On Tue, Sep 29, 2009 at 18:39, Andrew Morton wrote:
> On Thu, 17 Sep 2009 00:22:37 -0400 Mike Frysinger wrote:
>> This driver supports the non-volatile digital potentiometers via I2C:
>> AD5258, AD5259, AD5251, AD5252, AD5253, AD5254, and AD5255
>>
>> It provides a sysfs interface to each device for reading/writing.
>
> This sysfs interface is by far the most important aspect of this
> driver.  For both its users and for its reviewers.
>
> Yet you tell us nothing about it!  Not in code comments, not in the
> changelog, not in supporting documentation.
>
> So some poor idiot (ie: me) is left having to scratch his way through
> the implementation trying to work out what the propoed userspace
> interface is supposed to look like.

i'll have to get a description from some people

> +       if (reg & AD525X_REG_TOL)
> +               return sprintf(buf, "0x%04x\n", value & 0xFFFF);
> +       else
> +               return sprintf(buf, "%u\n", value & data->rdac_mask);
>
> which makes me suspect that the proposed userspace interface is quite
> poor.

not really.  the proposed interface was discussed with a few different
people (devs and end users) and reviewed for use across multiple parts
rather than just slapping together a raw interface onto it.  if you
actually read the datasheet as indicated in the comment you snipped,
you'll see that the value isnt formatted for trivial conversion.  if
the math can easily be done in the kernel, then that's one thing, but
it doesnt look like it to me.

this field is a read-only percentage programmed into the part by the
factory.  it can be interpreted by:
MSB: 0 = +
Next 7 MSB: 001 1100 = 28
8 LSB: 0000 1111 = 15 * 2^-8 = 0.06
Tolerance = 28.06%
Rounded Tolerance = 28.1% and therefore
RAB_ACTUAL = 12.810 kΩ

seems to me that this requires floating point support
-mike