* [lm-sensors] [PATCH] hwmon: (atxp1) Fix device detection logic
2008-06-29 10:31 [lm-sensors] [PATCH] hwmon: (atxp1) Fix device detection logic Jean Delvare
@ 2008-08-13 19:12 ` Jean Delvare
2008-08-14 8:49 ` Jean Delvare
2008-08-19 20:58 ` Sebastian Witt
2 siblings, 0 replies; 4+ messages in thread
From: Jean Delvare @ 2008-08-13 19:12 UTC (permalink / raw)
To: lm-sensors
Subject: hwmon: (atxp1) Fix device detection logic
The atxp1 device detection code has a major logic flaw, fix it. Not
sure how we managed to miss this when the driver was merged...
Signed-off-by: Jean Delvare <khali@linux-fr.org>
Cc: Sebastian Witt <se.witt@gmx.net>
---
Patch refreshed to apply on top of 2.6.27-rc3.
I didn't hear of Sebastian in 6 weeks. Can anyone please review this?
drivers/hwmon/atxp1.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
--- linux-2.6.27-rc3.orig/drivers/hwmon/atxp1.c 2008-08-05 18:17:05.000000000 +0200
+++ linux-2.6.27-rc3/drivers/hwmon/atxp1.c 2008-08-13 19:37:43.000000000 +0200
@@ -31,7 +31,7 @@
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("System voltages control via Attansic ATXP1");
-MODULE_VERSION("0.6.2");
+MODULE_VERSION("0.6.3");
MODULE_AUTHOR("Sebastian Witt <se.witt@gmx.net>");
#define ATXP1_VID 0x00
@@ -289,16 +289,16 @@ static int atxp1_detect(struct i2c_clien
if (!((i2c_smbus_read_byte_data(new_client, 0x3e) = 0) &&
(i2c_smbus_read_byte_data(new_client, 0x3f) = 0) &&
(i2c_smbus_read_byte_data(new_client, 0xfe) = 0) &&
- (i2c_smbus_read_byte_data(new_client, 0xff) = 0) )) {
+ (i2c_smbus_read_byte_data(new_client, 0xff) = 0)))
+ return -ENODEV;
- /* No vendor ID, now checking if registers 0x10,0x11 (non-existent)
- * showing the same as register 0x00 */
- temp = i2c_smbus_read_byte_data(new_client, 0x00);
-
- if (!((i2c_smbus_read_byte_data(new_client, 0x10) = temp) &&
- (i2c_smbus_read_byte_data(new_client, 0x11) = temp) ))
- return -ENODEV;
- }
+ /* No vendor ID, now checking if registers 0x10,0x11 (non-existent)
+ * showing the same as register 0x00 */
+ temp = i2c_smbus_read_byte_data(new_client, 0x00);
+
+ if (!((i2c_smbus_read_byte_data(new_client, 0x10) = temp) &&
+ (i2c_smbus_read_byte_data(new_client, 0x11) = temp)))
+ return -ENODEV;
/* Get VRM */
temp = vid_which_vrm();
--
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] 4+ messages in thread* Re: [lm-sensors] [PATCH] hwmon: (atxp1) Fix device detection logic
2008-06-29 10:31 [lm-sensors] [PATCH] hwmon: (atxp1) Fix device detection logic Jean Delvare
2008-08-13 19:12 ` Jean Delvare
@ 2008-08-14 8:49 ` Jean Delvare
2008-08-19 20:58 ` Sebastian Witt
2 siblings, 0 replies; 4+ messages in thread
From: Jean Delvare @ 2008-08-14 8:49 UTC (permalink / raw)
To: lm-sensors
Hi Sebastian,
Please keep the lm-sensors list Cc'd.
On Thu, 14 Aug 2008 00:09:32 +0200, Sebastian Witt wrote:
> Hello Jean,
>
> sorry, missed your mail. The test sequence was:
>
> 1. If vendor ID registers are all zero, assume ATXP1.
> 2. If not, do second test. If it succeeds it is a ATXP1.
>
> I'm not sure anymore why it was this way, maybe I thought there would be a ATXP1
> with vendor ID so it works for both...
That's indeed what the current code does, but that's not correct. We
want to check that _both_ conditions are met. Even with that, the
detection is rather weak... Without that it's very, very weak.
> However aborting when the first test fails and doing always the second test
> should be fine, but I can't test it because I gave away the board some time ago.
If you look at the comments in the original code, that's what they say:
we have "No vendor ID" inside the block which is executed when there
_are_ IDs. So it's clear to me that the code doesn't do what we
intended to do in the first place, and was "working" only by accident.
Even if you can't test my patch, can you please review it and add your
Acked-by, so that I can push it upstream? Thanks.
--
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] 4+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon: (atxp1) Fix device detection logic
2008-06-29 10:31 [lm-sensors] [PATCH] hwmon: (atxp1) Fix device detection logic Jean Delvare
2008-08-13 19:12 ` Jean Delvare
2008-08-14 8:49 ` Jean Delvare
@ 2008-08-19 20:58 ` Sebastian Witt
2 siblings, 0 replies; 4+ messages in thread
From: Sebastian Witt @ 2008-08-19 20:58 UTC (permalink / raw)
To: lm-sensors
Jean Delvare wrote:
> Subject: hwmon: (atxp1) Fix device detection logic
>
> The atxp1 device detection code has a major logic flaw, fix it. Not
> sure how we managed to miss this when the driver was merged...
>
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
Acked-by: Sebastian Witt <se.witt@gmx.net>
> Cc: Sebastian Witt <se.witt@gmx.net>
> ---
> Patch refreshed to apply on top of 2.6.27-rc3.
>
> I didn't hear of Sebastian in 6 weeks. Can anyone please review this?
>
> drivers/hwmon/atxp1.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> --- linux-2.6.27-rc3.orig/drivers/hwmon/atxp1.c 2008-08-05 18:17:05.000000000 +0200
> +++ linux-2.6.27-rc3/drivers/hwmon/atxp1.c 2008-08-13 19:37:43.000000000 +0200
> @@ -31,7 +31,7 @@
>
> MODULE_LICENSE("GPL");
> MODULE_DESCRIPTION("System voltages control via Attansic ATXP1");
> -MODULE_VERSION("0.6.2");
> +MODULE_VERSION("0.6.3");
> MODULE_AUTHOR("Sebastian Witt <se.witt@gmx.net>");
>
> #define ATXP1_VID 0x00
> @@ -289,16 +289,16 @@ static int atxp1_detect(struct i2c_clien
> if (!((i2c_smbus_read_byte_data(new_client, 0x3e) = 0) &&
> (i2c_smbus_read_byte_data(new_client, 0x3f) = 0) &&
> (i2c_smbus_read_byte_data(new_client, 0xfe) = 0) &&
> - (i2c_smbus_read_byte_data(new_client, 0xff) = 0) )) {
> + (i2c_smbus_read_byte_data(new_client, 0xff) = 0)))
> + return -ENODEV;
>
> - /* No vendor ID, now checking if registers 0x10,0x11 (non-existent)
> - * showing the same as register 0x00 */
> - temp = i2c_smbus_read_byte_data(new_client, 0x00);
> -
> - if (!((i2c_smbus_read_byte_data(new_client, 0x10) = temp) &&
> - (i2c_smbus_read_byte_data(new_client, 0x11) = temp) ))
> - return -ENODEV;
> - }
> + /* No vendor ID, now checking if registers 0x10,0x11 (non-existent)
> + * showing the same as register 0x00 */
> + temp = i2c_smbus_read_byte_data(new_client, 0x00);
> +
> + if (!((i2c_smbus_read_byte_data(new_client, 0x10) = temp) &&
> + (i2c_smbus_read_byte_data(new_client, 0x11) = temp)))
> + return -ENODEV;
>
> /* Get VRM */
> temp = vid_which_vrm();
>
>
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 4+ messages in thread