On 11/8/2016 6:09 AM, Felipe Balbi wrote: > > Hi, > > John Youn <John.Youn@xxxxxxxxxxxx> writes: >> 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)? > > good point. That's why I wasn't clearing events until I actually > processed them :-) > >>>>> + 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? > > do you really need to clear at least one event for this? > Unfortunately yes. That's how the workaround works. We need to write this during IMOD to de-assert the interrupt since the mask bit doesn't work. We could do this only for 3.00a as well. Regards, John -- 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
