Re: [PATCH v2 3/3] ata: ahci: Skip 200 ms debounce delay for AMD 300 Series Chipset SATA Controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/14/22 00:28, Limonciello, Mario wrote:
[Public]



-----Original Message-----
From: Paul Menzel <pmenzel@xxxxxxxxxxxxx>
Sent: Tuesday, September 13, 2022 10:23
To: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
Cc: Limonciello, Mario <Mario.Limonciello@xxxxxxx>; Hans de Goede
<hdegoede@xxxxxxxxxx>; linux-ide@xxxxxxxxxxxxxxx; LKML <linux-
kernel@xxxxxxxxxxxxxxx>
Subject: Re: [PATCH v2 3/3] ata: ahci: Skip 200 ms debounce delay for AMD 300
Series Chipset SATA Controller

Dear Damien,


Am 01.09.22 um 00:13 schrieb Damien Le Moal:
On 8/30/22 18:05, Paul Menzel wrote:

[…]

Am 01.06.22 um 10:58 schrieb Damien Le Moal:
On 6/1/22 01:18, Paul Menzel wrote:
With that in mind, I am not planning to apply your previous patches
for 5.18, as they would conflict and would only end up being churn
since the delay removal by default will undo your changes.
Obviously, I do not agree, as this would give the a little bit more
testing already, if changing the default is a good idea. Also, if the
conflict will be hard to resolve, I happily do it (the patches could
even be reverted on top – git commits are cheap and easy to handle).

The conflict is not hard to resolve. The point is that my patches changing
the default to no debounce delay completely remove the changes of your
patch to do the same for one or some adapters. So adding your patches
now
and then my patches on top does not make much sense at all.

If too many problems show up and I end up reverting/removing the
patches,
then I will be happy to take your patches for the adapter you tested. Note
that *all* the machines I have tested so far are OK without a debounce
delay too. So we could add them too... And endup with a long list of
adapters that use the default ahci driver without debounce delay. The
goal
of changing the default to no delay is to avoid that. So far, the adapters
I have identified that need the delay have their own declaration, so we
only need to add a flag there. Simpler change that listing up adapters
that are OK without the delay.

Anyway, I wrote my piece, but you are the maintainer, so it’s your call
and I stop bothering you.

I just wanted to inquire about the status of your changes? I do not find
them in your `for-5.19` branch. As they should be tested in linux-next
before the merge window opens, if these are not ready yet, could you
please apply my (tested) patches?

I could, but 5.19 now has an updated libata.force kernel parameter that
allows one to disable the debounce delay for a particular port or for all
ports of an adapter. See libata.force=x.y:nodbdelay for a port y of
adapter x or libata.force=x:nodbdelay for all ports of adapter x.

This is commit 3af9ca4d341d (ata: libata-core: Improve link flags forced
settings) [1]. Thank you, this is really useful, but easily overlooked. ;-)

I still plan to revisit the arbitrary link debounce timers but I prefer to
have the power management cleanup applied first. The reason is that link
debounce depends on PHY readiness, which itself depends heavily on power
mode transitions. My plan is to get this done during this cycle for
release with 5.20 and then fix on top the arbitrary delays for 5.21.

Nice. Can you share the current status?

No progress. I need to put together a series with all the patches that
were sent already. Unless Mario can resend something ?

No reply from Mario.

I think what happened here is there was related patches from another party
that got tangled up with this.

Niklas and I are investigating this again now because Niklas discovered that one AMD AHCI adapter leads to drive resets when the drive goes to low power mode. The adapter is:

SATA controller [0106]: Advanced Micro Devices, Inc. [AMD] FCH SATA Controller [AHCI mode] [1022:7901] (rev 51)

If we switch to performance mode (no LPM), the reset disapears. But if LPM is enabled, any command sent after the disk goes to low poer mode (device initiated), there is a link reset...


--
Damien Le Moal
Western Digital Research





[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux