| Message ID | 20240627151127.284884-16-karol.kolacinski@intel.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ice: Cleanup and refactor PTP pin handling | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
Dear Karol, dear Sergey, Thank you for the patch. Am 27.06.24 um 17:09 schrieb Karol Kolacinski: > From: Sergey Temerkhanov <sergey.temerkhanov@intel.com> > > Implement 1PPS signal enabling/disabling in CGU. The amplitude is > always the maximum in this implementation (Please add a dot/period at the end of sentences.) Could you please elaborate why using the maximum is alright, that means what are the downsides, and what is the alternative approahc. > Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> > Signed-off-by: Sergey Temerkhanov <sergey.temerkhanov@intel.com> > Co-developed-by: Karol Kolacinski <karol.kolacinski@intel.com> > Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com> > --- > drivers/net/ethernet/intel/ice/ice_ptp.c | 10 ++++++++++ > drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 21 +++++++++++++++++++++ > drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 1 + > 3 files changed, 32 insertions(+) > > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c > index d9ff94a4b293..b97ea2b61e5c 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ptp.c > +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c > @@ -4,6 +4,7 @@ > #include "ice.h" > #include "ice_lib.h" > #include "ice_trace.h" > +#include "ice_cgu_regs.h" > > static const char ice_pin_names[][64] = { > "SDP0", > @@ -1708,6 +1709,15 @@ static int ice_ptp_write_perout(struct ice_hw *hw, unsigned int chan, > /* 0. Reset mode & out_en in AUX_OUT */ > wr32(hw, GLTSYN_AUX_OUT(chan, tmr_idx), 0); > > + if (ice_is_e825c(hw)) { > + int err; > + > + /* Enable/disable CGU 1PPS output for E825C */ > + err = ice_cgu_ena_pps_out(hw, !!period); > + if (err) > + return err; > + } Does only E825C products support this feature? > + > /* 1. Write perout with half of required period value. > * HW toggles output when source clock hits the TGT and then adds > * GLTSYN_CLKO value to the target, so it ends up with 50% duty cycle. > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c > index 07ecf2a86742..fa7cf8453b88 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c > +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c > @@ -661,6 +661,27 @@ static int ice_cfg_cgu_pll_e825c(struct ice_hw *hw, > return 0; > } > > +#define ICE_ONE_PPS_OUT_AMP_MAX 3 > + > +/** > + * ice_cgu_ena_pps_out - Enable/disable 1PPS output > + * @hw: pointer to the HW struct > + * @ena: Enable/disable 1PPS output > + */ > +int ice_cgu_ena_pps_out(struct ice_hw *hw, bool ena) Is `ena` short for enable? > +{ > + union nac_cgu_dword9 dw9; > + int err; > + > + err = ice_read_cgu_reg_e82x(hw, NAC_CGU_DWORD9, &dw9.val); > + if (err) > + return err; > + > + dw9.one_pps_out_en = ena; > + dw9.one_pps_out_amp = ena * ICE_ONE_PPS_OUT_AMP_MAX; > + return ice_write_cgu_reg_e82x(hw, NAC_CGU_DWORD9, dw9.val); > +} > + > /** > * ice_cfg_cgu_pll_dis_sticky_bits_e82x - disable TS PLL sticky bits > * @hw: pointer to the HW struct > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h > index ff98f76969e3..382e84568256 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h > +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h > @@ -331,6 +331,7 @@ extern const struct ice_vernier_info_e82x e822_vernier[NUM_ICE_PTP_LNK_SPD]; > > /* Device agnostic functions */ > u8 ice_get_ptp_src_clock_index(struct ice_hw *hw); > +int ice_cgu_ena_pps_out(struct ice_hw *hw, bool ena); If *ena* means “enable”, I do not like this pattern very much, and I’d prefer an enable and a disable function. > bool ice_ptp_lock(struct ice_hw *hw); > void ice_ptp_unlock(struct ice_hw *hw); > void ice_ptp_src_cmd(struct ice_hw *hw, enum ice_ptp_tmr_cmd cmd); Kind regards, Paul
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c index d9ff94a4b293..b97ea2b61e5c 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp.c +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c @@ -4,6 +4,7 @@ #include "ice.h" #include "ice_lib.h" #include "ice_trace.h" +#include "ice_cgu_regs.h" static const char ice_pin_names[][64] = { "SDP0", @@ -1708,6 +1709,15 @@ static int ice_ptp_write_perout(struct ice_hw *hw, unsigned int chan, /* 0. Reset mode & out_en in AUX_OUT */ wr32(hw, GLTSYN_AUX_OUT(chan, tmr_idx), 0); + if (ice_is_e825c(hw)) { + int err; + + /* Enable/disable CGU 1PPS output for E825C */ + err = ice_cgu_ena_pps_out(hw, !!period); + if (err) + return err; + } + /* 1. Write perout with half of required period value. * HW toggles output when source clock hits the TGT and then adds * GLTSYN_CLKO value to the target, so it ends up with 50% duty cycle. diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c index 07ecf2a86742..fa7cf8453b88 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c @@ -661,6 +661,27 @@ static int ice_cfg_cgu_pll_e825c(struct ice_hw *hw, return 0; } +#define ICE_ONE_PPS_OUT_AMP_MAX 3 + +/** + * ice_cgu_ena_pps_out - Enable/disable 1PPS output + * @hw: pointer to the HW struct + * @ena: Enable/disable 1PPS output + */ +int ice_cgu_ena_pps_out(struct ice_hw *hw, bool ena) +{ + union nac_cgu_dword9 dw9; + int err; + + err = ice_read_cgu_reg_e82x(hw, NAC_CGU_DWORD9, &dw9.val); + if (err) + return err; + + dw9.one_pps_out_en = ena; + dw9.one_pps_out_amp = ena * ICE_ONE_PPS_OUT_AMP_MAX; + return ice_write_cgu_reg_e82x(hw, NAC_CGU_DWORD9, dw9.val); +} + /** * ice_cfg_cgu_pll_dis_sticky_bits_e82x - disable TS PLL sticky bits * @hw: pointer to the HW struct diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h index ff98f76969e3..382e84568256 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h @@ -331,6 +331,7 @@ extern const struct ice_vernier_info_e82x e822_vernier[NUM_ICE_PTP_LNK_SPD]; /* Device agnostic functions */ u8 ice_get_ptp_src_clock_index(struct ice_hw *hw); +int ice_cgu_ena_pps_out(struct ice_hw *hw, bool ena); bool ice_ptp_lock(struct ice_hw *hw); void ice_ptp_unlock(struct ice_hw *hw); void ice_ptp_src_cmd(struct ice_hw *hw, enum ice_ptp_tmr_cmd cmd);