| Message ID | 1380634271-27588-4-git-send-email-zonque@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
* Daniel Mack | 2013-10-01 15:31:11 [+0200]: >In cppi41_tear_down_chan(), bail out earlier in case td_seen is unset >instead of popping another descriptor when td_desc_seen is also unset. > >My system ran into WARN() condition multiple times when >cppi41_tear_down_chan() was called for channels that had all of >td_queued, td_seen and td_seen set to false. Which one? >Signed-off-by: Daniel Mack <zonque@gmail.com> >--- > drivers/dma/cppi41.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > >diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c >index 7747bf7..6decf34 100644 >--- a/drivers/dma/cppi41.c >+++ b/drivers/dma/cppi41.c >@@ -586,6 +586,9 @@ static int cppi41_tear_down_chan(struct cppi41_channel *c) > } > c->td_seen = 1; > } >+ >+ if (c->td_retry) >+ return -EAGAIN; So you return right away since the retry counter should be > 0 here. And then you want to get the TDDOWN bit set and retry. Hmmm. Let me answer to you 0/3 on this. > } > if (!c->td_desc_seen) { > desc_phys = cppi41_pop_desc(cdd, c->q_comp_num); >@@ -606,8 +609,6 @@ static int cppi41_tear_down_chan(struct cppi41_channel *c) > * descriptor before the TD we fetch it from enqueue, it has to be > * there waiting for us. > */ >- if (!c->td_seen && c->td_retry) >- return -EAGAIN; > > WARN_ON(!c->td_retry); > if (!c->td_desc_seen) { Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sebastian, On 02.10.2013 10:29, Sebastian Andrzej Siewior wrote: > * Daniel Mack | 2013-10-01 15:31:11 [+0200]: >> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c >> index 7747bf7..6decf34 100644 >> --- a/drivers/dma/cppi41.c >> +++ b/drivers/dma/cppi41.c >> @@ -586,6 +586,9 @@ static int cppi41_tear_down_chan(struct cppi41_channel *c) >> } >> c->td_seen = 1; >> } >> + >> + if (c->td_retry) >> + return -EAGAIN; > > So you return right away since the retry counter should be > 0 here. And > then you want to get the TDDOWN bit set and retry. Hmmm. > Let me answer to you 0/3 on this. Thanks a lot for having a look! As I'm going to be off for a couple of days now, and only be able to read my mails sporadically, maybe you can also try the musb suspend functions on your hardware. I'll give you a quick wrap-up of how my test setup looks like. On an AM33xx board, I have a host-only (type A) connector with a USB memory stick plugged in. The relevant config settings are: CONFIG_USB_MUSB_HDRC=m # CONFIG_USB_MUSB_HOST is not set # CONFIG_USB_MUSB_GADGET is not set CONFIG_USB_MUSB_DUAL_ROLE=y # CONFIG_USB_MUSB_TUSB6010 is not set # CONFIG_USB_MUSB_OMAP2PLUS is not set # CONFIG_USB_MUSB_AM35X is not set CONFIG_USB_MUSB_DSPS=m # CONFIG_USB_MUSB_UX500 is not set CONFIG_USB_MUSB_AM335X_CHILD=m # CONFIG_MUSB_PIO_ONLY is not set CONFIG_USB_TI_CPPI41_DMA=y CONFIG_TI_CPPI41=y Once the system is booted up and the USB media is detected, I send the system to sleep mode with "cat mem >/sys/power/state". After wakeup, I access the media by mounting and unmounting it once, then send the system back to sleep. Repeating the above cycle multiple times will sooner or later make the warning kick in without the discussed patch. Sometimes it happened on first try, sometimes it took me up to ~20 cycles to make it happen. I'd be curious whether you see the same behavior on your board as well, and whether the fix work for you, too. For reference, I just pushed my current working tree here: https://github.com/zonque/linux/tree/am33xx-3.12 Thanks, Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/02/2013 11:19 AM, Daniel Mack wrote: > Hi Sebastian, Hi Daniel, > On 02.10.2013 10:29, Sebastian Andrzej Siewior wrote: >> * Daniel Mack | 2013-10-01 15:31:11 [+0200]: > > Thanks a lot for having a look! As I'm going to be off for a couple of > days now, and only be able to read my mails sporadically, maybe you can > also try the musb suspend functions on your hardware. I'll give you a > quick wrap-up of how my test setup looks like. > > On an AM33xx board, I have a host-only (type A) connector with a USB > memory stick plugged in. The relevant config settings are: > > CONFIG_USB_MUSB_HDRC=m > # CONFIG_USB_MUSB_HOST is not set > # CONFIG_USB_MUSB_GADGET is not set > CONFIG_USB_MUSB_DUAL_ROLE=y > # CONFIG_USB_MUSB_TUSB6010 is not set > # CONFIG_USB_MUSB_OMAP2PLUS is not set > # CONFIG_USB_MUSB_AM35X is not set > CONFIG_USB_MUSB_DSPS=m > # CONFIG_USB_MUSB_UX500 is not set > CONFIG_USB_MUSB_AM335X_CHILD=m > # CONFIG_MUSB_PIO_ONLY is not set > CONFIG_USB_TI_CPPI41_DMA=y > CONFIG_TI_CPPI41=y > > Once the system is booted up and the USB media is detected, I send the > system to sleep mode with "cat mem >/sys/power/state". After wakeup, I > access the media by mounting and unmounting it once, then send the > system back to sleep. Okay. Going to sleep is probably easy, I need to figure out how to wakeup… > Repeating the above cycle multiple times will sooner or later make the > warning kick in without the discussed patch. Sometimes it happened on > first try, sometimes it took me up to ~20 cycles to make it happen. Ah. Okay. > I'd be curious whether you see the same behavior on your board as well, > and whether the fix work for you, too. > > For reference, I just pushed my current working tree here: > > https://github.com/zonque/linux/tree/am33xx-3.12 Thanks. > > > Thanks, > Daniel > Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02.10.2013 12:25, Sebastian Andrzej Siewior wrote: > On 10/02/2013 11:19 AM, Daniel Mack wrote: >> Once the system is booted up and the USB media is detected, I send the >> system to sleep mode with "cat mem >/sys/power/state". After wakeup, I >> access the media by mounting and unmounting it once, then send the >> system back to sleep. > > Okay. Going to sleep is probably easy, I need to figure out how to > wakeup… Unless you pass no_console_suspend in your cmdline, you can just wake up the system via UART0. IOW, just press enter on the serial console. Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c index 7747bf7..6decf34 100644 --- a/drivers/dma/cppi41.c +++ b/drivers/dma/cppi41.c @@ -586,6 +586,9 @@ static int cppi41_tear_down_chan(struct cppi41_channel *c) } c->td_seen = 1; } + + if (c->td_retry) + return -EAGAIN; } if (!c->td_desc_seen) { desc_phys = cppi41_pop_desc(cdd, c->q_comp_num); @@ -606,8 +609,6 @@ static int cppi41_tear_down_chan(struct cppi41_channel *c) * descriptor before the TD we fetch it from enqueue, it has to be * there waiting for us. */ - if (!c->td_seen && c->td_retry) - return -EAGAIN; WARN_ON(!c->td_retry); if (!c->td_desc_seen) {
In cppi41_tear_down_chan(), bail out earlier in case td_seen is unset instead of popping another descriptor when td_desc_seen is also unset. My system ran into WARN() condition multiple times when cppi41_tear_down_chan() was called for channels that had all of td_queued, td_seen and td_seen set to false. Signed-off-by: Daniel Mack <zonque@gmail.com> --- drivers/dma/cppi41.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)