linux-kernel - Re: [PATCH v1 1/2] dt-bindings: usb: snps,dwc3: Add property for imod

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAPTae5JYeuUwbwNfDTgZ1DdNDK-SDhNURuqoB547ecSUydOZQA@mail.gmail.com>
Date: Thu, 6 Feb 2025 21:15:22 -0800
From: Badhri Jagan Sridharan <badhri@...gle.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Thinh.Nguyen@...opsys.com, gregkh@...uxfoundation.org, 
	felipe.balbi@...ux.intel.com, robh@...nel.org, krzk+dt@...nel.org, 
	conor+dt@...nel.org, johnyoun@...opsys.com, linux-usb@...r.kernel.org, 
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org, jameswei@...gle.com, 
	stable@...nel.org
Subject: Re: [PATCH v1 1/2] dt-bindings: usb: snps,dwc3: Add property for imod

On Wed, Feb 5, 2025 at 11:35 AM Krzysztof Kozlowski <krzk@...nel.org> wrote:
>
> On 05/02/2025 19:19, Badhri Jagan Sridharan wrote:
> >>>>
> >>>>> +      during device mode operation. A value of 0 disables the interrupt
> >>>>> +      throttling logic and interrupts are generated immediately if event
> >>>>> +      count becomes non-zero. Otherwise, the interrupt is signaled when
> >>>>> +      all of the following conditons are met which are, EVNT_HANDLER_BUSY
> >>>>> +      is 0, event count is non-zero and at least 250ns increments of this
> >>>>> +      value has elapsed since the last time interrupt was de-asserted.
> >>>>
> >>>> Why is this property of a board? Why different boards would wait
> >>>> different amount of time?
> >>>
> >>> Interrupt moderation allows batch processing of events reported by the
> >>> controller.
> >>> A very low value of snps,device-mode-intrpt-mod-interval-ns implies
> >>> that the controller will interrupt more often to make the host
> >>> processor process a smaller set of events Vs a larger value will wake
> >>> up the host processor at longer intervals to process events (likely
> >>> more). So depending what the board is designed for this can be tuned
> >>> to achieve the needed outcome.
> >>
> >> I do not see dependency on the board. Host has the same CPU always, so
> >> it processes with the same speed.
> >
> >  By "host processor", I am referring to the CPU which is scheduling
> > the TRBs and responding to the events reported by the Synopsys DWC3
> > controller. In a nutshell, the CPU which is driving the  Synopsys DWC3
> > controller. The Synopsys DWC3 controller could be paired with any CPU
> > configuration and therefore is "Host has the same CPU always" a fair
> > assumption to be made ?
>
> Not really, this is part of a SoC, so DWC3 controller is always here
> fixed per given setup with given CPU. You claim that this is independent
> of SoC, but so far arguments do not support that statement. This is
> related to given SoC, so no need for this property and you can deduce
> everything from SoC.
>
> You push this as a property because you (or vendor) do not want to
> upstream your SoC. That's common pattern, seen here many times. BTW,
> good counter argument would be to show me patches for upstream DTS users
> of this. Actually that would be very easy way to finish discussion...
> but there are no patches, right? Why? Because it is not upstream and it
> is done for downstream solution. Sorry, no. Develop code how upstream
> develops, not downstream.

Thanks for persisting in explaining this Krzysztof ! Much appreciated
! I finally understand your perspective.
What I missed to understand is that when
https://lore.kernel.org/linux-arm-kernel/9cb2e5fc-1bec-c19c-c04e-fe56e5c1bc16@codeaurora.org/T/#m392cc1fe604499984c61ac07dacc840616194efe
added "imod-interval-ns", it also added how the property was used. I
will look into upstreaming the SOC driver which makes use of the
property as well.
We can drop this patch till then.

>>> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
>>> @@ -46,6 +46,7 @@ Optional properties:
>>>    - pinctrl-names : a pinctrl state named "default" must be defined
>>>    - pinctrl-0 : pin control group
>>>   See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
>>> + - imod-interval-ns: default interrupt moderation interval is 5000ns
>>>
>>>   Example:
>>>   usb30: usb at 11270000 {
>>> @@ -66,6 +67,7 @@ usb30: usb at 11270000 {
>>>   usb3-lpm-capable;
>>>   mediatek,syscon-wakeup = <&pericfg>;
>>>   mediatek,wakeup-src = <1>;
>>> + imod-interval-ns = <10000>;
>>>   };


Thanks,
Badhri

>
> Best regards,
> Krzysztof