[PATCH 1/3] uas: Allow uas_use_uas_driver to return usb-storage flags

[PATCH 1/3] uas: Allow uas_use_uas_driver to return usb-storage flags

[Hans de Goede]

uas_use_uas_driver may set some US_FL_foo flags during detection, currently
these are stored in a local variable and then throw away, but these may be
of interest to the caller, so add an extra parameter to (optionally) return
the detected flags, and use this in the uas driver.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 drivers/usb/storage/uas-detect.h | 6 +++++-
 drivers/usb/storage/uas.c        | 6 +++---
 drivers/usb/storage/usb.c        | 2 +-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/storage/uas-detect.h b/drivers/usb/storage/uas-detect.h
index 9893d69..63ae161 100644
--- a/drivers/usb/storage/uas-detect.h
+++ b/drivers/usb/storage/uas-detect.h
@@ -51,7 +51,8 @@ static int uas_find_endpoints(struct usb_host_interface *alt,
 }
 
 static int uas_use_uas_driver(struct usb_interface *intf,
-			      const struct usb_device_id *id)
+			      const struct usb_device_id *id,
+			      unsigned long *flags_ret)
 {
 	struct usb_host_endpoint *eps[4] = { };
 	struct usb_device *udev = interface_to_usbdev(intf);
@@ -132,5 +133,8 @@ static int uas_use_uas_driver(struct usb_interface *intf,
 		return 0;
 	}
 
+	if (flags_ret)
+		*flags_ret = flags;
+
 	return 1;
 }
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 6cdabdc..c6109c1 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -887,8 +887,9 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id)
 	struct Scsi_Host *shost = NULL;
 	struct uas_dev_info *devinfo;
 	struct usb_device *udev = interface_to_usbdev(intf);
+	unsigned long dev_flags;
 
-	if (!uas_use_uas_driver(intf, id))
+	if (!uas_use_uas_driver(intf, id, &dev_flags))
 		return -ENODEV;
 
 	if (uas_switch_interface(udev, intf))
@@ -910,8 +911,7 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id)
 	devinfo->udev = udev;
 	devinfo->resetting = 0;
 	devinfo->shutdown = 0;
-	devinfo->flags = id->driver_info;
-	usb_stor_adjust_quirks(udev, &devinfo->flags);
+	devinfo->flags = dev_flags;
 	init_usb_anchor(&devinfo->cmd_urbs);
 	init_usb_anchor(&devinfo->sense_urbs);
 	init_usb_anchor(&devinfo->data_urbs);
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 5600c33..db6f6b5 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -1080,7 +1080,7 @@ static int storage_probe(struct usb_interface *intf,
 
 	/* If uas is enabled and this device can do uas then ignore it. */
 #if IS_ENABLED(CONFIG_USB_UAS)
-	if (uas_use_uas_driver(intf, id))
+	if (uas_use_uas_driver(intf, id, NULL))
 		return -ENXIO;
 #endif
 
-- 
2.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] uas: Add US_FL_MAX_SECTORS_240 flag

[Hans de Goede]

The usb-storage driver sets max_sectors = 240 in its scsi-host template, for
uas we do not want to do that for all devices, but testing has shown that
some devices need it.

This commit adds a US_FL_MAX_SECTORS_240 flag for such devices, and implements
support for it in uas.c, while at it it also adds support for
US_FL_MAX_SECTORS_64 to uas.c.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 drivers/usb/storage/uas.c | 10 +++++++++-
 drivers/usb/storage/usb.c |  6 +++++-
 include/linux/usb_usual.h |  2 ++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index c6109c1..6d3122a 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -759,7 +759,10 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd)
 
 static int uas_slave_alloc(struct scsi_device *sdev)
 {
-	sdev->hostdata = (void *)sdev->host->hostdata;
+	struct uas_dev_info *devinfo =
+		(struct uas_dev_info *)sdev->host->hostdata;
+
+	sdev->hostdata = devinfo;
 
 	/* USB has unusual DMA-alignment requirements: Although the
 	 * starting address of each scatter-gather element doesn't matter,
@@ -778,6 +781,11 @@ static int uas_slave_alloc(struct scsi_device *sdev)
 	 */
 	blk_queue_update_dma_alignment(sdev->request_queue, (512 - 1));
 
+	if (devinfo->flags & US_FL_MAX_SECTORS_64)
+		blk_queue_max_hw_sectors(sdev->request_queue, 64);
+	else if (devinfo->flags & US_FL_MAX_SECTORS_240)
+		blk_queue_max_hw_sectors(sdev->request_queue, 240);
+
 	return 0;
 }
 
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index db6f6b5..6c10c88 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -479,7 +479,8 @@ void usb_stor_adjust_quirks(struct usb_device *udev, unsigned long *fflags)
 			US_FL_SINGLE_LUN | US_FL_NO_WP_DETECT |
 			US_FL_NO_READ_DISC_INFO | US_FL_NO_READ_CAPACITY_16 |
 			US_FL_INITIAL_READ10 | US_FL_WRITE_CACHE |
-			US_FL_NO_ATA_1X | US_FL_NO_REPORT_OPCODES);
+			US_FL_NO_ATA_1X | US_FL_NO_REPORT_OPCODES |
+			US_FL_MAX_SECTORS_240);
 
 	p = quirks;
 	while (*p) {
@@ -520,6 +521,9 @@ void usb_stor_adjust_quirks(struct usb_device *udev, unsigned long *fflags)
 		case 'f':
 			f |= US_FL_NO_REPORT_OPCODES;
 			break;
+		case 'g':
+			f |= US_FL_MAX_SECTORS_240;
+			break;
 		case 'h':
 			f |= US_FL_CAPACITY_HEURISTICS;
 			break;
diff --git a/include/linux/usb_usual.h b/include/linux/usb_usual.h
index a7f2604..7f5f78b 100644
--- a/include/linux/usb_usual.h
+++ b/include/linux/usb_usual.h
@@ -77,6 +77,8 @@
 		/* Cannot handle ATA_12 or ATA_16 CDBs */	\
 	US_FLAG(NO_REPORT_OPCODES,	0x04000000)		\
 		/* Cannot handle MI_REPORT_SUPPORTED_OPERATION_CODES */	\
+	US_FLAG(MAX_SECTORS_240,	0x08000000)		\
+		/* Sets max_sectors to 240 */			\
 
 #define US_FLAG(name, value)	US_FL_##name = value ,
 enum { US_DO_ALL_FLAGS };
-- 
2.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] uas: Add US_FL_MAX_SECTORS_240 flag

[Alan Stern]

On Thu, 16 Apr 2015, Hans de Goede wrote:

> The usb-storage driver sets max_sectors = 240 in its scsi-host template, for
> uas we do not want to do that for all devices, but testing has shown that
> some devices need it.
> 
> This commit adds a US_FL_MAX_SECTORS_240 flag for such devices, and implements
> support for it in uas.c, while at it it also adds support for
> US_FL_MAX_SECTORS_64 to uas.c.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

You forgot to update Documentation/kernel-parameters.txt.

Alan Stern

Re: [PATCH 1/3] uas: Allow uas_use_uas_driver to return usb-storage flags

[Steve Bangert]

On Thu, 2015-04-16 at 14:17 +0200, Hans de Goede wrote:
> uas_use_uas_driver may set some US_FL_foo flags during detection, currently
> these are stored in a local variable and then throw away, but these may be
> of interest to the caller, so add an extra parameter to (optionally) return
> the detected flags, and use this in the uas driver.
> 
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>


Patched (all 3 of them) and compile tested on the current Fedora kernel
(3.19.3-200), 

device is accessible and functioning without a kernel parameter

Steve


> ---
>  drivers/usb/storage/uas-detect.h | 6 +++++-
>  drivers/usb/storage/uas.c        | 6 +++---
>  drivers/usb/storage/usb.c        | 2 +-
>  3 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/storage/uas-detect.h b/drivers/usb/storage/uas-detect.h
> index 9893d69..63ae161 100644
> --- a/drivers/usb/storage/uas-detect.h
> +++ b/drivers/usb/storage/uas-detect.h
> @@ -51,7 +51,8 @@ static int uas_find_endpoints(struct usb_host_interface *alt,
>  }
>  
>  static int uas_use_uas_driver(struct usb_interface *intf,
> -			      const struct usb_device_id *id)
> +			      const struct usb_device_id *id,
> +			      unsigned long *flags_ret)
>  {
>  	struct usb_host_endpoint *eps[4] = { };
>  	struct usb_device *udev = interface_to_usbdev(intf);
> @@ -132,5 +133,8 @@ static int uas_use_uas_driver(struct usb_interface *intf,
>  		return 0;
>  	}
>  
> +	if (flags_ret)
> +		*flags_ret = flags;
> +
>  	return 1;
>  }
> diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
> index 6cdabdc..c6109c1 100644
> --- a/drivers/usb/storage/uas.c
> +++ b/drivers/usb/storage/uas.c
> @@ -887,8 +887,9 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id)
>  	struct Scsi_Host *shost = NULL;
>  	struct uas_dev_info *devinfo;
>  	struct usb_device *udev = interface_to_usbdev(intf);
> +	unsigned long dev_flags;
>  
> -	if (!uas_use_uas_driver(intf, id))
> +	if (!uas_use_uas_driver(intf, id, &dev_flags))
>  		return -ENODEV;
>  
>  	if (uas_switch_interface(udev, intf))
> @@ -910,8 +911,7 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id)
>  	devinfo->udev = udev;
>  	devinfo->resetting = 0;
>  	devinfo->shutdown = 0;
> -	devinfo->flags = id->driver_info;
> -	usb_stor_adjust_quirks(udev, &devinfo->flags);
> +	devinfo->flags = dev_flags;
>  	init_usb_anchor(&devinfo->cmd_urbs);
>  	init_usb_anchor(&devinfo->sense_urbs);
>  	init_usb_anchor(&devinfo->data_urbs);
> diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
> index 5600c33..db6f6b5 100644
> --- a/drivers/usb/storage/usb.c
> +++ b/drivers/usb/storage/usb.c
> @@ -1080,7 +1080,7 @@ static int storage_probe(struct usb_interface *intf,
>  
>  	/* If uas is enabled and this device can do uas then ignore it. */
>  #if IS_ENABLED(CONFIG_USB_UAS)
> -	if (uas_use_uas_driver(intf, id))
> +	if (uas_use_uas_driver(intf, id, NULL))
>  		return -ENXIO;
>  #endif
>  


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] uas: Set max_sectors_240 quirk for ASM1053 devices

[Hans de Goede]

Testing has shown that ASM1053 devices do not work properly with transfers
larger than 240 sectors, so set max_sectors to 240 on these.

Reported-by: Steve Bangert <sbangert@frontier.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/storage/uas-detect.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/storage/uas-detect.h b/drivers/usb/storage/uas-detect.h
index 63ae161..f58caa9 100644
--- a/drivers/usb/storage/uas-detect.h
+++ b/drivers/usb/storage/uas-detect.h
@@ -74,7 +74,7 @@ static int uas_use_uas_driver(struct usb_interface *intf,
 	 * this writing the following versions exist:
 	 * ASM1051 - no uas support version
 	 * ASM1051 - with broken (*) uas support
-	 * ASM1053 - with working uas support
+	 * ASM1053 - with working uas support, but problems with large xfers
 	 * ASM1153 - with working uas support
 	 *
 	 * Devices with these chips re-use a number of device-ids over the
@@ -104,6 +104,9 @@ static int uas_use_uas_driver(struct usb_interface *intf,
 		} else if (usb_ss_max_streams(&eps[1]->ss_ep_comp) == 32) {
 			/* Possibly an ASM1051, disable uas */
 			flags |= US_FL_IGNORE_UAS;
+		} else {
+			/* ASM1053, these have issues with large transfers */
+			flags |= US_FL_MAX_SECTORS_240;
 		}
 	}
 
-- 
2.3.5