Re: [PATCH v2 14/16] iio: health: max30100: do not use internal iio_dev lock

Jonathan Cameron
2022-10-09T12:14:16Z

On Wed, 05 Oct 2022 10:09:29 +0200
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Tue, 2022-10-04 at 17:13 +0300, Andy Shevchenko wrote:
> > On Tue, Oct 4, 2022 at 4:49 PM Nuno Sá <nuno.sa@analog.com> wrote:
> > >
> > > The pattern used in this device does not quite fit in the
> > > iio_device_claim_direct_mode() typical usage. In this case,
> > > iio_buffer_enabled() was being used not to prevent the raw access
> > > but to
> > > allow it. Hence, let's make use of the new
> > > iio_device_claim_buffer_mode() API to make sure we stay in buffered
> > > mode
> > > during the complete read.
> >
> > ...
> >
> > > +               if (iio_device_claim_buffer_mode(indio_dev)) {
> > >                         ret = -EAGAIN;
> >
> > Why is the error code shadowed? Isn't it better to return exactly the
> > one you resend to the upper caller(s)? Each unclear error code
> > shadowing should be at least explained.
> > >           }
>
> I'm keeping the same error that was returned before. Changing the error
> code returned to userspace can break some apps relying on it. But if
> everyone is ok with assuming the risk and changing it, fine by me.

Hmm. For most drivers I'd say change it, but these weird health parts
use a highly custom userspace so it's just possible we'd break it
by changing the return code. Unfortunately I don't know of anyone with current
access to the code of that software stack to check this for us as
there have been a lot of changes at TI in recent years.

So probably best to leave it alone, but add a comment to the patch description
to give reasoning.

Thanks,

Jonathan

>
>
> - Nuno Sá