On Tue, 19 Aug 2008 19:12:39 +0100, Ben Hutchings wrote:
> Jean Delvare wrote:
> [...]
> > I agree that the current code is incorrect, but I disagree with your
> > analysis (I don't think the 0xF7 is a typo) and the proposed fix.
> >
> > First of all, checking for bit 7 (Initialization) doesn't make sense.
> > This bit self-clears, so we're never going to see it set.
>
> Right.
>
> > Secondly, clearing bit 3 (INT#_Clear) is a good idea. According to the
> > datasheet, monitoring is stopped while this bit is set, and it is set
> > by default. So we really want to clear it as part of the
> > initialization. Meaning that we also want to check if it is set.
> >
> > So I would change the code as follows:
> >
> > if ((config & 0x09) != 0x01) {
> > /* Start monitoring */
> > lm87_write_value(client, LM87_REG_CONFIG,
> > (config & 0x77) | 0x01);
> > }
> >
> > What do you think?
>
> That makes sense. Perhaps the comment should also say that we're
> clearing INT#_Clear.
Alright. If you update your patch to do that, I'll review it and queue
it for 2.6.27.
--
Jean Delvare
