Re: [PATCH net-next 3/5] dpll: zl3073x: add ref sync and output clock type helpers

[Petr Oros <poros@redhat.com>]

> Add ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR and ZL_REF_SYNC_CTRL_PAIR
> register definitions.
>
> Add inline helpers to get and set the sync control mode and sync pair
> fields of the reference sync control register:
>
>    zl3073x_ref_sync_mode_get/set() - ZL_REF_SYNC_CTRL_MODE field
>    zl3073x_ref_sync_pair_get/set() - ZL_REF_SYNC_CTRL_PAIR field
>
> Add inline helpers to get and set the clock type field of the output
> mode register:
>
>    zl3073x_out_clock_type_get/set() - ZL_OUTPUT_MODE_CLOCK_TYPE field
>
> Convert existing esync callbacks to use the new helpers.
>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
>   drivers/dpll/zl3073x/dpll.c | 24 ++++++++-----------
>   drivers/dpll/zl3073x/out.h  | 22 ++++++++++++++++++
>   drivers/dpll/zl3073x/ref.h  | 46 +++++++++++++++++++++++++++++++++++++
>   drivers/dpll/zl3073x/regs.h |  2 ++
>   4 files changed, 80 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
> index 79ef62d69a32d..276f0a92db0b1 100644
> --- a/drivers/dpll/zl3073x/dpll.c
> +++ b/drivers/dpll/zl3073x/dpll.c
> @@ -137,7 +137,7 @@ zl3073x_dpll_input_pin_esync_get(const struct dpll_pin *dpll_pin,
>   	esync->range = esync_freq_ranges;
>   	esync->range_num = ARRAY_SIZE(esync_freq_ranges);
>   
> -	switch (FIELD_GET(ZL_REF_SYNC_CTRL_MODE, ref->sync_ctrl)) {
> +	switch (zl3073x_ref_sync_mode_get(ref)) {
>   	case ZL_REF_SYNC_CTRL_MODE_50_50_ESYNC_25_75:
>   		esync->freq = ref->esync_n_div == ZL_REF_ESYNC_DIV_1HZ ? 1 : 0;
>   		esync->pulse = 25;
> @@ -173,8 +173,7 @@ zl3073x_dpll_input_pin_esync_set(const struct dpll_pin *dpll_pin,
>   	else
>   		sync_mode = ZL_REF_SYNC_CTRL_MODE_50_50_ESYNC_25_75;
>   
> -	ref.sync_ctrl &= ~ZL_REF_SYNC_CTRL_MODE;
> -	ref.sync_ctrl |= FIELD_PREP(ZL_REF_SYNC_CTRL_MODE, sync_mode);
> +	zl3073x_ref_sync_mode_set(&ref, sync_mode);
>   
>   	if (freq) {
>   		/* 1 Hz is only supported frequency now */
> @@ -578,7 +577,7 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
>   	const struct zl3073x_synth *synth;
>   	const struct zl3073x_out *out;
>   	u32 synth_freq, out_freq;
> -	u8 clock_type, out_id;
> +	u8 out_id;
>   
>   	out_id = zl3073x_output_pin_out_get(pin->id);
>   	out = zl3073x_out_state_get(zldev, out_id);
> @@ -601,8 +600,7 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
>   	esync->range = esync_freq_ranges;
>   	esync->range_num = ARRAY_SIZE(esync_freq_ranges);
>   
> -	clock_type = FIELD_GET(ZL_OUTPUT_MODE_CLOCK_TYPE, out->mode);
> -	if (clock_type != ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC) {
> +	if (zl3073x_out_clock_type_get(out) != ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC) {
>   		/* No need to read esync data if it is not enabled */
>   		esync->freq = 0;
>   		esync->pulse = 0;
> @@ -635,8 +633,8 @@ zl3073x_dpll_output_pin_esync_set(const struct dpll_pin *dpll_pin,
>   	struct zl3073x_dpll_pin *pin = pin_priv;
>   	const struct zl3073x_synth *synth;
>   	struct zl3073x_out out;
> -	u8 clock_type, out_id;
>   	u32 synth_freq;
> +	u8 out_id;
>   
>   	out_id = zl3073x_output_pin_out_get(pin->id);
>   	out = *zl3073x_out_state_get(zldev, out_id);
> @@ -648,15 +646,13 @@ zl3073x_dpll_output_pin_esync_set(const struct dpll_pin *dpll_pin,
>   	if (zl3073x_out_is_ndiv(&out))
>   		return -EOPNOTSUPP;
>   
> -	/* Select clock type */
> +	/* Update clock type in output mode */
>   	if (freq)
> -		clock_type = ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC;
> +		zl3073x_out_clock_type_set(&out,
> +					   ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC);
>   	else
> -		clock_type = ZL_OUTPUT_MODE_CLOCK_TYPE_NORMAL;
> -
> -	/* Update clock type in output mode */
> -	out.mode &= ~ZL_OUTPUT_MODE_CLOCK_TYPE;
> -	out.mode |= FIELD_PREP(ZL_OUTPUT_MODE_CLOCK_TYPE, clock_type);
> +		zl3073x_out_clock_type_set(&out,
> +					   ZL_OUTPUT_MODE_CLOCK_TYPE_NORMAL);
>   
>   	/* If esync is being disabled just write mailbox and finish */
>   	if (!freq)
> diff --git a/drivers/dpll/zl3073x/out.h b/drivers/dpll/zl3073x/out.h
> index edf40432bba5f..660889c57bffa 100644
> --- a/drivers/dpll/zl3073x/out.h
> +++ b/drivers/dpll/zl3073x/out.h
> @@ -42,6 +42,28 @@ const struct zl3073x_out *zl3073x_out_state_get(struct zl3073x_dev *zldev,
>   int zl3073x_out_state_set(struct zl3073x_dev *zldev, u8 index,
>   			  const struct zl3073x_out *out);
>   
> +/**
> + * zl3073x_out_clock_type_get - get output clock type
> + * @out: pointer to out state
> + *
> + * Return: clock type of given output (ZL_OUTPUT_MODE_CLOCK_TYPE_*)
> + */
> +static inline u8 zl3073x_out_clock_type_get(const struct zl3073x_out *out)
> +{
> +	return FIELD_GET(ZL_OUTPUT_MODE_CLOCK_TYPE, out->mode);
> +}
> +
> +/**
> + * zl3073x_out_clock_type_set - set output clock type
> + * @out: pointer to out state
> + * @type: clock type (ZL_OUTPUT_MODE_CLOCK_TYPE_*)
> + */
> +static inline void
> +zl3073x_out_clock_type_set(struct zl3073x_out *out, u8 type)
> +{
> +	FIELD_MODIFY(ZL_OUTPUT_MODE_CLOCK_TYPE, &out->mode, type);
> +}
> +
>   /**
>    * zl3073x_out_signal_format_get - get output signal format
>    * @out: pointer to out state
> diff --git a/drivers/dpll/zl3073x/ref.h b/drivers/dpll/zl3073x/ref.h
> index 06d8d4d97ea26..09fab97a71d7e 100644
> --- a/drivers/dpll/zl3073x/ref.h
> +++ b/drivers/dpll/zl3073x/ref.h
> @@ -106,6 +106,52 @@ zl3073x_ref_freq_set(struct zl3073x_ref *ref, u32 freq)
>   	return 0;
>   }
>   
> +/**
> + * zl3073x_ref_sync_mode_get - get sync control mode
> + * @ref: pointer to ref state
> + *
> + * Return: sync control mode (ZL_REF_SYNC_CTRL_MODE_*)
> + */
> +static inline u8
> +zl3073x_ref_sync_mode_get(const struct zl3073x_ref *ref)
> +{
> +	return FIELD_GET(ZL_REF_SYNC_CTRL_MODE, ref->sync_ctrl);
> +}
> +
> +/**
> + * zl3073x_ref_sync_mode_set - set sync control mode
> + * @ref: pointer to ref state
> + * @mode: sync control mode (ZL_REF_SYNC_CTRL_MODE_*)
> + */
> +static inline void
> +zl3073x_ref_sync_mode_set(struct zl3073x_ref *ref, u8 mode)
> +{
> +	FIELD_MODIFY(ZL_REF_SYNC_CTRL_MODE, &ref->sync_ctrl, mode);
> +}
> +
> +/**
> + * zl3073x_ref_sync_pair_get - get sync pair reference index
> + * @ref: pointer to ref state
> + *
> + * Return: paired reference index
> + */
> +static inline u8
> +zl3073x_ref_sync_pair_get(const struct zl3073x_ref *ref)
> +{
> +	return FIELD_GET(ZL_REF_SYNC_CTRL_PAIR, ref->sync_ctrl);
> +}
> +
> +/**
> + * zl3073x_ref_sync_pair_set - set sync pair reference index
> + * @ref: pointer to ref state
> + * @pair: paired reference index
> + */
> +static inline void
> +zl3073x_ref_sync_pair_set(struct zl3073x_ref *ref, u8 pair)
> +{
> +	FIELD_MODIFY(ZL_REF_SYNC_CTRL_PAIR, &ref->sync_ctrl, pair);
> +}
> +
>   /**
>    * zl3073x_ref_is_diff - check if the given input reference is differential
>    * @ref: pointer to ref state
> diff --git a/drivers/dpll/zl3073x/regs.h b/drivers/dpll/zl3073x/regs.h
> index 5ae50cb761a97..d425dc67250fe 100644
> --- a/drivers/dpll/zl3073x/regs.h
> +++ b/drivers/dpll/zl3073x/regs.h
> @@ -213,7 +213,9 @@
>   #define ZL_REG_REF_SYNC_CTRL			ZL_REG(10, 0x2e, 1)
>   #define ZL_REF_SYNC_CTRL_MODE			GENMASK(2, 0)
>   #define ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR_OFF	0
> +#define ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR	1
>   #define ZL_REF_SYNC_CTRL_MODE_50_50_ESYNC_25_75	2
> +#define ZL_REF_SYNC_CTRL_PAIR			GENMASK(7, 4)
>   
>   #define ZL_REG_REF_ESYNC_DIV			ZL_REG(10, 0x30, 4)
>   #define ZL_REF_ESYNC_DIV_1HZ			0

LGTM.

Reviewed-by: Petr Oros <poros@redhat.com>