[PATCH net-next 0/5] dpll: zl3073x: add ref-sync pair support

[PATCH net-next 0/5] dpll: zl3073x: add ref-sync pair support

[Ivan Vecera]

This series adds Reference-Sync pair support to the ZL3073x DPLL driver.
A Ref-Sync pair consists of a clock reference and a low-frequency sync
signal (e.g. 1 PPS) where the DPLL locks to the clock reference but
phase-aligns to the sync reference.

Patches 1-3 are preparatory cleanups and helper additions:
- Clean up esync get/set callbacks with early returns and use the
  zl3073x_out_is_ndiv() helper
- Convert open-coded clear-and-set bitfield patterns to FIELD_MODIFY()
- Add ref sync control and output clock type accessor helpers

Patch 4 adds the 'ref-sync-sources' phandle-array property to the
dpll-pin device tree binding schema and updates the ZL3073x binding
examples.

Patch 5 implements the driver support:
- ref_sync_get/set callbacks with frequency validation
- Automatic sync source exclusion from reference selection
- Device tree based ref-sync pair registration

Tested and verified on Microchip EDS2 (pcb8385) development board.

Ivan Vecera (5):
  dpll: zl3073x: clean up esync get/set and use zl3073x_out_is_ndiv()
  dpll: zl3073x: use FIELD_MODIFY() for clear-and-set patterns
  dpll: zl3073x: add ref sync and output clock type helpers
  dt-bindings: dpll: add ref-sync-sources property
  dpll: zl3073x: add ref-sync pair support

 .../devicetree/bindings/dpll/dpll-pin.yaml    |  11 +
 .../bindings/dpll/microchip,zl30731.yaml      |  30 +-
 drivers/dpll/zl3073x/chan.h                   |  17 +-
 drivers/dpll/zl3073x/core.c                   |   3 +-
 drivers/dpll/zl3073x/dpll.c                   | 295 ++++++++++++++----
 drivers/dpll/zl3073x/flash.c                  |   3 +-
 drivers/dpll/zl3073x/out.h                    |  22 ++
 drivers/dpll/zl3073x/ref.h                    |  46 +++
 drivers/dpll/zl3073x/regs.h                   |   2 +
 9 files changed, 348 insertions(+), 81 deletions(-)

-- 
2.52.0

[PATCH net-next 1/5] dpll: zl3073x: clean up esync get/set and use zl3073x_out_is_ndiv()

[Ivan Vecera]

Return -EOPNOTSUPP early in esync_get callbacks when esync is not
supported instead of conditionally populating the range at the end.
This simplifies the control flow by removing the finish label/goto
in the output variant and the conditional range assignment in both
input and output variants.

Replace open-coded N-div signal format switch statements with
zl3073x_out_is_ndiv() helper in esync_get, esync_set and
frequency_set callbacks.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/dpll.c | 64 ++++++++++++-------------------------
 1 file changed, 20 insertions(+), 44 deletions(-)

diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
index a29f606318f6d..79ef62d69a32d 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -131,6 +131,12 @@ zl3073x_dpll_input_pin_esync_get(const struct dpll_pin *dpll_pin,
 	ref_id = zl3073x_input_pin_ref_get(pin->id);
 	ref = zl3073x_ref_state_get(zldev, ref_id);
 
+	if (!pin->esync_control || zl3073x_ref_freq_get(ref) == 1)
+		return -EOPNOTSUPP;
+
+	esync->range = esync_freq_ranges;
+	esync->range_num = ARRAY_SIZE(esync_freq_ranges);
+
 	switch (FIELD_GET(ZL_REF_SYNC_CTRL_MODE, ref->sync_ctrl)) {
 	case ZL_REF_SYNC_CTRL_MODE_50_50_ESYNC_25_75:
 		esync->freq = ref->esync_n_div == ZL_REF_ESYNC_DIV_1HZ ? 1 : 0;
@@ -142,17 +148,6 @@ zl3073x_dpll_input_pin_esync_get(const struct dpll_pin *dpll_pin,
 		break;
 	}
 
-	/* If the pin supports esync control expose its range but only
-	 * if the current reference frequency is > 1 Hz.
-	 */
-	if (pin->esync_control && zl3073x_ref_freq_get(ref) > 1) {
-		esync->range = esync_freq_ranges;
-		esync->range_num = ARRAY_SIZE(esync_freq_ranges);
-	} else {
-		esync->range = NULL;
-		esync->range_num = 0;
-	}
-
 	return 0;
 }
 
@@ -582,8 +577,8 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
 	struct zl3073x_dpll_pin *pin = pin_priv;
 	const struct zl3073x_synth *synth;
 	const struct zl3073x_out *out;
+	u32 synth_freq, out_freq;
 	u8 clock_type, out_id;
-	u32 synth_freq;
 
 	out_id = zl3073x_output_pin_out_get(pin->id);
 	out = zl3073x_out_state_get(zldev, out_id);
@@ -592,17 +587,19 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
 	 * for N-division is also used for the esync divider so both cannot
 	 * be used.
 	 */
-	switch (zl3073x_out_signal_format_get(out)) {
-	case ZL_OUTPUT_MODE_SIGNAL_FORMAT_2_NDIV:
-	case ZL_OUTPUT_MODE_SIGNAL_FORMAT_2_NDIV_INV:
+	if (zl3073x_out_is_ndiv(out))
 		return -EOPNOTSUPP;
-	default:
-		break;
-	}
 
 	/* Get attached synth frequency */
 	synth = zl3073x_synth_state_get(zldev, zl3073x_out_synth_get(out));
 	synth_freq = zl3073x_synth_freq_get(synth);
+	out_freq = synth_freq / out->div;
+
+	if (!pin->esync_control || out_freq == 1)
+		return -EOPNOTSUPP;
+
+	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) {
@@ -610,11 +607,11 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
 		esync->freq = 0;
 		esync->pulse = 0;
 
-		goto finish;
+		return 0;
 	}
 
 	/* Compute esync frequency */
-	esync->freq = synth_freq / out->div / out->esync_n_period;
+	esync->freq = out_freq / out->esync_n_period;
 
 	/* By comparing the esync_pulse_width to the half of the pulse width
 	 * the esync pulse percentage can be determined.
@@ -623,18 +620,6 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
 	 */
 	esync->pulse = (50 * out->esync_n_width) / out->div;
 
-finish:
-	/* Set supported esync ranges if the pin supports esync control and
-	 * if the output frequency is > 1 Hz.
-	 */
-	if (pin->esync_control && (synth_freq / out->div) > 1) {
-		esync->range = esync_freq_ranges;
-		esync->range_num = ARRAY_SIZE(esync_freq_ranges);
-	} else {
-		esync->range = NULL;
-		esync->range_num = 0;
-	}
-
 	return 0;
 }
 
@@ -660,13 +645,8 @@ zl3073x_dpll_output_pin_esync_set(const struct dpll_pin *dpll_pin,
 	 * for N-division is also used for the esync divider so both cannot
 	 * be used.
 	 */
-	switch (zl3073x_out_signal_format_get(&out)) {
-	case ZL_OUTPUT_MODE_SIGNAL_FORMAT_2_NDIV:
-	case ZL_OUTPUT_MODE_SIGNAL_FORMAT_2_NDIV_INV:
+	if (zl3073x_out_is_ndiv(&out))
 		return -EOPNOTSUPP;
-	default:
-		break;
-	}
 
 	/* Select clock type */
 	if (freq)
@@ -728,9 +708,9 @@ zl3073x_dpll_output_pin_frequency_set(const struct dpll_pin *dpll_pin,
 	struct zl3073x_dev *zldev = zldpll->dev;
 	struct zl3073x_dpll_pin *pin = pin_priv;
 	const struct zl3073x_synth *synth;
-	u8 out_id, signal_format;
 	u32 new_div, synth_freq;
 	struct zl3073x_out out;
+	u8 out_id;
 
 	out_id = zl3073x_output_pin_out_get(pin->id);
 	out = *zl3073x_out_state_get(zldev, out_id);
@@ -740,12 +720,8 @@ zl3073x_dpll_output_pin_frequency_set(const struct dpll_pin *dpll_pin,
 	synth_freq = zl3073x_synth_freq_get(synth);
 	new_div = synth_freq / (u32)frequency;
 
-	/* Get used signal format for the given output */
-	signal_format = zl3073x_out_signal_format_get(&out);
-
 	/* Check signal format */
-	if (signal_format != ZL_OUTPUT_MODE_SIGNAL_FORMAT_2_NDIV &&
-	    signal_format != ZL_OUTPUT_MODE_SIGNAL_FORMAT_2_NDIV_INV) {
+	if (!zl3073x_out_is_ndiv(&out)) {
 		/* For non N-divided signal formats the frequency is computed
 		 * as division of synth frequency and output divisor.
 		 */
-- 
2.52.0

Re: [PATCH net-next 1/5] dpll: zl3073x: clean up esync get/set and use zl3073x_out_is_ndiv()

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
dpll: zl3073x: clean up esync get/set and use zl3073x_out_is_ndiv()

Return -EOPNOTSUPP early in esync_get callbacks when esync is not
supported instead of conditionally populating the range at the end.
This simplifies the control flow by removing the finish label/goto
in the output variant and the conditional range assignment in both
input and output variants.

Replace open-coded N-div signal format switch statements with
zl3073x_out_is_ndiv() helper in esync_get, esync_set and
frequency_set callbacks.

> diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
> index a29f606318f6..79ef62d69a32 100644
> --- a/drivers/dpll/zl3073x/dpll.c
> +++ b/drivers/dpll/zl3073x/dpll.c
> @@ -131,6 +131,12 @@ zl3073x_dpll_input_pin_esync_get(const struct dpll_pin *dpll_pin,
>  	ref_id = zl3073x_input_pin_ref_get(pin->id);
>  	ref = zl3073x_ref_state_get(zldev, ref_id);
>
> +	if (!pin->esync_control || zl3073x_ref_freq_get(ref) == 1)
> +		return -EOPNOTSUPP;
> +
> +	esync->range = esync_freq_ranges;
> +	esync->range_num = ARRAY_SIZE(esync_freq_ranges);
> +
>  	switch (FIELD_GET(ZL_REF_SYNC_CTRL_MODE, ref->sync_ctrl)) {
>  	case ZL_REF_SYNC_CTRL_MODE_50_50_ESYNC_25_75:
>  		esync->freq = ref->esync_n_div == ZL_REF_ESYNC_DIV_1HZ ? 1 : 0;
> @@ -142,17 +148,6 @@ zl3073x_dpll_input_pin_esync_get(const struct dpll_pin *dpll_pin,
>  		break;
>  	}
>
> -	/* If the pin supports esync control expose its range but only
> -	 * if the current reference frequency is > 1 Hz.
> -	 */
> -	if (pin->esync_control && zl3073x_ref_freq_get(ref) > 1) {
> -		esync->range = esync_freq_ranges;
> -		esync->range_num = ARRAY_SIZE(esync_freq_ranges);
> -	} else {
> -		esync->range = NULL;
> -		esync->range_num = 0;
> -	}
> -
>  	return 0;
>  }

The refactoring changes the behavior when the reference frequency is 0 Hz.

The old code checked 'zl3073x_ref_freq_get(ref) > 1' which excluded both
freq==0 and freq==1 from having the esync range exposed. The new code checks
'zl3073x_ref_freq_get(ref) == 1' which only excludes freq==1.

When freq==0 (which can happen when freq_base, freq_mult, or freq_ratio_m
is 0), the old code would return 0 with esync->range=NULL and
esync->range_num=0, while the new code returns 0 with
esync->range=esync_freq_ranges and esync->range_num=1.

The removed comment documented the original intent: "if the current reference
frequency is > 1 Hz". Should the new check be 'zl3073x_ref_freq_get(ref) <= 1'
to preserve the original behavior for 0 Hz frequencies?

>
> @@ -582,8 +577,8 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
>  	struct zl3073x_dpll_pin *pin = pin_priv;
>  	const struct zl3073x_synth *synth;
>  	const struct zl3073x_out *out;
> +	u32 synth_freq, out_freq;
>  	u8 clock_type, out_id;
> -	u32 synth_freq;
>
>  	out_id = zl3073x_output_pin_out_get(pin->id);
>  	out = zl3073x_out_state_get(zldev, out_id);
> @@ -592,17 +587,19 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
>  	 * for N-division is also used for the esync divider so both cannot
>  	 * be used.
>  	 */
> -	switch (zl3073x_out_signal_format_get(out)) {
> -	case ZL_OUTPUT_MODE_SIGNAL_FORMAT_2_NDIV:
> -	case ZL_OUTPUT_MODE_SIGNAL_FORMAT_2_NDIV_INV:
> +	if (zl3073x_out_is_ndiv(out))
>  		return -EOPNOTSUPP;
> -	default:
> -		break;
> -	}
>
>  	/* Get attached synth frequency */
>  	synth = zl3073x_synth_state_get(zldev, zl3073x_out_synth_get(out));
>  	synth_freq = zl3073x_synth_freq_get(synth);
> +	out_freq = synth_freq / out->div;
> +
> +	if (!pin->esync_control || out_freq == 1)
> +		return -EOPNOTSUPP;
> +
> +	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) {
> @@ -610,11 +607,11 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
>  		esync->freq = 0;
>  		esync->pulse = 0;
>
> -		goto finish;
> +		return 0;
>  	}
>
>  	/* Compute esync frequency */
> -	esync->freq = synth_freq / out->div / out->esync_n_period;
> +	esync->freq = out_freq / out->esync_n_period;
>
>  	/* By comparing the esync_pulse_width to the half of the pulse width
>  	 * the esync pulse percentage can be determined.
> @@ -623,18 +620,6 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
>  	 */
>  	esync->pulse = (50 * out->esync_n_width) / out->div;
>
> -finish:
> -	/* Set supported esync ranges if the pin supports esync control and
> -	 * if the output frequency is > 1 Hz.
> -	 */
> -	if (pin->esync_control && (synth_freq / out->div) > 1) {
> -		esync->range = esync_freq_ranges;
> -		esync->range_num = ARRAY_SIZE(esync_freq_ranges);
> -	} else {
> -		esync->range = NULL;
> -		esync->range_num = 0;
> -	}
> -
>  	return 0;
>  }

Similar to the input variant, this changes the behavior for 0 Hz output
frequencies.

The old code used '(synth_freq / out->div) > 1' to decide whether to expose
the esync range, which excluded both out_freq==0 and out_freq==1. The new
code uses 'out_freq == 1' for the early -EOPNOTSUPP return, only excluding
out_freq==1.

When out_freq==0 (which can happen when synth_freq is 0 or synth_freq <
out->div), the old code returned 0 with esync->range=NULL and
esync->range_num=0, while the new code returns 0 with
esync->range=esync_freq_ranges and esync->range_num=1.

The removed comment documented the intent: "if the output frequency is > 1
Hz". Should this also use 'out_freq <= 1' to maintain consistency with the
original behavior?

[ ... ]

Re: [PATCH net-next 1/5] dpll: zl3073x: clean up esync get/set and use zl3073x_out_is_ndiv()

[Ivan Vecera]

Hi Simon,

On 3/20/26 6:17 PM, Simon Horman wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> 
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> dpll: zl3073x: clean up esync get/set and use zl3073x_out_is_ndiv()
> 
> Return -EOPNOTSUPP early in esync_get callbacks when esync is not
> supported instead of conditionally populating the range at the end.
> This simplifies the control flow by removing the finish label/goto
> in the output variant and the conditional range assignment in both
> input and output variants.
> 
> Replace open-coded N-div signal format switch statements with
> zl3073x_out_is_ndiv() helper in esync_get, esync_set and
> frequency_set callbacks.
> 
>> diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
>> index a29f606318f6..79ef62d69a32 100644
>> --- a/drivers/dpll/zl3073x/dpll.c
>> +++ b/drivers/dpll/zl3073x/dpll.c
>> @@ -131,6 +131,12 @@ zl3073x_dpll_input_pin_esync_get(const struct dpll_pin *dpll_pin,
>>   	ref_id = zl3073x_input_pin_ref_get(pin->id);
>>   	ref = zl3073x_ref_state_get(zldev, ref_id);
>>
>> +	if (!pin->esync_control || zl3073x_ref_freq_get(ref) == 1)
>> +		return -EOPNOTSUPP;
>> +
>> +	esync->range = esync_freq_ranges;
>> +	esync->range_num = ARRAY_SIZE(esync_freq_ranges);
>> +
>>   	switch (FIELD_GET(ZL_REF_SYNC_CTRL_MODE, ref->sync_ctrl)) {
>>   	case ZL_REF_SYNC_CTRL_MODE_50_50_ESYNC_25_75:
>>   		esync->freq = ref->esync_n_div == ZL_REF_ESYNC_DIV_1HZ ? 1 : 0;
>> @@ -142,17 +148,6 @@ zl3073x_dpll_input_pin_esync_get(const struct dpll_pin *dpll_pin,
>>   		break;
>>   	}
>>
>> -	/* If the pin supports esync control expose its range but only
>> -	 * if the current reference frequency is > 1 Hz.
>> -	 */
>> -	if (pin->esync_control && zl3073x_ref_freq_get(ref) > 1) {
>> -		esync->range = esync_freq_ranges;
>> -		esync->range_num = ARRAY_SIZE(esync_freq_ranges);
>> -	} else {
>> -		esync->range = NULL;
>> -		esync->range_num = 0;
>> -	}
>> -
>>   	return 0;
>>   }
> 
> The refactoring changes the behavior when the reference frequency is 0 Hz.
> 
> The old code checked 'zl3073x_ref_freq_get(ref) > 1' which excluded both
> freq==0 and freq==1 from having the esync range exposed. The new code checks
> 'zl3073x_ref_freq_get(ref) == 1' which only excludes freq==1.
> 
> When freq==0 (which can happen when freq_base, freq_mult, or freq_ratio_m
> is 0), the old code would return 0 with esync->range=NULL and
> esync->range_num=0, while the new code returns 0 with
> esync->range=esync_freq_ranges and esync->range_num=1.
> 
> The removed comment documented the original intent: "if the current reference
> frequency is > 1 Hz". Should the new check be 'zl3073x_ref_freq_get(ref) <= 1'
> to preserve the original behavior for 0 Hz frequencies?
> 
>>
>> @@ -582,8 +577,8 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
>>   	struct zl3073x_dpll_pin *pin = pin_priv;
>>   	const struct zl3073x_synth *synth;
>>   	const struct zl3073x_out *out;
>> +	u32 synth_freq, out_freq;
>>   	u8 clock_type, out_id;
>> -	u32 synth_freq;
>>
>>   	out_id = zl3073x_output_pin_out_get(pin->id);
>>   	out = zl3073x_out_state_get(zldev, out_id);
>> @@ -592,17 +587,19 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
>>   	 * for N-division is also used for the esync divider so both cannot
>>   	 * be used.
>>   	 */
>> -	switch (zl3073x_out_signal_format_get(out)) {
>> -	case ZL_OUTPUT_MODE_SIGNAL_FORMAT_2_NDIV:
>> -	case ZL_OUTPUT_MODE_SIGNAL_FORMAT_2_NDIV_INV:
>> +	if (zl3073x_out_is_ndiv(out))
>>   		return -EOPNOTSUPP;
>> -	default:
>> -		break;
>> -	}
>>
>>   	/* Get attached synth frequency */
>>   	synth = zl3073x_synth_state_get(zldev, zl3073x_out_synth_get(out));
>>   	synth_freq = zl3073x_synth_freq_get(synth);
>> +	out_freq = synth_freq / out->div;
>> +
>> +	if (!pin->esync_control || out_freq == 1)
>> +		return -EOPNOTSUPP;
>> +
>> +	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) {
>> @@ -610,11 +607,11 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
>>   		esync->freq = 0;
>>   		esync->pulse = 0;
>>
>> -		goto finish;
>> +		return 0;
>>   	}
>>
>>   	/* Compute esync frequency */
>> -	esync->freq = synth_freq / out->div / out->esync_n_period;
>> +	esync->freq = out_freq / out->esync_n_period;
>>
>>   	/* By comparing the esync_pulse_width to the half of the pulse width
>>   	 * the esync pulse percentage can be determined.
>> @@ -623,18 +620,6 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
>>   	 */
>>   	esync->pulse = (50 * out->esync_n_width) / out->div;
>>
>> -finish:
>> -	/* Set supported esync ranges if the pin supports esync control and
>> -	 * if the output frequency is > 1 Hz.
>> -	 */
>> -	if (pin->esync_control && (synth_freq / out->div) > 1) {
>> -		esync->range = esync_freq_ranges;
>> -		esync->range_num = ARRAY_SIZE(esync_freq_ranges);
>> -	} else {
>> -		esync->range = NULL;
>> -		esync->range_num = 0;
>> -	}
>> -
>>   	return 0;
>>   }
> 
> Similar to the input variant, this changes the behavior for 0 Hz output
> frequencies.
> 
> The old code used '(synth_freq / out->div) > 1' to decide whether to expose
> the esync range, which excluded both out_freq==0 and out_freq==1. The new
> code uses 'out_freq == 1' for the early -EOPNOTSUPP return, only excluding
> out_freq==1.
> 
> When out_freq==0 (which can happen when synth_freq is 0 or synth_freq <
> out->div), the old code returned 0 with esync->range=NULL and
> esync->range_num=0, while the new code returns 0 with
> esync->range=esync_freq_ranges and esync->range_num=1.
> 
> The removed comment documented the intent: "if the output frequency is > 1
> Hz". Should this also use 'out_freq <= 1' to maintain consistency with the
> original behavior?
> 
> [ ... ]
> 

The frequency for both input and output pins cannot be zero... Cannot be
set to 0 and device never returns configured frequency to be 0.

I can modify the lines to be:

if (!pin->esync_control || out_freq <= 1)
	return -EOPNOTSUPP;

but it is needless - but let me know.

Thanks,
Ivan

Re: [PATCH net-next 1/5] dpll: zl3073x: clean up esync get/set and use zl3073x_out_is_ndiv()

[Simon Horman]

On Fri, Mar 20, 2026 at 06:31:36PM +0100, Ivan Vecera wrote:
> Hi Simon,
> 
> On 3/20/26 6:17 PM, Simon Horman wrote:
> > This is an AI-generated review of your patch. The human sending this
> > email has considered the AI review valid, or at least plausible.
> > 
> > For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> > ---
> > dpll: zl3073x: clean up esync get/set and use zl3073x_out_is_ndiv()
> > 
> > Return -EOPNOTSUPP early in esync_get callbacks when esync is not
> > supported instead of conditionally populating the range at the end.
> > This simplifies the control flow by removing the finish label/goto
> > in the output variant and the conditional range assignment in both
> > input and output variants.
> > 
> > Replace open-coded N-div signal format switch statements with
> > zl3073x_out_is_ndiv() helper in esync_get, esync_set and
> > frequency_set callbacks.
> > 
> > > diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
> > > index a29f606318f6..79ef62d69a32 100644
> > > --- a/drivers/dpll/zl3073x/dpll.c
> > > +++ b/drivers/dpll/zl3073x/dpll.c
> > > @@ -131,6 +131,12 @@ zl3073x_dpll_input_pin_esync_get(const struct dpll_pin *dpll_pin,
> > >   	ref_id = zl3073x_input_pin_ref_get(pin->id);
> > >   	ref = zl3073x_ref_state_get(zldev, ref_id);
> > > 
> > > +	if (!pin->esync_control || zl3073x_ref_freq_get(ref) == 1)
> > > +		return -EOPNOTSUPP;
> > > +
> > > +	esync->range = esync_freq_ranges;
> > > +	esync->range_num = ARRAY_SIZE(esync_freq_ranges);
> > > +
> > >   	switch (FIELD_GET(ZL_REF_SYNC_CTRL_MODE, ref->sync_ctrl)) {
> > >   	case ZL_REF_SYNC_CTRL_MODE_50_50_ESYNC_25_75:
> > >   		esync->freq = ref->esync_n_div == ZL_REF_ESYNC_DIV_1HZ ? 1 : 0;
> > > @@ -142,17 +148,6 @@ zl3073x_dpll_input_pin_esync_get(const struct dpll_pin *dpll_pin,
> > >   		break;
> > >   	}
> > > 
> > > -	/* If the pin supports esync control expose its range but only
> > > -	 * if the current reference frequency is > 1 Hz.
> > > -	 */
> > > -	if (pin->esync_control && zl3073x_ref_freq_get(ref) > 1) {
> > > -		esync->range = esync_freq_ranges;
> > > -		esync->range_num = ARRAY_SIZE(esync_freq_ranges);
> > > -	} else {
> > > -		esync->range = NULL;
> > > -		esync->range_num = 0;
> > > -	}
> > > -
> > >   	return 0;
> > >   }
> > 
> > The refactoring changes the behavior when the reference frequency is 0 Hz.
> > 
> > The old code checked 'zl3073x_ref_freq_get(ref) > 1' which excluded both
> > freq==0 and freq==1 from having the esync range exposed. The new code checks
> > 'zl3073x_ref_freq_get(ref) == 1' which only excludes freq==1.
> > 
> > When freq==0 (which can happen when freq_base, freq_mult, or freq_ratio_m
> > is 0), the old code would return 0 with esync->range=NULL and
> > esync->range_num=0, while the new code returns 0 with
> > esync->range=esync_freq_ranges and esync->range_num=1.
> > 
> > The removed comment documented the original intent: "if the current reference
> > frequency is > 1 Hz". Should the new check be 'zl3073x_ref_freq_get(ref) <= 1'
> > to preserve the original behavior for 0 Hz frequencies?
> > 
> > > 
> > > @@ -582,8 +577,8 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
> > >   	struct zl3073x_dpll_pin *pin = pin_priv;
> > >   	const struct zl3073x_synth *synth;
> > >   	const struct zl3073x_out *out;
> > > +	u32 synth_freq, out_freq;
> > >   	u8 clock_type, out_id;
> > > -	u32 synth_freq;
> > > 
> > >   	out_id = zl3073x_output_pin_out_get(pin->id);
> > >   	out = zl3073x_out_state_get(zldev, out_id);
> > > @@ -592,17 +587,19 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
> > >   	 * for N-division is also used for the esync divider so both cannot
> > >   	 * be used.
> > >   	 */
> > > -	switch (zl3073x_out_signal_format_get(out)) {
> > > -	case ZL_OUTPUT_MODE_SIGNAL_FORMAT_2_NDIV:
> > > -	case ZL_OUTPUT_MODE_SIGNAL_FORMAT_2_NDIV_INV:
> > > +	if (zl3073x_out_is_ndiv(out))
> > >   		return -EOPNOTSUPP;
> > > -	default:
> > > -		break;
> > > -	}
> > > 
> > >   	/* Get attached synth frequency */
> > >   	synth = zl3073x_synth_state_get(zldev, zl3073x_out_synth_get(out));
> > >   	synth_freq = zl3073x_synth_freq_get(synth);
> > > +	out_freq = synth_freq / out->div;
> > > +
> > > +	if (!pin->esync_control || out_freq == 1)
> > > +		return -EOPNOTSUPP;
> > > +
> > > +	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) {
> > > @@ -610,11 +607,11 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
> > >   		esync->freq = 0;
> > >   		esync->pulse = 0;
> > > 
> > > -		goto finish;
> > > +		return 0;
> > >   	}
> > > 
> > >   	/* Compute esync frequency */
> > > -	esync->freq = synth_freq / out->div / out->esync_n_period;
> > > +	esync->freq = out_freq / out->esync_n_period;
> > > 
> > >   	/* By comparing the esync_pulse_width to the half of the pulse width
> > >   	 * the esync pulse percentage can be determined.
> > > @@ -623,18 +620,6 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
> > >   	 */
> > >   	esync->pulse = (50 * out->esync_n_width) / out->div;
> > > 
> > > -finish:
> > > -	/* Set supported esync ranges if the pin supports esync control and
> > > -	 * if the output frequency is > 1 Hz.
> > > -	 */
> > > -	if (pin->esync_control && (synth_freq / out->div) > 1) {
> > > -		esync->range = esync_freq_ranges;
> > > -		esync->range_num = ARRAY_SIZE(esync_freq_ranges);
> > > -	} else {
> > > -		esync->range = NULL;
> > > -		esync->range_num = 0;
> > > -	}
> > > -
> > >   	return 0;
> > >   }
> > 
> > Similar to the input variant, this changes the behavior for 0 Hz output
> > frequencies.
> > 
> > The old code used '(synth_freq / out->div) > 1' to decide whether to expose
> > the esync range, which excluded both out_freq==0 and out_freq==1. The new
> > code uses 'out_freq == 1' for the early -EOPNOTSUPP return, only excluding
> > out_freq==1.
> > 
> > When out_freq==0 (which can happen when synth_freq is 0 or synth_freq <
> > out->div), the old code returned 0 with esync->range=NULL and
> > esync->range_num=0, while the new code returns 0 with
> > esync->range=esync_freq_ranges and esync->range_num=1.
> > 
> > The removed comment documented the intent: "if the output frequency is > 1
> > Hz". Should this also use 'out_freq <= 1' to maintain consistency with the
> > original behavior?
> > 
> > [ ... ]
> > 
> 
> The frequency for both input and output pins cannot be zero... Cannot be
> set to 0 and device never returns configured frequency to be 0.
> 
> I can modify the lines to be:
> 
> if (!pin->esync_control || out_freq <= 1)
> 	return -EOPNOTSUPP;
> 
> but it is needless - but let me know.

Thanks Ivan,

If it is needless then there is no need to handle it in the code.
Sorry for not understanding that before forwarding on the AI generated review.

Re: [PATCH net-next 1/5] dpll: zl3073x: clean up esync get/set and use zl3073x_out_is_ndiv()

[Petr Oros]

> Return -EOPNOTSUPP early in esync_get callbacks when esync is not
> supported instead of conditionally populating the range at the end.
> This simplifies the control flow by removing the finish label/goto
> in the output variant and the conditional range assignment in both
> input and output variants.
>
> Replace open-coded N-div signal format switch statements with
> zl3073x_out_is_ndiv() helper in esync_get, esync_set and
> frequency_set callbacks.
>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
>   drivers/dpll/zl3073x/dpll.c | 64 ++++++++++++-------------------------
>   1 file changed, 20 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
> index a29f606318f6d..79ef62d69a32d 100644
> --- a/drivers/dpll/zl3073x/dpll.c
> +++ b/drivers/dpll/zl3073x/dpll.c
> @@ -131,6 +131,12 @@ zl3073x_dpll_input_pin_esync_get(const struct dpll_pin *dpll_pin,
>   	ref_id = zl3073x_input_pin_ref_get(pin->id);
>   	ref = zl3073x_ref_state_get(zldev, ref_id);
>   
> +	if (!pin->esync_control || zl3073x_ref_freq_get(ref) == 1)
> +		return -EOPNOTSUPP;
> +
> +	esync->range = esync_freq_ranges;
> +	esync->range_num = ARRAY_SIZE(esync_freq_ranges);
> +
>   	switch (FIELD_GET(ZL_REF_SYNC_CTRL_MODE, ref->sync_ctrl)) {
>   	case ZL_REF_SYNC_CTRL_MODE_50_50_ESYNC_25_75:
>   		esync->freq = ref->esync_n_div == ZL_REF_ESYNC_DIV_1HZ ? 1 : 0;
> @@ -142,17 +148,6 @@ zl3073x_dpll_input_pin_esync_get(const struct dpll_pin *dpll_pin,
>   		break;
>   	}
>   
> -	/* If the pin supports esync control expose its range but only
> -	 * if the current reference frequency is > 1 Hz.
> -	 */
> -	if (pin->esync_control && zl3073x_ref_freq_get(ref) > 1) {
> -		esync->range = esync_freq_ranges;
> -		esync->range_num = ARRAY_SIZE(esync_freq_ranges);
> -	} else {
> -		esync->range = NULL;
> -		esync->range_num = 0;
> -	}
> -
>   	return 0;
>   }
>   
> @@ -582,8 +577,8 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
>   	struct zl3073x_dpll_pin *pin = pin_priv;
>   	const struct zl3073x_synth *synth;
>   	const struct zl3073x_out *out;
> +	u32 synth_freq, out_freq;
>   	u8 clock_type, out_id;
> -	u32 synth_freq;
>   
>   	out_id = zl3073x_output_pin_out_get(pin->id);
>   	out = zl3073x_out_state_get(zldev, out_id);
> @@ -592,17 +587,19 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
>   	 * for N-division is also used for the esync divider so both cannot
>   	 * be used.
>   	 */
> -	switch (zl3073x_out_signal_format_get(out)) {
> -	case ZL_OUTPUT_MODE_SIGNAL_FORMAT_2_NDIV:
> -	case ZL_OUTPUT_MODE_SIGNAL_FORMAT_2_NDIV_INV:
> +	if (zl3073x_out_is_ndiv(out))
>   		return -EOPNOTSUPP;
> -	default:
> -		break;
> -	}
>   
>   	/* Get attached synth frequency */
>   	synth = zl3073x_synth_state_get(zldev, zl3073x_out_synth_get(out));
>   	synth_freq = zl3073x_synth_freq_get(synth);
> +	out_freq = synth_freq / out->div;
> +
> +	if (!pin->esync_control || out_freq == 1)
> +		return -EOPNOTSUPP;
> +
> +	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) {
> @@ -610,11 +607,11 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
>   		esync->freq = 0;
>   		esync->pulse = 0;
>   
> -		goto finish;
> +		return 0;
>   	}
>   
>   	/* Compute esync frequency */
> -	esync->freq = synth_freq / out->div / out->esync_n_period;
> +	esync->freq = out_freq / out->esync_n_period;
>   
>   	/* By comparing the esync_pulse_width to the half of the pulse width
>   	 * the esync pulse percentage can be determined.
> @@ -623,18 +620,6 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
>   	 */
>   	esync->pulse = (50 * out->esync_n_width) / out->div;
>   
> -finish:
> -	/* Set supported esync ranges if the pin supports esync control and
> -	 * if the output frequency is > 1 Hz.
> -	 */
> -	if (pin->esync_control && (synth_freq / out->div) > 1) {
> -		esync->range = esync_freq_ranges;
> -		esync->range_num = ARRAY_SIZE(esync_freq_ranges);
> -	} else {
> -		esync->range = NULL;
> -		esync->range_num = 0;
> -	}
> -
>   	return 0;
>   }
>   
> @@ -660,13 +645,8 @@ zl3073x_dpll_output_pin_esync_set(const struct dpll_pin *dpll_pin,
>   	 * for N-division is also used for the esync divider so both cannot
>   	 * be used.
>   	 */
> -	switch (zl3073x_out_signal_format_get(&out)) {
> -	case ZL_OUTPUT_MODE_SIGNAL_FORMAT_2_NDIV:
> -	case ZL_OUTPUT_MODE_SIGNAL_FORMAT_2_NDIV_INV:
> +	if (zl3073x_out_is_ndiv(&out))
>   		return -EOPNOTSUPP;
> -	default:
> -		break;
> -	}
>   
>   	/* Select clock type */
>   	if (freq)
> @@ -728,9 +708,9 @@ zl3073x_dpll_output_pin_frequency_set(const struct dpll_pin *dpll_pin,
>   	struct zl3073x_dev *zldev = zldpll->dev;
>   	struct zl3073x_dpll_pin *pin = pin_priv;
>   	const struct zl3073x_synth *synth;
> -	u8 out_id, signal_format;
>   	u32 new_div, synth_freq;
>   	struct zl3073x_out out;
> +	u8 out_id;
>   
>   	out_id = zl3073x_output_pin_out_get(pin->id);
>   	out = *zl3073x_out_state_get(zldev, out_id);
> @@ -740,12 +720,8 @@ zl3073x_dpll_output_pin_frequency_set(const struct dpll_pin *dpll_pin,
>   	synth_freq = zl3073x_synth_freq_get(synth);
>   	new_div = synth_freq / (u32)frequency;
>   
> -	/* Get used signal format for the given output */
> -	signal_format = zl3073x_out_signal_format_get(&out);
> -
>   	/* Check signal format */
> -	if (signal_format != ZL_OUTPUT_MODE_SIGNAL_FORMAT_2_NDIV &&
> -	    signal_format != ZL_OUTPUT_MODE_SIGNAL_FORMAT_2_NDIV_INV) {
> +	if (!zl3073x_out_is_ndiv(&out)) {
>   		/* For non N-divided signal formats the frequency is computed
>   		 * as division of synth frequency and output divisor.
>   		 */

LGTM,

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

[PATCH net-next 2/5] dpll: zl3073x: use FIELD_MODIFY() for clear-and-set patterns

[Ivan Vecera]

Replace open-coded clear-and-set bitfield operations with
FIELD_MODIFY().

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/chan.h  | 17 ++++++-----------
 drivers/dpll/zl3073x/core.c  |  3 +--
 drivers/dpll/zl3073x/flash.c |  3 +--
 3 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/dpll/zl3073x/chan.h b/drivers/dpll/zl3073x/chan.h
index e0f02d3432086..481da2133202b 100644
--- a/drivers/dpll/zl3073x/chan.h
+++ b/drivers/dpll/zl3073x/chan.h
@@ -66,8 +66,7 @@ static inline u8 zl3073x_chan_ref_get(const struct zl3073x_chan *chan)
  */
 static inline void zl3073x_chan_mode_set(struct zl3073x_chan *chan, u8 mode)
 {
-	chan->mode_refsel &= ~ZL_DPLL_MODE_REFSEL_MODE;
-	chan->mode_refsel |= FIELD_PREP(ZL_DPLL_MODE_REFSEL_MODE, mode);
+	FIELD_MODIFY(ZL_DPLL_MODE_REFSEL_MODE, &chan->mode_refsel, mode);
 }
 
 /**
@@ -77,8 +76,7 @@ static inline void zl3073x_chan_mode_set(struct zl3073x_chan *chan, u8 mode)
  */
 static inline void zl3073x_chan_ref_set(struct zl3073x_chan *chan, u8 ref)
 {
-	chan->mode_refsel &= ~ZL_DPLL_MODE_REFSEL_REF;
-	chan->mode_refsel |= FIELD_PREP(ZL_DPLL_MODE_REFSEL_REF, ref);
+	FIELD_MODIFY(ZL_DPLL_MODE_REFSEL_REF, &chan->mode_refsel, ref);
 }
 
 /**
@@ -110,13 +108,10 @@ zl3073x_chan_ref_prio_set(struct zl3073x_chan *chan, u8 ref, u8 prio)
 {
 	u8 *val = &chan->ref_prio[ref / 2];
 
-	if (!(ref & 1)) {
-		*val &= ~ZL_DPLL_REF_PRIO_REF_P;
-		*val |= FIELD_PREP(ZL_DPLL_REF_PRIO_REF_P, prio);
-	} else {
-		*val &= ~ZL_DPLL_REF_PRIO_REF_N;
-		*val |= FIELD_PREP(ZL_DPLL_REF_PRIO_REF_N, prio);
-	}
+	if (!(ref & 1))
+		FIELD_MODIFY(ZL_DPLL_REF_PRIO_REF_P, val, prio);
+	else
+		FIELD_MODIFY(ZL_DPLL_REF_PRIO_REF_N, val, prio);
 }
 
 /**
diff --git a/drivers/dpll/zl3073x/core.c b/drivers/dpll/zl3073x/core.c
index 6363002d48d46..7eebfc1ad1019 100644
--- a/drivers/dpll/zl3073x/core.c
+++ b/drivers/dpll/zl3073x/core.c
@@ -743,8 +743,7 @@ int zl3073x_dev_phase_avg_factor_set(struct zl3073x_dev *zldev, u8 factor)
 	value = (factor + 1) & 0x0f;
 
 	/* Update phase measurement control register */
-	dpll_meas_ctrl &= ~ZL_DPLL_MEAS_CTRL_AVG_FACTOR;
-	dpll_meas_ctrl |= FIELD_PREP(ZL_DPLL_MEAS_CTRL_AVG_FACTOR, value);
+	FIELD_MODIFY(ZL_DPLL_MEAS_CTRL_AVG_FACTOR, &dpll_meas_ctrl, value);
 	rc = zl3073x_write_u8(zldev, ZL_REG_DPLL_MEAS_CTRL, dpll_meas_ctrl);
 	if (rc)
 		return rc;
diff --git a/drivers/dpll/zl3073x/flash.c b/drivers/dpll/zl3073x/flash.c
index 83452a77e3e98..f85535c8ad246 100644
--- a/drivers/dpll/zl3073x/flash.c
+++ b/drivers/dpll/zl3073x/flash.c
@@ -194,8 +194,7 @@ zl3073x_flash_cmd_wait(struct zl3073x_dev *zldev, u32 operation,
 	if (rc)
 		return rc;
 
-	value &= ~ZL_WRITE_FLASH_OP;
-	value |= FIELD_PREP(ZL_WRITE_FLASH_OP, operation);
+	FIELD_MODIFY(ZL_WRITE_FLASH_OP, &value, operation);
 
 	rc = zl3073x_write_u8(zldev, ZL_REG_WRITE_FLASH, value);
 	if (rc)
-- 
2.52.0

Re: [PATCH net-next 2/5] dpll: zl3073x: use FIELD_MODIFY() for clear-and-set patterns

[Petr Oros]

> Replace open-coded clear-and-set bitfield operations with
> FIELD_MODIFY().
>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
>   drivers/dpll/zl3073x/chan.h  | 17 ++++++-----------
>   drivers/dpll/zl3073x/core.c  |  3 +--
>   drivers/dpll/zl3073x/flash.c |  3 +--
>   3 files changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/dpll/zl3073x/chan.h b/drivers/dpll/zl3073x/chan.h
> index e0f02d3432086..481da2133202b 100644
> --- a/drivers/dpll/zl3073x/chan.h
> +++ b/drivers/dpll/zl3073x/chan.h
> @@ -66,8 +66,7 @@ static inline u8 zl3073x_chan_ref_get(const struct zl3073x_chan *chan)
>    */
>   static inline void zl3073x_chan_mode_set(struct zl3073x_chan *chan, u8 mode)
>   {
> -	chan->mode_refsel &= ~ZL_DPLL_MODE_REFSEL_MODE;
> -	chan->mode_refsel |= FIELD_PREP(ZL_DPLL_MODE_REFSEL_MODE, mode);
> +	FIELD_MODIFY(ZL_DPLL_MODE_REFSEL_MODE, &chan->mode_refsel, mode);
>   }
>   
>   /**
> @@ -77,8 +76,7 @@ static inline void zl3073x_chan_mode_set(struct zl3073x_chan *chan, u8 mode)
>    */
>   static inline void zl3073x_chan_ref_set(struct zl3073x_chan *chan, u8 ref)
>   {
> -	chan->mode_refsel &= ~ZL_DPLL_MODE_REFSEL_REF;
> -	chan->mode_refsel |= FIELD_PREP(ZL_DPLL_MODE_REFSEL_REF, ref);
> +	FIELD_MODIFY(ZL_DPLL_MODE_REFSEL_REF, &chan->mode_refsel, ref);
>   }
>   
>   /**
> @@ -110,13 +108,10 @@ zl3073x_chan_ref_prio_set(struct zl3073x_chan *chan, u8 ref, u8 prio)
>   {
>   	u8 *val = &chan->ref_prio[ref / 2];
>   
> -	if (!(ref & 1)) {
> -		*val &= ~ZL_DPLL_REF_PRIO_REF_P;
> -		*val |= FIELD_PREP(ZL_DPLL_REF_PRIO_REF_P, prio);
> -	} else {
> -		*val &= ~ZL_DPLL_REF_PRIO_REF_N;
> -		*val |= FIELD_PREP(ZL_DPLL_REF_PRIO_REF_N, prio);
> -	}
> +	if (!(ref & 1))
> +		FIELD_MODIFY(ZL_DPLL_REF_PRIO_REF_P, val, prio);
> +	else
> +		FIELD_MODIFY(ZL_DPLL_REF_PRIO_REF_N, val, prio);
>   }
>   
>   /**
> diff --git a/drivers/dpll/zl3073x/core.c b/drivers/dpll/zl3073x/core.c
> index 6363002d48d46..7eebfc1ad1019 100644
> --- a/drivers/dpll/zl3073x/core.c
> +++ b/drivers/dpll/zl3073x/core.c
> @@ -743,8 +743,7 @@ int zl3073x_dev_phase_avg_factor_set(struct zl3073x_dev *zldev, u8 factor)
>   	value = (factor + 1) & 0x0f;
>   
>   	/* Update phase measurement control register */
> -	dpll_meas_ctrl &= ~ZL_DPLL_MEAS_CTRL_AVG_FACTOR;
> -	dpll_meas_ctrl |= FIELD_PREP(ZL_DPLL_MEAS_CTRL_AVG_FACTOR, value);
> +	FIELD_MODIFY(ZL_DPLL_MEAS_CTRL_AVG_FACTOR, &dpll_meas_ctrl, value);
>   	rc = zl3073x_write_u8(zldev, ZL_REG_DPLL_MEAS_CTRL, dpll_meas_ctrl);
>   	if (rc)
>   		return rc;
> diff --git a/drivers/dpll/zl3073x/flash.c b/drivers/dpll/zl3073x/flash.c
> index 83452a77e3e98..f85535c8ad246 100644
> --- a/drivers/dpll/zl3073x/flash.c
> +++ b/drivers/dpll/zl3073x/flash.c
> @@ -194,8 +194,7 @@ zl3073x_flash_cmd_wait(struct zl3073x_dev *zldev, u32 operation,
>   	if (rc)
>   		return rc;
>   
> -	value &= ~ZL_WRITE_FLASH_OP;
> -	value |= FIELD_PREP(ZL_WRITE_FLASH_OP, operation);
> +	FIELD_MODIFY(ZL_WRITE_FLASH_OP, &value, operation);
>   
>   	rc = zl3073x_write_u8(zldev, ZL_REG_WRITE_FLASH, value);
>   	if (rc)
LGTM.

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

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

[Ivan Vecera]

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
-- 
2.52.0

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

[Petr Oros]

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

[PATCH net-next 4/5] dt-bindings: dpll: add ref-sync-sources property

[Ivan Vecera]

Add ref-sync-sources phandle-array property to the dpll-pin schema
allowing board designers to declare which input pins can serve as
sync sources in a Reference-Sync pair.  A Ref-Sync pair consists of
a clock reference and a low-frequency sync signal where the DPLL locks
to the clock but phase-aligns to the sync reference.

Update both examples in the Microchip ZL3073x binding to demonstrate
the new property with a 1 PPS sync source paired to a clock source.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 .../devicetree/bindings/dpll/dpll-pin.yaml    | 11 +++++++
 .../bindings/dpll/microchip,zl30731.yaml      | 30 ++++++++++++++-----
 2 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/dpll/dpll-pin.yaml b/Documentation/devicetree/bindings/dpll/dpll-pin.yaml
index 51db93b77306f..7084f102e274c 100644
--- a/Documentation/devicetree/bindings/dpll/dpll-pin.yaml
+++ b/Documentation/devicetree/bindings/dpll/dpll-pin.yaml
@@ -36,6 +36,17 @@ properties:
     description: String exposed as the pin board label
     $ref: /schemas/types.yaml#/definitions/string
 
+  ref-sync-sources:
+    description: |
+      List of phandles to input pins that can serve as the sync source
+      in a Reference-Sync pair with this pin acting as the clock source.
+      A Ref-Sync pair consists of a clock reference and a low-frequency
+      sync signal.  The DPLL locks to the clock reference but
+      phase-aligns to the sync reference.
+      Only valid for input pins.  Each referenced pin must be a
+      different input pin on the same device.
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+
   supported-frequencies-hz:
     description: List of supported frequencies for this pin, expressed in Hz.
 
diff --git a/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml b/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
index 17747f754b845..fa5a8f8e390cd 100644
--- a/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
+++ b/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
@@ -52,11 +52,19 @@ examples:
           #address-cells = <1>;
           #size-cells = <0>;
 
-          pin@0 { /* REF0P */
+          sync0: pin@0 { /* REF0P - 1 PPS sync source */
             reg = <0>;
             connection-type = "ext";
-            label = "Input 0";
-            supported-frequencies-hz = /bits/ 64 <1 1000>;
+            label = "SMA1";
+            supported-frequencies-hz = /bits/ 64 <1>;
+          };
+
+          pin@1 { /* REF0N - clock source, can pair with sync0 */
+            reg = <1>;
+            connection-type = "ext";
+            label = "SMA2";
+            supported-frequencies-hz = /bits/ 64 <10000 10000000>;
+            ref-sync-sources = <&sync0>;
           };
         };
 
@@ -90,11 +98,19 @@ examples:
           #address-cells = <1>;
           #size-cells = <0>;
 
-          pin@0 { /* REF0P */
+          sync1: pin@0 { /* REF0P - 1 PPS sync source */
             reg = <0>;
-            connection-type = "ext";
-            label = "Input 0";
-            supported-frequencies-hz = /bits/ 64 <1 1000>;
+            connection-type = "gnss";
+            label = "GNSS_1PPS_IN";
+            supported-frequencies-hz = /bits/ 64 <1>;
+          };
+
+          pin@1 { /* REF0N - clock source */
+            reg = <1>;
+            connection-type = "gnss";
+            label = "GNSS_10M_IN";
+            supported-frequencies-hz = /bits/ 64 <10000000>;
+            ref-sync-sources = <&sync1>;
           };
         };
 
-- 
2.52.0

Re: [PATCH net-next 4/5] dt-bindings: dpll: add ref-sync-sources property

[Petr Oros]

> Add ref-sync-sources phandle-array property to the dpll-pin schema
> allowing board designers to declare which input pins can serve as
> sync sources in a Reference-Sync pair.  A Ref-Sync pair consists of
> a clock reference and a low-frequency sync signal where the DPLL locks
> to the clock but phase-aligns to the sync reference.
>
> Update both examples in the Microchip ZL3073x binding to demonstrate
> the new property with a 1 PPS sync source paired to a clock source.
>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
>   .../devicetree/bindings/dpll/dpll-pin.yaml    | 11 +++++++
>   .../bindings/dpll/microchip,zl30731.yaml      | 30 ++++++++++++++-----
>   2 files changed, 34 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/dpll/dpll-pin.yaml b/Documentation/devicetree/bindings/dpll/dpll-pin.yaml
> index 51db93b77306f..7084f102e274c 100644
> --- a/Documentation/devicetree/bindings/dpll/dpll-pin.yaml
> +++ b/Documentation/devicetree/bindings/dpll/dpll-pin.yaml
> @@ -36,6 +36,17 @@ properties:
>       description: String exposed as the pin board label
>       $ref: /schemas/types.yaml#/definitions/string
>   
> +  ref-sync-sources:
> +    description: |
> +      List of phandles to input pins that can serve as the sync source
> +      in a Reference-Sync pair with this pin acting as the clock source.
> +      A Ref-Sync pair consists of a clock reference and a low-frequency
> +      sync signal.  The DPLL locks to the clock reference but
> +      phase-aligns to the sync reference.
> +      Only valid for input pins.  Each referenced pin must be a
> +      different input pin on the same device.
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +
>     supported-frequencies-hz:
>       description: List of supported frequencies for this pin, expressed in Hz.
>   
> diff --git a/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml b/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
> index 17747f754b845..fa5a8f8e390cd 100644
> --- a/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
> +++ b/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
> @@ -52,11 +52,19 @@ examples:
>             #address-cells = <1>;
>             #size-cells = <0>;
>   
> -          pin@0 { /* REF0P */
> +          sync0: pin@0 { /* REF0P - 1 PPS sync source */
>               reg = <0>;
>               connection-type = "ext";
> -            label = "Input 0";
> -            supported-frequencies-hz = /bits/ 64 <1 1000>;
> +            label = "SMA1";
> +            supported-frequencies-hz = /bits/ 64 <1>;
> +          };
> +
> +          pin@1 { /* REF0N - clock source, can pair with sync0 */
> +            reg = <1>;
> +            connection-type = "ext";
> +            label = "SMA2";
> +            supported-frequencies-hz = /bits/ 64 <10000 10000000>;
> +            ref-sync-sources = <&sync0>;
>             };
>           };
>   
> @@ -90,11 +98,19 @@ examples:
>             #address-cells = <1>;
>             #size-cells = <0>;
>   
> -          pin@0 { /* REF0P */
> +          sync1: pin@0 { /* REF0P - 1 PPS sync source */
>               reg = <0>;
> -            connection-type = "ext";
> -            label = "Input 0";
> -            supported-frequencies-hz = /bits/ 64 <1 1000>;
> +            connection-type = "gnss";
> +            label = "GNSS_1PPS_IN";
> +            supported-frequencies-hz = /bits/ 64 <1>;
> +          };
> +
> +          pin@1 { /* REF0N - clock source */
> +            reg = <1>;
> +            connection-type = "gnss";
> +            label = "GNSS_10M_IN";
> +            supported-frequencies-hz = /bits/ 64 <10000000>;
> +            ref-sync-sources = <&sync1>;
>             };
>           };
>   
LGTM.

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

[PATCH net-next 5/5] dpll: zl3073x: add ref-sync pair support

[Ivan Vecera]

Add support for ref-sync pair registration using the 'ref-sync-sources'
phandle property from device tree. A ref-sync pair consists of a clock
reference and a low-frequency sync signal where the DPLL locks to the
clock reference but phase-aligns to the sync reference.

The implementation:
- Stores fwnode handle in zl3073x_dpll_pin during pin registration
- Adds ref_sync_get/set callbacks to read and write the sync control
  mode and pair registers
- Validates ref-sync frequency constraints: sync signal must be 8 kHz
  or less, clock reference must be 1 kHz or more and higher than sync
- Excludes sync source from automatic reference selection by setting
  its priority to NONE on connect; on disconnect the priority is left
  as NONE and the user must explicitly make the pin selectable again
- Iterates ref-sync-sources phandles to register declared pairings
  via dpll_pin_ref_sync_pair_add()

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/dpll.c | 207 +++++++++++++++++++++++++++++++++++-
 1 file changed, 206 insertions(+), 1 deletion(-)

diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
index 276f0a92db0b1..8010e2635f641 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/netlink.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/sprintf.h>
 
@@ -30,6 +31,7 @@
  * @dpll: DPLL the pin is registered to
  * @dpll_pin: pointer to registered dpll_pin
  * @tracker: tracking object for the acquired reference
+ * @fwnode: firmware node handle
  * @label: package label
  * @dir: pin direction
  * @id: pin id
@@ -45,6 +47,7 @@ struct zl3073x_dpll_pin {
 	struct zl3073x_dpll	*dpll;
 	struct dpll_pin		*dpll_pin;
 	dpll_tracker		tracker;
+	struct fwnode_handle	*fwnode;
 	char			label[8];
 	enum dpll_pin_direction	dir;
 	u8			id;
@@ -184,6 +187,109 @@ zl3073x_dpll_input_pin_esync_set(const struct dpll_pin *dpll_pin,
 	return zl3073x_ref_state_set(zldev, ref_id, &ref);
 }
 
+static int
+zl3073x_dpll_input_pin_ref_sync_get(const struct dpll_pin *dpll_pin,
+				    void *pin_priv,
+				    const struct dpll_pin *ref_sync_pin,
+				    void *ref_sync_pin_priv,
+				    enum dpll_pin_state *state,
+				    struct netlink_ext_ack *extack)
+{
+	struct zl3073x_dpll_pin *sync_pin = ref_sync_pin_priv;
+	struct zl3073x_dpll_pin *pin = pin_priv;
+	struct zl3073x_dpll *zldpll = pin->dpll;
+	struct zl3073x_dev *zldev = zldpll->dev;
+	const struct zl3073x_ref *ref;
+	u8 ref_id, mode, pair;
+
+	ref_id = zl3073x_input_pin_ref_get(pin->id);
+	ref = zl3073x_ref_state_get(zldev, ref_id);
+	mode = zl3073x_ref_sync_mode_get(ref);
+	pair = zl3073x_ref_sync_pair_get(ref);
+
+	if (mode == ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR &&
+	    pair == zl3073x_input_pin_ref_get(sync_pin->id))
+		*state = DPLL_PIN_STATE_CONNECTED;
+	else
+		*state = DPLL_PIN_STATE_DISCONNECTED;
+
+	return 0;
+}
+
+static int
+zl3073x_dpll_input_pin_ref_sync_set(const struct dpll_pin *dpll_pin,
+				    void *pin_priv,
+				    const struct dpll_pin *ref_sync_pin,
+				    void *ref_sync_pin_priv,
+				    const enum dpll_pin_state state,
+				    struct netlink_ext_ack *extack)
+{
+	struct zl3073x_dpll_pin *sync_pin = ref_sync_pin_priv;
+	struct zl3073x_dpll_pin *pin = pin_priv;
+	struct zl3073x_dpll *zldpll = pin->dpll;
+	struct zl3073x_dev *zldev = zldpll->dev;
+	u8 mode, ref_id, sync_ref_id;
+	struct zl3073x_chan chan;
+	struct zl3073x_ref ref;
+	int rc;
+
+	ref_id = zl3073x_input_pin_ref_get(pin->id);
+	sync_ref_id = zl3073x_input_pin_ref_get(sync_pin->id);
+	ref = *zl3073x_ref_state_get(zldev, ref_id);
+
+	if (state == DPLL_PIN_STATE_CONNECTED) {
+		const struct zl3073x_ref *sync_ref;
+		u32 ref_freq, sync_freq;
+
+		sync_ref = zl3073x_ref_state_get(zldev, sync_ref_id);
+		ref_freq = zl3073x_ref_freq_get(&ref);
+		sync_freq = zl3073x_ref_freq_get(sync_ref);
+
+		/* Sync signal must be 8 kHz or less and clock reference
+		 * must be 1 kHz or more and higher than the sync signal.
+		 */
+		if (sync_freq > 8000) {
+			NL_SET_ERR_MSG(extack,
+				       "sync frequency must be 8 kHz or less");
+			return -EINVAL;
+		}
+		if (ref_freq < 1000) {
+			NL_SET_ERR_MSG(extack,
+				       "clock frequency must be 1 kHz or more");
+			return -EINVAL;
+		}
+		if (ref_freq <= sync_freq) {
+			NL_SET_ERR_MSG(extack,
+				       "clock frequency must be higher than sync frequency");
+			return -EINVAL;
+		}
+
+		zl3073x_ref_sync_pair_set(&ref, sync_ref_id);
+		mode = ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR;
+	} else {
+		mode = ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR_OFF;
+	}
+
+	zl3073x_ref_sync_mode_set(&ref, mode);
+
+	rc = zl3073x_ref_state_set(zldev, ref_id, &ref);
+	if (rc)
+		return rc;
+
+	/* Exclude sync source from automatic reference selection by setting
+	 * its priority to NONE. On disconnect the priority is left as NONE
+	 * and the user must explicitly make the pin selectable again.
+	 */
+	if (state == DPLL_PIN_STATE_CONNECTED) {
+		chan = *zl3073x_chan_state_get(zldev, zldpll->id);
+		zl3073x_chan_ref_prio_set(&chan, sync_ref_id,
+					  ZL_DPLL_REF_PRIO_NONE);
+		return zl3073x_chan_state_set(zldev, zldpll->id, &chan);
+	}
+
+	return 0;
+}
+
 static int
 zl3073x_dpll_input_pin_ffo_get(const struct dpll_pin *dpll_pin, void *pin_priv,
 			       const struct dpll_device *dpll, void *dpll_priv,
@@ -1100,6 +1206,8 @@ static const struct dpll_pin_ops zl3073x_dpll_input_pin_ops = {
 	.phase_adjust_set = zl3073x_dpll_input_pin_phase_adjust_set,
 	.prio_get = zl3073x_dpll_input_pin_prio_get,
 	.prio_set = zl3073x_dpll_input_pin_prio_set,
+	.ref_sync_get = zl3073x_dpll_input_pin_ref_sync_get,
+	.ref_sync_set = zl3073x_dpll_input_pin_ref_sync_set,
 	.state_on_dpll_get = zl3073x_dpll_input_pin_state_on_dpll_get,
 	.state_on_dpll_set = zl3073x_dpll_input_pin_state_on_dpll_set,
 };
@@ -1190,8 +1298,11 @@ zl3073x_dpll_pin_register(struct zl3073x_dpll_pin *pin, u32 index)
 	if (IS_ERR(props))
 		return PTR_ERR(props);
 
-	/* Save package label, esync capability and phase adjust granularity */
+	/* Save package label, fwnode, esync capability and phase adjust
+	 * granularity.
+	 */
 	strscpy(pin->label, props->package_label);
+	pin->fwnode = fwnode_handle_get(props->fwnode);
 	pin->esync_control = props->esync_control;
 	pin->phase_gran = props->dpll_props.phase_gran;
 
@@ -1236,6 +1347,8 @@ zl3073x_dpll_pin_register(struct zl3073x_dpll_pin *pin, u32 index)
 	dpll_pin_put(pin->dpll_pin, &pin->tracker);
 	pin->dpll_pin = NULL;
 err_pin_get:
+	fwnode_handle_put(pin->fwnode);
+	pin->fwnode = NULL;
 	zl3073x_pin_props_put(props);
 
 	return rc;
@@ -1265,6 +1378,9 @@ zl3073x_dpll_pin_unregister(struct zl3073x_dpll_pin *pin)
 
 	dpll_pin_put(pin->dpll_pin, &pin->tracker);
 	pin->dpll_pin = NULL;
+
+	fwnode_handle_put(pin->fwnode);
+	pin->fwnode = NULL;
 }
 
 /**
@@ -1735,6 +1851,88 @@ zl3073x_dpll_free(struct zl3073x_dpll *zldpll)
 	kfree(zldpll);
 }
 
+/**
+ * zl3073x_dpll_ref_sync_pair_register - register ref_sync pairs for a pin
+ * @pin: pointer to zl3073x_dpll_pin structure
+ *
+ * Iterates 'ref-sync-sources' phandles in the pin's firmware node and
+ * registers each declared pairing.
+ *
+ * Return: 0 on success, <0 on error
+ */
+static int
+zl3073x_dpll_ref_sync_pair_register(struct zl3073x_dpll_pin *pin)
+{
+	struct zl3073x_dev *zldev = pin->dpll->dev;
+	struct fwnode_handle *fwnode;
+	struct dpll_pin *sync_pin;
+	dpll_tracker tracker;
+	int n, rc;
+
+	for (n = 0; ; n++) {
+		/* Get n'th ref-sync source */
+		fwnode = fwnode_find_reference(pin->fwnode, "ref-sync-sources",
+					       n);
+		if (IS_ERR(fwnode)) {
+			rc = PTR_ERR(fwnode);
+			break;
+		}
+
+		/* Find associated dpll pin */
+		sync_pin = fwnode_dpll_pin_find(fwnode, &tracker);
+		fwnode_handle_put(fwnode);
+		if (!sync_pin) {
+			dev_warn(zldev->dev, "%s: ref-sync source %d not found",
+				 pin->label, n);
+			continue;
+		}
+
+		/* Register new ref-sync pair */
+		rc = dpll_pin_ref_sync_pair_add(pin->dpll_pin, sync_pin);
+		dpll_pin_put(sync_pin, &tracker);
+
+		/* -EBUSY means pairing already exists from another DPLL's
+		 * registration.
+		 */
+		if (rc && rc != -EBUSY) {
+			dev_err(zldev->dev,
+				"%s: failed to add ref-sync source %d: %pe",
+				pin->label, n, ERR_PTR(rc));
+			break;
+		}
+	}
+
+	return rc != -ENOENT ? rc : 0;
+}
+
+/**
+ * zl3073x_dpll_ref_sync_pairs_register - register ref_sync pairs for a DPLL
+ * @zldpll: pointer to zl3073x_dpll structure
+ *
+ * Iterates all registered input pins of the given DPLL and establishes
+ * ref_sync pairings declared by 'ref-sync-sources' phandles in the
+ * device tree.
+ *
+ * Return: 0 on success, <0 on error
+ */
+static int
+zl3073x_dpll_ref_sync_pairs_register(struct zl3073x_dpll *zldpll)
+{
+	struct zl3073x_dpll_pin *pin;
+	int rc;
+
+	list_for_each_entry(pin, &zldpll->pins, list) {
+		if (!zl3073x_dpll_is_input_pin(pin) || !pin->fwnode)
+			continue;
+
+		rc = zl3073x_dpll_ref_sync_pair_register(pin);
+		if (rc)
+			return rc;
+	}
+
+	return 0;
+}
+
 /**
  * zl3073x_dpll_register - register DPLL device and all its pins
  * @zldpll: pointer to zl3073x_dpll structure
@@ -1758,6 +1956,13 @@ zl3073x_dpll_register(struct zl3073x_dpll *zldpll)
 		return rc;
 	}
 
+	rc = zl3073x_dpll_ref_sync_pairs_register(zldpll);
+	if (rc) {
+		zl3073x_dpll_pins_unregister(zldpll);
+		zl3073x_dpll_device_unregister(zldpll);
+		return rc;
+	}
+
 	return 0;
 }
 
-- 
2.52.0

Re: [PATCH net-next 5/5] dpll: zl3073x: add ref-sync pair support

[Petr Oros]

> Add support for ref-sync pair registration using the 'ref-sync-sources'
> phandle property from device tree. A ref-sync pair consists of a clock
> reference and a low-frequency sync signal where the DPLL locks to the
> clock reference but phase-aligns to the sync reference.
>
> The implementation:
> - Stores fwnode handle in zl3073x_dpll_pin during pin registration
> - Adds ref_sync_get/set callbacks to read and write the sync control
>    mode and pair registers
> - Validates ref-sync frequency constraints: sync signal must be 8 kHz
>    or less, clock reference must be 1 kHz or more and higher than sync
> - Excludes sync source from automatic reference selection by setting
>    its priority to NONE on connect; on disconnect the priority is left
>    as NONE and the user must explicitly make the pin selectable again
> - Iterates ref-sync-sources phandles to register declared pairings
>    via dpll_pin_ref_sync_pair_add()
>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
>   drivers/dpll/zl3073x/dpll.c | 207 +++++++++++++++++++++++++++++++++++-
>   1 file changed, 206 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
> index 276f0a92db0b1..8010e2635f641 100644
> --- a/drivers/dpll/zl3073x/dpll.c
> +++ b/drivers/dpll/zl3073x/dpll.c
> @@ -13,6 +13,7 @@
>   #include <linux/module.h>
>   #include <linux/netlink.h>
>   #include <linux/platform_device.h>
> +#include <linux/property.h>
>   #include <linux/slab.h>
>   #include <linux/sprintf.h>
>   
> @@ -30,6 +31,7 @@
>    * @dpll: DPLL the pin is registered to
>    * @dpll_pin: pointer to registered dpll_pin
>    * @tracker: tracking object for the acquired reference
> + * @fwnode: firmware node handle
>    * @label: package label
>    * @dir: pin direction
>    * @id: pin id
> @@ -45,6 +47,7 @@ struct zl3073x_dpll_pin {
>   	struct zl3073x_dpll	*dpll;
>   	struct dpll_pin		*dpll_pin;
>   	dpll_tracker		tracker;
> +	struct fwnode_handle	*fwnode;
>   	char			label[8];
>   	enum dpll_pin_direction	dir;
>   	u8			id;
> @@ -184,6 +187,109 @@ zl3073x_dpll_input_pin_esync_set(const struct dpll_pin *dpll_pin,
>   	return zl3073x_ref_state_set(zldev, ref_id, &ref);
>   }
>   
> +static int
> +zl3073x_dpll_input_pin_ref_sync_get(const struct dpll_pin *dpll_pin,
> +				    void *pin_priv,
> +				    const struct dpll_pin *ref_sync_pin,
> +				    void *ref_sync_pin_priv,
> +				    enum dpll_pin_state *state,
> +				    struct netlink_ext_ack *extack)
> +{
> +	struct zl3073x_dpll_pin *sync_pin = ref_sync_pin_priv;
> +	struct zl3073x_dpll_pin *pin = pin_priv;
> +	struct zl3073x_dpll *zldpll = pin->dpll;
> +	struct zl3073x_dev *zldev = zldpll->dev;
> +	const struct zl3073x_ref *ref;
> +	u8 ref_id, mode, pair;
> +
> +	ref_id = zl3073x_input_pin_ref_get(pin->id);
> +	ref = zl3073x_ref_state_get(zldev, ref_id);
> +	mode = zl3073x_ref_sync_mode_get(ref);
> +	pair = zl3073x_ref_sync_pair_get(ref);
> +
> +	if (mode == ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR &&
> +	    pair == zl3073x_input_pin_ref_get(sync_pin->id))
> +		*state = DPLL_PIN_STATE_CONNECTED;
> +	else
> +		*state = DPLL_PIN_STATE_DISCONNECTED;
> +
> +	return 0;
> +}
> +
> +static int
> +zl3073x_dpll_input_pin_ref_sync_set(const struct dpll_pin *dpll_pin,
> +				    void *pin_priv,
> +				    const struct dpll_pin *ref_sync_pin,
> +				    void *ref_sync_pin_priv,
> +				    const enum dpll_pin_state state,
> +				    struct netlink_ext_ack *extack)
> +{
> +	struct zl3073x_dpll_pin *sync_pin = ref_sync_pin_priv;
> +	struct zl3073x_dpll_pin *pin = pin_priv;
> +	struct zl3073x_dpll *zldpll = pin->dpll;
> +	struct zl3073x_dev *zldev = zldpll->dev;
> +	u8 mode, ref_id, sync_ref_id;
> +	struct zl3073x_chan chan;
> +	struct zl3073x_ref ref;
> +	int rc;
> +
> +	ref_id = zl3073x_input_pin_ref_get(pin->id);
> +	sync_ref_id = zl3073x_input_pin_ref_get(sync_pin->id);
> +	ref = *zl3073x_ref_state_get(zldev, ref_id);
> +
> +	if (state == DPLL_PIN_STATE_CONNECTED) {
> +		const struct zl3073x_ref *sync_ref;
> +		u32 ref_freq, sync_freq;
> +
> +		sync_ref = zl3073x_ref_state_get(zldev, sync_ref_id);
> +		ref_freq = zl3073x_ref_freq_get(&ref);
> +		sync_freq = zl3073x_ref_freq_get(sync_ref);
> +
> +		/* Sync signal must be 8 kHz or less and clock reference
> +		 * must be 1 kHz or more and higher than the sync signal.
> +		 */
> +		if (sync_freq > 8000) {
> +			NL_SET_ERR_MSG(extack,
> +				       "sync frequency must be 8 kHz or less");
> +			return -EINVAL;
> +		}
> +		if (ref_freq < 1000) {
> +			NL_SET_ERR_MSG(extack,
> +				       "clock frequency must be 1 kHz or more");
> +			return -EINVAL;
> +		}
> +		if (ref_freq <= sync_freq) {
> +			NL_SET_ERR_MSG(extack,
> +				       "clock frequency must be higher than sync frequency");
> +			return -EINVAL;
> +		}
> +
> +		zl3073x_ref_sync_pair_set(&ref, sync_ref_id);
> +		mode = ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR;
> +	} else {
> +		mode = ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR_OFF;
> +	}
> +
> +	zl3073x_ref_sync_mode_set(&ref, mode);
> +
> +	rc = zl3073x_ref_state_set(zldev, ref_id, &ref);
> +	if (rc)
> +		return rc;
> +
> +	/* Exclude sync source from automatic reference selection by setting
> +	 * its priority to NONE. On disconnect the priority is left as NONE
> +	 * and the user must explicitly make the pin selectable again.
> +	 */
> +	if (state == DPLL_PIN_STATE_CONNECTED) {
> +		chan = *zl3073x_chan_state_get(zldev, zldpll->id);
> +		zl3073x_chan_ref_prio_set(&chan, sync_ref_id,
> +					  ZL_DPLL_REF_PRIO_NONE);
> +		return zl3073x_chan_state_set(zldev, zldpll->id, &chan);
> +	}
> +
> +	return 0;
> +}
> +
>   static int
>   zl3073x_dpll_input_pin_ffo_get(const struct dpll_pin *dpll_pin, void *pin_priv,
>   			       const struct dpll_device *dpll, void *dpll_priv,
> @@ -1100,6 +1206,8 @@ static const struct dpll_pin_ops zl3073x_dpll_input_pin_ops = {
>   	.phase_adjust_set = zl3073x_dpll_input_pin_phase_adjust_set,
>   	.prio_get = zl3073x_dpll_input_pin_prio_get,
>   	.prio_set = zl3073x_dpll_input_pin_prio_set,
> +	.ref_sync_get = zl3073x_dpll_input_pin_ref_sync_get,
> +	.ref_sync_set = zl3073x_dpll_input_pin_ref_sync_set,
>   	.state_on_dpll_get = zl3073x_dpll_input_pin_state_on_dpll_get,
>   	.state_on_dpll_set = zl3073x_dpll_input_pin_state_on_dpll_set,
>   };
> @@ -1190,8 +1298,11 @@ zl3073x_dpll_pin_register(struct zl3073x_dpll_pin *pin, u32 index)
>   	if (IS_ERR(props))
>   		return PTR_ERR(props);
>   
> -	/* Save package label, esync capability and phase adjust granularity */
> +	/* Save package label, fwnode, esync capability and phase adjust
> +	 * granularity.
> +	 */
>   	strscpy(pin->label, props->package_label);
> +	pin->fwnode = fwnode_handle_get(props->fwnode);
>   	pin->esync_control = props->esync_control;
>   	pin->phase_gran = props->dpll_props.phase_gran;
>   
> @@ -1236,6 +1347,8 @@ zl3073x_dpll_pin_register(struct zl3073x_dpll_pin *pin, u32 index)
>   	dpll_pin_put(pin->dpll_pin, &pin->tracker);
>   	pin->dpll_pin = NULL;
>   err_pin_get:
> +	fwnode_handle_put(pin->fwnode);
> +	pin->fwnode = NULL;
>   	zl3073x_pin_props_put(props);
>   
>   	return rc;
> @@ -1265,6 +1378,9 @@ zl3073x_dpll_pin_unregister(struct zl3073x_dpll_pin *pin)
>   
>   	dpll_pin_put(pin->dpll_pin, &pin->tracker);
>   	pin->dpll_pin = NULL;
> +
> +	fwnode_handle_put(pin->fwnode);
> +	pin->fwnode = NULL;
>   }
>   
>   /**
> @@ -1735,6 +1851,88 @@ zl3073x_dpll_free(struct zl3073x_dpll *zldpll)
>   	kfree(zldpll);
>   }
>   
> +/**
> + * zl3073x_dpll_ref_sync_pair_register - register ref_sync pairs for a pin
> + * @pin: pointer to zl3073x_dpll_pin structure
> + *
> + * Iterates 'ref-sync-sources' phandles in the pin's firmware node and
> + * registers each declared pairing.
> + *
> + * Return: 0 on success, <0 on error
> + */
> +static int
> +zl3073x_dpll_ref_sync_pair_register(struct zl3073x_dpll_pin *pin)
> +{
> +	struct zl3073x_dev *zldev = pin->dpll->dev;
> +	struct fwnode_handle *fwnode;
> +	struct dpll_pin *sync_pin;
> +	dpll_tracker tracker;
> +	int n, rc;
> +
> +	for (n = 0; ; n++) {
> +		/* Get n'th ref-sync source */
> +		fwnode = fwnode_find_reference(pin->fwnode, "ref-sync-sources",
> +					       n);
> +		if (IS_ERR(fwnode)) {
> +			rc = PTR_ERR(fwnode);
> +			break;
> +		}
> +
> +		/* Find associated dpll pin */
> +		sync_pin = fwnode_dpll_pin_find(fwnode, &tracker);
> +		fwnode_handle_put(fwnode);
> +		if (!sync_pin) {
> +			dev_warn(zldev->dev, "%s: ref-sync source %d not found",
> +				 pin->label, n);
> +			continue;
> +		}
> +
> +		/* Register new ref-sync pair */
> +		rc = dpll_pin_ref_sync_pair_add(pin->dpll_pin, sync_pin);
> +		dpll_pin_put(sync_pin, &tracker);
> +
> +		/* -EBUSY means pairing already exists from another DPLL's
> +		 * registration.
> +		 */
> +		if (rc && rc != -EBUSY) {
> +			dev_err(zldev->dev,
> +				"%s: failed to add ref-sync source %d: %pe",
> +				pin->label, n, ERR_PTR(rc));
> +			break;
> +		}
> +	}
> +
> +	return rc != -ENOENT ? rc : 0;
> +}
> +
> +/**
> + * zl3073x_dpll_ref_sync_pairs_register - register ref_sync pairs for a DPLL
> + * @zldpll: pointer to zl3073x_dpll structure
> + *
> + * Iterates all registered input pins of the given DPLL and establishes
> + * ref_sync pairings declared by 'ref-sync-sources' phandles in the
> + * device tree.
> + *
> + * Return: 0 on success, <0 on error
> + */
> +static int
> +zl3073x_dpll_ref_sync_pairs_register(struct zl3073x_dpll *zldpll)
> +{
> +	struct zl3073x_dpll_pin *pin;
> +	int rc;
> +
> +	list_for_each_entry(pin, &zldpll->pins, list) {
> +		if (!zl3073x_dpll_is_input_pin(pin) || !pin->fwnode)
> +			continue;
> +
> +		rc = zl3073x_dpll_ref_sync_pair_register(pin);
> +		if (rc)
> +			return rc;
> +	}
> +
> +	return 0;
> +}
> +
>   /**
>    * zl3073x_dpll_register - register DPLL device and all its pins
>    * @zldpll: pointer to zl3073x_dpll structure
> @@ -1758,6 +1956,13 @@ zl3073x_dpll_register(struct zl3073x_dpll *zldpll)
>   		return rc;
>   	}
>   
> +	rc = zl3073x_dpll_ref_sync_pairs_register(zldpll);
> +	if (rc) {
> +		zl3073x_dpll_pins_unregister(zldpll);
> +		zl3073x_dpll_device_unregister(zldpll);
> +		return rc;
> +	}
> +
>   	return 0;
>   }
>   

LGTM.


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