linux-kernel - Re: [PATCH v3 1/3] iio: mpl3115: use get_unaligned

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: <20251109163840.64144586@jic23-huawei>
Date: Sun, 9 Nov 2025 16:38:40 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Marcelo Schmitt <marcelo.schmitt1@...il.com>
Cc: Antoni Pokusinski <apokusinski01@...il.com>, dlechner@...libre.com,
 nuno.sa@...log.com, andy@...nel.org, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/3] iio: mpl3115: use get_unaligned_be24 to retrieve
 pressure data

On Thu, 6 Nov 2025 22:33:49 -0300
Marcelo Schmitt <marcelo.schmitt1@...il.com> wrote:

> On 11/05, Antoni Pokusinski wrote:
> > The pressure measurement result is arranged as 20-bit unsigned value
> > residing in three 8-bit registers. Hence, it can be retrieved using
> > get_unaligned_be24 and by applying 4-bit shift.
> > 
> > Signed-off-by: Antoni Pokusinski <apokusinski01@...il.com>
> > ---
> >  drivers/iio/pressure/mpl3115.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/iio/pressure/mpl3115.c b/drivers/iio/pressure/mpl3115.c  
> ...
> >  
> > -		*val = be32_to_cpu(tmp) >> chan->scan_type.shift;
> > +		*val = get_unaligned_be24(tmp) >> 4;  
> hmm, now the number of bits shifted is dissociated from the channel characteristics.
> We can do
> 		*val = get_unaligned_be24(tmp) >> (24 - chan->scan_type.realbits);
This encodes that the field is always aligned to the maximum bit. Whilst it might
be true, there is nothing inherent that says it must be.

I'm not sure why we aren't using chan->scan_type.shift though.

> or maybe
> 		*val = get_unaligned_be24(tmp) >> (sizeof(tmp) - chan->scan_type.realbits);

That one needs a BYTES_TO_BITS factor too.

> but it starts becoming too long IMO. Even longer if `tmp` gets a more meaningful
> name. Ah well, any of the three forms should work the same at the end of day so
> no strong opinion.
> 
> Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@...il.com>
> 
> >  		return IIO_VAL_INT;
> >  	}
> >  	case IIO_TEMP: { /* in 0.0625 celsius / LSB */
> > -- 
> > 2.25.1
> >