Re: [PATCH net 1/2] ice: dpll: serialize changes of the SyncE CGU registers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] ice: wrong port modulo in ice_tspll_bypass_mux_active_e825c()
  [1]
--

commit 54d93c3b2bb6d3588addc95fab52eae6473a669b
Author: Ivan Vecera <ivecera@redhat.com>

ice: dpll: serialize changes of the SyncE CGU registers

This commit adds a mutex to struct ice_adapter to serialize read-modify-write
updates to the shared E825C SyncE CGU registers R10 and R11 across PFs.

> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index 7980eaf9ce253..449819cf8c879 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -1368,7 +1368,9 @@ void ice_ptp_link_change(struct ice_pf *pf, bool linkup)
>  				break;
>  			}
>  
> +			mutex_lock(&pf->adapter->cgu_synce_lock);
>  			err = ice_tspll_cfg_synce_ethdiv_e825c(hw, clk_pin);
> +			mutex_unlock(&pf->adapter->cgu_synce_lock);
>  			if (active && err) {
>  				dev_err_once(ice_pf_to_dev(pf),
>  					     "Failed to configure SyncE ETH divider for pin %d, err %d\n",

[Severity: Medium]
This problem wasn't introduced by this patch, but does
ice_tspll_bypass_mux_active_e825c() compute the wrong port index when setting
active here [1]?

drivers/net/ethernet/intel/ice/ice_tspll.c:ice_tspll_bypass_mux_active_e825c() {
    ...
	if (active_clk == port % hw->ptp.ports_per_phy +
			  ICE_CGU_BYPASS_MUX_OFFSET_E825C)
		*active = true;
    ...
}

ice_tspll_cfg_bypass_mux_e825c() programs the shared CGU mux with
port_num + ICE_CGU_BYPASS_MUX_OFFSET_E825C without reducing port_num modulo
ports_per_phy, and ice_dpll_rclk_update_e825c() checks it the same way.  On
dual-complex E825C, taking port % hw->ptp.ports_per_phy folds secondary
complex ports 4..7 onto 0..3, so a secondary complex port is never recognized
as active when selected and is falsely treated as active when the
corresponding primary complex port is selected.

[1] https://sashiko.dev/#/bug/linux-800face2-554f-486e-9268-16b7de29259d

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110094350.184185-1-ivecera@redhat.com?part=1