[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 at 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 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.
>
> 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