RE: [microblaze-uclinux] xilinx_gpio driver patch

["Jeff Fellin" <jeff.fellin@xxxxxxxxxxxxxxxxxxx>]

-----Original Message-----
From: owner-microblaze-uclinux@xxxxxxxxxxxxxx
[mailto:owner-microblaze-uclinux@xxxxxxxxxxxxxx] On Behalf Of John Williams
Sent: Tuesday, July 31, 2007 6:28 PM
To: microblaze-uclinux@xxxxxxxxxxxxxx
Subject: Re: [microblaze-uclinux] xilinx_gpio driver patch

Hi Jeff,

Jeff Fellin wrote:
> When configuring my system with three Xilinx Gpio devices using
> drivers/char/xilinx_gpio, my system would hang during bootup. I isolated
the
> problem to the xilinx_gpio driver, on an error condition in xgpio_probe.
> When the second device was probed, the request_irq failed leaving the
> inst_list_sem locked, so when the third probe started it blocked on
> acquiring the lock. 

Just having a closer look, I think there is still a little more to be 
done here.  There are a couple of errors in the failure path out of the 
probe() function that we should fix.

* The !miscdev test (line 401) should set retval and goto failed3, 
instead of returning immediately (currently would do so with lock held).

* The test after the misc_register call (line 412-415) should not do the 
up_write - that should be handled by the failed3: exit path

Also, I'm wondering why your call to request_irq failed in the first 
place.  Have you doubled up multiple GPIO instances on the same IRQ, or 
something else?

What do you think?

John

========== Reply
John,
I agree with the observations about the other places in probe, lines 401,
and 412-415. I'm enclosing a patch to be applied at
petalinux-dist/linux-2.6.x. 

I don't yet know why my request_irq failed, I thought I was requesting a
duplicate IRQ, but my .config has no duplicate IRQ's. Yet another puzzle to
solve.

Jeff Fellin
--- drivers/char/xilinx_gpio/adapter.c  2007-07-31 19:42:53.000000000 -0400
+++ drivers/char/xilinx_gpio/Oadapter.c 2007-07-30 18:16:49.000000000 -0400
@@ -401,8 +401,7 @@
                printk(KERN_ERR
                       "%s #%d: Couldn't allocate device private record\n",
                       "xgpio", pdev->id);
-               retval = -ENOMEM;
-        goto failed3;
+               return -ENOMEM;
        }

        miscdev->minor = minor;
@@ -410,6 +409,7 @@
        miscdev->fops = &xgpio_fops;
        retval = misc_register(miscdev);
        if (retval != 0) {
+               up_write(&inst_list_sem);
                printk(KERN_ERR "%s #%d: Could not register miscdev.\n",
                       miscdev->name, pdev->id);
                goto failed3;
@@ -449,7 +449,6 @@
        minor--;

       failed3:
-       up_write(&inst_list_sem);
        iounmap((void *)(xgpio_config.BaseAddress));

       failed2:

___________________________
microblaze-uclinux mailing list
microblaze-uclinux@xxxxxxxxxxxxxx
Project Home Page : http://www.itee.uq.edu.au/~jwilliams/mblaze-uclinux
Mailing List Archive :
http://www.itee.uq.edu.au/~listarch/microblaze-uclinux/

___________________________
microblaze-uclinux mailing list
microblaze-uclinux@xxxxxxxxxxxxxx
Project Home Page : http://www.itee.uq.edu.au/~jwilliams/mblaze-uclinux
Mailing List Archive : http://www.itee.uq.edu.au/~listarch/microblaze-uclinux/