mbox series

[v3,0/4] support USB offload feature

Message ID 1647853194-62147-1-git-send-email-dh10.jung@samsung.com (mailing list archive)
Headers show
Series support USB offload feature | expand

Message

Jung Daehwan March 21, 2022, 8:59 a.m. UTC
This patchset is for USB offload feature, which makes Co-processor to use
some memories of xhci. Especially it's useful for USB Audio scenario.
Audio stream would get shortcut because Co-processor directly write/read
data in xhci memories. It could get speed-up using faster memory like SRAM.
That's why this gives vendors flexibilty of memory management.
Several pathches have been merged in AOSP kernel(android12-5.10) and I put
together and split into 3 patches. Plus let me add user(xhci-exynos)
module to see how user could use it.

To sum up, it's for providing xhci memories to Co-Processor.
It would cover DCBAA, Device Context, Transfer Ring, Event Ring, ERST.
It needs xhci hooks and to export some xhci symbols.

Changes in v2 :
- Fix commit message by adding Signed-off-by in each patch.
- Fix conflict on latest.

Changes in v3 :
- Remove export symbols and xhci hooks which xhci-exynos don't need.
- Modify commit message to clarify why it needs to export symbols.
- Check compiling of xhci-exynos.

Daehwan Jung (4):
  usb: host: export symbols for xhci hooks usage
  usb: host: add xhci hooks for USB offload
  usb: host: add some to xhci overrides for USB offload
  usb: host: add xhci-exynos driver

 drivers/usb/host/Kconfig       |   9 +
 drivers/usb/host/Makefile      |   1 +
 drivers/usb/host/xhci-exynos.c | 982 +++++++++++++++++++++++++++++++++
 drivers/usb/host/xhci-exynos.h |  63 +++
 drivers/usb/host/xhci-hub.c    |   7 +
 drivers/usb/host/xhci-mem.c    | 150 ++++-
 drivers/usb/host/xhci-plat.c   |  43 +-
 drivers/usb/host/xhci-plat.h   |   7 +
 drivers/usb/host/xhci-ring.c   |   1 +
 drivers/usb/host/xhci.c        |  90 ++-
 drivers/usb/host/xhci.h        |  50 ++
 11 files changed, 1379 insertions(+), 24 deletions(-)
 create mode 100644 drivers/usb/host/xhci-exynos.c
 create mode 100644 drivers/usb/host/xhci-exynos.h

Comments

Greg KH March 21, 2022, 9:14 a.m. UTC | #1
On Mon, Mar 21, 2022 at 05:59:50PM +0900, Daehwan Jung wrote:
> This patchset is for USB offload feature, which makes Co-processor to use
> some memories of xhci. Especially it's useful for USB Audio scenario.
> Audio stream would get shortcut because Co-processor directly write/read
> data in xhci memories. It could get speed-up using faster memory like SRAM.
> That's why this gives vendors flexibilty of memory management.
> Several pathches have been merged in AOSP kernel(android12-5.10) and I put
> together and split into 3 patches. Plus let me add user(xhci-exynos)
> module to see how user could use it.
> 
> To sum up, it's for providing xhci memories to Co-Processor.
> It would cover DCBAA, Device Context, Transfer Ring, Event Ring, ERST.
> It needs xhci hooks and to export some xhci symbols.
> 
> Changes in v2 :
> - Fix commit message by adding Signed-off-by in each patch.
> - Fix conflict on latest.
> 
> Changes in v3 :
> - Remove export symbols and xhci hooks which xhci-exynos don't need.
> - Modify commit message to clarify why it needs to export symbols.
> - Check compiling of xhci-exynos.

As I asked for in the previous submission, you MUST have a user for
these hooks, otherwise we can not accept them (nor would you WANT us to
accept them).  Please fix that up and add them to the next submission as
we can not do anything with this one.

thanks,

greg k-h
Jung Daehwan March 21, 2022, 9:24 a.m. UTC | #2
On Mon, Mar 21, 2022 at 10:14:23AM +0100, Greg Kroah-Hartman wrote:
> On Mon, Mar 21, 2022 at 05:59:50PM +0900, Daehwan Jung wrote:
> > This patchset is for USB offload feature, which makes Co-processor to use
> > some memories of xhci. Especially it's useful for USB Audio scenario.
> > Audio stream would get shortcut because Co-processor directly write/read
> > data in xhci memories. It could get speed-up using faster memory like SRAM.
> > That's why this gives vendors flexibilty of memory management.
> > Several pathches have been merged in AOSP kernel(android12-5.10) and I put
> > together and split into 3 patches. Plus let me add user(xhci-exynos)
> > module to see how user could use it.
> > 
> > To sum up, it's for providing xhci memories to Co-Processor.
> > It would cover DCBAA, Device Context, Transfer Ring, Event Ring, ERST.
> > It needs xhci hooks and to export some xhci symbols.
> > 
> > Changes in v2 :
> > - Fix commit message by adding Signed-off-by in each patch.
> > - Fix conflict on latest.
> > 
> > Changes in v3 :
> > - Remove export symbols and xhci hooks which xhci-exynos don't need.
> > - Modify commit message to clarify why it needs to export symbols.
> > - Check compiling of xhci-exynos.
> 
> As I asked for in the previous submission, you MUST have a user for
> these hooks, otherwise we can not accept them (nor would you WANT us to
> accept them).  Please fix that up and add them to the next submission as
> we can not do anything with this one.
> 
> thanks,
> 
> greg k-h
> 

Hi greg,

I've submitted the user(xhci-exynos) together on the last patch of the patchset.
You can see xhci-exynos uses these hooks and symbols.

[PATCH v3 4/4] usb: host: add xhci-exynos driver

Best Regards,
Jung Daehwan
Greg KH March 21, 2022, 9:32 a.m. UTC | #3
On Mon, Mar 21, 2022 at 06:24:09PM +0900, Jung Daehwan wrote:
> On Mon, Mar 21, 2022 at 10:14:23AM +0100, Greg Kroah-Hartman wrote:
> > On Mon, Mar 21, 2022 at 05:59:50PM +0900, Daehwan Jung wrote:
> > > This patchset is for USB offload feature, which makes Co-processor to use
> > > some memories of xhci. Especially it's useful for USB Audio scenario.
> > > Audio stream would get shortcut because Co-processor directly write/read
> > > data in xhci memories. It could get speed-up using faster memory like SRAM.
> > > That's why this gives vendors flexibilty of memory management.
> > > Several pathches have been merged in AOSP kernel(android12-5.10) and I put
> > > together and split into 3 patches. Plus let me add user(xhci-exynos)
> > > module to see how user could use it.
> > > 
> > > To sum up, it's for providing xhci memories to Co-Processor.
> > > It would cover DCBAA, Device Context, Transfer Ring, Event Ring, ERST.
> > > It needs xhci hooks and to export some xhci symbols.
> > > 
> > > Changes in v2 :
> > > - Fix commit message by adding Signed-off-by in each patch.
> > > - Fix conflict on latest.
> > > 
> > > Changes in v3 :
> > > - Remove export symbols and xhci hooks which xhci-exynos don't need.
> > > - Modify commit message to clarify why it needs to export symbols.
> > > - Check compiling of xhci-exynos.
> > 
> > As I asked for in the previous submission, you MUST have a user for
> > these hooks, otherwise we can not accept them (nor would you WANT us to
> > accept them).  Please fix that up and add them to the next submission as
> > we can not do anything with this one.
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> Hi greg,
> 
> I've submitted the user(xhci-exynos) together on the last patch of the patchset.
> You can see xhci-exynos uses these hooks and symbols.
> 
> [PATCH v3 4/4] usb: host: add xhci-exynos driver

Then this is not "offload" hooks at all.  They are merely "support
another xhci platform driver, right?

I see a lot of exports and function hooks added, are they _ALL_ used by
the xhci driver?  If so, please reword this series as it is not very
obvious at all what you are doing.

thanks,

greg k-h
Jung Daehwan March 21, 2022, 10:06 a.m. UTC | #4
On Mon, Mar 21, 2022 at 10:32:25AM +0100, Greg Kroah-Hartman wrote:
> On Mon, Mar 21, 2022 at 06:24:09PM +0900, Jung Daehwan wrote:
> > On Mon, Mar 21, 2022 at 10:14:23AM +0100, Greg Kroah-Hartman wrote:
> > > On Mon, Mar 21, 2022 at 05:59:50PM +0900, Daehwan Jung wrote:
> > > > This patchset is for USB offload feature, which makes Co-processor to use
> > > > some memories of xhci. Especially it's useful for USB Audio scenario.
> > > > Audio stream would get shortcut because Co-processor directly write/read
> > > > data in xhci memories. It could get speed-up using faster memory like SRAM.
> > > > That's why this gives vendors flexibilty of memory management.
> > > > Several pathches have been merged in AOSP kernel(android12-5.10) and I put
> > > > together and split into 3 patches. Plus let me add user(xhci-exynos)
> > > > module to see how user could use it.
> > > > 
> > > > To sum up, it's for providing xhci memories to Co-Processor.
> > > > It would cover DCBAA, Device Context, Transfer Ring, Event Ring, ERST.
> > > > It needs xhci hooks and to export some xhci symbols.
> > > > 
> > > > Changes in v2 :
> > > > - Fix commit message by adding Signed-off-by in each patch.
> > > > - Fix conflict on latest.
> > > > 
> > > > Changes in v3 :
> > > > - Remove export symbols and xhci hooks which xhci-exynos don't need.
> > > > - Modify commit message to clarify why it needs to export symbols.
> > > > - Check compiling of xhci-exynos.
> > > 
> > > As I asked for in the previous submission, you MUST have a user for
> > > these hooks, otherwise we can not accept them (nor would you WANT us to
> > > accept them).  Please fix that up and add them to the next submission as
> > > we can not do anything with this one.
> > > 
> > > thanks,
> > > 
> > > greg k-h
> > > 
> > 
> > Hi greg,
> > 
> > I've submitted the user(xhci-exynos) together on the last patch of the patchset.
> > You can see xhci-exynos uses these hooks and symbols.
> > 
> > [PATCH v3 4/4] usb: host: add xhci-exynos driver
> 
> Then this is not "offload" hooks at all.  They are merely "support
> another xhci platform driver, right?

Yes, right.

> 
> I see a lot of exports and function hooks added, are they _ALL_ used by
> the xhci driver?  If so, please reword this series as it is not very
> obvious at all what you are doing.

Yes, they are all used by the xhci driver. Is it OK for me to use "xhci-exynos"
instead of "USB offload" on series like below?

[v3, 0/4] add xhci-exynos driver

This patchset is for support xhci-exynos driver....
....

  usb: host: export symbols for xhci-exynos to use xhci hooks
  usb: host: add xhci hooks for xhci-exynos
  usb: host: add some to xhci overrides for xhci-exynos
  usb: host: add xhci-exynos driver

Best Regards,
Jung Daehwan

> 
> thanks,
> 
> greg k-h
>
Greg KH March 21, 2022, 10:16 a.m. UTC | #5
On Mon, Mar 21, 2022 at 07:06:31PM +0900, Jung Daehwan wrote:
> On Mon, Mar 21, 2022 at 10:32:25AM +0100, Greg Kroah-Hartman wrote:
> > On Mon, Mar 21, 2022 at 06:24:09PM +0900, Jung Daehwan wrote:
> > > On Mon, Mar 21, 2022 at 10:14:23AM +0100, Greg Kroah-Hartman wrote:
> > > > On Mon, Mar 21, 2022 at 05:59:50PM +0900, Daehwan Jung wrote:
> > > > > This patchset is for USB offload feature, which makes Co-processor to use
> > > > > some memories of xhci. Especially it's useful for USB Audio scenario.
> > > > > Audio stream would get shortcut because Co-processor directly write/read
> > > > > data in xhci memories. It could get speed-up using faster memory like SRAM.
> > > > > That's why this gives vendors flexibilty of memory management.
> > > > > Several pathches have been merged in AOSP kernel(android12-5.10) and I put
> > > > > together and split into 3 patches. Plus let me add user(xhci-exynos)
> > > > > module to see how user could use it.
> > > > > 
> > > > > To sum up, it's for providing xhci memories to Co-Processor.
> > > > > It would cover DCBAA, Device Context, Transfer Ring, Event Ring, ERST.
> > > > > It needs xhci hooks and to export some xhci symbols.
> > > > > 
> > > > > Changes in v2 :
> > > > > - Fix commit message by adding Signed-off-by in each patch.
> > > > > - Fix conflict on latest.
> > > > > 
> > > > > Changes in v3 :
> > > > > - Remove export symbols and xhci hooks which xhci-exynos don't need.
> > > > > - Modify commit message to clarify why it needs to export symbols.
> > > > > - Check compiling of xhci-exynos.
> > > > 
> > > > As I asked for in the previous submission, you MUST have a user for
> > > > these hooks, otherwise we can not accept them (nor would you WANT us to
> > > > accept them).  Please fix that up and add them to the next submission as
> > > > we can not do anything with this one.
> > > > 
> > > > thanks,
> > > > 
> > > > greg k-h
> > > > 
> > > 
> > > Hi greg,
> > > 
> > > I've submitted the user(xhci-exynos) together on the last patch of the patchset.
> > > You can see xhci-exynos uses these hooks and symbols.
> > > 
> > > [PATCH v3 4/4] usb: host: add xhci-exynos driver
> > 
> > Then this is not "offload" hooks at all.  They are merely "support
> > another xhci platform driver, right?
> 
> Yes, right.
> 
> > 
> > I see a lot of exports and function hooks added, are they _ALL_ used by
> > the xhci driver?  If so, please reword this series as it is not very
> > obvious at all what you are doing.
> 
> Yes, they are all used by the xhci driver. Is it OK for me to use "xhci-exynos"
> instead of "USB offload" on series like below?
> 
> [v3, 0/4] add xhci-exynos driver
> 
> This patchset is for support xhci-exynos driver....
> ....
> 
>   usb: host: export symbols for xhci-exynos to use xhci hooks
>   usb: host: add xhci hooks for xhci-exynos
>   usb: host: add some to xhci overrides for xhci-exynos
>   usb: host: add xhci-exynos driver

Yes, that makes much more sense.  What would you want to see if you had
to review such a series?

thanks,

greg k-h
Jung Daehwan March 22, 2022, 2:17 a.m. UTC | #6
On Mon, Mar 21, 2022 at 11:16:35AM +0100, Greg Kroah-Hartman wrote:
> On Mon, Mar 21, 2022 at 07:06:31PM +0900, Jung Daehwan wrote:
> > On Mon, Mar 21, 2022 at 10:32:25AM +0100, Greg Kroah-Hartman wrote:
> > > On Mon, Mar 21, 2022 at 06:24:09PM +0900, Jung Daehwan wrote:
> > > > On Mon, Mar 21, 2022 at 10:14:23AM +0100, Greg Kroah-Hartman wrote:
> > > > > On Mon, Mar 21, 2022 at 05:59:50PM +0900, Daehwan Jung wrote:
> > > > > > This patchset is for USB offload feature, which makes Co-processor to use
> > > > > > some memories of xhci. Especially it's useful for USB Audio scenario.
> > > > > > Audio stream would get shortcut because Co-processor directly write/read
> > > > > > data in xhci memories. It could get speed-up using faster memory like SRAM.
> > > > > > That's why this gives vendors flexibilty of memory management.
> > > > > > Several pathches have been merged in AOSP kernel(android12-5.10) and I put
> > > > > > together and split into 3 patches. Plus let me add user(xhci-exynos)
> > > > > > module to see how user could use it.
> > > > > > 
> > > > > > To sum up, it's for providing xhci memories to Co-Processor.
> > > > > > It would cover DCBAA, Device Context, Transfer Ring, Event Ring, ERST.
> > > > > > It needs xhci hooks and to export some xhci symbols.
> > > > > > 
> > > > > > Changes in v2 :
> > > > > > - Fix commit message by adding Signed-off-by in each patch.
> > > > > > - Fix conflict on latest.
> > > > > > 
> > > > > > Changes in v3 :
> > > > > > - Remove export symbols and xhci hooks which xhci-exynos don't need.
> > > > > > - Modify commit message to clarify why it needs to export symbols.
> > > > > > - Check compiling of xhci-exynos.
> > > > > 
> > > > > As I asked for in the previous submission, you MUST have a user for
> > > > > these hooks, otherwise we can not accept them (nor would you WANT us to
> > > > > accept them).  Please fix that up and add them to the next submission as
> > > > > we can not do anything with this one.
> > > > > 
> > > > > thanks,
> > > > > 
> > > > > greg k-h
> > > > > 
> > > > 
> > > > Hi greg,
> > > > 
> > > > I've submitted the user(xhci-exynos) together on the last patch of the patchset.
> > > > You can see xhci-exynos uses these hooks and symbols.
> > > > 
> > > > [PATCH v3 4/4] usb: host: add xhci-exynos driver
> > > 
> > > Then this is not "offload" hooks at all.  They are merely "support
> > > another xhci platform driver, right?
> > 
> > Yes, right.
> > 
> > > 
> > > I see a lot of exports and function hooks added, are they _ALL_ used by
> > > the xhci driver?  If so, please reword this series as it is not very
> > > obvious at all what you are doing.
> > 
> > Yes, they are all used by the xhci driver. Is it OK for me to use "xhci-exynos"
> > instead of "USB offload" on series like below?
> > 
> > [v3, 0/4] add xhci-exynos driver
> > 
> > This patchset is for support xhci-exynos driver....
> > ....
> > 
> >   usb: host: export symbols for xhci-exynos to use xhci hooks
> >   usb: host: add xhci hooks for xhci-exynos
> >   usb: host: add some to xhci overrides for xhci-exynos
> >   usb: host: add xhci-exynos driver
> 
> Yes, that makes much more sense.  What would you want to see if you had
> to review such a series?
> 
> thanks,
> 
> greg k-h
> 

Thanks for your comment. I'm going to modify it on next submission.

Best Regards,
Jung Daehwan
Krzysztof Kozlowski March 22, 2022, 5:05 p.m. UTC | #7
On Mon, 21 Mar 2022 at 11:16, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> >
> > [v3, 0/4] add xhci-exynos driver
> >
> > This patchset is for support xhci-exynos driver....
> > ....
> >
> >   usb: host: export symbols for xhci-exynos to use xhci hooks
> >   usb: host: add xhci hooks for xhci-exynos
> >   usb: host: add some to xhci overrides for xhci-exynos
> >   usb: host: add xhci-exynos driver
>
> Yes, that makes much more sense.  What would you want to see if you had
> to review such a series?

Unfortunately it might not make more sense, because last time
xhci-exynos driver was a fake driver, not for submission. It did not
compile, it did not work in mainline.

That driver was not even sent to proper mailing lists, as pointed out
by get_maintainers.pl, maybe because it was not developed on the
mainline kernel, so there is no MAINTAINERS file?


Best regards,
Krzysztof
Jung Daehwan March 23, 2022, 1:31 a.m. UTC | #8
On Tue, Mar 22, 2022 at 06:05:49PM +0100, Krzysztof Kozlowski wrote:
> On Mon, 21 Mar 2022 at 11:16, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > >
> > > [v3, 0/4] add xhci-exynos driver
> > >
> > > This patchset is for support xhci-exynos driver....
> > > ....
> > >
> > >   usb: host: export symbols for xhci-exynos to use xhci hooks
> > >   usb: host: add xhci hooks for xhci-exynos
> > >   usb: host: add some to xhci overrides for xhci-exynos
> > >   usb: host: add xhci-exynos driver
> >
> > Yes, that makes much more sense.  What would you want to see if you had
> > to review such a series?
> 
> Unfortunately it might not make more sense, because last time
> xhci-exynos driver was a fake driver, not for submission. It did not
> compile, it did not work in mainline.
> 
xhci-exynos driver wasn't compiled on v1,v2 but can be compiled on v3 series.

> That driver was not even sent to proper mailing lists, as pointed out
> by get_maintainers.pl, maybe because it was not developed on the
> mainline kernel, so there is no MAINTAINERS file?

There's no MAINTAINERS file yet as you guess.

Best Regards,
Jung Daehwan

> 
> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski March 23, 2022, 8:25 a.m. UTC | #9
On 23/03/2022 02:31, Jung Daehwan wrote:
> On Tue, Mar 22, 2022 at 06:05:49PM +0100, Krzysztof Kozlowski wrote:
>> On Mon, 21 Mar 2022 at 11:16, Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org> wrote:
>>>>
>>>> [v3, 0/4] add xhci-exynos driver
>>>>
>>>> This patchset is for support xhci-exynos driver....
>>>> ....
>>>>
>>>>   usb: host: export symbols for xhci-exynos to use xhci hooks
>>>>   usb: host: add xhci hooks for xhci-exynos
>>>>   usb: host: add some to xhci overrides for xhci-exynos
>>>>   usb: host: add xhci-exynos driver
>>>
>>> Yes, that makes much more sense.  What would you want to see if you had
>>> to review such a series?
>>
>> Unfortunately it might not make more sense, because last time
>> xhci-exynos driver was a fake driver, not for submission. It did not
>> compile, it did not work in mainline.
>>
> xhci-exynos driver wasn't compiled on v1,v2 but can be compiled on v3 series.
> 
>> That driver was not even sent to proper mailing lists, as pointed out
>> by get_maintainers.pl, maybe because it was not developed on the
>> mainline kernel, so there is no MAINTAINERS file?
> 
> There's no MAINTAINERS file yet as you guess.
> 

Which is a proof you develop on some custom tree, not on Linux kernel.

Don't. We cannot accept code which was not based on Linux kernel, but
some out of tree local flavor.

All submissions must be based on upstream trees (mainline, maintainer's
for-next or linux-next).


Best regards,
Krzysztof