Re: [PATCH v2] usb: core: setup dma_pfn_offset for USB devices and, interfaces

[Alan Stern]

On Mon, 12 Sep 2016, Arnd Bergmann wrote:

> On Monday, September 12, 2016 9:09:16 AM CEST Alan Stern wrote:
> > > diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> > > index 0406a59..66364ea 100644
> > > --- a/drivers/usb/core/message.c
> > > +++ b/drivers/usb/core/message.c
> > > @@ -1863,6 +1863,12 @@ free_interfaces:
> > >               intf->dev.type = &usb_if_device_type;
> > >               intf->dev.groups = usb_interface_groups;
> > >               intf->dev.dma_mask = dev->dev.dma_mask;
> > > +             /* Propagate dma_pfn_offset to USB interface.
> > > +              * This is especially required by mass storage interface
> > > +              * which relies on SCSI layer and scsi_calculate_bounce_limit()
> > > +              * to set the bounce buffer limit based on dma_pfn_offset.
> > > +              */
> > > +             intf->dev.dma_pfn_offset = dev->dev.dma_pfn_offset;
> > 
> > I really think this comment isn't necessary.  It seems pretty obvious 
> > that if you're copying DMA-related fields from one device to another 
> > then you'll want to copy the dma_pfn_offset along with everything else.
> > No?
> > 
> > The explanation in the Changelog is enough, IMO.
> 
> I asked for a code comment here as what we are doing here is a bit
> fishy, but the comment (evidently) doesn't quite capture it. I would
> add (above the dma_mask assignment) something like
> 
> 	/*
> 	 * Fake a dma_mask/offset for the USB device:
> 	 * We cannot really use the dma-mapping API (dma_alloc_* and
> 	 * dma_map_*) for USB devices but instead need to use
> 	 * usb_alloc_coherent and pass data in 'urb's, but some subsystems
> 	 * manually look into the mask/offset pair to determine whether
> 	 * they need bounce buffers.
> 	 * Note: calling dma_set_mask() on a USB device would set the
> 	 * mask for the entire HCD, so don't do that.
> 	 */

I'm okay with this.  But at least put this explanation only in usb.c,
and have the comment in message.c refer back to it.

(Also, 'urb' doesn't need to be in quotes and should be capitalized: 
URBs.)

Alan Stern