On Fri, 14 Oct 2022 09:25:59 +0200 Nuno Sá <noname.nuno@xxxxxxxxx> wrote: > On Wed, 2022-10-12 at 20:45 +0200, Miquel Raynal wrote: > > Hi Nuno, > > > > nuno.sa@xxxxxxxxxx wrote on Wed, 12 Oct 2022 17:16:19 +0200: > > > > > 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. > > > > What about adding a TODO comment saying something like: "this comes > > from static analysis and helped dropping mlock access, but someone > > with > > the device needs to figure out if we can simplify this dance"? > > Because > > the reason behind all this is that we don't want to risk breaking the > > driver, but perhaps a simpler approach would work, right? > > > > Hi Miquel, > > AFAIU, either the device is powered (when buffer mode enabled) and we > can do the reading or it's not and we need to power it on/off > "manually" while making sure we don't race against enable/disabling > buffers. This "dance" is needed mainly to make sure that we grab > 'mlock' one way or another... The other way would be to use some > specific device lock together with a flag (as discussed) but as > discussed with Jonathan we decided to go down this road... So, > honestly, I don't really see the necessity of "marking" this code with > a TODO but of course if someone comes in with something simpler, great > :). Agreed. I don't expect to see any improvement in this in the future so a TODO would just be noise and might encourage people to propose the 'get the lock on it's own function' that we are going through this dance to avoid adding. Jonathan > > Anyways, as I said, I'm not really keen in spinning a new version to > add this comment so I will defer the decision to Jonathan :) > > Thanks for the help! > - Nuno Sá >
