[PATCH v3 0/2] Add ADF4371 Reference Doubler and Reference Divider

[PATCH v3 0/2] Add ADF4371 Reference Doubler and Reference Divider

[Antoniu Miclaus]

This patch series add support for for reference doubler block and reference
divide by 2 clock block within the ADF4371.

The doubler is useful for increasing the PFD comparison frequency which will
result in a noise performance of the system.

The reference divide by 2 divides the reference signal by 2,
resulting in a 50% duty cycle PFD frequency.

Both features were requested from customers that purchased hundreds of adf4371
parts to use in their project. They need a way to adjust these blocks either
from userspace or devicetree.

The patch series aims to both satisfy the customer needs and be compliant with
the current kernel. The devicetree approach was chosen since these kind of
features are already present in the mainline kernel for parts such as adf4350.

Antoniu Miclaus (2):
  dt-bindings: iio: adf4371: add rdiv2 and doubler
  iio: frequency: adf4371: add ref doubler and div2

 .../bindings/iio/frequency/adf4371.yaml       | 11 ++++++
 drivers/iio/frequency/adf4371.c               | 34 +++++++++++++++++--
 2 files changed, 43 insertions(+), 2 deletions(-)

-- 
2.47.1

[PATCH v3 1/2] dt-bindings: iio: adf4371: add rdiv2 and doubler

[Antoniu Miclaus]

Add support for reference doubler enable and reference divide by 2
clock.

Both of these blocks are optional on the frequency path within the
chip and can be adjusted depending on the custom needs of the
applications.

The doubler is useful for increasing the PFD comparison frequency
which will result in a noise performance of the system.

The reference divide by 2 divides the reference signal by 2,
resulting in a 50% duty cycle PFD frequency.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v3:
 - add explanation in commit body
 .../devicetree/bindings/iio/frequency/adf4371.yaml    | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml b/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml
index 1cb2adaf66f9..ef241c38520c 100644
--- a/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml
+++ b/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml
@@ -40,6 +40,17 @@ properties:
       output stage will shut down until the ADF4371/ADF4372 achieves lock as
       measured by the digital lock detect circuitry.
 
+  adi,reference-doubler-enable:
+    type: boolean
+    description:
+      If this property is present, the reference doubler block is enabled.
+
+  adi,adi,reference-div2-enable:
+    type: boolean
+    description:
+      If this property is present, the reference divide by 2 clock is enabled.
+      This feature can be used to provide a 50% duty cycle signal to the PFD.
+
 required:
   - compatible
   - reg
-- 
2.47.1

Re: [PATCH v3 1/2] dt-bindings: iio: adf4371: add rdiv2 and doubler

[Jonathan Cameron]

On Fri, 29 Nov 2024 17:33:52 +0200
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> Add support for reference doubler enable and reference divide by 2
> clock.
> 
> Both of these blocks are optional on the frequency path within the
> chip and can be adjusted depending on the custom needs of the
> applications.
Thanks for the additional info!
> 
> The doubler is useful for increasing the PFD comparison frequency
> which will result in a noise performance of the system.

So I'll play devil's advocate. Improved noise performance sounds
good. If it doesn't take me out of range of allowed frequencies, why
would I not turn it on?  What is it about the surrounding circuitry
etc that would make this a bad idea for some uses of this chip
but not others?

> 
> The reference divide by 2 divides the reference signal by 2,
> resulting in a 50% duty cycle PFD frequency.

why would I want one of those? My 'guess' is this makes sense
if the reference frequency is too high after the application of
the scaling done by the 5 bit counter.  In effect it means the
division circuitry does divide by 1-31, 2-64 in steps of 2.

That could all be wrapped up in the existing control of the
frequency, and so far I'm still not seeing a strong reason why
it belongs in DT.

The 50% cycle thing is a bit of a red herring as assuming it
is triggered on say the rising edge of the high frequency signal
to toggle the divided signal, that will always be a 50% duty cycle.

Jonathan

> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> changes in v3:
>  - add explanation in commit body
>  .../devicetree/bindings/iio/frequency/adf4371.yaml    | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml b/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml
> index 1cb2adaf66f9..ef241c38520c 100644
> --- a/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml
> +++ b/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml
> @@ -40,6 +40,17 @@ properties:
>        output stage will shut down until the ADF4371/ADF4372 achieves lock as
>        measured by the digital lock detect circuitry.
>  
> +  adi,reference-doubler-enable:
> +    type: boolean
> +    description:
> +      If this property is present, the reference doubler block is enabled.
> +
> +  adi,adi,reference-div2-enable:
> +    type: boolean
> +    description:
> +      If this property is present, the reference divide by 2 clock is enabled.
> +      This feature can be used to provide a 50% duty cycle signal to the PFD.
> +
>  required:
>    - compatible
>    - reg

RE: [PATCH v3 1/2] dt-bindings: iio: adf4371: add rdiv2 and doubler

[Miclaus, Antoniu]

--
Antoniu Miclăuş

> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Saturday, November 30, 2024 6:40 PM
> To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com>
> Cc: 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 v3 1/2] dt-bindings: iio: adf4371: add rdiv2 and doubler
> 
> [External]
> 
> On Fri, 29 Nov 2024 17:33:52 +0200
> Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:
> 
> > Add support for reference doubler enable and reference divide by 2
> > clock.
> >
> > Both of these blocks are optional on the frequency path within the
> > chip and can be adjusted depending on the custom needs of the
> > applications.
> Thanks for the additional info!
> >
> > The doubler is useful for increasing the PFD comparison frequency
> > which will result in a noise performance of the system.
> 
> So I'll play devil's advocate. Improved noise performance sounds
> good. If it doesn't take me out of range of allowed frequencies, why
> would I not turn it on?  What is it about the surrounding circuitry
> etc that would make this a bad idea for some uses of this chip
> but not others?
> 
> >
> > The reference divide by 2 divides the reference signal by 2,
> > resulting in a 50% duty cycle PFD frequency.
> 
> why would I want one of those? My 'guess' is this makes sense
> if the reference frequency is too high after the application of
> the scaling done by the 5 bit counter.  In effect it means the
> division circuitry does divide by 1-31, 2-64 in steps of 2.
> 
> That could all be wrapped up in the existing control of the
> frequency, and so far I'm still not seeing a strong reason why
> it belongs in DT.
> 
> The 50% cycle thing is a bit of a red herring as assuming it
> is triggered on say the rising edge of the high frequency signal
> to toggle the divided signal, that will always be a 50% duty cycle.
> 
As mentioned in the cover letter this was mostly a request from
customers that are using adf4371 on a large scale and they need
these features to be controllable somehow by the user.

Since these attributes were already validated as devicetree properties
for adf4350 on mainline, I found this as the best approach to satisfy
both ends.

Antoniu
> Jonathan
> 
> >
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > ---
> > changes in v3:
> >  - add explanation in commit body
> >  .../devicetree/bindings/iio/frequency/adf4371.yaml    | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml
> b/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml
> > index 1cb2adaf66f9..ef241c38520c 100644
> > --- a/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml
> > +++ b/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml
> > @@ -40,6 +40,17 @@ properties:
> >        output stage will shut down until the ADF4371/ADF4372 achieves lock
> as
> >        measured by the digital lock detect circuitry.
> >
> > +  adi,reference-doubler-enable:
> > +    type: boolean
> > +    description:
> > +      If this property is present, the reference doubler block is enabled.
> > +
> > +  adi,adi,reference-div2-enable:
> > +    type: boolean
> > +    description:
> > +      If this property is present, the reference divide by 2 clock is enabled.
> > +      This feature can be used to provide a 50% duty cycle signal to the PFD.
> > +
> >  required:
> >    - compatible
> >    - reg

Re: [PATCH v3 1/2] dt-bindings: iio: adf4371: add rdiv2 and doubler

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2639 bytes --]

On Mon, Dec 02, 2024 at 09:47:21AM +0000, Miclaus, Antoniu wrote:

> > -----Original Message-----
> > From: Jonathan Cameron <jic23@kernel.org>
> > Sent: Saturday, November 30, 2024 6:40 PM
> > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com>
> > Cc: 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 v3 1/2] dt-bindings: iio: adf4371: add rdiv2 and doubler
> > 
> > [External]
> > 
> > On Fri, 29 Nov 2024 17:33:52 +0200
> > Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:
> > 
> > > Add support for reference doubler enable and reference divide by 2
> > > clock.
> > >
> > > Both of these blocks are optional on the frequency path within the
> > > chip and can be adjusted depending on the custom needs of the
> > > applications.
> > Thanks for the additional info!
> > >
> > > The doubler is useful for increasing the PFD comparison frequency
> > > which will result in a noise performance of the system.
> > 
> > So I'll play devil's advocate. Improved noise performance sounds
> > good. If it doesn't take me out of range of allowed frequencies, why
> > would I not turn it on?  What is it about the surrounding circuitry
> > etc that would make this a bad idea for some uses of this chip
> > but not others?

Did I miss a response to this?

> > > The reference divide by 2 divides the reference signal by 2,
> > > resulting in a 50% duty cycle PFD frequency.
> > 
> > why would I want one of those? My 'guess' is this makes sense
> > if the reference frequency is too high after the application of
> > the scaling done by the 5 bit counter.  In effect it means the
> > division circuitry does divide by 1-31, 2-64 in steps of 2.
> > 
> > That could all be wrapped up in the existing control of the
> > frequency, and so far I'm still not seeing a strong reason why
> > it belongs in DT.
> > 
> > The 50% cycle thing is a bit of a red herring as assuming it
> > is triggered on say the rising edge of the high frequency signal
> > to toggle the divided signal, that will always be a 50% duty cycle.
> > 
> As mentioned in the cover letter this was mostly a request from
> customers that are using adf4371 on a large scale and they need
> these features to be controllable somehow by the user.
> 
> Since these attributes were already validated as devicetree properties
> for adf4350 on mainline, I found this as the best approach to satisfy
> both ends.

Probably shouldn't have allowed it then, but things were different a
decade ago.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v3 2/2] iio: frequency: adf4371: add ref doubler and div2

[Antoniu Miclaus]

Add support for the reference doubler and the reference divide by 2
clock.

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

diff --git a/drivers/iio/frequency/adf4371.c b/drivers/iio/frequency/adf4371.c
index b27088464826..b643a08f06ed 100644
--- a/drivers/iio/frequency/adf4371.c
+++ b/drivers/iio/frequency/adf4371.c
@@ -41,6 +41,12 @@
 #define ADF4371_MOD2WORD_MSK		GENMASK(5, 0)
 #define ADF4371_MOD2WORD(x)		FIELD_PREP(ADF4371_MOD2WORD_MSK, x)
 
+/* ADF4371_REG22 */
+#define ADF4371_REF_DOUB_MASK		BIT(5)
+#define ADF4371_REF_DOUB(x)		FIELD_PREP(ADF4371_REF_DOUB_MASK, x)
+#define ADF4371_RDIV2_MASK		BIT(4)
+#define ADF4371_RDIV2(x)		FIELD_PREP(ADF4371_RDIV2_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)
@@ -70,6 +76,9 @@
 #define ADF4371_MAX_FREQ_PFD		250000000UL /* Hz */
 #define ADF4371_MAX_FREQ_REFIN		600000000UL /* 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 */
@@ -175,6 +184,8 @@ struct adf4371_state {
 	unsigned int mod2;
 	unsigned int rf_div_sel;
 	unsigned int ref_div_factor;
+	bool ref_doubler_en;
+	bool ref_div2_en;
 	u8 buf[10] __aligned(IIO_DMA_MINALIGN);
 };
 
@@ -476,6 +487,7 @@ 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;
+	struct device *dev = &st->spi->dev;
 	int ret;
 
 	/* Perform a software reset */
@@ -497,22 +509,40 @@ static int adf4371_setup(struct adf4371_state *st)
 			return ret;
 	}
 
+	st->ref_doubler_en = device_property_read_bool(dev, "adi,reference-doubler-enable");
+
+	st->ref_div2_en = device_property_read_bool(dev, "adi,reference-div2-enable");
+
 	/* Set address in ascending order, so the bulk_write() will work */
 	ret = regmap_update_bits(st->regmap, ADF4371_REG(0x0),
 				 ADF4371_ADDR_ASC_MSK | ADF4371_ADDR_ASC_R_MSK,
 				 ADF4371_ADDR_ASC(1) | ADF4371_ADDR_ASC_R(1));
 	if (ret < 0)
 		return ret;
+
+	if (st->ref_doubler_en &&
+	    (st->clkin_freq > ADF4371_MAX_CLKIN_DOUB_FREQ ||
+	     st->clkin_freq < ADF4371_MIN_CLKIN_DOUB_FREQ))
+		st->ref_doubler_en = false;
+
+	ret = regmap_update_bits(st->regmap,  ADF4371_REG(0x22),
+				 ADF4371_REF_DOUB_MASK |
+				 ADF4371_RDIV2_MASK,
+				 ADF4371_REF_DOUB(st->ref_doubler_en) |
+				 ADF4371_RDIV2(st->ref_div2_en));
+	if (ret < 0)
+		return ret;
+
 	/*
 	 * Calculate and maximize PFD frequency
 	 * fPFD = REFIN × ((1 + D)/(R × (1 + T)))
 	 * Where D is the REFIN doubler bit, T is the reference divide by 2,
 	 * R is the reference division factor
-	 * TODO: it is assumed D and T equal 0.
 	 */
 	do {
 		st->ref_div_factor++;
-		st->fpfd = st->clkin_freq / st->ref_div_factor;
+		st->fpfd = (st->clkin_freq * (st->ref_doubler_en ? 2 : 1)) /
+			   (st->ref_div_factor * (st->ref_div2_en ? 2 : 1));
 	} while (st->fpfd > ADF4371_MAX_FREQ_PFD);
 
 	/* Calculate Timeouts */
-- 
2.47.1