Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 2/2] clk: axi-clkgen: Add support for FPGA info
To:     Alexandru Ardelean <alexandru.ardelean@analog.com>,
        linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     mturquette@baylibre.com, sboyd@kernel.org, lars@metafoo.de,
        linux-fpga@vger.kernel.org, mdf@kernel.org, ardeleanalex@gmail.com,
        Mircea Caprioru <mircea.caprioru@analog.com>
References: <20210210101535.47979-1-alexandru.ardelean@analog.com>
 <20210210101535.47979-2-alexandru.ardelean@analog.com>
From:   Tom Rix <trix@redhat.com>
Message-ID: <26eae7d2-768d-4df4-5646-c44d53b8659e@redhat.com>
Date:   Thu, 11 Feb 2021 06:44:34 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.7.0
MIME-Version: 1.0
In-Reply-To: <20210210101535.47979-2-alexandru.ardelean@analog.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Precedence: bulk


On 2/10/21 2:15 AM, 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 | 52 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/clk-axi-clkgen.c b/drivers/clk/clk-axi-clkgen.c
> index ac6ff736ac8f..e4d6c87f8a07 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>
> @@ -240,6 +241,50 @@ 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)
> +{
> +	struct axi_clkgen_limits *limits = &axi_clkgen->limits;
> +	unsigned int reg_value;
> +	unsigned int tech, family, speed_grade, voltage;
> +
> +	axi_clkgen_read(axi_clkgen, ADI_AXI_REG_FPGA_INFO, &reg_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, &reg_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:
> +		limits->fvco_max = 1200000;
> +		limits->fpfd_max = 450000;
> +		break;
> +	case ADI_AXI_FPGA_SPEED_GRADE_XILINX_2 ... ADI_AXI_FPGA_SPEED_GRADE_XILINX_2LV:
> +		limits->fvco_max = 1440000;
> +		limits->fpfd_max = 500000;
> +		if ((family == ADI_AXI_FPGA_FAMILY_XILINX_KINTEX) |
> +		    (family == ADI_AXI_FPGA_FAMILY_XILINX_ARTIX)) {
NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.
3a419c9317b157ef06ca347b7e6bbab846a3d605 clk: axi-clkgen: Add support for FPGA info
CHECK: Unnecessary parentheses around 'family == ADI_AXI_FPGA_FAMILY_XILINX_KINTEX'
#57: FILE: drivers/clk/clk-axi-clkgen.c:266:
+        if ((family == ADI_AXI_FPGA_FAMILY_XILINX_KINTEX) |
+            (family == ADI_AXI_FPGA_FAMILY_XILINX_ARTIX)) {

Please use checkpatch -strict to find this problem.

Likely the '|', should be '||'

Tom

> +			if (voltage < 950) {
> +				limits->fvco_max = 1200000;
> +				limits->fpfd_max = 450000;
> +			}
> +		}
> +		break;
> +	case ADI_AXI_FPGA_SPEED_GRADE_XILINX_3:
> +		limits->fvco_max = 1600000;
> +		limits->fpfd_max = 550000;
> +		break;
> +	default:
> +		break;
> +	};
> +
> +	if (tech == ADI_AXI_FPGA_TECH_XILINX_ULTRASCALE_PLUS) {
> +		limits->fvco_max = 1600000;
> +		limits->fvco_min = 800000;
> +	}
> +}
> +
>  static int axi_clkgen_wait_non_busy(struct axi_clkgen *axi_clkgen)
>  {
>  	unsigned int timeout = 10000;
> @@ -510,7 +555,7 @@ static int axi_clkgen_probe(struct platform_device *pdev)
>  	struct clk_init_data init;
>  	const char *parent_names[2];
>  	const char *clk_name;
> -	unsigned int i;
> +	unsigned int i, ver;
>  	int ret;
>  
>  	dflt_limits = device_get_match_data(&pdev->dev);
> @@ -537,6 +582,11 @@ static int axi_clkgen_probe(struct platform_device *pdev)
>  
>  	memcpy(&axi_clkgen->limits, dflt_limits, sizeof(axi_clkgen->limits));
>  
> +	axi_clkgen_read(axi_clkgen, ADI_AXI_REG_VERSION, &ver);
> +
> +	if (ADI_AXI_PCORE_VER_MAJOR(ver) > 0x04)
> +		axi_clkgen_setup_ranges(axi_clkgen);
> +
>  	clk_name = pdev->dev.of_node->name;
>  	of_property_read_string(pdev->dev.of_node, "clock-output-names",
>  		&clk_name);