[PATCH v4 0/3] ADF4371 refin mode and doubler support

[PATCH v4 0/3] ADF4371 refin mode and doubler support

[Antoniu Miclaus]

Add support for selecting between single-ended and differential
reference input. By default the single-ended input is enabled.

Input frequency boundaries are change based on the mode selected
(single-ended/differential).

Add support for the reference doubler. This feature is enabled
automatically to improve noise performance if the input frequency
is within the accepted range.

Antoniu Miclaus (3):
  dt-bindings: iio: adf4371: add refin mode
  iio: frequency: adf4371: add refin mode
  iio: frequency: adf4371: add ref doubler

 .../bindings/iio/frequency/adf4371.yaml       |  7 ++-
 drivers/iio/frequency/adf4371.c               | 44 +++++++++++++++++--
 2 files changed, 45 insertions(+), 6 deletions(-)

-- 
2.47.1

[PATCH v4 1/3] dt-bindings: iio: adf4371: add refin mode

[Antoniu Miclaus]

Add support for selecting between single-ended and differential
reference input.

By default the single-ended input is enabled.

Input frequency boundaries are change based on the mode selected
(single-ended/differential).

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v4:
 - add enum in clock-names to support both se and diff input.
 .../devicetree/bindings/iio/frequency/adf4371.yaml         | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml b/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml
index 1cb2adaf66f9..9ad5b22c59d7 100644
--- a/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml
+++ b/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml
@@ -30,8 +30,11 @@ properties:
 
   clock-names:
     description:
-      Must be "clkin"
-    maxItems: 1
+      Must be "clkin" if the input reference is single ended or "clkin-diff"
+      if the input reference is differential. By default single ended input is
+      applied.
+    enum: [clkin, clkin-diff]
+    default: clkin
 
   adi,mute-till-lock-en:
     type: boolean
-- 
2.47.1

Re: [PATCH v4 1/3] dt-bindings: iio: adf4371: add refin mode

[Jonathan Cameron]

On Thu, 9 Jan 2025 15:37:05 +0200
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> Add support for selecting between single-ended and differential
> reference input.
> 
> By default the single-ended input is enabled.
> 
> Input frequency boundaries are change based on the mode selected
> (single-ended/differential).
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> changes in v4:
>  - add enum in clock-names to support both se and diff input.
>  .../devicetree/bindings/iio/frequency/adf4371.yaml         | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml b/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml
> index 1cb2adaf66f9..9ad5b22c59d7 100644
> --- a/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml
> +++ b/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml
> @@ -30,8 +30,11 @@ properties:
>  
>    clock-names:
>      description:
> -      Must be "clkin"
> -    maxItems: 1
> +      Must be "clkin" if the input reference is single ended or "clkin-diff"
> +      if the input reference is differential. By default single ended input is
> +      applied.
> +    enum: [clkin, clkin-diff]
> +    default: clkin
Given the property is required, what does a default mean?

I don't think you want to apply a default, or mention it in the description.
Otherwise LGTM.

>  
>    adi,mute-till-lock-en:
>      type: boolean

Re: [PATCH v4 1/3] dt-bindings: iio: adf4371: add refin mode

[Krzysztof Kozlowski]

On Thu, Jan 09, 2025 at 03:37:05PM +0200, Antoniu Miclaus wrote:
>    clock-names:
>      description:
> -      Must be "clkin"
> -    maxItems: 1
> +      Must be "clkin" if the input reference is single ended or "clkin-diff"
> +      if the input reference is differential. By default single ended input is
> +      applied.
> +    enum: [clkin, clkin-diff]
> +    default: clkin

Which pins are these? I went through adf4371 datasheet and no reference
on clock inputs like clkin or clkin-diff.

Best regards,
Krzysztof

Re: [PATCH v4 1/3] dt-bindings: iio: adf4371: add refin mode

[Nuno Sá]

On Mon, 2025-01-13 at 09:37 +0100, Krzysztof Kozlowski wrote:
> On Thu, Jan 09, 2025 at 03:37:05PM +0200, Antoniu Miclaus wrote:
> >    clock-names:
> >      description:
> > -      Must be "clkin"
> > -    maxItems: 1
> > +      Must be "clkin" if the input reference is single ended or "clkin-
> > diff"
> > +      if the input reference is differential. By default single ended input
> > is
> > +      applied.
> > +    enum: [clkin, clkin-diff]
> > +    default: clkin
> 
> Which pins are these? I went through adf4371 datasheet and no reference
> on clock inputs like clkin or clkin-diff.
> 
> 

Hmm, I guess we should call this 'refp' and 'refp-n' then (the latter seems a
bit more odd)? Or just 'ref' and 'ref-diff'?

- Nuno Sá

RE: [PATCH v4 1/3] dt-bindings: iio: adf4371: add refin mode

[Miclaus, Antoniu]

--
Antoniu Miclăuş

> -----Original Message-----
> From: Nuno Sá <noname.nuno@gmail.com>
> Sent: Monday, January 13, 2025 11:56 AM
> To: Krzysztof Kozlowski <krzk@kernel.org>; Miclaus, Antoniu
> <Antoniu.Miclaus@analog.com>
> Cc: jic23@kernel.org; robh@kernel.org; conor+dt@kernel.org; linux-
> iio@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-pwm@vger.kernel.org
> Subject: Re: [PATCH v4 1/3] dt-bindings: iio: adf4371: add refin mode
> 
> [External]
> 
> On Mon, 2025-01-13 at 09:37 +0100, Krzysztof Kozlowski wrote:
> > On Thu, Jan 09, 2025 at 03:37:05PM +0200, Antoniu Miclaus wrote:
> > >    clock-names:
> > >      description:
> > > -      Must be "clkin"
> > > -    maxItems: 1
> > > +      Must be "clkin" if the input reference is single ended or "clkin-
> > > diff"
> > > +      if the input reference is differential. By default single ended input
> > > is
> > > +      applied.
> > > +    enum: [clkin, clkin-diff]
> > > +    default: clkin
> >
> > Which pins are these? I went through adf4371 datasheet and no reference
> > on clock inputs like clkin or clkin-diff.
> >
> >
> 
> Hmm, I guess we should call this 'refp' and 'refp-n' then (the latter seems a
> bit more odd)? Or just 'ref' and 'ref-diff'?

Regarding both naming and approach, I followed Jonathan's suggestion in v3.
Initially it was a separate property for refin mode (see previous versions).
If he is fine with changing again the name/approach I'll do it, but I'd wait for
his comment.

> - Nuno Sá

Re: [PATCH v4 1/3] dt-bindings: iio: adf4371: add refin mode

[Krzysztof Kozlowski]

On 13/01/2025 10:55, Nuno Sá wrote:
> On Mon, 2025-01-13 at 09:37 +0100, Krzysztof Kozlowski wrote:
>> On Thu, Jan 09, 2025 at 03:37:05PM +0200, Antoniu Miclaus wrote:
>>>    clock-names:
>>>      description:
>>> -      Must be "clkin"
>>> -    maxItems: 1
>>> +      Must be "clkin" if the input reference is single ended or "clkin-
>>> diff"
>>> +      if the input reference is differential. By default single ended input
>>> is
>>> +      applied.
>>> +    enum: [clkin, clkin-diff]
>>> +    default: clkin
>>
>> Which pins are these? I went through adf4371 datasheet and no reference
>> on clock inputs like clkin or clkin-diff.
>>
>>
> 
> Hmm, I guess we should call this 'refp' and 'refp-n' then (the latter seems a
> bit more odd)? Or just 'ref' and 'ref-diff'?

That mistake was done at the beginning - the "clkin" is just useless
name. It cannot be "clkout" and it cannot be anything else than clk, so
it is 100% redundant.

But looking for pins brought second point - here you claim these are
mutually exclusive while datasheet suggests that both inputs can be
connected. Unless they come from the same source always?

Best regards,
Krzysztof

Re: [PATCH v4 1/3] dt-bindings: iio: adf4371: add refin mode

[Nuno Sá]

On Mon, 2025-01-13 at 11:21 +0100, Krzysztof Kozlowski wrote:
> On 13/01/2025 10:55, Nuno Sá wrote:
> > On Mon, 2025-01-13 at 09:37 +0100, Krzysztof Kozlowski wrote:
> > > On Thu, Jan 09, 2025 at 03:37:05PM +0200, Antoniu Miclaus wrote:
> > > >    clock-names:
> > > >      description:
> > > > -      Must be "clkin"
> > > > -    maxItems: 1
> > > > +      Must be "clkin" if the input reference is single ended or "clkin-
> > > > diff"
> > > > +      if the input reference is differential. By default single ended
> > > > input
> > > > is
> > > > +      applied.
> > > > +    enum: [clkin, clkin-diff]
> > > > +    default: clkin
> > > 
> > > Which pins are these? I went through adf4371 datasheet and no reference
> > > on clock inputs like clkin or clkin-diff.
> > > 
> > > 
> > 
> > Hmm, I guess we should call this 'refp' and 'refp-n' then (the latter seems
> > a
> > bit more odd)? Or just 'ref' and 'ref-diff'?
> 
> That mistake was done at the beginning - the "clkin" is just useless
> name. It cannot be "clkout" and it cannot be anything else than clk, so
> it is 100% redundant.
> 

Oh sure... Makes sense and I forgot that the property is not new...

> But looking for pins brought second point - here you claim these are
> mutually exclusive while datasheet suggests that both inputs can be
> connected. Unless they come from the same source always?
> 

If you have a single ended input then only one pin (the positive one) will be
used. If the input signal is differential, then both pins will be used. So they
are mutually exclusive... You either have single ended or a differential input. 
And depending on the input type, the limit of the input frequency varies.

- Nuno Sá

Re: [PATCH v4 1/3] dt-bindings: iio: adf4371: add refin mode

[Krzysztof Kozlowski]

On 13/01/2025 12:17, Nuno Sá wrote:
> On Mon, 2025-01-13 at 11:21 +0100, Krzysztof Kozlowski wrote:
>> On 13/01/2025 10:55, Nuno Sá wrote:
>>> On Mon, 2025-01-13 at 09:37 +0100, Krzysztof Kozlowski wrote:
>>>> On Thu, Jan 09, 2025 at 03:37:05PM +0200, Antoniu Miclaus wrote:
>>>>>    clock-names:
>>>>>      description:
>>>>> -      Must be "clkin"
>>>>> -    maxItems: 1
>>>>> +      Must be "clkin" if the input reference is single ended or "clkin-
>>>>> diff"
>>>>> +      if the input reference is differential. By default single ended
>>>>> input
>>>>> is
>>>>> +      applied.
>>>>> +    enum: [clkin, clkin-diff]
>>>>> +    default: clkin
>>>>
>>>> Which pins are these? I went through adf4371 datasheet and no reference
>>>> on clock inputs like clkin or clkin-diff.
>>>>
>>>>
>>>
>>> Hmm, I guess we should call this 'refp' and 'refp-n' then (the latter seems
>>> a
>>> bit more odd)? Or just 'ref' and 'ref-diff'?
>>
>> That mistake was done at the beginning - the "clkin" is just useless
>> name. It cannot be "clkout" and it cannot be anything else than clk, so
>> it is 100% redundant.
>>
> 
> Oh sure... Makes sense and I forgot that the property is not new...
> 
>> But looking for pins brought second point - here you claim these are
>> mutually exclusive while datasheet suggests that both inputs can be
>> connected. Unless they come from the same source always?
>>
> 
> If you have a single ended input then only one pin (the positive one) will be
> used. If the input signal is differential, then both pins will be used. So they

But the clocks describe input pins, at least in typical case, so that's
my question: how many clock sources do you have here? One or two?

> are mutually exclusive... You either have single ended or a differential input. 
> And depending on the input type, the limit of the input frequency varies.

Based on this, this is the same clock, so using "diff" is not a property
of "clocks". Look at other bindings how they encode differential choice
for some signals - usually bool property, but not always - see other
adi/admv devices.

Best regards,
Krzysztof

RE: [PATCH v4 1/3] dt-bindings: iio: adf4371: add refin mode

[Miclaus, Antoniu]

--
Antoniu Miclăuş

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Monday, January 13, 2025 1:24 PM
> To: Nuno Sá <noname.nuno@gmail.com>; Miclaus, Antoniu
> <Antoniu.Miclaus@analog.com>
> Cc: jic23@kernel.org; robh@kernel.org; conor+dt@kernel.org; linux-
> iio@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-pwm@vger.kernel.org
> Subject: Re: [PATCH v4 1/3] dt-bindings: iio: adf4371: add refin mode
> 
> [External]
> 
> On 13/01/2025 12:17, Nuno Sá wrote:
> > On Mon, 2025-01-13 at 11:21 +0100, Krzysztof Kozlowski wrote:
> >> On 13/01/2025 10:55, Nuno Sá wrote:
> >>> On Mon, 2025-01-13 at 09:37 +0100, Krzysztof Kozlowski wrote:
> >>>> On Thu, Jan 09, 2025 at 03:37:05PM +0200, Antoniu Miclaus wrote:
> >>>>>    clock-names:
> >>>>>      description:
> >>>>> -      Must be "clkin"
> >>>>> -    maxItems: 1
> >>>>> +      Must be "clkin" if the input reference is single ended or "clkin-
> >>>>> diff"
> >>>>> +      if the input reference is differential. By default single ended
> >>>>> input
> >>>>> is
> >>>>> +      applied.
> >>>>> +    enum: [clkin, clkin-diff]
> >>>>> +    default: clkin
> >>>>
> >>>> Which pins are these? I went through adf4371 datasheet and no
> reference
> >>>> on clock inputs like clkin or clkin-diff.
> >>>>
> >>>>
> >>>
> >>> Hmm, I guess we should call this 'refp' and 'refp-n' then (the latter seems
> >>> a
> >>> bit more odd)? Or just 'ref' and 'ref-diff'?
> >>
> >> That mistake was done at the beginning - the "clkin" is just useless
> >> name. It cannot be "clkout" and it cannot be anything else than clk, so
> >> it is 100% redundant.
> >>
> >
> > Oh sure... Makes sense and I forgot that the property is not new...
> >
> >> But looking for pins brought second point - here you claim these are
> >> mutually exclusive while datasheet suggests that both inputs can be
> >> connected. Unless they come from the same source always?
> >>
> >
> > If you have a single ended input then only one pin (the positive one) will be
> > used. If the input signal is differential, then both pins will be used. So they
> 
> But the clocks describe input pins, at least in typical case, so that's
> my question: how many clock sources do you have here? One or two?
> 
> > are mutually exclusive... You either have single ended or a differential input.
> > And depending on the input type, the limit of the input frequency varies.
> 
> Based on this, this is the same clock, so using "diff" is not a property
> of "clocks". Look at other bindings how they encode differential choice
> for some signals - usually bool property, but not always - see other
> adi/admv devices.

This approach that you suggest was implemented in patch series v1 (as boolean) / v3 (as enum).
Based on feedback in v3 received from Jonathan I switched to this. Should I revert it?

> Best regards,
> Krzysztof

Re: [PATCH v4 1/3] dt-bindings: iio: adf4371: add refin mode

[Krzysztof Kozlowski]

On 13/01/2025 12:50, Miclaus, Antoniu wrote:
>>>
>>> Oh sure... Makes sense and I forgot that the property is not new...
>>>
>>>> But looking for pins brought second point - here you claim these are
>>>> mutually exclusive while datasheet suggests that both inputs can be
>>>> connected. Unless they come from the same source always?
>>>>
>>>
>>> If you have a single ended input then only one pin (the positive one) will be
>>> used. If the input signal is differential, then both pins will be used. So they
>>
>> But the clocks describe input pins, at least in typical case, so that's
>> my question: how many clock sources do you have here? One or two?
>>
>>> are mutually exclusive... You either have single ended or a differential input.
>>> And depending on the input type, the limit of the input frequency varies.
>>
>> Based on this, this is the same clock, so using "diff" is not a property
>> of "clocks". Look at other bindings how they encode differential choice
>> for some signals - usually bool property, but not always - see other
>> adi/admv devices.
> 
> This approach that you suggest was implemented in patch series v1 (as boolean) / v3 (as enum).
> Based on feedback in v3 received from Jonathan I switched to this. Should I revert it?

I see:
https://lore.kernel.org/lkml/20241220195220.1e1e1d6f@jic23-huawei/

I think that v3 was preferred with arguments above. You have one clock
input and you want to configure the device differently, based on how
this clock is wired. But it is still one clock.

Wait for Jonathan before reverting to v3.

Best regards,
Krzysztof

Re: [PATCH v4 1/3] dt-bindings: iio: adf4371: add refin mode

[Jonathan Cameron]

On Mon, 13 Jan 2025 13:05:30 +0100
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On 13/01/2025 12:50, Miclaus, Antoniu wrote:
> >>>
> >>> Oh sure... Makes sense and I forgot that the property is not new...
> >>>  
> >>>> But looking for pins brought second point - here you claim these are
> >>>> mutually exclusive while datasheet suggests that both inputs can be
> >>>> connected. Unless they come from the same source always?
> >>>>  
> >>>
> >>> If you have a single ended input then only one pin (the positive one) will be
> >>> used. If the input signal is differential, then both pins will be used. So they  
> >>
> >> But the clocks describe input pins, at least in typical case, so that's
> >> my question: how many clock sources do you have here? One or two?
> >>  
> >>> are mutually exclusive... You either have single ended or a differential input.
> >>> And depending on the input type, the limit of the input frequency varies.  
> >>
> >> Based on this, this is the same clock, so using "diff" is not a property
> >> of "clocks". Look at other bindings how they encode differential choice
> >> for some signals - usually bool property, but not always - see other
> >> adi/admv devices.  
> > 
> > This approach that you suggest was implemented in patch series v1 (as boolean) / v3 (as enum).
> > Based on feedback in v3 received from Jonathan I switched to this. Should I revert it?  
> 
> I see:
> https://lore.kernel.org/lkml/20241220195220.1e1e1d6f@jic23-huawei/
> 
> I think that v3 was preferred with arguments above. You have one clock
> input and you want to configure the device differently, based on how
> this clock is wired. But it is still one clock.
> 
> Wait for Jonathan before reverting to v3.

This is following the approach we've previously used for crystal (xtal) vs clock
(typically one pin shared in those cases as well).

They are effectively different types of clock that might be connected.

e.g. adi,ad7173.yaml or adi,ad7192.yaml

I'm struggling to find the original discussion of that case.
Easy enough to find where they were added, but they mysteriously
just appear and I'm sure we had a detailed discussion prior to that.

I don't mind that much if we go to a boolean, the clock names just seems
cleaner to me as they are effectively different possible clock inputs
(with one shared pin). Whereas one of the pins is not shared as only applies
in the differential case.

Jonathan

> 
> Best regards,
> Krzysztof

[PATCH v4 2/3] iio: frequency: adf4371: add refin mode

[Antoniu Miclaus]

Add support for single-ended/differential reference input mode.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v4:
 - parse input clock depending on the name and adjust the refin mode
   accordingly.
 drivers/iio/frequency/adf4371.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/frequency/adf4371.c b/drivers/iio/frequency/adf4371.c
index b27088464826..50450e7b5464 100644
--- a/drivers/iio/frequency/adf4371.c
+++ b/drivers/iio/frequency/adf4371.c
@@ -41,6 +41,10 @@
 #define ADF4371_MOD2WORD_MSK		GENMASK(5, 0)
 #define ADF4371_MOD2WORD(x)		FIELD_PREP(ADF4371_MOD2WORD_MSK, x)
 
+/* ADF4371_REG22 */
+#define ADF4371_REFIN_MODE_MASK		BIT(6)
+#define ADF4371_REFIN_MODE(x)		FIELD_PREP(ADF4371_REFIN_MODE_MASK, x)
+
 /* ADF4371_REG24 */
 #define ADF4371_RF_DIV_SEL_MSK		GENMASK(6, 4)
 #define ADF4371_RF_DIV_SEL(x)		FIELD_PREP(ADF4371_RF_DIV_SEL_MSK, x)
@@ -69,6 +73,7 @@
 
 #define ADF4371_MAX_FREQ_PFD		250000000UL /* Hz */
 #define ADF4371_MAX_FREQ_REFIN		600000000UL /* Hz */
+#define ADF4371_MAX_FREQ_REFIN_SE	500000000UL /* Hz */
 
 /* MOD1 is a 24-bit primary modulus with fixed value of 2^25 */
 #define ADF4371_MODULUS1		33554432ULL
@@ -175,6 +180,7 @@ struct adf4371_state {
 	unsigned int mod2;
 	unsigned int rf_div_sel;
 	unsigned int ref_div_factor;
+	bool ref_diff_en;
 	u8 buf[10] __aligned(IIO_DMA_MINALIGN);
 };
 
@@ -503,6 +509,17 @@ static int adf4371_setup(struct adf4371_state *st)
 				 ADF4371_ADDR_ASC(1) | ADF4371_ADDR_ASC_R(1));
 	if (ret < 0)
 		return ret;
+
+	if ((st->ref_diff_en && st->clkin_freq > ADF4371_MAX_FREQ_REFIN) ||
+	    (!st->ref_diff_en && st->clkin_freq > ADF4371_MAX_FREQ_REFIN_SE))
+		return -EINVAL;
+
+	ret = regmap_update_bits(st->regmap,  ADF4371_REG(0x22),
+				 ADF4371_REFIN_MODE_MASK,
+				 ADF4371_REFIN_MODE(st->ref_diff_en));
+	if (ret < 0)
+		return ret;
+
 	/*
 	 * Calculate and maximize PFD frequency
 	 * fPFD = REFIN × ((1 + D)/(R × (1 + T)))
@@ -572,9 +589,16 @@ static int adf4371_probe(struct spi_device *spi)
 	indio_dev->channels = st->chip_info->channels;
 	indio_dev->num_channels = st->chip_info->num_channels;
 
+	st->ref_diff_en = false;
+
 	st->clkin = devm_clk_get_enabled(&spi->dev, "clkin");
-	if (IS_ERR(st->clkin))
-		return PTR_ERR(st->clkin);
+	if (IS_ERR(st->clkin)) {
+		st->clkin = devm_clk_get_enabled(&spi->dev, "clkin-diff");
+		if (IS_ERR(st->clkin))
+			return PTR_ERR(st->clkin);
+
+		st->ref_diff_en = true;
+	}
 
 	st->clkin_freq = clk_get_rate(st->clkin);
 
-- 
2.47.1

Re: [PATCH v4 2/3] iio: frequency: adf4371: add refin mode

[Nuno Sá]

On Thu, 2025-01-09 at 15:37 +0200, Antoniu Miclaus wrote:
> Add support for single-ended/differential reference input mode.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---

Reviewed-by: Nuno Sa <nunbo.sa@analog.com>

> changes in v4:
>  - parse input clock depending on the name and adjust the refin mode
>    accordingly.
>  drivers/iio/frequency/adf4371.c | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/frequency/adf4371.c b/drivers/iio/frequency/adf4371.c
> index b27088464826..50450e7b5464 100644
> --- a/drivers/iio/frequency/adf4371.c
> +++ b/drivers/iio/frequency/adf4371.c
> @@ -41,6 +41,10 @@
>  #define ADF4371_MOD2WORD_MSK		GENMASK(5, 0)
>  #define ADF4371_MOD2WORD(x)		FIELD_PREP(ADF4371_MOD2WORD_MSK, x)
>  
> +/* ADF4371_REG22 */
> +#define ADF4371_REFIN_MODE_MASK		BIT(6)
> +#define ADF4371_REFIN_MODE(x)		FIELD_PREP(ADF4371_REFIN_MODE_MASK,
> x)
> +
>  /* ADF4371_REG24 */
>  #define ADF4371_RF_DIV_SEL_MSK		GENMASK(6, 4)
>  #define ADF4371_RF_DIV_SEL(x)		FIELD_PREP(ADF4371_RF_DIV_SEL_MSK, x)
> @@ -69,6 +73,7 @@
>  
>  #define ADF4371_MAX_FREQ_PFD		250000000UL /* Hz */
>  #define ADF4371_MAX_FREQ_REFIN		600000000UL /* Hz */
> +#define ADF4371_MAX_FREQ_REFIN_SE	500000000UL /* Hz */
>  
>  /* MOD1 is a 24-bit primary modulus with fixed value of 2^25 */
>  #define ADF4371_MODULUS1		33554432ULL
> @@ -175,6 +180,7 @@ struct adf4371_state {
>  	unsigned int mod2;
>  	unsigned int rf_div_sel;
>  	unsigned int ref_div_factor;
> +	bool ref_diff_en;
>  	u8 buf[10] __aligned(IIO_DMA_MINALIGN);
>  };
>  
> @@ -503,6 +509,17 @@ static int adf4371_setup(struct adf4371_state *st)
>  				 ADF4371_ADDR_ASC(1) |
> ADF4371_ADDR_ASC_R(1));
>  	if (ret < 0)
>  		return ret;
> +
> +	if ((st->ref_diff_en && st->clkin_freq > ADF4371_MAX_FREQ_REFIN) ||
> +	    (!st->ref_diff_en && st->clkin_freq > ADF4371_MAX_FREQ_REFIN_SE))
> +		return -EINVAL;
> +
> +	ret = regmap_update_bits(st->regmap,  ADF4371_REG(0x22),
> +				 ADF4371_REFIN_MODE_MASK,
> +				 ADF4371_REFIN_MODE(st->ref_diff_en));
> +	if (ret < 0)
> +		return ret;
> +
>  	/*
>  	 * Calculate and maximize PFD frequency
>  	 * fPFD = REFIN × ((1 + D)/(R × (1 + T)))
> @@ -572,9 +589,16 @@ static int adf4371_probe(struct spi_device *spi)
>  	indio_dev->channels = st->chip_info->channels;
>  	indio_dev->num_channels = st->chip_info->num_channels;
>  
> +	st->ref_diff_en = false;
> +
>  	st->clkin = devm_clk_get_enabled(&spi->dev, "clkin");
> -	if (IS_ERR(st->clkin))
> -		return PTR_ERR(st->clkin);
> +	if (IS_ERR(st->clkin)) {
> +		st->clkin = devm_clk_get_enabled(&spi->dev, "clkin-diff");
> +		if (IS_ERR(st->clkin))
> +			return PTR_ERR(st->clkin);
> +
> +		st->ref_diff_en = true;
> +	}
>  
>  	st->clkin_freq = clk_get_rate(st->clkin);
>  

Re: [PATCH v4 2/3] iio: frequency: adf4371: add refin mode

[Nuno Sá]

On Mon, 2025-01-13 at 09:57 +0000, Nuno Sá wrote:
> On Thu, 2025-01-09 at 15:37 +0200, Antoniu Miclaus wrote:
> > Add support for single-ended/differential reference input mode.
> > 
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > ---
> 
> Reviewed-by: Nuno Sa <nunbo.sa@analog.com>

Ups, typo:

Reviewed-by: Nuno Sa <nuno.sa@analog.com>

> 
> > changes in v4:
> >  - parse input clock depending on the name and adjust the refin mode
> >    accordingly.
> >  drivers/iio/frequency/adf4371.c | 28 ++++++++++++++++++++++++++--
> >  1 file changed, 26 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/frequency/adf4371.c
> > b/drivers/iio/frequency/adf4371.c
> > index b27088464826..50450e7b5464 100644
> > --- a/drivers/iio/frequency/adf4371.c
> > +++ b/drivers/iio/frequency/adf4371.c
> > @@ -41,6 +41,10 @@
> >  #define ADF4371_MOD2WORD_MSK		GENMASK(5, 0)
> >  #define ADF4371_MOD2WORD(x)		FIELD_PREP(ADF4371_MOD2WORD_MSK, x)
> >  
> > +/* ADF4371_REG22 */
> > +#define ADF4371_REFIN_MODE_MASK		BIT(6)
> > +#define ADF4371_REFIN_MODE(x)		FIELD_PREP(ADF4371_REFIN_MODE_MASK,
> > x)
> > +
> >  /* ADF4371_REG24 */
> >  #define ADF4371_RF_DIV_SEL_MSK		GENMASK(6, 4)
> >  #define ADF4371_RF_DIV_SEL(x)		FIELD_PREP(ADF4371_RF_DIV_SEL_MSK,
> > x)
> > @@ -69,6 +73,7 @@
> >  
> >  #define ADF4371_MAX_FREQ_PFD		250000000UL /* Hz */
> >  #define ADF4371_MAX_FREQ_REFIN		600000000UL /* Hz */
> > +#define ADF4371_MAX_FREQ_REFIN_SE	500000000UL /* Hz */
> >  
> >  /* MOD1 is a 24-bit primary modulus with fixed value of 2^25 */
> >  #define ADF4371_MODULUS1		33554432ULL
> > @@ -175,6 +180,7 @@ struct adf4371_state {
> >  	unsigned int mod2;
> >  	unsigned int rf_div_sel;
> >  	unsigned int ref_div_factor;
> > +	bool ref_diff_en;
> >  	u8 buf[10] __aligned(IIO_DMA_MINALIGN);
> >  };
> >  
> > @@ -503,6 +509,17 @@ static int adf4371_setup(struct adf4371_state *st)
> >  				 ADF4371_ADDR_ASC(1) |
> > ADF4371_ADDR_ASC_R(1));
> >  	if (ret < 0)
> >  		return ret;
> > +
> > +	if ((st->ref_diff_en && st->clkin_freq > ADF4371_MAX_FREQ_REFIN) ||
> > +	    (!st->ref_diff_en && st->clkin_freq >
> > ADF4371_MAX_FREQ_REFIN_SE))
> > +		return -EINVAL;
> > +
> > +	ret = regmap_update_bits(st->regmap,  ADF4371_REG(0x22),
> > +				 ADF4371_REFIN_MODE_MASK,
> > +				 ADF4371_REFIN_MODE(st->ref_diff_en));
> > +	if (ret < 0)
> > +		return ret;
> > +
> >  	/*
> >  	 * Calculate and maximize PFD frequency
> >  	 * fPFD = REFIN × ((1 + D)/(R × (1 + T)))
> > @@ -572,9 +589,16 @@ static int adf4371_probe(struct spi_device *spi)
> >  	indio_dev->channels = st->chip_info->channels;
> >  	indio_dev->num_channels = st->chip_info->num_channels;
> >  
> > +	st->ref_diff_en = false;
> > +
> >  	st->clkin = devm_clk_get_enabled(&spi->dev, "clkin");
> > -	if (IS_ERR(st->clkin))
> > -		return PTR_ERR(st->clkin);
> > +	if (IS_ERR(st->clkin)) {
> > +		st->clkin = devm_clk_get_enabled(&spi->dev, "clkin-diff");
> > +		if (IS_ERR(st->clkin))
> > +			return PTR_ERR(st->clkin);
> > +
> > +		st->ref_diff_en = true;
> > +	}
> >  
> >  	st->clkin_freq = clk_get_rate(st->clkin);
> >  
> 

[PATCH v4 3/3] iio: frequency: adf4371: add ref doubler

[Antoniu Miclaus]

Add support for the reference doubler.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
no changes in v4.
 drivers/iio/frequency/adf4371.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/frequency/adf4371.c b/drivers/iio/frequency/adf4371.c
index 50450e7b5464..a57fc24a6799 100644
--- a/drivers/iio/frequency/adf4371.c
+++ b/drivers/iio/frequency/adf4371.c
@@ -44,6 +44,8 @@
 /* ADF4371_REG22 */
 #define ADF4371_REFIN_MODE_MASK		BIT(6)
 #define ADF4371_REFIN_MODE(x)		FIELD_PREP(ADF4371_REFIN_MODE_MASK, x)
+#define ADF4371_REF_DOUB_MASK		BIT(5)
+#define ADF4371_REF_DOUB(x)		FIELD_PREP(ADF4371_REF_DOUB_MASK, x)\
 
 /* ADF4371_REG24 */
 #define ADF4371_RF_DIV_SEL_MSK		GENMASK(6, 4)
@@ -75,6 +77,9 @@
 #define ADF4371_MAX_FREQ_REFIN		600000000UL /* Hz */
 #define ADF4371_MAX_FREQ_REFIN_SE	500000000UL /* Hz */
 
+#define ADF4371_MIN_CLKIN_DOUB_FREQ	10000000ULL /* Hz */
+#define ADF4371_MAX_CLKIN_DOUB_FREQ	125000000ULL /* Hz */
+
 /* MOD1 is a 24-bit primary modulus with fixed value of 2^25 */
 #define ADF4371_MODULUS1		33554432ULL
 /* MOD2 is the programmable, 14-bit auxiliary fractional modulus */
@@ -481,7 +486,7 @@ static const struct iio_info adf4371_info = {
 static int adf4371_setup(struct adf4371_state *st)
 {
 	unsigned int synth_timeout = 2, timeout = 1, vco_alc_timeout = 1;
-	unsigned int vco_band_div, tmp;
+	unsigned int vco_band_div, tmp, ref_doubler_en = 0;
 	int ret;
 
 	/* Perform a software reset */
@@ -514,8 +519,14 @@ static int adf4371_setup(struct adf4371_state *st)
 	    (!st->ref_diff_en && st->clkin_freq > ADF4371_MAX_FREQ_REFIN_SE))
 		return -EINVAL;
 
+	if (st->clkin_freq < ADF4371_MAX_CLKIN_DOUB_FREQ &&
+	    st->clkin_freq > ADF4371_MIN_CLKIN_DOUB_FREQ)
+		ref_doubler_en = 1;
+
 	ret = regmap_update_bits(st->regmap,  ADF4371_REG(0x22),
+				 ADF4371_REF_DOUB_MASK |
 				 ADF4371_REFIN_MODE_MASK,
+				 ADF4371_REF_DOUB(ref_doubler_en) |
 				 ADF4371_REFIN_MODE(st->ref_diff_en));
 	if (ret < 0)
 		return ret;
@@ -529,7 +540,8 @@ static int adf4371_setup(struct adf4371_state *st)
 	 */
 	do {
 		st->ref_div_factor++;
-		st->fpfd = st->clkin_freq / st->ref_div_factor;
+		st->fpfd = st->clkin_freq * (1 + ref_doubler_en) /
+			   st->ref_div_factor;
 	} while (st->fpfd > ADF4371_MAX_FREQ_PFD);
 
 	/* Calculate Timeouts */
-- 
2.47.1

Re: [PATCH v4 3/3] iio: frequency: adf4371: add ref doubler

[Nuno Sá]

On Thu, 2025-01-09 at 15:37 +0200, Antoniu Miclaus wrote:
> Add support for the reference doubler.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---

LGTM:

Reviewed-by: Nuno Sa <nuno.sa@analog.com>

> no changes in v4.
>  drivers/iio/frequency/adf4371.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/frequency/adf4371.c b/drivers/iio/frequency/adf4371.c
> index 50450e7b5464..a57fc24a6799 100644
> --- a/drivers/iio/frequency/adf4371.c
> +++ b/drivers/iio/frequency/adf4371.c
> @@ -44,6 +44,8 @@
>  /* ADF4371_REG22 */
>  #define ADF4371_REFIN_MODE_MASK		BIT(6)
>  #define ADF4371_REFIN_MODE(x)		FIELD_PREP(ADF4371_REFIN_MODE_MASK,
> x)
> +#define ADF4371_REF_DOUB_MASK		BIT(5)
> +#define ADF4371_REF_DOUB(x)		FIELD_PREP(ADF4371_REF_DOUB_MASK, x)\
>  
>  /* ADF4371_REG24 */
>  #define ADF4371_RF_DIV_SEL_MSK		GENMASK(6, 4)
> @@ -75,6 +77,9 @@
>  #define ADF4371_MAX_FREQ_REFIN		600000000UL /* Hz */
>  #define ADF4371_MAX_FREQ_REFIN_SE	500000000UL /* Hz */
>  
> +#define ADF4371_MIN_CLKIN_DOUB_FREQ	10000000ULL /* Hz */
> +#define ADF4371_MAX_CLKIN_DOUB_FREQ	125000000ULL /* Hz */
> +
>  /* MOD1 is a 24-bit primary modulus with fixed value of 2^25 */
>  #define ADF4371_MODULUS1		33554432ULL
>  /* MOD2 is the programmable, 14-bit auxiliary fractional modulus */
> @@ -481,7 +486,7 @@ static const struct iio_info adf4371_info = {
>  static int adf4371_setup(struct adf4371_state *st)
>  {
>  	unsigned int synth_timeout = 2, timeout = 1, vco_alc_timeout = 1;
> -	unsigned int vco_band_div, tmp;
> +	unsigned int vco_band_div, tmp, ref_doubler_en = 0;
>  	int ret;
>  
>  	/* Perform a software reset */
> @@ -514,8 +519,14 @@ static int adf4371_setup(struct adf4371_state *st)
>  	    (!st->ref_diff_en && st->clkin_freq > ADF4371_MAX_FREQ_REFIN_SE))
>  		return -EINVAL;
>  
> +	if (st->clkin_freq < ADF4371_MAX_CLKIN_DOUB_FREQ &&
> +	    st->clkin_freq > ADF4371_MIN_CLKIN_DOUB_FREQ)
> +		ref_doubler_en = 1;
> +
>  	ret = regmap_update_bits(st->regmap,  ADF4371_REG(0x22),
> +				 ADF4371_REF_DOUB_MASK |
>  				 ADF4371_REFIN_MODE_MASK,
> +				 ADF4371_REF_DOUB(ref_doubler_en) |
>  				 ADF4371_REFIN_MODE(st->ref_diff_en));
>  	if (ret < 0)
>  		return ret;
> @@ -529,7 +540,8 @@ static int adf4371_setup(struct adf4371_state *st)
>  	 */
>  	do {
>  		st->ref_div_factor++;
> -		st->fpfd = st->clkin_freq / st->ref_div_factor;
> +		st->fpfd = st->clkin_freq * (1 + ref_doubler_en) /
> +			   st->ref_div_factor;
>  	} while (st->fpfd > ADF4371_MAX_FREQ_PFD);
>  
>  	/* Calculate Timeouts */

Re: [PATCH v4 0/3] ADF4371 refin mode and doubler support

[Jonathan Cameron]

On Thu, 9 Jan 2025 15:37:04 +0200
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> Add support for selecting between single-ended and differential
> reference input. By default the single-ended input is enabled.
> 
> Input frequency boundaries are change based on the mode selected
> (single-ended/differential).
> 
> Add support for the reference doubler. This feature is enabled
> automatically to improve noise performance if the input frequency
> is within the accepted range.
> 
> Antoniu Miclaus (3):
>   dt-bindings: iio: adf4371: add refin mode
>   iio: frequency: adf4371: add refin mode
>   iio: frequency: adf4371: add ref doubler

Other than that little thing about defaults in the dt binding
this now looks good to me.

thanks,

Jonathan

> 
>  .../bindings/iio/frequency/adf4371.yaml       |  7 ++-
>  drivers/iio/frequency/adf4371.c               | 44 +++++++++++++++++--
>  2 files changed, 45 insertions(+), 6 deletions(-)
>