[Intel-wired-lan] [PATCH iwl-net v2] ice: dpll: fix phase offset value

[Paul Menzel]

Dear Arkadiusz,


Am 18.12.23 um 15:58 schrieb Arkadiusz Kubalewski:
> Stop dividing the phase_offset value received from firmware. This fault
> is present since the initial implementation.
> The phase_offset value received from firmware is in 0.01ps resolution.
> Dpll subsystem is using the value in 0.001ps, raw value is adjusted
> before providing it to the user.
> 
> The user can observe the value of phase offset with response to
> `pin-get` netlink message of dpll subsystem for an active pin:
> $ ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml \
> 	--do pin-get --json '{"id":2}'
> 
> Where example of correct response would be:
> {'board-label': 'C827_0-RCLKA',
>   'capabilities': 6,
>   'clock-id': 4658613174691613800,
>   'frequency': 1953125,
>   'id': 2,
>   'module-name': 'ice',
>   'parent-device': [{'direction': 'input',
>                      'parent-id': 6,
>                      'phase-offset': -216839550,
>                      'prio': 9,
>                      'state': 'connected'},
>                     {'direction': 'input',
>                      'parent-id': 7,
>                      'phase-offset': -42930,
>                      'prio': 8,
>                      'state': 'connected'}],
>   'phase-adjust': 0,
>   'phase-adjust-max': 16723,
>   'phase-adjust-min': -16723,
>   'type': 'mux'}
> 
> Provided phase-offset value (-42930) shall be divided by the user with
> DPLL_PHASE_OFFSET_DIVIDER to get actual value of -42.930 ps.
> 
> Before the fix, the response was not correct:
> {'board-label': 'C827_0-RCLKA',
>   'capabilities': 6,
>   'clock-id': 4658613174691613800,
>   'frequency': 1953125,
>   'id': 2,
>   'module-name': 'ice',
>   'parent-device': [{'direction': 'input',
>                      'parent-id': 6,
>                      'phase-offset': -216839,
>                      'prio': 9,
>                      'state': 'connected'},
>                     {'direction': 'input',
>                      'parent-id': 7,
>                      'phase-offset': -42,
>                      'prio': 8,
>                      'state': 'connected'}],
>   'phase-adjust': 0,
>   'phase-adjust-max': 16723,
>   'phase-adjust-min': -16723,
>   'type': 'mux'}
> 
> Where phase-offset value (-42), after division
> (DPLL_PHASE_OFFSET_DIVIDER) would be: -0.042 ps.

Thank you for the detailed description.

> Fixes: 8a3a565ff210 ("ice: add admin commands to access cgu configuration")
> Fixes: 90e1c90750d7 ("ice: dpll: implement phase related callbacks")
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov at intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel at intel.com>
> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski at intel.com>
> ---
>   drivers/net/ethernet/intel/ice/ice_common.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
> index 9a6c25f98632..edac34c796ce 100644
> --- a/drivers/net/ethernet/intel/ice/ice_common.c
> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
> @@ -5332,7 +5332,6 @@ ice_aq_get_cgu_dpll_status(struct ice_hw *hw, u8 dpll_num, u8 *ref_state,
>   			   u8 *eec_mode)
>   {
>   	struct ice_aqc_get_cgu_dpll_status *cmd;
> -	const s64 nsec_per_psec = 1000LL;
>   	struct ice_aq_desc desc;
>   	int status;
>   
> @@ -5348,8 +5347,7 @@ ice_aq_get_cgu_dpll_status(struct ice_hw *hw, u8 dpll_num, u8 *ref_state,
>   		*phase_offset = le32_to_cpu(cmd->phase_offset_h);
>   		*phase_offset <<= 32;
>   		*phase_offset += le32_to_cpu(cmd->phase_offset_l);
> -		*phase_offset = div64_s64(sign_extend64(*phase_offset, 47),
> -					  nsec_per_psec);
> +		*phase_offset = sign_extend64(*phase_offset, 47);
>   		*eec_mode = cmd->eec_mode;
>   	}

Reviewed-by: Paul Menzel <pmenzel at molgen.mpg.de>


Kind regards,

Paul