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

[Paul Menzel]

Dear Damien,


Sorry for the late reply, and thank you for your great work.

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?

> Is the libata.force solution OK for you until we have a final more solid
> fix that can benefit most modern adapters (and not just the ones you
> identified)? If you do have a use case that needs a "nodbdelay" horkage
> due to some constraint in the field, then I will apply your patches, but
> they likely will be voided by coming changes. Let me know.

I think, applying the patch would be an improvement, as people wouldn’t 
need to update their Linux kernel command line, and I do not mind, if it 
gets reverted/dropped later.


Kind regards,

Paul


[1]: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3af9ca4d341d2b8756fa9056ca0715915480e251