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 > <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?
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
