PCMCIA driver resource allocation

PCMCIA driver resource allocation

[Bjorn Helgaas]

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

  PCMCIA SUBSYSTEM
  P:      Linux PCMCIA Team
  L:      linux-pcmcia@lists.infradead.org
  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)

Re: PCMCIA driver resource allocation

[Russell King]

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:      linux-pcmcia@lists.infradead.org
>   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:

Re: PCMCIA driver resource allocation

[Russell King]

On Sun, Oct 21, 2007 at 10:46:17AM -0700, Kristoffer Ericson wrote:
> On Fri, 19 Oct 2007 23:40:22 +0100
> Russell King <rmk+lkml@arm.linux.org.uk> 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:

Re: PCMCIA driver resource allocation

[Kristoffer Ericson]

On Fri, 19 Oct 2007 23:40:22 +0100
Russell King <rmk+lkml@arm.linux.org.uk> 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:      linux-pcmcia@lists.infradead.org
> >   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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


-- 
Kristoffer Ericson <Kristoffer.Ericson@Gmail.com>

Re: PCMCIA driver resource allocation

[Bjorn Helgaas]

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