linux-kernel - Re: [PATCH v5 03/10] iio: accel: adxl345: measure right-justified

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: <CAFXKEHbPmFc8DNZW=Ww39j+XkAfLOyFY2qgvz+uEUaBYri_3hA@mail.gmail.com>
Date: Mon, 9 Dec 2024 23:18:45 +0100
From: Lothar Rubusch <l.rubusch@...il.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: lars@...afoo.de, Michael.Hennerich@...log.com, robh@...nel.org, 
	krzk+dt@...nel.org, conor+dt@...nel.org, devicetree@...r.kernel.org, 
	linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org, eraretuya@...il.com
Subject: Re: [PATCH v5 03/10] iio: accel: adxl345: measure right-justified

Dear IIO-ML, Hi Jonathan!

On Sun, Dec 8, 2024 at 2:35 PM Jonathan Cameron <jic23@...nel.org> wrote:
>
> On Thu,  5 Dec 2024 17:13:36 +0000
> Lothar Rubusch <l.rubusch@...il.com> wrote:
>
> > Make measurements right-justified, since it is the default for the
> > driver and sensor. By not setting the ADXL345_DATA_FORMAT_JUSTIFY bit,
> > the data becomes right-judstified. This was the original setting, there
> > is no reason to change it to left-justified, where right-justified
> > simplifies working on the registers.
> >
> > Signed-off-by: Lothar Rubusch <l.rubusch@...il.com>
>
> I'm still confused by this one.  Does this change affect the data output
> to userspace?  If seems like it definitely should. If it does we have
> an ABI regression somewhere. Is it currently broken and wasn't at some
> earlier stage, or is this the patch breaking things?

No, it should not affect the userspace.

This setting opens the mask for regmap/update bits to allow for
changing the data format.
My point is rather, does it actually makes sense to allow to change
the data format, since
the driver will use just one format. The bit was never applied, it's
just the mask here.

May I ask you, if you could also could give me a brief feedback to the
three questions in
the cover letter to this series?

I would really appreciate, since I'm still unsure if I actually
verified everything correctly.
>From what I did about this bit, I removed and set the justified bit in
STREAM and in
BYPASSED mode (current mode), without any difference in the results in
iio_info or
iio_readdev. The numbers look generally odd to me, though. And, I'd
rather like to ask
to still wait with applying the patches, if this is ok for you? But,
perhaps with the answers
of the cover letter items, it could become clearer to me. I'm still
about to measure and
verify against the old and the input driver results as comparison.

Best,
L


> If it worked and currently doesn't send a fix.  If this changes a previously
> working ABI then drop this patch.  Alternative being to fix up the scale
> handling to incorporate this justification change.
>
> Jonathan
>
> > ---
> >  drivers/iio/accel/adxl345_core.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> > index 88df9547bd6..98ff37271f1 100644
> > --- a/drivers/iio/accel/adxl345_core.c
> > +++ b/drivers/iio/accel/adxl345_core.c
> > @@ -184,7 +184,6 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
> >       struct iio_dev *indio_dev;
> >       u32 regval;
> >       unsigned int data_format_mask = (ADXL345_DATA_FORMAT_RANGE |
> > -                                      ADXL345_DATA_FORMAT_JUSTIFY |
> >                                        ADXL345_DATA_FORMAT_FULL_RES |
> >                                        ADXL345_DATA_FORMAT_SELF_TEST);
> >       int ret;
>