[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 at 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 at 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 at denx.de>
> >> Cc: Bin Meng <bmeng.cn at gmail.com>
> >> Cc: Marek Vasut <marex at 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