[v3] ADF41513/ADF41510 PLL frequency synthesizers

[PATCH v3 3/6] iio: frequency: adf41513: handle LE synchronization feature

Posted by Rodrigo Alencar via B4 Relay 1 month ago

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

When LE sync is enabled, it is must be set after powering up and must be
disabled when powering down. It is recommended when using the PLL as
a frequency synthesizer, where reference signal will always be present
while the device is being configured.

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/frequency/adf41513.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/frequency/adf41513.c b/drivers/iio/frequency/adf41513.c
index 69dcbbc1f393..0cdf24989c93 100644
--- a/drivers/iio/frequency/adf41513.c
+++ b/drivers/iio/frequency/adf41513.c
@@ -220,6 +220,7 @@ struct adf41513_data {
 	bool phase_detector_polarity;
 
 	bool logic_lvl_1v8_en;
+	bool le_sync_en;
 };
 
 struct adf41513_pll_settings {
@@ -697,13 +698,25 @@ static int adf41513_set_frequency(struct adf41513_state *st, u64 freq_uhz, u16 s
 static int adf41513_suspend(struct adf41513_state *st)
 {
 	st->regs[ADF41513_REG6] |= FIELD_PREP(ADF41513_REG6_POWER_DOWN_MSK, 1);
+	st->regs[ADF41513_REG12] &= ~ADF41513_REG12_LE_SELECT_MSK;
 	return adf41513_sync_config(st, ADF41513_SYNC_DIFF);
 }
 
 static int adf41513_resume(struct adf41513_state *st)
 {
+	int ret;
+
 	st->regs[ADF41513_REG6] &= ~ADF41513_REG6_POWER_DOWN_MSK;
-	return adf41513_sync_config(st, ADF41513_SYNC_DIFF);
+	ret = adf41513_sync_config(st, ADF41513_SYNC_DIFF);
+	if (ret < 0)
+		return ret;
+
+	if (st->data.le_sync_en) {
+		st->regs[ADF41513_REG12] |= ADF41513_REG12_LE_SELECT_MSK;
+		ret = adf41513_sync_config(st, ADF41513_SYNC_DIFF);
+	}
+
+	return ret;
 }
 
 static ssize_t adf41513_read_uhz(struct iio_dev *indio_dev,
@@ -994,6 +1007,8 @@ static int adf41513_parse_fw(struct adf41513_state *st)
 		st->data.lock_detect_count = tmp;
 	}
 
+	/* load enable sync */
+	st->data.le_sync_en = device_property_read_bool(dev, "adi,le-sync-enable");
 	st->data.freq_resolution_uhz = MICROHZ_PER_HZ;
 
 	return 0;
@@ -1001,6 +1016,7 @@ static int adf41513_parse_fw(struct adf41513_state *st)
 
 static int adf41513_setup(struct adf41513_state *st)
 {
+	int ret;
 	u32 tmp;
 
 	memset(st->regs_hw, 0xFF, sizeof(st->regs_hw));
@@ -1034,8 +1050,18 @@ static int adf41513_setup(struct adf41513_state *st)
 					      st->data.logic_lvl_1v8_en ? 0 : 1);
 
 	/* perform initialization sequence with power-up frequency */
-	return adf41513_set_frequency(st, st->data.power_up_frequency_hz * MICROHZ_PER_HZ,
-				      ADF41513_SYNC_ALL);
+	ret = adf41513_set_frequency(st,
+				     st->data.power_up_frequency_hz * MICROHZ_PER_HZ,
+				     ADF41513_SYNC_ALL);
+	if (ret < 0)
+		return ret;
+
+	if (st->data.le_sync_en) {
+		st->regs[ADF41513_REG12] |= ADF41513_REG12_LE_SELECT_MSK;
+		ret = adf41513_sync_config(st, ADF41513_SYNC_DIFF);
+	}
+
+	return ret;
 }
 
 static void adf41513_power_down(void *data)

-- 
2.43.0

Re: [PATCH v3 3/6] iio: frequency: adf41513: handle LE synchronization feature

Posted by Jonathan Cameron 4 weeks, 1 day ago

On Thu, 08 Jan 2026 12:14:52 +0000
Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:

> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> 
> When LE sync is enabled, it is must be set after powering up and must be
> disabled when powering down. It is recommended when using the PLL as
> a frequency synthesizer, where reference signal will always be present
> while the device is being configured.
> 
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
Hi Rodrigo,

A few comments inline.

> ---
>  drivers/iio/frequency/adf41513.c | 32 +++++++++++++++++++++++++++++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/frequency/adf41513.c b/drivers/iio/frequency/adf41513.c
> index 69dcbbc1f393..0cdf24989c93 100644
> --- a/drivers/iio/frequency/adf41513.c
> +++ b/drivers/iio/frequency/adf41513.c
> @@ -220,6 +220,7 @@ struct adf41513_data {
>  	bool phase_detector_polarity;
>  
>  	bool logic_lvl_1v8_en;
> +	bool le_sync_en;
>  };
>  
>  struct adf41513_pll_settings {
> @@ -697,13 +698,25 @@ static int adf41513_set_frequency(struct adf41513_state *st, u64 freq_uhz, u16 s
>  static int adf41513_suspend(struct adf41513_state *st)
>  {
>  	st->regs[ADF41513_REG6] |= FIELD_PREP(ADF41513_REG6_POWER_DOWN_MSK, 1);
> +	st->regs[ADF41513_REG12] &= ~ADF41513_REG12_LE_SELECT_MSK;
>  	return adf41513_sync_config(st, ADF41513_SYNC_DIFF);
>  }
>  
>  static int adf41513_resume(struct adf41513_state *st)
>  {
> +	int ret;
> +
>  	st->regs[ADF41513_REG6] &= ~ADF41513_REG6_POWER_DOWN_MSK;
> -	return adf41513_sync_config(st, ADF41513_SYNC_DIFF);
> +	ret = adf41513_sync_config(st, ADF41513_SYNC_DIFF);
> +	if (ret < 0)
If you know it is either 0 for good or less than zero, prefer
	if (ret) (for reason that follows)

> +		return ret;
> +
> +	if (st->data.le_sync_en) {
> +		st->regs[ADF41513_REG12] |= ADF41513_REG12_LE_SELECT_MSK;
> +		ret = adf41513_sync_config(st, ADF41513_SYNC_DIFF);
Similar to below - I'd like to see 	
		if (ret)
			return ret;
here to avoid returning stale parameter from above (which might be postive
based on local code).

> +	}
> +
> +	return ret;
>  }
>  
>  static ssize_t adf41513_read_uhz(struct iio_dev *indio_dev,
> @@ -994,6 +1007,8 @@ static int adf41513_parse_fw(struct adf41513_state *st)
>  		st->data.lock_detect_count = tmp;
>  	}
>  
> +	/* load enable sync */
> +	st->data.le_sync_en = device_property_read_bool(dev, "adi,le-sync-enable");
>  	st->data.freq_resolution_uhz = MICROHZ_PER_HZ;
>  
>  	return 0;
> @@ -1001,6 +1016,7 @@ static int adf41513_parse_fw(struct adf41513_state *st)
>  
>  static int adf41513_setup(struct adf41513_state *st)
>  {
> +	int ret;
>  	u32 tmp;
>  
>  	memset(st->regs_hw, 0xFF, sizeof(st->regs_hw));
> @@ -1034,8 +1050,18 @@ static int adf41513_setup(struct adf41513_state *st)
>  					      st->data.logic_lvl_1v8_en ? 0 : 1);
>  
>  	/* perform initialization sequence with power-up frequency */
> -	return adf41513_set_frequency(st, st->data.power_up_frequency_hz * MICROHZ_PER_HZ,
> -				      ADF41513_SYNC_ALL);
> +	ret = adf41513_set_frequency(st,
> +				     st->data.power_up_frequency_hz * MICROHZ_PER_HZ,
> +				     ADF41513_SYNC_ALL);
> +	if (ret < 0)
	if (ret)
		assuming set_frequency never returns a positive.

> +		return ret;
> +
> +	if (st->data.le_sync_en) {
> +		st->regs[ADF41513_REG12] |= ADF41513_REG12_LE_SELECT_MSK;
> +		ret = adf41513_sync_config(st, ADF41513_SYNC_DIFF);
Slightly preference for
		if (ret)
			return ret;
	}

	return 0;

Because otherwise a 'stale' (though it is zero) return value is used and that sort
of code pattern tends to be a little fragile and hard to read.

> +	}
> +
> +	return ret;
>  }
>  
>  static void adf41513_power_down(void *data)
>

[PATCH v3 1/6] dt-bindings: iio: frequency: add adf41513
[PATCH v3 2/6] iio: frequency: adf41513: driver implementation
[PATCH v3 3/6] iio: frequency: adf41513: handle LE synchronization feature
[PATCH v3 4/6] iio: frequency: adf41513: features on frequency change
[PATCH v3 5/6] docs: iio: add documentation for adf41513 driver
[PATCH v3 6/6] Documentation: ABI: testing: add common ABI file for iio/frequency