Re: [PATCH] usb: dwc3: Clear DWC3_EVENT_PENDING when count is 0

From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
To: Linyu Yuan <quic_linyyuan@quicinc.com>
Cc: "Thinh Nguyen" <Thinh.Nguyen@synopsys.com>,
	정재훈 <jh0801.jung@samsung.com>,
	"'Felipe Balbi'" <balbi@kernel.org>,
	"'Greg Kroah-Hartman'" <gregkh@linuxfoundation.org>,
	"'open list:USB XHCI DRIVER'" <linux-usb@vger.kernel.org>,
	"'open list'" <linux-kernel@vger.kernel.org>,
	"'Seungchull Suh'" <sc.suh@samsung.com>,
	"'Daehwan Jung'" <dh10.jung@samsung.com>
Subject: Re: [PATCH] usb: dwc3: Clear DWC3_EVENT_PENDING when count is 0
Date: Tue, 10 Jan 2023 02:53:25 +0000	[thread overview]
Message-ID: <20230110025310.nowjnrmo3oag76xd@synopsys.com> (raw)
In-Reply-To: <e7217471-5b08-eee6-fba8-06e879118d77@quicinc.com>

On Tue, Jan 10, 2023, Linyu Yuan wrote:
> 
> On 1/10/2023 2:28 AM, Thinh Nguyen wrote:
> > On Fri, Jan 06, 2023, Linyu Yuan wrote:
> > > On 1/5/2023 5:54 PM, 정재훈 wrote:
> > > > > -----Original Message-----
> > > > > From: Linyu Yuan [mailto:quic_linyyuan@quicinc.com]
> > > > > Sent: Thursday, January 5, 2023 12:35 PM
> > > > > To: JaeHun Jung; Felipe Balbi; Greg Kroah-Hartman; Thinh Nguyen
> > > > > Cc: open list:USB XHCI DRIVER; open list; Seungchull Suh; Daehwan Jung
> > > > > Subject: Re: [PATCH] usb: dwc3: Clear DWC3_EVENT_PENDING when count is 0
> > > > > 
> > > > > 
> > > > > On 1/5/2023 11:29 AM, Linyu Yuan wrote:
> > > > > > On 1/2/2023 1:08 PM, JaeHun Jung wrote:
> > > > > > > Sometimes very rarely, The count is 0 and the DWC3 flag is set has
> > > > > > > status.
> > > > > > > It must not have these status. Because, It can make happen interrupt
> > > > > > > storming status.
> > > > > > could you help explain without clear the flag, how interrupt storming
> > > > > > happen ?
> > > > > > 
> > > > > > as your change didn't touch any hardware register, i don't know how it
> > > > > > fix storming.
> > > > > > 
> > > > H/W interrupts are still occur on IP.
> > > I guess we should fix it from IP layer.
> > > 
> > How are you certain the problem is from IP layer?
> 
> I think all IRQ is from DWC3 controller IP. if it is not IP layer, could you
> share how to prevent from SW layer ?
> 
> seem IRQ can happen when event count is zero ,  why this can happen ? does
> it mean event count register is not trust ?

When the interrupt is unmasked, the controller will generate interrupts
as events are received. Normally, the flag checking for pending event
should be cleared before unmasking the interrupt, but we clear it after
to account for possible false interrupt due to the nature of legacy pci
interrupt. This exposes a gap where the interrupts can come but the flag
isn't cleared. While it should be rare and shouldn't be too much of a
problem, we can avoid this scenario with some additional checks.

> 
> > 
> > > but when checking DWC3_EVENT_PENDING flag, it will auto clear in dwc3 thread
> > > irq handler.
> > > 
> > > there is one possible root cause is it cleared only after irq enabled in
> > > dwc3_process_event_buf(),
> > > 
> > > we should move unmask irq operation at end of this function.
> > > 
> > This interrupt storm can happen because we clear the evt->flags _after_
> > we unmask the interrupt. This was done to prevent false interrupt from
> > delay interrupt deassertion, which can be a problem for legacy pci
> > interrupt.
> thanks for explain, i didn't know that.
> > 
> > see 7441b273388b ("usb: dwc3: gadget: Fix event pending check")
> > 
> > The change JaeHun Jung did should be fine.
> agree.

The change may still need some additional check as suggested in my
response:
https://lore.kernel.org/linux-usb/20230109190914.3blihjfjdcszazdd@synopsys.com/T/#m7b907aa6da4023cb20fa00a57813d31fd84e081f

BR,
Thinh