[U-Boot] [PATCH] musb: set MUSB speed based on CONFIG
Hans de Goede
hdegoede at redhat.com
Sun Jul 19 13:01:54 CEST 2015
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 at 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 at ti.com> Reviewed-by: Tom Rini
>>>>>>>> <trini at konsulko.com> Tested-by: Samuel Egli
>>>>>>>> <samuel.egli at siemens.com> CC: Marek Vasut <marex at denx.de> CC:
>>>>>>>> Heiko Schocher <hs at denx.de> CC: Daniel Mack
>>>>>>>> <zonque at gmail.com> CC: Roger Meier <r.meier at 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
More information about the U-Boot
mailing list