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: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.
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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
