[PATCH net-next v2 6/9] ice: add admin commands to access cgu configuration

[Kubalewski, Arkadiusz]

>From: Simon Horman <horms at kernel.org>
>Sent: Sunday, August 6, 2023 7:32 PM
>
>On Fri, Aug 04, 2023 at 08:04:51PM +0100, Vadim Fedorenko wrote:
>> From: Arkadiusz Kubalewski <arkadiusz.kubalewski at intel.com>
>>
>> Add firmware admin command to access clock generation unit
>> configuration, it is required to enable Extended PTP and SyncE features
>> in the driver.
>> Add definitions of possible hardware variations of input and output pins
>> related to clock generation unit and functions to access the data.
>>
>> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski at intel.com>
>> Signed-off-by: Vadim Fedorenko <vadim.fedorenko at linux.dev>
>
>Hi Arkadiusz and Vadim,
>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
>b/drivers/net/ethernet/intel/ice/ice_common.c
>
>...
>
>> +/**
>> + * ice_aq_get_cgu_dpll_status - get dpll status
>> + * @hw: pointer to the HW struct
>> + * @dpll_num: DPLL index
>> + * @ref_state: Reference clock state
>> + * @config: current DPLL config
>> + * @dpll_state: current DPLL state
>> + * @phase_offset: Phase offset in ns
>> + * @eec_mode: EEC_mode
>> + *
>> + * Get CGU DPLL status (0x0C66)
>> + * Return: 0 on success or negative value on failure.
>> + */
>> +int
>> +ice_aq_get_cgu_dpll_status(struct ice_hw *hw, u8 dpll_num, u8
>>*ref_state,
>> +			   u8 *dpll_state, u8 *config, s64 *phase_offset,
>> +			   u8 *eec_mode)
>> +{
>> +	struct ice_aqc_get_cgu_dpll_status *cmd;
>> +	const s64 NSEC_PER_PSEC = 1000LL;
>
>Probably this should be in lower case, or an (upper case) #define.
>In the case of the latter it should probably be moved outside of the
>function.

Hi Simon,

Sure, will fix.

>
>> +	struct ice_aq_desc desc;
>> +	int status;
>> +
>> +	ice_fill_dflt_direct_cmd_desc(&desc,
>>ice_aqc_opc_get_cgu_dpll_status);
>> +	cmd = &desc.params.get_cgu_dpll_status;
>> +	cmd->dpll_num = dpll_num;
>> +
>> +	status = ice_aq_send_cmd(hw, &desc, NULL, 0, NULL);
>> +	if (!status) {
>> +		*ref_state = cmd->ref_state;
>> +		*dpll_state = cmd->dpll_state;
>> +		*config = cmd->config;
>> +		*phase_offset = le32_to_cpu(cmd->phase_offset_h);
>> +		*phase_offset <<= 32;
>> +		*phase_offset += le32_to_cpu(cmd->phase_offset_l);
>> +		*phase_offset = sign_extend64(*phase_offset, 47) /
>> +			NSEC_PER_PSEC;
>
>This causes a build failure on x86_32.
>
>  ERROR: modpost: "__divdi3" [drivers/net/ethernet/intel/ice/ice.ko]
>undefined!
>
>Possibly you want (please do check for yourself):
>
>		*phase_offset = div64_s64(sign_extend64(*phase_offset, 47),
>					  NSEC_PER_PSEC);
>

Yes, makes sense, thanks for catching this, will fix.

>> +		*eec_mode = cmd->eec_mode;
>> +	}
>> +
>> +	return status;
>> +}
>> +
>> +/**
>> + * ice_aq_set_cgu_dpll_config - set dpll config
>> + * @hw: pointer to the HW struct
>> + * @dpll_num: DPLL index
>> + * @ref_state: Reference clock state
>> + * @config: DPLL config
>> + * @eec_mode: EEC mode
>> + *
>> + * Set CGU DPLL config (0x0C67)
>> + * Return: 0 on success or negative value on failure.
>> + */
>> +int
>> +ice_aq_set_cgu_dpll_config(struct ice_hw *hw, u8 dpll_num, u8 ref_state,
>> +			   u8 config, u8 eec_mode)
>> +{
>> +	struct ice_aqc_set_cgu_dpll_config *cmd;
>> +	struct ice_aq_desc desc;
>> +
>> +	ice_fill_dflt_direct_cmd_desc(&desc,
>>ice_aqc_opc_set_cgu_dpll_config);
>> +	cmd = &desc.params.set_cgu_dpll_config;
>> +	cmd->dpll_num = dpll_num;
>> +	cmd->ref_state = ref_state;
>> +	cmd->config = config;
>> +	cmd->eec_mode = eec_mode;
>> +
>> +	return ice_aq_send_cmd(hw, &desc, NULL, 0, NULL);
>> +}
>> +
>> +/**
>> + * ice_aq_set_cgu_ref_prio - set input refernce priority
>
>nit: refernce -> reference
>

Sure, will fix.

Thank you!
Arkadiusz

>> + * @hw: pointer to the HW struct
>> + * @dpll_num: DPLL index
>> + * @ref_idx: Reference pin index
>> + * @ref_priority: Reference input priority
>> + *
>> + * Set CGU reference priority (0x0C68)
>> + * Return: 0 on success or negative value on failure.
>> + */
>
>...
>
>--
>pw-bot: changes-requested