| Message ID | 1368332581-94691-4-git-send-email-dt.tangr@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sunday 12 May 2013, Daniel Tang wrote: > > Signed-off-by: Daniel Tang <dt.tangr@gmail.com> > --- > drivers/clk/Makefile | 1 + > drivers/clk/clk-nspire.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 142 insertions(+) > create mode 100644 drivers/clk/clk-nspire.c You are missing a binding in Documentation/devicetree, same as for some of the other drivers in this series. > +static int nspire_clk_read(struct device_node *node, > + struct nspire_clk_info *clk) > +{ > + u32 val; > + int ret; > + void __iomem *io; > + const char *type = NULL; > + > + ret = of_property_read_string(node, "io-type", &type); > + if (ret) > + return ret; Using a string here feels clumsy. Why not use the "compatible" property to distinguish the different models? > +static void __init nspire_ahbdiv_setup(struct device_node *node) > +{ > + int ret; > + struct clk *clk; > + const char *clk_name = node->name; > + const char *parent_name; > + struct nspire_clk_info info; > + > + ret = nspire_clk_read(node, &info); > + if (WARN_ON(ret)) > + return; > + > + of_property_read_string(node, "clock-output-names", &clk_name); It seems strange to assign the clk_name variable to node->name first and then overriding it with the clock-output-names property. Is that intentional? If so, please explain it in a comment. > + > + pr_info("TI-NSPIRE Base: %uMHz CPU: %uMHz AHB: %uMHz\n", > + info.base_clock / MHZ, > + info.base_clock / info.base_cpu_ratio / MHZ, > + info.base_clock / info.base_ahb_ratio / MHZ); > +} > + > +CLK_OF_DECLARE(nspire_clk, "nspire-clock", nspire_clk_setup); > +CLK_OF_DECLARE(nspire_ahbdiv, "nspire-ahb-divider", nspire_ahbdiv_setup); I would put each of these lines directly under the function it references. Arnd
On 16/05/2013, at 12:07 AM, Arnd Bergmann <arnd@arndb.de> wrote: > You are missing a binding in Documentation/devicetree, same as for some of > the other drivers in this series. Should we be adding a vendor prefix to it too? If so, we're not sure whether to use "ti," or not since this isn't an official port by TI. > It seems strange to assign the clk_name variable to node->name > first and then overriding it with the clock-output-names property. > Is that intentional? If so, please explain it in a comment. > I copied that bit of boilerplate from drivers/clk/clk-fixed-rate.c but I'm guessing it's to use the node name as the clock name unless there is a property called "clock-output-names" Cheers, Daniel Tang
On Thursday 16 May 2013, Daniel Tang wrote: > > On 16/05/2013, at 12:07 AM, Arnd Bergmann <arnd@arndb.de> wrote: > > > You are missing a binding in Documentation/devicetree, same as for some of > > the other drivers in this series. > > Should we be adding a vendor prefix to it too? If so, we're not sure whether > to use "ti," or not since this isn't an official port by TI. The binding describes the hardware, it should not matter who does the port. However, any part of the tree that is not actually from TI should have a vendor prefix indicating who made that part. IIRC, the SoC used in there is from TI, so you should use something else for the on-soc components. > > It seems strange to assign the clk_name variable to node->name > > first and then overriding it with the clock-output-names property. > > Is that intentional? If so, please explain it in a comment. > > > > I copied that bit of boilerplate from drivers/clk/clk-fixed-rate.c but > I'm guessing it's to use the node name as the clock name unless there > is a property called "clock-output-names" Ah, I see. It seems you forgot to add the clock maintainer to Cc in the mail. Mike is the one who will have to take you patch anyway, so I assume he will comment on this if you did it wrong. Arnd
On 16/05/2013, at 10:17 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Thursday 16 May 2013, Daniel Tang wrote: >> >> On 16/05/2013, at 12:07 AM, Arnd Bergmann <arnd@arndb.de> wrote: >> >>> You are missing a binding in Documentation/devicetree, same as for some of >>> the other drivers in this series. >> >> Should we be adding a vendor prefix to it too? If so, we're not sure whether >> to use "ti," or not since this isn't an official port by TI. > > The binding describes the hardware, it should not matter who does the port. > However, any part of the tree that is not actually from TI should have a > vendor prefix indicating who made that part. IIRC, the SoC used in there > is from TI, so you should use something else for the on-soc components. > If the vendors for the on-SOC components are unknown, should we just leave the compatible strings as is (i.e. "nspire-XXX")? >>> It seems strange to assign the clk_name variable to node->name >>> first and then overriding it with the clock-output-names property. >>> Is that intentional? If so, please explain it in a comment. >>> >> >> I copied that bit of boilerplate from drivers/clk/clk-fixed-rate.c but >> I'm guessing it's to use the node name as the clock name unless there >> is a property called "clock-output-names" > > Ah, I see. It seems you forgot to add the clock maintainer to Cc in the > mail. Mike is the one who will have to take you patch anyway, so I assume > he will comment on this if you did it wrong. > > Arnd
On Sunday 19 May 2013, Daniel Tang wrote: > If the vendors for the on-SOC components are unknown, should we just > leave the compatible strings as is (i.e. "nspire-XXX")? In that case, I would use the name of the company that made the SoC. I believe someone mentioned it was made by LSI logic. Arnd
On 20/05/2013, at 5:48 AM, Arnd Bergmann <arnd@arndb.de> wrote: > On Sunday 19 May 2013, Daniel Tang wrote: >> If the vendors for the on-SOC components are unknown, should we just >> leave the compatible strings as is (i.e. "nspire-XXX")? > > In that case, I would use the name of the company that made the SoC. > I believe someone mentioned it was made by LSI logic. Yep, that's right. So "lsi,nspire-XXX" should be fine? > > Arnd
On Monday 20 May 2013 21:19:33 Daniel Tang wrote: > On 20/05/2013, at 5:48 AM, Arnd Bergmann <arnd@arndb.de> wrote: > > > On Sunday 19 May 2013, Daniel Tang wrote: > >> If the vendors for the on-SOC components are unknown, should we just > >> leave the compatible strings as is (i.e. "nspire-XXX")? > > > > In that case, I would use the name of the company that made the SoC. > > I believe someone mentioned it was made by LSI logic. > > Yep, that's right. So "lsi,nspire-XXX" should be fine? I wouldn't use that combination, since lsi did not make the nspire. For anything on the LSI chip, "lsi,zevio-XXX" would be correct, or maybe "lsi,zevio1020-XXX" for something specific to a particular SoC variant. For off-SoC parts, "ti,nspire-XXX" would be right, or "ti,nspire-cx-XXX" if you care about the model. Arnd
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index 137d3e7..72ebbe1 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -13,6 +13,7 @@ obj-$(CONFIG_COMMON_CLK) += clk-composite.o obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.o obj-$(CONFIG_ARCH_NOMADIK) += clk-nomadik.o obj-$(CONFIG_ARCH_HIGHBANK) += clk-highbank.o +obj-$(CONFIG_ARCH_NSPIRE) += clk-nspire.o obj-$(CONFIG_ARCH_MXS) += mxs/ obj-$(CONFIG_ARCH_SOCFPGA) += socfpga/ obj-$(CONFIG_PLAT_SPEAR) += spear/ diff --git a/drivers/clk/clk-nspire.c b/drivers/clk/clk-nspire.c new file mode 100644 index 0000000..2a8d14c --- /dev/null +++ b/drivers/clk/clk-nspire.c @@ -0,0 +1,141 @@ +/* + * linux/drivers/clk/clk-nspire.c + * + * Copyright (C) 2013 Daniel Tang <tangrs@tangrs.id.au> + * + * 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. + * + */ + +#include <linux/clk-provider.h> +#include <linux/err.h> +#include <linux/io.h> +#include <linux/of.h> +#include <linux/of_address.h> + +#define MHZ (1000 * 1000) + +#define BASE_CPU_SHIFT 1 +#define BASE_CPU_MASK 0x7F + +#define CPU_AHB_SHIFT 12 +#define CPU_AHB_MASK 0x07 + +#define FIXED_BASE_SHIFT 8 +#define FIXED_BASE_MASK 0x01 + +#define CLASSIC_BASE_SHIFT 16 +#define CLASSIC_BASE_MASK 0x1F + +#define CX_BASE_SHIFT 15 +#define CX_BASE_MASK 0x3F + +#define CX_UNKNOWN_SHIFT 21 +#define CX_UNKNOWN_MASK 0x03 + +#define EXTRACT(var, prop) (((var)>>prop##_SHIFT) & prop##_MASK) + +struct nspire_clk_info { + u32 base_clock; + u16 base_cpu_ratio; + u16 base_ahb_ratio; +}; + +static int nspire_clk_read(struct device_node *node, + struct nspire_clk_info *clk) +{ + u32 val; + int ret; + void __iomem *io; + const char *type = NULL; + + ret = of_property_read_string(node, "io-type", &type); + if (ret) + return ret; + + io = of_iomap(node, 0); + if (!io) + return -ENOMEM; + val = readl(io); + iounmap(io); + + if (!strcmp(type, "cx")) { + if (EXTRACT(val, FIXED_BASE)) { + clk->base_clock = 48 * MHZ; + } else { + clk->base_clock = 6 * EXTRACT(val, CX_BASE) * MHZ; + } + + clk->base_cpu_ratio = EXTRACT(val, BASE_CPU) * + EXTRACT(val, CX_UNKNOWN); + clk->base_ahb_ratio = clk->base_cpu_ratio * + (EXTRACT(val, CPU_AHB) + 1); + } else if (!strcmp(type, "classic")) { + if (EXTRACT(val, FIXED_BASE)) { + clk->base_clock = 27 * MHZ; + } else { + clk->base_clock = (300 - + 6 * EXTRACT(val, CLASSIC_BASE)) * MHZ; + } + + clk->base_cpu_ratio = EXTRACT(val, BASE_CPU) * 2; + clk->base_ahb_ratio = clk->base_cpu_ratio * + (EXTRACT(val, CPU_AHB) + 1); + } else { + return -EINVAL; + } + + return 0; +} + +static void __init nspire_ahbdiv_setup(struct device_node *node) +{ + int ret; + struct clk *clk; + const char *clk_name = node->name; + const char *parent_name; + struct nspire_clk_info info; + + ret = nspire_clk_read(node, &info); + if (WARN_ON(ret)) + return; + + of_property_read_string(node, "clock-output-names", &clk_name); + parent_name = of_clk_get_parent_name(node, 0); + + clk = clk_register_fixed_factor(NULL, clk_name, parent_name, 0, + 1, info.base_ahb_ratio); + if (!IS_ERR(clk)) + of_clk_add_provider(node, of_clk_src_simple_get, clk); +} + +static void __init nspire_clk_setup(struct device_node *node) +{ + int ret; + struct clk *clk; + const char *clk_name = node->name; + struct nspire_clk_info info; + + ret = nspire_clk_read(node, &info); + if (WARN_ON(ret)) + return; + + of_property_read_string(node, "clock-output-names", &clk_name); + + clk = clk_register_fixed_rate(NULL, clk_name, NULL, CLK_IS_ROOT, + info.base_clock); + if (!IS_ERR(clk)) + of_clk_add_provider(node, of_clk_src_simple_get, clk); + else + return; + + pr_info("TI-NSPIRE Base: %uMHz CPU: %uMHz AHB: %uMHz\n", + info.base_clock / MHZ, + info.base_clock / info.base_cpu_ratio / MHZ, + info.base_clock / info.base_ahb_ratio / MHZ); +} + +CLK_OF_DECLARE(nspire_clk, "nspire-clock", nspire_clk_setup); +CLK_OF_DECLARE(nspire_ahbdiv, "nspire-ahb-divider", nspire_ahbdiv_setup);
Signed-off-by: Daniel Tang <dt.tangr@gmail.com> --- drivers/clk/Makefile | 1 + drivers/clk/clk-nspire.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+) create mode 100644 drivers/clk/clk-nspire.c