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

Stefan Roese sr at denx.de
Fri Jul 17 12:17:40 CEST 2020


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?

>>          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 at gmail.com>
> 
> Regards,
> Bin
>

Thanks,
Stefan


More information about the U-Boot mailing list