Re: [PATCH v2 3/8] iio: accel: kx022a: Use cleanup.h helpers

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 25 Nov 2024 11:34:36 +0200
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> Hello Jonathan,
> 
> Thanks again!
> 
> On 23/11/2024 18:42, Jonathan Cameron wrote:
> > On Thu, 21 Nov 2024 10:20:23 +0200
> > Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> >   
> >> A few functions in KX022A need to use mutex for protecting the
> >> enabling/disabling of the measurement while configurations are being
> >> made. Some of the functions can be slightly simplified by using the
> >> __cleanup based scoped mutexes, which allows dropping the goto based
> >> unlocking at error path.
> >>
> >> Simplify error paths using guard(mutex).
> >>
> >> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>  
> > Now we have guard(), the main reason (I think) for the
> > combined on + lock and off + unlock paths is gone. So can
> > we just flatten those and do the locking at caller.  
> 
> I did consider this too :)
> 
> Why I decided to keep it as it is, (even though we need the extra 
> mutex_unlock() at certain error path) is because I kind of like the 
> lock+off and unlock+on functions. This locking does not protect data, 
> but really a sequence of operations that needs to be done while sensor 
> is OFF state. It's almost like a doc saying that "please, ensure the 
> sensor is OFF for the following operations" :)

hmm.  I really don't like them because they are 'unusual'  :)

I'd argue they just ensure a sequence of writes go in as an atomic thing.
Two of those writes happen to be turn it off and turn it on.

So the data the are protecting is the device internal state data.

> 
> (Another thing is that we do claim the direct mode in write_raw, and 
> goto is still handy for releasing it. Scoped guards won't play nicely 
> with goto. Yes, we could probably use the __cleanup for direct mode, but 
> I still like the lock+off, unlock+on for the reason above)
There is a nice new cleanup that David did to make the direct mode
handling much cleaner.

	if_not_cond_guard(iio_claim_direct_try, indio_dev)
		return -EBUSY;


> 
> Yours,
> 	-- Matti
>