diff mbox series

[fixes,4.20] dt-bindings: pinctrl: bcm4708-pinmux: rework binding to use syscon

Message ID 20181218155744.19409-1-zajec5@gmail.com (mailing list archive)
State New, archived
Headers show
Series [fixes,4.20] dt-bindings: pinctrl: bcm4708-pinmux: rework binding to use syscon | expand

Commit Message

Rafał Miłecki Dec. 18, 2018, 3:57 p.m. UTC 
From: Rafał Miłecki <rafal@milecki.pl>

As pointed by Rob, CRU is a kind of block that can't be guaranteed to
have everything exposed as subnodes. It's a set of various registers
that aren't tied to any single device. It could be described much more
accurately as MFD (Multi-Function Device).

Some hardware blocks may indeed want to access a register or two of the
CRU which requires describing it as the "syscon".

While at it replace exmple node name with the standard "pinctrl" (also
pointed out by Rob).

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
Hi Linus,

After being pinged about the pinctrl driver I realized I never addressed
Rob's comments from the e-mail thread:

[PATCH] dt-bindings: pinctrl: bcm4708-pinmux: improve example binding
https://www.spinics.net/lists/arm-kernel/msg682838.html
https://patchwork.ozlabs.org/patch/984024/

Rob has pointed correctly (as always) that describing CRU using
"simple-bus" has its implications and may hunt us back if we ever
realize we will want to reference it as "syscon". That is pretty likely
actually.

To fix that while still possible (before having that Documentation in
any stable release) I'd like you to consider taking this patch for the
4.20 release if you find it possible.

I'm well aware it's damn late. I'm aware I've screwed up. I'm sorry.

I'm afraid I cannot fix it anyhow. Just take a look at that patch and
feel free to say I'm crazy coming with it so late.
---
 .../devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt  | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

Comments

Linus Walleij Dec. 21, 2018, 10:45 a.m. UTC | #1 
On Tue, Dec 18, 2018 at 4:58 PM Rafał Miłecki <zajec5@gmail.com> wrote:

> From: Rafał Miłecki <rafal@milecki.pl>
>
> As pointed by Rob, CRU is a kind of block that can't be guaranteed to
> have everything exposed as subnodes. It's a set of various registers
> that aren't tied to any single device. It could be described much more
> accurately as MFD (Multi-Function Device).
>
> Some hardware blocks may indeed want to access a register or two of the
> CRU which requires describing it as the "syscon".
>
> While at it replace exmple node name with the standard "pinctrl" (also
> pointed out by Rob).
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> Hi Linus,
>
> After being pinged about the pinctrl driver I realized I never addressed
> Rob's comments from the e-mail thread:
>
> [PATCH] dt-bindings: pinctrl: bcm4708-pinmux: improve example binding
> https://www.spinics.net/lists/arm-kernel/msg682838.html
> https://patchwork.ozlabs.org/patch/984024/
>
> Rob has pointed correctly (as always) that describing CRU using
> "simple-bus" has its implications and may hunt us back if we ever
> realize we will want to reference it as "syscon". That is pretty likely
> actually.
>
> To fix that while still possible (before having that Documentation in
> any stable release) I'd like you to consider taking this patch for the
> 4.20 release if you find it possible.
>
> I'm well aware it's damn late. I'm aware I've screwed up. I'm sorry.
>
> I'm afraid I cannot fix it anyhow. Just take a look at that patch and
> feel free to say I'm crazy coming with it so late.

I just applied it. It's not like it's etched in stone though some
people may have that mental model.

If the DT maintainers have further concerns we can patch it
again.

Rough consensus and running code.

Yours,
Linus Walleij
Rafał Miłecki Dec. 21, 2018, 11:07 a.m. UTC | #2 
On Fri, 21 Dec 2018 at 11:45, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Dec 18, 2018 at 4:58 PM Rafał Miłecki <zajec5@gmail.com> wrote:
>
> > From: Rafał Miłecki <rafal@milecki.pl>
> >
> > As pointed by Rob, CRU is a kind of block that can't be guaranteed to
> > have everything exposed as subnodes. It's a set of various registers
> > that aren't tied to any single device. It could be described much more
> > accurately as MFD (Multi-Function Device).
> >
> > Some hardware blocks may indeed want to access a register or two of the
> > CRU which requires describing it as the "syscon".
> >
> > While at it replace exmple node name with the standard "pinctrl" (also
> > pointed out by Rob).
> >
> > Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> > ---
> > Hi Linus,
> >
> > After being pinged about the pinctrl driver I realized I never addressed
> > Rob's comments from the e-mail thread:
> >
> > [PATCH] dt-bindings: pinctrl: bcm4708-pinmux: improve example binding
> > https://www.spinics.net/lists/arm-kernel/msg682838.html
> > https://patchwork.ozlabs.org/patch/984024/
> >
> > Rob has pointed correctly (as always) that describing CRU using
> > "simple-bus" has its implications and may hunt us back if we ever
> > realize we will want to reference it as "syscon". That is pretty likely
> > actually.
> >
> > To fix that while still possible (before having that Documentation in
> > any stable release) I'd like you to consider taking this patch for the
> > 4.20 release if you find it possible.
> >
> > I'm well aware it's damn late. I'm aware I've screwed up. I'm sorry.
> >
> > I'm afraid I cannot fix it anyhow. Just take a look at that patch and
> > feel free to say I'm crazy coming with it so late.
>
> I just applied it. It's not like it's etched in stone though some
> people may have that mental model.
>
> If the DT maintainers have further concerns we can patch it
> again.
>
> Rough consensus and running code.

Thank you, I appreciate it!
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt b/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt
index 4fa9539070cb..8ab2d468dbdb 100644
--- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt
+++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt
@@ -7,13 +7,15 @@  configure controller correctly.
 
 A list of pins varies across chipsets so few bindings are available.
 
+Node of the pinmux must be nested in the CRU (Central Resource Unit) "syscon"
+noce.
+
 Required properties:
 - compatible: must be one of:
 	"brcm,bcm4708-pinmux"
 	"brcm,bcm4709-pinmux"
 	"brcm,bcm53012-pinmux"
-- reg: iomem address range of CRU (Central Resource Unit) pin registers
-- reg-names: "cru_gpio_control" - the only needed & supported reg right now
+- offset: offset of pin registers in the CRU block
 
 Functions and their groups available for all chipsets:
 - "spi": "spi_grp"
@@ -37,16 +39,12 @@  Example:
 		#size-cells = <1>;
 
 		cru@100 {
-			compatible = "simple-bus";
+			compatible = "syscon", "simple-mfd";
 			reg = <0x100 0x1a4>;
-			ranges;
-			#address-cells = <1>;
-			#size-cells = <1>;
 
-			pin-controller@1c0 {
+			pinctrl {
 				compatible = "brcm,bcm4708-pinmux";
-				reg = <0x1c0 0x24>;
-				reg-names = "cru_gpio_control";
+				offset = <0xc0>;
 
 				spi-pins {
 					function = "spi";