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

[Tom Rix <trix@redhat.com>]

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,