[PATCH 05/10] ARM: clps711x: Add CLPS711X clk driver
Mark Rutland
mark.rutland at arm.com
Fri Aug 2 06:25:40 EDT 2013
On Thu, Jul 18, 2013 at 07:34:56PM +0100, Alexander Shiyan wrote:
> This adds the clock driver for Cirrus Logic CLPS711X series SoCs
> using common clock infrastructure.
> Designed primarily for migration CLPS711X subarch for multiplatform & DT,
> for this as the "OF" and "not-OF" calls implemented.
> Additionally, patch removes unnecessary symbol CLKDEV_LOOKUP for CLPS711X
> from main ARM Kconfig since this symbol is selected automatically by COMMON_CLK.
>
> Signed-off-by: Alexander Shiyan <shc_work at mail.ru>
> ---
> .../devicetree/bindings/clock/clps711x-clock.txt | 47 +++++++
> arch/arm/Kconfig | 1 -
> drivers/clk/Makefile | 1 +
> drivers/clk/clk-clps711x.c | 150 +++++++++++++++++++++
> 4 files changed, 198 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/clock/clps711x-clock.txt
> create mode 100644 drivers/clk/clk-clps711x.c
>
> diff --git a/Documentation/devicetree/bindings/clock/clps711x-clock.txt b/Documentation/devicetree/bindings/clock/clps711x-clock.txt
> new file mode 100644
> index 0000000..0fb9fff
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/clps711x-clock.txt
> @@ -0,0 +1,47 @@
> +Device Tree Clock bindings for the Cirrus Logic CLPS711X CPUs
> +
> +=== Clock subsystem ===
> +Required properties:
> +- compatible: Should be "cirrus,clps711x-clk"
> +- reg: Address of the internal register set
> +- #clock-cells: Should be <1>
> +
> +The clock consumer should specify the desired clock by having the clock
> +ID in its "clocks" phandle cell. The following table is a full list of
> +CLPS711X clocks and IDs.
> +
> + Clock ID
> + ------------------
> + dummy 0
> + cpu 1
> + pll 2
> + bus 3
> + timer_hf 4
> + timer_lf 5
> + tc1 6
> + tc2 7
> + spi 8
> + uart 9
> +
> +=== Clocksource subsystem ===
> +Required properties:
> +- compatible: Should be "cirrus,clps711x-clocksource"
> +- reg: Address of the internal register set
> +- clocks: phandle to the source clocks TC1 and TC2
> +- interrupts: The interrupt of the TC2 timer
That sounds odd. What's the interrupt generated by?
> +
> +Example:
> +
> +clks: clks at 80000000 {
> + compatible = "cirrus,clps711x-clk";
> + reg = <0x80000000 0>;
Zero length?
> + #clock-cells = <1>;
> +};
> +
> +timer {
> + compatible = "cirrus,clps711x-clocksource";
> + reg = <0x80000000 0>;
Zero length again?
> + clocks = <&clks 6>, <&clks 7>;
> + clock-names = "tc1", "tc2";
> + interrupts = <9>;
> +};
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index ba412e0..dfb1e7f 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -370,7 +370,6 @@ config ARCH_CLPS711X
> bool "Cirrus Logic CLPS711x/EP721x/EP731x-based"
> select ARCH_REQUIRE_GPIOLIB
> select AUTO_ZRELADDR
> - select CLKDEV_LOOKUP
> select CLKSRC_MMIO
> select COMMON_CLK
> select CPU_ARM720T
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 4038c2b..a7e9b73 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_COMMON_CLK) += clk-composite.o
>
> # SoCs specific
> obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.o
> +obj-$(CONFIG_ARCH_CLPS711X) += clk-clps711x.o
> obj-$(CONFIG_ARCH_NOMADIK) += clk-nomadik.o
> obj-$(CONFIG_ARCH_HIGHBANK) += clk-highbank.o
> obj-$(CONFIG_ARCH_NSPIRE) += clk-nspire.o
> diff --git a/drivers/clk/clk-clps711x.c b/drivers/clk/clk-clps711x.c
> new file mode 100644
> index 0000000..662908c
> --- /dev/null
> +++ b/drivers/clk/clk-clps711x.c
> @@ -0,0 +1,150 @@
> +/*
> + * CLPS711X clk driver
> + *
> + * Copyright (C) 2013 Alexander Shiyan <shc_work at mail.ru>
> + *
> + * 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; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#define pr_fmt(fmt) "clk-clps711x: " fmt
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/clkdev.h>
> +#include <linux/io.h>
> +#include <linux/ioport.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +
> +#include <linux/mfd/syscon/clps711x.h>
> +
> +#define CLPS711X_SYSCON1 (0x0100)
> +#define CLPS711X_SYSCON2 (0x1100)
> +#define CLPS711X_PLLR (0xa5a8)
> +
> +#define CLPS711X_EXT_FREQ (13000000)
> +#define CLPS711X_OSC_FREQ (3686400)
> +
> +enum clps711x_clks {
> + dummy, cpu, pll, bus, timer_hf, timer_lf, tc1, tc2, spi, uart, clk_max
> +};
> +
> +static struct {
> + struct clk_onecell_data clk_data;
> + struct clk *clks[clk_max];
> +} *clps711x_clk;
> +
> +static void __init _clps711x_clk_init(phys_addr_t phys_base)
> +{
> + const char *tc_parents[] = { "timer_lf", "timer_hf" };
> + u32 tmp, f_cpu, f_pll, f_bus, f_timh, f_spi;
> + void __iomem *pllr, *syscon1, *syscon2;
> +
> + BUG_ON(!request_mem_region(phys_base + CLPS711X_PLLR, SZ_4, NULL));
> + BUG_ON(!request_mem_region(phys_base + CLPS711X_SYSCON1, SZ_128, NULL));
> + BUG_ON(!request_mem_region(phys_base + CLPS711X_SYSCON2, SZ_128, NULL));
This is a wider issue than just this driver, but is BUG_ON appropriate
here? Is a pr_warn("Could not initialise [some clocks], it may not be
possible to use the machine") and a return not good enough?
Also, doesn't that leak an allocated resource? Surely there's a better
way to check that the window given to you was big enough (e.g. make this
take a resource or device_node, and check the size on that).
> +
> + clps711x_clk = kzalloc(sizeof(*clps711x_clk), GFP_KERNEL);
> + BUG_ON(!clps711x_clk);
> +
> + pllr = ioremap(phys_base + CLPS711X_PLLR, SZ_4);
> + BUG_ON(!pllr);
> +
> + syscon1 = ioremap(phys_base + CLPS711X_SYSCON1, SZ_128);
> + BUG_ON(!syscon1);
> +
> + syscon2 = ioremap(phys_base + CLPS711X_SYSCON2, SZ_128);
> + BUG_ON(!syscon2);
> +
> + tmp = readl(pllr) >> 24;
> + if (tmp)
> + f_pll = DIV_ROUND_UP(CLPS711X_OSC_FREQ * tmp, 2);
> + else
> + f_pll = 73728000; /* Old CPUs default value */
A comment explaining what the value of ((readl(pllr) >> 24) represents
would be nice.
> +
> + tmp = readl(syscon2 + SYSFLG_OFFSET);
> + if (tmp & SYSFLG2_CKMODE) {
> + f_cpu = CLPS711X_EXT_FREQ;
> + f_bus = CLPS711X_EXT_FREQ;
> + f_spi = 135400;
> + f_pll = 0;
> + } else {
> + f_cpu = f_pll;
> + if (f_cpu >= 36864000)
> + f_bus = DIV_ROUND_UP(f_cpu, 2);
> + else
> + f_bus = 36864000 / 2;
> + f_spi = DIV_ROUND_CLOSEST(f_cpu, 576);
> + }
> +
> + if (tmp & SYSFLG2_CKMODE) {
> + if (readl(syscon2 + SYSCON_OFFSET) & SYSCON2_OSTB)
> + f_timh = DIV_ROUND_CLOSEST(CLPS711X_EXT_FREQ, 26);
> + else
> + f_timh = DIV_ROUND_CLOSEST(CLPS711X_EXT_FREQ, 24);
> + } else
> + f_timh = DIV_ROUND_CLOSEST(f_cpu, 144);
> +
> + clps711x_clk->clks[dummy] =
> + clk_register_fixed_rate(NULL, "dummy", NULL, CLK_IS_ROOT, 0);
> + clps711x_clk->clks[cpu] =
> + clk_register_fixed_rate(NULL, "cpu", NULL, CLK_IS_ROOT, f_cpu);
> + clps711x_clk->clks[pll] =
> + clk_register_fixed_rate(NULL, "pll", NULL, CLK_IS_ROOT, f_pll);
> + clps711x_clk->clks[bus] =
> + clk_register_fixed_rate(NULL, "bus", NULL, CLK_IS_ROOT, f_bus);
> + clps711x_clk->clks[timer_hf] =
> + clk_register_fixed_rate(NULL, "timer_hf", NULL, CLK_IS_ROOT,
> + f_timh);
> + clps711x_clk->clks[timer_lf] =
> + clk_register_fixed_factor(NULL, "timer_lf", "timer_hf",
> + 0, 1, 256);
> + clps711x_clk->clks[tc1] =
> + clk_register_mux(NULL, "tc1", tc_parents, 2,
> + CLK_GET_RATE_NOCACHE, syscon1, 5, 1, 0, NULL);
> + clps711x_clk->clks[tc2] =
> + clk_register_mux(NULL, "tc2", tc_parents, 2,
> + CLK_GET_RATE_NOCACHE, syscon1, 7, 1, 0, NULL);
> + clps711x_clk->clks[spi] =
> + clk_register_fixed_rate(NULL, "spi", NULL, CLK_IS_ROOT, f_spi);
> + clps711x_clk->clks[uart] =
> + clk_register_fixed_factor(NULL, "uart", "bus", 0, 1, 10);
> +
> + clk_set_parent(clps711x_clk->clks[tc1], clps711x_clk->clks[timer_lf]);
> + clk_set_parent(clps711x_clk->clks[tc2], clps711x_clk->clks[timer_hf]);
> +
> + /* Temporary export clocks for non-DT capable drivers */
> + clk_register_clkdev(clps711x_clk->clks[pll], "pll", NULL);
> + clk_register_clkdev(clps711x_clk->clks[spi], "spi", NULL);
> + clk_register_clkdev(clps711x_clk->clks[uart], "uart", NULL);
If you've booted with dt, do those drivers get probed from platform
data, and use these clocks?
I have a concern that if we allow that, we're forced to keep platform
data for those devices to maintain backwards compatibility, as otherwise
we require dt changes later (adding those devices and their clock
properties) to keep those devices functioning, rather than only
requiring dt changes later to add new device support.
> +
> + pr_debug("CPU: %u Hz, Timer: %u Hz\n", f_cpu, f_timh);
> +}
> +
> +void __init clps711x_clk_init(phys_addr_t phys_base)
> +{
This could take a resource and pass it on...
> + _clps711x_clk_init(phys_base);
> +
> + clk_register_clkdev(clps711x_clk->clks[tc1], "tc1", NULL);
> + clk_register_clkdev(clps711x_clk->clks[tc2], "tc2", NULL);
> +}
Thanks,
Mark.
More information about the linux-arm-kernel
mailing list