Super-IO and Driver model ISA bus Was: cdev to pdev

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Rene Herman wrote:
Jim Cromie wrote:

**Subject: Driver model ISA bus** http://marc.theaimsgroup.com/?l=linux-kernel&m=114678055902797&w=2

It looks interesting - anything that simplifies stuff is - but
alas - theres no empty-isa-driver.c for me to start from.

Now there is!

thanks.
The .match() method is used to guarantee that a later "bind" (echo -n empty.0 >/sys/bus/isa/driver/empty/bind) could conceivably succeed. If it can not (in this example due to no port value having been passed in) then the method returns a non-match (0) meaning the bus code will unregister the device again. Otherwise, it keeps it registered and the probe() method gets called where all the real work can happen. Note that this should include things like requesting the I/O ports -- even if they are busy now, an unload of another driver could free them, after which a bind could succeed.

You are being passed the "unsigned int id" in all methods, but expected use is for probe() to hang its private data off dev_get_drvdata() as in this example. Your choice...

remove() gets called on device unbind / driver unload. There are also suspend and resume callbacks available. Hope the example is useful. As Greg said though, this code is not currently available in any public kernel yet anyway, so ...

+       pdev = platform_device_register_simple(DRVNAME, 0, NULL, 0);

... if you will be using platform devices instead, please don't use the platform_device_register_simple() API; it's going away. Use:

Done - thanks.

soekris:~# dmesg |grep scx
[   97.619411] scx200: NatSemi SCx200 Driver
[   98.254871] scx200: GPIO base 0x6100
[   98.277567] scx200: Configuration Block base 0x6000
[   98.580221] scx200_gpio: NatSemi SCx200 GPIO Driver init
[   99.333520]  scx200_gpio.0: NatSemi SCx200 GPIO Driver dev_dbg
[   99.485484]  scx200_gpio.0: init complete: 32 pins

I like the indent and the .0, Im just not sure it makes sense in my context.

Rene.


So, if I might follow up..

Gregs wording suggested that platform_dev was the last choice,
and it seems that the scx200 driver has a the pci_device, and could
EXPORT-GPL that structure directly for use by scx200_gpio,
which then just uses it for dev_dbg() and friends.
Could being the operative word. - Should ? is the other.


So, let me assume the answer to the EXPORT Q  is yes,
and try a bait-and-switch ;-)

Attached is a superio-locks module, which helps drivers using a super-IO device to coordinate with other drivers doing the same. Super-IO chips are often on ISA busses, which implies that I should rethink my module, on several fronts:


- I have a 'probe' activity, in find_superio()  /  or maybe get_superio().
   where I find the requested superio ports (from user provided ranges)
   is there a distinction whether probe includes scan, or just test ?

- Ive 'used' it exactly once, in the init-phase of LM-Sensors pc87360
   so its almost entirely untested.
   will add it to scx200 WIP to try.  which may also be __init usage.

- tested against a pc87366 on a soekris net-4801
   chip has 16 functional units selected via LDN register

-how much chance of raciness is there between 2 modules that are initializing,
and which might want to use the same superio while doing so ?
IOW, is it possible to modprobe modules in parallel ?
or is the module fighting imaginary demons ?

-I probably need locking on get_superio.
if theres a race potential, it would be 'caught' by contention on the lock ?

Should a super-IO device have any particular
representation wrt a bus ? a sub-bus ?
place/spot in the /sys hieriarchy ?



begin


diff -ruNp -X dontdiff -X exclude-diffs LM-3/arch/i386/Kconfig LM-4/arch/i386/Kconfig
--- LM-3/arch/i386/Kconfig	2006-05-20 11:58:07.000000000 -0600
+++ LM-4/arch/i386/Kconfig	2006-05-20 23:31:22.000000000 -0600
@@ -1160,3 +1160,14 @@ config X86_TRAMPOLINE
config KTIME_SCALAR
	bool
	default y
+
+config SUPERIO_LOCKS
+	tristate "Super-IO port sharing"
+	depends X86 && EXPERIMENTAL
+	default n
+	# select'd by PC87360 now, others later
+	# name superio_locks # ie: if you build as module, its named ..
+	help
+	  this module used by driver modules which want to coordinate
+	  access via a superio-port, into the multifunction chip its
+	  part of.
diff -ruNp -X dontdiff -X exclude-diffs LM-3/drivers/hwmon/Kconfig LM-4/drivers/hwmon/Kconfig
--- LM-3/drivers/hwmon/Kconfig	2006-05-20 09:11:16.000000000 -0600
+++ LM-4/drivers/hwmon/Kconfig	2006-05-20 23:15:40.000000000 -0600
@@ -309,6 +309,7 @@ config SENSORS_PC87360
	depends on HWMON && I2C && EXPERIMENTAL
	select I2C_ISA
	select HWMON_VID
+	select SUPERIO_LOCKS
	help
	  If you say yes here you get access to the hardware monitoring
	  functions of the National Semiconductor PC8736x Super-I/O chips.
diff -ruNp -X dontdiff -X exclude-diffs LM-3/drivers/hwmon/Makefile LM-4/drivers/hwmon/Makefile
--- LM-3/drivers/hwmon/Makefile	2006-05-20 09:11:16.000000000 -0600
+++ LM-4/drivers/hwmon/Makefile	2006-05-20 23:11:25.000000000 -0600
@@ -47,6 +47,8 @@ obj-$(CONFIG_SENSORS_VT8231)	+= vt8231.o
obj-$(CONFIG_SENSORS_W83627EHF)	+= w83627ehf.o
obj-$(CONFIG_SENSORS_W83L785TS)	+= w83l785ts.o

+obj-$(CONFIG_SUPERIO_LOCKS)	+= superio_locks.o
+
ifeq ($(CONFIG_HWMON_DEBUG_CHIP),y)
EXTRA_CFLAGS += -DDEBUG
endif
diff -ruNp -X dontdiff -X exclude-diffs LM-3/drivers/hwmon/superio_locks.c LM-4/drivers/hwmon/superio_locks.c
--- LM-3/drivers/hwmon/superio_locks.c	1969-12-31 17:00:00.000000000 -0700
+++ LM-4/drivers/hwmon/superio_locks.c	2006-05-20 22:13:12.000000000 -0600
@@ -0,0 +1,126 @@
+#include <linux/config.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/mutex.h>
+#include <linux/superio-locks.h>
+#include <asm/io.h>
+
+#include <linux/slab.h>
+#include <linux/jiffies.h>
+#include <linux/i2c.h>
+#include <linux/i2c-isa.h>
+#include <linux/err.h>
+
+MODULE_AUTHOR("Jim Cromie <jim.cromie@xxxxxxxxx");
+MODULE_LICENSE("GPL");
+
+/**
+   module provides a means for modules to register their use of a
+   Super-IO port, and provides an access-lock for the registering
+   modules to use to coordinate with each other.  Consider it a
+   parking-attendant's key-board.  Design is perhaps ISA centric,
+   maybe formalize this, with (platform|isa)_driver.
+*/
+
+static int max_locks = 3;	/* 1 is enough for 90% uses */
+module_param(max_locks, int, 0);
+MODULE_PARM_DESC(max_locks,
+		 " Number of sio-lock clients to serve (default=3)");
+
+static struct superio *sio_locks;
+static int num_locks;
+
+/* TB-Replaced by something better: platform_driver ? */
+
+#define dprintk(...)	printk(KERN_NOTICE "superio: " __VA_ARGS__)
+
+/**
+   get_superio() checks whether the expected SuperIO device is
+   present at a specific cmd-addr.  Use in loop to scan.
+*/
+struct superio* get_superio(int cmd_addr, int dev_id_addr,
+			    int want_devid)
+{
+	int i, rc, mydevid;
+
+	/* share any already allocated lock for this cmd_addr */
+	for (i = 0; i < num_locks; i++) {
+		if (cmd_addr == sio_locks[i].sioaddr
+		    && want_devid == sio_locks[i].devid)
+		{
+			dprintk("sharing port %x\n", cmd_addr);
+			return &sio_locks[i];
+		}
+	}
+	/* read the device-id-address */
+	outb(dev_id_addr, cmd_addr);
+	mydevid = inb(cmd_addr+1);
+
+	/* but 1st, sanity check the cmd reg */
+	rc = inb(cmd_addr);
+	if (rc != dev_id_addr) {
+		dprintk("superio_cmdaddr %x absent %d\n", cmd_addr, rc);
+		return NULL;
+	}
+	/* test for the desired device id value */
+	if (mydevid != want_devid) {
+		return NULL;
+	}
+
+	if (num_locks >= max_locks) {
+		printk(KERN_ERR "No locks left. increase max_locks param!\n");
+		return NULL;
+	}
+	dprintk("allocating slot %d, addr %x for device %x\n",
+		num_locks, cmd_addr, want_devid);
+
+	mutex_init(&sio_locks[num_locks].lock);
+	sio_locks[num_locks].sioaddr = cmd_addr;
+	sio_locks[num_locks].devid = want_devid;
+
+	return &sio_locks[num_locks++];
+}
+
+/* array args must be null terminated */
+struct superio* find_superio(u8 cmd_addr[], int devid_addr,
+			     u8 want_devid[])
+{
+	int i, j;
+	struct superio* gate;
+
+	for (i = 0; cmd_addr[i]; i++) {
+		for (j = 0; want_devid[j]; j++) {
+			gate = get_superio(cmd_addr[i], devid_addr,
+					   want_devid[j]);
+			if (gate) {
+				dprintk("devid %x found at %x \n",
+					want_devid[j], cmd_addr[i]);
+				return gate;
+			} else
+				dprintk("no devid %x at %x\n",
+					want_devid[j], cmd_addr[i]);
+		}
+	}
+	return NULL;
+}
+
+int superio_locks_init_module(void)
+{
+	dprintk("superio_locks: %d reservations\n", max_locks);
+	sio_locks = kzalloc(max_locks*sizeof(struct superio), GFP_KERNEL);
+	return 0;
+}
+
+EXPORT_SYMBOL(get_superio);
+EXPORT_SYMBOL(find_superio);
+
+module_init(superio_locks_init_module);
+
+/*
+  module_exit(superio_locks_cleanup_module);
+
+  We dont do this, since we'd destroy the locks used by others.
+  Tracking registrants is pointless here, since we just hand out the
+  same struct superio over again.
+*/
diff -ruNp -X dontdiff -X exclude-diffs LM-3/include/linux/superio-locks.h LM-4/include/linux/superio-locks.h
--- LM-3/include/linux/superio-locks.h	1969-12-31 17:00:00.000000000 -0700
+++ LM-4/include/linux/superio-locks.h	2006-05-20 22:13:12.000000000 -0600
@@ -0,0 +1,17 @@
+#include <linux/mutex.h>
+
+/* Super-IO ports are found in low-pin-count hardware (typically ISA,
+   any others ?), and can hide 15 functional units, requiring up to 15
+   drivers to share the superio.  This struct lets us share them, in
+   hwmon/superio_locks.c
+*/
+struct superio {
+	struct mutex lock;	/* lock shared amongst user drivers */
+	int sioaddr;		/* port's tested cmd-address */
+	int devid;		/* devid found by the registering driver */
+};
+
+struct superio* get_superio(int sioaddr, int devid_addr, int devid_val);
+struct superio* find_superio(u8 sioaddr[],
+			     int devid_addr, u8 devid_val[]);
+








--
Kernelnewbies: Help each other learn about the Linux kernel.
Archive:       http://mail.nl.linux.org/kernelnewbies/
FAQ:           http://kernelnewbies.org/faq/


[Index of Archives]     [Newbies FAQ]     [Linux Kernel Mentors]     [Linux Kernel Development]     [IETF Annouce]     [Git]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux SCSI]     [Linux ACPI]
  Powered by Linux