On Tue, Apr 14, 2020 at 12:31:59PM +0200, Michael Tretter wrote:
> 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@xxxxxxxxxxxxxx>
> ---
> Changelog:
>
> v1 -> v2:
> - unregister registered clocks when removing the driver
> ---
> drivers/soc/xilinx/Kconfig | 2 +-
> drivers/soc/xilinx/xlnx_vcu.c | 76 ++++++++++++++++++++++++++++++++++-
> 2 files changed, 75 insertions(+), 3 deletions(-)
>
[...]
> diff --git a/drivers/soc/xilinx/xlnx_vcu.c b/drivers/soc/xilinx/xlnx_vcu.c
> index dcd8e7824b06..f07a1361a2a8 100644
> --- a/drivers/soc/xilinx/xlnx_vcu.c
> +++ b/drivers/soc/xilinx/xlnx_vcu.c
[...]
> @@ -485,6 +490,63 @@ 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]);
> +}
> +
> +static void xvcu_reset(struct xvcu_device *xvcu)
> +{
> + if (!xvcu->reset_gpio)
> + return;
> +
> + gpiod_set_value(xvcu->reset_gpio, 0);
> + /* min 2 clock cycle of vcu pll_ref, slowest freq is 33.33KHz */
> + usleep_range(60, 120);
> + gpiod_set_value(xvcu->reset_gpio, 1);
> + usleep_range(60, 120);
> +}
> +
The xvcu_reset function shouldn't have been there at all. Looks like it
slipped through, when I rebased the patches. I'll send a v3.
Michael
> /**
> * xvcu_probe - Probe existence of the logicoreIP
> * and initialize PLL
> @@ -569,10 +631,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 +666,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
>
>
