> -----Original Message----- > From: Jacob Keller <[email protected]> > Sent: Friday, September 25, 2026 4:57 PM > To: Intel Wired LAN <[email protected]>; Machnikowski, Maciej > <[email protected]>; Keller, Jacob E <[email protected]>; > Korba, Przemyslaw <[email protected]>; Nguyen, Anthony L > <[email protected]>; Nitka, Grzegorz <[email protected]>; > Kubalewski, Arkadiusz <[email protected]> > Cc: Keller, Jacob E <[email protected]> > Subject: [PATCH iwl-net v3 03/15] ice: fix PHY port restart serialization > > The ice_port_phy_restart() function is responsible for restarting a PHY port, > primarily after a link transition. PHY ports must also all be restarted after > a reset, and the E822 devices also restart the PHY after the .settime64 > operation. > > If a PHY restart occurs concurrently with a link transition, it is possible > that the link state read could be re-ordered such that the link transition > would start the PHY but a concurrent restart (such as from .settime64) could > see the old link state and decide it must stop the PHY. > > This occurs because the restart procedure depends on the link state value but > setting that value is not serialized with the per-port ps_lock. A following > fix for E825 is going to modify the .setttime64 to also correctly restart the > PHY timers, opening the E825 device to a race between > .settime64 and a concurrent link change. > > To avoid this, we need to ensure that the port link_up field is set within > the same critical section as the restart procedure. This way we ensure that > the end result is the PHY programmed to the correct state. > > Additionally note that the PHY restart procedure already cannot run > concurrently across two ports. The procedure requires executing timer > commands which in turn require the PTP hardware semaphore. > > Thus, to simplify the locking behavior, convert the per-port ps_lock to a > single mutex in the ice_adapter. Acquire this in ice_ptp_restart_all_phy(), > and hold it while calling ice_ptp_port_phy_stop() and > ice_ptp_port_phy_restart(). > > The ice_ptp_link_change() function now acquires the port lock for the entire > change. This ensures that the link state is set under lock. Note the function > does also acquire the dplls.lock for E825 devices. This ordering is safe > because the other callers of dplls.lock do not acquire the ps_lock. > > The switch to a single lock for the adapter instead of one per port is easier > to reason about. It also could be a first step in a plan to replace the PTP > hardware semaphore completely, which is under investigation for the future. > > This fixes one of the issues reported by Sashiko during a previous review of > this series, linked here as the Closes tag. > > Closes: > https://lore.kernel.org/netdev/[email protected]/ > Fixes: 3a7496234d17 ("ice: implement basic E822 PTP support") > Signed-off-by: Jacob Keller <[email protected]> > --- > drivers/net/ethernet/intel/ice/ice_adapter.h | 4 ++ > drivers/net/ethernet/intel/ice/ice_ptp.h | 2 - > drivers/net/ethernet/intel/ice/ice_adapter.c | 3 ++ > drivers/net/ethernet/intel/ice/ice_ptp.c | 58 > ++++++++++++++-------------- > 4 files changed, 36 insertions(+), 31 deletions(-)
Tested-by: Alexander Nowlin <[email protected]>
