lm78: No longer use i2c-isa

Reimplement the ISA device support as a platform driver, so that we no longer rely on i2c-isa.

---
 drivers/hwmon/Kconfig |    1 -
 drivers/hwmon/lm78.c  |  377 +++++++++++++++++++++++++++++++++++--------------
 2 files changed, 267 insertions(+), 111 deletions(-)

@@ -291,7 +291,6 @@ config SENSORS_LM77
 config SENSORS_LM78
 	tristate "National Semiconductor LM78 and compatibles"
 	depends on HWMON && I2C
-	select I2C_ISA
 	select HWMON_VID
 	help
 	  If you say yes here you get support for National Semiconductor LM78,

return data;
}

+/* return 1 if a supported chip is found, 0 otherwise */ +static int __init lm78_isa_found(unsigned short address) +{
+ int val, save, found = 0;
+
+ if (!request_region(address, LM78_EXTENT, "lm78"))
+ return 0;
+
+#define REALLY_SLOW_IO
+ /* We need the timeouts for at least some LM78-like
+ chips. But only if we read 'undefined' registers. */
+ val = inb_p(address + 1);
+ if (inb_p(address + 2) != val

+	 || inb_p(address + 3) != val
+	 || inb_p(address + 7) != val)

+ goto release;
+#undef REALLY_SLOW_IO
+
+ /* We should be able to change the 7 LSB of the address port. The
+ MSB (busy flag) should be clear initially, set after the write. */
+ save = inb_p(address + LM78_ADDR_REG_OFFSET);
+ if (save & 0x80)
+ goto release;
+ val = ~save & 0x7f;
+ outb_p(val, address + LM78_ADDR_REG_OFFSET);
+ if (inb_p(address + LM78_ADDR_REG_OFFSET) != (val | 0x80)) {
+ outb_p(save, address + LM78_ADDR_REG_OFFSET);
+ goto release;
+ }
+
+ /* We found a device, now see if it could be an LM78 */
+ outb_p(LM78_REG_CONFIG, address + LM78_ADDR_REG_OFFSET);
+ val = inb_p(address + LM78_DATA_REG_OFFSET);
+ if (val & 0x80)
+ goto release;
+ outb_p(LM78_REG_I2C_ADDR, address + LM78_ADDR_REG_OFFSET);
+ val = inb_p(address + LM78_DATA_REG_OFFSET);
+ if (val < 0x03 || val > 0x77) /* Not a valid I2C address */
+ goto release;
+
+ /* The busy flag should be clear again */
+ if (inb_p(address + LM78_ADDR_REG_OFFSET) & 0x80)
+ goto release;
+
+ /* Explicitly prevent the misdetection of Winbond chips */
+ outb_p(0x4f, address + LM78_ADDR_REG_OFFSET);
+ val = inb_p(address + LM78_DATA_REG_OFFSET);
+ if (val = 0xa3 || val = 0x5c)
+ goto release;
+
+ /* Explicitly prevent the misdetection of ITE chips */
+ outb_p(0x58, address + LM78_ADDR_REG_OFFSET);
+ val = inb_p(address + LM78_DATA_REG_OFFSET);
+ if (val = 0x90)
+ goto release;
+
+ /* Determine the chip type */
+ outb_p(LM78_REG_CHIPID, address + LM78_ADDR_REG_OFFSET);
+ val = inb_p(address + LM78_DATA_REG_OFFSET);
+ if (val = 0x00 /* LM78 */

+	 || val = 0x40			/* LM78-J */
+	 || (val & 0xfe) = 0xc0)	/* LM79 */

+ found = 1;
+
+ if (found)
+ pr_info("lm78: Found an %s chip at %#x\n",
+ val & 0x80 ? "LM79" : "LM78", (int)address);
+
+ release:
+ release_region(address, LM78_EXTENT);
+ return found;
+}
+
+static int __init lm78_isa_device_add(unsigned short address) +{
+ struct resource res = {
+ .start = address,
+ .end = address + LM78_EXTENT,
+ .name = "lm78",
+ .flags = IORESOURCE_IO,
+ };
+ int err;
+
+ pdev = platform_device_alloc("lm78", address);
+ if (!pdev) {
+ err = -ENOMEM;
+ printk(KERN_ERR "lm78: Device allocation failed\n");
+ goto exit;
+ }
+
+ err = platform_device_add_resources(pdev, &res, 1);
+ if (err) {
+ printk(KERN_ERR "lm78: Device resource addition failed "
+ "(%d)\n", err);
+ goto exit_device_put;
+ }
+
+ err = platform_device_add(pdev);
+ if (err) {
+ printk(KERN_ERR "lm78: Device addition failed (%d)\n",
+ err);
+ goto exit_device_put;
+ }
+
+ return 0;
+
+ exit_device_put:
+ platform_device_put(pdev);
+ exit:
+ pdev = NULL;
+ return err;
+}
+
static int __init sm_lm78_init(void)
{
int res;

res = i2c_add_driver(&lm78_driver);
if (res)
- return res;
+ goto exit;

- /* Don't exit if this one fails, we still want the I2C variants
- to work! */
- if (i2c_isa_add_driver(&lm78_isa_driver))
- isa_address = 0;
+ if (lm78_isa_found(isa_address)) {
+ res = platform_driver_register(&lm78_isa_driver);
+ if (res)
+ goto exit_unreg_i2c_driver;
+
+ /* Sets global pdev as a side effect */
+ res = lm78_isa_device_add(isa_address);
+ if (res)
+ goto exit_unreg_isa_driver;
+ }

return 0;
+
+ exit_unreg_isa_driver:
+ platform_driver_unregister(&lm78_isa_driver);
+ exit_unreg_i2c_driver:
+ i2c_del_driver(&lm78_driver);
+ exit:
+ return res;
}

static void __exit sm_lm78_exit(void)
{
- if (isa_address)
- i2c_isa_del_driver(&lm78_isa_driver);
+ if (pdev) {
+ platform_device_unregister(pdev);
+ platform_driver_unregister(&lm78_isa_driver);
+ }
i2c_del_driver(&lm78_driver);
}

Jean Delvare

Hi Jean, all,

I see that you're using a platform here not a isa driver. There was mention of maybe mentioning the isa driver structure/class in the future does this mean that the plan is to keep using platfrom driver for isa sensor chips?

Regards,

Hans

p.s.

I'm very interested in the work as the PC8374L chip for which I'm planning to write a driver can be accessed through both isa and i2c too.

I notice that you always register the i2c driver in case of the lm78, for the PC8374L I was thinking about only registering then i2c driver if ISA access is disabled. ISA access can be disabled through the superio config registers, in this case the i2c address can still be read from those superio config registers and one can try to fallback to i2c access. What do you think is best?

Hi Hans,

I have been considering an isa driver, but fell back to a platform driver for two reasons:

1* libsensors would most probably need another update to support yet another type of hardware monitoring driver. While this could be done, it would delay my plan to kill i2c-isa by several months, which isn't really acceptable.

2* isa drivers are only available on some architectures, as the driver core code for them depends on CONFIG_ISA which cannot be selected on some architectures. While I suppose that LM78 chips connected on the ISA bus are only found on x86 systems, LM78 chips connected to the I2C bus could be found on virtually any system. So, in order to use an isa driver rather than a platform driver, I would first have had to either clutter the driver with #ifdef-CONFIG_ISAs to let other architectures use the I2C part of the driver, or split the driver in two or three modules. Again, this was not impossible, but it would have taken more time than I have available, and more time than I think is worth spending on an otherwise very old driver.

So yes, I admit that going with a platform driver is the easy way and not necessarily the best absolute technical solution. That being said, I seem to understand that CONFIG_ISA=y means "the system has ISA slots", which suggests that isa drivers should only be used for legacy ISA daughter boards.

Contrary to the LM78 / W83781D, which were dedicated hardware monitoring chips, the PC8374L is a Super-I/O, so it is very unlikely to be ever found on a daughter board (while for example the W83781D was used on graphics adapters). So it is rather unlikely that the ISA (actually LPC) access will not be available. If I were you, I'd write an ISA/LPC-only driver to start with, and only think of adding I2C support later if really needed. As a matter of fact, the Winbond W83627HF and ITE IT8712F have their hardware monitoring features exposed on both the LPC bus and the SMBus, and we ended up dropping SMBus access support entirely from the it87 driver. Dropping support for the W83627HF SMBus access is somewhere on my TODO list, too.

BTW, the lm78 and w83781d drivers cannot safely disable the I2C access support when a device is found on the ISA bus. Think of the case where I plug a graphics adapter with a W83781D chip in a motherboard which itself has a W83781D chip.

Jean Delvare

Well unfortunately the person who brought PC8374L support up on the list and who is willing todo the testing for me has an intel motherboard and intel loves smbus, iow the hwmon logical device is disabled through the superio config registers, this doesn't actually disable the device, but does disable isa access, so I need to go i2c there, now I could force the logical device to on (assuming the config isn't locked by the bios) but that might interfere with any ACPI / other bios code accessing it (which doesn't seem to be the case but could be) and would require assigning ioports to it. To make things even more interesting the chip can be configured for memorymapped access too, now mmap access would be ideal, but as said I don't like the idea of changing the superio config. Thus the plan is to write a driver with supports all 3, and decides which one to use based on the superio config. This is actually easier then it sounds, because the only code that needs to take this into account is the probe code and the writebyte / readbyte functions.

I'm also thinking about adding a force ioport / mmap io options, so that those modes can be actually tested.

Any input/advice on this is much appreciated.

Ah I see.

Regards,

Hans

Your plan sounds good.

Jean Delvare