2007-10-19 16:52:07

by Bjorn Helgaas

[permalink] [raw]
Subject: PCMCIA driver resource allocation

Question 1: Does the linux-pcmcia list still exist? It's in MAINTAINERS:

PCMCIA SUBSYSTEM
P: Linux PCMCIA Team
L: [email protected]
L: http://lists.infradead.org/mailman/listinfo/linux-pcmcia
T: git kernel.org:/pub/scm/linux/kernel/git/brodo/pcmcia-2.6.git
S: Maintained

but the archive: http://lists.infradead.org/mailman/listinfo/linux-pcmcia
seems dead.

Question 2: Documentation/pcmcia/driver-changes.txt says drivers should
now claim their own resources:

* Resource management. (as of 2.6.8)
Although the PCMCIA subsystem will allocate resources for cards,
it no longer marks these resources busy. This means that driver
authors are now responsible for claiming your resources as per
other drivers in Linux.

But I don't see any drivers that do that. It looks like there should
be a bunch of changes like the one below. Is there a reason these
changes didn't happen, other than just lack of interest?

Bjorn


Index: work3/drivers/net/wireless/orinoco.h
===================================================================
--- work3.orig/drivers/net/wireless/orinoco.h 2007-10-18 10:56:34.000000000 -0600
+++ work3/drivers/net/wireless/orinoco.h 2007-10-18 13:22:06.000000000 -0600
@@ -57,6 +57,7 @@
struct iw_statistics wstats;

/* Hardware control variables */
+ struct resource *io_resource;
hermes_t hw;
u16 txfid;

Index: work3/drivers/net/wireless/orinoco_cs.c
===================================================================
--- work3.orig/drivers/net/wireless/orinoco_cs.c 2007-10-18 10:56:34.000000000 -0600
+++ work3/drivers/net/wireless/orinoco_cs.c 2007-10-18 13:22:44.000000000 -0600
@@ -296,6 +296,10 @@
/* We initialize the hermes structure before completing PCMCIA
* configuration just in case the interrupt handler gets
* called. */
+ priv->io_resource = request_region(link->io.BasePort1,
+ link->io.NumPorts1, DRIVER_NAME);
+ if (!priv->io_resource)
+ goto cs_failed;
mem = ioport_map(link->io.BasePort1, link->io.NumPorts1);
if (!mem)
goto cs_failed;
@@ -366,6 +370,10 @@
pcmcia_disable_device(link);
if (priv->hw.iobase)
ioport_unmap(priv->hw.iobase);
+ if (priv->io_resource) {
+ release_resource(priv->io_resource);
+ priv->io_resource = NULL;
+ }
} /* orinoco_cs_release */

static int orinoco_cs_suspend(struct pcmcia_device *link)


2007-10-19 22:41:11

by Russell King

[permalink] [raw]
Subject: Re: PCMCIA driver resource allocation

On Fri, Oct 19, 2007 at 10:51:51AM -0600, Bjorn Helgaas wrote:
> Question 1: Does the linux-pcmcia list still exist? It's in MAINTAINERS:
>
> PCMCIA SUBSYSTEM
> P: Linux PCMCIA Team
> L: [email protected]
> L: http://lists.infradead.org/mailman/listinfo/linux-pcmcia
> T: git kernel.org:/pub/scm/linux/kernel/git/brodo/pcmcia-2.6.git
> S: Maintained
>
> but the archive: http://lists.infradead.org/mailman/listinfo/linux-pcmcia
> seems dead.

The list is still around, but Dominik seems to have vanished.

> Question 2: Documentation/pcmcia/driver-changes.txt says drivers should
> now claim their own resources:
>
> * Resource management. (as of 2.6.8)
> Although the PCMCIA subsystem will allocate resources for cards,
> it no longer marks these resources busy. This means that driver
> authors are now responsible for claiming your resources as per
> other drivers in Linux.
>
> But I don't see any drivers that do that. It looks like there should
> be a bunch of changes like the one below. Is there a reason these
> changes didn't happen, other than just lack of interest?

That's from around the time that I handed PCMCIA over to Dominik, and was
a to-do item. I had some drivers converted over - mainly the few that I
was using, those being serial and pcnet_cs (serial is converted over but
the patch I had for pcnet_cs is below.)

However, in spite of me pointing Dominik at my remaining patch sets several
times, as far as I could tell they got ignored. So essentially I all lost
interest in helping out with PCMCIA.

> Index: work3/drivers/net/wireless/orinoco_cs.c
> ===================================================================
> --- work3.orig/drivers/net/wireless/orinoco_cs.c 2007-10-18 10:56:34.000000000 -0600
> +++ work3/drivers/net/wireless/orinoco_cs.c 2007-10-18 13:22:44.000000000 -0600
> @@ -296,6 +296,10 @@
> /* We initialize the hermes structure before completing PCMCIA
> * configuration just in case the interrupt handler gets
> * called. */
> + priv->io_resource = request_region(link->io.BasePort1,
> + link->io.NumPorts1, DRIVER_NAME);
> + if (!priv->io_resource)
> + goto cs_failed;
> mem = ioport_map(link->io.BasePort1, link->io.NumPorts1);
> if (!mem)
> goto cs_failed;
> @@ -366,6 +370,10 @@
> pcmcia_disable_device(link);
> if (priv->hw.iobase)
> ioport_unmap(priv->hw.iobase);
> + if (priv->io_resource) {
> + release_resource(priv->io_resource);
> + priv->io_resource = NULL;

Wrong function. release_resource() doesn't pair with request_region().
request_region() allocates memory for the struct resource.
release_resource() merely removes the struct resource from the tree.
release_region() on the other hand removes the struct resource and
frees it.

---

Convert pcnet_cs and serial_cs to request their IO regions, thereby
marking them busy. These are only two of many drivers which need this
update.

diff -u -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x '*.orig' -x '*.rej' ref/drivers/net/pcmcia/pcnet_cs.c linux/drivers/net/pcmcia/pcnet_cs.c
--- ref/drivers/net/pcmcia/pcnet_cs.c Sun Nov 16 19:12:33 2003
+++ linux/drivers/net/pcmcia/pcnet_cs.c Tue Dec 23 09:43:22 2003
@@ -687,8 +687,15 @@
dev->poll_controller = ei_poll;
#endif

+ if (!request_region(dev->base_addr, link->io.NumPorts1, "pcnet_cs")) {
+ printk(KERN_NOTICE "pcnet_cs: request_region() failed\n");
+ link->dev = NULL;
+ goto failed;
+ }
+
if (register_netdev(dev) != 0) {
printk(KERN_NOTICE "pcnet_cs: register_netdev() failed\n");
+ release_region(dev->base_addr, link->io.NumPorts1);
link->dev = NULL;
goto failed;
}
@@ -736,6 +743,9 @@

DEBUG(0, "pcnet_release(0x%p)\n", link);

+ if (link->dev)
+ release_region(link->io.BasePort1, link->io.NumPorts1);
+
if (info->flags & USE_SHMEM) {
iounmap(info->base);
pcmcia_release_window(link->win);


--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:

2007-10-21 08:38:33

by Kristoffer Ericson

[permalink] [raw]
Subject: Re: PCMCIA driver resource allocation

On Fri, 19 Oct 2007 23:40:22 +0100
Russell King <[email protected]> wrote:

Ive been meaning to talk to dominik about some hp6xx pcmcia issues Im having, but seem to be quite hard to get in touch with him or the linux-pcmcia list.

> On Fri, Oct 19, 2007 at 10:51:51AM -0600, Bjorn Helgaas wrote:
> > Question 1: Does the linux-pcmcia list still exist? It's in MAINTAINERS:
> >
> > PCMCIA SUBSYSTEM
> > P: Linux PCMCIA Team
> > L: [email protected]
> > L: http://lists.infradead.org/mailman/listinfo/linux-pcmcia
> > T: git kernel.org:/pub/scm/linux/kernel/git/brodo/pcmcia-2.6.git
> > S: Maintained
> >
> > but the archive: http://lists.infradead.org/mailman/listinfo/linux-pcmcia
> > seems dead.
>
> The list is still around, but Dominik seems to have vanished.
Ive tried to subscribbe to the list, but with no luck.

>
> > Question 2: Documentation/pcmcia/driver-changes.txt says drivers should
> > now claim their own resources:
> >
> > * Resource management. (as of 2.6.8)
> > Although the PCMCIA subsystem will allocate resources for cards,
> > it no longer marks these resources busy. This means that driver
> > authors are now responsible for claiming your resources as per
> > other drivers in Linux.
> >
> > But I don't see any drivers that do that. It looks like there should
> > be a bunch of changes like the one below. Is there a reason these
> > changes didn't happen, other than just lack of interest?
>
> That's from around the time that I handed PCMCIA over to Dominik, and was
> a to-do item. I had some drivers converted over - mainly the few that I
> was using, those being serial and pcnet_cs (serial is converted over but
> the patch I had for pcnet_cs is below.)
>
> However, in spite of me pointing Dominik at my remaining patch sets several
> times, as far as I could tell they got ignored. So essentially I all lost
> interest in helping out with PCMCIA.
>
> > Index: work3/drivers/net/wireless/orinoco_cs.c
> > ===================================================================
> > --- work3.orig/drivers/net/wireless/orinoco_cs.c 2007-10-18 10:56:34.000000000 -0600
> > +++ work3/drivers/net/wireless/orinoco_cs.c 2007-10-18 13:22:44.000000000 -0600
> > @@ -296,6 +296,10 @@
> > /* We initialize the hermes structure before completing PCMCIA
> > * configuration just in case the interrupt handler gets
> > * called. */
> > + priv->io_resource = request_region(link->io.BasePort1,
> > + link->io.NumPorts1, DRIVER_NAME);
> > + if (!priv->io_resource)
> > + goto cs_failed;
> > mem = ioport_map(link->io.BasePort1, link->io.NumPorts1);
> > if (!mem)
> > goto cs_failed;
> > @@ -366,6 +370,10 @@
> > pcmcia_disable_device(link);
> > if (priv->hw.iobase)
> > ioport_unmap(priv->hw.iobase);
> > + if (priv->io_resource) {
> > + release_resource(priv->io_resource);
> > + priv->io_resource = NULL;
>
> Wrong function. release_resource() doesn't pair with request_region().
> request_region() allocates memory for the struct resource.
> release_resource() merely removes the struct resource from the tree.
> release_region() on the other hand removes the struct resource and
> frees it.
>
> ---
>
> Convert pcnet_cs and serial_cs to request their IO regions, thereby
> marking them busy. These are only two of many drivers which need this
> update.
>
> diff -u -x BitKeeper -x ChangeSet -x SCCS -x _xlk -x '*.orig' -x '*.rej' ref/drivers/net/pcmcia/pcnet_cs.c linux/drivers/net/pcmcia/pcnet_cs.c
> --- ref/drivers/net/pcmcia/pcnet_cs.c Sun Nov 16 19:12:33 2003
> +++ linux/drivers/net/pcmcia/pcnet_cs.c Tue Dec 23 09:43:22 2003
> @@ -687,8 +687,15 @@
> dev->poll_controller = ei_poll;
> #endif
>
> + if (!request_region(dev->base_addr, link->io.NumPorts1, "pcnet_cs")) {
> + printk(KERN_NOTICE "pcnet_cs: request_region() failed\n");
> + link->dev = NULL;
> + goto failed;
> + }
> +
> if (register_netdev(dev) != 0) {
> printk(KERN_NOTICE "pcnet_cs: register_netdev() failed\n");
> + release_region(dev->base_addr, link->io.NumPorts1);
> link->dev = NULL;
> goto failed;
> }
> @@ -736,6 +743,9 @@
>
> DEBUG(0, "pcnet_release(0x%p)\n", link);
>
> + if (link->dev)
> + release_region(link->io.BasePort1, link->io.NumPorts1);
> +
> if (info->flags & USE_SHMEM) {
> iounmap(info->base);
> pcmcia_release_window(link->win);
>
>
> --
> Russell King
> Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
> maintainer of:
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/


--
Kristoffer Ericson <[email protected]>

2007-10-21 08:42:45

by Russell King

[permalink] [raw]
Subject: Re: PCMCIA driver resource allocation

On Sun, Oct 21, 2007 at 10:46:17AM -0700, Kristoffer Ericson wrote:
> On Fri, 19 Oct 2007 23:40:22 +0100
> Russell King <[email protected]> wrote:
> > The list is still around, but Dominik seems to have vanished.
> Ive tried to subscribbe to the list, but with no luck.

You might want to talk to David Woodhouse about that - he runs the
mailing lists on lists.infradead.org.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:

2007-10-22 15:28:36

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: PCMCIA driver resource allocation

On Friday 19 October 2007 04:40:22 pm Russell King wrote:
> On Fri, Oct 19, 2007 at 10:51:51AM -0600, Bjorn Helgaas wrote:

> > + priv->io_resource = request_region(link->io.BasePort1,
> > + link->io.NumPorts1, DRIVER_NAME);
> > + if (!priv->io_resource)
> > + goto cs_failed;
> > mem = ioport_map(link->io.BasePort1, link->io.NumPorts1);
> > if (!mem)
> > goto cs_failed;
> > @@ -366,6 +370,10 @@
> > pcmcia_disable_device(link);
> > if (priv->hw.iobase)
> > ioport_unmap(priv->hw.iobase);
> > + if (priv->io_resource) {
> > + release_resource(priv->io_resource);
> > + priv->io_resource = NULL;
>
> Wrong function. release_resource() doesn't pair with request_region().
> request_region() allocates memory for the struct resource.
> release_resource() merely removes the struct resource from the tree.
> release_region() on the other hand removes the struct resource and
> frees it.

Oh, thanks! I didn't notice that difference between release_region()
and release_resource(). I'll fix the patch.

Bjorn