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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZM/Y/PjPVNxbwLOL@vergenet.net>
Date: Sun, 6 Aug 2023 19:31:40 +0200
From: Simon Horman <horms@...nel.org>
To: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
Cc: Jakub Kicinski <kuba@...nel.org>, Jiri Pirko <jiri@...nulli.us>,
	Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>,
	Jonathan Lemon <jonathan.lemon@...il.com>,
	Paolo Abeni <pabeni@...hat.com>,
	Milena Olech <milena.olech@...el.com>,
	Michal Michalik <michal.michalik@...el.com>,
	linux-arm-kernel@...ts.infradead.org, poros@...hat.com,
	mschmidt@...hat.com, netdev@...r.kernel.org,
	linux-clk@...r.kernel.org, Bart Van Assche <bvanassche@....org>,
	intel-wired-lan@...ts.osuosl.org
Subject: Re: [PATCH net-next v2 6/9] ice: add admin commands to access cgu
 configuration

On Fri, Aug 04, 2023 at 08:04:51PM +0100, Vadim Fedorenko wrote:
> From: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.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@...el.com>
> Signed-off-by: Vadim Fedorenko <vadim.fedorenko@...ux.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.

> +	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);

> +		*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

> + * @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