Re: [Intel-wired-lan] [PATCH iwl-next 4/4] ice: check netlist before enabling ICE_F_GNSS

[Jacob Keller <jacob.e.keller@intel.com>]

On 8/15/2023 3:35 PM, Jacob Keller wrote:
> Similar to the change made for ICE_F_SMA_CTRL, check the netlist before
> enabling support for ICE_F_GNSS. This ensures that the driver only enables
> the GNSS feature on devices which actually have the feature enabled in the
> firmware device configuration.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_adminq_cmd.h |  2 ++
>  drivers/net/ethernet/intel/ice/ice_lib.c        |  7 +++----
>  drivers/net/ethernet/intel/ice/ice_ptp_hw.c     | 15 +++++++++++++++
>  drivers/net/ethernet/intel/ice/ice_ptp_hw.h     |  1 +
>  4 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> index 82c4daf0a825..2f0d4bffe210 100644
> --- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> +++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> @@ -1368,6 +1368,7 @@ struct ice_aqc_link_topo_params {
>  #define ICE_AQC_LINK_TOPO_NODE_TYPE_MEZZ	7
>  #define ICE_AQC_LINK_TOPO_NODE_TYPE_ID_EEPROM	8
>  #define ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_MUX	10
> +#define ICE_AQC_LINK_TOPO_NODE_TYPE_GPS		11
>  #define ICE_AQC_LINK_TOPO_NODE_CTX_S		4
>  #define ICE_AQC_LINK_TOPO_NODE_CTX_M		\
>  				(0xF << ICE_AQC_LINK_TOPO_NODE_CTX_S)
> @@ -1407,6 +1408,7 @@ struct ice_aqc_get_link_topo {
>  #define ICE_AQC_GET_LINK_TOPO_NODE_NR_PCA9575			0x21
>  #define ICE_AQC_GET_LINK_TOPO_NODE_NR_C827			0x31
>  #define ICE_ACQ_GET_LINK_TOPO_NODE_NR_GEN_CLK_MUX		0x47
> +#define ICE_ACQ_GET_LINK_TOPO_NODE_NR_GEN_GPS			0x48
>  	u8 rsvd[9];
>  };
>  
> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
> index 4ac8998cb964..24a30b55c7ff 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -3991,10 +3991,9 @@ void ice_init_feature_support(struct ice_pf *pf)
>  			break;
>  		if (ice_is_clock_mux_present_e810t(&pf->hw))
>  			ice_set_feature_support(pf, ICE_F_SMA_CTRL);
> -		if (ice_is_e810t(&pf->hw)) {
> -			if (ice_gnss_is_gps_present(&pf->hw))
> -				ice_set_feature_support(pf, ICE_F_GNSS);
> -		}
> +		if (ice_is_gps_present_e810t(&pf->hw) &&
> +		    ice_gnss_is_gps_present(&pf->hw))
> +			ice_set_feature_support(pf, ICE_F_GNSS);

The names for these two functions are incredibly similar, and I'm not
sure if I have a better name. @Karol, this is what we have in our
sourceforge driver now, but I'm not sure I like it.

I think I'll drop this patch from the series and we can investigate this
and sort things out a bit?

Thanks,
Jake

>  		break;
>  	default:
>  		break;
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> index bd3f32bfbc78..455d7a15de26 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> @@ -3034,6 +3034,21 @@ bool ice_is_clock_mux_present_e810t(struct ice_hw *hw)
>  	return true;
>  }
>  
> +/**
> + * ice_is_gps_present_e810t
> + * @hw: pointer to the hw struct
> + *
> + * Check if the GPS generic device is present in the netlist
> + */
> +bool ice_is_gps_present_e810t(struct ice_hw *hw)
> +{
> +	if (ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_GPS,
> +				  ICE_ACQ_GET_LINK_TOPO_NODE_NR_GEN_GPS, NULL))
> +		return false;
> +
> +	return true;
> +}
> +
>  /**
>   * ice_read_sma_ctrl_e810t
>   * @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 3768e7a01920..4399338b7347 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> @@ -196,6 +196,7 @@ int ice_phy_cfg_rx_offset_e822(struct ice_hw *hw, u8 port);
>  
>  /* E810 family functions */
>  bool ice_is_clock_mux_present_e810t(struct ice_hw *hw);
> +bool ice_is_gps_present_e810t(struct ice_hw *hw);
>  int ice_ptp_init_phy_e810(struct ice_hw *hw);
>  int ice_read_sma_ctrl_e810t(struct ice_hw *hw, u8 *data);
>  int ice_write_sma_ctrl_e810t(struct ice_hw *hw, u8 data);
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan