From: Karol Kolacinski <[email protected]> Date: Thu, 25 Jul 2024 11:34:48 +0200
> From: Michal Michalik <[email protected]> > > Add specific functions and definitions for E830 devices to enable > PTP support. > Introduce new PHY model ICE_PHY_E830. > E830 devices support direct write to GLTSYN_ registers without shadow > registers and 64 bit read of PHC time. > > Reviewed-by: Przemek Kitszel <[email protected]> > Co-developed-by: Milena Olech <[email protected]> > Signed-off-by: Milena Olech <[email protected]> > Co-developed-by: Paul Greenwalt <[email protected]> > Signed-off-by: Paul Greenwalt <[email protected]> > Signed-off-by: Michal Michalik <[email protected]> > Co-developed-by: Karol Kolacinski <[email protected]> > Signed-off-by: Karol Kolacinski <[email protected]> > --- > V2 -> V3: Fixed kdoc for ice_is_e***() and ice_ptp_init_phy_e830() > V1 -> V2: Fixed compilation issue with GENMASK bits higher than 32 > > drivers/net/ethernet/intel/ice/ice_common.c | 13 +- > drivers/net/ethernet/intel/ice/ice_common.h | 1 + > .../net/ethernet/intel/ice/ice_hw_autogen.h | 4 + > drivers/net/ethernet/intel/ice/ice_ptp.c | 11 +- > drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 199 ++++++++++++++++-- > drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 25 ++- > drivers/net/ethernet/intel/ice/ice_type.h | 1 + > 7 files changed, 226 insertions(+), 28 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_common.c > b/drivers/net/ethernet/intel/ice/ice_common.c > index 009716a12a26..8bff63d3d664 100644 > --- a/drivers/net/ethernet/intel/ice/ice_common.c > +++ b/drivers/net/ethernet/intel/ice/ice_common.c > @@ -267,7 +267,7 @@ bool ice_is_e822(struct ice_hw *hw) > * ice_is_e823 > * @hw: pointer to the hardware structure > * > - * returns true if the device is E823-L or E823-C based, false if not. > + * Return: true if the device is E823-L or E823-C based, false if not. I'd move this, together with FIELD_{GET,PREP}() conversion, to a separate commit maybe (one is enough). > */ > bool ice_is_e823(struct ice_hw *hw) > { > @@ -307,6 +307,17 @@ bool ice_is_e825c(struct ice_hw *hw) > } > } > > +/** > + * ice_is_e830 > + * @hw: pointer to the hardware structure > + * > + * Return: true if the device is E830 based, false if not. > + */ > +bool ice_is_e830(const struct ice_hw *hw) > +{ > + return hw->mac_type == ICE_MAC_E830; > +} I think making this one check external instead of a static inline only increases object code size. Can't they reside in the header? [...] > +/** > + * ice_ptp_init_phc_e830 - Perform E830 specific PHC initialization > + * @hw: pointer to HW struct > + * > + * Perform E830-specific PTP hardware clock initialization steps. > + * > + * Return: 0 on success > + */ > +static int ice_ptp_init_phc_e830(struct ice_hw *hw) > +{ > + ice_ptp_cfg_sync_delay(hw, ICE_E810_E830_SYNC_DELAY); > + return 0; Can't this and the below functions be void since they always return zero? > +} > + > +/** > + * ice_ptp_write_direct_incval_e830 - Prep PHY port increment value change > + * @hw: pointer to HW struct > + * @incval: The new 40bit increment value to prepare > + * > + * Prepare the PHY port for a new increment value by programming the PHC > + * GLTSYN_INCVAL_L and GLTSYN_INCVAL_H registers. The actual change is > + * completed by FW automatically. > + */ > +static int ice_ptp_write_direct_incval_e830(struct ice_hw *hw, u64 incval) const @hw (below as well)? [...] > @@ -5474,12 +5618,14 @@ void ice_ptp_init_hw(struct ice_hw *hw) > { > struct ice_ptp_hw *ptp = &hw->ptp; > > - if (ice_is_e822(hw) || ice_is_e823(hw)) > - ice_ptp_init_phy_e82x(ptp); > - else if (ice_is_e810(hw)) > + if (ice_is_e810(hw)) > ice_ptp_init_phy_e810(ptp); > + else if (ice_is_e822(hw) || ice_is_e823(hw)) > + ice_ptp_init_phy_e82x(ptp); > else if (ice_is_e825c(hw)) > ice_ptp_init_phy_e825c(hw); > + else if (ice_is_e830(hw)) > + ice_ptp_init_phy_e830(ptp); > else > ptp->phy_model = ICE_PHY_UNSUP; Since at least most of these functions just check for one field, can't this be one switch-case on that fields (if you open-code that check)? > } > @@ -5570,6 +5716,8 @@ static int ice_ptp_port_cmd(struct ice_hw *hw, enum > ice_ptp_tmr_cmd cmd) > switch (hw->ptp.phy_model) { > case ICE_PHY_E810: > return ice_ptp_port_cmd_e810(hw, cmd); > + case ICE_PHY_E830: > + return ice_ptp_port_cmd_e830(hw, cmd); > default: > break; > } > @@ -5640,6 +5788,10 @@ int ice_ptp_init_time(struct ice_hw *hw, u64 time) > tmr_idx = hw->func_caps.ts_func_info.tmr_index_owned; > > /* Source timers */ > + /* For E830 we don't need to use shadow registers, its automatic */ ^^^ it's [...] Thanks, Olek
