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

[Krzysztof Kozlowski]

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.

> 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

> 
> 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.


> 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.

> +      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?

> +    $ref: /schemas/types.yaml#/definitions/uint16

Drop, use proper name suffix.

Best regards,
Krzysztof