[PATCH v1 0/4]

[PATCH v1 0/4]

[Stefan Roese]

usb: xhci: Prepare xHCI driver for MIPS Octeon big-endian support

These patches fix a few issues, found while porting the xHCI to the MIPS
Octeon platforms. The basic issues here are:

- Endianess issues: missing cpu_to_leXX() & leXX_to_cpu() conversions
- Use physical (DMA) address for the xHCI DMA controller

These patches are the groundwork for the upcoming xHCI Octeon support
that will follow soon.

Thanks,
Stefan


Stefan Roese (4):
  usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu)
  usb: xhci: xhci_mem_init: Use cpu_to_le64() and not xhci_writeq()
  usb: usb-uclass.c: Drop le16_to_cpu() as values are already swapped
  usb: xhci: Add virt_to_phys() to support mapped platforms

 drivers/usb/host/usb-uclass.c |  8 ++++----
 drivers/usb/host/xhci-mem.c   | 30 +++++++++++++++---------------
 drivers/usb/host/xhci-ring.c  |  8 ++++----
 drivers/usb/host/xhci.c       |  3 +--
 4 files changed, 24 insertions(+), 25 deletions(-)

-- 
2.27.0

[PATCH v1 1/4] usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu)

[Stefan Roese]

While trying to use the U-Boot xHCI driver on the MIPS Octeon platform,
which is big endian, I noticed that the driver is missing a few endian
conversion calls. This patch adds these missing endian conversion
calls.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: Marek Vasut <marex@denx.de>
---

 drivers/usb/host/xhci-mem.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 2d968aafb0..bd959b4093 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -110,7 +110,7 @@ static void xhci_scratchpad_free(struct xhci_ctrl *ctrl)
 
 	ctrl->dcbaa->dev_context_ptrs[0] = 0;
 
-	free((void *)(uintptr_t)ctrl->scratchpad->sp_array[0]);
+	free((void *)le64_to_cpu(ctrl->scratchpad->sp_array[0]));
 	free(ctrl->scratchpad->sp_array);
 	free(ctrl->scratchpad);
 	ctrl->scratchpad = NULL;
@@ -225,7 +225,8 @@ static void xhci_link_segments(struct xhci_segment *prev,
 	prev->next = next;
 	if (link_trbs) {
 		val_64 = (uintptr_t)next->trbs;
-		prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr = val_64;
+		prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr =
+			cpu_to_le64(val_64);
 
 		/*
 		 * Set the last TRB in the segment to
@@ -486,7 +487,7 @@ int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id)
 	byte_64 = (uintptr_t)(virt_dev->out_ctx->bytes);
 
 	/* Point to output device context in dcbaa. */
-	ctrl->dcbaa->dev_context_ptrs[slot_id] = byte_64;
+	ctrl->dcbaa->dev_context_ptrs[slot_id] = cpu_to_le64(byte_64);
 
 	xhci_flush_cache((uintptr_t)&ctrl->dcbaa->dev_context_ptrs[slot_id],
 			 sizeof(__le64));
@@ -768,7 +769,7 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
 
 	debug("route string %x\n", route);
 #endif
-	slot_ctx->dev_info |= route;
+	slot_ctx->dev_info |= cpu_to_le32(route);
 
 	switch (speed) {
 	case USB_SPEED_SUPER:
-- 
2.27.0

[PATCH v1 1/4] usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu)

[Stefan Roese]

Hi Bin,

On 02.07.20 10:47, Stefan Roese wrote:
> While trying to use the U-Boot xHCI driver on the MIPS Octeon platform,
> which is big endian, I noticed that the driver is missing a few endian
> conversion calls. This patch adds these missing endian conversion
> calls.

Did you find the time to review this patchset with endianess corrections
and support for physical vs virtual addresses? Its in preparation for
the Octeon (MIPS big endian) xHCI support.

I would like to have this common xhci parts integrated in mainline early
in this release (if there are no issues of course), so that these
changes can get a broad testing on multiple platforms.

Thanks,
Stefan

> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Marek Vasut <marex@denx.de>
> ---
> 
>   drivers/usb/host/xhci-mem.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 2d968aafb0..bd959b4093 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -110,7 +110,7 @@ static void xhci_scratchpad_free(struct xhci_ctrl *ctrl)
>   
>   	ctrl->dcbaa->dev_context_ptrs[0] = 0;
>   
> -	free((void *)(uintptr_t)ctrl->scratchpad->sp_array[0]);
> +	free((void *)le64_to_cpu(ctrl->scratchpad->sp_array[0]));
>   	free(ctrl->scratchpad->sp_array);
>   	free(ctrl->scratchpad);
>   	ctrl->scratchpad = NULL;
> @@ -225,7 +225,8 @@ static void xhci_link_segments(struct xhci_segment *prev,
>   	prev->next = next;
>   	if (link_trbs) {
>   		val_64 = (uintptr_t)next->trbs;
> -		prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr = val_64;
> +		prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr =
> +			cpu_to_le64(val_64);
>   
>   		/*
>   		 * Set the last TRB in the segment to
> @@ -486,7 +487,7 @@ int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id)
>   	byte_64 = (uintptr_t)(virt_dev->out_ctx->bytes);
>   
>   	/* Point to output device context in dcbaa. */
> -	ctrl->dcbaa->dev_context_ptrs[slot_id] = byte_64;
> +	ctrl->dcbaa->dev_context_ptrs[slot_id] = cpu_to_le64(byte_64);
>   
>   	xhci_flush_cache((uintptr_t)&ctrl->dcbaa->dev_context_ptrs[slot_id],
>   			 sizeof(__le64));
> @@ -768,7 +769,7 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
>   
>   	debug("route string %x\n", route);
>   #endif
> -	slot_ctx->dev_info |= route;
> +	slot_ctx->dev_info |= cpu_to_le32(route);
>   
>   	switch (speed) {
>   	case USB_SPEED_SUPER:
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH v1 1/4] usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu)

[Bin Meng]

Hi Stefan,

On Thu, Jul 16, 2020 at 5:25 PM Stefan Roese <sr@denx.de> wrote:
>
> Hi Bin,
>
> On 02.07.20 10:47, Stefan Roese wrote:
> > While trying to use the U-Boot xHCI driver on the MIPS Octeon platform,
> > which is big endian, I noticed that the driver is missing a few endian
> > conversion calls. This patch adds these missing endian conversion
> > calls.
>
> Did you find the time to review this patchset with endianess corrections
> and support for physical vs virtual addresses? Its in preparation for
> the Octeon (MIPS big endian) xHCI support.

Sorry I missed this series. I will take a look.

>
> I would like to have this common xhci parts integrated in mainline early
> in this release (if there are no issues of course), so that these
> changes can get a broad testing on multiple platforms.

Regards,
Bin

[PATCH v1 1/4] usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu)

[Bin Meng]

On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
>
> While trying to use the U-Boot xHCI driver on the MIPS Octeon platform,
> which is big endian, I noticed that the driver is missing a few endian
> conversion calls. This patch adds these missing endian conversion
> calls.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Marek Vasut <marex@denx.de>
> ---
>
>  drivers/usb/host/xhci-mem.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>

Good catch! It's hard to detect these problems if we only validate
xHCI on ARM/x86 which are little-endian. Apparently there is no xHCI
on PPC, so MIPS becomes the first big endian platform using xHCI.

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH v1 1/4] usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu)

[Stefan Roese]

On 17.07.20 07:15, Bin Meng wrote:
> On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
>>
>> While trying to use the U-Boot xHCI driver on the MIPS Octeon platform,
>> which is big endian, I noticed that the driver is missing a few endian
>> conversion calls. This patch adds these missing endian conversion
>> calls.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> Cc: Bin Meng <bmeng.cn@gmail.com>
>> Cc: Marek Vasut <marex@denx.de>
>> ---
>>
>>   drivers/usb/host/xhci-mem.c | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>
> 
> Good catch! It's hard to detect these problems if we only validate
> xHCI on ARM/x86 which are little-endian. Apparently there is no xHCI
> on PPC, so MIPS becomes the first big endian platform using xHCI.

Yes, I was also astonished that the xHCI driver has only been used by
little-endian platforms so far. I did expect that at least some PPC
platforms supported this code. Apparently this is not the case.

> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Thanks,
Stefan

[PATCH v1 1/4] usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu)

[Bin Meng]

Hi Stefan,

On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
>
> While trying to use the U-Boot xHCI driver on the MIPS Octeon platform,
> which is big endian, I noticed that the driver is missing a few endian
> conversion calls. This patch adds these missing endian conversion
> calls.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Marek Vasut <marex@denx.de>
> ---
>
>  drivers/usb/host/xhci-mem.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 2d968aafb0..bd959b4093 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -110,7 +110,7 @@ static void xhci_scratchpad_free(struct xhci_ctrl *ctrl)
>
>         ctrl->dcbaa->dev_context_ptrs[0] = 0;
>
> -       free((void *)(uintptr_t)ctrl->scratchpad->sp_array[0]);
> +       free((void *)le64_to_cpu(ctrl->scratchpad->sp_array[0]));

There is a build warning for this:

drivers/usb/host/xhci-mem.c: In function 'xhci_scratchpad_free':
drivers/usb/host/xhci-mem.c:113:7: warning: cast to pointer from
integer of different size [-Wint-to-pointer-cast]
  free((void *)le64_to_cpu(ctrl->scratchpad->sp_array[0]));
       ^

>         free(ctrl->scratchpad->sp_array);
>         free(ctrl->scratchpad);
>         ctrl->scratchpad = NULL;
> @@ -225,7 +225,8 @@ static void xhci_link_segments(struct xhci_segment *prev,
>         prev->next = next;
>         if (link_trbs) {
>                 val_64 = (uintptr_t)next->trbs;
> -               prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr = val_64;
> +               prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr =
> +                       cpu_to_le64(val_64);
>
>                 /*
>                  * Set the last TRB in the segment to
> @@ -486,7 +487,7 @@ int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id)
>         byte_64 = (uintptr_t)(virt_dev->out_ctx->bytes);
>
>         /* Point to output device context in dcbaa. */
> -       ctrl->dcbaa->dev_context_ptrs[slot_id] = byte_64;
> +       ctrl->dcbaa->dev_context_ptrs[slot_id] = cpu_to_le64(byte_64);
>
>         xhci_flush_cache((uintptr_t)&ctrl->dcbaa->dev_context_ptrs[slot_id],
>                          sizeof(__le64));
> @@ -768,7 +769,7 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
>
>         debug("route string %x\n", route);
>  #endif
> -       slot_ctx->dev_info |= route;
> +       slot_ctx->dev_info |= cpu_to_le32(route);
>
>         switch (speed) {
>         case USB_SPEED_SUPER:
> --

Test results on Minnowmax which has one USB 2.0 port and one 3.0 port:

USB 2.0 flash drive inserted to USB 2.0 port: recognized, read/write OK
USB 2.0 flash drive inserted to USB 3.0 port: recognized, read/write OK

USB 3.0 flash drive inserted to USB 2.0 port: recognized, read/write OK
USB 3.0 flash drive inserted to USB 3.0 port: recognized, read/write OK

USB 2.0 flash drive connected to a USB 3.0 HUB inserted to USB 2.0
port: recognized, read/write OK
USB 2.0 flash drive connected to a USB 3.0 HUB inserted to USB 3.0
port: recognized, read/write OK

USB 3.0 flash drive connected to a USB 3.0 HUB inserted to USB 2.0
port: recognized, read/write OK
USB 3.0 flash drive connected to a USB 3.0 HUB inserted to USB 3.0
port: recognized, read/write OK

Tested-by: Bin Meng <bmeng.cn@gmail.com>

Regards,
Bin

[PATCH v1 1/4] usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu)

[Stefan Roese]

Hi Bin,

On 17.07.20 13:18, Bin Meng wrote:
> Hi Stefan,
> 
> On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
>>
>> While trying to use the U-Boot xHCI driver on the MIPS Octeon platform,
>> which is big endian, I noticed that the driver is missing a few endian
>> conversion calls. This patch adds these missing endian conversion
>> calls.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> Cc: Bin Meng <bmeng.cn@gmail.com>
>> Cc: Marek Vasut <marex@denx.de>
>> ---
>>
>>   drivers/usb/host/xhci-mem.c | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
>> index 2d968aafb0..bd959b4093 100644
>> --- a/drivers/usb/host/xhci-mem.c
>> +++ b/drivers/usb/host/xhci-mem.c
>> @@ -110,7 +110,7 @@ static void xhci_scratchpad_free(struct xhci_ctrl *ctrl)
>>
>>          ctrl->dcbaa->dev_context_ptrs[0] = 0;
>>
>> -       free((void *)(uintptr_t)ctrl->scratchpad->sp_array[0]);
>> +       free((void *)le64_to_cpu(ctrl->scratchpad->sp_array[0]));
> 
> There is a build warning for this:
> 
> drivers/usb/host/xhci-mem.c: In function 'xhci_scratchpad_free':
> drivers/usb/host/xhci-mem.c:113:7: warning: cast to pointer from
> integer of different size [-Wint-to-pointer-cast]
>    free((void *)le64_to_cpu(ctrl->scratchpad->sp_array[0]));
>         ^

So we need the (uintptr_t) here as well?

   free((void *)(uintptr_t)le64_to_cpu(ctrl->scratchpad->sp_array[0]));

Should I send v2 for this patch?

>>          free(ctrl->scratchpad->sp_array);
>>          free(ctrl->scratchpad);
>>          ctrl->scratchpad = NULL;
>> @@ -225,7 +225,8 @@ static void xhci_link_segments(struct xhci_segment *prev,
>>          prev->next = next;
>>          if (link_trbs) {
>>                  val_64 = (uintptr_t)next->trbs;
>> -               prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr = val_64;
>> +               prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr =
>> +                       cpu_to_le64(val_64);
>>
>>                  /*
>>                   * Set the last TRB in the segment to
>> @@ -486,7 +487,7 @@ int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id)
>>          byte_64 = (uintptr_t)(virt_dev->out_ctx->bytes);
>>
>>          /* Point to output device context in dcbaa. */
>> -       ctrl->dcbaa->dev_context_ptrs[slot_id] = byte_64;
>> +       ctrl->dcbaa->dev_context_ptrs[slot_id] = cpu_to_le64(byte_64);
>>
>>          xhci_flush_cache((uintptr_t)&ctrl->dcbaa->dev_context_ptrs[slot_id],
>>                           sizeof(__le64));
>> @@ -768,7 +769,7 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
>>
>>          debug("route string %x\n", route);
>>   #endif
>> -       slot_ctx->dev_info |= route;
>> +       slot_ctx->dev_info |= cpu_to_le32(route);
>>
>>          switch (speed) {
>>          case USB_SPEED_SUPER:
>> --
> 
> Test results on Minnowmax which has one USB 2.0 port and one 3.0 port:
> 
> USB 2.0 flash drive inserted to USB 2.0 port: recognized, read/write OK
> USB 2.0 flash drive inserted to USB 3.0 port: recognized, read/write OK
> 
> USB 3.0 flash drive inserted to USB 2.0 port: recognized, read/write OK
> USB 3.0 flash drive inserted to USB 3.0 port: recognized, read/write OK
> 
> USB 2.0 flash drive connected to a USB 3.0 HUB inserted to USB 2.0
> port: recognized, read/write OK
> USB 2.0 flash drive connected to a USB 3.0 HUB inserted to USB 3.0
> port: recognized, read/write OK
> 
> USB 3.0 flash drive connected to a USB 3.0 HUB inserted to USB 2.0
> port: recognized, read/write OK
> USB 3.0 flash drive connected to a USB 3.0 HUB inserted to USB 3.0
> port: recognized, read/write OK
> 
> Tested-by: Bin Meng <bmeng.cn@gmail.com>

Very good. :)

Thanks,
Stefan

[PATCH v1 1/4] usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu)

[Bin Meng]

Hi Stefan,

On Fri, Jul 17, 2020 at 7:34 PM Stefan Roese <sr@denx.de> wrote:
>
> Hi Bin,
>
> On 17.07.20 13:18, Bin Meng wrote:
> > Hi Stefan,
> >
> > On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
> >>
> >> While trying to use the U-Boot xHCI driver on the MIPS Octeon platform,
> >> which is big endian, I noticed that the driver is missing a few endian
> >> conversion calls. This patch adds these missing endian conversion
> >> calls.
> >>
> >> Signed-off-by: Stefan Roese <sr@denx.de>
> >> Cc: Bin Meng <bmeng.cn@gmail.com>
> >> Cc: Marek Vasut <marex@denx.de>
> >> ---
> >>
> >>   drivers/usb/host/xhci-mem.c | 9 +++++----
> >>   1 file changed, 5 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> >> index 2d968aafb0..bd959b4093 100644
> >> --- a/drivers/usb/host/xhci-mem.c
> >> +++ b/drivers/usb/host/xhci-mem.c
> >> @@ -110,7 +110,7 @@ static void xhci_scratchpad_free(struct xhci_ctrl *ctrl)
> >>
> >>          ctrl->dcbaa->dev_context_ptrs[0] = 0;
> >>
> >> -       free((void *)(uintptr_t)ctrl->scratchpad->sp_array[0]);
> >> +       free((void *)le64_to_cpu(ctrl->scratchpad->sp_array[0]));
> >
> > There is a build warning for this:
> >
> > drivers/usb/host/xhci-mem.c: In function 'xhci_scratchpad_free':
> > drivers/usb/host/xhci-mem.c:113:7: warning: cast to pointer from
> > integer of different size [-Wint-to-pointer-cast]
> >    free((void *)le64_to_cpu(ctrl->scratchpad->sp_array[0]));
> >         ^
>
> So we need the (uintptr_t) here as well?
>
>    free((void *)(uintptr_t)le64_to_cpu(ctrl->scratchpad->sp_array[0]));

I think so.

>
> Should I send v2 for this patch?

Yes, please.

>
> >>          free(ctrl->scratchpad->sp_array);
> >>          free(ctrl->scratchpad);
> >>          ctrl->scratchpad = NULL;
> >> @@ -225,7 +225,8 @@ static void xhci_link_segments(struct xhci_segment *prev,
> >>          prev->next = next;
> >>          if (link_trbs) {
> >>                  val_64 = (uintptr_t)next->trbs;
> >> -               prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr = val_64;
> >> +               prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr =
> >> +                       cpu_to_le64(val_64);
> >>
> >>                  /*
> >>                   * Set the last TRB in the segment to
> >> @@ -486,7 +487,7 @@ int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id)
> >>          byte_64 = (uintptr_t)(virt_dev->out_ctx->bytes);
> >>
> >>          /* Point to output device context in dcbaa. */
> >> -       ctrl->dcbaa->dev_context_ptrs[slot_id] = byte_64;
> >> +       ctrl->dcbaa->dev_context_ptrs[slot_id] = cpu_to_le64(byte_64);
> >>
> >>          xhci_flush_cache((uintptr_t)&ctrl->dcbaa->dev_context_ptrs[slot_id],
> >>                           sizeof(__le64));
> >> @@ -768,7 +769,7 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
> >>
> >>          debug("route string %x\n", route);
> >>   #endif
> >> -       slot_ctx->dev_info |= route;
> >> +       slot_ctx->dev_info |= cpu_to_le32(route);
> >>
> >>          switch (speed) {
> >>          case USB_SPEED_SUPER:
> >> --
> >
> > Test results on Minnowmax which has one USB 2.0 port and one 3.0 port:
> >
> > USB 2.0 flash drive inserted to USB 2.0 port: recognized, read/write OK
> > USB 2.0 flash drive inserted to USB 3.0 port: recognized, read/write OK
> >
> > USB 3.0 flash drive inserted to USB 2.0 port: recognized, read/write OK
> > USB 3.0 flash drive inserted to USB 3.0 port: recognized, read/write OK
> >
> > USB 2.0 flash drive connected to a USB 3.0 HUB inserted to USB 2.0
> > port: recognized, read/write OK
> > USB 2.0 flash drive connected to a USB 3.0 HUB inserted to USB 3.0
> > port: recognized, read/write OK
> >
> > USB 3.0 flash drive connected to a USB 3.0 HUB inserted to USB 2.0
> > port: recognized, read/write OK
> > USB 3.0 flash drive connected to a USB 3.0 HUB inserted to USB 3.0
> > port: recognized, read/write OK
> >
> > Tested-by: Bin Meng <bmeng.cn@gmail.com>
>
> Very good. :)

Regards,
Bin

[PATCH v2 1/4] usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu)

[Stefan Roese]

While trying to use the U-Boot xHCI driver on the MIPS Octeon platform,
which is big endian, I noticed that the driver is missing a few endian
conversion calls. This patch adds these missing endian conversion
calls.

Signed-off-by: Stefan Roese <sr@denx.de>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: Marek Vasut <marex@denx.de>
---
v2:
- Add missing (uintptr_t) cast to remove compile time warning

 drivers/usb/host/xhci-mem.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 2d968aafb0..7d55944765 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -110,7 +110,7 @@ static void xhci_scratchpad_free(struct xhci_ctrl *ctrl)
 
 	ctrl->dcbaa->dev_context_ptrs[0] = 0;
 
-	free((void *)(uintptr_t)ctrl->scratchpad->sp_array[0]);
+	free((void *)(uintptr_t)le64_to_cpu(ctrl->scratchpad->sp_array[0]));
 	free(ctrl->scratchpad->sp_array);
 	free(ctrl->scratchpad);
 	ctrl->scratchpad = NULL;
@@ -225,7 +225,8 @@ static void xhci_link_segments(struct xhci_segment *prev,
 	prev->next = next;
 	if (link_trbs) {
 		val_64 = (uintptr_t)next->trbs;
-		prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr = val_64;
+		prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr =
+			cpu_to_le64(val_64);
 
 		/*
 		 * Set the last TRB in the segment to
@@ -486,7 +487,7 @@ int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id)
 	byte_64 = (uintptr_t)(virt_dev->out_ctx->bytes);
 
 	/* Point to output device context in dcbaa. */
-	ctrl->dcbaa->dev_context_ptrs[slot_id] = byte_64;
+	ctrl->dcbaa->dev_context_ptrs[slot_id] = cpu_to_le64(byte_64);
 
 	xhci_flush_cache((uintptr_t)&ctrl->dcbaa->dev_context_ptrs[slot_id],
 			 sizeof(__le64));
@@ -768,7 +769,7 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
 
 	debug("route string %x\n", route);
 #endif
-	slot_ctx->dev_info |= route;
+	slot_ctx->dev_info |= cpu_to_le32(route);
 
 	switch (speed) {
 	case USB_SPEED_SUPER:
-- 
2.27.0

[PATCH v2 1/4] usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu)

[Bin Meng]

Hi Stefan,

On Fri, Jul 17, 2020 at 10:04 PM Stefan Roese <sr@denx.de> wrote:
>
> While trying to use the U-Boot xHCI driver on the MIPS Octeon platform,
> which is big endian, I noticed that the driver is missing a few endian
> conversion calls. This patch adds these missing endian conversion
> calls.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Marek Vasut <marex@denx.de>
> ---
> v2:
> - Add missing (uintptr_t) cast to remove compile time warning
>
>  drivers/usb/host/xhci-mem.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)

Tested-by: Bin Meng <bmeng.cn@gmail.com>

Only this patch is sent as v2?

Regards,
Bin

[PATCH v2 1/4] usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu)

[Stefan Roese]

Hi Bin,

On 17.07.20 16:14, Bin Meng wrote:
> Hi Stefan,
> 
> On Fri, Jul 17, 2020 at 10:04 PM Stefan Roese <sr@denx.de> wrote:
>>
>> While trying to use the U-Boot xHCI driver on the MIPS Octeon platform,
>> which is big endian, I noticed that the driver is missing a few endian
>> conversion calls. This patch adds these missing endian conversion
>> calls.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
>> Cc: Bin Meng <bmeng.cn@gmail.com>
>> Cc: Marek Vasut <marex@denx.de>
>> ---
>> v2:
>> - Add missing (uintptr_t) cast to remove compile time warning
>>
>>   drivers/usb/host/xhci-mem.c | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> Tested-by: Bin Meng <bmeng.cn@gmail.com>
> 
> Only this patch is sent as v2?

Yes. I didn't want to pollute the list with unneeded patches. If you
prefer a complete patchset in the new version, then I can of course
do as as well.

Thanks,
Stefan

[PATCH v2 1/4] usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu)

[Bin Meng]

Hi Stefan,

On Sat, Jul 18, 2020 at 2:22 AM Stefan Roese <sr@denx.de> wrote:
>
> Hi Bin,
>
> On 17.07.20 16:14, Bin Meng wrote:
> > Hi Stefan,
> >
> > On Fri, Jul 17, 2020 at 10:04 PM Stefan Roese <sr@denx.de> wrote:
> >>
> >> While trying to use the U-Boot xHCI driver on the MIPS Octeon platform,
> >> which is big endian, I noticed that the driver is missing a few endian
> >> conversion calls. This patch adds these missing endian conversion
> >> calls.
> >>
> >> Signed-off-by: Stefan Roese <sr@denx.de>
> >> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> >> Cc: Bin Meng <bmeng.cn@gmail.com>
> >> Cc: Marek Vasut <marex@denx.de>
> >> ---
> >> v2:
> >> - Add missing (uintptr_t) cast to remove compile time warning
> >>
> >>   drivers/usb/host/xhci-mem.c | 9 +++++----
> >>   1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > Tested-by: Bin Meng <bmeng.cn@gmail.com>
> >
> > Only this patch is sent as v2?
>
> Yes. I didn't want to pollute the list with unneeded patches. If you
> prefer a complete patchset in the new version, then I can of course
> do as as well.

IMHO it would be good for maintainers to apply this whole series from
patchwork in a batch if we send the complete series.

Regards,
Bin

[PATCH v2 1/4] usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu)

[Stefan Roese]

Hi Bin,

On 20.07.20 03:48, Bin Meng wrote:
> Hi Stefan,
> 
> On Sat, Jul 18, 2020 at 2:22 AM Stefan Roese <sr@denx.de> wrote:
>>
>> Hi Bin,
>>
>> On 17.07.20 16:14, Bin Meng wrote:
>>> Hi Stefan,
>>>
>>> On Fri, Jul 17, 2020 at 10:04 PM Stefan Roese <sr@denx.de> wrote:
>>>>
>>>> While trying to use the U-Boot xHCI driver on the MIPS Octeon platform,
>>>> which is big endian, I noticed that the driver is missing a few endian
>>>> conversion calls. This patch adds these missing endian conversion
>>>> calls.
>>>>
>>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
>>>> Cc: Bin Meng <bmeng.cn@gmail.com>
>>>> Cc: Marek Vasut <marex@denx.de>
>>>> ---
>>>> v2:
>>>> - Add missing (uintptr_t) cast to remove compile time warning
>>>>
>>>>    drivers/usb/host/xhci-mem.c | 9 +++++----
>>>>    1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> Tested-by: Bin Meng <bmeng.cn@gmail.com>
>>>
>>> Only this patch is sent as v2?
>>
>> Yes. I didn't want to pollute the list with unneeded patches. If you
>> prefer a complete patchset in the new version, then I can of course
>> do as as well.
> 
> IMHO it would be good for maintainers to apply this whole series from
> patchwork in a batch if we send the complete series.

No problem. v2 series will be out shortly.

Thanks,
Stefan

[PATCH v1 2/4] usb: xhci: xhci_mem_init: Use cpu_to_le64() and not xhci_writeq()

[Stefan Roese]

xhci_writeq() makes the CPU->LE swapping only when addressing registers
in the xHCI controller address range and not in the local memory (RAM).
We need to use cpu_to_le64() here to ensure that the conversion is done
correctly.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: Marek Vasut <marex@denx.de>
---

 drivers/usb/host/xhci-mem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index bd959b4093..3b805ecb9e 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -562,7 +562,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
 		trb_64 = 0;
 		trb_64 = (uintptr_t)seg->trbs;
 		struct xhci_erst_entry *entry = &ctrl->erst.entries[val];
-		xhci_writeq(&entry->seg_addr, trb_64);
+		entry->seg_addr = cpu_to_le64(trb_64);
 		entry->seg_size = cpu_to_le32(TRBS_PER_SEGMENT);
 		entry->rsvd = 0;
 		seg = seg->next;
-- 
2.27.0

[PATCH v1 2/4] usb: xhci: xhci_mem_init: Use cpu_to_le64() and not xhci_writeq()

[Bin Meng]

Hi Stefan,

On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
>
> xhci_writeq() makes the CPU->LE swapping only when addressing registers
> in the xHCI controller address range and not in the local memory (RAM).

Is the above behavior exposed by the MIPS platform's writel()?

> We need to use cpu_to_le64() here to ensure that the conversion is done
> correctly.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Marek Vasut <marex@denx.de>
> ---
>
>  drivers/usb/host/xhci-mem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index bd959b4093..3b805ecb9e 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -562,7 +562,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
>                 trb_64 = 0;
>                 trb_64 = (uintptr_t)seg->trbs;
>                 struct xhci_erst_entry *entry = &ctrl->erst.entries[val];
> -               xhci_writeq(&entry->seg_addr, trb_64);
> +               entry->seg_addr = cpu_to_le64(trb_64);
>                 entry->seg_size = cpu_to_le32(TRBS_PER_SEGMENT);
>                 entry->rsvd = 0;
>                 seg = seg->next;

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Regards,
Bin

[PATCH v1 2/4] usb: xhci: xhci_mem_init: Use cpu_to_le64() and not xhci_writeq()

[Stefan Roese]

On 17.07.20 07:24, Bin Meng wrote:
> Hi Stefan,
> 
> On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
>>
>> xhci_writeq() makes the CPU->LE swapping only when addressing registers
>> in the xHCI controller address range and not in the local memory (RAM).
> 
> Is the above behavior exposed by the MIPS platform's writel()?

Not sure what you mean with this. Without this patch, xhci_writeq()
will not swap on Octeon MIPS, as the destination address is located
in local memory (DDR). Using the xhci_read/write accessor functions
should be restricted to accessing the controller registers.

BTW: The Octeon MIPS writel will swap to little-endian, when the
location is in the xHCI controller address space (and PCI etc). This
support for selective swapping is not pushed into mainline yet. I
will send it in some follow up patches.

Thanks,
Stefan

>> We need to use cpu_to_le64() here to ensure that the conversion is done
>> correctly.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> Cc: Bin Meng <bmeng.cn@gmail.com>
>> Cc: Marek Vasut <marex@denx.de>
>> ---
>>
>>   drivers/usb/host/xhci-mem.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
>> index bd959b4093..3b805ecb9e 100644
>> --- a/drivers/usb/host/xhci-mem.c
>> +++ b/drivers/usb/host/xhci-mem.c
>> @@ -562,7 +562,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
>>                  trb_64 = 0;
>>                  trb_64 = (uintptr_t)seg->trbs;
>>                  struct xhci_erst_entry *entry = &ctrl->erst.entries[val];
>> -               xhci_writeq(&entry->seg_addr, trb_64);
>> +               entry->seg_addr = cpu_to_le64(trb_64);
>>                  entry->seg_size = cpu_to_le32(TRBS_PER_SEGMENT);
>>                  entry->rsvd = 0;
>>                  seg = seg->next;
> 
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> 
> Regards,
> Bin
> 

Thanks,
Stefan

[PATCH v1 2/4] usb: xhci: xhci_mem_init: Use cpu_to_le64() and not xhci_writeq()

[Bin Meng]

Hi Stefan,

On Fri, Jul 17, 2020 at 6:04 PM Stefan Roese <sr@denx.de> wrote:
>
> On 17.07.20 07:24, Bin Meng wrote:
> > Hi Stefan,
> >
> > On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
> >>
> >> xhci_writeq() makes the CPU->LE swapping only when addressing registers
> >> in the xHCI controller address range and not in the local memory (RAM).
> >
> > Is the above behavior exposed by the MIPS platform's writel()?
>
> Not sure what you mean with this. Without this patch, xhci_writeq()
> will not swap on Octeon MIPS, as the destination address is located
> in local memory (DDR).

I wonder why xhci_writeq() does not swap? Is this due to the writel()
implementation on Octeon MIPS?

> Using the xhci_read/write accessor functions
> should be restricted to accessing the controller registers.

Yes, this is the supposed usage that xhci_read/write should be called
to operate on xHCI registers. However my question was why
xhci_read/write does not swap even it is called on memory space, hence
the writel() question.

>
> BTW: The Octeon MIPS writel will swap to little-endian, when the
> location is in the xHCI controller address space (and PCI etc). This
> support for selective swapping is not pushed into mainline yet. I
> will send it in some follow up patches.
>

Regards,
Bin

[PATCH v1 2/4] usb: xhci: xhci_mem_init: Use cpu_to_le64() and not xhci_writeq()

[Stefan Roese]

Hi Bin,

On 17.07.20 12:09, Bin Meng wrote:
> Hi Stefan,
> 
> On Fri, Jul 17, 2020 at 6:04 PM Stefan Roese <sr@denx.de> wrote:
>>
>> On 17.07.20 07:24, Bin Meng wrote:
>>> Hi Stefan,
>>>
>>> On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
>>>>
>>>> xhci_writeq() makes the CPU->LE swapping only when addressing registers
>>>> in the xHCI controller address range and not in the local memory (RAM).
>>>
>>> Is the above behavior exposed by the MIPS platform's writel()?
>>
>> Not sure what you mean with this. Without this patch, xhci_writeq()
>> will not swap on Octeon MIPS, as the destination address is located
>> in local memory (DDR).
> 
> I wonder why xhci_writeq() does not swap? Is this due to the writel()
> implementation on Octeon MIPS?

Ah, okay. Please see my comment below for this. Here again:

BTW: The Octeon MIPS writel will swap to little-endian, when the
location is in the xHCI controller address space (and PCI etc). This
support for selective swapping is not pushed into mainline yet. I
will send it in some follow up patches.

So to answer your question: writel will not swap when addressing
local memory.

>> Using the xhci_read/write accessor functions
>> should be restricted to accessing the controller registers.
> 
> Yes, this is the supposed usage that xhci_read/write should be called
> to operate on xHCI registers. However my question was why
> xhci_read/write does not swap even it is called on memory space, hence
> the writel() question.
> 
>>
>> BTW: The Octeon MIPS writel will swap to little-endian, when the
>> location is in the xHCI controller address space (and PCI etc). This
>> support for selective swapping is not pushed into mainline yet. I
>> will send it in some follow up patches.
>>
> 
> Regards,
> Bin
> 

Thanks,
Stefan

[PATCH v1 2/4] usb: xhci: xhci_mem_init: Use cpu_to_le64() and not xhci_writeq()

[Bin Meng]

Hi Stefan,

On Fri, Jul 17, 2020 at 6:11 PM Stefan Roese <sr@denx.de> wrote:
>
> Hi Bin,
>
> On 17.07.20 12:09, Bin Meng wrote:
> > Hi Stefan,
> >
> > On Fri, Jul 17, 2020 at 6:04 PM Stefan Roese <sr@denx.de> wrote:
> >>
> >> On 17.07.20 07:24, Bin Meng wrote:
> >>> Hi Stefan,
> >>>
> >>> On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
> >>>>
> >>>> xhci_writeq() makes the CPU->LE swapping only when addressing registers
> >>>> in the xHCI controller address range and not in the local memory (RAM).
> >>>
> >>> Is the above behavior exposed by the MIPS platform's writel()?
> >>
> >> Not sure what you mean with this. Without this patch, xhci_writeq()
> >> will not swap on Octeon MIPS, as the destination address is located
> >> in local memory (DDR).
> >
> > I wonder why xhci_writeq() does not swap? Is this due to the writel()
> > implementation on Octeon MIPS?
>
> Ah, okay. Please see my comment below for this. Here again:
>
> BTW: The Octeon MIPS writel will swap to little-endian, when the
> location is in the xHCI controller address space (and PCI etc). This
> support for selective swapping is not pushed into mainline yet. I
> will send it in some follow up patches.
>
> So to answer your question: writel will not swap when addressing
> local memory.

Thanks. That explains.

>
> >> Using the xhci_read/write accessor functions
> >> should be restricted to accessing the controller registers.
> >
> > Yes, this is the supposed usage that xhci_read/write should be called
> > to operate on xHCI registers. However my question was why
> > xhci_read/write does not swap even it is called on memory space, hence
> > the writel() question.
> >

Regards,
Bin

[PATCH v1 2/4] usb: xhci: xhci_mem_init: Use cpu_to_le64() and not xhci_writeq()

[Bin Meng]

On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
>
> xhci_writeq() makes the CPU->LE swapping only when addressing registers
> in the xHCI controller address range and not in the local memory (RAM).
> We need to use cpu_to_le64() here to ensure that the conversion is done
> correctly.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Marek Vasut <marex@denx.de>
> ---
>
>  drivers/usb/host/xhci-mem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Tested-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH v1 3/4] usb: usb-uclass.c: Drop le16_to_cpu() as values are already swapped

[Stefan Roese]

These values are already swapped to CPU endianess, so swapping them
again is a bug. Let's remove the swap here instead.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: Marek Vasut <marex@denx.de>
---

 drivers/usb/host/usb-uclass.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index cb79dfbbd5..aa08d4ffc6 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -416,21 +416,21 @@ static int usb_match_device(const struct usb_device_descriptor *desc,
 			    const struct usb_device_id *id)
 {
 	if ((id->match_flags & USB_DEVICE_ID_MATCH_VENDOR) &&
-	    id->idVendor != le16_to_cpu(desc->idVendor))
+	    id->idVendor != desc->idVendor)
 		return 0;
 
 	if ((id->match_flags & USB_DEVICE_ID_MATCH_PRODUCT) &&
-	    id->idProduct != le16_to_cpu(desc->idProduct))
+	    id->idProduct != desc->idProduct)
 		return 0;
 
 	/* No need to test id->bcdDevice_lo != 0, since 0 is never
 	   greater than any unsigned number. */
 	if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_LO) &&
-	    (id->bcdDevice_lo > le16_to_cpu(desc->bcdDevice)))
+	    (id->bcdDevice_lo > desc->bcdDevice))
 		return 0;
 
 	if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_HI) &&
-	    (id->bcdDevice_hi < le16_to_cpu(desc->bcdDevice)))
+	    (id->bcdDevice_hi < desc->bcdDevice))
 		return 0;
 
 	if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_CLASS) &&
-- 
2.27.0

[PATCH v1 3/4] usb: usb-uclass.c: Drop le16_to_cpu() as values are already swapped

[Bin Meng]

Hi Stefan,

On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
>
> These values are already swapped to CPU endianess, so swapping them

Can you please add more details as to when these values are swapped? I
assume this is inside usb_select_config() which is called before this
function is called?

But I wonder how this code ever worked on ARM/x86?

> again is a bug. Let's remove the swap here instead.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Marek Vasut <marex@denx.de>
> ---
>
>  drivers/usb/host/usb-uclass.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
> index cb79dfbbd5..aa08d4ffc6 100644
> --- a/drivers/usb/host/usb-uclass.c
> +++ b/drivers/usb/host/usb-uclass.c
> @@ -416,21 +416,21 @@ static int usb_match_device(const struct usb_device_descriptor *desc,
>                             const struct usb_device_id *id)
>  {
>         if ((id->match_flags & USB_DEVICE_ID_MATCH_VENDOR) &&
> -           id->idVendor != le16_to_cpu(desc->idVendor))
> +           id->idVendor != desc->idVendor)
>                 return 0;
>
>         if ((id->match_flags & USB_DEVICE_ID_MATCH_PRODUCT) &&
> -           id->idProduct != le16_to_cpu(desc->idProduct))
> +           id->idProduct != desc->idProduct)
>                 return 0;
>
>         /* No need to test id->bcdDevice_lo != 0, since 0 is never
>            greater than any unsigned number. */
>         if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_LO) &&
> -           (id->bcdDevice_lo > le16_to_cpu(desc->bcdDevice)))
> +           (id->bcdDevice_lo > desc->bcdDevice))
>                 return 0;
>
>         if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_HI) &&
> -           (id->bcdDevice_hi < le16_to_cpu(desc->bcdDevice)))
> +           (id->bcdDevice_hi < desc->bcdDevice))
>                 return 0;
>
>         if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_CLASS) &&

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Regards,
Bin

[PATCH v1 3/4] usb: usb-uclass.c: Drop le16_to_cpu() as values are already swapped

[Stefan Roese]

On 17.07.20 07:33, Bin Meng wrote:
> Hi Stefan,
> 
> On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
>>
>> These values are already swapped to CPU endianess, so swapping them
> 
> Can you please add more details as to when these values are swapped? I
> assume this is inside usb_select_config() which is called before this
> function is called?

Yes. Its swapped exactly here:

int usb_select_config(struct usb_device *dev)
{
	unsigned char *tmpbuf = NULL;
	int err;

	err = get_descriptor_len(dev, USB_DT_DEVICE_SIZE, USB_DT_DEVICE_SIZE);
	if (err)
		return err;

	/* correct le values */
	le16_to_cpus(&dev->descriptor.bcdUSB);
	le16_to_cpus(&dev->descriptor.idVendor);
	le16_to_cpus(&dev->descriptor.idProduct);
	le16_to_cpus(&dev->descriptor.bcdDevice);

> But I wonder how this code ever worked on ARM/x86?

On ARM/x86, the little-endian swapping is a no-op. So it doesn't matter
if you swap once or twice or... ;)

>> again is a bug. Let's remove the swap here instead.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> Cc: Bin Meng <bmeng.cn@gmail.com>
>> Cc: Marek Vasut <marex@denx.de>
>> ---
>>
>>   drivers/usb/host/usb-uclass.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
>> index cb79dfbbd5..aa08d4ffc6 100644
>> --- a/drivers/usb/host/usb-uclass.c
>> +++ b/drivers/usb/host/usb-uclass.c
>> @@ -416,21 +416,21 @@ static int usb_match_device(const struct usb_device_descriptor *desc,
>>                              const struct usb_device_id *id)
>>   {
>>          if ((id->match_flags & USB_DEVICE_ID_MATCH_VENDOR) &&
>> -           id->idVendor != le16_to_cpu(desc->idVendor))
>> +           id->idVendor != desc->idVendor)
>>                  return 0;
>>
>>          if ((id->match_flags & USB_DEVICE_ID_MATCH_PRODUCT) &&
>> -           id->idProduct != le16_to_cpu(desc->idProduct))
>> +           id->idProduct != desc->idProduct)
>>                  return 0;
>>
>>          /* No need to test id->bcdDevice_lo != 0, since 0 is never
>>             greater than any unsigned number. */
>>          if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_LO) &&
>> -           (id->bcdDevice_lo > le16_to_cpu(desc->bcdDevice)))
>> +           (id->bcdDevice_lo > desc->bcdDevice))
>>                  return 0;
>>
>>          if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_HI) &&
>> -           (id->bcdDevice_hi < le16_to_cpu(desc->bcdDevice)))
>> +           (id->bcdDevice_hi < desc->bcdDevice))
>>                  return 0;
>>
>>          if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_CLASS) &&
> 
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> 
> Regards,
> Bin

Thanks,
Stefan

[PATCH v1 3/4] usb: usb-uclass.c: Drop le16_to_cpu() as values are already swapped

[Bin Meng]

Hi Stefan,

On Fri, Jul 17, 2020 at 6:08 PM Stefan Roese <sr@denx.de> wrote:
>
> On 17.07.20 07:33, Bin Meng wrote:
> > Hi Stefan,
> >
> > On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
> >>
> >> These values are already swapped to CPU endianess, so swapping them
> >
> > Can you please add more details as to when these values are swapped? I
> > assume this is inside usb_select_config() which is called before this
> > function is called?
>
> Yes. Its swapped exactly here:
>
> int usb_select_config(struct usb_device *dev)
> {
>         unsigned char *tmpbuf = NULL;
>         int err;
>
>         err = get_descriptor_len(dev, USB_DT_DEVICE_SIZE, USB_DT_DEVICE_SIZE);
>         if (err)
>                 return err;
>
>         /* correct le values */
>         le16_to_cpus(&dev->descriptor.bcdUSB);
>         le16_to_cpus(&dev->descriptor.idVendor);
>         le16_to_cpus(&dev->descriptor.idProduct);
>         le16_to_cpus(&dev->descriptor.bcdDevice);
>
> > But I wonder how this code ever worked on ARM/x86?
>
> On ARM/x86, the little-endian swapping is a no-op. So it doesn't matter
> if you swap once or twice or... ;)
>

Ah, yes! I misread this as an unconditional swap ...

> >> again is a bug. Let's remove the swap here instead.
> >>
> >> Signed-off-by: Stefan Roese <sr@denx.de>
> >> Cc: Bin Meng <bmeng.cn@gmail.com>
> >> Cc: Marek Vasut <marex@denx.de>
> >> ---
> >>
> >>   drivers/usb/host/usb-uclass.c | 8 ++++----
> >>   1 file changed, 4 insertions(+), 4 deletions(-)
> >>

Regards,
Bin

[PATCH v1 3/4] usb: usb-uclass.c: Drop le16_to_cpu() as values are already swapped

[Bin Meng]

On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
>
> These values are already swapped to CPU endianess, so swapping them
> again is a bug. Let's remove the swap here instead.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Marek Vasut <marex@denx.de>
> ---
>
>  drivers/usb/host/usb-uclass.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>

Tested-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH v1 4/4] usb: xhci: Add virt_to_phys() to support mapped platforms

[Stefan Roese]

Some platforms, like MIPS Octeon, use mapped addresses (virtual address
!= physical address). On these platforms we need to make sure, that the
local virtual addresses are converted to physical (DMA) addresses for
the xHCI controller. This patch adds the missing virt_to_phys() calls,
so that the correct addresses are used.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: Marek Vasut <marex@denx.de>

---

 drivers/usb/host/xhci-mem.c  | 19 +++++++++----------
 drivers/usb/host/xhci-ring.c |  8 ++++----
 drivers/usb/host/xhci.c      |  3 +--
 3 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 3b805ecb9e..874caf4761 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -224,7 +224,7 @@ static void xhci_link_segments(struct xhci_segment *prev,
 		return;
 	prev->next = next;
 	if (link_trbs) {
-		val_64 = (uintptr_t)next->trbs;
+		val_64 = virt_to_phys(next->trbs);
 		prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr =
 			cpu_to_le64(val_64);
 
@@ -484,7 +484,7 @@ int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id)
 	/* Allocate endpoint 0 ring */
 	virt_dev->eps[0].ring = xhci_ring_alloc(1, true);
 
-	byte_64 = (uintptr_t)(virt_dev->out_ctx->bytes);
+	byte_64 = virt_to_phys(virt_dev->out_ctx->bytes);
 
 	/* Point to output device context in dcbaa. */
 	ctrl->dcbaa->dev_context_ptrs[slot_id] = cpu_to_le64(byte_64);
@@ -509,7 +509,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
 	uint64_t val_64;
 	uint64_t trb_64;
 	uint32_t val;
-	unsigned long deq;
+	uint64_t deq;
 	int i;
 	struct xhci_segment *seg;
 
@@ -521,7 +521,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
 		return -ENOMEM;
 	}
 
-	val_64 = (uintptr_t)ctrl->dcbaa;
+	val_64 = virt_to_phys(ctrl->dcbaa);
 	/* Set the pointer in DCBAA register */
 	xhci_writeq(&hcor->or_dcbaap, val_64);
 
@@ -529,7 +529,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
 	ctrl->cmd_ring = xhci_ring_alloc(1, true);
 
 	/* Set the address in the Command Ring Control register */
-	trb_64 = (uintptr_t)ctrl->cmd_ring->first_seg->trbs;
+	trb_64 = virt_to_phys(ctrl->cmd_ring->first_seg->trbs);
 	val_64 = xhci_readq(&hcor->or_crcr);
 	val_64 = (val_64 & (u64) CMD_RING_RSVD_BITS) |
 		(trb_64 & (u64) ~CMD_RING_RSVD_BITS) |
@@ -559,8 +559,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
 	for (val = 0, seg = ctrl->event_ring->first_seg;
 			val < ERST_NUM_SEGS;
 			val++) {
-		trb_64 = 0;
-		trb_64 = (uintptr_t)seg->trbs;
+		trb_64 = virt_to_phys(seg->trbs);
 		struct xhci_erst_entry *entry = &ctrl->erst.entries[val];
 		entry->seg_addr = cpu_to_le64(trb_64);
 		entry->seg_size = cpu_to_le32(TRBS_PER_SEGMENT);
@@ -570,7 +569,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
 	xhci_flush_cache((uintptr_t)ctrl->erst.entries,
 			 ERST_NUM_SEGS * sizeof(struct xhci_erst_entry));
 
-	deq = (unsigned long)ctrl->event_ring->dequeue;
+	deq = virt_to_phys(ctrl->event_ring->dequeue);
 
 	/* Update HC event ring dequeue pointer */
 	xhci_writeq(&ctrl->ir_set->erst_dequeue,
@@ -585,7 +584,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
 	/* this is the event ring segment table pointer */
 	val_64 = xhci_readq(&ctrl->ir_set->erst_base);
 	val_64 &= ERST_PTR_MASK;
-	val_64 |= ((uintptr_t)(ctrl->erst.entries) & ~ERST_PTR_MASK);
+	val_64 |= virt_to_phys(ctrl->erst.entries) & ~ERST_PTR_MASK;
 
 	xhci_writeq(&ctrl->ir_set->erst_base, val_64);
 
@@ -853,7 +852,7 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
 			cpu_to_le32(((0 & MAX_BURST_MASK) << MAX_BURST_SHIFT) |
 			((3 & ERROR_COUNT_MASK) << ERROR_COUNT_SHIFT));
 
-	trb_64 = (uintptr_t)virt_dev->eps[0].ring->first_seg->trbs;
+	trb_64 = virt_to_phys(virt_dev->eps[0].ring->first_seg->trbs);
 	ep0_ctx->deq = cpu_to_le64(trb_64 | virt_dev->eps[0].ring->cycle_state);
 
 	/*
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 86aeaab412..092ed6eaf1 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -275,7 +275,7 @@ void xhci_queue_command(struct xhci_ctrl *ctrl, u8 *ptr, u32 slot_id,
 			u32 ep_index, trb_type cmd)
 {
 	u32 fields[4];
-	u64 val_64 = (uintptr_t)ptr;
+	u64 val_64 = virt_to_phys(ptr);
 
 	BUG_ON(prepare_ring(ctrl, ctrl->cmd_ring, EP_STATE_RUNNING));
 
@@ -399,7 +399,7 @@ void xhci_acknowledge_event(struct xhci_ctrl *ctrl)
 
 	/* Inform the hardware */
 	xhci_writeq(&ctrl->ir_set->erst_dequeue,
-		(uintptr_t)ctrl->event_ring->dequeue | ERST_EHB);
+		    virt_to_phys(ctrl->event_ring->dequeue) | ERST_EHB);
 }
 
 /**
@@ -577,7 +577,7 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
 	u64 addr;
 	int ret;
 	u32 trb_fields[4];
-	u64 val_64 = (uintptr_t)buffer;
+	u64 val_64 = virt_to_phys(buffer);
 
 	debug("dev=%p, pipe=%lx, buffer=%p, length=%d\n",
 		udev, pipe, buffer, length);
@@ -876,7 +876,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 	if (length > 0) {
 		if (req->requesttype & USB_DIR_IN)
 			field |= TRB_DIR_IN;
-		buf_64 = (uintptr_t)buffer;
+		buf_64 = virt_to_phys(buffer);
 
 		trb_fields[0] = lower_32_bits(buf_64);
 		trb_fields[1] = upper_32_bits(buf_64);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ebd2954571..f89e274b0d 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -600,8 +600,7 @@ static int xhci_set_configuration(struct usb_device *udev)
 			cpu_to_le32(MAX_BURST(max_burst) |
 			ERROR_COUNT(err_count));
 
-		trb_64 = (uintptr_t)
-				virt_dev->eps[ep_index].ring->enqueue;
+		trb_64 = virt_to_phys(virt_dev->eps[ep_index].ring->enqueue);
 		ep_ctx[ep_index]->deq = cpu_to_le64(trb_64 |
 				virt_dev->eps[ep_index].ring->cycle_state);
 
-- 
2.27.0

[PATCH v1 4/4] usb: xhci: Add virt_to_phys() to support mapped platforms

[Bin Meng]

Hi Stefan,

On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
>
> Some platforms, like MIPS Octeon, use mapped addresses (virtual address
> != physical address). On these platforms we need to make sure, that the
> local virtual addresses are converted to physical (DMA) addresses for
> the xHCI controller. This patch adds the missing virt_to_phys() calls,
> so that the correct addresses are used.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Marek Vasut <marex@denx.de>
>
> ---
>
>  drivers/usb/host/xhci-mem.c  | 19 +++++++++----------
>  drivers/usb/host/xhci-ring.c |  8 ++++----
>  drivers/usb/host/xhci.c      |  3 +--
>  3 files changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 3b805ecb9e..874caf4761 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -224,7 +224,7 @@ static void xhci_link_segments(struct xhci_segment *prev,
>                 return;
>         prev->next = next;
>         if (link_trbs) {
> -               val_64 = (uintptr_t)next->trbs;
> +               val_64 = virt_to_phys(next->trbs);
>                 prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr =
>                         cpu_to_le64(val_64);
>
> @@ -484,7 +484,7 @@ int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id)
>         /* Allocate endpoint 0 ring */
>         virt_dev->eps[0].ring = xhci_ring_alloc(1, true);
>
> -       byte_64 = (uintptr_t)(virt_dev->out_ctx->bytes);
> +       byte_64 = virt_to_phys(virt_dev->out_ctx->bytes);
>
>         /* Point to output device context in dcbaa. */
>         ctrl->dcbaa->dev_context_ptrs[slot_id] = cpu_to_le64(byte_64);
> @@ -509,7 +509,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
>         uint64_t val_64;
>         uint64_t trb_64;
>         uint32_t val;
> -       unsigned long deq;
> +       uint64_t deq;

This change seems unnecessary?

>         int i;
>         struct xhci_segment *seg;
>
> @@ -521,7 +521,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
>                 return -ENOMEM;
>         }
>
> -       val_64 = (uintptr_t)ctrl->dcbaa;
> +       val_64 = virt_to_phys(ctrl->dcbaa);
>         /* Set the pointer in DCBAA register */
>         xhci_writeq(&hcor->or_dcbaap, val_64);
>
> @@ -529,7 +529,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
>         ctrl->cmd_ring = xhci_ring_alloc(1, true);
>
>         /* Set the address in the Command Ring Control register */
> -       trb_64 = (uintptr_t)ctrl->cmd_ring->first_seg->trbs;
> +       trb_64 = virt_to_phys(ctrl->cmd_ring->first_seg->trbs);
>         val_64 = xhci_readq(&hcor->or_crcr);
>         val_64 = (val_64 & (u64) CMD_RING_RSVD_BITS) |
>                 (trb_64 & (u64) ~CMD_RING_RSVD_BITS) |
> @@ -559,8 +559,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
>         for (val = 0, seg = ctrl->event_ring->first_seg;
>                         val < ERST_NUM_SEGS;
>                         val++) {
> -               trb_64 = 0;
> -               trb_64 = (uintptr_t)seg->trbs;
> +               trb_64 = virt_to_phys(seg->trbs);
>                 struct xhci_erst_entry *entry = &ctrl->erst.entries[val];
>                 entry->seg_addr = cpu_to_le64(trb_64);
>                 entry->seg_size = cpu_to_le32(TRBS_PER_SEGMENT);
> @@ -570,7 +569,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
>         xhci_flush_cache((uintptr_t)ctrl->erst.entries,
>                          ERST_NUM_SEGS * sizeof(struct xhci_erst_entry));
>
> -       deq = (unsigned long)ctrl->event_ring->dequeue;
> +       deq = virt_to_phys(ctrl->event_ring->dequeue);
>
>         /* Update HC event ring dequeue pointer */
>         xhci_writeq(&ctrl->ir_set->erst_dequeue,
> @@ -585,7 +584,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
>         /* this is the event ring segment table pointer */
>         val_64 = xhci_readq(&ctrl->ir_set->erst_base);
>         val_64 &= ERST_PTR_MASK;
> -       val_64 |= ((uintptr_t)(ctrl->erst.entries) & ~ERST_PTR_MASK);
> +       val_64 |= virt_to_phys(ctrl->erst.entries) & ~ERST_PTR_MASK;
>
>         xhci_writeq(&ctrl->ir_set->erst_base, val_64);
>
> @@ -853,7 +852,7 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
>                         cpu_to_le32(((0 & MAX_BURST_MASK) << MAX_BURST_SHIFT) |
>                         ((3 & ERROR_COUNT_MASK) << ERROR_COUNT_SHIFT));
>
> -       trb_64 = (uintptr_t)virt_dev->eps[0].ring->first_seg->trbs;
> +       trb_64 = virt_to_phys(virt_dev->eps[0].ring->first_seg->trbs);
>         ep0_ctx->deq = cpu_to_le64(trb_64 | virt_dev->eps[0].ring->cycle_state);
>
>         /*
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 86aeaab412..092ed6eaf1 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -275,7 +275,7 @@ void xhci_queue_command(struct xhci_ctrl *ctrl, u8 *ptr, u32 slot_id,
>                         u32 ep_index, trb_type cmd)
>  {
>         u32 fields[4];
> -       u64 val_64 = (uintptr_t)ptr;
> +       u64 val_64 = virt_to_phys(ptr);
>
>         BUG_ON(prepare_ring(ctrl, ctrl->cmd_ring, EP_STATE_RUNNING));
>
> @@ -399,7 +399,7 @@ void xhci_acknowledge_event(struct xhci_ctrl *ctrl)
>
>         /* Inform the hardware */
>         xhci_writeq(&ctrl->ir_set->erst_dequeue,
> -               (uintptr_t)ctrl->event_ring->dequeue | ERST_EHB);
> +                   virt_to_phys(ctrl->event_ring->dequeue) | ERST_EHB);
>  }
>
>  /**
> @@ -577,7 +577,7 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
>         u64 addr;
>         int ret;
>         u32 trb_fields[4];
> -       u64 val_64 = (uintptr_t)buffer;
> +       u64 val_64 = virt_to_phys(buffer);
>
>         debug("dev=%p, pipe=%lx, buffer=%p, length=%d\n",
>                 udev, pipe, buffer, length);
> @@ -876,7 +876,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
>         if (length > 0) {
>                 if (req->requesttype & USB_DIR_IN)
>                         field |= TRB_DIR_IN;
> -               buf_64 = (uintptr_t)buffer;
> +               buf_64 = virt_to_phys(buffer);
>
>                 trb_fields[0] = lower_32_bits(buf_64);
>                 trb_fields[1] = upper_32_bits(buf_64);
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index ebd2954571..f89e274b0d 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -600,8 +600,7 @@ static int xhci_set_configuration(struct usb_device *udev)
>                         cpu_to_le32(MAX_BURST(max_burst) |
>                         ERROR_COUNT(err_count));
>
> -               trb_64 = (uintptr_t)
> -                               virt_dev->eps[ep_index].ring->enqueue;
> +               trb_64 = virt_to_phys(virt_dev->eps[ep_index].ring->enqueue);
>                 ep_ctx[ep_index]->deq = cpu_to_le64(trb_64 |
>                                 virt_dev->eps[ep_index].ring->cycle_state);
>
> --

Other than above,
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Regards,
Bin

[PATCH v1 4/4] usb: xhci: Add virt_to_phys() to support mapped platforms

[Stefan Roese]

Hi Bin,

On 17.07.20 07:57, Bin Meng wrote:
> Hi Stefan,
> 
> On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
>>
>> Some platforms, like MIPS Octeon, use mapped addresses (virtual address
>> != physical address). On these platforms we need to make sure, that the
>> local virtual addresses are converted to physical (DMA) addresses for
>> the xHCI controller. This patch adds the missing virt_to_phys() calls,
>> so that the correct addresses are used.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> Cc: Bin Meng <bmeng.cn@gmail.com>
>> Cc: Marek Vasut <marex@denx.de>
>>
>> ---
>>
>>   drivers/usb/host/xhci-mem.c  | 19 +++++++++----------
>>   drivers/usb/host/xhci-ring.c |  8 ++++----
>>   drivers/usb/host/xhci.c      |  3 +--
>>   3 files changed, 14 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
>> index 3b805ecb9e..874caf4761 100644
>> --- a/drivers/usb/host/xhci-mem.c
>> +++ b/drivers/usb/host/xhci-mem.c
>> @@ -224,7 +224,7 @@ static void xhci_link_segments(struct xhci_segment *prev,
>>                  return;
>>          prev->next = next;
>>          if (link_trbs) {
>> -               val_64 = (uintptr_t)next->trbs;
>> +               val_64 = virt_to_phys(next->trbs);
>>                  prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr =
>>                          cpu_to_le64(val_64);
>>
>> @@ -484,7 +484,7 @@ int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id)
>>          /* Allocate endpoint 0 ring */
>>          virt_dev->eps[0].ring = xhci_ring_alloc(1, true);
>>
>> -       byte_64 = (uintptr_t)(virt_dev->out_ctx->bytes);
>> +       byte_64 = virt_to_phys(virt_dev->out_ctx->bytes);
>>
>>          /* Point to output device context in dcbaa. */
>>          ctrl->dcbaa->dev_context_ptrs[slot_id] = cpu_to_le64(byte_64);
>> @@ -509,7 +509,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
>>          uint64_t val_64;
>>          uint64_t trb_64;
>>          uint32_t val;
>> -       unsigned long deq;
>> +       uint64_t deq;
> 
> This change seems unnecessary?

In most (all other?) places, the variable containing these 64 bit
addresses has the type uint64_t in this driver. So I wanted to make the
code more consistant here. Or do you see a problem with this change?

>>          int i;
>>          struct xhci_segment *seg;
>>
>> @@ -521,7 +521,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
>>                  return -ENOMEM;
>>          }
>>
>> -       val_64 = (uintptr_t)ctrl->dcbaa;
>> +       val_64 = virt_to_phys(ctrl->dcbaa);
>>          /* Set the pointer in DCBAA register */
>>          xhci_writeq(&hcor->or_dcbaap, val_64);
>>
>> @@ -529,7 +529,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
>>          ctrl->cmd_ring = xhci_ring_alloc(1, true);
>>
>>          /* Set the address in the Command Ring Control register */
>> -       trb_64 = (uintptr_t)ctrl->cmd_ring->first_seg->trbs;
>> +       trb_64 = virt_to_phys(ctrl->cmd_ring->first_seg->trbs);
>>          val_64 = xhci_readq(&hcor->or_crcr);
>>          val_64 = (val_64 & (u64) CMD_RING_RSVD_BITS) |
>>                  (trb_64 & (u64) ~CMD_RING_RSVD_BITS) |
>> @@ -559,8 +559,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
>>          for (val = 0, seg = ctrl->event_ring->first_seg;
>>                          val < ERST_NUM_SEGS;
>>                          val++) {
>> -               trb_64 = 0;
>> -               trb_64 = (uintptr_t)seg->trbs;
>> +               trb_64 = virt_to_phys(seg->trbs);
>>                  struct xhci_erst_entry *entry = &ctrl->erst.entries[val];
>>                  entry->seg_addr = cpu_to_le64(trb_64);
>>                  entry->seg_size = cpu_to_le32(TRBS_PER_SEGMENT);
>> @@ -570,7 +569,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
>>          xhci_flush_cache((uintptr_t)ctrl->erst.entries,
>>                           ERST_NUM_SEGS * sizeof(struct xhci_erst_entry));
>>
>> -       deq = (unsigned long)ctrl->event_ring->dequeue;
>> +       deq = virt_to_phys(ctrl->event_ring->dequeue);
>>
>>          /* Update HC event ring dequeue pointer */
>>          xhci_writeq(&ctrl->ir_set->erst_dequeue,
>> @@ -585,7 +584,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
>>          /* this is the event ring segment table pointer */
>>          val_64 = xhci_readq(&ctrl->ir_set->erst_base);
>>          val_64 &= ERST_PTR_MASK;
>> -       val_64 |= ((uintptr_t)(ctrl->erst.entries) & ~ERST_PTR_MASK);
>> +       val_64 |= virt_to_phys(ctrl->erst.entries) & ~ERST_PTR_MASK;
>>
>>          xhci_writeq(&ctrl->ir_set->erst_base, val_64);
>>
>> @@ -853,7 +852,7 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
>>                          cpu_to_le32(((0 & MAX_BURST_MASK) << MAX_BURST_SHIFT) |
>>                          ((3 & ERROR_COUNT_MASK) << ERROR_COUNT_SHIFT));
>>
>> -       trb_64 = (uintptr_t)virt_dev->eps[0].ring->first_seg->trbs;
>> +       trb_64 = virt_to_phys(virt_dev->eps[0].ring->first_seg->trbs);
>>          ep0_ctx->deq = cpu_to_le64(trb_64 | virt_dev->eps[0].ring->cycle_state);
>>
>>          /*
>> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
>> index 86aeaab412..092ed6eaf1 100644
>> --- a/drivers/usb/host/xhci-ring.c
>> +++ b/drivers/usb/host/xhci-ring.c
>> @@ -275,7 +275,7 @@ void xhci_queue_command(struct xhci_ctrl *ctrl, u8 *ptr, u32 slot_id,
>>                          u32 ep_index, trb_type cmd)
>>   {
>>          u32 fields[4];
>> -       u64 val_64 = (uintptr_t)ptr;
>> +       u64 val_64 = virt_to_phys(ptr);
>>
>>          BUG_ON(prepare_ring(ctrl, ctrl->cmd_ring, EP_STATE_RUNNING));
>>
>> @@ -399,7 +399,7 @@ void xhci_acknowledge_event(struct xhci_ctrl *ctrl)
>>
>>          /* Inform the hardware */
>>          xhci_writeq(&ctrl->ir_set->erst_dequeue,
>> -               (uintptr_t)ctrl->event_ring->dequeue | ERST_EHB);
>> +                   virt_to_phys(ctrl->event_ring->dequeue) | ERST_EHB);
>>   }
>>
>>   /**
>> @@ -577,7 +577,7 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
>>          u64 addr;
>>          int ret;
>>          u32 trb_fields[4];
>> -       u64 val_64 = (uintptr_t)buffer;
>> +       u64 val_64 = virt_to_phys(buffer);
>>
>>          debug("dev=%p, pipe=%lx, buffer=%p, length=%d\n",
>>                  udev, pipe, buffer, length);
>> @@ -876,7 +876,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
>>          if (length > 0) {
>>                  if (req->requesttype & USB_DIR_IN)
>>                          field |= TRB_DIR_IN;
>> -               buf_64 = (uintptr_t)buffer;
>> +               buf_64 = virt_to_phys(buffer);
>>
>>                  trb_fields[0] = lower_32_bits(buf_64);
>>                  trb_fields[1] = upper_32_bits(buf_64);
>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> index ebd2954571..f89e274b0d 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -600,8 +600,7 @@ static int xhci_set_configuration(struct usb_device *udev)
>>                          cpu_to_le32(MAX_BURST(max_burst) |
>>                          ERROR_COUNT(err_count));
>>
>> -               trb_64 = (uintptr_t)
>> -                               virt_dev->eps[ep_index].ring->enqueue;
>> +               trb_64 = virt_to_phys(virt_dev->eps[ep_index].ring->enqueue);
>>                  ep_ctx[ep_index]->deq = cpu_to_le64(trb_64 |
>>                                  virt_dev->eps[ep_index].ring->cycle_state);
>>
>> --
> 
> Other than above,
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> 
> Regards,
> Bin
>

Thanks,
Stefan

[PATCH v1 4/4] usb: xhci: Add virt_to_phys() to support mapped platforms

[Bin Meng]

Hi Stefan,

On Fri, Jul 17, 2020 at 6:17 PM Stefan Roese <sr@denx.de> wrote:
>
> Hi Bin,
>
> On 17.07.20 07:57, Bin Meng wrote:
> > Hi Stefan,
> >
> > On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
> >>
> >> Some platforms, like MIPS Octeon, use mapped addresses (virtual address
> >> != physical address). On these platforms we need to make sure, that the
> >> local virtual addresses are converted to physical (DMA) addresses for
> >> the xHCI controller. This patch adds the missing virt_to_phys() calls,
> >> so that the correct addresses are used.
> >>
> >> Signed-off-by: Stefan Roese <sr@denx.de>
> >> Cc: Bin Meng <bmeng.cn@gmail.com>
> >> Cc: Marek Vasut <marex@denx.de>
> >>
> >> ---
> >>
> >>   drivers/usb/host/xhci-mem.c  | 19 +++++++++----------
> >>   drivers/usb/host/xhci-ring.c |  8 ++++----
> >>   drivers/usb/host/xhci.c      |  3 +--
> >>   3 files changed, 14 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> >> index 3b805ecb9e..874caf4761 100644
> >> --- a/drivers/usb/host/xhci-mem.c
> >> +++ b/drivers/usb/host/xhci-mem.c
> >> @@ -224,7 +224,7 @@ static void xhci_link_segments(struct xhci_segment *prev,
> >>                  return;
> >>          prev->next = next;
> >>          if (link_trbs) {
> >> -               val_64 = (uintptr_t)next->trbs;
> >> +               val_64 = virt_to_phys(next->trbs);
> >>                  prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr =
> >>                          cpu_to_le64(val_64);
> >>
> >> @@ -484,7 +484,7 @@ int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id)
> >>          /* Allocate endpoint 0 ring */
> >>          virt_dev->eps[0].ring = xhci_ring_alloc(1, true);
> >>
> >> -       byte_64 = (uintptr_t)(virt_dev->out_ctx->bytes);
> >> +       byte_64 = virt_to_phys(virt_dev->out_ctx->bytes);
> >>
> >>          /* Point to output device context in dcbaa. */
> >>          ctrl->dcbaa->dev_context_ptrs[slot_id] = cpu_to_le64(byte_64);
> >> @@ -509,7 +509,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
> >>          uint64_t val_64;
> >>          uint64_t trb_64;
> >>          uint32_t val;
> >> -       unsigned long deq;
> >> +       uint64_t deq;
> >
> > This change seems unnecessary?
>
> In most (all other?) places, the variable containing these 64 bit
> addresses has the type uint64_t in this driver. So I wanted to make the
> code more consistant here. Or do you see a problem with this change?

No, just wondering if you see something is wrong with (unsigned long)
on a 32-bit platform.

Regards,
Bin

[PATCH v1 4/4] usb: xhci: Add virt_to_phys() to support mapped platforms

[Stefan Roese]

Hi Bin,

On 17.07.20 12:23, Bin Meng wrote:
> Hi Stefan,
> 
> On Fri, Jul 17, 2020 at 6:17 PM Stefan Roese <sr@denx.de> wrote:
>>
>> Hi Bin,
>>
>> On 17.07.20 07:57, Bin Meng wrote:
>>> Hi Stefan,
>>>
>>> On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
>>>>
>>>> Some platforms, like MIPS Octeon, use mapped addresses (virtual address
>>>> != physical address). On these platforms we need to make sure, that the
>>>> local virtual addresses are converted to physical (DMA) addresses for
>>>> the xHCI controller. This patch adds the missing virt_to_phys() calls,
>>>> so that the correct addresses are used.
>>>>
>>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>>> Cc: Bin Meng <bmeng.cn@gmail.com>
>>>> Cc: Marek Vasut <marex@denx.de>
>>>>
>>>> ---
>>>>
>>>>    drivers/usb/host/xhci-mem.c  | 19 +++++++++----------
>>>>    drivers/usb/host/xhci-ring.c |  8 ++++----
>>>>    drivers/usb/host/xhci.c      |  3 +--
>>>>    3 files changed, 14 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
>>>> index 3b805ecb9e..874caf4761 100644
>>>> --- a/drivers/usb/host/xhci-mem.c
>>>> +++ b/drivers/usb/host/xhci-mem.c
>>>> @@ -224,7 +224,7 @@ static void xhci_link_segments(struct xhci_segment *prev,
>>>>                   return;
>>>>           prev->next = next;
>>>>           if (link_trbs) {
>>>> -               val_64 = (uintptr_t)next->trbs;
>>>> +               val_64 = virt_to_phys(next->trbs);
>>>>                   prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr =
>>>>                           cpu_to_le64(val_64);
>>>>
>>>> @@ -484,7 +484,7 @@ int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id)
>>>>           /* Allocate endpoint 0 ring */
>>>>           virt_dev->eps[0].ring = xhci_ring_alloc(1, true);
>>>>
>>>> -       byte_64 = (uintptr_t)(virt_dev->out_ctx->bytes);
>>>> +       byte_64 = virt_to_phys(virt_dev->out_ctx->bytes);
>>>>
>>>>           /* Point to output device context in dcbaa. */
>>>>           ctrl->dcbaa->dev_context_ptrs[slot_id] = cpu_to_le64(byte_64);
>>>> @@ -509,7 +509,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
>>>>           uint64_t val_64;
>>>>           uint64_t trb_64;
>>>>           uint32_t val;
>>>> -       unsigned long deq;
>>>> +       uint64_t deq;
>>>
>>> This change seems unnecessary?
>>
>> In most (all other?) places, the variable containing these 64 bit
>> addresses has the type uint64_t in this driver. So I wanted to make the
>> code more consistant here. Or do you see a problem with this change?
> 
> No, just wondering if you see something is wrong with (unsigned long)
> on a 32-bit platform.

I see. Thanks for checking.

Do you have a (little-endian) platform that you could use to test this
patchset on? I currently do not have such a board on my desk and would
very much like to see it tested on little-endian systems before being
applied.

Thanks,
Stefan

[PATCH v1 4/4] usb: xhci: Add virt_to_phys() to support mapped platforms

[Bin Meng]

Hi Stefan,

On Fri, Jul 17, 2020 at 6:28 PM Stefan Roese <sr@denx.de> wrote:
>
> Hi Bin,
>
> On 17.07.20 12:23, Bin Meng wrote:
> > Hi Stefan,
> >
> > On Fri, Jul 17, 2020 at 6:17 PM Stefan Roese <sr@denx.de> wrote:
> >>
> >> Hi Bin,
> >>
> >> On 17.07.20 07:57, Bin Meng wrote:
> >>> Hi Stefan,
> >>>
> >>> On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
> >>>>
> >>>> Some platforms, like MIPS Octeon, use mapped addresses (virtual address
> >>>> != physical address). On these platforms we need to make sure, that the
> >>>> local virtual addresses are converted to physical (DMA) addresses for
> >>>> the xHCI controller. This patch adds the missing virt_to_phys() calls,
> >>>> so that the correct addresses are used.
> >>>>
> >>>> Signed-off-by: Stefan Roese <sr@denx.de>
> >>>> Cc: Bin Meng <bmeng.cn@gmail.com>
> >>>> Cc: Marek Vasut <marex@denx.de>
> >>>>
> >>>> ---
> >>>>
> >>>>    drivers/usb/host/xhci-mem.c  | 19 +++++++++----------
> >>>>    drivers/usb/host/xhci-ring.c |  8 ++++----
> >>>>    drivers/usb/host/xhci.c      |  3 +--
> >>>>    3 files changed, 14 insertions(+), 16 deletions(-)
> >>>>
> >>>> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> >>>> index 3b805ecb9e..874caf4761 100644
> >>>> --- a/drivers/usb/host/xhci-mem.c
> >>>> +++ b/drivers/usb/host/xhci-mem.c
> >>>> @@ -224,7 +224,7 @@ static void xhci_link_segments(struct xhci_segment *prev,
> >>>>                   return;
> >>>>           prev->next = next;
> >>>>           if (link_trbs) {
> >>>> -               val_64 = (uintptr_t)next->trbs;
> >>>> +               val_64 = virt_to_phys(next->trbs);
> >>>>                   prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr =
> >>>>                           cpu_to_le64(val_64);
> >>>>
> >>>> @@ -484,7 +484,7 @@ int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id)
> >>>>           /* Allocate endpoint 0 ring */
> >>>>           virt_dev->eps[0].ring = xhci_ring_alloc(1, true);
> >>>>
> >>>> -       byte_64 = (uintptr_t)(virt_dev->out_ctx->bytes);
> >>>> +       byte_64 = virt_to_phys(virt_dev->out_ctx->bytes);
> >>>>
> >>>>           /* Point to output device context in dcbaa. */
> >>>>           ctrl->dcbaa->dev_context_ptrs[slot_id] = cpu_to_le64(byte_64);
> >>>> @@ -509,7 +509,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
> >>>>           uint64_t val_64;
> >>>>           uint64_t trb_64;
> >>>>           uint32_t val;
> >>>> -       unsigned long deq;
> >>>> +       uint64_t deq;
> >>>
> >>> This change seems unnecessary?
> >>
> >> In most (all other?) places, the variable containing these 64 bit
> >> addresses has the type uint64_t in this driver. So I wanted to make the
> >> code more consistant here. Or do you see a problem with this change?
> >
> > No, just wondering if you see something is wrong with (unsigned long)
> > on a 32-bit platform.
>
> I see. Thanks for checking.
>
> Do you have a (little-endian) platform that you could use to test this
> patchset on? I currently do not have such a board on my desk and would
> very much like to see it tested on little-endian systems before being
> applied.

Yes, I can test this series on x86 Minnowmax. Will send results then.

Regards,
Bin

[PATCH v1 4/4] usb: xhci: Add virt_to_phys() to support mapped platforms

[Stefan Roese]

Hi Bin,

On 17.07.20 12:29, Bin Meng wrote:
> Hi Stefan,
> 
> On Fri, Jul 17, 2020 at 6:28 PM Stefan Roese <sr@denx.de> wrote:
>>
>> Hi Bin,
>>
>> On 17.07.20 12:23, Bin Meng wrote:
>>> Hi Stefan,
>>>
>>> On Fri, Jul 17, 2020 at 6:17 PM Stefan Roese <sr@denx.de> wrote:
>>>>
>>>> Hi Bin,
>>>>
>>>> On 17.07.20 07:57, Bin Meng wrote:
>>>>> Hi Stefan,
>>>>>
>>>>> On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
>>>>>>
>>>>>> Some platforms, like MIPS Octeon, use mapped addresses (virtual address
>>>>>> != physical address). On these platforms we need to make sure, that the
>>>>>> local virtual addresses are converted to physical (DMA) addresses for
>>>>>> the xHCI controller. This patch adds the missing virt_to_phys() calls,
>>>>>> so that the correct addresses are used.
>>>>>>
>>>>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>>>>> Cc: Bin Meng <bmeng.cn@gmail.com>
>>>>>> Cc: Marek Vasut <marex@denx.de>
>>>>>>
>>>>>> ---
>>>>>>
>>>>>>     drivers/usb/host/xhci-mem.c  | 19 +++++++++----------
>>>>>>     drivers/usb/host/xhci-ring.c |  8 ++++----
>>>>>>     drivers/usb/host/xhci.c      |  3 +--
>>>>>>     3 files changed, 14 insertions(+), 16 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
>>>>>> index 3b805ecb9e..874caf4761 100644
>>>>>> --- a/drivers/usb/host/xhci-mem.c
>>>>>> +++ b/drivers/usb/host/xhci-mem.c
>>>>>> @@ -224,7 +224,7 @@ static void xhci_link_segments(struct xhci_segment *prev,
>>>>>>                    return;
>>>>>>            prev->next = next;
>>>>>>            if (link_trbs) {
>>>>>> -               val_64 = (uintptr_t)next->trbs;
>>>>>> +               val_64 = virt_to_phys(next->trbs);
>>>>>>                    prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr =
>>>>>>                            cpu_to_le64(val_64);
>>>>>>
>>>>>> @@ -484,7 +484,7 @@ int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id)
>>>>>>            /* Allocate endpoint 0 ring */
>>>>>>            virt_dev->eps[0].ring = xhci_ring_alloc(1, true);
>>>>>>
>>>>>> -       byte_64 = (uintptr_t)(virt_dev->out_ctx->bytes);
>>>>>> +       byte_64 = virt_to_phys(virt_dev->out_ctx->bytes);
>>>>>>
>>>>>>            /* Point to output device context in dcbaa. */
>>>>>>            ctrl->dcbaa->dev_context_ptrs[slot_id] = cpu_to_le64(byte_64);
>>>>>> @@ -509,7 +509,7 @@ int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
>>>>>>            uint64_t val_64;
>>>>>>            uint64_t trb_64;
>>>>>>            uint32_t val;
>>>>>> -       unsigned long deq;
>>>>>> +       uint64_t deq;
>>>>>
>>>>> This change seems unnecessary?
>>>>
>>>> In most (all other?) places, the variable containing these 64 bit
>>>> addresses has the type uint64_t in this driver. So I wanted to make the
>>>> code more consistant here. Or do you see a problem with this change?
>>>
>>> No, just wondering if you see something is wrong with (unsigned long)
>>> on a 32-bit platform.
>>
>> I see. Thanks for checking.
>>
>> Do you have a (little-endian) platform that you could use to test this
>> patchset on? I currently do not have such a board on my desk and would
>> very much like to see it tested on little-endian systems before being
>> applied.
> 
> Yes, I can test this series on x86 Minnowmax. Will send results then.

Thanks a lot.

Thanks,
Stefan

[PATCH v1 4/4] usb: xhci: Add virt_to_phys() to support mapped platforms

[Bin Meng]

On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr@denx.de> wrote:
>
> Some platforms, like MIPS Octeon, use mapped addresses (virtual address
> != physical address). On these platforms we need to make sure, that the
> local virtual addresses are converted to physical (DMA) addresses for
> the xHCI controller. This patch adds the missing virt_to_phys() calls,
> so that the correct addresses are used.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Marek Vasut <marex@denx.de>
>
> ---
>
>  drivers/usb/host/xhci-mem.c  | 19 +++++++++----------
>  drivers/usb/host/xhci-ring.c |  8 ++++----
>  drivers/usb/host/xhci.c      |  3 +--
>  3 files changed, 14 insertions(+), 16 deletions(-)
>

Tested-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH v2 0/4] usb: xhci: Prepare xHCI driver for MIPS Octeon big-endian support

[Stefan Roese]

These patches fix a few issues, found while porting the xHCI to the MIPS
Octeon platforms. The basic issues here are:

- Endianess issues: missing cpu_to_leXX() & leXX_to_cpu() conversions
- Use physical (DMA) address for the xHCI DMA controller

These patches are the groundwork for the upcoming xHCI Octeon support
that will follow soon.

Thanks,
Stefan

Changes in v2:
- Add missing (uintptr_t) cast to remove compile time warning

Stefan Roese (4):
  usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu)
  usb: xhci: xhci_mem_init: Use cpu_to_le64() and not xhci_writeq()
  usb: usb-uclass.c: Drop le16_to_cpu() as values are already swapped
  usb: xhci: Add virt_to_phys() to support mapped platforms

 drivers/usb/host/usb-uclass.c |  8 ++++----
 drivers/usb/host/xhci-mem.c   | 30 +++++++++++++++---------------
 drivers/usb/host/xhci-ring.c  |  8 ++++----
 drivers/usb/host/xhci.c       |  3 +--
 4 files changed, 24 insertions(+), 25 deletions(-)

-- 
2.27.0

[PATCH v2 1/4] usb: xhci: Add missing endian conversions (cpu_to_leXX / leXX_to_cpu)

[Stefan Roese]

While trying to use the U-Boot xHCI driver on the MIPS Octeon platform,
which is big endian, I noticed that the driver is missing a few endian
conversion calls. This patch adds these missing endian conversion
calls.

Signed-off-by: Stefan Roese <sr@denx.de>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Tested-by: Bin Meng <bmeng.cn@gmail.com>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: Marek Vasut <marex@denx.de>

---

Changes in v2:
- Add missing (uintptr_t) cast to remove compile time warning

 drivers/usb/host/xhci-mem.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 2d968aafb0..7d55944765 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -110,7 +110,7 @@ static void xhci_scratchpad_free(struct xhci_ctrl *ctrl)
 
 	ctrl->dcbaa->dev_context_ptrs[0] = 0;
 
-	free((void *)(uintptr_t)ctrl->scratchpad->sp_array[0]);
+	free((void *)(uintptr_t)le64_to_cpu(ctrl->scratchpad->sp_array[0]));
 	free(ctrl->scratchpad->sp_array);
 	free(ctrl->scratchpad);
 	ctrl->scratchpad = NULL;
@@ -225,7 +225,8 @@ static void xhci_link_segments(struct xhci_segment *prev,
 	prev->next = next;
 	if (link_trbs) {
 		val_64 = (uintptr_t)next->trbs;
-		prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr = val_64;
+		prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr =
+			cpu_to_le64(val_64);
 
 		/*
 		 * Set the last TRB in the segment to
@@ -486,7 +487,7 @@ int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id)
 	byte_64 = (uintptr_t)(virt_dev->out_ctx->bytes);
 
 	/* Point to output device context in dcbaa. */
-	ctrl->dcbaa->dev_context_ptrs[slot_id] = byte_64;
+	ctrl->dcbaa->dev_context_ptrs[slot_id] = cpu_to_le64(byte_64);
 
 	xhci_flush_cache((uintptr_t)&ctrl->dcbaa->dev_context_ptrs[slot_id],
 			 sizeof(__le64));
@@ -768,7 +769,7 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
 
 	debug("route string %x\n", route);
 #endif
-	slot_ctx->dev_info |= route;
+	slot_ctx->dev_info |= cpu_to_le32(route);
 
 	switch (speed) {
 	case USB_SPEED_SUPER:
-- 
2.27.0