All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hyun Kwon <hyun.kwon@xilinx.com>
To: Michael Tretter <m.tretter@pengutronix.de>
Cc: dshah@xilinx.com, Hyun Kwon <hyun.kwon@xilinx.com>,
	tejasp@xilinx.com, gregkh@linuxfoundation.org, rajanv@xilinx.com,
	michals@xilinx.com, rvisaval@xilinx.com, kernel@pengutronix.de,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4 3/4] soc: xilinx: vcu: use vcu-settings syscon registers
Date: Fri, 13 Nov 2020 12:52:48 -0800	[thread overview]
Message-ID: <20201113205248.GA1052580@xilinx.com> (raw)
In-Reply-To: <20201109134818.4159342-4-m.tretter@pengutronix.de>

Hi Michael,

Thank you for the patch.

On Mon, Nov 09, 2020 at 02:48:17PM +0100, Michael Tretter wrote:
> Switch the "logicoreip" registers to the new xlnx,vcu-settings binding
> to be able to read the settings if the settings are specified in a
> separate device tree node that is shared with other drivers.
> 
> If the driver is not able to find a node with the new binding, fall back
> to check for the logicore register bank to be backwards compatible.
> 
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
>  drivers/soc/xilinx/Kconfig          |  1 +
>  drivers/soc/xilinx/xlnx_vcu.c       | 94 ++++++++++++++---------------
>  include/linux/mfd/syscon/xlnx-vcu.h | 38 ++++++++++++
>  3 files changed, 86 insertions(+), 47 deletions(-)
>  create mode 100644 include/linux/mfd/syscon/xlnx-vcu.h
> 
> diff --git a/drivers/soc/xilinx/Kconfig b/drivers/soc/xilinx/Kconfig
> index 646512d7276f..0b1708dae361 100644
> --- a/drivers/soc/xilinx/Kconfig
> +++ b/drivers/soc/xilinx/Kconfig
> @@ -4,6 +4,7 @@ menu "Xilinx SoC drivers"
>  config XILINX_VCU
>  	tristate "Xilinx VCU logicoreIP Init"
>  	depends on HAS_IOMEM
> +	select REGMAP_MMIO

This should be 'depends on'? Please check: https://www.spinics.net/lists/kernel/msg3607924.html

The rest look fine to me. After confirming above, for 1/4, 3/4, and 4/4,
please feel free to add,

    Reviewed-by: Hyun Kwon <hyun.kwon@xilinx.com>

Thanks!

-hyun

>  	help
>  	  Provides the driver to enable and disable the isolation between the
>  	  processing system and programmable logic part by using the logicoreIP
> diff --git a/drivers/soc/xilinx/xlnx_vcu.c b/drivers/soc/xilinx/xlnx_vcu.c
> index dcd8e7824b06..14daad4efc58 100644
> --- a/drivers/soc/xilinx/xlnx_vcu.c
> +++ b/drivers/soc/xilinx/xlnx_vcu.c
> @@ -10,39 +10,12 @@
>  #include <linux/device.h>
>  #include <linux/errno.h>
>  #include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/mfd/syscon/xlnx-vcu.h>
>  #include <linux/module.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> -
> -/* Address map for different registers implemented in the VCU LogiCORE IP. */
> -#define VCU_ECODER_ENABLE		0x00
> -#define VCU_DECODER_ENABLE		0x04
> -#define VCU_MEMORY_DEPTH		0x08
> -#define VCU_ENC_COLOR_DEPTH		0x0c
> -#define VCU_ENC_VERTICAL_RANGE		0x10
> -#define VCU_ENC_FRAME_SIZE_X		0x14
> -#define VCU_ENC_FRAME_SIZE_Y		0x18
> -#define VCU_ENC_COLOR_FORMAT		0x1c
> -#define VCU_ENC_FPS			0x20
> -#define VCU_MCU_CLK			0x24
> -#define VCU_CORE_CLK			0x28
> -#define VCU_PLL_BYPASS			0x2c
> -#define VCU_ENC_CLK			0x30
> -#define VCU_PLL_CLK			0x34
> -#define VCU_ENC_VIDEO_STANDARD		0x38
> -#define VCU_STATUS			0x3c
> -#define VCU_AXI_ENC_CLK			0x40
> -#define VCU_AXI_DEC_CLK			0x44
> -#define VCU_AXI_MCU_CLK			0x48
> -#define VCU_DEC_VIDEO_STANDARD		0x4c
> -#define VCU_DEC_FRAME_SIZE_X		0x50
> -#define VCU_DEC_FRAME_SIZE_Y		0x54
> -#define VCU_DEC_FPS			0x58
> -#define VCU_BUFFER_B_FRAME		0x5c
> -#define VCU_WPP_EN			0x60
> -#define VCU_PLL_CLK_DEC			0x64
> -#define VCU_GASKET_INIT			0x74
> -#define VCU_GASKET_VALUE		0x03
> +#include <linux/regmap.h>
>  
>  /* vcu slcr registers, bitmask and shift */
>  #define VCU_PLL_CTRL			0x24
> @@ -106,11 +79,20 @@ struct xvcu_device {
>  	struct device *dev;
>  	struct clk *pll_ref;
>  	struct clk *aclk;
> -	void __iomem *logicore_reg_ba;
> +	struct regmap *logicore_reg_ba;
>  	void __iomem *vcu_slcr_ba;
>  	u32 coreclk;
>  };
>  
> +static struct regmap_config vcu_settings_regmap_config = {
> +	.name = "regmap",
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +	.max_register = 0xfff,
> +	.cache_type = REGCACHE_NONE,
> +};
> +
>  /**
>   * struct xvcu_pll_cfg - Helper data
>   * @fbdiv: The integer portion of the feedback divider to the PLL
> @@ -300,10 +282,12 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
>  	int ret, i;
>  	const struct xvcu_pll_cfg *found = NULL;
>  
> -	inte = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK);
> -	deci = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC);
> -	coreclk = xvcu_read(xvcu->logicore_reg_ba, VCU_CORE_CLK) * MHZ;
> -	mcuclk = xvcu_read(xvcu->logicore_reg_ba, VCU_MCU_CLK) * MHZ;
> +	regmap_read(xvcu->logicore_reg_ba, VCU_PLL_CLK, &inte);
> +	regmap_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC, &deci);
> +	regmap_read(xvcu->logicore_reg_ba, VCU_CORE_CLK, &coreclk);
> +	coreclk *= MHZ;
> +	regmap_read(xvcu->logicore_reg_ba, VCU_MCU_CLK, &mcuclk);
> +	mcuclk *= MHZ;
>  	if (!mcuclk || !coreclk) {
>  		dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
>  		return -EINVAL;
> @@ -498,6 +482,7 @@ static int xvcu_probe(struct platform_device *pdev)
>  {
>  	struct resource *res;
>  	struct xvcu_device *xvcu;
> +	void __iomem *regs;
>  	int ret;
>  
>  	xvcu = devm_kzalloc(&pdev->dev, sizeof(*xvcu), GFP_KERNEL);
> @@ -518,17 +503,32 @@ static int xvcu_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	}
>  
> -	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "logicore");
> -	if (!res) {
> -		dev_err(&pdev->dev, "get logicore memory resource failed.\n");
> -		return -ENODEV;
> -	}
> +	xvcu->logicore_reg_ba =
> +		syscon_regmap_lookup_by_compatible("xlnx,vcu-settings");
> +	if (IS_ERR(xvcu->logicore_reg_ba)) {
> +		dev_info(&pdev->dev,
> +			 "could not find xlnx,vcu-settings: trying direct register access\n");
> +
> +		res = platform_get_resource_byname(pdev,
> +						   IORESOURCE_MEM, "logicore");
> +		if (!res) {
> +			dev_err(&pdev->dev, "get logicore memory resource failed.\n");
> +			return -ENODEV;
> +		}
>  
> -	xvcu->logicore_reg_ba = devm_ioremap(&pdev->dev, res->start,
> -						     resource_size(res));
> -	if (!xvcu->logicore_reg_ba) {
> -		dev_err(&pdev->dev, "logicore register mapping failed.\n");
> -		return -ENOMEM;
> +		regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> +		if (!regs) {
> +			dev_err(&pdev->dev, "logicore register mapping failed.\n");
> +			return -ENOMEM;
> +		}
> +
> +		xvcu->logicore_reg_ba =
> +			devm_regmap_init_mmio(&pdev->dev, regs,
> +					      &vcu_settings_regmap_config);
> +		if (IS_ERR(xvcu->logicore_reg_ba)) {
> +			dev_err(&pdev->dev, "failed to init regmap\n");
> +			return PTR_ERR(xvcu->logicore_reg_ba);
> +		}
>  	}
>  
>  	xvcu->aclk = devm_clk_get(&pdev->dev, "aclk");
> @@ -560,7 +560,7 @@ static int xvcu_probe(struct platform_device *pdev)
>  	 * Bit 0 : Gasket isolation
>  	 * Bit 1 : put VCU out of reset
>  	 */
> -	xvcu_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, VCU_GASKET_VALUE);
> +	regmap_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, VCU_GASKET_VALUE);
>  
>  	/* Do the PLL Settings based on the ref clk,core and mcu clk freq */
>  	ret = xvcu_set_pll(xvcu);
> @@ -597,7 +597,7 @@ static int xvcu_remove(struct platform_device *pdev)
>  		return -ENODEV;
>  
>  	/* Add the the Gasket isolation and put the VCU in reset. */
> -	xvcu_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, 0);
> +	regmap_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, 0);
>  
>  	clk_disable_unprepare(xvcu->pll_ref);
>  	clk_disable_unprepare(xvcu->aclk);
> diff --git a/include/linux/mfd/syscon/xlnx-vcu.h b/include/linux/mfd/syscon/xlnx-vcu.h
> new file mode 100644
> index 000000000000..d3edec4b7b1d
> --- /dev/null
> +++ b/include/linux/mfd/syscon/xlnx-vcu.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2020 Pengutronix, Michael Tretter <kernel@pengutronix.de>
> + */
> +
> +#ifndef __XLNX_VCU_H
> +#define __XLNX_VCU_H
> +
> +#define VCU_ECODER_ENABLE		0x00
> +#define VCU_DECODER_ENABLE		0x04
> +#define VCU_MEMORY_DEPTH		0x08
> +#define VCU_ENC_COLOR_DEPTH		0x0c
> +#define VCU_ENC_VERTICAL_RANGE		0x10
> +#define VCU_ENC_FRAME_SIZE_X		0x14
> +#define VCU_ENC_FRAME_SIZE_Y		0x18
> +#define VCU_ENC_COLOR_FORMAT		0x1c
> +#define VCU_ENC_FPS			0x20
> +#define VCU_MCU_CLK			0x24
> +#define VCU_CORE_CLK			0x28
> +#define VCU_PLL_BYPASS			0x2c
> +#define VCU_ENC_CLK			0x30
> +#define VCU_PLL_CLK			0x34
> +#define VCU_ENC_VIDEO_STANDARD		0x38
> +#define VCU_STATUS			0x3c
> +#define VCU_AXI_ENC_CLK			0x40
> +#define VCU_AXI_DEC_CLK			0x44
> +#define VCU_AXI_MCU_CLK			0x48
> +#define VCU_DEC_VIDEO_STANDARD		0x4c
> +#define VCU_DEC_FRAME_SIZE_X		0x50
> +#define VCU_DEC_FRAME_SIZE_Y		0x54
> +#define VCU_DEC_FPS			0x58
> +#define VCU_BUFFER_B_FRAME		0x5c
> +#define VCU_WPP_EN			0x60
> +#define VCU_PLL_CLK_DEC			0x64
> +#define VCU_GASKET_INIT			0x74
> +#define VCU_GASKET_VALUE		0x03
> +
> +#endif /* __XLNX_VCU_H */

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-11-13 20:54 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-09 13:48 [PATCH v4 0/4] soc: xilinx: vcu: provide interfaces for other drivers Michael Tretter
2020-11-09 13:48 ` [PATCH v4 1/4] soc: xilinx: vcu: drop useless success message Michael Tretter
2020-11-09 13:48 ` [PATCH v4 2/4] dt-bindings: soc: xlnx: extract xlnx, vcu-settings to separate binding Michael Tretter
2020-11-09 13:48 ` [PATCH v4 3/4] soc: xilinx: vcu: use vcu-settings syscon registers Michael Tretter
2020-11-13 20:52   ` Hyun Kwon [this message]
2020-11-17  8:08     ` Michael Tretter
2020-11-09 13:48 ` [PATCH v4 4/4] soc: xilinx: vcu: add missing register NUM_CORE Michael Tretter
2020-11-20 13:55 ` [PATCH v4 0/4] soc: xilinx: vcu: provide interfaces for other drivers Michal Simek

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=20201113205248.GA1052580@xilinx.com \
    --to=hyun.kwon@xilinx.com \
    --cc=dshah@xilinx.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=m.tretter@pengutronix.de \
    --cc=michals@xilinx.com \
    --cc=rajanv@xilinx.com \
    --cc=rvisaval@xilinx.com \
    --cc=tejasp@xilinx.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.