* [PATCH] hwmon: Add basic support for lm64 to lm63.c
@ 2010-03-17 19:55 Matthew Garrett
2010-03-18 7:41 ` [lm-sensors] " Jean Delvare
0 siblings, 1 reply; 5+ messages in thread
From: Matthew Garrett @ 2010-03-17 19:55 UTC (permalink / raw)
To: lm-sensors; +Cc: linux-kernel, Matthew Garrett
The lm64 appears to be an lm63 with added gpio lines. Add support for the
hwmon functionality - gpio can be added at some later stage if someone
has a need for them.
Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
Documentation/hwmon/lm63 | 7 +++++++
drivers/hwmon/lm63.c | 19 +++++++++++++++----
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/Documentation/hwmon/lm63 b/Documentation/hwmon/lm63
index 31660bf..b6f0495 100644
--- a/Documentation/hwmon/lm63
+++ b/Documentation/hwmon/lm63
@@ -7,6 +7,11 @@ Supported chips:
Addresses scanned: I2C 0x4c
Datasheet: Publicly available at the National Semiconductor website
http://www.national.com/pf/LM/LM63.html
+ * National Semiconductor LM64
+ Prefix: 'lm64'
+ Addresses scanned: I2C 0x18 and 0x4e
+ Datasheet: Publicly available at the National Semiconductor website
+ http://www.national.com/pf/LM/LM64.html
Author: Jean Delvare <khali@linux-fr.org>
@@ -55,3 +60,5 @@ The lm63 driver will not update its values more frequently than every
second; reading them more often will do no harm, but will return 'old'
values.
+The lm64 is effectively an lm63 with gpio lines. The driver does not
+support these gpio lines at present.
\ No newline at end of file
diff --git a/drivers/hwmon/lm63.c b/drivers/hwmon/lm63.c
index bf81aff..01b23ef 100644
--- a/drivers/hwmon/lm63.c
+++ b/drivers/hwmon/lm63.c
@@ -53,7 +53,7 @@
* Address is fully defined internally and cannot be changed.
*/
-static const unsigned short normal_i2c[] = { 0x4c, I2C_CLIENT_END };
+static const unsigned short normal_i2c[] = { 0x18, 0x4c, 0x4e, I2C_CLIENT_END };
/*
* The LM63 registers
@@ -131,12 +131,15 @@ static struct lm63_data *lm63_update_device(struct device *dev);
static int lm63_detect(struct i2c_client *client, struct i2c_board_info *info);
static void lm63_init_client(struct i2c_client *client);
+enum chips { lm63, lm64 };
+
/*
* Driver data (common to all clients)
*/
static const struct i2c_device_id lm63_id[] = {
- { "lm63", 0 },
+ { "lm63", lm63 },
+ { "lm64", lm64 },
{ }
};
MODULE_DEVICE_TABLE(i2c, lm63_id);
@@ -177,6 +180,7 @@ struct lm63_data {
2: remote high limit */
u8 temp2_crit_hyst;
u8 alarms;
+ int kind;
};
/*
@@ -422,6 +426,7 @@ static int lm63_detect(struct i2c_client *new_client,
struct i2c_adapter *adapter = new_client->adapter;
u8 man_id, chip_id, reg_config1, reg_config2;
u8 reg_alert_status, reg_alert_mask;
+ int address = new_client->addr;
if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
return -ENODEV;
@@ -439,7 +444,6 @@ static int lm63_detect(struct i2c_client *new_client,
LM63_REG_ALERT_MASK);
if (man_id != 0x01 /* National Semiconductor */
- || chip_id != 0x41 /* LM63 */
|| (reg_config1 & 0x18) != 0x00
|| (reg_config2 & 0xF8) != 0x00
|| (reg_alert_status & 0x20) != 0x00
@@ -450,7 +454,12 @@ static int lm63_detect(struct i2c_client *new_client,
return -ENODEV;
}
- strlcpy(info->type, "lm63", I2C_NAME_SIZE);
+ if (chip_id == 0x41 && address == 0x4c)
+ strlcpy(info->type, "lm63", I2C_NAME_SIZE);
+ else if (chip_id == 0x51 && (address == 0x18 || address == 0x4e))
+ strlcpy(info->type, "lm64", I2C_NAME_SIZE);
+ else
+ return -ENODEV;
return 0;
}
@@ -471,6 +480,8 @@ static int lm63_probe(struct i2c_client *new_client,
data->valid = 0;
mutex_init(&data->update_lock);
+ data->kind = id->driver_data;
+
/* Initialize the LM63 chip */
lm63_init_client(new_client);
--
1.7.0.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [lm-sensors] [PATCH] hwmon: Add basic support for lm64 to lm63.c
2010-03-17 19:55 [PATCH] hwmon: Add basic support for lm64 to lm63.c Matthew Garrett
@ 2010-03-18 7:41 ` Jean Delvare
2010-03-18 13:36 ` [PATCH] hwmon: Add basic support for LM64 " Matthew Garrett
2010-03-18 13:39 ` [lm-sensors] [PATCH] hwmon: Add basic support for lm64 " Matthew Garrett
0 siblings, 2 replies; 5+ messages in thread
From: Jean Delvare @ 2010-03-18 7:41 UTC (permalink / raw)
To: Matthew Garrett; +Cc: lm-sensors, linux-kernel
Hi Matthew,
On Wed, 17 Mar 2010 15:55:22 -0400, Matthew Garrett wrote:
> The lm64 appears to be an lm63 with added gpio lines. Add support for the
> hwmon functionality - gpio can be added at some later stage if someone
> has a need for them.
>
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> ---
> Documentation/hwmon/lm63 | 7 +++++++
> drivers/hwmon/lm63.c | 19 +++++++++++++++----
> 2 files changed, 22 insertions(+), 4 deletions(-)
Can you please send me a dump of your LM64 chip?
Review:
>
> diff --git a/Documentation/hwmon/lm63 b/Documentation/hwmon/lm63
> index 31660bf..b6f0495 100644
> --- a/Documentation/hwmon/lm63
> +++ b/Documentation/hwmon/lm63
> @@ -7,6 +7,11 @@ Supported chips:
> Addresses scanned: I2C 0x4c
> Datasheet: Publicly available at the National Semiconductor website
> http://www.national.com/pf/LM/LM63.html
> + * National Semiconductor LM64
> + Prefix: 'lm64'
> + Addresses scanned: I2C 0x18 and 0x4e
> + Datasheet: Publicly available at the National Semiconductor website
> + http://www.national.com/pf/LM/LM64.html
>
> Author: Jean Delvare <khali@linux-fr.org>
>
> @@ -55,3 +60,5 @@ The lm63 driver will not update its values more frequently than every
> second; reading them more often will do no harm, but will return 'old'
> values.
>
> +The lm64 is effectively an lm63 with gpio lines. The driver does not
> +support these gpio lines at present.
Please use "LM64" and "LM63" when referring to the devices themselves.
And spell GPIO with capitals, too. Same applies to the patch
description, BTW.
> \ No newline at end of file
Please add the missing newline.
> diff --git a/drivers/hwmon/lm63.c b/drivers/hwmon/lm63.c
> index bf81aff..01b23ef 100644
> --- a/drivers/hwmon/lm63.c
> +++ b/drivers/hwmon/lm63.c
> @@ -53,7 +53,7 @@
> * Address is fully defined internally and cannot be changed.
> */
>
> -static const unsigned short normal_i2c[] = { 0x4c, I2C_CLIENT_END };
> +static const unsigned short normal_i2c[] = { 0x18, 0x4c, 0x4e, I2C_CLIENT_END };
>
> /*
> * The LM63 registers
> @@ -131,12 +131,15 @@ static struct lm63_data *lm63_update_device(struct device *dev);
> static int lm63_detect(struct i2c_client *client, struct i2c_board_info *info);
> static void lm63_init_client(struct i2c_client *client);
>
> +enum chips { lm63, lm64 };
> +
> /*
> * Driver data (common to all clients)
> */
>
> static const struct i2c_device_id lm63_id[] = {
> - { "lm63", 0 },
> + { "lm63", lm63 },
> + { "lm64", lm64 },
> { }
> };
> MODULE_DEVICE_TABLE(i2c, lm63_id);
> @@ -177,6 +180,7 @@ struct lm63_data {
> 2: remote high limit */
> u8 temp2_crit_hyst;
> u8 alarms;
> + int kind;
> };
>
> /*
> @@ -422,6 +426,7 @@ static int lm63_detect(struct i2c_client *new_client,
> struct i2c_adapter *adapter = new_client->adapter;
> u8 man_id, chip_id, reg_config1, reg_config2;
> u8 reg_alert_status, reg_alert_mask;
> + int address = new_client->addr;
>
> if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> return -ENODEV;
> @@ -439,7 +444,6 @@ static int lm63_detect(struct i2c_client *new_client,
> LM63_REG_ALERT_MASK);
>
> if (man_id != 0x01 /* National Semiconductor */
> - || chip_id != 0x41 /* LM63 */
> || (reg_config1 & 0x18) != 0x00
> || (reg_config2 & 0xF8) != 0x00
> || (reg_alert_status & 0x20) != 0x00
> @@ -450,7 +454,12 @@ static int lm63_detect(struct i2c_client *new_client,
> return -ENODEV;
> }
>
> - strlcpy(info->type, "lm63", I2C_NAME_SIZE);
> + if (chip_id == 0x41 && address == 0x4c)
> + strlcpy(info->type, "lm63", I2C_NAME_SIZE);
> + else if (chip_id == 0x51 && (address == 0x18 || address == 0x4e))
> + strlcpy(info->type, "lm64", I2C_NAME_SIZE);
> + else
> + return -ENODEV;
>
> return 0;
> }
> @@ -471,6 +480,8 @@ static int lm63_probe(struct i2c_client *new_client,
> data->valid = 0;
> mutex_init(&data->update_lock);
>
> + data->kind = id->driver_data;
You don't use this value anywhere, so why bother storing it?
> +
> /* Initialize the LM63 chip */
> lm63_init_client(new_client);
>
Your patch is missing an update to drivers/hwmon/Kconfig.
Please submit an updated patch and I'll apply it.
--
Jean Delvare
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH] hwmon: Add basic support for LM64 to lm63.c
2010-03-18 7:41 ` [lm-sensors] " Jean Delvare
@ 2010-03-18 13:36 ` Matthew Garrett
2010-03-19 10:18 ` Jean Delvare
2010-03-18 13:39 ` [lm-sensors] [PATCH] hwmon: Add basic support for lm64 " Matthew Garrett
1 sibling, 1 reply; 5+ messages in thread
From: Matthew Garrett @ 2010-03-18 13:36 UTC (permalink / raw)
To: khali; +Cc: lm-sensors, linux-kernel, Matthew Garrett
The LM64 appears to be an LM63 with added GPIO lines. Add support for the
hwmon functionality - GPIO can be added at some later stage if someone
has a need for them.
Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
Capitalisation fixes and remove an unused variable.
Documentation/hwmon/lm63 | 7 +++++++
drivers/hwmon/Kconfig | 9 +++++----
drivers/hwmon/lm63.c | 16 ++++++++++++----
3 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/Documentation/hwmon/lm63 b/Documentation/hwmon/lm63
index 31660bf..b9843ea 100644
--- a/Documentation/hwmon/lm63
+++ b/Documentation/hwmon/lm63
@@ -7,6 +7,11 @@ Supported chips:
Addresses scanned: I2C 0x4c
Datasheet: Publicly available at the National Semiconductor website
http://www.national.com/pf/LM/LM63.html
+ * National Semiconductor LM64
+ Prefix: 'lm64'
+ Addresses scanned: I2C 0x18 and 0x4e
+ Datasheet: Publicly available at the National Semiconductor website
+ http://www.national.com/pf/LM/LM64.html
Author: Jean Delvare <khali@linux-fr.org>
@@ -55,3 +60,5 @@ The lm63 driver will not update its values more frequently than every
second; reading them more often will do no harm, but will return 'old'
values.
+The LM64 is effectively an LM63 with GPIO lines. The driver does not
+support these GPIO lines at present.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index e4595e6..1aca601 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -450,10 +450,11 @@ config SENSORS_LM63
tristate "National Semiconductor LM63"
depends on I2C
help
- If you say yes here you get support for the National Semiconductor
- LM63 remote diode digital temperature sensor with integrated fan
- control. Such chips are found on the Tyan S4882 (Thunder K8QS Pro)
- motherboard, among others.
+ If you say yes here you get support for the National
+ Semiconductor LM63 and LM64 remote diode digital temperature
+ sensors with integrated fan control. Such chips are found
+ on the Tyan S4882 (Thunder K8QS Pro) motherboard, among
+ others.
This driver can also be built as a module. If so, the module
will be called lm63.
diff --git a/drivers/hwmon/lm63.c b/drivers/hwmon/lm63.c
index bf81aff..776aeb3 100644
--- a/drivers/hwmon/lm63.c
+++ b/drivers/hwmon/lm63.c
@@ -53,7 +53,7 @@
* Address is fully defined internally and cannot be changed.
*/
-static const unsigned short normal_i2c[] = { 0x4c, I2C_CLIENT_END };
+static const unsigned short normal_i2c[] = { 0x18, 0x4c, 0x4e, I2C_CLIENT_END };
/*
* The LM63 registers
@@ -131,12 +131,15 @@ static struct lm63_data *lm63_update_device(struct device *dev);
static int lm63_detect(struct i2c_client *client, struct i2c_board_info *info);
static void lm63_init_client(struct i2c_client *client);
+enum chips { lm63, lm64 };
+
/*
* Driver data (common to all clients)
*/
static const struct i2c_device_id lm63_id[] = {
- { "lm63", 0 },
+ { "lm63", lm63 },
+ { "lm64", lm64 },
{ }
};
MODULE_DEVICE_TABLE(i2c, lm63_id);
@@ -422,6 +425,7 @@ static int lm63_detect(struct i2c_client *new_client,
struct i2c_adapter *adapter = new_client->adapter;
u8 man_id, chip_id, reg_config1, reg_config2;
u8 reg_alert_status, reg_alert_mask;
+ int address = new_client->addr;
if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
return -ENODEV;
@@ -439,7 +443,6 @@ static int lm63_detect(struct i2c_client *new_client,
LM63_REG_ALERT_MASK);
if (man_id != 0x01 /* National Semiconductor */
- || chip_id != 0x41 /* LM63 */
|| (reg_config1 & 0x18) != 0x00
|| (reg_config2 & 0xF8) != 0x00
|| (reg_alert_status & 0x20) != 0x00
@@ -450,7 +453,12 @@ static int lm63_detect(struct i2c_client *new_client,
return -ENODEV;
}
- strlcpy(info->type, "lm63", I2C_NAME_SIZE);
+ if (chip_id == 0x41 && address == 0x4c)
+ strlcpy(info->type, "lm63", I2C_NAME_SIZE);
+ else if (chip_id == 0x51 && (address == 0x18 || address == 0x4e))
+ strlcpy(info->type, "lm64", I2C_NAME_SIZE);
+ else
+ return -ENODEV;
return 0;
}
--
1.7.0.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] hwmon: Add basic support for LM64 to lm63.c
2010-03-18 13:36 ` [PATCH] hwmon: Add basic support for LM64 " Matthew Garrett
@ 2010-03-19 10:18 ` Jean Delvare
0 siblings, 0 replies; 5+ messages in thread
From: Jean Delvare @ 2010-03-19 10:18 UTC (permalink / raw)
To: Matthew Garrett; +Cc: lm-sensors, linux-kernel
On Thu, 18 Mar 2010 09:36:59 -0400, Matthew Garrett wrote:
> The LM64 appears to be an LM63 with added GPIO lines. Add support for the
> hwmon functionality - GPIO can be added at some later stage if someone
> has a need for them.
>
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> ---
>
> Capitalisation fixes and remove an unused variable.
Applied, thanks!
--
Jean Delvare
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon: Add basic support for lm64 to lm63.c
2010-03-18 7:41 ` [lm-sensors] " Jean Delvare
2010-03-18 13:36 ` [PATCH] hwmon: Add basic support for LM64 " Matthew Garrett
@ 2010-03-18 13:39 ` Matthew Garrett
1 sibling, 0 replies; 5+ messages in thread
From: Matthew Garrett @ 2010-03-18 13:39 UTC (permalink / raw)
To: Jean Delvare; +Cc: lm-sensors, linux-kernel
Here's the dump from the lm64.
0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef
00: 34 3e 00 03 08 46 00 46 00 03 08 46 00 46 00 00 4>.??F.F.??F.F..
10: 60 10 00 00 00 00 e5 00 00 7b 1f 1f 00 00 00 00 `?....?..{??....
20: 00 64 00 00 00 00 00 00 00 00 00 00 00 00 00 00 .d..............
30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
40: 00 00 00 00 00 00 af 0f ff ff 00 00 05 08 00 05 ......??....??.?
50: 00 04 32 05 46 06 5a 07 5f 0b 64 0d 69 10 69 10 .?2?F?Z?_?d?i?i?
60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
b0: c0 20 2a d1 19 01 0d 00 00 00 00 00 00 00 00 00 ? *????.........
c0: 00 00 00 00 00 00 00 27 00 00 00 00 00 00 00 02 .......'.......?
d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 51 ..............?Q
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-03-19 10:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-17 19:55 [PATCH] hwmon: Add basic support for lm64 to lm63.c Matthew Garrett
2010-03-18 7:41 ` [lm-sensors] " Jean Delvare
2010-03-18 13:36 ` [PATCH] hwmon: Add basic support for LM64 " Matthew Garrett
2010-03-19 10:18 ` Jean Delvare
2010-03-18 13:39 ` [lm-sensors] [PATCH] hwmon: Add basic support for lm64 " Matthew Garrett
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox