Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1459342 > unrolled thread

[PATCH V3] clk: bcm: Add driver for Northstar ILP clock

Back to article view | Back to linux.kernel

Contents

  [PATCH V3] clk: bcm: Add driver for Northstar ILP clock Rafał Miłecki <zajec5@gmail.com> - 2016-08-10 20:10 +0200
    Re: [PATCH V3] clk: bcm: Add driver for Northstar ILP clock Jon Mason <jon.mason@broadcom.com> - 2016-08-10 20:20 +0200
      Re: [PATCH V3] clk: bcm: Add driver for Northstar ILP clock Jon Mason <jon.mason@broadcom.com> - 2016-08-10 20:30 +0200
        Re: [PATCH V3] clk: bcm: Add driver for Northstar ILP clock Rafał Miłecki <zajec5@gmail.com> - 2016-08-11 11:00 +0200
          Re: [PATCH V3] clk: bcm: Add driver for Northstar ILP clock Jon Mason <jon.mason@broadcom.com> - 2016-08-11 16:30 +0200
            Re: [PATCH V3] clk: bcm: Add driver for Northstar ILP clock Rafał Miłecki <zajec5@gmail.com> - 2016-08-12 08:30 +0200
      Re: [PATCH V3] clk: bcm: Add driver for Northstar ILP clock Ray Jui <ray.jui@broadcom.com> - 2016-08-10 22:40 +0200
      Re: [PATCH V3] clk: bcm: Add driver for Northstar ILP clock Rafał Miłecki <zajec5@gmail.com> - 2016-08-10 22:40 +0200
    [PATCH V4] clk: bcm: Add driver for BCM53573 ILP clock Rafał Miłecki <zajec5@gmail.com> - 2016-08-23 08:30 +0200
      [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock Rafał Miłecki <zajec5@gmail.com> - 2016-08-23 08:30 +0200
        Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock Rob Herring <robh@kernel.org> - 2016-08-23 22:20 +0200
        Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock Stephen Boyd <sboyd@codeaurora.org> - 2016-08-24 11:00 +0200

#1459342 — [PATCH V3] clk: bcm: Add driver for Northstar ILP clock

From: Rafał Miłecki <rafal@milecki.pl>

This clock is present on cheaper Northstar devices like BCM53573 or
BCM47189 using Corex-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
---
 .../devicetree/bindings/clock/brcm,ns-ilp.txt      |  40 ++++++
 drivers/clk/bcm/Makefile                           |   1 +
 drivers/clk/bcm/clk-ns-ilp.c                       | 146 +++++++++++++++++++++
 3 files changed, 187 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
 create mode 100644 drivers/clk/bcm/clk-ns-ilp.c

diff --git a/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
new file mode 100644
index 0000000..a18c73f
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
@@ -0,0 +1,40 @@
+Broadcom Northstar 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 Northstar devices using Corex-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,ns-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,ns-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..1389379 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_5301X)	+= clk-ns-ilp.o
diff --git a/drivers/clk/bcm/clk-ns-ilp.c b/drivers/clk/bcm/clk-ns-ilp.c
new file mode 100644
index 0000000..53b4a23
--- /dev/null
+++ b/drivers/clk/bcm/clk-ns-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 ns_ilp {
+	struct clk *clk;
+	struct clk_hw hw;
+	void __iomem *pmu;
+};
+
+static int ns_ilp_enable(struct clk_hw *hw)
+{
+	struct ns_ilp *ilp = container_of(hw, struct ns_ilp, hw);
+
+	writel(0x10199, ilp->pmu + PMU_SLOW_CLK_PERIOD);
+	writel(0x10000, ilp->pmu + 0x674);
+
+	return 0;
+}
+
+static unsigned long ns_ilp_recalc_rate(struct clk_hw *hw,
+					unsigned long parent_rate)
+{
+	struct ns_ilp *ilp = container_of(hw, struct ns_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 ns_ilp_clk_ops = {
+	.enable = ns_ilp_enable,
+	.recalc_rate = ns_ilp_recalc_rate,
+};
+
+static void ns_ilp_init(struct device_node *np)
+{
+	struct ns_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 = &ns_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(ns_ilp_clk, "brcm,ns-ilp", ns_ilp_init);
-- 
1.8.4.5

[toc] | [next] | [standalone]

#1459391

On Wed, Aug 10, 2016 at 8:05 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> This clock is present on cheaper Northstar devices like BCM53573 or
> BCM47189 using Corex-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
> ---
>  .../devicetree/bindings/clock/brcm,ns-ilp.txt      |  40 ++++++
>  drivers/clk/bcm/Makefile                           |   1 +
>  drivers/clk/bcm/clk-ns-ilp.c                       | 146 +++++++++++++++++++++
>  3 files changed, 187 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
>  create mode 100644 drivers/clk/bcm/clk-ns-ilp.c
>
> diff --git a/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
> new file mode 100644
> index 0000000..a18c73f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
> @@ -0,0 +1,40 @@
> +Broadcom Northstar 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 Northstar devices using Corex-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,ns-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>;

I don't see a corresponding DT entry in this patch, but 18012000 is
the PCI block.  So, I am concerned this will collide if used there.

I looked at the NS register reference guide, and I cannot find the
registers you are trying to reference.  Is this supposed to be
referencing the LCPLL clock registers in DMU?  If so, there is already
a driver in there for this (see drivers/clk/bcm/clk-nsp.c).

> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       ilp: ilp@0 {
> +               compatible = "brcm,ns-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..1389379 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_5301X)   += clk-ns-ilp.o
> diff --git a/drivers/clk/bcm/clk-ns-ilp.c b/drivers/clk/bcm/clk-ns-ilp.c
> new file mode 100644
> index 0000000..53b4a23
> --- /dev/null
> +++ b/drivers/clk/bcm/clk-ns-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 ns_ilp {
> +       struct clk *clk;
> +       struct clk_hw hw;
> +       void __iomem *pmu;
> +};
> +
> +static int ns_ilp_enable(struct clk_hw *hw)
> +{
> +       struct ns_ilp *ilp = container_of(hw, struct ns_ilp, hw);
> +
> +       writel(0x10199, ilp->pmu + PMU_SLOW_CLK_PERIOD);
> +       writel(0x10000, ilp->pmu + 0x674);
> +
> +       return 0;
> +}
> +
> +static unsigned long ns_ilp_recalc_rate(struct clk_hw *hw,
> +                                       unsigned long parent_rate)
> +{
> +       struct ns_ilp *ilp = container_of(hw, struct ns_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 ns_ilp_clk_ops = {
> +       .enable = ns_ilp_enable,
> +       .recalc_rate = ns_ilp_recalc_rate,
> +};
> +
> +static void ns_ilp_init(struct device_node *np)
> +{
> +       struct ns_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 = &ns_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(ns_ilp_clk, "brcm,ns-ilp", ns_ilp_init);
> --
> 1.8.4.5
>

[toc] | [prev] | [next] | [standalone]

#1459423

On Wed, Aug 10, 2016 at 1:44 PM, Ray Jui <ray.jui@broadcom.com> wrote:
>
>
> On 8/10/2016 10:28 AM, Rafał Miłecki wrote:
>>
>> On 10 August 2016 at 19:22, Jon Mason <jon.mason@broadcom.com> wrote:
>>>
>>> On Wed, Aug 10, 2016 at 8:05 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
>>>> b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
>>>> new file mode 100644
>>>> index 0000000..a18c73f
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
>>>> @@ -0,0 +1,40 @@
>>>> +Broadcom Northstar 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 Northstar devices using Corex-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,ns-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>;
>>>
>>>
>>> I don't see a corresponding DT entry in this patch, but 18012000 is
>>> the PCI block.  So, I am concerned this will collide if used there.
>>>
>>> I looked at the NS register reference guide, and I cannot find the
>>> registers you are trying to reference.  Is this supposed to be
>>> referencing the LCPLL clock registers in DMU?  If so, there is already
>>> a driver in there for this (see drivers/clk/bcm/clk-nsp.c).
>>
>>
>> This patch is for BCM53573 family, not BCM4708 family you are looking at.
>>
>> Found chip with id 53573, rev 0x02 and package 0x01
>> Core 0 found: ChipCommon (manuf 0x4BF, id 0x800, rev 0x36, class 0x0)
>> Core 1 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x38, class 0x0)
>> Core 2 found: PCIe Gen 2 (manuf 0x4BF, id 0x501, rev 0x05, class 0x0)
>> Core 3 found: ARM CA7 (manuf 0x4BF, id 0x847, rev 0x00, class 0x0)
>> Core 4 found: USB 2.0 Host (manuf 0x4BF, id 0x819, rev 0x05, class 0x0)
>> Core 5 found: GBit MAC (manuf 0x4BF, id 0x82D, rev 0x08, class 0x0)
>> Core 6 found: I2S (manuf 0x4BF, id 0x834, rev 0x06, class 0x0)
>> Core 7 found: CNDS DDR2/3 memory controller (manuf 0x4BF, id 0x846,
>> rev 0x00, class 0x0)
>> Core 8 found: NAND flash controller (manuf 0x4BF, id 0x509, rev 0x01,
>> class 0x0)
>> Core 9 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x38, class 0x0)
>> Core 10 found: GBit MAC (manuf 0x4BF, id 0x82D, rev 0x08, class 0x0)
>> Core 11 found: I2S (manuf 0x4BF, id 0x834, rev 0x06, class 0x0)
>> Core 12 found: GCI (manuf 0x4BF, id 0x840, rev 0x08, class 0x0)
>> Core 13 found: PMU (manuf 0x4BF, id 0x827, rev 0x1C, class 0x0)
>>
>
> Out of curiosity, I searched the datasheet and found this is a wireless
> router SoC done by the WLAN team. It happens to share some peripherals with
> other iProc based SoCs.
>
> I cannot find a code name for this SoC from our internal documents. I guess
> that name "Northstar" used here has confused both Jon and me.
>
> Thanks,
>
> Ray

Ray is right.  I just spoke to one of the people here with knowledge
of the HW, and this is not related at all to the 4708/9/5301X.  It MAY
have some of the same peripherals, but the core is different (Cortex
A7 instead of A9).

I think we are best off to change the name and turn this into a
separate device tree, driver base, etc.  I wasn't able to get a code
name, so perhaps simply call it "BCM53573".

Thanks,
Jon

[toc] | [prev] | [next] | [standalone]

#1460287

On 10 August 2016 at 20:21, Jon Mason <jon.mason@broadcom.com> wrote:
> On Wed, Aug 10, 2016 at 1:44 PM, Ray Jui <ray.jui@broadcom.com> wrote:
>> On 8/10/2016 10:28 AM, Rafał Miłecki wrote:
>>>
>>> On 10 August 2016 at 19:22, Jon Mason <jon.mason@broadcom.com> wrote:
>>>>
>>>> On Wed, Aug 10, 2016 at 8:05 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
>>>>> b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
>>>>> new file mode 100644
>>>>> index 0000000..a18c73f
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
>>>>> @@ -0,0 +1,40 @@
>>>>> +Broadcom Northstar 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 Northstar devices using Corex-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,ns-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>;
>>>>
>>>>
>>>> I don't see a corresponding DT entry in this patch, but 18012000 is
>>>> the PCI block.  So, I am concerned this will collide if used there.
>>>>
>>>> I looked at the NS register reference guide, and I cannot find the
>>>> registers you are trying to reference.  Is this supposed to be
>>>> referencing the LCPLL clock registers in DMU?  If so, there is already
>>>> a driver in there for this (see drivers/clk/bcm/clk-nsp.c).
>>>
>>>
>>> This patch is for BCM53573 family, not BCM4708 family you are looking at.
>>>
>>> Found chip with id 53573, rev 0x02 and package 0x01
>>> Core 0 found: ChipCommon (manuf 0x4BF, id 0x800, rev 0x36, class 0x0)
>>> Core 1 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x38, class 0x0)
>>> Core 2 found: PCIe Gen 2 (manuf 0x4BF, id 0x501, rev 0x05, class 0x0)
>>> Core 3 found: ARM CA7 (manuf 0x4BF, id 0x847, rev 0x00, class 0x0)
>>> Core 4 found: USB 2.0 Host (manuf 0x4BF, id 0x819, rev 0x05, class 0x0)
>>> Core 5 found: GBit MAC (manuf 0x4BF, id 0x82D, rev 0x08, class 0x0)
>>> Core 6 found: I2S (manuf 0x4BF, id 0x834, rev 0x06, class 0x0)
>>> Core 7 found: CNDS DDR2/3 memory controller (manuf 0x4BF, id 0x846,
>>> rev 0x00, class 0x0)
>>> Core 8 found: NAND flash controller (manuf 0x4BF, id 0x509, rev 0x01,
>>> class 0x0)
>>> Core 9 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x38, class 0x0)
>>> Core 10 found: GBit MAC (manuf 0x4BF, id 0x82D, rev 0x08, class 0x0)
>>> Core 11 found: I2S (manuf 0x4BF, id 0x834, rev 0x06, class 0x0)
>>> Core 12 found: GCI (manuf 0x4BF, id 0x840, rev 0x08, class 0x0)
>>> Core 13 found: PMU (manuf 0x4BF, id 0x827, rev 0x1C, class 0x0)
>>>
>>
>> Out of curiosity, I searched the datasheet and found this is a wireless
>> router SoC done by the WLAN team. It happens to share some peripherals with
>> other iProc based SoCs.
>>
>> I cannot find a code name for this SoC from our internal documents. I guess
>> that name "Northstar" used here has confused both Jon and me.
>
> Ray is right.  I just spoke to one of the people here with knowledge
> of the HW, and this is not related at all to the 4708/9/5301X.  It MAY
> have some of the same peripherals, but the core is different (Cortex
> A7 instead of A9).
>
> I think we are best off to change the name and turn this into a
> separate device tree, driver base, etc.  I wasn't able to get a code
> name, so perhaps simply call it "BCM53573".

Yes, I said clearly it uses Corex-A7 in the commit message and
Documentation entry.

Florian already shared his doubts about BCM53573 belonging to the
Northstar, but I found out [1] that your (Broadcom's) SDK treats it as
Northstar device:

Asus RT-AC1200G+

# cat /proc/cpuinfo
Processor : ARMv7 Processor rev 5 (v7l)
BogoMIPS : 1795.68
Features : swp half thumb fastmult edsp
CPU implementer : 0x41
CPU architecture: 7
CPU variant : 0x0
CPU part : 0xc07
CPU revision : 5

Hardware : Northstar Prototype
Revision : 0000
Serial : 0000000000000000


It seems Broadcom's WLAN team claims it's a Northstar and you claim
it's not. Could you discuss this internally and let us know, please?

[1] https://lkml.org/lkml/2016/7/29/345

-- 
Rafał

[toc] | [prev] | [next] | [standalone]

#1460570

On Thu, Aug 11, 2016 at 4:49 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
> On 10 August 2016 at 20:21, Jon Mason <jon.mason@broadcom.com> wrote:
>> On Wed, Aug 10, 2016 at 1:44 PM, Ray Jui <ray.jui@broadcom.com> wrote:
>>> On 8/10/2016 10:28 AM, Rafał Miłecki wrote:
>>>>
>>>> On 10 August 2016 at 19:22, Jon Mason <jon.mason@broadcom.com> wrote:
>>>>>
>>>>> On Wed, Aug 10, 2016 at 8:05 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
>>>>>> b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
>>>>>> new file mode 100644
>>>>>> index 0000000..a18c73f
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
>>>>>> @@ -0,0 +1,40 @@
>>>>>> +Broadcom Northstar 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 Northstar devices using Corex-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,ns-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>;
>>>>>
>>>>>
>>>>> I don't see a corresponding DT entry in this patch, but 18012000 is
>>>>> the PCI block.  So, I am concerned this will collide if used there.
>>>>>
>>>>> I looked at the NS register reference guide, and I cannot find the
>>>>> registers you are trying to reference.  Is this supposed to be
>>>>> referencing the LCPLL clock registers in DMU?  If so, there is already
>>>>> a driver in there for this (see drivers/clk/bcm/clk-nsp.c).
>>>>
>>>>
>>>> This patch is for BCM53573 family, not BCM4708 family you are looking at.
>>>>
>>>> Found chip with id 53573, rev 0x02 and package 0x01
>>>> Core 0 found: ChipCommon (manuf 0x4BF, id 0x800, rev 0x36, class 0x0)
>>>> Core 1 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x38, class 0x0)
>>>> Core 2 found: PCIe Gen 2 (manuf 0x4BF, id 0x501, rev 0x05, class 0x0)
>>>> Core 3 found: ARM CA7 (manuf 0x4BF, id 0x847, rev 0x00, class 0x0)
>>>> Core 4 found: USB 2.0 Host (manuf 0x4BF, id 0x819, rev 0x05, class 0x0)
>>>> Core 5 found: GBit MAC (manuf 0x4BF, id 0x82D, rev 0x08, class 0x0)
>>>> Core 6 found: I2S (manuf 0x4BF, id 0x834, rev 0x06, class 0x0)
>>>> Core 7 found: CNDS DDR2/3 memory controller (manuf 0x4BF, id 0x846,
>>>> rev 0x00, class 0x0)
>>>> Core 8 found: NAND flash controller (manuf 0x4BF, id 0x509, rev 0x01,
>>>> class 0x0)
>>>> Core 9 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x38, class 0x0)
>>>> Core 10 found: GBit MAC (manuf 0x4BF, id 0x82D, rev 0x08, class 0x0)
>>>> Core 11 found: I2S (manuf 0x4BF, id 0x834, rev 0x06, class 0x0)
>>>> Core 12 found: GCI (manuf 0x4BF, id 0x840, rev 0x08, class 0x0)
>>>> Core 13 found: PMU (manuf 0x4BF, id 0x827, rev 0x1C, class 0x0)
>>>>
>>>
>>> Out of curiosity, I searched the datasheet and found this is a wireless
>>> router SoC done by the WLAN team. It happens to share some peripherals with
>>> other iProc based SoCs.
>>>
>>> I cannot find a code name for this SoC from our internal documents. I guess
>>> that name "Northstar" used here has confused both Jon and me.
>>
>> Ray is right.  I just spoke to one of the people here with knowledge
>> of the HW, and this is not related at all to the 4708/9/5301X.  It MAY
>> have some of the same peripherals, but the core is different (Cortex
>> A7 instead of A9).
>>
>> I think we are best off to change the name and turn this into a
>> separate device tree, driver base, etc.  I wasn't able to get a code
>> name, so perhaps simply call it "BCM53573".
>
> Yes, I said clearly it uses Corex-A7 in the commit message and
> Documentation entry.
>
> Florian already shared his doubts about BCM53573 belonging to the
> Northstar, but I found out [1] that your (Broadcom's) SDK treats it as
> Northstar device:
>
> Asus RT-AC1200G+
>
> # cat /proc/cpuinfo
> Processor : ARMv7 Processor rev 5 (v7l)
> BogoMIPS : 1795.68
> Features : swp half thumb fastmult edsp
> CPU implementer : 0x41
> CPU architecture: 7
> CPU variant : 0x0
> CPU part : 0xc07
> CPU revision : 5
>
> Hardware : Northstar Prototype
> Revision : 0000
> Serial : 0000000000000000
>
>
> It seems Broadcom's WLAN team claims it's a Northstar and you claim
> it's not. Could you discuss this internally and let us know, please?

I discussed it with a member of the WLAN team prior to my previous
email, which is why I made the previous statement.  It is a completely
different SoC than the 5301x family.  It appears that someone in
Marketing decided to reuse the name without discussing it with anyone
in WLAN SW.  It is a completely different family of products than the
iProc Northstar.  If we want to use the Northstar name, then perhaps
we could call it something like "WLAN Northstar".  Either way, it
needs to have a different mach-bcm entry.

BTW, neither Ray, nor I, (and not even Florian) are in the WLAN
Business unit.  So, the best that we can do is to ask around
internally and try to provide as much help as possible.

Thanks,
Jon

>
> [1] https://lkml.org/lkml/2016/7/29/345
>
> --
> Rafał

[toc] | [prev] | [next] | [standalone]

#1460965

On 11 August 2016 at 16:25, Jon Mason <jon.mason@broadcom.com> wrote:
> On Thu, Aug 11, 2016 at 4:49 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
>> On 10 August 2016 at 20:21, Jon Mason <jon.mason@broadcom.com> wrote:
>>> On Wed, Aug 10, 2016 at 1:44 PM, Ray Jui <ray.jui@broadcom.com> wrote:
>>>> On 8/10/2016 10:28 AM, Rafał Miłecki wrote:
>>>>>
>>>>> On 10 August 2016 at 19:22, Jon Mason <jon.mason@broadcom.com> wrote:
>>>>>>
>>>>>> On Wed, Aug 10, 2016 at 8:05 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
>>>>>>> b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
>>>>>>> new file mode 100644
>>>>>>> index 0000000..a18c73f
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
>>>>>>> @@ -0,0 +1,40 @@
>>>>>>> +Broadcom Northstar 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 Northstar devices using Corex-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,ns-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>;
>>>>>>
>>>>>>
>>>>>> I don't see a corresponding DT entry in this patch, but 18012000 is
>>>>>> the PCI block.  So, I am concerned this will collide if used there.
>>>>>>
>>>>>> I looked at the NS register reference guide, and I cannot find the
>>>>>> registers you are trying to reference.  Is this supposed to be
>>>>>> referencing the LCPLL clock registers in DMU?  If so, there is already
>>>>>> a driver in there for this (see drivers/clk/bcm/clk-nsp.c).
>>>>>
>>>>>
>>>>> This patch is for BCM53573 family, not BCM4708 family you are looking at.
>>>>>
>>>>> Found chip with id 53573, rev 0x02 and package 0x01
>>>>> Core 0 found: ChipCommon (manuf 0x4BF, id 0x800, rev 0x36, class 0x0)
>>>>> Core 1 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x38, class 0x0)
>>>>> Core 2 found: PCIe Gen 2 (manuf 0x4BF, id 0x501, rev 0x05, class 0x0)
>>>>> Core 3 found: ARM CA7 (manuf 0x4BF, id 0x847, rev 0x00, class 0x0)
>>>>> Core 4 found: USB 2.0 Host (manuf 0x4BF, id 0x819, rev 0x05, class 0x0)
>>>>> Core 5 found: GBit MAC (manuf 0x4BF, id 0x82D, rev 0x08, class 0x0)
>>>>> Core 6 found: I2S (manuf 0x4BF, id 0x834, rev 0x06, class 0x0)
>>>>> Core 7 found: CNDS DDR2/3 memory controller (manuf 0x4BF, id 0x846,
>>>>> rev 0x00, class 0x0)
>>>>> Core 8 found: NAND flash controller (manuf 0x4BF, id 0x509, rev 0x01,
>>>>> class 0x0)
>>>>> Core 9 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x38, class 0x0)
>>>>> Core 10 found: GBit MAC (manuf 0x4BF, id 0x82D, rev 0x08, class 0x0)
>>>>> Core 11 found: I2S (manuf 0x4BF, id 0x834, rev 0x06, class 0x0)
>>>>> Core 12 found: GCI (manuf 0x4BF, id 0x840, rev 0x08, class 0x0)
>>>>> Core 13 found: PMU (manuf 0x4BF, id 0x827, rev 0x1C, class 0x0)
>>>>>
>>>>
>>>> Out of curiosity, I searched the datasheet and found this is a wireless
>>>> router SoC done by the WLAN team. It happens to share some peripherals with
>>>> other iProc based SoCs.
>>>>
>>>> I cannot find a code name for this SoC from our internal documents. I guess
>>>> that name "Northstar" used here has confused both Jon and me.
>>>
>>> Ray is right.  I just spoke to one of the people here with knowledge
>>> of the HW, and this is not related at all to the 4708/9/5301X.  It MAY
>>> have some of the same peripherals, but the core is different (Cortex
>>> A7 instead of A9).
>>>
>>> I think we are best off to change the name and turn this into a
>>> separate device tree, driver base, etc.  I wasn't able to get a code
>>> name, so perhaps simply call it "BCM53573".
>>
>> Yes, I said clearly it uses Corex-A7 in the commit message and
>> Documentation entry.
>>
>> Florian already shared his doubts about BCM53573 belonging to the
>> Northstar, but I found out [1] that your (Broadcom's) SDK treats it as
>> Northstar device:
>>
>> Asus RT-AC1200G+
>>
>> # cat /proc/cpuinfo
>> Processor : ARMv7 Processor rev 5 (v7l)
>> BogoMIPS : 1795.68
>> Features : swp half thumb fastmult edsp
>> CPU implementer : 0x41
>> CPU architecture: 7
>> CPU variant : 0x0
>> CPU part : 0xc07
>> CPU revision : 5
>>
>> Hardware : Northstar Prototype
>> Revision : 0000
>> Serial : 0000000000000000
>>
>>
>> It seems Broadcom's WLAN team claims it's a Northstar and you claim
>> it's not. Could you discuss this internally and let us know, please?
>
> I discussed it with a member of the WLAN team prior to my previous
> email, which is why I made the previous statement.  It is a completely
> different SoC than the 5301x family.  It appears that someone in
> Marketing decided to reuse the name without discussing it with anyone
> in WLAN SW.  It is a completely different family of products than the
> iProc Northstar.  If we want to use the Northstar name, then perhaps
> we could call it something like "WLAN Northstar".  Either way, it
> needs to have a different mach-bcm entry.

Sorry, it wasn't clear to me initially, thanks for clarification. I'll
resend using different prefix.

-- 
Rafał

[toc] | [prev] | [next] | [standalone]

#1459867

On 8/10/2016 10:28 AM, Rafał Miłecki wrote:
> On 10 August 2016 at 19:22, Jon Mason <jon.mason@broadcom.com> wrote:
>> On Wed, Aug 10, 2016 at 8:05 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
>>> diff --git a/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
>>> new file mode 100644
>>> index 0000000..a18c73f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
>>> @@ -0,0 +1,40 @@
>>> +Broadcom Northstar 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 Northstar devices using Corex-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,ns-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>;
>>
>> I don't see a corresponding DT entry in this patch, but 18012000 is
>> the PCI block.  So, I am concerned this will collide if used there.
>>
>> I looked at the NS register reference guide, and I cannot find the
>> registers you are trying to reference.  Is this supposed to be
>> referencing the LCPLL clock registers in DMU?  If so, there is already
>> a driver in there for this (see drivers/clk/bcm/clk-nsp.c).
>
> This patch is for BCM53573 family, not BCM4708 family you are looking at.
>
> Found chip with id 53573, rev 0x02 and package 0x01
> Core 0 found: ChipCommon (manuf 0x4BF, id 0x800, rev 0x36, class 0x0)
> Core 1 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x38, class 0x0)
> Core 2 found: PCIe Gen 2 (manuf 0x4BF, id 0x501, rev 0x05, class 0x0)
> Core 3 found: ARM CA7 (manuf 0x4BF, id 0x847, rev 0x00, class 0x0)
> Core 4 found: USB 2.0 Host (manuf 0x4BF, id 0x819, rev 0x05, class 0x0)
> Core 5 found: GBit MAC (manuf 0x4BF, id 0x82D, rev 0x08, class 0x0)
> Core 6 found: I2S (manuf 0x4BF, id 0x834, rev 0x06, class 0x0)
> Core 7 found: CNDS DDR2/3 memory controller (manuf 0x4BF, id 0x846,
> rev 0x00, class 0x0)
> Core 8 found: NAND flash controller (manuf 0x4BF, id 0x509, rev 0x01, class 0x0)
> Core 9 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x38, class 0x0)
> Core 10 found: GBit MAC (manuf 0x4BF, id 0x82D, rev 0x08, class 0x0)
> Core 11 found: I2S (manuf 0x4BF, id 0x834, rev 0x06, class 0x0)
> Core 12 found: GCI (manuf 0x4BF, id 0x840, rev 0x08, class 0x0)
> Core 13 found: PMU (manuf 0x4BF, id 0x827, rev 0x1C, class 0x0)
>

Out of curiosity, I searched the datasheet and found this is a wireless 
router SoC done by the WLAN team. It happens to share some peripherals 
with other iProc based SoCs.

I cannot find a code name for this SoC from our internal documents. I 
guess that name "Northstar" used here has confused both Jon and me.

Thanks,

Ray

[toc] | [prev] | [next] | [standalone]

#1459878

On 10 August 2016 at 19:22, Jon Mason <jon.mason@broadcom.com> wrote:
> On Wed, Aug 10, 2016 at 8:05 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
>> diff --git a/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
>> new file mode 100644
>> index 0000000..a18c73f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt
>> @@ -0,0 +1,40 @@
>> +Broadcom Northstar 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 Northstar devices using Corex-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,ns-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>;
>
> I don't see a corresponding DT entry in this patch, but 18012000 is
> the PCI block.  So, I am concerned this will collide if used there.
>
> I looked at the NS register reference guide, and I cannot find the
> registers you are trying to reference.  Is this supposed to be
> referencing the LCPLL clock registers in DMU?  If so, there is already
> a driver in there for this (see drivers/clk/bcm/clk-nsp.c).

This patch is for BCM53573 family, not BCM4708 family you are looking at.

Found chip with id 53573, rev 0x02 and package 0x01
Core 0 found: ChipCommon (manuf 0x4BF, id 0x800, rev 0x36, class 0x0)
Core 1 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x38, class 0x0)
Core 2 found: PCIe Gen 2 (manuf 0x4BF, id 0x501, rev 0x05, class 0x0)
Core 3 found: ARM CA7 (manuf 0x4BF, id 0x847, rev 0x00, class 0x0)
Core 4 found: USB 2.0 Host (manuf 0x4BF, id 0x819, rev 0x05, class 0x0)
Core 5 found: GBit MAC (manuf 0x4BF, id 0x82D, rev 0x08, class 0x0)
Core 6 found: I2S (manuf 0x4BF, id 0x834, rev 0x06, class 0x0)
Core 7 found: CNDS DDR2/3 memory controller (manuf 0x4BF, id 0x846,
rev 0x00, class 0x0)
Core 8 found: NAND flash controller (manuf 0x4BF, id 0x509, rev 0x01, class 0x0)
Core 9 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x38, class 0x0)
Core 10 found: GBit MAC (manuf 0x4BF, id 0x82D, rev 0x08, class 0x0)
Core 11 found: I2S (manuf 0x4BF, id 0x834, rev 0x06, class 0x0)
Core 12 found: GCI (manuf 0x4BF, id 0x840, rev 0x08, class 0x0)
Core 13 found: PMU (manuf 0x4BF, id 0x827, rev 0x1C, class 0x0)

[toc] | [prev] | [next] | [standalone]

#1468314 — [PATCH V4] clk: bcm: Add driver for BCM53573 ILP clock

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.
---
 .../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..73f7d2b
--- /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,ns-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,ns-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..53b4a23
--- /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 ns_ilp {
+	struct clk *clk;
+	struct clk_hw hw;
+	void __iomem *pmu;
+};
+
+static int ns_ilp_enable(struct clk_hw *hw)
+{
+	struct ns_ilp *ilp = container_of(hw, struct ns_ilp, hw);
+
+	writel(0x10199, ilp->pmu + PMU_SLOW_CLK_PERIOD);
+	writel(0x10000, ilp->pmu + 0x674);
+
+	return 0;
+}
+
+static unsigned long ns_ilp_recalc_rate(struct clk_hw *hw,
+					unsigned long parent_rate)
+{
+	struct ns_ilp *ilp = container_of(hw, struct ns_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 ns_ilp_clk_ops = {
+	.enable = ns_ilp_enable,
+	.recalc_rate = ns_ilp_recalc_rate,
+};
+
+static void ns_ilp_init(struct device_node *np)
+{
+	struct ns_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 = &ns_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(ns_ilp_clk, "brcm,ns-ilp", ns_ilp_init);
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1468317 — [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock

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

[toc] | [prev] | [next] | [standalone]

#1468817 — Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock

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

[toc] | [prev] | [next] | [standalone]

#1469234 — Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web