* [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock
@ 2016-08-23 6:25 ` Rafał Miłecki
0 siblings, 0 replies; 50+ messages in thread
From: Rafał Miłecki @ 2016-08-23 6:25 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd
Cc: linux-clk, bcm-kernel-feedback-list, Rafał Miłecki,
Rob Herring, Mark Rutland, Eric Anholt, Jon Mason,
Florian Fainelli, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
From: Rafał Miłecki <rafal@milecki.pl>
This clock is present on BCM53573 devices (including BCM47189) that use
Cortex-A7. ILP is a part of PMU (Power Management Unit) and so it should
be defined as one of its subnodes (subdevices). For more details see
Documentation entry.
Unfortunately there isn't a set of registers related to ILP clock only.
We use registers 0x66c, 0x674 and 0x6dc and between them there are e.g.
"retention*" and "control_ext" regs. This is why this driver maps all
0x1000 B of space.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Rebase on top of clk-next
Use ALP as parent clock
Improve comments
Switch from ioremap_nocache to ioremap
Check of_clk_add_provide result for error
V3: Drop #include <linux/moduleh>
Make ILP DT entry part of PMU
Describe ILP as subdevice of PMU in Documentation
V4: Use BCM53573 name as suggested by Jon and Ray. It seems "Northstar"
(even if used in some resources) should be used in relation to
Cortex-A9 devices only.
V5: Rename remaining "ns" references to "bcm53573", sorry, I sent V4 too
early.
---
.../bindings/clock/brcm,bcm53573-ilp.txt | 40 ++++++
drivers/clk/bcm/Makefile | 1 +
drivers/clk/bcm/clk-bcm53573-ilp.c | 146 +++++++++++++++++++++
3 files changed, 187 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
create mode 100644 drivers/clk/bcm/clk-bcm53573-ilp.c
diff --git a/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
new file mode 100644
index 0000000..5ab3107
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
@@ -0,0 +1,40 @@
+Broadcom BCM53573 ILP clock
+===========================
+
+This binding uses the common clock binding:
+ Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+This binding is used for ILP clock (sometimes referred as "slow clock")
+on Broadcom BCM53573 devices using Cortex-A7 CPU.
+
+This clock is part of PMU (Power Management Unit), a Broadcom's device
+handing power-related aspects. Please note PMU contains more subdevices,
+ILP is only one of them.
+
+ILP's rate has to be calculated on runtime and it depends on ALP clock
+which has to be referenced.
+
+Required properties:
+- compatible: "brcm,bcm53573-ilp"
+- reg: iomem address range of PMU (Power Management Unit)
+- reg-names: "pmu", the only needed & supported reg right now
+- clocks: has to reference an ALP clock
+- #clock-cells: should be <0>
+
+Example:
+
+pmu@18012000 {
+ compatible = "simple-bus";
+ ranges = <0x00000000 0x18012000 0x00001000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ ilp: ilp@0 {
+ compatible = "brcm,bcm53573-ilp";
+ reg = <0 0x1000>;
+ reg-names = "pmu";
+ clocks = <&alp>;
+ #clock-cells = <0>;
+ clock-output-names = "ilp";
+ };
+};
diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile
index 1d79bd2..4b8c56d 100644
--- a/drivers/clk/bcm/Makefile
+++ b/drivers/clk/bcm/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_COMMON_CLK_IPROC) += clk-ns2.o
obj-$(CONFIG_ARCH_BCM_CYGNUS) += clk-cygnus.o
obj-$(CONFIG_ARCH_BCM_NSP) += clk-nsp.o
obj-$(CONFIG_ARCH_BCM_5301X) += clk-nsp.o
+obj-$(CONFIG_ARCH_BCM_53573) += clk-bcm53573-ilp.o
diff --git a/drivers/clk/bcm/clk-bcm53573-ilp.c b/drivers/clk/bcm/clk-bcm53573-ilp.c
new file mode 100644
index 0000000..b7ac0eb
--- /dev/null
+++ b/drivers/clk/bcm/clk-bcm53573-ilp.c
@@ -0,0 +1,146 @@
+/*
+ * Copyright (C) 2016 Rafał Miłecki <rafal@milecki.pl>
+ *
+ * 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.
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+
+#define PMU_XTAL_FREQ_RATIO 0x66c
+#define XTAL_ALP_PER_4ILP 0x00001fff
+#define XTAL_CTL_EN 0x80000000
+#define PMU_SLOW_CLK_PERIOD 0x6dc
+
+struct bcm53573_ilp {
+ struct clk *clk;
+ struct clk_hw hw;
+ void __iomem *pmu;
+};
+
+static int bcm53573_ilp_enable(struct clk_hw *hw)
+{
+ struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
+
+ writel(0x10199, ilp->pmu + PMU_SLOW_CLK_PERIOD);
+ writel(0x10000, ilp->pmu + 0x674);
+
+ return 0;
+}
+
+static unsigned long bcm53573_ilp_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
+ void __iomem *pmu = ilp->pmu;
+ u32 last_val, cur_val;
+ u32 sum = 0, num = 0, loop_num = 0;
+ u32 avg;
+
+ /* Enable measurement */
+ writel(XTAL_CTL_EN, pmu + PMU_XTAL_FREQ_RATIO);
+
+ /* Read initial value */
+ last_val = readl(pmu + PMU_XTAL_FREQ_RATIO) & XTAL_ALP_PER_4ILP;
+
+ /*
+ * At minimum we should loop for a bit to let hardware do the
+ * measurement. This isn't very accurate however, so for a better
+ * precision lets try getting 20 different values for and use average.
+ */
+ while (num < 20) {
+ cur_val = readl(pmu + PMU_XTAL_FREQ_RATIO) & XTAL_ALP_PER_4ILP;
+
+ if (cur_val != last_val) {
+ /* Got different value, use it */
+ sum += cur_val;
+ num++;
+ loop_num = 0;
+ last_val = cur_val;
+ } else if (++loop_num > 5000) {
+ /* Same value over and over, give up */
+ sum += cur_val;
+ num++;
+ break;
+ }
+ }
+
+ /* Disable measurement to save power */
+ writel(0x0, pmu + PMU_XTAL_FREQ_RATIO);
+
+ avg = sum / num;
+
+ return parent_rate * 4 / avg;
+}
+
+static const struct clk_ops bcm53573_ilp_clk_ops = {
+ .enable = bcm53573_ilp_enable,
+ .recalc_rate = bcm53573_ilp_recalc_rate,
+};
+
+static void bcm53573_ilp_init(struct device_node *np)
+{
+ struct bcm53573_ilp *ilp;
+ struct resource res;
+ struct clk_init_data init = { 0 };
+ const char *parent_name;
+ int index;
+ int err;
+
+ ilp = kzalloc(sizeof(*ilp), GFP_KERNEL);
+ if (!ilp)
+ return;
+
+ parent_name = of_clk_get_parent_name(np, 0);
+ if (!parent_name) {
+ err = -ENOENT;
+ goto err_free_ilp;
+ }
+
+ /* TODO: This looks generic, try making it OF helper. */
+ index = of_property_match_string(np, "reg-names", "pmu");
+ if (index < 0) {
+ err = index;
+ goto err_free_ilp;
+ }
+ err = of_address_to_resource(np, index, &res);
+ if (err)
+ goto err_free_ilp;
+ ilp->pmu = ioremap(res.start, resource_size(&res));
+ if (IS_ERR(ilp->pmu)) {
+ err = PTR_ERR(ilp->pmu);
+ goto err_free_ilp;
+ }
+
+ init.name = np->name;
+ init.ops = &bcm53573_ilp_clk_ops;
+ init.parent_names = &parent_name;
+ init.num_parents = 1;
+
+ ilp->hw.init = &init;
+ ilp->clk = clk_register(NULL, &ilp->hw);
+ if (WARN_ON(IS_ERR(ilp->clk)))
+ goto err_unmap_pmu;
+
+ err = of_clk_add_provider(np, of_clk_src_simple_get, ilp->clk);
+ if (err)
+ goto err_clk_unregister;
+
+ return;
+
+err_clk_unregister:
+ clk_unregister(ilp->clk);
+err_unmap_pmu:
+ iounmap(ilp->pmu);
+err_free_ilp:
+ kfree(ilp);
+ pr_err("Failed to init ILP clock: %d\n", err);
+}
+CLK_OF_DECLARE(bcm53573_ilp_clk, "brcm,bcm53573-ilp", bcm53573_ilp_init);
--
2.9.3
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock
2016-08-23 6:25 ` Rafał Miłecki
(?)
@ 2016-08-23 19:55 ` Rob Herring
2016-08-25 12:42 ` Rafał Miłecki
-1 siblings, 1 reply; 50+ messages in thread
From: Rob Herring @ 2016-08-23 19:55 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Michael Turquette, Stephen Boyd, linux-clk,
bcm-kernel-feedback-list, Rafał Miłecki, Mark Rutland,
Eric Anholt, Jon Mason, Florian Fainelli, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On Tue, Aug 23, 2016 at 08:25:59AM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> This clock is present on BCM53573 devices (including BCM47189) that use
> Cortex-A7. ILP is a part of PMU (Power Management Unit) and so it should
> be defined as one of its subnodes (subdevices). For more details see
> Documentation entry.
>
> Unfortunately there isn't a set of registers related to ILP clock only.
> We use registers 0x66c, 0x674 and 0x6dc and between them there are e.g.
> "retention*" and "control_ext" regs. This is why this driver maps all
> 0x1000 B of space.
Then describe the block as a syscon which has several functions of
which clocks are one.
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V2: Rebase on top of clk-next
> Use ALP as parent clock
> Improve comments
> Switch from ioremap_nocache to ioremap
> Check of_clk_add_provide result for error
> V3: Drop #include <linux/moduleh>
> Make ILP DT entry part of PMU
> Describe ILP as subdevice of PMU in Documentation
> V4: Use BCM53573 name as suggested by Jon and Ray. It seems "Northstar"
> (even if used in some resources) should be used in relation to
> Cortex-A9 devices only.
> V5: Rename remaining "ns" references to "bcm53573", sorry, I sent V4 too
> early.
> ---
> .../bindings/clock/brcm,bcm53573-ilp.txt | 40 ++++++
Acked-by: Rob Herring <robh@kernel.org>
> drivers/clk/bcm/Makefile | 1 +
> drivers/clk/bcm/clk-bcm53573-ilp.c | 146 +++++++++++++++++++++
> 3 files changed, 187 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
> create mode 100644 drivers/clk/bcm/clk-bcm53573-ilp.c
>
> diff --git a/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
> new file mode 100644
> index 0000000..5ab3107
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
> @@ -0,0 +1,40 @@
> +Broadcom BCM53573 ILP clock
> +===========================
> +
> +This binding uses the common clock binding:
> + Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +This binding is used for ILP clock (sometimes referred as "slow clock")
> +on Broadcom BCM53573 devices using Cortex-A7 CPU.
> +
> +This clock is part of PMU (Power Management Unit), a Broadcom's device
> +handing power-related aspects. Please note PMU contains more subdevices,
> +ILP is only one of them.
> +
> +ILP's rate has to be calculated on runtime and it depends on ALP clock
> +which has to be referenced.
> +
> +Required properties:
> +- compatible: "brcm,bcm53573-ilp"
> +- reg: iomem address range of PMU (Power Management Unit)
> +- reg-names: "pmu", the only needed & supported reg right now
right now? Please describe all of the h/w. You don't get to change it
later.
Rob
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock
2016-08-23 19:55 ` Rob Herring
2016-08-25 12:42 ` Rafał Miłecki
@ 2016-08-25 12:42 ` Rafał Miłecki
0 siblings, 0 replies; 50+ messages in thread
From: Rafał Miłecki @ 2016-08-25 12:42 UTC (permalink / raw)
To: Rob Herring
Cc: Michael Turquette, Stephen Boyd, linux-clk,
bcm-kernel-feedback-list, Rafał Miłecki, Mark Rutland,
Eric Anholt, Jon Mason, Florian Fainelli, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On 23 August 2016 at 21:55, Rob Herring <robh@kernel.org> wrote:
> On Tue, Aug 23, 2016 at 08:25:59AM +0200, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> This clock is present on BCM53573 devices (including BCM47189) that use
>> Cortex-A7. ILP is a part of PMU (Power Management Unit) and so it should
>> be defined as one of its subnodes (subdevices). For more details see
>> Documentation entry.
>>
>> Unfortunately there isn't a set of registers related to ILP clock only.
>> We use registers 0x66c, 0x674 and 0x6dc and between them there are e.g.
>> "retention*" and "control_ext" regs. This is why this driver maps all
>> 0x1000 B of space.
>
> Then describe the block as a syscon which has several functions of
> which clocks are one.
This isn't clear to me, sorry, could you describe it? Would you like
me to update commit message or documentation? Is code fine as is?
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock
@ 2016-08-25 12:42 ` Rafał Miłecki
0 siblings, 0 replies; 50+ messages in thread
From: Rafał Miłecki @ 2016-08-25 12:42 UTC (permalink / raw)
To: Rob Herring
Cc: Michael Turquette, Stephen Boyd, linux-clk,
bcm-kernel-feedback-list, Rafał Miłecki, Mark Rutland,
Eric Anholt, Jon Mason, Florian Fainelli, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On 23 August 2016 at 21:55, Rob Herring <robh@kernel.org> wrote:
> On Tue, Aug 23, 2016 at 08:25:59AM +0200, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>
>> This clock is present on BCM53573 devices (including BCM47189) that use
>> Cortex-A7. ILP is a part of PMU (Power Management Unit) and so it should
>> be defined as one of its subnodes (subdevices). For more details see
>> Documentation entry.
>>
>> Unfortunately there isn't a set of registers related to ILP clock only.
>> We use registers 0x66c, 0x674 and 0x6dc and between them there are e.g.
>> "retention*" and "control_ext" regs. This is why this driver maps all
>> 0x1000 B of space.
>
> Then describe the block as a syscon which has several functions of
> which clocks are one.
This isn't clear to me, sorry, could you describe it? Would you like
me to update commit message or documentation? Is code fine as is?
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock
@ 2016-08-25 12:42 ` Rafał Miłecki
0 siblings, 0 replies; 50+ messages in thread
From: Rafał Miłecki @ 2016-08-25 12:42 UTC (permalink / raw)
To: Rob Herring
Cc: Michael Turquette, Stephen Boyd,
linux-clk-u79uwXL29TY76Z2rM5mHXA, bcm-kernel-feedback-list,
Rafał Miłecki, Mark Rutland, Eric Anholt, Jon Mason,
Florian Fainelli, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On 23 August 2016 at 21:55, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Aug 23, 2016 at 08:25:59AM +0200, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>
>> This clock is present on BCM53573 devices (including BCM47189) that use
>> Cortex-A7. ILP is a part of PMU (Power Management Unit) and so it should
>> be defined as one of its subnodes (subdevices). For more details see
>> Documentation entry.
>>
>> Unfortunately there isn't a set of registers related to ILP clock only.
>> We use registers 0x66c, 0x674 and 0x6dc and between them there are e.g.
>> "retention*" and "control_ext" regs. This is why this driver maps all
>> 0x1000 B of space.
>
> Then describe the block as a syscon which has several functions of
> which clocks are one.
This isn't clear to me, sorry, could you describe it? Would you like
me to update commit message or documentation? Is code fine as is?
--
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 [flat|nested] 50+ messages in thread
* Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock
2016-08-25 12:42 ` Rafał Miłecki
(?)
(?)
@ 2016-08-31 16:16 ` Rob Herring
2016-09-05 12:12 ` Rafał Miłecki
-1 siblings, 1 reply; 50+ messages in thread
From: Rob Herring @ 2016-08-31 16:16 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Michael Turquette, Stephen Boyd, linux-clk,
bcm-kernel-feedback-list, Rafał Miłecki, Mark Rutland,
Eric Anholt, Jon Mason, Florian Fainelli, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On Thu, Aug 25, 2016 at 02:42:52PM +0200, Rafał Miłecki wrote:
> On 23 August 2016 at 21:55, Rob Herring <robh@kernel.org> wrote:
> > On Tue, Aug 23, 2016 at 08:25:59AM +0200, Rafał Miłecki wrote:
> >> From: Rafał Miłecki <rafal@milecki.pl>
> >>
> >> This clock is present on BCM53573 devices (including BCM47189) that use
> >> Cortex-A7. ILP is a part of PMU (Power Management Unit) and so it should
> >> be defined as one of its subnodes (subdevices). For more details see
> >> Documentation entry.
> >>
> >> Unfortunately there isn't a set of registers related to ILP clock only.
> >> We use registers 0x66c, 0x674 and 0x6dc and between them there are e.g.
> >> "retention*" and "control_ext" regs. This is why this driver maps all
> >> 0x1000 B of space.
> >
> > Then describe the block as a syscon which has several functions of
> > which clocks are one.
>
> This isn't clear to me, sorry, could you describe it? Would you like
> me to update commit message or documentation? Is code fine as is?
Let me put it another way, when you need to use the other registers, how
do you plan to describe them in DT? We don't really want a node per
register, nor do I want to get binding docs one by one as you add each
function. Instead describe the block with all the misc functions as a
whole. What would you call that block? Still ILP or something else?
Also, you if you do have multiple drivers all needing to access this
single block, then that is when you need syscon and regmap.
Rob
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock
2016-08-31 16:16 ` Rob Herring
2016-09-05 12:12 ` Rafał Miłecki
@ 2016-09-05 12:12 ` Rafał Miłecki
0 siblings, 0 replies; 50+ messages in thread
From: Rafał Miłecki @ 2016-09-05 12:12 UTC (permalink / raw)
To: Rob Herring
Cc: Michael Turquette, Stephen Boyd, linux-clk,
bcm-kernel-feedback-list, Rafał Miłecki, Mark Rutland,
Eric Anholt, Jon Mason, Florian Fainelli, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On 31 August 2016 at 18:16, Rob Herring <robh@kernel.org> wrote:
> On Thu, Aug 25, 2016 at 02:42:52PM +0200, Rafał Miłecki wrote:
>> On 23 August 2016 at 21:55, Rob Herring <robh@kernel.org> wrote:
>> > On Tue, Aug 23, 2016 at 08:25:59AM +0200, Rafał Miłecki wrote:
>> >> From: Rafał Miłecki <rafal@milecki.pl>
>> >>
>> >> This clock is present on BCM53573 devices (including BCM47189) that use
>> >> Cortex-A7. ILP is a part of PMU (Power Management Unit) and so it should
>> >> be defined as one of its subnodes (subdevices). For more details see
>> >> Documentation entry.
>> >>
>> >> Unfortunately there isn't a set of registers related to ILP clock only.
>> >> We use registers 0x66c, 0x674 and 0x6dc and between them there are e.g.
>> >> "retention*" and "control_ext" regs. This is why this driver maps all
>> >> 0x1000 B of space.
>> >
>> > Then describe the block as a syscon which has several functions of
>> > which clocks are one.
>>
>> This isn't clear to me, sorry, could you describe it? Would you like
>> me to update commit message or documentation? Is code fine as is?
>
> Let me put it another way, when you need to use the other registers, how
> do you plan to describe them in DT? We don't really want a node per
> register, nor do I want to get binding docs one by one as you add each
> function. Instead describe the block with all the misc functions as a
> whole. What would you call that block? Still ILP or something else?
>
> Also, you if you do have multiple drivers all needing to access this
> single block, then that is when you need syscon and regmap.
I got this now, thanks for your patience.
One last question: what is preferred register reference:
1) Parent-child (of_get_parent + syscon_node_to_regmap)
or
2) DT property (syscon_regmap_lookup_by_phandle(dt, "foo"))
--
Rafał
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock
@ 2016-09-05 12:12 ` Rafał Miłecki
0 siblings, 0 replies; 50+ messages in thread
From: Rafał Miłecki @ 2016-09-05 12:12 UTC (permalink / raw)
To: Rob Herring
Cc: Michael Turquette, Stephen Boyd, linux-clk,
bcm-kernel-feedback-list, Rafał Miłecki, Mark Rutland,
Eric Anholt, Jon Mason, Florian Fainelli, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On 31 August 2016 at 18:16, Rob Herring <robh@kernel.org> wrote:
> On Thu, Aug 25, 2016 at 02:42:52PM +0200, Rafa=C5=82 Mi=C5=82ecki wrote:
>> On 23 August 2016 at 21:55, Rob Herring <robh@kernel.org> wrote:
>> > On Tue, Aug 23, 2016 at 08:25:59AM +0200, Rafa=C5=82 Mi=C5=82ecki wrot=
e:
>> >> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>> >>
>> >> This clock is present on BCM53573 devices (including BCM47189) that u=
se
>> >> Cortex-A7. ILP is a part of PMU (Power Management Unit) and so it sho=
uld
>> >> be defined as one of its subnodes (subdevices). For more details see
>> >> Documentation entry.
>> >>
>> >> Unfortunately there isn't a set of registers related to ILP clock onl=
y.
>> >> We use registers 0x66c, 0x674 and 0x6dc and between them there are e.=
g.
>> >> "retention*" and "control_ext" regs. This is why this driver maps all
>> >> 0x1000 B of space.
>> >
>> > Then describe the block as a syscon which has several functions of
>> > which clocks are one.
>>
>> This isn't clear to me, sorry, could you describe it? Would you like
>> me to update commit message or documentation? Is code fine as is?
>
> Let me put it another way, when you need to use the other registers, how
> do you plan to describe them in DT? We don't really want a node per
> register, nor do I want to get binding docs one by one as you add each
> function. Instead describe the block with all the misc functions as a
> whole. What would you call that block? Still ILP or something else?
>
> Also, you if you do have multiple drivers all needing to access this
> single block, then that is when you need syscon and regmap.
I got this now, thanks for your patience.
One last question: what is preferred register reference:
1) Parent-child (of_get_parent + syscon_node_to_regmap)
or
2) DT property (syscon_regmap_lookup_by_phandle(dt, "foo"))
--=20
Rafa=C5=82
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock
@ 2016-09-05 12:12 ` Rafał Miłecki
0 siblings, 0 replies; 50+ messages in thread
From: Rafał Miłecki @ 2016-09-05 12:12 UTC (permalink / raw)
To: Rob Herring
Cc: Michael Turquette, Stephen Boyd,
linux-clk-u79uwXL29TY76Z2rM5mHXA, bcm-kernel-feedback-list,
Rafał Miłecki, Mark Rutland, Eric Anholt, Jon Mason,
Florian Fainelli, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On 31 August 2016 at 18:16, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Thu, Aug 25, 2016 at 02:42:52PM +0200, Rafał Miłecki wrote:
>> On 23 August 2016 at 21:55, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> > On Tue, Aug 23, 2016 at 08:25:59AM +0200, Rafał Miłecki wrote:
>> >> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>> >>
>> >> This clock is present on BCM53573 devices (including BCM47189) that use
>> >> Cortex-A7. ILP is a part of PMU (Power Management Unit) and so it should
>> >> be defined as one of its subnodes (subdevices). For more details see
>> >> Documentation entry.
>> >>
>> >> Unfortunately there isn't a set of registers related to ILP clock only.
>> >> We use registers 0x66c, 0x674 and 0x6dc and between them there are e.g.
>> >> "retention*" and "control_ext" regs. This is why this driver maps all
>> >> 0x1000 B of space.
>> >
>> > Then describe the block as a syscon which has several functions of
>> > which clocks are one.
>>
>> This isn't clear to me, sorry, could you describe it? Would you like
>> me to update commit message or documentation? Is code fine as is?
>
> Let me put it another way, when you need to use the other registers, how
> do you plan to describe them in DT? We don't really want a node per
> register, nor do I want to get binding docs one by one as you add each
> function. Instead describe the block with all the misc functions as a
> whole. What would you call that block? Still ILP or something else?
>
> Also, you if you do have multiple drivers all needing to access this
> single block, then that is when you need syscon and regmap.
I got this now, thanks for your patience.
One last question: what is preferred register reference:
1) Parent-child (of_get_parent + syscon_node_to_regmap)
or
2) DT property (syscon_regmap_lookup_by_phandle(dt, "foo"))
--
Rafał
--
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 [flat|nested] 50+ messages in thread
* Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock
@ 2016-08-24 8:47 ` Stephen Boyd
0 siblings, 0 replies; 50+ messages in thread
From: Stephen Boyd @ 2016-08-24 8:47 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Michael Turquette, linux-clk, bcm-kernel-feedback-list,
Rafał Miłecki, Rob Herring, Mark Rutland, Eric Anholt,
Jon Mason, Florian Fainelli, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On 08/23, Rafał Miłecki wrote:
> diff --git a/drivers/clk/bcm/clk-bcm53573-ilp.c b/drivers/clk/bcm/clk-bcm53573-ilp.c
> new file mode 100644
> index 0000000..b7ac0eb
> --- /dev/null
> +++ b/drivers/clk/bcm/clk-bcm53573-ilp.c
> @@ -0,0 +1,146 @@
> +/*
> + * Copyright (C) 2016 Rafał Miłecki <rafal@milecki.pl>
> + *
> + * 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.
> + */
> +
> +#include <linux/clk.h>
Is this include used?
> +#include <linux/clk-provider.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +
> +#define PMU_XTAL_FREQ_RATIO 0x66c
> +#define XTAL_ALP_PER_4ILP 0x00001fff
> +#define XTAL_CTL_EN 0x80000000
> +#define PMU_SLOW_CLK_PERIOD 0x6dc
> +
> +struct bcm53573_ilp {
> + struct clk *clk;
> + struct clk_hw hw;
> + void __iomem *pmu;
> +};
> +
> +static int bcm53573_ilp_enable(struct clk_hw *hw)
> +{
> + struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
> +
> + writel(0x10199, ilp->pmu + PMU_SLOW_CLK_PERIOD);
> + writel(0x10000, ilp->pmu + 0x674);
Is there a name for 0x674?
> +
> + return 0;
> +}
> +
> +static unsigned long bcm53573_ilp_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
> + void __iomem *pmu = ilp->pmu;
> + u32 last_val, cur_val;
> + u32 sum = 0, num = 0, loop_num = 0;
Should these just be plain ints? Do we care about sizes for these
variables?
> + u32 avg;
This one too.
> +
> + /* Enable measurement */
> + writel(XTAL_CTL_EN, pmu + PMU_XTAL_FREQ_RATIO);
> +
> + /* Read initial value */
> + last_val = readl(pmu + PMU_XTAL_FREQ_RATIO) & XTAL_ALP_PER_4ILP;
> +
> + /*
> + * At minimum we should loop for a bit to let hardware do the
> + * measurement. This isn't very accurate however, so for a better
> + * precision lets try getting 20 different values for and use average.
> + */
> + while (num < 20) {
> + cur_val = readl(pmu + PMU_XTAL_FREQ_RATIO) & XTAL_ALP_PER_4ILP;
> +
> + if (cur_val != last_val) {
> + /* Got different value, use it */
> + sum += cur_val;
> + num++;
> + loop_num = 0;
> + last_val = cur_val;
> + } else if (++loop_num > 5000) {
> + /* Same value over and over, give up */
> + sum += cur_val;
> + num++;
> + break;
> + }
Should there be a udelay() here? Or we're expected to tight loop
read the hardware? If so we should throw in a cpu_relax() here to
indicate tight loop.
> + }
> +
> + /* Disable measurement to save power */
> + writel(0x0, pmu + PMU_XTAL_FREQ_RATIO);
> +
> + avg = sum / num;
> +
> + return parent_rate * 4 / avg;
> +}
> +
> +static const struct clk_ops bcm53573_ilp_clk_ops = {
> + .enable = bcm53573_ilp_enable,
No disable? Or .is_enabled?
> + .recalc_rate = bcm53573_ilp_recalc_rate,
> +};
> +
> +static void bcm53573_ilp_init(struct device_node *np)
> +{
> + struct bcm53573_ilp *ilp;
> + struct resource res;
> + struct clk_init_data init = { 0 };
> + const char *parent_name;
> + int index;
> + int err;
> +
> + ilp = kzalloc(sizeof(*ilp), GFP_KERNEL);
> + if (!ilp)
> + return;
> +
> + parent_name = of_clk_get_parent_name(np, 0);
> + if (!parent_name) {
> + err = -ENOENT;
> + goto err_free_ilp;
> + }
> +
> + /* TODO: This looks generic, try making it OF helper. */
> + index = of_property_match_string(np, "reg-names", "pmu");
> + if (index < 0) {
> + err = index;
> + goto err_free_ilp;
> + }
> + err = of_address_to_resource(np, index, &res);
> + if (err)
> + goto err_free_ilp;
> + ilp->pmu = ioremap(res.start, resource_size(&res));
> + if (IS_ERR(ilp->pmu)) {
> + err = PTR_ERR(ilp->pmu);
> + goto err_free_ilp;
> + }
> +
> + init.name = np->name;
> + init.ops = &bcm53573_ilp_clk_ops;
> + init.parent_names = &parent_name;
> + init.num_parents = 1;
> +
> + ilp->hw.init = &init;
> + ilp->clk = clk_register(NULL, &ilp->hw);
please use clk_hw_register() and of_clk_add_hw_provider().
> + if (WARN_ON(IS_ERR(ilp->clk)))
> + goto err_unmap_pmu;
> +
> + err = of_clk_add_provider(np, of_clk_src_simple_get, ilp->clk);
> + if (err)
> + goto err_clk_unregister;
> +
> + return;
> +
> +err_clk_unregister:
> + clk_unregister(ilp->clk);
> +err_unmap_pmu:
> + iounmap(ilp->pmu);
> +err_free_ilp:
> + kfree(ilp);
> + pr_err("Failed to init ILP clock: %d\n", err);
> +}
> +CLK_OF_DECLARE(bcm53573_ilp_clk, "brcm,bcm53573-ilp", bcm53573_ilp_init);
Can this be a platform driver instead?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock
@ 2016-08-24 8:47 ` Stephen Boyd
0 siblings, 0 replies; 50+ messages in thread
From: Stephen Boyd @ 2016-08-24 8:47 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Michael Turquette, linux-clk-u79uwXL29TY76Z2rM5mHXA,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
Rafał Miłecki, Rob Herring, Mark Rutland, Eric Anholt,
Jon Mason, Florian Fainelli, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On 08/23, Rafał Miłecki wrote:
> diff --git a/drivers/clk/bcm/clk-bcm53573-ilp.c b/drivers/clk/bcm/clk-bcm53573-ilp.c
> new file mode 100644
> index 0000000..b7ac0eb
> --- /dev/null
> +++ b/drivers/clk/bcm/clk-bcm53573-ilp.c
> @@ -0,0 +1,146 @@
> +/*
> + * Copyright (C) 2016 Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> + *
> + * 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.
> + */
> +
> +#include <linux/clk.h>
Is this include used?
> +#include <linux/clk-provider.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +
> +#define PMU_XTAL_FREQ_RATIO 0x66c
> +#define XTAL_ALP_PER_4ILP 0x00001fff
> +#define XTAL_CTL_EN 0x80000000
> +#define PMU_SLOW_CLK_PERIOD 0x6dc
> +
> +struct bcm53573_ilp {
> + struct clk *clk;
> + struct clk_hw hw;
> + void __iomem *pmu;
> +};
> +
> +static int bcm53573_ilp_enable(struct clk_hw *hw)
> +{
> + struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
> +
> + writel(0x10199, ilp->pmu + PMU_SLOW_CLK_PERIOD);
> + writel(0x10000, ilp->pmu + 0x674);
Is there a name for 0x674?
> +
> + return 0;
> +}
> +
> +static unsigned long bcm53573_ilp_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
> + void __iomem *pmu = ilp->pmu;
> + u32 last_val, cur_val;
> + u32 sum = 0, num = 0, loop_num = 0;
Should these just be plain ints? Do we care about sizes for these
variables?
> + u32 avg;
This one too.
> +
> + /* Enable measurement */
> + writel(XTAL_CTL_EN, pmu + PMU_XTAL_FREQ_RATIO);
> +
> + /* Read initial value */
> + last_val = readl(pmu + PMU_XTAL_FREQ_RATIO) & XTAL_ALP_PER_4ILP;
> +
> + /*
> + * At minimum we should loop for a bit to let hardware do the
> + * measurement. This isn't very accurate however, so for a better
> + * precision lets try getting 20 different values for and use average.
> + */
> + while (num < 20) {
> + cur_val = readl(pmu + PMU_XTAL_FREQ_RATIO) & XTAL_ALP_PER_4ILP;
> +
> + if (cur_val != last_val) {
> + /* Got different value, use it */
> + sum += cur_val;
> + num++;
> + loop_num = 0;
> + last_val = cur_val;
> + } else if (++loop_num > 5000) {
> + /* Same value over and over, give up */
> + sum += cur_val;
> + num++;
> + break;
> + }
Should there be a udelay() here? Or we're expected to tight loop
read the hardware? If so we should throw in a cpu_relax() here to
indicate tight loop.
> + }
> +
> + /* Disable measurement to save power */
> + writel(0x0, pmu + PMU_XTAL_FREQ_RATIO);
> +
> + avg = sum / num;
> +
> + return parent_rate * 4 / avg;
> +}
> +
> +static const struct clk_ops bcm53573_ilp_clk_ops = {
> + .enable = bcm53573_ilp_enable,
No disable? Or .is_enabled?
> + .recalc_rate = bcm53573_ilp_recalc_rate,
> +};
> +
> +static void bcm53573_ilp_init(struct device_node *np)
> +{
> + struct bcm53573_ilp *ilp;
> + struct resource res;
> + struct clk_init_data init = { 0 };
> + const char *parent_name;
> + int index;
> + int err;
> +
> + ilp = kzalloc(sizeof(*ilp), GFP_KERNEL);
> + if (!ilp)
> + return;
> +
> + parent_name = of_clk_get_parent_name(np, 0);
> + if (!parent_name) {
> + err = -ENOENT;
> + goto err_free_ilp;
> + }
> +
> + /* TODO: This looks generic, try making it OF helper. */
> + index = of_property_match_string(np, "reg-names", "pmu");
> + if (index < 0) {
> + err = index;
> + goto err_free_ilp;
> + }
> + err = of_address_to_resource(np, index, &res);
> + if (err)
> + goto err_free_ilp;
> + ilp->pmu = ioremap(res.start, resource_size(&res));
> + if (IS_ERR(ilp->pmu)) {
> + err = PTR_ERR(ilp->pmu);
> + goto err_free_ilp;
> + }
> +
> + init.name = np->name;
> + init.ops = &bcm53573_ilp_clk_ops;
> + init.parent_names = &parent_name;
> + init.num_parents = 1;
> +
> + ilp->hw.init = &init;
> + ilp->clk = clk_register(NULL, &ilp->hw);
please use clk_hw_register() and of_clk_add_hw_provider().
> + if (WARN_ON(IS_ERR(ilp->clk)))
> + goto err_unmap_pmu;
> +
> + err = of_clk_add_provider(np, of_clk_src_simple_get, ilp->clk);
> + if (err)
> + goto err_clk_unregister;
> +
> + return;
> +
> +err_clk_unregister:
> + clk_unregister(ilp->clk);
> +err_unmap_pmu:
> + iounmap(ilp->pmu);
> +err_free_ilp:
> + kfree(ilp);
> + pr_err("Failed to init ILP clock: %d\n", err);
> +}
> +CLK_OF_DECLARE(bcm53573_ilp_clk, "brcm,bcm53573-ilp", bcm53573_ilp_init);
Can this be a platform driver instead?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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 [flat|nested] 50+ messages in thread
* Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock
2016-08-24 8:47 ` Stephen Boyd
@ 2016-08-25 13:28 ` Rafał Miłecki
-1 siblings, 0 replies; 50+ messages in thread
From: Rafał Miłecki @ 2016-08-25 13:28 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, linux-clk, bcm-kernel-feedback-list,
Rafał Miłecki, Rob Herring, Mark Rutland, Eric Anholt,
Jon Mason, Florian Fainelli, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On 24 August 2016 at 10:47, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 08/23, Rafał Miłecki wrote:
>> diff --git a/drivers/clk/bcm/clk-bcm53573-ilp.c b/drivers/clk/bcm/clk-bcm53573-ilp.c
>> new file mode 100644
>> index 0000000..b7ac0eb
>> --- /dev/null
>> +++ b/drivers/clk/bcm/clk-bcm53573-ilp.c
>> @@ -0,0 +1,146 @@
>> +/*
>> + * Copyright (C) 2016 Rafał Miłecki <rafal@milecki.pl>
>> + *
>> + * 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.
>> + */
>> +
>> +#include <linux/clk.h>
>
> Is this include used?
No. Good point.
>> +#include <linux/clk-provider.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/slab.h>
>> +
>> +#define PMU_XTAL_FREQ_RATIO 0x66c
>> +#define XTAL_ALP_PER_4ILP 0x00001fff
>> +#define XTAL_CTL_EN 0x80000000
>> +#define PMU_SLOW_CLK_PERIOD 0x6dc
>> +
>> +struct bcm53573_ilp {
>> + struct clk *clk;
>> + struct clk_hw hw;
>> + void __iomem *pmu;
>> +};
>> +
>> +static int bcm53573_ilp_enable(struct clk_hw *hw)
>> +{
>> + struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
>> +
>> + writel(0x10199, ilp->pmu + PMU_SLOW_CLK_PERIOD);
>> + writel(0x10000, ilp->pmu + 0x674);
>
> Is there a name for 0x674?
>> +
>> + return 0;
>> +}
>> +
>> +static unsigned long bcm53573_ilp_recalc_rate(struct clk_hw *hw,
>> + unsigned long parent_rate)
>> +{
>> + struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
>> + void __iomem *pmu = ilp->pmu;
>> + u32 last_val, cur_val;
>> + u32 sum = 0, num = 0, loop_num = 0;
>
> Should these just be plain ints? Do we care about sizes for these
> variables?
>
>> + u32 avg;
>
> This one too.
Right.
>> +
>> + /* Enable measurement */
>> + writel(XTAL_CTL_EN, pmu + PMU_XTAL_FREQ_RATIO);
>> +
>> + /* Read initial value */
>> + last_val = readl(pmu + PMU_XTAL_FREQ_RATIO) & XTAL_ALP_PER_4ILP;
>> +
>> + /*
>> + * At minimum we should loop for a bit to let hardware do the
>> + * measurement. This isn't very accurate however, so for a better
>> + * precision lets try getting 20 different values for and use average.
>> + */
>> + while (num < 20) {
>> + cur_val = readl(pmu + PMU_XTAL_FREQ_RATIO) & XTAL_ALP_PER_4ILP;
>> +
>> + if (cur_val != last_val) {
>> + /* Got different value, use it */
>> + sum += cur_val;
>> + num++;
>> + loop_num = 0;
>> + last_val = cur_val;
>> + } else if (++loop_num > 5000) {
>> + /* Same value over and over, give up */
>> + sum += cur_val;
>> + num++;
>> + break;
>> + }
>
> Should there be a udelay() here? Or we're expected to tight loop
> read the hardware? If so we should throw in a cpu_relax() here to
> indicate tight loop.
>
>> + }
>> +
>> + /* Disable measurement to save power */
>> + writel(0x0, pmu + PMU_XTAL_FREQ_RATIO);
>> +
>> + avg = sum / num;
>> +
>> + return parent_rate * 4 / avg;
>> +}
>> +
>> +static const struct clk_ops bcm53573_ilp_clk_ops = {
>> + .enable = bcm53573_ilp_enable,
>
> No disable? Or .is_enabled?
The beauty of working without datasheets... I'll compare initial reg
state with one after enabling and see if there is sth obvious.
>> + .recalc_rate = bcm53573_ilp_recalc_rate,
>> +};
>> +
>> +static void bcm53573_ilp_init(struct device_node *np)
>> +{
>> + struct bcm53573_ilp *ilp;
>> + struct resource res;
>> + struct clk_init_data init = { 0 };
>> + const char *parent_name;
>> + int index;
>> + int err;
>> +
>> + ilp = kzalloc(sizeof(*ilp), GFP_KERNEL);
>> + if (!ilp)
>> + return;
>> +
>> + parent_name = of_clk_get_parent_name(np, 0);
>> + if (!parent_name) {
>> + err = -ENOENT;
>> + goto err_free_ilp;
>> + }
>> +
>> + /* TODO: This looks generic, try making it OF helper. */
>> + index = of_property_match_string(np, "reg-names", "pmu");
>> + if (index < 0) {
>> + err = index;
>> + goto err_free_ilp;
>> + }
>> + err = of_address_to_resource(np, index, &res);
>> + if (err)
>> + goto err_free_ilp;
>> + ilp->pmu = ioremap(res.start, resource_size(&res));
>> + if (IS_ERR(ilp->pmu)) {
>> + err = PTR_ERR(ilp->pmu);
>> + goto err_free_ilp;
>> + }
>> +
>> + init.name = np->name;
>> + init.ops = &bcm53573_ilp_clk_ops;
>> + init.parent_names = &parent_name;
>> + init.num_parents = 1;
>> +
>> + ilp->hw.init = &init;
>> + ilp->clk = clk_register(NULL, &ilp->hw);
>
> please use clk_hw_register() and of_clk_add_hw_provider().
I wasn't aware of this API change, thanks.
>> + if (WARN_ON(IS_ERR(ilp->clk)))
>> + goto err_unmap_pmu;
>> +
>> + err = of_clk_add_provider(np, of_clk_src_simple_get, ilp->clk);
>> + if (err)
>> + goto err_clk_unregister;
>> +
>> + return;
>> +
>> +err_clk_unregister:
>> + clk_unregister(ilp->clk);
>> +err_unmap_pmu:
>> + iounmap(ilp->pmu);
>> +err_free_ilp:
>> + kfree(ilp);
>> + pr_err("Failed to init ILP clock: %d\n", err);
>> +}
>> +CLK_OF_DECLARE(bcm53573_ilp_clk, "brcm,bcm53573-ilp", bcm53573_ilp_init);
>
> Can this be a platform driver instead?
I guess it can. Should it? It's not clear to me when CLK_OF_DECLARE is
preferred and when it's not.
--
Rafał
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock
@ 2016-08-25 13:28 ` Rafał Miłecki
0 siblings, 0 replies; 50+ messages in thread
From: Rafał Miłecki @ 2016-08-25 13:28 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, linux-clk, bcm-kernel-feedback-list,
Rafał Miłecki, Rob Herring, Mark Rutland, Eric Anholt,
Jon Mason, Florian Fainelli, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On 24 August 2016 at 10:47, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 08/23, Rafa=C5=82 Mi=C5=82ecki wrote:
>> diff --git a/drivers/clk/bcm/clk-bcm53573-ilp.c b/drivers/clk/bcm/clk-bc=
m53573-ilp.c
>> new file mode 100644
>> index 0000000..b7ac0eb
>> --- /dev/null
>> +++ b/drivers/clk/bcm/clk-bcm53573-ilp.c
>> @@ -0,0 +1,146 @@
>> +/*
>> + * Copyright (C) 2016 Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>> + *
>> + * 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.
>> + */
>> +
>> +#include <linux/clk.h>
>
> Is this include used?
No. Good point.
>> +#include <linux/clk-provider.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/slab.h>
>> +
>> +#define PMU_XTAL_FREQ_RATIO 0x66c
>> +#define XTAL_ALP_PER_4ILP 0x00001fff
>> +#define XTAL_CTL_EN 0x80000000
>> +#define PMU_SLOW_CLK_PERIOD 0x6dc
>> +
>> +struct bcm53573_ilp {
>> + struct clk *clk;
>> + struct clk_hw hw;
>> + void __iomem *pmu;
>> +};
>> +
>> +static int bcm53573_ilp_enable(struct clk_hw *hw)
>> +{
>> + struct bcm53573_ilp *ilp =3D container_of(hw, struct bcm53573_ilp,=
hw);
>> +
>> + writel(0x10199, ilp->pmu + PMU_SLOW_CLK_PERIOD);
>> + writel(0x10000, ilp->pmu + 0x674);
>
> Is there a name for 0x674?
>> +
>> + return 0;
>> +}
>> +
>> +static unsigned long bcm53573_ilp_recalc_rate(struct clk_hw *hw,
>> + unsigned long parent_rate)
>> +{
>> + struct bcm53573_ilp *ilp =3D container_of(hw, struct bcm53573_ilp,=
hw);
>> + void __iomem *pmu =3D ilp->pmu;
>> + u32 last_val, cur_val;
>> + u32 sum =3D 0, num =3D 0, loop_num =3D 0;
>
> Should these just be plain ints? Do we care about sizes for these
> variables?
>
>> + u32 avg;
>
> This one too.
Right.
>> +
>> + /* Enable measurement */
>> + writel(XTAL_CTL_EN, pmu + PMU_XTAL_FREQ_RATIO);
>> +
>> + /* Read initial value */
>> + last_val =3D readl(pmu + PMU_XTAL_FREQ_RATIO) & XTAL_ALP_PER_4ILP;
>> +
>> + /*
>> + * At minimum we should loop for a bit to let hardware do the
>> + * measurement. This isn't very accurate however, so for a better
>> + * precision lets try getting 20 different values for and use aver=
age.
>> + */
>> + while (num < 20) {
>> + cur_val =3D readl(pmu + PMU_XTAL_FREQ_RATIO) & XTAL_ALP_PE=
R_4ILP;
>> +
>> + if (cur_val !=3D last_val) {
>> + /* Got different value, use it */
>> + sum +=3D cur_val;
>> + num++;
>> + loop_num =3D 0;
>> + last_val =3D cur_val;
>> + } else if (++loop_num > 5000) {
>> + /* Same value over and over, give up */
>> + sum +=3D cur_val;
>> + num++;
>> + break;
>> + }
>
> Should there be a udelay() here? Or we're expected to tight loop
> read the hardware? If so we should throw in a cpu_relax() here to
> indicate tight loop.
>
>> + }
>> +
>> + /* Disable measurement to save power */
>> + writel(0x0, pmu + PMU_XTAL_FREQ_RATIO);
>> +
>> + avg =3D sum / num;
>> +
>> + return parent_rate * 4 / avg;
>> +}
>> +
>> +static const struct clk_ops bcm53573_ilp_clk_ops =3D {
>> + .enable =3D bcm53573_ilp_enable,
>
> No disable? Or .is_enabled?
The beauty of working without datasheets... I'll compare initial reg
state with one after enabling and see if there is sth obvious.
>> + .recalc_rate =3D bcm53573_ilp_recalc_rate,
>> +};
>> +
>> +static void bcm53573_ilp_init(struct device_node *np)
>> +{
>> + struct bcm53573_ilp *ilp;
>> + struct resource res;
>> + struct clk_init_data init =3D { 0 };
>> + const char *parent_name;
>> + int index;
>> + int err;
>> +
>> + ilp =3D kzalloc(sizeof(*ilp), GFP_KERNEL);
>> + if (!ilp)
>> + return;
>> +
>> + parent_name =3D of_clk_get_parent_name(np, 0);
>> + if (!parent_name) {
>> + err =3D -ENOENT;
>> + goto err_free_ilp;
>> + }
>> +
>> + /* TODO: This looks generic, try making it OF helper. */
>> + index =3D of_property_match_string(np, "reg-names", "pmu");
>> + if (index < 0) {
>> + err =3D index;
>> + goto err_free_ilp;
>> + }
>> + err =3D of_address_to_resource(np, index, &res);
>> + if (err)
>> + goto err_free_ilp;
>> + ilp->pmu =3D ioremap(res.start, resource_size(&res));
>> + if (IS_ERR(ilp->pmu)) {
>> + err =3D PTR_ERR(ilp->pmu);
>> + goto err_free_ilp;
>> + }
>> +
>> + init.name =3D np->name;
>> + init.ops =3D &bcm53573_ilp_clk_ops;
>> + init.parent_names =3D &parent_name;
>> + init.num_parents =3D 1;
>> +
>> + ilp->hw.init =3D &init;
>> + ilp->clk =3D clk_register(NULL, &ilp->hw);
>
> please use clk_hw_register() and of_clk_add_hw_provider().
I wasn't aware of this API change, thanks.
>> + if (WARN_ON(IS_ERR(ilp->clk)))
>> + goto err_unmap_pmu;
>> +
>> + err =3D of_clk_add_provider(np, of_clk_src_simple_get, ilp->clk);
>> + if (err)
>> + goto err_clk_unregister;
>> +
>> + return;
>> +
>> +err_clk_unregister:
>> + clk_unregister(ilp->clk);
>> +err_unmap_pmu:
>> + iounmap(ilp->pmu);
>> +err_free_ilp:
>> + kfree(ilp);
>> + pr_err("Failed to init ILP clock: %d\n", err);
>> +}
>> +CLK_OF_DECLARE(bcm53573_ilp_clk, "brcm,bcm53573-ilp", bcm53573_ilp_init=
);
>
> Can this be a platform driver instead?
I guess it can. Should it? It's not clear to me when CLK_OF_DECLARE is
preferred and when it's not.
--=20
Rafa=C5=82
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock
2016-08-25 13:28 ` Rafał Miłecki
(?)
@ 2016-08-25 19:57 ` Stephen Boyd
-1 siblings, 0 replies; 50+ messages in thread
From: Stephen Boyd @ 2016-08-25 19:57 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Michael Turquette, linux-clk, bcm-kernel-feedback-list,
Rafał Miłecki, Rob Herring, Mark Rutland, Eric Anholt,
Jon Mason, Florian Fainelli, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On 08/25, Rafał Miłecki wrote:
> On 24 August 2016 at 10:47, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 08/23, Rafał Miłecki wrote:
> >> + }
> >> +
> >> + /* Disable measurement to save power */
> >> + writel(0x0, pmu + PMU_XTAL_FREQ_RATIO);
> >> +
> >> + avg = sum / num;
> >> +
> >> + return parent_rate * 4 / avg;
> >> +}
> >> +
> >> +static const struct clk_ops bcm53573_ilp_clk_ops = {
> >> + .enable = bcm53573_ilp_enable,
> >
> > No disable? Or .is_enabled?
>
> The beauty of working without datasheets... I'll compare initial reg
> state with one after enabling and see if there is sth obvious.
>
>
Ok.
> >
> > Can this be a platform driver instead?
>
> I guess it can. Should it? It's not clear to me when CLK_OF_DECLARE is
> preferred and when it's not.
We've been taking an approach of using CLK_OF_DECLARE() for clks
that must be registered for timers and interrupt controllers (as
these things are done before the device model is initialized),
and struct drivers otherwise. If the DT node has a mix of
timer/interrupt controller clks and other clks then
CLK_OF_DECLARE_DRIVER() can be used and then we can still have a
builtin driver.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock
2016-08-24 8:47 ` Stephen Boyd
@ 2016-08-25 14:11 ` Rafał Miłecki
-1 siblings, 0 replies; 50+ messages in thread
From: Rafał Miłecki @ 2016-08-25 14:11 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, linux-clk, bcm-kernel-feedback-list,
Rafał Miłecki, Rob Herring, Mark Rutland, Eric Anholt,
Jon Mason, Florian Fainelli, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On 24 August 2016 at 10:47, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 08/23, Rafał Miłecki wrote:
>> +static int bcm53573_ilp_enable(struct clk_hw *hw)
>> +{
>> + struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
>> +
>> + writel(0x10199, ilp->pmu + PMU_SLOW_CLK_PERIOD);
>> + writel(0x10000, ilp->pmu + 0x674);
>
> Is there a name for 0x674?
No in any sources published by Broadcom. I was experimenting with bit
0x10000 in PMU_SLOW_CLK_PERIOD and 0x10000 in 0x674. Both have to be
set. I was suspecting that maybe one of them is just some trigger and
it can be switched back to 0, but it's not the case. I definitely need
to set both of them to get clock working.
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock
@ 2016-08-25 14:11 ` Rafał Miłecki
0 siblings, 0 replies; 50+ messages in thread
From: Rafał Miłecki @ 2016-08-25 14:11 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, linux-clk, bcm-kernel-feedback-list,
Rafał Miłecki, Rob Herring, Mark Rutland, Eric Anholt,
Jon Mason, Florian Fainelli, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On 24 August 2016 at 10:47, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 08/23, Rafa=C5=82 Mi=C5=82ecki wrote:
>> +static int bcm53573_ilp_enable(struct clk_hw *hw)
>> +{
>> + struct bcm53573_ilp *ilp =3D container_of(hw, struct bcm53573_ilp,=
hw);
>> +
>> + writel(0x10199, ilp->pmu + PMU_SLOW_CLK_PERIOD);
>> + writel(0x10000, ilp->pmu + 0x674);
>
> Is there a name for 0x674?
No in any sources published by Broadcom. I was experimenting with bit
0x10000 in PMU_SLOW_CLK_PERIOD and 0x10000 in 0x674. Both have to be
set. I was suspecting that maybe one of them is just some trigger and
it can be switched back to 0, but it's not the case. I definitely need
to set both of them to get clock working.
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock
2016-08-25 14:11 ` Rafał Miłecki
(?)
@ 2016-08-25 19:58 ` Stephen Boyd
-1 siblings, 0 replies; 50+ messages in thread
From: Stephen Boyd @ 2016-08-25 19:58 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Michael Turquette, linux-clk, bcm-kernel-feedback-list,
Rafał Miłecki, Rob Herring, Mark Rutland, Eric Anholt,
Jon Mason, Florian Fainelli, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On 08/25, Rafał Miłecki wrote:
> On 24 August 2016 at 10:47, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 08/23, Rafał Miłecki wrote:
> >> +static int bcm53573_ilp_enable(struct clk_hw *hw)
> >> +{
> >> + struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
> >> +
> >> + writel(0x10199, ilp->pmu + PMU_SLOW_CLK_PERIOD);
> >> + writel(0x10000, ilp->pmu + 0x674);
> >
> > Is there a name for 0x674?
>
> No in any sources published by Broadcom. I was experimenting with bit
> 0x10000 in PMU_SLOW_CLK_PERIOD and 0x10000 in 0x674. Both have to be
> set. I was suspecting that maybe one of them is just some trigger and
> it can be switched back to 0, but it's not the case. I definitely need
> to set both of them to get clock working.
Ok no worries, raw number is fine.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH V6] clk: bcm: Add driver for BCM53573 ILP clock
2016-08-23 6:25 ` Rafał Miłecki
@ 2016-08-26 12:25 ` Rafał Miłecki
-1 siblings, 0 replies; 50+ messages in thread
From: Rafał Miłecki @ 2016-08-26 12:25 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd
Cc: linux-clk, bcm-kernel-feedback-list, Rafał Miłecki,
Rob Herring, Mark Rutland, Florian Fainelli, Jon Mason,
Eric Anholt,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
From: Rafał Miłecki <rafal@milecki.pl>
This clock is present on BCM53573 devices (including BCM47189) that use
Cortex-A7. ILP is a part of PMU (Power Management Unit) and so it should
be defined as one of its subnodes (subdevices). For more details see
Documentation entry.
Unfortunately there isn't a set of registers related to ILP clock only.
We use registers 0x66c, 0x674 and 0x6dc and between them there are e.g.
"retention*" and "control_ext" regs. This is why this driver maps all
0x1000 B of space.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Rebase on top of clk-next
Use ALP as parent clock
Improve comments
Switch from ioremap_nocache to ioremap
Check of_clk_add_provide result for error
V3: Drop #include <linux/moduleh>
Make ILP DT entry part of PMU
Describe ILP as subdevice of PMU in Documentation
V4: Use BCM53573 name as suggested by Jon and Ray. It seems "Northstar"
(even if used in some resources) should be used in relation to
Cortex-A9 devices only.
V5: Rename remaining "ns" references to "bcm53573", sorry, I sent V4 too
early.
V6: Drop #include <linux/clk.h>
Use "int" as type where it matches usage
Add cpu_relax() in the loop
Add disable callback
Use _hw_ functions for registering struct clk_hw (new API)
Thanks a lot Stephen!
---
.../bindings/clock/brcm,bcm53573-ilp.txt | 40 ++++++
drivers/clk/bcm/Makefile | 1 +
drivers/clk/bcm/clk-bcm53573-ilp.c | 157 +++++++++++++++++++++
3 files changed, 198 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
create mode 100644 drivers/clk/bcm/clk-bcm53573-ilp.c
diff --git a/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
new file mode 100644
index 0000000..5ab3107
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
@@ -0,0 +1,40 @@
+Broadcom BCM53573 ILP clock
+===========================
+
+This binding uses the common clock binding:
+ Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+This binding is used for ILP clock (sometimes referred as "slow clock")
+on Broadcom BCM53573 devices using Cortex-A7 CPU.
+
+This clock is part of PMU (Power Management Unit), a Broadcom's device
+handing power-related aspects. Please note PMU contains more subdevices,
+ILP is only one of them.
+
+ILP's rate has to be calculated on runtime and it depends on ALP clock
+which has to be referenced.
+
+Required properties:
+- compatible: "brcm,bcm53573-ilp"
+- reg: iomem address range of PMU (Power Management Unit)
+- reg-names: "pmu", the only needed & supported reg right now
+- clocks: has to reference an ALP clock
+- #clock-cells: should be <0>
+
+Example:
+
+pmu@18012000 {
+ compatible = "simple-bus";
+ ranges = <0x00000000 0x18012000 0x00001000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ ilp: ilp@0 {
+ compatible = "brcm,bcm53573-ilp";
+ reg = <0 0x1000>;
+ reg-names = "pmu";
+ clocks = <&alp>;
+ #clock-cells = <0>;
+ clock-output-names = "ilp";
+ };
+};
diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile
index 1d79bd2..4b8c56d 100644
--- a/drivers/clk/bcm/Makefile
+++ b/drivers/clk/bcm/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_COMMON_CLK_IPROC) += clk-ns2.o
obj-$(CONFIG_ARCH_BCM_CYGNUS) += clk-cygnus.o
obj-$(CONFIG_ARCH_BCM_NSP) += clk-nsp.o
obj-$(CONFIG_ARCH_BCM_5301X) += clk-nsp.o
+obj-$(CONFIG_ARCH_BCM_53573) += clk-bcm53573-ilp.o
diff --git a/drivers/clk/bcm/clk-bcm53573-ilp.c b/drivers/clk/bcm/clk-bcm53573-ilp.c
new file mode 100644
index 0000000..faf2b0a
--- /dev/null
+++ b/drivers/clk/bcm/clk-bcm53573-ilp.c
@@ -0,0 +1,157 @@
+/*
+ * Copyright (C) 2016 Rafał Miłecki <rafal@milecki.pl>
+ *
+ * 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.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+
+#define PMU_XTAL_FREQ_RATIO 0x66c
+#define XTAL_ALP_PER_4ILP 0x00001fff
+#define XTAL_CTL_EN 0x80000000
+#define PMU_SLOW_CLK_PERIOD 0x6dc
+
+struct bcm53573_ilp {
+ struct clk_hw hw;
+ void __iomem *pmu;
+};
+
+static int bcm53573_ilp_enable(struct clk_hw *hw)
+{
+ struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
+
+ writel(0x10199, ilp->pmu + PMU_SLOW_CLK_PERIOD);
+ writel(0x10000, ilp->pmu + 0x674);
+
+ return 0;
+}
+
+static void bcm53573_ilp_disable(struct clk_hw *hw)
+{
+ struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
+
+ writel(0, ilp->pmu + PMU_SLOW_CLK_PERIOD);
+ writel(0, ilp->pmu + 0x674);
+}
+
+static unsigned long bcm53573_ilp_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
+ void __iomem *pmu = ilp->pmu;
+ u32 last_val, cur_val;
+ int sum = 0, num = 0, loop_num = 0;
+ int avg;
+
+ /* Enable measurement */
+ writel(XTAL_CTL_EN, pmu + PMU_XTAL_FREQ_RATIO);
+
+ /* Read initial value */
+ last_val = readl(pmu + PMU_XTAL_FREQ_RATIO) & XTAL_ALP_PER_4ILP;
+
+ /*
+ * At minimum we should loop for a bit to let hardware do the
+ * measurement. This isn't very accurate however, so for a better
+ * precision lets try getting 20 different values for and use average.
+ */
+ while (num < 20) {
+ cur_val = readl(pmu + PMU_XTAL_FREQ_RATIO) & XTAL_ALP_PER_4ILP;
+
+ if (cur_val != last_val) {
+ /* Got different value, use it */
+ sum += cur_val;
+ num++;
+ loop_num = 0;
+ last_val = cur_val;
+ } else if (++loop_num > 5000) {
+ /* Same value over and over, give up */
+ sum += cur_val;
+ num++;
+ break;
+ }
+
+ cpu_relax();
+ }
+
+ /* Disable measurement to save power */
+ writel(0x0, pmu + PMU_XTAL_FREQ_RATIO);
+
+ avg = sum / num;
+
+ return parent_rate * 4 / avg;
+}
+
+static const struct clk_ops bcm53573_ilp_clk_ops = {
+ .enable = bcm53573_ilp_enable,
+ .disable = bcm53573_ilp_disable,
+ .recalc_rate = bcm53573_ilp_recalc_rate,
+};
+
+static void bcm53573_ilp_init(struct device_node *np)
+{
+ struct bcm53573_ilp *ilp;
+ struct resource res;
+ struct clk_init_data init = { 0 };
+ const char *parent_name;
+ int index;
+ int err;
+
+ ilp = kzalloc(sizeof(*ilp), GFP_KERNEL);
+ if (!ilp)
+ return;
+
+ parent_name = of_clk_get_parent_name(np, 0);
+ if (!parent_name) {
+ err = -ENOENT;
+ goto err_free_ilp;
+ }
+
+ /* TODO: This looks generic, try making it OF helper. */
+ index = of_property_match_string(np, "reg-names", "pmu");
+ if (index < 0) {
+ err = index;
+ goto err_free_ilp;
+ }
+ err = of_address_to_resource(np, index, &res);
+ if (err)
+ goto err_free_ilp;
+ ilp->pmu = ioremap(res.start, resource_size(&res));
+ if (IS_ERR(ilp->pmu)) {
+ err = PTR_ERR(ilp->pmu);
+ goto err_free_ilp;
+ }
+
+ init.name = np->name;
+ init.ops = &bcm53573_ilp_clk_ops;
+ init.parent_names = &parent_name;
+ init.num_parents = 1;
+
+ ilp->hw.init = &init;
+ err = clk_hw_register(NULL, &ilp->hw);
+ if (err)
+ goto err_unmap_pmu;
+
+ err = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &ilp->hw);
+ if (err)
+ goto err_clk_hw_unregister;
+
+ return;
+
+err_clk_hw_unregister:
+ clk_hw_unregister(&ilp->hw);
+err_unmap_pmu:
+ iounmap(ilp->pmu);
+err_free_ilp:
+ kfree(ilp);
+ pr_err("Failed to init ILP clock: %d\n", err);
+}
+
+/* We need it very early for arch code, before device model gets ready */
+CLK_OF_DECLARE(bcm53573_ilp_clk, "brcm,bcm53573-ilp", bcm53573_ilp_init);
--
2.9.3
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH V6] clk: bcm: Add driver for BCM53573 ILP clock
@ 2016-08-26 12:25 ` Rafał Miłecki
0 siblings, 0 replies; 50+ messages in thread
From: Rafał Miłecki @ 2016-08-26 12:25 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd
Cc: linux-clk, bcm-kernel-feedback-list, Rafał Miłecki,
Rob Herring, Mark Rutland, Florian Fainelli, Jon Mason,
Eric Anholt,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
From: Rafał Miłecki <rafal@milecki.pl>
This clock is present on BCM53573 devices (including BCM47189) that use
Cortex-A7. ILP is a part of PMU (Power Management Unit) and so it should
be defined as one of its subnodes (subdevices). For more details see
Documentation entry.
Unfortunately there isn't a set of registers related to ILP clock only.
We use registers 0x66c, 0x674 and 0x6dc and between them there are e.g.
"retention*" and "control_ext" regs. This is why this driver maps all
0x1000 B of space.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Rebase on top of clk-next
Use ALP as parent clock
Improve comments
Switch from ioremap_nocache to ioremap
Check of_clk_add_provide result for error
V3: Drop #include <linux/moduleh>
Make ILP DT entry part of PMU
Describe ILP as subdevice of PMU in Documentation
V4: Use BCM53573 name as suggested by Jon and Ray. It seems "Northstar"
(even if used in some resources) should be used in relation to
Cortex-A9 devices only.
V5: Rename remaining "ns" references to "bcm53573", sorry, I sent V4 too
early.
V6: Drop #include <linux/clk.h>
Use "int" as type where it matches usage
Add cpu_relax() in the loop
Add disable callback
Use _hw_ functions for registering struct clk_hw (new API)
Thanks a lot Stephen!
---
.../bindings/clock/brcm,bcm53573-ilp.txt | 40 ++++++
drivers/clk/bcm/Makefile | 1 +
drivers/clk/bcm/clk-bcm53573-ilp.c | 157 +++++++++++++++++++++
3 files changed, 198 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
create mode 100644 drivers/clk/bcm/clk-bcm53573-ilp.c
diff --git a/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
new file mode 100644
index 0000000..5ab3107
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
@@ -0,0 +1,40 @@
+Broadcom BCM53573 ILP clock
+===========================
+
+This binding uses the common clock binding:
+ Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+This binding is used for ILP clock (sometimes referred as "slow clock")
+on Broadcom BCM53573 devices using Cortex-A7 CPU.
+
+This clock is part of PMU (Power Management Unit), a Broadcom's device
+handing power-related aspects. Please note PMU contains more subdevices,
+ILP is only one of them.
+
+ILP's rate has to be calculated on runtime and it depends on ALP clock
+which has to be referenced.
+
+Required properties:
+- compatible: "brcm,bcm53573-ilp"
+- reg: iomem address range of PMU (Power Management Unit)
+- reg-names: "pmu", the only needed & supported reg right now
+- clocks: has to reference an ALP clock
+- #clock-cells: should be <0>
+
+Example:
+
+pmu@18012000 {
+ compatible = "simple-bus";
+ ranges = <0x00000000 0x18012000 0x00001000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ ilp: ilp@0 {
+ compatible = "brcm,bcm53573-ilp";
+ reg = <0 0x1000>;
+ reg-names = "pmu";
+ clocks = <&alp>;
+ #clock-cells = <0>;
+ clock-output-names = "ilp";
+ };
+};
diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile
index 1d79bd2..4b8c56d 100644
--- a/drivers/clk/bcm/Makefile
+++ b/drivers/clk/bcm/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_COMMON_CLK_IPROC) += clk-ns2.o
obj-$(CONFIG_ARCH_BCM_CYGNUS) += clk-cygnus.o
obj-$(CONFIG_ARCH_BCM_NSP) += clk-nsp.o
obj-$(CONFIG_ARCH_BCM_5301X) += clk-nsp.o
+obj-$(CONFIG_ARCH_BCM_53573) += clk-bcm53573-ilp.o
diff --git a/drivers/clk/bcm/clk-bcm53573-ilp.c b/drivers/clk/bcm/clk-bcm53573-ilp.c
new file mode 100644
index 0000000..faf2b0a
--- /dev/null
+++ b/drivers/clk/bcm/clk-bcm53573-ilp.c
@@ -0,0 +1,157 @@
+/*
+ * Copyright (C) 2016 Rafał Miłecki <rafal@milecki.pl>
+ *
+ * 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.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+
+#define PMU_XTAL_FREQ_RATIO 0x66c
+#define XTAL_ALP_PER_4ILP 0x00001fff
+#define XTAL_CTL_EN 0x80000000
+#define PMU_SLOW_CLK_PERIOD 0x6dc
+
+struct bcm53573_ilp {
+ struct clk_hw hw;
+ void __iomem *pmu;
+};
+
+static int bcm53573_ilp_enable(struct clk_hw *hw)
+{
+ struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
+
+ writel(0x10199, ilp->pmu + PMU_SLOW_CLK_PERIOD);
+ writel(0x10000, ilp->pmu + 0x674);
+
+ return 0;
+}
+
+static void bcm53573_ilp_disable(struct clk_hw *hw)
+{
+ struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
+
+ writel(0, ilp->pmu + PMU_SLOW_CLK_PERIOD);
+ writel(0, ilp->pmu + 0x674);
+}
+
+static unsigned long bcm53573_ilp_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
+ void __iomem *pmu = ilp->pmu;
+ u32 last_val, cur_val;
+ int sum = 0, num = 0, loop_num = 0;
+ int avg;
+
+ /* Enable measurement */
+ writel(XTAL_CTL_EN, pmu + PMU_XTAL_FREQ_RATIO);
+
+ /* Read initial value */
+ last_val = readl(pmu + PMU_XTAL_FREQ_RATIO) & XTAL_ALP_PER_4ILP;
+
+ /*
+ * At minimum we should loop for a bit to let hardware do the
+ * measurement. This isn't very accurate however, so for a better
+ * precision lets try getting 20 different values for and use average.
+ */
+ while (num < 20) {
+ cur_val = readl(pmu + PMU_XTAL_FREQ_RATIO) & XTAL_ALP_PER_4ILP;
+
+ if (cur_val != last_val) {
+ /* Got different value, use it */
+ sum += cur_val;
+ num++;
+ loop_num = 0;
+ last_val = cur_val;
+ } else if (++loop_num > 5000) {
+ /* Same value over and over, give up */
+ sum += cur_val;
+ num++;
+ break;
+ }
+
+ cpu_relax();
+ }
+
+ /* Disable measurement to save power */
+ writel(0x0, pmu + PMU_XTAL_FREQ_RATIO);
+
+ avg = sum / num;
+
+ return parent_rate * 4 / avg;
+}
+
+static const struct clk_ops bcm53573_ilp_clk_ops = {
+ .enable = bcm53573_ilp_enable,
+ .disable = bcm53573_ilp_disable,
+ .recalc_rate = bcm53573_ilp_recalc_rate,
+};
+
+static void bcm53573_ilp_init(struct device_node *np)
+{
+ struct bcm53573_ilp *ilp;
+ struct resource res;
+ struct clk_init_data init = { 0 };
+ const char *parent_name;
+ int index;
+ int err;
+
+ ilp = kzalloc(sizeof(*ilp), GFP_KERNEL);
+ if (!ilp)
+ return;
+
+ parent_name = of_clk_get_parent_name(np, 0);
+ if (!parent_name) {
+ err = -ENOENT;
+ goto err_free_ilp;
+ }
+
+ /* TODO: This looks generic, try making it OF helper. */
+ index = of_property_match_string(np, "reg-names", "pmu");
+ if (index < 0) {
+ err = index;
+ goto err_free_ilp;
+ }
+ err = of_address_to_resource(np, index, &res);
+ if (err)
+ goto err_free_ilp;
+ ilp->pmu = ioremap(res.start, resource_size(&res));
+ if (IS_ERR(ilp->pmu)) {
+ err = PTR_ERR(ilp->pmu);
+ goto err_free_ilp;
+ }
+
+ init.name = np->name;
+ init.ops = &bcm53573_ilp_clk_ops;
+ init.parent_names = &parent_name;
+ init.num_parents = 1;
+
+ ilp->hw.init = &init;
+ err = clk_hw_register(NULL, &ilp->hw);
+ if (err)
+ goto err_unmap_pmu;
+
+ err = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &ilp->hw);
+ if (err)
+ goto err_clk_hw_unregister;
+
+ return;
+
+err_clk_hw_unregister:
+ clk_hw_unregister(&ilp->hw);
+err_unmap_pmu:
+ iounmap(ilp->pmu);
+err_free_ilp:
+ kfree(ilp);
+ pr_err("Failed to init ILP clock: %d\n", err);
+}
+
+/* We need it very early for arch code, before device model gets ready */
+CLK_OF_DECLARE(bcm53573_ilp_clk, "brcm,bcm53573-ilp", bcm53573_ilp_init);
--
2.9.3
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH V7] clk: bcm: Add driver for BCM53573 ILP clock
2016-08-26 12:25 ` Rafał Miłecki
@ 2016-09-05 14:29 ` Rafał Miłecki
-1 siblings, 0 replies; 50+ messages in thread
From: Rafał Miłecki @ 2016-09-05 14:29 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd
Cc: linux-clk, bcm-kernel-feedback-list, Rafał Miłecki,
Rob Herring, Mark Rutland, Jon Mason, Florian Fainelli,
Eric Anholt,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
From: Rafał Miłecki <rafal@milecki.pl>
This clock is present on BCM53573 devices (including BCM47189) that use
Cortex-A7. ILP is a part of PMU (Power Management Unit) multi-function
device so we use syscon (and regmap) for it.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Rebase on top of clk-next
Use ALP as parent clock
Improve comments
Switch from ioremap_nocache to ioremap
Check of_clk_add_provide result for error
V3: Drop #include <linux/moduleh>
Make ILP DT entry part of PMU
Describe ILP as subdevice of PMU in Documentation
V4: Use BCM53573 name as suggested by Jon and Ray. It seems "Northstar"
(even if used in some resources) should be used in relation to
Cortex-A9 devices only.
V5: Rename remaining "ns" references to "bcm53573", sorry, I sent V4 too
early.
V6: Drop #include <linux/clk.h>
Use "int" as type where it matches usage
Add cpu_relax() in the loop
Add disable callback
Use _hw_ functions for registering struct clk_hw (new API)
Thanks a lot Stephen!
V7: Use syscon and regmap (thanks Rob!)
---
.../bindings/clock/brcm,bcm53573-ilp.txt | 36 +++++
drivers/clk/bcm/Makefile | 1 +
drivers/clk/bcm/clk-bcm53573-ilp.c | 148 +++++++++++++++++++++
3 files changed, 185 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
create mode 100644 drivers/clk/bcm/clk-bcm53573-ilp.c
diff --git a/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
new file mode 100644
index 0000000..4554f70
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
@@ -0,0 +1,36 @@
+Broadcom BCM53573 ILP clock
+===========================
+
+This binding uses the common clock binding:
+ Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+This binding is used for ILP clock (sometimes referred as "slow clock")
+on Broadcom BCM53573 devices using Cortex-A7 CPU.
+
+ILP's rate has to be calculated on runtime and it depends on ALP clock
+which has to be referenced.
+
+This clock is part of PMU (Power Management Unit), a Broadcom's device
+handing power-related aspects. Its node must be sub-node of the PMU
+device.
+
+Required properties:
+- compatible: "brcm,bcm53573-ilp"
+- reg: iomem address range of PMU (Power Management Unit)
+- reg-names: "pmu", the only needed & supported reg right now
+- clocks: has to reference an ALP clock
+- #clock-cells: should be <0>
+
+Example:
+
+pmu@18012000 {
+ compatible = "simple-mfd", "syscon";
+ reg = <0x18012000 0x00001000>;
+
+ ilp: ilp@0 {
+ compatible = "brcm,bcm53573-ilp";
+ clocks = <&alp>;
+ #clock-cells = <0>;
+ clock-output-names = "ilp";
+ };
+};
diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile
index 1d79bd2..4b8c56d 100644
--- a/drivers/clk/bcm/Makefile
+++ b/drivers/clk/bcm/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_COMMON_CLK_IPROC) += clk-ns2.o
obj-$(CONFIG_ARCH_BCM_CYGNUS) += clk-cygnus.o
obj-$(CONFIG_ARCH_BCM_NSP) += clk-nsp.o
obj-$(CONFIG_ARCH_BCM_5301X) += clk-nsp.o
+obj-$(CONFIG_ARCH_BCM_53573) += clk-bcm53573-ilp.o
diff --git a/drivers/clk/bcm/clk-bcm53573-ilp.c b/drivers/clk/bcm/clk-bcm53573-ilp.c
new file mode 100644
index 0000000..1f4e30d
--- /dev/null
+++ b/drivers/clk/bcm/clk-bcm53573-ilp.c
@@ -0,0 +1,148 @@
+/*
+ * Copyright (C) 2016 Rafał Miłecki <rafal@milecki.pl>
+ *
+ * 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.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#define PMU_XTAL_FREQ_RATIO 0x66c
+#define XTAL_ALP_PER_4ILP 0x00001fff
+#define XTAL_CTL_EN 0x80000000
+#define PMU_SLOW_CLK_PERIOD 0x6dc
+
+struct bcm53573_ilp {
+ struct clk_hw hw;
+ struct regmap *regmap;
+};
+
+static int bcm53573_ilp_enable(struct clk_hw *hw)
+{
+ struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
+
+ regmap_write(ilp->regmap, PMU_SLOW_CLK_PERIOD, 0x10199);
+ regmap_write(ilp->regmap, 0x674, 0x10000);
+
+ return 0;
+}
+
+static void bcm53573_ilp_disable(struct clk_hw *hw)
+{
+ struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
+
+ regmap_write(ilp->regmap, PMU_SLOW_CLK_PERIOD, 0);
+ regmap_write(ilp->regmap, 0x674, 0);
+}
+
+static unsigned long bcm53573_ilp_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
+ struct regmap *regmap = ilp->regmap;
+ u32 last_val, cur_val;
+ int sum = 0, num = 0, loop_num = 0;
+ int avg;
+
+ /* Enable measurement */
+ regmap_write(regmap, PMU_XTAL_FREQ_RATIO, XTAL_CTL_EN);
+
+ /* Read initial value */
+ regmap_read(regmap, PMU_XTAL_FREQ_RATIO, &last_val);
+ last_val &= XTAL_ALP_PER_4ILP;
+
+ /*
+ * At minimum we should loop for a bit to let hardware do the
+ * measurement. This isn't very accurate however, so for a better
+ * precision lets try getting 20 different values for and use average.
+ */
+ while (num < 20) {
+ regmap_read(regmap, PMU_XTAL_FREQ_RATIO, &cur_val);
+ cur_val &= XTAL_ALP_PER_4ILP;
+
+ if (cur_val != last_val) {
+ /* Got different value, use it */
+ sum += cur_val;
+ num++;
+ loop_num = 0;
+ last_val = cur_val;
+ } else if (++loop_num > 5000) {
+ /* Same value over and over, give up */
+ sum += cur_val;
+ num++;
+ break;
+ }
+
+ cpu_relax();
+ }
+
+ /* Disable measurement to save power */
+ regmap_write(regmap, PMU_XTAL_FREQ_RATIO, 0x0);
+
+ avg = sum / num;
+
+ return parent_rate * 4 / avg;
+}
+
+static const struct clk_ops bcm53573_ilp_clk_ops = {
+ .enable = bcm53573_ilp_enable,
+ .disable = bcm53573_ilp_disable,
+ .recalc_rate = bcm53573_ilp_recalc_rate,
+};
+
+static void bcm53573_ilp_init(struct device_node *np)
+{
+ struct bcm53573_ilp *ilp;
+ struct clk_init_data init = { 0 };
+ const char *parent_name;
+ int err;
+
+ ilp = kzalloc(sizeof(*ilp), GFP_KERNEL);
+ if (!ilp)
+ return;
+
+ parent_name = of_clk_get_parent_name(np, 0);
+ if (!parent_name) {
+ err = -ENOENT;
+ goto err_free_ilp;
+ }
+
+ ilp->regmap = syscon_node_to_regmap(of_get_parent(np));
+ if (IS_ERR(ilp->regmap)) {
+ err = PTR_ERR(ilp->regmap);
+ goto err_free_ilp;
+ }
+
+ init.name = np->name;
+ init.ops = &bcm53573_ilp_clk_ops;
+ init.parent_names = &parent_name;
+ init.num_parents = 1;
+
+ ilp->hw.init = &init;
+ err = clk_hw_register(NULL, &ilp->hw);
+ if (err)
+ goto err_free_ilp;
+
+ err = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &ilp->hw);
+ if (err)
+ goto err_clk_hw_unregister;
+
+ return;
+
+err_clk_hw_unregister:
+ clk_hw_unregister(&ilp->hw);
+err_free_ilp:
+ kfree(ilp);
+ pr_err("Failed to init ILP clock: %d\n", err);
+}
+
+/* We need it very early for arch code, before device model gets ready */
+CLK_OF_DECLARE(bcm53573_ilp_clk, "brcm,bcm53573-ilp", bcm53573_ilp_init);
--
2.9.3
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH V7] clk: bcm: Add driver for BCM53573 ILP clock
@ 2016-09-05 14:29 ` Rafał Miłecki
0 siblings, 0 replies; 50+ messages in thread
From: Rafał Miłecki @ 2016-09-05 14:29 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd
Cc: linux-clk, bcm-kernel-feedback-list, Rafał Miłecki,
Rob Herring, Mark Rutland, Jon Mason, Florian Fainelli,
Eric Anholt,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
From: Rafał Miłecki <rafal@milecki.pl>
This clock is present on BCM53573 devices (including BCM47189) that use
Cortex-A7. ILP is a part of PMU (Power Management Unit) multi-function
device so we use syscon (and regmap) for it.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Rebase on top of clk-next
Use ALP as parent clock
Improve comments
Switch from ioremap_nocache to ioremap
Check of_clk_add_provide result for error
V3: Drop #include <linux/moduleh>
Make ILP DT entry part of PMU
Describe ILP as subdevice of PMU in Documentation
V4: Use BCM53573 name as suggested by Jon and Ray. It seems "Northstar"
(even if used in some resources) should be used in relation to
Cortex-A9 devices only.
V5: Rename remaining "ns" references to "bcm53573", sorry, I sent V4 too
early.
V6: Drop #include <linux/clk.h>
Use "int" as type where it matches usage
Add cpu_relax() in the loop
Add disable callback
Use _hw_ functions for registering struct clk_hw (new API)
Thanks a lot Stephen!
V7: Use syscon and regmap (thanks Rob!)
---
.../bindings/clock/brcm,bcm53573-ilp.txt | 36 +++++
drivers/clk/bcm/Makefile | 1 +
drivers/clk/bcm/clk-bcm53573-ilp.c | 148 +++++++++++++++++++++
3 files changed, 185 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
create mode 100644 drivers/clk/bcm/clk-bcm53573-ilp.c
diff --git a/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
new file mode 100644
index 0000000..4554f70
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
@@ -0,0 +1,36 @@
+Broadcom BCM53573 ILP clock
+===========================
+
+This binding uses the common clock binding:
+ Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+This binding is used for ILP clock (sometimes referred as "slow clock")
+on Broadcom BCM53573 devices using Cortex-A7 CPU.
+
+ILP's rate has to be calculated on runtime and it depends on ALP clock
+which has to be referenced.
+
+This clock is part of PMU (Power Management Unit), a Broadcom's device
+handing power-related aspects. Its node must be sub-node of the PMU
+device.
+
+Required properties:
+- compatible: "brcm,bcm53573-ilp"
+- reg: iomem address range of PMU (Power Management Unit)
+- reg-names: "pmu", the only needed & supported reg right now
+- clocks: has to reference an ALP clock
+- #clock-cells: should be <0>
+
+Example:
+
+pmu@18012000 {
+ compatible = "simple-mfd", "syscon";
+ reg = <0x18012000 0x00001000>;
+
+ ilp: ilp@0 {
+ compatible = "brcm,bcm53573-ilp";
+ clocks = <&alp>;
+ #clock-cells = <0>;
+ clock-output-names = "ilp";
+ };
+};
diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile
index 1d79bd2..4b8c56d 100644
--- a/drivers/clk/bcm/Makefile
+++ b/drivers/clk/bcm/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_COMMON_CLK_IPROC) += clk-ns2.o
obj-$(CONFIG_ARCH_BCM_CYGNUS) += clk-cygnus.o
obj-$(CONFIG_ARCH_BCM_NSP) += clk-nsp.o
obj-$(CONFIG_ARCH_BCM_5301X) += clk-nsp.o
+obj-$(CONFIG_ARCH_BCM_53573) += clk-bcm53573-ilp.o
diff --git a/drivers/clk/bcm/clk-bcm53573-ilp.c b/drivers/clk/bcm/clk-bcm53573-ilp.c
new file mode 100644
index 0000000..1f4e30d
--- /dev/null
+++ b/drivers/clk/bcm/clk-bcm53573-ilp.c
@@ -0,0 +1,148 @@
+/*
+ * Copyright (C) 2016 Rafał Miłecki <rafal@milecki.pl>
+ *
+ * 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.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#define PMU_XTAL_FREQ_RATIO 0x66c
+#define XTAL_ALP_PER_4ILP 0x00001fff
+#define XTAL_CTL_EN 0x80000000
+#define PMU_SLOW_CLK_PERIOD 0x6dc
+
+struct bcm53573_ilp {
+ struct clk_hw hw;
+ struct regmap *regmap;
+};
+
+static int bcm53573_ilp_enable(struct clk_hw *hw)
+{
+ struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
+
+ regmap_write(ilp->regmap, PMU_SLOW_CLK_PERIOD, 0x10199);
+ regmap_write(ilp->regmap, 0x674, 0x10000);
+
+ return 0;
+}
+
+static void bcm53573_ilp_disable(struct clk_hw *hw)
+{
+ struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
+
+ regmap_write(ilp->regmap, PMU_SLOW_CLK_PERIOD, 0);
+ regmap_write(ilp->regmap, 0x674, 0);
+}
+
+static unsigned long bcm53573_ilp_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
+ struct regmap *regmap = ilp->regmap;
+ u32 last_val, cur_val;
+ int sum = 0, num = 0, loop_num = 0;
+ int avg;
+
+ /* Enable measurement */
+ regmap_write(regmap, PMU_XTAL_FREQ_RATIO, XTAL_CTL_EN);
+
+ /* Read initial value */
+ regmap_read(regmap, PMU_XTAL_FREQ_RATIO, &last_val);
+ last_val &= XTAL_ALP_PER_4ILP;
+
+ /*
+ * At minimum we should loop for a bit to let hardware do the
+ * measurement. This isn't very accurate however, so for a better
+ * precision lets try getting 20 different values for and use average.
+ */
+ while (num < 20) {
+ regmap_read(regmap, PMU_XTAL_FREQ_RATIO, &cur_val);
+ cur_val &= XTAL_ALP_PER_4ILP;
+
+ if (cur_val != last_val) {
+ /* Got different value, use it */
+ sum += cur_val;
+ num++;
+ loop_num = 0;
+ last_val = cur_val;
+ } else if (++loop_num > 5000) {
+ /* Same value over and over, give up */
+ sum += cur_val;
+ num++;
+ break;
+ }
+
+ cpu_relax();
+ }
+
+ /* Disable measurement to save power */
+ regmap_write(regmap, PMU_XTAL_FREQ_RATIO, 0x0);
+
+ avg = sum / num;
+
+ return parent_rate * 4 / avg;
+}
+
+static const struct clk_ops bcm53573_ilp_clk_ops = {
+ .enable = bcm53573_ilp_enable,
+ .disable = bcm53573_ilp_disable,
+ .recalc_rate = bcm53573_ilp_recalc_rate,
+};
+
+static void bcm53573_ilp_init(struct device_node *np)
+{
+ struct bcm53573_ilp *ilp;
+ struct clk_init_data init = { 0 };
+ const char *parent_name;
+ int err;
+
+ ilp = kzalloc(sizeof(*ilp), GFP_KERNEL);
+ if (!ilp)
+ return;
+
+ parent_name = of_clk_get_parent_name(np, 0);
+ if (!parent_name) {
+ err = -ENOENT;
+ goto err_free_ilp;
+ }
+
+ ilp->regmap = syscon_node_to_regmap(of_get_parent(np));
+ if (IS_ERR(ilp->regmap)) {
+ err = PTR_ERR(ilp->regmap);
+ goto err_free_ilp;
+ }
+
+ init.name = np->name;
+ init.ops = &bcm53573_ilp_clk_ops;
+ init.parent_names = &parent_name;
+ init.num_parents = 1;
+
+ ilp->hw.init = &init;
+ err = clk_hw_register(NULL, &ilp->hw);
+ if (err)
+ goto err_free_ilp;
+
+ err = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &ilp->hw);
+ if (err)
+ goto err_clk_hw_unregister;
+
+ return;
+
+err_clk_hw_unregister:
+ clk_hw_unregister(&ilp->hw);
+err_free_ilp:
+ kfree(ilp);
+ pr_err("Failed to init ILP clock: %d\n", err);
+}
+
+/* We need it very early for arch code, before device model gets ready */
+CLK_OF_DECLARE(bcm53573_ilp_clk, "brcm,bcm53573-ilp", bcm53573_ilp_init);
--
2.9.3
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH V7] clk: bcm: Add driver for BCM53573 ILP clock
2016-09-05 14:29 ` Rafał Miłecki
(?)
@ 2016-09-12 16:56 ` Rob Herring
-1 siblings, 0 replies; 50+ messages in thread
From: Rob Herring @ 2016-09-12 16:56 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Michael Turquette, Stephen Boyd, linux-clk,
bcm-kernel-feedback-list, Rafał Miłecki, Mark Rutland,
Jon Mason, Florian Fainelli, Eric Anholt,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On Mon, Sep 05, 2016 at 04:29:27PM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> This clock is present on BCM53573 devices (including BCM47189) that use
> Cortex-A7. ILP is a part of PMU (Power Management Unit) multi-function
> device so we use syscon (and regmap) for it.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V2: Rebase on top of clk-next
> Use ALP as parent clock
> Improve comments
> Switch from ioremap_nocache to ioremap
> Check of_clk_add_provide result for error
> V3: Drop #include <linux/moduleh>
> Make ILP DT entry part of PMU
> Describe ILP as subdevice of PMU in Documentation
> V4: Use BCM53573 name as suggested by Jon and Ray. It seems "Northstar"
> (even if used in some resources) should be used in relation to
> Cortex-A9 devices only.
> V5: Rename remaining "ns" references to "bcm53573", sorry, I sent V4 too
> early.
> V6: Drop #include <linux/clk.h>
> Use "int" as type where it matches usage
> Add cpu_relax() in the loop
> Add disable callback
> Use _hw_ functions for registering struct clk_hw (new API)
>
> Thanks a lot Stephen!
> V7: Use syscon and regmap (thanks Rob!)
> ---
> .../bindings/clock/brcm,bcm53573-ilp.txt | 36 +++++
> drivers/clk/bcm/Makefile | 1 +
> drivers/clk/bcm/clk-bcm53573-ilp.c | 148 +++++++++++++++++++++
> 3 files changed, 185 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
> create mode 100644 drivers/clk/bcm/clk-bcm53573-ilp.c
>
> diff --git a/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
> new file mode 100644
> index 0000000..4554f70
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
> @@ -0,0 +1,36 @@
> +Broadcom BCM53573 ILP clock
> +===========================
> +
> +This binding uses the common clock binding:
> + Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +This binding is used for ILP clock (sometimes referred as "slow clock")
> +on Broadcom BCM53573 devices using Cortex-A7 CPU.
> +
> +ILP's rate has to be calculated on runtime and it depends on ALP clock
> +which has to be referenced.
> +
> +This clock is part of PMU (Power Management Unit), a Broadcom's device
> +handing power-related aspects. Its node must be sub-node of the PMU
> +device.
> +
> +Required properties:
> +- compatible: "brcm,bcm53573-ilp"
> +- reg: iomem address range of PMU (Power Management Unit)
> +- reg-names: "pmu", the only needed & supported reg right now
There's no reg prop in the example.
> +- clocks: has to reference an ALP clock
> +- #clock-cells: should be <0>
> +
> +Example:
> +
> +pmu@18012000 {
> + compatible = "simple-mfd", "syscon";
> + reg = <0x18012000 0x00001000>;
> +
> + ilp: ilp@0 {
Drop the unit address if there is in fact no reg prop.
> + compatible = "brcm,bcm53573-ilp";
> + clocks = <&alp>;
> + #clock-cells = <0>;
> + clock-output-names = "ilp";
> + };
> +};
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH V8] clk: bcm: Add driver for BCM53573 ILP clock
2016-09-05 14:29 ` Rafał Miłecki
@ 2016-09-13 7:06 ` Rafał Miłecki
-1 siblings, 0 replies; 50+ messages in thread
From: Rafał Miłecki @ 2016-09-13 7:06 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd
Cc: linux-clk, bcm-kernel-feedback-list, Rafał Miłecki,
Rob Herring, Mark Rutland, Florian Fainelli, Jon Mason,
Eric Anholt, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
From: Rafał Miłecki <rafal@milecki.pl>
This clock is present on BCM53573 devices (including BCM47189) that use
Cortex-A7. ILP is a part of PMU (Power Management Unit) multi-function
device so we use syscon (and regmap) for it.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Rebase on top of clk-next
Use ALP as parent clock
Improve comments
Switch from ioremap_nocache to ioremap
Check of_clk_add_provide result for error
V3: Drop #include <linux/moduleh>
Make ILP DT entry part of PMU
Describe ILP as subdevice of PMU in Documentation
V4: Use BCM53573 name as suggested by Jon and Ray. It seems "Northstar"
(even if used in some resources) should be used in relation to
Cortex-A9 devices only.
V5: Rename remaining "ns" references to "bcm53573", sorry, I sent V4 too
early.
V6: Drop #include <linux/clk.h>
Use "int" as type where it matches usage
Add cpu_relax() in the loop
Add disable callback
Use _hw_ functions for registering struct clk_hw (new API)
Thanks a lot Stephen!
V7: Use syscon and regmap (thanks Rob!)
V8: Update Documentation (drop unused "reg", unit address)
---
.../bindings/clock/brcm,bcm53573-ilp.txt | 36 +++++
drivers/clk/bcm/Makefile | 1 +
drivers/clk/bcm/clk-bcm53573-ilp.c | 148 +++++++++++++++++++++
3 files changed, 185 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
create mode 100644 drivers/clk/bcm/clk-bcm53573-ilp.c
diff --git a/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
new file mode 100644
index 0000000..2ebb107
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
@@ -0,0 +1,36 @@
+Broadcom BCM53573 ILP clock
+===========================
+
+This binding uses the common clock binding:
+ Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+This binding is used for ILP clock (sometimes referred as "slow clock")
+on Broadcom BCM53573 devices using Cortex-A7 CPU.
+
+ILP's rate has to be calculated on runtime and it depends on ALP clock
+which has to be referenced.
+
+This clock is part of PMU (Power Management Unit), a Broadcom's device
+handing power-related aspects. Its node must be sub-node of the PMU
+device.
+
+Required properties:
+- compatible: "brcm,bcm53573-ilp"
+- clocks: has to reference an ALP clock
+- #clock-cells: should be <0>
+- clock-output-names: from common clock bindings, should contain clock
+ name
+
+Example:
+
+pmu@18012000 {
+ compatible = "simple-mfd", "syscon";
+ reg = <0x18012000 0x00001000>;
+
+ ilp {
+ compatible = "brcm,bcm53573-ilp";
+ clocks = <&alp>;
+ #clock-cells = <0>;
+ clock-output-names = "ilp";
+ };
+};
diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile
index 1d79bd2..4b8c56d 100644
--- a/drivers/clk/bcm/Makefile
+++ b/drivers/clk/bcm/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_COMMON_CLK_IPROC) += clk-ns2.o
obj-$(CONFIG_ARCH_BCM_CYGNUS) += clk-cygnus.o
obj-$(CONFIG_ARCH_BCM_NSP) += clk-nsp.o
obj-$(CONFIG_ARCH_BCM_5301X) += clk-nsp.o
+obj-$(CONFIG_ARCH_BCM_53573) += clk-bcm53573-ilp.o
diff --git a/drivers/clk/bcm/clk-bcm53573-ilp.c b/drivers/clk/bcm/clk-bcm53573-ilp.c
new file mode 100644
index 0000000..1f4e30d
--- /dev/null
+++ b/drivers/clk/bcm/clk-bcm53573-ilp.c
@@ -0,0 +1,148 @@
+/*
+ * Copyright (C) 2016 Rafał Miłecki <rafal@milecki.pl>
+ *
+ * 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.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#define PMU_XTAL_FREQ_RATIO 0x66c
+#define XTAL_ALP_PER_4ILP 0x00001fff
+#define XTAL_CTL_EN 0x80000000
+#define PMU_SLOW_CLK_PERIOD 0x6dc
+
+struct bcm53573_ilp {
+ struct clk_hw hw;
+ struct regmap *regmap;
+};
+
+static int bcm53573_ilp_enable(struct clk_hw *hw)
+{
+ struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
+
+ regmap_write(ilp->regmap, PMU_SLOW_CLK_PERIOD, 0x10199);
+ regmap_write(ilp->regmap, 0x674, 0x10000);
+
+ return 0;
+}
+
+static void bcm53573_ilp_disable(struct clk_hw *hw)
+{
+ struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
+
+ regmap_write(ilp->regmap, PMU_SLOW_CLK_PERIOD, 0);
+ regmap_write(ilp->regmap, 0x674, 0);
+}
+
+static unsigned long bcm53573_ilp_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
+ struct regmap *regmap = ilp->regmap;
+ u32 last_val, cur_val;
+ int sum = 0, num = 0, loop_num = 0;
+ int avg;
+
+ /* Enable measurement */
+ regmap_write(regmap, PMU_XTAL_FREQ_RATIO, XTAL_CTL_EN);
+
+ /* Read initial value */
+ regmap_read(regmap, PMU_XTAL_FREQ_RATIO, &last_val);
+ last_val &= XTAL_ALP_PER_4ILP;
+
+ /*
+ * At minimum we should loop for a bit to let hardware do the
+ * measurement. This isn't very accurate however, so for a better
+ * precision lets try getting 20 different values for and use average.
+ */
+ while (num < 20) {
+ regmap_read(regmap, PMU_XTAL_FREQ_RATIO, &cur_val);
+ cur_val &= XTAL_ALP_PER_4ILP;
+
+ if (cur_val != last_val) {
+ /* Got different value, use it */
+ sum += cur_val;
+ num++;
+ loop_num = 0;
+ last_val = cur_val;
+ } else if (++loop_num > 5000) {
+ /* Same value over and over, give up */
+ sum += cur_val;
+ num++;
+ break;
+ }
+
+ cpu_relax();
+ }
+
+ /* Disable measurement to save power */
+ regmap_write(regmap, PMU_XTAL_FREQ_RATIO, 0x0);
+
+ avg = sum / num;
+
+ return parent_rate * 4 / avg;
+}
+
+static const struct clk_ops bcm53573_ilp_clk_ops = {
+ .enable = bcm53573_ilp_enable,
+ .disable = bcm53573_ilp_disable,
+ .recalc_rate = bcm53573_ilp_recalc_rate,
+};
+
+static void bcm53573_ilp_init(struct device_node *np)
+{
+ struct bcm53573_ilp *ilp;
+ struct clk_init_data init = { 0 };
+ const char *parent_name;
+ int err;
+
+ ilp = kzalloc(sizeof(*ilp), GFP_KERNEL);
+ if (!ilp)
+ return;
+
+ parent_name = of_clk_get_parent_name(np, 0);
+ if (!parent_name) {
+ err = -ENOENT;
+ goto err_free_ilp;
+ }
+
+ ilp->regmap = syscon_node_to_regmap(of_get_parent(np));
+ if (IS_ERR(ilp->regmap)) {
+ err = PTR_ERR(ilp->regmap);
+ goto err_free_ilp;
+ }
+
+ init.name = np->name;
+ init.ops = &bcm53573_ilp_clk_ops;
+ init.parent_names = &parent_name;
+ init.num_parents = 1;
+
+ ilp->hw.init = &init;
+ err = clk_hw_register(NULL, &ilp->hw);
+ if (err)
+ goto err_free_ilp;
+
+ err = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &ilp->hw);
+ if (err)
+ goto err_clk_hw_unregister;
+
+ return;
+
+err_clk_hw_unregister:
+ clk_hw_unregister(&ilp->hw);
+err_free_ilp:
+ kfree(ilp);
+ pr_err("Failed to init ILP clock: %d\n", err);
+}
+
+/* We need it very early for arch code, before device model gets ready */
+CLK_OF_DECLARE(bcm53573_ilp_clk, "brcm,bcm53573-ilp", bcm53573_ilp_init);
--
2.9.3
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH V8] clk: bcm: Add driver for BCM53573 ILP clock
@ 2016-09-13 7:06 ` Rafał Miłecki
0 siblings, 0 replies; 50+ messages in thread
From: Rafał Miłecki @ 2016-09-13 7:06 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd
Cc: linux-clk, bcm-kernel-feedback-list, Rafał Miłecki,
Rob Herring, Mark Rutland, Florian Fainelli, Jon Mason,
Eric Anholt, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
From: Rafał Miłecki <rafal@milecki.pl>
This clock is present on BCM53573 devices (including BCM47189) that use
Cortex-A7. ILP is a part of PMU (Power Management Unit) multi-function
device so we use syscon (and regmap) for it.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Rebase on top of clk-next
Use ALP as parent clock
Improve comments
Switch from ioremap_nocache to ioremap
Check of_clk_add_provide result for error
V3: Drop #include <linux/moduleh>
Make ILP DT entry part of PMU
Describe ILP as subdevice of PMU in Documentation
V4: Use BCM53573 name as suggested by Jon and Ray. It seems "Northstar"
(even if used in some resources) should be used in relation to
Cortex-A9 devices only.
V5: Rename remaining "ns" references to "bcm53573", sorry, I sent V4 too
early.
V6: Drop #include <linux/clk.h>
Use "int" as type where it matches usage
Add cpu_relax() in the loop
Add disable callback
Use _hw_ functions for registering struct clk_hw (new API)
Thanks a lot Stephen!
V7: Use syscon and regmap (thanks Rob!)
V8: Update Documentation (drop unused "reg", unit address)
---
.../bindings/clock/brcm,bcm53573-ilp.txt | 36 +++++
drivers/clk/bcm/Makefile | 1 +
drivers/clk/bcm/clk-bcm53573-ilp.c | 148 +++++++++++++++++++++
3 files changed, 185 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
create mode 100644 drivers/clk/bcm/clk-bcm53573-ilp.c
diff --git a/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
new file mode 100644
index 0000000..2ebb107
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
@@ -0,0 +1,36 @@
+Broadcom BCM53573 ILP clock
+===========================
+
+This binding uses the common clock binding:
+ Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+This binding is used for ILP clock (sometimes referred as "slow clock")
+on Broadcom BCM53573 devices using Cortex-A7 CPU.
+
+ILP's rate has to be calculated on runtime and it depends on ALP clock
+which has to be referenced.
+
+This clock is part of PMU (Power Management Unit), a Broadcom's device
+handing power-related aspects. Its node must be sub-node of the PMU
+device.
+
+Required properties:
+- compatible: "brcm,bcm53573-ilp"
+- clocks: has to reference an ALP clock
+- #clock-cells: should be <0>
+- clock-output-names: from common clock bindings, should contain clock
+ name
+
+Example:
+
+pmu@18012000 {
+ compatible = "simple-mfd", "syscon";
+ reg = <0x18012000 0x00001000>;
+
+ ilp {
+ compatible = "brcm,bcm53573-ilp";
+ clocks = <&alp>;
+ #clock-cells = <0>;
+ clock-output-names = "ilp";
+ };
+};
diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile
index 1d79bd2..4b8c56d 100644
--- a/drivers/clk/bcm/Makefile
+++ b/drivers/clk/bcm/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_COMMON_CLK_IPROC) += clk-ns2.o
obj-$(CONFIG_ARCH_BCM_CYGNUS) += clk-cygnus.o
obj-$(CONFIG_ARCH_BCM_NSP) += clk-nsp.o
obj-$(CONFIG_ARCH_BCM_5301X) += clk-nsp.o
+obj-$(CONFIG_ARCH_BCM_53573) += clk-bcm53573-ilp.o
diff --git a/drivers/clk/bcm/clk-bcm53573-ilp.c b/drivers/clk/bcm/clk-bcm53573-ilp.c
new file mode 100644
index 0000000..1f4e30d
--- /dev/null
+++ b/drivers/clk/bcm/clk-bcm53573-ilp.c
@@ -0,0 +1,148 @@
+/*
+ * Copyright (C) 2016 Rafał Miłecki <rafal@milecki.pl>
+ *
+ * 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.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#define PMU_XTAL_FREQ_RATIO 0x66c
+#define XTAL_ALP_PER_4ILP 0x00001fff
+#define XTAL_CTL_EN 0x80000000
+#define PMU_SLOW_CLK_PERIOD 0x6dc
+
+struct bcm53573_ilp {
+ struct clk_hw hw;
+ struct regmap *regmap;
+};
+
+static int bcm53573_ilp_enable(struct clk_hw *hw)
+{
+ struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
+
+ regmap_write(ilp->regmap, PMU_SLOW_CLK_PERIOD, 0x10199);
+ regmap_write(ilp->regmap, 0x674, 0x10000);
+
+ return 0;
+}
+
+static void bcm53573_ilp_disable(struct clk_hw *hw)
+{
+ struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
+
+ regmap_write(ilp->regmap, PMU_SLOW_CLK_PERIOD, 0);
+ regmap_write(ilp->regmap, 0x674, 0);
+}
+
+static unsigned long bcm53573_ilp_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw);
+ struct regmap *regmap = ilp->regmap;
+ u32 last_val, cur_val;
+ int sum = 0, num = 0, loop_num = 0;
+ int avg;
+
+ /* Enable measurement */
+ regmap_write(regmap, PMU_XTAL_FREQ_RATIO, XTAL_CTL_EN);
+
+ /* Read initial value */
+ regmap_read(regmap, PMU_XTAL_FREQ_RATIO, &last_val);
+ last_val &= XTAL_ALP_PER_4ILP;
+
+ /*
+ * At minimum we should loop for a bit to let hardware do the
+ * measurement. This isn't very accurate however, so for a better
+ * precision lets try getting 20 different values for and use average.
+ */
+ while (num < 20) {
+ regmap_read(regmap, PMU_XTAL_FREQ_RATIO, &cur_val);
+ cur_val &= XTAL_ALP_PER_4ILP;
+
+ if (cur_val != last_val) {
+ /* Got different value, use it */
+ sum += cur_val;
+ num++;
+ loop_num = 0;
+ last_val = cur_val;
+ } else if (++loop_num > 5000) {
+ /* Same value over and over, give up */
+ sum += cur_val;
+ num++;
+ break;
+ }
+
+ cpu_relax();
+ }
+
+ /* Disable measurement to save power */
+ regmap_write(regmap, PMU_XTAL_FREQ_RATIO, 0x0);
+
+ avg = sum / num;
+
+ return parent_rate * 4 / avg;
+}
+
+static const struct clk_ops bcm53573_ilp_clk_ops = {
+ .enable = bcm53573_ilp_enable,
+ .disable = bcm53573_ilp_disable,
+ .recalc_rate = bcm53573_ilp_recalc_rate,
+};
+
+static void bcm53573_ilp_init(struct device_node *np)
+{
+ struct bcm53573_ilp *ilp;
+ struct clk_init_data init = { 0 };
+ const char *parent_name;
+ int err;
+
+ ilp = kzalloc(sizeof(*ilp), GFP_KERNEL);
+ if (!ilp)
+ return;
+
+ parent_name = of_clk_get_parent_name(np, 0);
+ if (!parent_name) {
+ err = -ENOENT;
+ goto err_free_ilp;
+ }
+
+ ilp->regmap = syscon_node_to_regmap(of_get_parent(np));
+ if (IS_ERR(ilp->regmap)) {
+ err = PTR_ERR(ilp->regmap);
+ goto err_free_ilp;
+ }
+
+ init.name = np->name;
+ init.ops = &bcm53573_ilp_clk_ops;
+ init.parent_names = &parent_name;
+ init.num_parents = 1;
+
+ ilp->hw.init = &init;
+ err = clk_hw_register(NULL, &ilp->hw);
+ if (err)
+ goto err_free_ilp;
+
+ err = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &ilp->hw);
+ if (err)
+ goto err_clk_hw_unregister;
+
+ return;
+
+err_clk_hw_unregister:
+ clk_hw_unregister(&ilp->hw);
+err_free_ilp:
+ kfree(ilp);
+ pr_err("Failed to init ILP clock: %d\n", err);
+}
+
+/* We need it very early for arch code, before device model gets ready */
+CLK_OF_DECLARE(bcm53573_ilp_clk, "brcm,bcm53573-ilp", bcm53573_ilp_init);
--
2.9.3
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH V8] clk: bcm: Add driver for BCM53573 ILP clock
@ 2016-09-14 23:44 ` Rob Herring
0 siblings, 0 replies; 50+ messages in thread
From: Rob Herring @ 2016-09-14 23:44 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Michael Turquette, Stephen Boyd, linux-clk,
bcm-kernel-feedback-list, Rafał Miłecki, Mark Rutland,
Florian Fainelli, Jon Mason, Eric Anholt, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On Tue, Sep 13, 2016 at 2:06 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> This clock is present on BCM53573 devices (including BCM47189) that use
> Cortex-A7. ILP is a part of PMU (Power Management Unit) multi-function
> device so we use syscon (and regmap) for it.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V2: Rebase on top of clk-next
> Use ALP as parent clock
> Improve comments
> Switch from ioremap_nocache to ioremap
> Check of_clk_add_provide result for error
> V3: Drop #include <linux/moduleh>
> Make ILP DT entry part of PMU
> Describe ILP as subdevice of PMU in Documentation
> V4: Use BCM53573 name as suggested by Jon and Ray. It seems "Northstar"
> (even if used in some resources) should be used in relation to
> Cortex-A9 devices only.
> V5: Rename remaining "ns" references to "bcm53573", sorry, I sent V4 too
> early.
> V6: Drop #include <linux/clk.h>
> Use "int" as type where it matches usage
> Add cpu_relax() in the loop
> Add disable callback
> Use _hw_ functions for registering struct clk_hw (new API)
>
> Thanks a lot Stephen!
> V7: Use syscon and regmap (thanks Rob!)
> V8: Update Documentation (drop unused "reg", unit address)
> ---
> .../bindings/clock/brcm,bcm53573-ilp.txt | 36 +++++
Acked-by: Rob Herring <robh@kernel.org>
> drivers/clk/bcm/Makefile | 1 +
> drivers/clk/bcm/clk-bcm53573-ilp.c | 148 +++++++++++++++++++++
> 3 files changed, 185 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
> create mode 100644 drivers/clk/bcm/clk-bcm53573-ilp.c
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V8] clk: bcm: Add driver for BCM53573 ILP clock
@ 2016-09-14 23:44 ` Rob Herring
0 siblings, 0 replies; 50+ messages in thread
From: Rob Herring @ 2016-09-14 23:44 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Michael Turquette, Stephen Boyd, linux-clk,
bcm-kernel-feedback-list, Rafał Miłecki, Mark Rutland,
Florian Fainelli, Jon Mason, Eric Anholt, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On Tue, Sep 13, 2016 at 2:06 AM, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com>=
wrote:
> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>
> This clock is present on BCM53573 devices (including BCM47189) that use
> Cortex-A7. ILP is a part of PMU (Power Management Unit) multi-function
> device so we use syscon (and regmap) for it.
>
> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
> ---
> V2: Rebase on top of clk-next
> Use ALP as parent clock
> Improve comments
> Switch from ioremap_nocache to ioremap
> Check of_clk_add_provide result for error
> V3: Drop #include <linux/moduleh>
> Make ILP DT entry part of PMU
> Describe ILP as subdevice of PMU in Documentation
> V4: Use BCM53573 name as suggested by Jon and Ray. It seems "Northstar"
> (even if used in some resources) should be used in relation to
> Cortex-A9 devices only.
> V5: Rename remaining "ns" references to "bcm53573", sorry, I sent V4 too
> early.
> V6: Drop #include <linux/clk.h>
> Use "int" as type where it matches usage
> Add cpu_relax() in the loop
> Add disable callback
> Use _hw_ functions for registering struct clk_hw (new API)
>
> Thanks a lot Stephen!
> V7: Use syscon and regmap (thanks Rob!)
> V8: Update Documentation (drop unused "reg", unit address)
> ---
> .../bindings/clock/brcm,bcm53573-ilp.txt | 36 +++++
Acked-by: Rob Herring <robh@kernel.org>
> drivers/clk/bcm/Makefile | 1 +
> drivers/clk/bcm/clk-bcm53573-ilp.c | 148 +++++++++++++++=
++++++
> 3 files changed, 185 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm53573=
-ilp.txt
> create mode 100644 drivers/clk/bcm/clk-bcm53573-ilp.c
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V8] clk: bcm: Add driver for BCM53573 ILP clock
@ 2016-09-14 23:44 ` Rob Herring
0 siblings, 0 replies; 50+ messages in thread
From: Rob Herring @ 2016-09-14 23:44 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Michael Turquette, Stephen Boyd, linux-clk,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
Rafał Miłecki, Mark Rutland, Florian Fainelli,
Jon Mason, Eric Anholt, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On Tue, Sep 13, 2016 at 2:06 AM, Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>
> This clock is present on BCM53573 devices (including BCM47189) that use
> Cortex-A7. ILP is a part of PMU (Power Management Unit) multi-function
> device so we use syscon (and regmap) for it.
>
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> ---
> V2: Rebase on top of clk-next
> Use ALP as parent clock
> Improve comments
> Switch from ioremap_nocache to ioremap
> Check of_clk_add_provide result for error
> V3: Drop #include <linux/moduleh>
> Make ILP DT entry part of PMU
> Describe ILP as subdevice of PMU in Documentation
> V4: Use BCM53573 name as suggested by Jon and Ray. It seems "Northstar"
> (even if used in some resources) should be used in relation to
> Cortex-A9 devices only.
> V5: Rename remaining "ns" references to "bcm53573", sorry, I sent V4 too
> early.
> V6: Drop #include <linux/clk.h>
> Use "int" as type where it matches usage
> Add cpu_relax() in the loop
> Add disable callback
> Use _hw_ functions for registering struct clk_hw (new API)
>
> Thanks a lot Stephen!
> V7: Use syscon and regmap (thanks Rob!)
> V8: Update Documentation (drop unused "reg", unit address)
> ---
> .../bindings/clock/brcm,bcm53573-ilp.txt | 36 +++++
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> drivers/clk/bcm/Makefile | 1 +
> drivers/clk/bcm/clk-bcm53573-ilp.c | 148 +++++++++++++++++++++
> 3 files changed, 185 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt
> create mode 100644 drivers/clk/bcm/clk-bcm53573-ilp.c
--
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 [flat|nested] 50+ messages in thread
* Re: [PATCH V8] clk: bcm: Add driver for BCM53573 ILP clock
@ 2016-09-16 23:23 ` Stephen Boyd
0 siblings, 0 replies; 50+ messages in thread
From: Stephen Boyd @ 2016-09-16 23:23 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Michael Turquette, linux-clk, bcm-kernel-feedback-list,
Rafał Miłecki, Rob Herring, Mark Rutland,
Florian Fainelli, Jon Mason, Eric Anholt, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On 09/13, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> This clock is present on BCM53573 devices (including BCM47189) that use
> Cortex-A7. ILP is a part of PMU (Power Management Unit) multi-function
> device so we use syscon (and regmap) for it.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
Applied to clk-next
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V8] clk: bcm: Add driver for BCM53573 ILP clock
@ 2016-09-16 23:23 ` Stephen Boyd
0 siblings, 0 replies; 50+ messages in thread
From: Stephen Boyd @ 2016-09-16 23:23 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Michael Turquette, linux-clk-u79uwXL29TY76Z2rM5mHXA,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
Rafał Miłecki, Rob Herring, Mark Rutland,
Florian Fainelli, Jon Mason, Eric Anholt, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On 09/13, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>
> This clock is present on BCM53573 devices (including BCM47189) that use
> Cortex-A7. ILP is a part of PMU (Power Management Unit) multi-function
> device so we use syscon (and regmap) for it.
>
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> ---
Applied to clk-next
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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 [flat|nested] 50+ messages in thread
* Re: [PATCH V8] clk: bcm: Add driver for BCM53573 ILP clock
@ 2016-09-17 19:20 ` Rafał Miłecki
0 siblings, 0 replies; 50+ messages in thread
From: Rafał Miłecki @ 2016-09-17 19:20 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, linux-clk, bcm-kernel-feedback-list,
Rafał Miłecki, Rob Herring, Mark Rutland,
Florian Fainelli, Jon Mason, Eric Anholt, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On 17 September 2016 at 01:23, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 09/13, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> This clock is present on BCM53573 devices (including BCM47189) that use
>> Cortex-A7. ILP is a part of PMU (Power Management Unit) multi-function
>> device so we use syscon (and regmap) for it.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>
> Applied to clk-next
Thank you and everyone who helped me getting this driver into a proper
shape. I appreciate your help!
--
Rafał
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V8] clk: bcm: Add driver for BCM53573 ILP clock
@ 2016-09-17 19:20 ` Rafał Miłecki
0 siblings, 0 replies; 50+ messages in thread
From: Rafał Miłecki @ 2016-09-17 19:20 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, linux-clk, bcm-kernel-feedback-list,
Rafał Miłecki, Rob Herring, Mark Rutland,
Florian Fainelli, Jon Mason, Eric Anholt, Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On 17 September 2016 at 01:23, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 09/13, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>
>> This clock is present on BCM53573 devices (including BCM47189) that use
>> Cortex-A7. ILP is a part of PMU (Power Management Unit) multi-function
>> device so we use syscon (and regmap) for it.
>>
>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>> ---
>
> Applied to clk-next
Thank you and everyone who helped me getting this driver into a proper
shape. I appreciate your help!
--=20
Rafa=C5=82
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH V8] clk: bcm: Add driver for BCM53573 ILP clock
@ 2016-09-17 19:20 ` Rafał Miłecki
0 siblings, 0 replies; 50+ messages in thread
From: Rafał Miłecki @ 2016-09-17 19:20 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, linux-clk-u79uwXL29TY76Z2rM5mHXA,
bcm-kernel-feedback-list, Rafał Miłecki, Rob Herring,
Mark Rutland, Florian Fainelli, Jon Mason, Eric Anholt,
Stephen Warren,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
On 17 September 2016 at 01:23, Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> On 09/13, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>
>> This clock is present on BCM53573 devices (including BCM47189) that use
>> Cortex-A7. ILP is a part of PMU (Power Management Unit) multi-function
>> device so we use syscon (and regmap) for it.
>>
>> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>> ---
>
> Applied to clk-next
Thank you and everyone who helped me getting this driver into a proper
shape. I appreciate your help!
--
Rafał
--
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 [flat|nested] 50+ messages in thread