netdev - Re: [PATCH net-next v3 4/5] ice: dpll: implement phase related callbacks

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: <ZR/+noRCdnsy6QJo@kernel.org>
Date: Fri, 6 Oct 2023 14:33:34 +0200
From: Simon Horman <horms@...nel.org>
To: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>
Cc: netdev@...r.kernel.org, vadim.fedorenko@...ux.dev, jiri@...nulli.us,
	corbet@....net, davem@...emloft.net, kuba@...nel.org,
	pabeni@...hat.com, jesse.brandeburg@...el.com,
	anthony.l.nguyen@...el.com, linux-doc@...r.kernel.org,
	intel-wired-lan@...ts.osuosl.org
Subject: Re: [PATCH net-next v3 4/5] ice: dpll: implement phase related
 callbacks

On Fri, Oct 06, 2023 at 01:41:00PM +0200, Arkadiusz Kubalewski wrote:
> Implement new callback ops related to measurment and adjustment of
> signal phase for pin-dpll in ice driver.
> 
> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>

Hi Arkadiusz,

some minor feedback from my side.

If you do end up re-spinning the series, please consider
running checkpatch.pl --codespell.

> ---
>  drivers/net/ethernet/intel/ice/ice_dpll.c | 224 +++++++++++++++++++++-
>  drivers/net/ethernet/intel/ice/ice_dpll.h |  10 +-
>  2 files changed, 230 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c

...

> +/**
> + * ice_dpll_phase_offset_get - callback for get dpll phase shift value
> + * @pin: pointer to a pin
> + * @pin_priv: private data pointer passed on pin registration
> + * @dpll: registered dpll pointer
> + * @dpll_priv: private data pointer passed on dpll registration
> + * @phase_adjust: on success holds pin phase_adjust value

nit: The parameter is called phase_offset, not phase_adjust in the code below

> + * @extack: error reporting
> + *
> + * Dpll subsystem callback. Handler for getting phase shift value between
> + * dpll's input and output.
> + *
> + * Context: Acquires pf->dplls.lock
> + * Return:
> + * * 0 - success
> + * * negative - error
> + */
> +static int
> +ice_dpll_phase_offset_get(const struct dpll_pin *pin, void *pin_priv,
> +			  const struct dpll_device *dpll, void *dpll_priv,
> +			  s64 *phase_offset, struct netlink_ext_ack *extack)
> +{
> +	struct ice_dpll *d = dpll_priv;
> +	struct ice_pf *pf = d->pf;
> +
> +	mutex_lock(&pf->dplls.lock);
> +	if (d->active_input == pin)
> +		*phase_offset = d->phase_offset * ICE_DPLL_PHASE_OFFSET_FACTOR;
> +	else
> +		*phase_offset = 0;
> +	mutex_unlock(&pf->dplls.lock);
> +
> +	return 0;
> +}
> +
>  /**
>   * ice_dpll_rclk_state_on_pin_set - set a state on rclk pin
>   * @pin: pointer to a pin

...

> @@ -1656,6 +1867,15 @@ ice_dpll_init_info_direct_pins(struct ice_pf *pf,
>  				return ret;
>  			pins[i].prop.capabilities |=
>  				DPLL_PIN_CAPABILITIES_PRIORITY_CAN_CHANGE;
> +			pins[i].prop.phase_range.min =
> +				pf->dplls.input_phase_adj_max;
> +			pins[i].prop.phase_range.max =
> +				-pf->dplls.input_phase_adj_max;
> +		} else {
> +			pins[i].prop.phase_range.min =
> +				pf->dplls.output_phase_adj_max,

nit: It probably doesn't make any difference, but perhaps ',' should be ';'.

As flagged by clang-16 with -Wcomma

> +			pins[i].prop.phase_range.max =
> +				-pf->dplls.output_phase_adj_max;
>  		}
>  		pins[i].prop.capabilities |=
>  			DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE;

...