2020-12-21 14:41:51

by Alexandru Ardelean

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

From: Dragos Bogdan <[email protected]>

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 <[email protected]>
Signed-off-by: Mathias Tausen <[email protected]>
Signed-off-by: Alexandru Ardelean <[email protected]>
---

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/[email protected]/

It's 2 patches squashed into one:
https://patchwork.kernel.org/project/linux-clk/patch/[email protected]/
https://patchwork.kernel.org/project/linux-clk/patch/[email protected]/

The series from where all this started is:
https://lore.kernel.org/linux-clk/[email protected]/

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


2020-12-24 14:05:24

by Tom Rix

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


On 12/21/20 6:42 AM, Alexandru Ardelean wrote:
> From: Dragos Bogdan <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Mathias Tausen <[email protected]>
> Signed-off-by: Alexandru Ardelean <[email protected]>
> ---
>
> 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/[email protected]/
>
> It's 2 patches squashed into one:
> https://patchwork.kernel.org/project/linux-clk/patch/[email protected]/
> https://patchwork.kernel.org/project/linux-clk/patch/[email protected]/
>
> The series from where all this started is:
> https://lore.kernel.org/linux-clk/[email protected]/
>
> 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,

2021-01-12 23:19:39

by Alexandru Ardelean

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



> -----Original Message-----
> From: Tom Rix <[email protected]>
> Sent: Thursday, December 24, 2020 4:03 PM
> To: Ardelean, Alexandru <[email protected]>; linux-
> [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; Bogdan,
> Dragos <[email protected]>; Mathias Tausen
> <[email protected]>
> 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 <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Mathias Tausen <[email protected]>
> > Signed-off-by: Alexandru Ardelean <[email protected]>
> > ---
> >
> > 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/[email protected]/__;!!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/[email protected]/__;!!
> > A3Ni8CS0y2Y!sPnN7f9iWjhGkjI-Gu8RTo73gVDV6x0-5m42Pu2kZwvVbHN-
> pZzEaFqTrD
> > YiPPAkwzbuMA$
> > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux
> > -clk/patch/[email protected]/__;!!
> > 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-
> [email protected]/__;!!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,