Dear Jiaxun,

Thank you for v2.

Am 28.05.25 um 13:02 schrieb Jiaxun Yang:
After adding AHCI controller reset functionality there are multiple
reports on AHCI booting regression.

As per my experiments on various machines, to reset controller
properly it is necessary to poll HOST_CTL_RESET bit until it's
clear. It is also required to read back HOST_CTL after changing
HOST_CTL_AHCI_EN bits to ensure the controller has accepted write.

Tested on ASMedia ASM1061, Intel H61 native SATA and AMD Phoenix
native SATA.

Link: 
https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/RDNRKWBN4N5XQX2TQMM5P4WZ2OOPPNAM/
Link: https://github.com/FlyGoat/csmwrap/issues/14
Fixes: 8863cbbd15a7 ("ahci: add controller reset")
Signed-off-by: Jiaxun Yang <jiaxun.y...@flygoat.com>
Acked-by: Paul Menzel <pmen...@molgen.mpg.de>
---
To: seabios@seabios.org
---
Changes in v2:
- Commit message (Paul Menzel)
- Link to v1: 
https://lore.kernel.org/r/20250522-ahci-v1-1-a9de19585...@flygoat.com
---
  src/hw/ahci.c | 19 +++++++++++++++++--
  1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/hw/ahci.c b/src/hw/ahci.c
index 
2285d33d4ae21335e5222ab04608685b5bff3763..7e7a03ddd010d53fa439c8ca49505a2da00241f4
 100644
--- a/src/hw/ahci.c
+++ b/src/hw/ahci.c
@@ -660,8 +660,23 @@ ahci_controller_setup(struct pci_device *pci)
pci_enable_busmaster(pci); - ahci_ctrl_writel(ctrl, HOST_CTL, HOST_CTL_RESET);
-    ahci_ctrl_writel(ctrl, HOST_CTL, HOST_CTL_AHCI_EN);
+    u32 val = ahci_ctrl_readl(ctrl, HOST_CTL);
+    ahci_ctrl_writel(ctrl, HOST_CTL, val | HOST_CTL_RESET);
+    u32 end = timer_calc(AHCI_RESET_TIMEOUT);
+    for (;;) {
+        val = ahci_ctrl_readl(ctrl, HOST_CTL);
+        if (!(val & HOST_CTL_RESET))
+            break;
+        if (timer_check(end)) {
+            warn_timeout();
+            dprintf(1, "AHCI: controller reset failed\n");

Although not common in the current code base, it might be useful (for the user) to log `AHCI_RESET_TIMEOUT`, and maybe even `val`. But it could be added in a follow-up, should too many reports with this reset problem come in.

+            free(ctrl);
+            return;
+        }
+        yield();
+    }
+    ahci_ctrl_writel(ctrl, HOST_CTL, val | HOST_CTL_AHCI_EN);
+    (void)ahci_ctrl_readl(ctrl, HOST_CTL); /* Flush */
ctrl->caps = ahci_ctrl_readl(ctrl, HOST_CAP);
      ctrl->ports = ahci_ctrl_readl(ctrl, HOST_PORTS_IMPL);
Reviewed-by: Paul Menzel <pmen...@molgen.mpg.de>


Kind regards,

Paul
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org

Reply via email to