| Message ID | 1421857262-16607-5-git-send-email-ulrich.hecht+renesas@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Ulrich, Thank you for the patch. On Wednesday 21 January 2015 17:20:55 Ulrich Hecht wrote: > Driver for the r8a7778's clocks that depend on the mode bits. > > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> > --- > .../bindings/clock/renesas,r8a7778-cpg-clocks.txt | 24 ++++ > arch/arm/mach-shmobile/board-bockw-reference.c | 2 + > arch/arm/mach-shmobile/setup-r8a7778.c | 19 +++ > drivers/clk/shmobile/Makefile | 1 + > drivers/clk/shmobile/clk-r8a7778.c | 146 ++++++++++++++++++ > include/linux/clk/shmobile.h | 1 + > 6 files changed, 193 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/clock/renesas,r8a7778-cpg-clocks.txt > create mode 100644 drivers/clk/shmobile/clk-r8a7778.c [snip] > diff --git a/drivers/clk/shmobile/clk-r8a7778.c > b/drivers/clk/shmobile/clk-r8a7778.c new file mode 100644 > index 0000000..30c5c14 > --- /dev/null > +++ b/drivers/clk/shmobile/clk-r8a7778.c > @@ -0,0 +1,146 @@ > +/* > + * r8a7778 Core CPG Clocks > + * > + * Copyright (C) 2014 Ulrich Hecht > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; version 2 of the License. > + */ > + > +#include <linux/clk-provider.h> > +#include <linux/clkdev.h> > +#include <linux/clk/shmobile.h> > +#include <linux/of_address.h> > + > +struct r8a7778_cpg { > + struct clk_onecell_data data; > + spinlock_t lock; > + void __iomem *reg; > +}; > + > +/* EXTAL rate and PLL multipliers per bits 11, 12, and 18 of MODEMR */ > +struct { > + unsigned long extal_rate; > + unsigned long plla_mult; > + unsigned long pllb_mult; > +} r8a7778_rates[] __initdata = { > + [0] = { 38000000, 21, 21}, > + [1] = { 33333333, 24, 24}, > + [2] = { 28500000, 28, 28}, > + [3] = { 25000000, 32, 32}, > + [5] = { 33333333, 24, 21}, > + [6] = { 28500000, 28, 21}, > + [7] = { 25000000, 32, 24}, > +}; > + > +/* Clock dividers per bits 1 and 2 of MODEMR */ > +struct { > + const char *name; > + unsigned int div[4]; > +} r8a7778_divs[6] __initdata = { > + { "b", { 12, 12, 16, 18 } }, > + { "out", { 12, 12, 16, 18 } }, > + { "p", { 16, 12, 16, 12 } }, > + { "s", { 4, 3, 4, 3 } }, > + { "s1", { 8, 6, 8, 6 } }, > + { NULL }, > +}; > + > +static u32 cpg_mode_rates __initdata; > +static u32 cpg_mode_divs __initdata; > + > +static struct clk * __init > +r8a7778_cpg_register_clock(struct device_node *np, struct r8a7778_cpg *cpg, > + const char *name) > +{ > + if (!strcmp(name, "extal")) { > + return clk_register_fixed_rate(NULL, "extal", NULL, > + CLK_IS_ROOT, r8a7778_rates[cpg_mode_rates].extal_rate); As the external clock is an input to the CPG, shouldn't it be specified by a separate fixed-frequency clock DT node, and referenced by the CPG using the clocks attributes ? You could then remove the extal_rate field from the r8a7778_rates array above, and possibly rename the array to r8a7778_pll_mults or similar. > + } else if (!strcmp(name, "plla")) { > + return clk_register_fixed_factor(NULL, "plla", "extal", 0, > + r8a7778_rates[cpg_mode_rates].plla_mult, 1); > + } else if (!strcmp(name, "pllb")) { > + return clk_register_fixed_factor(NULL, "pllb", "extal", 0, > + r8a7778_rates[cpg_mode_rates].pllb_mult, 1); > + } else { > + unsigned int i; > + > + for (i = 0; r8a7778_divs[i].name; i++) { You could use i < ARRAY_SIZE(r8a7778_divs) here and avoid the NULL entry at the end of the array. > + if (!strcmp(name, r8a7778_divs[i].name)) { > + return clk_register_fixed_factor(NULL, > + r8a7778_divs[i].name, > + "plla", 0, 1, > + r8a7778_divs[i].div[cpg_mode_divs]); > + } > + } > + } > + > + return ERR_PTR(-EINVAL); > +} > + > + > +static void __init r8a7778_cpg_clocks_init(struct device_node *np) > +{ > + struct r8a7778_cpg *cpg; > + struct clk **clks; > + unsigned int i; > + int num_clks; > + > + num_clks = of_property_count_strings(np, "clock-output-names"); > + if (num_clks < 0) { > + pr_err("%s: failed to count clocks\n", __func__); > + return; > + } > + > + cpg = kzalloc(sizeof(*cpg), GFP_KERNEL); > + clks = kcalloc(num_clks, sizeof(*clks), GFP_KERNEL); > + if (cpg == NULL || clks == NULL) { > + /* We're leaking memory on purpose, there's no point in cleaning > + * up as the system won't boot anyway. > + */ > + return; > + } > + > + spin_lock_init(&cpg->lock); > + > + cpg->data.clks = clks; > + cpg->data.clk_num = num_clks; > + > + cpg->reg = of_iomap(np, 0); > + if (WARN_ON(cpg->reg == NULL)) > + return; > + > + for (i = 0; i < num_clks; ++i) { > + const char *name; > + struct clk *clk; > + > + of_property_read_string_index(np, "clock-output-names", i, > + &name); > + > + clk = r8a7778_cpg_register_clock(np, cpg, name); > + if (IS_ERR(clk)) > + pr_err("%s: failed to register %s %s clock (%ld)\n", > + __func__, np->name, name, PTR_ERR(clk)); > + else > + cpg->data.clks[i] = clk; > + } > + > + of_clk_add_provider(np, of_clk_src_onecell_get, &cpg->data); > +} > + > +CLK_OF_DECLARE(r8a7778_cpg_clks, "renesas,r8a7778-cpg-clocks", > + r8a7778_cpg_clocks_init); > + > +void __init r8a7778_clocks_init(u32 mode) > +{ > + if (!(mode & BIT(19))) > + BUG(); Nitpicking here, you could use BUG_ON(). > + cpg_mode_rates = (!!(mode & BIT(18)) << 2) | > + (!!(mode & BIT(12)) << 1) | > + (!!(mode & BIT(11))); > + cpg_mode_divs = (!!(mode & BIT(2)) << 1) | > + (!!(mode & BIT(1))); > + > + of_clk_init(NULL); > +}
Hi Ulrich, On Wed, Jan 21, 2015 at 5:20 PM, Ulrich Hecht <ulrich.hecht+renesas@gmail.com> wrote: > Driver for the r8a7778's clocks that depend on the mode bits. > > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> > --- > .../bindings/clock/renesas,r8a7778-cpg-clocks.txt | 24 ++++ > arch/arm/mach-shmobile/board-bockw-reference.c | 2 + > arch/arm/mach-shmobile/setup-r8a7778.c | 19 +++ > drivers/clk/shmobile/Makefile | 1 + > drivers/clk/shmobile/clk-r8a7778.c | 146 +++++++++++++++++++++ > include/linux/clk/shmobile.h | 1 + Shouldn't this be split in two parts: clk and arch/arm/mach-shmobile? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 22, 2015 at 12:37 AM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: >> + if (!strcmp(name, "extal")) { >> + return clk_register_fixed_rate(NULL, "extal", NULL, >> + CLK_IS_ROOT, r8a7778_rates[cpg_mode_rates].extal_rate); > > As the external clock is an input to the CPG, shouldn't it be specified by a > separate fixed-frequency clock DT node, and referenced by the CPG using the > clocks attributes ? You could then remove the extal_rate field from the > r8a7778_rates array above, and possibly rename the array to r8a7778_pll_mults > or similar. > This is a translation of what the legacy clock driver does. These frequencies seem to be assumptions as to what input clocks would be reasonable given the current mode bits. If I interpret the schematics correctly, the Bock-W board has a socketed oscillator, and the mode bits are switchable, so it's up to the user to configure the board to a sane setting. To me, neither a fixed frequency nor this list of educated guesses seems quite "right" under these circumstances... CU Uli -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulrich, On Friday 23 January 2015 18:00:02 Ulrich Hecht wrote: > On Thu, Jan 22, 2015 at 12:37 AM, Laurent Pinchart wrote: > >> + if (!strcmp(name, "extal")) { > >> + return clk_register_fixed_rate(NULL, "extal", NULL, > >> + CLK_IS_ROOT, > >> r8a7778_rates[cpg_mode_rates].extal_rate); > > > > As the external clock is an input to the CPG, shouldn't it be specified by > > a separate fixed-frequency clock DT node, and referenced by the CPG using > > the clocks attributes ? You could then remove the extal_rate field from > > the r8a7778_rates array above, and possibly rename the array to > > r8a7778_pll_mults or similar. > > This is a translation of what the legacy clock driver does. These > frequencies seem to be assumptions as to what input clocks would be > reasonable given the current mode bits. If I interpret the schematics > correctly, the Bock-W board has a socketed oscillator, and the mode > bits are switchable, so it's up to the user to configure the board to > a sane setting. To me, neither a fixed frequency nor this list of > educated guesses seems quite "right" under these circumstances... A separate fixed frequency clock DT matches the board configuration, so I think that's what should be used. It's true that the user could replace the oscillator, and in that case the DT will need to be updated. There's not much we can do for non-discoverable devices. Even if that solution isn't perfect, I think it's better than making the CPG node provide the external clock source, as it clearly doesn't from a hardware point of view.
diff --git a/Documentation/devicetree/bindings/clock/renesas,r8a7778-cpg-clocks.txt b/Documentation/devicetree/bindings/clock/renesas,r8a7778-cpg-clocks.txt new file mode 100644 index 0000000..c9a9544 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/renesas,r8a7778-cpg-clocks.txt @@ -0,0 +1,24 @@ +* Renesas R8A7778 Clock Pulse Generator (CPG) + +The CPG generates core clocks for the R8A7778. It includes two PLLs and +several fixed ratio dividers + +Required Properties: + + - compatible: Must be "renesas,r8a7778-cpg-clocks" + - reg: Base address and length of the memory resource used by the CPG + - #clock-cells: Must be 1 + - clock-output-names: The names of the clocks. Supported clocks are + "extal", "plla", "pllb", "b", "out", "p", "s", and "s1". + + +Example +------- + + cpg_clocks: cpg_clocks@ffc80000 { + compatible = "renesas,r8a7778-cpg-clocks"; + reg = <0xffc80000 0x80>; + #clock-cells = <1>; + clock-output-names = "extal", "plla", "pllb", "b", + "out", "p", "s", "s1"; + }; diff --git a/arch/arm/mach-shmobile/board-bockw-reference.c b/arch/arm/mach-shmobile/board-bockw-reference.c index d649ade..9a74efd 100644 --- a/arch/arm/mach-shmobile/board-bockw-reference.c +++ b/arch/arm/mach-shmobile/board-bockw-reference.c @@ -36,7 +36,9 @@ static void __init bockw_init(void) void __iomem *fpga; void __iomem *pfc; +#ifndef CONFIG_COMMON_CLK r8a7778_clock_init(); +#endif r8a7778_init_irq_extpin_dt(1); r8a7778_add_dt_devices(); diff --git a/arch/arm/mach-shmobile/setup-r8a7778.c b/arch/arm/mach-shmobile/setup-r8a7778.c index cef8895..c49aa09 100644 --- a/arch/arm/mach-shmobile/setup-r8a7778.c +++ b/arch/arm/mach-shmobile/setup-r8a7778.c @@ -15,6 +15,7 @@ * GNU General Public License for more details. */ +#include <linux/clk/shmobile.h> #include <linux/kernel.h> #include <linux/io.h> #include <linux/irqchip/arm-gic.h> @@ -41,6 +42,21 @@ #include "irqs.h" #include "r8a7778.h" +#define MODEMR 0xffcc0020 + +#ifdef CONFIG_COMMON_CLK +static void __init r8a7778_timer_init(void) +{ + u32 mode; + void __iomem *modemr = ioremap_nocache(MODEMR, 4); + + BUG_ON(!modemr); + mode = ioread32(modemr); + iounmap(modemr); + r8a7778_clocks_init(mode); +} +#endif + /* SCIF */ #define R8A7778_SCIF(index, baseaddr, irq) \ static struct plat_sci_port scif##index##_platform_data = { \ @@ -608,6 +624,9 @@ DT_MACHINE_START(R8A7778_DT, "Generic R8A7778 (Flattened Device Tree)") .init_early = shmobile_init_delay, .init_irq = r8a7778_init_irq_dt, .init_late = shmobile_init_late, +#ifdef CONFIG_COMMON_CLK + .init_time = r8a7778_timer_init, +#endif .dt_compat = r8a7778_compat_dt, MACHINE_END diff --git a/drivers/clk/shmobile/Makefile b/drivers/clk/shmobile/Makefile index 0689d7f..97c71c8 100644 --- a/drivers/clk/shmobile/Makefile +++ b/drivers/clk/shmobile/Makefile @@ -2,6 +2,7 @@ obj-$(CONFIG_ARCH_EMEV2) += clk-emev2.o obj-$(CONFIG_ARCH_R7S72100) += clk-rz.o obj-$(CONFIG_ARCH_R8A73A4) += clk-r8a73a4.o obj-$(CONFIG_ARCH_R8A7740) += clk-r8a7740.o +obj-$(CONFIG_ARCH_R8A7778) += clk-r8a7778.o obj-$(CONFIG_ARCH_R8A7779) += clk-r8a7779.o obj-$(CONFIG_ARCH_R8A7790) += clk-rcar-gen2.o obj-$(CONFIG_ARCH_R8A7791) += clk-rcar-gen2.o diff --git a/drivers/clk/shmobile/clk-r8a7778.c b/drivers/clk/shmobile/clk-r8a7778.c new file mode 100644 index 0000000..30c5c14 --- /dev/null +++ b/drivers/clk/shmobile/clk-r8a7778.c @@ -0,0 +1,146 @@ +/* + * r8a7778 Core CPG Clocks + * + * Copyright (C) 2014 Ulrich Hecht + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + */ + +#include <linux/clk-provider.h> +#include <linux/clkdev.h> +#include <linux/clk/shmobile.h> +#include <linux/of_address.h> + +struct r8a7778_cpg { + struct clk_onecell_data data; + spinlock_t lock; + void __iomem *reg; +}; + +/* EXTAL rate and PLL multipliers per bits 11, 12, and 18 of MODEMR */ +struct { + unsigned long extal_rate; + unsigned long plla_mult; + unsigned long pllb_mult; +} r8a7778_rates[] __initdata = { + [0] = { 38000000, 21, 21}, + [1] = { 33333333, 24, 24}, + [2] = { 28500000, 28, 28}, + [3] = { 25000000, 32, 32}, + [5] = { 33333333, 24, 21}, + [6] = { 28500000, 28, 21}, + [7] = { 25000000, 32, 24}, +}; + +/* Clock dividers per bits 1 and 2 of MODEMR */ +struct { + const char *name; + unsigned int div[4]; +} r8a7778_divs[6] __initdata = { + { "b", { 12, 12, 16, 18 } }, + { "out", { 12, 12, 16, 18 } }, + { "p", { 16, 12, 16, 12 } }, + { "s", { 4, 3, 4, 3 } }, + { "s1", { 8, 6, 8, 6 } }, + { NULL }, +}; + +static u32 cpg_mode_rates __initdata; +static u32 cpg_mode_divs __initdata; + +static struct clk * __init +r8a7778_cpg_register_clock(struct device_node *np, struct r8a7778_cpg *cpg, + const char *name) +{ + if (!strcmp(name, "extal")) { + return clk_register_fixed_rate(NULL, "extal", NULL, + CLK_IS_ROOT, r8a7778_rates[cpg_mode_rates].extal_rate); + } else if (!strcmp(name, "plla")) { + return clk_register_fixed_factor(NULL, "plla", "extal", 0, + r8a7778_rates[cpg_mode_rates].plla_mult, 1); + } else if (!strcmp(name, "pllb")) { + return clk_register_fixed_factor(NULL, "pllb", "extal", 0, + r8a7778_rates[cpg_mode_rates].pllb_mult, 1); + } else { + unsigned int i; + + for (i = 0; r8a7778_divs[i].name; i++) { + if (!strcmp(name, r8a7778_divs[i].name)) { + return clk_register_fixed_factor(NULL, + r8a7778_divs[i].name, + "plla", 0, 1, + r8a7778_divs[i].div[cpg_mode_divs]); + } + } + } + + return ERR_PTR(-EINVAL); +} + + +static void __init r8a7778_cpg_clocks_init(struct device_node *np) +{ + struct r8a7778_cpg *cpg; + struct clk **clks; + unsigned int i; + int num_clks; + + num_clks = of_property_count_strings(np, "clock-output-names"); + if (num_clks < 0) { + pr_err("%s: failed to count clocks\n", __func__); + return; + } + + cpg = kzalloc(sizeof(*cpg), GFP_KERNEL); + clks = kcalloc(num_clks, sizeof(*clks), GFP_KERNEL); + if (cpg == NULL || clks == NULL) { + /* We're leaking memory on purpose, there's no point in cleaning + * up as the system won't boot anyway. + */ + return; + } + + spin_lock_init(&cpg->lock); + + cpg->data.clks = clks; + cpg->data.clk_num = num_clks; + + cpg->reg = of_iomap(np, 0); + if (WARN_ON(cpg->reg == NULL)) + return; + + for (i = 0; i < num_clks; ++i) { + const char *name; + struct clk *clk; + + of_property_read_string_index(np, "clock-output-names", i, + &name); + + clk = r8a7778_cpg_register_clock(np, cpg, name); + if (IS_ERR(clk)) + pr_err("%s: failed to register %s %s clock (%ld)\n", + __func__, np->name, name, PTR_ERR(clk)); + else + cpg->data.clks[i] = clk; + } + + of_clk_add_provider(np, of_clk_src_onecell_get, &cpg->data); +} + +CLK_OF_DECLARE(r8a7778_cpg_clks, "renesas,r8a7778-cpg-clocks", + r8a7778_cpg_clocks_init); + +void __init r8a7778_clocks_init(u32 mode) +{ + if (!(mode & BIT(19))) + BUG(); + cpg_mode_rates = (!!(mode & BIT(18)) << 2) | + (!!(mode & BIT(12)) << 1) | + (!!(mode & BIT(11))); + cpg_mode_divs = (!!(mode & BIT(2)) << 1) | + (!!(mode & BIT(1))); + + of_clk_init(NULL); +} diff --git a/include/linux/clk/shmobile.h b/include/linux/clk/shmobile.h index 9f8a140..63a8159 100644 --- a/include/linux/clk/shmobile.h +++ b/include/linux/clk/shmobile.h @@ -16,6 +16,7 @@ #include <linux/types.h> +void r8a7778_clocks_init(u32 mode); void r8a7779_clocks_init(u32 mode); void rcar_gen2_clocks_init(u32 mode);
Driver for the r8a7778's clocks that depend on the mode bits. Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> --- .../bindings/clock/renesas,r8a7778-cpg-clocks.txt | 24 ++++ arch/arm/mach-shmobile/board-bockw-reference.c | 2 + arch/arm/mach-shmobile/setup-r8a7778.c | 19 +++ drivers/clk/shmobile/Makefile | 1 + drivers/clk/shmobile/clk-r8a7778.c | 146 +++++++++++++++++++++ include/linux/clk/shmobile.h | 1 + 6 files changed, 193 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/renesas,r8a7778-cpg-clocks.txt create mode 100644 drivers/clk/shmobile/clk-r8a7778.c