Re: [PATCH v8 3/9] usb: dwc3: core: Access XHCI address space temporarily to read port info

[Johan Hovold]

On Wed, May 17, 2023 at 11:21:50PM +0000, Thinh Nguyen wrote:
> On Wed, May 17, 2023, Johan Hovold wrote:
> > On Wed, May 17, 2023 at 03:21:24AM +0000, Thinh Nguyen wrote:
> > > On Wed, May 17, 2023, Krishna Kurapati PSSNV wrote:
> > > > On 5/16/2023 8:32 PM, Krishna Kurapati PSSNV wrote:
> > > > > On 5/16/2023 5:41 PM, Johan Hovold wrote:
> > 
> > > > > > You should not make another copy of xhci_find_next_ext_cap(), but rather
> > > > > > use it directly.
> > > > > > 
> > > > > > We already have drivers outside of usb/host using this function so it
> > > > > > should be fine to do the same for now:
> > > > > > 
> > > > > >     #include "../host/xhci-ext-caps.h"
> > 
> > > > >    This was the approach which we followed when we first introduced the
> > > > > patch [1]. But Thinh suggested to duplicate code so that we can avoid
> > > > > any dependency on xhci (which seems to be right). So since its just one
> > > > > function, I duplicated it here.
> > 
> > > >   Would like to know your opinion here on how to proceed further.
> > 
> > > Please keep them separated. The xhci-ext-caps.h is for xhci driver only.
> > > It's not meant to be exposed to other drivers. Same with other *.h files
> > > under drivers/usb/host.
> > 
> > As I mentioned earlier, it is already used by the xdbc earlyprintk
> > driver which lives outside of drivers/usb/host, even if such a debug
> > driver could be considered a special case.
> > 
> > If it turns out that there's no way to avoid mapping those registers
> > from the qcom glue driver, then I think at least the register defines
> > need to be provided in a global header rather than being copied
> > verbatim.
> 
> It would be good to properly define the global header with common
> offset/interface that can be public for other drivers.

Yes, either drivers outside of xhci should be allowed to access this
registers and then the defines should go in a public header or we need
to find another way for drivers to get their hands on the corresponding
information.

I agree that accessing the header from inside the host directory is not
very nice, but it looked we already had drivers violating this.

If this turns out to be at all needed, it should even be possible to
share the implementation even if that means adding an explicit
dependency on xhci for host mode. The current inline function really is
just a hack.

Johan