[PATCH 1/3] ARM: dts: bcm5301x: Add TWD WD Support to DT

[PATCH 1/3] ARM: dts: bcm5301x: Add TWD WD Support to DT

[Jon Mason]

Add support for the ARM TWD Watchdog to the bcm5301x device tree.  The
ARM TWD timer allocated the register space for the WDT, so this patch
necessitated shrinking that.  Also, the GIC masks were added for these.

Signed-off-by: Jon Mason <jonmason-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 arch/arm/boot/dts/bcm5301x.dtsi | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
index 65a1309..c31fafe 100644
--- a/arch/arm/boot/dts/bcm5301x.dtsi
+++ b/arch/arm/boot/dts/bcm5301x.dtsi
@@ -66,10 +66,19 @@
 			clocks = <&periph_clk>;
 		};
 
-		local-timer@20600 {
+		twd-timer@20600 {
 			compatible = "arm,cortex-a9-twd-timer";
-			reg = <0x20600 0x100>;
-			interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>;
+			reg = <0x20600 0x20>;
+			interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) |
+						  IRQ_TYPE_LEVEL_HIGH)>;
+			clocks = <&periph_clk>;
+		};
+
+		twd-watchdog@20620 {
+			compatible = "arm,cortex-a9-twd-wdt";
+			reg = <0x20620 0x20>;
+			interrupts = <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(2) |
+						  IRQ_TYPE_LEVEL_HIGH)>;
 			clocks = <&periph_clk>;
 		};
 
-- 
1.9.1

--
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

[PATCH 2/3] ARM: dts: bcm5301x: Add I2C support to the DT

[Jon Mason]

Add I2C support to the bcm5301x Device Tree.  Since no driver changes
are needed to enable this hardware, only the device tree changes are
required to make this functional.

Signed-off-by: Jon Mason <jonmason-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 arch/arm/boot/dts/bcm5301x.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
index c31fafe..553b2ec 100644
--- a/arch/arm/boot/dts/bcm5301x.dtsi
+++ b/arch/arm/boot/dts/bcm5301x.dtsi
@@ -218,6 +218,15 @@
 		};
 	};
 
+	i2c0: i2c@18009000 {
+		compatible = "brcm,iproc-i2c";
+		reg = <0x18009000 0x50>;
+		interrupts = <GIC_SPI 121 IRQ_TYPE_NONE>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		clock-frequency = <100000>;
+	};
+
 	lcpll0: lcpll0@1800c100 {
 		#clock-cells = <1>;
 		compatible = "brcm,nsp-lcpll0";
-- 
1.9.1

--
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

[PATCH 3/3] arm: dts: bcm5301x: Add syscon based reboot in DT

[Jon Mason]

Add the ability to reboot via a reset of the processor.  This is
achieved via a write of 0x39 to the CRU Reset Register.  Unfortunately,
this only resets the core and not the other IP blocks.  So if possible,
other methods should be used on the individual boards.

Signed-off-by: Jon Mason <jonmason@broadcom.com>
---
 arch/arm/boot/dts/bcm5301x.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
index 553b2ec..2a482ec 100644
--- a/arch/arm/boot/dts/bcm5301x.dtsi
+++ b/arch/arm/boot/dts/bcm5301x.dtsi
@@ -246,6 +246,18 @@
 				     "sata2";
 	};
 
+	cru: cru@1800c184 {
+		compatible = "syscon";
+		reg = <0x1800c184 0xc>;
+	};
+
+	reboot@1800c184 {
+		compatible ="syscon-reboot";
+		regmap = <&cru>;
+		offset = <0>;
+		mask = <0x39>;
+	};
+
 	nand: nand@18028000 {
 		compatible = "brcm,nand-iproc", "brcm,brcmnand-v6.1", "brcm,brcmnand";
 		reg = <0x18028000 0x600>, <0x1811a408 0x600>, <0x18028f00 0x20>;
-- 
1.9.1

Re: [PATCH 3/3] arm: dts: bcm5301x: Add syscon based reboot in DT

[Arnd Bergmann]

On Friday 18 December 2015 16:37:56 Jon Mason wrote:
> +       cru: cru@1800c184 {
> +               compatible = "syscon";
> +               reg = <0x1800c184 0xc>;
> +       };

It's unusual for a device to start at such an odd address. Are you sure
it's not a larger device starting at 0x1800c000 or 0x18000000?

Also, please provide a more specific compatible string based on the
name of the device in the data sheet. The node name in contrast should
be more generic, e.g.

	cru: system-controller@1800c000 {
		compatible = "brcm,bcm53010-cru", "syscon";
		reg = <0x1800c000 0x400>; /* whatever the data sheet says */
	};


	Arnd
--
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

Re: [PATCH 3/3] arm: dts: bcm5301x: Add syscon based reboot in DT

[Jon Mason]

On Fri, Dec 18, 2015 at 10:44:28PM +0100, Arnd Bergmann wrote:
> On Friday 18 December 2015 16:37:56 Jon Mason wrote:
> > +       cru: cru@1800c184 {
> > +               compatible = "syscon";
> > +               reg = <0x1800c184 0xc>;
> > +       };
> 
> It's unusual for a device to start at such an odd address. Are you sure
> it's not a larger device starting at 0x1800c000 or 0x18000000?

The CRU (Clock and Reset Unit) starts at 0x1800c100, with the
following layout:

CRU Clock Management at 0x1800c100-0x1800c180
CRU Reset at 0x1800c184
CRU Period Sample Clock at 0x1800c188
CRU Interrupt register at 0x1800c18c
CRU MDIO Control at 0x1800c190
CRU GPIO at 0x1800c1c0-0x1800c1e0
CRU SDIO 0x1800c200-0x1800c214
CRU RoboSW Interrupt at 0x1800c280
CRU Straps Control at 0x1800c2a0

The clock driver is already referencing the registers between
0x1800c100-0x1800c180, and the GPIO driver is referencing registers
between 0x1800c1c0-0x1800c1e0.

The reset part of the syscon seems to be the only useful thing in this
block.  Am I approaching this incorrectly?


> Also, please provide a more specific compatible string based on the
> name of the device in the data sheet. The node name in contrast should
> be more generic, e.g.
> 
> 	cru: system-controller@1800c000 {
> 		compatible = "brcm,bcm53010-cru", "syscon";

This is very similar between the NS and NSP (and NS2) platforms.  I'll
verify the layout and see if this can't be "brcm,iproc-cru" or
something similar.

Thanks,
Jon

> 		reg = <0x1800c000 0x400>; /* whatever the data sheet says */
> 	};
> 
> 
> 	Arnd
> 
--
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

Re: [PATCH 3/3] arm: dts: bcm5301x: Add syscon based reboot in DT

[Arnd Bergmann]

On Tuesday 05 January 2016 17:26:06 Jon Mason wrote:
> On Fri, Dec 18, 2015 at 10:44:28PM +0100, Arnd Bergmann wrote:
> > On Friday 18 December 2015 16:37:56 Jon Mason wrote:
> > > +       cru: cru@1800c184 {
> > > +               compatible = "syscon";
> > > +               reg = <0x1800c184 0xc>;
> > > +       };
> > 
> > It's unusual for a device to start at such an odd address. Are you sure
> > it's not a larger device starting at 0x1800c000 or 0x18000000?
> 
> The CRU (Clock and Reset Unit) starts at 0x1800c100, with the
> following layout:
> 
> CRU Clock Management at 0x1800c100-0x1800c180
> CRU Reset at 0x1800c184
> CRU Period Sample Clock at 0x1800c188
> CRU Interrupt register at 0x1800c18c
> CRU MDIO Control at 0x1800c190
> CRU GPIO at 0x1800c1c0-0x1800c1e0
> CRU SDIO 0x1800c200-0x1800c214
> CRU RoboSW Interrupt at 0x1800c280
> CRU Straps Control at 0x1800c2a0
> 
> The clock driver is already referencing the registers between
> 0x1800c100-0x1800c180, and the GPIO driver is referencing registers
> between 0x1800c1c0-0x1800c1e0.
> 
> The reset part of the syscon seems to be the only useful thing in this
> block.  Am I approaching this incorrectly?

I think the problem is how the gpio controller has a device node
that overlaps with one of the devices of the chip.

If the data sheet lists a "Clock and Reset Unit" at those addresses,
that is a strong indication that this is actually a specific
piece of hardware, and it should be represented as one device node
in DT, with the sub-registers being exposed by that driver in
some way. A typical way would be to have a syscon node like

	cru: syscon@1800c100 {
		compatible = "brcm,bcm53010-clock-reset-unit", "syscon";
		reg = <0x1800c100 100>;
	};

that represents the entire unit. You can then have syscon references
in each driver that uses it, and/or create a high-level driver that
binds to the "brcm,bcm53010-clock-reset-unit" compatible string and that
exports a set of functions for other drivers to be used if you prefer
to do this as functional abstraction rather than register based.

> > Also, please provide a more specific compatible string based on the
> > name of the device in the data sheet. The node name in contrast should
> > be more generic, e.g.
> > 
> > 	cru: system-controller@1800c000 {
> > 		compatible = "brcm,bcm53010-cru", "syscon";
> 
> This is very similar between the NS and NSP (and NS2) platforms.  I'll
> verify the layout and see if this can't be "brcm,iproc-cru" or
> something similar.

If it's only "very similar" but not identical, don't use the same compatible
string, at least not as the only one. You should be able to identify the
specific device by looking at its compatible string in case you want to
write a high-level driver that knows about the differences later (the driver
should not need to inquire the chip name, it should only look at one device
node).

	Arnd
--
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

Re: [PATCH 1/3] ARM: dts: bcm5301x: Add TWD WD Support to DT

[Sergei Shtylyov]

Hello.

On 12/19/2015 12:37 AM, Jon Mason wrote:

> Add support for the ARM TWD Watchdog to the bcm5301x device tree.  The
> ARM TWD timer allocated the register space for the WDT, so this patch
> necessitated shrinking that.  Also, the GIC masks were added for these.
>
> Signed-off-by: Jon Mason <jonmason@broadcom.com>
> ---
>   arch/arm/boot/dts/bcm5301x.dtsi | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
> index 65a1309..c31fafe 100644
> --- a/arch/arm/boot/dts/bcm5301x.dtsi
> +++ b/arch/arm/boot/dts/bcm5301x.dtsi
> @@ -66,10 +66,19 @@
>   			clocks = <&periph_clk>;
>   		};
>
> -		local-timer@20600 {
> +		twd-timer@20600 {

    Please just "timer". The node names should be very generic, according to 
the ePAPR standard.

>   			compatible = "arm,cortex-a9-twd-timer";
> -			reg = <0x20600 0x100>;
> -			interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>;
> +			reg = <0x20600 0x20>;
> +			interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) |
> +						  IRQ_TYPE_LEVEL_HIGH)>;
> +			clocks = <&periph_clk>;
> +		};
> +
> +		twd-watchdog@20620 {

    Please, just "watchdog".

MBR, Sergei