* Re: [lm-sensors] PATCH:
2007-09-21 15:03 [lm-sensors] PATCH: Hans de Goede
@ 2007-09-24 14:28 ` Jean Delvare
2007-10-07 8:19 ` Jean Delvare
` (18 subsequent siblings)
19 siblings, 0 replies; 22+ messages in thread
From: Jean Delvare @ 2007-09-24 14:28 UTC (permalink / raw)
To: lm-sensors
Hi Hans,
On Fri, 21 Sep 2007 17:03:32 +0200, Hans de Goede wrote:
> Here is a patch adding some text to the sysfs interface documentation on how
> settings written to sysfs attributes should be handled, focussing mainly on
> error handling. This version incorperates Jean's latest comments.
>
> signed-off-by: j.w.r.degoede@hhs.nl
Thanks for doing this.
Acked-by: Jean Delvare <khali@linux-fr.org>
--
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] 22+ messages in thread* Re: [lm-sensors] PATCH:
2007-09-21 15:03 [lm-sensors] PATCH: Hans de Goede
2007-09-24 14:28 ` Jean Delvare
@ 2007-10-07 8:19 ` Jean Delvare
2007-10-07 11:37 ` Hans de Goede
` (17 subsequent siblings)
19 siblings, 0 replies; 22+ messages in thread
From: Jean Delvare @ 2007-10-07 8:19 UTC (permalink / raw)
To: lm-sensors
Hi Hans,
On Mon, 24 Sep 2007 16:28:16 +0200, Jean Delvare wrote:
> Hi Hans,
>
> On Fri, 21 Sep 2007 17:03:32 +0200, Hans de Goede wrote:
> > Here is a patch adding some text to the sysfs interface documentation on how
> > settings written to sysfs attributes should be handled, focussing mainly on
> > error handling. This version incorperates Jean's latest comments.
> >
> > signed-off-by: j.w.r.degoede@hhs.nl
>
> Thanks for doing this.
>
> Acked-by: Jean Delvare <khali@linux-fr.org>
While reviewing your new fschmd driver, I found some mistakes which
also appear in the code examples of this documentation update:
> +--- begin code ---
> +long v = simple_strtol(buf, NULL, 10) / 1000;
> +SENSORS_LIMIT(v, -128, 127);
This is a no-op, should be v = ...
> +/* write v to register */
> +--- end code ---
> +--- begin code ---
> +unsigned long v = simple_strtoul(buf, NULL, 10);
> +
> +switch (v) {
> + case 2: v = 1; break;
> + case 4: v = 2; break;
> + case 8: v = 3; break;
> + default:
> + return -EINVAL;
> +}
Indentation doesn't comply with CodingStyle.
> +/* write v to register */
> +--- end code ---
Additionally, I think that the examples are a bit difficult to read,
one level of indentation would probably be better than the begin/end
code markers. Thus, I propose the following incremental patch:
Documentation/hwmon/sysfs-interface | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
--- linux-2.6.23-rc9.orig/Documentation/hwmon/sysfs-interface 2007-10-07 10:12:33.000000000 +0200
+++ linux-2.6.23-rc9/Documentation/hwmon/sysfs-interface 2007-10-07 10:13:43.000000000 +0200
@@ -450,22 +450,20 @@ continuous like for example a tempX_type
written, -EINVAL should be returned.
Example1, temp1_max, register is a signed 8 bit value (-128 - 127 degrees):
---- begin code ---
-long v = simple_strtol(buf, NULL, 10) / 1000;
-SENSORS_LIMIT(v, -128, 127);
-/* write v to register */
---- end code ---
+
+ long v = simple_strtol(buf, NULL, 10) / 1000;
+ v = SENSORS_LIMIT(v, -128, 127);
+ /* write v to register */
Example2, fan divider setting, valid values 2, 4 and 8:
---- begin code ---
-unsigned long v = simple_strtoul(buf, NULL, 10);
-switch (v) {
- case 2: v = 1; break;
- case 4: v = 2; break;
- case 8: v = 3; break;
- default:
- return -EINVAL;
-}
-/* write v to register */
---- end code ---
+ unsigned long v = simple_strtoul(buf, NULL, 10);
+
+ switch (v) {
+ case 2: v = 1; break;
+ case 4: v = 2; break;
+ case 8: v = 3; break;
+ default:
+ return -EINVAL;
+ }
+ /* write v to register */
--
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] 22+ messages in thread* Re: [lm-sensors] PATCH:
2007-09-21 15:03 [lm-sensors] PATCH: Hans de Goede
2007-09-24 14:28 ` Jean Delvare
2007-10-07 8:19 ` Jean Delvare
@ 2007-10-07 11:37 ` Hans de Goede
2007-10-07 18:24 ` Mark M. Hoffman
` (16 subsequent siblings)
19 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2007-10-07 11:37 UTC (permalink / raw)
To: lm-sensors
Jean Delvare wrote:
> Hi Hans,
>
> On Mon, 24 Sep 2007 16:28:16 +0200, Jean Delvare wrote:
>> Hi Hans,
>>
>> On Fri, 21 Sep 2007 17:03:32 +0200, Hans de Goede wrote:
>>> Here is a patch adding some text to the sysfs interface documentation on how
>>> settings written to sysfs attributes should be handled, focussing mainly on
>>> error handling. This version incorperates Jean's latest comments.
>>>
>>> signed-off-by: j.w.r.degoede@hhs.nl
>> Thanks for doing this.
>>
>> Acked-by: Jean Delvare <khali@linux-fr.org>
>
> While reviewing your new fschmd driver, I found some mistakes which
> also appear in the code examples of this documentation update:
>
>> +--- begin code ---
>> +long v = simple_strtol(buf, NULL, 10) / 1000;
>> +SENSORS_LIMIT(v, -128, 127);
>
> This is a no-op, should be v = ...
>
Yes, I already noticed myself, but you beat me to writing a patch, thanks!
Notice that this is what happens when you make a function look like a macro by
giving it all caps names. Note: I'm not trying to talk my mistake right, just
trying to explain how I came to the mistake. Maybe we should rename SENSORS_LIMIT?
>> +/* write v to register */
>> +--- end code ---
>
>> +--- begin code ---
>> +unsigned long v = simple_strtoul(buf, NULL, 10);
>> +
>> +switch (v) {
>> + case 2: v = 1; break;
>> + case 4: v = 2; break;
>> + case 8: v = 3; break;
>> + default:
>> + return -EINVAL;
>> +}
>
> Indentation doesn't comply with CodingStyle.
>
>> +/* write v to register */
>> +--- end code ---
>
> Additionally, I think that the examples are a bit difficult to read,
> one level of indentation would probably be better than the begin/end
> code markers. Thus, I propose the following incremental patch:
>
> Documentation/hwmon/sysfs-interface | 30 ++++++++++++++----------------
> 1 file changed, 14 insertions(+), 16 deletions(-)
>
> --- linux-2.6.23-rc9.orig/Documentation/hwmon/sysfs-interface 2007-10-07 10:12:33.000000000 +0200
> +++ linux-2.6.23-rc9/Documentation/hwmon/sysfs-interface 2007-10-07 10:13:43.000000000 +0200
> @@ -450,22 +450,20 @@ continuous like for example a tempX_type
> written, -EINVAL should be returned.
>
> Example1, temp1_max, register is a signed 8 bit value (-128 - 127 degrees):
> ---- begin code ---
> -long v = simple_strtol(buf, NULL, 10) / 1000;
> -SENSORS_LIMIT(v, -128, 127);
> -/* write v to register */
> ---- end code ---
> +
> + long v = simple_strtol(buf, NULL, 10) / 1000;
> + v = SENSORS_LIMIT(v, -128, 127);
> + /* write v to register */
>
> Example2, fan divider setting, valid values 2, 4 and 8:
> ---- begin code ---
> -unsigned long v = simple_strtoul(buf, NULL, 10);
>
> -switch (v) {
> - case 2: v = 1; break;
> - case 4: v = 2; break;
> - case 8: v = 3; break;
> - default:
> - return -EINVAL;
> -}
> -/* write v to register */
> ---- end code ---
> + unsigned long v = simple_strtoul(buf, NULL, 10);
> +
> + switch (v) {
> + case 2: v = 1; break;
> + case 4: v = 2; break;
> + case 8: v = 3; break;
> + default:
> + return -EINVAL;
> + }
> + /* write v to register */
>
>
I agree with all changes, Mark pleaee apply:
Acked-by: Hans de Goede <j.w.r.degoede@hhs.nl>
Regards,
Hans
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [lm-sensors] PATCH:
2007-09-21 15:03 [lm-sensors] PATCH: Hans de Goede
` (2 preceding siblings ...)
2007-10-07 11:37 ` Hans de Goede
@ 2007-10-07 18:24 ` Mark M. Hoffman
2007-10-07 18:28 ` Mark M. Hoffman
` (15 subsequent siblings)
19 siblings, 0 replies; 22+ messages in thread
From: Mark M. Hoffman @ 2007-10-07 18:24 UTC (permalink / raw)
To: lm-sensors
Hi:
* Jean Delvare <khali@linux-fr.org> [2007-09-24 16:28:16 +0200]:
> Hi Hans,
>
> On Fri, 21 Sep 2007 17:03:32 +0200, Hans de Goede wrote:
> > Here is a patch adding some text to the sysfs interface documentation on how
> > settings written to sysfs attributes should be handled, focussing mainly on
> > error handling. This version incorperates Jean's latest comments.
> >
> > signed-off-by: j.w.r.degoede@hhs.nl
>
> Thanks for doing this.
>
> Acked-by: Jean Delvare <khali@linux-fr.org>
Applied to hwmon-2.6.git/testing, thanks.
--
Mark M. Hoffman
mhoffman@lightlink.com
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [lm-sensors] PATCH:
2007-09-21 15:03 [lm-sensors] PATCH: Hans de Goede
` (3 preceding siblings ...)
2007-10-07 18:24 ` Mark M. Hoffman
@ 2007-10-07 18:28 ` Mark M. Hoffman
2007-10-07 20:41 ` Jean Delvare
` (14 subsequent siblings)
19 siblings, 0 replies; 22+ messages in thread
From: Mark M. Hoffman @ 2007-10-07 18:28 UTC (permalink / raw)
To: lm-sensors
Hi Jean:
> Jean Delvare wrote:
> > Additionally, I think that the examples are a bit difficult to read,
> > one level of indentation would probably be better than the begin/end
> > code markers. Thus, I propose the following incremental patch:
> >
> > Documentation/hwmon/sysfs-interface | 30 ++++++++++++++----------------
> > 1 file changed, 14 insertions(+), 16 deletions(-)
* Hans de Goede <j.w.r.degoede@hhs.nl> [2007-10-07 13:37:53 +0200]:
> I agree with all changes, Mark pleaee apply:
>
> Acked-by: Hans de Goede <j.w.r.degoede@hhs.nl>
Could you please re-generate this one against the hwmon testing tree and
w/ a Signed-off-by?
Thanks & regards,
--
Mark M. Hoffman
mhoffman@lightlink.com
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [lm-sensors] PATCH:
2007-09-21 15:03 [lm-sensors] PATCH: Hans de Goede
` (4 preceding siblings ...)
2007-10-07 18:28 ` Mark M. Hoffman
@ 2007-10-07 20:41 ` Jean Delvare
2007-10-07 22:05 ` Jean Delvare
` (13 subsequent siblings)
19 siblings, 0 replies; 22+ messages in thread
From: Jean Delvare @ 2007-10-07 20:41 UTC (permalink / raw)
To: lm-sensors
Hi Hans,
On Sun, 07 Oct 2007 13:37:53 +0200, Hans de Goede wrote:
> Notice that this is what happens when you make a function look like a macro by
> giving it all caps names. Note: I'm not trying to talk my mistake right, just
> trying to explain how I came to the mistake. Maybe we should rename SENSORS_LIMIT?
David Brownell would agree:
http://lists.lm-sensors.org/pipermail/lm-sensors/2007-September/021304.html
And I do agree, too.
--
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] 22+ messages in thread* Re: [lm-sensors] PATCH:
2007-09-21 15:03 [lm-sensors] PATCH: Hans de Goede
` (5 preceding siblings ...)
2007-10-07 20:41 ` Jean Delvare
@ 2007-10-07 22:05 ` Jean Delvare
2007-10-31 8:42 ` Hans de Goede
` (12 subsequent siblings)
19 siblings, 0 replies; 22+ messages in thread
From: Jean Delvare @ 2007-10-07 22:05 UTC (permalink / raw)
To: lm-sensors
Hi Mark,
On Sun, 7 Oct 2007 15:33:01 -0400, Mark M. Hoffman wrote:
> Although, I suppose it would be easier for you and maybe Jean if I just publish
> a patch series myself. It shouldn't be too hard to script that up; I'll look
> into it.
I don't need it too much, as I am tracking the hwmon patches myself so
my stack and yours are very similar; when I'm missing a patch, I
cherry-pick it from git-web.
But I agree that it would be convenient to have the possibility for
others to get all the patches as either a single patch or a patch
series. For now I have been pointing developers to git-hwmon.patch in
Andrew's tree, but it may become quickly out-of-date if Andrew doesn't
release a new kernel in a while.
--
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] 22+ messages in thread* [lm-sensors] PATCH:
2007-09-21 15:03 [lm-sensors] PATCH: Hans de Goede
` (6 preceding siblings ...)
2007-10-07 22:05 ` Jean Delvare
@ 2007-10-31 8:42 ` Hans de Goede
2007-10-31 15:17 ` Jean Delvare
` (11 subsequent siblings)
19 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2007-10-31 8:42 UTC (permalink / raw)
To: lm-sensors
[-- Attachment #1: Type: text/plain, Size: 858 bytes --]
Hi All,
As discussed on the lm-sensors list, using the fixed scaling factors from the
fscpos datasheet for the fscher and newer models as wel, was nood a good idea,
as the actual scaling factors may be different and can be read from OEM DMI
entries.
This patch changes the behaviour of the merged FSC familiy fschmd driver to be
the same as the old standalone fscher driver for fscher and newer models,
leaving the scaling to userspace.
This also preserves compatibility of existing sensors.conf files.
Signed-of-by: Hans de Goede <j.w.r.degoede@hhs.nl>
---
Jean, can you review this please?
Mark, can you send this to Linus for 2.6.24 please, otherwise we will have one
official kernel release with different scaling for the voltage inputs of the
fschmd compared to the next releases.
And yes I tested my patch this time.
Thanks & Regards,
Hans
[-- Attachment #2: hwmon-fschmd-fscher-and-newer-no-fixed-scaling.patch --]
[-- Type: text/x-patch, Size: 1730 bytes --]
As discussed on the lm-sensors list, using the fixed scaling factors from the
fscpos datasheet for the fscher and newer models as wel, was nood a good idea,
as the actual scaling factors may be different and can be read from OEM DMI
entries.
This patch changes the behaviour of the merged FSC familiy fschmd driver to be
the same as the old standalone fscher driver for fscher and newer models,
leaving the scaling to userspace.
This also preserves compatibility of existing sensors.conf files.
Signed-of-by: Hans de Goede <j.w.r.degoede@hhs.nl>
diff -up linux-2.6.24-rc1-git7/drivers/hwmon/fschmd.c~ linux-2.6.24-rc1-git7/drivers/hwmon/fschmd.c
--- linux-2.6.24-rc1-git7/drivers/hwmon/fschmd.c~ 2007-10-30 22:30:43.000000000 +0100
+++ linux-2.6.24-rc1-git7/drivers/hwmon/fschmd.c 2007-10-30 22:30:43.000000000 +0100
@@ -221,8 +221,17 @@ static ssize_t show_in_value(struct devi
int index = to_sensor_dev_attr(devattr)->index;
struct fschmd_data *data = fschmd_update_device(dev);
- return sprintf(buf, "%d\n", (data->volt[index] *
- max_reading[index] + 128) / 255);
+ /* Only use fixed scaling factor for the fscpos and fscscy, the newer
+ models have the scaling factors stored in the BIOS DMI tables, so
+ here we use a scale of 10mv / step like the standalone fscher driver
+ does (did) and let userspace do the real scaling. There is a
+ userspace utility called fscher-dmi2compute to generate the proper
+ compute lines from the DMI info for the fscher and newer chips. */
+ if (data->kind == (fscpos - 1) || data->kind == (fscscy - 1))
+ return sprintf(buf, "%d\n", (data->volt[index] *
+ max_reading[index] + 128) / 255);
+ else
+ return sprintf(buf, "%d\n", data->volt[index] * 10);
}
[-- Attachment #3: Type: text/plain, Size: 153 bytes --]
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [lm-sensors] PATCH:
2007-09-21 15:03 [lm-sensors] PATCH: Hans de Goede
` (7 preceding siblings ...)
2007-10-31 8:42 ` Hans de Goede
@ 2007-10-31 15:17 ` Jean Delvare
2007-11-01 23:00 ` Jean Delvare
` (10 subsequent siblings)
19 siblings, 0 replies; 22+ messages in thread
From: Jean Delvare @ 2007-10-31 15:17 UTC (permalink / raw)
To: lm-sensors
On Wed, 31 Oct 2007 09:42:38 +0100, Hans de Goede wrote:
> Hi All,
>
> As discussed on the lm-sensors list, using the fixed scaling factors from the
> fscpos datasheet for the fscher and newer models as wel, was nood a good idea,
> as the actual scaling factors may be different and can be read from OEM DMI
> entries.
>
> This patch changes the behaviour of the merged FSC familiy fschmd driver to be
> the same as the old standalone fscher driver for fscher and newer models,
> leaving the scaling to userspace.
>
> This also preserves compatibility of existing sensors.conf files.
>
> Signed-of-by: Hans de Goede <j.w.r.degoede@hhs.nl>
>
> ---
>
> Jean, can you review this please?
> Mark, can you send this to Linus for 2.6.24 please, otherwise we will have one
> official kernel release with different scaling for the voltage inputs of the
> fschmd compared to the next releases.
>
>
> And yes I tested my patch this time.
Assuming that you definitely excluded the possibility to retrieve the
DMI data from the kernel and you have additional information about the
Heimdall and Heracles also using DMI data for the scaling, then the
patch is correct:
Acked-by: Jean Delvare <khali@linux-fr.org>
--
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] 22+ messages in thread* Re: [lm-sensors] PATCH:
2007-09-21 15:03 [lm-sensors] PATCH: Hans de Goede
` (8 preceding siblings ...)
2007-10-31 15:17 ` Jean Delvare
@ 2007-11-01 23:00 ` Jean Delvare
2007-11-02 7:35 ` Hans de Goede
` (9 subsequent siblings)
19 siblings, 0 replies; 22+ messages in thread
From: Jean Delvare @ 2007-11-01 23:00 UTC (permalink / raw)
To: lm-sensors
Hi Hans,
On Wed, 31 Oct 2007 16:29:44 +0100, Hans de Goede wrote:
> Jean Delvare wrote:
> > Assuming that you definitely excluded the possibility to retrieve the
> > DMI data from the kernel
>
> Definitely is a big word, I've looked at the DMI parser currently in the
> kernel, tried to come up with a way to get the info there which would be more
> generic then just being an fschmd specific hack and couldn't. Then I slept a
> night on it and still couldn't come up with something clean, esp since there
> are many type 185 entries in the DMI table of FSC machines, of which we need
> only one.
Please take a look at this completely untested patch. It might need
some cleanups before it's ready for submission, but is there any reason
why something like this wouldn't do the job?
---
drivers/firmware/dmi_scan.c | 50 ++++++++++++++++++++++++++++++++++++++++++-
drivers/hwmon/fschmd.c | 9 +++++++
include/linux/dmi.h | 3 ++
3 files changed, 61 insertions(+), 1 deletion(-)
--- linux-2.6.24-rc1.orig/drivers/firmware/dmi_scan.c 2007-10-24 09:59:28.000000000 +0200
+++ linux-2.6.24-rc1/drivers/firmware/dmi_scan.c 2007-11-01 23:51:46.000000000 +0100
@@ -86,6 +86,50 @@ static int __init dmi_checksum(const u8
static char *dmi_ident[DMI_STRING_MAX];
static LIST_HEAD(dmi_devices);
int dmi_available;
+static u32 dmi_base;
+static u16 dmi_len;
+static u16 dmi_num;
+
+/* Similar to dmi_table, but for use at a later time */
+int dmi_walk(void (*decode)(const struct dmi_header *))
+{
+ u8 *buf, *data;
+ int i = 0;
+
+ if (!dmi_available)
+ return -1;
+
+ buf = ioremap(dmi_base, dmi_len);
+ if (buf = NULL)
+ return -1;
+
+ data = buf;
+
+ /*
+ * Stop when we see all the items the table claimed to have
+ * OR we run off the end of the table (also happens)
+ */
+ while ((i < dmi_num) &&
+ (data - buf + sizeof(struct dmi_header)) <= dmi_len) {
+ const struct dmi_header *dm = (const struct dmi_header *)data;
+
+ /*
+ * We want to know the total length (formated area and strings)
+ * before decoding to make sure we won't run off the table in
+ * dmi_decode or dmi_string
+ */
+ data += dm->length;
+ while ((data - buf < dmi_len - 1) && (data[0] || data[1]))
+ data++;
+ if (data - buf < dmi_len - 1)
+ decode(dm);
+ data += 2;
+ i++;
+ }
+ iounmap(buf);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(dmi_walk);
/*
* Save a DMI string
@@ -287,8 +331,12 @@ static int __init dmi_present(const char
buf[14] >> 4, buf[14] & 0xF);
else
printk(KERN_INFO "DMI present.\n");
- if (dmi_table(base,len, num, dmi_decode) = 0)
+ if (dmi_table(base,len, num, dmi_decode) = 0) {
+ dmi_base = base;
+ dmi_len = len;
+ dmi_num = num;
return 0;
+ }
}
return 1;
}
--- linux-2.6.24-rc1.orig/drivers/hwmon/fschmd.c 2007-11-01 23:16:54.000000000 +0100
+++ linux-2.6.24-rc1/drivers/hwmon/fschmd.c 2007-11-01 23:56:02.000000000 +0100
@@ -41,6 +41,7 @@
#include <linux/err.h>
#include <linux/mutex.h>
#include <linux/sysfs.h>
+#include <linux/dmi.h>
/* Addresses to scan */
static unsigned short normal_i2c[] = { 0x73, I2C_CLIENT_END };
@@ -768,8 +769,16 @@ static struct fschmd_data *fschmd_update
return data;
}
+static void __init fschmd_get_dmi_data(const struct dmi_header *h)
+{
+ if (h->type = 185) {
+ /* Do stuff */
+ }
+}
+
static int __init fschmd_init(void)
{
+ dmi_walk(fschmd_get_dmi_data);
return i2c_add_driver(&fschmd_driver);
}
--- linux-2.6.24-rc1.orig/include/linux/dmi.h 2007-10-24 09:59:51.000000000 +0200
+++ linux-2.6.24-rc1/include/linux/dmi.h 2007-11-01 23:52:54.000000000 +0100
@@ -78,6 +78,7 @@ extern const struct dmi_device * dmi_fin
extern void dmi_scan_machine(void);
extern int dmi_get_year(int field);
extern int dmi_name_in_vendors(const char *str);
+extern int dmi_walk(void (*decode)(const struct dmi_header *));
#else
@@ -87,6 +88,8 @@ static inline const struct dmi_device *
const struct dmi_device *from) { return NULL; }
static inline int dmi_get_year(int year) { return 0; }
static inline int dmi_name_in_vendors(const char *s) { return 0; }
+static inline int dmi_walk(void (*decode)(const struct dmi_header *))
+ { return -1; }
#endif
--
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] 22+ messages in thread* Re: [lm-sensors] PATCH:
2007-09-21 15:03 [lm-sensors] PATCH: Hans de Goede
` (9 preceding siblings ...)
2007-11-01 23:00 ` Jean Delvare
@ 2007-11-02 7:35 ` Hans de Goede
2007-11-02 9:44 ` Jean Delvare
` (8 subsequent siblings)
19 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2007-11-02 7:35 UTC (permalink / raw)
To: lm-sensors
Jean Delvare wrote:
> Hi Hans,
>
> On Wed, 31 Oct 2007 16:29:44 +0100, Hans de Goede wrote:
>> Jean Delvare wrote:
>>> Assuming that you definitely excluded the possibility to retrieve the
>>> DMI data from the kernel
>> Definitely is a big word, I've looked at the DMI parser currently in the
>> kernel, tried to come up with a way to get the info there which would be more
>> generic then just being an fschmd specific hack and couldn't. Then I slept a
>> night on it and still couldn't come up with something clean, esp since there
>> are many type 185 entries in the DMI table of FSC machines, of which we need
>> only one.
>
> Please take a look at this completely untested patch. It might need
> some cleanups before it's ready for submission, but is there any reason
> why something like this wouldn't do the job?
>
Good work Jean, yes that should do the job, I stared myself blind at adding a
clean way to store the data for later retrieval to the existing __init parser,
your way indeed will work and is much more generic.
I'll compile a kernel with the dmi_scan.c and dmi.h parts patched in and then
start working on integrating this into fschmd.c
Question, what do we do if the dmi data doesn't get found, use some defaults I
guess?
Mark,
Please drop the hwmon-fschmd-fscher-and-newer-no-fixed-scaling.patch from your
queue (for now, might need to introduce it later if this won't work, but thats
unlikely).
Thanks & Regards,
Hans
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [lm-sensors] PATCH:
2007-09-21 15:03 [lm-sensors] PATCH: Hans de Goede
` (10 preceding siblings ...)
2007-11-02 7:35 ` Hans de Goede
@ 2007-11-02 9:44 ` Jean Delvare
2007-11-02 19:41 ` Hans de Goede
` (7 subsequent siblings)
19 siblings, 0 replies; 22+ messages in thread
From: Jean Delvare @ 2007-11-02 9:44 UTC (permalink / raw)
To: lm-sensors
On Fri, 02 Nov 2007 08:35:53 +0100, Hans de Goede wrote:
> Good work Jean, yes that should do the job, I stared myself blind at adding a
> clean way to store the data for later retrieval to the existing __init parser,
> your way indeed will work and is much more generic.
>
> I'll compile a kernel with the dmi_scan.c and dmi.h parts patched in and then
> start working on integrating this into fschmd.c
Here's an updated version of my patch, with no code duplication this
time. This should be easier to get this accepted upstream. To make the
code even smaller, dmi_table() could be merged into dmi_present() but
that's about it.
> Question, what do we do if the dmi data doesn't get found, use some defaults I
> guess?
Don't export the voltage values at all? I'd rather not export anything
than silently export values which may not be correct. But hopefully it
will never happen so it doesn't really matter what we decide here.
* * * * *
Subject: dmi: Let drivers walk the DMI table
Let drivers walk the DMI table for their own needs. Some drivers need
data stored in OEM-specific DMI records for proper operation.
Signed-off-by: Jean Delvare <khali@linux-fr.org>
---
drivers/firmware/dmi_scan.c | 59 +++++++++++++++++++++++++++++++++----------
include/linux/dmi.h | 3 ++
2 files changed, 49 insertions(+), 13 deletions(-)
--- linux-2.6.24-rc1.orig/drivers/firmware/dmi_scan.c 2007-11-02 10:27:01.000000000 +0100
+++ linux-2.6.24-rc1/drivers/firmware/dmi_scan.c 2007-11-02 10:27:19.000000000 +0100
@@ -36,18 +36,12 @@ static char * __init dmi_string(const st
* We have to be cautious here. We have seen BIOSes with DMI pointers
* pointing to completely the wrong place for example
*/
-static int __init dmi_table(u32 base, int len, int num,
- void (*decode)(const struct dmi_header *))
+static void __dmi_table(u8 *buf, int len, int num,
+ void (*decode)(const struct dmi_header *))
{
- u8 *buf, *data;
+ u8 *data = buf;
int i = 0;
- buf = dmi_ioremap(base, len);
- if (buf = NULL)
- return -1;
-
- data = buf;
-
/*
* Stop when we see all the items the table claimed to have
* OR we run off the end of the table (also happens)
@@ -68,6 +62,19 @@ static int __init dmi_table(u32 base, in
data += 2;
i++;
}
+}
+
+static int __init dmi_table(u32 base, int len, int num,
+ void (*decode)(const struct dmi_header *))
+{
+ u8 *buf;
+
+ buf = dmi_ioremap(base, len);
+ if (buf = NULL)
+ return -1;
+
+ __dmi_table(buf, len, num, decode);
+
dmi_iounmap(buf, len);
return 0;
}
@@ -86,6 +93,9 @@ static int __init dmi_checksum(const u8
static char *dmi_ident[DMI_STRING_MAX];
static LIST_HEAD(dmi_devices);
int dmi_available;
+static u32 dmi_base;
+static u16 dmi_len;
+static u16 dmi_num;
/*
* Save a DMI string
@@ -273,9 +283,9 @@ static int __init dmi_present(const char
memcpy_fromio(buf, p, 15);
if ((memcmp(buf, "_DMI_", 5) = 0) && dmi_checksum(buf)) {
- u16 num = (buf[13] << 8) | buf[12];
- u16 len = (buf[7] << 8) | buf[6];
- u32 base = (buf[11] << 24) | (buf[10] << 16) |
+ dmi_num = (buf[13] << 8) | buf[12];
+ dmi_len = (buf[7] << 8) | buf[6];
+ dmi_base = (buf[11] << 24) | (buf[10] << 16) |
(buf[9] << 8) | buf[8];
/*
@@ -287,7 +297,7 @@ static int __init dmi_present(const char
buf[14] >> 4, buf[14] & 0xF);
else
printk(KERN_INFO "DMI present.\n");
- if (dmi_table(base,len, num, dmi_decode) = 0)
+ if (dmi_table(dmi_base, dmi_len, dmi_num, dmi_decode) = 0)
return 0;
}
return 1;
@@ -470,3 +480,26 @@ int dmi_get_year(int field)
return year;
}
+/**
+ * dmi_walk - Walk the DMI table and get called back for every record
+ * @decode: Callback function
+ *
+ * Returns -1 when the DMI table can't be reached, 0 on success.
+ */
+int dmi_walk(void (*decode)(const struct dmi_header *))
+{
+ u8 *buf;
+
+ if (!dmi_available)
+ return -1;
+
+ buf = ioremap(dmi_base, dmi_len);
+ if (buf = NULL)
+ return -1;
+
+ __dmi_table(buf, dmi_len, dmi_num, decode);
+
+ iounmap(buf);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(dmi_walk);
--- linux-2.6.24-rc1.orig/include/linux/dmi.h 2007-11-02 10:27:01.000000000 +0100
+++ linux-2.6.24-rc1/include/linux/dmi.h 2007-11-02 10:27:19.000000000 +0100
@@ -78,6 +78,7 @@ extern const struct dmi_device * dmi_fin
extern void dmi_scan_machine(void);
extern int dmi_get_year(int field);
extern int dmi_name_in_vendors(const char *str);
+extern int dmi_walk(void (*decode)(const struct dmi_header *));
#else
@@ -87,6 +88,8 @@ static inline const struct dmi_device *
const struct dmi_device *from) { return NULL; }
static inline int dmi_get_year(int year) { return 0; }
static inline int dmi_name_in_vendors(const char *s) { return 0; }
+static inline int dmi_walk(void (*decode)(const struct dmi_header *))
+ { return -1; }
#endif
--
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] 22+ messages in thread* Re: [lm-sensors] PATCH:
2007-09-21 15:03 [lm-sensors] PATCH: Hans de Goede
` (11 preceding siblings ...)
2007-11-02 9:44 ` Jean Delvare
@ 2007-11-02 19:41 ` Hans de Goede
2007-11-03 16:29 ` Jean Delvare
` (6 subsequent siblings)
19 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2007-11-02 19:41 UTC (permalink / raw)
To: lm-sensors
Jean Delvare wrote:
> On Fri, 02 Nov 2007 08:35:53 +0100, Hans de Goede wrote:
>> Good work Jean, yes that should do the job, I stared myself blind at adding a
>> clean way to store the data for later retrieval to the existing __init parser,
>> your way indeed will work and is much more generic.
>>
>> I'll compile a kernel with the dmi_scan.c and dmi.h parts patched in and then
>> start working on integrating this into fschmd.c
>
> Here's an updated version of my patch, with no code duplication this
> time. This should be easier to get this accepted upstream. To make the
> code even smaller, dmi_table() could be merged into dmi_present() but
> that's about it.
>
Looks good, I found inconsistency though, in essence the dmi_table() and
dmi_walk() are the same, (except for dmi_ioremap versus ioremap ofcourse) but
for one you just use the base, len and number of items stored in global
variables, while for the other you pass those same global variables through
parameters, for consistency sake I think you should to both the same.
>> Question, what do we do if the dmi data doesn't get found, use some defaults I
>> guess?
>
> Don't export the voltage values at all? I'd rather not export anything
> than silently export values which may not be correct. But hopefully it
> will never happen so it doesn't really matter what we decide here.
>
I would rather just printk a warning and use some sane defaults in this
scenario. We must definetively alert the user, that I agree with. The problem
with a printk is ofcourse that it will hardly be noticed.
Regards,
Hans
> * * * * *
>
> Subject: dmi: Let drivers walk the DMI table
>
> Let drivers walk the DMI table for their own needs. Some drivers need
> data stored in OEM-specific DMI records for proper operation.
>
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> ---
> drivers/firmware/dmi_scan.c | 59 +++++++++++++++++++++++++++++++++----------
> include/linux/dmi.h | 3 ++
> 2 files changed, 49 insertions(+), 13 deletions(-)
>
> --- linux-2.6.24-rc1.orig/drivers/firmware/dmi_scan.c 2007-11-02 10:27:01.000000000 +0100
> +++ linux-2.6.24-rc1/drivers/firmware/dmi_scan.c 2007-11-02 10:27:19.000000000 +0100
> @@ -36,18 +36,12 @@ static char * __init dmi_string(const st
> * We have to be cautious here. We have seen BIOSes with DMI pointers
> * pointing to completely the wrong place for example
> */
> -static int __init dmi_table(u32 base, int len, int num,
> - void (*decode)(const struct dmi_header *))
> +static void __dmi_table(u8 *buf, int len, int num,
> + void (*decode)(const struct dmi_header *))
> {
> - u8 *buf, *data;
> + u8 *data = buf;
> int i = 0;
>
> - buf = dmi_ioremap(base, len);
> - if (buf = NULL)
> - return -1;
> -
> - data = buf;
> -
> /*
> * Stop when we see all the items the table claimed to have
> * OR we run off the end of the table (also happens)
> @@ -68,6 +62,19 @@ static int __init dmi_table(u32 base, in
> data += 2;
> i++;
> }
> +}
> +
> +static int __init dmi_table(u32 base, int len, int num,
> + void (*decode)(const struct dmi_header *))
> +{
> + u8 *buf;
> +
> + buf = dmi_ioremap(base, len);
> + if (buf = NULL)
> + return -1;
> +
> + __dmi_table(buf, len, num, decode);
> +
> dmi_iounmap(buf, len);
> return 0;
> }
> @@ -86,6 +93,9 @@ static int __init dmi_checksum(const u8
> static char *dmi_ident[DMI_STRING_MAX];
> static LIST_HEAD(dmi_devices);
> int dmi_available;
> +static u32 dmi_base;
> +static u16 dmi_len;
> +static u16 dmi_num;
>
> /*
> * Save a DMI string
> @@ -273,9 +283,9 @@ static int __init dmi_present(const char
>
> memcpy_fromio(buf, p, 15);
> if ((memcmp(buf, "_DMI_", 5) = 0) && dmi_checksum(buf)) {
> - u16 num = (buf[13] << 8) | buf[12];
> - u16 len = (buf[7] << 8) | buf[6];
> - u32 base = (buf[11] << 24) | (buf[10] << 16) |
> + dmi_num = (buf[13] << 8) | buf[12];
> + dmi_len = (buf[7] << 8) | buf[6];
> + dmi_base = (buf[11] << 24) | (buf[10] << 16) |
> (buf[9] << 8) | buf[8];
>
> /*
> @@ -287,7 +297,7 @@ static int __init dmi_present(const char
> buf[14] >> 4, buf[14] & 0xF);
> else
> printk(KERN_INFO "DMI present.\n");
> - if (dmi_table(base,len, num, dmi_decode) = 0)
> + if (dmi_table(dmi_base, dmi_len, dmi_num, dmi_decode) = 0)
> return 0;
> }
> return 1;
> @@ -470,3 +480,26 @@ int dmi_get_year(int field)
> return year;
> }
>
> +/**
> + * dmi_walk - Walk the DMI table and get called back for every record
> + * @decode: Callback function
> + *
> + * Returns -1 when the DMI table can't be reached, 0 on success.
> + */
> +int dmi_walk(void (*decode)(const struct dmi_header *))
> +{
> + u8 *buf;
> +
> + if (!dmi_available)
> + return -1;
> +
> + buf = ioremap(dmi_base, dmi_len);
> + if (buf = NULL)
> + return -1;
> +
> + __dmi_table(buf, dmi_len, dmi_num, decode);
> +
> + iounmap(buf);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(dmi_walk);
> --- linux-2.6.24-rc1.orig/include/linux/dmi.h 2007-11-02 10:27:01.000000000 +0100
> +++ linux-2.6.24-rc1/include/linux/dmi.h 2007-11-02 10:27:19.000000000 +0100
> @@ -78,6 +78,7 @@ extern const struct dmi_device * dmi_fin
> extern void dmi_scan_machine(void);
> extern int dmi_get_year(int field);
> extern int dmi_name_in_vendors(const char *str);
> +extern int dmi_walk(void (*decode)(const struct dmi_header *));
>
> #else
>
> @@ -87,6 +88,8 @@ static inline const struct dmi_device *
> const struct dmi_device *from) { return NULL; }
> static inline int dmi_get_year(int year) { return 0; }
> static inline int dmi_name_in_vendors(const char *s) { return 0; }
> +static inline int dmi_walk(void (*decode)(const struct dmi_header *))
> + { return -1; }
>
> #endif
>
>
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [lm-sensors] PATCH:
2007-09-21 15:03 [lm-sensors] PATCH: Hans de Goede
` (12 preceding siblings ...)
2007-11-02 19:41 ` Hans de Goede
@ 2007-11-03 16:29 ` Jean Delvare
2007-11-04 13:21 ` Hans de Goede
` (5 subsequent siblings)
19 siblings, 0 replies; 22+ messages in thread
From: Jean Delvare @ 2007-11-03 16:29 UTC (permalink / raw)
To: lm-sensors
Hi Hans,
On Fri, 02 Nov 2007 20:41:19 +0100, Hans de Goede wrote:
> Jean Delvare wrote:
> > On Fri, 02 Nov 2007 08:35:53 +0100, Hans de Goede wrote:
> >> Good work Jean, yes that should do the job, I stared myself blind at adding a
> >> clean way to store the data for later retrieval to the existing __init parser,
> >> your way indeed will work and is much more generic.
> >>
> >> I'll compile a kernel with the dmi_scan.c and dmi.h parts patched in and then
> >> start working on integrating this into fschmd.c
> >
> > Here's an updated version of my patch, with no code duplication this
> > time. This should be easier to get this accepted upstream. To make the
> > code even smaller, dmi_table() could be merged into dmi_present() but
> > that's about it.
>
> Looks good, I found inconsistency though, in essence the dmi_table() and
> dmi_walk() are the same, (except for dmi_ioremap versus ioremap ofcourse) but
> for one you just use the base, len and number of items stored in global
> variables, while for the other you pass those same global variables through
> parameters, for consistency sake I think you should to both the same.
That's an asymmetry rather than an inconsistency. There are two reasons
for it, firstly I avoid using global variables when I don't have to,
secondly this happened to make the patch smaller. It is of course
possible to use the global variables dmi_base, dmi_len and dmi_num in
dmi_table() rather than passing them as parameters; doing so makes the
binary code slightly smaller. I'm attaching a version of the patch that
does this, together with function renames to make it look better. If
you like this one, I could send it upstream for review and comments.
> >> Question, what do we do if the dmi data doesn't get found, use some defaults I
> >> guess?
> >
> > Don't export the voltage values at all? I'd rather not export anything
> > than silently export values which may not be correct. But hopefully it
> > will never happen so it doesn't really matter what we decide here.
> >
>
> I would rather just printk a warning and use some sane defaults in this
> scenario. We must definetively alert the user, that I agree with. The problem
> with a printk is ofcourse that it will hardly be noticed.
That's my worry as well. But that's your driver, you'll do the support
for it so it's really up to you how you want to handle this unlikely
situation.
* * * * *
Subject: dmi: Let drivers walk the DMI table
Let drivers walk the DMI table for their own needs. Some drivers need
data stored in OEM-specific DMI records for proper operation.
Signed-off-by: Jean Delvare <khali@linux-fr.org>
---
drivers/firmware/dmi_scan.c | 61 +++++++++++++++++++++++++++++++++----------
include/linux/dmi.h | 3 ++
2 files changed, 50 insertions(+), 14 deletions(-)
--- linux-2.6.24-rc1.orig/drivers/firmware/dmi_scan.c 2007-11-03 09:22:23.000000000 +0100
+++ linux-2.6.24-rc1/drivers/firmware/dmi_scan.c 2007-11-03 17:24:45.000000000 +0100
@@ -36,18 +36,12 @@ static char * __init dmi_string(const st
* We have to be cautious here. We have seen BIOSes with DMI pointers
* pointing to completely the wrong place for example
*/
-static int __init dmi_table(u32 base, int len, int num,
- void (*decode)(const struct dmi_header *))
+static void dmi_table(u8 *buf, int len, int num,
+ void (*decode)(const struct dmi_header *))
{
- u8 *buf, *data;
+ u8 *data = buf;
int i = 0;
- buf = dmi_ioremap(base, len);
- if (buf = NULL)
- return -1;
-
- data = buf;
-
/*
* Stop when we see all the items the table claimed to have
* OR we run off the end of the table (also happens)
@@ -68,7 +62,23 @@ static int __init dmi_table(u32 base, in
data += 2;
i++;
}
- dmi_iounmap(buf, len);
+}
+
+static u32 dmi_base;
+static u16 dmi_len;
+static u16 dmi_num;
+
+static int __init dmi_walk_early(void (*decode)(const struct dmi_header *))
+{
+ u8 *buf;
+
+ buf = dmi_ioremap(dmi_base, dmi_len);
+ if (buf = NULL)
+ return -1;
+
+ dmi_table(buf, dmi_len, dmi_num, decode);
+
+ dmi_iounmap(buf, dmi_len);
return 0;
}
@@ -273,9 +283,9 @@ static int __init dmi_present(const char
memcpy_fromio(buf, p, 15);
if ((memcmp(buf, "_DMI_", 5) = 0) && dmi_checksum(buf)) {
- u16 num = (buf[13] << 8) | buf[12];
- u16 len = (buf[7] << 8) | buf[6];
- u32 base = (buf[11] << 24) | (buf[10] << 16) |
+ dmi_num = (buf[13] << 8) | buf[12];
+ dmi_len = (buf[7] << 8) | buf[6];
+ dmi_base = (buf[11] << 24) | (buf[10] << 16) |
(buf[9] << 8) | buf[8];
/*
@@ -287,7 +297,7 @@ static int __init dmi_present(const char
buf[14] >> 4, buf[14] & 0xF);
else
printk(KERN_INFO "DMI present.\n");
- if (dmi_table(base,len, num, dmi_decode) = 0)
+ if (dmi_walk_early(dmi_decode) = 0)
return 0;
}
return 1;
@@ -470,3 +480,26 @@ int dmi_get_year(int field)
return year;
}
+/**
+ * dmi_walk - Walk the DMI table and get called back for every record
+ * @decode: Callback function
+ *
+ * Returns -1 when the DMI table can't be reached, 0 on success.
+ */
+int dmi_walk(void (*decode)(const struct dmi_header *))
+{
+ u8 *buf;
+
+ if (!dmi_available)
+ return -1;
+
+ buf = ioremap(dmi_base, dmi_len);
+ if (buf = NULL)
+ return -1;
+
+ dmi_table(buf, dmi_len, dmi_num, decode);
+
+ iounmap(buf);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(dmi_walk);
--- linux-2.6.24-rc1.orig/include/linux/dmi.h 2007-11-03 09:22:23.000000000 +0100
+++ linux-2.6.24-rc1/include/linux/dmi.h 2007-11-03 09:28:06.000000000 +0100
@@ -78,6 +78,7 @@ extern const struct dmi_device * dmi_fin
extern void dmi_scan_machine(void);
extern int dmi_get_year(int field);
extern int dmi_name_in_vendors(const char *str);
+extern int dmi_walk(void (*decode)(const struct dmi_header *));
#else
@@ -87,6 +88,8 @@ static inline const struct dmi_device *
const struct dmi_device *from) { return NULL; }
static inline int dmi_get_year(int year) { return 0; }
static inline int dmi_name_in_vendors(const char *s) { return 0; }
+static inline int dmi_walk(void (*decode)(const struct dmi_header *))
+ { return -1; }
#endif
--
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] 22+ messages in thread* Re: [lm-sensors] PATCH:
2007-09-21 15:03 [lm-sensors] PATCH: Hans de Goede
` (13 preceding siblings ...)
2007-11-03 16:29 ` Jean Delvare
@ 2007-11-04 13:21 ` Hans de Goede
2007-12-17 15:56 ` Hans de Goede
` (4 subsequent siblings)
19 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2007-11-04 13:21 UTC (permalink / raw)
To: lm-sensors
Jean Delvare wrote:
> Hi Hans,
>
> On Fri, 02 Nov 2007 20:41:19 +0100, Hans de Goede wrote:
>> Jean Delvare wrote:
>>> On Fri, 02 Nov 2007 08:35:53 +0100, Hans de Goede wrote:
>>>> Good work Jean, yes that should do the job, I stared myself blind at adding a
>>>> clean way to store the data for later retrieval to the existing __init parser,
>>>> your way indeed will work and is much more generic.
>>>>
>>>> I'll compile a kernel with the dmi_scan.c and dmi.h parts patched in and then
>>>> start working on integrating this into fschmd.c
>>> Here's an updated version of my patch, with no code duplication this
>>> time. This should be easier to get this accepted upstream. To make the
>>> code even smaller, dmi_table() could be merged into dmi_present() but
>>> that's about it.
>> Looks good, I found inconsistency though, in essence the dmi_table() and
>> dmi_walk() are the same, (except for dmi_ioremap versus ioremap ofcourse) but
>> for one you just use the base, len and number of items stored in global
>> variables, while for the other you pass those same global variables through
>> parameters, for consistency sake I think you should to both the same.
>
> That's an asymmetry rather than an inconsistency. There are two reasons
> for it, firstly I avoid using global variables when I don't have to,
> secondly this happened to make the patch smaller. It is of course
> possible to use the global variables dmi_base, dmi_len and dmi_num in
> dmi_table() rather than passing them as parameters; doing so makes the
> binary code slightly smaller. I'm attaching a version of the patch that
> does this, together with function renames to make it look better. If
> you like this one, I could send it upstream for review and comments.
>
I like this one, you may want to wait with sending it upstream till I've
actually tested if it solves the problem at hand though.
I'm currently rather busy at work, so this may take a week or so.
Regards,
Hans
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [lm-sensors] PATCH:
2007-09-21 15:03 [lm-sensors] PATCH: Hans de Goede
` (14 preceding siblings ...)
2007-11-04 13:21 ` Hans de Goede
@ 2007-12-17 15:56 ` Hans de Goede
2007-12-18 13:29 ` Jean Delvare
` (3 subsequent siblings)
19 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2007-12-17 15:56 UTC (permalink / raw)
To: lm-sensors
Hi Jean et all,
I finally found / made some time to try out using your below patch in the
fschmd driver. Your patch works as advertised, I'll send a mail with a patch to
the fschmd driver using its functionality right after this mail. I guess this
is 2.6.25 material and that we should queue both patches for 2.6.25.
Thank & Regards,
hans
Jean Delvare wrote:
> Subject: dmi: Let drivers walk the DMI table
>
> Let drivers walk the DMI table for their own needs. Some drivers need
> data stored in OEM-specific DMI records for proper operation.
>
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> ---
> drivers/firmware/dmi_scan.c | 61 +++++++++++++++++++++++++++++++++----------
> include/linux/dmi.h | 3 ++
> 2 files changed, 50 insertions(+), 14 deletions(-)
>
> --- linux-2.6.24-rc1.orig/drivers/firmware/dmi_scan.c 2007-11-03 09:22:23.000000000 +0100
> +++ linux-2.6.24-rc1/drivers/firmware/dmi_scan.c 2007-11-03 17:24:45.000000000 +0100
> @@ -36,18 +36,12 @@ static char * __init dmi_string(const st
> * We have to be cautious here. We have seen BIOSes with DMI pointers
> * pointing to completely the wrong place for example
> */
> -static int __init dmi_table(u32 base, int len, int num,
> - void (*decode)(const struct dmi_header *))
> +static void dmi_table(u8 *buf, int len, int num,
> + void (*decode)(const struct dmi_header *))
> {
> - u8 *buf, *data;
> + u8 *data = buf;
> int i = 0;
>
> - buf = dmi_ioremap(base, len);
> - if (buf = NULL)
> - return -1;
> -
> - data = buf;
> -
> /*
> * Stop when we see all the items the table claimed to have
> * OR we run off the end of the table (also happens)
> @@ -68,7 +62,23 @@ static int __init dmi_table(u32 base, in
> data += 2;
> i++;
> }
> - dmi_iounmap(buf, len);
> +}
> +
> +static u32 dmi_base;
> +static u16 dmi_len;
> +static u16 dmi_num;
> +
> +static int __init dmi_walk_early(void (*decode)(const struct dmi_header *))
> +{
> + u8 *buf;
> +
> + buf = dmi_ioremap(dmi_base, dmi_len);
> + if (buf = NULL)
> + return -1;
> +
> + dmi_table(buf, dmi_len, dmi_num, decode);
> +
> + dmi_iounmap(buf, dmi_len);
> return 0;
> }
>
> @@ -273,9 +283,9 @@ static int __init dmi_present(const char
>
> memcpy_fromio(buf, p, 15);
> if ((memcmp(buf, "_DMI_", 5) = 0) && dmi_checksum(buf)) {
> - u16 num = (buf[13] << 8) | buf[12];
> - u16 len = (buf[7] << 8) | buf[6];
> - u32 base = (buf[11] << 24) | (buf[10] << 16) |
> + dmi_num = (buf[13] << 8) | buf[12];
> + dmi_len = (buf[7] << 8) | buf[6];
> + dmi_base = (buf[11] << 24) | (buf[10] << 16) |
> (buf[9] << 8) | buf[8];
>
> /*
> @@ -287,7 +297,7 @@ static int __init dmi_present(const char
> buf[14] >> 4, buf[14] & 0xF);
> else
> printk(KERN_INFO "DMI present.\n");
> - if (dmi_table(base,len, num, dmi_decode) = 0)
> + if (dmi_walk_early(dmi_decode) = 0)
> return 0;
> }
> return 1;
> @@ -470,3 +480,26 @@ int dmi_get_year(int field)
> return year;
> }
>
> +/**
> + * dmi_walk - Walk the DMI table and get called back for every record
> + * @decode: Callback function
> + *
> + * Returns -1 when the DMI table can't be reached, 0 on success.
> + */
> +int dmi_walk(void (*decode)(const struct dmi_header *))
> +{
> + u8 *buf;
> +
> + if (!dmi_available)
> + return -1;
> +
> + buf = ioremap(dmi_base, dmi_len);
> + if (buf = NULL)
> + return -1;
> +
> + dmi_table(buf, dmi_len, dmi_num, decode);
> +
> + iounmap(buf);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(dmi_walk);
> --- linux-2.6.24-rc1.orig/include/linux/dmi.h 2007-11-03 09:22:23.000000000 +0100
> +++ linux-2.6.24-rc1/include/linux/dmi.h 2007-11-03 09:28:06.000000000 +0100
> @@ -78,6 +78,7 @@ extern const struct dmi_device * dmi_fin
> extern void dmi_scan_machine(void);
> extern int dmi_get_year(int field);
> extern int dmi_name_in_vendors(const char *str);
> +extern int dmi_walk(void (*decode)(const struct dmi_header *));
>
> #else
>
> @@ -87,6 +88,8 @@ static inline const struct dmi_device *
> const struct dmi_device *from) { return NULL; }
> static inline int dmi_get_year(int year) { return 0; }
> static inline int dmi_name_in_vendors(const char *s) { return 0; }
> +static inline int dmi_walk(void (*decode)(const struct dmi_header *))
> + { return -1; }
>
> #endif
>
>
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [lm-sensors] PATCH:
2007-09-21 15:03 [lm-sensors] PATCH: Hans de Goede
` (15 preceding siblings ...)
2007-12-17 15:56 ` Hans de Goede
@ 2007-12-18 13:29 ` Jean Delvare
2008-06-30 14:53 ` Hans de Goede
` (2 subsequent siblings)
19 siblings, 0 replies; 22+ messages in thread
From: Jean Delvare @ 2007-12-18 13:29 UTC (permalink / raw)
To: lm-sensors
Hi Hans,
On Mon, 17 Dec 2007 16:56:24 +0100, Hans de Goede wrote:
> I finally found / made some time to try out using your below patch in the
> fschmd driver. Your patch works as advertised, I'll send a mail with a patch to
> the fschmd driver using its functionality right after this mail. I guess this
> is 2.6.25 material and that we should queue both patches for 2.6.25.
OK, great. I agree that this is 2.6.25 material
My patch is not hwmon-specific so even if it is easier to push it
through Mark's hwmon tree, I'd like to know what other developers think
about it; I'll post it to LKML for comments later today.
--
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] 22+ messages in thread* [lm-sensors] PATCH:
2007-09-21 15:03 [lm-sensors] PATCH: Hans de Goede
` (16 preceding siblings ...)
2007-12-18 13:29 ` Jean Delvare
@ 2008-06-30 14:53 ` Hans de Goede
2009-01-30 10:29 ` Hans de Goede
2009-01-30 11:06 ` Jean Delvare
19 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2008-06-30 14:53 UTC (permalink / raw)
To: lm-sensors
[-- Attachment #1: Type: text/plain, Size: 256 bytes --]
Hi All,
As promised, this patch adds support for the watchdog part found in _all_
supported FSC sensor chips. Once this patch is in we can deprecate the old
fscpos and fscher drivers.
signed-off-by: Hans de Goede <j.w.r.degoede@hhs.nl>
Regards,
Hans
[-- Attachment #2: fschmd-watchdog.patch --]
[-- Type: text/plain, Size: 17227 bytes --]
This patch adds support for the watchdog part found in _all_ supported FSC
sensor chips.
signed-off-by: Hans de Goede <j.w.r.degoede@hhs.nl>
diff -up vanilla-2.6.26-rc8/drivers/hwmon/Kconfig.orig vanilla-2.6.26-rc8/drivers/hwmon/Kconfig
--- vanilla-2.6.26-rc8/drivers/hwmon/Kconfig.orig 2008-06-30 16:37:17.000000000 +0200
+++ vanilla-2.6.26-rc8/drivers/hwmon/Kconfig 2008-06-30 16:38:34.000000000 +0200
@@ -292,10 +292,11 @@ config SENSORS_FSCHMD
depends on X86 && I2C && EXPERIMENTAL
help
If you say yes here you get support for various Fujitsu Siemens
- Computers sensor chips.
+ Computers sensor chips, including support for the integrated
+ watchdog.
This is a new merged driver for FSC sensor chips which is intended
- as a replacment for the fscpos, fscscy and fscher drivers and adds
+ as a replacement for the fscpos, fscscy and fscher drivers and adds
support for several other FCS sensor chips.
This driver can also be built as a module. If so, the module
diff -up vanilla-2.6.26-rc8/drivers/hwmon/fschmd.c.orig vanilla-2.6.26-rc8/drivers/hwmon/fschmd.c
--- vanilla-2.6.26-rc8/drivers/hwmon/fschmd.c.orig 2008-06-30 16:36:32.000000000 +0200
+++ vanilla-2.6.26-rc8/drivers/hwmon/fschmd.c 2008-06-30 16:51:34.000000000 +0200
@@ -1,6 +1,6 @@
/* fschmd.c
*
- * Copyright (C) 2007 Hans de Goede <j.w.r.degoede@hhs.nl>
+ * Copyright (C) 2007,2008 Hans de Goede <j.w.r.degoede@hhs.nl>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -42,13 +42,21 @@
#include <linux/mutex.h>
#include <linux/sysfs.h>
#include <linux/dmi.h>
+#include <linux/fs.h>
+#include <linux/watchdog.h>
+#include <linux/miscdevice.h>
+#include <asm/uaccess.h>
/* Addresses to scan */
static const unsigned short normal_i2c[] = { 0x73, I2C_CLIENT_END };
/* Insmod parameters */
+static int nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, int, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
I2C_CLIENT_INSMOD_5(fscpos, fscher, fscscy, fschrc, fschmd);
+
/*
* The FSCHMD registers and other defines
*/
@@ -65,11 +73,19 @@ I2C_CLIENT_INSMOD_5(fscpos, fscher, fscs
#define FSCHMD_CONTROL_ALERT_LED_MASK 0x01
-/* watchdog (support to be implemented) */
+/* watchdog */
#define FSCHMD_REG_WDOG_PRESET 0x28
#define FSCHMD_REG_WDOG_STATE 0x23
#define FSCHMD_REG_WDOG_CONTROL 0x21
+#define FSCHMD_WDOG_CONTROL_TRIGGER_MASK 0x10
+#define FSCHMD_WDOG_CONTROL_STARTED_MASK 0x10 /* the same as trigger */
+#define FSCHMD_WDOG_CONTROL_STOP_MASK 0x20
+#define FSCHMD_WDOG_CONTROL_RESOLUTION_MASK 0x40
+
+#define FSCHMD_WDOG_STATE_CARDRESET_MASK 0x02
+
+
/* voltages, weird order is to keep the same order as the old drivers */
static const u8 FSCHMD_REG_VOLT[3] = { 0x45, 0x42, 0x48 };
@@ -167,6 +183,22 @@ static const int FSCHMD_NO_TEMP_SENSORS[
/* our driver name */
#define FSCHMD_NAME "fschmd"
+/* Watchdog filp->private_data pointer use utility macros. We store our minor
+ (to find our device data) in the private_data pointer, since the
+ private_data pointer can hold atleast an int, we use the 'major" space
+ of that int to store an per open expect_close flag. */
+#define WATCHDOG_INIT_FILP(filp, minor) \
+ (filp)->private_data = (void *)(long)(minor)
+#define WATCHDOG_GET_MINOR(filp) \
+ MINOR((long)(filp)->private_data)
+#define WATCHDOG_CLEAR_EXPECT_CLOSE(filp) \
+ (filp)->private_data = (void *)(long)WATCHDOG_GET_MINOR(filp)
+#define WATCHDOG_SET_EXPECT_CLOSE(filp) \
+ (filp)->private_data = (void *)(long)MKDEV(1, WATCHDOG_GET_MINOR(filp))
+#define WATCHDOG_GET_EXPECT_CLOSE(filp) \
+ MAJOR((long)(filp)->private_data)
+
+
/*
* Functions declarations
*/
@@ -195,12 +227,20 @@ struct fschmd_data {
struct i2c_client client;
struct device *hwmon_dev;
struct mutex update_lock;
+ struct list_head list;
+ struct miscdevice watchdog_miscdev;
int kind;
+ int watchdog_open_count;
+ char watchdog_name[10]; /* must be unique to avoid sysfs conflict */
char valid; /* zero until following fields are valid */
unsigned long last_updated; /* in jiffies */
/* register values */
+ u8 revision; /* chip revision */
u8 global_control; /* global control register */
+ u8 watchdog_control; /* watchdog control register */
+ u8 watchdog_status; /* watchdog status register */
+ u8 watchdog_preset; /* watchdog counter preset on trigger val */
u8 volt[3]; /* 12, 5, battery voltage */
u8 temp_act[5]; /* temperature */
u8 temp_status[5]; /* status of sensor */
@@ -212,11 +252,20 @@ struct fschmd_data {
};
/* Global variables to hold information read from special DMI tables, which are
- available on FSC machines with an fscher or later chip. */
+ available on FSC machines with an fscher or later chip. There is no need to
+ protect these with a lock as they are only modified from our attach function
+ which always gets called with the i2c-core lock held and never accessed
+ before the attach function is done with them. */
static int dmi_mult[3] = { 490, 200, 100 };
static int dmi_offset[3] = { 0, 0, 0 };
static int dmi_vref = -1;
+/* Somewhat ugly :( global data pointer list with all fschmd devices, so that
+ we can find our device data as when using misc_register there is no other
+ method to get to ones device data from the open fop. */
+static LIST_HEAD(watchdog_data_list);
+static DEFINE_MUTEX(watchdog_data_mutex);
+
/*
* Sysfs attr show / store functions
@@ -535,7 +584,286 @@ static struct sensor_device_attribute fs
/*
- * Real code
+ * Watchdog routines
+ */
+static struct fschmd_data *watchdog_get_data_unlocked(int minor)
+{
+ struct fschmd_data *data = NULL, *pos;
+
+ list_for_each_entry(pos, &watchdog_data_list, list) {
+ if (pos->watchdog_miscdev.minor == minor) {
+ data = pos;
+ break;
+ }
+ }
+
+ return data;
+}
+
+static int watchdog_set_timeout_unlocked(struct fschmd_data *data, int timeout)
+{
+ int resolution;
+ int kind = data->kind + 1; /* 0-x array index -> 1-x module param */
+
+ /* 2 second or 60 second resolution? */
+ if (timeout <= 510 || kind == fscpos || kind == fscscy) {
+ data->watchdog_control &= ~FSCHMD_WDOG_CONTROL_RESOLUTION_MASK;
+ resolution = 2;
+ } else {
+ data->watchdog_control |= FSCHMD_WDOG_CONTROL_RESOLUTION_MASK;
+ resolution = 60;
+ }
+
+ if (timeout > (resolution * 255))
+ data->watchdog_preset = 255;
+ else
+ data->watchdog_preset = (timeout + (resolution - 1)) /
+ resolution;
+
+ /* Write new timeout value */
+ i2c_smbus_write_byte_data(&data->client, FSCHMD_REG_WDOG_PRESET,
+ data->watchdog_preset);
+ /* Write new control register, do not trigger! */
+ i2c_smbus_write_byte_data(&data->client, FSCHMD_REG_WDOG_CONTROL,
+ data->watchdog_control & ~FSCHMD_WDOG_CONTROL_TRIGGER_MASK);
+
+ return data->watchdog_preset * resolution;
+}
+
+static int watchdog_get_timeout(struct fschmd_data *data)
+{
+ int timeout;
+
+ mutex_lock(&data->update_lock);
+ if (data->watchdog_control & FSCHMD_WDOG_CONTROL_RESOLUTION_MASK)
+ timeout = data->watchdog_preset * 60;
+ else
+ timeout = data->watchdog_preset * 2;
+ mutex_unlock(&data->update_lock);
+
+ return timeout;
+}
+
+static void watchdog_trigger(struct fschmd_data *data)
+{
+ mutex_lock(&data->update_lock);
+ data->watchdog_control |= FSCHMD_WDOG_CONTROL_TRIGGER_MASK;
+ i2c_smbus_write_byte_data(&data->client, FSCHMD_REG_WDOG_CONTROL,
+ data->watchdog_control);
+ mutex_unlock(&data->update_lock);
+}
+
+static void watchdog_stop(struct fschmd_data *data)
+{
+ mutex_lock(&data->update_lock);
+ data->watchdog_control &= ~FSCHMD_WDOG_CONTROL_STARTED_MASK;
+ /* Don't store the stop flag in our watchdog control register copy, as
+ its a write only bit (read always returns 0) */
+ i2c_smbus_write_byte_data(&data->client, FSCHMD_REG_WDOG_CONTROL,
+ data->watchdog_control | FSCHMD_WDOG_CONTROL_STOP_MASK);
+ mutex_unlock(&data->update_lock);
+}
+
+
+static int watchdog_open(struct inode *inode, struct file *filp)
+{
+ int ret;
+ struct fschmd_data *data;
+
+ mutex_lock(&watchdog_data_mutex);
+ data = watchdog_get_data_unlocked(iminor(inode));
+ /* Check our i2c client didn't get removed from underneath us */
+ if (!data) {
+ ret = -ENODEV;
+ goto unlock_mutex;
+ }
+
+ /* Set a default timeout if necessary */
+ mutex_lock(&data->update_lock);
+ if (data->watchdog_preset == 0)
+ watchdog_set_timeout_unlocked(data, 60);
+ mutex_unlock(&data->update_lock);
+
+ /* Start the watchdog */
+ watchdog_trigger(data);
+
+ data->watchdog_open_count++;
+ WATCHDOG_INIT_FILP(filp, data->watchdog_miscdev.minor);
+
+ ret = nonseekable_open(inode, filp);
+
+unlock_mutex:
+ mutex_unlock(&watchdog_data_mutex);
+
+ return ret;
+}
+
+static int watchdog_release(struct inode *inode, struct file *filp)
+{
+ struct fschmd_data *data;
+
+ mutex_lock(&watchdog_data_mutex);
+ data = watchdog_get_data_unlocked(WATCHDOG_GET_MINOR(filp));
+ /* Check our i2c client didn't get removed from underneath us */
+ if (!data)
+ goto unlock_mutex;
+
+ data->watchdog_open_count--;
+
+ if (WATCHDOG_GET_EXPECT_CLOSE(filp)) {
+ if (data->watchdog_open_count)
+ printk(KERN_WARNING FSCHMD_NAME
+ ": watchdog still opened %d time(s), "
+ "not stopping\n", data->watchdog_open_count);
+ else
+ watchdog_stop(data);
+ } else {
+ watchdog_trigger(data);
+ printk(KERN_CRIT FSCHMD_NAME
+ ": unexpected close, not stopping watchdog!\n");
+ }
+
+unlock_mutex:
+ mutex_unlock(&watchdog_data_mutex);
+
+ return 0;
+}
+
+static ssize_t watchdog_write(struct file *filp, const char __user *buf,
+ size_t count, loff_t *offset)
+{
+ ssize_t ret = count;
+ struct fschmd_data *data;
+
+ mutex_lock(&watchdog_data_mutex);
+ data = watchdog_get_data_unlocked(WATCHDOG_GET_MINOR(filp));
+ /* Check our i2c client didn't get removed from underneath us */
+ if (!data) {
+ ret = -ENODEV;
+ goto unlock_mutex;
+ }
+
+ if (count) {
+ if (!nowayout) {
+ size_t i;
+
+ /* Clear it in case it was set with a previous write */
+ WATCHDOG_CLEAR_EXPECT_CLOSE(filp);
+
+ for (i = 0; i != count; i++) {
+ char c;
+ if (get_user(c, buf + i)) {
+ ret = -EFAULT;
+ goto unlock_mutex;
+ }
+ if (c == 'V')
+ WATCHDOG_SET_EXPECT_CLOSE(filp);
+ }
+ }
+ watchdog_trigger(data);
+ }
+
+unlock_mutex:
+ mutex_unlock(&watchdog_data_mutex);
+
+ return ret;
+}
+
+static int watchdog_ioctl(struct inode *inode, struct file *filp,
+ unsigned int cmd, unsigned long arg)
+{
+ static struct watchdog_info ident = {
+ .options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT |
+ WDIOF_CARDRESET,
+ .identity = "FSC watchdog"
+ };
+ int i, ret = 0;
+ struct fschmd_data *data;
+
+ mutex_lock(&watchdog_data_mutex);
+ data = watchdog_get_data_unlocked(WATCHDOG_GET_MINOR(filp));
+ /* Check our i2c client didn't get removed from underneath us */
+ if (!data) {
+ ret = -ENODEV;
+ goto unlock_mutex;
+ }
+
+ switch (cmd) {
+ case WDIOC_GETSUPPORT:
+ ident.firmware_version = data->revision;
+ if (!nowayout)
+ ident.options |= WDIOF_MAGICCLOSE;
+ if (copy_to_user((void __user *)arg, &ident, sizeof(ident)))
+ ret = -EFAULT;
+ break;
+
+ case WDIOC_GETSTATUS:
+ ret = put_user(0, (int *)arg);
+ break;
+
+ case WDIOC_GETBOOTSTATUS:
+ if (data->watchdog_status & FSCHMD_WDOG_STATE_CARDRESET_MASK)
+ ret = put_user(WDIOF_CARDRESET, (int *)arg);
+ else
+ ret = put_user(0, (int *)arg);
+ break;
+
+ case WDIOC_KEEPALIVE:
+ watchdog_trigger(data);
+ break;
+
+ case WDIOC_GETTIMEOUT:
+ i = watchdog_get_timeout(data);
+ ret = put_user(i, (int *)arg);
+ break;
+
+ case WDIOC_SETTIMEOUT:
+ if (get_user(i, (int *)arg)) {
+ ret = -EFAULT;
+ break;
+ }
+ mutex_lock(&data->update_lock);
+ i = watchdog_set_timeout_unlocked(data, i);
+ mutex_unlock(&data->update_lock);
+ ret = put_user(i, (int *)arg);
+ break;
+
+ case WDIOC_SETOPTIONS:
+ if (get_user(i, (int *)arg)) {
+ ret = -EFAULT;
+ break;
+ }
+
+ if (i & WDIOS_DISABLECARD)
+ watchdog_stop(data);
+ else if (i & WDIOS_ENABLECARD)
+ watchdog_trigger(data);
+ else
+ ret = -EINVAL;
+
+ break;
+ default:
+ ret = -ENOTTY;
+ }
+
+unlock_mutex:
+ mutex_unlock(&watchdog_data_mutex);
+
+ return ret;
+}
+
+static struct file_operations watchdog_fops = {
+ .owner = THIS_MODULE,
+ .llseek = no_llseek,
+ .open = watchdog_open,
+ .release = watchdog_release,
+ .write = watchdog_write,
+ .ioctl = watchdog_ioctl,
+};
+
+
+/*
+ * Detect, register, unregister and update device functions
*/
/* DMI decode routine to read voltage scaling factors from special DMI tables,
@@ -604,11 +932,11 @@ static int fschmd_detect(struct i2c_adap
{
struct i2c_client *client;
struct fschmd_data *data;
- u8 revision;
const char * const names[5] = { "Poseidon", "Hermes", "Scylla",
"Heracles", "Heimdall" };
const char * const client_names[5] = { "fscpos", "fscher", "fscscy",
"fschrc", "fschmd" };
+ const int watchdog_minors[] = { 130, 212, 213, 214, 215 };
int i, err = 0;
if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
@@ -626,6 +954,7 @@ static int fschmd_detect(struct i2c_adap
client->adapter = adapter;
client->driver = &fschmd_driver;
mutex_init(&data->update_lock);
+ INIT_LIST_HEAD(&data->list);
/* Detect & Identify the chip */
if (kind <= 0) {
@@ -662,7 +991,7 @@ static int fschmd_detect(struct i2c_adap
}
/* Read the special DMI table for fscher and newer chips */
- if (kind == fscher || kind >= fschrc) {
+ if ((kind == fscher || kind >= fschrc) && dmi_vref == -1){
dmi_walk(fschmd_dmi_decode);
if (dmi_vref == -1) {
printk(KERN_WARNING FSCHMD_NAME
@@ -672,6 +1001,17 @@ static int fschmd_detect(struct i2c_adap
}
}
+ /* Read in some never changing registers */
+ data->revision = i2c_smbus_read_byte_data(client, FSCHMD_REG_REVISION);
+ data->global_control = i2c_smbus_read_byte_data(client,
+ FSCHMD_REG_CONTROL);
+ data->watchdog_control = i2c_smbus_read_byte_data(client,
+ FSCHMD_REG_WDOG_CONTROL);
+ data->watchdog_status = i2c_smbus_read_byte_data(client,
+ FSCHMD_REG_WDOG_STATE);
+ data->watchdog_preset = i2c_smbus_read_byte_data(client,
+ FSCHMD_REG_WDOG_PRESET);
+
/* i2c kind goes from 1-5, we want from 0-4 to address arrays */
data->kind = kind - 1;
strlcpy(client->name, client_names[data->kind], I2C_NAME_SIZE);
@@ -719,9 +1059,36 @@ static int fschmd_detect(struct i2c_adap
goto exit_detach;
}
- revision = i2c_smbus_read_byte_data(client, FSCHMD_REG_REVISION);
+ /* Register our watchdog part */
+ for (i = 0; i < ARRAY_SIZE(watchdog_minors); i++) {
+ snprintf(data->watchdog_name, sizeof(data->watchdog_name),
+ "watchdog%c", (i == 0) ? '\0' : ('0' + i));
+ data->watchdog_miscdev.name = data->watchdog_name;
+ data->watchdog_miscdev.fops = &watchdog_fops;
+ data->watchdog_miscdev.minor = watchdog_minors[i];
+ err = misc_register(&data->watchdog_miscdev);
+ if (err == -EBUSY)
+ continue;
+ if (err)
+ goto exit_detach;
+
+ mutex_lock(&watchdog_data_mutex);
+ list_add(&data->list, &watchdog_data_list);
+ mutex_unlock(&watchdog_data_mutex);
+ printk(KERN_INFO FSCHMD_NAME
+ ": Registered watchdog chardev major 10, minor: %d\n",
+ watchdog_minors[i]);
+ break;
+ }
+ if (i == ARRAY_SIZE(watchdog_minors)) {
+ data->watchdog_miscdev.minor = 0;
+ printk(KERN_WARNING FSCHMD_NAME
+ ": Couldn't register watchdog chardev "
+ "(due to no free minor)\n");
+ }
+
printk(KERN_INFO FSCHMD_NAME ": Detected FSC %s chip, revision: %d\n",
- names[data->kind], (int) revision);
+ names[data->kind], (int) data->revision);
return 0;
@@ -746,6 +1113,20 @@ static int fschmd_detach_client(struct i
struct fschmd_data *data = i2c_get_clientdata(client);
int i, err;
+ /* Unregister the watchdog (if registered) */
+ if (data->watchdog_miscdev.minor) {
+ mutex_lock(&watchdog_data_mutex);
+ if (data->watchdog_open_count) {
+ printk(KERN_WARNING FSCHMD_NAME
+ ": i2c client detached with watchdog open! "
+ "Stopping watchdog.\n");
+ watchdog_stop(data);
+ }
+ misc_deregister(&data->watchdog_miscdev);
+ list_del(&data->list);
+ mutex_unlock(&watchdog_data_mutex);
+ }
+
/* Check if registered in case we're called from fschmd_detect
to cleanup after an error */
if (data->hwmon_dev)
@@ -825,17 +1206,6 @@ static struct fschmd_data *fschmd_update
data->volt[i] = i2c_smbus_read_byte_data(client,
FSCHMD_REG_VOLT[i]);
- data->global_control = i2c_smbus_read_byte_data(client,
- FSCHMD_REG_CONTROL);
-
- /* To be implemented in the future
- data->watchdog[0] = i2c_smbus_read_byte_data(client,
- FSCHMD_REG_WDOG_PRESET);
- data->watchdog[1] = i2c_smbus_read_byte_data(client,
- FSCHMD_REG_WDOG_STATE);
- data->watchdog[2] = i2c_smbus_read_byte_data(client,
- FSCHMD_REG_WDOG_CONTROL); */
-
data->last_updated = jiffies;
data->valid = 1;
}
@@ -859,6 +1229,7 @@ MODULE_AUTHOR("Hans de Goede <j.w.r.dego
MODULE_DESCRIPTION("FSC Poseidon, Hermes, Scylla, Heracles and "
"Heimdall driver");
MODULE_LICENSE("GPL");
+MODULE_ALIAS_MISCDEV(WATCHDOG_GET_MINOR);
module_init(fschmd_init);
module_exit(fschmd_exit);
[-- Attachment #3: Type: text/plain, Size: 153 bytes --]
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 22+ messages in thread* [lm-sensors] PATCH:
2007-09-21 15:03 [lm-sensors] PATCH: Hans de Goede
` (17 preceding siblings ...)
2008-06-30 14:53 ` Hans de Goede
@ 2009-01-30 10:29 ` Hans de Goede
2009-01-30 11:06 ` Jean Delvare
19 siblings, 0 replies; 22+ messages in thread
From: Hans de Goede @ 2009-01-30 10:29 UTC (permalink / raw)
To: lm-sensors
[-- Attachment #1: Type: text/plain, Size: 380 bytes --]
Hi Jean,
<Trying to win the competition for the patch with the longest name :) >
This patch fixes a number of cases where things were not properly cleaned up
when acpi_check_resource_conflict() returned an error, causing oopses such as
the one reported here:
https://bugzilla.redhat.com/show_bug.cgi?id=483208
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
[-- Attachment #2: hwmon-superio-fix-acpi-resource-check-error-handling.patch --]
[-- Type: text/plain, Size: 1836 bytes --]
This patch fixes a number of cases where things were not properly cleaned up
when acpi_check_resource_conflict() returned an error, causing oopses such as
the one reported here:
https://bugzilla.redhat.com/show_bug.cgi?id=483208
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
diff -up linux-2.6.28.x86_64/drivers/hwmon/f71882fg.c~ linux-2.6.28.x86_64/drivers/hwmon/f71882fg.c
--- linux-2.6.28.x86_64/drivers/hwmon/f71882fg.c~ 2009-01-30 11:18:04.000000000 +0100
+++ linux-2.6.28.x86_64/drivers/hwmon/f71882fg.c 2009-01-30 11:18:04.000000000 +0100
@@ -1932,7 +1932,7 @@ static int __init f71882fg_device_add(un
res.name = f71882fg_pdev->name;
err = acpi_check_resource_conflict(&res);
if (err)
- return err;
+ goto exit_device_put;
err = platform_device_add_resources(f71882fg_pdev, &res, 1);
if (err) {
diff -up linux-2.6.28.x86_64/drivers/hwmon/vt1211.c~ linux-2.6.28.x86_64/drivers/hwmon/vt1211.c
--- linux-2.6.28.x86_64/drivers/hwmon/vt1211.c~ 2009-01-30 11:21:32.000000000 +0100
+++ linux-2.6.28.x86_64/drivers/hwmon/vt1211.c 2009-01-30 11:21:32.000000000 +0100
@@ -1262,7 +1262,7 @@ static int __init vt1211_device_add(unsi
res.name = pdev->name;
err = acpi_check_resource_conflict(&res);
if (err)
- goto EXIT;
+ goto EXIT_DEV_PUT;
err = platform_device_add_resources(pdev, &res, 1);
if (err) {
diff -up linux-2.6.28.x86_64/drivers/hwmon/w83627ehf.c~ linux-2.6.28.x86_64/drivers/hwmon/w83627ehf.c
--- linux-2.6.28.x86_64/drivers/hwmon/w83627ehf.c~ 2009-01-30 11:22:26.000000000 +0100
+++ linux-2.6.28.x86_64/drivers/hwmon/w83627ehf.c 2009-01-30 11:22:26.000000000 +0100
@@ -1548,7 +1548,7 @@ static int __init sensors_w83627ehf_init
err = acpi_check_resource_conflict(&res);
if (err)
- goto exit;
+ goto exit_device_put;
err = platform_device_add_resources(pdev, &res, 1);
if (err) {
[-- Attachment #3: Type: text/plain, Size: 153 bytes --]
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [lm-sensors] PATCH:
2007-09-21 15:03 [lm-sensors] PATCH: Hans de Goede
` (18 preceding siblings ...)
2009-01-30 10:29 ` Hans de Goede
@ 2009-01-30 11:06 ` Jean Delvare
19 siblings, 0 replies; 22+ messages in thread
From: Jean Delvare @ 2009-01-30 11:06 UTC (permalink / raw)
To: lm-sensors
On Fri, 30 Jan 2009 11:29:47 +0100, Hans de Goede wrote:
> Hi Jean,
>
> <Trying to win the competition for the patch with the longest name :) >
>
> This patch fixes a number of cases where things were not properly cleaned up
> when acpi_check_resource_conflict() returned an error, causing oopses such as
> the one reported here:
> https://bugzilla.redhat.com/show_bug.cgi?idH3208
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Applied, 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] 22+ messages in thread