[PATCH v3 0/4] support USB offload feature

[PATCH v3 0/4] support USB offload feature

[Daehwan Jung]

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

-- 
2.31.1

[PATCH v3 1/4] usb: host: export symbols for xhci hooks usage

[Daehwan Jung]

Export symbols for xhci hooks usage:
    xhci_get_slot_ctx
    xhci_get_endpoint_address
    - Allow xhci hook to get ep_ctx from the xhci_container_ctx for
      getting the ep_ctx information to know which ep is offloading and
      comparing the context in remote subsystem memory if needed.

    xhci_ring_alloc
    - Allow xhci hook to allocate vendor specific ring.

    xhci_trb_virt_to_dma
    - Used to retrieve the DMA address of vendor specific ring.

    xhci_segment_free
    xhci_link_segments
    - Allow xhci hook to handle vendor specific segment.

    xhci_initialize_ring_info
    - Allow xhci hook to initialize vendor specific ring.

    xhci_check_trb_in_td_math
    - Allow xhci hook to Check TRB math for validation.

    xhci_address_device
    - Allow override to give configuration info to Co-processor.

    xhci_bus_suspend
    xhci_bus_resume
    - Allow override of suspend and resume for power scenario.

    xhci_remove_stream_mapping
    - Allow xhci hook to remove stream mapping.

Signed-off-by: Daehwan Jung <dh10.jung@samsung.com>
Signed-off-by: Jack Pham <jackp@codeaurora.org>
Signed-off-by: Howard Yen <howardyen@google.com>
---
 drivers/usb/host/xhci-hub.c  |  2 ++
 drivers/usb/host/xhci-mem.c  | 19 +++++++++++++------
 drivers/usb/host/xhci-ring.c |  1 +
 drivers/usb/host/xhci.c      |  4 +++-
 4 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 1e7dc130c39a..56546aaa93c7 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1812,6 +1812,7 @@ int xhci_bus_suspend(struct usb_hcd *hcd)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(xhci_bus_suspend);
 
 /*
  * Workaround for missing Cold Attach Status (CAS) if device re-plugged in S3.
@@ -1956,6 +1957,7 @@ int xhci_bus_resume(struct usb_hcd *hcd)
 	spin_unlock_irqrestore(&xhci->lock, flags);
 	return 0;
 }
+EXPORT_SYMBOL_GPL(xhci_bus_resume);
 
 unsigned long xhci_get_resuming_ports(struct usb_hcd *hcd)
 {
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index bbb27ee2c6a3..82b9f90c0f27 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -65,7 +65,7 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci,
 	return seg;
 }
 
-static void xhci_segment_free(struct xhci_hcd *xhci, struct xhci_segment *seg)
+void xhci_segment_free(struct xhci_hcd *xhci, struct xhci_segment *seg)
 {
 	if (seg->trbs) {
 		dma_pool_free(xhci->segment_pool, seg->trbs, seg->dma);
@@ -74,6 +74,7 @@ static void xhci_segment_free(struct xhci_hcd *xhci, struct xhci_segment *seg)
 	kfree(seg->bounce_buf);
 	kfree(seg);
 }
+EXPORT_SYMBOL_GPL(xhci_segment_free);
 
 static void xhci_free_segments_for_ring(struct xhci_hcd *xhci,
 				struct xhci_segment *first)
@@ -96,9 +97,9 @@ static void xhci_free_segments_for_ring(struct xhci_hcd *xhci,
  * DMA address of the next segment.  The caller needs to set any Link TRB
  * related flags, such as End TRB, Toggle Cycle, and no snoop.
  */
-static void xhci_link_segments(struct xhci_segment *prev,
-			       struct xhci_segment *next,
-			       enum xhci_ring_type type, bool chain_links)
+void xhci_link_segments(struct xhci_segment *prev,
+			struct xhci_segment *next,
+			enum xhci_ring_type type, bool chain_links)
 {
 	u32 val;
 
@@ -118,6 +119,7 @@ static void xhci_link_segments(struct xhci_segment *prev,
 		prev->trbs[TRBS_PER_SEGMENT-1].link.control = cpu_to_le32(val);
 	}
 }
+EXPORT_SYMBOL_GPL(xhci_link_segments);
 
 /*
  * Link the ring to the new segments.
@@ -256,7 +258,7 @@ static int xhci_update_stream_segment_mapping(
 	return ret;
 }
 
-static void xhci_remove_stream_mapping(struct xhci_ring *ring)
+void xhci_remove_stream_mapping(struct xhci_ring *ring)
 {
 	struct xhci_segment *seg;
 
@@ -269,6 +271,7 @@ static void xhci_remove_stream_mapping(struct xhci_ring *ring)
 		seg = seg->next;
 	} while (seg != ring->first_seg);
 }
+EXPORT_SYMBOL_GPL(xhci_remove_stream_mapping);
 
 static int xhci_update_stream_mapping(struct xhci_ring *ring, gfp_t mem_flags)
 {
@@ -316,6 +319,7 @@ void xhci_initialize_ring_info(struct xhci_ring *ring,
 	 */
 	ring->num_trbs_free = ring->num_segs * (TRBS_PER_SEGMENT - 1) - 1;
 }
+EXPORT_SYMBOL_GPL(xhci_initialize_ring_info);
 
 /* Allocate segments and link them for a ring */
 static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci,
@@ -407,6 +411,7 @@ struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci,
 	kfree(ring);
 	return NULL;
 }
+EXPORT_SYMBOL_GPL(xhci_ring_alloc);
 
 void xhci_free_endpoint_ring(struct xhci_hcd *xhci,
 		struct xhci_virt_device *virt_dev,
@@ -518,6 +523,7 @@ struct xhci_slot_ctx *xhci_get_slot_ctx(struct xhci_hcd *xhci,
 	return (struct xhci_slot_ctx *)
 		(ctx->bytes + CTX_SIZE(xhci->hcc_params));
 }
+EXPORT_SYMBOL_GPL(xhci_get_slot_ctx);
 
 struct xhci_ep_ctx *xhci_get_ep_ctx(struct xhci_hcd *xhci,
 				    struct xhci_container_ctx *ctx,
@@ -1965,7 +1971,7 @@ static int xhci_test_trb_in_td(struct xhci_hcd *xhci,
 }
 
 /* TRB math checks for xhci_trb_in_td(), using the command and event rings. */
-static int xhci_check_trb_in_td_math(struct xhci_hcd *xhci)
+int xhci_check_trb_in_td_math(struct xhci_hcd *xhci)
 {
 	struct {
 		dma_addr_t		input_dma;
@@ -2085,6 +2091,7 @@ static int xhci_check_trb_in_td_math(struct xhci_hcd *xhci)
 	xhci_dbg(xhci, "TRB math tests passed.\n");
 	return 0;
 }
+EXPORT_SYMBOL_GPL(xhci_check_trb_in_td_math);
 
 static void xhci_set_hc_event_deq(struct xhci_hcd *xhci)
 {
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index d0b6806275e0..c6896bdab763 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -79,6 +79,7 @@ dma_addr_t xhci_trb_virt_to_dma(struct xhci_segment *seg,
 		return 0;
 	return seg->dma + (segment_offset * sizeof(*trb));
 }
+EXPORT_SYMBOL_GPL(xhci_trb_virt_to_dma);
 
 static bool trb_is_noop(union xhci_trb *trb)
 {
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 642610c78f58..8f53672dcd97 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1457,6 +1457,7 @@ unsigned int xhci_get_endpoint_address(unsigned int ep_index)
 	unsigned int direction = ep_index % 2 ? USB_DIR_OUT : USB_DIR_IN;
 	return direction | number;
 }
+EXPORT_SYMBOL_GPL(xhci_get_endpoint_address);
 
 /* Find the flag for this endpoint (for use in the control context).  Use the
  * endpoint index to create a bitmask.  The slot context is bit 0, endpoint 0 is
@@ -4313,10 +4314,11 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
 	return ret;
 }
 
-static int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev)
+int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev)
 {
 	return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ADDRESS);
 }
+EXPORT_SYMBOL_GPL(xhci_address_device);
 
 static int xhci_enable_device(struct usb_hcd *hcd, struct usb_device *udev)
 {
-- 
2.31.1

Re: [PATCH v3 1/4] usb: host: export symbols for xhci hooks usage

[kernel test robot]

Hi Daehwan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20220318]
[also build test WARNING on v5.17]
[cannot apply to usb/usb-testing krzk/for-next char-misc/char-misc-testing v5.17 v5.17-rc8 v5.17-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Daehwan-Jung/usb-host-export-symbols-for-xhci-hooks-usage/20220321-180046
base:    6d72dda014a4753974eb08950089ddf71fec4f60
config: ia64-allmodconfig (https://download.01.org/0day-ci/archive/20220321/202203212326.t8A838in-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/0577f386af5e7bc6400e76dc23c22fbcf45d715f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Daehwan-Jung/usb-host-export-symbols-for-xhci-hooks-usage/20220321-180046
        git checkout 0577f386af5e7bc6400e76dc23c22fbcf45d715f
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/usb/host/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/usb/host/xhci-mem.c:68:6: warning: no previous prototype for 'xhci_segment_free' [-Wmissing-prototypes]
      68 | void xhci_segment_free(struct xhci_hcd *xhci, struct xhci_segment *seg)
         |      ^~~~~~~~~~~~~~~~~
   drivers/usb/host/xhci-mem.c:100:6: warning: no previous prototype for 'xhci_link_segments' [-Wmissing-prototypes]
     100 | void xhci_link_segments(struct xhci_segment *prev,
         |      ^~~~~~~~~~~~~~~~~~
>> drivers/usb/host/xhci-mem.c:261:6: warning: no previous prototype for 'xhci_remove_stream_mapping' [-Wmissing-prototypes]
     261 | void xhci_remove_stream_mapping(struct xhci_ring *ring)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/usb/host/xhci-mem.c:1974:5: warning: no previous prototype for 'xhci_check_trb_in_td_math' [-Wmissing-prototypes]
    1974 | int xhci_check_trb_in_td_math(struct xhci_hcd *xhci)
         |     ^~~~~~~~~~~~~~~~~~~~~~~~~


vim +/xhci_remove_stream_mapping +261 drivers/usb/host/xhci-mem.c

   260	
 > 261	void xhci_remove_stream_mapping(struct xhci_ring *ring)
   262	{
   263		struct xhci_segment *seg;
   264	
   265		if (WARN_ON_ONCE(ring->trb_address_map == NULL))
   266			return;
   267	
   268		seg = ring->first_seg;
   269		do {
   270			xhci_remove_segment_mapping(ring->trb_address_map, seg);
   271			seg = seg->next;
   272		} while (seg != ring->first_seg);
   273	}
   274	EXPORT_SYMBOL_GPL(xhci_remove_stream_mapping);
   275	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH v3 1/4] usb: host: export symbols for xhci hooks usage

[Krzysztof Kozlowski]

On 21/03/2022 09:59, Daehwan Jung wrote:
> Export symbols for xhci hooks usage:
>     xhci_get_slot_ctx
>     xhci_get_endpoint_address
>     - Allow xhci hook to get ep_ctx from the xhci_container_ctx for
>       getting the ep_ctx information to know which ep is offloading and
>       comparing the context in remote subsystem memory if needed.
> 
>     xhci_ring_alloc
>     - Allow xhci hook to allocate vendor specific ring.
> 
>     xhci_trb_virt_to_dma
>     - Used to retrieve the DMA address of vendor specific ring.
> 
>     xhci_segment_free
>     xhci_link_segments
>     - Allow xhci hook to handle vendor specific segment.
> 
>     xhci_initialize_ring_info
>     - Allow xhci hook to initialize vendor specific ring.
> 
>     xhci_check_trb_in_td_math
>     - Allow xhci hook to Check TRB math for validation.
> 
>     xhci_address_device
>     - Allow override to give configuration info to Co-processor.
> 
>     xhci_bus_suspend
>     xhci_bus_resume
>     - Allow override of suspend and resume for power scenario.
> 
>     xhci_remove_stream_mapping
>     - Allow xhci hook to remove stream mapping.
> 

NAK, because you ignored my comments from previous submission. It seems
you prefer to silently skip answering to them, not CC me and then hope I
will not remember what I asked for.

I am sorry, but that is not how it works. This is not how Linux kernel
is developed. Please answer my questions. If they are unclear, ask for
explanation. Ignoring all my questions/comments and resending without CC
is a NAK.


Best regards,
Krzysztof

Re: [PATCH v3 1/4] usb: host: export symbols for xhci hooks usage

[Jung Daehwan]

[-- Attachment #1: Type: text/plain, Size: 1993 bytes --]

On Tue, Mar 22, 2022 at 06:12:58PM +0100, Krzysztof Kozlowski wrote:
> On 21/03/2022 09:59, Daehwan Jung wrote:
> > Export symbols for xhci hooks usage:
> >     xhci_get_slot_ctx
> >     xhci_get_endpoint_address
> >     - Allow xhci hook to get ep_ctx from the xhci_container_ctx for
> >       getting the ep_ctx information to know which ep is offloading and
> >       comparing the context in remote subsystem memory if needed.
> > 
> >     xhci_ring_alloc
> >     - Allow xhci hook to allocate vendor specific ring.
> > 
> >     xhci_trb_virt_to_dma
> >     - Used to retrieve the DMA address of vendor specific ring.
> > 
> >     xhci_segment_free
> >     xhci_link_segments
> >     - Allow xhci hook to handle vendor specific segment.
> > 
> >     xhci_initialize_ring_info
> >     - Allow xhci hook to initialize vendor specific ring.
> > 
> >     xhci_check_trb_in_td_math
> >     - Allow xhci hook to Check TRB math for validation.
> > 
> >     xhci_address_device
> >     - Allow override to give configuration info to Co-processor.
> > 
> >     xhci_bus_suspend
> >     xhci_bus_resume
> >     - Allow override of suspend and resume for power scenario.
> > 
> >     xhci_remove_stream_mapping
> >     - Allow xhci hook to remove stream mapping.
> > 
> 
> NAK, because you ignored my comments from previous submission. It seems
> you prefer to silently skip answering to them, not CC me and then hope I
> will not remember what I asked for.
> 
> I am sorry, but that is not how it works. This is not how Linux kernel
> is developed. Please answer my questions. If they are unclear, ask for
> explanation. Ignoring all my questions/comments and resending without CC
> is a NAK.
> 
> 
> Best regards,
> Krzysztof
> 

Hi Krzysztof

I didn't ignore your comments but focused on your comment to
be in-tree user on previous submission. I've modified my driver to be compiled
as you asked and submitted it.

I'm sorry I missed other comments. I will check them.

Best Regards,
Jung Daehwan

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH v3 2/4] usb: host: add xhci hooks for USB offload

[Daehwan Jung]

To enable supporting for USB offload, define "offload" in usb controller
node of device tree. "offload" value can be used to determine which type
of offload was been enabled in the SoC.

For example:

&usbdrd_dwc3 {
	...
	/* support usb offloading, 0: disabled, 1: audio */
	offload = <1>;
	...
};

There are several vendor_ops introduced by this patch:

struct xhci_vendor_ops - function callbacks for vendor specific operations
{
	@vendor_init:
		- called for vendor init process during xhci-plat-hcd
		  probe.
	@vendor_cleanup:
		- called for vendor cleanup process during xhci-plat-hcd
		  remove.
	@is_usb_offload_enabled:
		- called to check if usb offload enabled.
	@alloc_dcbaa:
		- called when allocating vendor specific dcbaa during
		  memory initializtion.
	@free_dcbaa:
		- called to free vendor specific dcbaa when cleanup the
		  memory.
	@alloc_transfer_ring:
		- called when vendor specific transfer ring allocation is required
	@free_transfer_ring:
		- called to free vendor specific transfer ring
	@sync_dev_ctx:
		- called when synchronization for device context is required
}

The xhci hooks with prefix "xhci_vendor_" on the ops in xhci_vendor_ops.
For example, vendor_init ops will be invoked by xhci_vendor_init() hook,
is_usb_offload_enabled ops will be invoked by
xhci_vendor_is_usb_offload_enabled(), and so on.

Signed-off-by: Daehwan Jung <dh10.jung@samsung.com>
Signed-off-by: J. Avila <elavila@google.com>
Signed-off-by: Puma Hsu <pumahsu@google.com>
Signed-off-by: Howard Yen <howardyen@google.com>
---
 drivers/usb/host/xhci-hub.c  |   5 ++
 drivers/usb/host/xhci-mem.c  | 131 +++++++++++++++++++++++++++++++----
 drivers/usb/host/xhci-plat.c |  43 +++++++++++-
 drivers/usb/host/xhci-plat.h |   7 ++
 drivers/usb/host/xhci.c      |  80 ++++++++++++++++++++-
 drivers/usb/host/xhci.h      |  46 ++++++++++++
 6 files changed, 295 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 56546aaa93c7..aea72ffce820 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -535,8 +535,13 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
 	    cmd->status == COMP_COMMAND_RING_STOPPED) {
 		xhci_warn(xhci, "Timeout while waiting for stop endpoint command\n");
 		ret = -ETIME;
+		goto cmd_cleanup;
 	}
 
+	ret = xhci_vendor_sync_dev_ctx(xhci, slot_id);
+	if (ret)
+		xhci_warn(xhci, "Sync device context failed, ret=%d\n", ret);
+
 cmd_cleanup:
 	xhci_free_command(xhci, cmd);
 	return ret;
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 82b9f90c0f27..5ee0ffb676d3 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -365,6 +365,54 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci,
 	return 0;
 }
 
+static void xhci_vendor_free_container_ctx(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx)
+{
+	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
+
+	if (ops && ops->free_container_ctx)
+		ops->free_container_ctx(xhci, ctx);
+}
+
+static void xhci_vendor_alloc_container_ctx(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
+					    int type, gfp_t flags)
+{
+	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
+
+	if (ops && ops->alloc_container_ctx)
+		ops->alloc_container_ctx(xhci, ctx, type, flags);
+}
+
+static struct xhci_ring *xhci_vendor_alloc_transfer_ring(struct xhci_hcd *xhci,
+		u32 endpoint_type, enum xhci_ring_type ring_type,
+		unsigned int max_packet, gfp_t mem_flags)
+{
+	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
+
+	if (ops && ops->alloc_transfer_ring)
+		return ops->alloc_transfer_ring(xhci, endpoint_type, ring_type,
+				max_packet, mem_flags);
+	return 0;
+}
+
+void xhci_vendor_free_transfer_ring(struct xhci_hcd *xhci,
+		struct xhci_ring *ring, unsigned int ep_index)
+{
+	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
+
+	if (ops && ops->free_transfer_ring)
+		ops->free_transfer_ring(xhci, ring, ep_index);
+}
+
+bool xhci_vendor_is_usb_offload_enabled(struct xhci_hcd *xhci,
+		struct xhci_virt_device *virt_dev, unsigned int ep_index)
+{
+	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
+
+	if (ops && ops->is_usb_offload_enabled)
+		return ops->is_usb_offload_enabled(xhci, virt_dev, ep_index);
+	return false;
+}
+
 /*
  * Create a new ring with zero or more segments.
  *
@@ -417,7 +465,11 @@ void xhci_free_endpoint_ring(struct xhci_hcd *xhci,
 		struct xhci_virt_device *virt_dev,
 		unsigned int ep_index)
 {
-	xhci_ring_free(xhci, virt_dev->eps[ep_index].ring);
+	if (xhci_vendor_is_usb_offload_enabled(xhci, virt_dev, ep_index))
+		xhci_vendor_free_transfer_ring(xhci, virt_dev->eps[ep_index].ring, ep_index);
+	else
+		xhci_ring_free(xhci, virt_dev->eps[ep_index].ring);
+
 	virt_dev->eps[ep_index].ring = NULL;
 }
 
@@ -475,6 +527,7 @@ struct xhci_container_ctx *xhci_alloc_container_ctx(struct xhci_hcd *xhci,
 {
 	struct xhci_container_ctx *ctx;
 	struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
+	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
 
 	if ((type != XHCI_CTX_TYPE_DEVICE) && (type != XHCI_CTX_TYPE_INPUT))
 		return NULL;
@@ -488,7 +541,12 @@ struct xhci_container_ctx *xhci_alloc_container_ctx(struct xhci_hcd *xhci,
 	if (type == XHCI_CTX_TYPE_INPUT)
 		ctx->size += CTX_SIZE(xhci->hcc_params);
 
-	ctx->bytes = dma_pool_zalloc(xhci->device_pool, flags, &ctx->dma);
+	if (xhci_vendor_is_usb_offload_enabled(xhci, NULL, 0) &&
+	    (ops && ops->alloc_container_ctx))
+		xhci_vendor_alloc_container_ctx(xhci, ctx, type, flags);
+	else
+		ctx->bytes = dma_pool_zalloc(xhci->device_pool, flags, &ctx->dma);
+
 	if (!ctx->bytes) {
 		kfree(ctx);
 		return NULL;
@@ -499,9 +557,16 @@ struct xhci_container_ctx *xhci_alloc_container_ctx(struct xhci_hcd *xhci,
 void xhci_free_container_ctx(struct xhci_hcd *xhci,
 			     struct xhci_container_ctx *ctx)
 {
+	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
+
 	if (!ctx)
 		return;
-	dma_pool_free(xhci->device_pool, ctx->bytes, ctx->dma);
+	if (xhci_vendor_is_usb_offload_enabled(xhci, NULL, 0) &&
+	    (ops && ops->free_container_ctx))
+		xhci_vendor_free_container_ctx(xhci, ctx);
+	else
+		dma_pool_free(xhci->device_pool, ctx->bytes, ctx->dma);
+
 	kfree(ctx);
 }
 
@@ -894,7 +959,7 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, int slot_id)
 
 	for (i = 0; i < 31; i++) {
 		if (dev->eps[i].ring)
-			xhci_ring_free(xhci, dev->eps[i].ring);
+			xhci_free_endpoint_ring(xhci, dev, i);
 		if (dev->eps[i].stream_info)
 			xhci_free_stream_info(xhci,
 					dev->eps[i].stream_info);
@@ -1492,8 +1557,16 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
 		mult = 0;
 
 	/* Set up the endpoint ring */
-	virt_dev->eps[ep_index].new_ring =
-		xhci_ring_alloc(xhci, 2, 1, ring_type, max_packet, mem_flags);
+	if (xhci_vendor_is_usb_offload_enabled(xhci, virt_dev, ep_index) &&
+	    usb_endpoint_xfer_isoc(&ep->desc)) {
+		virt_dev->eps[ep_index].new_ring =
+			xhci_vendor_alloc_transfer_ring(xhci, endpoint_type, ring_type,
+							max_packet, mem_flags);
+	} else {
+		virt_dev->eps[ep_index].new_ring =
+			xhci_ring_alloc(xhci, 2, 1, ring_type, max_packet, mem_flags);
+	}
+
 	if (!virt_dev->eps[ep_index].new_ring)
 		return -ENOMEM;
 
@@ -1837,6 +1910,24 @@ void xhci_free_erst(struct xhci_hcd *xhci, struct xhci_erst *erst)
 	erst->entries = NULL;
 }
 
+static struct xhci_device_context_array *xhci_vendor_alloc_dcbaa(
+		struct xhci_hcd *xhci, gfp_t flags)
+{
+	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
+
+	if (ops && ops->alloc_dcbaa)
+		return ops->alloc_dcbaa(xhci, flags);
+	return 0;
+}
+
+static void xhci_vendor_free_dcbaa(struct xhci_hcd *xhci)
+{
+	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
+
+	if (ops && ops->free_dcbaa)
+		ops->free_dcbaa(xhci);
+}
+
 void xhci_mem_cleanup(struct xhci_hcd *xhci)
 {
 	struct device	*dev = xhci_to_hcd(xhci)->self.sysdev;
@@ -1888,9 +1979,13 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
 			"Freed medium stream array pool");
 
-	if (xhci->dcbaa)
-		dma_free_coherent(dev, sizeof(*xhci->dcbaa),
-				xhci->dcbaa, xhci->dcbaa->dma);
+	if (xhci_vendor_is_usb_offload_enabled(xhci, NULL, 0)) {
+		xhci_vendor_free_dcbaa(xhci);
+	} else {
+		if (xhci->dcbaa)
+			dma_free_coherent(dev, sizeof(*xhci->dcbaa),
+					xhci->dcbaa, xhci->dcbaa->dma);
+	}
 	xhci->dcbaa = NULL;
 
 	scratchpad_free(xhci);
@@ -2427,15 +2522,21 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	 * xHCI section 5.4.6 - Device Context array must be
 	 * "physically contiguous and 64-byte (cache line) aligned".
 	 */
-	xhci->dcbaa = dma_alloc_coherent(dev, sizeof(*xhci->dcbaa), &dma,
-			flags);
-	if (!xhci->dcbaa)
-		goto fail;
-	xhci->dcbaa->dma = dma;
+	if (xhci_vendor_is_usb_offload_enabled(xhci, NULL, 0)) {
+		xhci->dcbaa = xhci_vendor_alloc_dcbaa(xhci, flags);
+		if (!xhci->dcbaa)
+			goto fail;
+	} else {
+		xhci->dcbaa = dma_alloc_coherent(dev, sizeof(*xhci->dcbaa), &dma,
+				flags);
+		if (!xhci->dcbaa)
+			goto fail;
+		xhci->dcbaa->dma = dma;
+	}
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
 			"// Device context base array address = 0x%llx (DMA), %p (virt)",
 			(unsigned long long)xhci->dcbaa->dma, xhci->dcbaa);
-	xhci_write_64(xhci, dma, &xhci->op_regs->dcbaa_ptr);
+	xhci_write_64(xhci, xhci->dcbaa->dma, &xhci->op_regs->dcbaa_ptr);
 
 	/*
 	 * Initialize the ring segment pool.  The ring must be a contiguous
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 649ffd861b44..3d2fe4d77f38 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -173,6 +173,41 @@ static const struct of_device_id usb_xhci_of_match[] = {
 MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
 #endif
 
+static struct xhci_plat_priv_overwrite xhci_plat_vendor_overwrite;
+
+int xhci_plat_register_vendor_ops(struct xhci_vendor_ops *vendor_ops)
+{
+	if (vendor_ops == NULL)
+		return -EINVAL;
+
+	xhci_plat_vendor_overwrite.vendor_ops = vendor_ops;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(xhci_plat_register_vendor_ops);
+
+static int xhci_vendor_init(struct xhci_hcd *xhci)
+{
+	struct xhci_vendor_ops *ops = NULL;
+
+	if (xhci_plat_vendor_overwrite.vendor_ops)
+		ops = xhci->vendor_ops = xhci_plat_vendor_overwrite.vendor_ops;
+
+	if (ops && ops->vendor_init)
+		return ops->vendor_init(xhci);
+	return 0;
+}
+
+static void xhci_vendor_cleanup(struct xhci_hcd *xhci)
+{
+	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
+
+	if (ops && ops->vendor_cleanup)
+		ops->vendor_cleanup(xhci);
+
+	xhci->vendor_ops = NULL;
+}
+
 static int xhci_plat_probe(struct platform_device *pdev)
 {
 	const struct xhci_plat_priv *priv_match;
@@ -321,6 +356,10 @@ static int xhci_plat_probe(struct platform_device *pdev)
 			goto put_usb3_hcd;
 	}
 
+	ret = xhci_vendor_init(xhci);
+	if (ret)
+		goto disable_usb_phy;
+
 	hcd->tpl_support = of_usb_host_tpl_support(sysdev->of_node);
 	xhci->shared_hcd->tpl_support = hcd->tpl_support;
 	if (priv && (priv->quirks & XHCI_SKIP_PHY_INIT))
@@ -393,8 +432,10 @@ static int xhci_plat_remove(struct platform_device *dev)
 	usb_phy_shutdown(hcd->usb_phy);
 
 	usb_remove_hcd(hcd);
-	usb_put_hcd(shared_hcd);
 
+	xhci_vendor_cleanup(xhci);
+
+	usb_put_hcd(shared_hcd);
 	clk_disable_unprepare(clk);
 	clk_disable_unprepare(reg_clk);
 	usb_put_hcd(hcd);
diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h
index 1fb149d1fbce..18b349883eea 100644
--- a/drivers/usb/host/xhci-plat.h
+++ b/drivers/usb/host/xhci-plat.h
@@ -21,4 +21,11 @@ struct xhci_plat_priv {
 
 #define hcd_to_xhci_priv(h) ((struct xhci_plat_priv *)hcd_to_xhci(h)->priv)
 #define xhci_to_priv(x) ((struct xhci_plat_priv *)(x)->priv)
+
+struct xhci_plat_priv_overwrite {
+	struct xhci_vendor_ops *vendor_ops;
+};
+
+int xhci_plat_register_vendor_ops(struct xhci_vendor_ops *vendor_ops);
+
 #endif	/* _XHCI_PLAT_H */
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 8f53672dcd97..4e4fcf97ba42 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -2975,6 +2975,14 @@ static int xhci_configure_endpoint(struct xhci_hcd *xhci,
 			xhci_finish_resource_reservation(xhci, ctrl_ctx);
 		spin_unlock_irqrestore(&xhci->lock, flags);
 	}
+	if (ret)
+		goto failed;
+
+	ret = xhci_vendor_sync_dev_ctx(xhci, udev->slot_id);
+	if (ret)
+		xhci_warn(xhci, "sync device context failed, ret=%d", ret);
+
+failed:
 	return ret;
 }
 
@@ -3118,7 +3126,11 @@ void xhci_reset_bandwidth(struct usb_hcd *hcd, struct usb_device *udev)
 	for (i = 0; i < 31; i++) {
 		if (virt_dev->eps[i].new_ring) {
 			xhci_debugfs_remove_endpoint(xhci, virt_dev, i);
-			xhci_ring_free(xhci, virt_dev->eps[i].new_ring);
+			if (xhci_vendor_is_usb_offload_enabled(xhci, virt_dev, i))
+				xhci_vendor_free_transfer_ring(xhci, virt_dev->eps[i].new_ring, i);
+			else
+				xhci_ring_free(xhci, virt_dev->eps[i].new_ring);
+
 			virt_dev->eps[i].new_ring = NULL;
 		}
 	}
@@ -3279,6 +3291,13 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
 
 	wait_for_completion(stop_cmd->completion);
 
+	err = xhci_vendor_sync_dev_ctx(xhci, udev->slot_id);
+	if (err) {
+		xhci_warn(xhci, "%s: Failed to sync device context failed, err=%d",
+			  __func__, err);
+		goto cleanup;
+	}
+
 	spin_lock_irqsave(&xhci->lock, flags);
 
 	/* config ep command clears toggle if add and drop ep flags are set */
@@ -3310,6 +3329,11 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
 
 	wait_for_completion(cfg_cmd->completion);
 
+	err = xhci_vendor_sync_dev_ctx(xhci, udev->slot_id);
+	if (err)
+		xhci_warn(xhci, "%s: Failed to sync device context failed, err=%d",
+			  __func__, err);
+
 	xhci_free_command(xhci, cfg_cmd);
 cleanup:
 	xhci_free_command(xhci, stop_cmd);
@@ -3855,6 +3879,13 @@ static int xhci_discover_or_reset_device(struct usb_hcd *hcd,
 	/* Wait for the Reset Device command to finish */
 	wait_for_completion(reset_device_cmd->completion);
 
+	ret = xhci_vendor_sync_dev_ctx(xhci, slot_id);
+	if (ret) {
+		xhci_warn(xhci, "%s: Failed to sync device context failed, err=%d",
+			  __func__, ret);
+		goto command_cleanup;
+	}
+
 	/* The Reset Device command can't fail, according to the 0.95/0.96 spec,
 	 * unless we tried to reset a slot ID that wasn't enabled,
 	 * or the device wasn't in the addressed or configured state.
@@ -4100,6 +4131,14 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
 		xhci_warn(xhci, "Could not allocate xHCI USB device data structures\n");
 		goto disable_slot;
 	}
+
+	ret = xhci_vendor_sync_dev_ctx(xhci, slot_id);
+	if (ret) {
+		xhci_warn(xhci, "%s: Failed to sync device context failed, err=%d",
+			  __func__, ret);
+		goto disable_slot;
+	}
+
 	vdev = xhci->devs[slot_id];
 	slot_ctx = xhci_get_slot_ctx(xhci, vdev->out_ctx);
 	trace_xhci_alloc_dev(slot_ctx);
@@ -4230,6 +4269,13 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
 	/* ctrl tx can take up to 5 sec; XXX: need more time for xHC? */
 	wait_for_completion(command->completion);
 
+	ret = xhci_vendor_sync_dev_ctx(xhci, udev->slot_id);
+	if (ret) {
+		xhci_warn(xhci, "%s: Failed to sync device context failed, err=%d",
+			  __func__, ret);
+		goto out;
+	}
+
 	/* FIXME: From section 4.3.4: "Software shall be responsible for timing
 	 * the SetAddress() "recovery interval" required by USB and aborting the
 	 * command on a timeout.
@@ -4382,6 +4428,14 @@ static int __maybe_unused xhci_change_max_exit_latency(struct xhci_hcd *xhci,
 		return -ENOMEM;
 	}
 
+	ret = xhci_vendor_sync_dev_ctx(xhci, udev->slot_id);
+	if (ret) {
+		spin_unlock_irqrestore(&xhci->lock, flags);
+		xhci_warn(xhci, "%s: Failed to sync device context failed, err=%d",
+			  __func__, ret);
+		return ret;
+	}
+
 	xhci_slot_copy(xhci, command->in_ctx, virt_dev->out_ctx);
 	spin_unlock_irqrestore(&xhci->lock, flags);
 
@@ -4409,6 +4463,21 @@ static int __maybe_unused xhci_change_max_exit_latency(struct xhci_hcd *xhci,
 	return ret;
 }
 
+struct xhci_vendor_ops *xhci_vendor_get_ops(struct xhci_hcd *xhci)
+{
+	return xhci->vendor_ops;
+}
+EXPORT_SYMBOL_GPL(xhci_vendor_get_ops);
+
+int xhci_vendor_sync_dev_ctx(struct xhci_hcd *xhci, unsigned int slot_id)
+{
+	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
+
+	if (ops && ops->sync_dev_ctx)
+		return ops->sync_dev_ctx(xhci, slot_id);
+	return 0;
+}
+
 #ifdef CONFIG_PM
 
 /* BESL to HIRD Encoding array for USB2 LPM */
@@ -5133,6 +5202,15 @@ static int xhci_update_hub_device(struct usb_hcd *hcd, struct usb_device *hdev,
 		return -ENOMEM;
 	}
 
+	ret = xhci_vendor_sync_dev_ctx(xhci, hdev->slot_id);
+	if (ret) {
+		xhci_warn(xhci, "%s: Failed to sync device context failed, err=%d",
+			  __func__, ret);
+		xhci_free_command(xhci, config_cmd);
+		spin_unlock_irqrestore(&xhci->lock, flags);
+		return ret;
+	}
+
 	xhci_slot_copy(xhci, config_cmd->in_ctx, vdev->out_ctx);
 	ctrl_ctx->add_flags |= cpu_to_le32(SLOT_FLAG);
 	slot_ctx = xhci_get_slot_ctx(xhci, config_cmd->in_ctx);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 473a33ce299e..f690a5dc9eb3 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1929,6 +1929,9 @@ struct xhci_hcd {
 	struct list_head	regset_list;
 
 	void			*dbc;
+
+	struct xhci_vendor_ops *vendor_ops;
+
 	/* platform-specific data -- must come last */
 	unsigned long		priv[] __aligned(sizeof(s64));
 };
@@ -2207,6 +2210,49 @@ static inline struct xhci_ring *xhci_urb_to_transfer_ring(struct xhci_hcd *xhci,
 					urb->stream_id);
 }
 
+/**
+ * struct xhci_vendor_ops - function callbacks for vendor specific operations
+ * @vendor_init: called for vendor init process
+ * @vendor_cleanup: called for vendor cleanup process
+ * @is_usb_offload_enabled: called to check if usb offload enabled
+ * @alloc_dcbaa: called when allocating vendor specific dcbaa
+ * @free_dcbaa: called to free vendor specific dcbaa
+ * @alloc_transfer_ring: called when remote transfer ring allocation is required
+ * @free_transfer_ring: called to free vendor specific transfer ring
+ * @sync_dev_ctx: called when synchronization for device context is required
+ * @alloc_container_ctx: called when allocating vendor specific container context
+ * @free_container_ctx: called to free vendor specific container context
+ */
+struct xhci_vendor_ops {
+	int (*vendor_init)(struct xhci_hcd *xhci);
+	void (*vendor_cleanup)(struct xhci_hcd *xhci);
+	bool (*is_usb_offload_enabled)(struct xhci_hcd *xhci,
+				       struct xhci_virt_device *vdev,
+				       unsigned int ep_index);
+
+	struct xhci_device_context_array *(*alloc_dcbaa)(struct xhci_hcd *xhci,
+							 gfp_t flags);
+	void (*free_dcbaa)(struct xhci_hcd *xhci);
+
+	struct xhci_ring *(*alloc_transfer_ring)(struct xhci_hcd *xhci,
+			u32 endpoint_type, enum xhci_ring_type ring_type,
+			unsigned int max_packet, gfp_t mem_flags);
+	void (*free_transfer_ring)(struct xhci_hcd *xhci,
+			struct xhci_ring *ring, unsigned int ep_index);
+	int (*sync_dev_ctx)(struct xhci_hcd *xhci, unsigned int slot_id);
+	void (*alloc_container_ctx)(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
+				    int type, gfp_t flags);
+	void (*free_container_ctx)(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx);
+};
+
+struct xhci_vendor_ops *xhci_vendor_get_ops(struct xhci_hcd *xhci);
+
+int xhci_vendor_sync_dev_ctx(struct xhci_hcd *xhci, unsigned int slot_id);
+void xhci_vendor_free_transfer_ring(struct xhci_hcd *xhci,
+		struct xhci_ring *ring, unsigned int ep_index);
+bool xhci_vendor_is_usb_offload_enabled(struct xhci_hcd *xhci,
+		struct xhci_virt_device *virt_dev, unsigned int ep_index);
+
 /*
  * TODO: As per spec Isochronous IDT transmissions are supported. We bypass
  * them anyways as we where unable to find a device that matches the
-- 
2.31.1

Re: [PATCH v3 2/4] usb: host: add xhci hooks for USB offload

[Mathias Nyman]

On 21.3.2022 10.59, Daehwan Jung wrote:
> To enable supporting for USB offload, define "offload" in usb controller
> node of device tree. "offload" value can be used to determine which type
> of offload was been enabled in the SoC.
> 
> For example:
> 
> &usbdrd_dwc3 {
> 	...
> 	/* support usb offloading, 0: disabled, 1: audio */
> 	offload = <1>;
> 	...
> };
> 
> There are several vendor_ops introduced by this patch:
> 
> struct xhci_vendor_ops - function callbacks for vendor specific operations
> {
> 	@vendor_init:
> 		- called for vendor init process during xhci-plat-hcd
> 		  probe.
> 	@vendor_cleanup:
> 		- called for vendor cleanup process during xhci-plat-hcd
> 		  remove.

The vendor_init() and vendor_cleanup() aren't really useful.
you are calling them from platform_probe() and platform_remove(),
just modify the probe and remove functions of the xhci-exynos driver directly.


> 	@is_usb_offload_enabled:
> 		- called to check if usb offload enabled.

Looks like this is being used more like a quirk bit.
I think we can get rid of this as well

> 	@alloc_dcbaa:
> 		- called when allocating vendor specific dcbaa during
> 		  memory initializtion.
> 	@free_dcbaa:
> 		- called to free vendor specific dcbaa when cleanup the
> 		  memory.
> 	@alloc_transfer_ring:
> 		- called when vendor specific transfer ring allocation is required
> 	@free_transfer_ring:
> 		- called to free vendor specific transfer ring
> 	@sync_dev_ctx:
> 		- called when synchronization for device context is required
> }
> 
> The xhci hooks with prefix "xhci_vendor_" on the ops in xhci_vendor_ops.
> For example, vendor_init ops will be invoked by xhci_vendor_init() hook,
> is_usb_offload_enabled ops will be invoked by
> xhci_vendor_is_usb_offload_enabled(), and so on.
> 
> Signed-off-by: Daehwan Jung <dh10.jung@samsung.com>
> Signed-off-by: J. Avila <elavila@google.com>
> Signed-off-by: Puma Hsu <pumahsu@google.com>
> Signed-off-by: Howard Yen <howardyen@google.com>
> ---
>  drivers/usb/host/xhci-hub.c  |   5 ++
>  drivers/usb/host/xhci-mem.c  | 131 +++++++++++++++++++++++++++++++----
>  drivers/usb/host/xhci-plat.c |  43 +++++++++++-
>  drivers/usb/host/xhci-plat.h |   7 ++
>  drivers/usb/host/xhci.c      |  80 ++++++++++++++++++++-
>  drivers/usb/host/xhci.h      |  46 ++++++++++++
>  6 files changed, 295 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index 56546aaa93c7..aea72ffce820 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -535,8 +535,13 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
>  	    cmd->status == COMP_COMMAND_RING_STOPPED) {
>  		xhci_warn(xhci, "Timeout while waiting for stop endpoint command\n");
>  		ret = -ETIME;
> +		goto cmd_cleanup;
>  	}
>  
> +	ret = xhci_vendor_sync_dev_ctx(xhci, slot_id);
> +	if (ret)
> +		xhci_warn(xhci, "Sync device context failed, ret=%d\n", ret);
> +
>  cmd_cleanup:
>  	xhci_free_command(xhci, cmd);
>  	return ret;
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 82b9f90c0f27..5ee0ffb676d3 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -365,6 +365,54 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci,
>  	return 0;
>  }
>  
> +static void xhci_vendor_free_container_ctx(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx)
> +{
> +	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
> +
> +	if (ops && ops->free_container_ctx)
> +		ops->free_container_ctx(xhci, ctx);
> +}


Looks suspicious, we always need to free container contexts, why this much checking?


> +
> +static void xhci_vendor_alloc_container_ctx(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
> +					    int type, gfp_t flags)
> +{
> +	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
> +
> +	if (ops && ops->alloc_container_ctx)
> +		ops->alloc_container_ctx(xhci, ctx, type, flags);
> +}

same, there should always be a function to allocate container context..

> +
> +static struct xhci_ring *xhci_vendor_alloc_transfer_ring(struct xhci_hcd *xhci,
> +		u32 endpoint_type, enum xhci_ring_type ring_type,
> +		unsigned int max_packet, gfp_t mem_flags)
> +{
> +	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
> +
> +	if (ops && ops->alloc_transfer_ring)
> +		return ops->alloc_transfer_ring(xhci, endpoint_type, ring_type,
> +				max_packet, mem_flags);
> +	return 0;

same, looks like a lot of extra code.

> +}
> +
> +void xhci_vendor_free_transfer_ring(struct xhci_hcd *xhci,
> +		struct xhci_ring *ring, unsigned int ep_index)
> +{
> +	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
> +
> +	if (ops && ops->free_transfer_ring)
> +		ops->free_transfer_ring(xhci, ring, ep_index);
> +}
> +

same.

> +bool xhci_vendor_is_usb_offload_enabled(struct xhci_hcd *xhci,
> +		struct xhci_virt_device *virt_dev, unsigned int ep_index)
> +{
> +	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
> +
> +	if (ops && ops->is_usb_offload_enabled)
> +		return ops->is_usb_offload_enabled(xhci, virt_dev, ep_index);
> +	return false;
> +}
> +
>  /*
>   * Create a new ring with zero or more segments.
>   *
> @@ -417,7 +465,11 @@ void xhci_free_endpoint_ring(struct xhci_hcd *xhci,
>  		struct xhci_virt_device *virt_dev,
>  		unsigned int ep_index)
>  {
> -	xhci_ring_free(xhci, virt_dev->eps[ep_index].ring);
> +	if (xhci_vendor_is_usb_offload_enabled(xhci, virt_dev, ep_index))
> +		xhci_vendor_free_transfer_ring(xhci, virt_dev->eps[ep_index].ring, ep_index);
> +	else
> +		xhci_ring_free(xhci, virt_dev->eps[ep_index].ring);
> +
>  	virt_dev->eps[ep_index].ring = NULL;
>  }

Ok, I see.
So idea is to override some functions that allocate and free DMA memory.
Your vendor_ops structure filled with function callbacks could be a 
mem_ops structure that allows your driver to directly override those
functions.

For example here we would only call

xhci->mem_ops->ring_free(...);
This would set to xhci_ring_free() by default, but your xhci-exonys driver could
set it to xhci_exonys_free_ring(), which would do any needed is_offload_enabled()
checks and custom freeing.

Same goes for most most of the other functions in this patch

If possible see if it's enough to override the existing callbacks in
struct hc_driver instead of creating new function pointers.

-Mathias

Re: [PATCH v3 2/4] usb: host: add xhci hooks for USB offload

[Jung Daehwan]

[-- Attachment #1: Type: text/plain, Size: 7413 bytes --]

On Mon, Mar 21, 2022 at 07:00:55PM +0200, Mathias Nyman wrote:
> On 21.3.2022 10.59, Daehwan Jung wrote:
> > To enable supporting for USB offload, define "offload" in usb controller
> > node of device tree. "offload" value can be used to determine which type
> > of offload was been enabled in the SoC.
> > 
> > For example:
> > 
> > &usbdrd_dwc3 {
> > 	...
> > 	/* support usb offloading, 0: disabled, 1: audio */
> > 	offload = <1>;
> > 	...
> > };
> > 
> > There are several vendor_ops introduced by this patch:
> > 
> > struct xhci_vendor_ops - function callbacks for vendor specific operations
> > {
> > 	@vendor_init:
> > 		- called for vendor init process during xhci-plat-hcd
> > 		  probe.
> > 	@vendor_cleanup:
> > 		- called for vendor cleanup process during xhci-plat-hcd
> > 		  remove.
> 
> The vendor_init() and vendor_cleanup() aren't really useful.
> you are calling them from platform_probe() and platform_remove(),
> just modify the probe and remove functions of the xhci-exynos driver directly.
> 
>

OK. I agree with you and I'm going to modify it on next submission.

> > 	@is_usb_offload_enabled:
> > 		- called to check if usb offload enabled.
> 
> Looks like this is being used more like a quirk bit.
> I think we can get rid of this as well
> 

Yes. I don't need it if I could get a quirk bit.

> > 	@alloc_dcbaa:
> > 		- called when allocating vendor specific dcbaa during
> > 		  memory initializtion.
> > 	@free_dcbaa:
> > 		- called to free vendor specific dcbaa when cleanup the
> > 		  memory.
> > 	@alloc_transfer_ring:
> > 		- called when vendor specific transfer ring allocation is required
> > 	@free_transfer_ring:
> > 		- called to free vendor specific transfer ring
> > 	@sync_dev_ctx:
> > 		- called when synchronization for device context is required
> > }
> > 
> > The xhci hooks with prefix "xhci_vendor_" on the ops in xhci_vendor_ops.
> > For example, vendor_init ops will be invoked by xhci_vendor_init() hook,
> > is_usb_offload_enabled ops will be invoked by
> > xhci_vendor_is_usb_offload_enabled(), and so on.
> > 
> > Signed-off-by: Daehwan Jung <dh10.jung@samsung.com>
> > Signed-off-by: J. Avila <elavila@google.com>
> > Signed-off-by: Puma Hsu <pumahsu@google.com>
> > Signed-off-by: Howard Yen <howardyen@google.com>
> > ---
> >  drivers/usb/host/xhci-hub.c  |   5 ++
> >  drivers/usb/host/xhci-mem.c  | 131 +++++++++++++++++++++++++++++++----
> >  drivers/usb/host/xhci-plat.c |  43 +++++++++++-
> >  drivers/usb/host/xhci-plat.h |   7 ++
> >  drivers/usb/host/xhci.c      |  80 ++++++++++++++++++++-
> >  drivers/usb/host/xhci.h      |  46 ++++++++++++
> >  6 files changed, 295 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> > index 56546aaa93c7..aea72ffce820 100644
> > --- a/drivers/usb/host/xhci-hub.c
> > +++ b/drivers/usb/host/xhci-hub.c
> > @@ -535,8 +535,13 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
> >  	    cmd->status == COMP_COMMAND_RING_STOPPED) {
> >  		xhci_warn(xhci, "Timeout while waiting for stop endpoint command\n");
> >  		ret = -ETIME;
> > +		goto cmd_cleanup;
> >  	}
> >  
> > +	ret = xhci_vendor_sync_dev_ctx(xhci, slot_id);
> > +	if (ret)
> > +		xhci_warn(xhci, "Sync device context failed, ret=%d\n", ret);
> > +
> >  cmd_cleanup:
> >  	xhci_free_command(xhci, cmd);
> >  	return ret;
> > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> > index 82b9f90c0f27..5ee0ffb676d3 100644
> > --- a/drivers/usb/host/xhci-mem.c
> > +++ b/drivers/usb/host/xhci-mem.c
> > @@ -365,6 +365,54 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd *xhci,
> >  	return 0;
> >  }
> >  
> > +static void xhci_vendor_free_container_ctx(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx)
> > +{
> > +	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
> > +
> > +	if (ops && ops->free_container_ctx)
> > +		ops->free_container_ctx(xhci, ctx);
> > +}
> 
> 
> Looks suspicious, we always need to free container contexts, why this much checking?
> 
> 

Calling funcion could cause a problem if some functions haven't been mapped
on ops yet. Others below are same.

> > +
> > +static void xhci_vendor_alloc_container_ctx(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
> > +					    int type, gfp_t flags)
> > +{
> > +	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
> > +
> > +	if (ops && ops->alloc_container_ctx)
> > +		ops->alloc_container_ctx(xhci, ctx, type, flags);
> > +}
> 
> same, there should always be a function to allocate container context..
> 
> > +
> > +static struct xhci_ring *xhci_vendor_alloc_transfer_ring(struct xhci_hcd *xhci,
> > +		u32 endpoint_type, enum xhci_ring_type ring_type,
> > +		unsigned int max_packet, gfp_t mem_flags)
> > +{
> > +	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
> > +
> > +	if (ops && ops->alloc_transfer_ring)
> > +		return ops->alloc_transfer_ring(xhci, endpoint_type, ring_type,
> > +				max_packet, mem_flags);
> > +	return 0;
> 
> same, looks like a lot of extra code.
> 
> > +}
> > +
> > +void xhci_vendor_free_transfer_ring(struct xhci_hcd *xhci,
> > +		struct xhci_ring *ring, unsigned int ep_index)
> > +{
> > +	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
> > +
> > +	if (ops && ops->free_transfer_ring)
> > +		ops->free_transfer_ring(xhci, ring, ep_index);
> > +}
> > +
> 
> same.
> 
> > +bool xhci_vendor_is_usb_offload_enabled(struct xhci_hcd *xhci,
> > +		struct xhci_virt_device *virt_dev, unsigned int ep_index)
> > +{
> > +	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
> > +
> > +	if (ops && ops->is_usb_offload_enabled)
> > +		return ops->is_usb_offload_enabled(xhci, virt_dev, ep_index);
> > +	return false;
> > +}
> > +
> >  /*
> >   * Create a new ring with zero or more segments.
> >   *
> > @@ -417,7 +465,11 @@ void xhci_free_endpoint_ring(struct xhci_hcd *xhci,
> >  		struct xhci_virt_device *virt_dev,
> >  		unsigned int ep_index)
> >  {
> > -	xhci_ring_free(xhci, virt_dev->eps[ep_index].ring);
> > +	if (xhci_vendor_is_usb_offload_enabled(xhci, virt_dev, ep_index))
> > +		xhci_vendor_free_transfer_ring(xhci, virt_dev->eps[ep_index].ring, ep_index);
> > +	else
> > +		xhci_ring_free(xhci, virt_dev->eps[ep_index].ring);
> > +
> >  	virt_dev->eps[ep_index].ring = NULL;
> >  }
> 
> Ok, I see.
> So idea is to override some functions that allocate and free DMA memory.
> Your vendor_ops structure filled with function callbacks could be a 
> mem_ops structure that allows your driver to directly override those
> functions.
> 
> For example here we would only call
> 
> xhci->mem_ops->ring_free(...);
> This would set to xhci_ring_free() by default, but your xhci-exonys driver could
> set it to xhci_exonys_free_ring(), which would do any needed is_offload_enabled()
> checks and custom freeing.
> 
> Same goes for most most of the other functions in this patch
> 

Exactly. I'm appreciate for your understanding my patch.

> If possible see if it's enough to override the existing callbacks in
> struct hc_driver instead of creating new function pointers.
> 
> -Mathias
>

I've used override for reset, start, add_endpoint, address_device, bus_suspend,
and bus_resume in hc_driver. I could use reset_bandwidth more but I don't
think override is better in this case because allocation / free of ep ring are
always done with xhci hooks on others.

Best Regards,
Jung Daehwan

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH v3 3/4] usb: host: add some to xhci overrides for USB offload

[Daehwan Jung]

Co-processor needs some information about connected usb device.
It's proper to pass information after usb device gets address when
getting "Set Address" command. It supports vendors to implement it
using xhci overrides. There're several power scenarios depending
on vendors. It gives vendors flexibilty to meet their power requirement.
They can override suspend and resume of root hub.

Signed-off-by: Daehwan Jung <dh10.jung@samsung.com>
---
 drivers/usb/host/xhci.c | 6 ++++++
 drivers/usb/host/xhci.h | 4 ++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 4e4fcf97ba42..d3c3d57c89f9 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -5544,6 +5544,12 @@ void xhci_init_driver(struct hc_driver *drv,
 			drv->check_bandwidth = over->check_bandwidth;
 		if (over->reset_bandwidth)
 			drv->reset_bandwidth = over->reset_bandwidth;
+		if (over->address_device)
+			drv->address_device = over->address_device;
+		if (over->bus_suspend)
+			drv->bus_suspend = over->bus_suspend;
+		if (over->bus_resume)
+			drv->bus_resume = over->bus_resume;
 	}
 }
 EXPORT_SYMBOL_GPL(xhci_init_driver);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index f690a5dc9eb3..de37d72847f2 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1947,6 +1947,9 @@ struct xhci_driver_overrides {
 			     struct usb_host_endpoint *ep);
 	int (*check_bandwidth)(struct usb_hcd *, struct usb_device *);
 	void (*reset_bandwidth)(struct usb_hcd *, struct usb_device *);
+	int (*address_device)(struct usb_hcd *hcd, struct usb_device *udev);
+	int (*bus_suspend)(struct usb_hcd *hcd);
+	int (*bus_resume)(struct usb_hcd *hcd);
 };
 
 #define	XHCI_CFC_DELAY		10
@@ -2103,6 +2106,7 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
 		       struct usb_host_endpoint *ep);
 int xhci_check_bandwidth(struct usb_hcd *hcd, struct usb_device *udev);
 void xhci_reset_bandwidth(struct usb_hcd *hcd, struct usb_device *udev);
+int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev);
 int xhci_disable_slot(struct xhci_hcd *xhci, u32 slot_id);
 int xhci_ext_cap_init(struct xhci_hcd *xhci);
 
-- 
2.31.1

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

[Daehwan Jung]

This driver supports USB Audio offload with Co-processor.
It only cares DCBAA, Device Context, Transfer Ring, Event Ring, and ERST.
They are allocated on specific address with xhci hooks.
Co-processor could use them directly without xhci driver after then.

Signed-off-by: Daehwan Jung <dh10.jung@samsung.com>
---
 drivers/usb/host/Kconfig       |   9 +
 drivers/usb/host/Makefile      |   1 +
 drivers/usb/host/xhci-exynos.c | 982 +++++++++++++++++++++++++++++++++
 drivers/usb/host/xhci-exynos.h |  63 +++
 4 files changed, 1055 insertions(+)
 create mode 100644 drivers/usb/host/xhci-exynos.c
 create mode 100644 drivers/usb/host/xhci-exynos.h

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 57ca5f97a3dc..850e6b71fac5 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -783,3 +783,12 @@ config USB_XEN_HCD
 	  by the Xen host (usually Dom0).
 	  Only needed if the kernel is running in a Xen guest and generic
 	  access to a USB device is needed.
+
+config USB_XHCI_EXYNOS
+	tristate "XHCI support for Samsung Exynos SoC Series"
+	depends on ARCH_EXYNOS || COMPILE_TEST
+	help
+	  Enable support for the Samsung Exynos SOC's on-chip XHCI
+	  controller.
+
+	  If unsure, say N.
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 2948983618fb..300f22b6eb1b 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -86,3 +86,4 @@ obj-$(CONFIG_USB_HCD_SSB)	+= ssb-hcd.o
 obj-$(CONFIG_USB_FOTG210_HCD)	+= fotg210-hcd.o
 obj-$(CONFIG_USB_MAX3421_HCD)	+= max3421-hcd.o
 obj-$(CONFIG_USB_XEN_HCD)	+= xen-hcd.o
+obj-$(CONFIG_USB_XHCI_EXYNOS)	+= xhci-exynos.o
diff --git a/drivers/usb/host/xhci-exynos.c b/drivers/usb/host/xhci-exynos.c
new file mode 100644
index 000000000000..19ee21f1d024
--- /dev/null
+++ b/drivers/usb/host/xhci-exynos.c
@@ -0,0 +1,982 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * xhci-exynos.c - xHCI host controller driver platform Bus Glue for Exynos.
+ *
+ * Copyright (C) 2022 Samsung Electronics Incorporated - http://www.samsung.com
+ * Author: Daehwan Jung <dh10.jung@samsung.com>
+ *
+ * A lot of code borrowed from the Linux xHCI driver.
+ */
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/usb/phy.h>
+#include <linux/slab.h>
+#include <linux/acpi.h>
+#include <linux/usb/of.h>
+
+#include "xhci.h"
+#include "xhci-plat.h"
+#include "xhci-mvebu.h"
+#include "xhci-rcar.h"
+
+#include "xhci-exynos.h"
+
+static struct hc_driver __read_mostly xhci_exynos_hc_driver;
+
+static void xhci_exynos_free_event_ring(struct xhci_hcd *xhci);
+static struct xhci_ring *xhci_ring_alloc_uram(struct xhci_hcd *xhci,
+		unsigned int num_segs, unsigned int cycle_state,
+		enum xhci_ring_type type, unsigned int max_packet, gfp_t flags,
+		u32 endpoint_type);
+static void xhci_exynos_ring_free(struct xhci_hcd *xhci, struct xhci_ring *ring);
+
+static struct xhci_plat_priv_overwrite xhci_plat_vendor_overwrite;
+
+static int xhci_exynos_setup(struct usb_hcd *hcd);
+static int xhci_exynos_start(struct usb_hcd *hcd);
+
+static void xhci_priv_exynos_start(struct usb_hcd *hcd)
+{
+	struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
+
+	if (priv->plat_start)
+		priv->plat_start(hcd);
+}
+
+static int xhci_priv_exynos_setup(struct usb_hcd *hcd)
+{
+	struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
+
+	if (!priv->plat_setup)
+		return 0;
+
+	return priv->plat_setup(hcd);
+}
+
+
+static int xhci_exynos_register_vendor_ops(struct xhci_vendor_ops *vendor_ops)
+{
+	if (vendor_ops == NULL)
+		return -EINVAL;
+
+	xhci_plat_vendor_overwrite.vendor_ops = vendor_ops;
+
+	return 0;
+}
+
+static int xhci_exynos_vendor_init(struct xhci_hcd *xhci)
+{
+	/* TODO */
+	return 0;
+}
+
+static void xhci_exynos_vendor_cleanup(struct xhci_hcd *xhci)
+{
+	xhci_exynos_free_event_ring(xhci);
+}
+
+static bool xhci_exynos_is_usb_offload_enabled(struct xhci_hcd *xhci,
+		struct xhci_virt_device *virt_dev, unsigned int ep_index)
+{
+	/* TODO */
+	return true;
+}
+
+static struct xhci_device_context_array *xhci_exynos_alloc_dcbaa(
+		struct xhci_hcd *xhci, gfp_t flags)
+{
+	int i;
+
+	xhci->dcbaa = ioremap(EXYNOS_URAM_DCBAA_ADDR,
+					sizeof(*xhci->dcbaa));
+	if (!xhci->dcbaa)
+		return NULL;
+	/* Clear DCBAA */
+	for (i = 0; i < MAX_HC_SLOTS; i++)
+		xhci->dcbaa->dev_context_ptrs[i] = 0x0;
+
+	xhci->dcbaa->dma = EXYNOS_URAM_DCBAA_ADDR;
+
+	return xhci->dcbaa;
+}
+
+static void xhci_exynos_free_dcbaa(struct xhci_hcd *xhci)
+{
+	iounmap(xhci->dcbaa);
+	xhci->dcbaa = NULL;
+}
+
+static struct xhci_ring *xhci_exynos_alloc_transfer_ring(struct xhci_hcd *xhci, u32 endpoint_type,
+			enum xhci_ring_type ring_type, unsigned int max_packet, gfp_t mem_flags)
+{
+	return xhci_ring_alloc_uram(xhci, 1, 1, ring_type, max_packet, mem_flags, endpoint_type);
+}
+
+static void xhci_exynos_free_transfer_ring(struct xhci_hcd *xhci, struct xhci_ring *ring,
+						unsigned int ep_index)
+{
+	xhci_exynos_ring_free(xhci, ring);
+}
+
+static void xhci_exynos_alloc_container_ctx(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
+						int type, gfp_t flags)
+{
+	/* Only first Device Context uses URAM */
+	int i;
+
+	ctx->bytes = ioremap(EXYNOS_URAM_DEVICE_CTX_ADDR, EXYNOS_URAM_CTX_SIZE);
+	if (!ctx->bytes)
+		return;
+
+	for (i = 0; i < EXYNOS_URAM_CTX_SIZE; i++)
+		ctx->bytes[i] = 0;
+
+	ctx->dma = EXYNOS_URAM_DEVICE_CTX_ADDR;
+}
+
+static void xhci_exynos_free_container_ctx(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx)
+{
+	/* Ignore dma_pool_free if it is allocated from URAM */
+	if (ctx->dma != EXYNOS_URAM_DEVICE_CTX_ADDR)
+		dma_pool_free(xhci->device_pool, ctx->bytes, ctx->dma);
+}
+
+static int xhci_exynos_sync_dev_ctx(struct xhci_hcd *xhci, unsigned int slot_id)
+{
+	struct xhci_exynos_priv *priv = xhci_to_exynos_priv(xhci);
+	struct xhci_hcd_exynos *xhci_exynos = priv->xhci_exynos;
+	struct xhci_virt_device *virt_dev;
+	struct xhci_slot_ctx *slot_ctx;
+
+	int i;
+	int last_ep;
+	int last_ep_ctx = 31;
+
+	virt_dev = xhci->devs[slot_id];
+	slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->in_ctx);
+
+	last_ep = LAST_CTX_TO_EP_NUM(le32_to_cpu(slot_ctx->dev_info));
+
+	if (last_ep < 31)
+		last_ep_ctx = last_ep + 1;
+
+	for (i = 0; i < last_ep_ctx; ++i) {
+		unsigned int epaddr = xhci_get_endpoint_address(i);
+		struct xhci_ep_ctx *ep_ctx = xhci_get_ep_ctx(xhci, virt_dev->out_ctx, i);
+
+		if (epaddr == xhci_exynos->in_ep)
+			xhci_exynos->in_deq = ep_ctx->deq;
+		else if (epaddr == xhci_exynos->out_ep)
+			xhci_exynos->out_deq = ep_ctx->deq;
+	}
+
+	return 0;
+}
+static struct xhci_vendor_ops ops = {
+	.vendor_init = xhci_exynos_vendor_init,
+	.vendor_cleanup = xhci_exynos_vendor_cleanup,
+	.is_usb_offload_enabled = xhci_exynos_is_usb_offload_enabled,
+	.alloc_dcbaa = xhci_exynos_alloc_dcbaa,
+	.free_dcbaa = xhci_exynos_free_dcbaa,
+	.alloc_transfer_ring = xhci_exynos_alloc_transfer_ring,
+	.free_transfer_ring = xhci_exynos_free_transfer_ring,
+	.alloc_container_ctx = xhci_exynos_alloc_container_ctx,
+	.free_container_ctx = xhci_exynos_free_container_ctx,
+	.sync_dev_ctx = xhci_exynos_sync_dev_ctx,
+};
+
+static int xhci_exynos_wake_lock(struct xhci_hcd_exynos *xhci_exynos,
+				   int is_main_hcd, int is_lock)
+{
+	struct usb_hcd	*hcd = xhci_exynos->hcd;
+	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+	struct wakeup_source *main_wakelock, *shared_wakelock;
+
+	main_wakelock = xhci_exynos->main_wakelock;
+	shared_wakelock = xhci_exynos->shared_wakelock;
+
+	if (xhci->xhc_state & XHCI_STATE_REMOVING)
+		return -ESHUTDOWN;
+
+	if (is_lock) {
+		if (is_main_hcd)
+			__pm_stay_awake(main_wakelock);
+		else
+			__pm_stay_awake(shared_wakelock);
+	} else {
+		if (is_main_hcd)
+			__pm_relax(main_wakelock);
+		else
+			__pm_relax(shared_wakelock);
+	}
+
+	return 0;
+}
+
+static int xhci_exynos_bus_suspend(struct usb_hcd *hcd)
+{
+	struct xhci_exynos_priv *priv = hcd_to_xhci_exynos_priv(hcd);
+	struct xhci_hcd_exynos *xhci_exynos = priv->xhci_exynos;
+	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+
+	int ret, main_hcd;
+
+	if (hcd == xhci->main_hcd)
+		main_hcd = 1;
+	else
+		main_hcd = 0;
+
+	ret = xhci_bus_suspend(hcd);
+	xhci_exynos_wake_lock(xhci_exynos, main_hcd, 0);
+
+	return ret;
+}
+
+static int xhci_exynos_bus_resume(struct usb_hcd *hcd)
+{
+	struct xhci_exynos_priv *priv = hcd_to_xhci_exynos_priv(hcd);
+	struct xhci_hcd_exynos *xhci_exynos = priv->xhci_exynos;
+	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+	int ret, main_hcd;
+
+	if (hcd == xhci->main_hcd)
+		main_hcd = 1;
+	else
+		main_hcd = 0;
+
+	ret = xhci_bus_resume(hcd);
+	xhci_exynos_wake_lock(xhci_exynos, main_hcd, 1);
+
+	return ret;
+}
+
+static int xhci_exynos_address_device(struct usb_hcd *hcd, struct usb_device *udev)
+{
+	struct xhci_hcd *xhci;
+	int ret;
+
+	ret = xhci_address_device(hcd, udev);
+	xhci = hcd_to_xhci(hcd);
+
+	return ret;
+}
+
+static void xhci_exynos_parse_endpoint(struct xhci_hcd *xhci, struct usb_device *udev,
+		struct usb_endpoint_descriptor *desc, struct xhci_container_ctx *ctx)
+{
+	struct xhci_exynos_priv *priv = xhci_to_exynos_priv(xhci);
+	struct xhci_hcd_exynos *xhci_exynos = priv->xhci_exynos;
+	struct usb_endpoint_descriptor *d = desc;
+	unsigned int ep_index;
+	struct xhci_ep_ctx *ep_ctx;
+
+	ep_index = xhci_get_endpoint_index(d);
+	ep_ctx = xhci_get_ep_ctx(xhci, ctx, ep_index);
+
+	if ((d->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) ==
+				USB_ENDPOINT_XFER_ISOC) {
+		if (d->bEndpointAddress & USB_ENDPOINT_DIR_MASK)
+			xhci_exynos->in_ep = d->bEndpointAddress;
+		else
+			xhci_exynos->out_ep = d->bEndpointAddress;
+	}
+}
+
+static int xhci_exynos_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
+		struct usb_host_endpoint *ep)
+{
+	int ret;
+	struct xhci_hcd *xhci;
+	struct xhci_virt_device *virt_dev;
+
+	ret = xhci_add_endpoint(hcd, udev, ep);
+
+	if (!ret && udev->slot_id) {
+		xhci = hcd_to_xhci(hcd);
+		virt_dev = xhci->devs[udev->slot_id];
+		xhci_exynos_parse_endpoint(xhci, udev, &ep->desc, virt_dev->out_ctx);
+	}
+
+	return ret;
+}
+static const struct xhci_driver_overrides xhci_exynos_overrides __initconst = {
+	.extra_priv_size = sizeof(struct xhci_plat_priv),
+	.reset = xhci_exynos_setup,
+	.start = xhci_exynos_start,
+	.add_endpoint = xhci_exynos_add_endpoint,
+	.address_device = xhci_exynos_address_device,
+	.bus_suspend = xhci_exynos_bus_suspend,
+	.bus_resume = xhci_exynos_bus_resume,
+};
+
+
+static void xhci_exynos_segment_free_skip(struct xhci_hcd *xhci, struct xhci_segment *seg)
+{
+	kfree(seg->bounce_buf);
+	kfree(seg);
+}
+
+static void xhci_exynos_free_segments_for_ring(struct xhci_hcd *xhci,
+				struct xhci_segment *first)
+{
+	struct xhci_segment *seg;
+
+	seg = first->next;
+
+	while (seg != first) {
+		struct xhci_segment *next = seg->next;
+
+		xhci_exynos_segment_free_skip(xhci, seg);
+		seg = next;
+	}
+	xhci_exynos_segment_free_skip(xhci, first);
+}
+
+static void xhci_exynos_ring_free(struct xhci_hcd *xhci, struct xhci_ring *ring)
+{
+	if (!ring)
+		return;
+
+	//trace_xhci_ring_free(ring);
+
+	if (ring->first_seg) {
+		if (ring->type == TYPE_STREAM)
+			xhci_remove_stream_mapping(ring);
+
+		xhci_exynos_free_segments_for_ring(xhci, ring->first_seg);
+	}
+
+	kfree(ring);
+}
+
+static void xhci_exynos_free_event_ring(struct xhci_hcd *xhci)
+{
+	struct xhci_exynos_priv *priv = xhci_to_exynos_priv(xhci);
+	struct xhci_hcd_exynos *xhci_exynos = priv->xhci_exynos;
+
+	xhci_exynos_ring_free(xhci, xhci_exynos->event_ring_audio);
+}
+
+static void xhci_exynos_set_hc_event_deq_audio(struct xhci_hcd *xhci)
+{
+	struct xhci_exynos_priv *priv = xhci_to_exynos_priv(xhci);
+	struct xhci_hcd_exynos *xhci_exynos = priv->xhci_exynos;
+
+	dma_addr_t deq;
+
+	deq = xhci_trb_virt_to_dma(xhci_exynos->event_ring_audio->deq_seg,
+			xhci_exynos->event_ring_audio->dequeue);
+}
+
+
+
+static int xhci_exynos_alloc_event_ring(struct xhci_hcd *xhci, gfp_t flags)
+{
+	struct xhci_exynos_priv *priv = xhci_to_exynos_priv(xhci);
+	struct xhci_hcd_exynos *xhci_exynos = priv->xhci_exynos;
+
+	if (xhci_check_trb_in_td_math(xhci) < 0)
+		goto fail;
+
+	xhci_exynos->event_ring_audio = xhci_ring_alloc(xhci, ERST_NUM_SEGS, 1,
+						TYPE_EVENT, 0, flags);
+	/* Set the event ring dequeue address */
+	xhci_exynos_set_hc_event_deq_audio(xhci);
+
+	return 0;
+fail:
+	return -1;
+}
+
+static int xhci_alloc_segments_for_ring_uram(struct xhci_hcd *xhci,
+		struct xhci_segment **first, struct xhci_segment **last,
+		unsigned int num_segs, unsigned int cycle_state,
+		enum xhci_ring_type type, unsigned int max_packet, gfp_t flags,
+		u32 endpoint_type)
+{
+	struct xhci_segment *prev;
+	bool chain_links = false;
+
+	while (num_segs > 0) {
+		struct xhci_segment *next = NULL;
+
+		if (!next) {
+			prev = *first;
+			while (prev) {
+				next = prev->next;
+				xhci_segment_free(xhci, prev);
+				prev = next;
+			}
+			return -ENOMEM;
+		}
+		xhci_link_segments(prev, next, type, chain_links);
+
+		prev = next;
+		num_segs--;
+	}
+	xhci_link_segments(prev, *first, type, chain_links);
+	*last = prev;
+
+	return 0;
+}
+
+static struct xhci_ring *xhci_ring_alloc_uram(struct xhci_hcd *xhci,
+		unsigned int num_segs, unsigned int cycle_state,
+		enum xhci_ring_type type, unsigned int max_packet, gfp_t flags,
+		u32 endpoint_type)
+{
+	struct xhci_ring	*ring;
+	int ret;
+	struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
+
+	ring = kzalloc_node(sizeof(*ring), flags, dev_to_node(dev));
+	if (!ring)
+		return NULL;
+
+	ring->num_segs = num_segs;
+	ring->bounce_buf_len = max_packet;
+	INIT_LIST_HEAD(&ring->td_list);
+	ring->type = type;
+	if (num_segs == 0)
+		return ring;
+
+	ret = xhci_alloc_segments_for_ring_uram(xhci, &ring->first_seg,
+			&ring->last_seg, num_segs, cycle_state, type,
+			max_packet, flags, endpoint_type);
+	if (ret)
+		goto fail;
+
+	/* Only event ring does not use link TRB */
+	if (type != TYPE_EVENT) {
+		/* See section 4.9.2.1 and 6.4.4.1 */
+		ring->last_seg->trbs[TRBS_PER_SEGMENT - 1].link.control |=
+			cpu_to_le32(LINK_TOGGLE);
+	}
+	xhci_initialize_ring_info(ring, cycle_state);
+	//trace_xhci_ring_alloc(ring);
+	return ring;
+
+fail:
+	kfree(ring);
+	return NULL;
+}
+
+static void xhci_exynos_usb_offload_enable_event_ring(struct xhci_hcd *xhci)
+{
+	struct xhci_exynos_priv *priv = xhci_to_exynos_priv(xhci);
+	struct xhci_hcd_exynos *xhci_exynos = priv->xhci_exynos;
+
+	u32 temp;
+	u64 temp_64;
+
+	temp_64 = xhci_read_64(xhci, &xhci_exynos->ir_set_audio->erst_dequeue);
+	temp_64 &= ~ERST_PTR_MASK;
+	xhci_info(xhci,	"ERST2 deq = 64'h%0lx", (unsigned long) temp_64);
+
+	xhci_info(xhci, "// [USB Audio] Set the interrupt modulation register");
+	temp = readl(&xhci_exynos->ir_set_audio->irq_control);
+	temp &= ~ER_IRQ_INTERVAL_MASK;
+	/*
+	 * the increment interval is 8 times as much as that defined
+	 * in xHCI spec on MTK's controller
+	 */
+	temp |= (u32) ((xhci->quirks & XHCI_MTK_HOST) ? 20 : 160);
+	writel(temp, &xhci_exynos->ir_set_audio->irq_control);
+
+	temp = readl(&xhci_exynos->ir_set_audio->irq_pending);
+	xhci_info(xhci, "// [USB Audio] Enabling event ring interrupter %p by writing 0x%x to irq_pending",
+			xhci_exynos->ir_set_audio, (unsigned int) ER_IRQ_ENABLE(temp));
+	writel(ER_IRQ_ENABLE(temp), &xhci_exynos->ir_set_audio->irq_pending);
+}
+
+static int xhci_priv_init_quirk(struct usb_hcd *hcd)
+{
+	struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
+
+	if (!priv->init_quirk)
+		return 0;
+
+	return priv->init_quirk(hcd);
+}
+
+static int xhci_priv_suspend_quirk(struct usb_hcd *hcd)
+{
+	struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
+
+	if (!priv->suspend_quirk)
+		return 0;
+
+	return priv->suspend_quirk(hcd);
+}
+
+static int xhci_priv_resume_quirk(struct usb_hcd *hcd)
+{
+	struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
+
+	if (!priv->resume_quirk)
+		return 0;
+
+	return priv->resume_quirk(hcd);
+}
+
+static void xhci_exynos_quirks(struct device *dev, struct xhci_hcd *xhci)
+{
+	struct xhci_plat_priv *priv = xhci_to_priv(xhci);
+
+	/*
+	 * As of now platform drivers don't provide MSI support so we ensure
+	 * here that the generic code does not try to make a pci_dev from our
+	 * dev struct in order to setup MSI
+	 */
+	xhci->quirks |= XHCI_PLAT | priv->quirks;
+}
+
+/* called during probe() after chip reset completes */
+static int xhci_exynos_setup(struct usb_hcd *hcd)
+{
+	int ret;
+	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+
+	ret = xhci_priv_init_quirk(hcd);
+	if (ret)
+		return ret;
+
+	ret = xhci_gen_setup(hcd, xhci_exynos_quirks);
+	xhci_exynos_alloc_event_ring(xhci, GFP_KERNEL);
+
+	return ret;
+}
+
+static int xhci_exynos_start(struct usb_hcd *hcd)
+{
+	struct xhci_hcd *xhci;
+	int ret;
+
+	xhci_priv_exynos_start(hcd);
+
+	ret = xhci_run(hcd);
+
+	xhci = hcd_to_xhci(hcd);
+	xhci_exynos_usb_offload_enable_event_ring(xhci);
+
+	return ret;
+}
+
+#ifdef CONFIG_OF
+static const struct xhci_plat_priv xhci_plat_marvell_armada = {
+	.init_quirk = xhci_mvebu_mbus_init_quirk,
+};
+
+static const struct xhci_plat_priv xhci_plat_marvell_armada3700 = {
+	.plat_setup = xhci_mvebu_a3700_plat_setup,
+	.init_quirk = xhci_mvebu_a3700_init_quirk,
+};
+
+static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen2 = {
+	SET_XHCI_PLAT_PRIV_FOR_RCAR(XHCI_RCAR_FIRMWARE_NAME_V1)
+};
+
+static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen3 = {
+	SET_XHCI_PLAT_PRIV_FOR_RCAR(XHCI_RCAR_FIRMWARE_NAME_V3)
+};
+
+static const struct xhci_plat_priv xhci_plat_brcm = {
+	.quirks = XHCI_RESET_ON_RESUME,
+};
+
+static const struct of_device_id usb_xhci_of_match[] = {
+	{
+		.compatible = "generic-xhci",
+	}, {
+		.compatible = "xhci-platform",
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
+#endif
+
+static int xhci_vendor_init(struct xhci_hcd *xhci)
+{
+	struct xhci_vendor_ops *ops = NULL;
+
+	if (xhci_plat_vendor_overwrite.vendor_ops)
+		ops = xhci->vendor_ops = xhci_plat_vendor_overwrite.vendor_ops;
+
+	if (ops && ops->vendor_init)
+		return ops->vendor_init(xhci);
+	return 0;
+}
+
+static void xhci_vendor_cleanup(struct xhci_hcd *xhci)
+{
+	struct xhci_vendor_ops *ops = xhci_vendor_get_ops(xhci);
+
+	if (ops && ops->vendor_cleanup)
+		ops->vendor_cleanup(xhci);
+
+	xhci->vendor_ops = NULL;
+}
+
+static int xhci_exynos_probe(struct platform_device *pdev)
+{
+	const struct xhci_plat_priv *priv_match;
+	const struct hc_driver	*driver;
+	struct device		*sysdev, *tmpdev;
+	struct xhci_hcd		*xhci;
+	struct resource         *res;
+	struct usb_hcd		*hcd;
+	int			ret;
+	int			irq;
+	struct xhci_plat_priv	*priv = NULL;
+
+
+	xhci_exynos_register_vendor_ops(&ops);
+
+	if (usb_disabled())
+		return -ENODEV;
+
+	driver = &xhci_exynos_hc_driver;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
+	/*
+	 * sysdev must point to a device that is known to the system firmware
+	 * or PCI hardware. We handle these three cases here:
+	 * 1. xhci_plat comes from firmware
+	 * 2. xhci_plat is child of a device from firmware (dwc3-plat)
+	 * 3. xhci_plat is grandchild of a pci device (dwc3-pci)
+	 */
+	for (sysdev = &pdev->dev; sysdev; sysdev = sysdev->parent) {
+		if (is_of_node(sysdev->fwnode) ||
+			is_acpi_device_node(sysdev->fwnode))
+			break;
+#ifdef CONFIG_PCI
+		else if (sysdev->bus == &pci_bus_type)
+			break;
+#endif
+	}
+
+	if (!sysdev)
+		sysdev = &pdev->dev;
+
+	/* Try to set 64-bit DMA first */
+	if (WARN_ON(!sysdev->dma_mask))
+		/* Platform did not initialize dma_mask */
+		ret = dma_coerce_mask_and_coherent(sysdev,
+						   DMA_BIT_MASK(64));
+	else
+		ret = dma_set_mask_and_coherent(sysdev, DMA_BIT_MASK(64));
+
+	/* If seting 64-bit DMA mask fails, fall back to 32-bit DMA mask */
+	if (ret) {
+		ret = dma_set_mask_and_coherent(sysdev, DMA_BIT_MASK(32));
+		if (ret)
+			return ret;
+	}
+
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+	pm_runtime_get_noresume(&pdev->dev);
+
+	hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
+			       dev_name(&pdev->dev), NULL);
+	if (!hcd) {
+		ret = -ENOMEM;
+		goto disable_runtime;
+	}
+
+	hcd->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+	if (IS_ERR(hcd->regs)) {
+		ret = PTR_ERR(hcd->regs);
+		goto put_hcd;
+	}
+
+	hcd->rsrc_start = res->start;
+	hcd->rsrc_len = resource_size(res);
+
+	xhci = hcd_to_xhci(hcd);
+
+	/*
+	 * Not all platforms have clks so it is not an error if the
+	 * clock do not exist.
+	 */
+	xhci->reg_clk = devm_clk_get_optional(&pdev->dev, "reg");
+	if (IS_ERR(xhci->reg_clk)) {
+		ret = PTR_ERR(xhci->reg_clk);
+		goto put_hcd;
+	}
+
+	ret = clk_prepare_enable(xhci->reg_clk);
+	if (ret)
+		goto put_hcd;
+
+	xhci->clk = devm_clk_get_optional(&pdev->dev, NULL);
+	if (IS_ERR(xhci->clk)) {
+		ret = PTR_ERR(xhci->clk);
+		goto disable_reg_clk;
+	}
+
+	ret = clk_prepare_enable(xhci->clk);
+	if (ret)
+		goto disable_reg_clk;
+
+	if (pdev->dev.of_node)
+		priv_match = of_device_get_match_data(&pdev->dev);
+	else
+		priv_match = dev_get_platdata(&pdev->dev);
+
+	if (priv_match) {
+		priv = hcd_to_xhci_priv(hcd);
+		/* Just copy data for now */
+		*priv = *priv_match;
+	}
+
+	device_set_wakeup_capable(&pdev->dev, true);
+
+	xhci->main_hcd = hcd;
+	xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
+			dev_name(&pdev->dev), hcd);
+	if (!xhci->shared_hcd) {
+		ret = -ENOMEM;
+		goto disable_clk;
+	}
+
+	/* imod_interval is the interrupt moderation value in nanoseconds. */
+	xhci->imod_interval = 40000;
+
+	/* Iterate over all parent nodes for finding quirks */
+	for (tmpdev = &pdev->dev; tmpdev; tmpdev = tmpdev->parent) {
+
+		if (device_property_read_bool(tmpdev, "usb2-lpm-disable"))
+			xhci->quirks |= XHCI_HW_LPM_DISABLE;
+
+		if (device_property_read_bool(tmpdev, "usb3-lpm-capable"))
+			xhci->quirks |= XHCI_LPM_SUPPORT;
+
+		if (device_property_read_bool(tmpdev, "quirk-broken-port-ped"))
+			xhci->quirks |= XHCI_BROKEN_PORT_PED;
+
+		device_property_read_u32(tmpdev, "imod-interval-ns",
+					 &xhci->imod_interval);
+	}
+
+	hcd->usb_phy = devm_usb_get_phy_by_phandle(sysdev, "usb-phy", 0);
+	if (IS_ERR(hcd->usb_phy)) {
+		ret = PTR_ERR(hcd->usb_phy);
+		if (ret == -EPROBE_DEFER)
+			goto put_usb3_hcd;
+		hcd->usb_phy = NULL;
+	} else {
+		ret = usb_phy_init(hcd->usb_phy);
+		if (ret)
+			goto put_usb3_hcd;
+	}
+
+	ret = xhci_vendor_init(xhci);
+	if (ret)
+		goto disable_usb_phy;
+
+	hcd->tpl_support = of_usb_host_tpl_support(sysdev->of_node);
+	xhci->shared_hcd->tpl_support = hcd->tpl_support;
+
+	if (priv) {
+		ret = xhci_priv_exynos_setup(hcd);
+		if (ret)
+			goto disable_usb_phy;
+	}
+
+	if ((xhci->quirks & XHCI_SKIP_PHY_INIT) || (priv && (priv->quirks & XHCI_SKIP_PHY_INIT)))
+		hcd->skip_phy_initialization = 1;
+
+	if (priv && (priv->quirks & XHCI_SG_TRB_CACHE_SIZE_QUIRK))
+		xhci->quirks |= XHCI_SG_TRB_CACHE_SIZE_QUIRK;
+
+	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
+	if (ret)
+		goto disable_usb_phy;
+
+	if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
+		xhci->shared_hcd->can_do_streams = 1;
+
+	ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
+	if (ret)
+		goto dealloc_usb2_hcd;
+
+	device_enable_async_suspend(&pdev->dev);
+	pm_runtime_put_noidle(&pdev->dev);
+
+	/*
+	 * Prevent runtime pm from being on as default, users should enable
+	 * runtime pm using power/control in sysfs.
+	 */
+	pm_runtime_forbid(&pdev->dev);
+
+	return 0;
+
+
+dealloc_usb2_hcd:
+	usb_remove_hcd(hcd);
+
+disable_usb_phy:
+	usb_phy_shutdown(hcd->usb_phy);
+
+put_usb3_hcd:
+	usb_put_hcd(xhci->shared_hcd);
+
+disable_clk:
+	clk_disable_unprepare(xhci->clk);
+
+disable_reg_clk:
+	clk_disable_unprepare(xhci->reg_clk);
+
+put_hcd:
+	usb_put_hcd(hcd);
+
+disable_runtime:
+	pm_runtime_put_noidle(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+
+	return ret;
+}
+
+static int xhci_exynos_remove(struct platform_device *dev)
+{
+	struct usb_hcd	*hcd = platform_get_drvdata(dev);
+	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
+	struct clk *clk = xhci->clk;
+	struct clk *reg_clk = xhci->reg_clk;
+	struct usb_hcd *shared_hcd = xhci->shared_hcd;
+
+	pm_runtime_get_sync(&dev->dev);
+	xhci->xhc_state |= XHCI_STATE_REMOVING;
+
+	usb_remove_hcd(shared_hcd);
+	xhci->shared_hcd = NULL;
+	usb_phy_shutdown(hcd->usb_phy);
+
+	usb_remove_hcd(hcd);
+
+	xhci_vendor_cleanup(xhci);
+
+	usb_put_hcd(shared_hcd);
+	clk_disable_unprepare(clk);
+	clk_disable_unprepare(reg_clk);
+	usb_put_hcd(hcd);
+
+	pm_runtime_disable(&dev->dev);
+	pm_runtime_put_noidle(&dev->dev);
+	pm_runtime_set_suspended(&dev->dev);
+
+	return 0;
+}
+
+static int __maybe_unused xhci_exynos_suspend(struct device *dev)
+{
+	struct usb_hcd	*hcd = dev_get_drvdata(dev);
+	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
+	int ret;
+
+	ret = xhci_priv_suspend_quirk(hcd);
+	if (ret)
+		return ret;
+	/*
+	 * xhci_suspend() needs `do_wakeup` to know whether host is allowed
+	 * to do wakeup during suspend.
+	 */
+	return xhci_suspend(xhci, device_may_wakeup(dev));
+}
+
+static int __maybe_unused xhci_exynos_resume(struct device *dev)
+{
+	struct usb_hcd	*hcd = dev_get_drvdata(dev);
+	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
+	int ret;
+
+	ret = xhci_priv_resume_quirk(hcd);
+	if (ret)
+		return ret;
+
+	ret = xhci_resume(xhci, 0);
+	if (ret)
+		return ret;
+
+	pm_runtime_disable(dev);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+
+	return 0;
+}
+
+static int __maybe_unused xhci_exynos_runtime_suspend(struct device *dev)
+{
+	struct usb_hcd  *hcd = dev_get_drvdata(dev);
+	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+	int ret;
+
+	ret = xhci_priv_suspend_quirk(hcd);
+	if (ret)
+		return ret;
+
+	return xhci_suspend(xhci, true);
+}
+
+static int __maybe_unused xhci_exynos_runtime_resume(struct device *dev)
+{
+	struct usb_hcd  *hcd = dev_get_drvdata(dev);
+	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+
+	return xhci_resume(xhci, 0);
+}
+
+static const struct dev_pm_ops xhci_exynos_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(xhci_exynos_suspend, xhci_exynos_resume)
+
+	SET_RUNTIME_PM_OPS(xhci_exynos_runtime_suspend,
+			   xhci_exynos_runtime_resume,
+			   NULL)
+};
+
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id usb_xhci_acpi_match[] = {
+	/* XHCI-compliant USB Controller */
+	{ "PNP0D10", },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, usb_xhci_acpi_match);
+#endif
+
+static struct platform_driver usb_xhci_driver = {
+	.probe	= xhci_exynos_probe,
+	.remove	= xhci_exynos_remove,
+	.shutdown = usb_hcd_platform_shutdown,
+	.driver	= {
+		.name = "xhci-exynos",
+		.pm = &xhci_exynos_pm_ops,
+		.of_match_table = of_match_ptr(usb_xhci_of_match),
+		.acpi_match_table = ACPI_PTR(usb_xhci_acpi_match),
+	},
+};
+MODULE_ALIAS("platform:xhci-exynos");
+
+static int __init xhci_exynos_init(void)
+{
+	xhci_init_driver(&xhci_exynos_hc_driver, &xhci_exynos_overrides);
+	return platform_driver_register(&usb_xhci_driver);
+}
+module_init(xhci_exynos_init);
+
+static void __exit xhci_exynos_exit(void)
+{
+	platform_driver_unregister(&usb_xhci_driver);
+}
+module_exit(xhci_exynos_exit);
+
+MODULE_DESCRIPTION("xHCI Exynos Host Controller Driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/usb/host/xhci-exynos.h b/drivers/usb/host/xhci-exynos.h
new file mode 100644
index 000000000000..91860a3a1c7d
--- /dev/null
+++ b/drivers/usb/host/xhci-exynos.h
@@ -0,0 +1,63 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * xhci-exynos.h - xHCI host controller driver platform Bus Glue for Exynos.
+ *
+ * Copyright (C) 2022 Samsung Electronics Incorporated - http://www.samsung.com
+ * Author: Daehwan Jung <dh10.jung@samsung.com>
+ *
+ * A lot of code borrowed from the Linux xHCI driver.
+ */
+
+#include "xhci.h"
+
+/* EXYNOS uram memory map */
+#define EXYNOS_URAM_ABOX_EVT_RING_ADDR	0x02a00000
+#define EXYNOS_URAM_ISOC_OUT_RING_ADDR	0x02a01000
+#define EXYNOS_URAM_ISOC_IN_RING_ADDR	0x02a02000
+#define EXYNOS_URAM_DEVICE_CTX_ADDR	0x02a03000
+#define EXYNOS_URAM_DCBAA_ADDR		0x02a03880
+#define EXYNOS_URAM_ABOX_ERST_SEG_ADDR	0x02a03C80
+#define EXYNOS_URAM_CTX_SIZE		2112
+
+struct xhci_hcd_exynos {
+	struct	xhci_intr_reg __iomem *ir_set_audio;
+
+	struct xhci_ring	*event_ring_audio;
+	struct xhci_erst	erst_audio;
+
+	struct device		*dev;
+	struct usb_hcd		*hcd;
+	struct usb_hcd		*shared_hcd;
+
+	struct wakeup_source *main_wakelock; /* Wakelock for HS HCD */
+	struct wakeup_source *shared_wakelock; /* Wakelock for SS HCD */
+
+	u32 in_ep;
+	u32 out_ep;
+	u32 in_deq;
+	u32 out_deq;
+};
+
+struct xhci_exynos_priv {
+	const char *firmware_name;
+	unsigned long long quirks;
+	struct xhci_vendor_data *vendor_data;
+	int (*plat_setup)(struct usb_hcd *);
+	void (*plat_start)(struct usb_hcd *);
+	int (*init_quirk)(struct usb_hcd *);
+	int (*suspend_quirk)(struct usb_hcd *);
+	int (*resume_quirk)(struct usb_hcd *);
+	struct xhci_hcd_exynos *xhci_exynos;
+};
+
+#define hcd_to_xhci_exynos_priv(h) ((struct xhci_exynos_priv *)hcd_to_xhci(h)->priv)
+#define xhci_to_exynos_priv(x) ((struct xhci_exynos_priv *)(x)->priv)
+
+int xhci_check_trb_in_td_math(struct xhci_hcd *xhci);
+void xhci_segment_free(struct xhci_hcd *xhci, struct xhci_segment *seg);
+void xhci_link_segments(struct xhci_segment *prev,
+		struct xhci_segment *next,
+		enum xhci_ring_type type, bool chain_links);
+void xhci_initialize_ring_info(struct xhci_ring *ring,
+					unsigned int cycle_state);
+void xhci_remove_stream_mapping(struct xhci_ring *ring);
-- 
2.31.1

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

[Bjørn Mork]

Daehwan Jung <dh10.jung@samsung.com> writes:

> +++ b/drivers/usb/host/xhci-exynos.c
> @@ -0,0 +1,982 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * xhci-exynos.c - xHCI host controller driver platform Bus Glue for Exynos.
> + *
> + * Copyright (C) 2022 Samsung Electronics Incorporated - http://www.samsung.com
> + * Author: Daehwan Jung <dh10.jung@samsung.com>
> + *
> + * A lot of code borrowed from the Linux xHCI driver.
> + */
> +#include <linux/clk.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/usb/phy.h>
> +#include <linux/slab.h>
> +#include <linux/acpi.h>
> +#include <linux/usb/of.h>
> +
> +#include "xhci.h"
> +#include "xhci-plat.h"
> +#include "xhci-mvebu.h"
> +#include "xhci-rcar.h"

The xhci-plat.c file is Copyright (C) 2012 Texas Instruments Incorporated
You can't just steal it.

Besides, even if you could, this isn't about copying as much code as
posible from A to B.  The point is to add as *little* code as possible
to support your hardware.

> +static int xhci_exynos_vendor_init(struct xhci_hcd *xhci)
> +{
> +	/* TODO */
> +	return 0;
> +}

And you didn't even add that?

> +static int xhci_exynos_wake_lock(struct xhci_hcd_exynos *xhci_exynos,
> +				   int is_main_hcd, int is_lock)
> +{
> +	struct usb_hcd	*hcd = xhci_exynos->hcd;
> +	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> +	struct wakeup_source *main_wakelock, *shared_wakelock;
> +
> +	main_wakelock = xhci_exynos->main_wakelock;
> +	shared_wakelock = xhci_exynos->shared_wakelock;

Are these fields initialized anywhere?


> +
> +	if (xhci->xhc_state & XHCI_STATE_REMOVING)
> +		return -ESHUTDOWN;
> +
> +	if (is_lock) {

bool?

> +		if (is_main_hcd)

another bool?

> +			__pm_stay_awake(main_wakelock);
> +		else
> +			__pm_stay_awake(shared_wakelock);
> +	} else {
> +		if (is_main_hcd)
> +			__pm_relax(main_wakelock);
> +		else
> +			__pm_relax(shared_wakelock);
> +	}

Looks interesting.   Are you signalling relax/wakeups events to the PM
core on device suspend/resume?  Why?

> +static int xhci_exynos_address_device(struct usb_hcd *hcd, struct usb_device *udev)
> +{
> +	struct xhci_hcd *xhci;
> +	int ret;
> +
> +	ret = xhci_address_device(hcd, udev);
> +	xhci = hcd_to_xhci(hcd);
> +
> +	return ret;
> +}

What's left here if we drop the unused parts?

> +#ifdef CONFIG_OF
> +static const struct xhci_plat_priv xhci_plat_marvell_armada = {
> +	.init_quirk = xhci_mvebu_mbus_init_quirk,
> +};
> +
> +static const struct xhci_plat_priv xhci_plat_marvell_armada3700 = {
> +	.plat_setup = xhci_mvebu_a3700_plat_setup,
> +	.init_quirk = xhci_mvebu_a3700_init_quirk,
> +};


Right...

> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id usb_xhci_acpi_match[] = {
> +	/* XHCI-compliant USB Controller */
> +	{ "PNP0D10", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(acpi, usb_xhci_acpi_match);
> +#endif

Nice one

There's no need to copy me if you plan to resend any of this.  I'm just
a drive-by reader here anyway, and I've seen enough.

Good luck!




Bjørn

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

[Jung Daehwan]

[-- Attachment #1: Type: text/plain, Size: 3732 bytes --]

On Mon, Mar 21, 2022 at 04:45:01PM +0100, Bjørn Mork wrote:
> Daehwan Jung <dh10.jung@samsung.com> writes:
> 
> > +++ b/drivers/usb/host/xhci-exynos.c
> > @@ -0,0 +1,982 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * xhci-exynos.c - xHCI host controller driver platform Bus Glue for Exynos.
> > + *
> > + * Copyright (C) 2022 Samsung Electronics Incorporated - http://www.samsung.com
> > + * Author: Daehwan Jung <dh10.jung@samsung.com>
> > + *
> > + * A lot of code borrowed from the Linux xHCI driver.
> > + */
> > +#include <linux/clk.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/module.h>
> > +#include <linux/pci.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/usb/phy.h>
> > +#include <linux/slab.h>
> > +#include <linux/acpi.h>
> > +#include <linux/usb/of.h>
> > +
> > +#include "xhci.h"
> > +#include "xhci-plat.h"
> > +#include "xhci-mvebu.h"
> > +#include "xhci-rcar.h"

> 
> The xhci-plat.c file is Copyright (C) 2012 Texas Instruments Incorporated
> You can't just steal it.
> 
> Besides, even if you could, this isn't about copying as much code as
> posible from A to B.  The point is to add as *little* code as possible
> to support your hardware.
> 

Hi bjorn,

Thanks for your comment. I will write more detail about copyright on
next submission.

> > +static int xhci_exynos_vendor_init(struct xhci_hcd *xhci)
> > +{
> > +	/* TODO */
> > +	return 0;
> > +}
> 
> And you didn't even add that?
> 
> > +static int xhci_exynos_wake_lock(struct xhci_hcd_exynos *xhci_exynos,
> > +				   int is_main_hcd, int is_lock)
> > +{
> > +	struct usb_hcd	*hcd = xhci_exynos->hcd;
> > +	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> > +	struct wakeup_source *main_wakelock, *shared_wakelock;
> > +
> > +	main_wakelock = xhci_exynos->main_wakelock;
> > +	shared_wakelock = xhci_exynos->shared_wakelock;
> 
> Are these fields initialized anywhere?
> 
> 
> > +
> > +	if (xhci->xhc_state & XHCI_STATE_REMOVING)
> > +		return -ESHUTDOWN;
> > +
> > +	if (is_lock) {
> 
> bool?
> 

Yes, Currently I use it as if bool. 

> > +		if (is_main_hcd)
> 
> another bool?

Same

> 
> > +			__pm_stay_awake(main_wakelock);
> > +		else
> > +			__pm_stay_awake(shared_wakelock);
> > +	} else {
> > +		if (is_main_hcd)
> > +			__pm_relax(main_wakelock);
> > +		else
> > +			__pm_relax(shared_wakelock);
> > +	}
> 
> Looks interesting.   Are you signalling relax/wakeups events to the PM
> core on device suspend/resume?  Why?

I want to enter system sleep if possible at this point. It's related to
power scenario on my SOC.

> 
> > +static int xhci_exynos_address_device(struct usb_hcd *hcd, struct usb_device *udev)
> > +{
> > +	struct xhci_hcd *xhci;
> > +	int ret;
> > +
> > +	ret = xhci_address_device(hcd, udev);
> > +	xhci = hcd_to_xhci(hcd);
> > +
> > +	return ret;
> > +}
> 
> What's left here if we drop the unused parts?
> 

There's some missing code. Thanks..


> > +#ifdef CONFIG_OF
> > +static const struct xhci_plat_priv xhci_plat_marvell_armada = {
> > +	.init_quirk = xhci_mvebu_mbus_init_quirk,
> > +};
> > +
> > +static const struct xhci_plat_priv xhci_plat_marvell_armada3700 = {
> > +	.plat_setup = xhci_mvebu_a3700_plat_setup,
> > +	.init_quirk = xhci_mvebu_a3700_init_quirk,
> > +};
> 
> 
> Right...
> 
> > +#ifdef CONFIG_ACPI
> > +static const struct acpi_device_id usb_xhci_acpi_match[] = {
> > +	/* XHCI-compliant USB Controller */
> > +	{ "PNP0D10", },
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(acpi, usb_xhci_acpi_match);
> > +#endif
> 
> Nice one
> 
> There's no need to copy me if you plan to resend any of this.  I'm just
> a drive-by reader here anyway, and I've seen enough.
> 
> Good luck!
> 
> 
> 
> 
> Bjørn
> 

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

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

[kernel test robot]

Hi Daehwan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20220318]
[also build test ERROR on v5.17]
[cannot apply to usb/usb-testing krzk/for-next char-misc/char-misc-testing v5.17 v5.17-rc8 v5.17-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Daehwan-Jung/usb-host-export-symbols-for-xhci-hooks-usage/20220321-180046
base:    6d72dda014a4753974eb08950089ddf71fec4f60
config: nios2-allyesconfig (https://download.01.org/0day-ci/archive/20220322/202203220053.4OkItVxU-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/517a7258fc6a2861b66ae9893b39d8bd4d6739e7
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Daehwan-Jung/usb-host-export-symbols-for-xhci-hooks-usage/20220321-180046
        git checkout 517a7258fc6a2861b66ae9893b39d8bd4d6739e7
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nios2 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/usb/host/xhci-exynos.c: In function 'xhci_priv_exynos_setup':
   drivers/usb/host/xhci-exynos.c:55:18: error: 'struct xhci_plat_priv' has no member named 'plat_setup'
      55 |         if (!priv->plat_setup)
         |                  ^~
   drivers/usb/host/xhci-exynos.c:58:20: error: 'struct xhci_plat_priv' has no member named 'plat_setup'
      58 |         return priv->plat_setup(hcd);
         |                    ^~
   drivers/usb/host/xhci-exynos.c: In function 'xhci_exynos_free_container_ctx':
>> drivers/usb/host/xhci-exynos.c:146:17: error: implicit declaration of function 'dma_pool_free'; did you mean 'mempool_free'? [-Werror=implicit-function-declaration]
     146 |                 dma_pool_free(xhci->device_pool, ctx->bytes, ctx->dma);
         |                 ^~~~~~~~~~~~~
         |                 mempool_free
   In file included from include/uapi/linux/posix_types.h:5,
                    from include/uapi/linux/types.h:14,
                    from include/linux/types.h:6,
                    from include/linux/kasan-checks.h:5,
                    from include/asm-generic/rwonce.h:26,
                    from ./arch/nios2/include/generated/asm/rwonce.h:1,
                    from include/linux/compiler.h:248,
                    from include/linux/err.h:5,
                    from include/linux/clk.h:12,
                    from drivers/usb/host/xhci-exynos.c:10:
   drivers/usb/host/xhci-exynos.c: In function 'xhci_exynos_bus_suspend':
   include/linux/stddef.h:8:14: error: called object is not a function or function pointer
       8 | #define NULL ((void *)0)
         |              ^
   drivers/usb/host/xhci.h:2190:33: note: in expansion of macro 'NULL'
    2190 | #define xhci_bus_suspend        NULL
         |                                 ^~~~
   drivers/usb/host/xhci-exynos.c:234:15: note: in expansion of macro 'xhci_bus_suspend'
     234 |         ret = xhci_bus_suspend(hcd);
         |               ^~~~~~~~~~~~~~~~
   drivers/usb/host/xhci-exynos.c: In function 'xhci_exynos_bus_resume':
   include/linux/stddef.h:8:14: error: called object is not a function or function pointer
       8 | #define NULL ((void *)0)
         |              ^
   drivers/usb/host/xhci.h:2191:33: note: in expansion of macro 'NULL'
    2191 | #define xhci_bus_resume         NULL
         |                                 ^~~~
   drivers/usb/host/xhci-exynos.c:252:15: note: in expansion of macro 'xhci_bus_resume'
     252 |         ret = xhci_bus_resume(hcd);
         |               ^~~~~~~~~~~~~~~
   drivers/usb/host/xhci-exynos.c: In function 'xhci_exynos_address_device':
   drivers/usb/host/xhci-exynos.c:260:26: warning: variable 'xhci' set but not used [-Wunused-but-set-variable]
     260 |         struct xhci_hcd *xhci;
         |                          ^~~~
   drivers/usb/host/xhci-exynos.c: In function 'xhci_exynos_parse_endpoint':
   drivers/usb/host/xhci-exynos.c:276:29: warning: variable 'ep_ctx' set but not used [-Wunused-but-set-variable]
     276 |         struct xhci_ep_ctx *ep_ctx;
         |                             ^~~~~~
   drivers/usb/host/xhci-exynos.c: In function 'xhci_exynos_set_hc_event_deq_audio':
   drivers/usb/host/xhci-exynos.c:370:20: warning: variable 'deq' set but not used [-Wunused-but-set-variable]
     370 |         dma_addr_t deq;
         |                    ^~~
   drivers/usb/host/xhci-exynos.c: At top level:
   drivers/usb/host/xhci-exynos.c:576:10: error: 'const struct xhci_plat_priv' has no member named 'plat_setup'
     576 |         .plat_setup = xhci_mvebu_a3700_plat_setup,
         |          ^~~~~~~~~~
   drivers/usb/host/xhci-exynos.c:576:23: error: 'xhci_mvebu_a3700_plat_setup' undeclared here (not in a function); did you mean 'xhci_mvebu_a3700_init_quirk'?
     576 |         .plat_setup = xhci_mvebu_a3700_plat_setup,
         |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                       xhci_mvebu_a3700_init_quirk
   drivers/usb/host/xhci-exynos.c: In function 'xhci_priv_exynos_setup':
   drivers/usb/host/xhci-exynos.c:59:1: error: control reaches end of non-void function [-Werror=return-type]
      59 | }
         | ^
   At top level:
   drivers/usb/host/xhci-exynos.c:588:36: warning: 'xhci_plat_brcm' defined but not used [-Wunused-const-variable=]
     588 | static const struct xhci_plat_priv xhci_plat_brcm = {
         |                                    ^~~~~~~~~~~~~~
   drivers/usb/host/xhci-exynos.c:584:36: warning: 'xhci_plat_renesas_rcar_gen3' defined but not used [-Wunused-const-variable=]
     584 | static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen3 = {
         |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/usb/host/xhci-exynos.c:580:36: warning: 'xhci_plat_renesas_rcar_gen2' defined but not used [-Wunused-const-variable=]
     580 | static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen2 = {
         |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/usb/host/xhci-exynos.c:575:36: warning: 'xhci_plat_marvell_armada3700' defined but not used [-Wunused-const-variable=]
     575 | static const struct xhci_plat_priv xhci_plat_marvell_armada3700 = {
         |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/usb/host/xhci-exynos.c:571:36: warning: 'xhci_plat_marvell_armada' defined but not used [-Wunused-const-variable=]
     571 | static const struct xhci_plat_priv xhci_plat_marvell_armada = {
         |                                    ^~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +146 drivers/usb/host/xhci-exynos.c

   141	
   142	static void xhci_exynos_free_container_ctx(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx)
   143	{
   144		/* Ignore dma_pool_free if it is allocated from URAM */
   145		if (ctx->dma != EXYNOS_URAM_DEVICE_CTX_ADDR)
 > 146			dma_pool_free(xhci->device_pool, ctx->bytes, ctx->dma);
   147	}
   148	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

[kernel test robot]

Hi Daehwan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20220318]
[also build test ERROR on v5.17]
[cannot apply to usb/usb-testing krzk/for-next char-misc/char-misc-testing v5.17 v5.17-rc8 v5.17-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Daehwan-Jung/usb-host-export-symbols-for-xhci-hooks-usage/20220321-180046
base:    6d72dda014a4753974eb08950089ddf71fec4f60
config: ia64-allmodconfig (https://download.01.org/0day-ci/archive/20220322/202203220027.WbsbZkyk-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/517a7258fc6a2861b66ae9893b39d8bd4d6739e7
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Daehwan-Jung/usb-host-export-symbols-for-xhci-hooks-usage/20220321-180046
        git checkout 517a7258fc6a2861b66ae9893b39d8bd4d6739e7
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/usb/host/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   drivers/usb/host/xhci-exynos.c: In function 'xhci_priv_exynos_setup':
>> drivers/usb/host/xhci-exynos.c:55:18: error: 'struct xhci_plat_priv' has no member named 'plat_setup'
      55 |         if (!priv->plat_setup)
         |                  ^~
   drivers/usb/host/xhci-exynos.c:58:20: error: 'struct xhci_plat_priv' has no member named 'plat_setup'
      58 |         return priv->plat_setup(hcd);
         |                    ^~
   drivers/usb/host/xhci-exynos.c: In function 'xhci_exynos_address_device':
>> drivers/usb/host/xhci-exynos.c:260:26: warning: variable 'xhci' set but not used [-Wunused-but-set-variable]
     260 |         struct xhci_hcd *xhci;
         |                          ^~~~
   drivers/usb/host/xhci-exynos.c: In function 'xhci_exynos_parse_endpoint':
>> drivers/usb/host/xhci-exynos.c:276:29: warning: variable 'ep_ctx' set but not used [-Wunused-but-set-variable]
     276 |         struct xhci_ep_ctx *ep_ctx;
         |                             ^~~~~~
   drivers/usb/host/xhci-exynos.c: In function 'xhci_exynos_set_hc_event_deq_audio':
>> drivers/usb/host/xhci-exynos.c:370:20: warning: variable 'deq' set but not used [-Wunused-but-set-variable]
     370 |         dma_addr_t deq;
         |                    ^~~
   drivers/usb/host/xhci-exynos.c: At top level:
>> drivers/usb/host/xhci-exynos.c:576:10: error: 'const struct xhci_plat_priv' has no member named 'plat_setup'
     576 |         .plat_setup = xhci_mvebu_a3700_plat_setup,
         |          ^~~~~~~~~~
>> drivers/usb/host/xhci-exynos.c:576:23: error: 'xhci_mvebu_a3700_plat_setup' undeclared here (not in a function); did you mean 'xhci_mvebu_a3700_init_quirk'?
     576 |         .plat_setup = xhci_mvebu_a3700_plat_setup,
         |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                       xhci_mvebu_a3700_init_quirk
   drivers/usb/host/xhci-exynos.c: In function 'xhci_priv_exynos_setup':
   drivers/usb/host/xhci-exynos.c:59:1: error: control reaches end of non-void function [-Werror=return-type]
      59 | }
         | ^
   At top level:
   drivers/usb/host/xhci-exynos.c:588:36: warning: 'xhci_plat_brcm' defined but not used [-Wunused-const-variable=]
     588 | static const struct xhci_plat_priv xhci_plat_brcm = {
         |                                    ^~~~~~~~~~~~~~
   drivers/usb/host/xhci-exynos.c:584:36: warning: 'xhci_plat_renesas_rcar_gen3' defined but not used [-Wunused-const-variable=]
     584 | static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen3 = {
         |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/usb/host/xhci-exynos.c:580:36: warning: 'xhci_plat_renesas_rcar_gen2' defined but not used [-Wunused-const-variable=]
     580 | static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen2 = {
         |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/usb/host/xhci-exynos.c:575:36: warning: 'xhci_plat_marvell_armada3700' defined but not used [-Wunused-const-variable=]
     575 | static const struct xhci_plat_priv xhci_plat_marvell_armada3700 = {
         |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/usb/host/xhci-exynos.c:571:36: warning: 'xhci_plat_marvell_armada' defined but not used [-Wunused-const-variable=]
     571 | static const struct xhci_plat_priv xhci_plat_marvell_armada = {
         |                                    ^~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +55 drivers/usb/host/xhci-exynos.c

    50	
    51	static int xhci_priv_exynos_setup(struct usb_hcd *hcd)
    52	{
    53		struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
    54	
  > 55		if (!priv->plat_setup)
    56			return 0;
    57	
    58		return priv->plat_setup(hcd);
    59	}
    60	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

[Krzysztof Kozlowski]

On 21/03/2022 09:59, Daehwan Jung wrote:
> This driver supports USB Audio offload with Co-processor.

One do you need one more XHCI driver? How does it differ from existing
and why existing cannot be extended?

> It only cares DCBAA, Device Context, Transfer Ring, Event Ring, and ERST.
> They are allocated on specific address with xhci hooks.
> Co-processor could use them directly without xhci driver after then.
> 
> Signed-off-by: Daehwan Jung <dh10.jung@samsung.com>
> ---
>  drivers/usb/host/Kconfig       |   9 +
>  drivers/usb/host/Makefile      |   1 +
>  drivers/usb/host/xhci-exynos.c | 982 +++++++++++++++++++++++++++++++++
>  drivers/usb/host/xhci-exynos.h |  63 +++
>  4 files changed, 1055 insertions(+)
>  create mode 100644 drivers/usb/host/xhci-exynos.c
>  create mode 100644 drivers/usb/host/xhci-exynos.h
> 

Please address all the questions I raised in your v1, do not ignore them.

Please use get_maintainers.pl to CC all necessary people and lists. It
makes me very sad that you do not follow the kernel development process
(as mentioned in submitting-patches.rst and other documents).

Best regards,
Krzysztof

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

[Jung Daehwan]

[-- Attachment #1: Type: text/plain, Size: 1607 bytes --]

On Tue, Mar 22, 2022 at 06:10:00PM +0100, Krzysztof Kozlowski wrote:
> On 21/03/2022 09:59, Daehwan Jung wrote:
> > This driver supports USB Audio offload with Co-processor.
> 
> One do you need one more XHCI driver? How does it differ from existing
> and why existing cannot be extended?

I have some exynos specific features and that's because need own driver.
It's not good to modify other XHCI driver for my features.

Additinally, I've been developing some features with other IP or Co-Processors.
Extending is not enough to cover them and I think owning driver is also better
on structural view.

Best Regards,
Jung Daehwan

> 
> > It only cares DCBAA, Device Context, Transfer Ring, Event Ring, and ERST.
> > They are allocated on specific address with xhci hooks.
> > Co-processor could use them directly without xhci driver after then.
> > 
> > Signed-off-by: Daehwan Jung <dh10.jung@samsung.com>
> > ---
> >  drivers/usb/host/Kconfig       |   9 +
> >  drivers/usb/host/Makefile      |   1 +
> >  drivers/usb/host/xhci-exynos.c | 982 +++++++++++++++++++++++++++++++++
> >  drivers/usb/host/xhci-exynos.h |  63 +++
> >  4 files changed, 1055 insertions(+)
> >  create mode 100644 drivers/usb/host/xhci-exynos.c
> >  create mode 100644 drivers/usb/host/xhci-exynos.h
> > 
> 
> Please address all the questions I raised in your v1, do not ignore them.
> 
> Please use get_maintainers.pl to CC all necessary people and lists. It
> makes me very sad that you do not follow the kernel development process
> (as mentioned in submitting-patches.rst and other documents).
> 
> Best regards,
> Krzysztof
> 

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

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

[Krzysztof Kozlowski]

On 23/03/2022 03:34, Jung Daehwan wrote:
> On Tue, Mar 22, 2022 at 06:10:00PM +0100, Krzysztof Kozlowski wrote:
>> On 21/03/2022 09:59, Daehwan Jung wrote:
>>> This driver supports USB Audio offload with Co-processor.
>>
>> One do you need one more XHCI driver? How does it differ from existing
>> and why existing cannot be extended?
> 
> I have some exynos specific features and that's because need own driver.
> It's not good to modify other XHCI driver for my features.
> 
> Additinally, I've been developing some features with other IP or Co-Processors.
> Extending is not enough to cover them and I think owning driver is also better
> on structural view.

This reason is not good enough because it's not visible that you cannot
extend existing drivers.


Best regards,
Krzysztof

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

[Jung Daehwan]

[-- Attachment #1: Type: text/plain, Size: 1083 bytes --]

On Wed, Mar 23, 2022 at 09:26:13AM +0100, Krzysztof Kozlowski wrote:
> On 23/03/2022 03:34, Jung Daehwan wrote:
> > On Tue, Mar 22, 2022 at 06:10:00PM +0100, Krzysztof Kozlowski wrote:
> >> On 21/03/2022 09:59, Daehwan Jung wrote:
> >>> This driver supports USB Audio offload with Co-processor.
> >>
> >> One do you need one more XHCI driver? How does it differ from existing
> >> and why existing cannot be extended?
> > 
> > I have some exynos specific features and that's because need own driver.
> > It's not good to modify other XHCI driver for my features.
> > 
> > Additinally, I've been developing some features with other IP or Co-Processors.
> > Extending is not enough to cover them and I think owning driver is also better
> > on structural view.
> 
> This reason is not good enough because it's not visible that you cannot
> extend existing drivers.
> 
> 
> Best regards,
> Krzysztof
> 

I could extend existing drivers as you said. It needs big change on my
driver and it could take long time... I'm going to try it. Thanks for
the comment.

Best Regards,
Jung Daehwan

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

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

[Krzysztof Kozlowski]

On 21/03/2022 09:59, Daehwan Jung wrote:
> This driver supports USB Audio offload with Co-processor.
> It only cares DCBAA, Device Context, Transfer Ring, Event Ring, and ERST.
> They are allocated on specific address with xhci hooks.
> Co-processor could use them directly without xhci driver after then.
> 
> Signed-off-by: Daehwan Jung <dh10.jung@samsung.com>
> ---
>  drivers/usb/host/Kconfig       |   9 +
>  drivers/usb/host/Makefile      |   1 +
>  drivers/usb/host/xhci-exynos.c | 982 +++++++++++++++++++++++++++++++++
>  drivers/usb/host/xhci-exynos.h |  63 +++
>  4 files changed, 1055 insertions(+)
>  create mode 100644 drivers/usb/host/xhci-exynos.c
>  create mode 100644 drivers/usb/host/xhci-exynos.h
> 
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 57ca5f97a3dc..850e6b71fac5 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -783,3 +783,12 @@ config USB_XEN_HCD
>  	  by the Xen host (usually Dom0).
>  	  Only needed if the kernel is running in a Xen guest and generic
>  	  access to a USB device is needed.
> +
> +config USB_XHCI_EXYNOS
> +	tristate "XHCI support for Samsung Exynos SoC Series"
> +	depends on ARCH_EXYNOS || COMPILE_TEST
> +	help
> +	  Enable support for the Samsung Exynos SOC's on-chip XHCI
> +	  controller.
> +
> +	  If unsure, say N.
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index 2948983618fb..300f22b6eb1b 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -86,3 +86,4 @@ obj-$(CONFIG_USB_HCD_SSB)	+= ssb-hcd.o
>  obj-$(CONFIG_USB_FOTG210_HCD)	+= fotg210-hcd.o
>  obj-$(CONFIG_USB_MAX3421_HCD)	+= max3421-hcd.o
>  obj-$(CONFIG_USB_XEN_HCD)	+= xen-hcd.o
> +obj-$(CONFIG_USB_XHCI_EXYNOS)	+= xhci-exynos.o
> diff --git a/drivers/usb/host/xhci-exynos.c b/drivers/usb/host/xhci-exynos.c
> new file mode 100644
> index 000000000000..19ee21f1d024
> --- /dev/null
> +++ b/drivers/usb/host/xhci-exynos.c
> @@ -0,0 +1,982 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * xhci-exynos.c - xHCI host controller driver platform Bus Glue for Exynos.
> + *
> + * Copyright (C) 2022 Samsung Electronics Incorporated - http://www.samsung.com
> + * Author: Daehwan Jung <dh10.jung@samsung.com>
> + *
> + * A lot of code borrowed from the Linux xHCI driver.
> + */
> +#include <linux/clk.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/usb/phy.h>
> +#include <linux/slab.h>
> +#include <linux/acpi.h>
> +#include <linux/usb/of.h>
> +
> +#include "xhci.h"
> +#include "xhci-plat.h"
> +#include "xhci-mvebu.h"
> +#include "xhci-rcar.h"

Could you explain why do you need RCAR and Marvell code in Exynos? Is it
even a real driver here? On what platforms this can be tested? Where are
the bindings?


(...)


> +static const struct of_device_id usb_xhci_of_match[] = {
> +	{
> +		.compatible = "generic-xhci",
> +	}, {
> +		.compatible = "xhci-platform",
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
> +#endif

No, this not generic-xhci but Exynos driver. This does not make any sense.


Best regards,
Krzysztof

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

[Jung Daehwan]

[-- Attachment #1: Type: text/plain, Size: 3704 bytes --]

On Tue, Mar 22, 2022 at 06:16:58PM +0100, Krzysztof Kozlowski wrote:
> On 21/03/2022 09:59, Daehwan Jung wrote:
> > This driver supports USB Audio offload with Co-processor.
> > It only cares DCBAA, Device Context, Transfer Ring, Event Ring, and ERST.
> > They are allocated on specific address with xhci hooks.
> > Co-processor could use them directly without xhci driver after then.
> > 
> > Signed-off-by: Daehwan Jung <dh10.jung@samsung.com>
> > ---
> >  drivers/usb/host/Kconfig       |   9 +
> >  drivers/usb/host/Makefile      |   1 +
> >  drivers/usb/host/xhci-exynos.c | 982 +++++++++++++++++++++++++++++++++
> >  drivers/usb/host/xhci-exynos.h |  63 +++
> >  4 files changed, 1055 insertions(+)
> >  create mode 100644 drivers/usb/host/xhci-exynos.c
> >  create mode 100644 drivers/usb/host/xhci-exynos.h
> > 
> > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> > index 57ca5f97a3dc..850e6b71fac5 100644
> > --- a/drivers/usb/host/Kconfig
> > +++ b/drivers/usb/host/Kconfig
> > @@ -783,3 +783,12 @@ config USB_XEN_HCD
> >  	  by the Xen host (usually Dom0).
> >  	  Only needed if the kernel is running in a Xen guest and generic
> >  	  access to a USB device is needed.
> > +
> > +config USB_XHCI_EXYNOS
> > +	tristate "XHCI support for Samsung Exynos SoC Series"
> > +	depends on ARCH_EXYNOS || COMPILE_TEST
> > +	help
> > +	  Enable support for the Samsung Exynos SOC's on-chip XHCI
> > +	  controller.
> > +
> > +	  If unsure, say N.
> > diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> > index 2948983618fb..300f22b6eb1b 100644
> > --- a/drivers/usb/host/Makefile
> > +++ b/drivers/usb/host/Makefile
> > @@ -86,3 +86,4 @@ obj-$(CONFIG_USB_HCD_SSB)	+= ssb-hcd.o
> >  obj-$(CONFIG_USB_FOTG210_HCD)	+= fotg210-hcd.o
> >  obj-$(CONFIG_USB_MAX3421_HCD)	+= max3421-hcd.o
> >  obj-$(CONFIG_USB_XEN_HCD)	+= xen-hcd.o
> > +obj-$(CONFIG_USB_XHCI_EXYNOS)	+= xhci-exynos.o
> > diff --git a/drivers/usb/host/xhci-exynos.c b/drivers/usb/host/xhci-exynos.c
> > new file mode 100644
> > index 000000000000..19ee21f1d024
> > --- /dev/null
> > +++ b/drivers/usb/host/xhci-exynos.c
> > @@ -0,0 +1,982 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * xhci-exynos.c - xHCI host controller driver platform Bus Glue for Exynos.
> > + *
> > + * Copyright (C) 2022 Samsung Electronics Incorporated - http://www.samsung.com
> > + * Author: Daehwan Jung <dh10.jung@samsung.com>
> > + *
> > + * A lot of code borrowed from the Linux xHCI driver.
> > + */
> > +#include <linux/clk.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/module.h>
> > +#include <linux/pci.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/usb/phy.h>
> > +#include <linux/slab.h>
> > +#include <linux/acpi.h>
> > +#include <linux/usb/of.h>
> > +
> > +#include "xhci.h"
> > +#include "xhci-plat.h"
> > +#include "xhci-mvebu.h"
> > +#include "xhci-rcar.h"
> 
> Could you explain why do you need RCAR and Marvell code in Exynos? Is it
> even a real driver here? On what platforms this can be tested? Where are
> the bindings?

2 headers you said are not needed. I'm going to remove it on next
submission. I tested on Exynos platform and it's real driver. I haven't
made bindings.

> 
> 
> (...)
> 
> 
> > +static const struct of_device_id usb_xhci_of_match[] = {
> > +	{
> > +		.compatible = "generic-xhci",
> > +	}, {
> > +		.compatible = "xhci-platform",
> > +	},
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
> > +#endif
> 
> No, this not generic-xhci but Exynos driver. This does not make any sense.
> 

I agree with you and I'm going to remove it.

> 
> Best regards,
> Krzysztof
> 

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

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

[Krzysztof Kozlowski]

On 23/03/2022 06:17, Jung Daehwan wrote:
> On Tue, Mar 22, 2022 at 06:16:58PM +0100, Krzysztof Kozlowski wrote:
>> On 21/03/2022 09:59, Daehwan Jung wrote:
>>> This driver supports USB Audio offload with Co-processor.
>>> It only cares DCBAA, Device Context, Transfer Ring, Event Ring, and ERST.
>>> They are allocated on specific address with xhci hooks.
>>> Co-processor could use them directly without xhci driver after then.
>>>
>>> Signed-off-by: Daehwan Jung <dh10.jung@samsung.com>
>>> ---
>>>  drivers/usb/host/Kconfig       |   9 +
>>>  drivers/usb/host/Makefile      |   1 +
>>>  drivers/usb/host/xhci-exynos.c | 982 +++++++++++++++++++++++++++++++++
>>>  drivers/usb/host/xhci-exynos.h |  63 +++
>>>  4 files changed, 1055 insertions(+)
>>>  create mode 100644 drivers/usb/host/xhci-exynos.c
>>>  create mode 100644 drivers/usb/host/xhci-exynos.h
>>>
>>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>>> index 57ca5f97a3dc..850e6b71fac5 100644
>>> --- a/drivers/usb/host/Kconfig
>>> +++ b/drivers/usb/host/Kconfig
>>> @@ -783,3 +783,12 @@ config USB_XEN_HCD
>>>  	  by the Xen host (usually Dom0).
>>>  	  Only needed if the kernel is running in a Xen guest and generic
>>>  	  access to a USB device is needed.
>>> +
>>> +config USB_XHCI_EXYNOS
>>> +	tristate "XHCI support for Samsung Exynos SoC Series"
>>> +	depends on ARCH_EXYNOS || COMPILE_TEST
>>> +	help
>>> +	  Enable support for the Samsung Exynos SOC's on-chip XHCI
>>> +	  controller.
>>> +
>>> +	  If unsure, say N.
>>> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
>>> index 2948983618fb..300f22b6eb1b 100644
>>> --- a/drivers/usb/host/Makefile
>>> +++ b/drivers/usb/host/Makefile
>>> @@ -86,3 +86,4 @@ obj-$(CONFIG_USB_HCD_SSB)	+= ssb-hcd.o
>>>  obj-$(CONFIG_USB_FOTG210_HCD)	+= fotg210-hcd.o
>>>  obj-$(CONFIG_USB_MAX3421_HCD)	+= max3421-hcd.o
>>>  obj-$(CONFIG_USB_XEN_HCD)	+= xen-hcd.o
>>> +obj-$(CONFIG_USB_XHCI_EXYNOS)	+= xhci-exynos.o
>>> diff --git a/drivers/usb/host/xhci-exynos.c b/drivers/usb/host/xhci-exynos.c
>>> new file mode 100644
>>> index 000000000000..19ee21f1d024
>>> --- /dev/null
>>> +++ b/drivers/usb/host/xhci-exynos.c
>>> @@ -0,0 +1,982 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * xhci-exynos.c - xHCI host controller driver platform Bus Glue for Exynos.
>>> + *
>>> + * Copyright (C) 2022 Samsung Electronics Incorporated - http://www.samsung.com
>>> + * Author: Daehwan Jung <dh10.jung@samsung.com>
>>> + *
>>> + * A lot of code borrowed from the Linux xHCI driver.
>>> + */
>>> +#include <linux/clk.h>
>>> +#include <linux/dma-mapping.h>
>>> +#include <linux/module.h>
>>> +#include <linux/pci.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/usb/phy.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/acpi.h>
>>> +#include <linux/usb/of.h>
>>> +
>>> +#include "xhci.h"
>>> +#include "xhci-plat.h"
>>> +#include "xhci-mvebu.h"
>>> +#include "xhci-rcar.h"
>>
>> Could you explain why do you need RCAR and Marvell code in Exynos? Is it
>> even a real driver here? On what platforms this can be tested? Where are
>> the bindings?
> 
> 2 headers you said are not needed. I'm going to remove it on next
> submission. I tested on Exynos platform and it's real driver. I haven't
> made bindings.

This driver does not fit Linux development style at all. You duplicate
code instead of integrating with existing drivers. You call a driver
"Exynos" but include Marvell, Renesas code and actually do not include
Exynos related bindings. Usage of "generic-xhci" in Exynos-specific
driver looks like it is some hacking of downstream kernel. This driver
does not look at all like ready for submission.

There was entire team in DMC @Samsung and in Polish Samsung R&D devoted
for upstream Linux kernel work. There was in the past Open Source
Samsung Group. There were several folks from SoC division (Samsung LSI)
having significant experience in mainlining code (one of them recently
got as a co-maintainer of Exynos). I can point you to specific names, if
needed. All of them have huge experience in mainlining drivers, so
please reach them for help and get a training on working with upstream.
I mean, around 10 years ago Samsung was ramping up with open-source
submissions, so it was excused from not knowing the process and from
beginner's mistakes. After 10 years it's not a beginner anymore. It is
not expected to make beginner's mistakes anymore...

Best regards,
Krzysztof

Re: [PATCH v3 0/4] support USB offload feature

[Greg Kroah-Hartman]

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

Re: [PATCH v3 0/4] support USB offload feature

[Jung Daehwan]

[-- Attachment #1: Type: text/plain, Size: 1745 bytes --]

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v3 0/4] support USB offload feature

[Greg Kroah-Hartman]

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

Re: [PATCH v3 0/4] support USB offload feature

[Jung Daehwan]

[-- Attachment #1: Type: text/plain, Size: 2797 bytes --]

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
> 

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v3 0/4] support USB offload feature

[Greg Kroah-Hartman]

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

Re: [PATCH v3 0/4] support USB offload feature

[Jung Daehwan]

[-- Attachment #1: Type: text/plain, Size: 3361 bytes --]

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v3 0/4] support USB offload feature

[Krzysztof Kozlowski]

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

Re: [PATCH v3 0/4] support USB offload feature

[Jung Daehwan]

[-- Attachment #1: Type: text/plain, Size: 1180 bytes --]

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
> 


[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v3 0/4] support USB offload feature

[Krzysztof Kozlowski]

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