On Fri, Feb 21, 2025 at 01:31:23PM +0100, Grzegorz Nitka wrote:
> From: Karol Kolacinski <karol.kolacin...@intel.com>
>
> According to the E825C specification, SBQ address for ports on a single
> complex is device 2 for PHY 0 and device 13 for PHY1.
> For accessing ports on a dual complex E825C (so called 2xNAC mode),
> the driver should use destination device 2 (referred as phy_0) for
> the current complex PHY and device 13 (referred as phy_0_peer) for
> peer complex PHY.
>
> Differentiate SBQ destination device by checking if current PF port
> number is on the same PHY as target port number.
>
> Adjust 'ice_get_lane_number' function to provide unique port number for
> ports from PHY1 in 'dual' mode config (by adding fixed offset for PHY1
> ports). Cache this value in ice_hw struct.
>
> Introduce ice_get_primary_hw wrapper to get access to timesync register
> not available from second NAC.
>
> Reviewed-by: Przemek Kitszel <przemyslaw.kits...@intel.com>
> Signed-off-by: Karol Kolacinski <karol.kolacin...@intel.com>
> Co-developed-by: Grzegorz Nitka <grzegorz.ni...@intel.com>
> Signed-off-by: Grzegorz Nitka <grzegorz.ni...@intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice.h | 60 ++++++++++++++++++++-
> drivers/net/ethernet/intel/ice/ice_common.c | 6 ++-
> drivers/net/ethernet/intel/ice/ice_ptp.c | 49 ++++++++++++-----
> drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 39 +++++++++++---
> drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 5 --
> drivers/net/ethernet/intel/ice/ice_type.h | 1 +
> 6 files changed, 134 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice.h
> b/drivers/net/ethernet/intel/ice/ice.h
> index 53b990e2e850..d80ab48402f1 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -193,8 +193,6 @@
>
> #define ice_pf_to_dev(pf) (&((pf)->pdev->dev))
>
> -#define ice_pf_src_tmr_owned(pf)
> ((pf)->hw.func_caps.ts_func_info.src_tmr_owned)
> -
> enum ice_feature {
> ICE_F_DSCP,
> ICE_F_PHY_RCLK,
> @@ -1049,4 +1047,62 @@ static inline void ice_clear_rdma_cap(struct ice_pf
> *pf)
> }
>
> extern const struct xdp_metadata_ops ice_xdp_md_ops;
> +
> +/**
> + * ice_is_dual - Check if given config is multi-NAC
> + * @hw: pointer to HW structure
> + *
> + * Return: true if the device is running in mutli-NAC (Network
> + * Acceleration Complex) configuration variant, false otherwise
> + * (always false for non-E825 devices).
> + */
> +static inline bool ice_is_dual(struct ice_hw *hw)
> +{
> + return hw->mac_type == ICE_MAC_GENERIC_3K_E825 &&
> + (hw->dev_caps.nac_topo.mode & ICE_NAC_TOPO_DUAL_M);
> +}
Thanks for the complete Kernel doc, and not adding unnecessary () around
the value returned. Overall these helpers seem nice and clean to me.
...
> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
> b/drivers/net/ethernet/intel/ice/ice_common.c
> index ed7ef8608cbb..4ff4c99d0872 100644
> --- a/drivers/net/ethernet/intel/ice/ice_common.c
> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
> @@ -1135,6 +1135,8 @@ int ice_init_hw(struct ice_hw *hw)
> }
> }
>
> + hw->lane_num = ice_get_phy_lane_number(hw);
> +
Unfortunately there seems to be a bit of a problem here:
The type of hw->lane number is u8.
But ice_get_phy_lane_number returns an int,
which may be a negative error value...
...
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c
> b/drivers/net/ethernet/intel/ice/ice_ptp.c
...
> @@ -3177,17 +3203,16 @@ void ice_ptp_init(struct ice_pf *pf)
> {
> struct ice_ptp *ptp = &pf->ptp;
> struct ice_hw *hw = &pf->hw;
> - int lane_num, err;
> + int err;
>
> ptp->state = ICE_PTP_INITIALIZING;
>
> - lane_num = ice_get_phy_lane_number(hw);
> - if (lane_num < 0) {
> - err = lane_num;
> + if (hw->lane_num < 0) {
... which is checked here.
But because hw->lane_num is unsigned, this condition is always false.
FWIIW, I think it is nice that that hw->lane is a u8.
But error handling seems broken here.
> + err = hw->lane_num;
> goto err_exit;
> }
> + ptp->port.port_num = hw->lane_num;
>
> - ptp->port.port_num = (u8)lane_num;
> ice_ptp_init_hw(hw);
>
> ice_ptp_init_tx_interrupt_mode(pf);
...
