* [RESEND PATCH v3 1/6] soc: xilinx: vcu: drop useless success message
2020-06-19 7:59 [RESEND PATCH v3 0/6] soc: xilinx: vcu: provide interfaces for other drivers Michael Tretter
@ 2020-06-19 7:59 ` Michael Tretter
2020-06-19 7:59 ` [RESEND PATCH v3 2/6] ARM: dts: define indexes for output clocks Michael Tretter
` (5 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Michael Tretter @ 2020-06-19 7:59 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Dhaval Shah, Greg Kroah-Hartman, Michael Tretter, Michal Simek,
kernel, Rohit Visavalia
The message that the driver was successfully probed only adds useless
noise. Drop the message.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
drivers/soc/xilinx/xlnx_vcu.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/soc/xilinx/xlnx_vcu.c b/drivers/soc/xilinx/xlnx_vcu.c
index a3aa40996f13..dcd8e7824b06 100644
--- a/drivers/soc/xilinx/xlnx_vcu.c
+++ b/drivers/soc/xilinx/xlnx_vcu.c
@@ -571,8 +571,6 @@ static int xvcu_probe(struct platform_device *pdev)
dev_set_drvdata(&pdev->dev, xvcu);
- dev_info(&pdev->dev, "%s: Probed successfully\n", __func__);
-
return 0;
error_pll_ref:
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 13+ messages in thread* [RESEND PATCH v3 2/6] ARM: dts: define indexes for output clocks
2020-06-19 7:59 [RESEND PATCH v3 0/6] soc: xilinx: vcu: provide interfaces for other drivers Michael Tretter
2020-06-19 7:59 ` [RESEND PATCH v3 1/6] soc: xilinx: vcu: drop useless success message Michael Tretter
@ 2020-06-19 7:59 ` Michael Tretter
2020-06-19 7:59 ` [RESEND PATCH v3 3/6] soc: xilinx: vcu: implement clock provider " Michael Tretter
` (4 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Michael Tretter @ 2020-06-19 7:59 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Dhaval Shah, Rob Herring, Greg Kroah-Hartman, Michael Tretter,
Michal Simek, kernel, Rohit Visavalia
The VCU System-Level Control provides 4 clocks. Defined indexes for
these clocks.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Acked-by: Rob Herring <robh@kernel.org>
---
include/dt-bindings/clock/xlnx-vcu.h | 15 +++++++++++++++
1 file changed, 15 insertions(+)
create mode 100644 include/dt-bindings/clock/xlnx-vcu.h
diff --git a/include/dt-bindings/clock/xlnx-vcu.h b/include/dt-bindings/clock/xlnx-vcu.h
new file mode 100644
index 000000000000..f2a5ea9c4155
--- /dev/null
+++ b/include/dt-bindings/clock/xlnx-vcu.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2020 Pengutronix, Michael Tretter <kernel@pengutronix.de>
+ */
+
+#ifndef _DT_BINDINGS_CLOCK_XLNX_VCU_H
+#define _DT_BINDINGS_CLOCK_XLNX_VCU_H
+
+#define CLK_XVCU_ENC_CORE 0
+#define CLK_XVCU_ENC_MCU 1
+#define CLK_XVCU_DEC_CORE 2
+#define CLK_XVCU_DEC_MCU 3
+#define CLK_XVCU_MAX 4
+
+#endif /* _DT_BINDINGS_CLOCK_XLNX_VCU_H */
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 13+ messages in thread* [RESEND PATCH v3 3/6] soc: xilinx: vcu: implement clock provider for output clocks
2020-06-19 7:59 [RESEND PATCH v3 0/6] soc: xilinx: vcu: provide interfaces for other drivers Michael Tretter
2020-06-19 7:59 ` [RESEND PATCH v3 1/6] soc: xilinx: vcu: drop useless success message Michael Tretter
2020-06-19 7:59 ` [RESEND PATCH v3 2/6] ARM: dts: define indexes for output clocks Michael Tretter
@ 2020-06-19 7:59 ` Michael Tretter
2020-07-21 6:59 ` Rajan Vaja
2020-06-19 7:59 ` [RESEND PATCH v3 4/6] dt-bindings: soc: xlnx: extract xlnx, vcu-settings to separate binding Michael Tretter
` (3 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Michael Tretter @ 2020-06-19 7:59 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Dhaval Shah, Greg Kroah-Hartman, Michael Tretter, Michal Simek,
kernel, Rohit Visavalia
The VCU System-Level Control uses an internal PLL to drive the core and
MCU clock for the allegro encoder and decoder based on an external PL
clock.
In order be able to ensure that the clocks are enabled and to get their
rate from other drivers, the module must implement a clock provider and
register the clocks at the common clock framework. Other drivers are
then able to access the clock via devicetree bindings.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
drivers/soc/xilinx/Kconfig | 2 +-
drivers/soc/xilinx/xlnx_vcu.c | 64 +++++++++++++++++++++++++++++++++--
2 files changed, 63 insertions(+), 3 deletions(-)
diff --git a/drivers/soc/xilinx/Kconfig b/drivers/soc/xilinx/Kconfig
index 646512d7276f..2a4e80a36f78 100644
--- a/drivers/soc/xilinx/Kconfig
+++ b/drivers/soc/xilinx/Kconfig
@@ -3,7 +3,7 @@ menu "Xilinx SoC drivers"
config XILINX_VCU
tristate "Xilinx VCU logicoreIP Init"
- depends on HAS_IOMEM
+ depends on HAS_IOMEM && COMMON_CLK
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..03bf252737aa 100644
--- a/drivers/soc/xilinx/xlnx_vcu.c
+++ b/drivers/soc/xilinx/xlnx_vcu.c
@@ -7,6 +7,7 @@
* Contacts Dhaval Shah <dshah@xilinx.com>
*/
#include <linux/clk.h>
+#include <linux/clk-provider.h>
#include <linux/device.h>
#include <linux/errno.h>
#include <linux/io.h>
@@ -14,6 +15,8 @@
#include <linux/of_platform.h>
#include <linux/platform_device.h>
+#include <dt-bindings/clock/xlnx-vcu.h>
+
/* Address map for different registers implemented in the VCU LogiCORE IP. */
#define VCU_ECODER_ENABLE 0x00
#define VCU_DECODER_ENABLE 0x04
@@ -108,7 +111,9 @@ struct xvcu_device {
struct clk *aclk;
void __iomem *logicore_reg_ba;
void __iomem *vcu_slcr_ba;
+ struct clk_onecell_data clk_data;
u32 coreclk;
+ u32 mcuclk;
};
/**
@@ -375,10 +380,10 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
}
xvcu->coreclk = pll_clk / divisor_core;
- mcuclk = pll_clk / divisor_mcu;
+ xvcu->mcuclk = pll_clk / divisor_mcu;
dev_dbg(xvcu->dev, "Actual Ref clock freq is %uHz\n", refclk);
dev_dbg(xvcu->dev, "Actual Core clock freq is %uHz\n", xvcu->coreclk);
- dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", mcuclk);
+ dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", xvcu->mcuclk);
vcu_pll_ctrl &= ~(VCU_PLL_CTRL_FBDIV_MASK << VCU_PLL_CTRL_FBDIV_SHIFT);
vcu_pll_ctrl |= (found->fbdiv & VCU_PLL_CTRL_FBDIV_MASK) <<
@@ -485,6 +490,51 @@ static int xvcu_set_pll(struct xvcu_device *xvcu)
return -ETIMEDOUT;
}
+static int xvcu_register_clock_provider(struct xvcu_device *xvcu)
+{
+ struct device *dev = xvcu->dev;
+ const char *parent_name = __clk_get_name(xvcu->pll_ref);
+ struct clk_onecell_data *data = &xvcu->clk_data;
+ struct clk **clks;
+ size_t num_clks = CLK_XVCU_MAX;
+
+ clks = devm_kcalloc(dev, num_clks, sizeof(*clks), GFP_KERNEL);
+ if (!clks)
+ return -ENOMEM;
+
+ data->clk_num = num_clks;
+ data->clks = clks;
+
+ clks[CLK_XVCU_ENC_CORE] =
+ clk_register_fixed_rate(dev, "venc_core_clk",
+ parent_name, 0, xvcu->coreclk);
+ clks[CLK_XVCU_ENC_MCU] =
+ clk_register_fixed_rate(dev, "venc_mcu_clk",
+ parent_name, 0, xvcu->mcuclk);
+ clks[CLK_XVCU_DEC_CORE] =
+ clk_register_fixed_rate(dev, "vdec_core_clk",
+ parent_name, 0, xvcu->coreclk);
+ clks[CLK_XVCU_DEC_MCU] =
+ clk_register_fixed_rate(dev, "vdec_mcu_clk",
+ parent_name, 0, xvcu->mcuclk);
+
+ return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get, data);
+}
+
+static void xvcu_unregister_clock_provider(struct xvcu_device *xvcu)
+{
+ struct device *dev = xvcu->dev;
+ struct clk_onecell_data *data = &xvcu->clk_data;
+ struct clk **clks = data->clks;
+
+ of_clk_del_provider(dev->of_node);
+
+ clk_unregister_fixed_rate(clks[CLK_XVCU_DEC_MCU]);
+ clk_unregister_fixed_rate(clks[CLK_XVCU_DEC_CORE]);
+ clk_unregister_fixed_rate(clks[CLK_XVCU_ENC_MCU]);
+ clk_unregister_fixed_rate(clks[CLK_XVCU_ENC_CORE]);
+}
+
/**
* xvcu_probe - Probe existence of the logicoreIP
* and initialize PLL
@@ -569,10 +619,18 @@ static int xvcu_probe(struct platform_device *pdev)
goto error_pll_ref;
}
+ ret = xvcu_register_clock_provider(xvcu);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register clock provider\n");
+ goto error_clk_provider;
+ }
+
dev_set_drvdata(&pdev->dev, xvcu);
return 0;
+error_clk_provider:
+ xvcu_unregister_clock_provider(xvcu);
error_pll_ref:
clk_disable_unprepare(xvcu->pll_ref);
error_aclk:
@@ -596,6 +654,8 @@ static int xvcu_remove(struct platform_device *pdev)
if (!xvcu)
return -ENODEV;
+ xvcu_unregister_clock_provider(xvcu);
+
/* Add the the Gasket isolation and put the VCU in reset. */
xvcu_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, 0);
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 13+ messages in thread* RE: [RESEND PATCH v3 3/6] soc: xilinx: vcu: implement clock provider for output clocks
2020-06-19 7:59 ` [RESEND PATCH v3 3/6] soc: xilinx: vcu: implement clock provider " Michael Tretter
@ 2020-07-21 6:59 ` Rajan Vaja
2020-07-22 14:52 ` Michael Tretter
0 siblings, 1 reply; 13+ messages in thread
From: Rajan Vaja @ 2020-07-21 6:59 UTC (permalink / raw)
To: Michael Tretter, linux-arm-kernel
Cc: Dhaval Rajeshbhai Shah, Michal Simek, kernel, Rohit Visavalia,
Greg Kroah-Hartman
Hi Michael,
Thanks for the patch.
-----Original Message-----
> From: Michael Tretter <m.tretter@pengutronix.de>
> Sent: 19 June 2020 01:29 PM
> To: linux-arm-kernel@lists.infradead.org
> Cc: Rohit Visavalia <RVISAVAL@xilinx.com>; Michal Simek <michals@xilinx.com>;
> Dhaval Rajeshbhai Shah <dshah@xilinx.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; kernel@pengutronix.de; Michael Tretter
> <m.tretter@pengutronix.de>
> Subject: [RESEND PATCH v3 3/6] soc: xilinx: vcu: implement clock provider for
> output clocks
>
> CAUTION: This message has originated from an External Source. Please use proper
> judgment and caution when opening attachments, clicking links, or responding to
> this email.
>
>
> The VCU System-Level Control uses an internal PLL to drive the core and
> MCU clock for the allegro encoder and decoder based on an external PL
> clock.
>
> In order be able to ensure that the clocks are enabled and to get their
> rate from other drivers, the module must implement a clock provider and
> register the clocks at the common clock framework. Other drivers are
> then able to access the clock via devicetree bindings.
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> drivers/soc/xilinx/Kconfig | 2 +-
> drivers/soc/xilinx/xlnx_vcu.c | 64 +++++++++++++++++++++++++++++++++--
> 2 files changed, 63 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/soc/xilinx/Kconfig b/drivers/soc/xilinx/Kconfig
> index 646512d7276f..2a4e80a36f78 100644
> --- a/drivers/soc/xilinx/Kconfig
> +++ b/drivers/soc/xilinx/Kconfig
> @@ -3,7 +3,7 @@ menu "Xilinx SoC drivers"
>
> config XILINX_VCU
> tristate "Xilinx VCU logicoreIP Init"
> - depends on HAS_IOMEM
> + depends on HAS_IOMEM && COMMON_CLK
> 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..03bf252737aa 100644
> --- a/drivers/soc/xilinx/xlnx_vcu.c
> +++ b/drivers/soc/xilinx/xlnx_vcu.c
> @@ -7,6 +7,7 @@
> * Contacts Dhaval Shah <dshah@xilinx.com>
> */
> #include <linux/clk.h>
> +#include <linux/clk-provider.h>
> #include <linux/device.h>
> #include <linux/errno.h>
> #include <linux/io.h>
> @@ -14,6 +15,8 @@
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
>
> +#include <dt-bindings/clock/xlnx-vcu.h>
> +
> /* Address map for different registers implemented in the VCU LogiCORE IP. */
> #define VCU_ECODER_ENABLE 0x00
> #define VCU_DECODER_ENABLE 0x04
> @@ -108,7 +111,9 @@ struct xvcu_device {
> struct clk *aclk;
> void __iomem *logicore_reg_ba;
> void __iomem *vcu_slcr_ba;
> + struct clk_onecell_data clk_data;
> u32 coreclk;
> + u32 mcuclk;
> };
>
> /**
> @@ -375,10 +380,10 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device
> *xvcu)
> }
>
> xvcu->coreclk = pll_clk / divisor_core;
> - mcuclk = pll_clk / divisor_mcu;
> + xvcu->mcuclk = pll_clk / divisor_mcu;
> dev_dbg(xvcu->dev, "Actual Ref clock freq is %uHz\n", refclk);
> dev_dbg(xvcu->dev, "Actual Core clock freq is %uHz\n", xvcu->coreclk);
> - dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", mcuclk);
> + dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", xvcu->mcuclk);
>
> vcu_pll_ctrl &= ~(VCU_PLL_CTRL_FBDIV_MASK <<
> VCU_PLL_CTRL_FBDIV_SHIFT);
> vcu_pll_ctrl |= (found->fbdiv & VCU_PLL_CTRL_FBDIV_MASK) <<
> @@ -485,6 +490,51 @@ static int xvcu_set_pll(struct xvcu_device *xvcu)
> return -ETIMEDOUT;
> }
>
> +static int xvcu_register_clock_provider(struct xvcu_device *xvcu)
> +{
> + struct device *dev = xvcu->dev;
> + const char *parent_name = __clk_get_name(xvcu->pll_ref);
> + struct clk_onecell_data *data = &xvcu->clk_data;
> + struct clk **clks;
> + size_t num_clks = CLK_XVCU_MAX;
> +
> + clks = devm_kcalloc(dev, num_clks, sizeof(*clks), GFP_KERNEL);
> + if (!clks)
> + return -ENOMEM;
> +
> + data->clk_num = num_clks;
> + data->clks = clks;
> +
> + clks[CLK_XVCU_ENC_CORE] =
> + clk_register_fixed_rate(dev, "venc_core_clk",
> + parent_name, 0, xvcu->coreclk);
> + clks[CLK_XVCU_ENC_MCU] =
> + clk_register_fixed_rate(dev, "venc_mcu_clk",
> + parent_name, 0, xvcu->mcuclk);
> + clks[CLK_XVCU_DEC_CORE] =
> + clk_register_fixed_rate(dev, "vdec_core_clk",
> + parent_name, 0, xvcu->coreclk);
> + clks[CLK_XVCU_DEC_MCU] =
> + clk_register_fixed_rate(dev, "vdec_mcu_clk",
> + parent_name, 0, xvcu->mcuclk);
> +
[Rajan] These clocks are not fixed rate clock. These clocks are mux-->div-->gate clock.
Can we separate out clock provider as a different kernel modules instead of having it in VCU driver it self?
We have already implemented clock provider https://github.com/Xilinx/linux-xlnx/blob/master/drivers/soc/xilinx/xlnx_vcu_clk.c and removed manual rate calculation
as separate kernel module. You can refer it, and see if it works for you.
Thanks,
Rajan
> + return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get, data);
> +}
> +
> +static void xvcu_unregister_clock_provider(struct xvcu_device *xvcu)
> +{
> + struct device *dev = xvcu->dev;
> + struct clk_onecell_data *data = &xvcu->clk_data;
> + struct clk **clks = data->clks;
> +
> + of_clk_del_provider(dev->of_node);
> +
> + clk_unregister_fixed_rate(clks[CLK_XVCU_DEC_MCU]);
> + clk_unregister_fixed_rate(clks[CLK_XVCU_DEC_CORE]);
> + clk_unregister_fixed_rate(clks[CLK_XVCU_ENC_MCU]);
> + clk_unregister_fixed_rate(clks[CLK_XVCU_ENC_CORE]);
> +}
> +
> /**
> * xvcu_probe - Probe existence of the logicoreIP
> * and initialize PLL
> @@ -569,10 +619,18 @@ static int xvcu_probe(struct platform_device *pdev)
> goto error_pll_ref;
> }
>
> + ret = xvcu_register_clock_provider(xvcu);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register clock provider\n");
> + goto error_clk_provider;
> + }
> +
> dev_set_drvdata(&pdev->dev, xvcu);
>
> return 0;
>
> +error_clk_provider:
> + xvcu_unregister_clock_provider(xvcu);
> error_pll_ref:
> clk_disable_unprepare(xvcu->pll_ref);
> error_aclk:
> @@ -596,6 +654,8 @@ static int xvcu_remove(struct platform_device *pdev)
> if (!xvcu)
> return -ENODEV;
>
> + xvcu_unregister_clock_provider(xvcu);
> +
> /* Add the the Gasket isolation and put the VCU in reset. */
> xvcu_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, 0);
>
> --
> 2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RESEND PATCH v3 3/6] soc: xilinx: vcu: implement clock provider for output clocks
2020-07-21 6:59 ` Rajan Vaja
@ 2020-07-22 14:52 ` Michael Tretter
2020-07-24 6:44 ` Rajan Vaja
0 siblings, 1 reply; 13+ messages in thread
From: Michael Tretter @ 2020-07-22 14:52 UTC (permalink / raw)
To: Rajan Vaja
Cc: Dhaval Rajeshbhai Shah, Greg Kroah-Hartman, Michal Simek,
linux-arm-kernel, kernel, Rohit Visavalia
Hi Rajan,
On Tue, 21 Jul 2020 06:59:47 +0000, Rajan Vaja wrote:
> Hi Michael,
>
> Thanks for the patch.
>
> -----Original Message-----
> > From: Michael Tretter <m.tretter@pengutronix.de>
> > Sent: 19 June 2020 01:29 PM
> > To: linux-arm-kernel@lists.infradead.org
> > Cc: Rohit Visavalia <RVISAVAL@xilinx.com>; Michal Simek <michals@xilinx.com>;
> > Dhaval Rajeshbhai Shah <dshah@xilinx.com>; Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org>; kernel@pengutronix.de; Michael Tretter
> > <m.tretter@pengutronix.de>
> > Subject: [RESEND PATCH v3 3/6] soc: xilinx: vcu: implement clock provider for
> > output clocks
> >
> > CAUTION: This message has originated from an External Source. Please use proper
> > judgment and caution when opening attachments, clicking links, or responding to
> > this email.
> >
> >
> > The VCU System-Level Control uses an internal PLL to drive the core and
> > MCU clock for the allegro encoder and decoder based on an external PL
> > clock.
> >
> > In order be able to ensure that the clocks are enabled and to get their
> > rate from other drivers, the module must implement a clock provider and
> > register the clocks at the common clock framework. Other drivers are
> > then able to access the clock via devicetree bindings.
> >
> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > ---
> > drivers/soc/xilinx/Kconfig | 2 +-
> > drivers/soc/xilinx/xlnx_vcu.c | 64 +++++++++++++++++++++++++++++++++--
> > 2 files changed, 63 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/soc/xilinx/Kconfig b/drivers/soc/xilinx/Kconfig
> > index 646512d7276f..2a4e80a36f78 100644
> > --- a/drivers/soc/xilinx/Kconfig
> > +++ b/drivers/soc/xilinx/Kconfig
> > @@ -3,7 +3,7 @@ menu "Xilinx SoC drivers"
> >
> > config XILINX_VCU
> > tristate "Xilinx VCU logicoreIP Init"
> > - depends on HAS_IOMEM
> > + depends on HAS_IOMEM && COMMON_CLK
> > 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..03bf252737aa 100644
> > --- a/drivers/soc/xilinx/xlnx_vcu.c
> > +++ b/drivers/soc/xilinx/xlnx_vcu.c
> > @@ -7,6 +7,7 @@
> > * Contacts Dhaval Shah <dshah@xilinx.com>
> > */
> > #include <linux/clk.h>
> > +#include <linux/clk-provider.h>
> > #include <linux/device.h>
> > #include <linux/errno.h>
> > #include <linux/io.h>
> > @@ -14,6 +15,8 @@
> > #include <linux/of_platform.h>
> > #include <linux/platform_device.h>
> >
> > +#include <dt-bindings/clock/xlnx-vcu.h>
> > +
> > /* Address map for different registers implemented in the VCU LogiCORE IP. */
> > #define VCU_ECODER_ENABLE 0x00
> > #define VCU_DECODER_ENABLE 0x04
> > @@ -108,7 +111,9 @@ struct xvcu_device {
> > struct clk *aclk;
> > void __iomem *logicore_reg_ba;
> > void __iomem *vcu_slcr_ba;
> > + struct clk_onecell_data clk_data;
> > u32 coreclk;
> > + u32 mcuclk;
> > };
> >
> > /**
> > @@ -375,10 +380,10 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device
> > *xvcu)
> > }
> >
> > xvcu->coreclk = pll_clk / divisor_core;
> > - mcuclk = pll_clk / divisor_mcu;
> > + xvcu->mcuclk = pll_clk / divisor_mcu;
> > dev_dbg(xvcu->dev, "Actual Ref clock freq is %uHz\n", refclk);
> > dev_dbg(xvcu->dev, "Actual Core clock freq is %uHz\n", xvcu->coreclk);
> > - dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", mcuclk);
> > + dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", xvcu->mcuclk);
> >
> > vcu_pll_ctrl &= ~(VCU_PLL_CTRL_FBDIV_MASK <<
> > VCU_PLL_CTRL_FBDIV_SHIFT);
> > vcu_pll_ctrl |= (found->fbdiv & VCU_PLL_CTRL_FBDIV_MASK) <<
> > @@ -485,6 +490,51 @@ static int xvcu_set_pll(struct xvcu_device *xvcu)
> > return -ETIMEDOUT;
> > }
> >
> > +static int xvcu_register_clock_provider(struct xvcu_device *xvcu)
> > +{
> > + struct device *dev = xvcu->dev;
> > + const char *parent_name = __clk_get_name(xvcu->pll_ref);
> > + struct clk_onecell_data *data = &xvcu->clk_data;
> > + struct clk **clks;
> > + size_t num_clks = CLK_XVCU_MAX;
> > +
> > + clks = devm_kcalloc(dev, num_clks, sizeof(*clks), GFP_KERNEL);
> > + if (!clks)
> > + return -ENOMEM;
> > +
> > + data->clk_num = num_clks;
> > + data->clks = clks;
> > +
> > + clks[CLK_XVCU_ENC_CORE] =
> > + clk_register_fixed_rate(dev, "venc_core_clk",
> > + parent_name, 0, xvcu->coreclk);
> > + clks[CLK_XVCU_ENC_MCU] =
> > + clk_register_fixed_rate(dev, "venc_mcu_clk",
> > + parent_name, 0, xvcu->mcuclk);
> > + clks[CLK_XVCU_DEC_CORE] =
> > + clk_register_fixed_rate(dev, "vdec_core_clk",
> > + parent_name, 0, xvcu->coreclk);
> > + clks[CLK_XVCU_DEC_MCU] =
> > + clk_register_fixed_rate(dev, "vdec_mcu_clk",
> > + parent_name, 0, xvcu->mcuclk);
> > +
> [Rajan] These clocks are not fixed rate clock. These clocks are mux-->div-->gate clock.
I agree that the mux->div->gate clock structure should be properly
implemented, but for clock consumers it doesn't matter, if the clocks are
fixed rate or mux-->div-->rate if this driver already sets the expected rate.
The expected rate is already read from the logicore registers. If properly
implemented, this boils down to an implementation detail of the VCU driver.
> Can we separate out clock provider as a different kernel modules instead of having it in VCU driver it self?
I am not sure, if separating the clock driver as a different kernel module is
actually useful. With a separated clock driver, the only things left in this
driver are the configuration of the gasket isolation and the reset GPIO.
These just don't justify a separate driver.
>
> We have already implemented clock provider https://github.com/Xilinx/linux-xlnx/blob/master/drivers/soc/xilinx/xlnx_vcu_clk.c and removed manual rate calculation
> as separate kernel module. You can refer it, and see if it works for you.
What I am missing from the downstream driver is the ability to reference the
clocks from other drivers via device tree binding (i.e. the CLK_XVCU_ENC_CORE,
CLK_XVCU_ENC_MCU, CLK_XVCU_DEC_CORE, CLK_XVCU_DEC_MCU defines). The codec
driver needs to enable the clocks and, therefore, somehow needs to find the
clocks.
As I fail to see, why this would require an MFD driver and a separate kernel
module for the clocks, I would prefer to implement the full clock driver
(including the mux-->div-->gate clock) in the VCU driver and export the clocks
as suggested by this series.
Michael
>
> Thanks,
> Rajan
> > + return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get, data);
> > +}
> > +
> > +static void xvcu_unregister_clock_provider(struct xvcu_device *xvcu)
> > +{
> > + struct device *dev = xvcu->dev;
> > + struct clk_onecell_data *data = &xvcu->clk_data;
> > + struct clk **clks = data->clks;
> > +
> > + of_clk_del_provider(dev->of_node);
> > +
> > + clk_unregister_fixed_rate(clks[CLK_XVCU_DEC_MCU]);
> > + clk_unregister_fixed_rate(clks[CLK_XVCU_DEC_CORE]);
> > + clk_unregister_fixed_rate(clks[CLK_XVCU_ENC_MCU]);
> > + clk_unregister_fixed_rate(clks[CLK_XVCU_ENC_CORE]);
> > +}
> > +
> > /**
> > * xvcu_probe - Probe existence of the logicoreIP
> > * and initialize PLL
> > @@ -569,10 +619,18 @@ static int xvcu_probe(struct platform_device *pdev)
> > goto error_pll_ref;
> > }
> >
> > + ret = xvcu_register_clock_provider(xvcu);
> > + if (ret) {
> > + dev_err(&pdev->dev, "failed to register clock provider\n");
> > + goto error_clk_provider;
> > + }
> > +
> > dev_set_drvdata(&pdev->dev, xvcu);
> >
> > return 0;
> >
> > +error_clk_provider:
> > + xvcu_unregister_clock_provider(xvcu);
> > error_pll_ref:
> > clk_disable_unprepare(xvcu->pll_ref);
> > error_aclk:
> > @@ -596,6 +654,8 @@ static int xvcu_remove(struct platform_device *pdev)
> > if (!xvcu)
> > return -ENODEV;
> >
> > + xvcu_unregister_clock_provider(xvcu);
> > +
> > /* Add the the Gasket isolation and put the VCU in reset. */
> > xvcu_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, 0);
> >
> > --
> > 2.20.1
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [RESEND PATCH v3 3/6] soc: xilinx: vcu: implement clock provider for output clocks
2020-07-22 14:52 ` Michael Tretter
@ 2020-07-24 6:44 ` Rajan Vaja
2020-07-24 7:28 ` Michael Tretter
0 siblings, 1 reply; 13+ messages in thread
From: Rajan Vaja @ 2020-07-24 6:44 UTC (permalink / raw)
To: Michael Tretter
Cc: Dhaval Rajeshbhai Shah, Greg Kroah-Hartman, Michal Simek,
linux-arm-kernel, kernel, Rohit Visavalia
Hi Michael,
> -----Original Message-----
> From: Michael Tretter <m.tretter@pengutronix.de>
> Sent: 22 July 2020 08:22 PM
> To: Rajan Vaja <RAJANV@xilinx.com>
> Cc: linux-arm-kernel@lists.infradead.org; Rohit Visavalia <RVISAVAL@xilinx.com>;
> Michal Simek <michals@xilinx.com>; Dhaval Rajeshbhai Shah
> <dshah@xilinx.com>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> kernel@pengutronix.de
> Subject: Re: [RESEND PATCH v3 3/6] soc: xilinx: vcu: implement clock provider for
> output clocks
>
> CAUTION: This message has originated from an External Source. Please use proper
> judgment and caution when opening attachments, clicking links, or responding to
> this email.
>
>
> Hi Rajan,
>
> On Tue, 21 Jul 2020 06:59:47 +0000, Rajan Vaja wrote:
> > Hi Michael,
> >
> > Thanks for the patch.
> >
> > -----Original Message-----
> > > From: Michael Tretter <m.tretter@pengutronix.de>
> > > Sent: 19 June 2020 01:29 PM
> > > To: linux-arm-kernel@lists.infradead.org
> > > Cc: Rohit Visavalia <RVISAVAL@xilinx.com>; Michal Simek
> <michals@xilinx.com>;
> > > Dhaval Rajeshbhai Shah <dshah@xilinx.com>; Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org>; kernel@pengutronix.de; Michael Tretter
> > > <m.tretter@pengutronix.de>
> > > Subject: [RESEND PATCH v3 3/6] soc: xilinx: vcu: implement clock provider for
> > > output clocks
> > >
> > > CAUTION: This message has originated from an External Source. Please use
> proper
> > > judgment and caution when opening attachments, clicking links, or responding
> to
> > > this email.
> > >
> > >
> > > The VCU System-Level Control uses an internal PLL to drive the core and
> > > MCU clock for the allegro encoder and decoder based on an external PL
> > > clock.
> > >
> > > In order be able to ensure that the clocks are enabled and to get their
> > > rate from other drivers, the module must implement a clock provider and
> > > register the clocks at the common clock framework. Other drivers are
> > > then able to access the clock via devicetree bindings.
> > >
> > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > > ---
> > > drivers/soc/xilinx/Kconfig | 2 +-
> > > drivers/soc/xilinx/xlnx_vcu.c | 64 +++++++++++++++++++++++++++++++++--
> > > 2 files changed, 63 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/soc/xilinx/Kconfig b/drivers/soc/xilinx/Kconfig
> > > index 646512d7276f..2a4e80a36f78 100644
> > > --- a/drivers/soc/xilinx/Kconfig
> > > +++ b/drivers/soc/xilinx/Kconfig
> > > @@ -3,7 +3,7 @@ menu "Xilinx SoC drivers"
> > >
> > > config XILINX_VCU
> > > tristate "Xilinx VCU logicoreIP Init"
> > > - depends on HAS_IOMEM
> > > + depends on HAS_IOMEM && COMMON_CLK
> > > 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..03bf252737aa 100644
> > > --- a/drivers/soc/xilinx/xlnx_vcu.c
> > > +++ b/drivers/soc/xilinx/xlnx_vcu.c
> > > @@ -7,6 +7,7 @@
> > > * Contacts Dhaval Shah <dshah@xilinx.com>
> > > */
> > > #include <linux/clk.h>
> > > +#include <linux/clk-provider.h>
> > > #include <linux/device.h>
> > > #include <linux/errno.h>
> > > #include <linux/io.h>
> > > @@ -14,6 +15,8 @@
> > > #include <linux/of_platform.h>
> > > #include <linux/platform_device.h>
> > >
> > > +#include <dt-bindings/clock/xlnx-vcu.h>
> > > +
> > > /* Address map for different registers implemented in the VCU LogiCORE IP. */
> > > #define VCU_ECODER_ENABLE 0x00
> > > #define VCU_DECODER_ENABLE 0x04
> > > @@ -108,7 +111,9 @@ struct xvcu_device {
> > > struct clk *aclk;
> > > void __iomem *logicore_reg_ba;
> > > void __iomem *vcu_slcr_ba;
> > > + struct clk_onecell_data clk_data;
> > > u32 coreclk;
> > > + u32 mcuclk;
> > > };
> > >
> > > /**
> > > @@ -375,10 +380,10 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device
> > > *xvcu)
> > > }
> > >
> > > xvcu->coreclk = pll_clk / divisor_core;
> > > - mcuclk = pll_clk / divisor_mcu;
> > > + xvcu->mcuclk = pll_clk / divisor_mcu;
> > > dev_dbg(xvcu->dev, "Actual Ref clock freq is %uHz\n", refclk);
> > > dev_dbg(xvcu->dev, "Actual Core clock freq is %uHz\n", xvcu->coreclk);
> > > - dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", mcuclk);
> > > + dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", xvcu->mcuclk);
> > >
> > > vcu_pll_ctrl &= ~(VCU_PLL_CTRL_FBDIV_MASK <<
> > > VCU_PLL_CTRL_FBDIV_SHIFT);
> > > vcu_pll_ctrl |= (found->fbdiv & VCU_PLL_CTRL_FBDIV_MASK) <<
> > > @@ -485,6 +490,51 @@ static int xvcu_set_pll(struct xvcu_device *xvcu)
> > > return -ETIMEDOUT;
> > > }
> > >
> > > +static int xvcu_register_clock_provider(struct xvcu_device *xvcu)
> > > +{
> > > + struct device *dev = xvcu->dev;
> > > + const char *parent_name = __clk_get_name(xvcu->pll_ref);
> > > + struct clk_onecell_data *data = &xvcu->clk_data;
> > > + struct clk **clks;
> > > + size_t num_clks = CLK_XVCU_MAX;
> > > +
> > > + clks = devm_kcalloc(dev, num_clks, sizeof(*clks), GFP_KERNEL);
> > > + if (!clks)
> > > + return -ENOMEM;
> > > +
> > > + data->clk_num = num_clks;
> > > + data->clks = clks;
> > > +
> > > + clks[CLK_XVCU_ENC_CORE] =
> > > + clk_register_fixed_rate(dev, "venc_core_clk",
> > > + parent_name, 0, xvcu->coreclk);
> > > + clks[CLK_XVCU_ENC_MCU] =
> > > + clk_register_fixed_rate(dev, "venc_mcu_clk",
> > > + parent_name, 0, xvcu->mcuclk);
> > > + clks[CLK_XVCU_DEC_CORE] =
> > > + clk_register_fixed_rate(dev, "vdec_core_clk",
> > > + parent_name, 0, xvcu->coreclk);
> > > + clks[CLK_XVCU_DEC_MCU] =
> > > + clk_register_fixed_rate(dev, "vdec_mcu_clk",
> > > + parent_name, 0, xvcu->mcuclk);
> > > +
> > [Rajan] These clocks are not fixed rate clock. These clocks are mux-->div-->gate
> clock.
>
> I agree that the mux->div->gate clock structure should be properly
> implemented, but for clock consumers it doesn't matter, if the clocks are
> fixed rate or mux-->div-->rate if this driver already sets the expected rate.
> The expected rate is already read from the logicore registers. If properly
> implemented, this boils down to an implementation detail of the VCU driver.
>
> > Can we separate out clock provider as a different kernel modules instead of
> having it in VCU driver it self?
>
> I am not sure, if separating the clock driver as a different kernel module is
> actually useful. With a separated clock driver, the only things left in this
> driver are the configuration of the gasket isolation and the reset GPIO.
> These just don't justify a separate driver.
>
> >
> > We have already implemented clock provider https://github.com/Xilinx/linux-
> xlnx/blob/master/drivers/soc/xilinx/xlnx_vcu_clk.c and removed manual rate
> calculation
> > as separate kernel module. You can refer it, and see if it works for you.
>
> What I am missing from the downstream driver is the ability to reference the
> clocks from other drivers via device tree binding (i.e. the CLK_XVCU_ENC_CORE,
> CLK_XVCU_ENC_MCU, CLK_XVCU_DEC_CORE, CLK_XVCU_DEC_MCU defines). The
> codec
> driver needs to enable the clocks and, therefore, somehow needs to find the
> clocks.
>
> As I fail to see, why this would require an MFD driver and a separate kernel
> module for the clocks, I would prefer to implement the full clock driver
> (including the mux-->div-->gate clock) in the VCU driver and export the clocks
> as suggested by this series.
[Rajan] Reason we choose to go with MFD driver, as existing driver VCU will be clock consumer,
So, we thought to separate out provider and consume. Let us know if you think otherwise.
Thanks,
Rajan
>
> Michael
>
> >
> > Thanks,
> > Rajan
> > > + return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get, data);
> > > +}
> > > +
> > > +static void xvcu_unregister_clock_provider(struct xvcu_device *xvcu)
> > > +{
> > > + struct device *dev = xvcu->dev;
> > > + struct clk_onecell_data *data = &xvcu->clk_data;
> > > + struct clk **clks = data->clks;
> > > +
> > > + of_clk_del_provider(dev->of_node);
> > > +
> > > + clk_unregister_fixed_rate(clks[CLK_XVCU_DEC_MCU]);
> > > + clk_unregister_fixed_rate(clks[CLK_XVCU_DEC_CORE]);
> > > + clk_unregister_fixed_rate(clks[CLK_XVCU_ENC_MCU]);
> > > + clk_unregister_fixed_rate(clks[CLK_XVCU_ENC_CORE]);
> > > +}
> > > +
> > > /**
> > > * xvcu_probe - Probe existence of the logicoreIP
> > > * and initialize PLL
> > > @@ -569,10 +619,18 @@ static int xvcu_probe(struct platform_device *pdev)
> > > goto error_pll_ref;
> > > }
> > >
> > > + ret = xvcu_register_clock_provider(xvcu);
> > > + if (ret) {
> > > + dev_err(&pdev->dev, "failed to register clock provider\n");
> > > + goto error_clk_provider;
> > > + }
> > > +
> > > dev_set_drvdata(&pdev->dev, xvcu);
> > >
> > > return 0;
> > >
> > > +error_clk_provider:
> > > + xvcu_unregister_clock_provider(xvcu);
> > > error_pll_ref:
> > > clk_disable_unprepare(xvcu->pll_ref);
> > > error_aclk:
> > > @@ -596,6 +654,8 @@ static int xvcu_remove(struct platform_device *pdev)
> > > if (!xvcu)
> > > return -ENODEV;
> > >
> > > + xvcu_unregister_clock_provider(xvcu);
> > > +
> > > /* Add the the Gasket isolation and put the VCU in reset. */
> > > xvcu_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, 0);
> > >
> > > --
> > > 2.20.1
> >
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RESEND PATCH v3 3/6] soc: xilinx: vcu: implement clock provider for output clocks
2020-07-24 6:44 ` Rajan Vaja
@ 2020-07-24 7:28 ` Michael Tretter
0 siblings, 0 replies; 13+ messages in thread
From: Michael Tretter @ 2020-07-24 7:28 UTC (permalink / raw)
To: Rajan Vaja
Cc: Dhaval Rajeshbhai Shah, Greg Kroah-Hartman, Michal Simek,
linux-arm-kernel, kernel, Rohit Visavalia
Hi Rajan,
On Fri, 24 Jul 2020 06:44:22 +0000, Rajan Vaja wrote:
> > -----Original Message-----
> > From: Michael Tretter <m.tretter@pengutronix.de>
> > Sent: 22 July 2020 08:22 PM
> > To: Rajan Vaja <RAJANV@xilinx.com>
> > Cc: linux-arm-kernel@lists.infradead.org; Rohit Visavalia <RVISAVAL@xilinx.com>;
> > Michal Simek <michals@xilinx.com>; Dhaval Rajeshbhai Shah
> > <dshah@xilinx.com>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> > kernel@pengutronix.de
> > Subject: Re: [RESEND PATCH v3 3/6] soc: xilinx: vcu: implement clock provider for
> > output clocks
> >
> > CAUTION: This message has originated from an External Source. Please use proper
> > judgment and caution when opening attachments, clicking links, or responding to
> > this email.
> >
> >
> > Hi Rajan,
> >
> > On Tue, 21 Jul 2020 06:59:47 +0000, Rajan Vaja wrote:
> > > Hi Michael,
> > >
> > > Thanks for the patch.
> > >
> > > -----Original Message-----
> > > > From: Michael Tretter <m.tretter@pengutronix.de>
> > > > Sent: 19 June 2020 01:29 PM
> > > > To: linux-arm-kernel@lists.infradead.org
> > > > Cc: Rohit Visavalia <RVISAVAL@xilinx.com>; Michal Simek
> > <michals@xilinx.com>;
> > > > Dhaval Rajeshbhai Shah <dshah@xilinx.com>; Greg Kroah-Hartman
> > > > <gregkh@linuxfoundation.org>; kernel@pengutronix.de; Michael Tretter
> > > > <m.tretter@pengutronix.de>
> > > > Subject: [RESEND PATCH v3 3/6] soc: xilinx: vcu: implement clock provider for
> > > > output clocks
> > > >
> > > > CAUTION: This message has originated from an External Source. Please use
> > proper
> > > > judgment and caution when opening attachments, clicking links, or responding
> > to
> > > > this email.
> > > >
> > > >
> > > > The VCU System-Level Control uses an internal PLL to drive the core and
> > > > MCU clock for the allegro encoder and decoder based on an external PL
> > > > clock.
> > > >
> > > > In order be able to ensure that the clocks are enabled and to get their
> > > > rate from other drivers, the module must implement a clock provider and
> > > > register the clocks at the common clock framework. Other drivers are
> > > > then able to access the clock via devicetree bindings.
> > > >
> > > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > > > ---
> > > > drivers/soc/xilinx/Kconfig | 2 +-
> > > > drivers/soc/xilinx/xlnx_vcu.c | 64 +++++++++++++++++++++++++++++++++--
> > > > 2 files changed, 63 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/soc/xilinx/Kconfig b/drivers/soc/xilinx/Kconfig
> > > > index 646512d7276f..2a4e80a36f78 100644
> > > > --- a/drivers/soc/xilinx/Kconfig
> > > > +++ b/drivers/soc/xilinx/Kconfig
> > > > @@ -3,7 +3,7 @@ menu "Xilinx SoC drivers"
> > > >
> > > > config XILINX_VCU
> > > > tristate "Xilinx VCU logicoreIP Init"
> > > > - depends on HAS_IOMEM
> > > > + depends on HAS_IOMEM && COMMON_CLK
> > > > 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..03bf252737aa 100644
> > > > --- a/drivers/soc/xilinx/xlnx_vcu.c
> > > > +++ b/drivers/soc/xilinx/xlnx_vcu.c
> > > > @@ -7,6 +7,7 @@
> > > > * Contacts Dhaval Shah <dshah@xilinx.com>
> > > > */
> > > > #include <linux/clk.h>
> > > > +#include <linux/clk-provider.h>
> > > > #include <linux/device.h>
> > > > #include <linux/errno.h>
> > > > #include <linux/io.h>
> > > > @@ -14,6 +15,8 @@
> > > > #include <linux/of_platform.h>
> > > > #include <linux/platform_device.h>
> > > >
> > > > +#include <dt-bindings/clock/xlnx-vcu.h>
> > > > +
> > > > /* Address map for different registers implemented in the VCU LogiCORE IP. */
> > > > #define VCU_ECODER_ENABLE 0x00
> > > > #define VCU_DECODER_ENABLE 0x04
> > > > @@ -108,7 +111,9 @@ struct xvcu_device {
> > > > struct clk *aclk;
> > > > void __iomem *logicore_reg_ba;
> > > > void __iomem *vcu_slcr_ba;
> > > > + struct clk_onecell_data clk_data;
> > > > u32 coreclk;
> > > > + u32 mcuclk;
> > > > };
> > > >
> > > > /**
> > > > @@ -375,10 +380,10 @@ static int xvcu_set_vcu_pll_info(struct xvcu_device
> > > > *xvcu)
> > > > }
> > > >
> > > > xvcu->coreclk = pll_clk / divisor_core;
> > > > - mcuclk = pll_clk / divisor_mcu;
> > > > + xvcu->mcuclk = pll_clk / divisor_mcu;
> > > > dev_dbg(xvcu->dev, "Actual Ref clock freq is %uHz\n", refclk);
> > > > dev_dbg(xvcu->dev, "Actual Core clock freq is %uHz\n", xvcu->coreclk);
> > > > - dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", mcuclk);
> > > > + dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", xvcu->mcuclk);
> > > >
> > > > vcu_pll_ctrl &= ~(VCU_PLL_CTRL_FBDIV_MASK <<
> > > > VCU_PLL_CTRL_FBDIV_SHIFT);
> > > > vcu_pll_ctrl |= (found->fbdiv & VCU_PLL_CTRL_FBDIV_MASK) <<
> > > > @@ -485,6 +490,51 @@ static int xvcu_set_pll(struct xvcu_device *xvcu)
> > > > return -ETIMEDOUT;
> > > > }
> > > >
> > > > +static int xvcu_register_clock_provider(struct xvcu_device *xvcu)
> > > > +{
> > > > + struct device *dev = xvcu->dev;
> > > > + const char *parent_name = __clk_get_name(xvcu->pll_ref);
> > > > + struct clk_onecell_data *data = &xvcu->clk_data;
> > > > + struct clk **clks;
> > > > + size_t num_clks = CLK_XVCU_MAX;
> > > > +
> > > > + clks = devm_kcalloc(dev, num_clks, sizeof(*clks), GFP_KERNEL);
> > > > + if (!clks)
> > > > + return -ENOMEM;
> > > > +
> > > > + data->clk_num = num_clks;
> > > > + data->clks = clks;
> > > > +
> > > > + clks[CLK_XVCU_ENC_CORE] =
> > > > + clk_register_fixed_rate(dev, "venc_core_clk",
> > > > + parent_name, 0, xvcu->coreclk);
> > > > + clks[CLK_XVCU_ENC_MCU] =
> > > > + clk_register_fixed_rate(dev, "venc_mcu_clk",
> > > > + parent_name, 0, xvcu->mcuclk);
> > > > + clks[CLK_XVCU_DEC_CORE] =
> > > > + clk_register_fixed_rate(dev, "vdec_core_clk",
> > > > + parent_name, 0, xvcu->coreclk);
> > > > + clks[CLK_XVCU_DEC_MCU] =
> > > > + clk_register_fixed_rate(dev, "vdec_mcu_clk",
> > > > + parent_name, 0, xvcu->mcuclk);
> > > > +
> > > [Rajan] These clocks are not fixed rate clock. These clocks are mux-->div-->gate
> > clock.
> >
> > I agree that the mux->div->gate clock structure should be properly
> > implemented, but for clock consumers it doesn't matter, if the clocks are
> > fixed rate or mux-->div-->rate if this driver already sets the expected rate.
> > The expected rate is already read from the logicore registers. If properly
> > implemented, this boils down to an implementation detail of the VCU driver.
> >
> > > Can we separate out clock provider as a different kernel modules instead of
> > having it in VCU driver it self?
> >
> > I am not sure, if separating the clock driver as a different kernel module is
> > actually useful. With a separated clock driver, the only things left in this
> > driver are the configuration of the gasket isolation and the reset GPIO.
> > These just don't justify a separate driver.
> >
> > >
> > > We have already implemented clock provider https://github.com/Xilinx/linux-
> > xlnx/blob/master/drivers/soc/xilinx/xlnx_vcu_clk.c and removed manual rate
> > calculation
> > > as separate kernel module. You can refer it, and see if it works for you.
> >
> > What I am missing from the downstream driver is the ability to reference the
> > clocks from other drivers via device tree binding (i.e. the CLK_XVCU_ENC_CORE,
> > CLK_XVCU_ENC_MCU, CLK_XVCU_DEC_CORE, CLK_XVCU_DEC_MCU defines). The
> > codec
> > driver needs to enable the clocks and, therefore, somehow needs to find the
> > clocks.
> >
> > As I fail to see, why this would require an MFD driver and a separate kernel
> > module for the clocks, I would prefer to implement the full clock driver
> > (including the mux-->div-->gate clock) in the VCU driver and export the clocks
> > as suggested by this series.
>
> [Rajan] Reason we choose to go with MFD driver, as existing driver VCU will be clock consumer,
> So, we thought to separate out provider and consume. Let us know if you think otherwise.
The xlnx_vcu should only be the provider of the clocks. Please check out the
device tree bindings in Documentation/devicetree/bindings/media/allegro.txt.
That binding describes the actual encoder and decoder cores. It defines the
five clocks that are documented as input clocks to the encoder and decoder
blocks in PG252 H.264/H.265 Video Codec Unit v1.2, Figure 8-1.
Therefore, the driver for the encoder/decoder blocks should be the consumer of
the clocks that are provided by the xlnx_vcu driver. Currently, this would be
the allegro driver (drivers/staging/media/allegro-dvt). Once the xlnx_vcu
driver is a proper clock provider, the allegro driver will be able to get the
venc_core_clk and venc_mcu_clk via the device tree binding and enable and set
the rate of these clocks.
As your out-of-tree VCU driver is also a driver for the encoder/decoder
blocks, you would add the code to enable and set the clock rate there, too.
BTW: The allegro driver is also the reason for the vcu-settings bindings that
are part of the series. The vcu-settings are the replacement for the
xvcu_get_color_depth, xvcu_get_memory_depth, and xvcu_get_num_cores functions
defined downstream in the xlnx_vcu driver.
Thanks,
Michael
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [RESEND PATCH v3 4/6] dt-bindings: soc: xlnx: extract xlnx, vcu-settings to separate binding
2020-06-19 7:59 [RESEND PATCH v3 0/6] soc: xilinx: vcu: provide interfaces for other drivers Michael Tretter
` (2 preceding siblings ...)
2020-06-19 7:59 ` [RESEND PATCH v3 3/6] soc: xilinx: vcu: implement clock provider " Michael Tretter
@ 2020-06-19 7:59 ` Michael Tretter
2020-06-19 7:59 ` [RESEND PATCH v3 5/6] soc: xilinx: vcu: use vcu-settings syscon registers Michael Tretter
` (2 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Michael Tretter @ 2020-06-19 7:59 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Dhaval Shah, Rob Herring, Greg Kroah-Hartman, Michael Tretter,
Michal Simek, kernel, Rohit Visavalia
The xlnx,vcu binding comprises two adjacent register banks:
The first register bank ("vcu_slcr") contains registers for setting the
clocks of the vcu and controlling the performance monitors. The second
bank ("logicoreip") contains the configuration settings of the video codec
unit, which are set before synthesizing the bitstream.
Drivers that drive the actual video codec unit need to to read the
registers from the logicoreip register bank for configuring the vcu
firmware.
As logicoreip is a too generic name for this register bank, use
"vcu-settings" as a binding name, because the register bank basically
provides the configuration settings of the VCU.
Therefore, add the vcu-settings binding to provide a syscon interface
for other drivers to read these registers.
The alternative would have been to merge the two register banks of the
xlnx,vcu binding into one register bank and make xlnx,vcu provide a
syscon interface, but that would lead to more incompatibility than
making second register bank of xlnx,vcu optional.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../soc/xilinx/xlnx,vcu-settings.yaml | 34 +++++++++++++++++++
.../bindings/soc/xilinx/xlnx,vcu.txt | 9 ++---
2 files changed, 36 insertions(+), 7 deletions(-)
create mode 100644 Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
diff --git a/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml b/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
new file mode 100644
index 000000000000..378d0ced43c8
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
@@ -0,0 +1,34 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/xilinx/xlnx,vcu-settings.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx VCU Settings
+
+maintainers:
+ - Michael Tretter <kernel@pengutronix.de>
+
+description: |
+ The Xilinx VCU Settings provides information about the configuration of the
+ video codec unit.
+
+properties:
+ compatible:
+ items:
+ - const: xlnx,vcu-settings
+ - const: syscon
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+examples:
+ - |
+ xlnx_vcu: vcu@a0041000 {
+ compatible = "xlnx,vcu-settings", "syscon";
+ reg = <0x0 0xa0041000 0x0 0x1000>;
+ };
diff --git a/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu.txt b/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu.txt
index 6786d6715df0..2417b13ba468 100644
--- a/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu.txt
+++ b/Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu.txt
@@ -12,10 +12,7 @@ Required properties:
- compatible: shall be one of:
"xlnx,vcu"
"xlnx,vcu-logicoreip-1.0"
-- reg, reg-names: There are two sets of registers need to provide.
- 1. vcu slcr
- 2. Logicore
- reg-names should contain name for the each register sequence.
+- reg : The base offset and size of the VCU_PL_SLCR register space.
- clocks: phandle for aclk and pll_ref clocksource
- clock-names: The identification string, "aclk", is always required for
the axi clock. "pll_ref" is required for pll.
@@ -23,9 +20,7 @@ Example:
xlnx_vcu: vcu@a0040000 {
compatible = "xlnx,vcu-logicoreip-1.0";
- reg = <0x0 0xa0040000 0x0 0x1000>,
- <0x0 0xa0041000 0x0 0x1000>;
- reg-names = "vcu_slcr", "logicore";
+ reg = <0x0 0xa0040000 0x0 0x1000>;
clocks = <&si570_1>, <&clkc 71>;
clock-names = "pll_ref", "aclk";
};
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 13+ messages in thread* [RESEND PATCH v3 5/6] soc: xilinx: vcu: use vcu-settings syscon registers
2020-06-19 7:59 [RESEND PATCH v3 0/6] soc: xilinx: vcu: provide interfaces for other drivers Michael Tretter
` (3 preceding siblings ...)
2020-06-19 7:59 ` [RESEND PATCH v3 4/6] dt-bindings: soc: xlnx: extract xlnx, vcu-settings to separate binding Michael Tretter
@ 2020-06-19 7:59 ` Michael Tretter
2020-06-19 7:59 ` [RESEND PATCH v3 6/6] soc: xilinx: vcu: add missing register NUM_CORE Michael Tretter
2020-07-13 7:55 ` [RESEND PATCH v3 0/6] soc: xilinx: vcu: provide interfaces for other drivers Michal Simek
6 siblings, 0 replies; 13+ messages in thread
From: Michael Tretter @ 2020-06-19 7:59 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Dhaval Shah, Greg Kroah-Hartman, Michael Tretter, Michal Simek,
kernel, Rohit Visavalia
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 2a4e80a36f78..9fe703772e5a 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 && COMMON_CLK
+ select REGMAP_MMIO
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 03bf252737aa..ea4ae1e91fb7 100644
--- a/drivers/soc/xilinx/xlnx_vcu.c
+++ b/drivers/soc/xilinx/xlnx_vcu.c
@@ -11,42 +11,15 @@
#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>
+#include <linux/regmap.h>
#include <dt-bindings/clock/xlnx-vcu.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
-
/* vcu slcr registers, bitmask and shift */
#define VCU_PLL_CTRL 0x24
#define VCU_PLL_CTRL_RESET_MASK 0x01
@@ -109,13 +82,22 @@ 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;
struct clk_onecell_data clk_data;
u32 coreclk;
u32 mcuclk;
};
+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
@@ -305,10 +287,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;
@@ -548,6 +532,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);
@@ -568,17 +553,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");
@@ -610,7 +610,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);
@@ -657,7 +657,7 @@ static int xvcu_remove(struct platform_device *pdev)
xvcu_unregister_clock_provider(xvcu);
/* 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 */
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 13+ messages in thread* [RESEND PATCH v3 6/6] soc: xilinx: vcu: add missing register NUM_CORE
2020-06-19 7:59 [RESEND PATCH v3 0/6] soc: xilinx: vcu: provide interfaces for other drivers Michael Tretter
` (4 preceding siblings ...)
2020-06-19 7:59 ` [RESEND PATCH v3 5/6] soc: xilinx: vcu: use vcu-settings syscon registers Michael Tretter
@ 2020-06-19 7:59 ` Michael Tretter
2020-07-13 7:55 ` [RESEND PATCH v3 0/6] soc: xilinx: vcu: provide interfaces for other drivers Michal Simek
6 siblings, 0 replies; 13+ messages in thread
From: Michael Tretter @ 2020-06-19 7:59 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Dhaval Shah, Greg Kroah-Hartman, Michael Tretter, Michal Simek,
kernel, Rohit Visavalia
The H.264/H.265 Video Codec Unit v1.2 documentation describes this
register as follows:
Number of encoders core used for the provided configuration
This is required for configuring the VCU encoder buffer.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
include/linux/mfd/syscon/xlnx-vcu.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/mfd/syscon/xlnx-vcu.h b/include/linux/mfd/syscon/xlnx-vcu.h
index d3edec4b7b1d..ff7bc3656f6e 100644
--- a/include/linux/mfd/syscon/xlnx-vcu.h
+++ b/include/linux/mfd/syscon/xlnx-vcu.h
@@ -32,6 +32,7 @@
#define VCU_BUFFER_B_FRAME 0x5c
#define VCU_WPP_EN 0x60
#define VCU_PLL_CLK_DEC 0x64
+#define VCU_NUM_CORE 0x6c
#define VCU_GASKET_INIT 0x74
#define VCU_GASKET_VALUE 0x03
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [RESEND PATCH v3 0/6] soc: xilinx: vcu: provide interfaces for other drivers
2020-06-19 7:59 [RESEND PATCH v3 0/6] soc: xilinx: vcu: provide interfaces for other drivers Michael Tretter
` (5 preceding siblings ...)
2020-06-19 7:59 ` [RESEND PATCH v3 6/6] soc: xilinx: vcu: add missing register NUM_CORE Michael Tretter
@ 2020-07-13 7:55 ` Michal Simek
2020-07-15 8:20 ` Rohit Visavalia
6 siblings, 1 reply; 13+ messages in thread
From: Michal Simek @ 2020-07-13 7:55 UTC (permalink / raw)
To: Michael Tretter, linux-arm-kernel
Cc: Rohit Visavalia, Greg Kroah-Hartman, Michal Simek, kernel, Dhaval Shah
Hi,
On 19. 06. 20 9:59, Michael Tretter wrote:
> Hello,
>
> I'm resending v3 of the series to expose interfaces that can be used by other
> drivers from the xlnx_vcu driver, because unfortunately there wasn't any
> feedback on the actual code changes in the series.
>
> This driver controls the glue between the ZynqMP and the actual video codec
> unit. Therefore, a driver for the video codec unit (i.e. the allegro-dvt
> driver) needs to be able to control the glue and read information about the
> codec configuration from the glue. The interfaces to do so are exposed by this
> patch series.
>
> This resend contains Rob Herring's Acked-By and Reviewed-By for the device
> tree changes.
>
> Michael
>
> Changelog:
>
> v2 -> v3:
> - drop unused xvcu_reset() function
>
> v1 -> v2:
> - drop custom select for syscon
> - unregister registered clocks on driver remove
>
> Michael Tretter (6):
> soc: xilinx: vcu: drop useless success message
> ARM: dts: define indexes for output clocks
> soc: xilinx: vcu: implement clock provider for output clocks
> dt-bindings: soc: xlnx: extract xlnx, vcu-settings to separate binding
> soc: xilinx: vcu: use vcu-settings syscon registers
> soc: xilinx: vcu: add missing register NUM_CORE
>
> .../soc/xilinx/xlnx,vcu-settings.yaml | 34 ++++
> .../bindings/soc/xilinx/xlnx,vcu.txt | 9 +-
> drivers/soc/xilinx/Kconfig | 3 +-
> drivers/soc/xilinx/xlnx_vcu.c | 158 ++++++++++++------
> include/dt-bindings/clock/xlnx-vcu.h | 15 ++
> include/linux/mfd/syscon/xlnx-vcu.h | 39 +++++
> 6 files changed, 200 insertions(+), 58 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
> create mode 100644 include/dt-bindings/clock/xlnx-vcu.h
> create mode 100644 include/linux/mfd/syscon/xlnx-vcu.h
>
Rohit, Dhaval: Can you please comment it?
Thanks,
Michal
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [RESEND PATCH v3 0/6] soc: xilinx: vcu: provide interfaces for other drivers
2020-07-13 7:55 ` [RESEND PATCH v3 0/6] soc: xilinx: vcu: provide interfaces for other drivers Michal Simek
@ 2020-07-15 8:20 ` Rohit Visavalia
0 siblings, 0 replies; 13+ messages in thread
From: Rohit Visavalia @ 2020-07-15 8:20 UTC (permalink / raw)
To: Michal Simek, Michael Tretter, linux-arm-kernel, Rajan Vaja, Tejas Patel
Cc: Dhaval Rajeshbhai Shah, Michal Simek, kernel, Greg Kroah-Hartman
+Rajan & Tejas
Can you please review the patch series? As it contains changes related to clock providers & mfd/syscon and you have already made similar changes for CCF and MFD in current Xilinx repos, so that both changes can be in sync for future upgrades.
Thanks
Rohit
> -----Original Message-----
> From: Michal Simek [mailto:michal.simek@xilinx.com]
> Sent: Monday, July 13, 2020 1:25 PM
> To: Michael Tretter <m.tretter@pengutronix.de>; linux-arm-
> kernel@lists.infradead.org
> Cc: Rohit Visavalia <RVISAVAL@xilinx.com>; Michal Simek
> <michals@xilinx.com>; Dhaval Rajeshbhai Shah <dshah@xilinx.com>; Greg
> Kroah-Hartman <gregkh@linuxfoundation.org>; kernel@pengutronix.de
> Subject: Re: [RESEND PATCH v3 0/6] soc: xilinx: vcu: provide interfaces for
> other drivers
>
> Hi,
>
> On 19. 06. 20 9:59, Michael Tretter wrote:
> > Hello,
> >
> > I'm resending v3 of the series to expose interfaces that can be used
> > by other drivers from the xlnx_vcu driver, because unfortunately there
> > wasn't any feedback on the actual code changes in the series.
> >
> > This driver controls the glue between the ZynqMP and the actual video
> > codec unit. Therefore, a driver for the video codec unit (i.e. the
> > allegro-dvt
> > driver) needs to be able to control the glue and read information
> > about the codec configuration from the glue. The interfaces to do so
> > are exposed by this patch series.
> >
> > This resend contains Rob Herring's Acked-By and Reviewed-By for the
> > device tree changes.
> >
> > Michael
> >
> > Changelog:
> >
> > v2 -> v3:
> > - drop unused xvcu_reset() function
> >
> > v1 -> v2:
> > - drop custom select for syscon
> > - unregister registered clocks on driver remove
> >
> > Michael Tretter (6):
> > soc: xilinx: vcu: drop useless success message
> > ARM: dts: define indexes for output clocks
> > soc: xilinx: vcu: implement clock provider for output clocks
> > dt-bindings: soc: xlnx: extract xlnx, vcu-settings to separate binding
> > soc: xilinx: vcu: use vcu-settings syscon registers
> > soc: xilinx: vcu: add missing register NUM_CORE
> >
> > .../soc/xilinx/xlnx,vcu-settings.yaml | 34 ++++
> > .../bindings/soc/xilinx/xlnx,vcu.txt | 9 +-
> > drivers/soc/xilinx/Kconfig | 3 +-
> > drivers/soc/xilinx/xlnx_vcu.c | 158 ++++++++++++------
> > include/dt-bindings/clock/xlnx-vcu.h | 15 ++
> > include/linux/mfd/syscon/xlnx-vcu.h | 39 +++++
> > 6 files changed, 200 insertions(+), 58 deletions(-) create mode
> > 100644
> > Documentation/devicetree/bindings/soc/xilinx/xlnx,vcu-settings.yaml
> > create mode 100644 include/dt-bindings/clock/xlnx-vcu.h
> > create mode 100644 include/linux/mfd/syscon/xlnx-vcu.h
> >
>
> Rohit, Dhaval: Can you please comment it?
>
> Thanks,
> Michal
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread