Thread (13 messages) 13 messages, 4 authors, 2020-07-24

RE: [RESEND PATCH v3 3/6] soc: xilinx: vcu: implement clock provider for output clocks

From: Rajan Vaja <hidden>
Date: 2020-07-21 07:01:40

Hi Michael,

Thanks for the patch.

 -----Original Message-----
quoted hunk ↗ jump to hunk
From: Michael Tretter <m.tretter@pengutronix.de>
Sent: 19 June 2020 01:29 PM
To: linux-arm-kernel@lists.infradead.org
Cc: Rohit Visavalia <redacted>; Michal Simek <redacted>;
Dhaval Rajeshbhai Shah [off-list ref]; Greg Kroah-Hartman
[off-list ref]; kernel@pengutronix.de; Michael Tretter
[off-list ref]
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
quoted hunk ↗ jump to hunk
+       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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help