2005-01-12 05:29:48

by David Gibson

[permalink] [raw]
Subject: [0/8] orinoco driver updates

Following are a bunch of patches which make a few more steps towards
the long overdue merge of the CVS orinoco driver into mainline. These
do make behavioural changes to the driver, but they should all be
trivial and largely cosmetic.

Jeff, please apply. Patches are against the netdev BK tree.

orinoco-printks
Update printk()s and other cosmetic strings
orinoco-carrier
Use netif_carrier_() functions instead of homegrown connected
flag
orinoco-delays
Use mdelay() and ssleep() instead of schedule_timeout() and
other more complicated idioms.
orinoco-free-orinocodev
Introduce free_orinocodev() function as a wrapper around
free_netdev()
orinoco-cleanup-hermes
Make cleanups to low-level driver code
orinoco-pccard-cleanups
Cleanup initialization for PCMCIA/PC-Card devices.
orinoco-modparm
Replace obsolete MODULE_PARM() constructions from orinoco.c
orinoco-pci-updates
Cleanup initialization for PCI/PLX/TMD devices.

--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist. NOT _the_ _other_ _way_
| _around_!
http://www.ozlabs.org/people/dgibson


2005-01-12 05:29:57

by David Gibson

[permalink] [raw]
Subject: [3/8] orinoco: Use mdelay()/ssleep() instead of more complex delays

Use mdelay() or ssleep() instead of various silly more complicated
ways of delaying in the orinoco driver.

Signed-off-by: David Gibson <[email protected]>

Index: working-2.6/drivers/net/wireless/orinoco_pci.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco_pci.c 2005-01-12 15:13:18.819073992 +1100
+++ working-2.6/drivers/net/wireless/orinoco_pci.c 2005-01-12 15:15:33.137654464 +1100
@@ -151,19 +151,11 @@

/* Assert the reset until the card notice */
hermes_write_regn(hw, PCI_COR, HERMES_PCI_COR_MASK);
- timeout = jiffies + (HERMES_PCI_COR_ONT * HZ / 1000);
- while(time_before(jiffies, timeout)) {
- mdelay(1);
- }
- //mdelay(HERMES_PCI_COR_ONT);
+ mdelay(HERMES_PCI_COR_ONT);

/* Give time for the card to recover from this hard effort */
hermes_write_regn(hw, PCI_COR, 0x0000);
- timeout = jiffies + (HERMES_PCI_COR_OFFT * HZ / 1000);
- while(time_before(jiffies, timeout)) {
- mdelay(1);
- }
- //mdelay(HERMES_PCI_COR_OFFT);
+ mdelay(HERMES_PCI_COR_OFFT);

/* The card is ready when it's no longer busy */
timeout = jiffies + (HERMES_PCI_COR_BUSYT * HZ / 1000);
Index: working-2.6/drivers/net/wireless/orinoco_plx.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco_plx.c 2005-01-12 15:13:18.821073688 +1100
+++ working-2.6/drivers/net/wireless/orinoco_plx.c 2005-01-12 15:15:33.138654312 +1100
@@ -356,8 +356,7 @@
static void __exit orinoco_plx_exit(void)
{
pci_unregister_driver(&orinoco_plx_driver);
- current->state = TASK_UNINTERRUPTIBLE;
- schedule_timeout(HZ);
+ ssleep(1);
}

module_init(orinoco_plx_init);
Index: working-2.6/drivers/net/wireless/orinoco_tmd.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco_tmd.c 2005-01-12 15:13:18.820073840 +1100
+++ working-2.6/drivers/net/wireless/orinoco_tmd.c 2005-01-12 15:16:05.897674184 +1100
@@ -225,8 +225,7 @@
static void __exit orinoco_tmd_exit(void)
{
pci_unregister_driver(&orinoco_tmd_driver);
- current->state = TASK_UNINTERRUPTIBLE;
- schedule_timeout(HZ);
+ ssleep(1);
}

module_init(orinoco_tmd_init);


--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist. NOT _the_ _other_ _way_
| _around_!
http://www.ozlabs.org/people/dgibson

2005-01-12 05:32:35

by David Gibson

[permalink] [raw]
Subject: [4/8] orinoco: Introduce free_orinocodev() function

Introduce a free_orinocodev() function into the orinoco driver, used
by the hardware type/initialization modules to free the device
structure in preference to directly calling free_netdev(). At the
moment free_orinocodev() just calls free_netdev(). Future merges will
make it clean up internal scanning state, so merging this now will
reduce the diff noise.

Signed-off-by: David Gibson <[email protected]>

Index: working-2.6/drivers/net/wireless/orinoco.h
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco.h 2005-01-12 15:22:34.300627960 +1100
+++ working-2.6/drivers/net/wireless/orinoco.h 2005-01-12 15:22:34.360618840 +1100
@@ -107,6 +107,7 @@

extern struct net_device *alloc_orinocodev(int sizeof_card,
int (*hard_reset)(struct orinoco_private *));
+extern void free_orinocodev(struct net_device *dev);
extern int __orinoco_up(struct net_device *dev);
extern int __orinoco_down(struct net_device *dev);
extern int orinoco_stop(struct net_device *dev);
Index: working-2.6/drivers/net/wireless/orinoco.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco.c 2005-01-12 15:22:34.300627960 +1100
+++ working-2.6/drivers/net/wireless/orinoco.c 2005-01-12 15:22:34.362618536 +1100
@@ -2398,6 +2398,11 @@

}

+void free_orinocodev(struct net_device *dev)
+{
+ free_netdev(dev);
+}
+
/********************************************************************/
/* Wireless extensions */
/********************************************************************/
Index: working-2.6/drivers/net/wireless/orinoco_cs.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco_cs.c 2005-01-12 15:22:34.265633280 +1100
+++ working-2.6/drivers/net/wireless/orinoco_cs.c 2005-01-12 15:22:34.363618384 +1100
@@ -249,7 +249,7 @@
dev);
unregister_netdev(dev);
}
- free_netdev(dev);
+ free_orinocodev(dev);
} /* orinoco_cs_detach */

/*
Index: working-2.6/drivers/net/wireless/orinoco_pci.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco_pci.c 2005-01-12 15:22:34.330623400 +1100
+++ working-2.6/drivers/net/wireless/orinoco_pci.c 2005-01-12 15:22:34.363618384 +1100
@@ -254,7 +254,7 @@
if (dev->irq)
free_irq(dev->irq, dev);

- free_netdev(dev);
+ free_orinocodev(dev);
}

if (pci_ioaddr)
@@ -279,7 +279,7 @@
iounmap(priv->hw.iobase);

pci_set_drvdata(pdev, NULL);
- free_netdev(dev);
+ free_orinocodev(dev);

pci_disable_device(pdev);
}
Index: working-2.6/drivers/net/wireless/orinoco_plx.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco_plx.c 2005-01-12 15:22:34.330623400 +1100
+++ working-2.6/drivers/net/wireless/orinoco_plx.c 2005-01-12 15:22:34.363618384 +1100
@@ -279,7 +279,7 @@
fail:
free_irq(dev->irq, dev);
fail_irq:
- free_netdev(dev);
+ free_orinocodev(dev);
fail_alloc:
pci_iounmap(pdev, mem);
fail_map:
@@ -304,7 +304,7 @@

pci_set_drvdata(pdev, NULL);

- free_netdev(dev);
+ free_orinocodev(dev);

release_region(pci_resource_start(pdev, 3), pci_resource_len(pdev, 3));

Index: working-2.6/drivers/net/wireless/orinoco_tmd.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco_tmd.c 2005-01-12 15:22:34.331623248 +1100
+++ working-2.6/drivers/net/wireless/orinoco_tmd.c 2005-01-12 15:22:34.364618232 +1100
@@ -164,7 +164,7 @@
out4:
pci_iounmap(pdev, mem);
out3:
- free_netdev(dev);
+ free_orinocodev(dev);
out2:
release_region(pccard_ioaddr, pccard_iolen);
out:
@@ -188,7 +188,7 @@

pci_set_drvdata(pdev, NULL);

- free_netdev(dev);
+ free_orinocodev(dev);

release_region(pci_resource_start(pdev, 2), pci_resource_len(pdev, 2));

Index: working-2.6/drivers/net/wireless/airport.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/airport.c 2005-01-12 15:22:34.265633280 +1100
+++ working-2.6/drivers/net/wireless/airport.c 2005-01-12 15:25:06.000000000 +1100
@@ -149,7 +149,7 @@
ssleep(1);

macio_set_drvdata(mdev, NULL);
- free_netdev(dev);
+ free_orinocodev(dev);

return 0;
}
@@ -211,7 +211,7 @@

if (macio_request_resource(mdev, 0, "airport")) {
printk(KERN_ERR PFX "can't request IO resource !\n");
- free_netdev(dev);
+ free_orinocodev(dev);
return -EBUSY;
}



--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist. NOT _the_ _other_ _way_
| _around_!
http://www.ozlabs.org/people/dgibson

2005-01-12 05:33:19

by David Gibson

[permalink] [raw]
Subject: [2/8] orinoco: Use netif_carrier functions instead of homegrown flag

Removes the orinoco driver's custom and dodgy "connected" variable
used to track whether or not we're associated with an AP. Replaces it
instead with netif_carrier_ok() settings.

Signed-off-by: David Gibson <[email protected]>

Index: working-2.6/drivers/net/wireless/orinoco.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco.c 2004-11-03 14:32:15.000000000 +1100
+++ working-2.6/drivers/net/wireless/orinoco.c 2004-11-03 16:16:57.000000000 +1100
@@ -784,7 +784,7 @@
return 1;
}

- if (! priv->connected) {
+ if (! netif_carrier_ok(dev)) {
/* Oops, the firmware hasn't established a connection,
silently drop the packet (this seems to be the
safest approach). */
@@ -1271,6 +1271,7 @@
case HERMES_INQ_LINKSTATUS: {
struct hermes_linkstatus linkstatus;
u16 newstatus;
+ int connected;

if (len != sizeof(linkstatus)) {
printk(KERN_WARNING "%s: Unexpected size for linkstatus frame (%d bytes)\n",
@@ -1282,15 +1283,14 @@
len / 2);
newstatus = le16_to_cpu(linkstatus.linkstatus);

- if ( (newstatus == HERMES_LINKSTATUS_CONNECTED)
- || (newstatus == HERMES_LINKSTATUS_AP_CHANGE)
- || (newstatus == HERMES_LINKSTATUS_AP_IN_RANGE) )
- priv->connected = 1;
- else if ( (newstatus == HERMES_LINKSTATUS_NOT_CONNECTED)
- || (newstatus == HERMES_LINKSTATUS_DISCONNECTED)
- || (newstatus == HERMES_LINKSTATUS_AP_OUT_OF_RANGE)
- || (newstatus == HERMES_LINKSTATUS_ASSOC_FAILED) )
- priv->connected = 0;
+ connected = (newstatus == HERMES_LINKSTATUS_CONNECTED)
+ || (newstatus == HERMES_LINKSTATUS_AP_CHANGE)
+ || (newstatus == HERMES_LINKSTATUS_AP_IN_RANGE);
+
+ if (connected)
+ netif_carrier_on(dev);
+ else
+ netif_carrier_off(dev);

if (newstatus != priv->last_linkstatus)
print_linkstatus(dev, newstatus);
@@ -1368,8 +1368,8 @@
}

/* firmware will have to reassociate */
+ netif_carrier_off(dev);
priv->last_linkstatus = 0xffff;
- priv->connected = 0;

return 0;
}
@@ -1881,7 +1881,7 @@

priv->hw_unavailable++;
priv->last_linkstatus = 0xffff; /* firmware will have to reassociate */
- priv->connected = 0;
+ netif_carrier_off(dev);

orinoco_unlock(priv, &flags);

@@ -2391,8 +2391,8 @@
* hardware */
INIT_WORK(&priv->reset_work, (void (*)(void *))orinoco_reset, dev);

+ netif_carrier_off(dev);
priv->last_linkstatus = 0xffff;
- priv->connected = 0;

return dev;

Index: working-2.6/drivers/net/wireless/orinoco.h
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco.h 2004-11-03 14:32:15.000000000 +1100
+++ working-2.6/drivers/net/wireless/orinoco.h 2004-11-03 16:12:40.000000000 +1100
@@ -42,7 +42,6 @@
/* driver state */
int open;
u16 last_linkstatus;
- int connected;

/* Net device stuff */
struct net_device *ndev;

--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist. NOT _the_ _other_ _way_
| _around_!
http://www.ozlabs.org/people/dgibson

2005-01-12 05:36:43

by David Gibson

[permalink] [raw]
Subject: [6/8] orinoco: Cleanup PCMCIA/PC-Card code

Cleanup the various bits of initialization code for PCMCIA / PC-Card
orinoco devices. This includes one important bugfix where we could
fail to take the lock in some circumstances.

Signed-off-by: David Gibson <[email protected]>

Index: working-2.6/drivers/net/wireless/orinoco_cs.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco_cs.c 2005-01-12 15:34:50.847655792 +1100
+++ working-2.6/drivers/net/wireless/orinoco_cs.c 2004-11-05 14:31:07.000000000 +1100
@@ -54,19 +54,11 @@

/* Module parameters */

-/* The old way: bit map of interrupts to choose from */
-/* This means pick from 15, 14, 12, 11, 10, 9, 7, 5, 4, and 3 */
-static uint irq_mask = 0xdeb8;
-/* Newer, simpler way of listing specific interrupts */
-static int irq_list[4] = { -1 };
-
/* Some D-Link cards have buggy CIS. They do work at 5v properly, but
* don't have any CIS entry for it. This workaround it... */
static int ignore_cis_vcc; /* = 0 */
-
-MODULE_PARM(irq_mask, "i");
-MODULE_PARM(irq_list, "1-4i");
-MODULE_PARM(ignore_cis_vcc, "i");
+module_param(ignore_cis_vcc, int, 0644);
+MODULE_PARM_DESC(ignore_cis_vcc, "Allow voltage mismatch between card and socket");

/********************************************************************/
/* Magic constants */
@@ -136,6 +128,7 @@
if (err)
return err;

+ msleep(100);
clear_bit(0, &card->hard_reset_in_progress);

return 0;
@@ -161,7 +154,7 @@
struct orinoco_pccard *card;
dev_link_t *link;
client_reg_t client_reg;
- int ret, i;
+ int ret;

dev = alloc_orinocodev(sizeof(*card), orinoco_cs_hard_reset);
if (! dev)
@@ -174,14 +167,11 @@
link->priv = dev;

/* Interrupt setup */
- link->irq.Attributes = IRQ_TYPE_EXCLUSIVE;
+ link->irq.Attributes = IRQ_TYPE_EXCLUSIVE | IRQ_HANDLE_PRESENT;
link->irq.IRQInfo1 = IRQ_INFO2_VALID | IRQ_LEVEL_ID;
- if (irq_list[0] == -1)
- link->irq.IRQInfo2 = irq_mask;
- else
- for (i = 0; i < 4; i++)
- link->irq.IRQInfo2 |= 1 << irq_list[i];
- link->irq.Handler = NULL;
+ link->irq.IRQInfo2 = 0xffffffff; /* Any ISA IRQ */
+ link->irq.Handler = orinoco_interrupt;
+ link->irq.Instance = dev;

/* General socket configuration defaults can go here. In this
* client, we assume very little, and rely on the CIS for
@@ -262,6 +252,9 @@
last_fn = (fn); if ((last_ret = (ret)) != 0) goto cs_failed; \
} while (0)

+#define CFG_CHECK(fn, ret) \
+ if (ret != 0) goto next_entry
+
static void
orinoco_cs_config(dev_link_t *link)
{
@@ -323,9 +316,8 @@
cistpl_cftable_entry_t *cfg = &(parse.cftable_entry);
cistpl_cftable_entry_t dflt = { .index = 0 };

- if (pcmcia_get_tuple_data(handle, &tuple) != 0 ||
- pcmcia_parse_tuple(handle, &tuple, &parse) != 0)
- goto next_entry;
+ CFG_CHECK(GetTupleData, pcmcia_get_tuple_data(handle, &tuple));
+ CFG_CHECK(ParseTuple, pcmcia_parse_tuple(handle, &tuple, &parse));

if (cfg->flags & CISTPL_CFTABLE_DEFAULT)
dflt = *cfg;
@@ -363,8 +355,7 @@
dflt.vpp1.param[CISTPL_POWER_VNOM] / 10000;

/* Do we need to allocate an interrupt? */
- if (cfg->irq.IRQInfo1 || dflt.irq.IRQInfo1)
- link->conf.Attributes |= CONF_ENABLE_IRQ;
+ link->conf.Attributes |= CONF_ENABLE_IRQ;

/* IO window settings */
link->io.NumPorts1 = link->io.NumPorts2 = 0;
@@ -390,8 +381,8 @@
}

/* This reserves IO space but doesn't actually enable it */
- if (pcmcia_request_io(link->handle, &link->io) != 0)
- goto next_entry;
+ CFG_CHECK(RequestIO,
+ pcmcia_request_io(link->handle, &link->io));
}


@@ -416,22 +407,7 @@
* a handler to the interrupt, unless the 'Handler' member of
* the irq structure is initialized.
*/
- if (link->conf.Attributes & CONF_ENABLE_IRQ) {
- int i;
-
- link->irq.Attributes = IRQ_TYPE_EXCLUSIVE | IRQ_HANDLE_PRESENT;
- link->irq.IRQInfo1 = IRQ_INFO2_VALID | IRQ_LEVEL_ID;
- if (irq_list[0] == -1)
- link->irq.IRQInfo2 = irq_mask;
- else
- for (i=0; i<4; i++)
- link->irq.IRQInfo2 |= 1 << irq_list[i];
-
- link->irq.Handler = orinoco_interrupt;
- link->irq.Instance = dev;
-
- CS_CHECK(RequestIRQ, pcmcia_request_irq(link->handle, &link->irq));
- }
+ CS_CHECK(RequestIRQ, pcmcia_request_irq(link->handle, &link->irq));

/* We initialize the hermes structure before completing PCMCIA
* configuration just in case the interrupt handler gets
@@ -456,8 +432,6 @@
SET_MODULE_OWNER(dev);
card->node.major = card->node.minor = 0;

- /* register_netdev will give us an ethX name */
- dev->name[0] = '\0';
/* Tell the stack we exist */
if (register_netdev(dev) != 0) {
printk(KERN_ERR PFX "register_netdev() failed\n");
@@ -479,8 +453,7 @@
if (link->conf.Vpp1)
printk(", Vpp %d.%d", link->conf.Vpp1 / 10,
link->conf.Vpp1 % 10);
- if (link->conf.Attributes & CONF_ENABLE_IRQ)
- printk(", irq %d", link->irq.AssignedIRQ);
+ printk(", irq %d", link->irq.AssignedIRQ);
if (link->io.NumPorts1)
printk(", io 0x%04x-0x%04x", link->io.BasePort1,
link->io.BasePort1 + link->io.NumPorts1 - 1);
@@ -546,12 +519,12 @@
case CS_EVENT_CARD_REMOVAL:
link->state &= ~DEV_PRESENT;
if (link->state & DEV_CONFIG) {
- orinoco_lock(priv, &flags);
+ unsigned long flags;

+ spin_lock_irqsave(&priv->lock, flags);
netif_device_detach(dev);
priv->hw_unavailable++;
-
- orinoco_unlock(priv, &flags);
+ spin_unlock_irqrestore(&priv->lock, flags);
}
break;



--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist. NOT _the_ _other_ _way_
| _around_!
http://www.ozlabs.org/people/dgibson

2005-01-12 05:36:44

by David Gibson

[permalink] [raw]
Subject: [5/8] orinoco: Cleanup low-level hermes code

Apply some cleanups to the low-level orinoco handling code in
hermes.[ch]. This cleans up some error handling code, corrects an
error code to something more accurate, and also increases a timeout
value. This last can (when the hardware plays up) cause long delays
with spinlocks held, which is bad, but is rather less prone to
prematurely giving up, which has the unfortunate habit of fatally
confusing the hardware in other ways :-/.

Signed-off-by: David Gibson <[email protected]>

Index: working-2.6/drivers/net/wireless/hermes.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/hermes.c 2005-01-12 15:22:34.263633584 +1100
+++ working-2.6/drivers/net/wireless/hermes.c 2004-11-05 13:59:07.000000000 +1100
@@ -383,12 +383,17 @@
reg = hermes_read_reg(hw, oreg);
}

- if (reg & HERMES_OFFSET_BUSY) {
- return -ETIMEDOUT;
- }
+ if (reg != offset) {
+ printk(KERN_ERR "hermes @ %p: BAP%d offset %s: "
+ "reg=0x%x id=0x%x offset=0x%x\n", hw->iobase, bap,
+ (reg & HERMES_OFFSET_BUSY) ? "timeout" : "error",
+ reg, id, offset);
+
+ if (reg & HERMES_OFFSET_BUSY) {
+ return -ETIMEDOUT;
+ }

- if (reg & HERMES_OFFSET_ERR) {
- return -EIO;
+ return -EIO; /* error or wrong offset */
}

return 0;
@@ -476,7 +481,7 @@
rlength = hermes_read_reg(hw, dreg);

if (! rlength)
- return -ENOENT;
+ return -ENODATA;

rtype = hermes_read_reg(hw, dreg);

Index: working-2.6/drivers/net/wireless/hermes.h
===================================================================
--- working-2.6.orig/drivers/net/wireless/hermes.h 2005-01-12 11:13:41.000000000 +1100
+++ working-2.6/drivers/net/wireless/hermes.h 2004-11-05 13:53:55.000000000 +1100
@@ -340,7 +340,7 @@
#ifdef __KERNEL__

/* Timeouts */
-#define HERMES_BAP_BUSY_TIMEOUT (500) /* In iterations of ~1us */
+#define HERMES_BAP_BUSY_TIMEOUT (10000) /* In iterations of ~1us */

/* Basic control structure */
typedef struct hermes {


--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist. NOT _the_ _other_ _way_
| _around_!
http://www.ozlabs.org/people/dgibson

2005-01-12 05:36:43

by David Gibson

[permalink] [raw]
Subject: [7/8] orinoco: Replace obsolete MODULE_PARM()

Replace obsolete MODULE_PARM() with module_param() in the orinoco
driver.

Signed-off-by: David Gibson <[email protected]>

Index: working-2.6/drivers/net/wireless/orinoco.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco.c 2005-01-12 15:47:48.215477920 +1100
+++ working-2.6/drivers/net/wireless/orinoco.c 2005-01-12 15:50:14.156291544 +1100
@@ -461,12 +461,14 @@
/* Level of debugging. Used in the macros in orinoco.h */
#ifdef ORINOCO_DEBUG
int orinoco_debug = ORINOCO_DEBUG;
-MODULE_PARM(orinoco_debug, "i");
+module_param(orinoco_debug, int, 0644);
+MODULE_PARM_DESC(orinoco_debug, "Debug level");
EXPORT_SYMBOL(orinoco_debug);
#endif

static int suppress_linkstatus; /* = 0 */
-MODULE_PARM(suppress_linkstatus, "i");
+module_param(suppress_linkstatus, int, 0644);
+MODULE_PARM_DESC(suppress_linkstatus, "Don't log link status changes");

/********************************************************************/
/* Compile time configuration and compatibility stuff */


--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist. NOT _the_ _other_ _way_
| _around_!
http://www.ozlabs.org/people/dgibson

2005-01-12 05:41:43

by David Gibson

[permalink] [raw]
Subject: [8/8] orinoco: PCI/PLX/TMD driver updates

Update the initialization code in the various PCI incarnations of the
orinoco driver. This applies similar initialization and shutdown
cleanups to the orinoco_pci, orinoco_plx and orinoco_tmd drivers. It
also adds COR reset support to the orinoco_plx and orinoco_tmd
drivers, improves PCI power management support in the orinoco_pci
driver and adds a couple of extra supported cards to the ID tables.

Signed-off-by: David Gibson <[email protected]>

Index: working-2.6/drivers/net/wireless/orinoco_pci.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco_pci.c 2005-01-12 15:47:48.215477920 +1100
+++ working-2.6/drivers/net/wireless/orinoco_pci.c 2005-01-12 16:10:57.324301280 +1100
@@ -129,6 +129,11 @@
#define HERMES_PCI_COR_OFFT (500) /* ms */
#define HERMES_PCI_COR_BUSYT (500) /* ms */

+/* Orinoco PCI specific data */
+struct orinoco_pci_card {
+ void __iomem *pci_ioaddr;
+};
+
/*
* Do a soft reset of the PCI card using the Configuration Option Register
* We need this to get going...
@@ -164,8 +169,9 @@
mdelay(1);
reg = hermes_read_regn(hw, CMD);
}
- /* Did we timeout ? */
- if(time_after_eq(jiffies, timeout)) {
+
+ /* Still busy? */
+ if (reg & HERMES_CMD_BUSY) {
printk(KERN_ERR PFX "Busy timeout\n");
return -ETIMEDOUT;
}
@@ -184,6 +190,7 @@
u16 __iomem *pci_ioaddr = NULL;
unsigned long pci_iolen;
struct orinoco_private *priv = NULL;
+ struct orinoco_pci_card *card;
struct net_device *dev = NULL;

err = pci_enable_device(pdev);
@@ -192,24 +199,31 @@
return err;
}

+ err = pci_request_regions(pdev, DRIVER_NAME);
+ if (err != 0) {
+ printk(KERN_ERR PFX "Cannot obtain PCI resources\n");
+ goto fail_resources;
+ }
+
/* Resource 0 is mapped to the hermes registers */
pci_iorange = pci_resource_start(pdev, 0);
pci_iolen = pci_resource_len(pdev, 0);
pci_ioaddr = ioremap(pci_iorange, pci_iolen);
- if (! pci_iorange) {
+ if (!pci_iorange) {
printk(KERN_ERR PFX "Cannot remap hardware registers\n");
- goto fail;
+ goto fail_map;
}

/* Allocate network device */
- dev = alloc_orinocodev(0, NULL);
+ dev = alloc_orinocodev(sizeof(*card), orinoco_pci_cor_reset);
if (! dev) {
err = -ENOMEM;
- goto fail;
+ goto fail_alloc;
}

priv = netdev_priv(dev);
- dev->base_addr = (unsigned long) pci_ioaddr;
+ card = priv->card;
+ card->pci_ioaddr = pci_ioaddr;
dev->mem_start = pci_iorange;
dev->mem_end = pci_iorange + pci_iolen - 1;
SET_MODULE_OWNER(dev);
@@ -226,14 +240,14 @@
if (err) {
printk(KERN_ERR PFX "Cannot allocate IRQ %d\n", pdev->irq);
err = -EBUSY;
- goto fail;
+ goto fail_irq;
}
dev->irq = pdev->irq;

/* Perform a COR reset to start the card */
- if(orinoco_pci_cor_reset(priv) != 0) {
+ err = orinoco_pci_cor_reset(priv);
+ if (err) {
printk(KERN_ERR PFX "Initial reset failed\n");
- err = -ETIMEDOUT;
goto fail;
}

@@ -250,16 +264,19 @@
return 0;

fail:
- if (dev) {
- if (dev->irq)
- free_irq(dev->irq, dev);
+ free_irq(pdev->irq, dev);

- free_orinocodev(dev);
- }
+ fail_irq:
+ pci_set_drvdata(pdev, NULL);
+ free_orinocodev(dev);
+
+ fail_alloc:
+ iounmap(pci_ioaddr);

- if (pci_ioaddr)
- iounmap(pci_ioaddr);
+ fail_map:
+ pci_release_regions(pdev);

+ fail_resources:
pci_disable_device(pdev);

return err;
@@ -269,18 +286,14 @@
{
struct net_device *dev = pci_get_drvdata(pdev);
struct orinoco_private *priv = netdev_priv(dev);
+ struct orinoco_pci_card *card = priv->card;

unregister_netdev(dev);
-
- if (dev->irq)
- free_irq(dev->irq, dev);
-
- if (priv->hw.iobase)
- iounmap(priv->hw.iobase);
-
+ free_irq(dev->irq, dev);
pci_set_drvdata(pdev, NULL);
free_orinocodev(dev);
-
+ iounmap(card->pci_ioaddr);
+ pci_release_regions(pdev);
pci_disable_device(pdev);
}

@@ -312,6 +325,9 @@

orinoco_unlock(priv, &flags);

+ pci_save_state(pdev);
+ pci_set_power_state(pdev, 3);
+
return 0;
}

@@ -324,6 +340,9 @@

printk(KERN_DEBUG "%s: Orinoco-PCI waking up\n", dev->name);

+ pci_set_power_state(pdev, 0);
+ pci_restore_state(pdev);
+
err = orinoco_reinit_firmware(dev);
if (err) {
printk(KERN_ERR "%s: Error %d re-initializing firmware on orinoco_pci_resume()\n",
@@ -354,6 +373,8 @@
{0x1260, 0x3872, PCI_ANY_ID, PCI_ANY_ID,},
/* Intersil Prism 2.5 */
{0x1260, 0x3873, PCI_ANY_ID, PCI_ANY_ID,},
+ /* Samsung MagicLAN SWL-2210P */
+ {0x167d, 0xa000, PCI_ANY_ID, PCI_ANY_ID,},
{0,},
};

Index: working-2.6/drivers/net/wireless/orinoco_plx.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco_plx.c 2005-01-12 15:47:48.216477768 +1100
+++ working-2.6/drivers/net/wireless/orinoco_plx.c 2004-11-05 14:20:30.000000000 +1100
@@ -142,26 +142,68 @@
#include "hermes.h"
#include "orinoco.h"

-#define COR_OFFSET (0x3e0/2) /* COR attribute offset of Prism2 PC card */
+#define COR_OFFSET (0x3e0) /* COR attribute offset of Prism2 PC card */
#define COR_VALUE (COR_LEVEL_REQ | COR_FUNC_ENA) /* Enable PC card with interrupt in level trigger */
+#define COR_RESET (0x80) /* reset bit in the COR register */
+#define PLX_RESET_TIME (500) /* milliseconds */

#define PLX_INTCSR 0x4c /* Interrupt Control & Status Register */
#define PLX_INTCSR_INTEN (1<<6) /* Interrupt Enable bit */

-static const u16 cis_magic[] = {
- 0x0001, 0x0003, 0x0000, 0x0000, 0x00ff, 0x0017, 0x0004, 0x0067
+static const u8 cis_magic[] = {
+ 0x01, 0x03, 0x00, 0x00, 0xff, 0x17, 0x04, 0x67
};

+/* Orinoco PLX specific data */
+struct orinoco_plx_card {
+ void __iomem *attr_mem;
+};
+
+/*
+ * Do a soft reset of the card using the Configuration Option Register
+ */
+static int orinoco_plx_cor_reset(struct orinoco_private *priv)
+{
+ hermes_t *hw = &priv->hw;
+ struct orinoco_plx_card *card = priv->card;
+ u8 __iomem *attr_mem = card->attr_mem;
+ unsigned long timeout;
+ u16 reg;
+
+ writeb(COR_VALUE | COR_RESET, attr_mem + COR_OFFSET);
+ mdelay(1);
+
+ writeb(COR_VALUE, attr_mem + COR_OFFSET);
+ mdelay(1);
+
+ /* Just in case, wait more until the card is no longer busy */
+ timeout = jiffies + (PLX_RESET_TIME * HZ / 1000);
+ reg = hermes_read_regn(hw, CMD);
+ while (time_before(jiffies, timeout) && (reg & HERMES_CMD_BUSY)) {
+ mdelay(1);
+ reg = hermes_read_regn(hw, CMD);
+ }
+
+ /* Did we timeout ? */
+ if (reg & HERMES_CMD_BUSY) {
+ printk(KERN_ERR PFX "Busy timeout\n");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+
static int orinoco_plx_init_one(struct pci_dev *pdev,
const struct pci_device_id *ent)
{
int err = 0;
- u16 __iomem *attr_mem = NULL;
- u32 reg, addr;
+ u8 __iomem *attr_mem = NULL;
+ u32 csr_reg, plx_addr;
struct orinoco_private *priv = NULL;
+ struct orinoco_plx_card *card;
unsigned long pccard_ioaddr = 0;
unsigned long pccard_iolen = 0;
- u16 magic[8];
struct net_device *dev = NULL;
void __iomem *mem;
int i;
@@ -169,78 +211,38 @@
err = pci_enable_device(pdev);
if (err) {
printk(KERN_ERR PFX "Cannot enable PCI device\n");
- return -EIO;
+ return err;
}

- /* Resource 2 is mapped to the PCMCIA space */
- attr_mem = ioremap(pci_resource_start(pdev, 2), PAGE_SIZE);
- if (!attr_mem)
+ err = pci_request_regions(pdev, DRIVER_NAME);
+ if (err != 0) {
+ printk(KERN_ERR PFX "Cannot obtain PCI resources\n");
goto fail_resources;
-
- printk(KERN_DEBUG PFX "CIS: ");
- for (i = 0; i < 16; i++) {
- printk("%02X:", readw(attr_mem+i));
}
- printk("\n");
-
- /* Verify whether PC card is present */
- /* FIXME: we probably need to be smarted about this */
- memcpy_fromio(magic, attr_mem, 16);
- if (memcmp(magic, cis_magic, 16) != 0) {
- printk(KERN_ERR PFX "The CIS value of Prism2 PC "
- "card is unexpected\n");
- err = -EIO;
- iounmap(attr_mem);
- goto fail_resources;
- }
-
- /* PCMCIA COR is the first byte following CIS: this write should
- * enable I/O mode and select level-triggered interrupts */
- writew(COR_VALUE, attr_mem + COR_OFFSET);
- mdelay(1);
- reg = readw(attr_mem + COR_OFFSET);
- iounmap(attr_mem);

- if (reg != COR_VALUE) {
- printk(KERN_ERR PFX "Error setting COR value (reg=%x)\n", reg);
- goto fail_resources;
- }
+ /* Resource 1 is mapped to PLX-specific registers */
+ plx_addr = pci_resource_start(pdev, 1);

- /* bjoern: We need to tell the card to enable interrupts, in
- case the serial eprom didn't do this already. See the
- PLX9052 data book, p8-1 and 8-24 for reference. */
- addr = pci_resource_start(pdev, 1);
- reg = inl(addr+PLX_INTCSR);
- if (reg & PLX_INTCSR_INTEN)
- printk(KERN_DEBUG PFX "Local Interrupt already enabled\n");
- else {
- reg |= PLX_INTCSR_INTEN;
- outl(reg, addr+PLX_INTCSR);
- reg = inl(addr+PLX_INTCSR);
- if(!(reg & PLX_INTCSR_INTEN)) {
- printk(KERN_ERR PFX "Couldn't enable Local Interrupts\n");
- goto fail_resources;
- }
+ /* Resource 2 is mapped to the PCMCIA attribute memory */
+ attr_mem = ioremap(pci_resource_start(pdev, 2),
+ pci_resource_len(pdev, 2));
+ if (!attr_mem) {
+ printk(KERN_ERR PFX "Cannot remap PCMCIA space\n");
+ goto fail_map_attr;
}

/* Resource 3 is mapped to the PCMCIA I/O address space */
pccard_ioaddr = pci_resource_start(pdev, 3);
pccard_iolen = pci_resource_len(pdev, 3);
- if (! request_region(pccard_ioaddr, pccard_iolen, DRIVER_NAME)) {
- printk(KERN_ERR PFX "I/O resource 0x%lx @ 0x%lx busy\n",
- pccard_iolen, pccard_ioaddr);
- err = -EBUSY;
- goto fail_resources;
- }

mem = pci_iomap(pdev, 3, 0);
if (!mem) {
err = -ENOMEM;
- goto fail_map;
+ goto fail_map_io;
}

/* Allocate network device */
- dev = alloc_orinocodev(0, NULL);
+ dev = alloc_orinocodev(sizeof(*card), orinoco_plx_cor_reset);
if (!dev) {
printk(KERN_ERR PFX "Cannot allocate network device\n");
err = -ENOMEM;
@@ -248,6 +250,8 @@
}

priv = netdev_priv(dev);
+ card = priv->card;
+ card->attr_mem = attr_mem;
dev->base_addr = pccard_ioaddr;
SET_MODULE_OWNER(dev);
SET_NETDEV_DEV(dev, &pdev->dev);
@@ -268,6 +272,43 @@
}
dev->irq = pdev->irq;

+ /* bjoern: We need to tell the card to enable interrupts, in
+ case the serial eprom didn't do this already. See the
+ PLX9052 data book, p8-1 and 8-24 for reference. */
+ csr_reg = inl(plx_addr + PLX_INTCSR);
+ if (!(csr_reg & PLX_INTCSR_INTEN)) {
+ csr_reg |= PLX_INTCSR_INTEN;
+ outl(csr_reg, plx_addr + PLX_INTCSR);
+ csr_reg = inl(plx_addr + PLX_INTCSR);
+ if (!(csr_reg & PLX_INTCSR_INTEN)) {
+ printk(KERN_ERR PFX "Cannot enable interrupts\n");
+ goto fail;
+ }
+ }
+
+ err = orinoco_plx_cor_reset(priv);
+ if (err) {
+ printk(KERN_ERR PFX "Initial reset failed\n");
+ goto fail;
+ }
+
+ printk(KERN_DEBUG PFX "CIS: ");
+ for (i = 0; i < 16; i++) {
+ printk("%02X:", readb(attr_mem + 2*i));
+ }
+ printk("\n");
+
+ /* Verify whether a supported PC card is present */
+ /* FIXME: we probably need to be smarted about this */
+ for (i = 0; i < sizeof(cis_magic); i++) {
+ if (cis_magic[i] != readb(attr_mem +2*i)) {
+ printk(KERN_ERR PFX "The CIS value of Prism2 PC "
+ "card is unexpected\n");
+ err = -EIO;
+ goto fail;
+ }
+ }
+
err = register_netdev(dev);
if (err) {
printk(KERN_ERR PFX "Cannot register network device\n");
@@ -277,13 +318,21 @@
return 0;

fail:
- free_irq(dev->irq, dev);
+ free_irq(pdev->irq, dev);
+
fail_irq:
+ pci_set_drvdata(pdev, NULL);
free_orinocodev(dev);
+
fail_alloc:
pci_iounmap(pdev, mem);
- fail_map:
- release_region(pccard_ioaddr, pccard_iolen);
+
+ fail_map_io:
+ iounmap(attr_mem);
+
+ fail_map_attr:
+ pci_release_regions(pdev);
+
fail_resources:
pci_disable_device(pdev);

@@ -294,20 +343,18 @@
{
struct net_device *dev = pci_get_drvdata(pdev);
struct orinoco_private *priv = netdev_priv(dev);
+ struct orinoco_plx_card *card = priv->card;
+ u8 __iomem *attr_mem = card->attr_mem;

- unregister_netdev(dev);
-
- if (dev->irq)
- free_irq(dev->irq, dev);
+ BUG_ON(! dev);

- pci_iounmap(pdev, priv->hw.iobase);
-
+ unregister_netdev(dev);
+ free_irq(dev->irq, dev);
pci_set_drvdata(pdev, NULL);
-
free_orinocodev(dev);
-
- release_region(pci_resource_start(pdev, 3), pci_resource_len(pdev, 3));
-
+ pci_iounmap(pdev, priv->hw.iobase);
+ iounmap(attr_mem);
+ pci_release_regions(pdev);
pci_disable_device(pdev);
}

Index: working-2.6/drivers/net/wireless/orinoco_tmd.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco_tmd.c 2005-01-12 15:47:48.216477768 +1100
+++ working-2.6/drivers/net/wireless/orinoco_tmd.c 2004-11-05 14:37:35.000000000 +1100
@@ -79,59 +79,89 @@
#include "orinoco.h"

#define COR_VALUE (COR_LEVEL_REQ | COR_FUNC_ENA) /* Enable PC card with interrupt in level trigger */
+#define COR_RESET (0x80) /* reset bit in the COR register */
+#define TMD_RESET_TIME (500) /* milliseconds */
+
+/* Orinoco TMD specific data */
+struct orinoco_tmd_card {
+ u32 tmd_io;
+};
+
+
+/*
+ * Do a soft reset of the card using the Configuration Option Register
+ */
+static int orinoco_tmd_cor_reset(struct orinoco_private *priv)
+{
+ hermes_t *hw = &priv->hw;
+ struct orinoco_tmd_card *card = priv->card;
+ u32 addr = card->tmd_io;
+ unsigned long timeout;
+ u16 reg;
+
+ outb(COR_VALUE | COR_RESET, addr);
+ mdelay(1);
+
+ outb(COR_VALUE, addr);
+ mdelay(1);
+
+ /* Just in case, wait more until the card is no longer busy */
+ timeout = jiffies + (TMD_RESET_TIME * HZ / 1000);
+ reg = hermes_read_regn(hw, CMD);
+ while (time_before(jiffies, timeout) && (reg & HERMES_CMD_BUSY)) {
+ mdelay(1);
+ reg = hermes_read_regn(hw, CMD);
+ }
+
+ /* Did we timeout ? */
+ if (reg & HERMES_CMD_BUSY) {
+ printk(KERN_ERR PFX "Busy timeout\n");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+

static int orinoco_tmd_init_one(struct pci_dev *pdev,
const struct pci_device_id *ent)
{
int err = 0;
- u32 reg, addr;
struct orinoco_private *priv = NULL;
- unsigned long pccard_ioaddr = 0;
- unsigned long pccard_iolen = 0;
+ struct orinoco_tmd_card *card;
struct net_device *dev = NULL;
void __iomem *mem;

err = pci_enable_device(pdev);
if (err) {
printk(KERN_ERR PFX "Cannot enable PCI device\n");
- return -EIO;
+ return err;
}

- printk(KERN_DEBUG PFX "TMD setup\n");
- pccard_ioaddr = pci_resource_start(pdev, 2);
- pccard_iolen = pci_resource_len(pdev, 2);
- if (! request_region(pccard_ioaddr, pccard_iolen, DRIVER_NAME)) {
- printk(KERN_ERR PFX "I/O resource at 0x%lx len 0x%lx busy\n",
- pccard_ioaddr, pccard_iolen);
- err = -EBUSY;
- goto out;
+ err = pci_request_regions(pdev, DRIVER_NAME);
+ if (err != 0) {
+ printk(KERN_ERR PFX "Cannot obtain PCI resources\n");
+ goto fail_resources;
}
- addr = pci_resource_start(pdev, 1);
- outb(COR_VALUE, addr);
- mdelay(1);
- reg = inb(addr);
- if (reg != COR_VALUE) {
- printk(KERN_ERR PFX "Error setting TMD COR values %x should be %x\n", reg, COR_VALUE);
- err = -EIO;
- goto out2;
+
+ mem = pci_iomap(pdev, 2, 0);
+ if (! mem) {
+ err = -ENOMEM;
+ goto fail_iomap;
}

/* Allocate network device */
- dev = alloc_orinocodev(0, NULL);
+ dev = alloc_orinocodev(sizeof(*card), orinoco_tmd_cor_reset);
if (! dev) {
printk(KERN_ERR PFX "Cannot allocate network device\n");
err = -ENOMEM;
- goto out2;
- }
-
- mem = pci_iomap(pdev, 2, 0);
- if (!mem) {
- err = -ENOMEM;
- goto out3;
+ goto fail_alloc;
}

priv = netdev_priv(dev);
- dev->base_addr = pccard_ioaddr;
+ card = priv->card;
+ card->tmd_io = pci_resource_start(pdev, 1);
+ dev->base_addr = pci_resource_start(pdev, 2);
SET_MODULE_OWNER(dev);
SET_NETDEV_DEV(dev, &pdev->dev);

@@ -140,36 +170,46 @@

printk(KERN_DEBUG PFX "Detected Orinoco/Prism2 TMD device "
"at %s irq:%d, io addr:0x%lx\n", pci_name(pdev), pdev->irq,
- pccard_ioaddr);
+ dev->base_addr);

err = request_irq(pdev->irq, orinoco_interrupt, SA_SHIRQ,
dev->name, dev);
if (err) {
printk(KERN_ERR PFX "Cannot allocate IRQ %d\n", pdev->irq);
err = -EBUSY;
- goto out4;
+ goto fail_irq;
}
dev->irq = pdev->irq;

+ err = orinoco_tmd_cor_reset(priv);
+ if (err) {
+ printk(KERN_ERR PFX "Initial reset failed\n");
+ goto fail;
+ }
+
err = register_netdev(dev);
if (err) {
printk(KERN_ERR PFX "Cannot register network device\n");
- goto out5;
+ goto fail;
}

return 0;

-out5:
- free_irq(dev->irq, dev);
-out4:
- pci_iounmap(pdev, mem);
-out3:
+ fail:
+ free_irq(pdev->irq, dev);
+
+ fail_irq:
+ pci_set_drvdata(pdev, NULL);
free_orinocodev(dev);
-out2:
- release_region(pccard_ioaddr, pccard_iolen);
-out:
+
+ fail_alloc:
+ pci_iounmap(pdev, mem);
+
+ fail_iomap:
+ pci_release_regions(pdev);
+
+ fail_resources:
pci_disable_device(pdev);
- printk(KERN_DEBUG PFX "init_one(), FAIL!\n");

return err;
}
@@ -177,21 +217,16 @@
static void __devexit orinoco_tmd_remove_one(struct pci_dev *pdev)
{
struct net_device *dev = pci_get_drvdata(pdev);
- struct orinoco_private *priv = netdev_priv(dev);
+ struct orinoco_private *priv = dev->priv;

- unregister_netdev(dev);
-
- if (dev->irq)
- free_irq(dev->irq, dev);
-
- pci_iounmap(pdev, priv->hw.iobase);
+ BUG_ON(! dev);

+ unregister_netdev(dev);
+ free_irq(dev->irq, dev);
pci_set_drvdata(pdev, NULL);
-
free_orinocodev(dev);
-
- release_region(pci_resource_start(pdev, 2), pci_resource_len(pdev, 2));
-
+ pci_iounmap(pdev, priv->hw.iobase);
+ pci_release_regions(pdev);
pci_disable_device(pdev);
}



--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist. NOT _the_ _other_ _way_
| _around_!
http://www.ozlabs.org/people/dgibson

2005-01-12 05:45:50

by David Gibson

[permalink] [raw]
Subject: [1/8] orinoco driver updates

Reformats printk()s, comments, labels and other cosmetic strings in
the orinoco driver. Also moves, removes, and adds ratelimiting in
some places. Behavioural changes are trivial/cosmetic only. This
reduces the cosmetic/trivial differences between the current kernel
version, and the CVS version of the driver; one small step towards
full merge.

Signed-off-by: David Gibson <[email protected]>

Index: working-2.6/drivers/net/wireless/hermes.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/hermes.c 2005-01-12 11:13:41.000000000 +1100
+++ working-2.6/drivers/net/wireless/hermes.c 2005-01-12 14:47:25.791170120 +1100
@@ -48,6 +48,7 @@
#include <linux/delay.h>
#include <linux/init.h>
#include <linux/kernel.h>
+#include <linux/net.h>
#include <asm/errno.h>

#include "hermes.h"
@@ -232,13 +233,16 @@
err = hermes_issue_cmd(hw, cmd, parm0);
if (err) {
if (! hermes_present(hw)) {
- printk(KERN_WARNING "hermes @ %p: "
- "Card removed while issuing command.\n",
- hw->iobase);
+ if (net_ratelimit())
+ printk(KERN_WARNING "hermes @ %p: "
+ "Card removed while issuing command "
+ "0x%04x.\n", hw->iobase, cmd);
err = -ENODEV;
} else
- printk(KERN_ERR "hermes @ %p: Error %d issuing command.\n",
- hw->iobase, err);
+ if (net_ratelimit())
+ printk(KERN_ERR "hermes @ %p: "
+ "Error %d issuing command 0x%04x.\n",
+ hw->iobase, err, cmd);
goto out;
}

@@ -251,17 +255,16 @@
}

if (! hermes_present(hw)) {
- printk(KERN_WARNING "hermes @ %p: "
- "Card removed while waiting for command completion.\n",
- hw->iobase);
+ printk(KERN_WARNING "hermes @ %p: Card removed "
+ "while waiting for command 0x%04x completion.\n",
+ hw->iobase, cmd);
err = -ENODEV;
goto out;
}

if (! (reg & HERMES_EV_CMD)) {
- printk(KERN_ERR "hermes @ %p: "
- "Timeout waiting for command completion.\n",
- hw->iobase);
+ printk(KERN_ERR "hermes @ %p: Timeout waiting for "
+ "command 0x%04x completion.\n", hw->iobase, cmd);
err = -ETIMEDOUT;
goto out;
}
@@ -481,14 +484,13 @@
*length = rlength;

if (rtype != rid)
- printk(KERN_WARNING "hermes @ %p: "
- "hermes_read_ltv(): rid (0x%04x) does not match type (0x%04x)\n",
- hw->iobase, rid, rtype);
+ printk(KERN_WARNING "hermes @ %p: %s(): "
+ "rid (0x%04x) does not match type (0x%04x)\n",
+ hw->iobase, __FUNCTION__, rid, rtype);
if (HERMES_RECLEN_TO_BYTES(rlength) > bufsize)
printk(KERN_WARNING "hermes @ %p: "
"Truncating LTV record from %d to %d bytes. "
- "(rid=0x%04x, len=0x%04x)\n",
- hw->iobase,
+ "(rid=0x%04x, len=0x%04x)\n", hw->iobase,
HERMES_RECLEN_TO_BYTES(rlength), bufsize, rid, rlength);

nwords = min((unsigned)rlength - 1, bufsize / 2);
Index: working-2.6/drivers/net/wireless/orinoco_pci.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco_pci.c 2005-01-12 11:13:41.000000000 +1100
+++ working-2.6/drivers/net/wireless/orinoco_pci.c 2005-01-12 14:47:25.792169968 +1100
@@ -151,24 +151,18 @@

/* Assert the reset until the card notice */
hermes_write_regn(hw, PCI_COR, HERMES_PCI_COR_MASK);
- printk(KERN_NOTICE "Reset done");
timeout = jiffies + (HERMES_PCI_COR_ONT * HZ / 1000);
while(time_before(jiffies, timeout)) {
- printk(".");
mdelay(1);
}
- printk(";\n");
//mdelay(HERMES_PCI_COR_ONT);

/* Give time for the card to recover from this hard effort */
hermes_write_regn(hw, PCI_COR, 0x0000);
- printk(KERN_NOTICE "Clear Reset");
timeout = jiffies + (HERMES_PCI_COR_OFFT * HZ / 1000);
while(time_before(jiffies, timeout)) {
- printk(".");
mdelay(1);
}
- printk(";\n");
//mdelay(HERMES_PCI_COR_OFFT);

/* The card is ready when it's no longer busy */
@@ -183,7 +177,6 @@
printk(KERN_ERR PFX "Busy timeout\n");
return -ETIMEDOUT;
}
- printk(KERN_NOTICE "pci_cor : reg = 0x%X - %lX - %lX\n", reg, timeout, jiffies);

return 0;
}
@@ -202,15 +195,19 @@
struct net_device *dev = NULL;

err = pci_enable_device(pdev);
- if (err)
- return -EIO;
+ if (err) {
+ printk(KERN_ERR PFX "Cannot enable PCI device\n");
+ return err;
+ }

/* Resource 0 is mapped to the hermes registers */
pci_iorange = pci_resource_start(pdev, 0);
pci_iolen = pci_resource_len(pdev, 0);
pci_ioaddr = ioremap(pci_iorange, pci_iolen);
- if (! pci_iorange)
+ if (! pci_iorange) {
+ printk(KERN_ERR PFX "Cannot remap hardware registers\n");
goto fail;
+ }

/* Allocate network device */
dev = alloc_orinocodev(0, NULL);
@@ -226,18 +223,16 @@
SET_MODULE_OWNER(dev);
SET_NETDEV_DEV(dev, &pdev->dev);

- printk(KERN_DEBUG PFX
- "Detected Orinoco/Prism2 PCI device at %s, mem:0x%lX to 0x%lX -> 0x%p, irq:%d\n",
- pci_name(pdev), dev->mem_start, dev->mem_end, pci_ioaddr, pdev->irq);
-
hermes_struct_init(&priv->hw, pci_ioaddr, HERMES_32BIT_REGSPACING);
pci_set_drvdata(pdev, dev);

+ printk(KERN_DEBUG PFX "Detected device %s, mem:0x%lx-0x%lx, irq %d\n",
+ pci_name(pdev), dev->mem_start, dev->mem_end, pdev->irq);
+
err = request_irq(pdev->irq, orinoco_interrupt, SA_SHIRQ,
dev->name, dev);
if (err) {
- printk(KERN_ERR PFX "Error allocating IRQ %d.\n",
- pdev->irq);
+ printk(KERN_ERR PFX "Cannot allocate IRQ %d\n", pdev->irq);
err = -EBUSY;
goto fail;
}
@@ -245,7 +240,7 @@

/* Perform a COR reset to start the card */
if(orinoco_pci_cor_reset(priv) != 0) {
- printk(KERN_ERR "%s: Failed to start the card\n", dev->name);
+ printk(KERN_ERR PFX "Initial reset failed\n");
err = -ETIMEDOUT;
goto fail;
}
@@ -256,7 +251,7 @@

err = register_netdev(dev);
if (err) {
- printk(KERN_ERR "%s: Failed to register net device\n", dev->name);
+ printk(KERN_ERR PFX "Failed to register net device\n");
goto fail;
}

Index: working-2.6/drivers/net/wireless/orinoco.h
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco.h 2005-01-12 11:13:41.000000000 +1100
+++ working-2.6/drivers/net/wireless/orinoco.h 2005-01-12 14:47:25.792169968 +1100
@@ -127,7 +127,7 @@
{
spin_lock_irqsave(&priv->lock, *flags);
if (priv->hw_unavailable) {
- printk(KERN_DEBUG "orinoco_lock() called with hw_unavailable (dev=%p)\n",
+ DEBUG(1, "orinoco_lock() called with hw_unavailable (dev=%p)\n",
priv->ndev);
spin_unlock_irqrestore(&priv->lock, *flags);
return -EBUSY;
Index: working-2.6/drivers/net/wireless/orinoco_tmd.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco_tmd.c 2005-01-12 11:13:41.000000000 +1100
+++ working-2.6/drivers/net/wireless/orinoco_tmd.c 2005-01-12 14:47:25.793169816 +1100
@@ -92,8 +92,10 @@
void __iomem *mem;

err = pci_enable_device(pdev);
- if (err)
+ if (err) {
+ printk(KERN_ERR PFX "Cannot enable PCI device\n");
return -EIO;
+ }

printk(KERN_DEBUG PFX "TMD setup\n");
pccard_ioaddr = pci_resource_start(pdev, 2);
@@ -117,6 +119,7 @@
/* Allocate network device */
dev = alloc_orinocodev(0, NULL);
if (! dev) {
+ printk(KERN_ERR PFX "Cannot allocate network device\n");
err = -ENOMEM;
goto out2;
}
@@ -132,26 +135,27 @@
SET_MODULE_OWNER(dev);
SET_NETDEV_DEV(dev, &pdev->dev);

+ hermes_struct_init(&priv->hw, mem, HERMES_16BIT_REGSPACING);
+ pci_set_drvdata(pdev, dev);
+
printk(KERN_DEBUG PFX "Detected Orinoco/Prism2 TMD device "
"at %s irq:%d, io addr:0x%lx\n", pci_name(pdev), pdev->irq,
pccard_ioaddr);

- hermes_struct_init(&(priv->hw), mem, HERMES_16BIT_REGSPACING);
- pci_set_drvdata(pdev, dev);
-
err = request_irq(pdev->irq, orinoco_interrupt, SA_SHIRQ,
dev->name, dev);
if (err) {
- printk(KERN_ERR PFX "Error allocating IRQ %d.\n",
- pdev->irq);
+ printk(KERN_ERR PFX "Cannot allocate IRQ %d\n", pdev->irq);
err = -EBUSY;
goto out4;
}
dev->irq = pdev->irq;

err = register_netdev(dev);
- if (err)
+ if (err) {
+ printk(KERN_ERR PFX "Cannot register network device\n");
goto out5;
+ }

return 0;

Index: working-2.6/drivers/net/wireless/orinoco_cs.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco_cs.c 2005-01-12 11:13:41.000000000 +1100
+++ working-2.6/drivers/net/wireless/orinoco_cs.c 2005-01-12 14:47:25.793169816 +1100
@@ -405,7 +405,7 @@
last_ret = pcmcia_get_next_tuple(handle, &tuple);
if (last_ret == CS_NO_MORE_ITEMS) {
printk(KERN_ERR PFX "GetNextTuple(): No matching "
- "CIS configuration, maybe you need the "
+ "CIS configuration. Maybe you need the "
"ignore_cis_vcc=1 parameter.\n");
goto cs_failed;
}
Index: working-2.6/drivers/net/wireless/airport.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/airport.c 2005-01-12 11:13:41.000000000 +1100
+++ working-2.6/drivers/net/wireless/airport.c 2005-01-12 14:47:25.793169816 +1100
@@ -28,7 +28,6 @@
#include <linux/if_arp.h>
#include <linux/etherdevice.h>
#include <linux/wireless.h>
-#include <linux/delay.h>

#include <asm/io.h>
#include <asm/system.h>
@@ -194,14 +193,14 @@
hermes_t *hw;

if (macio_resource_count(mdev) < 1 || macio_irq_count(mdev) < 1) {
- printk(KERN_ERR PFX "wrong interrupt/addresses in OF tree\n");
+ printk(KERN_ERR PFX "Wrong interrupt/addresses in OF tree\n");
return -ENODEV;
}

/* Allocate space for private device-specific data */
dev = alloc_orinocodev(sizeof(*card), airport_hard_reset);
if (! dev) {
- printk(KERN_ERR PFX "can't allocate device datas\n");
+ printk(KERN_ERR PFX "Cannot allocate network device\n");
return -ENODEV;
}
priv = netdev_priv(dev);
@@ -224,11 +223,11 @@
/* Setup interrupts & base address */
dev->irq = macio_irq(mdev, 0);
phys_addr = macio_resource_start(mdev, 0); /* Physical address */
- printk(KERN_DEBUG PFX "Airport at physical address %lx\n", phys_addr);
+ printk(KERN_DEBUG PFX "Physical address %lx\n", phys_addr);
dev->base_addr = phys_addr;
card->vaddr = ioremap(phys_addr, AIRPORT_IO_LEN);
if (!card->vaddr) {
- printk(PFX "ioremap() failed\n");
+ printk(KERN_ERR PFX "ioremap() failed\n");
goto failed;
}

@@ -241,7 +240,7 @@
/* Reset it before we get the interrupt */
hermes_init(hw);

- if (request_irq(dev->irq, orinoco_interrupt, 0, "Airport", dev)) {
+ if (request_irq(dev->irq, orinoco_interrupt, 0, dev->name, dev)) {
printk(KERN_ERR PFX "Couldn't get IRQ %d\n", dev->irq);
goto failed;
}
@@ -252,7 +251,7 @@
printk(KERN_ERR PFX "register_netdev() failed\n");
goto failed;
}
- printk(KERN_DEBUG PFX "card registered for interface %s\n", dev->name);
+ printk(KERN_DEBUG PFX "Card registered for interface %s\n", dev->name);
card->ndev_registered = 1;
return 0;
failed:
Index: working-2.6/drivers/net/wireless/orinoco_plx.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco_plx.c 2005-01-12 11:13:41.000000000 +1100
+++ working-2.6/drivers/net/wireless/orinoco_plx.c 2005-01-12 14:48:57.890168944 +1100
@@ -167,15 +167,17 @@
int i;

err = pci_enable_device(pdev);
- if (err)
+ if (err) {
+ printk(KERN_ERR PFX "Cannot enable PCI device\n");
return -EIO;
+ }

/* Resource 2 is mapped to the PCMCIA space */
attr_mem = ioremap(pci_resource_start(pdev, 2), PAGE_SIZE);
if (!attr_mem)
- goto out;
+ goto fail_resources;

- printk(KERN_DEBUG "orinoco_plx: CIS: ");
+ printk(KERN_DEBUG PFX "CIS: ");
for (i = 0; i < 16; i++) {
printk("%02X:", readw(attr_mem+i));
}
@@ -185,10 +187,11 @@
/* FIXME: we probably need to be smarted about this */
memcpy_fromio(magic, attr_mem, 16);
if (memcmp(magic, cis_magic, 16) != 0) {
- printk(KERN_ERR "orinoco_plx: The CIS value of Prism2 PC card is invalid.\n");
+ printk(KERN_ERR PFX "The CIS value of Prism2 PC "
+ "card is unexpected\n");
err = -EIO;
iounmap(attr_mem);
- goto out;
+ goto fail_resources;
}

/* PCMCIA COR is the first byte following CIS: this write should
@@ -199,8 +202,8 @@
iounmap(attr_mem);

if (reg != COR_VALUE) {
- printk(KERN_ERR "orinoco_plx: Error setting COR value (reg=%x)\n", reg);
- goto out;
+ printk(KERN_ERR PFX "Error setting COR value (reg=%x)\n", reg);
+ goto fail_resources;
}

/* bjoern: We need to tell the card to enable interrupts, in
@@ -209,40 +212,39 @@
addr = pci_resource_start(pdev, 1);
reg = inl(addr+PLX_INTCSR);
if (reg & PLX_INTCSR_INTEN)
- printk(KERN_DEBUG "orinoco_plx: "
- "Local Interrupt already enabled\n");
+ printk(KERN_DEBUG PFX "Local Interrupt already enabled\n");
else {
reg |= PLX_INTCSR_INTEN;
outl(reg, addr+PLX_INTCSR);
reg = inl(addr+PLX_INTCSR);
if(!(reg & PLX_INTCSR_INTEN)) {
- printk(KERN_ERR "orinoco_plx: "
- "Couldn't enable Local Interrupts\n");
- goto out;
+ printk(KERN_ERR PFX "Couldn't enable Local Interrupts\n");
+ goto fail_resources;
}
}

- /* and 3 to the PCMCIA slot I/O address space */
+ /* Resource 3 is mapped to the PCMCIA I/O address space */
pccard_ioaddr = pci_resource_start(pdev, 3);
pccard_iolen = pci_resource_len(pdev, 3);
if (! request_region(pccard_ioaddr, pccard_iolen, DRIVER_NAME)) {
- printk(KERN_ERR "orinoco_plx: I/O resource 0x%lx @ 0x%lx busy\n",
+ printk(KERN_ERR PFX "I/O resource 0x%lx @ 0x%lx busy\n",
pccard_iolen, pccard_ioaddr);
err = -EBUSY;
- goto out;
+ goto fail_resources;
}

mem = pci_iomap(pdev, 3, 0);
if (!mem) {
err = -ENOMEM;
- goto out1;
+ goto fail_map;
}

/* Allocate network device */
dev = alloc_orinocodev(0, NULL);
if (!dev) {
+ printk(KERN_ERR PFX "Cannot allocate network device\n");
err = -ENOMEM;
- goto out2;
+ goto fail_alloc;
}

priv = netdev_priv(dev);
@@ -250,39 +252,41 @@
SET_MODULE_OWNER(dev);
SET_NETDEV_DEV(dev, &pdev->dev);

+ hermes_struct_init(&priv->hw, mem, HERMES_16BIT_REGSPACING);
+ pci_set_drvdata(pdev, dev);
+
printk(KERN_DEBUG PFX "Detected Orinoco/Prism2 PLX device "
"at %s irq:%d, io addr:0x%lx\n", pci_name(pdev), pdev->irq,
pccard_ioaddr);

- hermes_struct_init(&(priv->hw), mem, HERMES_16BIT_REGSPACING);
- pci_set_drvdata(pdev, dev);
-
err = request_irq(pdev->irq, orinoco_interrupt, SA_SHIRQ,
dev->name, dev);
if (err) {
- printk(KERN_ERR PFX "Error allocating IRQ %d.\n", pdev->irq);
+ printk(KERN_ERR PFX "Cannot allocate IRQ %d\n", pdev->irq);
err = -EBUSY;
- goto out3;
+ goto fail_irq;
}
dev->irq = pdev->irq;

err = register_netdev(dev);
- if (err)
- goto out4;
+ if (err) {
+ printk(KERN_ERR PFX "Cannot register network device\n");
+ goto fail;
+ }

return 0;

-out4:
+ fail:
free_irq(dev->irq, dev);
-out3:
+ fail_irq:
free_netdev(dev);
-out2:
+ fail_alloc:
pci_iounmap(pdev, mem);
-out1:
+ fail_map:
release_region(pccard_ioaddr, pccard_iolen);
-out:
+ fail_resources:
pci_disable_device(pdev);
- printk(KERN_DEBUG PFX "init_one(), FAIL!\n");
+
return err;
}

Index: working-2.6/drivers/net/wireless/orinoco.c
===================================================================
--- working-2.6.orig/drivers/net/wireless/orinoco.c 2005-01-12 11:13:41.000000000 +1100
+++ working-2.6/drivers/net/wireless/orinoco.c 2005-01-12 14:47:25.796169360 +1100
@@ -805,8 +805,9 @@
desc.tx_control = cpu_to_le16(HERMES_TXCTRL_TX_OK | HERMES_TXCTRL_TX_EX);
err = hermes_bap_pwrite(hw, USER_BAP, &desc, sizeof(desc), txfid, 0);
if (err) {
- printk(KERN_ERR "%s: Error %d writing Tx descriptor to BAP\n",
- dev->name, err);
+ if (net_ratelimit())
+ printk(KERN_ERR "%s: Error %d writing Tx descriptor "
+ "to BAP\n", dev->name, err);
stats->tx_errors++;
goto fail;
}
@@ -836,8 +837,9 @@
err = hermes_bap_pwrite(hw, USER_BAP, &hdr, sizeof(hdr),
txfid, HERMES_802_3_OFFSET);
if (err) {
- printk(KERN_ERR "%s: Error %d writing packet header to BAP\n",
- dev->name, err);
+ if (net_ratelimit())
+ printk(KERN_ERR "%s: Error %d writing packet "
+ "header to BAP\n", dev->name, err);
stats->tx_errors++;
goto fail;
}
@@ -1297,8 +1299,8 @@
}
break;
default:
- printk(KERN_DEBUG "%s: Unknown information frame received "
- "(type %04x).\n", dev->name, type);
+ printk(KERN_DEBUG "%s: Unknown information frame received: "
+ "type 0x%04x, length %d\n", dev->name, type, len);
/* We don't actually do anything about it */
break;
}
@@ -1307,7 +1309,7 @@
static void __orinoco_ev_infdrop(struct net_device *dev, hermes_t *hw)
{
if (net_ratelimit())
- printk(KERN_WARNING "%s: Information frame lost.\n", dev->name);
+ printk(KERN_DEBUG "%s: Information frame lost.\n", dev->name);
}

/********************************************************************/
@@ -1785,7 +1787,8 @@
}

if (p)
- printk(KERN_WARNING "Multicast list is longer than mc_count\n");
+ printk(KERN_WARNING "%s: Multicast list is "
+ "longer than mc_count\n", dev->name);

err = hermes_write_ltv(hw, USER_BAP, HERMES_RID_CNFGROUPADDRESSES,
HERMES_BYTES_TO_RECLEN(priv->mc_count * ETH_ALEN),
@@ -2053,7 +2056,7 @@
le16_to_cpus(&sta_id.variant);
le16_to_cpus(&sta_id.major);
le16_to_cpus(&sta_id.minor);
- printk(KERN_DEBUG "%s: Station identity %04x:%04x:%04x:%04x\n",
+ printk(KERN_DEBUG "%s: Station identity %04x:%04x:%04x:%04x\n",
dev->name, sta_id.id, sta_id.variant,
sta_id.major, sta_id.minor);

@@ -3045,8 +3048,9 @@
priv->mwo_robust = 0;
else {
if (frq->fixed)
- printk(KERN_WARNING "%s: Fixed fragmentation not \
-supported on this firmware. Using MWO robust instead.\n", dev->name);
+ printk(KERN_WARNING "%s: Fixed fragmentation is "
+ "not supported on this firmware. "
+ "Using MWO robust instead.\n", dev->name);
priv->mwo_robust = 1;
}
} else {
@@ -3518,7 +3522,7 @@
}
/* Copy stats */
/* In theory, we should disable irqs while copying the stats
- * because the rx path migh update it in the middle...
+ * because the rx path might update it in the middle...
* Bah, who care ? - Jean II */
memcpy(&spy_stat, priv->spy_stat,
sizeof(struct iw_quality) * IW_MAX_SPY);


--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist. NOT _the_ _other_ _way_
| _around_!
http://www.ozlabs.org/people/dgibson

2005-02-02 01:48:50

by Jeff Garzik

[permalink] [raw]
Subject: Re: [0/8] orinoco driver updates

David Gibson wrote:
> Following are a bunch of patches which make a few more steps towards
> the long overdue merge of the CVS orinoco driver into mainline. These
> do make behavioural changes to the driver, but they should all be
> trivial and largely cosmetic.

OK, the changes look good, but I was waiting for the previous stuff you
submitted to land in upstream.

Could I convince you to rediff and resend against the latest 2.6.x snapshot?

At the time you sent these, they conflicted with a previous cleanup you
had sent, and that I had applied.

Jeff



2005-02-10 02:53:58

by David Gibson

[permalink] [raw]
Subject: Re: [0/8] orinoco driver updates

On Tue, Feb 01, 2005 at 08:48:31PM -0500, Jeff Garzik wrote:
> David Gibson wrote:
> >Following are a bunch of patches which make a few more steps towards
> >the long overdue merge of the CVS orinoco driver into mainline. These
> >do make behavioural changes to the driver, but they should all be
> >trivial and largely cosmetic.
>
> OK, the changes look good, but I was waiting for the previous stuff you
> submitted to land in upstream.
>
> Could I convince you to rediff and resend against the latest 2.6.x snapshot?
>
> At the time you sent these, they conflicted with a previous cleanup you
> had sent, and that I had applied.

Ok, I'm a little confused. The last batch were all checked against
the then-current netdev-2.6 bk tree, or was the conflict with
something you'd applied between when I pulled the tree and you looked
at the patches? As far as I can tell all the previous batch of
patches I sent you are in netdev (plus a couple from this batch), but
most are still not in upstream.

I'm now not entirely clear on whether you want patches against the
netdev bk, or against Linus bk/snapshots.

--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist. NOT _the_ _other_ _way_
| _around_!
http://www.ozlabs.org/people/dgibson

2005-02-10 03:07:31

by Jeff Garzik

[permalink] [raw]
Subject: Re: [0/8] orinoco driver updates

David Gibson wrote:
> I'm now not entirely clear on whether you want patches against the
> netdev bk, or against Linus bk/snapshots.


Please send diff'd against vanilla Linus upstream.

Jeff


2005-02-10 03:23:13

by David Gibson

[permalink] [raw]
Subject: Re: [0/8] orinoco driver updates

On Wed, Feb 09, 2005 at 10:07:08PM -0500, Jeff Garzik wrote:
> David Gibson wrote:
> >I'm now not entirely clear on whether you want patches against the
> >netdev bk, or against Linus bk/snapshots.
>
>
> Please send diff'd against vanilla Linus upstream.

Ok, in that case do you want me to resend the things which are in
netdev but not vanilla?

--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist. NOT _the_ _other_ _way_
| _around_!
http://www.ozlabs.org/people/dgibson