On 3/28/22 16:41, Paul Menzel wrote: > Dear Damien, > > > Am 28.03.22 um 09:19 schrieb Damien Le Moal: >> On 3/25/22 18:10, Paul Menzel wrote: > >>> Am 23.03.22 um 09:17 schrieb Damien Le Moal: >>>> sata_link_debounce() polls the SStatus register DET field to ensure that >>>> a stable value is provided, to reliably detect device presence and PHY >>>> readiness. Polling is done for at least timing->duration if there is no >>>> device detected. For the device detected case, polling last up to >>> >>> last*s* >>> >>>> deadline to ensure that the PHY becomes ready. >>>> >>>> However, the PHY ready state is actually never checked, leading to the >>>> poll loop duration always reaching the maximum duration. >>>> >>>> For adapters that do not require a debounce delay (link flag >>>> ATA_LFLAG_DEBOUNCE_DELAY no set), add a check to test if DET indicates >>> >>> no*t*? >>> >>>> device present *and* PHY ready and bail out of the polling loop if it >>>> does. >>>> >>>> While at it, add comments to clarify the various checks in >>>> sata_link_debounce() polling loop. >>> >>> Were you able to verify that the check now terminates the loop on your >>> devices earlier? >> >> Yes it does for me. The function goes from taking about 100ms to taking >> only one iteration of the loop, which has only one 5ms sleep. >> >>> >>> Are below the right lines to compare? On the Dell OptiPlex 5055 (AMD FCH >>> SATA Controller [AHCI mode] [1022:7901]) I see: >>> >>> Before (this very patch) >>> >>> [ 0.428015] ata1: hard resetting link >>> [ 0.696316] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300) >>> >>> After: >>> >>> [ 0.428907] ata1: hard resetting link >>> [ 0.648092] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300) >>> >>> So a decrease of (268 - 219 = 49) ms. Nice. >> >> For a regular boot, sata_deb_timing_normal duration is used. So the save >> is 100ms almost (there is still one iteration of the loop needed, which >> takes interval = 5ms). >> >> Together with the long 200ms sleep in sata_link_resume, the gain overall >> should be about 300ms. > > To my knowledge this was a regular boot (`systemctl reboot`). So where > did the 50 ms go? ;-) The device also sometimes need time to reach DET == 3 state... This is an optimization that reduces the time waited to reach DET == 3. But we still wait for it :) On a hot reboot, as the device are already ready, I do see the time reduced to one interval (5ms). But for a cold boot, it takes longer sometimes. -- Damien Le Moal Western Digital Research