From: Moritz Fischer <mdf@kernel.org>
To: Alexandru Ardelean <alexandru.ardelean@analog.com>
Cc: linux-clk@vger.kernel.org, linux-fpga@vger.kernel.org,
linux-kernel@vger.kernel.org, mturquette@baylibre.com,
sboyd@kernel.org, mdf@kernel.org, ardeleanalex@gmail.com,
mircea.caprioru@analog.com
Subject: Re: [PATCH v3 6/6] clk: axi-clkgen: Add support for FPGA info
Date: Thu, 24 Sep 2020 07:21:08 -0700 [thread overview]
Message-ID: <20200924142108.GA60306@archbook> (raw)
In-Reply-To: <20200924065012.59605-7-alexandru.ardelean@analog.com>
On Thu, Sep 24, 2020 at 09:50:12AM +0300, Alexandru Ardelean wrote:
> From: Mircea Caprioru <mircea.caprioru@analog.com>
>
> This patch adds support for vco maximum and minimum ranges in accordance
> with fpga speed grade, voltage, device package, technology and family. This
> new information is extracted from two new registers implemented in the ip
> core: ADI_REG_FPGA_INFO and ADI_REG_FPGA_VOLTAGE, which are stored in the
> 'include/linux/fpga/adi-axi-common.h' file as they are common to all ADI
> FPGA cores.
>
> Signed-off-by: Mircea Caprioru <mircea.caprioru@analog.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> drivers/clk/clk-axi-clkgen.c | 67 +++++++++++++++++++++++++++++++-----
> 1 file changed, 59 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/clk-axi-clkgen.c b/drivers/clk/clk-axi-clkgen.c
> index 6ffc19e9d850..b03ea28270cb 100644
> --- a/drivers/clk/clk-axi-clkgen.c
> +++ b/drivers/clk/clk-axi-clkgen.c
> @@ -8,6 +8,7 @@
>
> #include <linux/platform_device.h>
> #include <linux/clk-provider.h>
> +#include <linux/fpga/adi-axi-common.h>
> #include <linux/slab.h>
> #include <linux/io.h>
> #include <linux/of.h>
> @@ -49,6 +50,7 @@
> struct axi_clkgen {
> void __iomem *base;
> struct clk_hw clk_hw;
> + unsigned int pcore_version;
> };
>
> static uint32_t axi_clkgen_lookup_filter(unsigned int m)
> @@ -101,15 +103,15 @@ static uint32_t axi_clkgen_lookup_lock(unsigned int m)
> }
>
> #ifdef ARCH_ZYNQMP
> -static const unsigned int fpfd_min = 10000;
> -static const unsigned int fpfd_max = 450000;
> -static const unsigned int fvco_min = 800000;
> -static const unsigned int fvco_max = 1600000;
> +static unsigned int fpfd_min = 10000;
> +static unsigned int fpfd_max = 450000;
> +static unsigned int fvco_min = 800000;
> +static unsigned int fvco_max = 1600000;
> #else
> -static const unsigned int fpfd_min = 10000;
> -static const unsigned int fpfd_max = 300000;
> -static const unsigned int fvco_min = 600000;
> -static const unsigned int fvco_max = 1200000;
> +static unsigned int fpfd_min = 10000;
> +static unsigned int fpfd_max = 300000;
> +static unsigned int fvco_min = 600000;
> +static unsigned int fvco_max = 1200000;
> #endif
Instead of modifying those wouldn't you want those to be part of your
struct axi_clkgen? I understand that they're technically properties of the
fabric you're implementing this IP block in, but are you sure you'll
never have a system with more than one of those in two different FPGAs,
are you never gonna use this beyond ARCH_ZYNQ/ZYNQMP/MICROBLAZE?
What about a PCIe plugin card for a ZynqMP system with a Zynq on it?
>
> static void axi_clkgen_calc_params(unsigned long fin, unsigned long fout,
> @@ -229,6 +231,49 @@ static void axi_clkgen_read(struct axi_clkgen *axi_clkgen,
> *val = readl(axi_clkgen->base + reg);
> }
>
> +static void axi_clkgen_setup_ranges(struct axi_clkgen *axi_clkgen)
> +{
> + unsigned int reg_value;
> + unsigned int tech, family, speed_grade, voltage;
> +
> + axi_clkgen_read(axi_clkgen, ADI_AXI_REG_FPGA_INFO, ®_value);
> + tech = ADI_AXI_INFO_FPGA_TECH(reg_value);
> + family = ADI_AXI_INFO_FPGA_FAMILY(reg_value);
> + speed_grade = ADI_AXI_INFO_FPGA_SPEED_GRADE(reg_value);
> +
> + axi_clkgen_read(axi_clkgen, ADI_AXI_REG_FPGA_VOLTAGE, ®_value);
> + voltage = ADI_AXI_INFO_FPGA_VOLTAGE(reg_value);
> +
> + switch (speed_grade) {
> + case ADI_AXI_FPGA_SPEED_GRADE_XILINX_1 ... ADI_AXI_FPGA_SPEED_GRADE_XILINX_1LV:
> + fvco_max = 1200000;
> + fpfd_max = 450000;
> + break;
> + case ADI_AXI_FPGA_SPEED_GRADE_XILINX_2 ... ADI_AXI_FPGA_SPEED_GRADE_XILINX_2LV:
> + fvco_max = 1440000;
> + fpfd_max = 500000;
> + if ((family == ADI_AXI_FPGA_FAMILY_XILINX_KINTEX) |
> + (family == ADI_AXI_FPGA_FAMILY_XILINX_ARTIX)) {
> + if (voltage < 950) {
> + fvco_max = 1200000;
> + fpfd_max = 450000;
> + }
> + }
> + break;
> + case ADI_AXI_FPGA_SPEED_GRADE_XILINX_3:
> + fvco_max = 1600000;
> + fpfd_max = 550000;
> + break;
> + default:
> + break;
> + };
> +
> + if (tech == ADI_AXI_FPGA_TECH_XILINX_ULTRASCALE_PLUS) {
> + fvco_max = 1600000;
> + fvco_min = 800000;
> + }
> +}
> +
> static int axi_clkgen_wait_non_busy(struct axi_clkgen *axi_clkgen)
> {
> unsigned int timeout = 10000;
> @@ -524,6 +569,12 @@ static int axi_clkgen_probe(struct platform_device *pdev)
> if (IS_ERR(axi_clkgen->base))
> return PTR_ERR(axi_clkgen->base);
>
> + axi_clkgen_read(axi_clkgen, ADI_AXI_REG_VERSION,
> + &axi_clkgen->pcore_version);
> +
> + if (ADI_AXI_PCORE_VER_MAJOR(axi_clkgen->pcore_version) > 0x04)
> + axi_clkgen_setup_ranges(axi_clkgen);
> +
> init.num_parents = of_clk_get_parent_count(pdev->dev.of_node);
> if (init.num_parents < 1 || init.num_parents > 2)
> return -EINVAL;
> --
> 2.25.1
>
Thanks,
Moritz
next prev parent reply other threads:[~2020-09-24 14:21 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-24 6:50 [PATCH v3 0/6] clk: axi-clk-gen: misc updates to the driver Alexandru Ardelean
2020-09-24 6:50 ` [PATCH v3 1/6] clk: axi-clkgen: Add support for fractional dividers Alexandru Ardelean
2020-09-24 6:50 ` [PATCH v3 2/6] clk: axi-clkgen: Set power bits for fractional mode Alexandru Ardelean
2020-09-24 6:50 ` [PATCH v3 3/6] clk: axi-clkgen: add support for ZynqMP (UltraScale) Alexandru Ardelean
2020-09-24 6:50 ` [PATCH v3 4/6] clk: axi-clkgen: Respect ZYNQMP PFD/VCO frequency limits Alexandru Ardelean
2020-09-24 6:50 ` [PATCH v3 5/6] include: fpga: adi-axi-common.h: add definitions for supported FPGAs Alexandru Ardelean
2020-09-24 6:50 ` [PATCH v3 6/6] clk: axi-clkgen: Add support for FPGA info Alexandru Ardelean
2020-09-24 14:21 ` Moritz Fischer [this message]
2020-09-24 14:53 ` Alexandru Ardelean
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200924142108.GA60306@archbook \
--to=mdf@kernel.org \
--cc=alexandru.ardelean@analog.com \
--cc=ardeleanalex@gmail.com \
--cc=linux-clk@vger.kernel.org \
--cc=linux-fpga@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mircea.caprioru@analog.com \
--cc=mturquette@baylibre.com \
--cc=sboyd@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).