From patchwork Sun Apr 14 21:19:17 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nishanth Menon X-Patchwork-Id: 2442761 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) by patchwork1.kernel.org (Postfix) with ESMTP id 47C353FCA5 for ; Sun, 14 Apr 2013 21:19:42 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1URUKi-0008Ln-GW; Sun, 14 Apr 2013 21:19:32 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1URUKg-0003EF-4L; Sun, 14 Apr 2013 21:19:30 +0000 Received: from arroyo.ext.ti.com ([192.94.94.40]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1URUKb-0003Dw-Mm for linux-arm-kernel@lists.infradead.org; Sun, 14 Apr 2013 21:19:27 +0000 Received: from dlelxv30.itg.ti.com ([172.17.2.17]) by arroyo.ext.ti.com (8.13.7/8.13.7) with ESMTP id r3ELJICX022968; Sun, 14 Apr 2013 16:19:18 -0500 Received: from DLEE71.ent.ti.com (dlee71.ent.ti.com [157.170.170.114]) by dlelxv30.itg.ti.com (8.13.8/8.13.8) with ESMTP id r3ELJIV9022147; Sun, 14 Apr 2013 16:19:18 -0500 Received: from dlelxv22.itg.ti.com (172.17.1.197) by DLEE71.ent.ti.com (157.170.170.114) with Microsoft SMTP Server id 14.2.342.3; Sun, 14 Apr 2013 16:19:17 -0500 Received: from localhost (h0-132.vpn.ti.com [172.24.0.132]) by dlelxv22.itg.ti.com (8.13.8/8.13.8) with ESMTP id r3ELJI61006788; Sun, 14 Apr 2013 16:19:18 -0500 Date: Sun, 14 Apr 2013 16:19:17 -0500 From: Nishanth Menon To: Tony Lindgren Subject: Re: [PATCH V4 1/6] clk: OMAP: introduce device tree binding to kernel clock data Message-ID: <20130414211917.GA3275@snafu> References: <1365807278-554-1-git-send-email-nm@ti.com> <1365807278-554-2-git-send-email-nm@ti.com> <20130412233142.GE10155@atomide.com> <20130412233900.GA1599@kahuna> <20130413172210.GM10155@atomide.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20130413172210.GM10155@atomide.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130414_171925_894864_F6C7EA46 X-CRM114-Status: GOOD ( 32.97 ) X-Spam-Score: -7.6 (-------) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-7.6 points) pts rule name description ---- ---------------------- -------------------------------------------------- -5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/, high trust [192.94.94.40 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -0.7 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Paul Walmsley , Mike Turquette , =?iso-8859-1?Q?Beno=EEt?= Cousson , Kevin Hilman , devicetree-discuss@lists.ozlabs.org, linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org On 10:22-20130413, Tony Lindgren wrote: > * Nishanth Menon [130412 16:43]: > > Thanks for checking up. Fixed all of them below, will post part of > > series again, only if I need to address further comments in other > > patches.. > > Thanks it seems that the other ones are ready to go, just one > more comment below. > [...] > > +static struct clk *omap_clk_src_get(struct of_phandle_args *clkspec, void *data) > > +{ > > + struct clk *clk; > > + char clk_name[32]; > > + struct device_node *np = clkspec->np; > > + > > + snprintf(clk_name, 32, "%s_ck", np->name); > > + clk = clk_get(NULL, clk_name); > > + if (IS_ERR(clk)) { > > + pr_err("%s: could not get clock %s(%ld)\n", __func__, > > + clk_name, PTR_ERR(clk)); > > + goto out; > > + } > > + clk_put(clk); > > It seems that clk_put() is actually wrong here. That's because > of_clk_get() should boild down to just the look up of the clock > and then clk_get() on it, so no double clk_get() is done in this > case. Once the consumer driver is done, it will just call clk_put() > on it. Yep - updated version below. From d0bf3fce235cff46feac7f5ef1a40e2fa0f2aa12 Mon Sep 17 00:00:00 2001 From: Nishanth Menon Date: Tue, 9 Apr 2013 19:26:40 -0500 Subject: [PATCH V5.1 1/6] clk: OMAP: introduce device tree binding to kernel clock data OMAP clock data is located in arch/arm/mach-omap2/cclockXYZ_data.c. However, this presents an obstacle for using these clock nodes in Device Tree definitions. This is especially true for board specific clocks initially. The fixed clocks are currently found via clock aliases table. There are many possible approaches to this problem as discussed in the following thread: http://marc.info/?t=136370325600009&r=1&w=2. Highlights of the options: a) device specific clk_add_alias: cons: driver handling required b) using an generic clk node and indexing to reach the clock required. This is similar in approach taken by tegra and few other platforms. Example usage: clock = <&clk 5>; cons: potential to have mismatches in indexed table and associated dtb data. In addition, managing continued documentation in bindings as clock indexing increases. Even though readability angle could be improved by using preprocessing of DT using macros, indexed approach is inherently risky from cases like the following: clk indexes in kernel: 1 - mpu_dpll 2 - aux_clk1 3 - core_clk DT entry for peripheral X uses <&clk 2> to reach aux_clk1. Now, let's say kernel updates indices to: 1 - mpu_dpll 2 - per_dpll 3 - aux_clk1 4 - core_clk using the old dtb(or dts missing an update), on new kernel which has updated indices will result in per_dpll now controlled for peripheral X without warning or any potential error detection. Even though we could claim this is user error, such errors are hard to track down and fix. An alternate approach introduced here is to introduce device tree bindings corresponding to the clock nodes required in DT definition for SoC which automatically maps back to the definitions in cclockXYZ_data.c. The driver introduced here to do this mapping will eventually be the place where the clock handling will migrate to. We need to consider this angle as well so that the solution will be an valid transition point for moving the clock data out of kernel image (into device tree or firmware load etc..). Overall strategy introduced here is simple: a clock node described in device tree blob is used to identify the exact clock provided in the SoC specific data. This is then linked back using of_clk_add_provider to the device node to be accessible by of_clk_get. Based on discussion contributions from Roger Quadros, Grygorii Strashko and others. Cc: Kevin Hilman Cc: Mike Turquette Cc: Paul Walmsley [tony@atomide.com: co-developed] Signed-off-by: Tony Lindgren Signed-off-by: Nishanth Menon --- .../devicetree/bindings/clock/omap-clock.txt | 40 +++++++++ drivers/clk/Makefile | 1 + drivers/clk/omap/Makefile | 1 + drivers/clk/omap/clk.c | 91 ++++++++++++++++++++ 4 files changed, 133 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/omap-clock.txt create mode 100644 drivers/clk/omap/Makefile create mode 100644 drivers/clk/omap/clk.c diff --git a/Documentation/devicetree/bindings/clock/omap-clock.txt b/Documentation/devicetree/bindings/clock/omap-clock.txt new file mode 100644 index 0000000..047c1e7 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/omap-clock.txt @@ -0,0 +1,40 @@ +Device Tree Clock bindings for Texas Instrument's OMAP compatible platforms + +This binding is an initial minimal binding that may be enhanced as part of +transitioning OMAP clock data out of kernel image. + +This binding uses the common clock binding[1]. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : shall be "ti,omap-clock" +- #clock-cells : from common clock binding; shall be set to 0. +NOTE: +node name should map to clock database in arch/arm/mach-omap2/cclock_data.c +Since all clocks are described with _ck, the node name is optimized to drop the +usage of _ck. For example, a clock called dpll1_ck will be defined as dpll1. + +Example #1: describing clock node for CPU on OMAP34xx platform: +Ref: arch/arm/mach-omap2/cclock3xxx_data.c +describes the CPU clock to be as follows + CLK(NULL, "dpll1_ck", &dpll1_ck, CK_3XXX), +Corresponding binding will be: + dpll1: dpll1 { + #clock-cells = <0>; + compatible = "ti,omap-clock"; + }; +And it's usage will be: + clocks = <&dpll1>; + +Example #2: describing clock node for auxilary clock #3 on OMAP443x platform: +Ref: arch/arm/mach-omap2/cclock44xx_data.c +describes the auxclk3 clock to be as follows: + CLK(NULL, "auxclk3_ck", &auxclk3_ck, CK_443X), +Corresponding binding will be: + auxclk3: auxclk3 { + #clock-cells = <0>; + compatible = "ti,omap-clock"; + }; +And it's usage will be: + clocks = <&auxclk3>; diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index e7f7fe9..dbf9b13 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -29,6 +29,7 @@ obj-$(CONFIG_ARCH_U8500) += ux500/ obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o obj-$(CONFIG_ARCH_ZYNQ) += clk-zynq.o obj-$(CONFIG_ARCH_TEGRA) += tegra/ +obj-$(CONFIG_ARCH_OMAP) += omap/ obj-$(CONFIG_X86) += x86/ diff --git a/drivers/clk/omap/Makefile b/drivers/clk/omap/Makefile new file mode 100644 index 0000000..8195931 --- /dev/null +++ b/drivers/clk/omap/Makefile @@ -0,0 +1 @@ +obj-y += clk.o diff --git a/drivers/clk/omap/clk.c b/drivers/clk/omap/clk.c new file mode 100644 index 0000000..5a3c6d9 --- /dev/null +++ b/drivers/clk/omap/clk.c @@ -0,0 +1,91 @@ +/* + * Texas Instruments OMAP Clock driver + * + * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com/ + * Nishanth Menon + * Tony Lindgren + * + * 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 +#include + +static const struct of_device_id omap_clk_of_match[] = { + {.compatible = "ti,omap-clock",}, + {}, +}; + +/** + * omap_clk_src_get() - Get OMAP clock from node name when needed + * @clkspec: clkspec argument + * @data: unused + * + * REVISIT: We assume the following: + * 1. omap clock names end with _ck + * 2. omap clock names are under 32 characters in length + */ +static struct clk *omap_clk_src_get(struct of_phandle_args *clkspec, void *data) +{ + struct clk *clk; + char clk_name[32]; + struct device_node *np = clkspec->np; + + snprintf(clk_name, 32, "%s_ck", np->name); + clk = clk_get(NULL, clk_name); + if (IS_ERR(clk)) + pr_err("%s: could not get clock %s(%ld)\n", __func__, + clk_name, PTR_ERR(clk)); + + return clk; +} + +/** + * omap_clk_probe() - create link from DT definition to clock data + * @pdev: device node + * + * NOTE: we look up the clock lazily when the consumer driver does + * of_clk_get() and initialize a NULL clock here. + */ +static int omap_clk_probe(struct platform_device *pdev) +{ + int res; + struct device_node *np = pdev->dev.of_node; + + /* This allows the driver to of_clk_get() */ + res = of_clk_add_provider(np, omap_clk_src_get, NULL); + if (res) + dev_err(&pdev->dev, "could not add provider(%d)\n", res); + + return res; +} + +static struct platform_driver omap_clk_driver = { + .probe = omap_clk_probe, + .driver = { + .name = "omap_clk", + .of_match_table = of_match_ptr(omap_clk_of_match), + }, +}; + +static int __init omap_clk_init(void) +{ + return platform_driver_register(&omap_clk_driver); +} +arch_initcall(omap_clk_init); + +MODULE_DESCRIPTION("OMAP Clock driver"); +MODULE_AUTHOR("Texas Instruments Inc."); +MODULE_LICENSE("GPL v2");