[cc: +jeff, +tejun]
Dear Damien,
Am 04.01.22 um 10:08 schrieb Damien Le Moal:
On 1/4/22 17:49, Paul Menzel wrote:
[cc: -dmitry, -guenter]
Am 04.01.22 um 09:36 schrieb Damien Le Moal:
On 12/31/21 16:08, Paul Menzel wrote:
Am 31.12.21 um 01:52 schrieb Damien Le Moal:
On 12/30/21 20:08, Paul Menzel wrote:
[…]
Jeff, Tejun, in `drivers/ata/libata-sata.c` contains a 200 ms sleep
delaying the boot noticeably for optimized setups, where Linux takes
less than 500 ms to start:
```
int sata_link_resume(struct ata_link *link, const unsigned long *params,
unsigned long deadline)
{
[…]
/*
* Writes to SControl sometimes get ignored under certain
* controllers (ata_piix SIDPR). Make sure DET actually is
* cleared.
*/
do {
scontrol = (scontrol & 0x0f0) | 0x300;
if ((rc = sata_scr_write(link, SCR_CONTROL, scontrol)))
return rc;
/*
* Some PHYs react badly if SStatus is pounded
* immediately after resuming. Delay 200ms before
* debouncing.
*/
if (!(link->flags & ATA_LFLAG_NO_DB_DELAY))
ata_msleep(link->ap, 200);
/* is SControl restored correctly? */
if ((rc = sata_scr_read(link, SCR_CONTROL, &scontrol)))
return rc;
} while ((scontrol & 0xf0f) != 0x300 && --tries);
[…]
}
```
It would indeed be great to have the default as "no delay on resume" and
add the delay only for chipsets that need it. However, it is unclear
which chipset need the delay, right?
Yes, it’s unclear for what chipset (PHY?) it was added, as the git
history i not available in the repository, and I have not found it yet.
I found the historical git archive [2], and Jeff’s commit 4effb658a0
from October 2003 [3] with the commit message
[libata] Merge Serial ATA core, and drivers for:
Intel ICH5 (production)
ServerWorks / Apple K2 (beta)
VIA (beta)
Silicon Image 3112 (broken!)
Various Promise (alpha/beta)
adds the code below:
```
void sata_phy_reset(struct ata_port *ap)
{
[…]
/* wait for phy to become ready, if necessary */
do {
msleep(200);
sstatus = scr_read(ap, SCR_STATUS);
if ((sstatus & 0xf) != 1)
break;
} while (time_before(jiffies, timeout));
[…]
}
```
Later on Tejun refactored the code in commit d7bb4cc75759 ([PATCH]
libata-hp-prep: implement sata_phy_debounce()) [4], and clarified the
comment.
(Sorry, if I mis-analyzed anything.)
Jeff, Tejun, do you know, what chipsets/PHYs had trouble with being
queried right away? Only ata_piix?
So I think we are stuck with switching chipsets to "no delay" one by
one by testing. Once the majority of drivers are converted, we can
reverse the default to be "no delay" and mark untested drivers as
needing the delay.
For easy testing, a new CLI parameter to skip the delay might be handy.
You mean a sysfs attribute may be ?
I am not sure it would help: on resume, the sysfs attributes would be
recreated and get the default value, not a new one.
No, I mean a module parameter for `libata` like `ata_probe_timeout`.
Then users could test it during boot, and, if there are no issue, tell
us the ID.
Kind regards,
Paul
[1]: https://lore.kernel.org/linux-ide/20211227162658.11314-2-pmenzel@xxxxxxxxxxxxx/T/#m697d2121463a4c946730e6b83940e12d6d7e6700
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/
[3]:
https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?id=4effb658a0f800e159c29a2d881cac76c326087a
[4]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d7bb4cc7575929a60b0a718daa1bce87bea9a9cc
