* [PATCH 0/2][RFC] Make delay before debouncing configurable
@ 2022-01-13 15:46 Paul Menzel
2022-01-13 15:46 ` [PATCH 1/2] ata: Add module parameter `debounce_delay_ms` Paul Menzel
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Paul Menzel @ 2022-01-13 15:46 UTC (permalink / raw)
To: Damien Le Moal; +Cc: linux-ide, Paul Menzel
The 200 ms delay before debouncing the PHY was introduced for some buggy
old controllers. To decrease the boot time to come closer do instant
boot, add a parameter so users can override that delay.
The current implementation has several drawbacks, and is just a proof of
concept, which some experienced Linux kernel developer can probably
implement in a better way.
One problem is, that the warning is shown for each link and not just per
controller.
Paul Menzel (2):
ata: Add module parameter `debounce_delay_ms`
ata: Warn about removal of debounce delay in Linux 5.19
Documentation/admin-guide/kernel-parameters.txt | 6 ++++++
drivers/ata/libata-core.c | 4 ++++
drivers/ata/libata-sata.c | 12 +++++++++---
drivers/ata/libata.h | 1 +
4 files changed, 20 insertions(+), 3 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/2] ata: Add module parameter `debounce_delay_ms` 2022-01-13 15:46 [PATCH 0/2][RFC] Make delay before debouncing configurable Paul Menzel @ 2022-01-13 15:46 ` Paul Menzel 2022-01-13 15:46 ` [PATCH 2/2] ata: Warn about removal of debounce delay in Linux 5.19 Paul Menzel 2022-01-14 9:23 ` [PATCH 0/2][RFC] Make delay before debouncing configurable Damien Le Moal 2 siblings, 0 replies; 9+ messages in thread From: Paul Menzel @ 2022-01-13 15:46 UTC (permalink / raw) To: Damien Le Moal, Jonathan Corbet Cc: linux-ide, Paul Menzel, linux-doc, linux-kernel The 200 ms delay in `sata_resume_lin()` is probably only needed for a few old controllers, so allow users to test this by setting the delay time (preferrably 0). To be able to track defaults, make it a signed integer with negative values being the default, which currently stays at 200 ms. This parameter could also be made boolean, but making the delay configurable adds more flexibility, but also does not directly match the existing boolean flag `ATA_LFLAG_NO_DEBOUNCE_DELAY`. Note, if that flag is set for a board, then the parameter is ignored. History: Commit 4effb658a0 from October 2003 [1, historical git archive] 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 in commit d7bb4cc75759 ([PATCH] libata-hp-prep: implement sata_phy_debounce()) the commit is refactored [2], and the comment clarified. /* * 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_DEBOUNCE_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); A lot of PHYs do not need a delay though, so delaying 200 ms increases the boot time by 30 percent unnecessarily for a lot of systems, making “instant booting” quite hard. [1]: https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?id=4effb658a0f800e159c29a2d881cac76c326087a [2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d7bb4cc7575929a60b0a718daa1bce87bea9a9cc Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de> --- Documentation/admin-guide/kernel-parameters.txt | 6 ++++++ drivers/ata/libata-core.c | 4 ++++ drivers/ata/libata-sata.c | 6 ++++-- drivers/ata/libata.h | 1 + 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 2fba82431efbe..8cc0e790f49d6 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -2548,6 +2548,12 @@ lapic_timer_c2_ok [X86,APIC] trust the local apic timer in C2 power state. + libata.libata_debounce_delay_ms= [LIBATA] Set debounce delay in + ms + + libata.dma<0 Use default value from code + libata.dma>1 Debounce delay in milliseconds + libata.dma= [LIBATA] DMA control libata.dma=0 Disable all PATA and SATA DMA libata.dma=1 PATA and SATA Disk DMA only diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 67f88027680ac..b0d76cb8a7531 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -154,6 +154,10 @@ static int atapi_an; module_param(atapi_an, int, 0444); MODULE_PARM_DESC(atapi_an, "Enable ATAPI AN media presence notification (0=0ff [default], 1=on)"); +int debounce_delay_ms = -1; +module_param_named(debounce_delay_ms, libata_debounce_delay_ms, int, 0644); +MODULE_PARM_DESC(debounce_delay_ms, "Delay amount milliseconds in sata_link_resume() to work around controller issues (negative values mean default value in code (200 ms)"); + MODULE_AUTHOR("Jeff Garzik"); MODULE_DESCRIPTION("Library module for ATA devices"); MODULE_LICENSE("GPL"); diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c index 071158c0c44c1..29a815e2b7248 100644 --- a/drivers/ata/libata-sata.c +++ b/drivers/ata/libata-sata.c @@ -315,10 +315,12 @@ int sata_link_resume(struct ata_link *link, const unsigned long *params, /* * Some PHYs react badly if SStatus is pounded * immediately after resuming. Delay 200ms before - * debouncing. + * debouncing. Duration can be configured with module + * parameter debounce_delay_ms. */ if (!(link->flags & ATA_LFLAG_NO_DEBOUNCE_DELAY)) - ata_msleep(link->ap, 200); + ata_msleep(link->ap, + (libata_debounce_delay_ms < 0) ? 200 : libata_debounce_delay_ms); /* is SControl restored correctly? */ if ((rc = sata_scr_read(link, SCR_CONTROL, &scontrol))) diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h index 51e01acdd2410..a26e77ee25aa2 100644 --- a/drivers/ata/libata.h +++ b/drivers/ata/libata.h @@ -35,6 +35,7 @@ extern int atapi_passthru16; extern int libata_fua; extern int libata_noacpi; extern int libata_allow_tpm; +extern int libata_debounce_delay_ms; extern const struct device_type ata_port_type; extern struct ata_link *ata_dev_phys_link(struct ata_device *dev); #ifdef CONFIG_ATA_FORCE -- 2.30.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] ata: Warn about removal of debounce delay in Linux 5.19 2022-01-13 15:46 [PATCH 0/2][RFC] Make delay before debouncing configurable Paul Menzel 2022-01-13 15:46 ` [PATCH 1/2] ata: Add module parameter `debounce_delay_ms` Paul Menzel @ 2022-01-13 15:46 ` Paul Menzel 2022-01-14 9:23 ` [PATCH 0/2][RFC] Make delay before debouncing configurable Damien Le Moal 2 siblings, 0 replies; 9+ messages in thread From: Paul Menzel @ 2022-01-13 15:46 UTC (permalink / raw) To: Damien Le Moal; +Cc: linux-ide, Paul Menzel, linux-kernel The delay is only needed for a few buggy chipsets (PHYs(?)). As 200 ms is quite a lot in today times, announce the change of the default in Linux 5.19, and call for tests and reports. --- drivers/ata/libata-sata.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c index 29a815e2b7248..026ffcfaeaf26 100644 --- a/drivers/ata/libata-sata.c +++ b/drivers/ata/libata-sata.c @@ -318,9 +318,13 @@ int sata_link_resume(struct ata_link *link, const unsigned long *params, * debouncing. Duration can be configured with module * parameter debounce_delay_ms. */ - if (!(link->flags & ATA_LFLAG_NO_DEBOUNCE_DELAY)) + if (!(link->flags & ATA_LFLAG_NO_DEBOUNCE_DELAY)) { ata_msleep(link->ap, (libata_debounce_delay_ms < 0) ? 200 : libata_debounce_delay_ms); + if (libata_debounce_delay_ms < 0) + /* negative values are default */ + ata_link_warn(link, "Due to historical reasons a 200 ms delay is applied in sata_link_resume(). Most controllers do not need that, so the default will change to 0 ms in Linux 5.19. Please test with lower values using libata.debounce_delay_ms and report the results <linux-ide@vger.kernel.org>.\n"); + } /* is SControl restored correctly? */ if ((rc = sata_scr_read(link, SCR_CONTROL, &scontrol))) -- 2.30.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2][RFC] Make delay before debouncing configurable 2022-01-13 15:46 [PATCH 0/2][RFC] Make delay before debouncing configurable Paul Menzel 2022-01-13 15:46 ` [PATCH 1/2] ata: Add module parameter `debounce_delay_ms` Paul Menzel 2022-01-13 15:46 ` [PATCH 2/2] ata: Warn about removal of debounce delay in Linux 5.19 Paul Menzel @ 2022-01-14 9:23 ` Damien Le Moal 2022-01-19 17:57 ` Robin H. Johnson 2 siblings, 1 reply; 9+ messages in thread From: Damien Le Moal @ 2022-01-14 9:23 UTC (permalink / raw) To: Paul Menzel; +Cc: linux-ide On 1/14/22 00:46, Paul Menzel wrote: > The 200 ms delay before debouncing the PHY was introduced for some buggy > old controllers. To decrease the boot time to come closer do instant > boot, add a parameter so users can override that delay. > > The current implementation has several drawbacks, and is just a proof of > concept, which some experienced Linux kernel developer can probably > implement in a better way. I do not think that a libata module parameter is not the way to go with this: libata is used by all drivers, so for a system that has multiple adapters, different delays cannot be specified easily. I am really thinking that the way to go about this is to remove the 200ms delay by default and add it only for drivers that request it with a link flag. That is, ATA_LFLAG_NO_DEBOUNCE_DELAY needs to become ATA_LFLAG_DEBOUNCE_DELAY. The other large delay is the link stability check in sata_link_debounce(). 100ms is added (more for hotplug case) to ensure that the SStatus register DET field provides a stable value. But I cannot find any text in the AHCI and SATA IO specs that mandate such large delay. I tried to address all of the above. Please have a look at the top 4 patches in the sata-timing branch of the libata tree: git@gitolite.kernel.org:pub/scm/linux/kernel/git/dlemoal/libata The sata-timing branch is for now based on libata for-5.17 branch. The 200ms delay in sata_link_resume() is gone by default, replaced with a 1ms delay (totally arbitrary). The 200ms delay is executed only if a driver has the ATA_LFLAG_DEBOUNCE_DELAY link flag set. The next part is sata_link_debounce(): I *think* that we can assume that a link is stable if we see cur_det == last_det == 3. In this case, bailing out early seems to be fine, at least on my test box (Intel dual-socket Xeon server with Intel AHCI chipset). But I only tested boot/reboot. Hotplug/unplug and suspend/resume need to be tested, but I need to go to the lab for that (working from home). Will try next week. Could you give this branch a try and check how that improves device scan times ? On my test box, which has *a lot* of drives, I see something like this: Before: [ 16.696140] ata4: SATA max UDMA/133 abar m524288@0x9d200000 port 0x9d200180 irq 341 [ 17.527446] ata4: SATA link up 6.0 Gbps (SStatus 133 SControl 300) ... -> 831 ms to get the link ready After: [ 15.957946] ata4: SATA max UDMA/133 abar m524288@0x9d200000 port 0x9d200180 irq 341 [ 16.245066] ata4: SATA link up 6.0 Gbps (SStatus 133 SControl 300) ... -> 287 ms to get the link ready There are differences between the many HDDs & SSDs I have connected though. There is a lot of scheduling side effects at play, so the gains are variable in my case. A system with a single disk attached should be used for proper evaluation. Going forward, if more testing do not show any problem, I am thinking of pushing these changes to for-next to get things tested more widely and see who screams that they lost their drives :) For now, I added the ATA_LFLAG_DEBOUNCE_DELAY to the ata_piix driver only. Likely, this flag will be needed for most legacy/old adapters (which I do not have). Cheers. -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2][RFC] Make delay before debouncing configurable 2022-01-14 9:23 ` [PATCH 0/2][RFC] Make delay before debouncing configurable Damien Le Moal @ 2022-01-19 17:57 ` Robin H. Johnson 2022-01-20 0:14 ` Damien Le Moal 0 siblings, 1 reply; 9+ messages in thread From: Robin H. Johnson @ 2022-01-19 17:57 UTC (permalink / raw) To: Damien Le Moal, linux-ide; +Cc: Paul Menzel Hi, not originally in the thread, but I've run into hardware where the delay was bumpy before, when I did early porting around SATA PMP code (https://dev.gentoo.org/~robbat2/patches/libata-development/ if you want to see really old patches from 2006) This series esp of a code approach that didn't get merged might be interesting, that implements hotplug by polling: https://dev.gentoo.org/~robbat2/patches/libata-development/2007/00-hp-poll/ On Fri, Jan 14, 2022 at 06:23:26PM +0900, Damien Le Moal wrote: > On 1/14/22 00:46, Paul Menzel wrote: > > The 200 ms delay before debouncing the PHY was introduced for some buggy > > old controllers. To decrease the boot time to come closer do instant > > boot, add a parameter so users can override that delay. > > > > The current implementation has several drawbacks, and is just a proof of > > concept, which some experienced Linux kernel developer can probably > > implement in a better way. > I do not think that a libata module parameter is not the way to go with > this: libata is used by all drivers, so for a system that has multiple > adapters, different delays cannot be specified easily. I think this is a key thing here; and I like that your patch moves to a flag. > I am really thinking that the way to go about this is to remove the > 200ms delay by default and add it only for drivers that request it with > a link flag. That is, ATA_LFLAG_NO_DEBOUNCE_DELAY needs to become > ATA_LFLAG_DEBOUNCE_DELAY. I agree that removing it by default is right, but I'd like to make one additional request here: Please add a libata.force= flag that lets users enable/disable the delay per adapter/link. I think this would be valuable to rule out issues where the debounce delay is needed on the drive side more than the controller side, esp. in cases of poorly implemented port multipliers as Tejun & I found back in 2006. Maybe libata.force=X.Y:no_debounce_delay & libata.force=X.Y:force_debounce_delay The ata_parse_force_one function as it stands can't handle a parameter to the value, so you cannot get libata.force=X.Y:debounce_delay=N without also improving ata_parse_force_one. > The other large delay is the link stability check in > sata_link_debounce(). 100ms is added (more for hotplug case) to ensure > that the SStatus register DET field provides a stable value. But I > cannot find any text in the AHCI and SATA IO specs that mandate such > large delay. Nice find! > There are differences between the many HDDs & SSDs I have connected > though. There is a lot of scheduling side effects at play, so the gains > are variable in my case. A system with a single disk attached should be > used for proper evaluation. That gets likely single-disk worst/best case, but I'm still worried about port multipliers (sadly I don't have the worst-implemented ones anymore, I sold them to some Windows users) -- Robin Hugh Johnson Gentoo Linux: Dev, Infra Lead, Foundation Treasurer E-Mail : robbat2@gentoo.org GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85 GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2][RFC] Make delay before debouncing configurable 2022-01-19 17:57 ` Robin H. Johnson @ 2022-01-20 0:14 ` Damien Le Moal 2022-02-14 7:09 ` Paul Menzel 0 siblings, 1 reply; 9+ messages in thread From: Damien Le Moal @ 2022-01-20 0:14 UTC (permalink / raw) To: Robin H. Johnson, linux-ide; +Cc: Paul Menzel On 2022/01/20 2:57, Robin H. Johnson wrote: > Hi, not originally in the thread, but I've run into hardware where the > delay was bumpy before, when I did early porting around SATA PMP code > (https://dev.gentoo.org/~robbat2/patches/libata-development/ if you want > to see really old patches from 2006) > > This series esp of a code approach that didn't get merged might be > interesting, that implements hotplug by polling: > https://dev.gentoo.org/~robbat2/patches/libata-development/2007/00-hp-poll/ > > On Fri, Jan 14, 2022 at 06:23:26PM +0900, Damien Le Moal wrote: >> On 1/14/22 00:46, Paul Menzel wrote: >>> The 200 ms delay before debouncing the PHY was introduced for some buggy >>> old controllers. To decrease the boot time to come closer do instant >>> boot, add a parameter so users can override that delay. >>> >>> The current implementation has several drawbacks, and is just a proof of >>> concept, which some experienced Linux kernel developer can probably >>> implement in a better way. >> I do not think that a libata module parameter is not the way to go with >> this: libata is used by all drivers, so for a system that has multiple >> adapters, different delays cannot be specified easily. > I think this is a key thing here; and I like that your patch moves to a > flag. > >> I am really thinking that the way to go about this is to remove the >> 200ms delay by default and add it only for drivers that request it with >> a link flag. That is, ATA_LFLAG_NO_DEBOUNCE_DELAY needs to become >> ATA_LFLAG_DEBOUNCE_DELAY. > I agree that removing it by default is right, but I'd like to make one > additional request here: > Please add a libata.force= flag that lets users enable/disable the delay > per adapter/link. > > I think this would be valuable to rule out issues where the debounce > delay is needed on the drive side more than the controller side, esp. in > cases of poorly implemented port multipliers as Tejun & I found back in > 2006. > > Maybe libata.force=X.Y:no_debounce_delay & libata.force=X.Y:force_debounce_delay > > The ata_parse_force_one function as it stands can't handle a parameter > to the value, so you cannot get libata.force=X.Y:debounce_delay=N > without also improving ata_parse_force_one. Good point. I will look into adding this. > >> The other large delay is the link stability check in >> sata_link_debounce(). 100ms is added (more for hotplug case) to ensure >> that the SStatus register DET field provides a stable value. But I >> cannot find any text in the AHCI and SATA IO specs that mandate such >> large delay. > Nice find! > >> There are differences between the many HDDs & SSDs I have connected >> though. There is a lot of scheduling side effects at play, so the gains >> are variable in my case. A system with a single disk attached should be >> used for proper evaluation. > That gets likely single-disk worst/best case, but I'm still worried > about port multipliers (sadly I don't have the worst-implemented ones > anymore, I sold them to some Windows users) :) I have a e-sata port-multiplier box in the lab. But I need to hook it up to my test box, which means that I have to get out of home for once and go to the office :) Will do that. Port-multiplier tests are also needed to complete Hannes series renaming sysfs fields to match the debug messages. -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2][RFC] Make delay before debouncing configurable 2022-01-20 0:14 ` Damien Le Moal @ 2022-02-14 7:09 ` Paul Menzel 2022-02-14 17:50 ` Robin H. Johnson [not found] ` <7187af82-3d35-0094-f998-7d20bfc5192f@molgen.mpg.de> 0 siblings, 2 replies; 9+ messages in thread From: Paul Menzel @ 2022-02-14 7:09 UTC (permalink / raw) To: Damien Le Moal, Robin H. Johnson; +Cc: linux-ide Dear Damien, dear Robin, Am 20.01.22 um 01:14 schrieb Damien Le Moal: > On 2022/01/20 2:57, Robin H. Johnson wrote: >> Hi, not originally in the thread, but I've run into hardware where the >> delay was bumpy before, when I did early porting around SATA PMP code >> (https://dev.gentoo.org/~robbat2/patches/libata-development/ if you want >> to see really old patches from 2006) >> >> This series esp of a code approach that didn't get merged might be >> interesting, that implements hotplug by polling: >> https://dev.gentoo.org/~robbat2/patches/libata-development/2007/00-hp-poll/ Polling and a warning, when polling time exceeds like 10 ms, so users can contact the hardware vendor, would indeed be the most flexible solution. Robin, do you remember, why these patches were not applied? Just lack of time and review, or where there issues? >> On Fri, Jan 14, 2022 at 06:23:26PM +0900, Damien Le Moal wrote: >>> On 1/14/22 00:46, Paul Menzel wrote: >>>> The 200 ms delay before debouncing the PHY was introduced for some buggy >>>> old controllers. To decrease the boot time to come closer do instant >>>> boot, add a parameter so users can override that delay. >>>> >>>> The current implementation has several drawbacks, and is just a proof of >>>> concept, which some experienced Linux kernel developer can probably >>>> implement in a better way. >>> I do not think that a libata module parameter is not the way to go with >>> this: libata is used by all drivers, so for a system that has multiple >>> adapters, different delays cannot be specified easily. >> I think this is a key thing here; and I like that your patch moves to a >> flag. Indeed, I did not think of that. >>> I am really thinking that the way to go about this is to remove the >>> 200ms delay by default and add it only for drivers that request it with >>> a link flag. That is, ATA_LFLAG_NO_DEBOUNCE_DELAY needs to become >>> ATA_LFLAG_DEBOUNCE_DELAY. >> I agree that removing it by default is right, but I'd like to make one >> additional request here: >> Please add a libata.force= flag that lets users enable/disable the delay >> per adapter/link. >> >> I think this would be valuable to rule out issues where the debounce >> delay is needed on the drive side more than the controller side, esp. in >> cases of poorly implemented port multipliers as Tejun & I found back in >> 2006. >> >> Maybe libata.force=X.Y:no_debounce_delay & libata.force=X.Y:force_debounce_delay >> >> The ata_parse_force_one function as it stands can't handle a parameter >> to the value, so you cannot get libata.force=X.Y:debounce_delay=N >> without also improving ata_parse_force_one. > > Good point. I will look into adding this. Awesome. >>> The other large delay is the link stability check in >>> sata_link_debounce(). 100ms is added (more for hotplug case) to ensure >>> that the SStatus register DET field provides a stable value. But I >>> cannot find any text in the AHCI and SATA IO specs that mandate such >>> large delay. >> Nice find! Adding back Damien’s answer text: >> I tried to address all of the above. Please have a look at the top 4 >> patches in the sata-timing branch of the libata tree: >> >> git@gitolite.kernel.org:pub/scm/linux/kernel/git/dlemoal/libata >> >> The sata-timing branch is for now based on libata for-5.17 branch. Thank you for cooking this up. I tested this on the ASUS F2A85-M PRO (AMD, 1022:0x7801), MSI B350M MORTAR (AMD, 1022:0x7901), and IBM S822LC (Marvell, 1b4b:9235) with no issues and the expected decrease in boot time. >>> There are differences between the many HDDs & SSDs I have connected >>> though. There is a lot of scheduling side effects at play, so the gains >>> are variable in my case. A system with a single disk attached should be >>> used for proper evaluation. >> That gets likely single-disk worst/best case, but I'm still worried >> about port multipliers (sadly I don't have the worst-implemented ones >> anymore, I sold them to some Windows users) > > :) > > I have a e-sata port-multiplier box in the lab. But I need to hook it up to my > test box, which means that I have to get out of home for once and go to the > office :) Will do that. Port-multiplier tests are also needed to complete Hannes > series renaming sysfs fields to match the debug messages. Kind regards, Paul ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2][RFC] Make delay before debouncing configurable 2022-02-14 7:09 ` Paul Menzel @ 2022-02-14 17:50 ` Robin H. Johnson [not found] ` <7187af82-3d35-0094-f998-7d20bfc5192f@molgen.mpg.de> 1 sibling, 0 replies; 9+ messages in thread From: Robin H. Johnson @ 2022-02-14 17:50 UTC (permalink / raw) To: Paul Menzel; +Cc: Damien Le Moal, Robin H. Johnson, linux-ide [-- Attachment #1: Type: text/plain, Size: 1002 bytes --] On Mon, Feb 14, 2022 at 08:09:53AM +0100, Paul Menzel wrote: > >> This series esp of a code approach that didn't get merged might be > >> interesting, that implements hotplug by polling: > >> https://dev.gentoo.org/~robbat2/patches/libata-development/2007/00-hp-poll/ > Polling and a warning, when polling time exceeds like 10 ms, so users > can contact the hardware vendor, would indeed be the most flexible solution. > > Robin, do you remember, why these patches were not applied? Just lack of > time and review, or where there issues? Disagreement on the per-link configurability via sysfs, and then lack of time to come to an agreed solution there. The 2007 linux-ide list has some of that discussion, but there was also some on IRC that is lost to time. -- Robin Hugh Johnson Gentoo Linux: Dev, Infra Lead, Foundation Treasurer E-Mail : robbat2@gentoo.org GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85 GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 1113 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <7187af82-3d35-0094-f998-7d20bfc5192f@molgen.mpg.de>]
* Re: [PATCH 0/2][RFC] Make delay before debouncing configurable [not found] ` <7187af82-3d35-0094-f998-7d20bfc5192f@molgen.mpg.de> @ 2022-02-25 1:15 ` Damien Le Moal 0 siblings, 0 replies; 9+ messages in thread From: Damien Le Moal @ 2022-02-25 1:15 UTC (permalink / raw) To: Paul Menzel; +Cc: linux-ide, Robin H. Johnson On 2/24/22 23:34, Paul Menzel wrote: [...] >> Thank you for cooking this up. I tested this on the ASUS F2A85-M PRO >> (AMD, 1022:0x7801), MSI B350M MORTAR (AMD, 1022:0x7901), and IBM S822LC >> (Marvell, 1b4b:9235) with no issues and the expected decrease in boot time. > > There is still one issue I noticed. The MSI B350M MORTAR has nine(?) > SATA ports, and only one is connected to an SSD. For some of the other > unpopulated ports a delay of 100 ms happens (although in Linux kernel > threads, but still in serial and not parallel). Unfortunately, that > system uses LUKS encryption, and as things are happening in initrd, I do > not know if the delays would hold up the overall boot. I need to do more > tests. > > ``` > $ grep sata_link_hardreset 20220220-sata-hardreset.txt > 0.706289 | 0) scsi_eh-70 | | > sata_link_hardreset() { > 0.718497 | 0) scsi_eh-92 | | > sata_link_hardreset() { > 0.728425 | 0) scsi_eh-92 | # 9927.978 us | } /* > sata_link_hardreset */ > 0.811159 | 2) scsi_eh-70 | @ 104870.3 us | } /* > sata_link_hardreset */ > 0.811329 | 2) scsi_eh-72 | | > sata_link_hardreset() { > 0.920672 | 3) scsi_eh-72 | @ 109343.5 us | } /* > sata_link_hardreset */ > 0.920915 | 2) scsi_eh-78 | | > sata_link_hardreset() { > 1.024618 | 2) scsi_eh-78 | @ 103703.7 us | } /* > sata_link_hardreset */ > 1.025027 | 0) scsi_eh-80 | | > sata_link_hardreset() { > 1.128589 | 0) scsi_eh-80 | @ 103561.6 us | } /* > sata_link_hardreset */ > ``` This looks like the delay for the link stability check in sata_link_debounce(). 100ms is added (more for hotplug case) to ensure that the SStatus register DET field provides a stable value. As mentioned in another email on this subject, I cannot find any text in the AHCI and SATA IO specs that mandate such large delay. Still need to dig the history of this delay and why it was added. -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-02-25 1:16 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-13 15:46 [PATCH 0/2][RFC] Make delay before debouncing configurable Paul Menzel
2022-01-13 15:46 ` [PATCH 1/2] ata: Add module parameter `debounce_delay_ms` Paul Menzel
2022-01-13 15:46 ` [PATCH 2/2] ata: Warn about removal of debounce delay in Linux 5.19 Paul Menzel
2022-01-14 9:23 ` [PATCH 0/2][RFC] Make delay before debouncing configurable Damien Le Moal
2022-01-19 17:57 ` Robin H. Johnson
2022-01-20 0:14 ` Damien Le Moal
2022-02-14 7:09 ` Paul Menzel
2022-02-14 17:50 ` Robin H. Johnson
[not found] ` <7187af82-3d35-0094-f998-7d20bfc5192f@molgen.mpg.de>
2022-02-25 1:15 ` Damien Le Moal
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox