| Message ID | 1444916813-31024-10-git-send-email-linus.walleij@linaro.org |
|---|---|
| State | Accepted |
| Commit | d430819d69a51dc4798bb98d841afa9af2f5c83a |
| Headers | show |
On 10/15, Linus Walleij wrote: > + > + if (of_device_is_compatible(np, "arm,syscon-icst525")) > + icst_desc.params = &icst525_params; > + else if (of_device_is_compatible(np, "arm,syscon-icst307")) > + icst_desc.params = &icst307_params; I guess if we add anymore here we should use an of_device_id array instead. > + else { > + pr_info("unknown ICST clock %s\n", name); pr_warn? pr_err? > + return; > + } > + > + /* Parent clock name is not the same as node parent */ > + parent_name = of_clk_get_parent_name(np, 0); > + > + regclk = icst_clk_setup(NULL, &icst_desc, name, parent_name, map); > + if (IS_ERR(regclk)) { > + pr_err("error setting up syscon ICST clock %s\n", name); > + return; > + } > + of_clk_add_provider(np, of_clk_src_simple_get, regclk); > + pr_info("registered syscon ICST clock %s\n", name); debug print? Please remove debug noise.
On 10/15, Linus Walleij wrote: > This adds support for the ARM syscon ICST clocks to initialized > directly from the device tree syscon node on ARM Integrator, > Versatile and RealView reference designs. > > Cc: Michael Turquette <mturquette@baylibre.com> > Cc: Stephen Boyd <sboyd@codeaurora.org> > Cc: linux-clk@vger.kernel.org > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- Also: WARNING: please, no space before tabs #47: FILE: drivers/clk/versatile/clk-icst.c:221: +^I.vd_min ^I= 8,$ WARNING: please, no space before tabs #48: FILE: drivers/clk/versatile/clk-icst.c:222: +^I.vd_max ^I= 263,$ WARNING: please, no space before tabs #49: FILE: drivers/clk/versatile/clk-icst.c:223: +^I.rd_min ^I= 3,$ WARNING: please, no space before tabs #50: FILE: drivers/clk/versatile/clk-icst.c:224: +^I.rd_max ^I= 65,$ total: 0 errors, 4 warnings, 98 lines checked
On Thu, Oct 15, 2015 at 9:26 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: > On 10/15, Linus Walleij wrote: >> + >> + if (of_device_is_compatible(np, "arm,syscon-icst525")) >> + icst_desc.params = &icst525_params; >> + else if (of_device_is_compatible(np, "arm,syscon-icst307")) >> + icst_desc.params = &icst307_params; > > I guess if we add anymore here we should use an of_device_id > array instead. As it happens those two are gonna be it. ARM never created any more integrated ICST devices, and they stopped using them since. Those two are the required ones. (Fixing all other comments.) Yours, Linus Walleij _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Oct 26, 2015 at 2:31 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > [Me] >> ARM never created any more integrated ICST devices, and >> they stopped using them since. Those two are the required >> ones. > > As ARM didn't create any ICST devices at all, that's hardly surprising. > These devices are created by Integrated Circuit Systems, Inc. The 525 > is a parallel-loaded clock generator, the 307 is a serial-loaded clock > generator. > > However, they have no "standard" software interface - indeed, the 525 > is marketed as a device that needs no processor or software to be used, > so it doesn't have a "software" interface as such. RIght, I wrote a blurb with the more elaborate and correct story for the device tree bindings. I get sloppy sometimes. > ARM Ltd's hardware on these boards provides interfaces to these, however > the underlying ICST support I wrote was factored to separate out the > interface from the chip support - I haven't been tracking what's been > going on with these, but I hope that separation has been kept as it's > entirely logical, and describing these things in DT as an ARM Ltd device, > combining the ICST device itself with its interface would be wrong. So the device tree bindings does say that, and the compatible strings are "arm,syscon-icst525" or "arm,syscon-icst307" indicating that it is indeed the ARM syscon register-mapped thing, and the ICST sits on the back of that register. The logical separation is indeed kept, if someone ever decides to interface the ICST clocks in some other way, the code is reusable. Yours, Linus Walleij _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/drivers/clk/versatile/clk-icst.c b/drivers/clk/versatile/clk-icst.c index ab5d5f73818b..b2dc06923ac1 100644 --- a/drivers/clk/versatile/clk-icst.c +++ b/drivers/clk/versatile/clk-icst.c @@ -3,7 +3,7 @@ * We wrap the custom interface from <asm/hardware/icst.h> into the generic * clock framework. * - * Copyright (C) 2012 Linus Walleij + * Copyright (C) 2012-2015 Linus Walleij * * 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 @@ -207,3 +207,90 @@ struct clk *icst_clk_register(struct device *dev, return icst_clk_setup(dev, desc, name, parent_name, map); } EXPORT_SYMBOL_GPL(icst_clk_register); + +#ifdef CONFIG_OF +/* + * In a device tree, an memory-mapped ICST clock appear as a child + * of a syscon node. Assume this and probe it only as a child of a + * syscon. + */ + +static const struct icst_params icst525_params = { + .vco_max = ICST525_VCO_MAX_5V, + .vco_min = ICST525_VCO_MIN, + .vd_min = 8, + .vd_max = 263, + .rd_min = 3, + .rd_max = 65, + .s2div = icst525_s2div, + .idx2s = icst525_idx2s, +}; + +static const struct icst_params icst307_params = { + .vco_max = ICST307_VCO_MAX, + .vco_min = ICST307_VCO_MIN, + .vd_min = 4 + 8, + .vd_max = 511 + 8, + .rd_min = 1 + 2, + .rd_max = 127 + 2, + .s2div = icst307_s2div, + .idx2s = icst307_idx2s, +}; + +static void __init of_syscon_icst_setup(struct device_node *np) +{ + struct device_node *parent; + struct regmap *map; + struct clk_icst_desc icst_desc; + const char *name = np->name; + const char *parent_name; + struct clk *regclk; + + /* We do not release this reference, we are using it perpetually */ + parent = of_get_parent(np); + if (!parent) { + pr_err("no parent node for syscon ICST clock\n"); + return; + } + map = syscon_node_to_regmap(parent); + if (IS_ERR(map)) { + pr_err("no regmap for syscon ICST clock parent\n"); + return; + } + + if (of_property_read_u32(np, "vco-offset", &icst_desc.vco_offset)) { + pr_err("no VCO register offset for ICST clock\n"); + return; + } + if (of_property_read_u32(np, "lock-offset", &icst_desc.lock_offset)) { + pr_err("no lock register offset for ICST clock\n"); + return; + } + + if (of_device_is_compatible(np, "arm,syscon-icst525")) + icst_desc.params = &icst525_params; + else if (of_device_is_compatible(np, "arm,syscon-icst307")) + icst_desc.params = &icst307_params; + else { + pr_info("unknown ICST clock %s\n", name); + return; + } + + /* Parent clock name is not the same as node parent */ + parent_name = of_clk_get_parent_name(np, 0); + + regclk = icst_clk_setup(NULL, &icst_desc, name, parent_name, map); + if (IS_ERR(regclk)) { + pr_err("error setting up syscon ICST clock %s\n", name); + return; + } + of_clk_add_provider(np, of_clk_src_simple_get, regclk); + pr_info("registered syscon ICST clock %s\n", name); +} + +CLK_OF_DECLARE(arm_syscon_icst525_clk, + "arm,syscon-icst525", of_syscon_icst_setup); +CLK_OF_DECLARE(arm_syscon_icst307_clk, + "arm,syscon-icst307", of_syscon_icst_setup); + +#endif
This adds support for the ARM syscon ICST clocks to initialized directly from the device tree syscon node on ARM Integrator, Versatile and RealView reference designs. Cc: Michael Turquette <mturquette@baylibre.com> Cc: Stephen Boyd <sboyd@codeaurora.org> Cc: linux-clk@vger.kernel.org Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- I'm looking for an ACK from the CLK maintainers to take this through the ARM SoC tree once the series stabilize. --- drivers/clk/versatile/clk-icst.c | 89 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 88 insertions(+), 1 deletion(-)