* Re: [lm-sensors] [PATCH] hwmon: (lm63) LM64 has a dedicated pin for tachometer
2012-01-13 9:13 [lm-sensors] [PATCH] hwmon: (lm63) LM64 has a dedicated pin for tachometer Jean Delvare
@ 2012-01-13 11:55 ` Guenter Roeck
2012-01-13 12:14 ` Guenter Roeck
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2012-01-13 11:55 UTC (permalink / raw)
To: lm-sensors
On Fri, Jan 13, 2012 at 04:13:21AM -0500, Jean Delvare wrote:
> On the LM64, the tachometer function has a dedicated pin and fan speed
> monitoring is always enabled.
>
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> ---
> Documentation/hwmon/lm63 | 4 +++-
> drivers/hwmon/lm63.c | 3 ++-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> --- linux-3.3-rc0.orig/Documentation/hwmon/lm63 2012-01-13 09:09:25.000000000 +0100
> +++ linux-3.3-rc0/Documentation/hwmon/lm63 2012-01-13 09:54:38.000000000 +0100
> @@ -54,7 +54,9 @@ value for measuring the speed of the fan
> Note that the pin used for fan monitoring is shared with an alert out
> function. Depending on how the board designer wanted to use the chip, fan
> speed monitoring will or will not be possible. The proper chip configuration
> -is left to the BIOS, and the driver will blindly trust it.
> +is left to the BIOS, and the driver will blindly trust it. Only the original
> +LM63 suffers from this limitation, the LM64 and LM96163 have separate pins
> +for fan monitoring and alert out.
>
Maybe add a note here indicating that fan control is always enabled on LM64 only ?
Otherwise people may wonder why fans don't (always) show up on LM96163.
Other than that,
Acked-by: Guenter Roeck <guenter.roeck@erisccon.com>
Thanks,
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [lm-sensors] [PATCH] hwmon: (lm63) LM64 has a dedicated pin for tachometer
2012-01-13 9:13 [lm-sensors] [PATCH] hwmon: (lm63) LM64 has a dedicated pin for tachometer Jean Delvare
2012-01-13 11:55 ` Guenter Roeck
@ 2012-01-13 12:14 ` Guenter Roeck
2012-01-13 12:38 ` Jean Delvare
2012-01-13 12:40 ` Jean Delvare
3 siblings, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2012-01-13 12:14 UTC (permalink / raw)
To: lm-sensors
On Fri, Jan 13, 2012 at 04:13:21AM -0500, Jean Delvare wrote:
> On the LM64, the tachometer function has a dedicated pin and fan speed
> monitoring is always enabled.
>
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> ---
> Documentation/hwmon/lm63 | 4 +++-
> drivers/hwmon/lm63.c | 3 ++-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> --- linux-3.3-rc0.orig/Documentation/hwmon/lm63 2012-01-13 09:09:25.000000000 +0100
> +++ linux-3.3-rc0/Documentation/hwmon/lm63 2012-01-13 09:54:38.000000000 +0100
> @@ -54,7 +54,9 @@ value for measuring the speed of the fan
> Note that the pin used for fan monitoring is shared with an alert out
> function. Depending on how the board designer wanted to use the chip, fan
> speed monitoring will or will not be possible. The proper chip configuration
> -is left to the BIOS, and the driver will blindly trust it.
> +is left to the BIOS, and the driver will blindly trust it. Only the original
> +LM63 suffers from this limitation, the LM64 and LM96163 have separate pins
> +for fan monitoring and alert out.
>
> A PWM output can be used to control the speed of the fan. The LM63 has two
> PWM modes: manual and automatic. Automatic mode is not fully implemented yet
> --- linux-3.3-rc0.orig/drivers/hwmon/lm63.c 2012-01-13 09:33:23.000000000 +0100
> +++ linux-3.3-rc0/drivers/hwmon/lm63.c 2012-01-13 09:53:19.000000000 +0100
> @@ -745,7 +745,8 @@ static int lm63_probe(struct i2c_client
> err = sysfs_create_group(&new_client->dev.kobj, &lm63_group);
> if (err)
> goto exit_free;
> - if (data->config & 0x04) { /* tachometer enabled */
> + if (data->kind = lm64 || /* tachometer has dedicated pin */
> + (data->config & 0x04)) { /* tachometer enabled */
> err = sysfs_create_group(&new_client->dev.kobj,
> &lm63_group_fan1);
> if (err)
>
Just realized: In lm63_update_device(), the code checks for data->config & 0x04 again
and never reads the fan registers if it is 0. Since the bit is reserved on LM64, fan status
will never be read for it.
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [lm-sensors] [PATCH] hwmon: (lm63) LM64 has a dedicated pin for tachometer
2012-01-13 9:13 [lm-sensors] [PATCH] hwmon: (lm63) LM64 has a dedicated pin for tachometer Jean Delvare
2012-01-13 11:55 ` Guenter Roeck
2012-01-13 12:14 ` Guenter Roeck
@ 2012-01-13 12:38 ` Jean Delvare
2012-01-13 12:40 ` Jean Delvare
3 siblings, 0 replies; 5+ messages in thread
From: Jean Delvare @ 2012-01-13 12:38 UTC (permalink / raw)
To: lm-sensors
On Fri, 13 Jan 2012 04:14:37 -0800, Guenter Roeck wrote:
> On Fri, Jan 13, 2012 at 04:13:21AM -0500, Jean Delvare wrote:
> > On the LM64, the tachometer function has a dedicated pin and fan speed
> > monitoring is always enabled.
> >
> > Signed-off-by: Jean Delvare <khali@linux-fr.org>
> > ---
> > Documentation/hwmon/lm63 | 4 +++-
> > drivers/hwmon/lm63.c | 3 ++-
> > 2 files changed, 5 insertions(+), 2 deletions(-)
> >
> > --- linux-3.3-rc0.orig/Documentation/hwmon/lm63 2012-01-13 09:09:25.000000000 +0100
> > +++ linux-3.3-rc0/Documentation/hwmon/lm63 2012-01-13 09:54:38.000000000 +0100
> > @@ -54,7 +54,9 @@ value for measuring the speed of the fan
> > Note that the pin used for fan monitoring is shared with an alert out
> > function. Depending on how the board designer wanted to use the chip, fan
> > speed monitoring will or will not be possible. The proper chip configuration
> > -is left to the BIOS, and the driver will blindly trust it.
> > +is left to the BIOS, and the driver will blindly trust it. Only the original
> > +LM63 suffers from this limitation, the LM64 and LM96163 have separate pins
> > +for fan monitoring and alert out.
> >
> > A PWM output can be used to control the speed of the fan. The LM63 has two
> > PWM modes: manual and automatic. Automatic mode is not fully implemented yet
> > --- linux-3.3-rc0.orig/drivers/hwmon/lm63.c 2012-01-13 09:33:23.000000000 +0100
> > +++ linux-3.3-rc0/drivers/hwmon/lm63.c 2012-01-13 09:53:19.000000000 +0100
> > @@ -745,7 +745,8 @@ static int lm63_probe(struct i2c_client
> > err = sysfs_create_group(&new_client->dev.kobj, &lm63_group);
> > if (err)
> > goto exit_free;
> > - if (data->config & 0x04) { /* tachometer enabled */
> > + if (data->kind = lm64 || /* tachometer has dedicated pin */
> > + (data->config & 0x04)) { /* tachometer enabled */
> > err = sysfs_create_group(&new_client->dev.kobj,
> > &lm63_group_fan1);
> > if (err)
> >
> Just realized: In lm63_update_device(), the code checks for data->config & 0x04 again
> and never reads the fan registers if it is 0. Since the bit is reserved on LM64, fan status
> will never be read for it.
Yeah, my patch is crap, I noticed right after sending it. I should have
tested before sending rather than the other way around, sorry. An
update is coming.
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [lm-sensors] [PATCH] hwmon: (lm63) LM64 has a dedicated pin for tachometer
2012-01-13 9:13 [lm-sensors] [PATCH] hwmon: (lm63) LM64 has a dedicated pin for tachometer Jean Delvare
` (2 preceding siblings ...)
2012-01-13 12:38 ` Jean Delvare
@ 2012-01-13 12:40 ` Jean Delvare
3 siblings, 0 replies; 5+ messages in thread
From: Jean Delvare @ 2012-01-13 12:40 UTC (permalink / raw)
To: lm-sensors
On Fri, 13 Jan 2012 03:55:04 -0800, Guenter Roeck wrote:
> On Fri, Jan 13, 2012 at 04:13:21AM -0500, Jean Delvare wrote:
> > On the LM64, the tachometer function has a dedicated pin and fan speed
> > monitoring is always enabled.
> >
> > Signed-off-by: Jean Delvare <khali@linux-fr.org>
>
> > ---
> > Documentation/hwmon/lm63 | 4 +++-
> > drivers/hwmon/lm63.c | 3 ++-
> > 2 files changed, 5 insertions(+), 2 deletions(-)
> >
> > --- linux-3.3-rc0.orig/Documentation/hwmon/lm63 2012-01-13 09:09:25.000000000 +0100
> > +++ linux-3.3-rc0/Documentation/hwmon/lm63 2012-01-13 09:54:38.000000000 +0100
> > @@ -54,7 +54,9 @@ value for measuring the speed of the fan
> > Note that the pin used for fan monitoring is shared with an alert out
> > function. Depending on how the board designer wanted to use the chip, fan
> > speed monitoring will or will not be possible. The proper chip configuration
> > -is left to the BIOS, and the driver will blindly trust it.
> > +is left to the BIOS, and the driver will blindly trust it. Only the original
> > +LM63 suffers from this limitation, the LM64 and LM96163 have separate pins
> > +for fan monitoring and alert out.
> >
> Maybe add a note here indicating that fan control is always enabled on LM64 only ?
You mean fan monitoring here, not fan control.
> Otherwise people may wonder why fans don't (always) show up on LM96163.
Will clarify, thanks for the suggestion.
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 5+ messages in thread