Skip to contentSkip to conversation

support for Cobalt Networks (x86 only) systems (for real this time)

PATCH In review Tim Hockin 3 participants 3 messages Jun 1, 2001

lore
Share & tools
lore

Tim Hockin

author
#0 lore raw

PATCH 1/1 support for Cobalt Networks (x86 only) systems (for real this time)

apparently, LKML silently (!) bounces messages > a certain size. So I'll try smaller patches. This is part 1/2 of the general Cobalt support.

Alan,

Aattached is a (large, but self contained) patch for Cobalt Networks suport for x86 systems (RaQ3, RaQ4, Qube3, RaQXTR). Please let me know if there is anything that would prevent this from general inclusion in the next release.

(patch against 2.4.5)

Thanks

Tim

Signature · 4 lines
Tim Hockin
Systems Software Engineer
Sun Microsystems, Cobalt Server Appliances
[email protected]
drivers/cobalt/acpi.c +218 -0
diff -ruN dist-2.4.5/drivers/cobalt/acpi.c cobalt-2.4.5/drivers/cobalt/acpi.c
@@ -0,0 +1,218 @@
+/* 
+ * cobalt acpi driver 
+ * Copyright (c) 2000, Cobalt Networks, Inc.
+ * Copyright (c) 2001, Sun Microsystems, Inc.
+ * $Id: acpi.c,v 1.10 2001/05/30 07:19:47 thockin Exp $
+ *
+ * author: [email protected], [email protected]
+ *
+ * this driver just sets stuff up for ACPI interrupts
+ *
+ * if acpi support really existed in the kernel, we would read
+ * data from the ACPI tables. however, it doesn't. as a result,
+ * we use some hardcoded values. 
+ *
+ * This should be SMP safe.  The only data that needs protection is the acpi
+ * handler list.  It gets scanned at timer-interrupts, must use
+ * irqsave/restore locks. Read/write locks would be useful if there were any
+ * other times that the list was read but never written. --TPH
+ */
+
+#include <stdarg.h>
+#include <stddef.h>
+#include <linux/init.h>
+#include <linux/sched.h>
+#include <linux/config.h>
+#include <linux/pci.h>
+#include <linux/proc_fs.h>
+#include <linux/sched.h>
+#include <linux/ioport.h>
+#include <linux/delay.h>
+#include <linux/spinlock.h>
+#include <asm/io.h>
+#include <asm/irq.h>
+#include <linux/cobalt.h>
+#include <linux/cobalt-systype.h>
+#include <linux/cobalt-acpi.h>
+#include <linux/cobalt-superio.h>
+
+#define POWER_BUTTON_SHUTDOWN     0
+
+#define ACPI_IRQ	10       /* XXX: hardcoded interrupt */
+#define ACPI_NAME	"sci"
+#define ACPI_MAGIC	0xc0b7ac21
+
+#define SUPERIO_EVENT	0xff
+#define OSB4_EVENT	0x40
+#define OSB4_INDEX_PORT	0x0cd6
+#define OSB4_DATA_PORT	0x0cd7
+
+/* for registering ACPI handlers */
+struct acpi_handler {
+	void (*function)(int irq, void *dev_id, struct pt_regs *regs);
+	struct acpi_handler *next;
+	struct acpi_handler *prev;
+};
+struct acpi_handler *acpi_handler_list;
+
+static spinlock_t acpi_lock = SPIN_LOCK_UNLOCKED;
+/* these two are for gen V */
+static u16 osb4_port;
+static u16 superio_port;
+
+static u16 
+get_reg(u16 index, u16 data, u8 port)
+{
+	if (cobt_is_5k()) {
+		u16 reg;
+
+		outb(port, index);
+		reg = inb(data);
+		outb(port + 1, index);
+		reg |= inb(data) << 8;
+		return reg;
+	}
+
+	return 0;
+}
+
+static void 
+acpi_interrupt(int irq, void *dev_id, struct pt_regs *regs)
+{
+	unsigned long flags, events;
+	struct acpi_handler *p;
+
+	spin_lock_irqsave(&acpi_lock, flags);
+
+	if (cobt_is_5k()) {
+		/* save the superio events */
+		events = inb(superio_port) | (inb(superio_port + 1) << 8);
+	
+		/* clear SCI interrupt generation */
+		outb(OSB4_EVENT, osb4_port); 
+		outb(SUPERIO_EVENT, superio_port);
+		outb(SUPERIO_EVENT, superio_port + 1);
+	}
+
+	/* call the ACPI handlers */
+	p = acpi_handler_list;
+	while (p) {
+		p->function(irq, dev_id, regs);
+		p = p->next;
+	}
+
+	spin_unlock_irqrestore(&acpi_lock, flags);
+}
+
+int
+cobalt_acpi_register_handler(void (*function)(int, void *, struct pt_regs *))
+{
+	struct acpi_handler *newh;
+	unsigned long flags;
+
+	newh = kmalloc(sizeof(*newh), GFP_ATOMIC);
+	if (!newh) {
+		EPRINTK("can't allocate memory for handler %p\n", function);
+		return -1;
+	}
+
+	spin_lock_irqsave(&acpi_lock, flags);
+
+	/* head insert */
+	newh->function = function;
+	newh->next = acpi_handler_list;
+	newh->prev = NULL;
+	if (acpi_handler_list) {
+		acpi_handler_list->prev = newh;
+	}
+	acpi_handler_list = newh;	
+
+	spin_unlock_irqrestore(&acpi_lock, flags);
+
+	return 0;
+}
+
+int
+cobalt_acpi_unregister_handler(void (*function)(int, void *, struct pt_regs *))
+{
+	struct acpi_handler *p;
+	unsigned long flags;
+	int r = -1;
+
+	spin_lock_irqsave(&acpi_lock, flags);
+
+	p = acpi_handler_list;
+	while (p) {
+		if (p->function == function) {
+			if (p->prev) {
+				p->prev->next = p->next;
+			}
+			if (p->next) {
+				p->next->prev = p->prev;
+			}
+			r = 0;
+			break;
+		}
+		p = p->next;
+	}
+
+	spin_unlock_irqrestore(&acpi_lock, flags);
+
+	return r;
+}
+
+int __init 
+cobalt_acpi_init(void)
+{
+	int err, reg;
+	u16 addr;
+	unsigned long flags;
+
+	if (cobt_is_5k()) {
+		/* setup osb4 i/o regions */
+		if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x20)))
+			request_region(reg, 4, "OSB4 (pm1a_evt_blk)");
+		if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x22)))
+			request_region(reg, 2, "OSB4 (pm1a_cnt_blk)");
+		if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x24)))
+			request_region(reg, 4, "OSB4 (pm_tmr_blk)");
+		if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x26)))
+			request_region(reg, 6, "OSB4 (p_cnt_blk)");
+		if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x28)))
+			request_region(reg, 8, "OSB4 (gpe0_blk)");
+		osb4_port = reg;
+
+		spin_lock_irqsave(&cobalt_superio_lock, flags);
+	
+		/* superi/o -- select pm logical device and get base address */
+		outb(SUPERIO_LOGICAL_DEV, SUPERIO_INDEX_PORT);
+		outb(SUPERIO_DEV_PM, SUPERIO_DATA_PORT);
+		outb(SUPERIO_ADDR_HIGH, SUPERIO_INDEX_PORT);
+		addr = inb(SUPERIO_DATA_PORT) << 8;
+		outb(SUPERIO_ADDR_LOW, SUPERIO_INDEX_PORT);
+		addr |= inb(SUPERIO_DATA_PORT); 
+		if (addr) {
+			/* get registers */
+			if ((reg = get_reg(addr, addr + 1, 0x08))) {
+				request_region(reg, 4, "SUPERIO (pm1_evt_blk)");
+				superio_port = reg;
+			}
+			if ((reg = get_reg(addr, addr + 1, 0x0c))) 
+				request_region(reg, 2, "SUPERIO (pm1_cnt_blk)");
+			if ((reg = get_reg(addr, addr + 1, 0x0e)))
+				request_region(reg, 16, "SUPERIO (gpe_blk)");
+		}
+	
+		spin_unlock_irqrestore(&cobalt_superio_lock, flags);
+	
+		/* setup an interrupt handler for an ACPI SCI */
+		err = request_irq(ACPI_IRQ, acpi_interrupt, 
+			  SA_SHIRQ, ACPI_NAME, (void *)ACPI_MAGIC);
+		if (err) {
+			EPRINTK("couldn't assign ACPI IRQ (%d)\n", ACPI_IRQ);
+			return -1;
+		}
+	}
+
+	return 0;
+}
drivers/cobalt/Config.in +23 -0
diff -ruN dist-2.4.5/drivers/cobalt/Config.in cobalt-2.4.5/drivers/cobalt/Config.in
@@ -0,0 +1,23 @@
+#
+# Cobalt Networks hardware support
+#
+mainmenu_option next_comment
+comment 'Cobalt Networks support'
+bool 'Support for Cobalt Networks x86 servers' CONFIG_COBALT
+
+if [ "$CONFIG_COBALT" != "n" ]; then
+   bool 'Gen III (3000 series) system support' CONFIG_COBALT_GEN_III
+   bool 'Gen V (5000 series) system support' CONFIG_COBALT_GEN_V
+   bool 'Create legacy /proc files' CONFIG_COBALT_OLDPROC
+
+   comment 'Cobalt hardware options'
+
+   bool 'Front panel (LCD) support' CONFIG_COBALT_LCD
+   bool 'Software controlled LED support' CONFIG_COBALT_LED
+   bool 'Serial number support' CONFIG_COBALT_SERNUM
+   bool 'Watchdog timer support' CONFIG_COBALT_WDT
+   bool 'Thermal sensor support' CONFIG_COBALT_THERMAL
+   bool 'Fan tachometer support' CONFIG_COBALT_FANS
+   bool 'Disk drive ruler support' CONFIG_COBALT_RULER
+fi
+endmenu
drivers/cobalt/fans.c +159 -0

159 lines not on this page · raw message

drivers/cobalt/i2c.c +402 -0

402 lines not on this page · raw message

drivers/cobalt/init.c +79 -0

79 lines not on this page · raw message

drivers/cobalt/lcd.c +645 -0

645 lines not on this page · raw message

drivers/cobalt/led.c +447 -0

447 lines not on this page · raw message

drivers/cobalt/Makefile +19 -0
diff -ruN dist-2.4.5/drivers/cobalt/Makefile cobalt-2.4.5/drivers/cobalt/Makefile
@@ -0,0 +1,19 @@
+#
+# Makefile for the Sun/Cobalt device drivers
+#
+
+O_TARGET := cobalt.o
+
+export-objs	:= init.o i2c.o lcd.o led.o systype.o serialnum.o wdt.o \
+		   thermal.o fans.o
+
+obj-$(CONFIG_COBALT)		+= init.o systype.o i2c.o acpi.o net.o
+obj-$(CONFIG_COBALT_SERNUM)	+= serialnum.o
+obj-$(CONFIG_COBALT_LCD)	+= lcd.o
+obj-$(CONFIG_COBALT_LED)	+= led.o
+obj-$(CONFIG_COBALT_WDT)	+= wdt.o
+obj-$(CONFIG_COBALT_THERMAL)	+= thermal.o
+obj-$(CONFIG_COBALT_FANS)	+= fans.o
+obj-$(CONFIG_COBALT_RULER)	+= ruler.o
+
+include $(TOPDIR)/Rules.make
drivers/cobalt/net.c +128 -0

128 lines not on this page · raw message

#1 lore raw
Quoted 30 lines · Tim Hockin
drivers/cobalt/acpi.c +30 -0
+int __init
+cobalt_acpi_init(void)
+{
+       int err, reg;
+       u16 addr;
+       unsigned long flags;
+
+       if (cobt_is_5k()) {
+               /* setup osb4 i/o regions */
+               if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x20)))
+                       request_region(reg, 4, "OSB4 (pm1a_evt_blk)");
+               if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x22)))
+                       request_region(reg, 2, "OSB4 (pm1a_cnt_blk)");
+               if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x24)))
+                       request_region(reg, 4, "OSB4 (pm_tmr_blk)");
+               if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x26)))
+                       request_region(reg, 6, "OSB4 (p_cnt_blk)");
+               if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x28)))
+                       request_region(reg, 8, "OSB4 (gpe0_blk)");
+               osb4_port = reg;
+
+               spin_lock_irqsave(&cobalt_superio_lock, flags);
+
+               /* superi/o -- select pm logical device and get base address */
+               outb(SUPERIO_LOGICAL_DEV, SUPERIO_INDEX_PORT);
+               outb(SUPERIO_DEV_PM, SUPERIO_DATA_PORT);
+               outb(SUPERIO_ADDR_HIGH, SUPERIO_INDEX_PORT);
+               addr = inb(SUPERIO_DATA_PORT) << 8;
+               outb(SUPERIO_ADDR_LOW, SUPERIO_INDEX_PORT);
+               addr |= inb(SUPERIO_DATA_PORT);

drivers/cobalt/acpi.c line 203

Quoted 10 lines
drivers/cobalt/acpi.c +10 -0
+               if (addr) {
+                       /* get registers */
+                       if ((reg = get_reg(addr, addr + 1, 0x08))) {
+                               request_region(reg, 4, "SUPERIO (pm1_evt_blk)");
+                               superio_port = reg;
+                       }
+                       if ((reg = get_reg(addr, addr + 1, 0x0c)))
+                               request_region(reg, 2, "SUPERIO (pm1_cnt_blk)");
+                       if ((reg = get_reg(addr, addr + 1, 0x0e)))
+                               request_region(reg, 16, "SUPERIO (gpe_blk)");

need to check for request_region failure. since you have a lot of request_region calls, you may want to consider breaking them out into a separate function which returns success or failure, and handles details of cleaning up after 4 request_regions succeed but the 5th one fails.

	if (!request_region(region1))
		goto out;
	if (!request_region(region2))
		goto cleanup_1;
	if (!request_region(region3))
		goto cleanup_2;
	return 0;

cleanup_2:

	release_region(region2);
cleanup_1:
	release_region(region1);
out:
	return -EBUSY;

drivers/cobalt/acpi.c line 214

Quoted 7 lines
drivers/cobalt/acpi.c +7 -0
+               /* setup an interrupt handler for an ACPI SCI */
+               err = request_irq(ACPI_IRQ, acpi_interrupt,
+                         SA_SHIRQ, ACPI_NAME, (void *)ACPI_MAGIC);
+               if (err) {
+                       EPRINTK("couldn't assign ACPI IRQ (%d)\n", ACPI_IRQ);
+                       return -1;
+               }

return 'err' not -1 here?

Quoted 7 lines
drivers/cobalt/Config.in +7 -0
+bool 'Support for Cobalt Networks x86 servers' CONFIG_COBALT
+
+if [ "$CONFIG_COBALT" != "n" ]; then
+   bool 'Gen III (3000 series) system support' CONFIG_COBALT_GEN_III
+   bool 'Gen V (5000 series) system support' CONFIG_COBALT_GEN_V
+   bool 'Create legacy /proc files' CONFIG_COBALT_OLDPROC
+

drivers/cobalt/Config.in line 22

Quoted 10 lines
drivers/cobalt/Config.in +10 -0
+   comment 'Cobalt hardware options'
+
+   bool 'Front panel (LCD) support' CONFIG_COBALT_LCD
+   bool 'Software controlled LED support' CONFIG_COBALT_LED
+   bool 'Serial number support' CONFIG_COBALT_SERNUM
+   bool 'Watchdog timer support' CONFIG_COBALT_WDT
+   bool 'Thermal sensor support' CONFIG_COBALT_THERMAL
+   bool 'Fan tachometer support' CONFIG_COBALT_FANS
+   bool 'Disk drive ruler support' CONFIG_COBALT_RULER
+fi

Style: s/bool '/bool ' /

Quoted 9 lines
drivers/cobalt/fans.c +9 -0
+#ifdef CONFIG_PROC_FS
+#ifdef CONFIG_COBALT_OLDPROC
+       proc_faninfo = create_proc_read_entry("faninfo", S_IFREG | S_IRUGO,
+               NULL, fan_read_proc, NULL);
+        if (!proc_faninfo) {
+               EPRINTK("can't create /proc/faninfo\n");
+       }
+#endif
+       proc_cfaninfo = create_proc_read_entry("faninfo", S_IFREG | S_IRUGO,

drivers/cobalt/fans.c line 56

Quoted 10 lines
drivers/cobalt/fans.c +10 -0
+               proc_cobalt, fan_read_proc, NULL);
+        if (!proc_cfaninfo) {
+               EPRINTK("can't create /proc/cobalt/faninfo\n");
+       }
+#endif
+
+       printk("Cobalt Networks fan tachometers v1.2\n");
+
+       return 0;
+}

S_IFREG|S_IRUGO is the default, so simply pass zero mode for this behavior.

But, each time a user cats this proc file, the user is banging the hardware. What happens when a malicious user forks off 100 processes to continually cat this file? :)

drivers/cobalt/lcd.c line 93

Quoted 6 lines
drivers/cobalt/lcd.c +6 -0
+/* various file operations we support for this driver */
+static struct file_operations lcd_fops = {
+       read:   cobalt_lcd_read,
+       ioctl:  cobalt_lcd_ioctl,
+       open:   cobalt_lcd_open,
+};

Needs owner field assigned a value.

Quoted 6 lines
drivers/cobalt/lcd.c +6 -0
+static int
+cobalt_lcd_ioctl(struct inode *inode, struct file *file,
+                           unsigned int cmd, unsigned long arg)
+{
+       struct lcd_display button_display, display;
+       unsigned long address, a;

drivers/cobalt/lcd.c line 295

Quoted 10 lines
drivers/cobalt/lcd.c +10 -0
+       int index;
+       int dlen = sizeof(struct lcd_display);
+
+       spin_lock(&lcd_lock);
+
+       switch (cmd) {
+       /* Turn the LCD on */
+       case LCD_On:
+               lcddev_write_inst(0x0F);
+               break;

Security: don't you want CAP_RAW_IO or something before executing any of these ioctls?

Quoted 5 lines
drivers/cobalt/lcd.c +5 -0
+       /* Read what's on the LCD */
+        case LCD_Read:
+                for (address = DD_R00; address <= DD_R01; address++) {
+                        lcddev_write_inst(address | LCD_Addr);
+                        display.line1[address] = lcddev_read_data();

drivers/cobalt/lcd.c line 414

Quoted 10 lines
drivers/cobalt/lcd.c +10 -0
+                }
+                display.line1[DD_R01] = '\0';
+
+                for (address = DD_R10; address <= DD_R11; address++) {
+                        lcddev_write_inst(address | LCD_Addr);
+                        display.line2[address - DD_R10] = lcddev_read_data();
+               }
+                display.line2[DD_R01] = '\0';
+
+                copy_to_user((struct lcd_display *)arg, &display, dlen);

unchecked copy_to_user. should be:

	if (copy_to_user(...))
		return -EFAULT;

drivers/cobalt/lcd.c line 423

Quoted 7 lines
drivers/cobalt/lcd.c +7 -0
+       case LCD_Raw_Inst:
+                copy_from_user(&display, (struct lcd_display *)arg, dlen);
+                lcddev_write_inst(display.character);
+                break;
+
+        case LCD_Raw_Data:
+                copy_from_user(&display, (struct lcd_display *)arg, dlen);

ditto. other unchecked copy_from_user and put_user cases snipped.

drivers/cobalt/lcd.c line 475

Quoted 10 lines
drivers/cobalt/lcd.c +10 -0
+       /* clear an led */
+       case LED_Bit_Clear:
+                copy_from_user(&display, (struct lcd_display *)arg, dlen);
+                cobalt_led_set_lazy(cobalt_led_get() & ~display.leds);
+               break;
+
+       default:
+       }
+
+       spin_unlock(&lcd_lock);

bug: you can't hold a spinlock while you are sleeping in copy_from_user

Quoted 13 lines
drivers/cobalt/lcd.c +13 -0
+/* LED daemon sits on this, we wake it up once a key is pressed */
+static ssize_t
+cobalt_lcd_read(struct file *file, char *buf, size_t count, loff_t *ppos)
+{
+       int buttons_now;
+       static atomic_t lcd_waiters = ATOMIC_INIT(0);
+
+       if (atomic_read(&lcd_waiters) > 0)
+               return -EINVAL;
+       atomic_inc(&lcd_waiters);
+
+       while (((buttons_now = button_pressed()) == 0) &&
+              !(signal_pending(current)))

drivers/cobalt/lcd.c line 511

Quoted 10 lines
drivers/cobalt/lcd.c +10 -0
+       {
+               current->state = TASK_INTERRUPTIBLE;
+               schedule_timeout((2 * HZ));
+       }
+       atomic_dec(&lcd_waiters);
+
+       if (signal_pending(current))
+               return -ERESTARTSYS;
+
+       return buttons_now;

why not use a semaphore here?
also, you do not have O_NONBLOCK.

Quoted 5 lines
drivers/cobalt/led.c +5 -0
+int
+cobalt_fpled_register(unsigned int (*function)(void *), void *data)
+{
+       int r = -1;
+

drivers/cobalt/led.c line 339

Quoted 10 lines
drivers/cobalt/led.c +10 -0
+       if (cobt_is_5k()) {
+               struct led_handler *newh;
+               unsigned long flags;
+
+               newh = kmalloc(sizeof(*newh), GFP_ATOMIC);
+               if (!newh) {
+                       EPRINTK("can't allocate memory for handler %p(%p)\n",
+                               function, data);
+                       return -1;
+               }

maybe I am misunderstanding the context in which this code is called. why not GFP_KERNEL?

Quoted 16 lines
drivers/cobalt/net.c +16 -0
+#define MAX_COBT_NETDEVS       2
+static struct net_device *netdevs[MAX_COBT_NETDEVS];
+static int n_netdevs;
+static spinlock_t cobaltnet_lock = SPIN_LOCK_UNLOCKED;
+
+#if defined(CONFIG_COBALT_LED)
+static unsigned int
+net_led_handler(void *data)
+{
+       int i;
+       unsigned int leds = 0;
+       static int txrxmap[MAX_COBT_NETDEVS] = {LED_ETH0_TXRX, LED_ETH1_TXRX};
+       static int linkmap[MAX_COBT_NETDEVS] = {LED_ETH0_LINK, LED_ETH1_LINK};
+       unsigned long flags;
+       static unsigned long net_old[MAX_COBT_NETDEVS];
+

drivers/cobalt/net.c line 49

Quoted 10 lines
drivers/cobalt/net.c +10 -0
+       spin_lock_irqsave(&cobaltnet_lock, flags);
+
+       for (i = 0; i < n_netdevs; i++) {
+               unsigned long txrxstate;
+               struct net_device *dev = netdevs[i];
+               if (!dev) {
+                       continue;
+               }
+               /* check for link */
+               if (dev->link_check && dev->link_check(dev)) {

Obviously dev->link_check is a new and nonstandard addition.

You should use: netif_carrier_{on,off,ok}

Quoted 17 lines
drivers/cobalt/net.c +17 -0
+void
+cobalt_net_register(struct net_device *ndev)
+{
+       unsigned long flags;
+       int i;
+
+        if (!ndev) {
+              return;
+       }
+
+       /* we'll track the first MAX_COBT_NETDEVS NICs */
+       if (n_netdevs >= MAX_COBT_NETDEVS) {
+              return;
+       }
+
+       spin_lock_irqsave(&cobaltnet_lock, flags);
+

drivers/cobalt/net.c line 95

Quoted 10 lines
drivers/cobalt/net.c +10 -0
+       /* find a free slot */
+       for (i = 0; i < n_netdevs; i++) {
+               if (!netdevs[i]) {
+                       netdevs[i] = ndev;
+                       n_netdevs++;
+               }
+       }
+
+       spin_unlock_irqrestore(&cobaltnet_lock, flags);
+}

Are you sure this is needed?

Each Cobalt NIC should store its struct net_device instance in struct pci_dev::private_data, using pci_{get,set}_drvdata wrapper.

So, the system already store a list of Cobalt net devices for you. Just search the pci_drivers list for all members whose pci_dev::driver value == &cobalt_net_pci_driver.

This of course assumes all the net devices are PCI, and that the net device uses current (a.k.a. "new style") PCI driver init.

It is heavily encouraged to convert your PCI drivers to struct pci_driver, because in 2.5 this will become a more generic struct driver, and struct pci_dev will become a more generic struct device, to be used by all kernel drivers.

Signature · 3 lines
Jeff Garzik      | Disbelief, that's why you fail.
Building 1024    |
MandrakeSoft     |
#2 lore raw

Re: [PATCH] support for Cobalt Networks (x86 only) systems (for real this

In reply to Tim Hockin

drivers/cobalt/acpi.c line 174

Quoted 3 lines
drivers/cobalt/acpi.c +3 -0
+		/* setup osb4 i/o regions */
+		if ((reg = get_reg(OSB4_INDEX_PORT, OSB4_DATA_PORT, 0x20)))
+			request_region(reg, 4, "OSB4 (pm1a_evt_blk)");

Check request_region worked

drivers/cobalt/i2c.c line 110

Quoted 2 lines
drivers/cobalt/i2c.c +2 -0
+static int 
+i2c_wait_for_smi(void)

Obvious question - why duplicate the i2c layer

drivers/cobalt/lcd.c line 97

Quoted 3 lines
drivers/cobalt/lcd.c +3 -0
+/* device structure */
+static struct miscdevice lcd_dev = {
+	COBALT_LCD_MINOR,

Is this an officially allocated minor ?

drivers/cobalt/lcd.c line 346

Quoted 5 lines
drivers/cobalt/lcd.c +5 -0
+	/* Get the current cursor position */
+        case LCD_Get_Cursor_Pos:
+                display.cursor_address = lcddev_read_inst();
+                display.cursor_address = display.cursor_address & 0x07F;
+                copy_to_user((struct lcd_display *)arg, &display, dlen);

Sleeping holding a spinlock

Review

Quoted 2 lines
(quoted diff) +2 -0
+        case LCD_Set_Cursor_Pos:
+                copy_from_user(&display, (struct lcd_display *)arg, dlen);

Ditto. Also should check the return and return -EFAULT if so

drivers/cobalt/lcd.c line 498

Quoted 10 lines
drivers/cobalt/lcd.c +10 -0
+/* LED daemon sits on this, we wake it up once a key is pressed */
+static ssize_t 
+cobalt_lcd_read(struct file *file, char *buf, size_t count, loff_t *ppos)
+{
+	int buttons_now;
+	static atomic_t lcd_waiters = ATOMIC_INIT(0);
+
+	if (atomic_read(&lcd_waiters) > 0)
+		return -EINVAL;
+	atomic_inc(&lcd_waiters);

Unsafe. Two people can execute the atomic_read before anyone executes the atomic_inc. You probably want test_and_set_bit() or a semaphore

drivers/cobalt/lcd.c line 503

Quoted 4 lines
drivers/cobalt/lcd.c +4 -0
+	while (((buttons_now = button_pressed()) == 0) &&
+	       !(signal_pending(current)))
+	{
+		current->state = TASK_INTERRUPTIBLE;

We have a set_ function for this.. ALso what stops an ioctl occuring at the same instant ?