From 8665ee92f90010f016002efe9cd8cc64d4306e08 Mon Sep 17 00:00:00 2001 From: Yuuki Takano Date: Sun, 19 Apr 2026 08:19:31 +0900 Subject: [PATCH 1/4] fix(igc): stabilize i225 phy bring-up Align the Awkernel I225 bring-up path more closely with the BSD igc drivers and improve diagnostics around the PHY reset path. This change makes the MAC reset path tolerant of auto-read completion timeouts, initializes and synchronizes the MDIC PHY address, powers the PHY up before reading its ID, and clears GO_LINKD/ULP before attempting the hardware reset. The PHY reset path now retries once, logs the relevant control/status registers, and treats a missing RST_COMPL bit as non-fatal when direct MDIC reads show that the PHY is already responding. On the driver side, tick-based link polling was added so link state can advance even when only the periodic path runs, and link transitions now log speed/duplex changes. In hardware testing over /dev/ttyUSB0, the previous hard timeout message was replaced by a benign "PHY reset completion bit did not assert, but PHY responded" diagnostic, and the port reached "Up (Full Duplex)" during boot. --- awkernel_drivers/src/pcie/intel/igc.rs | 24 +++++- awkernel_drivers/src/pcie/intel/igc/i225.rs | 16 +++- .../src/pcie/intel/igc/igc_phy.rs | 74 +++++++++++++++---- 3 files changed, 95 insertions(+), 19 deletions(-) diff --git a/awkernel_drivers/src/pcie/intel/igc.rs b/awkernel_drivers/src/pcie/intel/igc.rs index 4494c47eb..e6db7b625 100644 --- a/awkernel_drivers/src/pcie/intel/igc.rs +++ b/awkernel_drivers/src/pcie/intel/igc.rs @@ -251,11 +251,12 @@ impl Igc { Ok(igc) } - fn intr(&self, _irq: Option) -> Result<(), IgcDriverErr> { + fn intr(&self, irq: Option) -> Result<(), IgcDriverErr> { // TODO: Handle Tx/Rx interrupts. let mut inner = self.inner.read(); let igc_icr = read_reg(&inner.info, igc_regs::IGC_ICR)?; + let should_poll_link = irq.is_none() && (igc_icr & igc_defines::IGC_ICR_LSC) == 0; if (igc_icr & igc_defines::IGC_ICR_LSC) != 0 { // Link status change interrupt. @@ -265,6 +266,13 @@ impl Igc { inner.igc_intr_link()?; } inner = self.inner.read(); + } else if should_poll_link { + drop(inner); + { + let mut inner = self.inner.write(); + inner.igc_poll_link()?; + } + inner = self.inner.read(); } write_reg(&inner.info, igc_regs::IGC_IMS, igc_defines::IGC_IMS_LSC)?; @@ -802,6 +810,12 @@ impl IgcInner { ) } + #[inline(always)] + fn igc_poll_link(&mut self) -> Result<(), IgcDriverErr> { + self.hw.mac.get_link_status = true; + self.igc_intr_link() + } + fn igc_iff(&mut self) -> Result<(), IgcDriverErr> { use igc_regs::*; @@ -1154,6 +1168,8 @@ fn igc_update_link_status( hw: &mut IgcHw, link_info: &mut LinkInfo, ) -> Result<(), IgcDriverErr> { + let previous_status = link_info.link_status; + if hw.mac.get_link_status { ops.check_for_link(info, hw)?; } @@ -1164,6 +1180,7 @@ fn igc_update_link_status( link_info.link_speed = Some(speed); link_info.link_duplex = Some(duplex); link_info.link_active = true; + log::debug!("igc: link up: speed={speed:?}, duplex={duplex:?}"); } if link_info.link_duplex == Some(IgcDuplex::Full) { @@ -1176,10 +1193,15 @@ fn igc_update_link_status( link_info.link_speed = None; link_info.link_duplex = None; link_info.link_active = false; + log::debug!("igc: link down"); } LinkStatus::Down }; + if previous_status != link_info.link_status { + log::info!("igc: link status changed: {}", link_info.link_status); + } + Ok(()) } diff --git a/awkernel_drivers/src/pcie/intel/igc/i225.rs b/awkernel_drivers/src/pcie/intel/igc/i225.rs index b8ae6d63d..bd9d142b4 100644 --- a/awkernel_drivers/src/pcie/intel/igc/i225.rs +++ b/awkernel_drivers/src/pcie/intel/igc/i225.rs @@ -30,7 +30,7 @@ use super::{ igc_nvm::{acquire_nvm, igc_read_nvm_eerd, igc_validate_nvm_checksum_generic}, igc_phy::{ igc_get_phy_id, igc_phy_hw_reset_generic, igc_power_up_phy_copper, igc_read_phy_reg_gpy, - igc_write_phy_reg_gpy, + igc_sync_mdic_phy_addr, igc_write_phy_reg_gpy, }, igc_regs::*, read_reg, write_flush, write_reg, IgcDriverErr, @@ -341,7 +341,11 @@ fn igc_reset_hw_i225( let ctrl = read_reg(info, IGC_CTRL)?; write_reg(info, IGC_CTRL, ctrl | IGC_CTRL_DEV_RST)?; - igc_get_auto_rd_done_generic(info)?; + if let Err(e) = igc_get_auto_rd_done_generic(info) { + // Matching the BSD drivers here avoids failing bring-up on parts + // without usable NVM auto-read completion while still surfacing it. + log::debug!("igc: auto read done did not complete after MAC reset: {e:?}"); + } // Clear any pending interrupt events. write_reg(info, IGC_IMC, 0xffffffff)?; @@ -544,6 +548,13 @@ fn igc_init_phy_params_i225( phy.autoneg_mask = AUTONEG_ADVERTISE_SPEED_DEFAULT_2500; phy.reset_delay_us = 100; + let phy_addr = igc_sync_mdic_phy_addr(info, hw, 1)?; + + let mut phpm = read_reg(info, IGC_I225_PHPM)?; + phpm &= !(IGC_I225_PHPM_GO_LINKD | 0x0400); + write_reg(info, IGC_I225_PHPM, phpm)?; + + ops.power_up(info, hw)?; // Make sure the PHY is in a good state. Several people have reported // firmware leaving the PHY's page select register set to something @@ -553,6 +564,7 @@ fn igc_init_phy_params_i225( igc_get_phy_id(ops, info, hw)?; hw.phy.phy_type = IgcPhyType::I225; + log::debug!("igc: initialized PHY address {phy_addr}"); Ok(()) } diff --git a/awkernel_drivers/src/pcie/intel/igc/igc_phy.rs b/awkernel_drivers/src/pcie/intel/igc/igc_phy.rs index cea8e9616..472f88e8a 100644 --- a/awkernel_drivers/src/pcie/intel/igc/igc_phy.rs +++ b/awkernel_drivers/src/pcie/intel/igc/igc_phy.rs @@ -22,6 +22,8 @@ const IGP01IGC_PHY_PAGE_SELECT: u32 = 0x1F; // Page Select const BM_PHY_PAGE_SELECT: u32 = 22; // Page Select for BM const IGP_PAGE_SHIFT: u32 = 5; const PHY_REG_MASK: u32 = 0x1F; +const IGC_MDICNFG_PHY_MASK: u32 = 0x03E00000; +const IGC_MDICNFG_PHY_SHIFT: u32 = 21; pub(super) const IGC_I225_PHPM: usize = 0x0E14; // I225 PHY Power Management const IGC_I225_PHPM_DIS_1000_D3: u32 = 0x0008; // Disable 1G in D3 const IGC_I225_PHPM_LINK_ENERGY: u32 = 0x0010; // Link Energy Detect @@ -34,6 +36,22 @@ const IGC_I225_PHPM_ULP: u32 = 0x0400; // Ultra Low-Power Mode const IGC_I225_PHPM_DIS_2500: u32 = 0x0800; // Disable 2.5G globally const IGC_I225_PHPM_DIS_2500_D3: u32 = 0x1000; // Disable 2.5G in D3 +pub(super) fn igc_sync_mdic_phy_addr( + info: &mut PCIeInfo, + hw: &mut IgcHw, + default_addr: u32, +) -> Result { + let mdicnfg = read_reg(info, IGC_MDICNFG)?; + let phy_addr = (mdicnfg & IGC_MDICNFG_PHY_MASK) >> IGC_MDICNFG_PHY_SHIFT; + let phy_addr = if phy_addr == 0 { default_addr } else { phy_addr }; + + let mdicnfg = (mdicnfg & !IGC_MDICNFG_PHY_MASK) | (phy_addr << IGC_MDICNFG_PHY_SHIFT); + write_reg(info, IGC_MDICNFG, mdicnfg)?; + hw.phy.addr = phy_addr; + + Ok(phy_addr) +} + /// Reads the MDI control register in the PHY at offset and stores the /// information read to data. fn igc_read_phy_reg_mdic( @@ -250,7 +268,7 @@ fn acquire_phy( f: F, ) -> Result where - F: Fn(&dyn IgcPhyOperations, &mut PCIeInfo, &IgcHw) -> Result, + F: Fn(&dyn IgcPhyOperations, &mut PCIeInfo, &mut IgcHw) -> Result, { IgcPhyOperations::acquire(ops, info, hw)?; let result = f(ops, info, hw); @@ -277,29 +295,53 @@ pub(super) fn igc_phy_hw_reset_generic( _ => (), } - acquire_phy(ops, info, hw, |_, info, hw| { - let _phpm = read_reg(info, IGC_I225_PHPM)?; - + acquire_phy(ops, info, hw, |_ops, info, hw| { let ctrl = read_reg(info, IGC_CTRL)?; - write_reg(info, IGC_CTRL, ctrl | IGC_CTRL_PHY_RST)?; - write_flush(info)?; + let mut phpm = read_reg(info, IGC_I225_PHPM)?; - wait_microsec(hw.phy.reset_delay_us as u64); + for attempt in 0..2 { + // Firmware can leave the PHY in go-link-down or ULP state, which + // prevents the reset-complete bit from asserting reliably. + phpm &= !(IGC_I225_PHPM_GO_LINKD | IGC_I225_PHPM_ULP); + write_reg(info, IGC_I225_PHPM, phpm)?; + write_flush(info)?; - write_reg(info, IGC_CTRL, ctrl)?; - write_flush(info)?; + write_reg(info, IGC_CTRL, ctrl | IGC_CTRL_PHY_RST)?; + write_flush(info)?; - wait_microsec(150); + wait_microsec(hw.phy.reset_delay_us as u64); - for _ in 0..10000 { - let phpm = read_reg(info, IGC_I225_PHPM)?; - wait_microsec(1); - if phpm & IGC_I225_PHPM_RST_COMPL != 0 { - return Ok(()); + write_reg(info, IGC_CTRL, ctrl)?; + write_flush(info)?; + + wait_microsec(150); + + for _ in 0..100000 { + phpm = read_reg(info, IGC_I225_PHPM)?; + wait_microsec(1); + if phpm & IGC_I225_PHPM_RST_COMPL != 0 { + return Ok(()); + } + } + + if attempt == 0 { + wait_millisec(1); } } - log::debug!("Timeout expired after a phy reset"); + if igc_read_phy_reg_mdic(info, hw, PHY_ID1).is_ok() + && igc_read_phy_reg_mdic(info, hw, PHY_ID2).is_ok() + { + log::debug!( + "PHY reset completion bit did not assert, but PHY responded: ctrl={ctrl:#010x}, phpm={phpm:#010x}" + ); + return Ok(()); + } + + let status = read_reg(info, IGC_STATUS).unwrap_or(0); + log::debug!( + "Timeout expired after a phy reset: ctrl={ctrl:#010x}, status={status:#010x}, phpm={phpm:#010x}" + ); Ok(()) }) From 99b9429b7c2c13d98279242cd21c9e48fba9a260 Mon Sep 17 00:00:00 2001 From: Yuuki Takano Date: Mon, 20 Apr 2026 11:29:52 +0900 Subject: [PATCH 2/4] make fmt Signed-off-by: Yuuki Takano --- awkernel_drivers/src/pcie/intel/igc/igc_phy.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/awkernel_drivers/src/pcie/intel/igc/igc_phy.rs b/awkernel_drivers/src/pcie/intel/igc/igc_phy.rs index 472f88e8a..bee18b2b8 100644 --- a/awkernel_drivers/src/pcie/intel/igc/igc_phy.rs +++ b/awkernel_drivers/src/pcie/intel/igc/igc_phy.rs @@ -43,7 +43,11 @@ pub(super) fn igc_sync_mdic_phy_addr( ) -> Result { let mdicnfg = read_reg(info, IGC_MDICNFG)?; let phy_addr = (mdicnfg & IGC_MDICNFG_PHY_MASK) >> IGC_MDICNFG_PHY_SHIFT; - let phy_addr = if phy_addr == 0 { default_addr } else { phy_addr }; + let phy_addr = if phy_addr == 0 { + default_addr + } else { + phy_addr + }; let mdicnfg = (mdicnfg & !IGC_MDICNFG_PHY_MASK) | (phy_addr << IGC_MDICNFG_PHY_SHIFT); write_reg(info, IGC_MDICNFG, mdicnfg)?; From 13b4c0c963980cf4389c04f9a54f4f65ff9ed25f Mon Sep 17 00:00:00 2001 From: Yuuki Takano Date: Mon, 20 Apr 2026 11:48:35 +0900 Subject: [PATCH 3/4] fix(igc): reduce phy reset polling cost Address the review feedback on PR #680. Replace the unbounded 100,000-iteration 1us busy-wait in igc_phy_hw_reset_generic with a two-phase reset-completion poll: a short tight loop for the fast-success case followed by a 1ms backoff loop to avoid burning CPU during bring-up while preserving the existing MDIC-based fallback when RST_COMPL does not assert. Also replace the PHPM magic number 0x0400 in i225 bring-up with the shared IGC_I225_PHPM_ULP constant so the low-power bit definition stays named and centralized. Verification: - make fmt - make x86_64 RELEASE=1 - rebooted the physical x86_64 target over /dev/ttyUSB0 at 38400 baud - confirmed the rebuilt kernel booted via TFTP and igc still reached link up --- awkernel_drivers/src/pcie/intel/igc/i225.rs | 4 ++-- .../src/pcie/intel/igc/igc_phy.rs | 19 +++++++++++++++++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/awkernel_drivers/src/pcie/intel/igc/i225.rs b/awkernel_drivers/src/pcie/intel/igc/i225.rs index bd9d142b4..0592db955 100644 --- a/awkernel_drivers/src/pcie/intel/igc/i225.rs +++ b/awkernel_drivers/src/pcie/intel/igc/i225.rs @@ -5,7 +5,7 @@ use crate::pcie::{ igc_mac::igc_config_fc_after_link_up_generic, igc_phy::{ igc_check_downshift_generic, igc_phy_has_link_generic, igc_setup_copper_link_generic, - IGC_I225_PHPM, IGC_I225_PHPM_GO_LINKD, + IGC_I225_PHPM, IGC_I225_PHPM_GO_LINKD, IGC_I225_PHPM_ULP, }, IgcMacType, }, @@ -551,7 +551,7 @@ fn igc_init_phy_params_i225( let phy_addr = igc_sync_mdic_phy_addr(info, hw, 1)?; let mut phpm = read_reg(info, IGC_I225_PHPM)?; - phpm &= !(IGC_I225_PHPM_GO_LINKD | 0x0400); + phpm &= !(IGC_I225_PHPM_GO_LINKD | IGC_I225_PHPM_ULP); write_reg(info, IGC_I225_PHPM, phpm)?; ops.power_up(info, hw)?; diff --git a/awkernel_drivers/src/pcie/intel/igc/igc_phy.rs b/awkernel_drivers/src/pcie/intel/igc/igc_phy.rs index bee18b2b8..1f214ff8b 100644 --- a/awkernel_drivers/src/pcie/intel/igc/igc_phy.rs +++ b/awkernel_drivers/src/pcie/intel/igc/igc_phy.rs @@ -32,7 +32,7 @@ const IGC_I225_PHPM_DIS_1000: u32 = 0x0040; // Disable 1G globally const IGC_I225_PHPM_SPD_B2B_EN: u32 = 0x0080; // Smart Power Down Back2Back const IGC_I225_PHPM_RST_COMPL: u32 = 0x0100; // PHY Reset Completed const IGC_I225_PHPM_DIS_100_D3: u32 = 0x0200; // Disable 100M in D3 -const IGC_I225_PHPM_ULP: u32 = 0x0400; // Ultra Low-Power Mode +pub(super) const IGC_I225_PHPM_ULP: u32 = 0x0400; // Ultra Low-Power Mode const IGC_I225_PHPM_DIS_2500: u32 = 0x0800; // Disable 2.5G globally const IGC_I225_PHPM_DIS_2500_D3: u32 = 0x1000; // Disable 2.5G in D3 @@ -289,6 +289,9 @@ pub(super) fn igc_phy_hw_reset_generic( info: &mut PCIeInfo, hw: &mut IgcHw, ) -> Result<(), IgcDriverErr> { + const PHY_RESET_POLL_TIGHT_LOOPS: usize = 150; + const PHY_RESET_POLL_BACKOFF_MSEC: usize = 100; + match ops.check_reset_block(info) { Err(IgcDriverErr::BlkPhyReset) => { return Ok(()); @@ -320,12 +323,24 @@ pub(super) fn igc_phy_hw_reset_generic( wait_microsec(150); - for _ in 0..100000 { + // Some firmware leaves RST_COMPL deasserted even when the PHY is + // already responsive over MDIC. Keep a short tight poll for the + // fast-success case, then fall back to millisecond backoff to + // avoid burning CPU during bring-up. + for _ in 0..PHY_RESET_POLL_TIGHT_LOOPS { phpm = read_reg(info, IGC_I225_PHPM)?; + if phpm & IGC_I225_PHPM_RST_COMPL != 0 { + return Ok(()); + } wait_microsec(1); + } + + for _ in 0..PHY_RESET_POLL_BACKOFF_MSEC { + phpm = read_reg(info, IGC_I225_PHPM)?; if phpm & IGC_I225_PHPM_RST_COMPL != 0 { return Ok(()); } + wait_millisec(1); } if attempt == 0 { From 925a285406da3ae1651ebc41c8072890fdb09b0a Mon Sep 17 00:00:00 2001 From: Yuuki Takano Date: Tue, 21 Apr 2026 13:40:08 +0900 Subject: [PATCH 4/4] fix(igc): address review comments on PR #680 - Elevate PHY reset timeout log from debug to warn for visibility - Add write_flush after IGC_MDICNFG write in igc_sync_mdic_phy_addr - Add doc comments to igc_sync_mdic_phy_addr and igc_poll_link - Simplify redundant should_poll_link condition to irq.is_none() Co-Authored-By: Claude Sonnet 4.6 --- awkernel_drivers/src/pcie/intel/igc.rs | 8 +++++--- awkernel_drivers/src/pcie/intel/igc/igc_phy.rs | 7 ++++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/awkernel_drivers/src/pcie/intel/igc.rs b/awkernel_drivers/src/pcie/intel/igc.rs index e6db7b625..ea0426cc6 100644 --- a/awkernel_drivers/src/pcie/intel/igc.rs +++ b/awkernel_drivers/src/pcie/intel/igc.rs @@ -256,8 +256,6 @@ impl Igc { let mut inner = self.inner.read(); let igc_icr = read_reg(&inner.info, igc_regs::IGC_ICR)?; - let should_poll_link = irq.is_none() && (igc_icr & igc_defines::IGC_ICR_LSC) == 0; - if (igc_icr & igc_defines::IGC_ICR_LSC) != 0 { // Link status change interrupt. drop(inner); @@ -266,7 +264,7 @@ impl Igc { inner.igc_intr_link()?; } inner = self.inner.read(); - } else if should_poll_link { + } else if irq.is_none() { drop(inner); { let mut inner = self.inner.write(); @@ -810,6 +808,10 @@ impl IgcInner { ) } + /// Polls link status when called from a tick-driven path (i.e. `intr` is + /// invoked without a real IRQ). Forces `get_link_status = true` so that + /// `igc_intr_link` re-checks the hardware even if a link-change interrupt + /// was not observed. #[inline(always)] fn igc_poll_link(&mut self) -> Result<(), IgcDriverErr> { self.hw.mac.get_link_status = true; diff --git a/awkernel_drivers/src/pcie/intel/igc/igc_phy.rs b/awkernel_drivers/src/pcie/intel/igc/igc_phy.rs index 1f214ff8b..d837dc680 100644 --- a/awkernel_drivers/src/pcie/intel/igc/igc_phy.rs +++ b/awkernel_drivers/src/pcie/intel/igc/igc_phy.rs @@ -36,6 +36,10 @@ pub(super) const IGC_I225_PHPM_ULP: u32 = 0x0400; // Ultra Low-Power Mode const IGC_I225_PHPM_DIS_2500: u32 = 0x0800; // Disable 2.5G globally const IGC_I225_PHPM_DIS_2500_D3: u32 = 0x1000; // Disable 2.5G in D3 +/// Reads the PHY address from MDICNFG, falls back to `default_addr` if the +/// field is zero, writes the resolved address back to MDICNFG, and stores it +/// in `hw.phy.addr`. The default value of 1 matches the hardware power-on +/// default and is consistent with the BSD igc driver. pub(super) fn igc_sync_mdic_phy_addr( info: &mut PCIeInfo, hw: &mut IgcHw, @@ -51,6 +55,7 @@ pub(super) fn igc_sync_mdic_phy_addr( let mdicnfg = (mdicnfg & !IGC_MDICNFG_PHY_MASK) | (phy_addr << IGC_MDICNFG_PHY_SHIFT); write_reg(info, IGC_MDICNFG, mdicnfg)?; + write_flush(info)?; hw.phy.addr = phy_addr; Ok(phy_addr) @@ -358,7 +363,7 @@ pub(super) fn igc_phy_hw_reset_generic( } let status = read_reg(info, IGC_STATUS).unwrap_or(0); - log::debug!( + log::warn!( "Timeout expired after a phy reset: ctrl={ctrl:#010x}, status={status:#010x}, phpm={phpm:#010x}" );