Re: [PATCH v3 1/3] iio: mpl3115: use get_unaligned_be24 to retrieve pressure data

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 6 Nov 2025 22:33:49 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.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@gmail.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@gmail.com>
> 
> >  		return IIO_VAL_INT;
> >  	}
> >  	case IIO_TEMP: { /* in 0.0625 celsius / LSB */
> > -- 
> > 2.25.1
> >