Skip to contentSkip to conversation

ARM: brcmstb: add misc. DT bindings for brcm, brcmstb

PATCH RESEND Superseded Marc C 2 participants 4 messages Nov 23, 2013

Subsystems (2)

v1 of 7 Superseded Latest is v7 →

lore
Share & tools
lore

Marc C

author
#0 lore raw

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

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

---
 .../devicetree/bindings/arm/brcm-brcmstb.txt       |   72 ++++++++++++++++++++
 1 files changed, 72 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/brcm-brcmstb.txt
(quoted diff) +0 -0

b/Documentation/devicetree/bindings/arm/brcm-brcmstb.txt new file mode 100644
index 0000000..2f3cd50

Documentation/devicetree/bindings/arm/brcm-brcmstb.txt new file +72 -0

72 lines not on this page · raw message

Signature · 1 line
1.7.1
#1 lore raw

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

Documentation/devicetree/bindings/arm/brcm-brcmstb.txt line 23

Quoted 7 lines · Marc C
Documentation/devicetree/bindings/arm/brcm-brcmstb.txt new file +7 -0
+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

author
#2 lore raw

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

Hello Arnd,

Quoted 3 lines

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.

Quoted 5 lines

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

Quoted 21 lines · Arnd Bergmann

Documentation/devicetree/bindings/arm/brcm-brcmstb.txt line 23

Quoted 7 lines · Marc C
Documentation/devicetree/bindings/arm/brcm-brcmstb.txt new file +7 -0
+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

#3 lore raw

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

Quoted 13 lines · Marc C

Hello Arnd,

Quoted 3 lines

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.

Quoted 4 lines

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