Re: [PATCH 3/6] Documentation: devicetree: dwc3: Add interrupt moderation

[Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>]

On Fri, Oct 28, 2016 at 01:30:07PM +0300, Felipe Balbi wrote:
> Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> writes:
> > On Thu, Oct 27, 2016 at 02:08:25PM -0700, John Youn wrote:
> >> On 10/26/2016 3:57 AM, Mark Rutland wrote:
> >> > On Tue, Oct 25, 2016 at 12:42:46PM -0700, John Youn wrote:
> >> >> Add interrupt moderation interval binding for dwc3.
> >
> >> >> + - snps,imod_interval: the interrupt moderation interval.

> >> This series implements the feature and enables it as a workaround for
> >> a particular version of the controller.
> >
> > ... as a workaround for *what*? Is there a bug in that IP version, or an
> 
> you didn't receive the entire series, I guess. Here's last patch in the
> series:

No, I did not. Thanks for forwarding this.

>  This is a workaround for STAR 9000961433 which affects only version
>  3.00a of the DWC_usb3 core. This prevents the controller interrupt from
>  being masked while handling events. Enabling interrupt moderation allows
>  us to work around this issue because once the GEVNTCOUNT.count is
>  written the IRQ is immediately deasserted and won't be asserted again
>  until GEVNTCOUNT.EHB is cleared.
> 
>  Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>  ---
>   drivers/usb/dwc3/core.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
>  diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>  index 6733838..7fa0832 100644
>  --- a/drivers/usb/dwc3/core.c
>  +++ b/drivers/usb/dwc3/core.c
>  @@ -1050,6 +1050,18 @@ static void dwc3_check_params(struct dwc3 *dwc)
> 		 dwc->imod_interval = 0;
> 	 }
> 
>  +	/*
>  +	 * Workaround for STAR 9000961433 which affects only version
>  +	 * 3.00a of the DWC_usb3 core. This prevents the controller
>  +	 * interrupt from being masked while handling events. IMOD
>  +	 * allows us to work around this issue. Enable it for the
>  +	 * affected version.
>  +	 */
>  +	if (!dwc->imod_interval &&
>  +	    (dwc->revision == DWC3_REVISION_300A)) {
>  +		dwc->imod_interval = 1;
>  +	}
>  +
> 	 /* Check the maximum_speed parameter */
> 	 switch (dwc->maximum_speed) {
> 	 case USB_SPEED_LOW:
> 
> > integration issue? Does the problem vary per-board?
> >
> > Generally, if there's a problem that needs to be worked around, we
> > describe the problem in the DT (perhaps implicitly in the compatible
> > string), and then the kernel chooses the workaround.
> 
> Regardless of the silicon erratum, interrupt moderation is a *feature*
> of the IP, common to all instances since revision v3.00a (IIRC). John is
> just using interrupt moderation in the context of implementing this
> workaround. But the actual feature is valid also without the erratum.

Sure, I understand this.

> Another thing to remember is that different applications (i.e. boards)
> might want to moderate the interrupt for different periods. That's,
> again, not related to the erratum at all.

... again, the question is *why*?

If this varies per use-case, then it would be better to handle this
dynamically -- people can run wildly different use-cases on the same
hardware.

I'm not sure that it makes sense for this to be in the DT, though I may
have misunderstood.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html