| Message ID | 20231214085050.3161674-1-arkadiusz.kubalewski@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [iwl-net] ice: dpll: fix phase offset value | expand |
Dear Arkadiusz, Thank you for your patch. Am 14.12.23 um 09:50 schrieb Arkadiusz Kubalewski: > Stop dividing the phase_offset value received from firmware, this is > fault introduced with the initial implementation. … 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. What problems does this fault have, and how can it be tested, that it’s fixed? Kind regards, Paul > Fixes: 8a3a565ff210 ("ice: add admin commands to access cgu configuration") > Fixes: 90e1c90750d7 ("ice: dpll: implement phase related callbacks") > Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> > Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@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; > } >
>From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of >Paul Menzel >Sent: Thursday, December 14, 2023 10:31 AM > >Dear Arkadiusz, > > >Thank you for your patch. > >Am 14.12.23 um 09:50 schrieb Arkadiusz Kubalewski: >> Stop dividing the phase_offset value received from firmware, this is >> fault introduced with the initial implementation. > >… 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. > >What problems does this fault have, and how can it be tested, that it’s >fixed? > Hi Paul, Basically the userspace receives value which is lower than it should be. Instead of expected in DPLL subsystem 0.001ps phase offset resolution the user is provided with 1ps resolution. > >Kind regards, > >Paul > Thanks for your feedback, I will prepare better commit message and will send the v2. Thank you! Arkadiusz > >> Fixes: 8a3a565ff210 ("ice: add admin commands to access cgu >> configuration") >> Fixes: 90e1c90750d7 ("ice: dpll: implement phase related callbacks") >> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> >> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> >> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@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; >> } >> >_______________________________________________ >Intel-wired-lan mailing list >Intel-wired-lan@osuosl.org >https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
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; }
Stop dividing the phase_offset value received from firmware, this is fault introduced with 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. Fixes: 8a3a565ff210 ("ice: add admin commands to access cgu configuration") Fixes: 90e1c90750d7 ("ice: dpll: implement phase related callbacks") Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> --- drivers/net/ethernet/intel/ice/ice_common.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)