Hi Nicolas, On Tue, 15 Dec 2020 at 10:23, Nicolas Saenz Julienne <nsaenzjulie...@suse.de> 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 <nsaenzjulie...@suse.de> > --- > 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 <s...@chromium.org> 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 <phys2bus.h> > #include <reset.h> > #include <asm/types.h> > #include <asm/cache.h> > @@ -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