From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754213AbeAQRf7 (ORCPT ); Wed, 17 Jan 2018 12:35:59 -0500 Received: from vern.gendns.com ([206.190.152.46]:52249 "EHLO vern.gendns.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752522AbeAQRf4 (ORCPT ); Wed, 17 Jan 2018 12:35:56 -0500 Subject: Re: [PATCH v5 19/44] clk: davinci: New driver for TI DA8XX CFGCHIP clocks To: Sekhar Nori , linux-clk@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org Cc: Michael Turquette , Stephen Boyd , Rob Herring , Mark Rutland , Kevin Hilman , Adam Ford , linux-kernel@vger.kernel.org References: <1515377863-20358-1-git-send-email-david@lechnology.com> <1515377863-20358-20-git-send-email-david@lechnology.com> From: David Lechner Message-ID: <19f2830e-6d57-ebd9-1873-f79bea3e8439@lechnology.com> Date: Wed, 17 Jan 2018 11:35:54 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - vern.gendns.com X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - lechnology.com X-Get-Message-Sender-Via: vern.gendns.com: authenticated_id: davidmain+lechnology.com/only user confirmed/virtual account not confirmed X-Authenticated-Sender: vern.gendns.com: davidmain@lechnology.com X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/17/2018 09:31 AM, Sekhar Nori wrote: > On Monday 08 January 2018 07:47 AM, David Lechner wrote: >> This adds a new driver for the gate and multiplexer clocks in the >> CFGCHIPn syscon registers on TI DA8XX-type SoCs. >> >> Signed-off-by: David Lechner >> --- >> drivers/clk/davinci/Makefile | 2 + >> drivers/clk/davinci/da8xx-cfgchip.c | 203 ++++++++++++++++++++++++++++++++++++ >> 2 files changed, 205 insertions(+) >> create mode 100644 drivers/clk/davinci/da8xx-cfgchip.c >> >> diff --git a/drivers/clk/davinci/Makefile b/drivers/clk/davinci/Makefile >> index 6c388d4..11178b7 100644 >> --- a/drivers/clk/davinci/Makefile >> +++ b/drivers/clk/davinci/Makefile >> @@ -1,6 +1,8 @@ >> # SPDX-License-Identifier: GPL-2.0 >> >> ifeq ($(CONFIG_COMMON_CLK), y) >> +obj-$(CONFIG_ARCH_DAVINCI_DA8XX) += da8xx-cfgchip.o >> + >> obj-y += pll.o >> obj-$(CONFIG_ARCH_DAVINCI_DA830) += pll-da830.o >> obj-$(CONFIG_ARCH_DAVINCI_DA850) += pll-da850.o >> diff --git a/drivers/clk/davinci/da8xx-cfgchip.c b/drivers/clk/davinci/da8xx-cfgchip.c >> new file mode 100644 >> index 0000000..772e09a >> --- /dev/null >> +++ b/drivers/clk/davinci/da8xx-cfgchip.c >> @@ -0,0 +1,203 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Clock driver for DA8xx/AM17xx/AM18xx/OMAP-L13x CFGCHIP >> + * >> + * Copyright (C) 2017 David Lechner > > 2018 > >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#ifdef CONFIG_OF > > Is this ifdef really needed, or included to save space for non-OF > builds? I think it can be removed if not really needed. > >> +static void da8xx_cfgchip_gate_clk_init(struct device_node *np, u32 reg, >> + u32 mask) >> +{ >> + struct da8xx_cfgchip_gate_clk *clk; >> + struct clk_init_data init; >> + const char *name = np->name; >> + const char *parent_name; >> + struct regmap *regmap; >> + int ret; >> + >> + of_property_read_string(np, "clock-output-names", &name); >> + parent_name = of_clk_get_parent_name(np, 0); >> + >> + regmap = syscon_node_to_regmap(of_get_parent(np)); >> + if (IS_ERR(regmap)) { >> + pr_err("%s: no regmap for syscon parent of %s (%ld)\n", >> + __func__, np->full_name, PTR_ERR(regmap)); > > please use pr_fmt for this driver too. > >> +static void da8xx_cfgchip_mux_clk_init(struct device_node *np, u32 reg, >> + u32 mask) >> +{ >> + struct da8xx_cfgchip_mux_clk *clk; >> + struct clk_init_data init; >> + const char *name = np->name; >> + const char *parent_names[2]; >> + struct regmap *regmap; >> + int ret; >> + >> + ret = of_property_match_string(np, "clock-names", "pll0_sysclk2"); >> + parent_names[0] = of_clk_get_parent_name(np, ret); >> + if (!parent_names[0]) { >> + pr_err("%s: missing pll0_sysclk2 clock\n", __func__); >> + return; >> + } >> + >> + ret = of_property_match_string(np, "clock-names", "pll1_sysclk2"); >> + parent_names[1] = of_clk_get_parent_name(np, ret); >> + if (!parent_names[1]) { >> + pr_err("%s: missing pll1_sysclk2 clock\n", __func__); >> + return; >> + } > > The fact that you are looking specifically for pll0_sysclk2 and > pll1_sysclk2 makes it really specific to async3 and the same function > cannot be used for something like EMIFA clock source. Can this part of > the function be factored out so rest of the function can still be reused > for another clock? > Already fixed this stuff a couple days ago. ;-) And I went ahead and added the EMIFA clocks (the mux and the DIV4.5)