Dears, If there is/are event(s) in evt->buf, in dwc3_check_event_buf, dwc3_thread_interrupt will be woke up.
@@ -4718,6 +4723,8 @@ void dwc3_gadget_process_pending_events(struct dwc3 *dwc) { if (dwc->pending_events) { dwc3_interrupt(dwc->irq_gadget, dwc->ev_buf);
dwc3_thread_interrupt(dwc->irq_gadget, dwc->ev_buf);
pm_runtime_put(dwc->dev); dwc->pending_events = false; enable_irq(dwc->irq_gadget); }
I just want to know dwc3_thread_interrupt here is necessary or not?
On Tue, Aug 02, 2023 8:12 Thinh Nguyen wrote:
On Tue, Aug 01, 2023, Elson Roy Serrao wrote: If dwc3 is runtime suspended we defer processing the event buffer until resume, by setting the pending_events flag. Set this flag before triggering resume to avoid race with the runtime resume callback.
While handling the pending events, in addition to checking the event buffer we also need to process it. Handle this by explicitly calling dwc3_thread_interrupt(). Also balance the runtime pm get() operation that triggered this processing.
Cc: stable@vger.kernel.org Fixes: fc8bb91bc83e ("usb: dwc3: implement runtime PM") Signed-off-by: Elson Roy Serrao quic_eserrao@quicinc.com
Change separated from below series as an independent fix based on the earlier discussion https://urldefense.com/v3/__https://lore.kernel.org/all/be57511d-2005- a1f5-d5a5-809e71029aec@quicinc.com/__;!!A4F2R9G_pg!YtZ93ALEpDNTCEhf0WE yB5S090pPlKIKVtjqTOAEJdxdWbl8QG6eiRmlvfivUtD5qjKWWLX2C4fb4W8VAg5w9qasL uKB$
drivers/usb/dwc3/gadget.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 5fd067151fbf..858fe4c299b7 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -4455,9 +4455,14 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt) u32 count;
if (pm_runtime_suspended(dwc->dev)) {
dwc->pending_events = true;
/*
* Trigger runtime resume. The get() function will be balanced
* after processing the pending events in dwc3_process_pending
* events().
*/ pm_runtime_get(dwc->dev); disable_irq_nosync(dwc->irq_gadget);
dwc->pending_events = true; return IRQ_HANDLED; }
@@ -4718,6 +4723,8 @@ void dwc3_gadget_process_pending_events(struct dwc3 *dwc) { if (dwc->pending_events) { dwc3_interrupt(dwc->irq_gadget, dwc->ev_buf);
dwc3_thread_interrupt(dwc->irq_gadget, dwc->ev_buf);
pm_runtime_put(dwc->dev); dwc->pending_events = false; enable_irq(dwc->irq_gadget); }
-- 2.17.1
This fix is more complete.
Acked-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
Thanks, Thinh
Hi,
On 02/08/2023 04:39, 庞苏荣 (Surong Pang) wrote:
Dears, If there is/are event(s) in evt->buf, in dwc3_check_event_buf, dwc3_thread_interrupt will be woke up.
How will it be woken up if we had disabled the IRQ to begin with?
@@ -4718,6 +4723,8 @@ void dwc3_gadget_process_pending_events(struct dwc3 *dwc) { if (dwc->pending_events) { dwc3_interrupt(dwc->irq_gadget, dwc->ev_buf);
dwc3_thread_interrupt(dwc->irq_gadget, dwc->ev_buf);
pm_runtime_put(dwc->dev); dwc->pending_events = false; enable_irq(dwc->irq_gadget); }
I just want to know dwc3_thread_interrupt here is necessary or not?
As IRQ was disabled, we will have to call dwc3_thread_interrupt() to process the events that are still pending.
On Tue, Aug 02, 2023 8:12 Thinh Nguyen wrote:
On Tue, Aug 01, 2023, Elson Roy Serrao wrote: If dwc3 is runtime suspended we defer processing the event buffer until resume, by setting the pending_events flag. Set this flag before triggering resume to avoid race with the runtime resume callback.
While handling the pending events, in addition to checking the event buffer we also need to process it. Handle this by explicitly calling dwc3_thread_interrupt(). Also balance the runtime pm get() operation that triggered this processing.
Cc: stable@vger.kernel.org Fixes: fc8bb91bc83e ("usb: dwc3: implement runtime PM") Signed-off-by: Elson Roy Serrao quic_eserrao@quicinc.com
Change separated from below series as an independent fix based on the earlier discussion https://urldefense.com/v3/__https://lore.kernel.org/all/be57511d-2005- a1f5-d5a5-809e71029aec@quicinc.com/__;!!A4F2R9G_pg!YtZ93ALEpDNTCEhf0WE yB5S090pPlKIKVtjqTOAEJdxdWbl8QG6eiRmlvfivUtD5qjKWWLX2C4fb4W8VAg5w9qasL uKB$
drivers/usb/dwc3/gadget.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 5fd067151fbf..858fe4c299b7 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -4455,9 +4455,14 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt) u32 count;
if (pm_runtime_suspended(dwc->dev)) {
dwc->pending_events = true;
/*
* Trigger runtime resume. The get() function will be balanced
* after processing the pending events in dwc3_process_pending
* events().
*/ pm_runtime_get(dwc->dev); disable_irq_nosync(dwc->irq_gadget);
dwc->pending_events = true; return IRQ_HANDLED; }
@@ -4718,6 +4723,8 @@ void dwc3_gadget_process_pending_events(struct dwc3 *dwc) { if (dwc->pending_events) { dwc3_interrupt(dwc->irq_gadget, dwc->ev_buf);
dwc3_thread_interrupt(dwc->irq_gadget, dwc->ev_buf);
pm_runtime_put(dwc->dev); dwc->pending_events = false; enable_irq(dwc->irq_gadget); }
-- 2.17.1
This fix is more complete.
Acked-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
Thanks, Thinh
linux-stable-mirror@lists.linaro.org