STAGING:iio:light: fix ISL29018 init to handle brownout
Jonathan Cameron
jic23 at cam.ac.uk
Tue Aug 30 03:05:01 PDT 2011
On 08/26/11 22:58, Grant Grundler wrote:
> On Thu, Aug 25, 2011 at 6:15 PM, Dan Carpenter <error27 at gmail.com> wrote:
> ...
>> In isl29018_write_data() it uses reg (ISL29018_REG_TEST) as the
>> offset into the ->reg_cache[] array:
>> chip->reg_cache[reg] = regval;
>>
>> But ->reg_cache[] only has 3 elements, so we're past the end of the
>> array.
>
> Dan,
> I've attached a preliminary patch to fix this that applies on top of
> 176f9f29cec9 "STAGING:iio:light:
> fix ISL29018 init to handle brownout". This patch applies cleanly to
> the 2.6.38-based chromium.org tree.
>
> In a nutshell, the attached patch implements what I was thinking of
> last night: don't cache REG_TEST.
>
> 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.
Otherwise patch looks fine to me.
>For registers where we are frobbing bits, the later write to the
> same register might succeed and things will work anyway despite the
> transient failure. If I'm wrong, please comment and I'll repost with
> original behavior.
>
> I also noticed the original code had an "off-by-one" error in the
> allocation of reg_cache[] (allocated 3 but indexed up to offset +3).
>
> I'll submit a cleaned up version that should cleanly apply to gregkh's
> staging-2.6 tree.
>
> thanks again!
> grant
More information about the devel
mailing list