Hi Jean,
> Hi David,
>
> On Thu, 29 Mar 2007 12:13:29 -0700, David Hubbard wrote:
> > Here's a patch for your review.
>
> Sorry for the delay, here's my review. Next time please avoid patches
> as base64 attachments.
Gmail picks a bad encoding for .patch files, and I forgot to send them
as .patch.txt. Sorry :-)
> These are cleanups which do not belong to this patch.
I removed them.
(snip)
> >
> > -static int w83627ehf_detach_client(struct i2c_client *client)
> > +static int __devexit w83627ehf_remove(struct platform_device *pdev)
> > {
> > - struct w83627ehf_data *data = i2c_get_clientdata(client);
> > - int err;
> > + struct w83627ehf_data *data = platform_get_drvdata(pdev);
> >
> > hwmon_device_unregister(data->class_dev);
> > - w83627ehf_device_remove_files(&client->dev);
> > + w83627ehf_device_remove_files(&pdev->dev);
> > + platform_set_drvdata(pdev, NULL);
> >
> > - if ((err = i2c_detach_client(client)))
> > - return err;
> > - release_region(client->addr + REGION_OFFSET, REGION_LENGTH);
>
> You shouldn't remove this. You requested the region in .probe(), you
> _must_ release it in .remove()!
>
> > kfree(data);
>
> OTOH it's weird to free data here while you did not allocate it
> in .probe().
So I added release_region() to w83627ehf_remove(). I left the kfree()
in place (notice that the patch doesn't move it) even though I
modified the way the data was allocated, because it is the simplest
way of freeing the data. A more correct approach would be to get
pdev->dev.platform_data in sensors_w83627ehf_exit and free it there.
It does break the device driver model, and needs revision when the
driver is moved over to a Super-I/O bus. I added a comment noting
this.
> >
> > return 0;
> > }
> >
> > -static struct i2c_driver w83627ehf_driver = {
> > +static struct platform_driver w83627ehf_driver = {
> > .driver = {
> > .owner = THIS_MODULE,
> > - .name = "w83627ehf",
> > + .name = DRVNAME,
> > },
> > - .attach_adapter = w83627ehf_detect,
> > - .detach_client = w83627ehf_detach_client,
> > + .probe = w83627ehf_probe,
> > + .remove = __devexit_p(w83627ehf_remove),
> > };
> >
> > -static int __init w83627ehf_find(int sioaddr, unsigned short *addr)
> > +/* w83627ehf_superio_probe() looks for a '627 in the Super-I/O config space */
> > +static int __init w83627ehf_superio_probe(int sioaddr,
> > + struct w83627ehf_data *data)
> > {
> > u16 val;
> >
> > - REG = sioaddr;
> > - VAL = sioaddr + 1;
> > - superio_enter();
> > + static const char * __initdata sio_names[] = {
> > + "W83627EHF",
> > + "W83627EHG",
> > + "W83627DHG",
> > + };
>
> This probably doesn't do what you think it does. Here the __initdata
> only applies to the pointers, not to the strings. So you aren't saving
> anything compared to putting the strings directly in the switch/case
> code.
>
> If you really want the strings themselves to be __initdata, then you
> have to define an array of arrays of chars, not an array of pointers:
>
> static const char __initdata sio_names[][10] = {
> "W83627EHF",
> "W83627EHG",
> "W83627DHG",
> };
>
> With the drawback that you have to be careful to size the buffers
> properly. gcc will unfortunately not complain if the trailing \0
> doesn't fit, so it seems safer to explicitly add it yourself. (I
> consider it a bug in gcc.)
>
> Quite frankly, I don't think this is worth the trouble, and I would put
> the strings in the code directly.
I revised the code to declare three separate __initdata strings. This
should work.
> > +
> > + const char * sio_name;
> >
> > - val = (superio_inb(SIO_REG_DEVID) << 8)
> > - | superio_inb(SIO_REG_DEVID + 1);
> > + data->sioreg = sioaddr;
> > +
> > + superio_enter(data->sioreg);
> > +
> > + val = (superio_inb(data->sioreg, SIO_REG_DEVID) << 8)
> > + | superio_inb(data->sioreg, SIO_REG_DEVID + 1);
> > switch (val & SIO_ID_MASK) {
> > - case SIO_W83627DHG_ID:
> > - w83627ehf_num_in = 9;
> > - break;
> > case SIO_W83627EHF_ID:
> > + data->kind = w83627ehf;
> > + sio_name = sio_names[0];
> > + break;
> > case SIO_W83627EHG_ID:
> > - w83627ehf_num_in = 10;
> > + data->kind = w83627ehf;
> > + sio_name = sio_names[1];
> > + break;
> > + case SIO_W83627DHG_ID:
> > + data->kind = w83627dhg;
> > + sio_name = sio_names[2];
> > break;
> > default:
> > - printk(KERN_WARNING "w83627ehf: unsupported chip ID: 0x%04x\n",
> > + pr_info(DRVNAME ": unsupported chip ID: 0x%04x\n",
> > val);
> > - superio_exit();
> > + superio_exit(data->sioreg);
> > return -ENODEV;
> > }
> >
> > - superio_select(W83627EHF_LD_HWM);
> > - val = (superio_inb(SIO_REG_ADDR) << 8)
> > - | superio_inb(SIO_REG_ADDR + 1);
> > - *addr = val & REGION_ALIGNMENT;
> > - if (*addr == 0) {
> > - superio_exit();
> > + /* we have known chip find the HWMON base */
> > +
> > + superio_select(data->sioreg, W83627EHF_LD_HWM);
> > + val = (superio_inb(data->sioreg, SIO_REG_ADDR) << 8)
> > + | superio_inb(data->sioreg, SIO_REG_ADDR + 1);
> > + data->addr = val & REGION_ALIGNMENT;
> > + if (data->addr == 0) {
> > + superio_exit(data->sioreg);
>
> Note for a future patch: it would be user-friendly to log the reason
> why we're exiting here.
I am sending four patches this time. They apply to 2.6.22-rc2 and the
third adds a warning here, as you suggest.
> > return -ENODEV;
> > }
> >
> > /* Activate logical device if needed */
> > - val = superio_inb(SIO_REG_ENABLE);
> > + val = superio_inb(data->sioreg, SIO_REG_ENABLE);
> > if (!(val & 0x01))
> > - superio_outb(SIO_REG_ENABLE, val | 0x01);
> > + superio_outb(data->sioreg, SIO_REG_ENABLE, val | 0x01);
>
> Note for a future patch: it would be user-friendly to warn that the
> device was forcibly enabled and that the readings may not make sense.
Patch 03 adds a warning here also.
> >
> > - superio_exit();
> > + superio_exit(data->sioreg);
> > + pr_info(DRVNAME ": Found %s chip at %#x\n",
> > + sio_name, data->addr);
> > return 0;
> > }
> >
> > +/* when Super-I/O functions move to a separate file, the Super-I/O
> > + * bus will manage the lifetime of the device and the w83627ehf driver
> > + * but since we platform_device_alloc(), we must keep track of the device */
>
> The "and the w83627ehf driver" part of your sentence doesn't make much
> sense and should be removed? Either that, or a part is missing.
You're right. I updated the comment.
>
> > +struct platform_device * pdev;
>
> Should be declared static. Usually we leave no space between the "*"
> and the variable name.
Fixed.
> > +
> > static int __init sensors_w83627ehf_init(void)
> > {
> > - if (w83627ehf_find(0x2e, &address)
> > - && w83627ehf_find(0x4e, &address))
> > - return -ENODEV;
> > + int err;
> > + struct resource res;
> > + struct w83627ehf_data *data;
> > +
> > + if (!(data = kzalloc(sizeof(struct w83627ehf_data), GFP_KERNEL))) {
> > + err = -ENOMEM;
> > + goto exit;
> > + }
>
> This is an interesting approach. I did it differently in the other
> drivers. Your way saves some memory and some code, but OTOH it breaks
> the device driver model. The platform-specific data is supposed to be
> stored in dev.platform_data, NOT dev.driver_data. The code which
> instantiates the device isn't supposed to know how the driver is
> implemented. Nothing prevents a given device from being supported by
> more than one driver!
>
> So setting dev.driver_dat before .probe() is called sounds bad, and I
> suspect we will have to move this part to the .probe() function once
> we have a superio subsystem, anyway. So we might as well do it now.
> What do you think?
Yes, I've changed the code to use dev.platform_data instead of
dev.driver_data. I guess I was uncomfortable with dev.platform_data
because I looked at the function platform_device_add_data():
int platform_device_add_data(struct platform_device *pdev, const void
*data, size_t size)
{
void *d;
d = kmalloc(size, GFP_KERNEL);
if (d) {
memcpy(d, data, size);
pdev->dev.platform_data = d;
}
return d ? 0 : -ENOMEM;
}
EXPORT_SYMBOL_GPL(platform_device_add_data);
If pdev->dev.platform_data is non-null on entry to this function it
does not warn, it just leaks memory. (Example: a broken driver calls
platform_device_add_data() twice in a row.) Or, I have misunderstood
the function.
Is this a bug in platform.c? I notice you just modify
pdev->dev.platform_data directly in w83627hf.c (2.6.22-rc2).
Either way, the driver needs revision in the future when devices are
probed from a Super-I/O bus driver.
> > +
> > + /* initialize data->kind, data->sioreg, and data->addr.
> > + *
> > + * when Super-I/O functions move to a separate file, the Super-I/O
> > + * driver will probe 0x23 and 0x4e and auto-detect the presence of a
>
> Typo: 0x2e, not Ox23.
Good catch. Fixed.
> > + * w83627ehf hardware monitor, and call probe() */
> > + if (w83627ehf_superio_probe(0x2e, data) &&
> > + w83627ehf_superio_probe(0x4e, data)) {
> > + err = -ENODEV;
> > + goto exit_free;
> > + }
> >
> > - return i2c_isa_add_driver(&w83627ehf_driver);
> > + if ((err = platform_driver_register(&w83627ehf_driver))!=0)
> > + goto exit_free;
>
> Please remove the "!=0".
When I do that, I get the following warning:
CC [M] drivers/hwmon/w83627ehf.o
drivers/hwmon/w83627ehf.c: In function 'sensors_w83627ehf_init':
drivers/hwmon/w83627ehf.c:1428: warning: suggest parentheses around
assignment used as truth value
So for now, the !=0 is still there, but I also added a comment.
> > +
> > + if (!(pdev = platform_device_alloc(DRVNAME, data->addr))) {
> > + err = -ENOMEM;
> > + printk(KERN_ERR DRVNAME ": Device allocation failed\n");
> > + goto exit_unregister;
> > + }
> > +
> > + platform_set_drvdata(pdev, data);
> > +
> > + memset(&res, 0, sizeof(res));
> > + data->name = w83627ehf_device_names[data->kind];
> > + res.name = data->name;
> > + res.start = data->addr + REGION_OFFSET;
> > + res.end = data->addr + REGION_OFFSET + REGION_LENGTH - 1;
>
> No alignment in code blocks please.
Fixed.
> > + res.flags = IORESOURCE_IO;
> > + err = platform_device_add_resources(pdev, &res, 1);
> > + if (err) {
> > + printk(KERN_ERR DRVNAME ": Device resource addition failed "
> > + "(%d)\n", err);
> > + goto exit_device_put;
> > + }
> > +
> > + /* platform_device_add calls probe() */
> > + err = platform_device_add(pdev);
> > + if (err) {
> > + printk(KERN_ERR DRVNAME ": Device addition failed (%d)\n",
> > + err);
> > + goto exit_device_put;
> > + }
> > +
> > + return 0;
> > +
> > +exit_device_put:
> > + pdev->dev.platform_data = NULL;
> > + platform_device_put(pdev);
> > +exit_unregister:
> > + platform_driver_unregister(&w83627ehf_driver);
> > +exit_free:
> > + kfree(data);
> > +exit:
> > + return err;
> > }
> >
> > static void __exit sensors_w83627ehf_exit(void)
> > {
> > - i2c_isa_del_driver(&w83627ehf_driver);
> > + /* when Super-I/O functions move to a separate file, devices
> > + * will be unregistered before unloading the module.
>
> Not true. When Super-I/O handling is moved to a separate module,
> devices will live their life independently from a driver being loaded
> for them or not. So devices will survive the w83627ehf driver unloading.
Thank you for (patiently!) getting this through my head. So, I fixed
the comment. I'll try to keep it from creeping in again. ;-)
> > + *
> > + * for now, only one device is added in sensors_w83627ehf_init */
> > + platform_device_unregister(pdev);
> > + platform_driver_unregister(&w83627ehf_driver);
> > }
> >
> > MODULE_AUTHOR("Jean Delvare <khali at linux-fr.org>");
> > --- linux-2.6.21-rc5/drivers/hwmon/Kconfig 2007-03-28 17:25:43.000000000 -0700
> > +++ linux-2.6.21-rc5/drivers/hwmon/Kconfig 2007-03-28 20:50:11.000000000 -0700
> > @@ -527,7 +527,6 @@
> > config SENSORS_W83793
> > tristate "Winbond W83793"
> > depends on HWMON && I2C && EXPERIMENTAL
> > - select HWMON_VID
> > help
> > If you say yes here you get support for the Winbond W83793
> > hardware monitoring chip.
>
> You definitely don't want to do that.
Patch problems here stemmed from using the wrong file as the
"original." Fixed. Note that there are Kconfig changes in both patch
01 and patch 02.
> > @@ -560,17 +559,15 @@
> > will be called w83627hf.
> >
> > config SENSORS_W83627EHF
> > - tristate "Winbond W83627EHF"
> > - depends on HWMON && I2C && EXPERIMENTAL
> > - select I2C_ISA
> > + tristate "Winbond W83627EHF/DHG"
> > + depends on HWMON && EXPERIMENTAL
> > help
> > - If you say yes here you get preliminary support for the hardware
> > - monitoring functionality of the Winbond W83627EHF Super-I/O chip.
> > - Only fan and temperature inputs are supported at the moment, while
> > - the chip does much more than that.
> > + If you say yes here you get support for the hardware
> > + monitoring functionality of the Winbond W83627EHF/DHG Super-I/O chip.
> >
> > This driver also supports the W83627EHG, which is the lead-free
> > - version of the W83627EHF.
> > + version of the W83627EHF. The driver also supports the W83627DHG,
> > + which is a version suited for specific Intel processor sensing (PECI).
> >
> > This driver can also be built as a module. If so, the module
> > will be called w83627ehf.
>
> Most of this doesn't belong to this patch and should be moved to a
> separate patch.
So it's now patch 01. The stuff that belongs in patch 02 is (I think)
now all in patch 02. Patch 03 revises the error handling in a couple
of places to give more meaningful error messages. Patch 04 fixes a
long-standing bug where the driver outputs "Increasing fan 4 clock
divider from 4 to 8" and fills up the kernel message log.
Please review when you get time.
Thanks,
David
-------------- next part --------------
Add descriptions for w83627ehg and w83627dhg chips to Kconfig
Signed-off-by: David Hubbard <david.c.hubbard at gmail.com>
Index: linux-2.6.22-rc2/drivers/hwmon/Kconfig
===================================================================
--- linux-2.6.22-rc2/drivers/hwmon/Kconfig 2007-05-17 11:01:23.000000000 -0700
+++ linux-2.6.22-rc2/drivers/hwmon/Kconfig 2007-05-17 11:20:52.000000000 -0700
@@ -584,17 +584,17 @@
will be called w83627hf.
config SENSORS_W83627EHF
- tristate "Winbond W83627EHF"
+ tristate "Winbond W83627EHF/DHG"
depends on I2C && EXPERIMENTAL
select I2C_ISA
help
- If you say yes here you get preliminary support for the hardware
- monitoring functionality of the Winbond W83627EHF Super-I/O chip.
- Only fan and temperature inputs are supported at the moment, while
- the chip does much more than that.
+ If you say yes here you get support for the hardware
+ monitoring functionality of the Winbond W83627EHF/DHG Super-I/O chip.
This driver also supports the W83627EHG, which is the lead-free
- version of the W83627EHF.
+ version of the W83627EHF. The driver also supports the W83627DHG,
+ which is a similar chip suited for specific Intel processors that
+ use PECI such as the Core 2 Duo.
This driver can also be built as a module. If so, the module
will be called w83627ehf.
-------------- next part --------------
This patch removes i2c-isa from the w83627ehf driver, and uses
a platform driver instead.
Signed-off-by: David Hubbard <david.c.hubbard at gmail.com>
Index: linux-2.6.22-rc2/drivers/hwmon/w83627ehf.c
===================================================================
--- linux-2.6.22-rc2/drivers/hwmon/w83627ehf.c 2007-05-17 14:11:28.000000000 -0700
+++ linux-2.6.22-rc2/drivers/hwmon/w83627ehf.c 2007-05-19 17:29:21.000000000 -0700
@@ -41,8 +41,8 @@
#include <linux/module.h>
#include <linux/init.h>
#include <linux/slab.h>
-#include <linux/i2c.h>
-#include <linux/i2c-isa.h>
+#include <linux/jiffies.h>
+#include <linux/platform_device.h>
#include <linux/hwmon.h>
#include <linux/hwmon-sysfs.h>
#include <linux/err.h>
@@ -50,25 +50,19 @@
#include <asm/io.h>
#include "lm75.h"
-/* The actual ISA address is read from Super-I/O configuration space */
-static unsigned short address;
+enum kinds { w83627ehf, w83627dhg };
-/*
- * Super-I/O constants and functions
- */
+/* used to set data->name = w83627ehf_device_names[data->kind] */
+static const char * w83627ehf_device_names[] = {
+ "w83627ehf",
+ "w83627dhg",
+};
+
+#define DRVNAME "w83627ehf"
/*
- * The three following globals are initialized in w83627ehf_find(), before
- * the i2c-isa device is created. Otherwise, they could be stored in
- * w83627ehf_data. This is ugly, but necessary, and when the driver is next
- * updated to become a platform driver, the globals will disappear.
+ * Super-I/O constants and functions
*/
-static int REG; /* The register to read/write */
-static int VAL; /* The value to read/write */
-/* The w83627ehf/ehg have 10 voltage inputs, but the w83627dhg has 9. This
- * value is also used in w83627ehf_detect() to export a device name in sysfs
- * (e.g. w83627ehf or w83627dhg) */
-static int w83627ehf_num_in;
#define W83627EHF_LD_HWM 0x0b
@@ -83,47 +77,47 @@
#define SIO_ID_MASK 0xFFF0
static inline void
-superio_outb(int reg, int val)
+superio_outb(int ioreg, int reg, int val)
{
- outb(reg, REG);
- outb(val, VAL);
+ outb(reg, ioreg);
+ outb(val, ioreg + 1);
}
static inline int
-superio_inb(int reg)
+superio_inb(int ioreg, int reg)
{
- outb(reg, REG);
- return inb(VAL);
+ outb(reg, ioreg);
+ return inb(ioreg + 1);
}
static inline void
-superio_select(int ld)
+superio_select(int ioreg, int ld)
{
- outb(SIO_REG_LDSEL, REG);
- outb(ld, VAL);
+ outb(SIO_REG_LDSEL, ioreg);
+ outb(ld, ioreg + 1);
}
static inline void
-superio_enter(void)
+superio_enter(int ioreg)
{
- outb(0x87, REG);
- outb(0x87, REG);
+ outb(0x87, ioreg);
+ outb(0x87, ioreg);
}
static inline void
-superio_exit(void)
+superio_exit(int ioreg)
{
- outb(0x02, REG);
- outb(0x02, VAL);
+ outb(0x02, ioreg);
+ outb(0x02, ioreg + 1);
}
/*
* ISA constants
*/
-#define IOREGION_ALIGNMENT ~7
-#define IOREGION_OFFSET 5
-#define IOREGION_LENGTH 2
+#define REGION_ALIGNMENT ~7
+#define REGION_OFFSET 5
+#define REGION_LENGTH 2
#define ADDR_REG_OFFSET 5
#define DATA_REG_OFFSET 6
@@ -255,7 +249,11 @@
*/
struct w83627ehf_data {
- struct i2c_client client;
+ int sioreg;
+ enum kinds kind;
+ int addr; /* IO base of hw monitor block */
+ const char *name;
+
struct class_device *class_dev;
struct mutex lock;
@@ -264,6 +262,7 @@
unsigned long last_updated; /* In jiffies */
/* Register values */
+ u8 in_num; /* number of in inputs we have */
u8 in[10]; /* Register value */
u8 in_max[10]; /* Register value */
u8 in_min[10]; /* Register value */
@@ -303,121 +302,117 @@
nothing for registers which live in bank 0. For others, they respectively
set the bank register to the correct value (before the register is
accessed), and back to 0 (afterwards). */
-static inline void w83627ehf_set_bank(struct i2c_client *client, u16 reg)
+static inline void w83627ehf_set_bank(struct w83627ehf_data *data, u16 reg)
{
if (reg & 0xff00) {
- outb_p(W83627EHF_REG_BANK, client->addr + ADDR_REG_OFFSET);
- outb_p(reg >> 8, client->addr + DATA_REG_OFFSET);
+ outb_p(W83627EHF_REG_BANK, data->addr + ADDR_REG_OFFSET);
+ outb_p(reg >> 8, data->addr + DATA_REG_OFFSET);
}
}
-static inline void w83627ehf_reset_bank(struct i2c_client *client, u16 reg)
+static inline void w83627ehf_reset_bank(struct w83627ehf_data *data, u16 reg)
{
if (reg & 0xff00) {
- outb_p(W83627EHF_REG_BANK, client->addr + ADDR_REG_OFFSET);
- outb_p(0, client->addr + DATA_REG_OFFSET);
+ outb_p(W83627EHF_REG_BANK, data->addr + ADDR_REG_OFFSET);
+ outb_p(0, data->addr + DATA_REG_OFFSET);
}
}
-static u16 w83627ehf_read_value(struct i2c_client *client, u16 reg)
+static u16 w83627ehf_read_value(struct w83627ehf_data *data, u16 reg)
{
- struct w83627ehf_data *data = i2c_get_clientdata(client);
int res, word_sized = is_word_sized(reg);
mutex_lock(&data->lock);
- w83627ehf_set_bank(client, reg);
- outb_p(reg & 0xff, client->addr + ADDR_REG_OFFSET);
- res = inb_p(client->addr + DATA_REG_OFFSET);
+ w83627ehf_set_bank(data, reg);
+ outb_p(reg & 0xff, data->addr + ADDR_REG_OFFSET);
+ res = inb_p(data->addr + DATA_REG_OFFSET);
if (word_sized) {
outb_p((reg & 0xff) + 1,
- client->addr + ADDR_REG_OFFSET);
- res = (res << 8) + inb_p(client->addr + DATA_REG_OFFSET);
+ data->addr + ADDR_REG_OFFSET);
+ res = (res << 8) + inb_p(data->addr + DATA_REG_OFFSET);
}
- w83627ehf_reset_bank(client, reg);
+ w83627ehf_reset_bank(data, reg);
mutex_unlock(&data->lock);
return res;
}
-static int w83627ehf_write_value(struct i2c_client *client, u16 reg, u16 value)
+static int w83627ehf_write_value(struct w83627ehf_data *data, u16 reg, u16 value)
{
- struct w83627ehf_data *data = i2c_get_clientdata(client);
int word_sized = is_word_sized(reg);
mutex_lock(&data->lock);
- w83627ehf_set_bank(client, reg);
- outb_p(reg & 0xff, client->addr + ADDR_REG_OFFSET);
+ w83627ehf_set_bank(data, reg);
+ outb_p(reg & 0xff, data->addr + ADDR_REG_OFFSET);
if (word_sized) {
- outb_p(value >> 8, client->addr + DATA_REG_OFFSET);
+ outb_p(value >> 8, data->addr + DATA_REG_OFFSET);
outb_p((reg & 0xff) + 1,
- client->addr + ADDR_REG_OFFSET);
+ data->addr + ADDR_REG_OFFSET);
}
- outb_p(value & 0xff, client->addr + DATA_REG_OFFSET);
- w83627ehf_reset_bank(client, reg);
+ outb_p(value & 0xff, data->addr + DATA_REG_OFFSET);
+ w83627ehf_reset_bank(data, reg);
mutex_unlock(&data->lock);
return 0;
}
/* This function assumes that the caller holds data->update_lock */
-static void w83627ehf_write_fan_div(struct i2c_client *client, int nr)
+static void w83627ehf_write_fan_div(struct w83627ehf_data *data, int nr)
{
- struct w83627ehf_data *data = i2c_get_clientdata(client);
u8 reg;
switch (nr) {
case 0:
- reg = (w83627ehf_read_value(client, W83627EHF_REG_FANDIV1) & 0xcf)
+ reg = (w83627ehf_read_value(data, W83627EHF_REG_FANDIV1) & 0xcf)
| ((data->fan_div[0] & 0x03) << 4);
/* fan5 input control bit is write only, compute the value */
reg |= (data->has_fan & (1 << 4)) ? 1 : 0;
- w83627ehf_write_value(client, W83627EHF_REG_FANDIV1, reg);
- reg = (w83627ehf_read_value(client, W83627EHF_REG_VBAT) & 0xdf)
+ w83627ehf_write_value(data, W83627EHF_REG_FANDIV1, reg);
+ reg = (w83627ehf_read_value(data, W83627EHF_REG_VBAT) & 0xdf)
| ((data->fan_div[0] & 0x04) << 3);
- w83627ehf_write_value(client, W83627EHF_REG_VBAT, reg);
+ w83627ehf_write_value(data, W83627EHF_REG_VBAT, reg);
break;
case 1:
- reg = (w83627ehf_read_value(client, W83627EHF_REG_FANDIV1) & 0x3f)
+ reg = (w83627ehf_read_value(data, W83627EHF_REG_FANDIV1) & 0x3f)
| ((data->fan_div[1] & 0x03) << 6);
/* fan5 input control bit is write only, compute the value */
reg |= (data->has_fan & (1 << 4)) ? 1 : 0;
- w83627ehf_write_value(client, W83627EHF_REG_FANDIV1, reg);
- reg = (w83627ehf_read_value(client, W83627EHF_REG_VBAT) & 0xbf)
+ w83627ehf_write_value(data, W83627EHF_REG_FANDIV1, reg);
+ reg = (w83627ehf_read_value(data, W83627EHF_REG_VBAT) & 0xbf)
| ((data->fan_div[1] & 0x04) << 4);
- w83627ehf_write_value(client, W83627EHF_REG_VBAT, reg);
+ w83627ehf_write_value(data, W83627EHF_REG_VBAT, reg);
break;
case 2:
- reg = (w83627ehf_read_value(client, W83627EHF_REG_FANDIV2) & 0x3f)
+ reg = (w83627ehf_read_value(data, W83627EHF_REG_FANDIV2) & 0x3f)
| ((data->fan_div[2] & 0x03) << 6);
- w83627ehf_write_value(client, W83627EHF_REG_FANDIV2, reg);
- reg = (w83627ehf_read_value(client, W83627EHF_REG_VBAT) & 0x7f)
+ w83627ehf_write_value(data, W83627EHF_REG_FANDIV2, reg);
+ reg = (w83627ehf_read_value(data, W83627EHF_REG_VBAT) & 0x7f)
| ((data->fan_div[2] & 0x04) << 5);
- w83627ehf_write_value(client, W83627EHF_REG_VBAT, reg);
+ w83627ehf_write_value(data, W83627EHF_REG_VBAT, reg);
break;
case 3:
- reg = (w83627ehf_read_value(client, W83627EHF_REG_DIODE) & 0xfc)
+ reg = (w83627ehf_read_value(data, W83627EHF_REG_DIODE) & 0xfc)
| (data->fan_div[3] & 0x03);
- w83627ehf_write_value(client, W83627EHF_REG_DIODE, reg);
- reg = (w83627ehf_read_value(client, W83627EHF_REG_SMI_OVT) & 0x7f)
+ w83627ehf_write_value(data, W83627EHF_REG_DIODE, reg);
+ reg = (w83627ehf_read_value(data, W83627EHF_REG_SMI_OVT) & 0x7f)
| ((data->fan_div[3] & 0x04) << 5);
- w83627ehf_write_value(client, W83627EHF_REG_SMI_OVT, reg);
+ w83627ehf_write_value(data, W83627EHF_REG_SMI_OVT, reg);
break;
case 4:
- reg = (w83627ehf_read_value(client, W83627EHF_REG_DIODE) & 0x73)
- | ((data->fan_div[4] & 0x03) << 2)
+ reg = (w83627ehf_read_value(data, W83627EHF_REG_DIODE) & 0x73)
+ | ((data->fan_div[4] & 0x03) << 3)
| ((data->fan_div[4] & 0x04) << 5);
- w83627ehf_write_value(client, W83627EHF_REG_DIODE, reg);
+ w83627ehf_write_value(data, W83627EHF_REG_DIODE, reg);
break;
}
}
static struct w83627ehf_data *w83627ehf_update_device(struct device *dev)
{
- struct i2c_client *client = to_i2c_client(dev);
- struct w83627ehf_data *data = i2c_get_clientdata(client);
+ struct w83627ehf_data *data = dev->platform_data;
int pwmcfg = 0, tolerance = 0; /* shut up the compiler */
int i;
@@ -426,33 +421,33 @@
if (time_after(jiffies, data->last_updated + HZ)
|| !data->valid) {
/* Fan clock dividers */
- i = w83627ehf_read_value(client, W83627EHF_REG_FANDIV1);
+ i = w83627ehf_read_value(data, W83627EHF_REG_FANDIV1);
data->fan_div[0] = (i >> 4) & 0x03;
data->fan_div[1] = (i >> 6) & 0x03;
- i = w83627ehf_read_value(client, W83627EHF_REG_FANDIV2);
+ i = w83627ehf_read_value(data, W83627EHF_REG_FANDIV2);
data->fan_div[2] = (i >> 6) & 0x03;
- i = w83627ehf_read_value(client, W83627EHF_REG_VBAT);
+ i = w83627ehf_read_value(data, W83627EHF_REG_VBAT);
data->fan_div[0] |= (i >> 3) & 0x04;
data->fan_div[1] |= (i >> 4) & 0x04;
data->fan_div[2] |= (i >> 5) & 0x04;
if (data->has_fan & ((1 << 3) | (1 << 4))) {
- i = w83627ehf_read_value(client, W83627EHF_REG_DIODE);
+ i = w83627ehf_read_value(data, W83627EHF_REG_DIODE);
data->fan_div[3] = i & 0x03;
data->fan_div[4] = ((i >> 2) & 0x03)
| ((i >> 5) & 0x04);
}
if (data->has_fan & (1 << 3)) {
- i = w83627ehf_read_value(client, W83627EHF_REG_SMI_OVT);
+ i = w83627ehf_read_value(data, W83627EHF_REG_SMI_OVT);
data->fan_div[3] |= (i >> 5) & 0x04;
}
/* Measured voltages and limits */
- for (i = 0; i < w83627ehf_num_in; i++) {
- data->in[i] = w83627ehf_read_value(client,
+ for (i = 0; i < data->in_num; i++) {
+ data->in[i] = w83627ehf_read_value(data,
W83627EHF_REG_IN(i));
- data->in_min[i] = w83627ehf_read_value(client,
+ data->in_min[i] = w83627ehf_read_value(data,
W83627EHF_REG_IN_MIN(i));
- data->in_max[i] = w83627ehf_read_value(client,
+ data->in_max[i] = w83627ehf_read_value(data,
W83627EHF_REG_IN_MAX(i));
}
@@ -461,9 +456,9 @@
if (!(data->has_fan & (1 << i)))
continue;
- data->fan[i] = w83627ehf_read_value(client,
+ data->fan[i] = w83627ehf_read_value(data,
W83627EHF_REG_FAN[i]);
- data->fan_min[i] = w83627ehf_read_value(client,
+ data->fan_min[i] = w83627ehf_read_value(data,
W83627EHF_REG_FAN_MIN[i]);
/* If we failed to measure the fan speed and clock
@@ -471,16 +466,16 @@
time */
if (data->fan[i] == 0xff
&& data->fan_div[i] < 0x07) {
- dev_dbg(&client->dev, "Increasing fan%d "
+ dev_dbg(dev, "Increasing fan %d "
"clock divider from %u to %u\n",
- i + 1, div_from_reg(data->fan_div[i]),
+ i, div_from_reg(data->fan_div[i]),
div_from_reg(data->fan_div[i] + 1));
data->fan_div[i]++;
- w83627ehf_write_fan_div(client, i);
+ w83627ehf_write_fan_div(data, i);
/* Preserve min limit if possible */
if (data->fan_min[i] >= 2
&& data->fan_min[i] != 255)
- w83627ehf_write_value(client,
+ w83627ehf_write_value(data,
W83627EHF_REG_FAN_MIN[i],
(data->fan_min[i] /= 2));
}
@@ -489,9 +484,9 @@
for (i = 0; i < 4; i++) {
/* pwmcfg, tolarance mapped for i=0, i=1 to same reg */
if (i != 1) {
- pwmcfg = w83627ehf_read_value(client,
+ pwmcfg = w83627ehf_read_value(data,
W83627EHF_REG_PWM_ENABLE[i]);
- tolerance = w83627ehf_read_value(client,
+ tolerance = w83627ehf_read_value(data,
W83627EHF_REG_TOLERANCE[i]);
}
data->pwm_mode[i] =
@@ -500,14 +495,14 @@
data->pwm_enable[i] =
((pwmcfg >> W83627EHF_PWM_ENABLE_SHIFT[i])
& 3) + 1;
- data->pwm[i] = w83627ehf_read_value(client,
+ data->pwm[i] = w83627ehf_read_value(data,
W83627EHF_REG_PWM[i]);
- data->fan_min_output[i] = w83627ehf_read_value(client,
+ data->fan_min_output[i] = w83627ehf_read_value(data,
W83627EHF_REG_FAN_MIN_OUTPUT[i]);
- data->fan_stop_time[i] = w83627ehf_read_value(client,
+ data->fan_stop_time[i] = w83627ehf_read_value(data,
W83627EHF_REG_FAN_STOP_TIME[i]);
data->target_temp[i] =
- w83627ehf_read_value(client,
+ w83627ehf_read_value(data,
W83627EHF_REG_TARGET[i]) &
(data->pwm_mode[i] == 1 ? 0x7f : 0xff);
data->tolerance[i] = (tolerance >> (i == 1 ? 4 : 0))
@@ -515,26 +510,26 @@
}
/* Measured temperatures and limits */
- data->temp1 = w83627ehf_read_value(client,
+ data->temp1 = w83627ehf_read_value(data,
W83627EHF_REG_TEMP1);
- data->temp1_max = w83627ehf_read_value(client,
+ data->temp1_max = w83627ehf_read_value(data,
W83627EHF_REG_TEMP1_OVER);
- data->temp1_max_hyst = w83627ehf_read_value(client,
+ data->temp1_max_hyst = w83627ehf_read_value(data,
W83627EHF_REG_TEMP1_HYST);
for (i = 0; i < 2; i++) {
- data->temp[i] = w83627ehf_read_value(client,
+ data->temp[i] = w83627ehf_read_value(data,
W83627EHF_REG_TEMP[i]);
- data->temp_max[i] = w83627ehf_read_value(client,
+ data->temp_max[i] = w83627ehf_read_value(data,
W83627EHF_REG_TEMP_OVER[i]);
- data->temp_max_hyst[i] = w83627ehf_read_value(client,
+ data->temp_max_hyst[i] = w83627ehf_read_value(data,
W83627EHF_REG_TEMP_HYST[i]);
}
- data->alarms = w83627ehf_read_value(client,
+ data->alarms = w83627ehf_read_value(data,
W83627EHF_REG_ALARM1) |
- (w83627ehf_read_value(client,
+ (w83627ehf_read_value(data,
W83627EHF_REG_ALARM2) << 8) |
- (w83627ehf_read_value(client,
+ (w83627ehf_read_value(data,
W83627EHF_REG_ALARM3) << 16);
data->last_updated = jiffies;
@@ -567,15 +562,14 @@
store_in_##reg (struct device *dev, struct device_attribute *attr, \
const char *buf, size_t count) \
{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct w83627ehf_data *data = i2c_get_clientdata(client); \
+ struct w83627ehf_data *data = dev->platform_data; \
struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr); \
int nr = sensor_attr->index; \
u32 val = simple_strtoul(buf, NULL, 10); \
\
mutex_lock(&data->update_lock); \
data->in_##reg[nr] = in_to_reg(val, nr); \
- w83627ehf_write_value(client, W83627EHF_REG_IN_##REG(nr), \
+ w83627ehf_write_value(data, W83627EHF_REG_IN_##REG(nr), \
data->in_##reg[nr]); \
mutex_unlock(&data->update_lock); \
return count; \
@@ -673,8 +667,7 @@
store_fan_min(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
- struct i2c_client *client = to_i2c_client(dev);
- struct w83627ehf_data *data = i2c_get_clientdata(client);
+ struct w83627ehf_data *data = dev->platform_data;
struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
int nr = sensor_attr->index;
unsigned int val = simple_strtoul(buf, NULL, 10);
@@ -725,9 +718,9 @@
nr + 1, div_from_reg(data->fan_div[nr]),
div_from_reg(new_div));
data->fan_div[nr] = new_div;
- w83627ehf_write_fan_div(client, nr);
+ w83627ehf_write_fan_div(data, nr);
}
- w83627ehf_write_value(client, W83627EHF_REG_FAN_MIN[nr],
+ w83627ehf_write_value(data, W83627EHF_REG_FAN_MIN[nr],
data->fan_min[nr]);
mutex_unlock(&data->update_lock);
@@ -788,13 +781,12 @@
store_temp1_##reg(struct device *dev, struct device_attribute *attr, \
const char *buf, size_t count) \
{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct w83627ehf_data *data = i2c_get_clientdata(client); \
+ struct w83627ehf_data *data = dev->platform_data; \
u32 val = simple_strtoul(buf, NULL, 10); \
\
mutex_lock(&data->update_lock); \
data->temp1_##reg = temp1_to_reg(val, -128000, 127000); \
- w83627ehf_write_value(client, W83627EHF_REG_TEMP1_##REG, \
+ w83627ehf_write_value(data, W83627EHF_REG_TEMP1_##REG, \
data->temp1_##reg); \
mutex_unlock(&data->update_lock); \
return count; \
@@ -822,15 +814,14 @@
store_##reg(struct device *dev, struct device_attribute *attr, \
const char *buf, size_t count) \
{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct w83627ehf_data *data = i2c_get_clientdata(client); \
+ struct w83627ehf_data *data = dev->platform_data; \
struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr); \
int nr = sensor_attr->index; \
u32 val = simple_strtoul(buf, NULL, 10); \
\
mutex_lock(&data->update_lock); \
data->reg[nr] = LM75_TEMP_TO_REG(val); \
- w83627ehf_write_value(client, W83627EHF_REG_TEMP_##REG[nr], \
+ w83627ehf_write_value(data, W83627EHF_REG_TEMP_##REG[nr], \
data->reg[nr]); \
mutex_unlock(&data->update_lock); \
return count; \
@@ -877,8 +868,7 @@
store_pwm_mode(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
- struct i2c_client *client = to_i2c_client(dev);
- struct w83627ehf_data *data = i2c_get_clientdata(client);
+ struct w83627ehf_data *data = dev->platform_data;
struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
int nr = sensor_attr->index;
u32 val = simple_strtoul(buf, NULL, 10);
@@ -887,12 +877,12 @@
if (val > 1)
return -EINVAL;
mutex_lock(&data->update_lock);
- reg = w83627ehf_read_value(client, W83627EHF_REG_PWM_ENABLE[nr]);
+ reg = w83627ehf_read_value(data, W83627EHF_REG_PWM_ENABLE[nr]);
data->pwm_mode[nr] = val;
reg &= ~(1 << W83627EHF_PWM_MODE_SHIFT[nr]);
if (!val)
reg |= 1 << W83627EHF_PWM_MODE_SHIFT[nr];
- w83627ehf_write_value(client, W83627EHF_REG_PWM_ENABLE[nr], reg);
+ w83627ehf_write_value(data, W83627EHF_REG_PWM_ENABLE[nr], reg);
mutex_unlock(&data->update_lock);
return count;
}
@@ -901,15 +891,14 @@
store_pwm(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
- struct i2c_client *client = to_i2c_client(dev);
- struct w83627ehf_data *data = i2c_get_clientdata(client);
+ struct w83627ehf_data *data = dev->platform_data;
struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
int nr = sensor_attr->index;
u32 val = SENSORS_LIMIT(simple_strtoul(buf, NULL, 10), 0, 255);
mutex_lock(&data->update_lock);
data->pwm[nr] = val;
- w83627ehf_write_value(client, W83627EHF_REG_PWM[nr], val);
+ w83627ehf_write_value(data, W83627EHF_REG_PWM[nr], val);
mutex_unlock(&data->update_lock);
return count;
}
@@ -918,8 +907,7 @@
store_pwm_enable(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
- struct i2c_client *client = to_i2c_client(dev);
- struct w83627ehf_data *data = i2c_get_clientdata(client);
+ struct w83627ehf_data *data = dev->platform_data;
struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
int nr = sensor_attr->index;
u32 val = simple_strtoul(buf, NULL, 10);
@@ -928,11 +916,11 @@
if (!val || (val > 2)) /* only modes 1 and 2 are supported */
return -EINVAL;
mutex_lock(&data->update_lock);
- reg = w83627ehf_read_value(client, W83627EHF_REG_PWM_ENABLE[nr]);
+ reg = w83627ehf_read_value(data, W83627EHF_REG_PWM_ENABLE[nr]);
data->pwm_enable[nr] = val;
reg &= ~(0x03 << W83627EHF_PWM_ENABLE_SHIFT[nr]);
reg |= (val - 1) << W83627EHF_PWM_ENABLE_SHIFT[nr];
- w83627ehf_write_value(client, W83627EHF_REG_PWM_ENABLE[nr], reg);
+ w83627ehf_write_value(data, W83627EHF_REG_PWM_ENABLE[nr], reg);
mutex_unlock(&data->update_lock);
return count;
}
@@ -955,15 +943,14 @@
store_target_temp(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
- struct i2c_client *client = to_i2c_client(dev);
- struct w83627ehf_data *data = i2c_get_clientdata(client);
+ struct w83627ehf_data *data = dev->platform_data;
struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
int nr = sensor_attr->index;
u8 val = temp1_to_reg(simple_strtoul(buf, NULL, 10), 0, 127000);
mutex_lock(&data->update_lock);
data->target_temp[nr] = val;
- w83627ehf_write_value(client, W83627EHF_REG_TARGET[nr], val);
+ w83627ehf_write_value(data, W83627EHF_REG_TARGET[nr], val);
mutex_unlock(&data->update_lock);
return count;
}
@@ -972,8 +959,7 @@
store_tolerance(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
- struct i2c_client *client = to_i2c_client(dev);
- struct w83627ehf_data *data = i2c_get_clientdata(client);
+ struct w83627ehf_data *data = dev->platform_data;
struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
int nr = sensor_attr->index;
u16 reg;
@@ -981,13 +967,13 @@
u8 val = temp1_to_reg(simple_strtoul(buf, NULL, 10), 0, 15000);
mutex_lock(&data->update_lock);
- reg = w83627ehf_read_value(client, W83627EHF_REG_TOLERANCE[nr]);
+ reg = w83627ehf_read_value(data, W83627EHF_REG_TOLERANCE[nr]);
data->tolerance[nr] = val;
if (nr == 1)
reg = (reg & 0x0f) | (val << 4);
else
reg = (reg & 0xf0) | val;
- w83627ehf_write_value(client, W83627EHF_REG_TOLERANCE[nr], reg);
+ w83627ehf_write_value(data, W83627EHF_REG_TOLERANCE[nr], reg);
mutex_unlock(&data->update_lock);
return count;
}
@@ -1058,14 +1044,13 @@
store_##reg(struct device *dev, struct device_attribute *attr, \
const char *buf, size_t count) \
{\
- struct i2c_client *client = to_i2c_client(dev); \
- struct w83627ehf_data *data = i2c_get_clientdata(client); \
+ struct w83627ehf_data *data = dev->platform_data; \
struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr); \
int nr = sensor_attr->index; \
u32 val = SENSORS_LIMIT(simple_strtoul(buf, NULL, 10), 1, 255); \
mutex_lock(&data->update_lock); \
data->reg[nr] = val; \
- w83627ehf_write_value(client, W83627EHF_REG_##REG[nr], val); \
+ w83627ehf_write_value(data, W83627EHF_REG_##REG[nr], val); \
mutex_unlock(&data->update_lock); \
return count; \
}
@@ -1087,21 +1072,29 @@
store_##reg(struct device *dev, struct device_attribute *attr, \
const char *buf, size_t count) \
{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct w83627ehf_data *data = i2c_get_clientdata(client); \
+ struct w83627ehf_data *data = dev->platform_data; \
struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr); \
int nr = sensor_attr->index; \
u8 val = step_time_to_reg(simple_strtoul(buf, NULL, 10), \
data->pwm_mode[nr]); \
mutex_lock(&data->update_lock); \
data->reg[nr] = val; \
- w83627ehf_write_value(client, W83627EHF_REG_##REG[nr], val); \
+ w83627ehf_write_value(data, W83627EHF_REG_##REG[nr], val); \
mutex_unlock(&data->update_lock); \
return count; \
} \
fan_time_functions(fan_stop_time, FAN_STOP_TIME)
+static ssize_t show_name(struct device *dev, struct device_attribute
+ *devattr, char *buf)
+{
+ struct w83627ehf_data *data = dev->platform_data;
+
+ return sprintf(buf, "%s\n", data->name);
+}
+
+static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
static struct sensor_device_attribute sda_sf3_arrays_fan4[] = {
SENSOR_ATTR(pwm4_stop_time, S_IWUSR | S_IRUGO, show_fan_stop_time,
@@ -1126,7 +1119,7 @@
};
/*
- * Driver and client management
+ * Driver and platform management
*/
static void w83627ehf_device_remove_files(struct device *dev)
@@ -1134,12 +1127,13 @@
/* some entries in the following arrays may not have been used in
* device_create_file(), but device_remove_file() will ignore them */
int i;
+ struct w83627ehf_data *data = dev->platform_data;
for (i = 0; i < ARRAY_SIZE(sda_sf3_arrays); i++)
device_remove_file(dev, &sda_sf3_arrays[i].dev_attr);
for (i = 0; i < ARRAY_SIZE(sda_sf3_arrays_fan4); i++)
device_remove_file(dev, &sda_sf3_arrays_fan4[i].dev_attr);
- for (i = 0; i < w83627ehf_num_in; i++) {
+ for (i = 0; i < data->in_num; i++) {
device_remove_file(dev, &sda_in_input[i].dev_attr);
device_remove_file(dev, &sda_in_alarm[i].dev_attr);
device_remove_file(dev, &sda_in_min[i].dev_attr);
@@ -1160,86 +1154,73 @@
}
for (i = 0; i < ARRAY_SIZE(sda_temp); i++)
device_remove_file(dev, &sda_temp[i].dev_attr);
-}
-static struct i2c_driver w83627ehf_driver;
+ device_remove_file(dev, &dev_attr_name);
+}
-static void w83627ehf_init_client(struct i2c_client *client)
+/* w83627ehf_init_device() gets the monitoring functions started */
+static inline void __devinit w83627ehf_init_device(struct w83627ehf_data *data)
{
int i;
u8 tmp;
/* Start monitoring is needed */
- tmp = w83627ehf_read_value(client, W83627EHF_REG_CONFIG);
+ tmp = w83627ehf_read_value(data, W83627EHF_REG_CONFIG);
if (!(tmp & 0x01))
- w83627ehf_write_value(client, W83627EHF_REG_CONFIG,
+ w83627ehf_write_value(data, W83627EHF_REG_CONFIG,
tmp | 0x01);
/* Enable temp2 and temp3 if needed */
for (i = 0; i < 2; i++) {
- tmp = w83627ehf_read_value(client,
+ tmp = w83627ehf_read_value(data,
W83627EHF_REG_TEMP_CONFIG[i]);
if (tmp & 0x01)
- w83627ehf_write_value(client,
+ w83627ehf_write_value(data,
W83627EHF_REG_TEMP_CONFIG[i],
tmp & 0xfe);
}
}
-static int w83627ehf_detect(struct i2c_adapter *adapter)
+/* w83627ehf_probe() looks for a Super-I/O chip at known addresses */
+static int __devinit w83627ehf_probe(struct platform_device *pdev)
{
- struct i2c_client *client;
- struct w83627ehf_data *data;
- struct device *dev;
+ struct device *dev = &pdev->dev;
+ struct w83627ehf_data *data = pdev->dev.platform_data;
u8 fan4pin, fan5pin;
int i, err = 0;
- if (!request_region(address + IOREGION_OFFSET, IOREGION_LENGTH,
- w83627ehf_driver.driver.name)) {
+ if (!request_region(data->addr + REGION_OFFSET, REGION_LENGTH,
+ DRVNAME)) {
err = -EBUSY;
+ dev_err(dev, "Failed to request region 0x%lx-0x%lx\n",
+ (unsigned long) (data->addr + REGION_OFFSET),
+ (unsigned long) (
+ data->addr + REGION_OFFSET + REGION_LENGTH - 1));
goto exit;
}
- if (!(data = kzalloc(sizeof(struct w83627ehf_data), GFP_KERNEL))) {
- err = -ENOMEM;
- goto exit_release;
- }
-
- client = &data->client;
- i2c_set_clientdata(client, data);
- client->addr = address;
- mutex_init(&data->lock);
- client->adapter = adapter;
- client->driver = &w83627ehf_driver;
- client->flags = 0;
- dev = &client->dev;
-
- if (w83627ehf_num_in == 9)
- strlcpy(client->name, "w83627dhg", I2C_NAME_SIZE);
- else /* just say ehf. 627EHG is 627EHF in lead-free packaging. */
- strlcpy(client->name, "w83627ehf", I2C_NAME_SIZE);
+ /* fill in fields in *data so w83627ehf_read_value(), etc., work */
data->valid = 0;
+ mutex_init(&data->lock);
mutex_init(&data->update_lock);
- /* Tell the i2c layer a new client has arrived */
- if ((err = i2c_attach_client(client)))
- goto exit_free;
+ /* 627EHG and 627EHF have 10 voltage inputs; DHG has 9 */
+ data->in_num = (data->kind == w83627dhg) ? 9 : 10;
/* Initialize the chip */
- w83627ehf_init_client(client);
+ w83627ehf_init_device(data);
- /* A few vars need to be filled upon startup */
for (i = 0; i < 5; i++)
- data->fan_min[i] = w83627ehf_read_value(client,
+ data->fan_min[i] = w83627ehf_read_value(data,
W83627EHF_REG_FAN_MIN[i]);
/* fan4 and fan5 share some pins with the GPIO and serial flash */
- superio_enter();
- fan5pin = superio_inb(0x24) & 0x2;
- fan4pin = superio_inb(0x29) & 0x6;
- superio_exit();
+ superio_enter(data->sioreg);
+ fan5pin = superio_inb(data->sioreg, 0x24) & 0x2;
+ fan4pin = superio_inb(data->sioreg, 0x29) & 0x6;
+ superio_exit(data->sioreg);
/* It looks like fan4 and fan5 pins can be alternatively used
as fan on/off switches, but fan5 control is write only :/
@@ -1248,7 +1229,7 @@
is not the default. */
data->has_fan = 0x07; /* fan1, fan2 and fan3 */
- i = w83627ehf_read_value(client, W83627EHF_REG_FANDIV1);
+ i = w83627ehf_read_value(data, W83627EHF_REG_FANDIV1);
if ((i & (1 << 2)) && (!fan4pin))
data->has_fan |= (1 << 3);
if (!(i & (1 << 1)) && (!fan5pin))
@@ -1257,7 +1238,7 @@
/* Register sysfs hooks */
for (i = 0; i < ARRAY_SIZE(sda_sf3_arrays); i++)
if ((err = device_create_file(dev,
- &sda_sf3_arrays[i].dev_attr)))
+ &sda_sf3_arrays[i].dev_attr)))
goto exit_remove;
/* if fan4 is enabled create the sf3 files for it */
@@ -1268,7 +1249,7 @@
goto exit_remove;
}
- for (i = 0; i < w83627ehf_num_in; i++)
+ for (i = 0; i < data->in_num; i++)
if ((err = device_create_file(dev, &sda_in_input[i].dev_attr))
|| (err = device_create_file(dev,
&sda_in_alarm[i].dev_attr))
@@ -1308,6 +1289,10 @@
if ((err = device_create_file(dev, &sda_temp[i].dev_attr)))
goto exit_remove;
+ err = device_create_file(dev, &dev_attr_name);
+ if (err)
+ goto exit_remove;
+
data->class_dev = hwmon_device_register(dev);
if (IS_ERR(data->class_dev)) {
err = PTR_ERR(data->class_dev);
@@ -1318,95 +1303,184 @@
exit_remove:
w83627ehf_device_remove_files(dev);
- i2c_detach_client(client);
-exit_free:
- kfree(data);
-exit_release:
- release_region(address + IOREGION_OFFSET, IOREGION_LENGTH);
+ release_region(data->addr + REGION_OFFSET, REGION_LENGTH);
exit:
return err;
}
-static int w83627ehf_detach_client(struct i2c_client *client)
+static int __devexit w83627ehf_remove(struct platform_device *pdev)
{
- struct w83627ehf_data *data = i2c_get_clientdata(client);
- int err;
+ struct w83627ehf_data *data = pdev->dev.platform_data;
hwmon_device_unregister(data->class_dev);
- w83627ehf_device_remove_files(&client->dev);
-
- if ((err = i2c_detach_client(client)))
- return err;
- release_region(client->addr + IOREGION_OFFSET, IOREGION_LENGTH);
+ w83627ehf_device_remove_files(&pdev->dev);
+ pdev->dev.platform_data = NULL;
+ release_region(data->addr + REGION_OFFSET, REGION_LENGTH);
+
+ /* Please note that the kzalloc() is not in w83627ehf_probe() -- it is
+ * in sensors_w83627ehf_init() because Super-I/O configuration data
+ * is stored in it (specifically, data->sioreg and data->addr). This
+ * is not consistent with the device driver model, and needs to be
+ * fixed when a full Super-I/O bus driver is in place. kfree the data
+ * here for now because it is simpler than doing it in
+ * sensors_w83627ehf_exit() */
kfree(data);
return 0;
}
-static struct i2c_driver w83627ehf_driver = {
+static struct platform_driver w83627ehf_driver = {
.driver = {
.owner = THIS_MODULE,
- .name = "w83627ehf",
+ .name = DRVNAME,
},
- .attach_adapter = w83627ehf_detect,
- .detach_client = w83627ehf_detach_client,
+ .probe = w83627ehf_probe,
+ .remove = __devexit_p(w83627ehf_remove),
};
-static int __init w83627ehf_find(int sioaddr, unsigned short *addr)
+/* w83627ehf_superio_probe() looks for a '627 in the Super-I/O config space */
+static int __init w83627ehf_superio_probe(int sioaddr,
+ struct w83627ehf_data *data)
{
u16 val;
- REG = sioaddr;
- VAL = sioaddr + 1;
- superio_enter();
+ static const char __initdata sio_name_W83627EHF[] = "W83627EHF";
+ static const char __initdata sio_name_W83627EHG[] = "W83627EHG";
+ static const char __initdata sio_name_W83627DHG[] = "W83627DHG";
+
+ const char * sio_name;
- val = (superio_inb(SIO_REG_DEVID) << 8)
- | superio_inb(SIO_REG_DEVID + 1);
+ data->sioreg = sioaddr;
+
+ superio_enter(data->sioreg);
+
+ val = (superio_inb(data->sioreg, SIO_REG_DEVID) << 8)
+ | superio_inb(data->sioreg, SIO_REG_DEVID + 1);
switch (val & SIO_ID_MASK) {
- case SIO_W83627DHG_ID:
- w83627ehf_num_in = 9;
- break;
case SIO_W83627EHF_ID:
+ data->kind = w83627ehf;
+ sio_name = sio_name_W83627EHF;
+ break;
case SIO_W83627EHG_ID:
- w83627ehf_num_in = 10;
+ data->kind = w83627ehf;
+ sio_name = sio_name_W83627EHG;
+ break;
+ case SIO_W83627DHG_ID:
+ data->kind = w83627dhg;
+ sio_name = sio_name_W83627DHG;
break;
default:
- printk(KERN_WARNING "w83627ehf: unsupported chip ID: 0x%04x\n",
+ pr_info(DRVNAME ": unsupported chip ID: 0x%04x\n",
val);
- superio_exit();
+ superio_exit(data->sioreg);
return -ENODEV;
}
- superio_select(W83627EHF_LD_HWM);
- val = (superio_inb(SIO_REG_ADDR) << 8)
- | superio_inb(SIO_REG_ADDR + 1);
- *addr = val & IOREGION_ALIGNMENT;
- if (*addr == 0) {
- superio_exit();
+ /* we have known chip find the HWMON base */
+
+ superio_select(data->sioreg, W83627EHF_LD_HWM);
+ val = (superio_inb(data->sioreg, SIO_REG_ADDR) << 8)
+ | superio_inb(data->sioreg, SIO_REG_ADDR + 1);
+ data->addr = val & REGION_ALIGNMENT;
+ if (data->addr == 0) {
+ superio_exit(data->sioreg);
return -ENODEV;
}
/* Activate logical device if needed */
- val = superio_inb(SIO_REG_ENABLE);
+ val = superio_inb(data->sioreg, SIO_REG_ENABLE);
if (!(val & 0x01))
- superio_outb(SIO_REG_ENABLE, val | 0x01);
+ superio_outb(data->sioreg, SIO_REG_ENABLE, val | 0x01);
- superio_exit();
+ superio_exit(data->sioreg);
+ pr_info(DRVNAME ": Found %s chip at %#x\n",
+ sio_name, data->addr);
return 0;
}
+/* when Super-I/O functions move to a separate file, the Super-I/O
+ * bus will manage the lifetime of the device and this module will only keep
+ * track of the w83627ehf driver. But since we platform_device_alloc(), we
+ * must keep track of the device */
+static struct platform_device *pdev;
+
static int __init sensors_w83627ehf_init(void)
{
- if (w83627ehf_find(0x2e, &address)
- && w83627ehf_find(0x4e, &address))
- return -ENODEV;
+ int err;
+ struct resource res;
+ struct w83627ehf_data *data;
+
+ if (!(data = kzalloc(sizeof(struct w83627ehf_data), GFP_KERNEL))) {
+ err = -ENOMEM;
+ goto exit;
+ }
+
+ /* initialize data->kind, data->sioreg, and data->addr.
+ *
+ * when Super-I/O functions move to a separate file, the Super-I/O
+ * driver will probe 0x2e and 0x4e and auto-detect the presence of a
+ * w83627ehf hardware monitor, and call probe() */
+ if (w83627ehf_superio_probe(0x2e, data) &&
+ w83627ehf_superio_probe(0x4e, data)) {
+ err = -ENODEV;
+ goto exit_free;
+ }
- return i2c_isa_add_driver(&w83627ehf_driver);
+ /* !=0 to suppress warning:
+ * suggest parentheses around assignment used as truth value */
+ if ((err = platform_driver_register(&w83627ehf_driver))!=0)
+ goto exit_free;
+
+ if (!(pdev = platform_device_alloc(DRVNAME, data->addr))) {
+ err = -ENOMEM;
+ printk(KERN_ERR DRVNAME ": Device allocation failed\n");
+ goto exit_unregister;
+ }
+
+ pdev->dev.platform_data = data;
+
+ memset(&res, 0, sizeof(res));
+ data->name = w83627ehf_device_names[data->kind];
+ res.name = data->name;
+ res.start = data->addr + REGION_OFFSET;
+ res.end = data->addr + REGION_OFFSET + REGION_LENGTH - 1;
+ res.flags = IORESOURCE_IO;
+ err = platform_device_add_resources(pdev, &res, 1);
+ if (err) {
+ printk(KERN_ERR DRVNAME ": Device resource addition failed "
+ "(%d)\n", err);
+ goto exit_device_put;
+ }
+
+ /* platform_device_add calls probe() */
+ err = platform_device_add(pdev);
+ if (err) {
+ printk(KERN_ERR DRVNAME ": Device addition failed (%d)\n",
+ err);
+ goto exit_device_put;
+ }
+
+ return 0;
+
+exit_device_put:
+ pdev->dev.platform_data = NULL;
+ platform_device_put(pdev);
+exit_unregister:
+ platform_driver_unregister(&w83627ehf_driver);
+exit_free:
+ kfree(data);
+exit:
+ return err;
}
static void __exit sensors_w83627ehf_exit(void)
{
- i2c_isa_del_driver(&w83627ehf_driver);
+ /* when Super-I/O functions move to a separate file, devices
+ * will be unregistered before unloading the module.
+ *
+ * for now, only one device is added in sensors_w83627ehf_init */
+ platform_device_unregister(pdev);
+ platform_driver_unregister(&w83627ehf_driver);
}
MODULE_AUTHOR("Jean Delvare <khali at linux-fr.org>");
--- linux-2.6.22-rc2/drivers/hwmon/Kconfig 2007-05-17 11:20:52.000000000 -0700
+++ linux-2.6.22-rc2/drivers/hwmon/Kconfig 2007-05-17 11:20:50.000000000 -0700
@@ -585,8 +585,7 @@
config SENSORS_W83627EHF
tristate "Winbond W83627EHF/DHG"
- depends on I2C && EXPERIMENTAL
- select I2C_ISA
+ depends on HWMON && EXPERIMENTAL
help
If you say yes here you get support for the hardware
monitoring functionality of the Winbond W83627EHF/DHG Super-I/O chip.
-------------- next part --------------
This patch adds more useful error messages for two error cases.
If the Super-I/O device is disabled, it is likely the BIOS has a good reason
for leaving it disabled, so give a warning when enabling it -- it's not likely
to be wired correctly or be able to give good data.
Also, if the Super-I/O device is configured with an address of 0, the driver
refuses to initialize it.
Signed-off-by: David Hubbard <david.c.hubbard at gmail.com>
Index: linux-2.6.22-rc2/drivers/hwmon/w83627ehf.c
===================================================================
--- linux-2.6.22-rc2/drivers/hwmon/w83627ehf.c 2007-05-19 17:29:21.000000000 -0700
+++ linux-2.6.22-rc2/drivers/hwmon/w83627ehf.c 2007-05-19 17:38:53.000000000 -0700
@@ -1383,14 +1383,19 @@
| superio_inb(data->sioreg, SIO_REG_ADDR + 1);
data->addr = val & REGION_ALIGNMENT;
if (data->addr == 0) {
+ printk(KERN_ERR DRVNAME ": refusing to enable a Super-I/O "
+ "device with a base I/O port 0.\n");
superio_exit(data->sioreg);
return -ENODEV;
}
/* Activate logical device if needed */
val = superio_inb(data->sioreg, SIO_REG_ENABLE);
- if (!(val & 0x01))
+ if (!(val & 0x01)) {
+ printk(KERN_WARNING DRVNAME ": Forcibly enabling Super-I/O. "
+ "Sensor is probably unusable.\n");
superio_outb(data->sioreg, SIO_REG_ENABLE, val | 0x01);
+ }
superio_exit(data->sioreg);
pr_info(DRVNAME ": Found %s chip at %#x\n",
-------------- next part --------------
This patch fixes a bug where the driver outputs
"Increasing fan 4 clock divider from 4 to 8" and fills up the kernel message
log.
Signed-off-by: David Hubbard <david.c.hubbard at gmail.com>
Index: linux-2.6.22-rc2/drivers/hwmon/w83627ehf.c
===================================================================
--- linux-2.6.22-rc2/drivers/hwmon/w83627ehf.c 2007-05-19 17:38:53.000000000 -0700
+++ linux-2.6.22-rc2/drivers/hwmon/w83627ehf.c 2007-05-19 18:12:23.000000000 -0700
@@ -403,7 +403,7 @@
break;
case 4:
reg = (w83627ehf_read_value(data, W83627EHF_REG_DIODE) & 0x73)
- | ((data->fan_div[4] & 0x03) << 3)
+ | ((data->fan_div[4] & 0x03) << 2)
| ((data->fan_div[4] & 0x04) << 5);
w83627ehf_write_value(data, W83627EHF_REG_DIODE, reg);
break;
@@ -468,7 +468,7 @@
&& data->fan_div[i] < 0x07) {
dev_dbg(dev, "Increasing fan %d "
"clock divider from %u to %u\n",
- i, div_from_reg(data->fan_div[i]),
+ i+1, div_from_reg(data->fan_div[i]),
div_from_reg(data->fan_div[i] + 1));
data->fan_div[i]++;
w83627ehf_write_fan_div(data, i);
@@ -1243,11 +1243,10 @@
/* if fan4 is enabled create the sf3 files for it */
if (data->has_fan & (1 << 3))
- for (i = 0; i < ARRAY_SIZE(sda_sf3_arrays_fan4); i++) {
+ for (i = 0; i < ARRAY_SIZE(sda_sf3_arrays_fan4); i++)
if ((err = device_create_file(dev,
- &sda_sf3_arrays_fan4[i].dev_attr)))
+ &sda_sf3_arrays_fan4[i].dev_attr)))
goto exit_remove;
- }
for (i = 0; i < data->in_num; i++)
if ((err = device_create_file(dev, &sda_in_input[i].dev_attr))
