Re: [RFC PATCH 13/14] usb: devicetree: dwc3: Add property to disable mult TRB fetch

[Rob Herring <robh@kernel.org>]

On Thu, Dec 19, 2019 at 3:52 PM Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
>
> Hi,
>
> Rob Herring wrote:
> > On Fri, Dec 13, 2019 at 09:04:54AM +0200, Felipe Balbi wrote:
> >> Hi,
> >>
> >> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
> >>>> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
> >>>>> DWC_usb32 has a feature where it can issue multiple TRB fetch requests.
> >>>>> Add a new property to limit and only do only single TRB fetch request.
> >>>>>
> >>>>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> >>>>> ---
> >>>>>    Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++
> >>>>>    1 file changed, 2 insertions(+)
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
> >>>>> index ff35fa6de2eb..29d6f9b1fc70 100644
> >>>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> >>>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> >>>>> @@ -108,6 +108,8 @@ Optional properties:
> >>>>>     - snps,num-trb-prefetch: max value to do TRBs cache for DWC_usb32. The value
> >>>>>                           can be from 1 to DWC_USB32_CACHE_TRBS_PER_TRANSFER.
> >>>>>                           Default value is DWC_USB32_CACHE_TRBS_PER_TRANSFER.
> >>>>> + - snps,dis-mult-trb-fetch: set to issue only single TRB fetch request in
> >>>>> +                 DWC_usb32.
> >>>> two questions:
> >>>>
> >>>> - how is this different from passing 1 to the previous DT binding
> >>> The previous DT binding is related to the number TRBs to cache while
> >>> this one is related to whether the controller will send multiple
> >>> (internal) fetch commands to fetch the TRBs.
> >>>
> >>>> - do we know of anybody having issues with multi-trb prefetch?
> >>> No, we added this for various internal tests.
> >> We really a better way for you guys to have your test coverage enabled
> >> with upstream kernel. I wonder if DT guys would accept a set of bindings
> >> marked as "for testing purposes". In any case, we really need to enable
> >> Silicon Validation with upstream kernel.
> > Well, anything would be better than the endless stream of new
> > properties. Include 'test-mode' in the property names would be fine I
> > guess.
> >
>
> Generally the properties are for some quirks or configurations that the
> controller must use to work properly and not for debugging purposes.
> Unfortunately, this list can be long..

quirks or testing? Now I'm confused, which is it?

I'm sure I've said this before (for DWC3), but it is better to have a
compatible string for each implementation (usually an SoC) so you can
address new quirks without a dtb change and continually adding new
properties.

Rob