(no subject)
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
PATCH 1/1 musb: set MUSB speed based on CONFIG
From: Bin Liu <[email protected]>
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.
--- drivers/usb/musb-new/musb_core.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/usb/musb-new/musb_core.c +2 -0
@@ -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
PATCH 1/1 musb: set MUSB speed based on CONFIG
Hi,
Quoted 9 lines · Samuel Egli
From: Bin Liu <[email protected]>
Do not config MUSB to highspeed mode if
CONFIG_USB_GADGET_DUALSPEEDis 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.
Quoted 7 lines
Signed-off-by Bin Liu <[email protected]>Reviewed-by Tom Rini <[email protected]>Tested-by Samuel Egli <[email protected]>CC Marek Vasut <[email protected]>CC Heiko Schocher <[email protected]>CC Daniel Mack <[email protected]>CC Roger Meier <[email protected]>
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
Quoted 26 lines
--- drivers/usb/musb-new/musb_core.c | 2 ++ 1 file changed, 2 insertions(+)drivers/usb/musb-new/musb_core.c +2
-1@@ -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
PATCH 1/1 musb: set MUSB speed based on CONFIG
Hi,
Quoted 20 lines · Hans de Goede
Hi,
Quoted 9 lines · Samuel Egli
From: Bin Liu <[email protected]>
Do not config MUSB to highspeed mode if
CONFIG_USB_GADGET_DUALSPEEDis 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.Quoted 7 lines
Signed-off-by Bin Liu <[email protected]>Reviewed-by Tom Rini <[email protected]>Tested-by Samuel Egli <[email protected]>CC Marek Vasut <[email protected]>CC Heiko Schocher <[email protected]>CC Daniel Mack <[email protected]>CC Roger Meier <[email protected]>
Quoted 2 lines
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
Quoted 36 lines
Regards,
Hans
Quoted 21 lines
--- drivers/usb/musb-new/musb_core.c | 2 ++ 1 file changed, 2 insertions(+)drivers/usb/musb-new/musb_core.c +2
-1@@ -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
Quoted 5 lines
_______________________________________________
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
Egli
PATCH 1/1 musb: set MUSB speed based on CONFIG
Hi Hans,
Quoted 31 lines
-----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 CONFIGHi,
Quoted 20 lines · Hans de Goede
Hi,
Quoted 16 lines · Samuel Egli
From: Bin Liu <[email protected]>
Do not config MUSB to highspeed mode if
CONFIG_USB_GADGET_DUALSPEEDis 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 <[email protected]>Reviewed-by Tom Rini <[email protected]>Tested-by Samuel Egli <[email protected]>CC Marek Vasut <[email protected]>CC Heiko Schocher <[email protected]>CC Daniel Mack <[email protected]>CC Roger Meier <[email protected]>
Quoted 7 lines
Quoted 2 lines
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 :-(.
drivers/usb/musb-new/musb_core.c void musb_start(struct musb *musb) · line 947
Quoted 6 lines
titled:
CONFIG_USB_MUSB_NO_HIGHSPEEDand then do:drivers/usb/musb-new/musb_core.c +2
-0+#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.
Quoted 2 lines
Using
CONFIG_USB_GADGET_DUALSPEEDfor 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
PATCH 1/1 musb: set MUSB speed based on CONFIG
Hello Samuel,
Am 10.07.2015 um 16:50 schrieb Egli, Samuel:
Quoted 53 lines
Hi Hans,
Quoted 31 lines
-----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 CONFIGHi,
Quoted 20 lines · Hans de Goede
Hi,
Quoted 16 lines · Samuel Egli
From: Bin Liu <[email protected]>
Do not config MUSB to highspeed mode if
CONFIG_USB_GADGET_DUALSPEEDis 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 <[email protected]>Reviewed-by Tom Rini <[email protected]>Tested-by Samuel Egli <[email protected]>CC Marek Vasut <[email protected]>CC Heiko Schocher <[email protected]>CC Daniel Mack <[email protected]>CC Roger Meier <[email protected]>Quoted 7 lines
Quoted 2 lines
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 :-(.
drivers/usb/musb-new/musb_core.c void musb_start(struct musb *musb) · line 947
Quoted 6 lines
titled:
CONFIG_USB_MUSB_NO_HIGHSPEEDand then do:drivers/usb/musb-new/musb_core.c +2
-0+#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.
Quoted 4 lines
Quoted 2 lines
Using
CONFIG_USB_GADGET_DUALSPEEDfor 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
Signature · 2 lines
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH 1/1 musb: set MUSB speed based on CONFIG
Hi,
Quoted 35 lines · Heiko Schocher
Hello Samuel,
Am 10.07.2015 um 16:50 schrieb Egli, Samuel:
Quoted 32 lines
Hi Hans,
Quoted 30 lines
-----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 CONFIGHi,
Quoted 21 lines · Hans de Goede
Hi,
Quoted 10 lines · Samuel Egli
From: Bin Liu <[email protected]>
Do not config MUSB to highspeed mode if
CONFIG_USB_GADGET_DUALSPEEDis 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.
Quoted 5 lines
Signed-off-by Bin Liu <[email protected]> Reviewed-by: Tom Rini<[email protected]> Tested-by: Samuel Egli <[email protected]> CC: Marek Vasut <[email protected]> CC:
Heiko Schocher <[email protected]> CC: Daniel Mack <[email protected]> CC: Roger Meier <[email protected]>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.
Quoted 10 lines
Quoted 10 lines
Quoted 6 lines
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.
Quoted 9 lines
Quoted 9 lines
Review
Quoted 6 lines
titled:
CONFIG_USB_MUSB_NO_HIGHSPEEDand then do:(quoted diff) +2
-0+#ifndef CONFIG_USB_MUSB_NO_HIGHSPEED | MUSB_POWER_HSENAB +#endifThis would be good enough. The point is indeed to limit it to full speed.
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Using
CONFIG_USB_GADGET_DUALSPEEDfor 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.
Quoted 2 lines
bye, Heiko
PATCH 1/1 musb: set MUSB speed based on CONFIG
Hi,
Quoted 74 lines · Bin Liu
Hi,
Quoted 35 lines · Heiko Schocher
Hello Samuel,
Am 10.07.2015 um 16:50 schrieb Egli, Samuel:
Quoted 32 lines
Hi Hans,
Quoted 30 lines
-----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 CONFIGHi,
Quoted 21 lines · Hans de Goede
Hi,
Quoted 10 lines · Samuel Egli
From: Bin Liu <[email protected]>
Do not config MUSB to highspeed mode if
CONFIG_USB_GADGET_DUALSPEEDis 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.
Quoted 5 lines
Signed-off-by Bin Liu <[email protected]> Reviewed-by: Tom Rini<[email protected]> Tested-by: Samuel Egli <[email protected]> CC: Marek Vasut <[email protected]> CC:
Heiko Schocher <[email protected]> CC: Daniel Mack <[email protected]> CC: Roger Meier <[email protected]>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.
Quoted 10 lines
Quoted 10 lines
Quoted 6 lines
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.
Quoted 9 lines
Quoted 9 lines
Review
Quoted 6 lines
titled:
CONFIG_USB_MUSB_NO_HIGHSPEEDand then do:(quoted diff) +2
-0+#ifndef CONFIG_USB_MUSB_NO_HIGHSPEED | MUSB_POWER_HSENAB +#endifThis would be good enough. The point is indeed to limit it to full speed.
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Using
CONFIG_USB_GADGET_DUALSPEEDfor 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.
Quoted 7 lines
*Adding* a new macro
CONFIG_USB_MUSB_NO_HIGHSPEEDfor musb driver causesCONFIG_USB_GADGET_DUALSPEEDredundant, because both control for full-speed.I suggest to *rename*
CONFIG_USB_GADGET_DUALSPEEDtoCONFIG_USB_FULLSPEED_ONLY. So that we can use one macro for both gadget and musb drivers. Considering supper-speed exists and future, I thinkCONFIG_USB_NO_HIGHSPEEDis 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
PATCH 1/1 musb: set MUSB speed based on CONFIG
In reply to Bin LiuHi,
Quoted 74 lines · Bin Liu
Hi,
Quoted 35 lines · Heiko Schocher
Hello Samuel,
Am 10.07.2015 um 16:50 schrieb Egli, Samuel:
Quoted 32 lines
Hi Hans,
Quoted 30 lines
-----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 CONFIGHi,
Quoted 21 lines · Hans de Goede
Hi,
Quoted 10 lines · Samuel Egli
From: Bin Liu <[email protected]>
Do not config MUSB to highspeed mode if
CONFIG_USB_GADGET_DUALSPEEDis 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.
Quoted 5 lines
Signed-off-by Bin Liu <[email protected]> Reviewed-by: Tom Rini<[email protected]> Tested-by: Samuel Egli <[email protected]> CC: Marek Vasut <[email protected]> CC:
Heiko Schocher <[email protected]> CC: Daniel Mack <[email protected]> CC: Roger Meier <[email protected]>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.
Quoted 10 lines
Quoted 10 lines
Quoted 6 lines
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.
Quoted 9 lines
Quoted 9 lines
Review
Quoted 6 lines
titled:
CONFIG_USB_MUSB_NO_HIGHSPEEDand then do:(quoted diff) +2
-0+#ifndef CONFIG_USB_MUSB_NO_HIGHSPEED | MUSB_POWER_HSENAB +#endifThis would be good enough. The point is indeed to limit it to full speed.
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Using
CONFIG_USB_GADGET_DUALSPEEDfor 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.
Quoted 7 lines
*Adding* a new macro
CONFIG_USB_MUSB_NO_HIGHSPEEDfor musb driver causesCONFIG_USB_GADGET_DUALSPEEDredundant, because both control for full-speed.I suggest to *rename*
CONFIG_USB_GADGET_DUALSPEEDtoCONFIG_USB_FULLSPEED_ONLY. So that we can use one macro for both gadget and musb drivers. Considering supper-speed exists and future, I thinkCONFIG_USB_NO_HIGHSPEEDis 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
PATCH 1/1 musb: set MUSB speed based on CONFIG
Hi,
Quoted 93 lines · Hans de Goede
Hi,
Quoted 74 lines · Bin Liu
Hi,
Quoted 35 lines · Heiko Schocher
Hello Samuel,
Am 10.07.2015 um 16:50 schrieb Egli, Samuel:
Quoted 32 lines
Hi Hans,
Quoted 30 lines
-----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 CONFIGHi,
Quoted 21 lines · Hans de Goede
Hi,
Quoted 10 lines · Samuel Egli
From: Bin Liu <[email protected]>
Do not config MUSB to highspeed mode if
CONFIG_USB_GADGET_DUALSPEEDis 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.
Quoted 5 lines
Signed-off-by Bin Liu <[email protected]> Reviewed-by: Tom Rini<[email protected]> Tested-by: Samuel Egli <[email protected]> CC: Marek Vasut <[email protected]> CC:
Heiko Schocher <[email protected]> CC: Daniel Mack <[email protected]> CC: Roger Meier <[email protected]>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.
Quoted 10 lines
Quoted 10 lines
Quoted 6 lines
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.
Quoted 9 lines
Quoted 9 lines
Review
Quoted 6 lines
titled:
CONFIG_USB_MUSB_NO_HIGHSPEEDand then do:(quoted diff) +2
-0+#ifndef CONFIG_USB_MUSB_NO_HIGHSPEED | MUSB_POWER_HSENAB +#endifThis would be good enough. The point is indeed to limit it to full speed.
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Using
CONFIG_USB_GADGET_DUALSPEEDfor 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.
Quoted 8 lines
*Adding* a new macro
CONFIG_USB_MUSB_NO_HIGHSPEEDfor musb driver causesCONFIG_USB_GADGET_DUALSPEEDredundant, because both control for full-speed.I suggest to *rename*
CONFIG_USB_GADGET_DUALSPEEDtoCONFIG_USB_FULLSPEED_ONLY. So that we can use one macro for both gadget and musb drivers. Considering supper-speed exists and future, I thinkCONFIG_USB_NO_HIGHSPEEDis 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.
Quoted 3 lines
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.
Quoted 4 lines
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.
(quoted diff) +0 -0
b/drivers/usb/musb-new/musb_core.c
index eeaacdf..55d1c20 100644
drivers/usb/musb-new/musb_core.c +7 -5
@@ -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 );
(quoted diff) +0 -0
b/drivers/usb/musb-new/musb_uboot.c
index 85d3027..c240032 100644
drivers/usb/musb-new/musb_uboot.c +0 -0
@@ -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.
Quoted 7 lines
Is what we need here, as that sets things at the granularity which we may need in some cases.
Regards,
Hans
PATCH 1/1 musb: set MUSB speed based on CONFIG
Hi,
Quoted 100 lines · Bin Liu
Hi,
Quoted 93 lines · Hans de Goede
Hi,
Quoted 74 lines · Bin Liu
Hi,
Quoted 35 lines · Heiko Schocher
Hello Samuel,
Am 10.07.2015 um 16:50 schrieb Egli, Samuel:
Quoted 32 lines
Hi Hans,
Quoted 30 lines
-----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 CONFIGHi,
Quoted 21 lines · Hans de Goede
Hi,
Quoted 10 lines · Samuel Egli
From: Bin Liu <[email protected]>
Do not config MUSB to highspeed mode if
CONFIG_USB_GADGET_DUALSPEEDis 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.
Quoted 5 lines
Signed-off-by Bin Liu <[email protected]> Reviewed-by: Tom Rini<[email protected]> Tested-by: Samuel Egli <[email protected]> CC: Marek Vasut <[email protected]> CC:
Heiko Schocher <[email protected]> CC: Daniel Mack <[email protected]> CC: Roger Meier <[email protected]>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.
Quoted 10 lines
Quoted 10 lines
Quoted 6 lines
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.
Quoted 9 lines
Quoted 9 lines
Review
Quoted 6 lines
titled:
CONFIG_USB_MUSB_NO_HIGHSPEEDand then do:(quoted diff) +2
-0+#ifndef CONFIG_USB_MUSB_NO_HIGHSPEED | MUSB_POWER_HSENAB +#endifThis would be good enough. The point is indeed to limit it to full speed.
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Using
CONFIG_USB_GADGET_DUALSPEEDfor 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.
Quoted 8 lines
*Adding* a new macro
CONFIG_USB_MUSB_NO_HIGHSPEEDfor musb driver causesCONFIG_USB_GADGET_DUALSPEEDredundant, because both control for full-speed.I suggest to *rename*
CONFIG_USB_GADGET_DUALSPEEDtoCONFIG_USB_FULLSPEED_ONLY. So that we can use one macro for both gadget and musb drivers. Considering supper-speed exists and future, I thinkCONFIG_USB_NO_HIGHSPEEDis 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.Quoted 3 lines
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.
Quoted 8 lines
Good point.
Quoted 4 lines
I really believe that something like:
CONFIG_MUSB_FULLSPEED_ONLYI am not sure like the idea of using two macros in
config.hto control one thing. Please review the following patch which usesCONFIG_USB_GADGET_DUALSPEEDandmusb->board_modeinstead. 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
Quoted 55 lines
drivers/usb/musb-new/musb_core.c +7
-5@@ -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 );drivers/usb/musb-new/musb_uboot.c +1
-1@@ -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.Quoted 7 lines
Is what we need here, as that sets things at the granularity which we may need in some cases.
Regards,
Hans
PATCH 1/1 musb: set MUSB speed based on CONFIG
Hi,
Quoted 118 lines · Hans de Goede
Hi,
Quoted 106 lines · Bin Liu
Hi,
Quoted 95 lines · Hans de Goede
Hi,
Quoted 80 lines · Bin Liu
Hi,
Quoted 35 lines · Heiko Schocher
Hello Samuel,
Am 10.07.2015 um 16:50 schrieb Egli, Samuel:
Quoted 32 lines
Hi Hans,
Quoted 30 lines
-----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 CONFIGHi,
Quoted 21 lines · Hans de Goede
Hi,
Quoted 10 lines · Samuel Egli
From: Bin Liu <[email protected]>
Do not config MUSB to highspeed mode if
CONFIG_USB_GADGET_DUALSPEEDis 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.
Quoted 5 lines
Signed-off-by Bin Liu <[email protected]> Reviewed-by: Tom Rini<[email protected]> Tested-by: Samuel Egli <[email protected]> CC: Marek Vasut <[email protected]> CC:
Heiko Schocher <[email protected]> CC: Daniel Mack <[email protected]> CC: Roger Meier <[email protected]>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.Quoted 10 lines
Quoted 10 lines
Quoted 6 lines
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.
Quoted 9 lines
Quoted 9 lines
Review
Quoted 6 lines
titled:
CONFIG_USB_MUSB_NO_HIGHSPEEDand then do:(quoted diff) +2
-0+#ifndef CONFIG_USB_MUSB_NO_HIGHSPEED | MUSB_POWER_HSENAB +#endifThis would be good enough. The point is indeed to limit it to full speed.
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Using
CONFIG_USB_GADGET_DUALSPEEDfor 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_HIGHSPEEDfor musb driver causesCONFIG_USB_GADGET_DUALSPEEDredundant, because both control for full-speed.Quoted 4 lines
I suggest to *rename*
CONFIG_USB_GADGET_DUALSPEEDtoCONFIG_USB_FULLSPEED_ONLY. So that we can use one macro for both gadget and musb drivers. Considering supper-speed exists and future, I thinkCONFIG_USB_NO_HIGHSPEEDis 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.Quoted 4 lines
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.
Quoted 1 line
Quoted 8 lines
Quoted 3 lines
I really believe that something like:
CONFIG_MUSB_FULLSPEED_ONLYI am not sure like the idea of using two macros in
config.hto control one thing. Please review the following patch which usesCONFIG_USB_GADGET_DUALSPEEDandmusb->board_modeinstead. Beware that this patch is not tested.
Quoted 5 lines
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.
Quoted 71 lines
Regards,
Hans
Quoted 61 lines
drivers/usb/musb-new/musb_core.c +1
-3@@ -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*/drivers/usb/musb-new/musb_core.c +6
-1- ); + 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 );drivers/usb/musb-new/musb_uboot.c +1
-1@@ -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.Quoted 8 lines
Is what we need here, as that sets things at the granularity which we may
need in some cases.Regards,
Hans
PATCH 1/1 musb: set MUSB speed based on CONFIG
In reply to Bin LiuHello Bin Liu,
Am 10.07.2015 um 17:31 schrieb Bin Liu:
Quoted 74 lines
Hi,
Quoted 35 lines · Heiko Schocher
Hello Samuel,
Am 10.07.2015 um 16:50 schrieb Egli, Samuel:
Quoted 32 lines
Hi Hans,
Quoted 30 lines
-----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 CONFIGHi,
Quoted 21 lines · Hans de Goede
Hi,
Quoted 10 lines · Samuel Egli
From: Bin Liu <[email protected]>
Do not config MUSB to highspeed mode if
CONFIG_USB_GADGET_DUALSPEEDis 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.
Quoted 5 lines
Signed-off-by Bin Liu <[email protected]> Reviewed-by: Tom Rini<[email protected]> Tested-by: Samuel Egli <[email protected]> CC: Marek Vasut <[email protected]> CC:
Heiko Schocher <[email protected]> CC: Daniel Mack <[email protected]> CC: Roger Meier <[email protected]>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.
Quoted 10 lines
Quoted 10 lines
Quoted 6 lines
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.
Quoted 9 lines
Quoted 9 lines
Review
Quoted 6 lines
titled:
CONFIG_USB_MUSB_NO_HIGHSPEEDand then do:(quoted diff) +2
-0+#ifndef CONFIG_USB_MUSB_NO_HIGHSPEED | MUSB_POWER_HSENAB +#endifThis would be good enough. The point is indeed to limit it to full speed.
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Using
CONFIG_USB_GADGET_DUALSPEEDfor 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.
Quoted 7 lines
*Adding* a new macro
CONFIG_USB_MUSB_NO_HIGHSPEEDfor musb driver causesCONFIG_USB_GADGET_DUALSPEEDredundant, because both control for full-speed.I suggest to *rename*
CONFIG_USB_GADGET_DUALSPEEDtoCONFIG_USB_FULLSPEED_ONLY. So that we can use one macro for both gadget and musb drivers. Considering supper-speed exists and future, I thinkCONFIG_USB_NO_HIGHSPEEDis 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
Signature · 2 lines
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany