-----Original Message-----
From: Tom Rix <trix@redhat.com>
Sent: Thursday, December 24, 2020 4:03 PM
To: Ardelean, Alexandru <redacted>; 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 [off-list ref]; Mathias Tausen
[off-list ref]
Subject: Re: [PATCH 1/2] clk: axi-clkgen: add support for ZynqMP (UltraScale)

[External]


On 12/21/20 6:42 AM, Alexandru Ardelean wrote:

quoted

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

quoted

Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com>
Signed-off-by: Mathias Tausen <redacted>
Signed-off-by: Alexandru Ardelean <redacted>
---

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

quoted

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

quoted

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

quoted

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

quoted

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

quoted

 	help
 	  Support for the Analog Devices axi-clkgen pcore clock generator for

Xilinx

quoted

 	  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)

quoted

 	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/ (local)
https://lore.kernel.org/linux-clk/20200930171607.GA121420@archbook/ (local)

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.

quoted

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

quoted

+
 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

quoted

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