linux-kernel - Re: iio: dac: ad5624r_spi.c

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]

Message-ID: <296d9e03-1153-4589-95b8-87195d7bbdef@baylibre.com>
Date: Wed, 18 Dec 2024 14:53:57 -0600
From: David Lechner <dlechner@...libre.com>
To: Matti Vaittinen <mazziesaccount@...il.com>,
 Matti Vaittinen <matti.vaittinen@...rohmeurope.com>
Cc: Lars-Peter Clausen <lars@...afoo.de>,
 Michael Hennerich <Michael.Hennerich@...log.com>,
 Jonathan Cameron <jic23@...nel.org>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: iio: dac: ad5624r_spi.c - use of scan_type

On 12/18/24 2:38 AM, Matti Vaittinen wrote:
> Hi dee Ho peeps,
> 
> I started drafting a driver for a ROHM DAC. I took a quick look at the ad5624r_spi.c, and the use of the 'scan_type' -field in the struct iio_chan_spec puzzled me.
> 
> I think this field is used by the driver to convert the data from user to register format while performing the INDIO_DIRECT_MODE raw writes. I don't spot any buffer usage. Furthermore, as far as I can say the 'sign' and 'storagebits' are unused.
> 
> My understanding has been that the scan_type is only intended for parsing the buffered values, and usually when the data direction is from driver to user.
> 
> I suppose I shouldn't copy the ad5624r_spi.c use of scan_type to a new driver. I'm somewhat tempted to send a patch which drops the scan_type from the ad5624r_spi.c, and adds the 'realbits' and 'shift' to the driver's internal struct ad5624r_state. This, however, will change the interface to userland so maybe it's best to not do that.
> 
> I wonder if I am missing something? (That wouldn't be unheard of XD). If not, then at least a documentary patch with a comment "don't do this in new drivers" might be Ok, or how do you see this?
> 
> Yours,
>     -- Matti
> 

I think scan_type is a convenient place to store this information even if
buffers aren't implemented. The struct is there whether we use it or not, so
might as well use it. And if buffer support is ever added, that is one less
thing to do (removing the duplicate fields).