* [PATCH V6] clk: qcom: Add spmi_pmic clock divider support
@ 2017-11-17 9:48 Tirupathi Reddy
2017-11-17 20:22 ` Rob Herring
2017-11-17 23:56 ` Stephen Boyd
0 siblings, 2 replies; 5+ messages in thread
From: Tirupathi Reddy @ 2017-11-17 9:48 UTC (permalink / raw)
To: sboyd-sgV2jX0FEOL9JmXXK+q4OQ
Cc: mturquette-rdvid1DuHRBWk0Htik3J/w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, andy.gross-QSEj5FYQhm4dnm+yROfE0A,
david.brown-QSEj5FYQhm4dnm+yROfE0A,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
linux-soc-u79uwXL29TY76Z2rM5mHXA, Tirupathi Reddy
Clkdiv module provides a clock output on the PMIC with CXO as
the source. This clock can be routed through PMIC GPIOs. Add
a device driver to configure this clkdiv module.
Signed-off-by: Tirupathi Reddy <tirupath-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Signed-off-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
.../bindings/clock/clk-spmi-pmic-div.txt | 59 ++++
drivers/clk/qcom/Kconfig | 9 +
drivers/clk/qcom/Makefile | 1 +
drivers/clk/qcom/clk-spmi-pmic-div.c | 308 +++++++++++++++++++++
4 files changed, 377 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/clk-spmi-pmic-div.txt
create mode 100644 drivers/clk/qcom/clk-spmi-pmic-div.c
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..2cf2aba
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/clk-spmi-pmic-div.txt
@@ -0,0 +1,59 @@
+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: <string>
+ Definition: must be "qcom,spmi-clkdiv".
+
+- reg
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: base address of CLKDIV peripherals.
+
+- qcom,num-clkdivs
+ Usage: required
+ Value type: <u32>
+ Definition: number of CLKDIV peripherals.
+
+- clocks:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: reference to the xo clock.
+
+- clock-names:
+ Usage: required
+ Value type: <stringlist>
+ Definition: must be "xo".
+
+- clock-cells:
+ Usage: required
+ Value type: <u32>
+ Definition: shall contain 1.
+
+=======
+Example
+=======
+
+pm8998_clk_divs: clock-controller@5b00 {
+ compatible = "qcom,spmi-clkdiv";
+ reg = <0x5b00>;
+ #clock-cells = <1>;
+ qcom,num-clkdivs = <3>;
+ 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..20b5d6f 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 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 of the PMIC. These clocks are typically wired
+ through alternate functions on GPIO pins.
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 26410d3..602af38 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -34,3 +34,4 @@ obj-$(CONFIG_MSM_MMCC_8974) += mmcc-msm8974.o
obj-$(CONFIG_MSM_MMCC_8996) += mmcc-msm8996.o
obj-$(CONFIG_QCOM_CLK_RPM) += clk-rpm.o
obj-$(CONFIG_QCOM_CLK_SMD_RPM) += clk-smd-rpm.o
+obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o
diff --git a/drivers/clk/qcom/clk-spmi-pmic-div.c b/drivers/clk/qcom/clk-spmi-pmic-div.c
new file mode 100644
index 0000000..bf85a16
--- /dev/null
+++ b/drivers/clk/qcom/clk-spmi-pmic-div.c
@@ -0,0 +1,308 @@
+/* Copyright (c) 2017, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/log2.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#define REG_DIV_CTL1 0x43
+#define DIV_CTL1_DIV_FACTOR_MASK GENMASK(2, 0)
+
+#define REG_EN_CTL 0x46
+#define REG_EN_MASK BIT(7)
+
+struct clkdiv {
+ struct regmap *regmap;
+ u16 base;
+ spinlock_t lock;
+
+ struct clk_hw hw;
+ unsigned int cxo_period_ns;
+};
+
+static inline struct clkdiv *to_clkdiv(struct clk_hw *hw)
+{
+ return container_of(hw, struct clkdiv, hw);
+}
+
+static inline unsigned int div_factor_to_div(unsigned int div_factor)
+{
+ if (!div_factor)
+ div_factor = 1;
+
+ return 1 << (div_factor - 1);
+}
+
+static inline unsigned int div_to_div_factor(unsigned int div)
+{
+ 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;
+}
+
+static int
+__spmi_pmic_clkdiv_set_enable_state(struct clkdiv *clkdiv, bool enable,
+ unsigned int div_factor)
+{
+ int ret;
+ unsigned int ns = clkdiv->cxo_period_ns;
+ unsigned int div = div_factor_to_div(div_factor);
+
+ 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)
+ ndelay((2 + 3 * div) * ns);
+ else
+ ndelay(3 * div * ns);
+
+ return 0;
+}
+
+static int spmi_pmic_clkdiv_set_enable_state(struct clkdiv *clkdiv, bool enable)
+{
+ unsigned int div_factor;
+
+ regmap_read(clkdiv->regmap, clkdiv->base + REG_DIV_CTL1, &div_factor);
+ div_factor &= DIV_CTL1_DIV_FACTOR_MASK;
+
+ 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 ret;
+
+ spin_lock_irqsave(&clkdiv->lock, flags);
+ ret = spmi_pmic_clkdiv_set_enable_state(clkdiv, true);
+ spin_unlock_irqrestore(&clkdiv->lock, flags);
+
+ return ret;
+}
+
+static void clk_spmi_pmic_div_disable(struct clk_hw *hw)
+{
+ struct clkdiv *clkdiv = to_clkdiv(hw);
+ unsigned long flags;
+
+ spin_lock_irqsave(&clkdiv->lock, flags);
+ spmi_pmic_clkdiv_set_enable_state(clkdiv, false);
+ spin_unlock_irqrestore(&clkdiv->lock, flags);
+}
+
+static long clk_spmi_pmic_div_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *parent_rate)
+{
+ unsigned int div, div_factor;
+
+ div = DIV_ROUND_UP(*parent_rate, rate);
+ div_factor = div_to_div_factor(div);
+ div = div_factor_to_div(div_factor);
+
+ return *parent_rate / div;
+}
+
+static unsigned long
+clk_spmi_pmic_div_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
+{
+ struct clkdiv *clkdiv = to_clkdiv(hw);
+ unsigned int div_factor;
+
+ regmap_read(clkdiv->regmap, clkdiv->base + REG_DIV_CTL1, &div_factor);
+ div_factor &= DIV_CTL1_DIV_FACTOR_MASK;
+
+ 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)
+{
+ 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);
+
+unlock:
+ spin_unlock_irqrestore(&clkdiv->lock, flags);
+
+ return ret;
+}
+
+static const struct clk_ops clk_spmi_pmic_div_ops = {
+ .enable = clk_spmi_pmic_div_enable,
+ .disable = clk_spmi_pmic_div_disable,
+ .set_rate = clk_spmi_pmic_div_set_rate,
+ .recalc_rate = clk_spmi_pmic_div_recalc_rate,
+ .round_rate = clk_spmi_pmic_div_round_rate,
+};
+
+struct spmi_pmic_div_clk_cc {
+ int nclks;
+ struct clkdiv clks[];
+};
+
+static struct clk_hw *
+spmi_pmic_div_clk_hw_get(struct of_phandle_args *clkspec, void *data)
+{
+ struct spmi_pmic_div_clk_cc *cc = data;
+ int idx = clkspec->args[0] - 1; /* Start at 1 instead of 0 */
+
+ 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 &cc->clks[idx].hw;
+}
+
+static int spmi_pmic_clkdiv_probe(struct platform_device *pdev)
+{
+ 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, ret, cxo_hz;
+ u32 start;
+
+ ret = of_property_read_u32(of_node, "reg", &start);
+ if (ret < 0) {
+ dev_err(dev, "reg property reading failed\n");
+ return ret;
+ }
+
+ regmap = dev_get_regmap(dev->parent, NULL);
+ if (!regmap) {
+ dev_err(dev, "Couldn't get parent's regmap\n");
+ return -EINVAL;
+ }
+
+ ret = of_property_read_u32(of_node, "qcom,num-clkdivs", &nclks);
+ if (ret < 0) {
+ dev_err(dev, "qcom,num-clkdivs property reading failed, ret=%d\n",
+ ret);
+ return ret;
+ }
+
+ if (!nclks)
+ return -EINVAL;
+
+ 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)) {
+ 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(of_node, 0);
+ if (!parent_name) {
+ dev_err(dev, "missing parent clock\n");
+ return -ENODEV;
+ }
+
+ init.parent_names = &parent_name;
+ init.num_parents = 1;
+ init.ops = &clk_spmi_pmic_div_ops;
+
+ for (i = 0, clkdiv = cc->clks; i < nclks; i++) {
+ spin_lock_init(&clkdiv[i].lock);
+ 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;
+
+ ret = devm_clk_hw_register(dev, &clkdiv[i].hw);
+ kfree(init.name); /* clk framework made a copy */
+ if (ret)
+ return ret;
+ }
+
+ 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)
+{
+ of_clk_del_provider(pdev->dev.of_node);
+
+ return 0;
+}
+
+static const struct of_device_id spmi_pmic_clkdiv_match_table[] = {
+ { .compatible = "qcom,spmi-clkdiv" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, spmi_pmic_clkdiv_match_table);
+
+static struct platform_driver spmi_pmic_clkdiv_driver = {
+ .driver = {
+ .name = "qcom,spmi-pmic-clkdiv",
+ .of_match_table = spmi_pmic_clkdiv_match_table,
+ },
+ .probe = spmi_pmic_clkdiv_probe,
+ .remove = spmi_pmic_clkdiv_remove,
+};
+module_platform_driver(spmi_pmic_clkdiv_driver);
+
+MODULE_DESCRIPTION("QCOM SPMI PMIC clkdiv driver");
+MODULE_LICENSE("GPL v2");
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH V6] clk: qcom: Add spmi_pmic clock divider support
2017-11-17 9:48 [PATCH V6] clk: qcom: Add spmi_pmic clock divider support Tirupathi Reddy
@ 2017-11-17 20:22 ` Rob Herring
2017-11-21 9:12 ` Tirupathi Reddy T
2017-11-17 23:56 ` Stephen Boyd
1 sibling, 1 reply; 5+ messages in thread
From: Rob Herring @ 2017-11-17 20:22 UTC (permalink / raw)
To: Tirupathi Reddy
Cc: sboyd, mturquette, mark.rutland, andy.gross, david.brown,
linux-clk, devicetree, linux-kernel, linux-arm-msm, linux-soc
On Fri, Nov 17, 2017 at 03:18:47PM +0530, Tirupathi Reddy wrote:
> Clkdiv module provides a clock output on the PMIC with CXO as
> the source. This clock can be routed through PMIC GPIOs. Add
> a device driver to configure this clkdiv module.
>
> Signed-off-by: Tirupathi Reddy <tirupath@codeaurora.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Normally your S-o-B would be last.
> ---
> .../bindings/clock/clk-spmi-pmic-div.txt | 59 ++++
Please split bindings to a separate patch.
Otherwise,
Acked-by: Rob Herring <robh@kernel.org>
> drivers/clk/qcom/Kconfig | 9 +
> drivers/clk/qcom/Makefile | 1 +
> drivers/clk/qcom/clk-spmi-pmic-div.c | 308 +++++++++++++++++++++
> 4 files changed, 377 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/clk-spmi-pmic-div.txt
> create mode 100644 drivers/clk/qcom/clk-spmi-pmic-div.c
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V6] clk: qcom: Add spmi_pmic clock divider support
2017-11-17 9:48 [PATCH V6] clk: qcom: Add spmi_pmic clock divider support Tirupathi Reddy
2017-11-17 20:22 ` Rob Herring
@ 2017-11-17 23:56 ` Stephen Boyd
2017-11-21 9:12 ` Tirupathi Reddy T
1 sibling, 1 reply; 5+ messages in thread
From: Stephen Boyd @ 2017-11-17 23:56 UTC (permalink / raw)
To: Tirupathi Reddy
Cc: mturquette, robh+dt, mark.rutland, andy.gross, david.brown,
linux-clk, devicetree, linux-kernel, linux-arm-msm, linux-soc
On 11/17, 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..2cf2aba
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/clk-spmi-pmic-div.txt
Please move this to qcom,spmi-clkdiv.txt like other qcom bindings.
> @@ -0,0 +1,59 @@
> +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: <string>
> + Definition: must be "qcom,spmi-clkdiv".
> +
> +- reg
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: base address of CLKDIV peripherals.
> +
> +- qcom,num-clkdivs
> + Usage: required
> + Value type: <u32>
> + Definition: number of CLKDIV peripherals.
Would it work if we read the registers and looked for the clkdiv
subtype ID? If we read something that doesn't match, break and
stop adding clks? Or does reading the next "peripheral" cause
some sort of crash and burn scenario where the device see an
access control violation? Would be interesting to do it that way
and avoid needing a new property in DT.
> +
> +
> + 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 = 1;
> + init.ops = &clk_spmi_pmic_div_ops;
> +
> + for (i = 0, clkdiv = cc->clks; i < nclks; i++) {
> + spin_lock_init(&clkdiv[i].lock);
> + 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);
Hmm. Maybe we should just have this on the stack. 20 characters
should be plenty?
> + clkdiv[i].hw.init = &init;
> +
> + ret = devm_clk_hw_register(dev, &clkdiv[i].hw);
> + kfree(init.name); /* clk framework made a copy */
> + if (ret)
> + return ret;
> + }
> +
> + 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)
> +{
> + of_clk_del_provider(pdev->dev.of_node);
> +
> + return 0;
This can use devm now.
> +}
> +
> +static const struct of_device_id spmi_pmic_clkdiv_match_table[] = {
> + { .compatible = "qcom,spmi-clkdiv" },
> + { /* sentinel */ }
> +};
Nice!
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V6] clk: qcom: Add spmi_pmic clock divider support
2017-11-17 23:56 ` Stephen Boyd
@ 2017-11-21 9:12 ` Tirupathi Reddy T
0 siblings, 0 replies; 5+ messages in thread
From: Tirupathi Reddy T @ 2017-11-21 9:12 UTC (permalink / raw)
To: Stephen Boyd
Cc: mturquette, robh+dt, mark.rutland, andy.gross, david.brown,
linux-clk, devicetree, linux-kernel, linux-arm-msm, linux-soc
On 11/18/2017 5:26 AM, Stephen Boyd wrote:
> On 11/17, 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..2cf2aba
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/clk-spmi-pmic-div.txt
> Please move this to qcom,spmi-clkdiv.txt like other qcom bindings.
Addressed in next patch version[7]
>
>> @@ -0,0 +1,59 @@
>> +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: <string>
>> + Definition: must be "qcom,spmi-clkdiv".
>> +
>> +- reg
>> + Usage: required
>> + Value type: <prop-encoded-array>
>> + Definition: base address of CLKDIV peripherals.
>> +
>> +- qcom,num-clkdivs
>> + Usage: required
>> + Value type: <u32>
>> + Definition: number of CLKDIV peripherals.
> Would it work if we read the registers and looked for the clkdiv
> subtype ID? If we read something that doesn't match, break and
> stop adding clks? Or does reading the next "peripheral" cause
> some sort of crash and burn scenario where the device see an
> access control violation? Would be interesting to do it that way
> and avoid needing a new property in DT.
I feel it is better to go with the current design as we beforehand know the
#div-clk peripherals in a PMIC. Reading the next peripheral might result in
an unknown failure if there is no real peripheral there (PMIC peripherals
are not in contiguous address space).
>> +
>> +
>> + 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 = 1;
>> + init.ops = &clk_spmi_pmic_div_ops;
>> +
>> + for (i = 0, clkdiv = cc->clks; i < nclks; i++) {
>> + spin_lock_init(&clkdiv[i].lock);
>> + 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);
> Hmm. Maybe we should just have this on the stack. 20 characters
> should be plenty?
Addressed in next patch version[7]
>> + clkdiv[i].hw.init = &init;
>> +
>> + ret = devm_clk_hw_register(dev, &clkdiv[i].hw);
>> + kfree(init.name); /* clk framework made a copy */
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + 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)
>> +{
>> + of_clk_del_provider(pdev->dev.of_node);
>> +
>> + return 0;
> This can use devm now.
Addressed in next patch version.
>> +}
>> +
>> +static const struct of_device_id spmi_pmic_clkdiv_match_table[] = {
>> + { .compatible = "qcom,spmi-clkdiv" },
>> + { /* sentinel */ }
>> +};
> Nice!
Thanks
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V6] clk: qcom: Add spmi_pmic clock divider support
2017-11-17 20:22 ` Rob Herring
@ 2017-11-21 9:12 ` Tirupathi Reddy T
0 siblings, 0 replies; 5+ messages in thread
From: Tirupathi Reddy T @ 2017-11-21 9:12 UTC (permalink / raw)
To: Rob Herring
Cc: sboyd, mturquette, mark.rutland, andy.gross, david.brown,
linux-clk, devicetree, linux-kernel, linux-arm-msm, linux-soc
On 11/18/2017 1:52 AM, Rob Herring wrote:
> On Fri, Nov 17, 2017 at 03:18:47PM +0530, Tirupathi Reddy wrote:
>> Clkdiv module provides a clock output on the PMIC with CXO as
>> the source. This clock can be routed through PMIC GPIOs. Add
>> a device driver to configure this clkdiv module.
>>
>> Signed-off-by: Tirupathi Reddy <tirupath@codeaurora.org>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> Normally your S-o-B would be last.
Addressed in next patch version [7]
>
>> ---
>> .../bindings/clock/clk-spmi-pmic-div.txt | 59 ++++
> Please split bindings to a separate patch.
Addressed in next patch version [7]
>
> Otherwise,
>
> Acked-by: Rob Herring <robh@kernel.org>
>
>> drivers/clk/qcom/Kconfig | 9 +
>> drivers/clk/qcom/Makefile | 1 +
>> drivers/clk/qcom/clk-spmi-pmic-div.c | 308 +++++++++++++++++++++
>> 4 files changed, 377 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/clock/clk-spmi-pmic-div.txt
>> create mode 100644 drivers/clk/qcom/clk-spmi-pmic-div.c
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-11-21 9:12 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-17 9:48 [PATCH V6] clk: qcom: Add spmi_pmic clock divider support Tirupathi Reddy
2017-11-17 20:22 ` Rob Herring
2017-11-21 9:12 ` Tirupathi Reddy T
2017-11-17 23:56 ` Stephen Boyd
2017-11-21 9:12 ` Tirupathi Reddy T
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).