diff mbox

[U-Boot,4/5] usbtty: init endpoints prior to startup events

Message ID 1318864970-11608-4-git-send-email-sherbrec@cit-ec.uni-bielefeld.de
State Awaiting Upstream
Headers show

Commit Message

Stefan Herbrechtsmeier Oct. 17, 2011, 3:22 p.m. UTC
On some usb device controllers (pxa) the endpoint configuration must be programmed prior to enable it.

Signed-off-by: Stefan Herbrechtsmeier <sherbrec@cit-ec.uni-bielefeld.de>
CC: Marek Vasut <marek.vasut@gmail.com>
CC: Remy Bohmer  <linux@bohmer.net>
---
 drivers/serial/usbtty.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Comments

Remy Bohmer Nov. 26, 2011, 10:25 p.m. UTC | #1
Hi,

2011/10/17 Stefan Herbrechtsmeier <sherbrec@cit-ec.uni-bielefeld.de>:
> On some usb device controllers (pxa) the endpoint configuration must be programmed prior to enable it.
>
> Signed-off-by: Stefan Herbrechtsmeier <sherbrec@cit-ec.uni-bielefeld.de>
> CC: Marek Vasut <marek.vasut@gmail.com>
> CC: Remy Bohmer  <linux@bohmer.net>
> ---
>  drivers/serial/usbtty.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)

Applied to u-boot-usb

Kind regards,

Remy
Stefan Herbrechtsmeier Nov. 28, 2011, 10:05 a.m. UTC | #2
Am 26.11.2011 23:25, schrieb Remy Bohmer:
> 2011/10/17 Stefan Herbrechtsmeier<sherbrec@cit-ec.uni-bielefeld.de>:
>> On some usb device controllers (pxa) the endpoint configuration must be programmed prior to enable it.
>>
>> Signed-off-by: Stefan Herbrechtsmeier<sherbrec@cit-ec.uni-bielefeld.de>
>> CC: Marek Vasut<marek.vasut@gmail.com>
>> CC: Remy Bohmer<linux@bohmer.net>
>> ---
>>   drivers/serial/usbtty.c |    4 ++--
>>   1 files changed, 2 insertions(+), 2 deletions(-)
> Applied to u-boot-usb
I was surprised that the patch was applied without comments as it change 
the usbtty core
behaviour and has the possibility to break other drivers.

Should I CC the affected driver and board maintainer to hopefully get 
some feedback?

Regards,
     Stefan
Remy Bohmer Nov. 30, 2011, 9:21 p.m. UTC | #3
Hi Stefan,

2011/11/28 Stefan Herbrechtsmeier <sherbrec@cit-ec.uni-bielefeld.de>:
> Am 26.11.2011 23:25, schrieb Remy Bohmer:
>
>> 2011/10/17 Stefan Herbrechtsmeier<sherbrec@cit-ec.uni-bielefeld.de>:
>>>
>>> On some usb device controllers (pxa) the endpoint configuration must be
>>> programmed prior to enable it.
>>>
>>> Signed-off-by: Stefan Herbrechtsmeier<sherbrec@cit-ec.uni-bielefeld.de>
>>> CC: Marek Vasut<marek.vasut@gmail.com>
>>> CC: Remy Bohmer<linux@bohmer.net>
>>> ---
>>>  drivers/serial/usbtty.c |    4 ++--
>>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> Applied to u-boot-usb
>
> I was surprised that the patch was applied without comments as it change the
> usbtty core
> behaviour and has the possibility to break other drivers.

I was not really surprised ;-)
Looking at the code it seemed to me that it was indeed cleaner to
initialize the endpoints before enabling them.
That you have problems on your board confirmed that. So, to me this
was a plain bugfix.

> Should I CC the affected driver and board maintainer to hopefully get some
> feedback?

This would not harm at all. I did not ask Wolfgang to pull the
u-boot-usb tree yet, so it is not in mainline yet.
I need to do some more testing on the current u-boot-usb tree as well.
So, if more people look at it, it would be a good thing.

Kind regards,

Remy
Stefan Herbrechtsmeier Dec. 2, 2011, 12:12 p.m. UTC | #4
Am 30.11.2011 22:21, schrieb Remy Bohmer:
> 2011/11/28 Stefan Herbrechtsmeier<sherbrec@cit-ec.uni-bielefeld.de>:
>> Am 26.11.2011 23:25, schrieb Remy Bohmer:
>>
>>> 2011/10/17 Stefan Herbrechtsmeier<sherbrec@cit-ec.uni-bielefeld.de>:
>>>> On some usb device controllers (pxa) the endpoint configuration must be
>>>> programmed prior to enable it.
>>>>
>>>> Signed-off-by: Stefan Herbrechtsmeier<sherbrec@cit-ec.uni-bielefeld.de>
>>>> CC: Marek Vasut<marek.vasut@gmail.com>
>>>> CC: Remy Bohmer<linux@bohmer.net>
>>>> ---
>>>>   drivers/serial/usbtty.c |    4 ++--
>>>>   1 files changed, 2 insertions(+), 2 deletions(-)
>>> Applied to u-boot-usb
>> I was surprised that the patch was applied without comments as it change the
>> usbtty core
>> behaviour and has the possibility to break other drivers.
> I was not really surprised ;-)
> Looking at the code it seemed to me that it was indeed cleaner to
> initialize the endpoints before enabling them.
> That you have problems on your board confirmed that. So, to me this
> was a plain bugfix.
Ideally the change should not break other boards, as udc_setup_ep in 
usbtty_init_endpoints
is already used before udc_startup_events in usbtty_init_instances.
>> Should I CC the affected driver and board maintainer to hopefully get some
>> feedback?
> This would not harm at all. I did not ask Wolfgang to pull the
> u-boot-usb tree yet, so it is not in mainline yet.
> I need to do some more testing on the current u-boot-usb tree as well.
> So, if more people look at it, it would be a good thing.
I have cc the maintainer and / or author of the USB related changes for the
Adder (mpc8xx_udc) and SX1 (omap1510_udc),
the custodian of the omap and davinci (musb_udc)
and the maintainer of the spear boards (spr_udc).

It would be nice if the affected people could confirm, that the change 
don't break there
USB device support.

Kindly regards,
     Stefan
diff mbox

Patch

diff --git a/drivers/serial/usbtty.c b/drivers/serial/usbtty.c
index cffd5a2..e2e87fe 100644
--- a/drivers/serial/usbtty.c
+++ b/drivers/serial/usbtty.c
@@ -554,11 +554,11 @@  int drv_usbtty_init (void)
 	usbtty_init_strings ();
 	usbtty_init_instances ();
 
+	usbtty_init_endpoints ();
+
 	udc_startup_events (device_instance);/* Enable dev, init udc pointers */
 	udc_connect ();		/* Enable pullup for host detection */
 
-	usbtty_init_endpoints ();
-
 	/* Device initialization */
 	memset (&usbttydev, 0, sizeof (usbttydev));