[PATCH] usb: core: setup dma_pfn_offset for USB devices

[PATCH] usb: core: setup dma_pfn_offset for USB devices

[Roger Quadros]

If dma_pfn_offset is not inherited correctly from the host controller,
it might result in sub-optimal configuration as bounce
buffer limit might be set to less than optimal level.

e.g. On Keystone 2 systems, dma_max_pfn() is 0x87FFFF and dma_mask_pfn
is 0xFFFFF. Consider a mass storage use case: Without this patch,
usb scsi host device (usb-storage) will get a dma_pfn_offset of 0 resulting
in a dma_max_pfn() of 0xFFFFF within the scsi layer
(scsi_calculate_bounce_limit()).
This will result in bounce buffers being unnecessarily used.

Hint: On 32-bit ARM platforms dma_max_pfn() = dma_mask_pfn + dma_pfn_offset

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/core/message.c | 1 +
 drivers/usb/core/usb.c     | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 0406a59..6856b7a 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1863,6 +1863,7 @@ free_interfaces:
 		intf->dev.type = &usb_if_device_type;
 		intf->dev.groups = usb_interface_groups;
 		intf->dev.dma_mask = dev->dev.dma_mask;
+		intf->dev.dma_pfn_offset = dev->dev.dma_pfn_offset;
 		INIT_WORK(&intf->reset_ws, __usb_queue_reset_device);
 		intf->minor = -1;
 		device_initialize(&intf->dev);
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 5e80697..d81791a 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -441,6 +441,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,
 	dev->dev.type = &usb_device_type;
 	dev->dev.groups = usb_device_groups;
 	dev->dev.dma_mask = bus->controller->dma_mask;
+	dev->dev.dma_pfn_offset = bus->controller->dma_pfn_offset;
 	set_dev_node(&dev->dev, dev_to_node(bus->controller));
 	dev->state = USB_STATE_ATTACHED;
 	dev->lpm_disable_count = 1;
-- 
2.7.4

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

[Roger Quadros]

If dma_pfn_offset is not inherited correctly from the host controller,
it might result in sub-optimal configuration as bounce
buffer limit might be set to less than optimal level.

Consider the mass storage device case.
USB storage driver creates a scsi host for the mass storage interface in
drivers/usb/storage/usb.c
The scsi host parent device is nothing but the the USB interface device.
Now, __scsi_init_queue() calls scsi_calculate_bounce_limit() to find out
and set the block layer bounce limit.
scsi_calculate_bounce_limit() uses dma_max_pfn(host_dev) to get the
bounce_limit. host_dev is nothing but the device representing the
mass storage interface.
If that device doesn't have the right dma_pfn_offset, then dma_max_pfn()
is messed up and the bounce buffer limit is wrong.

e.g. On Keystone 2 systems, dma_max_pfn() is 0x87FFFF and dma_mask_pfn
is 0xFFFFF. Consider a mass storage use case: Without this patch,
usb scsi host device (usb-storage) will get a dma_pfn_offset of 0 resulting
in a dma_max_pfn() of 0xFFFFF within the scsi layer
(scsi_calculate_bounce_limit()).
This will result in bounce buffers being unnecessarily used.

Hint: On 32-bit ARM platforms dma_max_pfn() = dma_mask_pfn + dma_pfn_offset

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/core/message.c | 6 ++++++
 drivers/usb/core/usb.c     | 7 +++++++
 2 files changed, 13 insertions(+)

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;
 		INIT_WORK(&intf->reset_ws, __usb_queue_reset_device);
 		intf->minor = -1;
 		device_initialize(&intf->dev);
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 5e80697..7dde93d 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -441,6 +441,13 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,
 	dev->dev.type = &usb_device_type;
 	dev->dev.groups = usb_device_groups;
 	dev->dev.dma_mask = bus->controller->dma_mask;
+	/* Propagate bus controller's dma_pfn_offset to USB device,
+	 * as it would be needed to be propagated to USB interfaces.
+	 * 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.
+	 */
+	dev->dev.dma_pfn_offset = bus->controller->dma_pfn_offset;
 	set_dev_node(&dev->dev, dev_to_node(bus->controller));
 	dev->state = USB_STATE_ATTACHED;
 	dev->lpm_disable_count = 1;
-- 
2.7.4

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

[Alan Stern]

On Mon, 12 Sep 2016, Roger Quadros wrote:

> If dma_pfn_offset is not inherited correctly from the host controller,
> it might result in sub-optimal configuration as bounce
> buffer limit might be set to less than optimal level.
> 
> Consider the mass storage device case.
> USB storage driver creates a scsi host for the mass storage interface in
> drivers/usb/storage/usb.c
> The scsi host parent device is nothing but the the USB interface device.
> Now, __scsi_init_queue() calls scsi_calculate_bounce_limit() to find out
> and set the block layer bounce limit.
> scsi_calculate_bounce_limit() uses dma_max_pfn(host_dev) to get the
> bounce_limit. host_dev is nothing but the device representing the
> mass storage interface.
> If that device doesn't have the right dma_pfn_offset, then dma_max_pfn()
> is messed up and the bounce buffer limit is wrong.
> 
> e.g. On Keystone 2 systems, dma_max_pfn() is 0x87FFFF and dma_mask_pfn
> is 0xFFFFF. Consider a mass storage use case: Without this patch,
> usb scsi host device (usb-storage) will get a dma_pfn_offset of 0 resulting
> in a dma_max_pfn() of 0xFFFFF within the scsi layer
> (scsi_calculate_bounce_limit()).
> This will result in bounce buffers being unnecessarily used.
> 
> Hint: On 32-bit ARM platforms dma_max_pfn() = dma_mask_pfn + dma_pfn_offset
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---

How does v2 of this patch differ from v1?

>  drivers/usb/core/message.c | 6 ++++++
>  drivers/usb/core/usb.c     | 7 +++++++
>  2 files changed, 13 insertions(+)
> 
> 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.

>  		INIT_WORK(&intf->reset_ws, __usb_queue_reset_device);
>  		intf->minor = -1;
>  		device_initialize(&intf->dev);
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 5e80697..7dde93d 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -441,6 +441,13 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,
>  	dev->dev.type = &usb_device_type;
>  	dev->dev.groups = usb_device_groups;
>  	dev->dev.dma_mask = bus->controller->dma_mask;
> +	/* Propagate bus controller's dma_pfn_offset to USB device,
> +	 * as it would be needed to be propagated to USB interfaces.
> +	 * 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.
> +	 */
> +	dev->dev.dma_pfn_offset = bus->controller->dma_pfn_offset;

Same here.

>  	set_dev_node(&dev->dev, dev_to_node(bus->controller));
>  	dev->state = USB_STATE_ATTACHED;
>  	dev->lpm_disable_count = 1;

Alan Stern

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

[Roger Quadros]

Hi Alan,

On 12/09/16 16:09, Alan Stern wrote:
> On Mon, 12 Sep 2016, Roger Quadros wrote:
> 
>> If dma_pfn_offset is not inherited correctly from the host controller,
>> it might result in sub-optimal configuration as bounce
>> buffer limit might be set to less than optimal level.
>>
>> Consider the mass storage device case.
>> USB storage driver creates a scsi host for the mass storage interface in
>> drivers/usb/storage/usb.c
>> The scsi host parent device is nothing but the the USB interface device.
>> Now, __scsi_init_queue() calls scsi_calculate_bounce_limit() to find out
>> and set the block layer bounce limit.
>> scsi_calculate_bounce_limit() uses dma_max_pfn(host_dev) to get the
>> bounce_limit. host_dev is nothing but the device representing the
>> mass storage interface.
>> If that device doesn't have the right dma_pfn_offset, then dma_max_pfn()
>> is messed up and the bounce buffer limit is wrong.
>>
>> e.g. On Keystone 2 systems, dma_max_pfn() is 0x87FFFF and dma_mask_pfn
>> is 0xFFFFF. Consider a mass storage use case: Without this patch,
>> usb scsi host device (usb-storage) will get a dma_pfn_offset of 0 resulting
>> in a dma_max_pfn() of 0xFFFFF within the scsi layer
>> (scsi_calculate_bounce_limit()).
>> This will result in bounce buffers being unnecessarily used.
>>
>> Hint: On 32-bit ARM platforms dma_max_pfn() = dma_mask_pfn + dma_pfn_offset
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
> 
> How does v2 of this patch differ from v1?

No functional difference.
Just added more explanation in the commit log.

> 
>>  drivers/usb/core/message.c | 6 ++++++
>>  drivers/usb/core/usb.c     | 7 +++++++
>>  2 files changed, 13 insertions(+)
>>
>> 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.

Fine with me.

> 
>>  		INIT_WORK(&intf->reset_ws, __usb_queue_reset_device);
>>  		intf->minor = -1;
>>  		device_initialize(&intf->dev);
>> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
>> index 5e80697..7dde93d 100644
>> --- a/drivers/usb/core/usb.c
>> +++ b/drivers/usb/core/usb.c
>> @@ -441,6 +441,13 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,
>>  	dev->dev.type = &usb_device_type;
>>  	dev->dev.groups = usb_device_groups;
>>  	dev->dev.dma_mask = bus->controller->dma_mask;
>> +	/* Propagate bus controller's dma_pfn_offset to USB device,
>> +	 * as it would be needed to be propagated to USB interfaces.
>> +	 * 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.
>> +	 */
>> +	dev->dev.dma_pfn_offset = bus->controller->dma_pfn_offset;
> 
> Same here.

OK.

> 
>>  	set_dev_node(&dev->dev, dev_to_node(bus->controller));
>>  	dev->state = USB_STATE_ATTACHED;
>>  	dev->lpm_disable_count = 1;
> 
> Alan Stern
> 

cheers,
-roger

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

[Roger Quadros]

If dma_pfn_offset is not inherited correctly from the host controller,
it might result in sub-optimal configuration as bounce
buffer limit might be set to less than optimal level.

Consider the mass storage device case.
USB storage driver creates a scsi host for the mass storage interface in
drivers/usb/storage/usb.c
The scsi host parent device is nothing but the the USB interface device.
Now, __scsi_init_queue() calls scsi_calculate_bounce_limit() to find out
and set the block layer bounce limit.
scsi_calculate_bounce_limit() uses dma_max_pfn(host_dev) to get the
bounce_limit. host_dev is nothing but the device representing the
mass storage interface.
If that device doesn't have the right dma_pfn_offset, then dma_max_pfn()
is messed up and the bounce buffer limit is wrong.

e.g. On Keystone 2 systems, dma_max_pfn() is 0x87FFFF and dma_mask_pfn
is 0xFFFFF. Consider a mass storage use case: Without this patch,
usb scsi host device (usb-storage) will get a dma_pfn_offset of 0 resulting
in a dma_max_pfn() of 0xFFFFF within the scsi layer
(scsi_calculate_bounce_limit()).
This will result in bounce buffers being unnecessarily used.

Hint: On 32-bit ARM platforms dma_max_pfn() = dma_mask_pfn + dma_pfn_offset

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
Changelog:

v3:
- removed comments from code as commit log is sufficient.
v2:
- added more information in commit log and code.


 drivers/usb/core/message.c | 1 +
 drivers/usb/core/usb.c     | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 0406a59..6856b7a 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1863,6 +1863,7 @@ free_interfaces:
 		intf->dev.type = &usb_if_device_type;
 		intf->dev.groups = usb_interface_groups;
 		intf->dev.dma_mask = dev->dev.dma_mask;
+		intf->dev.dma_pfn_offset = dev->dev.dma_pfn_offset;
 		INIT_WORK(&intf->reset_ws, __usb_queue_reset_device);
 		intf->minor = -1;
 		device_initialize(&intf->dev);
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 5e80697..d81791a 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -441,6 +441,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,
 	dev->dev.type = &usb_device_type;
 	dev->dev.groups = usb_device_groups;
 	dev->dev.dma_mask = bus->controller->dma_mask;
+	dev->dev.dma_pfn_offset = bus->controller->dma_pfn_offset;
 	set_dev_node(&dev->dev, dev_to_node(bus->controller));
 	dev->state = USB_STATE_ATTACHED;
 	dev->lpm_disable_count = 1;
-- 
2.7.4

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

[Arnd Bergmann]

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.
	 */

	Arnd

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

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

[Roger Quadros]

If dma_pfn_offset is not inherited correctly from the host controller,
it might result in sub-optimal configuration as bounce
buffer limit might be set to less than optimal level.

Consider the mass storage device case.
USB storage driver creates a scsi host for the mass storage interface in
drivers/usb/storage/usb.c
The scsi host parent device is nothing but the the USB interface device.
Now, __scsi_init_queue() calls scsi_calculate_bounce_limit() to find out
and set the block layer bounce limit.
scsi_calculate_bounce_limit() uses dma_max_pfn(host_dev) to get the
bounce_limit. host_dev is nothing but the device representing the
mass storage interface.
If that device doesn't have the right dma_pfn_offset, then dma_max_pfn()
is messed up and the bounce buffer limit is wrong.

e.g. On Keystone 2 systems, dma_max_pfn() is 0x87FFFF and dma_mask_pfn
is 0xFFFFF. Consider a mass storage use case: Without this patch,
usb scsi host device (usb-storage) will get a dma_pfn_offset of 0 resulting
in a dma_max_pfn() of 0xFFFFF within the scsi layer
(scsi_calculate_bounce_limit()).
This will result in bounce buffers being unnecessarily used.

Hint: On 32-bit ARM platforms dma_max_pfn() = dma_mask_pfn + dma_pfn_offset

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
Changelog:

v4:
- added comment in the code as to why we need to set dma_mask and
dma_pfn_offset for usb devices.
v3:
- removed comments from code as commit log is sufficient.
v2:
- added more information in commit log and code.

 drivers/usb/core/message.c |  5 +++++
 drivers/usb/core/usb.c     | 11 +++++++++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 0406a59..bb617df 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1862,7 +1862,12 @@ free_interfaces:
 		intf->dev.bus = &usb_bus_type;
 		intf->dev.type = &usb_if_device_type;
 		intf->dev.groups = usb_interface_groups;
+		/*
+		 * Please refer to usb_alloc_dev() to see why we set
+		 * dma_mask and dma_pfn_offset.
+		 */
 		intf->dev.dma_mask = dev->dev.dma_mask;
+		intf->dev.dma_pfn_offset = dev->dev.dma_pfn_offset;
 		INIT_WORK(&intf->reset_ws, __usb_queue_reset_device);
 		intf->minor = -1;
 		device_initialize(&intf->dev);
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 5e80697..5921514 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -440,7 +440,18 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,
 	dev->dev.bus = &usb_bus_type;
 	dev->dev.type = &usb_device_type;
 	dev->dev.groups = usb_device_groups;
+	/*
+	 * 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.
+	 */
 	dev->dev.dma_mask = bus->controller->dma_mask;
+	dev->dev.dma_pfn_offset = bus->controller->dma_pfn_offset;
 	set_dev_node(&dev->dev, dev_to_node(bus->controller));
 	dev->state = USB_STATE_ATTACHED;
 	dev->lpm_disable_count = 1;
-- 
2.7.4

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

[Arnd Bergmann]

On Tuesday, September 13, 2016 11:16:03 AM CEST Roger Quadros wrote:
> If dma_pfn_offset is not inherited correctly from the host controller,
> it might result in sub-optimal configuration as bounce
> buffer limit might be set to less than optimal level.
> 
> Consider the mass storage device case.
> USB storage driver creates a scsi host for the mass storage interface in
> drivers/usb/storage/usb.c
> The scsi host parent device is nothing but the the USB interface device.
> Now, __scsi_init_queue() calls scsi_calculate_bounce_limit() to find out
> and set the block layer bounce limit.
> scsi_calculate_bounce_limit() uses dma_max_pfn(host_dev) to get the
> bounce_limit. host_dev is nothing but the device representing the
> mass storage interface.
> If that device doesn't have the right dma_pfn_offset, then dma_max_pfn()
> is messed up and the bounce buffer limit is wrong.
> 
> e.g. On Keystone 2 systems, dma_max_pfn() is 0x87FFFF and dma_mask_pfn
> is 0xFFFFF. Consider a mass storage use case: Without this patch,
> usb scsi host device (usb-storage) will get a dma_pfn_offset of 0 resulting
> in a dma_max_pfn() of 0xFFFFF within the scsi layer
> (scsi_calculate_bounce_limit()).
> This will result in bounce buffers being unnecessarily used.
> 
> Hint: On 32-bit ARM platforms dma_max_pfn() = dma_mask_pfn + dma_pfn_offset
> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>

Acked-by: Arnd Bergmann <arnd@arndb.de>

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

[Alan Stern]

On Tue, 13 Sep 2016, Arnd Bergmann wrote:

> On Tuesday, September 13, 2016 11:16:03 AM CEST Roger Quadros wrote:
> > If dma_pfn_offset is not inherited correctly from the host controller,
> > it might result in sub-optimal configuration as bounce
> > buffer limit might be set to less than optimal level.
> > 
> > Consider the mass storage device case.
> > USB storage driver creates a scsi host for the mass storage interface in
> > drivers/usb/storage/usb.c
> > The scsi host parent device is nothing but the the USB interface device.
> > Now, __scsi_init_queue() calls scsi_calculate_bounce_limit() to find out
> > and set the block layer bounce limit.
> > scsi_calculate_bounce_limit() uses dma_max_pfn(host_dev) to get the
> > bounce_limit. host_dev is nothing but the device representing the
> > mass storage interface.
> > If that device doesn't have the right dma_pfn_offset, then dma_max_pfn()
> > is messed up and the bounce buffer limit is wrong.
> > 
> > e.g. On Keystone 2 systems, dma_max_pfn() is 0x87FFFF and dma_mask_pfn
> > is 0xFFFFF. Consider a mass storage use case: Without this patch,
> > usb scsi host device (usb-storage) will get a dma_pfn_offset of 0 resulting
> > in a dma_max_pfn() of 0xFFFFF within the scsi layer
> > (scsi_calculate_bounce_limit()).
> > This will result in bounce buffers being unnecessarily used.
> > 
> > Hint: On 32-bit ARM platforms dma_max_pfn() = dma_mask_pfn + dma_pfn_offset
> > 
> > Signed-off-by: Roger Quadros <rogerq@ti.com>
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Alan Stern <stern@rowland.harvard.edu>