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

Thanks a lot.

Thanks,
Stefan