On 08/26/11 22:58, Grant Grundler wrote: > On Thu, Aug 25, 2011 at 6:15 PM, Dan Carpenter <error27@xxxxxxxxx> 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 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
