linux-kernel - Re: [PATCH v9 6/8] iio: adc: Support ROHM BD79124 ADC

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: <Z91WS-DoKoIZhRNs@smile.fi.intel.com>
Date: Fri, 21 Mar 2025 14:06:35 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Matti Vaittinen <mazziesaccount@...il.com>
Cc: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
	Jonathan Cameron <jic23@...nel.org>,
	Lars-Peter Clausen <lars@...afoo.de>, Nuno Sa <nuno.sa@...log.com>,
	David Lechner <dlechner@...libre.com>,
	Javier Carrasco <javier.carrasco.cruz@...il.com>,
	Olivier Moysan <olivier.moysan@...s.st.com>,
	Guillaume Stols <gstols@...libre.com>,
	Dumitru Ceclan <mitrutzceclan@...il.com>,
	Trevor Gamblin <tgamblin@...libre.com>,
	Matteo Martelli <matteomartelli3@...il.com>,
	Alisa-Dariana Roman <alisadariana@...il.com>,
	João Paulo Gonçalves <joao.goncalves@...adex.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org
Subject: Re: [PATCH v9 6/8] iio: adc: Support ROHM BD79124 ADC

On Fri, Mar 21, 2025 at 10:01:00AM +0200, Matti Vaittinen wrote:
> On 20/03/2025 15:16, Andy Shevchenko wrote:
> > On Thu, Mar 20, 2025 at 10:22:00AM +0200, Matti Vaittinen wrote:


...

> > > +struct bd79124_raw {
> > > +	u8 val_bit0_3; /* Is set in high bits of the byte */
> > > +	u8 val_bit4_11;
> > > +};
> > 
> > Again, this is confusing.
> > 
> > Just put a bit order map in the comment as I suggested previously.
> > When I see variable name containing bit range like above I think
> > about the same bit order, i.e. with your comment it makes like this
> > 
> > bit number	7 6 5 4 3 2 1 0
> > data bit	0 1 2 3 x x x x
> 
> Gah. I think I now understand what you're after. And, I agree, I haven't
> been as clear as I could've been.
> 
> The pit numbers in the struct members:
> 	u8 val_bit0_3; and u8 val_bit4_11;
> 
> are _not_ intended to represent the bit ordering - only the bit positions.
> Like, bits from bit 0 to bit 3 are stored in high bits of this u8 - where
> the "0 to 3" was just picked as order based on it being from the smaller to
> greater (which I believe is grammatically typical) - not based on how the
> bits are ordered in the register. If the order of the bits was indeed
> reverted, then we should see much more complex conversions than what is
> presented in these macros.
> 
> I will update the variable names to:
> 
> val_bit3_0; and val_bit11_4; I think it should sort out the confusion. I
> won't go to bit level representation of the full registers:
> 
> > bit number	7 6 5 4 3 2 1 0
> > data bit	3 2 1 0 x x x x
> 
> and
> 
> > bit number	7 6 5 4 3 2 1 0
> > data bit	b a 9 8 7 6 5 4
> 
> because it suggests there is something very strange in the registers (which
> is not the case) - and it is hard to spot if some bits have indeed changed
> the place.

You can get rid of all of these by simply using __le16. I do not understand why
it's not used so far. I thought that bits are mirrored, that may explain the
case, but now I do not see any problem to use __le16 directly.

-- 
With Best Regards,
Andy Shevchenko