Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754120AbcL0KwY (ORCPT ); Tue, 27 Dec 2016 05:52:24 -0500 Received: from mail-io0-f193.google.com ([209.85.223.193]:33219 "EHLO mail-io0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751667AbcL0KwQ (ORCPT ); Tue, 27 Dec 2016 05:52:16 -0500 MIME-Version: 1.0 In-Reply-To: <0d79eb1f34e409749a136173b68f365b545c4789.1482738764.git.baolin.wang@linaro.org> References: <0d79eb1f34e409749a136173b68f365b545c4789.1482738764.git.baolin.wang@linaro.org> From: Janusz Dziedzic Date: Tue, 27 Dec 2016 11:52:15 +0100 Message-ID: Subject: Re: [PATCH] usb: dwc3: gadget: Avoid race between dwc3 interrupt handler and irq thread handler To: Baolin Wang Cc: balbi@kernel.org, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, linaro-kernel@lists.linaro.org, broonie@kernel.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2273 Lines: 66 2016-12-26 9:01 GMT+01:00 Baolin Wang : > On some platfroms(like x86 platform), when one core is running the USB gadget > irq thread handler by dwc3_thread_interrupt(), meanwhile another core also can > respond other interrupts from dwc3 controller and modify the event buffer by > dwc3_interrupt() function, that will cause getting the wrong event count in > irq thread handler to make the USB function abnormal. > > We should add spin_lock/unlock() in dwc3_check_event_buf() to avoid this race. > Interesting, I always think we mask interrupt in dwc3_interrupt() by setting DWC3_GEVNTSIZ_INTMASK And unmask interrupt when we end dwc3_thread_interrupt(). So, we shouldn't get any IRQ from HW during dwc3_thread_interrupt(), or I miss something? Do you have some traces that indicate this masking will not work correctly? BTW, what value you get when problem occured, 0xFFFC? BR Janusz > Signed-off-by: Baolin Wang > --- > drivers/usb/dwc3/gadget.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 6785595..1a1e1f4 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -2894,10 +2894,13 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt) > return IRQ_HANDLED; > } > > + spin_lock(&dwc->lock); > count = dwc3_readl(dwc->regs, DWC3_GEVNTCOUNT(0)); > count &= DWC3_GEVNTCOUNT_MASK; > - if (!count) > + if (!count) { > + spin_unlock(&dwc->lock); > return IRQ_NONE; > + } > > evt->count = count; > evt->flags |= DWC3_EVENT_PENDING; > @@ -2914,6 +2917,7 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt) > memcpy(evt->cache, evt->buf, count - amount); > > dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), count); > + spin_unlock(&dwc->lock); > > return IRQ_WAKE_THREAD; > } > -- > 1.7.9.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Janusz Dziedzic