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:hdego...@redhat.com] Sent: Freitag, 10. Juli 2015 16:37
To: Egli, Samuel; ma...@denx.de Cc: u-boot@lists.denx.de;
tr...@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-...@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-...@ti.com> Reviewed-by: Tom Rini
<tr...@konsulko.com> Tested-by: Samuel Egli
<samuel.e...@siemens.com> CC: Marek Vasut <ma...@denx.de> CC:
Heiko Schocher <h...@denx.de> CC: Daniel Mack
<zon...@gmail.com> CC: Roger Meier <r.me...@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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
