[PATCH 03/12] clk: samsung: add clock driver for external clock outputs
From: Tomasz Figa <hidden>
Date: 2014-02-09 02:25:21
Also in:
linux-samsung-soc
Hi Heiko, On 13.12.2013 13:59, Heiko St?bner wrote:
quoted hunk ↗ jump to hunk
This adds a driver for controlling the external clock outputs of s3c24xx architectures including the dclk muxes and dividers. Signed-off-by: Heiko Stuebner <heiko@sntech.de> --- drivers/clk/samsung/Makefile | 1 + drivers/clk/samsung/clk-s3c2410-dclk.c | 517 ++++++++++++++++++++++ include/dt-bindings/clock/samsung,s3c2410-dclk.h | 28 ++ 3 files changed, 546 insertions(+) create mode 100644 drivers/clk/samsung/clk-s3c2410-dclk.c create mode 100644 include/dt-bindings/clock/samsung,s3c2410-dclk.hdiff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile index 4c892c6..568683c 100644 --- a/drivers/clk/samsung/Makefile +++ b/drivers/clk/samsung/Makefile@@ -8,5 +8,6 @@ obj-$(CONFIG_SOC_EXYNOS5250) += clk-exynos5250.o obj-$(CONFIG_SOC_EXYNOS5420) += clk-exynos5420.o obj-$(CONFIG_SOC_EXYNOS5440) += clk-exynos5440.o obj-$(CONFIG_ARCH_EXYNOS) += clk-exynos-audss.o +obj-$(CONFIG_S3C2410_COMMON_DCLK)+= clk-s3c2410-dclk.o obj-$(CONFIG_S3C2443_COMMON_CLK)+= clk-s3c2443.o obj-$(CONFIG_ARCH_S3C64XX) += clk-s3c64xx.odiff --git a/drivers/clk/samsung/clk-s3c2410-dclk.c b/drivers/clk/samsung/clk-s3c2410-dclk.c new file mode 100644 index 0000000..de10e5c --- /dev/null +++ b/drivers/clk/samsung/clk-s3c2410-dclk.c@@ -0,0 +1,517 @@ +/* + * Copyright (c) 2013 Heiko Stuebner <heiko@sntech.de> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * Common Clock Framework support for s3c24xx external clock output. + */ + +#include <linux/platform_device.h> +#include <linux/module.h> +#include <linux/mfd/syscon.h> +#include <linux/clk-provider.h> +#include <linux/regmap.h> +#include <linux/of.h> +#include <dt-bindings/clock/samsung,s3c2410-dclk.h> +#include "clk.h" + +/* legacy access to misccr, until dt conversion is finished */ +#include <mach/hardware.h> +#include <mach/regs-gpio.h> + +enum supported_socs { + S3C2410, + S3C2412, + S3C2440, + S3C2443, +}; + +struct s3c24xx_dclk_drv_data { + int cpu_type; +}; + +/* + * Clock for output-parent selection in misccr + */ + +struct s3c24xx_clkout { + struct clk_hw hw; + struct regmap *misccr; + u32 mask; + u8 shift; +}; + +#define to_s3c24xx_clkout(_hw) container_of(_hw, struct s3c24xx_clkout, hw) + +static u8 s3c24xx_clkout_get_parent(struct clk_hw *hw) +{ + struct s3c24xx_clkout *clkout = to_s3c24xx_clkout(hw); + int num_parents = __clk_get_num_parents(hw->clk); + u32 val; + int ret = 0; + + if (clkout->misccr) + ret = regmap_read(clkout->misccr, 0, &val); + else + val = readl_relaxed(S3C24XX_MISCCR) >> clkout->shift;
I wonder if this couldn't be simplified by always providing a regmap.
+ + if (ret) + return ret; + + val >>= clkout->shift; + val &= clkout->mask; + + if (val >= num_parents) + return -EINVAL; + + return val; +}
[snip]
+#define to_s3c24xx_dclk0(x) \ + container_of(x, struct s3c24xx_dclk, dclk0_div_change_nb) + +#define to_s3c24xx_dclk1(x) \ + container_of(x, struct s3c24xx_dclk, dclk1_div_change_nb) + +static const char dummy_nm[] __initconst = "dummy_name";
What's the advantage of having it defined this way instead of using "dummy_name" (or probably "reserved" or "none", as in Samsung clock drivers) directly in parent lists?
+
+PNAME(dclk_s3c2410_p) = { "pclk", "uclk" };
+PNAME(clkout0_s3c2410_p) = { "mpll", "upll", "fclk", "hclk", "pclk",
+ "gate_dclk0" };
+PNAME(clkout1_s3c2410_p) = { "mpll", "upll", "fclk", "hclk", "pclk",
+ "gate_dclk1" };
+
+PNAME(clkout0_s3c2412_p) = { "mpll", "upll", dummy_nm /* rtc clock output */,
+ "hclk", "pclk", "gate_dclk0" };Hmm, this would suggest that instead of dummy_nm, a real name should be used here, even if such clock doesn't exist yet. CCF will handle this fine.
+PNAME(clkout1_s3c2412_p) = { "xti", "upll", "fclk", "hclk", "pclk",
+ "gate_dclk1" };
+
+PNAME(clkout0_s3c2440_p) = { "xti", "upll", "fclk", "hclk", "pclk",
+ "gate_dclk0" };
+PNAME(clkout1_s3c2440_p) = { "mpll", "upll", dummy_nm /* rtc clock output */,
+ "hclk", "pclk", "gate_dclk1" };[snip]
+static int s3c24xx_dclk_probe(struct platform_device *pdev)
+{
+ struct s3c24xx_dclk *s3c24xx_dclk;
+ struct device_node *np = pdev->dev.of_node;
+ struct regmap *misccr = NULL;
+ struct resource *mem;
+ struct clk **clk_table;
+ const char **clkout0_parent_names, **clkout1_parent_names;
+ u8 clkout0_num_parents, clkout1_num_parents;
+ int current_soc, ret, i;
+
+ s3c24xx_dclk = devm_kzalloc(&pdev->dev, sizeof(*s3c24xx_dclk),
+ GFP_KERNEL);
+ if (!s3c24xx_dclk)
+ return -ENOMEM;
+
+ s3c24xx_dclk->dev = &pdev->dev;
+ platform_set_drvdata(pdev, s3c24xx_dclk);
+ spin_lock_init(&s3c24xx_dclk->dclk_lock);
+
+ clk_table = devm_kzalloc(&pdev->dev,
+ sizeof(struct clk *) * DCLK_MAX_CLKS,
+ GFP_KERNEL);
+ if (!clk_table)
+ return -ENOMEM;
+
+ s3c24xx_dclk->clk_data.clks = clk_table;
+ s3c24xx_dclk->clk_data.clk_num = DCLK_MAX_CLKS;
+
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ s3c24xx_dclk->base = devm_ioremap_resource(&pdev->dev, mem);
+ if (IS_ERR(s3c24xx_dclk->base))
+ return PTR_ERR(s3c24xx_dclk->base);
+
+ /* when run from devicetree, get the misccr through a syscon-regmap */
+ if (np) {
+ misccr = syscon_regmap_lookup_by_phandle(np, "samsung,misccr");
+ if (IS_ERR(misccr)) {
+ dev_err(&pdev->dev, "could not get misccr syscon, %ld\n",
+ PTR_ERR(misccr));
+ return PTR_ERR(misccr);
+ }
+ }
+
+ current_soc = s3c24xx_dclk_get_driver_data(pdev);
+
+ if (current_soc == S3C2443) {
+ clk_table[MUX_DCLK0] = clk_register_mux(&pdev->dev,
+ "mux_dclk0", dclk_s3c2443_p,
+ ARRAY_SIZE(dclk_s3c2443_p), 0,
+ s3c24xx_dclk->base, 1, 1, 0,
+ &s3c24xx_dclk->dclk_lock);
+ clk_table[MUX_DCLK1] = clk_register_mux(&pdev->dev,
+ "mux_dclk1", dclk_s3c2443_p,
+ ARRAY_SIZE(dclk_s3c2443_p), 0,
+ s3c24xx_dclk->base, 17, 1, 0,
+ &s3c24xx_dclk->dclk_lock);
+ } else {
+ clk_table[MUX_DCLK0] = clk_register_mux(&pdev->dev,
+ "mux_dclk0", dclk_s3c2410_p,
+ ARRAY_SIZE(dclk_s3c2410_p), 0,
+ s3c24xx_dclk->base, 1, 1, 0,
+ &s3c24xx_dclk->dclk_lock);
+ clk_table[MUX_DCLK1] = clk_register_mux(&pdev->dev,
+ "mux_dclk1", dclk_s3c2410_p,
+ ARRAY_SIZE(dclk_s3c2410_p), 0,
+ s3c24xx_dclk->base, 17, 1, 0,
+ &s3c24xx_dclk->dclk_lock);
+ }What about using a variant struct instead? Match tables would simply contain pointers to respective structs and here the code would refer to appropriate fields in a struct returned by s3c24xx_dclk_get_driver_data().
+
+ clk_table[DIV_DCLK0] = clk_register_divider(&pdev->dev, "div_dclk0",
+ "mux_dclk0", 0, s3c24xx_dclk->base,
+ 4, 4, 0, &s3c24xx_dclk->dclk_lock);
+ clk_table[DIV_DCLK1] = clk_register_divider(&pdev->dev, "div_dclk1",
+ "mux_dclk1", 0, s3c24xx_dclk->base,
+ 20, 4, 0, &s3c24xx_dclk->dclk_lock);
+
+ clk_table[GATE_DCLK0] = clk_register_gate(&pdev->dev, "gate_dclk0",
+ "div_dclk0", CLK_SET_RATE_PARENT,
+ s3c24xx_dclk->base, 0, 0,
+ &s3c24xx_dclk->dclk_lock);
+ clk_table[GATE_DCLK1] = clk_register_gate(&pdev->dev, "gate_dclk1",
+ "div_dclk1", CLK_SET_RATE_PARENT,
+ s3c24xx_dclk->base, 16, 0,
+ &s3c24xx_dclk->dclk_lock);
+
+ switch (current_soc) {
+ case S3C2410:
+ clkout0_parent_names = clkout0_s3c2410_p;
+ clkout0_num_parents = ARRAY_SIZE(clkout0_s3c2410_p);
+ clkout1_parent_names = clkout1_s3c2410_p;
+ clkout1_num_parents = ARRAY_SIZE(clkout1_s3c2410_p);
+ break;
+ case S3C2412:
+ clkout0_parent_names = clkout0_s3c2412_p;
+ clkout0_num_parents = ARRAY_SIZE(clkout0_s3c2412_p);
+ clkout1_parent_names = clkout1_s3c2412_p;
+ clkout1_num_parents = ARRAY_SIZE(clkout1_s3c2412_p);
+ break;
+ case S3C2440:
+ clkout0_parent_names = clkout0_s3c2440_p;
+ clkout0_num_parents = ARRAY_SIZE(clkout0_s3c2440_p);
+ clkout1_parent_names = clkout1_s3c2440_p;
+ clkout1_num_parents = ARRAY_SIZE(clkout1_s3c2440_p);
+ break;
+ case S3C2443:
+ clkout0_parent_names = clkout0_s3c2443_p;
+ clkout0_num_parents = ARRAY_SIZE(clkout0_s3c2443_p);
+ clkout1_parent_names = clkout1_s3c2443_p;
+ clkout1_num_parents = ARRAY_SIZE(clkout1_s3c2443_p);
+ break;
+ default:
+ dev_err(&pdev->dev, "unsupported soc %d\n", current_soc);
+ ret = -EINVAL;
+ goto err_clk_register;
+ }Ditto.
+
+ clk_table[MUX_CLKOUT0] = s3c24xx_register_clkout(&pdev->dev,
+ "clkout0", clkout0_parent_names,
+ clkout0_num_parents, misccr, 4, 7);
+ clk_table[MUX_CLKOUT1] = s3c24xx_register_clkout(&pdev->dev, "clkout1",
+ clkout1_parent_names,
+ clkout1_num_parents, misccr, 8, 7);
+
+ for (i = 0; i < DCLK_MAX_CLKS; i++)
+ if (IS_ERR(clk_table[i])) {
+ dev_err(&pdev->dev, "clock %d failed to register\n", i);
+ ret = PTR_ERR(clk_table[i]);
+ goto err_clk_register;
+ }
+
+ ret = clk_register_clkdev(clk_table[MUX_DCLK0], "dclk0", NULL);
+ ret |= clk_register_clkdev(clk_table[MUX_DCLK1], "dclk1", NULL);
+ ret |= clk_register_clkdev(clk_table[MUX_CLKOUT0], "clkout0", NULL);
+ ret |= clk_register_clkdev(clk_table[MUX_CLKOUT1], "clkout1", NULL);Hmm, won't it break the error value if two calls return different error codes? I guess that if (!ret) ret = ... if (!ret) ret = ... construct would be more appropriate here, if you don't want error message per call.
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register aliases\n");
+ goto err_clk_register;
+ }
+
+ s3c24xx_dclk->dclk0_div_change_nb.notifier_call =
+ s3c24xx_dclk0_div_notify;
+ s3c24xx_dclk->dclk0_div_change_nb.next = NULL;Do you need to set this field to NULL explicitly? Best regards, Tomasz