On 4/9/2025 5:25 AM, Karol Kolacinski wrote:
> diff --git a/drivers/net/ethernet/intel/ice/ice_tspll.c
> b/drivers/net/ethernet/intel/ice/ice_tspll.c
> index 6bbb570841bb..15b07b7842d2 100644
> --- a/drivers/net/ethernet/intel/ice/ice_tspll.c
> +++ b/drivers/net/ethernet/intel/ice/ice_tspll.c
> @@ -268,76 +270,89 @@ static int ice_tspll_cfg_e825c(struct ice_hw *hw, enum
> ice_tspll_freq clk_freq,
> return -EINVAL;
> }
>
> - ICE_READ_CGU_REG_OR_DIE(hw, ICE_CGU_R9, &dw9.val);
> - ICE_READ_CGU_REG_OR_DIE(hw, ICE_CGU_R24, &dw24.val);
> - ICE_READ_CGU_REG_OR_DIE(hw, ICE_CGU_R16, &dw16.val);
> - ICE_READ_CGU_REG_OR_DIE(hw, ICE_CGU_R23, &dw23.val);
> - ICE_READ_CGU_REG_OR_DIE(hw, TSPLL_RO_LOCK_E825C, &ro_lock.val);
> + ICE_READ_CGU_REG_OR_DIE(hw, ICE_CGU_R9, &r9);
> + ICE_READ_CGU_REG_OR_DIE(hw, ICE_CGU_R23, &r23);
> + ICE_READ_CGU_REG_OR_DIE(hw, ICE_CGU_RO_LOCK, &val);
>
> - ice_tspll_log_cfg(hw, dw24.ts_pll_enable, dw23.time_ref_sel,
> - dw9.time_ref_freq_sel,
> - ro_lock.plllock_true_lock_cri, false);
> + ice_tspll_log_cfg(hw, !!FIELD_GET(ICE_CGU_R23_R24_TSPLL_ENABLE, r23),
> + FIELD_GET(ICE_CGU_R23_R24_TIME_REF_SEL, r23),
> + FIELD_GET(ICE_CGU_R9_TIME_REF_FREQ_SEL, r9),
> + !!FIELD_GET(ICE_CGU_RO_LOCK_TRUE_LOCK, val),
> + false);
>
E825-C used to check dw24 for the enable flag, but now checks dw23. This
is a bug fix for the log message, but you don't call that out in the commit.
I would prefer to see this split so that we do the functional changes
either before or after the rework of the names.
At a minimum, please call out any of the behavioral changes or bug fixes
in the commit message.
> + if (FIELD_GET(ICE_CGU_R9_TIME_SYNC_EN, r9)) {
> + r9 &= ~ICE_CGU_R9_TIME_SYNC_EN;
> + ICE_WRITE_CGU_REG_OR_DIE(hw, ICE_CGU_R9, r9);
> }
Ditto here for the r9 changes which don't appear to be in the pre-image
either.
Thanks,
Jake
