Skip to contentSkip to conversation

Big pxa2[57]x_udc cleanup

PATCH New Dmitry Eremin-Solenikov 7 participants 37 messages Jul 5, 2011 – Jul 18, 2011

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

cover Big pxa2[57]x_udc cleanup

Hello colleagues,

Please review the following patch serie. It's major task is removal of code and functionality duplication between pxa2[57]x_udc drivers and OTG transceivers subsystem.

The following changes since commit fe0d42203cb5616eeff68b14576a0f7e2dd56625:

Linux 3.0-rc6 (2011-07-04 15:56:24 -0700)

are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/lumag/tosa-2.6.git misc/pxa-udc-cleanup

Dmitry Eremin-Solenikov (15):
ARM: pxa/balloon3: drop udc_is_connected
ARM: pxa/mioa701: drop udc_is_connected
ARM: pxa/stargate2: drop udc_is_connected
ARM: pxa25x_udc: separate lubbock handling to lubbock-usb transceiver
ARM: pxa/lubbock: switch to using lubbock_usb_xceiv transceiver
pxa: drop handling of udc_is_connected
pxa25x_udc: drop support for udc_command
otg: add gpio_pullup OTG transceiver for devices with no VBUS sensing
corgi: convert to use gpio-pullup
poodle: convert to use gpio-pullup
h5000: convert to use gpio-pullup
gpio-vbus: support disabling D+ pullup on suspend
mioa701: move gpio-pullup functionality to gpio-vbus
gpio-pullup: support disabling D+ pullup on suspend pxa2[57]x_udc: drop gpio_pullup handling

 arch/arm/include/asm/mach/udc_pxa2xx.h |    9 --
 arch/arm/mach-pxa/balloon3.c           |    7 -
 arch/arm/mach-pxa/colibri-pxa320.c     |    1 -
 arch/arm/mach-pxa/corgi.c              |   14 ++-
 arch/arm/mach-pxa/devices.c            |    2 +-
 arch/arm/mach-pxa/h5000.c              |   14 ++-
 arch/arm/mach-pxa/lubbock.c            |   17 +--
 arch/arm/mach-pxa/mioa701.c            |   19 +--
 arch/arm/mach-pxa/poodle.c             |   13 ++-
 arch/arm/mach-pxa/stargate2.c          |    6 -
 arch/arm/mach-pxa/vpac270.c            |    1 -
 drivers/usb/gadget/pxa25x_udc.c        |  132 ---------------------
 drivers/usb/gadget/pxa27x_udc.c        |   27 +----
 drivers/usb/otg/Kconfig                |   18 +++
 drivers/usb/otg/Makefile               |    2 +
 drivers/usb/otg/gpio_pullup.c          |  204 ++++++++++++++++++++++++++++++++
 drivers/usb/otg/gpio_vbus.c            |   42 +++++++
 drivers/usb/otg/lubbock-usb.c          |  178 ++++++++++++++++++++++++++++
 include/linux/usb/gpio_vbus.h          |    1 +
 19 files changed, 493 insertions(+), 214 deletions(-)
 create mode 100644 drivers/usb/otg/gpio_pullup.c
 create mode 100644 drivers/usb/otg/lubbock-usb.c
#1 lore raw

PATCH 1/15 ARM: pxa/balloon3: drop udc_is_connected

Balloon3 is a pxa270 machins and pxa27x_udc driver doesn't pay any attention to udc_is_connected, so drop it.

---
 arch/arm/mach-pxa/balloon3.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)
arch/arm/mach-pxa/balloon3.c +0 -6
index 810a982..4d0d8c3 100644
@@ -316,14 +316,8 @@ static void balloon3_udc_command(int cmd)
 		UP2OCR &= ~UP2OCR_DPPUE;
 }
 
-static int balloon3_udc_is_connected(void)
-{
-	return 1;
-}
-
 static struct pxa2xx_udc_mach_info balloon3_udc_info __initdata = {
 	.udc_command		= balloon3_udc_command,
-	.udc_is_connected	= balloon3_udc_is_connected,
 	.gpio_pullup		= -1,
 };
 
Signature · 1 line
1.7.5.4
#2 lore raw

PATCH 2/15 ARM: pxa/mioa701: drop udc_is_connected

In reply to Dmitry Eremin-Solenikov

Mioa701 is a pxa270 machine and pxa27x_udc driver doesn't pay any attention to udc_is_connected, so drop it.

---
 arch/arm/mach-pxa/mioa701.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)
arch/arm/mach-pxa/mioa701.c +5 -6
index e347013..d833a26 100644
@@ -402,13 +402,7 @@ static void gsm_exit(void)
 /*
  * USB UDC
  */
-static int is_usb_connected(void)
-{
-	return !gpio_get_value(GPIO13_nUSB_DETECT);
-}
-
 static struct pxa2xx_udc_mach_info mioa701_udc_info = {
-	.udc_is_connected = is_usb_connected,
 	.gpio_pullup	  = GPIO22_USB_ENABLE,
 };
 
@@ -569,6 +563,11 @@ static int is_ac_connected(void)
 	return gpio_get_value(GPIO96_AC_DETECT);
 }
 
+static int is_usb_connected(void)
+{
+	return !gpio_get_value(GPIO13_nUSB_DETECT);
+}
+
 static void mioa701_set_charge(int flags)
 {
 	gpio_set_value(GPIO9_CHARGE_EN, (flags == PDA_POWER_CHARGE_USB));
Signature · 1 line
1.7.5.4
#3 lore raw

PATCH 3/15 ARM: pxa/stargate2: drop udc_is_connected

In reply to Dmitry Eremin-Solenikov
---
 arch/arm/mach-pxa/stargate2.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)
arch/arm/mach-pxa/stargate2.c +0 -6
index cb5611d..3e3621d 100644
@@ -956,13 +956,7 @@ static struct i2c_board_info __initdata stargate2_i2c_board_info[] = {
 /* Board doesn't support cable detection - so always lie and say
  * something is there.
  */
-static int sg2_udc_detect(void)
-{
-	return 1;
-}
-
 static struct pxa2xx_udc_mach_info stargate2_udc_info __initdata = {
-	.udc_is_connected	= sg2_udc_detect,
 	.udc_command		= sg2_udc_command,
 };
 
Signature · 1 line
1.7.5.4
#4 lore raw

PATCH 4/15 ARM: pxa25x_udc: separate lubbock handling to lubbock-usb transceiver

In reply to Dmitry Eremin-Solenikov

Move all handling of lubbock board to separate lubbock-usb transceiver, removing cruft from pxa25x_udc.

---
 drivers/usb/gadget/pxa25x_udc.c |   74 ----------------
 drivers/usb/otg/Kconfig         |   10 ++
 drivers/usb/otg/Makefile        |    1 +
 drivers/usb/otg/lubbock-usb.c   |  178 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 189 insertions(+), 74 deletions(-)
 create mode 100644 drivers/usb/otg/lubbock-usb.c
drivers/usb/gadget/pxa25x_udc.c +0 -74
index 7745454..6f543c7 100644
@@ -66,10 +66,6 @@
 #include <mach/pxa25x-udc.h>
 #endif
 
-#ifdef CONFIG_ARCH_LUBBOCK
-#include <mach/lubbock.h>
-#endif
-
 #include <asm/mach/udc_pxa2xx.h>
 
 
@@ -1384,42 +1380,6 @@ EXPORT_SYMBOL(usb_gadget_unregister_driver);
 
 /*-------------------------------------------------------------------------*/
 
-#ifdef CONFIG_ARCH_LUBBOCK
-
-/* Lubbock has separate connect and disconnect irqs.  More typical designs
- * use one GPIO as the VBUS IRQ, and another to control the D+ pullup.
- */
-
-static irqreturn_t
-lubbock_vbus_irq(int irq, void *_dev)
-{
-	struct pxa25x_udc	*dev = _dev;
-	int			vbus;
-
-	dev->stats.irqs++;
-	switch (irq) {
-	case LUBBOCK_USB_IRQ:
-		vbus = 1;
-		disable_irq(LUBBOCK_USB_IRQ);
-		enable_irq(LUBBOCK_USB_DISC_IRQ);
-		break;
-	case LUBBOCK_USB_DISC_IRQ:
-		vbus = 0;
-		disable_irq(LUBBOCK_USB_DISC_IRQ);
-		enable_irq(LUBBOCK_USB_IRQ);
-		break;
-	default:
-		return IRQ_NONE;
-	}
-
-	pxa25x_udc_vbus_session(&dev->gadget, vbus);
-	return IRQ_HANDLED;
-}
-
-#endif
-
-
-/*-------------------------------------------------------------------------*/
 
 static inline void clear_ep_state (struct pxa25x_udc *dev)
 {
@@ -2207,38 +2167,10 @@ static int __init pxa25x_udc_probe(struct platform_device *pdev)
 	}
 	dev->got_irq = 1;
 
-#ifdef CONFIG_ARCH_LUBBOCK
-	if (machine_is_lubbock()) {
-		retval = request_irq(LUBBOCK_USB_DISC_IRQ,
-				lubbock_vbus_irq,
-				IRQF_DISABLED | IRQF_SAMPLE_RANDOM,
-				driver_name, dev);
-		if (retval != 0) {
-			pr_err("%s: can't get irq %i, err %d\n",
-				driver_name, LUBBOCK_USB_DISC_IRQ, retval);
-			goto err_irq_lub;
-		}
-		retval = request_irq(LUBBOCK_USB_IRQ,
-				lubbock_vbus_irq,
-				IRQF_DISABLED | IRQF_SAMPLE_RANDOM,
-				driver_name, dev);
-		if (retval != 0) {
-			pr_err("%s: can't get irq %i, err %d\n",
-				driver_name, LUBBOCK_USB_IRQ, retval);
-			goto lubbock_fail0;
-		}
-	} else
-#endif
 	create_debug_files(dev);
 
 	return 0;
 
-#ifdef	CONFIG_ARCH_LUBBOCK
-lubbock_fail0:
-	free_irq(LUBBOCK_USB_DISC_IRQ, dev);
- err_irq_lub:
-	free_irq(irq, dev);
-#endif
  err_irq1:
 	if (gpio_is_valid(dev->mach->gpio_pullup))
 		gpio_free(dev->mach->gpio_pullup);
@@ -2273,12 +2205,6 @@ static int __exit pxa25x_udc_remove(struct platform_device *pdev)
 		free_irq(platform_get_irq(pdev, 0), dev);
 		dev->got_irq = 0;
 	}
-#ifdef CONFIG_ARCH_LUBBOCK
-	if (machine_is_lubbock()) {
-		free_irq(LUBBOCK_USB_DISC_IRQ, dev);
-		free_irq(LUBBOCK_USB_IRQ, dev);
-	}
-#endif
 	if (gpio_is_valid(dev->mach->gpio_pullup))
 		gpio_free(dev->mach->gpio_pullup);
 
drivers/usb/otg/Kconfig +10 -0
index c66481a..cf3a245 100644
@@ -122,6 +122,16 @@ config AB8500_USB
           This transceiver supports high and full speed devices plus,
           in host mode, low speed.
 
+config LUBBOCK_USB
+	tristate "Intel DBPXA250 (Lubbock) Tranceiver Driver"
+	depends on ARCH_LUBBOCK
+        select USB_OTG_UTILS
+	help
+	  Enable this to support the USB UDC connectivity on Intel DBPXA250
+	  development platform (so-called Lubbock).
+	  This driver is of no use for all other platforms, so if you are
+	  using other board, you can safely say N here.
+
 config FSL_USB2_OTG
 	bool "Freescale USB OTG Transceiver Driver"
 	depends on USB_EHCI_FSL && USB_GADGET_FSL_USB2
drivers/usb/otg/Makefile +1 -0
index 566655c..24d4e63 100644
@@ -19,5 +19,6 @@ obj-$(CONFIG_USB_ULPI)		+= ulpi.o
 obj-$(CONFIG_USB_ULPI_VIEWPORT)	+= ulpi_viewport.o
 obj-$(CONFIG_USB_MSM_OTG)	+= msm_otg.o
 obj-$(CONFIG_AB8500_USB)	+= ab8500-usb.o
+obj-$(CONFIG_LUBBOCK_USB)	+= lubbock-usb.o
 fsl_usb2_otg-objs		:= fsl_otg.o otg_fsm.o
 obj-$(CONFIG_FSL_USB2_OTG)	+= fsl_usb2_otg.o
drivers/usb/otg/lubbock-usb.c new file +178 -0

178 lines not on this page · raw message

Signature · 1 line
1.7.5.4
#5 lore raw

PATCH 4/15 ARM: pxa25x_udc: separate lubbock handling to lubbock-usb transceiver

Hi,

Quoted 6 lines · Dmitry Eremin-Solenikov

Move all handling of lubbock board to separate lubbock-usb transceiver, removing cruft from pxa25x_udc.

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110705/519756fd/attachment.sig>
#6 lore raw

PATCH 5/15 ARM: pxa/lubbock: switch to using lubbock_usb_xceiv transceiver

In reply to Dmitry Eremin-Solenikov
---
 arch/arm/mach-pxa/lubbock.c |   17 ++++++-----------
 1 files changed, 6 insertions(+), 11 deletions(-)
arch/arm/mach-pxa/lubbock.c +6 -11
index e10ddb8..4dcc687 100644
@@ -198,16 +198,6 @@ device_initcall(lubbock_irq_device_init);
 
 #endif
 
-static int lubbock_udc_is_connected(void)
-{
-	return (LUB_MISC_RD & (1 << 9)) == 0;
-}
-
-static struct pxa2xx_udc_mach_info udc_info __initdata = {
-	.udc_is_connected	= lubbock_udc_is_connected,
-	// no D+ pullup; lubbock can't connect/disconnect in software
-};
-
 static struct resource sa1111_resources[] = {
 	[0] = {
 		.start	= 0x10000000,
@@ -379,11 +369,17 @@ static struct platform_device lubbock_flash_device[2] = {
 	},
 };
 
+static struct platform_device lubbock_usb_xceiv = {
+	.name	= "lubbock_usb_xceiv",
+	.id	= -1,
+};
+
 static struct platform_device *devices[] __initdata = {
 	&sa1111_device,
 	&smc91x_device,
 	&lubbock_flash_device[0],
 	&lubbock_flash_device[1],
+	&lubbock_usb_xceiv,
 };
 
 static struct pxafb_mode_info sharp_lm8v31_mode = {
@@ -511,7 +507,6 @@ static void __init lubbock_init(void)
 	pxa_set_stuart_info(NULL);
 
 	clk_add_alias("SA1111_CLK", NULL, "GPIO11_CLK", NULL);
-	pxa_set_udc_info(&udc_info);
 	pxa_set_fb_info(NULL, &sharp_lm8v31);
 	pxa_set_mci_info(&lubbock_mci_platform_data);
 	pxa_set_ficp_info(&lubbock_ficp_platform_data);
Signature · 1 line
1.7.5.4
#8 lore raw

PATCH 6/15 pxa: drop handling of udc_is_connected

In reply to Dmitry Eremin-Solenikov

There are no more users of udc_is_connected. Drop this field from mach_info and stop using it in pxa25x_udc.

---
 arch/arm/include/asm/mach/udc_pxa2xx.h |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
arch/arm/include/asm/mach/udc_pxa2xx.h +0 -1
index ea297ac..f0719d3 100644
@@ -10,7 +10,6 @@
  */
 
 struct pxa2xx_udc_mach_info {
-        int  (*udc_is_connected)(void);		/* do we see host? */
         void (*udc_command)(int cmd);
 #define	PXA2XX_UDC_CMD_CONNECT		0	/* let host see us */
 #define	PXA2XX_UDC_CMD_DISCONNECT	1	/* so host won't see us */
Signature · 1 line
1.7.5.4
#9 lore raw

PATCH 7/15 pxa25x_udc: drop support for udc_command

In reply to Dmitry Eremin-Solenikov

None of pxa25x devices use udc_command() for UDC functionality. Stop calling this callback from pxa25x_udc code.

---
 drivers/usb/gadget/pxa25x_udc.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)
drivers/usb/gadget/pxa25x_udc.c +2 -6
index 6f543c7..aa34698 100644
@@ -144,8 +144,6 @@ static void pullup_off(void)
 
 	if (gpio_is_valid(mach->gpio_pullup))
 		gpio_set_value(mach->gpio_pullup, off_level);
-	else if (mach->udc_command)
-		mach->udc_command(PXA2XX_UDC_CMD_DISCONNECT);
 }
 
 static void pullup_on(void)
@@ -155,8 +153,6 @@ static void pullup_on(void)
 
 	if (gpio_is_valid(mach->gpio_pullup))
 		gpio_set_value(mach->gpio_pullup, on_level);
-	else if (mach->udc_command)
-		mach->udc_command(PXA2XX_UDC_CMD_CONNECT);
 }
 
 static void pio_irq_enable(int bEndpointAddress)
@@ -984,7 +980,7 @@ static int pxa25x_udc_pullup(struct usb_gadget *_gadget, int is_active)
 	udc = container_of(_gadget, struct pxa25x_udc, gadget);
 
 	/* not all boards support pullup control */
-	if (!gpio_is_valid(udc->mach->gpio_pullup) && !udc->mach->udc_command)
+	if (!gpio_is_valid(udc->mach->gpio_pullup))
 		return -EOPNOTSUPP;
 
 	udc->pullup = (is_active != 0);
@@ -2241,7 +2237,7 @@ static int pxa25x_udc_suspend(struct platform_device *dev, pm_message_t state)
 	struct pxa25x_udc	*udc = platform_get_drvdata(dev);
 	unsigned long flags;
 
-	if (!gpio_is_valid(udc->mach->gpio_pullup) && !udc->mach->udc_command)
+	if (!gpio_is_valid(udc->mach->gpio_pullup))
 		WARNING("USB host won't detect disconnect!\n");
 	udc->suspended = 1;
 
Signature · 1 line
1.7.5.4
#10 lore raw

PATCH 7/15 pxa25x_udc: drop support for udc_command

Hi,

Quoted 2 lines · Dmitry Eremin-Solenikov

None of pxa25x devices use udc_command() for UDC functionality. Stop calling this callback from pxa25x_udc code.

looks good to me, Greg since this is a big re-work which depends on parts out of the Gadget Framework, can you take all the patches straight ? Here's my Acked-by:

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110705/4b6e3c94/attachment.sig>
#11 lore raw

PATCH 7/15 pxa25x_udc: drop support for udc_command

Quoted 11 lines · Felipe Balbi

Hi,

Quoted 2 lines · Dmitry Eremin-Solenikov

None of pxa25x devices use udc_command() for UDC functionality. Stop calling this callback from pxa25x_udc code.

looks good to me, Greg since this is a big re-work which depends on parts out of the Gadget Framework, can you take all the patches straight ? Here's my Acked-by:

I can't take these, they should go through the gpio subsystem maintainer instead.

thanks,

greg k-h

#12 lore raw

PATCH 7/15 pxa25x_udc: drop support for udc_command

Quoted 7 lines · Greg KH
Quoted 6 lines · Felipe Balbi

Hi,

Quoted 2 lines · Dmitry Eremin-Solenikov

None of pxa25x devices use udc_command() for UDC functionality. Stop calling this callback from pxa25x_udc code.

Quoted 8 lines
Quoted 5 lines

looks good to me, Greg since this is a big re-work which depends on parts out of the Gadget Framework, can you take all the patches straight ? Here's my Acked-by:

I can't take these, they should go through the gpio subsystem maintainer instead.

But why ? They only touch drivers/usb/gadget/pxa*.c

Would it be enough if Grant would Ack them ?

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110709/c10b73d3/attachment-0001.sig>
#13 lore raw

PATCH 7/15 pxa25x_udc: drop support for udc_command

Quoted 12 lines · Felipe Balbi
Quoted 11 lines · Greg KH
Quoted 10 lines · Felipe Balbi

Hi,

Quoted 2 lines · Dmitry Eremin-Solenikov

None of pxa25x devices use udc_command() for UDC functionality. Stop calling this callback from pxa25x_udc code.

looks good to me, Greg since this is a big re-work which depends on parts out of the Gadget Framework, can you take all the patches straight ? Here's my Acked-by:

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

I can't take these, they should go through the gpio subsystem maintainer instead.

But why ? They only touch drivers/usb/gadget/pxa*.c

All 15 patches? No they don't they touch all sorts of things.

Quoted 1 line

Would it be enough if Grant would Ack them ?

Sure, but it would be easier if Grant took them all, right?

thanks,

greg k-h

#14 lore raw

PATCH 7/15 pxa25x_udc: drop support for udc_command

Hi,

Quoted 10 lines · Greg KH
Quoted 4 lines
Quoted 2 lines

I can't take these, they should go through the gpio subsystem maintainer instead.

But why ? They only touch drivers/usb/gadget/pxa*.c

All 15 patches? No they don't they touch all sorts of things.

Quoted 1 line

Would it be enough if Grant would Ack them ?

Sure, but it would be easier if Grant took them all, right?

Makes sense too.

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110709/ade92949/attachment.sig>
#15 lore raw

PATCH 7/15 pxa25x_udc: drop support for udc_command

In reply to Greg KH
Quoted 19 lines · Greg KH
Quoted 18 lines · Felipe Balbi
Quoted 16 lines · Greg KH
Quoted 4 lines · Felipe Balbi

Hi,

On Tue, Jul 05, 2011 at 05:08:33PM +0400, Dmitry Eremin-Solenikov wrote:

Quoted 8 lines
Quoted 2 lines

None of pxa25x devices use udc_command() for UDC functionality. Stop calling this callback from pxa25x_udc code.

looks good to me, Greg since this is a big re-work which depends on parts out of the Gadget Framework, can you take all the patches straight ? Here's my Acked-by:

I can't take these, they should go through the gpio subsystem maintainer instead.

Quoted 7 lines
Quoted 1 line

But why ? They only touch drivers/usb/gadget/pxa*.c

All 15 patches? No they don't they touch all sorts of things.

Quoted 1 line

Would it be enough if Grant would Ack them ?

Sure, but it would be easier if Grant took them all, right?

Colleagues, can you please tell me the current status/future of this patches? Are they going to 3.1? 3.2? Should they go via ARM/PXA, via usb? usb-gadget? Via gpio(why?)?? Should I send them to somebody for merging?

Thank you!

Signature · 2 lines
With best wishes
Dmitry
#16 lore raw

PATCH 7/15 pxa25x_udc: drop support for udc_command

Quoted 24 lines · Dmitry Eremin-Solenikov
Quoted 23 lines · Greg KH
Quoted 13 lines · Felipe Balbi
Quoted 12 lines · Greg KH
Quoted 11 lines · Felipe Balbi

Hi,

Quoted 2 lines · Dmitry Eremin-Solenikov

None of pxa25x devices use udc_command() for UDC functionality. Stop calling this callback from pxa25x_udc code.

looks good to me, Greg since this is a big re-work which depends on parts out of the Gadget Framework, can you take all the patches straight ? Here's my Acked-by:

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

I can't take these, they should go through the gpio subsystem maintainer instead.

But why ? They only touch drivers/usb/gadget/pxa*.c

All 15 patches? ?No they don't they touch all sorts of things.

Quoted 7 lines
Quoted 3 lines
Quoted 1 line

Would it be enough if Grant would Ack them ?

Sure, but it would be easier if Grant took them all, right?

Colleagues, can you please tell me the current status/future of this patches? Are they going to 3.1? 3.2? Should they go via ARM/PXA, via usb? usb-gadget? Via gpio(why?)?? Should I send them to somebody for merging?

Well, I agree with Dmitry the whole patchset is not related to the GPIO subsystem at all. The major cleanup is actually to the PXA machines, and touches part of the pxa25x_udc driver. If some one Acks the USB part, I can help take the remaining patches.

maintainer
#17 lore raw

PATCH 7/15 pxa25x_udc: drop support for udc_command

Quoted 4 lines · Eric Miao

Well, I agree with Dmitry the whole patchset is not related to the GPIO subsystem at all. The major cleanup is actually to the PXA machines, and touches part of the pxa25x_udc driver. If some one Acks the USB part, I can help take the remaining patches.

Ok, I tested the patches for the pxa27x_udc and mioa701 parts. Therefore, as it is working, for patches 2, 13, and 15 (reduced to pxa27x_udc), I give my :

That leaves you with the pxa25x_udc, I'm afraid, but I don't maintain that one. Is there an identified maintainer now, or at least someone able to test it ?

Cheers.

--
Robert

maintainer
#18 lore raw

PATCH 7/15 pxa25x_udc: drop support for udc_command

Quoted 6 lines · Robert Jarzmik
Quoted 4 lines · Eric Miao

Well, I agree with Dmitry the whole patchset is not related to the GPIO subsystem at all. The major cleanup is actually to the PXA machines, and touches part of the pxa25x_udc driver. If some one Acks the USB part, I can help take the remaining patches.

And I forgot patch 12 :

Cheers.

--
Robert

#19 lore raw

PATCH 7/15 pxa25x_udc: drop support for udc_command

In reply to Robert Jarzmik

Not sure if zaurus people can help test it. Pavel?

- eric

Quoted 21 lines · Robert Jarzmik
Quoted 5 lines · Eric Miao

Well, I agree with Dmitry the whole patchset is not related to the GPIO subsystem at all. The major cleanup is actually to the PXA machines, and touches part of the pxa25x_udc driver. If some one Acks the USB part, I can help take the remaining patches.

Ok, I tested the patches for the pxa27x_udc and mioa701 parts. Therefore, as it is working, for patches 2, 13, and 15 (reduced to pxa27x_udc), I give my :

That leaves you with the pxa25x_udc, I'm afraid, but I don't maintain that one. Is there an identified maintainer now, or at least someone able to test it ?

Cheers.

--
Robert

#20 lore raw

PATCH 7/15 pxa25x_udc: drop support for udc_command

In reply to Eric Miao

Hi,

Quoted 30 lines · Eric Miao
Quoted 28 lines · Dmitry Eremin-Solenikov
Quoted 26 lines · Greg KH
Quoted 13 lines · Felipe Balbi
Quoted 12 lines · Greg KH
Quoted 11 lines · Felipe Balbi

Hi,

Quoted 2 lines · Dmitry Eremin-Solenikov

None of pxa25x devices use udc_command() for UDC functionality. Stop calling this callback from pxa25x_udc code.

looks good to me, Greg since this is a big re-work which depends on parts out of the Gadget Framework, can you take all the patches straight ? Here's my Acked-by:

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

I can't take these, they should go through the gpio subsystem maintainer instead.

But why ? They only touch drivers/usb/gadget/pxa*.c

All 15 patches? ?No they don't they touch all sorts of things.

Quoted 1 line

Would it be enough if Grant would Ack them ?

Sure, but it would be easier if Grant took them all, right?

Quoted 8 lines
Quoted 3 lines

Colleagues, can you please tell me the current status/future of this patches? Are they going to 3.1? 3.2? Should they go via ARM/PXA, via usb? usb-gadget? Via gpio(why?)?? Should I send them to somebody for merging?

Well, I agree with Dmitry the whole patchset is not related to the GPIO subsystem at all. The major cleanup is actually to the PXA machines, and touches part of the pxa25x_udc driver. If some one Acks the USB part, I can help take the remaining patches.

For the gadget part you can have my Ack. Although I don't have how to test those patches, they seem simple enough:

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110718/9c8c4546/attachment.sig>
#21 lore raw

PATCH 7/15 pxa25x_udc: drop support for udc_command

In reply to Greg KH

Hi,

Quoted 7 lines · Greg KH
Quoted 6 lines · Felipe Balbi

Hi,

Quoted 2 lines · Dmitry Eremin-Solenikov

None of pxa25x devices use udc_command() for UDC functionality. Stop calling this callback from pxa25x_udc code.

Quoted 8 lines
Quoted 5 lines

looks good to me, Greg since this is a big re-work which depends on parts out of the Gadget Framework, can you take all the patches straight ? Here's my Acked-by:

I can't take these, they should go through the gpio subsystem maintainer instead.

Why? I can understand if you'd say that it should go through the ARM/PXA trees, but why GPIO?

Signature · 2 lines
With best wishes
Dmitry
#22 lore raw

PATCH 8/15 otg: add gpio_pullup OTG transceiver for devices with no VBUS sensing

In reply to Dmitry Eremin-Solenikov

There are some devices which have D+ pullup GPIO, but no VBUS sensing support. As such they can't be supported by gpio-vbus transceiver but would benefit from OTG transceiver interface support. Provide simple driver that would just handle OTG API for gadgete drivers and provide a way to toggle D+ pullup.

---
 drivers/usb/otg/Kconfig       |    8 ++
 drivers/usb/otg/Makefile      |    1 +
 drivers/usb/otg/gpio_pullup.c |  159 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 168 insertions(+), 0 deletions(-)
 create mode 100644 drivers/usb/otg/gpio_pullup.c
drivers/usb/otg/Kconfig +8 -0
index cf3a245..18b9183 100644
@@ -140,4 +140,12 @@ config FSL_USB2_OTG
 	help
 	  Enable this to support Freescale USB OTG transceiver.
 
+config USB_GPIO_PULLUP
+	tristate "GPIO based peripheral-only D+ pullup handling"
+	depends on GENERIC_GPIO
+	select USB_OTG_UTILS
+	help
+	  Provides simple GPIO D+ pullup handling for controllers with
+	  internal transceiver via the otg_transceiver interface.
+
 endif # USB || OTG
drivers/usb/otg/Makefile +1 -0
index 24d4e63..b482e79 100644
@@ -22,3 +22,4 @@ obj-$(CONFIG_AB8500_USB)	+= ab8500-usb.o
 obj-$(CONFIG_LUBBOCK_USB)	+= lubbock-usb.o
 fsl_usb2_otg-objs		:= fsl_otg.o otg_fsm.o
 obj-$(CONFIG_FSL_USB2_OTG)	+= fsl_usb2_otg.o
+obj-$(CONFIG_USB_GPIO_PULLUP)	+= gpio_pullup.o
drivers/usb/otg/gpio_pullup.c new file +159 -0

159 lines not on this page · raw message

Signature · 1 line
1.7.5.4
#23 lore raw

PATCH 9/15 corgi: convert to use gpio-pullup

In reply to Dmitry Eremin-Solenikov

Convert to use gpio-pullup transceiver instead of relying on UDC driver managing GPIO.

---
 arch/arm/mach-pxa/corgi.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)
arch/arm/mach-pxa/corgi.c +11 -3
index 3a5507e..dd7b6a7 100644
@@ -32,6 +32,7 @@
 #include <linux/spi/pxa2xx_spi.h>
 #include <linux/mtd/sharpsl.h>
 #include <linux/input/matrix_keypad.h>
+#include <linux/usb/gpio_vbus.h>
 #include <video/w100fb.h>
 
 #include <asm/setup.h>
@@ -48,7 +49,6 @@
 #include <mach/pxa25x.h>
 #include <mach/irda.h>
 #include <mach/mmc.h>
-#include <mach/udc.h>
 #include <mach/corgi.h>
 #include <mach/sharpsl_pm.h>
 
@@ -461,11 +461,19 @@ static struct pxaficp_platform_data corgi_ficp_platform_data = {
 /*
  * USB Device Controller
  */
-static struct pxa2xx_udc_mach_info udc_info __initdata = {
+static struct gpio_vbus_mach_info udc_info = {
 	/* no connect GPIO; corgi can't tell connection status */
 	.gpio_pullup		= CORGI_GPIO_USB_PULLUP,
 };
 
+static struct platform_device corgi_gpio_pullup = {
+	.name	= "gpio-pullup",
+	.id	= -1,
+	.dev	= {
+		.platform_data	= &udc_info,
+	},
+};
+
 #if defined(CONFIG_SPI_PXA2XX) || defined(CONFIG_SPI_PXA2XX_MASTER)
 static struct pxa2xx_spi_master corgi_spi_info = {
 	.num_chipselect	= 3,
@@ -642,6 +650,7 @@ static struct platform_device *devices[] __initdata = {
 	&corgiled_device,
 	&sharpsl_nand_device,
 	&sharpsl_rom_device,
+	&corgi_gpio_pullup,
 };
 
 static struct i2c_board_info __initdata corgi_i2c_devices[] = {
@@ -691,7 +700,6 @@ static void __init corgi_init(void)
 
 	corgi_init_spi();
 
- 	pxa_set_udc_info(&udc_info);
 	pxa_set_mci_info(&corgi_mci_platform_data);
 	pxa_set_ficp_info(&corgi_ficp_platform_data);
 	pxa_set_i2c_info(NULL);
Signature · 1 line
1.7.5.4
#24 lore raw

PATCH 10/15 poodle: convert to use gpio-pullup

In reply to Dmitry Eremin-Solenikov

Convert to use gpio-pullup transceiver instead of relying on UDC driver managing GPIO.

---
 arch/arm/mach-pxa/poodle.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)
arch/arm/mach-pxa/poodle.c +10 -3
index 16d14fd..01aed35 100644
@@ -28,6 +28,7 @@
 #include <linux/spi/ads7846.h>
 #include <linux/spi/pxa2xx_spi.h>
 #include <linux/mtd/sharpsl.h>
+#include <linux/usb/gpio_vbus.h>
 
 #include <mach/hardware.h>
 #include <asm/mach-types.h>
@@ -41,7 +42,6 @@
 
 #include <mach/pxa25x.h>
 #include <mach/mmc.h>
-#include <mach/udc.h>
 #include <mach/irda.h>
 #include <mach/poodle.h>
 #include <mach/pxafb.h>
@@ -297,11 +297,18 @@ static struct pxaficp_platform_data poodle_ficp_platform_data = {
 /*
  * USB Device Controller
  */
-static struct pxa2xx_udc_mach_info udc_info __initdata = {
+static struct gpio_vbus_mach_info udc_info = {
 	/* no connect GPIO; poodle can't tell connection status */
 	.gpio_pullup	= POODLE_GPIO_USB_PULLUP,
 };
 
+static struct platform_device poodle_gpio_pullup = {
+	.name	= "gpio-pullup",
+	.id	= -1,
+	.dev	= {
+		.platform_data	= &udc_info,
+	},
+};
 
 /* PXAFB device */
 static struct pxafb_mode_info poodle_fb_mode = {
@@ -408,6 +415,7 @@ static struct platform_device *devices[] __initdata = {
 	&poodle_scoop_device,
 	&sharpsl_nand_device,
 	&sharpsl_rom_device,
+	&poodle_gpio_pullup,
 };
 
 static struct i2c_board_info __initdata poodle_i2c_devices[] = {
@@ -446,7 +454,6 @@ static void __init poodle_init(void)
 		pr_warning("poodle: Unable to register LoCoMo device\n");
 
 	pxa_set_fb_info(&poodle_locomo_device.dev, &poodle_fb_info);
-	pxa_set_udc_info(&udc_info);
 	pxa_set_mci_info(&poodle_mci_platform_data);
 	pxa_set_ficp_info(&poodle_ficp_platform_data);
 	pxa_set_i2c_info(NULL);
Signature · 1 line
1.7.5.4
#25 lore raw

PATCH 11/15 h5000: convert to use gpio-pullup

In reply to Dmitry Eremin-Solenikov

Convert to use gpio-pullup transceiver instead of relying on UDC driver managing GPIO.

---
 arch/arm/mach-pxa/h5000.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)
arch/arm/mach-pxa/h5000.c +11 -3
index 657db46..4b3ca92 100644
@@ -24,6 +24,7 @@
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/physmap.h>
+#include <linux/usb/gpio_vbus.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
@@ -31,7 +32,6 @@
 
 #include <mach/pxa25x.h>
 #include <mach/h5000.h>
-#include <mach/udc.h>
 #include <mach/smemc.h>
 
 #include "generic.h"
@@ -117,10 +117,18 @@ static struct platform_device h5000_flash[] = {
  * USB Device Controller
  */
 
-static struct pxa2xx_udc_mach_info h5000_udc_mach_info __initdata = {
+static struct gpio_vbus_mach_info h5000_udc_mach_info = {
 	.gpio_pullup = H5000_GPIO_USB_PULLUP,
 };
 
+static struct platform_device h5000_gpio_pullup = {
+	.name	= "gpio-pullup",
+	.id	= -1,
+	.dev	= {
+		.platform_data	= &h5000_udc_mach_info,
+	},
+};
+
 /*
  * GPIO setup
  */
@@ -187,6 +195,7 @@ static void fix_msc(void)
 static struct platform_device *devices[] __initdata = {
 	&h5000_flash[0],
 	&h5000_flash[1],
+	&h5000_gpio_pullup,
 };
 
 static void __init h5000_init(void)
@@ -197,7 +206,6 @@ static void __init h5000_init(void)
 	pxa_set_ffuart_info(NULL);
 	pxa_set_btuart_info(NULL);
 	pxa_set_stuart_info(NULL);
-	pxa_set_udc_info(&h5000_udc_mach_info);
 	platform_add_devices(ARRAY_AND_SIZE(devices));
 }
 
Signature · 1 line
1.7.5.4
#26 lore raw

PATCH 12/15 gpio-vbus: support disabling D+ pullup on suspend

In reply to Dmitry Eremin-Solenikov

Some platforms would like to disable D+ pullup on suspend, to drain as low power, as possible. E.g. this was requested by mioa701 board maintainers.

Suspend/resume is done in _noirq path, to force handling of pullup after the main UDC driver suspend/before resume.

---
 drivers/usb/otg/gpio_vbus.c   |   42 +++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/gpio_vbus.h |    1 +
 2 files changed, 43 insertions(+), 0 deletions(-)
drivers/usb/otg/gpio_vbus.c +42 -0
index 52733d9..3b27b1f 100644
@@ -327,6 +327,47 @@ static int __exit gpio_vbus_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int gpio_vbus_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct gpio_vbus_data *gpio_vbus = platform_get_drvdata(pdev);
+	struct gpio_vbus_mach_info *pdata = gpio_vbus->dev->platform_data;
+
+	if (gpio_vbus->otg.gadget && pdata->disconnect_on_suspend) {
+		/* optionally disable D+ pullup */
+		if (gpio_is_valid(pdata->gpio_pullup))
+			gpio_set_value(pdata->gpio_pullup,
+					pdata->gpio_pullup_inverted);
+
+		set_vbus_draw(gpio_vbus, 0);
+	}
+	return 0;
+}
+
+static int gpio_vbus_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct gpio_vbus_data *gpio_vbus = platform_get_drvdata(pdev);
+
+	if (gpio_vbus->otg.gadget)
+		schedule_work(&gpio_vbus->work);
+
+	return 0;
+}
+static const struct dev_pm_ops gpio_vbus_pm_ops = {
+	.suspend_noirq = gpio_vbus_suspend,
+	.resume_noirq = gpio_vbus_resume,
+	.freeze_noirq = gpio_vbus_suspend,
+	.thaw_noirq = gpio_vbus_resume,
+	.poweroff_noirq = gpio_vbus_suspend,
+	.restore_noirq = gpio_vbus_resume,
+};
+#define GPIO_VBUS_PM_OPS (&gpio_vbus_pm_ops)
+#else
+#define GPIO_VBUS_PM_OPS NULL
+#endif
+
 /* NOTE:  the gpio-vbus device may *NOT* be hotplugged */
 
 MODULE_ALIAS("platform:gpio-vbus");
@@ -335,6 +376,7 @@ static struct platform_driver gpio_vbus_driver = {
 	.driver = {
 		.name  = "gpio-vbus",
 		.owner = THIS_MODULE,
+		.pm = GPIO_VBUS_PM_OPS,
 	},
 	.remove  = __exit_p(gpio_vbus_remove),
 };
include/linux/usb/gpio_vbus.h +1 -0
index d9f03cc..2faa38d 100644
@@ -27,4 +27,5 @@ struct gpio_vbus_mach_info {
 	int gpio_pullup;
 	bool gpio_vbus_inverted;
 	bool gpio_pullup_inverted;
+	bool disconnect_on_suspend;
 };
Signature · 1 line
1.7.5.4
#27 lore raw

PATCH 13/15 mioa701: move gpio-pullup functionality to gpio-vbus

In reply to Dmitry Eremin-Solenikov

gpio-vbus can (and should?) handle D+ pullup gpio for us. Move gpio-pullup handling to gpio-vbus and stop providing udc_info as it's empty now. Also tell gpio-vbus that it should disable D+ pullup on suspend, as did pxa27x_udc driver.

---
 arch/arm/mach-pxa/mioa701.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)
arch/arm/mach-pxa/mioa701.c +2 -6
index d833a26..96c377c 100644
@@ -402,14 +402,11 @@ static void gsm_exit(void)
 /*
  * USB UDC
  */
-static struct pxa2xx_udc_mach_info mioa701_udc_info = {
-	.gpio_pullup	  = GPIO22_USB_ENABLE,
-};
-
 struct gpio_vbus_mach_info gpio_vbus_data = {
 	.gpio_vbus = GPIO13_nUSB_DETECT,
 	.gpio_vbus_inverted = 1,
-	.gpio_pullup = -1,
+	.gpio_pullup = GPIO22_USB_ENABLE,
+	.disconnect_on_suspend = 1,
 };
 
 /*
@@ -770,7 +767,6 @@ static void __init mioa701_machine_init(void)
 	pxa_set_fb_info(NULL, &mioa701_pxafb_info);
 	pxa_set_mci_info(&mioa701_mci_info);
 	pxa_set_keypad_info(&mioa701_keypad_info);
-	pxa_set_udc_info(&mioa701_udc_info);
 	pxa_set_ac97_info(&mioa701_ac97_info);
 	pm_power_off = mioa701_poweroff;
 	arm_pm_restart = mioa701_restart;
Signature · 1 line
1.7.5.4
#28 lore raw

PATCH 14/15 gpio-pullup: support disabling D+ pullup on suspend

In reply to Dmitry Eremin-Solenikov

As it's done by gpio-vbus, support disabling D+ pullup on suspend to save power.

---
 drivers/usb/otg/gpio_pullup.c |   45 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 45 insertions(+), 0 deletions(-)
drivers/usb/otg/gpio_pullup.c +45 -0
index 075d269..14c3322 100644
@@ -132,10 +132,55 @@ static int __devexit gpio_pullup_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int gpio_pullup_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct gpio_pullup_data *gpio_pullup = platform_get_drvdata(pdev);
+	struct gpio_vbus_mach_info *pdata = gpio_pullup->dev->platform_data;
+
+	/* optionally disable D+ pullup */
+	if (gpio_pullup->otg.gadget && pdata->disconnect_on_suspend)
+		gpio_set_value(pdata->gpio_pullup,
+				pdata->gpio_pullup_inverted);
+
+	return 0;
+}
+
+static int gpio_pullup_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct gpio_pullup_data *gpio_pullup = platform_get_drvdata(pdev);
+	struct gpio_vbus_mach_info *pdata = gpio_pullup->dev->platform_data;
+
+	if (gpio_pullup->otg.gadget)
+		gpio_set_value(pdata->gpio_pullup,
+				!pdata->gpio_pullup_inverted);
+
+	return 0;
+}
+static const struct dev_pm_ops gpio_pullup_pm_ops = {
+	.suspend_noirq = gpio_pullup_suspend,
+	.resume_noirq = gpio_pullup_resume,
+	.freeze_noirq = gpio_pullup_suspend,
+	.thaw_noirq = gpio_pullup_resume,
+	.poweroff_noirq = gpio_pullup_suspend,
+	.restore_noirq = gpio_pullup_resume,
+};
+#define GPIO_PULLUP_PM_OPS (&gpio_pullup_pm_ops)
+#else
+#define GPIO_PULLUP_PM_OPS NULL
+#endif
+
+/* NOTE:  the gpio-pullup device may *NOT* be hotplugged */
+
+MODULE_ALIAS("platform:gpio-pullup");
+
 static struct platform_driver gpio_pullup_driver = {
 	.driver = {
 		.name  = "gpio-pullup",
 		.owner = THIS_MODULE,
+		.pm = GPIO_PULLUP_PM_OPS,
 	},
 	.probe		= gpio_pullup_probe,
 	.remove		= __devexit_p(gpio_pullup_remove),
Signature · 1 line
1.7.5.4
#29 lore raw

PATCH 15/15 pxa2[57]x_udc: drop gpio_pullup handling

In reply to Dmitry Eremin-Solenikov

Now as all drivers were converted to using gpio-vbus, drop gpio-pullup handling from pxa UDC drivers, thus simplifying them a bit.

---
 arch/arm/include/asm/mach/udc_pxa2xx.h |    8 -----
 arch/arm/mach-pxa/balloon3.c           |    1 -
 arch/arm/mach-pxa/colibri-pxa320.c     |    1 -
 arch/arm/mach-pxa/devices.c            |    2 +-
 arch/arm/mach-pxa/vpac270.c            |    1 -
 drivers/usb/gadget/pxa25x_udc.c        |   54 --------------------------------
 drivers/usb/gadget/pxa27x_udc.c        |   27 +---------------
 7 files changed, 3 insertions(+), 91 deletions(-)
arch/arm/include/asm/mach/udc_pxa2xx.h +0 -8
index f0719d3..4cab1d6 100644
@@ -13,13 +13,5 @@ struct pxa2xx_udc_mach_info {
         void (*udc_command)(int cmd);
 #define	PXA2XX_UDC_CMD_CONNECT		0	/* let host see us */
 #define	PXA2XX_UDC_CMD_DISCONNECT	1	/* so host won't see us */
-
-	/* Boards following the design guidelines in the developer's manual,
-	 * with on-chip GPIOs not Lubbock's weird hardware, can have a sane
-	 * VBUS IRQ and omit the methods above.  Store the GPIO number
-	 * here.  Note that sometimes the signals go through inverters...
-	 */
-	bool	gpio_pullup_inverted;
-	int	gpio_pullup;			/* high == pullup activated */
 };
 
arch/arm/mach-pxa/balloon3.c +0 -1
index 4d0d8c3..a420c1e 100644
@@ -318,7 +318,6 @@ static void balloon3_udc_command(int cmd)
 
 static struct pxa2xx_udc_mach_info balloon3_udc_info __initdata = {
 	.udc_command		= balloon3_udc_command,
-	.gpio_pullup		= -1,
 };
 
 static void __init balloon3_udc_init(void)
arch/arm/mach-pxa/colibri-pxa320.c +0 -1
index ff9ff5f..f799b0a 100644
@@ -171,7 +171,6 @@ static void colibri_pxa320_udc_command(int cmd)
 
 static struct pxa2xx_udc_mach_info colibri_pxa320_udc_info __initdata = {
 	.udc_command		= colibri_pxa320_udc_command,
-	.gpio_pullup		= -1,
 };
 
 static void __init colibri_pxa320_init_udc(void)
arch/arm/mach-pxa/devices.c +1 -1
index 2e04254..59a3578 100644
@@ -89,7 +89,7 @@ void __init pxa_set_mci_info(struct pxamci_platform_data *info)
 
 
 static struct pxa2xx_udc_mach_info pxa_udc_info = {
-	.gpio_pullup = -1,
+	.udc_command = NULL,
 };
 
 void __init pxa_set_udc_info(struct pxa2xx_udc_mach_info *info)
arch/arm/mach-pxa/vpac270.c +0 -1
index 67bd414..3641935 100644
@@ -367,7 +367,6 @@ static void vpac270_udc_command(int cmd)
 
 static struct pxa2xx_udc_mach_info vpac270_udc_info __initdata = {
 	.udc_command		= vpac270_udc_command,
-	.gpio_pullup		= -1,
 };
 
 static void __init vpac270_udc_init(void)
drivers/usb/gadget/pxa25x_udc.c +0 -54

120 lines not on this page · raw message

drivers/usb/gadget/pxa27x_udc.c +2 -25
index 5760769..cdebfa9 100644
@@ -30,7 +30,6 @@
 #include <linux/proc_fs.h>
 #include <linux/clk.h>
 #include <linux/irq.h>
-#include <linux/gpio.h>
 #include <linux/slab.h>
 #include <linux/prefetch.h>
 
@@ -1519,15 +1518,9 @@ static struct usb_ep_ops pxa_ep_ops = {
 static void dplus_pullup(struct pxa_udc *udc, int on)
 {
 	if (on) {
-		if (gpio_is_valid(udc->mach->gpio_pullup))
-			gpio_set_value(udc->mach->gpio_pullup,
-				       !udc->mach->gpio_pullup_inverted);
 		if (udc->mach->udc_command)
 			udc->mach->udc_command(PXA2XX_UDC_CMD_CONNECT);
 	} else {
-		if (gpio_is_valid(udc->mach->gpio_pullup))
-			gpio_set_value(udc->mach->gpio_pullup,
-				       udc->mach->gpio_pullup_inverted);
 		if (udc->mach->udc_command)
 			udc->mach->udc_command(PXA2XX_UDC_CMD_DISCONNECT);
 	}
@@ -1620,7 +1613,7 @@ static int pxa_udc_pullup(struct usb_gadget *_gadget, int is_active)
 {
 	struct pxa_udc *udc = to_gadget_udc(_gadget);
 
-	if (!gpio_is_valid(udc->mach->gpio_pullup) && !udc->mach->udc_command)
+	if (!udc->mach->udc_command)
 		return -EOPNOTSUPP;
 
 	dplus_pullup(udc, is_active);
@@ -2459,7 +2452,7 @@ static int __init pxa_udc_probe(struct platform_device *pdev)
 {
 	struct resource *regs;
 	struct pxa_udc *udc = &memory;
-	int retval = 0, gpio;
+	int retval = 0;
 
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!regs)
@@ -2472,19 +2465,6 @@ static int __init pxa_udc_probe(struct platform_device *pdev)
 	udc->mach = pdev->dev.platform_data;
 	udc->transceiver = otg_get_transceiver();
 
-	gpio = udc->mach->gpio_pullup;
-	if (gpio_is_valid(gpio)) {
-		retval = gpio_request(gpio, "USB D+ pullup");
-		if (retval == 0)
-			gpio_direction_output(gpio,
-				       udc->mach->gpio_pullup_inverted);
-	}
-	if (retval) {
-		dev_err(&pdev->dev, "Couldn't request gpio %d : %d\n",
-			gpio, retval);
-		return retval;
-	}
-
 	udc->clk = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(udc->clk)) {
 		retval = PTR_ERR(udc->clk);
@@ -2535,13 +2515,10 @@ err_clk:
 static int __exit pxa_udc_remove(struct platform_device *_dev)
 {
 	struct pxa_udc *udc = platform_get_drvdata(_dev);
-	int gpio = udc->mach->gpio_pullup;
 
 	usb_gadget_unregister_driver(udc->driver);
 	free_irq(udc->irq, udc);
 	pxa_cleanup_debugfs(udc);
-	if (gpio_is_valid(gpio))
-		gpio_free(gpio);
 
 	otg_put_transceiver(udc->transceiver);
 
Signature · 1 line
1.7.5.4
#30 lore raw

PATCH 15/15 pxa2[57]x_udc: drop gpio_pullup handling

Quoted 4 lines · Dmitry Eremin-Solenikov

Now as all drivers were converted to using gpio-vbus, drop gpio-pullup handling from pxa UDC drivers, thus simplifying them a bit.

for drivers/usb/gadget:

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110705/e06a378e/attachment.sig>
maintainer
#31 lore raw

PATCH 15/15 pxa2[57]x_udc: drop gpio_pullup handling

In reply to Dmitry Eremin-Solenikov
Quoted 2 lines · Dmitry Eremin-Solenikov

Now as all drivers were converted to using gpio-vbus, drop gpio-pullup handling from pxa UDC drivers, thus simplifying them a bit.

Is this patch compliant with USB 2 specification regarding maximum time between pullup triggering and UDC ready to answer the "RESET" packet and be assigned an address ?

This is the sequence I'm worried about :
(1) The gpio-vbus is loaded
(2) The pxa27x_udc is loaded
(3) USB cable is plugged
=> VBUS is sensed
(4) gpio-vbus pulls up the D+ (Dmitry, is that right ?)
(5) the host waits for UDC to settle (100 ms ?)
(5) set address packet(s) are sent from host to UDC
(6) UDC is not enabled, as no gadget is registered => UDC doesn't answer
=> usb host cannot assign it an address
(7) a gadget is loaded (g_ether for example) => UDC is enabled, but too late

Alan, Gregh, could you confirm point (5) about a maximum time between D+ line activation and UDC duty to repond to "set address" packets, as well as the consequence of point (6) where the usb device is rejected because no address could be assigned ?

Dmitry, could you confirm the the D+ pullup is done at point (4) ?

Cheers.

--
Robert

#32 lore raw

PATCH 15/15 pxa2[57]x_udc: drop gpio_pullup handling

Quoted 22 lines · Robert Jarzmik
Quoted 2 lines · Dmitry Eremin-Solenikov

Now as all drivers were converted to using gpio-vbus, drop gpio-pullup handling from pxa UDC drivers, thus simplifying them a bit.

Is this patch compliant with USB 2 specification regarding maximum time between pullup triggering and UDC ready to answer the "RESET" packet and be assigned an address ?

This is the sequence I'm worried about :
(1) The gpio-vbus is loaded
(2) The pxa27x_udc is loaded
(3) USB cable is plugged
=> VBUS is sensed
(4) gpio-vbus pulls up the D+ (Dmitry, is that right ?)
(5) the host waits for UDC to settle (100 ms ?)
(5) set address packet(s) are sent from host to UDC
(6) UDC is not enabled, as no gadget is registered => UDC doesn't answer
=> usb host cannot assign it an address
(7) a gadget is loaded (g_ether for example) => UDC is enabled, but too late

Quoted 6 lines

Alan, Gregh, could you confirm point (5) about a maximum time between D+ line activation and UDC duty to repond to "set address" packets, as well as the consequence of point (6) where the usb device is rejected because no address could be assigned ?

Dmitry, could you confirm the the D+ pullup is done at point (4) ?

No, I can't confirm this. IIUC, evrything happens in abit different way:
(4) trancseiver notifies udc about VBUS sense
(5) a gadget is loaded (g_ether for example)
(6) gadget asks udc to enale pullup
(7) udc tells gpio_vbus to enable pullup.

etc...

Quoted 6 lines

Cheers.

--
Robert

Signature · 2 lines
With best wishes
Dmitry
#33 lore raw

PATCH 15/15 pxa2[57]x_udc: drop gpio_pullup handling

hi,

Quoted 28 lines · Dmitry Eremin-Solenikov
Quoted 27 lines · Robert Jarzmik
Quoted 2 lines · Dmitry Eremin-Solenikov

Now as all drivers were converted to using gpio-vbus, drop gpio-pullup handling from pxa UDC drivers, thus simplifying them a bit.

Is this patch compliant with USB 2 specification regarding maximum time between pullup triggering and UDC ready to answer the "RESET" packet and be assigned an address ?

This is the sequence I'm worried about :
(1) The gpio-vbus is loaded
(2) The pxa27x_udc is loaded
(3) USB cable is plugged
=> VBUS is sensed
(4) gpio-vbus pulls up the D+ (Dmitry, is that right ?)
(5) the host waits for UDC to settle (100 ms ?)
(5) set address packet(s) are sent from host to UDC
(6) UDC is not enabled, as no gadget is registered => UDC doesn't answer
=> usb host cannot assign it an address
(7) a gadget is loaded (g_ether for example) => UDC is enabled, but too late

Alan, Gregh, could you confirm point (5) about a maximum time between D+ line activation and UDC duty to repond to "set address" packets, as well as the consequence of point (6) where the usb device is rejected because no address could be assigned ?

Quoted 7 lines
Quoted 1 line

Dmitry, could you confirm the the D+ pullup is done at point (4) ?

No, I can't confirm this. IIUC, evrything happens in abit different way:
(4) trancseiver notifies udc about VBUS sense
(5) a gadget is loaded (g_ether for example)
(6) gadget asks udc to enale pullup
(7) udc tells gpio_vbus to enable pullup.

yeah, with udc-core.c we only enable pullup after gadget driver is loaded.

290 ret = bind(udc->gadget);
291 if (ret)
292 goto err1;
293 ret = usb_gadget_udc_start(udc->gadget, driver);
294 if (ret) {
295 driver->unbind(udc->gadget);
296 goto err1;
297 }
298 usb_gadget_connect(udc->gadget);

but this is the new style bind() and many UDCs still need to be converted.

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110706/eb2b3213/attachment.sig>
#34 lore raw

PATCH 15/15 pxa2[57]x_udc: drop gpio_pullup handling

Quoted 52 lines · Felipe Balbi

hi,

Quoted 28 lines · Dmitry Eremin-Solenikov
Quoted 27 lines · Robert Jarzmik
Quoted 2 lines · Dmitry Eremin-Solenikov

Now as all drivers were converted to using gpio-vbus, drop gpio-pullup handling from pxa UDC drivers, thus simplifying them a bit.

Is this patch compliant with USB 2 specification regarding maximum time between pullup triggering and UDC ready to answer the "RESET" packet and be assigned an address ?

This is the sequence I'm worried about :
(1) The gpio-vbus is loaded
(2) The pxa27x_udc is loaded
(3) USB cable is plugged
=> VBUS is sensed
(4) gpio-vbus pulls up the D+ (Dmitry, is that right ?)
(5) the host waits for UDC to settle (100 ms ?)
(5) set address packet(s) are sent from host to UDC
(6) UDC is not enabled, as no gadget is registered => UDC doesn't answer
=> usb host cannot assign it an address
(7) a gadget is loaded (g_ether for example) => UDC is enabled, but too late

Alan, Gregh, could you confirm point (5) about a maximum time between D+ line activation and UDC duty to repond to "set address" packets, as well as the consequence of point (6) where the usb device is rejected because no address could be assigned ?

Quoted 7 lines
Quoted 1 line

Dmitry, could you confirm the the D+ pullup is done at point (4) ?

No, I can't confirm this. IIUC, evrything happens in abit different way:
(4) trancseiver notifies udc about VBUS sense
(5) a gadget is loaded (g_ether for example)
(6) gadget asks udc to enale pullup
(7) udc tells gpio_vbus to enable pullup.

yeah, with udc-core.c we only enable pullup after gadget driver is loaded.

290 ret = bind(udc->gadget);
291 if (ret)
292 goto err1;
293 ret = usb_gadget_udc_start(udc->gadget, driver);
294 if (ret) {
295 driver->unbind(udc->gadget);
296 goto err1;
297 }
298 usb_gadget_connect(udc->gadget);

Quoted 2 lines

but this is the new style bind() and many UDCs still need to be converted.

This was working before the udc-core. If it doesn't work now for unconverted drivers (PXA e.g.) that's really a huge regression!

Signature · 2 lines
With best wishes
Dmitry
#35 lore raw

PATCH 15/15 pxa2[57]x_udc: drop gpio_pullup handling

Hi,

Quoted 53 lines · Dmitry Eremin-Solenikov
Quoted 52 lines · Felipe Balbi

hi,

Quoted 28 lines · Dmitry Eremin-Solenikov
Quoted 27 lines · Robert Jarzmik
Quoted 2 lines · Dmitry Eremin-Solenikov

Now as all drivers were converted to using gpio-vbus, drop gpio-pullup handling from pxa UDC drivers, thus simplifying them a bit.

Is this patch compliant with USB 2 specification regarding maximum time between pullup triggering and UDC ready to answer the "RESET" packet and be assigned an address ?

This is the sequence I'm worried about :
(1) The gpio-vbus is loaded
(2) The pxa27x_udc is loaded
(3) USB cable is plugged
=> VBUS is sensed
(4) gpio-vbus pulls up the D+ (Dmitry, is that right ?)
(5) the host waits for UDC to settle (100 ms ?)
(5) set address packet(s) are sent from host to UDC
(6) UDC is not enabled, as no gadget is registered => UDC doesn't answer
=> usb host cannot assign it an address
(7) a gadget is loaded (g_ether for example) => UDC is enabled, but too late

Alan, Gregh, could you confirm point (5) about a maximum time between D+ line activation and UDC duty to repond to "set address" packets, as well as the consequence of point (6) where the usb device is rejected because no address could be assigned ?

Quoted 7 lines
Quoted 1 line

Dmitry, could you confirm the the D+ pullup is done at point (4) ?

No, I can't confirm this. IIUC, evrything happens in abit different way:
(4) trancseiver notifies udc about VBUS sense
(5) a gadget is loaded (g_ether for example)
(6) gadget asks udc to enale pullup
(7) udc tells gpio_vbus to enable pullup.

yeah, with udc-core.c we only enable pullup after gadget driver is loaded.

290 ret = bind(udc->gadget);
291 if (ret)
292 goto err1;
293 ret = usb_gadget_udc_start(udc->gadget, driver);
294 if (ret) {
295 driver->unbind(udc->gadget);
296 goto err1;
297 }
298 usb_gadget_connect(udc->gadget);

Quoted 5 lines
Quoted 2 lines

but this is the new style bind() and many UDCs still need to be converted.

This was working before the udc-core. If it doesn't work now for unconverted drivers (PXA e.g.) that's really a huge regression!

the difference is that before every UDC had to implement that by itself and now we're moving the common parts to a generic layer. If your UDC already implemented it this way when exporting usb_gadget_probe_driver(), then no changes for you, but I guarantee there where UDCs which didn't mind being careful with data pullups and would connect even though a gadget driver was nowhere to be seen ;-)

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110706/a135c906/attachment.sig>
#36 lore raw

cover Big pxa2[57]x_udc cleanup

In reply to Dmitry Eremin-Solenikov
Quoted 49 lines · Dmitry Eremin-Solenikov

Hello colleagues,

Please review the following patch serie. It's major task is removal of code and functionality duplication between pxa2[57]x_udc drivers and OTG transceivers subsystem.

The following changes since commit fe0d42203cb5616eeff68b14576a0f7e2dd56625:

?Linux 3.0-rc6 (2011-07-04 15:56:24 -0700)

are available in the git repository at:
?git://git.kernel.org/pub/scm/linux/kernel/git/lumag/tosa-2.6.git misc/pxa-udc-cleanup

Dmitry Eremin-Solenikov (15):
? ? ?ARM: pxa/balloon3: drop udc_is_connected
? ? ?ARM: pxa/mioa701: drop udc_is_connected
? ? ?ARM: pxa/stargate2: drop udc_is_connected
? ? ?ARM: pxa25x_udc: separate lubbock handling to lubbock-usb transceiver ? ? ?ARM: pxa/lubbock: switch to using lubbock_usb_xceiv transceiver ? ? ?pxa: drop handling of udc_is_connected
? ? ?pxa25x_udc: drop support for udc_command
? ? ?otg: add gpio_pullup OTG transceiver for devices with no VBUS sensing ? ? ?corgi: convert to use gpio-pullup
? ? ?poodle: convert to use gpio-pullup
? ? ?h5000: convert to use gpio-pullup
? ? ?gpio-vbus: support disabling D+ pullup on suspend
? ? ?mioa701: move gpio-pullup functionality to gpio-vbus
? ? ?gpio-pullup: support disabling D+ pullup on suspend
? ? ?pxa2[57]x_udc: drop gpio_pullup handling

?arch/arm/include/asm/mach/udc_pxa2xx.h | ? ?9 --
?arch/arm/mach-pxa/balloon3.c ? ? ? ? ? | ? ?7 -
?arch/arm/mach-pxa/colibri-pxa320.c ? ? | ? ?1 -
?arch/arm/mach-pxa/corgi.c ? ? ? ? ? ? ?| ? 14 ++-
?arch/arm/mach-pxa/devices.c ? ? ? ? ? ?| ? ?2 +-
?arch/arm/mach-pxa/h5000.c ? ? ? ? ? ? ?| ? 14 ++-
?arch/arm/mach-pxa/lubbock.c ? ? ? ? ? ?| ? 17 +--
?arch/arm/mach-pxa/mioa701.c ? ? ? ? ? ?| ? 19 +--
?arch/arm/mach-pxa/poodle.c ? ? ? ? ? ? | ? 13 ++-
?arch/arm/mach-pxa/stargate2.c ? ? ? ? ?| ? ?6 -
?arch/arm/mach-pxa/vpac270.c ? ? ? ? ? ?| ? ?1 -
?drivers/usb/gadget/pxa25x_udc.c ? ? ? ?| ?132 ---------------------
?drivers/usb/gadget/pxa27x_udc.c ? ? ? ?| ? 27 +----
?drivers/usb/otg/Kconfig ? ? ? ? ? ? ? ?| ? 18 +++
?drivers/usb/otg/Makefile ? ? ? ? ? ? ? | ? ?2 +
?drivers/usb/otg/gpio_pullup.c ? ? ? ? ?| ?204 ++++++++++++++++++++++++++++++++
?drivers/usb/otg/gpio_vbus.c ? ? ? ? ? ?| ? 42 +++++++
?drivers/usb/otg/lubbock-usb.c ? ? ? ? ?| ?178 ++++++++++++++++++++++++++++
?include/linux/usb/gpio_vbus.h ? ? ? ? ?| ? ?1 +
Quoted 4 lines

?19 files changed, 493 insertions(+), 214 deletions(-) ?create mode 100644 drivers/usb/otg/gpio_pullup.c ?create mode 100644 drivers/usb/otg/lubbock-usb.c

Hi Dmitry,

The series looks good to me. It can be better re-ordered so that changes to drivers/usb are the first part, and changes to arch/arm later?

Dropping the udc_is_connected() in each board file, I don't mind they are in a same patch or separated.

Greg & Felipe,

Can you help merge the patches through the USB tree? Or if you can give your Acks here. Thanks.

- eric

Quoted 1 line