Hi Guenter, On Thu, 26 Jul 2012 06:37:50 -0700, Guenter Roeck wrote: > On Thu, Jul 26, 2012 at 02:57:45PM +0200, Jean Delvare wrote: > > Restoring the configuration register on device removal has the side > > effect of also resetting the hysteresis value. This is inconsistent as > > the other limits are not reset, only hysteresis. So, following the > > principle of least surprise, preserve the hysteresis value when > > restoring the configuration register. > > > > Signed-off-by: Jean Delvare <khali@xxxxxxxxxxxx> > > Cc: Guenter Roeck <linux@xxxxxxxxxxxx> > > Good catch. Applied. Doh, please drop it, the patch is buggy. I wrote | where I meant & and this hard-codes the hysteresis to -6°C at driver removal if the config register changed. I didn't notice during my testing because config doesn't change for me. Sorry for the noise. > > Note: this would be more readable if JC42_CFG_HYST_MASK was the mask > > _before_ shifting instead of after shifting. Guenter, do you want me > > to send a separate patch doing that, or do you like it the way it is? > > Please send a patch. I'll resend a two-patch series. > > Alternatively we could invert the logic and only restore the > > JC42_CFG_SHUTDOWN bit. As this bit and the two hysteresis bits are the > > only ones the driver touches, this is equivalent. I don't know if > > there is an intent to ever have the jc42 driver touch any other bit in > > the configuration register. Guenter, I am leaving the decision up to > > you. > > Hard to say, though looking into it seems unlikely. But who knows. I would > suggest to keep it generic as you implemented. Fine with me. -- Jean Delvare _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
