linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Michael Tretter <m.tretter@pengutronix.de>
To: Hyun Kwon <hyun.kwon@xilinx.com>
Cc: dshah@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: Tue, 17 Nov 2020 09:08:21 +0100	[thread overview]
Message-ID: <20201117080821.GA26611@pengutronix.de> (raw)
In-Reply-To: <20201113205248.GA1052580@xilinx.com>

On Fri, 13 Nov 2020 12:52:48 -0800, Hyun Kwon wrote:
> 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

No, the 'select' is correct for REGMAP_MMIO. The REGMAP* config options don't
have a prompt and are expected to be selected by drivers that are using them.

> 
> 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,

Michael

> 
> 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-17  8:09 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
2020-11-17  8:08     ` Michael Tretter [this message]
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=20201117080821.GA26611@pengutronix.de \
    --to=m.tretter@pengutronix.de \
    --cc=dshah@xilinx.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hyun.kwon@xilinx.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --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 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).