On Mon, Nov 29, 2010 at 5:49 PM, Gabriele Gorla <gorlik@xxxxxxxxxxxxxxx> wrote:
Ugh... Wow, that was clearly wrong logic now that you point it out...
The old code didn't throw an EINVAL on most invalid divisors, it just silently converted them to the next reasonable value in the way that DIV_TO_REG worked. If you're going to change the behavior and throw EINVAL for divisors greater than 8, perhaps you should check explicilty for the four legal values (val != 1 && val != 2 && val != 4 && val != 8)
Then you could eliminate the new_div DIV_TO_REG/REG_TO_DIV conversions and just test fan_div[nr] against val. That would eliminate the "new_div" variable which would be good since it doesn't actually hold a "div" but a "reg" value, so it's somewhat mis-named.
If you don't take the mutex before testing the fan_div, then isn't there a possible race where you test first, but then it changes before you take the mutex to "change" it? Is there any possible harm? If multiple threads are trying to set the same divisor or different divisors for the same or different fans? Just thinking out loud...
Phil P.
Prevent setting fan_div from stomping on other fans that share the same i2c register.
Ugh... Wow, that was clearly wrong logic now that you point it out...
Also allow div=1 (this is allowed in the ADM1026 datasheet)
The old code didn't throw an EINVAL on most invalid divisors, it just silently converted them to the next reasonable value in the way that DIV_TO_REG worked. If you're going to change the behavior and throw EINVAL for divisors greater than 8, perhaps you should check explicilty for the four legal values (val != 1 && val != 2 && val != 4 && val != 8)
Then you could eliminate the new_div DIV_TO_REG/REG_TO_DIV conversions and just test fan_div[nr] against val. That would eliminate the "new_div" variable which would be good since it doesn't actually hold a "div" but a "reg" value, so it's somewhat mis-named.
and prevent the mutex lock
when no update is necessary.
If you don't take the mutex before testing the fan_div, then isn't there a possible race where you test first, but then it changes before you take the mutex to "change" it? Is there any possible harm? If multiple threads are trying to set the same divisor or different divisors for the same or different fans? Just thinking out loud...
Phil P.
_______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
