diff mbox series

USB: UAS: introduce a quirk to set no_write_same

Message ID 20201209110734.30831-1-oneukum@suse.com (mailing list archive)
State Superseded
Headers show
Series USB: UAS: introduce a quirk to set no_write_same | expand

Commit Message

Oliver Neukum Dec. 9, 2020, 11:07 a.m. UTC
UAS does not share the pessimistic assumption storage
is making that devices cannot deal with WRITE_SAME.
A few devices supported by UAS, are reported to not
deal well with WRITE_SAME. Those need a quirk.

Add it to the device that needs it.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
Reported-by: David C. Partridge <david.partridge@perdrix.co.uk>
---
 drivers/usb/storage/uas.c         | 3 +++
 drivers/usb/storage/unusual_uas.h | 7 +++++--
 drivers/usb/storage/usb.c         | 3 +++
 include/linux/usb_usual.h         | 2 ++
 4 files changed, 13 insertions(+), 2 deletions(-)

Comments

Greg Kroah-Hartman Dec. 9, 2020, 12:03 p.m. UTC | #1
On Wed, Dec 09, 2020 at 12:07:34PM +0100, Oliver Neukum wrote:
> UAS does not share the pessimistic assumption storage
> is making that devices cannot deal with WRITE_SAME.
> A few devices supported by UAS, are reported to not
> deal well with WRITE_SAME. Those need a quirk.
> 
> Add it to the device that needs it.
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> Reported-by: David C. Partridge <david.partridge@perdrix.co.uk>
> ---
>  drivers/usb/storage/uas.c         | 3 +++
>  drivers/usb/storage/unusual_uas.h | 7 +++++--
>  drivers/usb/storage/usb.c         | 3 +++
>  include/linux/usb_usual.h         | 2 ++
>  4 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
> index 56422c4b4ff3..bef89c6bd1d7 100644
> --- a/drivers/usb/storage/uas.c
> +++ b/drivers/usb/storage/uas.c
> @@ -868,6 +868,9 @@ static int uas_slave_configure(struct scsi_device *sdev)
>  	if (devinfo->flags & US_FL_NO_READ_CAPACITY_16)
>  		sdev->no_read_capacity_16 = 1;
>  
> +	/* Some disks cannot handle WRITE_SAME */
> +	if (devinfo->flags & US_FL_NO_SAME)
> +		sdev->no_write_same = 1;
>  	/*
>  	 * Some disks return the total number of blocks in response
>  	 * to READ CAPACITY rather than the highest block number.
> diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h
> index 711ab240058c..870e9cf3d5dc 100644
> --- a/drivers/usb/storage/unusual_uas.h
> +++ b/drivers/usb/storage/unusual_uas.h
> @@ -35,12 +35,15 @@ UNUSUAL_DEV(0x054c, 0x087d, 0x0000, 0x9999,
>  		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
>  		US_FL_NO_REPORT_OPCODES),
>  
> -/* Reported-by: Julian Groß <julian.g@posteo.de> */
> +/*
> + *  Initially Reported-by: Julian Groß <julian.g@posteo.de>
> + *  Further reports David C. Partridge <david.partridge@perdrix.co.uk>
> + */
>  UNUSUAL_DEV(0x059f, 0x105f, 0x0000, 0x9999,
>  		"LaCie",
>  		"2Big Quadra USB3",
>  		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
> -		US_FL_NO_REPORT_OPCODES),
> +		US_FL_NO_REPORT_OPCODES | US_FL_NO_SAME),
>  
>  /*
>   * Apricorn USB3 dongle sometimes returns "USBSUSBSUSBS" in response to SCSI
> diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
> index 94a64729dc27..90aa9c12ffac 100644
> --- a/drivers/usb/storage/usb.c
> +++ b/drivers/usb/storage/usb.c
> @@ -541,6 +541,9 @@ void usb_stor_adjust_quirks(struct usb_device *udev, unsigned long *fflags)
>  		case 'j':
>  			f |= US_FL_NO_REPORT_LUNS;
>  			break;
> +		case 'k':
> +			f |= US_FL_NO_SAME;
> +			break;

Shouldn't this new flag be documented somewhere?

thanks,

greg k-h
David C. Partridge Dec. 9, 2020, 2:06 p.m. UTC | #2
Probably here:

https://www.kernel.org/doc/html/latest/admin-guide/kernel-parameters.html

in the usb-storage.quirks section?

I assume there's also a header file that defines all this stuff?

David

-----Original Message-----
From: Greg KH <gregkh@linuxfoundation.org> 
Sent: 09 December 2020 12:04
To: Oliver Neukum <oneukum@suse.com>
Cc: linux-usb@vger.kernel.org; david.partridge@perdrix.co.uk
Subject: Re: [PATCH] USB: UAS: introduce a quirk to set no_write_same

On Wed, Dec 09, 2020 at 12:07:34PM +0100, Oliver Neukum wrote:
> UAS does not share the pessimistic assumption storage
> is making that devices cannot deal with WRITE_SAME.
> A few devices supported by UAS, are reported to not
> deal well with WRITE_SAME. Those need a quirk.
> 
> Add it to the device that needs it.
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> Reported-by: David C. Partridge <david.partridge@perdrix.co.uk>
> ---
>  drivers/usb/storage/uas.c         | 3 +++
>  drivers/usb/storage/unusual_uas.h | 7 +++++--
>  drivers/usb/storage/usb.c         | 3 +++
>  include/linux/usb_usual.h         | 2 ++
>  4 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
> index 56422c4b4ff3..bef89c6bd1d7 100644
> --- a/drivers/usb/storage/uas.c
> +++ b/drivers/usb/storage/uas.c
> @@ -868,6 +868,9 @@ static int uas_slave_configure(struct scsi_device
*sdev)
>  	if (devinfo->flags & US_FL_NO_READ_CAPACITY_16)
>  		sdev->no_read_capacity_16 = 1;
>  
> +	/* Some disks cannot handle WRITE_SAME */
> +	if (devinfo->flags & US_FL_NO_SAME)
> +		sdev->no_write_same = 1;
>  	/*
>  	 * Some disks return the total number of blocks in response
>  	 * to READ CAPACITY rather than the highest block number.
> diff --git a/drivers/usb/storage/unusual_uas.h
b/drivers/usb/storage/unusual_uas.h
> index 711ab240058c..870e9cf3d5dc 100644
> --- a/drivers/usb/storage/unusual_uas.h
> +++ b/drivers/usb/storage/unusual_uas.h
> @@ -35,12 +35,15 @@ UNUSUAL_DEV(0x054c, 0x087d, 0x0000, 0x9999,
>  		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
>  		US_FL_NO_REPORT_OPCODES),
>  
> -/* Reported-by: Julian Groß <julian.g@posteo.de> */
> +/*
> + *  Initially Reported-by: Julian Groß <julian.g@posteo.de>
> + *  Further reports David C. Partridge <david.partridge@perdrix.co.uk>
> + */
>  UNUSUAL_DEV(0x059f, 0x105f, 0x0000, 0x9999,
>  		"LaCie",
>  		"2Big Quadra USB3",
>  		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
> -		US_FL_NO_REPORT_OPCODES),
> +		US_FL_NO_REPORT_OPCODES | US_FL_NO_SAME),
>  
>  /*
>   * Apricorn USB3 dongle sometimes returns "USBSUSBSUSBS" in response to
SCSI
> diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
> index 94a64729dc27..90aa9c12ffac 100644
> --- a/drivers/usb/storage/usb.c
> +++ b/drivers/usb/storage/usb.c
> @@ -541,6 +541,9 @@ void usb_stor_adjust_quirks(struct usb_device *udev,
unsigned long *fflags)
>  		case 'j':
>  			f |= US_FL_NO_REPORT_LUNS;
>  			break;
> +		case 'k':
> +			f |= US_FL_NO_SAME;
> +			break;

Shouldn't this new flag be documented somewhere?

thanks,

greg k-h
diff mbox series

Patch

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 56422c4b4ff3..bef89c6bd1d7 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -868,6 +868,9 @@  static int uas_slave_configure(struct scsi_device *sdev)
 	if (devinfo->flags & US_FL_NO_READ_CAPACITY_16)
 		sdev->no_read_capacity_16 = 1;
 
+	/* Some disks cannot handle WRITE_SAME */
+	if (devinfo->flags & US_FL_NO_SAME)
+		sdev->no_write_same = 1;
 	/*
 	 * Some disks return the total number of blocks in response
 	 * to READ CAPACITY rather than the highest block number.
diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h
index 711ab240058c..870e9cf3d5dc 100644
--- a/drivers/usb/storage/unusual_uas.h
+++ b/drivers/usb/storage/unusual_uas.h
@@ -35,12 +35,15 @@  UNUSUAL_DEV(0x054c, 0x087d, 0x0000, 0x9999,
 		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
 		US_FL_NO_REPORT_OPCODES),
 
-/* Reported-by: Julian Groß <julian.g@posteo.de> */
+/*
+ *  Initially Reported-by: Julian Groß <julian.g@posteo.de>
+ *  Further reports David C. Partridge <david.partridge@perdrix.co.uk>
+ */
 UNUSUAL_DEV(0x059f, 0x105f, 0x0000, 0x9999,
 		"LaCie",
 		"2Big Quadra USB3",
 		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
-		US_FL_NO_REPORT_OPCODES),
+		US_FL_NO_REPORT_OPCODES | US_FL_NO_SAME),
 
 /*
  * Apricorn USB3 dongle sometimes returns "USBSUSBSUSBS" in response to SCSI
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 94a64729dc27..90aa9c12ffac 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -541,6 +541,9 @@  void usb_stor_adjust_quirks(struct usb_device *udev, unsigned long *fflags)
 		case 'j':
 			f |= US_FL_NO_REPORT_LUNS;
 			break;
+		case 'k':
+			f |= US_FL_NO_SAME;
+			break;
 		case 'l':
 			f |= US_FL_NOT_LOCKABLE;
 			break;
diff --git a/include/linux/usb_usual.h b/include/linux/usb_usual.h
index 4a19ac3f24d0..6b03fdd69d27 100644
--- a/include/linux/usb_usual.h
+++ b/include/linux/usb_usual.h
@@ -84,6 +84,8 @@ 
 		/* Cannot handle REPORT_LUNS */			\
 	US_FLAG(ALWAYS_SYNC, 0x20000000)			\
 		/* lies about caching, so always sync */	\
+	US_FLAG(NO_SAME, 0x40000000)				\
+		/* Cannot handle WRITE_SAME */			\
 
 #define US_FLAG(name, value)	US_FL_##name = value ,
 enum { US_DO_ALL_FLAGS };