Re: [PATCH] unbreak all modern Seagate ATA pass-through for SMART

Hans de Goede
2021-04-25T10:43:18Z

Hi,

On 4/25/21 9:20 AM, René Rebe wrote:
> Hey,
>
>> On 25. Apr 2021, at 04:31, Alan Stern <stern@rowland.harvard.edu> wrote:
>>> Seagate devices" in 2017. Apparently some early ones where buggy, ...
>>>
>>> However, fast forward a couple of years and this is no longer true,
>>> this Segate Seven even is already from 2016, and apparently first
>>> available in 2015. I suggest removing this rather drastic global
>>> measure, and instead only add very old broken ones with individual
>>> quirks, should any of them still be alive ;-)
>>>
>>> Signed-off-by: René Rebe <rene@exactcode.com>
>>>
>>> --- linux-5.11/drivers/usb/storage/uas-detect.h.backup 2021-03-05 11:36:00.517423726 +0100
>>> +++ linux-5.11/drivers/usb/storage/uas-detect.h 2021-03-05 11:36:16.373424544 +0100
>>> @@ -113,8 +113,4 @@
>>> }
>>>
>>> - /* 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);
>>
>> I don't want to do this unless you can suggest an approach that won't
>> suddenly break all those old buggy drives. Just because they are now
>> five years old or more doesn't mean they are no longer in use.
>
> Well, what do you propose then? A allow quirk for all new devices going forward?
> Given that the user usually needs to actively run something like smartctl
> manually on the drive I don’t see that this should cause too many issues.

At least Fedora ships with smartmontools with a systemd service monitoring the
driver enabled by default; and I would not be surprised if other distros do
the same.

> I don’t have any non-supporting device - can we not just add them to the
> quirk list when someone reports one?

We have a no regressions rule in the kernel, this would cause regressions,
so NACK. Also the code which you are removing replaced a long list of non-working
Seagate devices, it is not just one or 2 models, we started out with model
specific quirks and when the list grew too long we went for this option.

Something else to keep in mind here is that:

1. Every single user out there wants to have a working drive, and removing the
quirk breaks that for older Seagate drive enclosures.

2. Only some advanced users care about SMART monitoring.

So what you are suggesting is breaking older Seagate drives for everyone to
enable a feature which only few users actually care about.

So as Greg said I believe that the best option would be to add a new
US_FL_ATA_1X_OK flag, add a quirk for your drive(s) and make the code setting
US_FL_NO_ATA_1X for all Seagate devices check that flag.

Regards,

Hans