From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tero Kristo Subject: Re: [PATCHv9 07/43] CLK: TI: add autoidle support Date: Fri, 1 Nov 2013 11:18:45 +0200 Message-ID: <527371F5.9080702@ti.com> References: <1382716658-6964-1-git-send-email-t-kristo@ti.com> <1382716658-6964-8-git-send-email-t-kristo@ti.com> <52727FDA.7060009@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <52727FDA.7060009@ti.com> Sender: linux-omap-owner@vger.kernel.org To: Nishanth Menon , linux-omap@vger.kernel.org, paul@pwsan.com, tony@atomide.com, bcousson@baylibre.com, rnayak@ti.com, mturquette@linaro.org Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org List-Id: devicetree@vger.kernel.org On 10/31/2013 06:05 PM, Nishanth Menon wrote: > On 10/25/2013 10:57 AM, Tero Kristo wrote: >> TI clk driver now routes some of the basic clocks through own >> registration routine to allow autoidle support. This routine just >> checks a couple of device node properties and adds autoidle support >> if required, and just passes the registration forward to basic clocks. >> >> Signed-off-by: Tero Kristo >> --- >> arch/arm/mach-omap2/clock.c | 6 +++ >> drivers/clk/ti/Makefile | 2 +- >> drivers/clk/ti/autoidle.c | 109 +++++++++++++++++++++++++++++++++++++++++++ >> include/linux/clk/ti.h | 9 ++++ >> 4 files changed, 125 insertions(+), 1 deletion(-) >> create mode 100644 drivers/clk/ti/autoidle.c >> >> diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c >> index 0c38ca9..223f432b 100644 >> --- a/arch/arm/mach-omap2/clock.c >> +++ b/arch/arm/mach-omap2/clock.c >> @@ -520,6 +520,9 @@ int omap2_clk_enable_autoidle_all(void) >> list_for_each_entry(c, &clk_hw_omap_clocks, node) >> if (c->ops && c->ops->allow_idle) >> c->ops->allow_idle(c); >> + >> + of_ti_clk_allow_autoidle_all(); >> + >> return 0; >> } >> >> @@ -539,6 +542,9 @@ int omap2_clk_disable_autoidle_all(void) >> list_for_each_entry(c, &clk_hw_omap_clocks, node) >> if (c->ops && c->ops->deny_idle) >> c->ops->deny_idle(c); >> + >> + of_ti_clk_deny_autoidle_all(); >> + >> return 0; >> } >> >> diff --git a/drivers/clk/ti/Makefile b/drivers/clk/ti/Makefile >> index 05af5d8..533efb4 100644 >> --- a/drivers/clk/ti/Makefile >> +++ b/drivers/clk/ti/Makefile >> @@ -1,3 +1,3 @@ >> ifneq ($(CONFIG_OF),) >> -obj-y += clk.o dpll.o >> +obj-y += clk.o dpll.o autoidle.o >> endif >> diff --git a/drivers/clk/ti/autoidle.c b/drivers/clk/ti/autoidle.c >> new file mode 100644 >> index 0000000..efa2a3e >> --- /dev/null >> +++ b/drivers/clk/ti/autoidle.c >> @@ -0,0 +1,109 @@ >> +/* >> + * TI clock autoidle support >> + * >> + * Copyright (C) 2013 Texas Instruments, Inc. >> + * >> + * Tero Kristo >> + * >> + * 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. >> + * >> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any >> + * kind, whether express or implied; without even the implied warranty >> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +struct clk_ti_autoidle { >> + u32 reg; >> + struct regmap *regmap; >> + u8 shift; >> + u8 flags; >> + const char *name; >> + struct list_head node; >> +}; >> + >> +#define AUTOIDLE_LOW 0x1 >> + >> +static LIST_HEAD(autoidle_clks); >> + >> +static void ti_allow_autoidle(struct clk_ti_autoidle *clk) >> +{ >> + u32 val; >> + >> + regmap_read(clk->regmap, clk->reg, &val); >> + >> + if (clk->flags & AUTOIDLE_LOW) >> + val &= ~(1 << clk->shift); >> + else >> + val |= (1 << clk->shift); >> + >> + regmap_write(clk->regmap, clk->reg, val); > regmap_update_bits ? Hmm yea that would work. >> +} >> + >> +static void ti_deny_autoidle(struct clk_ti_autoidle *clk) >> +{ >> + u32 val; >> + >> + regmap_read(clk->regmap, clk->reg, &val); >> + >> + if (clk->flags & AUTOIDLE_LOW) >> + val |= (1 << clk->shift); >> + else >> + val &= ~(1 << clk->shift); >> + >> + regmap_write(clk->regmap, clk->reg, val); > regmap_update_bits ? Same. > > and ofcourse error handling for regmap ops.. What do you propose to do in error case? Panic, WARN() or just printk? > >> +} >> + >> +void of_ti_clk_allow_autoidle_all(void) >> +{ >> + struct clk_ti_autoidle *c; >> + >> + list_for_each_entry(c, &autoidle_clks, node) >> + ti_allow_autoidle(c); >> +} >> + >> +void of_ti_clk_deny_autoidle_all(void) >> +{ >> + struct clk_ti_autoidle *c; >> + >> + list_for_each_entry(c, &autoidle_clks, node) >> + ti_deny_autoidle(c); >> +} >> + >> +int __init of_ti_autoidle_setup(struct device_node *node, >> + struct regmap *regmap) > > will be nice if you could kernel-doc all public functions at the very > least. Ok, I can try to do that for next. :P > >> +{ >> + u32 shift; >> + struct clk_ti_autoidle *clk; >> + >> + if (of_property_read_u32(node, "ti,autoidle-shift", &shift)) >> + return 0; >> + >> + clk = kzalloc(sizeof(*clk), GFP_KERNEL); >> + >> + if (!clk) { >> + pr_err("%s: kzalloc failed\n", __func__); >> + return -ENOMEM; >> + } >> + >> + clk->shift = shift; >> + clk->name = node->name; >> + of_property_read_u32(node, "reg", &clk->reg); > > dt binding? > what happens if someone forgets this? You get zero offset. I can add a check for this though. > >> + clk->regmap = regmap; >> + >> + if (of_property_read_bool(node, "ti,invert-autoidle-bit")) >> + clk->flags |= AUTOIDLE_LOW; >> + >> + list_add(&clk->node, &autoidle_clks); >> + >> + return 0; >> +} >> diff --git a/include/linux/clk/ti.h b/include/linux/clk/ti.h >> index 7ab02fa..e45005c 100644 >> --- a/include/linux/clk/ti.h >> +++ b/include/linux/clk/ti.h >> @@ -178,6 +178,15 @@ int omap3_dpll4_set_rate(struct clk_hw *clk, unsigned long rate, >> >> void ti_dt_clocks_register(struct ti_dt_clk *oclks); >> void ti_dt_clk_init_provider(struct device_node *np, struct regmap *regmap); >> +int of_ti_autoidle_setup(struct device_node *node, struct regmap *regmap); >> + >> +#ifdef CONFIG_OF >> +void of_ti_clk_allow_autoidle_all(void); >> +void of_ti_clk_deny_autoidle_all(void); >> +#else >> +static inline void of_ti_clk_allow_autoidle_all(void) { } >> +static inline void of_ti_clk_deny_autoidle_all(void) { } >> +#endif >> >> extern const struct clk_hw_omap_ops clkhwops_omap3_dpll; >> extern const struct clk_hw_omap_ops clkhwops_omap4_dpllmx; >> > >