| Message ID | 20180926193103.21241-1-zajec5@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [V3,1/2] dt-bindings: pinctrl: document Broadcom Northstar pin mux controller | expand |
On Wed, Sep 26, 2018 at 9:31 PM Rafał Miłecki <zajec5@gmail.com> wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > Northstar has mux controller just like Northstar Plus and Northstar2. > It's a bit different though (different registers & pins) so it requires > its own binding. > > It's needed to allow other block bindings specify required mux setup. > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> > --- > V2: Use "cru_gpio_control" > Add more functions & groups > Include Florian's Reviewed-by > V3: Use 3 different bindings as available pins depend on the chipset. > Strings match Documentation/devicetree/bindings/arm/bcm/brcm,bcm4708.txt Patch applied. Yours, Linus Walleij
On Wed, Sep 26, 2018 at 09:31:02PM +0200, Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > Northstar has mux controller just like Northstar Plus and Northstar2. > It's a bit different though (different registers & pins) so it requires > its own binding. > > It's needed to allow other block bindings specify required mux setup. > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> > --- > V2: Use "cru_gpio_control" > Add more functions & groups > Include Florian's Reviewed-by > V3: Use 3 different bindings as available pins depend on the chipset. > Strings match Documentation/devicetree/bindings/arm/bcm/brcm,bcm4708.txt > --- > .../bindings/pinctrl/brcm,bcm4708-pinmux.txt | 42 ++++++++++++++++++++++ > 1 file changed, 42 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt > > diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt b/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt > new file mode 100644 > index 000000000000..af906f196e8c > --- /dev/null > +++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt > @@ -0,0 +1,42 @@ > +Broadcom Northstar pins mux controller > + > +Some of Northstar SoCs's pins can be used for various purposes thanks to the mux > +controller. This binding allows describing mux controller and listing available > +functions. They can be referenced later by other bindings to let system > +configure controller correctly. > + > +A list of pins varies across chipsets so few bindings are available. > + > +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 Perhaps 'cru' in the compatible if that's what the h/w is called? Also, if this is a sub-block, then it should be a child of the block which should be defined here. > +- reg-names: "cru_gpio_control" - the only needed & supported reg right now > + > +Functions and their groups available for all chipsets: > +- "spi": "spi_grp" > +- "i2c": "i2c_grp" > +- "pwm": "pwm0_grp", "pwm1_grp", "pwm2_grp", "pwm3_grp" > +- "uart1": "uart1_grp" > + > +Additionally available on BCM4709 and BCM53012: > +- "mdio": "mdio_grp" > +- "uart2": "uart2_grp" > +- "sdio": "sdio_pwr_grp", "sdio_1p8v_grp" > + > +For documentation of subnodes see: > +Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt > + > +Example: > + pinctrl@1800c1c0 { > + compatible = "brcm,bcm4708-pinmux"; > + reg = <0x1800c1c0 0x24>; > + reg-names = "cru_gpio_control"; > + > + spi { You'll find this now causes dtc warnings. 'spi' is reserved for SPI controller nodes. So 'spi-pins' perhaps. > + function = "spi"; > + groups = "spi_grp"; > + }; > + }; > -- > 2.13.7 >
Rafał, could you patch the bindings with Rob comments so I don't have to pull the patch out of my tree or revert it? Yours, Linus Walleij
On 2018-10-12 00:22, Rob Herring wrote: > On Wed, Sep 26, 2018 at 09:31:02PM +0200, Rafał Miłecki wrote: >> From: Rafał Miłecki <rafal@milecki.pl> >> >> Northstar has mux controller just like Northstar Plus and Northstar2. >> It's a bit different though (different registers & pins) so it >> requires >> its own binding. >> >> It's needed to allow other block bindings specify required mux setup. >> >> Signed-off-by: Rafał Miłecki <rafal@milecki.pl> >> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> >> --- >> V2: Use "cru_gpio_control" >> Add more functions & groups >> Include Florian's Reviewed-by >> V3: Use 3 different bindings as available pins depend on the chipset. >> Strings match >> Documentation/devicetree/bindings/arm/bcm/brcm,bcm4708.txt >> --- >> .../bindings/pinctrl/brcm,bcm4708-pinmux.txt | 42 >> ++++++++++++++++++++++ >> 1 file changed, 42 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt >> >> diff --git >> a/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt >> b/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt >> new file mode 100644 >> index 000000000000..af906f196e8c >> --- /dev/null >> +++ >> b/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt >> @@ -0,0 +1,42 @@ >> +Broadcom Northstar pins mux controller >> + >> +Some of Northstar SoCs's pins can be used for various purposes thanks >> to the mux >> +controller. This binding allows describing mux controller and listing >> available >> +functions. They can be referenced later by other bindings to let >> system >> +configure controller correctly. >> + >> +A list of pins varies across chipsets so few bindings are available. >> + >> +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 > > Perhaps 'cru' in the compatible if that's what the h/w is called? > > Also, if this is a sub-block, then it should be a child of the block > which should be defined here. It took me some time to do some extra research on the whole CRU thing. Valuable resources: [1] bcm5301x_dmu.c (from the SDK) [2] bcm5301x_pcie.c (from the SDK) [3] https://patchwork.kernel.org/patch/7888651/ [4] https://patchwork.kernel.org/patch/5051471/ First of all CRU seems to be a sub-block of the DMU (which stands for "Device Management Unit" according to the [1]). DMU seems to be independent block at 0x1800c000 with a size of 0x1000. It isn't actually clear what the CRU stands for. In Broadcom's case: 1. According to the [3] it's "Clock and Reset Unit" 2. According to the [4] it's a "central resource unit" Other vendors seem to use CRU name for "Clock and Reset Unit". In any case, you're right Rob, it's a sub-block, a set of random registers that control SoC. So I think that: 1) We should have a node for DMU and CRU 2) We should not include "cru" in bindings as pinmuxing seems to be part of SoC that just happens to be controlled using CRU registers
On Mon, Oct 15, 2018 at 10:56 AM Rafał Miłecki <rafal@milecki.pl> wrote: > > Perhaps 'cru' in the compatible if that's what the h/w is called? > > > > Also, if this is a sub-block, then it should be a child of the block > > which should be defined here. > > It took me some time to do some extra research on the whole CRU thing. > > Valuable resources: > [1] bcm5301x_dmu.c (from the SDK) > [2] bcm5301x_pcie.c (from the SDK) > [3] https://patchwork.kernel.org/patch/7888651/ > [4] https://patchwork.kernel.org/patch/5051471/ > > First of all CRU seems to be a sub-block of the DMU (which stands for > "Device Management Unit" according to the [1]). DMU seems to be > independent block at 0x1800c000 with a size of 0x1000. > > It isn't actually clear what the CRU stands for. In Broadcom's case: > 1. According to the [3] it's "Clock and Reset Unit" > 2. According to the [4] it's a "central resource unit" > Other vendors seem to use CRU name for "Clock and Reset Unit". > > In any case, you're right Rob, it's a sub-block, a set of random > registers that control SoC. > > So I think that: > 1) We should have a node for DMU and CRU > 2) We should not include "cru" in bindings as pinmuxing seems to be part > of SoC that just happens to be controlled using CRU registers OK do you want me to revert the patches or do you want to fix this stuff on top of the patches that are already in the tree? I'm fine either way. Yours, Linus Walleij
On Tue, 16 Oct 2018 at 09:41, Linus Walleij <linus.walleij@linaro.org> wrote: > On Mon, Oct 15, 2018 at 10:56 AM Rafał Miłecki <rafal@milecki.pl> wrote: > > > > Perhaps 'cru' in the compatible if that's what the h/w is called? > > > > > > Also, if this is a sub-block, then it should be a child of the block > > > which should be defined here. > > > > It took me some time to do some extra research on the whole CRU thing. > > > > Valuable resources: > > [1] bcm5301x_dmu.c (from the SDK) > > [2] bcm5301x_pcie.c (from the SDK) > > [3] https://patchwork.kernel.org/patch/7888651/ > > [4] https://patchwork.kernel.org/patch/5051471/ > > > > First of all CRU seems to be a sub-block of the DMU (which stands for > > "Device Management Unit" according to the [1]). DMU seems to be > > independent block at 0x1800c000 with a size of 0x1000. > > > > It isn't actually clear what the CRU stands for. In Broadcom's case: > > 1. According to the [3] it's "Clock and Reset Unit" > > 2. According to the [4] it's a "central resource unit" > > Other vendors seem to use CRU name for "Clock and Reset Unit". > > > > In any case, you're right Rob, it's a sub-block, a set of random > > registers that control SoC. > > > > So I think that: > > 1) We should have a node for DMU and CRU > > 2) We should not include "cru" in bindings as pinmuxing seems to be part > > of SoC that just happens to be controlled using CRU registers > > OK do you want me to revert the patches or do you want to > fix this stuff on top of the patches that are already in the tree? > I'm fine either way. I'm fine either way, thanks for taking care of my work!
diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt b/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt new file mode 100644 index 000000000000..af906f196e8c --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm4708-pinmux.txt @@ -0,0 +1,42 @@ +Broadcom Northstar pins mux controller + +Some of Northstar SoCs's pins can be used for various purposes thanks to the mux +controller. This binding allows describing mux controller and listing available +functions. They can be referenced later by other bindings to let system +configure controller correctly. + +A list of pins varies across chipsets so few bindings are available. + +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 + +Functions and their groups available for all chipsets: +- "spi": "spi_grp" +- "i2c": "i2c_grp" +- "pwm": "pwm0_grp", "pwm1_grp", "pwm2_grp", "pwm3_grp" +- "uart1": "uart1_grp" + +Additionally available on BCM4709 and BCM53012: +- "mdio": "mdio_grp" +- "uart2": "uart2_grp" +- "sdio": "sdio_pwr_grp", "sdio_1p8v_grp" + +For documentation of subnodes see: +Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt + +Example: + pinctrl@1800c1c0 { + compatible = "brcm,bcm4708-pinmux"; + reg = <0x1800c1c0 0x24>; + reg-names = "cru_gpio_control"; + + spi { + function = "spi"; + groups = "spi_grp"; + }; + };