Re: [PATCH v1] usb: musb: dsps: set MUSB_DA8XX quirk for AM335x

From: Bin Liu <b-liu@ti.com>
To: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Michael Grzeschik <m.grzeschik@pengutronix.de>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	<linux-usb@vger.kernel.org>, <russell@personaltelco.net>,
	<fercerpav@gmail.com>
Subject: Re: [PATCH v1] usb: musb: dsps: set MUSB_DA8XX quirk for AM335x
Date: Wed, 20 May 2020 09:55:05 -0500	[thread overview]
Message-ID: <20200520145505.GC15845@iaqt7> (raw)
In-Reply-To: <20200520044934.hyngdg774ibqai46@pengutronix.de>

On Wed, May 20, 2020 at 06:49:34AM +0200, Oleksij Rempel wrote:
> On Tue, May 19, 2020 at 05:18:51PM -0500, Bin Liu wrote:
> > Hi,
> > 
> > On Fri, Mar 27, 2020 at 06:38:49AM +0100, Oleksij Rempel wrote:
> > > Beagle Bone Black has different memory corruptions if kernel is
> > > configured with USB_TI_CPPI41_DMA=y. This issue is reproducible with
> > > ath9k-htc driver (ar9271 based wifi usb controller):
> > > 
> > > root@AccessBox:~ iw dev wlan0 set monitor  fcsfail otherbss
> > > root@AccessBox:~ ip l s dev wlan0 up
> > > kmemleak: Cannot insert 0xda577e40 into the object search tree (overlaps existing)
> > > CPU: 0 PID: 176 Comm: ip Not tainted 5.5.0 #7
> > > Hardware name: Generic AM33XX (Flattened Device Tree)
> > > [<c0112c14>] (unwind_backtrace) from [<c010dc98>] (show_stack+0x18/0x1c)
> > > [<c010dc98>] (show_stack) from [<c08c7c2c>] (dump_stack+0x84/0x98)
> > > [<c08c7c2c>] (dump_stack) from [<c02c75a8>] (create_object+0x2f8/0x324)
> > > [<c02c75a8>] (create_object) from [<c02b8928>] (kmem_cache_alloc+0x1a8/0x39c)
> > > [<c02b8928>] (kmem_cache_alloc) from [<c072fb68>] (__alloc_skb+0x60/0x174)
> > > [<c072fb68>] (__alloc_skb) from [<bf0c5c58>] (ath9k_wmi_cmd+0x50/0x184 [ath9k_htc])
> > > [<bf0c5c58>] (ath9k_wmi_cmd [ath9k_htc]) from [<bf0cb410>] (ath9k_regwrite_multi+0x54/0x84 [ath9k_htc])
> > > [<bf0cb410>] (ath9k_regwrite_multi [ath9k_htc]) from [<bf0cb7fc>] (ath9k_regwrite+0xf0/0xfc [ath9k_htc])
> > > [<bf0cb7fc>] (ath9k_regwrite [ath9k_htc]) from [<bf1aca78>] (ar5008_hw_process_ini+0x280/0x6c0 [ath9k_hw])
> > > [<bf1aca78>] (ar5008_hw_process_ini [ath9k_hw]) from [<bf1a66ac>] (ath9k_hw_reset+0x270/0x1458 [ath9k_hw])
> > > [<bf1a66ac>] (ath9k_hw_reset [ath9k_hw]) from [<bf0c9588>] (ath9k_htc_start+0xb0/0x22c [ath9k_htc])
> > > [<bf0c9588>] (ath9k_htc_start [ath9k_htc]) from [<bf0eb3c0>] (drv_start+0x4c/0x1e8 [mac80211])
> > > [<bf0eb3c0>] (drv_start [mac80211]) from [<bf104a84>] (ieee80211_do_open+0x480/0x954 [mac80211])
> > > [<bf104a84>] (ieee80211_do_open [mac80211]) from [<c075127c>] (__dev_open+0xdc/0x160)
> > > [<c075127c>] (__dev_open) from [<c07516a8>] (__dev_change_flags+0x1a4/0x204)
> > > [<c07516a8>] (__dev_change_flags) from [<c0751728>] (dev_change_flags+0x20/0x50)
> > > [<c0751728>] (dev_change_flags) from [<c076971c>] (do_setlink+0x2ac/0x978)
> > > 
> > > After applying this patch, the system is running in monitor mode without
> > > noticeable issues.
> > > 
> > > Suggested-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > > ---
> > >  drivers/usb/musb/musb_dsps.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> > > index 88923175f71e..c01f9e9e69f5 100644
> > > --- a/drivers/usb/musb/musb_dsps.c
> > > +++ b/drivers/usb/musb/musb_dsps.c
> > > @@ -690,7 +690,7 @@ static void dsps_dma_controller_resume(struct dsps_glue *glue) {}
> > >  #endif /* CONFIG_USB_TI_CPPI41_DMA */
> > >  
> > >  static struct musb_platform_ops dsps_ops = {
> > > -	.quirks		= MUSB_DMA_CPPI41 | MUSB_INDEXED_EP,
> > > +	.quirks		= MUSB_DMA_CPPI41 | MUSB_INDEXED_EP | MUSB_DA8XX,
> > 
> > The MUSB_DA8XX flag cannot be simply applied to MUSB_DSPS, at least the
> > teardown and autoreq register offsets are different as show in
> > cppi41_dma_controller_create().
> 
> ok
> 
> > Do you understand what exactly caused the issue?
> 
> No.
> 
> Disabling DMA support "solve" this issue as well.
> 
> Beside, with DMA support, there remains one more crash with different symptoms.
> I can workaround it by disabling CPU Freq governor, or setting it to performance.
> 
> > The kernel trace above doesn't provide enuough information.
> 
> Do you have any suggestions how to instrument the kernel to get needed
> information? Or, should I try to capture USB traffic before the crash? 

First can you please try the following patch instead?

diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index 7fbb8a307145..26c996f8b2bd 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -614,7 +614,6 @@ static int cppi41_dma_channel_abort(struct dma_channel *channel)
        }

        /* DA8xx Advisory 2.3.27: wait 250 ms before to start the teardown */
-       if (musb->ops->quirks & MUSB_DA8XX)
                mdelay(250);

        tdbit = 1 << cppi41_channel->port_num;

> 
> If it helps, ath9k_htc is a usb wifi adapter. It generates a lot of
> USB traffic on multiple endpoints. Bulk with data packets and Interrupt
> with register accesses, LED blinking... etc.

Do you have a link to the picture or description of the adapter? I'd like
to see if I can buy the same to take a look.

-Bin.
