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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 29 Sep 2009 21:14:37 -0400
From:	Mike Frysinger <vapier.adi@...il.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	uclinux-dist-devel@...ckfin.uclinux.org, chrisv@...erswitching.com,
	linux-kernel@...r.kernel.org, michael.hennerich@...log.com
Subject: Re: [Uclinux-dist-devel] [PATCH] ad525x_dpot: new driver for AD525x 
	digital potentiometers

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/