[PATCH 0/2] ARM: pxa: palm27x: device initialization fixes

[PATCH 0/2] ARM: pxa: palm27x: device initialization fixes

[Mike Dunn]

Hi,

I/ve been working on getting the kernel running smoothly on my palm treo 680.
To start, here are a couple short patches that fix problems with udc and lcd
device initialization.  Thanks for looking.  Happy holidays!

Mike Dunn (2):
  ARM: pxa: palm27x: fix udc device initialization
  ARM: pxa: palm27x: fix lcd device initialization

 arch/arm/mach-pxa/palm27x.c |   26 ++++++++++++++++----------
 1 files changed, 16 insertions(+), 10 deletions(-)

-- 
1.7.8.6

[PATCH 1/2] ARM: pxa: palm27x: fix udc device initialization

[Mike Dunn]

This patch fixes some bad behaviour from the usb gadget during machine
initialization by changing the management of the D+ pull-up gpio from the
gpio-vbus driver to the pxa27x-udc driver.  Also, code that drives the pull-up
high is removed.  (The gpio-vbus driver can optionally manage the D+ line
pull-up, but the pxa27x-udc driver does this itself.)

Without this patch, the host senses the presence of the usb gadget during
machine initialization (when palm27x_udc_init() runs), at which point it tries
to enumerate the newly detected usb gadget.  But because the pxa27x-udc driver
has not been initialized yet, there's no gadget driver to respond to the host,
and enumeration fails.  Tested on my Palm Treo680.

Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
---
 arch/arm/mach-pxa/palm27x.c |   21 +++++++++++++--------
 1 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-pxa/palm27x.c b/arch/arm/mach-pxa/palm27x.c
index 17d4c53..298a8a9 100644
--- a/arch/arm/mach-pxa/palm27x.c
+++ b/arch/arm/mach-pxa/palm27x.c
@@ -167,7 +167,7 @@ void __init palm27x_lcd_init(int power, struct pxafb_mode_info *mode)
 #if	defined(CONFIG_USB_PXA27X) || \
 	defined(CONFIG_USB_PXA27X_MODULE)
 static struct gpio_vbus_mach_info palm27x_udc_info = {
-	.gpio_vbus_inverted	= 1,
+	.gpio_pullup = -1,  /* pxa27x-udc driver handles D+ pull-up, not vbus */
 };
 
 static struct platform_device palm27x_gpio_vbus = {
@@ -180,17 +180,22 @@ static struct platform_device palm27x_gpio_vbus = {
 
 void __init palm27x_udc_init(int vbus, int pullup, int vbus_inverted)
 {
-	palm27x_udc_info.gpio_vbus	= vbus;
-	palm27x_udc_info.gpio_pullup	= pullup;
+	struct pxa2xx_udc_mach_info udc_mach_info = {
+		.udc_is_connected = NULL,
+		.udc_command = NULL,
+		.gpio_pullup_inverted = false,
+	};
+	udc_mach_info.gpio_pullup = pullup;
 
+	palm27x_udc_info.gpio_vbus	= vbus;
 	palm27x_udc_info.gpio_vbus_inverted = vbus_inverted;
 
-	if (!gpio_request(pullup, "USB Pullup")) {
-		gpio_direction_output(pullup,
-			palm27x_udc_info.gpio_vbus_inverted);
-		gpio_free(pullup);
-	} else
+	/* driver will quietly ignore an invalid gpio */
+	if (!gpio_is_valid(pullup)) {
+		pr_err("Palm27x: USB D+ pull-up gpio is invalid!\n");
 		return;
+	}
+	pxa_set_udc_info(&udc_mach_info);
 
 	platform_device_register(&palm27x_gpio_vbus);
 }
-- 
1.7.8.6

[PATCH 1/2] ARM: pxa: palm27x: fix udc device initialization

[Marek Vasut]

Dear Mike Dunn,

> This patch fixes some bad behaviour from the usb gadget during machine
> initialization by changing the management of the D+ pull-up gpio from the
> gpio-vbus driver to the pxa27x-udc driver.  Also, code that drives the
> pull-up high is removed.  (The gpio-vbus driver can optionally manage the
> D+ line pull-up, but the pxa27x-udc driver does this itself.)
> 
> Without this patch, the host senses the presence of the usb gadget during
> machine initialization (when palm27x_udc_init() runs), at which point it
> tries to enumerate the newly detected usb gadget.  But because the
> pxa27x-udc driver has not been initialized yet, there's no gadget driver
> to respond to the host, and enumeration fails.  Tested on my Palm Treo680.
[...]

I think it was the whole big idea to let gpio-vbus manage this kind of stuff. 
But it's been a while, Ccing Haojian to review these.

Best regards,
Marek Vasut

[PATCH 1/2] ARM: pxa: palm27x: fix udc device initialization

[Robert Jarzmik]

Marek Vasut <marex@denx.de> writes:

> Dear Mike Dunn,
>
>> This patch fixes some bad behaviour from the usb gadget during machine
>> initialization by changing the management of the D+ pull-up gpio from the
>> gpio-vbus driver to the pxa27x-udc driver.  Also, code that drives the
>> pull-up high is removed.  (The gpio-vbus driver can optionally manage the
>> D+ line pull-up, but the pxa27x-udc driver does this itself.)
>> 
>> Without this patch, the host senses the presence of the usb gadget during
>> machine initialization (when palm27x_udc_init() runs), at which point it
>> tries to enumerate the newly detected usb gadget.  But because the
>> pxa27x-udc driver has not been initialized yet, there's no gadget driver
>> to respond to the host, and enumeration fails.  Tested on my Palm Treo680.
> [...]
>
> I think it was the whole big idea to let gpio-vbus manage this kind of stuff. 
> But it's been a while, Ccing Haojian to review these.

Actually gpio-vbus was designed to :
 - detect and handle VBus
 - and handle following D+ pullup if peripheral controller *can't*

But pxa27x_udc is a peripheral controller which does handle D+ pullup, and
expects to handle it by himself (ie. is not fully compatible with a D+ pullup
handling by gpio-vbus, one not working case being Mike's one).

So Mike's patch makes sense IMHO. He could have left the VBUS handling to
gpio-vbus and D+ to pxa27x_udc, or let pxa27x_udc handle both, that's up to you.

Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>

-- 
Robert

[PATCH 1/2] ARM: pxa: palm27x: fix udc device initialization

[Mike Dunn]

Hi Robert,

On 12/24/2012 07:34 AM, Robert Jarzmik wrote:

[..]


 Actually gpio-vbus was designed to :
>  - detect and handle VBus
>  - and handle following D+ pullup if peripheral controller *can't*
> 
> But pxa27x_udc is a peripheral controller which does handle D+ pullup, and
> expects to handle it by himself (ie. is not fully compatible with a D+ pullup
> handling by gpio-vbus, one not working case being Mike's one).
> 
> So Mike's patch makes sense IMHO. He could have left the VBUS handling to
> gpio-vbus and D+ to pxa27x_udc, or let pxa27x_udc handle both, that's up to you.


I didn't realize the pxa27x-udc driver could handle the vbus as well.


> Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>


Thanks Robert.  Should have CC'd you, since you're the udc driver author.  Glad
you saw this.


Thanks again,
MIke

[PATCH 2/2] ARM: pxa: palm27x: fix lcd device initialization

[Mike Dunn]

This fixes incorrect tests of the return codes from gpiolib functions in
palm27x_lcd_init().  Currently a return code of 0 is incorrectly interpreted as
an error.  A call to gpio_free() is also added.

Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
---
 arch/arm/mach-pxa/palm27x.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-pxa/palm27x.c b/arch/arm/mach-pxa/palm27x.c
index 298a8a9..473c9e3 100644
--- a/arch/arm/mach-pxa/palm27x.c
+++ b/arch/arm/mach-pxa/palm27x.c
@@ -145,12 +145,13 @@ void __init palm27x_lcd_init(int power, struct pxafb_mode_info *mode)
 	palm27x_lcd_screen.modes = mode;
 
 	if (gpio_is_valid(power)) {
-		if (!gpio_request(power, "LCD power")) {
+		if (gpio_request(power, "LCD power") < 0) {
 			pr_err("Palm27x: failed to claim lcd power gpio!\n");
 			return;
 		}
-		if (!gpio_direction_output(power, 1)) {
+		if (gpio_direction_output(power, 1) < 0) {
 			pr_err("Palm27x: lcd power configuration failed!\n");
+			gpio_free(power);
 			return;
 		}
 		palm27x_lcd_power = power;
-- 
1.7.8.6

[PATCH 2/2] ARM: pxa: palm27x: fix lcd device initialization

[Marek Vasut]

Dear Mike Dunn,

> This fixes incorrect tests of the return codes from gpiolib functions in
> palm27x_lcd_init().  Currently a return code of 0 is incorrectly
> interpreted as an error.  A call to gpio_free() is also added.
> 
> Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
> ---
>  arch/arm/mach-pxa/palm27x.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-pxa/palm27x.c b/arch/arm/mach-pxa/palm27x.c
> index 298a8a9..473c9e3 100644
> --- a/arch/arm/mach-pxa/palm27x.c
> +++ b/arch/arm/mach-pxa/palm27x.c
> @@ -145,12 +145,13 @@ void __init palm27x_lcd_init(int power, struct
> pxafb_mode_info *mode) palm27x_lcd_screen.modes = mode;
> 
>  	if (gpio_is_valid(power)) {
> -		if (!gpio_request(power, "LCD power")) {
> +		if (gpio_request(power, "LCD power") < 0) {
>  			pr_err("Palm27x: failed to claim lcd power gpio!\n");
>  			return;
>  		}
> -		if (!gpio_direction_output(power, 1)) {
> +		if (gpio_direction_output(power, 1) < 0) {
>  			pr_err("Palm27x: lcd power configuration failed!\n");
> +			gpio_free(power);
>  			return;
>  		}
>  		palm27x_lcd_power = power;

Acked-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut