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

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(+)

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

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

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.