On 9/20/2023 3:35 AM, Przemek Kitszel wrote:
> On 9/20/23 01:34, Jacob Keller wrote:
>> The ice_ptp_hw.c file has a few functions which check for whether the
>> device netlist has a node indicating hardware support for certain features.
>> These checks don't really make sense to be in ice_ptp_hw.c. In addition,
>> their names could be confusing as they just say "is_present" but really are
>> checking the netlist.
>>
>> Additionally, these functions are only compiled into the ice module if
>> CONFIG_PTP_1588_CLOCK is enabled. They are used in ice_lib.c which is
>> unconditionally compiled, so this can result in build errors if the PTP
>> support is disabled:
>>
>> ld: vmlinux.o: in function `ice_init_feature_support':
>> (.text+0x8702b8): undefined reference to `ice_is_phy_rclk_present'
>> ld: (.text+0x8702cd): undefined reference to `ice_is_cgu_present'
>> ld: (.text+0x8702d9): undefined reference to `ice_is_clock_mux_present_e810t'
>>
>
> You have two commits that combined are fixing a build error,
> one of which is realatively small (1st in this series).
> Could you please combine them, if only to provide Fixes: tag?
>
Sure I can squash these and add Fixes tag(s).
>> To fix this, we need to define these functions outside of ice_ptp.c or
>> ice_ptp_hw.c.
>>
>> The ice_common.c already hace ice_is_gps_in_netlist. Move the similar
>
> hace :)
Good catch.
>
> Codewise it looks good, I recall some of the code already :D
>
Yea, I sent a lot of the same code as part of another series a few weeks
ago but didn't notice the conflicts with DPLL, and the DPLL series
landed first. This aligns with the version I proposed.
>> functions in ice_ptp_hw.c into ice_common.c, renaming them to use the
>> postfix "_in_netlist" to match the GPS check. Where appropriate, also drop
>> the _e810t postfix as well.
>>
>> This also makes the ice_find_netlist_node only called from within
>> ice_common.c, so its safe to mark it static and stop declaring it in the
>> ice_common.h header.
>>
>> Signed-off-by: Jacob Keller <jacob.e.kel...@intel.com>
>> ---
>> drivers/net/ethernet/intel/ice/ice_common.c | 66 ++++++++++++++++++++-
>> drivers/net/ethernet/intel/ice/ice_common.h | 6 +-
>> drivers/net/ethernet/intel/ice/ice_lib.c | 6 +-
>> drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 66 ---------------------
>> drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 3 -
>> 5 files changed, 69 insertions(+), 78 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
>> b/drivers/net/ethernet/intel/ice/ice_common.c
>> index bcf7d9c56248..12c09374c2be 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_common.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
>> @@ -477,9 +477,8 @@ ice_aq_get_netlist_node(struct ice_hw *hw, struct
>> ice_aqc_get_link_topo *cmd,
>> * netlist. When found ICE_SUCCESS is returned, ICE_ERR_DOES_NOT_EXIST
>> * otherwise. If node_handle provided, it would be set to found node
>> handle.
>> */
>> -int
>> -ice_find_netlist_node(struct ice_hw *hw, u8 node_type_ctx, u8
>> node_part_number,
>> - u16 *node_handle)
>> +static int ice_find_netlist_node(struct ice_hw *hw, u8 node_type_ctx,
>> + u8 node_part_number, u16 *node_handle)
>> {
>> struct ice_aqc_get_link_topo cmd;
>> u8 rec_node_part_number;
>> @@ -2552,6 +2551,67 @@ bool ice_is_pf_c827(struct ice_hw *hw)
>> return false;
>> }
>>
>> +/**
>> + * ice_is_phy_rclk_in_netlist
>> + * @hw: pointer to the hw struct
>> + *
>> + * Check if the PHY Recovered Clock device is present in the netlist
>> + */
>> +bool ice_is_phy_rclk_in_netlist(struct ice_hw *hw)
>> +{
>> + if (ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL,
>> + ICE_AQC_GET_LINK_TOPO_NODE_NR_C827, NULL) &&
>> + ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL,
>> + ICE_AQC_GET_LINK_TOPO_NODE_NR_E822_PHY, NULL))
>> + return false;
>> +
>> + return true;
>> +}
>> +
>> +/**
>> + * ice_is_clock_mux_in_netlist
>> + * @hw: pointer to the hw struct
>> + *
>> + * Check if the Clock Multiplexer device is present in the netlist
>> + */
>> +bool ice_is_clock_mux_in_netlist(struct ice_hw *hw)
>> +{
>> + if (ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_MUX,
>> + ICE_AQC_GET_LINK_TOPO_NODE_NR_GEN_CLK_MUX,
>> + NULL))
>> + return false;
>> +
>> + return true;
>> +}
>> +
>> +/**
>> + * ice_is_cgu_in_netlist - check for CGU presence
>> + * @hw: pointer to the hw struct
>> + *
>> + * Check if the Clock Generation Unit (CGU) device is present in the
>> netlist.
>> + * Save the CGU part number in the hw structure for later use.
>> + * Return:
>> + * * true - cgu is present
>> + * * false - cgu is not present
>> + */
>> +bool ice_is_cgu_in_netlist(struct ice_hw *hw)
>> +{
>> + if (!ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL,
>> + ICE_AQC_GET_LINK_TOPO_NODE_NR_ZL30632_80032,
>> + NULL)) {
>> + hw->cgu_part_number =
>> ICE_AQC_GET_LINK_TOPO_NODE_NR_ZL30632_80032;
>> + return true;
>> + } else if (!ice_find_netlist_node(hw,
>> + ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL,
>> +
>> ICE_AQC_GET_LINK_TOPO_NODE_NR_SI5383_5384,
>> + NULL)) {
>> + hw->cgu_part_number = ICE_AQC_GET_LINK_TOPO_NODE_NR_SI5383_5384;
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> +
>> /**
>> * ice_is_gps_in_netlist
>> * @hw: pointer to the hw struct
>> diff --git a/drivers/net/ethernet/intel/ice/ice_common.h
>> b/drivers/net/ethernet/intel/ice/ice_common.h
>> index 59969f702dae..4a75c0c89301 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_common.h
>> +++ b/drivers/net/ethernet/intel/ice/ice_common.h
>> @@ -93,11 +93,11 @@ ice_aq_get_phy_caps(struct ice_port_info *pi, bool
>> qual_mods, u8 report_mode,
>> struct ice_aqc_get_phy_caps_data *caps,
>> struct ice_sq_cd *cd);
>> bool ice_is_pf_c827(struct ice_hw *hw);
>> +bool ice_is_phy_rclk_in_netlist(struct ice_hw *hw);
>> +bool ice_is_clock_mux_in_netlist(struct ice_hw *hw);
>> +bool ice_is_cgu_in_netlist(struct ice_hw *hw);
>> bool ice_is_gps_in_netlist(struct ice_hw *hw);
>> int
>> -ice_find_netlist_node(struct ice_hw *hw, u8 node_type_ctx, u8
>> node_part_number,
>> - u16 *node_handle);
>> -int
>> ice_aq_get_netlist_node(struct ice_hw *hw, struct ice_aqc_get_link_topo
>> *cmd,
>> u8 *node_part_number, u16 *node_handle);
>> int
>> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c
>> b/drivers/net/ethernet/intel/ice/ice_lib.c
>> index b96fbf76ca6d..6e70d678f323 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
>> @@ -3995,14 +3995,14 @@ void ice_init_feature_support(struct ice_pf *pf)
>> case ICE_DEV_ID_E810_XXV_QSFP:
>> case ICE_DEV_ID_E810_XXV_SFP:
>> ice_set_feature_support(pf, ICE_F_DSCP);
>> - if (ice_is_phy_rclk_present(&pf->hw))
>> + if (ice_is_phy_rclk_in_netlist(&pf->hw))
>> ice_set_feature_support(pf, ICE_F_PHY_RCLK);
>> /* If we don't own the timer - don't enable other caps */
>> if (!ice_pf_src_tmr_owned(pf))
>> break;
>> - if (ice_is_cgu_present(&pf->hw))
>> + if (ice_is_cgu_in_netlist(&pf->hw))
>> ice_set_feature_support(pf, ICE_F_CGU);
>> - if (ice_is_clock_mux_present_e810t(&pf->hw))
>> + if (ice_is_clock_mux_in_netlist(&pf->hw))
>> ice_set_feature_support(pf, ICE_F_SMA_CTRL);
>> if (ice_gnss_is_gps_present(&pf->hw))
>> ice_set_feature_support(pf, ICE_F_GNSS);
>> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
>> b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
>> index 5619644d5da7..882dfdad0021 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
>> @@ -3556,45 +3556,6 @@ int ice_clear_phy_tstamp(struct ice_hw *hw, u8 block,
>> u8 idx)
>> }
>> }
>>
>> -/**
>> - * ice_is_phy_rclk_present - check recovered clk presence
>> - * @hw: pointer to the hw struct
>> - *
>> - * Check if the PHY Recovered Clock device is present in the netlist
>> - * Return:
>> - * * true - device found in netlist
>> - * * false - device not found
>> - */
>> -bool ice_is_phy_rclk_present(struct ice_hw *hw)
>> -{
>> - if (ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL,
>> - ICE_AQC_GET_LINK_TOPO_NODE_NR_C827, NULL) &&
>> - ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL,
>> - ICE_AQC_GET_LINK_TOPO_NODE_NR_E822_PHY, NULL))
>> - return false;
>> -
>> - return true;
>> -}
>> -
>> -/**
>> - * ice_is_clock_mux_present_e810t
>> - * @hw: pointer to the hw struct
>> - *
>> - * Check if the Clock Multiplexer device is present in the netlist
>> - * Return:
>> - * * true - device found in netlist
>> - * * false - device not found
>> - */
>> -bool ice_is_clock_mux_present_e810t(struct ice_hw *hw)
>> -{
>> - if (ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_MUX,
>> - ICE_AQC_GET_LINK_TOPO_NODE_NR_GEN_CLK_MUX,
>> - NULL))
>> - return false;
>> -
>> - return true;
>> -}
>> -
>> /**
>> * ice_get_pf_c827_idx - find and return the C827 index for the current pf
>> * @hw: pointer to the hw struct
>> @@ -3708,33 +3669,6 @@ int ice_get_phy_tx_tstamp_ready(struct ice_hw *hw, u8
>> block, u64 *tstamp_ready)
>> }
>> }
>>
>> -/**
>> - * ice_is_cgu_present - check for CGU presence
>> - * @hw: pointer to the hw struct
>> - *
>> - * Check if the Clock Generation Unit (CGU) device is present in the netlist
>> - * Return:
>> - * * true - cgu is present
>> - * * false - cgu is not present
>> - */
>> -bool ice_is_cgu_present(struct ice_hw *hw)
>> -{
>> - if (!ice_find_netlist_node(hw, ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL,
>> - ICE_AQC_GET_LINK_TOPO_NODE_NR_ZL30632_80032,
>> - NULL)) {
>> - hw->cgu_part_number =
>> ICE_AQC_GET_LINK_TOPO_NODE_NR_ZL30632_80032;
>> - return true;
>> - } else if (!ice_find_netlist_node(hw,
>> - ICE_AQC_LINK_TOPO_NODE_TYPE_CLK_CTRL,
>> -
>> ICE_AQC_GET_LINK_TOPO_NODE_NR_SI5383_5384,
>> - NULL)) {
>> - hw->cgu_part_number = ICE_AQC_GET_LINK_TOPO_NODE_NR_SI5383_5384;
>> - return true;
>> - }
>> -
>> - return false;
>> -}
>> -
>> /**
>> * ice_cgu_get_pin_desc_e823 - get pin description array
>> * @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 6f277e7b06b9..18a993134826 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
>> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
>> @@ -271,10 +271,7 @@ int ice_read_sma_ctrl_e810t(struct ice_hw *hw, u8
>> *data);
>> int ice_write_sma_ctrl_e810t(struct ice_hw *hw, u8 data);
>> int ice_read_pca9575_reg_e810t(struct ice_hw *hw, u8 offset, u8 *data);
>> bool ice_is_pca9575_present(struct ice_hw *hw);
>> -bool ice_is_phy_rclk_present(struct ice_hw *hw);
>> -bool ice_is_clock_mux_present_e810t(struct ice_hw *hw);
>> int ice_get_pf_c827_idx(struct ice_hw *hw, u8 *idx);
>> -bool ice_is_cgu_present(struct ice_hw *hw);
>> enum dpll_pin_type ice_cgu_get_pin_type(struct ice_hw *hw, u8 pin, bool
>> input);
>> struct dpll_pin_frequency *
>> ice_cgu_get_pin_freq_supp(struct ice_hw *hw, u8 pin, bool input, u8 *num);
>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
