On 11/4/2016 2:13 AM, Janusz Dziedzic wrote: > On 4 November 2016 at 07:41, Janusz Dziedzic <janusz.dziedzic@xxxxxxxxx> wrote: >> On 4 November 2016 at 02:31, John Youn <johnyoun@xxxxxxxxxxxx> wrote: >>> >>> Since we are saving the event count and handling the events in the >>> threaded interrupt handler, we can write and clear out the eventcount in >>> the hard interrupt handler itself. >>> >>> This behavior will be required for IP 3.00a cores that need to use >>> interrupt moderation as a workaround for an RTL issue were the interrupt >>> line cannot be masked between the hard/soft interrupt handler. >>> >>> Signed-off-by: John Youn <johnyoun@xxxxxxxxxxxx> >>> --- >>> drivers/usb/dwc3/gadget.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>> index a9c1d75..ac9eb39 100644 >>> --- a/drivers/usb/dwc3/gadget.c >>> +++ b/drivers/usb/dwc3/gadget.c >>> @@ -2877,8 +2877,6 @@ static irqreturn_t dwc3_process_event_buf(struct dwc3_event_buffer *evt) >>> */ >>> evt->lpos = (evt->lpos + 4) % DWC3_EVENT_BUFFERS_SIZE; >>> left -= 4; >>> - >>> - dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), 4); >>> } >>> >>> evt->count = 0; >>> @@ -2928,6 +2926,8 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt) >>> evt->count = count; >>> evt->flags |= DWC3_EVENT_PENDING; >>> > Shouldn't you cache (somewhere): > u32 events[evt->buf + evt->lpos .... evt->buf + evt->lpos + count] > > before you will giveback this to HW (to be sure HW will not overwrite > this in case lot of events)? > >>> + dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), count); >>> + > After that evt->buf[lpos, lpos + count] seems goes back to HW, so > thread should not rely on this? > > Or I miss something? Hi, Yes, you're right. That's a possibility and to be safe we can cache those. We're relying on the same mechanism that keeps the event buffer from becoming full. Since it is just as (un)likely a possibility. That's why you must size the event buffer appropriately taking into account your system's latency, etc. And if the event buffer becomes full, that indicates something really wrong happened in the controller. You shouldn't be getting 100's of events at a time. But yes, we can address the overwriting issue. Either: 1) Cache all incoming events. Requires double the event buffer space. 2) Cache just one event and write back only '4' during hard interrupt. Then in threaded interrupt read the one event from cache, and process the remaining events from buffer as before. Doesn't require a large cache, but a bit messier. Any other thoughts or ideas? Regards, John > >>> /* Mask interrupt */ >>> reg = dwc3_readl(dwc->regs, DWC3_GEVNTSIZ(0)); >>> reg |= DWC3_GEVNTSIZ_INTMASK; >>> -- >> >> Hello, >> >> Are we sure this will work fine with 2.60a? >> >> Some time ago I have similar code (introduce event_pop) and move >> dwc3_write(dwc->regs, DWC3_GEVNTCOUNT(0), 4) >> before >> dwc3_process_event_entry() >> >> And have some issues ... >> Didn't work correctly in my case. >> >> BR >> Janusz > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
