On 26/11/2024 19:55, Jonathan Cameron wrote:
On Mon, 25 Nov 2024 11:34:36 +0200
Matti Vaittinen <mazziesaccount@xxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxx>
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 could argue these aren't totally unusual, perhaps unusual in IIO. I
fell in love with this type of functions when Guenter suggested this
approach for me in the wdg. Well, IIO is your territory so I'll mutilate
this file accordingly.
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.
Well, the data-sheet is very clear what comes to clearing the PC1 bit
when the various CNTL register are touched:
https://fscdn.rohm.com/kionix/en/datasheet/kx022acr-z-e.pdf
(at the beginning of various CNTL register descriptions). So, the on/off
thing is not something that just happens - and this is what these
functions did try to underline :)
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;
Ah. Nice. This is not yet in the iio_testing though. I'll add this 'drop
the off+lock, on+unlock -functions change as an individual patch. It'll
depend on the if_not_cond_guard() while the rest of the patches should
have no dependencies to any "not yet in iio_testing" stuff.
I do have the patches ready for sending but I don't have sensors to test
this at home. I'll give this a try at the office tomorrow and send it
out then.
Yours,
-- Matti
