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

["Nuno Sá" <noname.nuno@gmail.com>]

On Tue, 2022-10-04 at 17:15 +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, we want
> > to
> > know if we are in buffered mode or not to know if the device is
> > powered
> > (buffer mode) or not. And depending on that max30102_get_temp()
> > will
> > power on the device if needed. Hence, in order to keep the same
> > functionality, we try to:
> > 
> > 1. Claim Buffered mode;
> > 2: If 1) succeeds call max30102_get_temp() without powering on the
> >    device;
> > 3: Release Buffered mode;
> > 4: If 1) fails, Claim Direct mode;
> > 5: If 4) succeeds call max30102_get_temp() with powering on the
> > device;
> > 6: Release Direct mode;
> > 7: If 4) fails, goto to 1) and try again.
> > 
> > This dance between buffered and direct mode is not particularly
> > pretty
> > (as well as the loop introduced by the goto statement) but it does
> > allow
> > us to get rid of the mlock usage while keeping the same behavior.
> 
> ...
> 
> > +               if (iio_device_claim_buffer_mode(indio_dev)) {
> 
> Why is ret not used here?
> 
> > +                       /*
> > +                        * This one is a *bit* hacky. If we cannot
> > claim buffer
> > +                        * mode, then try direct mode so that we
> > make sure
> > +                        * things cannot concurrently change. And
> > we just keep
> > +                        * trying until we get one of the modes...
> > +                        */
> > +                       if
> > (iio_device_claim_direct_mode(indio_dev))
> 
> ...and here?
> 
> > +                               goto any_mode_retry;
> 
> > +               } else {
> 
> > +               }
> 
> I.o.w. what error code will be visible to the caller and why.
> 

Note that we do not really care about the error code returned by both
iio_device_claim_buffer_mode() and iio_device_claim_direct_mode(). We
just loop until we get one of the modes (thus ret = 0) so that we can
safely call one of the max30102_get_temp() variants. And that will be
the visible error code (if any). That said, you can look at the first
version of the series about this (and the previous patch) and why this
is being done like this (note that I'm also not 100% convinced about
this ping pong between getting one of the IIO modes but it's also not
that bad and if there's no major complains, I'm fine with it).

- Nuno Sá
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic