Hi,
On 4/25/21 12:41 PM, Rene Rebe wrote:
> Greg KH wrote:
>
>> On Sun, Apr 25, 2021 at 09:20:59AM +0200, René Rebe wrote:
>>> Hey,
>>>
>>>> On 25. Apr 2021, at 04:31, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
>>>>>
>>>>> --- 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.
>>> I don’t have any non-supporting device - can we not just add them to the
>>> quirk list when someone reports one?
>>
>> How about since you know your device works, you make the check detect
>> your specific device and not apply the flag to it? You should be able
>> to do so based on the
>
> Sure, while that does not really solve this for all the other newer
> Seagate drives other users might have at home, here is a patch
> checking for this one USB product ID. I hope that is what you meant:
>
> Signed-off-by: René Rebe <rene@xxxxxxxxxxxxx>
>
> --- 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,5 +113,6 @@
>
> /* All Seagate disk enclosures have broken ATA pass-through support */
> - if (le16_to_cpu(udev->descriptor.idVendor) == 0x0bc2)
> + if ((le16_to_cpu(udev->descriptor.idVendor) == 0x0bc2) &&
> + (le16_to_cpu(udev->descriptor.idProduct) != 0xab03))
> flags |= US_FL_NO_ATA_1X;
>
>
As I indicated in my other email which crossed with this one, please make this
more generic, add a new US_FL_ATA_1X_OK flag and make the above code check that +
add a new unusual_uas.h entry for your device setting the new flag.
Note there is no need to add support for the new flag to usb_stor_adjust_quirks()
if a user overrides quirks for a device on the kernel commandline without specifying
the "t" flag then the US_FL_NO_ATA_1X flag will already get cleared.
I deliberately put the:
if (le16_to_cpu(udev->descriptor.idVendor) == 0x0bc2)
flags |= US_FL_NO_ATA_1X;
code before the usb_stor_adjust_quirks() call to allow users to override this
from the kernel commandline.
Regards,
Hans
