mbox series

[0/4] add xhci hooks for USB offload

Message ID 20210119101044.1637023-1-howardyen@google.com (mailing list archive)
Headers show
Series add xhci hooks for USB offload | expand

Message

Howard Yen Jan. 19, 2021, 10:10 a.m. UTC
To let the xhci driver support USB offload, add hooks for vendor to have
customized behavior for the initialization, memory allocation, irq work, and 
device context synchronization. Detail is in each patch commit message.

Howard Yen (4):
  usb: host: add xhci hooks for USB offload
  usb: host: export symbols for xhci hooks usage
  usb: xhci-plat: add xhci_plat_priv_overwrite
  dt-bindings: usb: usb-xhci: add USB offload support

 .../devicetree/bindings/usb/usb-xhci.txt      |  1 +
 drivers/usb/host/xhci-hub.c                   |  5 +
 drivers/usb/host/xhci-mem.c                   | 99 ++++++++++++++++---
 drivers/usb/host/xhci-plat.c                  | 45 ++++++++-
 drivers/usb/host/xhci-plat.h                  |  9 ++
 drivers/usb/host/xhci-ring.c                  | 19 +++-
 drivers/usb/host/xhci.c                       | 89 +++++++++++++++++
 drivers/usb/host/xhci.h                       | 38 +++++++
 8 files changed, 289 insertions(+), 16 deletions(-)

Comments

Greg KH Jan. 19, 2021, 10:25 a.m. UTC | #1
On Tue, Jan 19, 2021 at 06:10:40PM +0800, Howard Yen wrote:
> To let the xhci driver support USB offload, add hooks for vendor to have
> customized behavior for the initialization, memory allocation, irq work, and 
> device context synchronization. Detail is in each patch commit message.
> 
> Howard Yen (4):
>   usb: host: add xhci hooks for USB offload
>   usb: host: export symbols for xhci hooks usage
>   usb: xhci-plat: add xhci_plat_priv_overwrite
>   dt-bindings: usb: usb-xhci: add USB offload support
> 
>  .../devicetree/bindings/usb/usb-xhci.txt      |  1 +
>  drivers/usb/host/xhci-hub.c                   |  5 +
>  drivers/usb/host/xhci-mem.c                   | 99 ++++++++++++++++---
>  drivers/usb/host/xhci-plat.c                  | 45 ++++++++-
>  drivers/usb/host/xhci-plat.h                  |  9 ++
>  drivers/usb/host/xhci-ring.c                  | 19 +++-
>  drivers/usb/host/xhci.c                       | 89 +++++++++++++++++
>  drivers/usb/host/xhci.h                       | 38 +++++++
>  8 files changed, 289 insertions(+), 16 deletions(-)

Thanks so much for posting this.

A bit of background for the lists.  I helped review previous versions of
this patchset from Howard as he worked to convert the hacks from a
previous vendor into something that would be semi-sane.  It would be
great if we can take the previously-submitted Samsung usb-audio hooks
(as published in their kernel sources for their last-year phones) and
get it into something mergable with this scheme as well, as this is the
"correct" way to do what they were wanting to do.

Although I know that is outside of the work you probably have time for,
maybe I will work on that over the next few weeks...

thanks,

greg k-h
Mathias Nyman Jan. 19, 2021, 12:49 p.m. UTC | #2
On 19.1.2021 12.10, Howard Yen wrote:
> To let the xhci driver support USB offload, add hooks for vendor to have
> customized behavior for the initialization, memory allocation, irq work, and 
> device context synchronization. Detail is in each patch commit message.

Is this related to the usb audio sideband capability that was added to the xHCI specification?
If yes, then we should probably implement the generic parts first, and then add
the vendor specific hooks.

-Mathias
Howard Yen Jan. 20, 2021, 10:04 a.m. UTC | #3
On Tue, Jan 19, 2021 at 8:47 PM Mathias Nyman <mathias.nyman@intel.com> wrote:
>
> On 19.1.2021 12.10, Howard Yen wrote:
> > To let the xhci driver support USB offload, add hooks for vendor to have
> > customized behavior for the initialization, memory allocation, irq work, and
> > device context synchronization. Detail is in each patch commit message.
>
> Is this related to the usb audio sideband capability that was added to the xHCI specification?
> If yes, then we should probably implement the generic parts first, and then add
> the vendor specific hooks.
>
> -Mathias
>
>

This is for offloading, no matter what type of offloading.
I made the hooks generically and can be used for usb audio on the xhci
which is not including the usb audio sideband capability.


- Howard
Mathias Nyman Jan. 22, 2021, 3:32 p.m. UTC | #4
On 20.1.2021 12.04, Howard Yen wrote:
> On Tue, Jan 19, 2021 at 8:47 PM Mathias Nyman <mathias.nyman@intel.com> wrote:
>>
>> On 19.1.2021 12.10, Howard Yen wrote:
>>> To let the xhci driver support USB offload, add hooks for vendor to have
>>> customized behavior for the initialization, memory allocation, irq work, and
>>> device context synchronization. Detail is in each patch commit message.
>>
>> Is this related to the usb audio sideband capability that was added to the xHCI specification?
>> If yes, then we should probably implement the generic parts first, and then add
>> the vendor specific hooks.
>>
>> -Mathias
>>
>>
> 
> This is for offloading, no matter what type of offloading.
> I made the hooks generically and can be used for usb audio on the xhci
> which is not including the usb audio sideband capability.
> 

Ok, before adding hooks like this I think we need to see how they are used.
Do you have the rest of the patches that go on top of this series?

Maybe it could make sense to use overrides for the functions in struct hc_driver
instead in some cases? There is support for that already.

Thanks
-Mathias
Greg KH Jan. 26, 2021, 2:19 p.m. UTC | #5
On Fri, Jan 22, 2021 at 05:32:58PM +0200, Mathias Nyman wrote:
> On 20.1.2021 12.04, Howard Yen wrote:
> > On Tue, Jan 19, 2021 at 8:47 PM Mathias Nyman <mathias.nyman@intel.com> wrote:
> >>
> >> On 19.1.2021 12.10, Howard Yen wrote:
> >>> To let the xhci driver support USB offload, add hooks for vendor to have
> >>> customized behavior for the initialization, memory allocation, irq work, and
> >>> device context synchronization. Detail is in each patch commit message.
> >>
> >> Is this related to the usb audio sideband capability that was added to the xHCI specification?
> >> If yes, then we should probably implement the generic parts first, and then add
> >> the vendor specific hooks.
> >>
> >> -Mathias
> >>
> >>
> > 
> > This is for offloading, no matter what type of offloading.
> > I made the hooks generically and can be used for usb audio on the xhci
> > which is not including the usb audio sideband capability.
> > 
> 
> Ok, before adding hooks like this I think we need to see how they are used.
> Do you have the rest of the patches that go on top of this series?
> 
> Maybe it could make sense to use overrides for the functions in struct hc_driver
> instead in some cases? There is support for that already.

What overrides could be done for these changes?  At first glance that
would seem to require a lot of duplicated code in whatever override
happens to be needed.

thanks,

greg k-h
Howard Yen Jan. 28, 2021, 3:38 a.m. UTC | #6
On Tue, Jan 26, 2021 at 10:19 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Fri, Jan 22, 2021 at 05:32:58PM +0200, Mathias Nyman wrote:
> > On 20.1.2021 12.04, Howard Yen wrote:
> > > On Tue, Jan 19, 2021 at 8:47 PM Mathias Nyman <mathias.nyman@intel.com> wrote:
> > >>
> > >> On 19.1.2021 12.10, Howard Yen wrote:
> > >>> To let the xhci driver support USB offload, add hooks for vendor to have
> > >>> customized behavior for the initialization, memory allocation, irq work, and
> > >>> device context synchronization. Detail is in each patch commit message.
> > >>
> > >> Is this related to the usb audio sideband capability that was added to the xHCI specification?
> > >> If yes, then we should probably implement the generic parts first, and then add
> > >> the vendor specific hooks.
> > >>
> > >> -Mathias
> > >>
> > >>
> > >
> > > This is for offloading, no matter what type of offloading.
> > > I made the hooks generically and can be used for usb audio on the xhci
> > > which is not including the usb audio sideband capability.
> > >
> >
> > Ok, before adding hooks like this I think we need to see how they are used.
> > Do you have the rest of the patches that go on top of this series?
> >
> > Maybe it could make sense to use overrides for the functions in struct hc_driver
> > instead in some cases? There is support for that already.
>
> What overrides could be done for these changes?  At first glance that
> would seem to require a lot of duplicated code in whatever override
> happens to be needed.
>
> thanks,
>
> greg k-h

This patch series is all the changes for the offload hooks currently.

I thought about this, but if I tried to override the functions in
struct hc_driver, that'll need to
copy many code to the override function, and it won't follow the
latest change in the core
xhci driver.


- Howard
Mathias Nyman Jan. 28, 2021, 6:31 a.m. UTC | #7
On 28.1.2021 5.38, Howard Yen wrote:
> On Tue, Jan 26, 2021 at 10:19 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>>
>> On Fri, Jan 22, 2021 at 05:32:58PM +0200, Mathias Nyman wrote:
>>>
>>> Ok, before adding hooks like this I think we need to see how they are used.
>>> Do you have the rest of the patches that go on top of this series?
>>>
>>> Maybe it could make sense to use overrides for the functions in struct hc_driver
>>> instead in some cases? There is support for that already.
>>
>> What overrides could be done for these changes?  At first glance that
>> would seem to require a lot of duplicated code in whatever override
>> happens to be needed.
>>
>> thanks,
>>
>> greg k-h
> 
> This patch series is all the changes for the offload hooks currently.
> 
> I thought about this, but if I tried to override the functions in
> struct hc_driver, that'll need to
> copy many code to the override function, and it won't follow the
> latest change in the core
> xhci driver.
> 
> 
> - Howard

Ok, I see. 

The point I'm trying to make is that there is no way for me to know if
these hooks are the right solution before I see any code using them.

Is the offloading code ready and public somewhere?

Thanks
-Mathias
Greg KH Jan. 28, 2021, 7:41 a.m. UTC | #8
On Thu, Jan 28, 2021 at 08:31:14AM +0200, Mathias Nyman wrote:
> On 28.1.2021 5.38, Howard Yen wrote:
> > On Tue, Jan 26, 2021 at 10:19 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >>
> >> On Fri, Jan 22, 2021 at 05:32:58PM +0200, Mathias Nyman wrote:
> >>>
> >>> Ok, before adding hooks like this I think we need to see how they are used.
> >>> Do you have the rest of the patches that go on top of this series?
> >>>
> >>> Maybe it could make sense to use overrides for the functions in struct hc_driver
> >>> instead in some cases? There is support for that already.
> >>
> >> What overrides could be done for these changes?  At first glance that
> >> would seem to require a lot of duplicated code in whatever override
> >> happens to be needed.
> >>
> >> thanks,
> >>
> >> greg k-h
> > 
> > This patch series is all the changes for the offload hooks currently.
> > 
> > I thought about this, but if I tried to override the functions in
> > struct hc_driver, that'll need to
> > copy many code to the override function, and it won't follow the
> > latest change in the core
> > xhci driver.
> > 
> > 
> > - Howard
> 
> Ok, I see. 
> 
> The point I'm trying to make is that there is no way for me to know if
> these hooks are the right solution before I see any code using them.
> 
> Is the offloading code ready and public somewhere?

There is offload code published in the last few Samsung phone kernels, I
want to get that ported to these hooks to see if that works properly.

Give me a few days and I'll see if I can get it working, I had a
half-finished port around here somewhere...

thanks,

greg k-h