[PATCH 1/2] clk: axi-clkgen: add support for ZynqMP (UltraScale)

[PATCH 1/2] clk: axi-clkgen: add support for ZynqMP (UltraScale)

[Alexandru Ardelean]

From: Dragos Bogdan <dragos.bogdan@analog.com>

This IP core also works and is supported on the Xilinx ZynqMP (UltraScale)
FPGA boards.
This patch enables the driver to be available on these platforms as well.

Since axi-clkgen is now supported on ZYNQMP, we need to make sure the
max/min frequencies of the PFD and VCO are respected.

This change adds two new compatible strings to select limits for Zynq or
ZynqMP from the device data (in the OF table). The old compatible string
(i.e. adi,axi-clkgen-2.00.a) is the same as adi,zynq-axi-clkgen-2.00.a,
since the original version of this driver was designed on top of that
platform.

Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com>
Signed-off-by: Mathias Tausen <mta@gomspace.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---

This is a re-spin of an older series.
It needed to wait a txt -> yaml dt conversion:
https://patchwork.kernel.org/project/linux-clk/patch/20201013143421.84188-1-alexandru.ardelean@analog.com/

It's 2 patches squashed into one:
https://patchwork.kernel.org/project/linux-clk/patch/20200929144417.89816-12-alexandru.ardelean@analog.com/
https://patchwork.kernel.org/project/linux-clk/patch/20200929144417.89816-14-alexandru.ardelean@analog.com/

The series from where all this started is:
https://lore.kernel.org/linux-clk/20200929144417.89816-1-alexandru.ardelean@analog.com/

Well, v4 was the point where I decided to split this into smaller
series, and also do the conversion of the binding to yaml.

 drivers/clk/Kconfig          |  2 +-
 drivers/clk/clk-axi-clkgen.c | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 85856cff506c..252333e585e7 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -247,7 +247,7 @@ config CLK_TWL6040
 
 config COMMON_CLK_AXI_CLKGEN
 	tristate "AXI clkgen driver"
-	depends on ARCH_ZYNQ || MICROBLAZE || COMPILE_TEST
+	depends on ARCH_ZYNQ || ARCH_ZYNQMP || MICROBLAZE || COMPILE_TEST
 	help
 	  Support for the Analog Devices axi-clkgen pcore clock generator for Xilinx
 	  FPGAs. It is commonly used in Analog Devices' reference designs.
diff --git a/drivers/clk/clk-axi-clkgen.c b/drivers/clk/clk-axi-clkgen.c
index ad86e031ba3e..a413c13334ff 100644
--- a/drivers/clk/clk-axi-clkgen.c
+++ b/drivers/clk/clk-axi-clkgen.c
@@ -108,6 +108,13 @@ static uint32_t axi_clkgen_lookup_lock(unsigned int m)
 	return 0x1f1f00fa;
 }
 
+static const struct axi_clkgen_limits axi_clkgen_zynqmp_default_limits = {
+	.fpfd_min = 10000,
+	.fpfd_max = 450000,
+	.fvco_min = 800000,
+	.fvco_max = 1600000,
+};
+
 static const struct axi_clkgen_limits axi_clkgen_zynq_default_limits = {
 	.fpfd_min = 10000,
 	.fpfd_max = 300000,
@@ -560,6 +567,14 @@ static int axi_clkgen_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id axi_clkgen_ids[] = {
+	{
+		.compatible = "adi,zynqmp-axi-clkgen-2.00.a",
+		.data = &axi_clkgen_zynqmp_default_limits,
+	},
+	{
+		.compatible = "adi,zynq-axi-clkgen-2.00.a",
+		.data = &axi_clkgen_zynq_default_limits,
+	},
 	{
 		.compatible = "adi,axi-clkgen-2.00.a",
 		.data = &axi_clkgen_zynq_default_limits,
-- 
2.17.1

[PATCH 2/2] dt-bindings: clock: adi,axi-clkgen: add Zynq & ZynqMP compatible strings

[Alexandru Ardelean]

The axi-clkgen driver now supports ZynqMP (UltraScale) as well, however the
driver needs to use different PFD & VCO limits.

For ZynqMP, these needs to be selected by using the
'adi,zynqmp-axi-clkgen-2.00.a' string. For consistency a
'adi,zynq-axi-clkgen-2.00.a' has been added which should behave as the
original compatible string (i.e. 'adi,axi-clkgen-2.00.a').

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 Documentation/devicetree/bindings/clock/adi,axi-clkgen.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/adi,axi-clkgen.yaml b/Documentation/devicetree/bindings/clock/adi,axi-clkgen.yaml
index 0d06387184d6..398954ec6767 100644
--- a/Documentation/devicetree/bindings/clock/adi,axi-clkgen.yaml
+++ b/Documentation/devicetree/bindings/clock/adi,axi-clkgen.yaml
@@ -20,6 +20,8 @@ properties:
   compatible:
     enum:
       - adi,axi-clkgen-2.00.a
+      - adi,zynq-axi-clkgen-2.00.a
+      - adi,zynqmp-axi-clkgen-2.00.a
 
   clocks:
     description:
-- 
2.17.1

Re: [PATCH 2/2] dt-bindings: clock: adi,axi-clkgen: add Zynq & ZynqMP compatible strings

[Rob Herring]

On Mon, Dec 21, 2020 at 04:42:24PM +0200, Alexandru Ardelean wrote:
> The axi-clkgen driver now supports ZynqMP (UltraScale) as well, however the
> driver needs to use different PFD & VCO limits.
> 
> For ZynqMP, these needs to be selected by using the
> 'adi,zynqmp-axi-clkgen-2.00.a' string. For consistency a
> 'adi,zynq-axi-clkgen-2.00.a' has been added which should behave as the
> original compatible string (i.e. 'adi,axi-clkgen-2.00.a').

Version numbers and SoC are kind of rendundant.

Does 'adi,axi-clkgen-2.00.a' apply to anything other than Zynq? If not, 
you don't really need a new string. If so, you really want it to be:

compatible = "adi,zynq-axi-clkgen-2.00.a", "adi,axi-clkgen-2.00.a";

To be forwards and backwards compatible.

> 
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
>  Documentation/devicetree/bindings/clock/adi,axi-clkgen.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/adi,axi-clkgen.yaml b/Documentation/devicetree/bindings/clock/adi,axi-clkgen.yaml
> index 0d06387184d6..398954ec6767 100644
> --- a/Documentation/devicetree/bindings/clock/adi,axi-clkgen.yaml
> +++ b/Documentation/devicetree/bindings/clock/adi,axi-clkgen.yaml
> @@ -20,6 +20,8 @@ properties:
>    compatible:
>      enum:
>        - adi,axi-clkgen-2.00.a
> +      - adi,zynq-axi-clkgen-2.00.a
> +      - adi,zynqmp-axi-clkgen-2.00.a
>  
>    clocks:
>      description:
> -- 
> 2.17.1
> 

Re: [PATCH 1/2] clk: axi-clkgen: add support for ZynqMP (UltraScale)

[Tom Rix]

On 12/21/20 6:42 AM, Alexandru Ardelean wrote:
> From: Dragos Bogdan <dragos.bogdan@analog.com>
>
> This IP core also works and is supported on the Xilinx ZynqMP (UltraScale)
> FPGA boards.
> This patch enables the driver to be available on these platforms as well.
>
> Since axi-clkgen is now supported on ZYNQMP, we need to make sure the
> max/min frequencies of the PFD and VCO are respected.
>
> This change adds two new compatible strings to select limits for Zynq or
> ZynqMP from the device data (in the OF table). The old compatible string
> (i.e. adi,axi-clkgen-2.00.a) is the same as adi,zynq-axi-clkgen-2.00.a,
> since the original version of this driver was designed on top of that
> platform.
>
> Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com>
> Signed-off-by: Mathias Tausen <mta@gomspace.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
>
> This is a re-spin of an older series.
> It needed to wait a txt -> yaml dt conversion:
> https://patchwork.kernel.org/project/linux-clk/patch/20201013143421.84188-1-alexandru.ardelean@analog.com/
>
> It's 2 patches squashed into one:
> https://patchwork.kernel.org/project/linux-clk/patch/20200929144417.89816-12-alexandru.ardelean@analog.com/
> https://patchwork.kernel.org/project/linux-clk/patch/20200929144417.89816-14-alexandru.ardelean@analog.com/
>
> The series from where all this started is:
> https://lore.kernel.org/linux-clk/20200929144417.89816-1-alexandru.ardelean@analog.com/
>
> Well, v4 was the point where I decided to split this into smaller
> series, and also do the conversion of the binding to yaml.
>
>  drivers/clk/Kconfig          |  2 +-
>  drivers/clk/clk-axi-clkgen.c | 15 +++++++++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 85856cff506c..252333e585e7 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -247,7 +247,7 @@ config CLK_TWL6040
>  
>  config COMMON_CLK_AXI_CLKGEN
>  	tristate "AXI clkgen driver"
> -	depends on ARCH_ZYNQ || MICROBLAZE || COMPILE_TEST
> +	depends on ARCH_ZYNQ || ARCH_ZYNQMP || MICROBLAZE || COMPILE_TEST
>  	help
>  	  Support for the Analog Devices axi-clkgen pcore clock generator for Xilinx
>  	  FPGAs. It is commonly used in Analog Devices' reference designs.
> diff --git a/drivers/clk/clk-axi-clkgen.c b/drivers/clk/clk-axi-clkgen.c
> index ad86e031ba3e..a413c13334ff 100644
> --- a/drivers/clk/clk-axi-clkgen.c
> +++ b/drivers/clk/clk-axi-clkgen.c
> @@ -108,6 +108,13 @@ static uint32_t axi_clkgen_lookup_lock(unsigned int m)
>  	return 0x1f1f00fa;
>  }
>  

Could something like

#ifdef ARCH_ZYNQMP

> +static const struct axi_clkgen_limits axi_clkgen_zynqmp_default_limits = {
> +	.fpfd_min = 10000,
> +	.fpfd_max = 450000,
> +	.fvco_min = 800000,
> +	.fvco_max = 1600000,
> +};

#endif

be added here and similar places to limit unused code ?

> +
>  static const struct axi_clkgen_limits axi_clkgen_zynq_default_limits = {
>  	.fpfd_min = 10000,
>  	.fpfd_max = 300000,
> @@ -560,6 +567,14 @@ static int axi_clkgen_remove(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id axi_clkgen_ids[] = {
> +	{
> +		.compatible = "adi,zynqmp-axi-clkgen-2.00.a",
> +		.data = &axi_clkgen_zynqmp_default_limits,
> +	},
> +	{
> +		.compatible = "adi,zynq-axi-clkgen-2.00.a",
> +		.data = &axi_clkgen_zynq_default_limits,
> +	},

This looks like zynqmp AND zynq are being added.

Is this a mistake ?

Tom

>  	{
>  		.compatible = "adi,axi-clkgen-2.00.a",
>  		.data = &axi_clkgen_zynq_default_limits,

RE: [PATCH 1/2] clk: axi-clkgen: add support for ZynqMP (UltraScale)

[Ardelean, Alexandru]

> -----Original Message-----
> From: Tom Rix <trix@redhat.com>
> Sent: Thursday, December 24, 2020 4:03 PM
> To: Ardelean, Alexandru <alexandru.Ardelean@analog.com>; linux-
> clk@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: mturquette@baylibre.com; sboyd@kernel.org; robh+dt@kernel.org;
> lars@metafoo.de; linux-fpga@vger.kernel.org; mdf@kernel.org; Bogdan,
> Dragos <Dragos.Bogdan@analog.com>; Mathias Tausen
> <mta@gomspace.com>
> Subject: Re: [PATCH 1/2] clk: axi-clkgen: add support for ZynqMP (UltraScale)
> 
> [External]
> 
> 
> On 12/21/20 6:42 AM, Alexandru Ardelean wrote:
> > From: Dragos Bogdan <dragos.bogdan@analog.com>
> >
> > This IP core also works and is supported on the Xilinx ZynqMP
> > (UltraScale) FPGA boards.
> > This patch enables the driver to be available on these platforms as well.
> >
> > Since axi-clkgen is now supported on ZYNQMP, we need to make sure the
> > max/min frequencies of the PFD and VCO are respected.
> >
> > This change adds two new compatible strings to select limits for Zynq
> > or ZynqMP from the device data (in the OF table). The old compatible
> > string (i.e. adi,axi-clkgen-2.00.a) is the same as
> > adi,zynq-axi-clkgen-2.00.a, since the original version of this driver
> > was designed on top of that platform.
> >

Apologies for the late reply on this, this looks like it went to some folder in my inbox and I forgot about it.
And Happy New Year :)

> > Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com>
> > Signed-off-by: Mathias Tausen <mta@gomspace.com>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> >
> > This is a re-spin of an older series.
> > It needed to wait a txt -> yaml dt conversion:
> > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux
> > -clk/patch/20201013143421.84188-1-alexandru.ardelean@analog.com/__;!!A
> > 3Ni8CS0y2Y!sPnN7f9iWjhGkjI-Gu8RTo73gVDV6x0-5m42Pu2kZwvVbHN-
> pZzEaFqTrDY
> > iPPDLdVc-wg$
> >
> > It's 2 patches squashed into one:
> > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux
> > -clk/patch/20200929144417.89816-12-alexandru.ardelean@analog.com/__;!!
> > A3Ni8CS0y2Y!sPnN7f9iWjhGkjI-Gu8RTo73gVDV6x0-5m42Pu2kZwvVbHN-
> pZzEaFqTrD
> > YiPPAkwzbuMA$
> > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux
> > -clk/patch/20200929144417.89816-14-alexandru.ardelean@analog.com/__;!!
> > A3Ni8CS0y2Y!sPnN7f9iWjhGkjI-Gu8RTo73gVDV6x0-5m42Pu2kZwvVbHN-
> pZzEaFqTrD
> > YiPPCaD6QXfQ$
> >
> > The series from where all this started is:
> > https://urldefense.com/v3/__https://lore.kernel.org/linux-clk/20200929
> > 144417.89816-1-
> alexandru.ardelean@analog.com/__;!!A3Ni8CS0y2Y!sPnN7f9i
> > WjhGkjI-Gu8RTo73gVDV6x0-5m42Pu2kZwvVbHN-pZzEaFqTrDYiPPAjuz4oHg$
> >
> > Well, v4 was the point where I decided to split this into smaller
> > series, and also do the conversion of the binding to yaml.
> >
> >  drivers/clk/Kconfig          |  2 +-
> >  drivers/clk/clk-axi-clkgen.c | 15 +++++++++++++++
> >  2 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index
> > 85856cff506c..252333e585e7 100644
> > --- a/drivers/clk/Kconfig
> > +++ b/drivers/clk/Kconfig
> > @@ -247,7 +247,7 @@ config CLK_TWL6040
> >
> >  config COMMON_CLK_AXI_CLKGEN
> >  	tristate "AXI clkgen driver"
> > -	depends on ARCH_ZYNQ || MICROBLAZE || COMPILE_TEST
> > +	depends on ARCH_ZYNQ || ARCH_ZYNQMP || MICROBLAZE ||
> COMPILE_TEST
> >  	help
> >  	  Support for the Analog Devices axi-clkgen pcore clock generator for
> Xilinx
> >  	  FPGAs. It is commonly used in Analog Devices' reference designs.
> > diff --git a/drivers/clk/clk-axi-clkgen.c
> > b/drivers/clk/clk-axi-clkgen.c index ad86e031ba3e..a413c13334ff 100644
> > --- a/drivers/clk/clk-axi-clkgen.c
> > +++ b/drivers/clk/clk-axi-clkgen.c
> > @@ -108,6 +108,13 @@ static uint32_t axi_clkgen_lookup_lock(unsigned int
> m)
> >  	return 0x1f1f00fa;
> >  }
> >
> 
> Could something like
> 
> #ifdef ARCH_ZYNQMP

So, we decided not to do this in an older discussion:
https://lore.kernel.org/linux-clk/20200929153040.GA114067@archbook/
https://lore.kernel.org/linux-clk/20200930171607.GA121420@archbook/

Because, these drivers should also be usable in cases where a ZynqMP or Zynq device can be plugged in via PCIe on a host device.
Thinking about it now, I think I should remove the "depends on ARCH_ZYNQ || ARCH_ZYNQMP" limitation.

> 
> > +static const struct axi_clkgen_limits axi_clkgen_zynqmp_default_limits = {
> > +	.fpfd_min = 10000,
> > +	.fpfd_max = 450000,
> > +	.fvco_min = 800000,
> > +	.fvco_max = 1600000,
> > +};
> 
> #endif
> 
> be added here and similar places to limit unused code ?
> 
> > +
> >  static const struct axi_clkgen_limits axi_clkgen_zynq_default_limits = {
> >  	.fpfd_min = 10000,
> >  	.fpfd_max = 300000,
> > @@ -560,6 +567,14 @@ static int axi_clkgen_remove(struct
> > platform_device *pdev)  }
> >
> >  static const struct of_device_id axi_clkgen_ids[] = {
> > +	{
> > +		.compatible = "adi,zynqmp-axi-clkgen-2.00.a",
> > +		.data = &axi_clkgen_zynqmp_default_limits,
> > +	},
> > +	{
> > +		.compatible = "adi,zynq-axi-clkgen-2.00.a",
> > +		.data = &axi_clkgen_zynq_default_limits,
> > +	},
> 
> This looks like zynqmp AND zynq are being added.
> 
> Is this a mistake ?

The original driver supported only Zynq & Microblaze.
I keep forgetting about Microblaze, because it's one of those target-devices which aren't that popular compared to Zynq/ZynqMP [because of their lower speeds]; though they do pop-up.
I thought I'd add an explicit extra compatible string for Zynq to be able to differentiate it [explicitly], in the device tree.
But I think, it could make sense to just add the zynqmp compat string for now. 

Will fix my inbox filters first and re-spin.

Thanks
Alex

> 
> Tom
> 
> >  	{
> >  		.compatible = "adi,axi-clkgen-2.00.a",
> >  		.data = &axi_clkgen_zynq_default_limits,