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

[Thinh Nguyen]

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.

> 
> But hopefully that can be avoided too as the xhci driver already parses
> these registers and stores the port information, even if accessing that
> data may require a bit more work currently.
> 

Not many drivers outside of xhci should care about its registers except
for some special cases. Even for those special cases, only a small
subset of xhci registers are used. So we may not need a global header
for that.

What we may need is a global header that holds all the xhci
quirks/configs with defined interface for drivers outside of xhci can
use and pass those configs to xhci (such as from dwc3). This requires
some work.

Thanks,
Thinh