> -----Original Message----- > From: Tom Rix <trix@xxxxxxxxxx> > Sent: Thursday, December 24, 2020 4:03 PM > To: Ardelean, Alexandru <alexandru.Ardelean@xxxxxxxxxx>; linux- > clk@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx > Cc: mturquette@xxxxxxxxxxxx; sboyd@xxxxxxxxxx; robh+dt@xxxxxxxxxx; > lars@xxxxxxxxxx; linux-fpga@xxxxxxxxxxxxxxx; mdf@xxxxxxxxxx; Bogdan, > Dragos <Dragos.Bogdan@xxxxxxxxxx>; Mathias Tausen > <mta@xxxxxxxxxxxx> > 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@xxxxxxxxxx> > > > > 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@xxxxxxxxxx> > > Signed-off-by: Mathias Tausen <mta@xxxxxxxxxxxx> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> > > --- > > > > 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@xxxxxxxxxx/__;!!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@xxxxxxxxxx/__;!! > > A3Ni8CS0y2Y!sPnN7f9iWjhGkjI-Gu8RTo73gVDV6x0-5m42Pu2kZwvVbHN- > pZzEaFqTrD > > YiPPAkwzbuMA$ > > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux > > -clk/patch/20200929144417.89816-14-alexandru.ardelean@xxxxxxxxxx/__;!! > > 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@xxxxxxxxxx/__;!!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,
