Hi,
On 4/25/21 12:47 PM, Hans de Goede wrote:
> 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.
p.s.
A "git log drivers/usb/storage/unusual_uas.h" quickly finds the commit which removed the
quirks which the generic Seagate check replaces. At that time there were US_FL_NO_ATA_1X
quirks for *9* different Seagate models present in unusual_uas.h and I assume someone
reporting a 10th model is what made me go for the just disable this for all Seagate
driver option.
See commit 92335ad9e895 ("uas: Remove US_FL_NO_ATA_1X unusual device entries for Seagate devices")
Also I did a quick websearch for the "Seagate Seven" and rather then the usual re-usable
drive-enclosure with a standard 2.5" or 3.5" drive in there, this seems to be a custom
model where the enclosure is actually integrated into the drive to make it smaller.
So I would not be surprised if this is using another chipset then their usual enclosures,
which would explain why it does have working ATA1x passthrough.
Regards,
Hans
