Re: [PATCH v3 10/12] xhci: translate virtual addresses into the bus's address space
On Fri, 2020-12-18 at 19:28 -0700, Simon Glass wrote:
> Hi Nicolas,
>
> On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
> wrote:
> >
> > So far we've been content with passing physical addresses when
> > configuring memory addresses into XHCI controllers, but not all
> > platforms have buses with transparent mappings. Specifically the
> > Raspberry Pi 4 might introduce an offset to memory accesses incoming
> > from its PCIe port.
> >
> > Introduce xhci_virt_to_bus() and xhci_bus_to_virt() to cater with these
> > limitations, and make sure we don't break non DM users.
> >
> > Signed-off-by: Nicolas Saenz Julienne
> > ---
> > drivers/usb/host/xhci-mem.c | 45 +++-
> > drivers/usb/host/xhci-ring.c | 11 +
> > drivers/usb/host/xhci.c | 4 ++--
> > include/usb/xhci.h | 22 +-
> > 4 files changed, 54 insertions(+), 28 deletions(-)
>
> Reviewed-by: Simon Glass
>
> nits below
>
> [..]
>
> > diff --git a/include/usb/xhci.h b/include/usb/xhci.h
> > index e1d382369a..b87210b9ba 100644
> > --- a/include/usb/xhci.h
> > +++ b/include/usb/xhci.h
> > @@ -16,6 +16,7 @@
> > #ifndef HOST_XHCI_H_
> > #define HOST_XHCI_H_
> >
> > +#include
> > #include
> > #include
> > #include
> > @@ -1250,7 +1251,8 @@ int xhci_check_maxpacket(struct usb_device *udev);
> > void xhci_flush_cache(uintptr_t addr, u32 type_len);
> > void xhci_inval_cache(uintptr_t addr, u32 type_len);
> > void xhci_cleanup(struct xhci_ctrl *ctrl);
> > -struct xhci_ring *xhci_ring_alloc(unsigned int num_segs, bool link_trbs);
> > +struct xhci_ring *xhci_ring_alloc(struct xhci_ctrl *ctrl, unsigned int
> > num_segs,
> > + bool link_trbs);
>
> Can you please add a comment while you are here?
The comment is in the xhci-mem.c. Should I move it here?
> > int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id);
> > int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
> > struct xhci_hcor *hcor);
> > @@ -1278,4 +1280,22 @@ extern struct dm_usb_ops xhci_usb_ops;
> >
> > struct xhci_ctrl *xhci_get_ctrl(struct usb_device *udev);
> >
> > +static inline dma_addr_t xhci_virt_to_bus(struct xhci_ctrl *ctrl, void
> > *addr)
> > +{
> > +#if CONFIG_IS_ENABLED(DM_DMA)
>
> Can this use if() ?
Yes, will do.
> > + return dev_phys_to_bus(ctrl->dev, virt_to_phys(addr));
> > +#else
> > + return phys_to_bus(virt_to_phys(addr));
> > +#endif
> > +}
> > +
> > +static inline void *xhci_bus_to_virt(struct xhci_ctrl *ctrl, dma_addr_t
> > addr)
> > +{
> > +#if CONFIG_IS_ENABLED(DM_DMA)
> > + return phys_to_virt(dev_bus_to_phys(ctrl->dev, addr));
> > +#else
> > + return phys_to_virt(bus_to_phys(addr));
> > +#endif
> > +}
> > +
> > #endif /* HOST_XHCI_H_ */
> > --
> > 2.29.2
> >
>
> Regards,
> Simon
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v3 10/12] xhci: translate virtual addresses into the bus's address space
Hi Nicolas,
On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne
wrote:
>
> So far we've been content with passing physical addresses when
> configuring memory addresses into XHCI controllers, but not all
> platforms have buses with transparent mappings. Specifically the
> Raspberry Pi 4 might introduce an offset to memory accesses incoming
> from its PCIe port.
>
> Introduce xhci_virt_to_bus() and xhci_bus_to_virt() to cater with these
> limitations, and make sure we don't break non DM users.
>
> Signed-off-by: Nicolas Saenz Julienne
> ---
> drivers/usb/host/xhci-mem.c | 45 +++-
> drivers/usb/host/xhci-ring.c | 11 +
> drivers/usb/host/xhci.c | 4 ++--
> include/usb/xhci.h | 22 +-
> 4 files changed, 54 insertions(+), 28 deletions(-)
Reviewed-by: Simon Glass
nits below
[..]
> diff --git a/include/usb/xhci.h b/include/usb/xhci.h
> index e1d382369a..b87210b9ba 100644
> --- a/include/usb/xhci.h
> +++ b/include/usb/xhci.h
> @@ -16,6 +16,7 @@
> #ifndef HOST_XHCI_H_
> #define HOST_XHCI_H_
>
> +#include
> #include
> #include
> #include
> @@ -1250,7 +1251,8 @@ int xhci_check_maxpacket(struct usb_device *udev);
> void xhci_flush_cache(uintptr_t addr, u32 type_len);
> void xhci_inval_cache(uintptr_t addr, u32 type_len);
> void xhci_cleanup(struct xhci_ctrl *ctrl);
> -struct xhci_ring *xhci_ring_alloc(unsigned int num_segs, bool link_trbs);
> +struct xhci_ring *xhci_ring_alloc(struct xhci_ctrl *ctrl, unsigned int
> num_segs,
> + bool link_trbs);
Can you please add a comment while you are here?
> int xhci_alloc_virt_device(struct xhci_ctrl *ctrl, unsigned int slot_id);
> int xhci_mem_init(struct xhci_ctrl *ctrl, struct xhci_hccr *hccr,
> struct xhci_hcor *hcor);
> @@ -1278,4 +1280,22 @@ extern struct dm_usb_ops xhci_usb_ops;
>
> struct xhci_ctrl *xhci_get_ctrl(struct usb_device *udev);
>
> +static inline dma_addr_t xhci_virt_to_bus(struct xhci_ctrl *ctrl, void *addr)
> +{
> +#if CONFIG_IS_ENABLED(DM_DMA)
Can this use if() ?
> + return dev_phys_to_bus(ctrl->dev, virt_to_phys(addr));
> +#else
> + return phys_to_bus(virt_to_phys(addr));
> +#endif
> +}
> +
> +static inline void *xhci_bus_to_virt(struct xhci_ctrl *ctrl, dma_addr_t addr)
> +{
> +#if CONFIG_IS_ENABLED(DM_DMA)
> + return phys_to_virt(dev_bus_to_phys(ctrl->dev, addr));
> +#else
> + return phys_to_virt(bus_to_phys(addr));
> +#endif
> +}
> +
> #endif /* HOST_XHCI_H_ */
> --
> 2.29.2
>
Regards,
Simon
[PATCH v3 10/12] xhci: translate virtual addresses into the bus's address space
So far we've been content with passing physical addresses when
configuring memory addresses into XHCI controllers, but not all
platforms have buses with transparent mappings. Specifically the
Raspberry Pi 4 might introduce an offset to memory accesses incoming
from its PCIe port.
Introduce xhci_virt_to_bus() and xhci_bus_to_virt() to cater with these
limitations, and make sure we don't break non DM users.
Signed-off-by: Nicolas Saenz Julienne
---
drivers/usb/host/xhci-mem.c | 45 +++-
drivers/usb/host/xhci-ring.c | 11 +
drivers/usb/host/xhci.c | 4 ++--
include/usb/xhci.h | 22 +-
4 files changed, 54 insertions(+), 28 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index b002d6f166..83147d51b5 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)le64_to_cpu(ctrl->scratchpad->sp_array[0]));
+ free(xhci_bus_to_virt(ctrl,
le64_to_cpu(ctrl->scratchpad->sp_array[0])));
free(ctrl->scratchpad->sp_array);
free(ctrl->scratchpad);
ctrl->scratchpad = NULL;
@@ -216,8 +216,8 @@ static void *xhci_malloc(unsigned int size)
* @param link_trbsflag to indicate whether to link the trbs or NOT
* @return none
*/
-static void xhci_link_segments(struct xhci_segment *prev,
- struct xhci_segment *next, bool link_trbs)
+static void xhci_link_segments(struct xhci_ctrl *ctrl, struct xhci_segment
*prev,
+ struct xhci_segment *next, bool link_trbs)
{
u32 val;
u64 val_64 = 0;
@@ -226,7 +226,7 @@ static void xhci_link_segments(struct xhci_segment *prev,
return;
prev->next = next;
if (link_trbs) {
- val_64 = virt_to_phys(next->trbs);
+ val_64 = xhci_virt_to_bus(ctrl, next->trbs);
prev->trbs[TRBS_PER_SEGMENT-1].link.segment_ptr =
cpu_to_le64(val_64);
@@ -304,7 +304,8 @@ static struct xhci_segment *xhci_segment_alloc(void)
* @param link_trbsflag to indicate whether to link the trbs or NOT
* @return pointer to the newly created RING
*/
-struct xhci_ring *xhci_ring_alloc(unsigned int num_segs, bool link_trbs)
+struct xhci_ring *xhci_ring_alloc(struct xhci_ctrl *ctrl, unsigned int
num_segs,
+ bool link_trbs)
{
struct xhci_ring *ring;
struct xhci_segment *prev;
@@ -327,12 +328,12 @@ struct xhci_ring *xhci_ring_alloc(unsigned int num_segs,
bool link_trbs)
next = xhci_segment_alloc();
BUG_ON(!next);
- xhci_link_segments(prev, next, link_trbs);
+ xhci_link_segments(ctrl, prev, next, link_trbs);
prev = next;
num_segs--;
}
- xhci_link_segments(prev, ring->first_seg, link_trbs);
+ xhci_link_segments(ctrl, prev, ring->first_seg, link_trbs);
if (link_trbs) {
/* See section 4.9.2.1 and 6.4.4.1 */
prev->trbs[TRBS_PER_SEGMENT-1].link.control |=
@@ -354,6 +355,7 @@ static int xhci_scratchpad_alloc(struct xhci_ctrl *ctrl)
struct xhci_hccr *hccr = ctrl->hccr;
struct xhci_hcor *hcor = ctrl->hcor;
struct xhci_scratchpad *scratchpad;
+ uint64_t val_64;
int num_sp;
uint32_t page_size;
void *buf;
@@ -371,8 +373,9 @@ static int xhci_scratchpad_alloc(struct xhci_ctrl *ctrl)
scratchpad->sp_array = xhci_malloc(num_sp * sizeof(u64));
if (!scratchpad->sp_array)
goto fail_sp2;
- ctrl->dcbaa->dev_context_ptrs[0] =
- cpu_to_le64((uintptr_t)scratchpad->sp_array);
+
+ val_64 = xhci_virt_to_bus(ctrl, scratchpad->sp_array);
+ ctrl->dcbaa->dev_context_ptrs[0] = cpu_to_le64(val_64);
xhci_flush_cache((uintptr_t)&ctrl->dcbaa->dev_context_ptrs[0],
sizeof(ctrl->dcbaa->dev_context_ptrs[0]));
@@ -393,8 +396,8 @@ static int xhci_scratchpad_alloc(struct xhci_ctrl *ctrl)
xhci_flush_cache((uintptr_t)buf, num_sp * page_size);
for (i = 0; i < num_sp; i++) {
- uintptr_t ptr = (uintptr_t)buf + i * page_size;
- scratchpad->sp_array[i] = cpu_to_le64(ptr);
+ val_64 = xhci_virt_to_bus(ctrl, buf + i * page_size);
+ scratchpad->sp_array[i] = cpu_to_le64(val_64);
}
xhci_flush_cache((uintptr_t)scratchpad->sp_array,
@@ -484,9 +487,9 @@ 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);
+ virt_dev->eps[0].ring = xhci_ring_alloc(ctrl, 1, true);
- byte_64 = virt_to_phys(virt_dev->out_ctx->bytes);
+ byte_
