From patchwork Wed Nov 15 17:06:47 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Boyd X-Patchwork-Id: 10059889 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 108FF60231 for ; Wed, 15 Nov 2017 17:07:09 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0293C28D80 for ; Wed, 15 Nov 2017 17:07:09 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id EB4EF29C21; Wed, 15 Nov 2017 17:07:08 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0E1B428F5B for ; Wed, 15 Nov 2017 17:07:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932439AbdKORGx (ORCPT ); Wed, 15 Nov 2017 12:06:53 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:45930 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932380AbdKORGv (ORCPT ); Wed, 15 Nov 2017 12:06:51 -0500 Received: by smtp.codeaurora.org (Postfix, from userid 1000) id 15FD36083D; Wed, 15 Nov 2017 17:06:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1510765611; bh=BiWZGrz9NwngRirScO5l9q+gd3a0PJftUlQxPtFU/E0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=RrPoX4M/qOv4WzFsORwaynV9nNF7biq+xwNjPRsYa9/uO1sJmwj+xg7tAJwBFk25Q mfK+lfv3H1mvqEkBBhrWzFPkY4LzwJxMypw3bpfs+cuqsNafft8Wt0Dd1qu3JWuRnk NMiJj56HA7ZEEApIAc6OPYPf64GA/QW8aaAhnTKw= Received: from localhost (i-global254.qualcomm.com [199.106.103.254]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) (Authenticated sender: sboyd@smtp.codeaurora.org) by smtp.codeaurora.org (Postfix) with ESMTPSA id 7A1D5606F8; Wed, 15 Nov 2017 17:06:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1510765608; bh=BiWZGrz9NwngRirScO5l9q+gd3a0PJftUlQxPtFU/E0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=KzEcPNL8lDHWHJiAIl/kKPDt83vxSARUUKUPqfva1Kt2b0Hdr/xOHry7Cl6LA18ub rZkmStd/EjOzhnme0KBg3/T/G0wZAv4xcEZK99XdijEIXWP7iGFIRu9UTDIg/LFC+O CvLe20qYWkMvDTnRbAWOxRoyPft+TbPIhhvZ3uis= DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 7A1D5606F8 Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=sboyd@codeaurora.org Date: Wed, 15 Nov 2017 09:06:47 -0800 From: Stephen Boyd To: Tirupathi Reddy Cc: mturquette@baylibre.com, robh+dt@kernel.org, mark.rutland@arm.com, andy.gross@linaro.org, david.brown@linaro.org, linux-clk@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org Subject: Re: [PATCH V5] clk: qcom: Add spmi_pmic clock divider support Message-ID: <20171115170647.GR11955@codeaurora.org> References: <1505817286-12445-1-git-send-email-tirupath@codeaurora.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1505817286-12445-1-git-send-email-tirupath@codeaurora.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-clk-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-clk@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 09/19, Tirupathi Reddy wrote: > diff --git a/Documentation/devicetree/bindings/clock/clk-spmi-pmic-div.txt b/Documentation/devicetree/bindings/clock/clk-spmi-pmic-div.txt > new file mode 100644 > index 0000000..e37a136 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/clk-spmi-pmic-div.txt > @@ -0,0 +1,56 @@ > +Qualcomm Technologies, Inc. SPMI PMIC clock divider (clkdiv) > + > +clkdiv configures the clock frequency of a set of outputs on the PMIC. > +These clocks are typically wired through alternate functions on > +gpio pins. > + > +======================= > +Properties > +======================= > + > +- compatible > + Usage: required > + Value type: > + Definition: must be one of: > + "qcom,spmi-clkdiv" > + "qcom,pm8998-clkdiv" > + > +- reg > + Usage: required > + Value type: > + Definition: Addresses and sizes for the memory of this CLKDIV There's no size though. > + peripheral. > + > +- clocks: > + Usage: required > + Value type: > + Definition: reference to the xo clock. > + > +- clock-names: > + Usage: required > + Value type: > + Definition: must be "xo". > + > +- clock-cells: > + Usage: required > + Value type: > + Definition: shall contain 1. Can we get a property to indicate the number of clks? Something like "qcom,num-clkdivs" or something like that. That would make things easier to handle in the driver because we could match the generic compatible and look for the property and then populate that many clks. Otherwise, we're going to need a "small" driver update for each PMIC just because the number of clks changes. Of course we're going to need to update the binding to add the new PMIC compatibles as new things are created, but I'd like to avoid driver updates for the compatible. If someone messes up the number we can always fallback to matching the more specific compatible and fix it up in the driver, but let's hope that doesn't happen. > + > +======= > +Example > +======= > + > +pm8998_clk_divs: qcom,clock@5b00 { clock-controller@5b00? > + compatible = "qcom,pm8998-clkdiv"; > + reg = <0x5b00>; > + #clock-cells = <1>; > + clocks = <&xo_board>; > + clock-names = "xo"; > + > + assigned-clocks = <&pm8998_clk_divs 1>, > + <&pm8998_clk_divs 2>, > + <&pm8998_clk_divs 3>; > + assigned-clock-rates = <9600000>, > + <9600000>, > + <9600000>; > +}; > diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig > index 9f6c278..dd5b18e 100644 > --- a/drivers/clk/qcom/Kconfig > +++ b/drivers/clk/qcom/Kconfig > @@ -196,3 +196,12 @@ config MSM_MMCC_8996 > Support for the multimedia clock controller on msm8996 devices. > Say Y if you want to support multimedia devices such as display, > graphics, video encode/decode, camera, etc. > + > +config SPMI_PMIC_CLKDIV > + tristate "spmi pmic clkdiv driver" Capitalize SPMI and PMIC and replace driver with something else. > + depends on (COMMON_CLK_QCOM && SPMI) || COMPILE_TEST > + help > + This driver supports the clkdiv functionality on the Qualcomm > + Technologies, Inc. SPMI PMIC. It configures the frequency of > + clkdiv outputs on the PMIC. These clocks are typically wired > + through alternate functions on gpio pins. I rewrote a bunch of stuff in the driver. Let me know if anything doesn't work. ----8<---- diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig index dd5b18ecd9d1..20b5d6fd501d 100644 --- a/drivers/clk/qcom/Kconfig +++ b/drivers/clk/qcom/Kconfig @@ -198,10 +198,10 @@ config MSM_MMCC_8996 graphics, video encode/decode, camera, etc. config SPMI_PMIC_CLKDIV - tristate "spmi pmic clkdiv driver" + tristate "SPMI PMIC clkdiv Support" depends on (COMMON_CLK_QCOM && SPMI) || COMPILE_TEST help This driver supports the clkdiv functionality on the Qualcomm Technologies, Inc. SPMI PMIC. It configures the frequency of - clkdiv outputs on the PMIC. These clocks are typically wired - through alternate functions on gpio pins. + clkdiv outputs of the PMIC. These clocks are typically wired + through alternate functions on GPIO pins. diff --git a/drivers/clk/qcom/clk-spmi-pmic-div.c b/drivers/clk/qcom/clk-spmi-pmic-div.c index af343ad2ee0b..a7217ee9f741 100644 --- a/drivers/clk/qcom/clk-spmi-pmic-div.c +++ b/drivers/clk/qcom/clk-spmi-pmic-div.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -29,29 +30,12 @@ #define REG_EN_CTL 0x46 #define REG_EN_MASK BIT(7) -#define ENABLE_DELAY_NS(cxo_ns, div) ((2 + 3 * div) * cxo_ns) -#define DISABLE_DELAY_NS(cxo_ns, div) ((3 * div) * cxo_ns) - -#define CLK_SPMI_PMIC_DIV_OFFSET 1 - -#define CLKDIV_XO_DIV_1_0 0 -#define CLKDIV_XO_DIV_1 1 -#define CLKDIV_XO_DIV_2 2 -#define CLKDIV_XO_DIV_4 3 -#define CLKDIV_XO_DIV_8 4 -#define CLKDIV_XO_DIV_16 5 -#define CLKDIV_XO_DIV_32 6 -#define CLKDIV_XO_DIV_64 7 -#define CLKDIV_MAX_ALLOWED 8 - struct clkdiv { struct regmap *regmap; u16 base; spinlock_t lock; - /* clock properties */ struct clk_hw hw; - unsigned int div_factor; unsigned int cxo_period_ns; }; @@ -62,94 +46,68 @@ static inline struct clkdiv *to_clkdiv(struct clk_hw *hw) static inline unsigned int div_factor_to_div(unsigned int div_factor) { - if (div_factor == CLKDIV_XO_DIV_1_0) - return 1; + if (!div_factor) + div_factor = 1; - return 1 << (div_factor - CLK_SPMI_PMIC_DIV_OFFSET); + return 1 << (div_factor - 1); } static inline unsigned int div_to_div_factor(unsigned int div) { - return min(ilog2(div) + CLK_SPMI_PMIC_DIV_OFFSET, - CLKDIV_MAX_ALLOWED - 1); + return min(ilog2(div) + 1, 7); } static bool is_spmi_pmic_clkdiv_enabled(struct clkdiv *clkdiv) { unsigned int val = 0; - regmap_read(clkdiv->regmap, clkdiv->base + REG_EN_CTL, - &val); - return (val & REG_EN_MASK) ? true : false; + regmap_read(clkdiv->regmap, clkdiv->base + REG_EN_CTL, &val); + + return val & REG_EN_MASK; } -static int spmi_pmic_clkdiv_set_enable_state(struct clkdiv *clkdiv, - bool enable_state) +static int +__spmi_pmic_clkdiv_set_enable_state(struct clkdiv *clkdiv, bool enable, + unsigned int div_factor) { - int rc; + int ret; + unsigned int ns = clkdiv->cxo_period_ns; + unsigned int div = div_factor_to_div(div_factor); - rc = regmap_update_bits(clkdiv->regmap, clkdiv->base + REG_EN_CTL, - REG_EN_MASK, - (enable_state == true) ? REG_EN_MASK : 0); - if (rc) - return rc; + ret = regmap_update_bits(clkdiv->regmap, clkdiv->base + REG_EN_CTL, + REG_EN_MASK, enable ? REG_EN_MASK : 0); + if (ret) + return ret; - if (enable_state == true) - ndelay(ENABLE_DELAY_NS(clkdiv->cxo_period_ns, - div_factor_to_div(clkdiv->div_factor))); + if (enable) + ndelay((2 + 3 * div) * ns); else - ndelay(DISABLE_DELAY_NS(clkdiv->cxo_period_ns, - div_factor_to_div(clkdiv->div_factor))); + ndelay(3 * div * ns); - return rc; + return 0; } -static int spmi_pmic_clkdiv_config_freq_div(struct clkdiv *clkdiv, - unsigned int div) +static int spmi_pmic_clkdiv_set_enable_state(struct clkdiv *clkdiv, bool enable) { unsigned int div_factor; - unsigned long flags; - bool enabled; - int rc; - - div_factor = div_to_div_factor(div); - - spin_lock_irqsave(&clkdiv->lock, flags); - - enabled = is_spmi_pmic_clkdiv_enabled(clkdiv); - if (enabled) { - rc = spmi_pmic_clkdiv_set_enable_state(clkdiv, false); - if (rc) - goto fail; - } - rc = regmap_update_bits(clkdiv->regmap, - clkdiv->base + REG_DIV_CTL1, - DIV_CTL1_DIV_FACTOR_MASK, div_factor); - if (rc) - goto fail; + regmap_read(clkdiv->regmap, clkdiv->base + REG_DIV_CTL1, &div_factor); + div_factor &= DIV_CTL1_DIV_FACTOR_MASK; - clkdiv->div_factor = div_factor; - - if (enabled) - rc = spmi_pmic_clkdiv_set_enable_state(clkdiv, true); - -fail: - spin_unlock_irqrestore(&clkdiv->lock, flags); - return rc; + return __spmi_pmic_clkdiv_set_enable_state(clkdiv, enable, div_factor); } static int clk_spmi_pmic_div_enable(struct clk_hw *hw) { struct clkdiv *clkdiv = to_clkdiv(hw); unsigned long flags; - int rc; + int ret; spin_lock_irqsave(&clkdiv->lock, flags); - rc = spmi_pmic_clkdiv_set_enable_state(clkdiv, true); + ret = spmi_pmic_clkdiv_set_enable_state(clkdiv, true); spin_unlock_irqrestore(&clkdiv->lock, flags); - return rc; + return ret; } static void clk_spmi_pmic_div_disable(struct clk_hw *hw) @@ -163,35 +121,59 @@ static void clk_spmi_pmic_div_disable(struct clk_hw *hw) } static long clk_spmi_pmic_div_round_rate(struct clk_hw *hw, unsigned long rate, - unsigned long *parent_rate) + unsigned long *parent_rate) { - unsigned long new_rate; unsigned int div, div_factor; div = DIV_ROUND_UP(*parent_rate, rate); div_factor = div_to_div_factor(div); - new_rate = *parent_rate / div_factor_to_div(div_factor); + div = div_factor_to_div(div_factor); - return new_rate; + return *parent_rate / div; } -static unsigned long clk_spmi_pmic_div_recalc_rate(struct clk_hw *hw, - unsigned long parent_rate) +static unsigned long +clk_spmi_pmic_div_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) { struct clkdiv *clkdiv = to_clkdiv(hw); - unsigned long rate; + unsigned int div_factor; - rate = parent_rate / div_factor_to_div(clkdiv->div_factor); + regmap_read(clkdiv->regmap, clkdiv->base + REG_DIV_CTL1, &div_factor); + div_factor &= DIV_CTL1_DIV_FACTOR_MASK; - return rate; + return parent_rate / div_factor_to_div(div_factor); } static int clk_spmi_pmic_div_set_rate(struct clk_hw *hw, unsigned long rate, - unsigned long parent_rate) + unsigned long parent_rate) { struct clkdiv *clkdiv = to_clkdiv(hw); + unsigned int div_factor = div_to_div_factor(parent_rate / rate); + unsigned long flags; + bool enabled; + int ret; + + spin_lock_irqsave(&clkdiv->lock, flags); + enabled = is_spmi_pmic_clkdiv_enabled(clkdiv); + if (enabled) { + ret = spmi_pmic_clkdiv_set_enable_state(clkdiv, false); + if (ret) + goto unlock; + } + + ret = regmap_update_bits(clkdiv->regmap, clkdiv->base + REG_DIV_CTL1, + DIV_CTL1_DIV_FACTOR_MASK, div_factor); + if (ret) + goto unlock; + + if (enabled) + ret = __spmi_pmic_clkdiv_set_enable_state(clkdiv, true, + div_factor); - return spmi_pmic_clkdiv_config_freq_div(clkdiv, parent_rate / rate); +unlock: + spin_unlock_irqrestore(&clkdiv->lock, flags); + + return ret; } static const struct clk_ops clk_spmi_pmic_div_ops = { @@ -203,30 +185,25 @@ static const struct clk_ops clk_spmi_pmic_div_ops = { }; struct spmi_pmic_div_clk_cc { - struct clk_hw **div_clks; int nclks; + struct clkdiv clks[]; }; -#define SPMI_PMIC_CLKDIV_MIN_INDEX 1 - -static struct clk_hw *spmi_pmic_div_clk_hw_get(struct of_phandle_args *clkspec, - void *data) +static struct clk_hw * +spmi_pmic_div_clk_hw_get(struct of_phandle_args *clkspec, void *data) { - struct spmi_pmic_div_clk_cc *clk_cc = data; - unsigned int idx = (clkspec->args[0] - SPMI_PMIC_CLKDIV_MIN_INDEX); + struct spmi_pmic_div_clk_cc *cc = data; + int idx = clkspec->args[0] - 1; /* Start at 1 instead of 0 */ - if (idx < 0 || idx >= clk_cc->nclks) { - pr_err("%s: index value %u is invalid; allowed range: [%d, %d]\n", - __func__, clkspec->args[0], SPMI_PMIC_CLKDIV_MIN_INDEX, - clk_cc->nclks); + if (idx < 0 || idx >= cc->nclks) { + pr_err("%s: index value %u is invalid; allowed range [1, %d]\n", + __func__, clkspec->args[0], cc->nclks); return ERR_PTR(-EINVAL); } - return clk_cc->div_clks[idx]; + return &cc->clks[idx].hw; } -#define SPMI_PMIC_DIV_CLK_SIZE 0x100 - static const struct of_device_id spmi_pmic_clkdiv_match_table[] = { { .compatible = "qcom,spmi-clkdiv", @@ -242,20 +219,21 @@ MODULE_DEVICE_TABLE(of, spmi_pmic_clkdiv_match_table); static int spmi_pmic_clkdiv_probe(struct platform_device *pdev) { - struct spmi_pmic_div_clk_cc *clk_cc; + struct spmi_pmic_div_clk_cc *cc; struct clk_init_data init = {}; struct clkdiv *clkdiv; struct clk *cxo; struct regmap *regmap; struct device *dev = &pdev->dev; + struct device_node *of_node = dev->of_node; const char *parent_name; - int nclks, i, rc, cxo_hz; + int nclks, i, ret, cxo_hz; u32 start; - rc = of_property_read_u32(dev->of_node, "reg", &start); - if (rc < 0) { + ret = of_property_read_u32(of_node, "reg", &start); + if (ret < 0) { dev_err(dev, "reg property reading failed\n"); - return rc; + return ret; } regmap = dev_get_regmap(dev->parent, NULL); @@ -264,62 +242,51 @@ static int spmi_pmic_clkdiv_probe(struct platform_device *pdev) return -EINVAL; } - nclks = (uintptr_t)of_match_node(spmi_pmic_clkdiv_match_table, - dev->of_node)->data; - - clkdiv = devm_kcalloc(dev, nclks, sizeof(*clkdiv), GFP_KERNEL); - if (!clkdiv) - return -ENOMEM; - - clk_cc = devm_kzalloc(&pdev->dev, sizeof(*clk_cc), GFP_KERNEL); - if (!clk_cc) - return -ENOMEM; + nclks = (uintptr_t)of_device_get_match_data(dev); + if (!nclks) + return -EINVAL; - clk_cc->div_clks = devm_kcalloc(&pdev->dev, nclks, - sizeof(*clk_cc->div_clks), GFP_KERNEL); - if (!clk_cc->div_clks) + cc = devm_kzalloc(dev, sizeof(*cc) + sizeof(*cc->clks) * nclks, + GFP_KERNEL); + if (!cc) return -ENOMEM; + cc->nclks = nclks; cxo = clk_get(dev, "xo"); if (IS_ERR(cxo)) { - rc = PTR_ERR(cxo); - if (rc != -EPROBE_DEFER) - dev_err(dev, "failed to get xo clock"); - return rc; + ret = PTR_ERR(cxo); + if (ret != -EPROBE_DEFER) + dev_err(dev, "failed to get xo clock\n"); + return ret; } cxo_hz = clk_get_rate(cxo); clk_put(cxo); - parent_name = of_clk_get_parent_name(dev->of_node, 0); + parent_name = of_clk_get_parent_name(of_node, 0); if (!parent_name) { dev_err(dev, "missing parent clock\n"); return -ENODEV; } init.parent_names = &parent_name; - init.num_parents = parent_name ? 1 : 0; + init.num_parents = 1; init.ops = &clk_spmi_pmic_div_ops; - init.flags = 0; - for (i = 0; i < nclks; i++) { + for (i = 0, clkdiv = cc->clks; i < nclks; i++) { spin_lock_init(&clkdiv[i].lock); - clkdiv[i].base = start + i * SPMI_PMIC_DIV_CLK_SIZE; + clkdiv[i].base = start + i * 0x100; clkdiv[i].regmap = regmap; clkdiv[i].cxo_period_ns = NSEC_PER_SEC / cxo_hz; init.name = kasprintf(GFP_KERNEL, "div_clk%d", i + 1); clkdiv[i].hw.init = &init; - rc = devm_clk_hw_register(dev, &clkdiv[i].hw); - kfree(init.name); /* clock framework made a copy of the name */ - if (rc) - return rc; - clk_cc->div_clks[i] = &clkdiv[i].hw; + ret = devm_clk_hw_register(dev, &clkdiv[i].hw); + kfree(init.name); /* clk framework made a copy */ + if (ret) + return ret; } - clk_cc->nclks = nclks; - rc = of_clk_add_hw_provider(pdev->dev.of_node, spmi_pmic_div_clk_hw_get, - clk_cc); - return rc; + return of_clk_add_hw_provider(of_node, spmi_pmic_div_clk_hw_get, cc); } static int spmi_pmic_clkdiv_remove(struct platform_device *pdev)