Re: [PATCH 089/128] USB: at91_udc uses generic GPIO calls; minor cleanup

Fri, 01 Feb 2008 18:18:21 -0800 

Hi Guys,
I have checked this patch on a board that inverts the signal via a transistor: 
I.E  the UcDragon YL9200
The patch appears to be functioning correctly.
The reason they did this was to allow greater driving potential via a transistor, rather than stressing the Atmel chip directly. Unfortunately...... the transistor should have been 'NPN' rather than the 'PNP' they used. But we do at least now have a system to allow manufacturing production variation .
Thanks again goes to David B., for incorporating this change, so rapidly. 

Steve



On Feb 2, 2008, at 7:18 AM, Greg Kroah-Hartman wrote:


From: David Brownell <[EMAIL PROTECTED]>

Various small at91_udc cleanups:

 - Use generic GPIO calls, not older platform-specific ones
 - Use gpio_request()/gpio_free()
 - Use VERBOSE_DEBUG convention, not older VERBOSE
 - Fix sparse complaint about parameter type (changed to gfp_t)
 - Add missing newline to some rarely-seen debug messages
 - Fix some old cleanup bugs on probe() fault paths

Also add a mechanism whereby rm9200 gpios can drive the D+ pullup
through an inverting transistor, based on a patch from Steve Birtles.
Most UDC drivers supporting a GPIO based pullup should probably have
such an option, but testing it requries such a board in hand!

Signed-off-by: David Brownell <[EMAIL PROTECTED]>
Cc: Steve Birtles <[EMAIL PROTECTED]>
Signed-off-by: Greg Kroah-Hartman <[EMAIL PROTECTED]>
---
drivers/usb/gadget/at91_udc.c | 79 ++++++++++++++++++++++++++ ++--------- 
 drivers/usb/gadget/at91_udc.h     |    2 +-
 include/asm-arm/arch-at91/board.h |    3 +-
 3 files changed, 63 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/ at91_udc.c 
index 305db36..a83e8b7 100644
--- a/drivers/usb/gadget/at91_udc.c
+++ b/drivers/usb/gadget/at91_udc.c
@@ -21,8 +21,7 @@
  * Boston, MA  02111-1307, USA.
  */

-#undef DEBUG
-#undef VERBOSE
+#undef VERBOSE_DEBUG
 #undef PACKET_TRACE

 #include <linux/kernel.h>
@@ -46,8 +45,8 @@
 #include <asm/irq.h>
 #include <asm/system.h>
 #include <asm/mach-types.h>
+#include <asm/gpio.h>

-#include <asm/arch/gpio.h>
 #include <asm/arch/board.h>
 #include <asm/arch/cpu.h>
 #include <asm/arch/at91sam9261_matrix.h>
@@ -580,7 +579,7 @@ static int at91_ep_disable (struct usb_ep * _ep)
  */

 static struct usb_request *
-at91_ep_alloc_request(struct usb_ep *_ep, unsigned int gfp_flags)
+at91_ep_alloc_request(struct usb_ep *_ep, gfp_t gfp_flags)
 {
        struct at91_request *req;

@@ -881,6 +880,8 @@ static void clk_off(struct at91_udc *udc)
  */
 static void pullup(struct at91_udc *udc, int is_on)
 {
+       int     active = !udc->board.pullup_active_low;
+
        if (!udc->enabled || !udc->vbus)
                is_on = 0;
        DBG("%sactive\n", is_on ? "" : "in");
@@ -890,7 +891,7 @@ static void pullup(struct at91_udc *udc, int is_on) 
                at91_udp_write(udc, AT91_UDP_ICR, AT91_UDP_RXRSM);
                at91_udp_write(udc, AT91_UDP_TXVC, 0);
                if (cpu_is_at91rm9200())
-                       at91_set_gpio_value(udc->board.pullup_pin, 1);
+                       gpio_set_value(udc->board.pullup_pin, active);
                else if (cpu_is_at91sam9260() || cpu_is_at91sam9263()) {
                        u32     txvc = at91_udp_read(udc, AT91_UDP_TXVC);
@@ -908,7 +909,7 @@ static void pullup(struct at91_udc *udc, int is_on) 
                at91_udp_write(udc, AT91_UDP_IDR, AT91_UDP_RXRSM);
                at91_udp_write(udc, AT91_UDP_TXVC, AT91_UDP_TXVC_TXVDIS);
                if (cpu_is_at91rm9200())
-                       at91_set_gpio_value(udc->board.pullup_pin, 0);
+                       gpio_set_value(udc->board.pullup_pin, !active);
                else if (cpu_is_at91sam9260() || cpu_is_at91sam9263()) {
                        u32     txvc = at91_udp_read(udc, AT91_UDP_TXVC);
@@ -1551,7 +1552,7 @@ static irqreturn_t at91_vbus_irq(int irq, void *_udc) 

        /* vbus needs at least brief debouncing */
        udelay(10);
-       value = at91_get_gpio_value(udc->board.vbus_pin);
+       value = gpio_get_value(udc->board.vbus_pin);
        if (value != udc->vbus)
                at91_vbus_session(&udc->gadget, value);
@@ -1647,12 +1648,12 @@ static int __init at91udc_probe(struct platform_device *pdev) 
        }

        if (pdev->num_resources != 2) {
-               DBG("invalid num_resources");
+               DBG("invalid num_resources\n");
                return -ENODEV;
        }
        if ((pdev->resource[0].flags != IORESOURCE_MEM)
                        || (pdev->resource[1].flags != IORESOURCE_IRQ)) {
-               DBG("invalid resource type");
+               DBG("invalid resource type\n");
                return -ENODEV;
        }
@@ -1674,10 +1675,26 @@ static int __init at91udc_probe(struct platform_device *pdev) 
        udc->pdev = pdev;
        udc->enabled = 0;

+       /* rm9200 needs manual D+ pullup; off by default */
+       if (cpu_is_at91rm9200()) {
+               if (udc->board.pullup_pin <= 0) {
+                       DBG("no D+ pullup?\n");
+                       retval = -ENODEV;
+                       goto fail0;
+               }
+               retval = gpio_request(udc->board.pullup_pin, "udc_pullup");
+               if (retval) {
+                       DBG("D+ pullup is busy\n");
+                       goto fail0;
+               }
+               gpio_direction_output(udc->board.pullup_pin,
+                               udc->board.pullup_active_low);
+       }
+
        udc->udp_baseaddr = ioremap(res->start, res->end - res->start + 1);
        if (!udc->udp_baseaddr) {
-               release_mem_region(res->start, res->end - res->start + 1);
-               return -ENOMEM;
+               retval = -ENOMEM;
+               goto fail0a;
        }

        udc_reinit(udc);
@@ -1688,12 +1705,13 @@ static int __init at91udc_probe(struct platform_device *pdev) 
        if (IS_ERR(udc->iclk) || IS_ERR(udc->fclk)) {
                DBG("clocks missing\n");
                retval = -ENODEV;
-               goto fail0;
+               /* NOTE: we "know" here that refcounts on these are NOPs */
+               goto fail0b;
        }

        retval = device_register(&udc->gadget.dev);
        if (retval < 0)
-               goto fail0;
+               goto fail0b;

        /* don't do anything until we have both gadget driver and VBUS */
        clk_enable(udc->iclk);
@@ -1705,25 +1723,32 @@ static int __init at91udc_probe(struct platform_device *pdev) 

        /* request UDC and maybe VBUS irqs */
        udc->udp_irq = platform_get_irq(pdev, 0);
-       if (request_irq(udc->udp_irq, at91_udc_irq,
-                       IRQF_DISABLED, driver_name, udc)) {
+       retval = request_irq(udc->udp_irq, at91_udc_irq,
+                       IRQF_DISABLED, driver_name, udc);
+       if (retval < 0) {
                DBG("request irq %d failed\n", udc->udp_irq);
-               retval = -EBUSY;
                goto fail1;
        }
        if (udc->board.vbus_pin > 0) {
+               retval = gpio_request(udc->board.vbus_pin, "udc_vbus");
+               if (retval < 0) {
+                       DBG("request vbus pin failed\n");
+                       goto fail2;
+               }
+               gpio_direction_input(udc->board.vbus_pin);
+
                /*
                 * Get the initial state of VBUS - we cannot expect
                 * a pending interrupt.
                 */
-               udc->vbus = at91_get_gpio_value(udc->board.vbus_pin);
+               udc->vbus = gpio_get_value(udc->board.vbus_pin);
                if (request_irq(udc->board.vbus_pin, at91_vbus_irq,
                                IRQF_DISABLED, driver_name, udc)) {
                        DBG("request vbus irq %d failed\n",
                                        udc->board.vbus_pin);
                        free_irq(udc->udp_irq, udc);
                        retval = -EBUSY;
-                       goto fail1;
+                       goto fail3;
                }
        } else {
                DBG("no VBUS detection, assuming always-on\n");
@@ -1736,8 +1761,18 @@ static int __init at91udc_probe(struct platform_device *pdev) 
        INFO("%s version %s\n", driver_name, DRIVER_VERSION);
        return 0;

+fail3:
+       if (udc->board.vbus_pin > 0)
+               gpio_free(udc->board.vbus_pin);
+fail2:
+       free_irq(udc->udp_irq, udc);
 fail1:
        device_unregister(&udc->gadget.dev);
+fail0b:
+       iounmap(udc->udp_baseaddr);
+fail0a:
+       if (cpu_is_at91rm9200())
+               gpio_free(udc->board.pullup_pin);
 fail0:
        release_mem_region(res->start, res->end - res->start + 1);
        DBG("%s probe failed, %d\n", driver_name, retval);
@@ -1758,12 +1793,18 @@ static int __exit at91udc_remove(struct platform_device *pdev) 

        device_init_wakeup(&pdev->dev, 0);
        remove_debug_file(udc);
-       if (udc->board.vbus_pin > 0)
+       if (udc->board.vbus_pin > 0) {
                free_irq(udc->board.vbus_pin, udc);
+               gpio_free(udc->board.vbus_pin);
+       }
        free_irq(udc->udp_irq, udc);
        device_unregister(&udc->gadget.dev);

        iounmap(udc->udp_baseaddr);
+
+       if (cpu_is_at91rm9200())
+               gpio_free(udc->board.pullup_pin);
+
        res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
        release_mem_region(res->start, res->end - res->start + 1);
diff --git a/drivers/usb/gadget/at91_udc.h b/drivers/usb/gadget/ at91_udc.h 
index f3095a9..a973f2a 100644
--- a/drivers/usb/gadget/at91_udc.h
+++ b/drivers/usb/gadget/at91_udc.h
@@ -158,7 +158,7 @@ struct at91_request {
/ *--------------------------------------------------------------------- ----*/ 

-#ifdef VERBOSE
+#ifdef VERBOSE_DEBUG
 #    define VDBG               DBG
 #else
 #    define VDBG(stuff...)     do{}while(0)
diff --git a/include/asm-arm/arch-at91/board.h b/include/asm-arm/ arch-at91/board.h 
index 55b07bd..dc189f0 100644
--- a/include/asm-arm/arch-at91/board.h
+++ b/include/asm-arm/arch-at91/board.h
@@ -40,7 +40,8 @@
  /* USB Device */
 struct at91_udc_data {
        u8      vbus_pin;               /* high == host powering us */
-       u8      pullup_pin;             /* high == D+ pulled up */
+       u8      pullup_pin;             /* active == D+ pulled up */
+       u8      pullup_active_low;      /* true == pullup_pin is active low */
 };
 extern void __init at91_add_device_udc(struct at91_udc_data *data);

--
1.5.3.8





-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to