Re: [PATCH v1 2/2] usb: dwc3: core: Obtain imod_interval from device tree

[Badhri Jagan Sridharan]

On Mon, Feb 3, 2025 at 4:46 PM Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> wrote:
>
> On Sun, Feb 02, 2025, Badhri Jagan Sridharan wrote:
> > Although commit cf40b86b6ef6 ("usb: dwc3: Implement interrupt
> > moderation") adds support for interrupt moderation in device
> > mode, it does not add an option to enable interrupt moderation
> > unless the version of the controller is 3.00a where the
> > interrupt moderation is automatically enabled. This patch
> > reads the interrupt moderation interval counter value from
> > device tree so that the interrupt moderation can be enabled
> > through the device tree.
> >
> > The explicit initialization to 0 can be avoided as the dwc3
> > struct is kzalloc'ed.
> >
> > Cc: stable@xxxxxxxxxx
> > Fixes: cf40b86b6ef6 ("usb: dwc3: Implement interrupt moderation")
> > Signed-off-by: Badhri Jagan Sridharan <badhri@xxxxxxxxxx>
> > ---
> >  drivers/usb/dwc3/core.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index dfa1b5fe48dc..be0d302bbab7 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -1818,6 +1818,9 @@ static void dwc3_get_properties(struct dwc3 *dwc)
> >       dwc->dis_split_quirk = device_property_read_bool(dev,
> >                               "snps,dis-split-quirk");
> >
> > +     device_property_read_u16(dev, "snps,device-mode-intrpt-mod-interval",
> > +                              &dwc->imod_interval);
> > +
>
> This value will get overwritten in dwc3_check_params() for DWC_usb3
> v3.00a.
>
> >       dwc->lpm_nyet_threshold = lpm_nyet_threshold;
> >       dwc->tx_de_emphasis = tx_de_emphasis;
> >
> > @@ -1835,8 +1838,6 @@ static void dwc3_get_properties(struct dwc3 *dwc)
> >       dwc->tx_thr_num_pkt_prd = tx_thr_num_pkt_prd;
> >       dwc->tx_max_burst_prd = tx_max_burst_prd;
> >
> > -     dwc->imod_interval = 0;
> > -
> >       dwc->tx_fifo_resize_max_num = tx_fifo_resize_max_num;
> >  }
> >
> > --
> > 2.48.1.362.g079036d154-goog
> >
>
> Do you need a property to adjust the IMOD interval? If not, just enable
> it for all capable devices (dwc3_has_imod) with IMODI of 1 for now. It
> should be good to have it enabled for all capable devices by default.
>

Yes, IMHO it would make sense to have a property to adjust IMOD
interval for two reasons:
1. This would be then identical to the "imod-interval-ns" property
that's used for XHCI based host mode controllers.
2. Also, potentially allowing the controller to interrupt once every
250ns might not be desirable for all platforms and should be left as
platform tunable to fully realize the capability of interrupt
moderation which the controller offers again similar to
"imod-interval-ns" property in host mode.

Along with this I will also set the default value to 1 in my v2 which
I will send out shortly.

Thanks,
Badhri

> BR,
> Thinh