Re: [PATCH 1/5] usb: xhci: cleanup xhci_hcd allocation

[Roger Quadros]

Hi Mathias,

On 13/04/15 15:48, Mathias Nyman wrote:
> Hi
>
> On 09.04.2015 12:22, Roger Quadros wrote:
> > Hi,
> >
> > On 07/04/15 17:23, Mathias Nyman wrote:
> > > Hi
> > >
> > > On 02.04.2015 15:23, Roger Quadros wrote:
> > > > HCD core allocates memory for HCD private data in
> > > > usb_create_[shared_]hcd() so make use of that
> > > > mechanism to allocate the struct xhci_hcd.
> > > >
> > > > Introduce struct xhci_driver_overrides to provide
> > > > the size of HCD private data and hc_driver operation
> > > > overrides. As of now we only need to override the
> > > > reset and start methods.
> > > >
> > > > Signed-off-by: Roger Quadros <rogerq@xxxxxx>
> > >
> > > I'm not sure I fully understand the what's going on, or what the
> > > intention of this patch is.
> >
> > The main intention is to have both primary and shared HCDs allocated
> > before calling usb_add_hcd() for the primary hcd.
> > This is so that at the first usb_add_hcd() the OTG core knows the HCD topology
> > (i.e. whether it uses a shared HCD or not).
> >
> >  From the OTG perspective we have to prevent the actual usb_add_hcd() till the
> > OTG state machine says so.
> > This means that xhci_gen_setup() won't be necessarily called immediately and
> > so we need to allocate for xhci somewhere else.
>
> Ok, thanks for explaining. I now understand the reason behind this.
>
> > > So currently xhci driver manages the allocation and freeing of
> > > the xhci_hcd structure. We store a pointer to the xhci_hcd structure in
> > > the content of both the primary and shared usb_hcds structures hcd_priv
> > > field.
> > >
> > > With this patch xhci would be part of the usb_hcd structure,
> > > starting at hcd_priv[0]. (Like EHCI I think) It allocates enough space to include
> > > the xhci_hcd in both the primary and shared usb_hcd, but always only use the one
> > > in the primary hcd.
> >
> > precisely.
> >
> > > I'm not sure what to do with the space allocated for the shared hcd's
> > > hcd_priv field.
> >
> > we just ignore the space allocated for the shared hcd.
>
> Ok, not a big loss.
>
> > > This also means that xhci goes away together with the primary hcd. It's possible
> > > this has some impact as the xhci driver expects xhci to always exists.
> >
> > Can you please point out where this impact is.
> >
> > I've been testing add/remove HCD extensively and didn't observe any issues after applying
> > these 5 patches. Well there is one issue that comes up but it has nothing to do with xhci
> > not being allocated. It has more to do with command being queued after the HCD has gone away
> > and so getting stuck forever without timing out.
>
> I went through the codepaths and you're right, should work fine. My concern wasn't valid.
> This patchset doesn't even touch the order how primary and shared HCDs are created and added
> in the PCI case, only for the platform device case.
>
> I'll try it out and send forward once rc1 is out.

did you get a chance to try this series?

cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/