Re: [PATCH 4/5] pxa25x_udc: stop handling gpio-vbus internally

Eric Miao <eric.y.miao@xxxxxxxxx> · Fri, 18 Feb 2011 15:33:43 +0800

On Mon, Feb 14, 2011 at 8:33 PM, Dmitry Eremin-Solenikov
<dbaryshkov@xxxxxxxxx> wrote:
> Stop handling gpio-vbus internally. All boards that depended on this
> functionality have been converted to use gpio-vbus tranceiver. All
> new boards can use it right from the start. Drop unused code.
>
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@xxxxxxxxx>

I'm good with the change below. The original intention of is_vbus_present()
is actually to detect if it's connected. A name of is_connected() would be
better, and I'm fine using gadget.speed for this.

David or any other USB people please have a review and give an Ack-by?
I'd like this being merged through the ARM tree as the other four patches
better go there as well.

Thanks

> ---
> Âdrivers/usb/gadget/pxa25x_udc.c | Â 76 ++++-----------------------------------
> Â1 files changed, 7 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/usb/gadget/pxa25x_udc.c b/drivers/usb/gadget/pxa25x_udc.c
> index b37f92c..444b60a 100644
> --- a/drivers/usb/gadget/pxa25x_udc.c
> +++ b/drivers/usb/gadget/pxa25x_udc.c
> @@ -139,24 +139,6 @@ static const char ep0name [] = "ep0";
> Âstatic void pxa25x_ep_fifo_flush (struct usb_ep *ep);
> Âstatic void nuke (struct pxa25x_ep *, int status);
>
> -/* one GPIO should be used to detect VBUS from the host */
> -static int is_vbus_present(void)
> -{
> - Â Â Â struct pxa2xx_udc_mach_info Â Â Â Â Â Â *mach = the_controller->mach;
> -
> - Â Â Â if (gpio_is_valid(mach->gpio_vbus)) {
> - Â Â Â Â Â Â Â int value = gpio_get_value(mach->gpio_vbus);
> -
> - Â Â Â Â Â Â Â if (mach->gpio_vbus_inverted)
> - Â Â Â Â Â Â Â Â Â Â Â return !value;
> - Â Â Â Â Â Â Â else
> - Â Â Â Â Â Â Â Â Â Â Â return !!value;
> - Â Â Â }
> - Â Â Â if (mach->udc_is_connected)
> - Â Â Â Â Â Â Â return mach->udc_is_connected();
> - Â Â Â return 1;
> -}
> -
> Â/* one GPIO should control a D+ pullup, so host sees this device (or not) */
> Âstatic void pullup_off(void)
> Â{
> @@ -1055,7 +1037,7 @@ udc_seq_show(struct seq_file *m, void *_d)
> Â Â Â Â Â Â Â Â"%s version: %s\nGadget driver: %s\nHost %s\n\n",
> Â Â Â Â Â Â Â Âdriver_name, DRIVER_VERSION SIZE_STR "(pio)",
> Â Â Â Â Â Â Â Âdev->driver ? dev->driver->driver.name : "(none)",
> - Â Â Â Â Â Â Â is_vbus_present() ? "full speed" : "disconnected");
> + Â Â Â Â Â Â Â dev->gadget.speed == USB_SPEED_FULL ? "full speed" : "disconnected");
>
> Â Â Â Â/* registers for device and ep0 */
> Â Â Â Âseq_printf(m,
> @@ -1094,7 +1076,7 @@ udc_seq_show(struct seq_file *m, void *_d)
> Â Â Â Â Â Â Â Â Â Â Â Â(tmp & UDCCFR_ACM) ? " acm" : "");
> Â Â Â Â}
>
> - Â Â Â if (!is_vbus_present() || !dev->driver)
> + Â Â Â if (dev->gadget.speed != USB_SPEED_FULL || !dev->driver)
> Â Â Â Â Â Â Â Âgoto done;
>
> Â Â Â Âseq_printf(m, "ep0 IN %lu/%lu, OUT %lu/%lu\nirqs %lu\n\n",
> @@ -1435,14 +1417,6 @@ lubbock_vbus_irq(int irq, void *_dev)
>
> Â#endif
>
> -static irqreturn_t udc_vbus_irq(int irq, void *_dev)
> -{
> - Â Â Â struct pxa25x_udc Â Â Â *dev = _dev;
> -
> - Â Â Â pxa25x_udc_vbus_session(&dev->gadget, is_vbus_present());
> - Â Â Â return IRQ_HANDLED;
> -}
> -
>
> Â/*-------------------------------------------------------------------------*/
>
> @@ -1766,12 +1740,9 @@ pxa25x_udc_irq(int irq, void *_dev)
> Â Â Â Â Â Â Â Âif (unlikely(udccr & UDCCR_SUSIR)) {
> Â Â Â Â Â Â Â Â Â Â Â Âudc_ack_int_UDCCR(UDCCR_SUSIR);
> Â Â Â Â Â Â Â Â Â Â Â Âhandled = 1;
> - Â Â Â Â Â Â Â Â Â Â Â DBG(DBG_VERBOSE, "USB suspend%s\n", is_vbus_present()
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ? "" : "+disconnect");
> + Â Â Â Â Â Â Â Â Â Â Â DBG(DBG_VERBOSE, "USB suspend\n");
>
> - Â Â Â Â Â Â Â Â Â Â Â if (!is_vbus_present())
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â stop_activity(dev, dev->driver);
> - Â Â Â Â Â Â Â Â Â Â Â else if (dev->gadget.speed != USB_SPEED_UNKNOWN
> + Â Â Â Â Â Â Â Â Â Â Â if (dev->gadget.speed != USB_SPEED_UNKNOWN
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â&& dev->driver
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â&& dev->driver->suspend)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âdev->driver->suspend(&dev->gadget);
> @@ -1786,8 +1757,7 @@ pxa25x_udc_irq(int irq, void *_dev)
>
> Â Â Â Â Â Â Â Â Â Â Â Âif (dev->gadget.speed != USB_SPEED_UNKNOWN
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â&& dev->driver
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â && dev->driver->resume
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â && is_vbus_present())
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â && dev->driver->resume)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âdev->driver->resume(&dev->gadget);
> Â Â Â Â Â Â Â Â}
>
> @@ -2137,7 +2107,7 @@ static struct pxa25x_udc memory = {
> Âstatic int __init pxa25x_udc_probe(struct platform_device *pdev)
> Â{
> Â Â Â Âstruct pxa25x_udc *dev = &memory;
> - Â Â Â int retval, vbus_irq, irq;
> + Â Â Â int retval, irq;
> Â Â Â Âu32 chiprev;
>
> Â Â Â Â/* insist on Intel/ARM/XScale */
> @@ -2199,19 +2169,6 @@ static int __init pxa25x_udc_probe(struct platform_device *pdev)
>
> Â Â Â Âdev->transceiver = otg_get_transceiver();
>
> - Â Â Â if (gpio_is_valid(dev->mach->gpio_vbus)) {
> - Â Â Â Â Â Â Â if ((retval = gpio_request(dev->mach->gpio_vbus,
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "pxa25x_udc GPIO VBUS"))) {
> - Â Â Â Â Â Â Â Â Â Â Â dev_dbg(&pdev->dev,
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "can't get vbus gpio %d, err: %d\n",
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â dev->mach->gpio_vbus, retval);
> - Â Â Â Â Â Â Â Â Â Â Â goto err_gpio_vbus;
> - Â Â Â Â Â Â Â }
> - Â Â Â Â Â Â Â gpio_direction_input(dev->mach->gpio_vbus);
> - Â Â Â Â Â Â Â vbus_irq = gpio_to_irq(dev->mach->gpio_vbus);
> - Â Â Â } else
> - Â Â Â Â Â Â Â vbus_irq = 0;
> -
> Â Â Â Âif (gpio_is_valid(dev->mach->gpio_pullup)) {
> Â Â Â Â Â Â Â Âif ((retval = gpio_request(dev->mach->gpio_pullup,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â"pca25x_udc GPIO PULLUP"))) {
> @@ -2237,7 +2194,7 @@ static int __init pxa25x_udc_probe(struct platform_device *pdev)
> Â Â Â Âudc_disable(dev);
> Â Â Â Âudc_reinit(dev);
>
> - Â Â Â dev->vbus = !!is_vbus_present();
> + Â Â Â dev->vbus = 0;
>
> Â Â Â Â/* irq setup after old hardware state is cleaned up */
> Â Â Â Âretval = request_irq(irq, pxa25x_udc_irq,
> @@ -2273,22 +2230,10 @@ lubbock_fail0:
> Â Â Â Â Â Â Â Â}
> Â Â Â Â} else
> Â#endif
> - Â Â Â if (vbus_irq) {
> - Â Â Â Â Â Â Â retval = request_irq(vbus_irq, udc_vbus_irq,
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â IRQF_DISABLED | IRQF_SAMPLE_RANDOM |
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â driver_name, dev);
> - Â Â Â Â Â Â Â if (retval != 0) {
> - Â Â Â Â Â Â Â Â Â Â Â pr_err("%s: can't get irq %i, err %d\n",
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â driver_name, vbus_irq, retval);
> - Â Â Â Â Â Â Â Â Â Â Â goto err_vbus_irq;
> - Â Â Â Â Â Â Â }
> - Â Â Â }
> Â Â Â Âcreate_debug_files(dev);
>
> Â Â Â Âreturn 0;
>
> - err_vbus_irq:
> Â#ifdef CONFIG_ARCH_LUBBOCK
> Â Â Â Âfree_irq(LUBBOCK_USB_DISC_IRQ, dev);
> Âerr_irq_lub:
> @@ -2298,9 +2243,6 @@ lubbock_fail0:
> Â Â Â Âif (gpio_is_valid(dev->mach->gpio_pullup))
> Â Â Â Â Â Â Â Âgpio_free(dev->mach->gpio_pullup);
> Âerr_gpio_pullup:
> - Â Â Â if (gpio_is_valid(dev->mach->gpio_vbus))
> - Â Â Â Â Â Â Â gpio_free(dev->mach->gpio_vbus);
> - err_gpio_vbus:
> Â Â Â Âif (dev->transceiver) {
> Â Â Â Â Â Â Â Âotg_put_transceiver(dev->transceiver);
> Â Â Â Â Â Â Â Âdev->transceiver = NULL;
> @@ -2337,10 +2279,6 @@ static int __exit pxa25x_udc_remove(struct platform_device *pdev)
> Â Â Â Â Â Â Â Âfree_irq(LUBBOCK_USB_IRQ, dev);
> Â Â Â Â}
> Â#endif
> - Â Â Â if (gpio_is_valid(dev->mach->gpio_vbus)) {
> - Â Â Â Â Â Â Â free_irq(gpio_to_irq(dev->mach->gpio_vbus), dev);
> - Â Â Â Â Â Â Â gpio_free(dev->mach->gpio_vbus);
> - Â Â Â }
> Â Â Â Âif (gpio_is_valid(dev->mach->gpio_pullup))
> Â Â Â Â Â Â Â Âgpio_free(dev->mach->gpio_pullup);
>
> --
> 1.7.2.3
>
>
ÿô.nÇ‰·Ÿ®‰†+%ŠË±é¥Šwÿº{.nÇ‰·¥Š{±þë)íèjg¬±¨¶‰šŽŠÝjÿ¾«þG«é¸¢·¦j:+v‰¨Šwèm¶Ÿÿþø®w¥þŠà£¢·hšâÿ†Ù