[U-Boot] [PATCH] usb: musb: only write CLRDATATOG when appropriate

From: Bryan Wu bryan.wu@analog.com
This is a change similar to what is already in the Linux driver. We should only program the CLRDATATOG bit when the current mode indicates that it is needed.
Signed-off-by: Bryan Wu bryan.wu@analog.com Signed-off-by: Cliff Cai cliff.cai@analog.com Signed-off-by: Mike Frysinger vapier@gentoo.org --- Note: can someone give this a spin on a non-Blackfin platform to make sure this doesn't break things ?
drivers/usb/musb/musb_hcd.c | 23 ++++++++++++++++------- 1 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/musb/musb_hcd.c b/drivers/usb/musb/musb_hcd.c index dd2aa7f..dd66275 100644 --- a/drivers/usb/musb/musb_hcd.c +++ b/drivers/usb/musb/musb_hcd.c @@ -144,19 +144,28 @@ static void write_toggle(struct usb_device *dev, u8 ep, u8 dir_out) u16 csr;
if (dir_out) { - if (!toggle) - writew(MUSB_TXCSR_CLRDATATOG, &musbr->txcsr); - else { - csr = readw(&musbr->txcsr); + csr = readw(&musbr->txcsr); + if (!toggle) { + if (csr & MUSB_TXCSR_MODE) + csr = MUSB_TXCSR_CLRDATATOG; + else + csr = 0; + writew(csr, &musbr->txcsr); + } else { csr |= MUSB_TXCSR_H_WR_DATATOGGLE; writew(csr, &musbr->txcsr); csr |= (toggle << MUSB_TXCSR_H_DATATOGGLE_SHIFT); writew(csr, &musbr->txcsr); } } else { - if (!toggle) - writew(MUSB_RXCSR_CLRDATATOG, &musbr->rxcsr); - else { + if (!toggle) { + csr = readw(&musbr->txcsr); + if (csr & MUSB_TXCSR_MODE) + csr = MUSB_RXCSR_CLRDATATOG; + else + csr = 0; + writew(csr, &musbr->rxcsr); + } else { csr = readw(&musbr->rxcsr); csr |= MUSB_RXCSR_H_WR_DATATOGGLE; writew(csr, &musbr->rxcsr);

Hello.
Mike Frysinger wrote:
From: Bryan Wu bryan.wu@analog.com
This is a change similar to what is already in the Linux driver. We should only program the CLRDATATOG bit when the current mode indicates that it is needed.
Signed-off-by: Bryan Wu bryan.wu@analog.com Signed-off-by: Cliff Cai cliff.cai@analog.com Signed-off-by: Mike Frysinger vapier@gentoo.org
Note: can someone give this a spin on a non-Blackfin platform to make sure this doesn't break things ?
drivers/usb/musb/musb_hcd.c | 23 ++++++++++++++++------- 1 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/musb/musb_hcd.c b/drivers/usb/musb/musb_hcd.c index dd2aa7f..dd66275 100644 --- a/drivers/usb/musb/musb_hcd.c +++ b/drivers/usb/musb/musb_hcd.c @@ -144,19 +144,28 @@ static void write_toggle(struct usb_device *dev, u8 ep, u8 dir_out) u16 csr;
if (dir_out) {
if (!toggle)
writew(MUSB_TXCSR_CLRDATATOG, &musbr->txcsr);
else {
csr = readw(&musbr->txcsr);
csr = readw(&musbr->txcsr);
if (!toggle) {
if (csr & MUSB_TXCSR_MODE)
csr = MUSB_TXCSR_CLRDATATOG;
else
csr = 0;
writew(csr, &musbr->txcsr);
} } else {} else { csr |= MUSB_TXCSR_H_WR_DATATOGGLE; writew(csr, &musbr->txcsr); csr |= (toggle << MUSB_TXCSR_H_DATATOGGLE_SHIFT); writew(csr, &musbr->txcsr);
if (!toggle)
writew(MUSB_RXCSR_CLRDATATOG, &musbr->rxcsr);
else {
if (!toggle) {
csr = readw(&musbr->txcsr);
if (csr & MUSB_TXCSR_MODE)
csr = MUSB_RXCSR_CLRDATATOG;
Clearing RXCSR when FIFO is in TX mode?
else
csr = 0;
writew(csr, &musbr->rxcsr);
} else { csr = readw(&musbr->rxcsr); csr |= MUSB_RXCSR_H_WR_DATATOGGLE; writew(csr, &musbr->rxcsr);
WBR, Sergei

On Tue, Aug 10, 2010 at 6:26 AM, Sergei Shtylyov wrote:
Mike Frysinger wrote:
--- a/drivers/usb/musb/musb_hcd.c +++ b/drivers/usb/musb/musb_hcd.c } else {
- if (!toggle)
- writew(MUSB_RXCSR_CLRDATATOG, &musbr->rxcsr);
- else {
- if (!toggle) {
- csr = readw(&musbr->txcsr);
- if (csr & MUSB_TXCSR_MODE)
- csr = MUSB_RXCSR_CLRDATATOG;
Clearing RXCSR when FIFO is in TX mode?
unless i missed something, that is what Linux is doing.
musb_host.c:musb_rx_reinit() csr = musb_readw(ep->regs, MUSB_RXCSR); if (csr & MUSB_RXCSR_RXPKTRDY) WARNING("rx%d, packet/%d ready?\n", ep->epnum, musb_readw(ep->regs, MUSB_RXCOUNT));
csr = musb_readw(ep->regs, MUSB_TXCSR); if (csr & MUSB_TXCSR_MODE) musb_h_flush_rxfifo(ep, MUSB_RXCSR_CLRDATATOG); else musb_h_flush_rxfifo(ep, 0);
although i see that i need to also extend the MUSB_TXCSR_MODE define for Blackfin musb ports in u-boot. but this shouldnt affect non-Blackfin musb ports. i'll send a sep patch for that. -mike

Hi,
2010/8/10 Mike Frysinger vapier@gentoo.org:
On Tue, Aug 10, 2010 at 6:26 AM, Sergei Shtylyov wrote:
Mike Frysinger wrote:
--- a/drivers/usb/musb/musb_hcd.c +++ b/drivers/usb/musb/musb_hcd.c } else {
- if (!toggle)
- writew(MUSB_RXCSR_CLRDATATOG, &musbr->rxcsr);
- else {
- if (!toggle) {
- csr = readw(&musbr->txcsr);
- if (csr & MUSB_TXCSR_MODE)
- csr = MUSB_RXCSR_CLRDATATOG;
Clearing RXCSR when FIFO is in TX mode?
unless i missed something, that is what Linux is doing.
Can Linux be wrong too?
musb_host.c:musb_rx_reinit() csr = musb_readw(ep->regs, MUSB_RXCSR); if (csr & MUSB_RXCSR_RXPKTRDY) WARNING("rx%d, packet/%d ready?\n", ep->epnum, musb_readw(ep->regs, MUSB_RXCOUNT));
csr = musb_readw(ep->regs, MUSB_TXCSR); if (csr & MUSB_TXCSR_MODE) musb_h_flush_rxfifo(ep, MUSB_RXCSR_CLRDATATOG); else musb_h_flush_rxfifo(ep, 0);
although i see that i need to also extend the MUSB_TXCSR_MODE define for Blackfin musb ports in u-boot. but this shouldnt affect non-Blackfin musb ports. i'll send a sep patch for that. -mike
Sergei, do you agree that I apply this patch?
Kind regards,
Remy

On Thu, Aug 12, 2010 at 2:04 PM, Remy Bohmer wrote:
2010/8/10 Mike Frysinger:
On Tue, Aug 10, 2010 at 6:26 AM, Sergei Shtylyov wrote:
Mike Frysinger wrote:
--- a/drivers/usb/musb/musb_hcd.c +++ b/drivers/usb/musb/musb_hcd.c } else {
- if (!toggle)
- writew(MUSB_RXCSR_CLRDATATOG, &musbr->rxcsr);
- else {
- if (!toggle) {
- csr = readw(&musbr->txcsr);
- if (csr & MUSB_TXCSR_MODE)
- csr = MUSB_RXCSR_CLRDATATOG;
Clearing RXCSR when FIFO is in TX mode?
unless i missed something, that is what Linux is doing.
Can Linux be wrong too?
anything is possible, but i dont believe people have complained about this working incorrectly on their platforms, else it'd be fixed by now i imagine -mike

Hello.
Remy Bohmer wrote:
Mike Frysinger wrote:
--- a/drivers/usb/musb/musb_hcd.c +++ b/drivers/usb/musb/musb_hcd.c } else {
if (!toggle)
writew(MUSB_RXCSR_CLRDATATOG, &musbr->rxcsr);
else {
if (!toggle) {
csr = readw(&musbr->txcsr);
if (csr & MUSB_TXCSR_MODE)
csr = MUSB_RXCSR_CLRDATATOG;
Clearing RXCSR when FIFO is in TX mode?
I meant to say "clearing RX toggle".
unless i missed something, that is what Linux is doing.
Can Linux be wrong too?
musb_host.c:musb_rx_reinit() csr = musb_readw(ep->regs, MUSB_RXCSR); if (csr & MUSB_RXCSR_RXPKTRDY) WARNING("rx%d, packet/%d ready?\n", ep->epnum, musb_readw(ep->regs, MUSB_RXCOUNT));
csr = musb_readw(ep->regs, MUSB_TXCSR); if (csr & MUSB_TXCSR_MODE) musb_h_flush_rxfifo(ep, MUSB_RXCSR_CLRDATATOG); else musb_h_flush_rxfifo(ep, 0);
although i see that i need to also extend the MUSB_TXCSR_MODE define for Blackfin musb ports in u-boot. but this shouldnt affect non-Blackfin musb ports. i'll send a sep patch for that. -mike
Sergei, do you agree that I apply this patch?
Well, if it's based on the Linux code... although it still does look wrong... I don't know. :-)
Kind regards,
Remy
WBR, Sergei

On Friday, August 13, 2010 06:25:05 Sergei Shtylyov wrote:
Remy Bohmer wrote:
Sergei, do you agree that I apply this patch?
Well, if it's based on the Linux code... although it still does look
wrong... I don't know. :-)
do you have a board you could test on ? otherwise, it seems this patch has stalled and will stay that way ... -mike

Hi,
2010/10/11 Mike Frysinger vapier@gentoo.org:
On Friday, August 13, 2010 06:25:05 Sergei Shtylyov wrote:
Remy Bohmer wrote:
Sergei, do you agree that I apply this patch?
Well, if it's based on the Linux code... although it still does look wrong... I don't know. :-)
do you have a board you could test on ? otherwise, it seems this patch has stalled and will stay that way ...
No, Sorry, I do not have such a board... But still, I applied it to u-boot-usb.
Thanks.
Remy

Hi Mike,
2010/8/9 Mike Frysinger vapier@gentoo.org:
From: Bryan Wu bryan.wu@analog.com
This is a change similar to what is already in the Linux driver. We should only program the CLRDATATOG bit when the current mode indicates that it is needed.
Signed-off-by: Bryan Wu bryan.wu@analog.com
You use a email adress here that gets bounced due its non-existence... Can you replace it by a valid address?
Remy

On Thu, Aug 12, 2010 at 2:08 PM, Remy Bohmer wrote:
2010/8/9 Mike Frysinger:
From: Bryan Wu bryan.wu@analog.com
This is a change similar to what is already in the Linux driver. We should only program the CLRDATATOG bit when the current mode indicates that it is needed.
Signed-off-by: Bryan Wu bryan.wu@analog.com
You use a email adress here that gets bounced due its non-existence... Can you replace it by a valid address?
the patch was originally written when he worked ADI (and that e-mail was valid). he has however moved on since. -mike
participants (3)
-
Mike Frysinger
-
Remy Bohmer
-
Sergei Shtylyov