diff mbox

uas: Add US_FL_NO_ATA_1X quirk for one more Seagate device

Message ID 9f0486e5-7ff7-b612-3465-04f47e8c2545@redhat.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Hans de Goede Nov. 14, 2017, 2:04 p.m. UTC 
Hi,

On 14-11-17 15:00, Hans de Goede wrote:
> Just like all previous UAS capable Seagate disk enclosures, this
> one needs a US_FL_NO_ATA_1X quirk.
> 
> Cc: stable@vger.kernel.org # 3.16
> Reported-by: Wido <wido.gg@gmail.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>


So far we've been adding quirks for Seagate drives on a one by one
basis (I started this myself) hoping that one day they will fix
their ATA_1X pass-through support. But that does not seem to
be happening.

Maybe we need to do something like this instead ?   :



Then we can remove a whole lot of quirks and we avoid future
churn when new Seagate device ids show up.

Regards,

Hans



> ---
>   drivers/usb/storage/unusual_uas.h | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h
> index 2fe4fd336446..a1ddcbfb7323 100644
> --- a/drivers/usb/storage/unusual_uas.h
> +++ b/drivers/usb/storage/unusual_uas.h
> @@ -114,6 +114,13 @@ UNUSUAL_DEV(0x0bc2, 0xab21, 0x0000, 0x9999,
>   		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
>   		US_FL_NO_ATA_1X),
>   
> +/* Reported-by: Wido <wido.gg@gmail.com> */
> +UNUSUAL_DEV(0x0bc2, 0xab24, 0x0000, 0x9999,
> +		"Seagate",
> +		"BUP Slim BK",
> +		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
> +		US_FL_NO_ATA_1X),
> +
>   /* Reported-by: G. Richard Bellamy <rbellamy@pteradigm.com> */
>   UNUSUAL_DEV(0x0bc2, 0xab2a, 0x0000, 0x9999,
>   		"Seagate",
>

Comments

Alan Stern Nov. 14, 2017, 3:25 p.m. UTC | #1 
On Tue, 14 Nov 2017, Hans de Goede wrote:

> Hi,
> 
> On 14-11-17 15:00, Hans de Goede wrote:
> > Just like all previous UAS capable Seagate disk enclosures, this
> > one needs a US_FL_NO_ATA_1X quirk.
> > 
> > Cc: stable@vger.kernel.org # 3.16
> > Reported-by: Wido <wido.gg@gmail.com>
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> 
> So far we've been adding quirks for Seagate drives on a one by one
> basis (I started this myself) hoping that one day they will fix
> their ATA_1X pass-through support. But that does not seem to
> be happening.
> 
> Maybe we need to do something like this instead ?   :
> 
> --- a/drivers/usb/storage/uas-detect.h
> +++ b/drivers/usb/storage/uas-detect.h
> @@ -111,6 +111,10 @@ static int uas_use_uas_driver(struct usb_interface *intf,
>                  }
>          }
> 
> + /* All Seagate disk enclosures have broken ATA pass-through support */
> + if (le16_to_cpu(udev->descriptor.idVendor) == 0x0bc2)
> +         flags |= US_FL_NO_ATA_1X;
> +
>          usb_stor_adjust_quirks(udev, &flags);
> 
>          if (flags & US_FL_IGNORE_UAS) {
> 
> 
> Then we can remove a whole lot of quirks and we avoid future
> churn when new Seagate device ids show up.

That is a reasonable approach.  For what it's worth, usb-storage has 
had similar code for many years, affecting devices from Nokia, Nikon, 
Pentax, and Motorola.

Alan Stern
Hans de Goede Nov. 14, 2017, 5:44 p.m. UTC | #2 
Hi,

On 11/14/2017 04:25 PM, Alan Stern wrote:
> On Tue, 14 Nov 2017, Hans de Goede wrote:
> 
>> Hi,
>>
>> On 14-11-17 15:00, Hans de Goede wrote:
>>> Just like all previous UAS capable Seagate disk enclosures, this
>>> one needs a US_FL_NO_ATA_1X quirk.
>>>
>>> Cc: stable@vger.kernel.org # 3.16
>>> Reported-by: Wido <wido.gg@gmail.com>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>
>>
>> So far we've been adding quirks for Seagate drives on a one by one
>> basis (I started this myself) hoping that one day they will fix
>> their ATA_1X pass-through support. But that does not seem to
>> be happening.
>>
>> Maybe we need to do something like this instead ?   :
>>
>> --- a/drivers/usb/storage/uas-detect.h
>> +++ b/drivers/usb/storage/uas-detect.h
>> @@ -111,6 +111,10 @@ static int uas_use_uas_driver(struct usb_interface *intf,
>>                   }
>>           }
>>
>> + /* All Seagate disk enclosures have broken ATA pass-through support */
>> + if (le16_to_cpu(udev->descriptor.idVendor) == 0x0bc2)
>> +         flags |= US_FL_NO_ATA_1X;
>> +
>>           usb_stor_adjust_quirks(udev, &flags);
>>
>>           if (flags & US_FL_IGNORE_UAS) {
>>
>>
>> Then we can remove a whole lot of quirks and we avoid future
>> churn when new Seagate device ids show up.
> 
> That is a reasonable approach.  For what it's worth, usb-storage has
> had similar code for many years, affecting devices from Nokia, Nikon,
> Pentax, and Motorola.


Ok.

Greg, please do no merge the 2 recent uas seagate quirks I send
then. I will submit a patch with the new approach right away.

Once the patch with the new approach is merged I will
submit a patch to remove all the seagate quirks.

Regards,

Hans
Oliver Neukum Nov. 15, 2017, 9:06 a.m. UTC | #3 
Am Dienstag, den 14.11.2017, 18:44 +0100 schrieb Hans de Goede:
> 
> Greg, please do no merge the 2 recent uas seagate quirks I send
> then. I will submit a patch with the new approach right away.

Hi,

I am afraid in that case we will need a way to override a
quirk in the other direction, that is, to not apply it.

	Regards
		Oliver
Hans de Goede Nov. 15, 2017, 9:32 a.m. UTC | #4 
Hi,

On 15-11-17 10:06, Oliver Neukum wrote:
> Am Dienstag, den 14.11.2017, 18:44 +0100 schrieb Hans de Goede:
>>
>> Greg, please do no merge the 2 recent uas seagate quirks I send
>> then. I will submit a patch with the new approach right away.
> 
> Hi,
> 
> I am afraid in that case we will need a way to override a
> quirk in the other direction, that is, to not apply it.

We already have that usb_stor_adjust_quirks when a match to the
device's vend:prod ids is found in usb_storage.quirks,
usb_stor_adjust_quirks will clear all quirks it allows to be set
through the usb_storage.quirks module option.

Regards,

Hans
diff mbox

Patch

--- a/drivers/usb/storage/uas-detect.h
+++ b/drivers/usb/storage/uas-detect.h
@@ -111,6 +111,10 @@  static int uas_use_uas_driver(struct usb_interface *intf,
                 }
         }

+ /* All Seagate disk enclosures have broken ATA pass-through support */
+ if (le16_to_cpu(udev->descriptor.idVendor) == 0x0bc2)
+         flags |= US_FL_NO_ATA_1X;
+
         usb_stor_adjust_quirks(udev, &flags);

         if (flags & US_FL_IGNORE_UAS) {