STAGING:iio:light: fix ISL29018 init to handle brownout

Grant Grundler grundler at google.com
Tue Aug 30 16:45:02 UTC 2011


On Tue, Aug 30, 2011 at 9:31 AM, Jonathan Cameron <jic23 at cam.ac.uk> wrote:
> On 08/30/11 17:14, Grant Grundler wrote:
>> On Tue, Aug 30, 2011 at 3:05 AM, Jonathan Cameron <jic23 at cam.ac.uk> wrote:
>> ....
>>>> I did change one basic behavior that I think was also broken: cache
>>>> the value regardless of if the transaction completed successfully or
>>>> not.
>>> Don't do that.  That means userspace will get an invalid value if it reads
>>> in the meantime. If you have an error on a hardware bus - tell userspace about
>>> it and don't 'guess' what is in the register.
>>
>> Ah ok - I didn't know this was part of the path to/from user space.
> It plausibly isn't, I didn't go over it closely enough to be sure.

Ok. After thinking about it more, it might not matter anyway.

> Even if not, it's a nasty complexity that will bite someone at some stage.
> Mostly comes down to code doing what other code does in the same situation
> rather than trying to be clever.

The whole error handling path one ugly bit of "nasty complexity". If a register
write fails, the register contents will be unknown. We will have no idea what
value is in the register or if the device is even still alive.
The cached value can't represent "known HW state" at that point and should
probably be marked "invalid" until either the value is succesfully
read or the next
write succeeds.

BTW, I didn't consider this particularly clever - just the code was
easier to read. :)

I don't think it really matters how the "cached value" is handled as
long as the error gets returned.  I'm just going to implement whatever
you prefer. :)

cheers,
grant



More information about the devel mailing list