On Fri, Jun 04, 2021 at 05:18:16PM +0300, Felipe Balbi wrote:
>
> Hi,
>
> Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> writes:
> > On Wed, May 19, 2021 at 12:49:16AM -0700, Wesley Cheng wrote:
> >> Changes in V9:
> >> - Fixed incorrect patch in series. Removed changes in DTSI, as dwc3-qcom will
> >> add the property by default from the kernel.
> >
> > This patch series has one build failure and one warning added:
> >
> > drivers/usb/dwc3/gadget.c: In function ‘dwc3_gadget_calc_tx_fifo_size’:
> > drivers/usb/dwc3/gadget.c:653:45: warning: passing argument 1 of ‘dwc3_mdwidth’ makes pointer from integer without a cast [-Wint-conversion]
> > 653 | mdwidth = dwc3_mdwidth(dwc->hwparams.hwparams0);
> > | ~~~~~~~~~~~~~^~~~~~~~~~
> > | |
> > | u32 {aka unsigned int}
> > In file included from drivers/usb/dwc3/debug.h:14,
> > from drivers/usb/dwc3/gadget.c:25:
> > drivers/usb/dwc3/core.h:1493:45: note: expected ‘struct dwc3 *’ but argument is of type ‘u32’ {aka ‘unsigned int’}
> > 1493 | static inline u32 dwc3_mdwidth(struct dwc3 *dwc)
> > | ~~~~~~~~~~~~~^~~
> >
> >
> > drivers/usb/dwc3/dwc3-qcom.c: In function ‘dwc3_qcom_of_register_core’:
> > drivers/usb/dwc3/dwc3-qcom.c:660:23: error: implicit declaration of function ‘of_add_property’; did you mean ‘of_get_property’? [-Werror=implicit-function-declaration]
> > 660 | ret = of_add_property(dwc3_np, prop);
> > | ^~~~~~~~~~~~~~~
> > | of_get_property
> >
> >
> > How did you test these?
>
> to be honest, I don't think these should go in (apart from the build
> failure) because it's likely to break instantiations of the core with
> differing FIFO sizes. Some instantiations even have some endpoints with
> dedicated functionality that requires the default FIFO size configured
> during coreConsultant instantiation. I know of at OMAP5 and some Intel
> implementations which have dedicated endpoints for processor tracing.
>
> With OMAP5, these endpoints are configured at the top of the available
> endpoints, which means that if a gadget driver gets loaded and takes
> over most of the FIFO space because of this resizing, processor tracing
> will have a hard time running. That being said, processor tracing isn't
> supported in upstream at this moment.
>
> I still think this may cause other places to break down. The promise the
> databook makes is that increasing the FIFO size over 2x wMaxPacketSize
> should bring little to no benefit, if we're not maintaining that, I
> wonder if the problem is with some of the BUSCFG registers instead,
> where we configure interconnect bursting and the like.
Good points.
Wesley, what kind of testing have you done on this on different devices?
thanks,
greg k-h
