[U-Boot] (no subject)

[U-Boot] (no subject)

[Samuel Egli]

Hi all,

this is a re-submission of Bin Liu's patch that was 
reverted by the commit 16b61d13bab361853564da401b15fc34ae1dfea7
from Daniel Mack.

It is not OK to set the MUSB_POWER_HSENAB unconditionally because
there are sometimes some HW limitations that allow only FULL SPEED.

In our case  we have some bad wiring protection in our USB circuit
that acts as a filter an will not allow signals to pass that
are faster than FULL SPEED.

I suggest to use CONFIG_USB_GADGET_DUALSPEED define to enable 
HIGH SPEED as it was done previously, which is by default 
enabled. 

Kind regards

Sam

[U-Boot] [PATCH] musb: set MUSB speed based on CONFIG

[Samuel Egli]

From: Bin Liu <b-liu@ti.com>

Do not config MUSB to highspeed mode if CONFIG_USB_GADGET_DUALSPEED
is not set, in which case Ether gadget only operates in fullspeed.

Note:
This patch is necessary for devices like some siemens boards
that allow only FULL SPEED USB 1.1, e.g. DFU download.

Signed-off-by: Bin Liu <b-liu@ti.com>
Reviewed-by: Tom Rini <trini@konsulko.com>
Tested-by: Samuel Egli <samuel.egli@siemens.com>
CC: Marek Vasut <marex@denx.de>
CC: Heiko Schocher <hs@denx.de>
CC: Daniel Mack <zonque@gmail.com>
CC: Roger Meier <r.meier@siemens.com>
---
 drivers/usb/musb-new/musb_core.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/musb-new/musb_core.c b/drivers/usb/musb-new/musb_core.c
index 242cc30..4edd6d7 100644
--- a/drivers/usb/musb-new/musb_core.c
+++ b/drivers/usb/musb-new/musb_core.c
@@ -942,7 +942,9 @@ void musb_start(struct musb *musb)

 	/* put into basic highspeed mode and start session */
 	musb_writeb(regs, MUSB_POWER, MUSB_POWER_ISOUPDATE
+#ifdef CONFIG_USB_GADGET_DUALSPEED
 						| MUSB_POWER_HSENAB
+#endif
 						/* ENSUSPEND wedges tusb */
 						/* | MUSB_POWER_ENSUSPEND */
 						);
--
1.7.10.4

[U-Boot] [PATCH] musb: set MUSB speed based on CONFIG

[Hans de Goede]

Hi,

On 10-07-15 15:16, Samuel Egli wrote:
> From: Bin Liu <b-liu@ti.com>
>
> Do not config MUSB to highspeed mode if CONFIG_USB_GADGET_DUALSPEED
> is not set, in which case Ether gadget only operates in fullspeed.
>
> Note:
> This patch is necessary for devices like some siemens boards
> that allow only FULL SPEED USB 1.1, e.g. DFU download.
>
> Signed-off-by: Bin Liu <b-liu@ti.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> Tested-by: Samuel Egli <samuel.egli@siemens.com>
> CC: Marek Vasut <marex@denx.de>
> CC: Heiko Schocher <hs@denx.de>
> CC: Daniel Mack <zonque@gmail.com>
> CC: Roger Meier <r.meier@siemens.com>

Nack this breaks highspeed mode on boards which use the musb
in host mode, and thus do not set CONFIG_USB_GADGET_DUALSPEED.

Regards,

Hans


> ---
>   drivers/usb/musb-new/musb_core.c |    2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/usb/musb-new/musb_core.c b/drivers/usb/musb-new/musb_core.c
> index 242cc30..4edd6d7 100644
> --- a/drivers/usb/musb-new/musb_core.c
> +++ b/drivers/usb/musb-new/musb_core.c
> @@ -942,7 +942,9 @@ void musb_start(struct musb *musb)
>
>   	/* put into basic highspeed mode and start session */
>   	musb_writeb(regs, MUSB_POWER, MUSB_POWER_ISOUPDATE
> +#ifdef CONFIG_USB_GADGET_DUALSPEED
>   						| MUSB_POWER_HSENAB
> +#endif
>   						/* ENSUSPEND wedges tusb */
>   						/* | MUSB_POWER_ENSUSPEND */
>   						);
> --
> 1.7.10.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

[U-Boot] [PATCH] musb: set MUSB speed based on CONFIG

[Hans de Goede]

Hi,

On 10-07-15 16:30, Hans de Goede wrote:
> Hi,
>
> On 10-07-15 15:16, Samuel Egli wrote:
>> From: Bin Liu <b-liu@ti.com>
>>
>> Do not config MUSB to highspeed mode if CONFIG_USB_GADGET_DUALSPEED
>> is not set, in which case Ether gadget only operates in fullspeed.
>>
>> Note:
>> This patch is necessary for devices like some siemens boards
>> that allow only FULL SPEED USB 1.1, e.g. DFU download.
>>
>> Signed-off-by: Bin Liu <b-liu@ti.com>
>> Reviewed-by: Tom Rini <trini@konsulko.com>
>> Tested-by: Samuel Egli <samuel.egli@siemens.com>
>> CC: Marek Vasut <marex@denx.de>
>> CC: Heiko Schocher <hs@denx.de>
>> CC: Daniel Mack <zonque@gmail.com>
>> CC: Roger Meier <r.meier@siemens.com>
>
> Nack this breaks highspeed mode on boards which use the musb
> in host mode, and thus do not set CONFIG_USB_GADGET_DUALSPEED.

p.s.

Given that you want to use this as a hack to work around the broken
pcb design of your board I suggest adding a new option for this
titled: CONFIG_USB_MUSB_NO_HIGHSPEED and then do:

+#ifndef CONFIG_USB_MUSB_NO_HIGHSPEED
                            | MUSB_POWER_HSENAB
+#endif

Using CONFIG_USB_GADGET_DUALSPEED for this seems wrong, since this
has nothing to do with enabling dualspeed mode for the gadget code
really.

Regards,

Hans


>
> Regards,
>
> Hans
>
>
>> ---
>>   drivers/usb/musb-new/musb_core.c |    2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/usb/musb-new/musb_core.c b/drivers/usb/musb-new/musb_core.c
>> index 242cc30..4edd6d7 100644
>> --- a/drivers/usb/musb-new/musb_core.c
>> +++ b/drivers/usb/musb-new/musb_core.c
>> @@ -942,7 +942,9 @@ void musb_start(struct musb *musb)
>>
>>       /* put into basic highspeed mode and start session */
>>       musb_writeb(regs, MUSB_POWER, MUSB_POWER_ISOUPDATE
>> +#ifdef CONFIG_USB_GADGET_DUALSPEED
>>                           | MUSB_POWER_HSENAB
>> +#endif
>>                           /* ENSUSPEND wedges tusb */
>>                           /* | MUSB_POWER_ENSUSPEND */
>>                           );
>> --
>> 1.7.10.4
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] musb: set MUSB speed based on CONFIG

[Egli, Samuel]

Hi Hans,

>-----Original Message-----
>From: Hans de Goede [mailto:hdegoede at redhat.com]
>Sent: Freitag, 10. Juli 2015 16:37
>To: Egli, Samuel; marex at denx.de
>Cc: u-boot at lists.denx.de; trini at konsulko.com; Bin Liu; Meier, Roger;
>Daniel Mack
>Subject: Re: [U-Boot] [PATCH] musb: set MUSB speed based on CONFIG
>
>Hi,
>
>On 10-07-15 16:30, Hans de Goede wrote:
>> Hi,
>>
>> On 10-07-15 15:16, Samuel Egli wrote:
>>> From: Bin Liu <b-liu@ti.com>
>>>
>>> Do not config MUSB to highspeed mode if CONFIG_USB_GADGET_DUALSPEED
>>> is not set, in which case Ether gadget only operates in fullspeed.
>>>
>>> Note:
>>> This patch is necessary for devices like some siemens boards that
>>> allow only FULL SPEED USB 1.1, e.g. DFU download.
>>>
>>> Signed-off-by: Bin Liu <b-liu@ti.com>
>>> Reviewed-by: Tom Rini <trini@konsulko.com>
>>> Tested-by: Samuel Egli <samuel.egli@siemens.com>
>>> CC: Marek Vasut <marex@denx.de>
>>> CC: Heiko Schocher <hs@denx.de>
>>> CC: Daniel Mack <zonque@gmail.com>
>>> CC: Roger Meier <r.meier@siemens.com>
>>
>> Nack this breaks highspeed mode on boards which use the musb in host
>> mode, and thus do not set CONFIG_USB_GADGET_DUALSPEED.
>
>p.s.
>
>Given that you want to use this as a hack to work around the broken pcb
>design of your board I suggest adding a new option for this

Well, lets not discuss the "broken" pcb design. It seems that wiring protection
is not that common. Unfortunately, such a protection is too expensive for 
USB High speed :-(.

>titled: CONFIG_USB_MUSB_NO_HIGHSPEED and then do:
>
>+#ifndef CONFIG_USB_MUSB_NO_HIGHSPEED
>                            | MUSB_POWER_HSENAB
>+#endif
>
This would be good enough. The point is indeed to limit it to full speed.

>Using CONFIG_USB_GADGET_DUALSPEED for this seems wrong, since this has
>nothing to do with enabling dualspeed mode for the gadget code really.

I agree that the name is confusing. 

Kind regards

Sam

[U-Boot] [PATCH] musb: set MUSB speed based on CONFIG

[Heiko Schocher]

Hello Samuel,

Am 10.07.2015 um 16:50 schrieb Egli, Samuel:
> Hi Hans,
>
>> -----Original Message-----
>> From: Hans de Goede [mailto:hdegoede at redhat.com]
>> Sent: Freitag, 10. Juli 2015 16:37
>> To: Egli, Samuel; marex at denx.de
>> Cc: u-boot at lists.denx.de; trini at konsulko.com; Bin Liu; Meier, Roger;
>> Daniel Mack
>> Subject: Re: [U-Boot] [PATCH] musb: set MUSB speed based on CONFIG
>>
>> Hi,
>>
>> On 10-07-15 16:30, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 10-07-15 15:16, Samuel Egli wrote:
>>>> From: Bin Liu <b-liu@ti.com>
>>>>
>>>> Do not config MUSB to highspeed mode if CONFIG_USB_GADGET_DUALSPEED
>>>> is not set, in which case Ether gadget only operates in fullspeed.
>>>>
>>>> Note:
>>>> This patch is necessary for devices like some siemens boards that
>>>> allow only FULL SPEED USB 1.1, e.g. DFU download.
>>>>
>>>> Signed-off-by: Bin Liu <b-liu@ti.com>
>>>> Reviewed-by: Tom Rini <trini@konsulko.com>
>>>> Tested-by: Samuel Egli <samuel.egli@siemens.com>
>>>> CC: Marek Vasut <marex@denx.de>
>>>> CC: Heiko Schocher <hs@denx.de>
>>>> CC: Daniel Mack <zonque@gmail.com>
>>>> CC: Roger Meier <r.meier@siemens.com>
>>>
>>> Nack this breaks highspeed mode on boards which use the musb in host
>>> mode, and thus do not set CONFIG_USB_GADGET_DUALSPEED.
>>
>> p.s.
>>
>> Given that you want to use this as a hack to work around the broken pcb
>> design of your board I suggest adding a new option for this
>
> Well, lets not discuss the "broken" pcb design. It seems that wiring protection
> is not that common. Unfortunately, such a protection is too expensive for
> USB High speed :-(.
>
>> titled: CONFIG_USB_MUSB_NO_HIGHSPEED and then do:
>>
>> +#ifndef CONFIG_USB_MUSB_NO_HIGHSPEED
>>                             | MUSB_POWER_HSENAB
>> +#endif
>>
> This would be good enough. The point is indeed to limit it to full speed.
>
>> Using CONFIG_USB_GADGET_DUALSPEED for this seems wrong, since this has
>> nothing to do with enabling dualspeed mode for the gadget code really.
>
> I agree that the name is confusing.

Yes, I vote for Hans suggestion.

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] musb: set MUSB speed based on CONFIG

[Bin Liu]

Hi,

On 07/10/2015 10:12 AM, Heiko Schocher wrote:
> Hello Samuel,
>
> Am 10.07.2015 um 16:50 schrieb Egli, Samuel:
>> Hi Hans,
>>
>>> -----Original Message----- From: Hans de Goede
>>> [mailto:hdegoede at redhat.com] Sent: Freitag, 10. Juli 2015 16:37
>>> To: Egli, Samuel; marex at denx.de Cc: u-boot at lists.denx.de;
>>> trini at konsulko.com; Bin Liu; Meier, Roger; Daniel Mack Subject:
>>> Re: [U-Boot] [PATCH] musb: set MUSB speed based on CONFIG
>>>
>>> Hi,
>>>
>>> On 10-07-15 16:30, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 10-07-15 15:16, Samuel Egli wrote:
>>>>> From: Bin Liu <b-liu@ti.com>
>>>>>
>>>>> Do not config MUSB to highspeed mode if
>>>>> CONFIG_USB_GADGET_DUALSPEED is not set, in which case Ether
>>>>> gadget only operates in fullspeed.
>>>>>
>>>>> Note: This patch is necessary for devices like some siemens
>>>>> boards that allow only FULL SPEED USB 1.1, e.g. DFU
>>>>> download.
>>>>>
>>>>> Signed-off-by: Bin Liu <b-liu@ti.com> Reviewed-by: Tom Rini
>>>>> <trini@konsulko.com> Tested-by: Samuel Egli
>>>>> <samuel.egli@siemens.com> CC: Marek Vasut <marex@denx.de> CC:
>>>>> Heiko Schocher <hs@denx.de> CC: Daniel Mack
>>>>> <zonque@gmail.com> CC: Roger Meier <r.meier@siemens.com>
>>>>
>>>> Nack this breaks highspeed mode on boards which use the musb in
>>>> host mode, and thus do not set CONFIG_USB_GADGET_DUALSPEED.

My bad, I had a short thought about this when I was initially working on
this patch, but did not really think about it throughly. Thanks for
bring it up.

>>>
>>> p.s.
>>>
>>> Given that you want to use this as a hack to work around the
>>> broken pcb design of your board I suggest adding a new option for
>>> this
>>
>> Well, lets not discuss the "broken" pcb design. It seems that
>> wiring protection is not that common. Unfortunately, such a
>> protection is too expensive for USB High speed :-(.

Agreed, we have seen many cases that have nothing to do with hardware
design, but MUSB has to work in full-speed mode.

>>
>>> titled: CONFIG_USB_MUSB_NO_HIGHSPEED and then do:
>>>
>>> +#ifndef CONFIG_USB_MUSB_NO_HIGHSPEED | MUSB_POWER_HSENAB
>>> +#endif
>>>
>> This would be good enough. The point is indeed to limit it to full
>> speed.
>>
>>> Using CONFIG_USB_GADGET_DUALSPEED for this seems wrong, since
>>> this has nothing to do with enabling dualspeed mode for the
>>> gadget code really.
>>
>> I agree that the name is confusing.
>
> Yes, I vote for Hans suggestion.

*Adding* a new macro CONFIG_USB_MUSB_NO_HIGHSPEED for musb driver causes
CONFIG_USB_GADGET_DUALSPEED redundant, because both control for full-speed.

I suggest to *rename* CONFIG_USB_GADGET_DUALSPEED to
CONFIG_USB_FULLSPEED_ONLY. So that we can use one macro for both gadget
and musb drivers. Considering supper-speed exists and future, I think
CONFIG_USB_NO_HIGHSPEED is confusing...

Regards,
-Bin.

>
> bye, Heiko

[U-Boot] [PATCH] musb: set MUSB speed based on CONFIG

[Hans de Goede]

Hi,

On 10-07-15 17:31, Bin Liu wrote:
> Hi,
>
> On 07/10/2015 10:12 AM, Heiko Schocher wrote:
>> Hello Samuel,
>>
>> Am 10.07.2015 um 16:50 schrieb Egli, Samuel:
>>> Hi Hans,
>>>
>>>> -----Original Message----- From: Hans de Goede
>>>> [mailto:hdegoede at redhat.com] Sent: Freitag, 10. Juli 2015 16:37
>>>> To: Egli, Samuel; marex at denx.de Cc: u-boot at lists.denx.de;
>>>> trini at konsulko.com; Bin Liu; Meier, Roger; Daniel Mack Subject:
>>>> Re: [U-Boot] [PATCH] musb: set MUSB speed based on CONFIG
>>>>
>>>> Hi,
>>>>
>>>> On 10-07-15 16:30, Hans de Goede wrote:
>>>>> Hi,
>>>>>
>>>>> On 10-07-15 15:16, Samuel Egli wrote:
>>>>>> From: Bin Liu <b-liu@ti.com>
>>>>>>
>>>>>> Do not config MUSB to highspeed mode if
>>>>>> CONFIG_USB_GADGET_DUALSPEED is not set, in which case Ether
>>>>>> gadget only operates in fullspeed.
>>>>>>
>>>>>> Note: This patch is necessary for devices like some siemens
>>>>>> boards that allow only FULL SPEED USB 1.1, e.g. DFU
>>>>>> download.
>>>>>>
>>>>>> Signed-off-by: Bin Liu <b-liu@ti.com> Reviewed-by: Tom Rini
>>>>>> <trini@konsulko.com> Tested-by: Samuel Egli
>>>>>> <samuel.egli@siemens.com> CC: Marek Vasut <marex@denx.de> CC:
>>>>>> Heiko Schocher <hs@denx.de> CC: Daniel Mack
>>>>>> <zonque@gmail.com> CC: Roger Meier <r.meier@siemens.com>
>>>>>
>>>>> Nack this breaks highspeed mode on boards which use the musb in
>>>>> host mode, and thus do not set CONFIG_USB_GADGET_DUALSPEED.
>
> My bad, I had a short thought about this when I was initially working on
> this patch, but did not really think about it throughly. Thanks for
> bring it up.
>
>>>>
>>>> p.s.
>>>>
>>>> Given that you want to use this as a hack to work around the
>>>> broken pcb design of your board I suggest adding a new option for
>>>> this
>>>
>>> Well, lets not discuss the "broken" pcb design. It seems that
>>> wiring protection is not that common. Unfortunately, such a
>>> protection is too expensive for USB High speed :-(.
>
> Agreed, we have seen many cases that have nothing to do with hardware
> design, but MUSB has to work in full-speed mode.
>
>>>
>>>> titled: CONFIG_USB_MUSB_NO_HIGHSPEED and then do:
>>>>
>>>> +#ifndef CONFIG_USB_MUSB_NO_HIGHSPEED | MUSB_POWER_HSENAB
>>>> +#endif
>>>>
>>> This would be good enough. The point is indeed to limit it to full
>>> speed.
>>>
>>>> Using CONFIG_USB_GADGET_DUALSPEED for this seems wrong, since
>>>> this has nothing to do with enabling dualspeed mode for the
>>>> gadget code really.
>>>
>>> I agree that the name is confusing.
>>
>> Yes, I vote for Hans suggestion.
>
> *Adding* a new macro CONFIG_USB_MUSB_NO_HIGHSPEED for musb driver causes
> CONFIG_USB_GADGET_DUALSPEED redundant, because both control for full-speed.
>
> I suggest to *rename* CONFIG_USB_GADGET_DUALSPEED to
> CONFIG_USB_FULLSPEED_ONLY. So that we can use one macro for both gadget
> and musb drivers. Considering supper-speed exists and future, I think
> CONFIG_USB_NO_HIGHSPEED is confusing...

This works for me, as long as the submitter of this change ensures that
things do not regress for existing musb users, such as all sunxi boards,
which would have regressed with the original patch.

Regards,

Hans

[U-Boot] [PATCH] musb: set MUSB speed based on CONFIG

[Hans de Goede]

Hi,

On 10-07-15 17:31, Bin Liu wrote:
> Hi,
>
> On 07/10/2015 10:12 AM, Heiko Schocher wrote:
>> Hello Samuel,
>>
>> Am 10.07.2015 um 16:50 schrieb Egli, Samuel:
>>> Hi Hans,
>>>
>>>> -----Original Message----- From: Hans de Goede
>>>> [mailto:hdegoede at redhat.com] Sent: Freitag, 10. Juli 2015 16:37
>>>> To: Egli, Samuel; marex at denx.de Cc: u-boot at lists.denx.de;
>>>> trini at konsulko.com; Bin Liu; Meier, Roger; Daniel Mack Subject:
>>>> Re: [U-Boot] [PATCH] musb: set MUSB speed based on CONFIG
>>>>
>>>> Hi,
>>>>
>>>> On 10-07-15 16:30, Hans de Goede wrote:
>>>>> Hi,
>>>>>
>>>>> On 10-07-15 15:16, Samuel Egli wrote:
>>>>>> From: Bin Liu <b-liu@ti.com>
>>>>>>
>>>>>> Do not config MUSB to highspeed mode if
>>>>>> CONFIG_USB_GADGET_DUALSPEED is not set, in which case Ether
>>>>>> gadget only operates in fullspeed.
>>>>>>
>>>>>> Note: This patch is necessary for devices like some siemens
>>>>>> boards that allow only FULL SPEED USB 1.1, e.g. DFU
>>>>>> download.
>>>>>>
>>>>>> Signed-off-by: Bin Liu <b-liu@ti.com> Reviewed-by: Tom Rini
>>>>>> <trini@konsulko.com> Tested-by: Samuel Egli
>>>>>> <samuel.egli@siemens.com> CC: Marek Vasut <marex@denx.de> CC:
>>>>>> Heiko Schocher <hs@denx.de> CC: Daniel Mack
>>>>>> <zonque@gmail.com> CC: Roger Meier <r.meier@siemens.com>
>>>>>
>>>>> Nack this breaks highspeed mode on boards which use the musb in
>>>>> host mode, and thus do not set CONFIG_USB_GADGET_DUALSPEED.
>
> My bad, I had a short thought about this when I was initially working on
> this patch, but did not really think about it throughly. Thanks for
> bring it up.
>
>>>>
>>>> p.s.
>>>>
>>>> Given that you want to use this as a hack to work around the
>>>> broken pcb design of your board I suggest adding a new option for
>>>> this
>>>
>>> Well, lets not discuss the "broken" pcb design. It seems that
>>> wiring protection is not that common. Unfortunately, such a
>>> protection is too expensive for USB High speed :-(.
>
> Agreed, we have seen many cases that have nothing to do with hardware
> design, but MUSB has to work in full-speed mode.
>
>>>
>>>> titled: CONFIG_USB_MUSB_NO_HIGHSPEED and then do:
>>>>
>>>> +#ifndef CONFIG_USB_MUSB_NO_HIGHSPEED | MUSB_POWER_HSENAB
>>>> +#endif
>>>>
>>> This would be good enough. The point is indeed to limit it to full
>>> speed.
>>>
>>>> Using CONFIG_USB_GADGET_DUALSPEED for this seems wrong, since
>>>> this has nothing to do with enabling dualspeed mode for the
>>>> gadget code really.
>>>
>>> I agree that the name is confusing.
>>
>> Yes, I vote for Hans suggestion.
>
> *Adding* a new macro CONFIG_USB_MUSB_NO_HIGHSPEED for musb driver causes
> CONFIG_USB_GADGET_DUALSPEED redundant, because both control for full-speed.
>
> I suggest to *rename* CONFIG_USB_GADGET_DUALSPEED to
> CONFIG_USB_FULLSPEED_ONLY. So that we can use one macro for both gadget
> and musb drivers. Considering supper-speed exists and future, I think
> CONFIG_USB_NO_HIGHSPEED is confusing...

Sorry I was too fast. "CONFIG_USB_FULLSPEED_ONLY" is IMHO not acceptable
as it is not granular enough. sunxi boards have both EHCI/OHCI usb controller
pairs for host ports and an musb controller. One may be limited to fullspeed
while the other is not.

Likewise I can see a case where some ports but not all ports are fullspeed
only, so we still want to build the gadged code with dual-speed descriptors.

I really believe that something like:

CONFIG_MUSB_FULLSPEED_ONLY

Is what we need here, as that sets things at the granularity which we may
need in some cases.

Regards,

Hans

[U-Boot] [PATCH] musb: set MUSB speed based on CONFIG

[Bin Liu]

Hi,

On 07/11/2015 08:04 AM, Hans de Goede wrote:
> Hi,
>
> On 10-07-15 17:31, Bin Liu wrote:
>> Hi,
>>
>> On 07/10/2015 10:12 AM, Heiko Schocher wrote:
>>> Hello Samuel,
>>>
>>> Am 10.07.2015 um 16:50 schrieb Egli, Samuel:
>>>> Hi Hans,
>>>>
>>>>> -----Original Message----- From: Hans de Goede
>>>>> [mailto:hdegoede at redhat.com] Sent: Freitag, 10. Juli 2015 16:37
>>>>> To: Egli, Samuel; marex at denx.de Cc: u-boot at lists.denx.de;
>>>>> trini at konsulko.com; Bin Liu; Meier, Roger; Daniel Mack Subject:
>>>>> Re: [U-Boot] [PATCH] musb: set MUSB speed based on CONFIG
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 10-07-15 16:30, Hans de Goede wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 10-07-15 15:16, Samuel Egli wrote:
>>>>>>> From: Bin Liu <b-liu@ti.com>
>>>>>>>
>>>>>>> Do not config MUSB to highspeed mode if
>>>>>>> CONFIG_USB_GADGET_DUALSPEED is not set, in which case Ether
>>>>>>> gadget only operates in fullspeed.
>>>>>>>
>>>>>>> Note: This patch is necessary for devices like some siemens
>>>>>>> boards that allow only FULL SPEED USB 1.1, e.g. DFU
>>>>>>> download.
>>>>>>>
>>>>>>> Signed-off-by: Bin Liu <b-liu@ti.com> Reviewed-by: Tom Rini
>>>>>>> <trini@konsulko.com> Tested-by: Samuel Egli
>>>>>>> <samuel.egli@siemens.com> CC: Marek Vasut <marex@denx.de> CC:
>>>>>>> Heiko Schocher <hs@denx.de> CC: Daniel Mack
>>>>>>> <zonque@gmail.com> CC: Roger Meier <r.meier@siemens.com>
>>>>>>
>>>>>> Nack this breaks highspeed mode on boards which use the musb in
>>>>>> host mode, and thus do not set CONFIG_USB_GADGET_DUALSPEED.
>>
>> My bad, I had a short thought about this when I was initially working on
>> this patch, but did not really think about it throughly. Thanks for
>> bring it up.
>>
>>>>>
>>>>> p.s.
>>>>>
>>>>> Given that you want to use this as a hack to work around the
>>>>> broken pcb design of your board I suggest adding a new option for
>>>>> this
>>>>
>>>> Well, lets not discuss the "broken" pcb design. It seems that
>>>> wiring protection is not that common. Unfortunately, such a
>>>> protection is too expensive for USB High speed :-(.
>>
>> Agreed, we have seen many cases that have nothing to do with hardware
>> design, but MUSB has to work in full-speed mode.
>>
>>>>
>>>>> titled: CONFIG_USB_MUSB_NO_HIGHSPEED and then do:
>>>>>
>>>>> +#ifndef CONFIG_USB_MUSB_NO_HIGHSPEED | MUSB_POWER_HSENAB
>>>>> +#endif
>>>>>
>>>> This would be good enough. The point is indeed to limit it to full
>>>> speed.
>>>>
>>>>> Using CONFIG_USB_GADGET_DUALSPEED for this seems wrong, since
>>>>> this has nothing to do with enabling dualspeed mode for the
>>>>> gadget code really.
>>>>
>>>> I agree that the name is confusing.
>>>
>>> Yes, I vote for Hans suggestion.
>>
>> *Adding* a new macro CONFIG_USB_MUSB_NO_HIGHSPEED for musb driver causes
>> CONFIG_USB_GADGET_DUALSPEED redundant, because both control for
>> full-speed.
>>
>> I suggest to *rename* CONFIG_USB_GADGET_DUALSPEED to
>> CONFIG_USB_FULLSPEED_ONLY. So that we can use one macro for both gadget
>> and musb drivers. Considering supper-speed exists and future, I think
>> CONFIG_USB_NO_HIGHSPEED is confusing...
>
> Sorry I was too fast. "CONFIG_USB_FULLSPEED_ONLY" is IMHO not acceptable
> as it is not granular enough. sunxi boards have both EHCI/OHCI usb
> controller
> pairs for host ports and an musb controller. One may be limited to
> fullspeed
> while the other is not.
>
> Likewise I can see a case where some ports but not all ports are fullspeed
> only, so we still want to build the gadged code with dual-speed
> descriptors.

Good point.

>
> I really believe that something like:
>
> CONFIG_MUSB_FULLSPEED_ONLY

I am not sure like the idea of using two macros in config.h to control 
one thing. Please review the following patch which uses 
CONFIG_USB_GADGET_DUALSPEED and musb->board_mode instead. Beware that 
this patch is not tested.

diff --git a/drivers/usb/musb-new/musb_core.c 
b/drivers/usb/musb-new/musb_core.c
index eeaacdf..55d1c20 100644
--- a/drivers/usb/musb-new/musb_core.c
+++ b/drivers/usb/musb-new/musb_core.c
@@ -931,6 +931,7 @@ void musb_start(struct musb *musb)
  {
         void __iomem    *regs = musb->mregs;
         u8              devctl = musb_readb(regs, MUSB_DEVCTL);
+       u8              power;

         dev_dbg(musb->controller, "<== devctl %02x\n", devctl);

@@ -942,11 +943,12 @@ void musb_start(struct musb *musb)
         musb_writeb(regs, MUSB_TESTMODE, 0);

         /* put into basic highspeed mode and start session */
-       musb_writeb(regs, MUSB_POWER, MUSB_POWER_ISOUPDATE
-                                               | MUSB_POWER_HSENAB
-                                               /* ENSUSPEND wedges tusb */
-                                               /* | MUSB_POWER_ENSUSPEND */
-                                               );
+       power = MUSB_POWER_ISOUPDATE | MUSB_POWER_HSENAB;
+#ifndef CONFIG_USB_GADGET_DUALSPEED
+       if (musb->board_mode != MUSB_HOST)
+               power &= ~MUSB_POWER_HSENAB;
+#endif
+       musb_writeb(regs, MUSB_POWER, power);

         musb->is_active = 0;
         devctl = musb_readb(regs, MUSB_DEVCTL);
diff --git a/drivers/usb/musb-new/musb_uboot.c 
b/drivers/usb/musb-new/musb_uboot.c
index 85d3027..c240032 100644
--- a/drivers/usb/musb-new/musb_uboot.c
+++ b/drivers/usb/musb-new/musb_uboot.c
@@ -176,7 +176,7 @@ int usb_gadget_register_driver(struct 
usb_gadget_driver *driver)
  {
         int ret;

-       if (!driver || driver->speed < USB_SPEED_HIGH || !driver->bind ||
+       if (!driver || driver->speed < USB_SPEED_FULL || !driver->bind ||
             !driver->setup) {
                 printf("bad parameter.\n");
                 return -EINVAL;

Regards,
-Bin.

>
> Is what we need here, as that sets things at the granularity which we may
> need in some cases.
>
> Regards,
>
> Hans

[U-Boot] [PATCH] musb: set MUSB speed based on CONFIG

[Hans de Goede]

Hi,

On 13-07-15 16:16, Bin Liu wrote:
> Hi,
>
> On 07/11/2015 08:04 AM, Hans de Goede wrote:
>> Hi,
>>
>> On 10-07-15 17:31, Bin Liu wrote:
>>> Hi,
>>>
>>> On 07/10/2015 10:12 AM, Heiko Schocher wrote:
>>>> Hello Samuel,
>>>>
>>>> Am 10.07.2015 um 16:50 schrieb Egli, Samuel:
>>>>> Hi Hans,
>>>>>
>>>>>> -----Original Message----- From: Hans de Goede
>>>>>> [mailto:hdegoede at redhat.com] Sent: Freitag, 10. Juli 2015 16:37
>>>>>> To: Egli, Samuel; marex at denx.de Cc: u-boot at lists.denx.de;
>>>>>> trini at konsulko.com; Bin Liu; Meier, Roger; Daniel Mack Subject:
>>>>>> Re: [U-Boot] [PATCH] musb: set MUSB speed based on CONFIG
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On 10-07-15 16:30, Hans de Goede wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 10-07-15 15:16, Samuel Egli wrote:
>>>>>>>> From: Bin Liu <b-liu@ti.com>
>>>>>>>>
>>>>>>>> Do not config MUSB to highspeed mode if
>>>>>>>> CONFIG_USB_GADGET_DUALSPEED is not set, in which case Ether
>>>>>>>> gadget only operates in fullspeed.
>>>>>>>>
>>>>>>>> Note: This patch is necessary for devices like some siemens
>>>>>>>> boards that allow only FULL SPEED USB 1.1, e.g. DFU
>>>>>>>> download.
>>>>>>>>
>>>>>>>> Signed-off-by: Bin Liu <b-liu@ti.com> Reviewed-by: Tom Rini
>>>>>>>> <trini@konsulko.com> Tested-by: Samuel Egli
>>>>>>>> <samuel.egli@siemens.com> CC: Marek Vasut <marex@denx.de> CC:
>>>>>>>> Heiko Schocher <hs@denx.de> CC: Daniel Mack
>>>>>>>> <zonque@gmail.com> CC: Roger Meier <r.meier@siemens.com>
>>>>>>>
>>>>>>> Nack this breaks highspeed mode on boards which use the musb in
>>>>>>> host mode, and thus do not set CONFIG_USB_GADGET_DUALSPEED.
>>>
>>> My bad, I had a short thought about this when I was initially working on
>>> this patch, but did not really think about it throughly. Thanks for
>>> bring it up.
>>>
>>>>>>
>>>>>> p.s.
>>>>>>
>>>>>> Given that you want to use this as a hack to work around the
>>>>>> broken pcb design of your board I suggest adding a new option for
>>>>>> this
>>>>>
>>>>> Well, lets not discuss the "broken" pcb design. It seems that
>>>>> wiring protection is not that common. Unfortunately, such a
>>>>> protection is too expensive for USB High speed :-(.
>>>
>>> Agreed, we have seen many cases that have nothing to do with hardware
>>> design, but MUSB has to work in full-speed mode.
>>>
>>>>>
>>>>>> titled: CONFIG_USB_MUSB_NO_HIGHSPEED and then do:
>>>>>>
>>>>>> +#ifndef CONFIG_USB_MUSB_NO_HIGHSPEED | MUSB_POWER_HSENAB
>>>>>> +#endif
>>>>>>
>>>>> This would be good enough. The point is indeed to limit it to full
>>>>> speed.
>>>>>
>>>>>> Using CONFIG_USB_GADGET_DUALSPEED for this seems wrong, since
>>>>>> this has nothing to do with enabling dualspeed mode for the
>>>>>> gadget code really.
>>>>>
>>>>> I agree that the name is confusing.
>>>>
>>>> Yes, I vote for Hans suggestion.
>>>
>>> *Adding* a new macro CONFIG_USB_MUSB_NO_HIGHSPEED for musb driver causes
>>> CONFIG_USB_GADGET_DUALSPEED redundant, because both control for
>>> full-speed.
>>>
>>> I suggest to *rename* CONFIG_USB_GADGET_DUALSPEED to
>>> CONFIG_USB_FULLSPEED_ONLY. So that we can use one macro for both gadget
>>> and musb drivers. Considering supper-speed exists and future, I think
>>> CONFIG_USB_NO_HIGHSPEED is confusing...
>>
>> Sorry I was too fast. "CONFIG_USB_FULLSPEED_ONLY" is IMHO not acceptable
>> as it is not granular enough. sunxi boards have both EHCI/OHCI usb
>> controller
>> pairs for host ports and an musb controller. One may be limited to
>> fullspeed
>> while the other is not.
>>
>> Likewise I can see a case where some ports but not all ports are fullspeed
>> only, so we still want to build the gadged code with dual-speed
>> descriptors.
>
> Good point.
>
>>
>> I really believe that something like:
>>
>> CONFIG_MUSB_FULLSPEED_ONLY
>
> I am not sure like the idea of using two macros in config.h to control one thing. Please review the following patch which uses CONFIG_USB_GADGET_DUALSPEED and musb->board_mode instead. Beware that this patch is not tested.

The 2 macros do not control one thing, it is for example perfectly possible
to have multiple otg controllers, one which can only do fullspeed and one
which can do both high/full speed in this case one will want to build
the gadget code in dualspeed mode even though one of the usb controllers
is fullspeed only.

Regards,

Hans






>
> diff --git a/drivers/usb/musb-new/musb_core.c b/drivers/usb/musb-new/musb_core.c
> index eeaacdf..55d1c20 100644
> --- a/drivers/usb/musb-new/musb_core.c
> +++ b/drivers/usb/musb-new/musb_core.c
> @@ -931,6 +931,7 @@ void musb_start(struct musb *musb)
>   {
>          void __iomem    *regs = musb->mregs;
>          u8              devctl = musb_readb(regs, MUSB_DEVCTL);
> +       u8              power;
>
>          dev_dbg(musb->controller, "<== devctl %02x\n", devctl);
>
> @@ -942,11 +943,12 @@ void musb_start(struct musb *musb)
>          musb_writeb(regs, MUSB_TESTMODE, 0);
>
>          /* put into basic highspeed mode and start session */
> -       musb_writeb(regs, MUSB_POWER, MUSB_POWER_ISOUPDATE
> -                                               | MUSB_POWER_HSENAB
> -                                               /* ENSUSPEND wedges tusb */
> -                                               /* | MUSB_POWER_ENSUSPEND */
> -                                               );
> +       power = MUSB_POWER_ISOUPDATE | MUSB_POWER_HSENAB;
> +#ifndef CONFIG_USB_GADGET_DUALSPEED
> +       if (musb->board_mode != MUSB_HOST)
> +               power &= ~MUSB_POWER_HSENAB;
> +#endif
> +       musb_writeb(regs, MUSB_POWER, power);
>
>          musb->is_active = 0;
>          devctl = musb_readb(regs, MUSB_DEVCTL);
> diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c
> index 85d3027..c240032 100644
> --- a/drivers/usb/musb-new/musb_uboot.c
> +++ b/drivers/usb/musb-new/musb_uboot.c
> @@ -176,7 +176,7 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver)
>   {
>          int ret;
>
> -       if (!driver || driver->speed < USB_SPEED_HIGH || !driver->bind ||
> +       if (!driver || driver->speed < USB_SPEED_FULL || !driver->bind ||
>              !driver->setup) {
>                  printf("bad parameter.\n");
>                  return -EINVAL;
>
> Regards,
> -Bin.
>
>>
>> Is what we need here, as that sets things at the granularity which we may
>> need in some cases.
>>
>> Regards,
>>
>> Hans

[U-Boot] [PATCH] musb: set MUSB speed based on CONFIG

[Bin Liu]

Hi,

On 07/19/2015 06:01 AM, Hans de Goede wrote:
> Hi,
>
> On 13-07-15 16:16, Bin Liu wrote:
>> Hi,
>>
>> On 07/11/2015 08:04 AM, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 10-07-15 17:31, Bin Liu wrote:
>>>> Hi,
>>>>
>>>> On 07/10/2015 10:12 AM, Heiko Schocher wrote:
>>>>> Hello Samuel,
>>>>>
>>>>> Am 10.07.2015 um 16:50 schrieb Egli, Samuel:
>>>>>> Hi Hans,
>>>>>>
>>>>>>> -----Original Message----- From: Hans de Goede
>>>>>>> [mailto:hdegoede at redhat.com] Sent: Freitag, 10. Juli 2015 16:37
>>>>>>> To: Egli, Samuel; marex at denx.de Cc: u-boot at lists.denx.de;
>>>>>>> trini at konsulko.com; Bin Liu; Meier, Roger; Daniel Mack Subject:
>>>>>>> Re: [U-Boot] [PATCH] musb: set MUSB speed based on CONFIG
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 10-07-15 16:30, Hans de Goede wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On 10-07-15 15:16, Samuel Egli wrote:
>>>>>>>>> From: Bin Liu <b-liu@ti.com>
>>>>>>>>>
>>>>>>>>> Do not config MUSB to highspeed mode if
>>>>>>>>> CONFIG_USB_GADGET_DUALSPEED is not set, in which case Ether
>>>>>>>>> gadget only operates in fullspeed.
>>>>>>>>>
>>>>>>>>> Note: This patch is necessary for devices like some siemens
>>>>>>>>> boards that allow only FULL SPEED USB 1.1, e.g. DFU
>>>>>>>>> download.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Bin Liu <b-liu@ti.com> Reviewed-by: Tom Rini
>>>>>>>>> <trini@konsulko.com> Tested-by: Samuel Egli
>>>>>>>>> <samuel.egli@siemens.com> CC: Marek Vasut <marex@denx.de> CC:
>>>>>>>>> Heiko Schocher <hs@denx.de> CC: Daniel Mack
>>>>>>>>> <zonque@gmail.com> CC: Roger Meier <r.meier@siemens.com>
>>>>>>>>
>>>>>>>> Nack this breaks highspeed mode on boards which use the musb in
>>>>>>>> host mode, and thus do not set CONFIG_USB_GADGET_DUALSPEED.
>>>>
>>>> My bad, I had a short thought about this when I was initially
>>>> working on
>>>> this patch, but did not really think about it throughly. Thanks for
>>>> bring it up.
>>>>
>>>>>>>
>>>>>>> p.s.
>>>>>>>
>>>>>>> Given that you want to use this as a hack to work around the
>>>>>>> broken pcb design of your board I suggest adding a new option for
>>>>>>> this
>>>>>>
>>>>>> Well, lets not discuss the "broken" pcb design. It seems that
>>>>>> wiring protection is not that common. Unfortunately, such a
>>>>>> protection is too expensive for USB High speed :-(.
>>>>
>>>> Agreed, we have seen many cases that have nothing to do with hardware
>>>> design, but MUSB has to work in full-speed mode.
>>>>
>>>>>>
>>>>>>> titled: CONFIG_USB_MUSB_NO_HIGHSPEED and then do:
>>>>>>>
>>>>>>> +#ifndef CONFIG_USB_MUSB_NO_HIGHSPEED | MUSB_POWER_HSENAB
>>>>>>> +#endif
>>>>>>>
>>>>>> This would be good enough. The point is indeed to limit it to full
>>>>>> speed.
>>>>>>
>>>>>>> Using CONFIG_USB_GADGET_DUALSPEED for this seems wrong, since
>>>>>>> this has nothing to do with enabling dualspeed mode for the
>>>>>>> gadget code really.
>>>>>>
>>>>>> I agree that the name is confusing.
>>>>>
>>>>> Yes, I vote for Hans suggestion.
>>>>
>>>> *Adding* a new macro CONFIG_USB_MUSB_NO_HIGHSPEED for musb driver
>>>> causes
>>>> CONFIG_USB_GADGET_DUALSPEED redundant, because both control for
>>>> full-speed.
>>>>
>>>> I suggest to *rename* CONFIG_USB_GADGET_DUALSPEED to
>>>> CONFIG_USB_FULLSPEED_ONLY. So that we can use one macro for both gadget
>>>> and musb drivers. Considering supper-speed exists and future, I think
>>>> CONFIG_USB_NO_HIGHSPEED is confusing...
>>>
>>> Sorry I was too fast. "CONFIG_USB_FULLSPEED_ONLY" is IMHO not acceptable
>>> as it is not granular enough. sunxi boards have both EHCI/OHCI usb
>>> controller
>>> pairs for host ports and an musb controller. One may be limited to
>>> fullspeed
>>> while the other is not.
>>>
>>> Likewise I can see a case where some ports but not all ports are
>>> fullspeed
>>> only, so we still want to build the gadged code with dual-speed
>>> descriptors.
>>
>> Good point.
>>
>>>
>>> I really believe that something like:
>>>
>>> CONFIG_MUSB_FULLSPEED_ONLY
>>
>> I am not sure like the idea of using two macros in config.h to control
>> one thing. Please review the following patch which uses
>> CONFIG_USB_GADGET_DUALSPEED and musb->board_mode instead. Beware that
>> this patch is not tested.
>
> The 2 macros do not control one thing, it is for example perfectly possible
> to have multiple otg controllers, one which can only do fullspeed and one
> which can do both high/full speed in this case one will want to build
> the gadget code in dualspeed mode even though one of the usb controllers
> is fullspeed only.

This case is a different problem, which requires a separate patch to 
solve, maybe CONFIG_MUSB_NO_HIGHSPEED can play here.

My original and 2nd patch were trying to solve the issue that MUSB 
controller was not set consistently with the gadget driver. Unless the 
patch breaks something else (as did the original patch), nothing else 
needs to be added in.

Regards,
-Bin.

>
> Regards,
>
> Hans
>
>
>
>
>
>
>>
>> diff --git a/drivers/usb/musb-new/musb_core.c
>> b/drivers/usb/musb-new/musb_core.c
>> index eeaacdf..55d1c20 100644
>> --- a/drivers/usb/musb-new/musb_core.c
>> +++ b/drivers/usb/musb-new/musb_core.c
>> @@ -931,6 +931,7 @@ void musb_start(struct musb *musb)
>>   {
>>          void __iomem    *regs = musb->mregs;
>>          u8              devctl = musb_readb(regs, MUSB_DEVCTL);
>> +       u8              power;
>>
>>          dev_dbg(musb->controller, "<== devctl %02x\n", devctl);
>>
>> @@ -942,11 +943,12 @@ void musb_start(struct musb *musb)
>>          musb_writeb(regs, MUSB_TESTMODE, 0);
>>
>>          /* put into basic highspeed mode and start session */
>> -       musb_writeb(regs, MUSB_POWER, MUSB_POWER_ISOUPDATE
>> -                                               | MUSB_POWER_HSENAB
>> -                                               /* ENSUSPEND wedges
>> tusb */
>> -                                               /* |
>> MUSB_POWER_ENSUSPEND */
>> -                                               );
>> +       power = MUSB_POWER_ISOUPDATE | MUSB_POWER_HSENAB;
>> +#ifndef CONFIG_USB_GADGET_DUALSPEED
>> +       if (musb->board_mode != MUSB_HOST)
>> +               power &= ~MUSB_POWER_HSENAB;
>> +#endif
>> +       musb_writeb(regs, MUSB_POWER, power);
>>
>>          musb->is_active = 0;
>>          devctl = musb_readb(regs, MUSB_DEVCTL);
>> diff --git a/drivers/usb/musb-new/musb_uboot.c
>> b/drivers/usb/musb-new/musb_uboot.c
>> index 85d3027..c240032 100644
>> --- a/drivers/usb/musb-new/musb_uboot.c
>> +++ b/drivers/usb/musb-new/musb_uboot.c
>> @@ -176,7 +176,7 @@ int usb_gadget_register_driver(struct
>> usb_gadget_driver *driver)
>>   {
>>          int ret;
>>
>> -       if (!driver || driver->speed < USB_SPEED_HIGH || !driver->bind ||
>> +       if (!driver || driver->speed < USB_SPEED_FULL || !driver->bind ||
>>              !driver->setup) {
>>                  printf("bad parameter.\n");
>>                  return -EINVAL;
>>
>> Regards,
>> -Bin.
>>
>>>
>>> Is what we need here, as that sets things at the granularity which we
>>> may
>>> need in some cases.
>>>
>>> Regards,
>>>
>>> Hans

[U-Boot] [PATCH] musb: set MUSB speed based on CONFIG

[Heiko Schocher]

Hello Bin Liu,

Am 10.07.2015 um 17:31 schrieb Bin Liu:
> Hi,
>
> On 07/10/2015 10:12 AM, Heiko Schocher wrote:
>> Hello Samuel,
>>
>> Am 10.07.2015 um 16:50 schrieb Egli, Samuel:
>>> Hi Hans,
>>>
>>>> -----Original Message----- From: Hans de Goede
>>>> [mailto:hdegoede at redhat.com] Sent: Freitag, 10. Juli 2015 16:37
>>>> To: Egli, Samuel; marex at denx.de Cc: u-boot at lists.denx.de;
>>>> trini at konsulko.com; Bin Liu; Meier, Roger; Daniel Mack Subject:
>>>> Re: [U-Boot] [PATCH] musb: set MUSB speed based on CONFIG
>>>>
>>>> Hi,
>>>>
>>>> On 10-07-15 16:30, Hans de Goede wrote:
>>>>> Hi,
>>>>>
>>>>> On 10-07-15 15:16, Samuel Egli wrote:
>>>>>> From: Bin Liu <b-liu@ti.com>
>>>>>>
>>>>>> Do not config MUSB to highspeed mode if
>>>>>> CONFIG_USB_GADGET_DUALSPEED is not set, in which case Ether
>>>>>> gadget only operates in fullspeed.
>>>>>>
>>>>>> Note: This patch is necessary for devices like some siemens
>>>>>> boards that allow only FULL SPEED USB 1.1, e.g. DFU
>>>>>> download.
>>>>>>
>>>>>> Signed-off-by: Bin Liu <b-liu@ti.com> Reviewed-by: Tom Rini
>>>>>> <trini@konsulko.com> Tested-by: Samuel Egli
>>>>>> <samuel.egli@siemens.com> CC: Marek Vasut <marex@denx.de> CC:
>>>>>> Heiko Schocher <hs@denx.de> CC: Daniel Mack
>>>>>> <zonque@gmail.com> CC: Roger Meier <r.meier@siemens.com>
>>>>>
>>>>> Nack this breaks highspeed mode on boards which use the musb in
>>>>> host mode, and thus do not set CONFIG_USB_GADGET_DUALSPEED.
>
> My bad, I had a short thought about this when I was initially working on
> this patch, but did not really think about it throughly. Thanks for
> bring it up.
>
>>>>
>>>> p.s.
>>>>
>>>> Given that you want to use this as a hack to work around the
>>>> broken pcb design of your board I suggest adding a new option for
>>>> this
>>>
>>> Well, lets not discuss the "broken" pcb design. It seems that
>>> wiring protection is not that common. Unfortunately, such a
>>> protection is too expensive for USB High speed :-(.
>
> Agreed, we have seen many cases that have nothing to do with hardware
> design, but MUSB has to work in full-speed mode.
>
>>>
>>>> titled: CONFIG_USB_MUSB_NO_HIGHSPEED and then do:
>>>>
>>>> +#ifndef CONFIG_USB_MUSB_NO_HIGHSPEED | MUSB_POWER_HSENAB
>>>> +#endif
>>>>
>>> This would be good enough. The point is indeed to limit it to full
>>> speed.
>>>
>>>> Using CONFIG_USB_GADGET_DUALSPEED for this seems wrong, since
>>>> this has nothing to do with enabling dualspeed mode for the
>>>> gadget code really.
>>>
>>> I agree that the name is confusing.
>>
>> Yes, I vote for Hans suggestion.
>
> *Adding* a new macro CONFIG_USB_MUSB_NO_HIGHSPEED for musb driver causes
> CONFIG_USB_GADGET_DUALSPEED redundant, because both control for full-speed.
>
> I suggest to *rename* CONFIG_USB_GADGET_DUALSPEED to
> CONFIG_USB_FULLSPEED_ONLY. So that we can use one macro for both gadget
> and musb drivers. Considering supper-speed exists and future, I think
> CONFIG_USB_NO_HIGHSPEED is confusing...

If this is so, yes, of course, CONFIG_USB_GADGET_DUALSPEED
should be renamed only. A entry in README would be helpful here.

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] musb: am335x: disable bulk split-combine feature

[Bin Liu]

On TI AM335x devices, MUSB has bulk split/combine feature enabled
in the ConfigData register, but the current MUSB driver does not
support it yet. Therefore, disable the feature for now, until the
driver adds the support.

One usecase which is broken because of this feature is that Ether
gadget stops working in Fullspeed mode (by un-defining
CONFIG_USB_GADGET_DUALSPEED)

After desabled this feature, MUSB driver send packets in proper size
(no more than 64 bytes) in Fullspeed mode.

This has been validated with Ether gadget in Fullspeed mode on AM335x
EVM.

Signed-off-by: Bin Liu <b-liu@ti.com>
---
 drivers/usb/musb-new/musb_core.c |    5 +++++
 include/configs/am335x_evm.h     |    1 +
 2 files changed, 6 insertions(+)

diff --git a/drivers/usb/musb-new/musb_core.c b/drivers/usb/musb-new/musb_core.c
index 040a5c0..aa647e6 100644
--- a/drivers/usb/musb-new/musb_core.c
+++ b/drivers/usb/musb-new/musb_core.c
@@ -1421,6 +1421,7 @@ static int __devinit musb_core_init(u16 musb_type, struct musb *musb)
 		strcat(aInfo, ", dyn FIFOs");
 		musb->dyn_fifo = true;
 	}
+#ifndef CONFIG_MUSB_DISABLE_BULK_COMBINE_SPLIT
 	if (reg & MUSB_CONFIGDATA_MPRXE) {
 		strcat(aInfo, ", bulk combine");
 		musb->bulk_combine = true;
@@ -1429,6 +1430,10 @@ static int __devinit musb_core_init(u16 musb_type, struct musb *musb)
 		strcat(aInfo, ", bulk split");
 		musb->bulk_split = true;
 	}
+#else
+	musb->bulk_combine = false;
+	musb->bulk_split = false;
+#endif
 	if (reg & MUSB_CONFIGDATA_HBRXE) {
 		strcat(aInfo, ", HB-ISO Rx");
 		musb->hb_iso_rx = true;
diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
index 74a70ee..faf9581 100644
--- a/include/configs/am335x_evm.h
+++ b/include/configs/am335x_evm.h
@@ -450,6 +450,7 @@
 #define CONFIG_ARCH_MISC_INIT
 #define CONFIG_MUSB_GADGET
 #define CONFIG_MUSB_PIO_ONLY
+#define CONFIG_MUSB_DISABLE_BULK_COMBINE_SPLIT
 #define CONFIG_USB_GADGET_DUALSPEED
 #define CONFIG_USB_GADGET_VBUS_DRAW	2
 #define CONFIG_MUSB_HOST
-- 
1.7.9.5

[U-Boot] [PATCH] musb: set MUSB speed based on CONFIG

[Bin Liu]

Do not config MUSB to highspeed mode if CONFIG_USB_GADGET_DUALSPEED
is not set, in which case Ether gadget only operates in fullspeed.

Reviewed-by: Tom Rini <trini@ti.com>
Signed-off-by: Bin Liu <b-liu@ti.com>
---
 drivers/usb/musb-new/musb_core.c  |    2 ++
 drivers/usb/musb-new/musb_uboot.c |    2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb-new/musb_core.c b/drivers/usb/musb-new/musb_core.c
index aa647e6..da93571 100644
--- a/drivers/usb/musb-new/musb_core.c
+++ b/drivers/usb/musb-new/musb_core.c
@@ -943,7 +943,9 @@ void musb_start(struct musb *musb)
 
 	/* put into basic highspeed mode and start session */
 	musb_writeb(regs, MUSB_POWER, MUSB_POWER_ISOUPDATE
+#ifdef CONFIG_USB_GADGET_DUALSPEED
 						| MUSB_POWER_HSENAB
+#endif
 						/* ENSUSPEND wedges tusb */
 						/* | MUSB_POWER_ENSUSPEND */
 						);
diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c
index 096c4f4..ee3e6c5 100644
--- a/drivers/usb/musb-new/musb_uboot.c
+++ b/drivers/usb/musb-new/musb_uboot.c
@@ -174,7 +174,7 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver)
 {
 	int ret;
 
-	if (!driver || driver->speed < USB_SPEED_HIGH || !driver->bind ||
+	if (!driver || driver->speed < USB_SPEED_FULL || !driver->bind ||
 	    !driver->setup) {
 		printf("bad parameter.\n");
 		return -EINVAL;
-- 
1.7.9.5