[PATCH RESEND 3/5] ARM: brcmstb: add misc. DT bindings for brcm, brcmstb

[PATCH RESEND 3/5] ARM: brcmstb: add misc. DT bindings for brcm, brcmstb

[Marc C]

Document the bindings that the Broadcom STB platform needs
for proper bootup.

Signed-off-by: Marc Carino <marc.ceeeee@gmail.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
---
 .../devicetree/bindings/arm/brcm-brcmstb.txt       |   72 ++++++++++++++++++++
 1 files changed, 72 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/brcm-brcmstb.txt

diff --git a/Documentation/devicetree/bindings/arm/brcm-brcmstb.txt
b/Documentation/devicetree/bindings/arm/brcm-brcmstb.txt
new file mode 100644
index 0000000..2f3cd50
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/brcm-brcmstb.txt
@@ -0,0 +1,72 @@
+Broadcom STB platforms Device Tree Bindings
+-------------------------------------------
+Boards with Broadcom Brahma15-based BCM7xxx SOC shall have the following
+properties.
+
+Required root node properties:
+    - compatible = "brcm,brcmstb";
+
+Further, the following platform nodes shall be defined:
+
+    - sun-top-ctrl
+    - cpu-biu-ctrl
+    - hif-continuation
+
+sun-top-ctrl
+------------
+This node describes the register block which is used for generic reset control.
+
+    - compatible: "brcm,brcmstb-sun-top-ctrl"
+    - properties:
+        o reg = <base_of_sun_top_ctrl_regs length>;
+        o reset-source-enable-reg = <offset_from_base>;
+        o sw-master-reset-reg = <offset_from_base>;
+
+cpu-biu-ctrl
+------------
+This node describes the register block used for configuring the CPU complex.
+
+    - compatible: "brcm,brcmstb-cpu-biu-ctrl"
+    - properties:
+        o reg = <base_of_cpu_biu_ctrl_regs length>;
+        o cpu-reset-config-reg = <offset_from_base>;
+        o cpu0-pwr-zone-ctrl-reg = <offset_from_base>;
+
+hif-continuation
+----------------
+This node describes the registers for setting the starting PC for each CPU core.
+
+    - compatible: "brcm,brcmstb-hif-continuation"
+    - properties:
+        o reg = <base_of_hif_continuation_regs length>;
+        o stb-boot-hi-addr0-reg = <offset_from_base>;
+
+example:
+
+/ {
+	model = "Broadcom STB";
+	compatible =  "brcm,brcmstb";
+
+	/* snip */
+
+	sun-top-ctrl at f0404000 {
+		compatible = "brcm,brcmstb-sun-top-ctrl";
+		reg = <0xf0404000 0x51c>;
+		reset-source-enable-reg = <0x304>;
+		sw-master-reset-reg = <0x308>;
+	};
+
+	cpu-biu-ctrl at f0442400 {
+		compatible = "brcm,brcmstb-cpu-biu-ctrl";
+		reg = <0xf0442400 0x5b4>;
+		cpu-reset-config-reg = <0x178>;
+		cpu0-pwr-zone-ctrl-reg = <0x88>;
+	};
+
+	hif-continuation at f0452000 {
+		compatible = "brcm,brcmstb-hif-continuation";
+		reg = <0xf0452000 0x100>;
+	};
+
+	/* snip */
+};
-- 
1.7.1

[PATCH RESEND 3/5] ARM: brcmstb: add misc. DT bindings for brcm, brcmstb

[Arnd Bergmann]

On Saturday 23 November 2013, Marc C wrote:
> +This node describes the register block which is used for generic reset control.
> +
> +    - compatible: "brcm,brcmstb-sun-top-ctrl"
> +    - properties:
> +        o reg = <base_of_sun_top_ctrl_regs length>;
> +        o reset-source-enable-reg = <offset_from_base>;
> +        o sw-master-reset-reg = <offset_from_base>;

Normally the "compatible" string identifies the register layout and you keep the
offsets in the device driver. Do you have strong reasons to do it differently
here?

I would also suggest moving this block into a separate file for the reset
controller. Make sure you follow the rules from
Documentation/devicetree/bindings/reset/reset.txt, most importantly adding
a "#reset-cells" property so other drivers can refer to this node using
the reset API.

	Arnd

[PATCH RESEND 3/5] ARM: brcmstb: add misc. DT bindings for brcm, brcmstb

[Marc C]

Hello Arnd,

> Normally the "compatible" string identifies the register layout and
> you keep the offsets in the device driver. Do you have strong reasons to do it
> differently here?

The register block that contains the reset registers also contains other
?general purpose? registers, which can potentially scoot the positions
of both ?sw-master-reset? and ?reset-source-enable?. Although we do
stress to the H/W designers that consistency between chips is important,
and that relative offsets of those registers should remain the same
between chip revisions, there is no guarantee that this will happen.

Now, rather than handling N revisions of the ?sun-top-ctrl? register
block at both the boot loader and kernel levels (N could get very large
as we continue to port support for more chips), I feel that a more
flexible solution would be to rely on the self-describing nature of the DT.

However, I do agree with your suggestion if the registers belong to a
standard and more-static block of registers, such as those for AHCI,
EHCI, or a network controller.

> I would also suggest moving this block into a separate file for the
> reset controller. Make sure you follow the rules from 
> Documentation/devicetree/bindings/reset/reset.txt, most importantly
> adding a "#reset-cells" property so other drivers can refer to this
> node using the reset API.

I will look into this.

Thank you for your feedback!
Marc C

On 11/23/13, 10:37 AM, Arnd Bergmann wrote:
> On Saturday 23 November 2013, Marc C wrote:
>> +This node describes the register block which is used for generic reset control.
>> +
>> +    - compatible: "brcm,brcmstb-sun-top-ctrl"
>> +    - properties:
>> +        o reg = <base_of_sun_top_ctrl_regs length>;
>> +        o reset-source-enable-reg = <offset_from_base>;
>> +        o sw-master-reset-reg = <offset_from_base>;
> 
> Normally the "compatible" string identifies the register layout and you keep the
> offsets in the device driver. Do you have strong reasons to do it differently
> here?
> 
> I would also suggest moving this block into a separate file for the reset
> controller. Make sure you follow the rules from
> Documentation/devicetree/bindings/reset/reset.txt, most importantly adding
> a "#reset-cells" property so other drivers can refer to this node using
> the reset API.
> 
> 	Arnd
> 

[PATCH RESEND 3/5] ARM: brcmstb: add misc. DT bindings for brcm, brcmstb

[Arnd Bergmann]

On Saturday 23 November 2013, Marc C wrote:
> Hello Arnd,
> 
> > Normally the "compatible" string identifies the register layout and
> > you keep the offsets in the device driver. Do you have strong reasons to do it
> > differently here?
> 
> The register block that contains the reset registers also contains other
> ?general purpose? registers, which can potentially scoot the positions
> of both ?sw-master-reset? and ?reset-source-enable?. Although we do
> stress to the H/W designers that consistency between chips is important,
> and that relative offsets of those registers should remain the same
> between chip revisions, there is no guarantee that this will happen.
> 
> Now, rather than handling N revisions of the ?sun-top-ctrl? register
> block at both the boot loader and kernel levels (N could get very large
> as we continue to port support for more chips), I feel that a more
> flexible solution would be to rely on the self-describing nature of the DT.

Ok, I see. I guess it's not even a uniform layout within the registers
then, right? Otherwise you could reference the number of the reset
register through the reset-api, but it gets kind of ugly when passing
the wrong value for a reset line touches a completely unrelated register
rather than "just" resetting the wrong device.

	Arnd