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

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

On Tue, Nov 01, 2016 at 01:18:17PM +0200, Felipe Balbi wrote:
> 
> Hi,
> 
> John Youn <John.Youn-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> writes:
> >>>> John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> writes:
> >>>>> Add interrupt moderation interval binding for dwc3.
> >>>>>
> >>>>> Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> >>>>> ---
> >>>>>  Documentation/devicetree/bindings/usb/dwc3.txt | 1 +
> >>>>>  1 file changed, 1 insertion(+)
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
> >>>>> index e3e6983..17de9fc 100644
> >>>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> >>>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> >>>>> @@ -53,6 +53,7 @@ Optional properties:
> >>>>>   - snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of GFLADJ
> >>>>>  	register for post-silicon frame length adjustment when the
> >>>>>  	fladj_30mhz_sdbnd signal is invalid or incorrect.
> >>>>> + - snps,imod_interval: the interrupt moderation interval.
> >>>>
> >>>> on top of all other comments, what's the unit here? nanoseconds? clock cycles?
> >>>>
> >>>
> >>> Number of 250 ns intervals. I'll update the description to clarify.
> >> 
> >> it's probably better to add it in nanoseconds itself, then let driver
> >> compute register value with DIV_ROUND_UP()
> >> 
> >
> > I'm fine with it either way, but I think "increments of 250 ns" is
> > slightly cleaner in that it reflects the exact settings that are
> > possible and documented, and also fits neatly into a u16.
> 
> I don't know, I'll leave this to Mark and the other devicetree folks,
> but I remember there was a preference of not passing register values via
> devicetree. What if a following HW revision decides to change 250ns
> increments to 125ns increments?
> 
> Mark?

Generally, directly human-readable units are preferable, with some
validation/reporting left to the driver for cases the HW cannot support.
In this case, some *-interval-ns property would be better.

That said, it sounds like we're all agreed that we don't need this
property for now. :)

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