On 05/02/2025 10:07, Badhri Jagan Sridharan wrote: > . > > On Sun, Feb 2, 2025 at 6:11 AM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: >> >> On Sun, Feb 02, 2025 at 03:50:59AM +0000, Badhri Jagan Sridharan wrote: >>> This change adds `snps,device-mode-intrpt-mod-interval` >> >> Thank you for your patch. There is something to discuss/improve. > > Hi Krzysztof, > > Thanks for taking the time to review ! Happy to address them. > >> >>> which allows enabling interrupt moderation through >>> snps,dwc3 node. >>> >>> `snps,device-mode-intrpt-mod-interval`specifies the >>> minimum inter-interrupt interval in 250ns increments >>> 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. >> >> Please wrap commit message according to Linux coding style / submission >> process (neither too early nor over the limit): >> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597 > > Ack! will do in V2 of this patch. > >> >>> >>> Cc: stable@xxxxxxxxxx >>> Fixes: cf40b86b6ef6 ("usb: dwc3: Implement interrupt moderation") >> >> I don't understand what are you fixing here. Above commit does not >> introduce that property. > > Although the above commit does not add this property, it has > implemented the entire feature except for the property so thought > sending this with "Fixes:" while CCing stable@ will allow the > backport. I am interested in having this patch in older kernel Not implementing DT bindings is not a bug. Otherwise provide any sort of proof that this was not intentional. I can easily provide you proof why this was intentional: negative review maintainers. > versions as well where imod support has been added. Wondering what > would be the right way to achieve this. Eager to know your thoughts ! So again, downstream and forks... NAK, you cannot push things to stable just because you want them backported by Greg. This is not acceptable. > >> >> >>> Signed-off-by: Badhri Jagan Sridharan <badhri@xxxxxxxxxx> >>> --- >>> .../devicetree/bindings/usb/snps,dwc3-common.yaml | 13 +++++++++++++ >>> 1 file changed, 13 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml >>> index c956053fd036..3957f1dac3c4 100644 >>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml >>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml >>> @@ -375,6 +375,19 @@ properties: >>> items: >>> enum: [1, 4, 8, 16, 32, 64, 128, 256] >>> >>> + snps,device-mode-intrpt-mod-interval: >>> + description: >>> + Specifies the minimum inter-interrupt interval in 250ns increments >> >> Then use proper property unit suffix. > > Ack ! changing to snps,device-mode-intrpt-mod-interval-ns in V2. > >> >>> + 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. Best regards, Krzysztof
