[PATCH] bcma: use device from DT (brcm,bus-gpio) for SoC GPIO chip

[PATCH] bcma: use device from DT (brcm,bus-gpio) for SoC GPIO chip

[Rafał Miłecki]

This will allow us to define GPIO-attached devices (LEDs, buttons) in
the the device tree.

Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com>
---
This is based on top of
[PATCH v6] bcma: register bcma as device tree driver
that I hope will reach wireless-next git tree.
---
 Documentation/devicetree/bindings/bus/bcma.txt | 15 +++++++++++++++
 drivers/bcma/driver_gpio.c                     |  5 +++++
 2 files changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/bus/bcma.txt b/Documentation/devicetree/bindings/bus/bcma.txt
index e9070c1..f1b381e 100644
--- a/Documentation/devicetree/bindings/bus/bcma.txt
+++ b/Documentation/devicetree/bindings/bus/bcma.txt
@@ -6,6 +6,15 @@ Required properties:
 
 - reg : iomem address range of chipcommon core
 
+The top-level axi bus may contain following children:
+
+- gpio: GPIO chip on the SoC
+
+  Required properties:
+  - compatible: "brcm,bus-gpio"
+  - gpio-controller : makes the node a GPIO controller
+  - #gpio-cells : size of the GPIO specifier, must be 2
+
 The cores on the AXI bus are automatically detected by bcma with the
 memory ranges they are using and they get registered afterwards.
 
@@ -17,4 +26,10 @@ Example:
 		ranges = <0x00000000 0x18000000 0x00100000>;
 		#address-cells = <1>;
 		#size-cells = <1>;
+
+		gpio at 0 {
+			compatible = "brcm,bus-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
 	};
diff --git a/drivers/bcma/driver_gpio.c b/drivers/bcma/driver_gpio.c
index 8ea497c..7ae39a8 100644
--- a/drivers/bcma/driver_gpio.c
+++ b/drivers/bcma/driver_gpio.c
@@ -218,6 +218,11 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
 #if IS_BUILTIN(CONFIG_BCM47XX)
 	chip->to_irq		= bcma_gpio_to_irq;
 #endif
+#if IS_BUILTIN(CONFIG_OF)
+	if (cc->core->bus->hosttype == BCMA_HOSTTYPE_SOC)
+		chip->of_node	= of_find_compatible_node(NULL, NULL,
+							  "brcm,bus-gpio");
+#endif
 	switch (cc->core->bus->chipinfo.id) {
 	case BCMA_CHIP_ID_BCM5357:
 	case BCMA_CHIP_ID_BCM53572:
-- 
1.8.4.5

[PATCH] bcma: use device from DT (brcm, bus-gpio) for SoC GPIO chip

[Arnd Bergmann]

On Friday 26 September 2014 16:28:53 Rafa? Mi?ecki wrote:
> +The top-level axi bus may contain following children:
> +
> +- gpio: GPIO chip on the SoC
> +
> +  Required properties:
> +  - compatible: "brcm,bus-gpio"
> +  - gpio-controller : makes the node a GPIO controller
> +  - #gpio-cells : size of the GPIO specifier, must be 2
> +
> 

I wonder if it would be better to avoid the subnode here and just
make the parent itself the gpio controller.

Is the gpio controller part of the bus itself in reality, or is it
a device that gets probed on the bus?

	Arnd

[PATCH] bcma: use device from DT (brcm, bus-gpio) for SoC GPIO chip

[Rafał Miłecki]

On 27 September 2014 00:03, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 26 September 2014 16:28:53 Rafa? Mi?ecki wrote:
>> +The top-level axi bus may contain following children:
>> +
>> +- gpio: GPIO chip on the SoC
>> +
>> +  Required properties:
>> +  - compatible: "brcm,bus-gpio"
>> +  - gpio-controller : makes the node a GPIO controller
>> +  - #gpio-cells : size of the GPIO specifier, must be 2
>> +
>>
>
> I wonder if it would be better to avoid the subnode here and just
> make the parent itself the gpio controller.
>
> Is the gpio controller part of the bus itself in reality, or is it
> a device that gets probed on the bus?

I'm not sure how to treat this chip.
We control GPIOs using ChipCommon regs (and ChipCommon is one of
cores/devices on the bus), so you could also consider GPIO chip a
sub-device of ChipCommon.
I believe every Broadcom bus has a GPIO chip. In the ancient (MIPS)
times, even if we didn't have ChipCommon, there was an EXTIF core that
used to provide access to the GPIO chip.

What do you think? Should I make it separated device, even it if
depends on the SoC and its ChipCommon core (device)?

-- 
Rafa?

[PATCH] bcma: use device from DT (brcm,bus-gpio) for SoC GPIO chip

[Hauke Mehrtens]

On 09/27/2014 10:05 AM, Rafa? Mi?ecki wrote:
> On 27 September 2014 00:03, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Friday 26 September 2014 16:28:53 Rafa? Mi?ecki wrote:
>>> +The top-level axi bus may contain following children:
>>> +
>>> +- gpio: GPIO chip on the SoC
>>> +
>>> +  Required properties:
>>> +  - compatible: "brcm,bus-gpio"
>>> +  - gpio-controller : makes the node a GPIO controller
>>> +  - #gpio-cells : size of the GPIO specifier, must be 2
>>> +
>>>
>>
>> I wonder if it would be better to avoid the subnode here and just
>> make the parent itself the gpio controller.
>>
>> Is the gpio controller part of the bus itself in reality, or is it
>> a device that gets probed on the bus?
> 
> I'm not sure how to treat this chip.
> We control GPIOs using ChipCommon regs (and ChipCommon is one of
> cores/devices on the bus), so you could also consider GPIO chip a
> sub-device of ChipCommon.
> I believe every Broadcom bus has a GPIO chip. In the ancient (MIPS)
> times, even if we didn't have ChipCommon, there was an EXTIF core that
> used to provide access to the GPIO chip.
> 
> What do you think? Should I make it separated device, even it if
> depends on the SoC and its ChipCommon core (device)?
> 
I would make GPIO a subdevive of chipcommon. The chipcommon core has an
own IRQ which is also used for GPIO.

Hauke

[PATCH] bcma: use device from DT (brcm, bus-gpio) for SoC GPIO chip

[Rafał Miłecki]

On 27 September 2014 10:33, Hauke Mehrtens <hauke@hauke-m.de> wrote:
> I would make GPIO a subdevive of chipcommon. The chipcommon core has an
> own IRQ which is also used for GPIO.

Which ChipCommon do yo mean?
1) chipcommonA (compatible = "simple-bus")
2) chipcommon at 0 (child of axi at 18000000 AKA brcm,bus-axi)

It seems that for some reason both of them use IRQ 85, while the IRQ
for ChipCommon is 117 I believe.

-- 
Rafa?

[PATCH] bcma: use device from DT (brcm,bus-gpio) for SoC GPIO chip

[Hauke Mehrtens]

On 09/27/2014 12:37 PM, Rafa? Mi?ecki wrote:
> On 27 September 2014 10:33, Hauke Mehrtens <hauke@hauke-m.de> wrote:
>> I would make GPIO a subdevive of chipcommon. The chipcommon core has an
>> own IRQ which is also used for GPIO.
> 
> Which ChipCommon do yo mean?
> 1) chipcommonA (compatible = "simple-bus")
> 2) chipcommon at 0 (child of axi at 18000000 AKA brcm,bus-axi)

We should combine this (both are describing the same core) I added
chipcommonA to get some serial without adding bcma support first. When
we have dts support added to bcma, I would like to remove chipcommonA
from dtsi and add a chipcommon as a child of axi.

> 
> It seems that for some reason both of them use IRQ 85, while the IRQ
> for ChipCommon is 117 I believe.

That's the same.

In the dtsi file it says:
interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
this will result in the IRQ number 117, when it is GIC_SPI you have to
add 32 to the irq number to get the actual number which will be given to
request_irq().

Hauke

[PATCH V2] bcma: use device from DT (brcm, bus-gpio) for SoC GPIO chip

[Rafał Miłecki]

This will allow us to define GPIO-attached devices (LEDs, buttons) in
the the device tree.

Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com>
---
This is based on top of
[PATCH v6] bcma: register bcma as device tree driver
that I hope will reach wireless-next git tree.

V2: Describe axi chilren and make gpio a child of chipcommon core.
---
 Documentation/devicetree/bindings/bus/bcma.txt | 24 ++++++++++++++++++++++++
 drivers/bcma/driver_gpio.c                     |  5 +++++
 2 files changed, 29 insertions(+)

diff --git a/Documentation/devicetree/bindings/bus/bcma.txt b/Documentation/devicetree/bindings/bus/bcma.txt
index e9070c1..26ef4b7 100644
--- a/Documentation/devicetree/bindings/bus/bcma.txt
+++ b/Documentation/devicetree/bindings/bus/bcma.txt
@@ -9,6 +9,22 @@ Required properties:
 The cores on the AXI bus are automatically detected by bcma with the
 memory ranges they are using and they get registered afterwards.
 
+The top-level axi bus may contain children representing attached cores
+(devices). This is needed since some hardware details can't be auto
+detected (e.g. IRQ numbers).
+
+There is also one special core called ChipCommon that may contain some
+extra sub-devices. This is because some devices (e.g. GPIO chip) are
+not standalone cores and can be access using ChipCommon regs only.
+Possible ChipCommon children:
+
+- gpio: GPIO chip on the SoC
+
+  Required properties:
+  - compatible: "brcm,bus-gpio"
+  - gpio-controller : makes the node a GPIO controller
+  - #gpio-cells : size of the GPIO specifier, must be 2
+
 Example:
 
 	axi at 18000000 {
@@ -17,4 +33,12 @@ Example:
 		ranges = <0x00000000 0x18000000 0x00100000>;
 		#address-cells = <1>;
 		#size-cells = <1>;
+
+		chipcommon at 0 {
+			gpio at 0 {
+				compatible = "brcm,bus-gpio";
+				gpio-controller;
+				#gpio-cells = <2>;
+			};
+		};
 	};
diff --git a/drivers/bcma/driver_gpio.c b/drivers/bcma/driver_gpio.c
index 8ea497c..7ae39a8 100644
--- a/drivers/bcma/driver_gpio.c
+++ b/drivers/bcma/driver_gpio.c
@@ -218,6 +218,11 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
 #if IS_BUILTIN(CONFIG_BCM47XX)
 	chip->to_irq		= bcma_gpio_to_irq;
 #endif
+#if IS_BUILTIN(CONFIG_OF)
+	if (cc->core->bus->hosttype == BCMA_HOSTTYPE_SOC)
+		chip->of_node	= of_find_compatible_node(NULL, NULL,
+							  "brcm,bus-gpio");
+#endif
 	switch (cc->core->bus->chipinfo.id) {
 	case BCMA_CHIP_ID_BCM5357:
 	case BCMA_CHIP_ID_BCM53572:
-- 
1.8.4.5

[PATCH V2] bcma: use device from DT (brcm, bus-gpio) for SoC GPIO chip

[Arnd Bergmann]

On Sunday 28 September 2014 10:24:01 Rafa? Mi?ecki wrote:
> This will allow us to define GPIO-attached devices (LEDs, buttons) in
> the the device tree.
> 
> Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com>
> ---
> This is based on top of
> [PATCH v6] bcma: register bcma as device tree driver
> that I hope will reach wireless-next git tree.
> 
> V2: Describe axi chilren and make gpio a child of chipcommon core.
> ---
>  Documentation/devicetree/bindings/bus/bcma.txt | 24 ++++++++++++++++++++++++
>  drivers/bcma/driver_gpio.c                     |  5 +++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/bus/bcma.txt b/Documentation/devicetree/bindings/bus/bcma.txt
> index e9070c1..26ef4b7 100644
> --- a/Documentation/devicetree/bindings/bus/bcma.txt
> +++ b/Documentation/devicetree/bindings/bus/bcma.txt
> @@ -9,6 +9,22 @@ Required properties:
>  The cores on the AXI bus are automatically detected by bcma with the
>  memory ranges they are using and they get registered afterwards.
>  
> +The top-level axi bus may contain children representing attached cores
> +(devices). This is needed since some hardware details can't be auto
> +detected (e.g. IRQ numbers).
> +
> +There is also one special core called ChipCommon that may contain some
> +extra sub-devices. This is because some devices (e.g. GPIO chip) are
> +not standalone cores and can be access using ChipCommon regs only.
> +Possible ChipCommon children:
> +
> +- gpio: GPIO chip on the SoC
> +
> +  Required properties:
> +  - compatible: "brcm,bus-gpio"
> +  - gpio-controller : makes the node a GPIO controller
> +  - #gpio-cells : size of the GPIO specifier, must be 2
> +
>  Example:
>  
>  	axi at 18000000 {
> @@ -17,4 +33,12 @@ Example:
>  		ranges = <0x00000000 0x18000000 0x00100000>;
>  		#address-cells = <1>;
>  		#size-cells = <1>;
> +
> +		chipcommon at 0 {
> +			gpio at 0 {
> +				compatible = "brcm,bus-gpio";
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +			};
> +		};

I think you should list the 'reg' property of the chipcommon area
and make that match the unit address, rather than using '0', mostly
for consistency.

Can you have multiple gpio areas in chipcommon? If not, I'd probably
leave out the extra node and mark the chipcommon device itself as
a 'gpio-controller'. It can also be other things at the same time,
e.g. 'interrupt-controller' or provide things like pwms or pinctrl
if that's what the hardware does. No need to have separate nodes
for those if it's all the same register set.


>  	};
> diff --git a/drivers/bcma/driver_gpio.c b/drivers/bcma/driver_gpio.c
> index 8ea497c..7ae39a8 100644
> --- a/drivers/bcma/driver_gpio.c
> +++ b/drivers/bcma/driver_gpio.c
> @@ -218,6 +218,11 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
>  #if IS_BUILTIN(CONFIG_BCM47XX)
>  	chip->to_irq		= bcma_gpio_to_irq;
>  #endif
> +#if IS_BUILTIN(CONFIG_OF)
> +	if (cc->core->bus->hosttype == BCMA_HOSTTYPE_SOC)
> +		chip->of_node	= of_find_compatible_node(NULL, NULL,
> +							  "brcm,bus-gpio");
> +#endif

Just commenting on the general style here, I think you can skip this
step entirely, as mentioned above.

You should use C syntax here:

	if (IS_BUILTIN(CONFIG_OF) && cc->core->bus->hosttype == BCMA_HOSTTYPE_SOC))
		chip->of_node = of_find_compatible_node(NULL, NULL, "brcm,bus-gpio");

If CONFIG_OF is not set, of_find_compatible_node() is turned into an inline
function that returns NULL, so you probably don't even need that.

Finally, of_find_compatible_node() is a really slow operation, and "brcm,bus-gpio"
is a much too generic name. If you need to look for something that is a child
node, use something based on for_each_available_child_of_node().


	Arnd

[PATCH V2] bcma: use device from DT (brcm, bus-gpio) for SoC GPIO chip

[Rafał Miłecki]

On 30 September 2014 11:37, Arnd Bergmann <arnd@arndb.de> wrote:
> On Sunday 28 September 2014 10:24:01 Rafa? Mi?ecki wrote:
>> @@ -17,4 +33,12 @@ Example:
>>               ranges = <0x00000000 0x18000000 0x00100000>;
>>               #address-cells = <1>;
>>               #size-cells = <1>;
>> +
>> +             chipcommon at 0 {
>> +                     gpio at 0 {
>> +                             compatible = "brcm,bus-gpio";
>> +                             gpio-controller;
>> +                             #gpio-cells = <2>;
>> +                     };
>> +             };
>
> I think you should list the 'reg' property of the chipcommon area
> and make that match the unit address, rather than using '0', mostly
> for consistency.

Do you mean this chipcommon at 0? We propose this foo at offset syntax since
V1 which was posted 1,5 month ago, it's nothing new.


> Can you have multiple gpio areas in chipcommon? If not, I'd probably
> leave out the extra node and mark the chipcommon device itself as
> a 'gpio-controller'. It can also be other things at the same time,
> e.g. 'interrupt-controller' or provide things like pwms or pinctrl
> if that's what the hardware does. No need to have separate nodes
> for those if it's all the same register set.

OK


>> @@ -218,6 +218,11 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
>>  #if IS_BUILTIN(CONFIG_BCM47XX)
>>       chip->to_irq            = bcma_gpio_to_irq;
>>  #endif
>> +#if IS_BUILTIN(CONFIG_OF)
>> +     if (cc->core->bus->hosttype == BCMA_HOSTTYPE_SOC)
>> +             chip->of_node   = of_find_compatible_node(NULL, NULL,
>> +                                                       "brcm,bus-gpio");
>> +#endif
>
> Just commenting on the general style here, I think you can skip this
> step entirely, as mentioned above.
>
> You should use C syntax here:
>
>         if (IS_BUILTIN(CONFIG_OF) && cc->core->bus->hosttype == BCMA_HOSTTYPE_SOC))
>                 chip->of_node = of_find_compatible_node(NULL, NULL, "brcm,bus-gpio");

OK


> If CONFIG_OF is not set, of_find_compatible_node() is turned into an inline
> function that returns NULL, so you probably don't even need that.

But I need to have of_node defined. Please check out "struct gpio_chip".


> Finally, of_find_compatible_node() is a really slow operation, and "brcm,bus-gpio"
> is a much too generic name. If you need to look for something that is a child
> node, use something based on for_each_available_child_of_node().

Will see what I can do.

-- 
Rafa?

[PATCH V2] bcma: use device from DT (brcm, bus-gpio) for SoC GPIO chip

[Arnd Bergmann]

On Tuesday 30 September 2014 11:56:20 Rafa? Mi?ecki wrote:
> On 30 September 2014 11:37, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Sunday 28 September 2014 10:24:01 Rafa? Mi?ecki wrote:
> >> @@ -17,4 +33,12 @@ Example:
> >>               ranges = <0x00000000 0x18000000 0x00100000>;
> >>               #address-cells = <1>;
> >>               #size-cells = <1>;
> >> +
> >> +             chipcommon at 0 {
> >> +                     gpio at 0 {
> >> +                             compatible = "brcm,bus-gpio";
> >> +                             gpio-controller;
> >> +                             #gpio-cells = <2>;
> >> +                     };
> >> +             };
> >
> > I think you should list the 'reg' property of the chipcommon area
> > and make that match the unit address, rather than using '0', mostly
> > for consistency.
> 
> Do you mean this chipcommon at 0? We propose this foo at offset syntax since
> V1 which was posted 1,5 month ago, it's nothing new.

Is 'chipcommon' at the start of the 0x18000000 range? If so, that's
great, but it would still be good to have the 'reg' property in there
explicitly.

> >> @@ -218,6 +218,11 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
> >>  #if IS_BUILTIN(CONFIG_BCM47XX)
> >>       chip->to_irq            = bcma_gpio_to_irq;
> >>  #endif
> >> +#if IS_BUILTIN(CONFIG_OF)
> >> +     if (cc->core->bus->hosttype == BCMA_HOSTTYPE_SOC)
> >> +             chip->of_node   = of_find_compatible_node(NULL, NULL,
> >> +                                                       "brcm,bus-gpio");
> >> +#endif
> >
> > Just commenting on the general style here, I think you can skip this
> > step entirely, as mentioned above.
> >
> > You should use C syntax here:
> >
> >         if (IS_BUILTIN(CONFIG_OF) && cc->core->bus->hosttype == BCMA_HOSTTYPE_SOC))
> >                 chip->of_node = of_find_compatible_node(NULL, NULL, "brcm,bus-gpio");
> 
> OK
> 
> 
> > If CONFIG_OF is not set, of_find_compatible_node() is turned into an inline
> > function that returns NULL, so you probably don't even need that.
> 
> But I need to have of_node defined. Please check out "struct gpio_chip".

I see. This is something we might want to change, but you don't need to
bother with it now, so just leave this part as it is.

> > Finally, of_find_compatible_node() is a really slow operation, and "brcm,bus-gpio"
> > is a much too generic name. If you need to look for something that is a child
> > node, use something based on for_each_available_child_of_node().
> 
> Will see what I can do.

ok, thanks.

	Arnd

[PATCH V3] bcma: use device from DT (brcm, bus-chipcommon) for SoC GPIO chip

[Rafał Miłecki]

This will allow us to define GPIO-attached devices (LEDs, buttons) in
the the device tree.

Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com>
---
This is based on top of
[PATCH v6] bcma: register bcma as device tree driver
that I hope will reach wireless-next git tree.

V2: Describe axi chilren and make gpio a child of chipcommon core.
V3: Make chipcommon a GPIO controller (avoid extra sub-child)
    Speed up finding OF node in driver_gpio.c
---
 Documentation/devicetree/bindings/bus/bcma.txt | 13 +++++++++++++
 drivers/bcma/driver_gpio.c                     |  6 ++++++
 2 files changed, 19 insertions(+)

diff --git a/Documentation/devicetree/bindings/bus/bcma.txt b/Documentation/devicetree/bindings/bus/bcma.txt
index e9070c1..0538692 100644
--- a/Documentation/devicetree/bindings/bus/bcma.txt
+++ b/Documentation/devicetree/bindings/bus/bcma.txt
@@ -9,6 +9,11 @@ Required properties:
 The cores on the AXI bus are automatically detected by bcma with the
 memory ranges they are using and they get registered afterwards.
 
+The top-level axi bus may contain children representing attached cores
+(devices). This is needed since some hardware details can't be auto
+detected (e.g. IRQ numbers). Also some of the cores may be responsible
+for extra things, e.g. ChipCommon providing access to the GPIO chip.
+
 Example:
 
 	axi at 18000000 {
@@ -17,4 +22,12 @@ Example:
 		ranges = <0x00000000 0x18000000 0x00100000>;
 		#address-cells = <1>;
 		#size-cells = <1>;
+
+		chipcommon {
+			compatible = "brcm,bus-chipcommon";
+			reg = <0x00000000 0x1000>;
+
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
 	};
diff --git a/drivers/bcma/driver_gpio.c b/drivers/bcma/driver_gpio.c
index 8ea497c..28bdbe5 100644
--- a/drivers/bcma/driver_gpio.c
+++ b/drivers/bcma/driver_gpio.c
@@ -218,6 +218,12 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
 #if IS_BUILTIN(CONFIG_BCM47XX)
 	chip->to_irq		= bcma_gpio_to_irq;
 #endif
+#if IS_BUILTIN(CONFIG_OF)
+	if (cc->core->bus->hosttype == BCMA_HOSTTYPE_SOC)
+		chip->of_node	= of_find_compatible_node(
+					bus->host_pdev->dev.of_node, NULL,
+					"brcm,bus-chipcommon");
+#endif
 	switch (cc->core->bus->chipinfo.id) {
 	case BCMA_CHIP_ID_BCM5357:
 	case BCMA_CHIP_ID_BCM53572:
-- 
1.8.4.5

[PATCH V3] bcma: use device from DT (brcm, bus-chipcommon) for SoC GPIO chip

[Arnd Bergmann]

On Tuesday 30 September 2014 12:22:26 Rafa? Mi?ecki wrote:
> 
> +The top-level axi bus may contain children representing attached cores
> +(devices). This is needed since some hardware details can't be auto
> +detected (e.g. IRQ numbers). Also some of the cores may be responsible
> +for extra things, e.g. ChipCommon providing access to the GPIO chip.
> +
>  Example:
>  
>         axi at 18000000 {
> @@ -17,4 +22,12 @@ Example:
>                 ranges = <0x00000000 0x18000000 0x00100000>;
>                 #address-cells = <1>;
>                 #size-cells = <1>;
> +
> +               chipcommon {
> +                       compatible = "brcm,bus-chipcommon";
> +                       reg = <0x00000000 0x1000>;
> +
> +                       gpio-controller;
> +                       #gpio-cells = <2>;
> +               };
>         };

Looks good.


> diff --git a/drivers/bcma/driver_gpio.c b/drivers/bcma/driver_gpio.c
> index 8ea497c..28bdbe5 100644
> --- a/drivers/bcma/driver_gpio.c
> +++ b/drivers/bcma/driver_gpio.c
> @@ -218,6 +218,12 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
>  #if IS_BUILTIN(CONFIG_BCM47XX)
>         chip->to_irq            = bcma_gpio_to_irq;
>  #endif
> +#if IS_BUILTIN(CONFIG_OF)
> +       if (cc->core->bus->hosttype == BCMA_HOSTTYPE_SOC)
> +               chip->of_node   = of_find_compatible_node(
> +                                       bus->host_pdev->dev.of_node, NULL,
> +                                       "brcm,bus-chipcommon");
> +#endif
>         switch (cc->core->bus->chipinfo.id) {

This doesn't: you are now searching through all nodes starting at the
axi node rather than searching just through the children.

I think it would be better with the first change in place to set
chip->of_node to cc->core->dev.of_node, and set that pointer in
bcma_bus_scan by matching the 'reg' number. I think that is what
an earlier version of the bcma DT support did in order to find the
IRQs. We no longer need it for that purpose, but it seems like a
good idea anyway, as I expect other bcma_devices to have similar
requirements to add additional properties.

	Arnd

[PATCH V3] bcma: use device from DT (brcm, bus-chipcommon) for SoC GPIO chip

[Rafał Miłecki]

On 30 September 2014 12:36, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 30 September 2014 12:22:26 Rafa? Mi?ecki wrote:
>> @@ -218,6 +218,12 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
>>  #if IS_BUILTIN(CONFIG_BCM47XX)
>>         chip->to_irq            = bcma_gpio_to_irq;
>>  #endif
>> +#if IS_BUILTIN(CONFIG_OF)
>> +       if (cc->core->bus->hosttype == BCMA_HOSTTYPE_SOC)
>> +               chip->of_node   = of_find_compatible_node(
>> +                                       bus->host_pdev->dev.of_node, NULL,
>> +                                       "brcm,bus-chipcommon");
>> +#endif
>>         switch (cc->core->bus->chipinfo.id) {
>
> This doesn't: you are now searching through all nodes starting at the
> axi node rather than searching just through the children.
>
> I think it would be better with the first change in place to set
> chip->of_node to cc->core->dev.of_node, and set that pointer in
> bcma_bus_scan by matching the 'reg' number. I think that is what
> an earlier version of the bcma DT support did in order to find the
> IRQs. We no longer need it for that purpose, but it seems like a
> good idea anyway, as I expect other bcma_devices to have similar
> requirements to add additional properties.

Ohh, of course, I didn't notice that
[PATCH v6] bcma: register bcma as device tree driver
already contains bcma_of_fill_device. This will simplify my search a lot!

-- 
Rafa?

[PATCH V4] bcma: use chipcommon node from DT for SoC GPIO chip

[Rafał Miłecki]

This will allow us to define GPIO-attached devices (LEDs, buttons) in
the the device tree.

Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com>
---
This is based on top of
[PATCH v6] bcma: register bcma as device tree driver
that I hope will reach wireless-next git tree.

V2: Describe axi chilren and make gpio a child of chipcommon core.
V3: Make chipcommon a GPIO controller (avoid extra sub-child)
    Speed up finding OF node in driver_gpio.c
V4: Simplify setting of_node in driver_gpio.c
---
 Documentation/devicetree/bindings/bus/bcma.txt | 12 ++++++++++++
 drivers/bcma/driver_gpio.c                     |  4 ++++
 2 files changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/bus/bcma.txt b/Documentation/devicetree/bindings/bus/bcma.txt
index e9070c1..62a4834 100644
--- a/Documentation/devicetree/bindings/bus/bcma.txt
+++ b/Documentation/devicetree/bindings/bus/bcma.txt
@@ -9,6 +9,11 @@ Required properties:
 The cores on the AXI bus are automatically detected by bcma with the
 memory ranges they are using and they get registered afterwards.
 
+The top-level axi bus may contain children representing attached cores
+(devices). This is needed since some hardware details can't be auto
+detected (e.g. IRQ numbers). Also some of the cores may be responsible
+for extra things, e.g. ChipCommon providing access to the GPIO chip.
+
 Example:
 
 	axi at 18000000 {
@@ -17,4 +22,11 @@ Example:
 		ranges = <0x00000000 0x18000000 0x00100000>;
 		#address-cells = <1>;
 		#size-cells = <1>;
+
+		chipcommon {
+			reg = <0x00000000 0x1000>;
+
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
 	};
diff --git a/drivers/bcma/driver_gpio.c b/drivers/bcma/driver_gpio.c
index 8ea497c..57ce5fe 100644
--- a/drivers/bcma/driver_gpio.c
+++ b/drivers/bcma/driver_gpio.c
@@ -218,6 +218,10 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
 #if IS_BUILTIN(CONFIG_BCM47XX)
 	chip->to_irq		= bcma_gpio_to_irq;
 #endif
+#if IS_BUILTIN(CONFIG_OF)
+	if (cc->core->bus->hosttype == BCMA_HOSTTYPE_SOC)
+		chip->of_node	= cc->core->dev.of_node;
+#endif
 	switch (cc->core->bus->chipinfo.id) {
 	case BCMA_CHIP_ID_BCM5357:
 	case BCMA_CHIP_ID_BCM53572:
-- 
1.8.4.5

[PATCH V4] bcma: use chipcommon node from DT for SoC GPIO chip

[Arnd Bergmann]

On Tuesday 30 September 2014 12:55:48 Rafa? Mi?ecki wrote:
> This will allow us to define GPIO-attached devices (LEDs, buttons) in
> the the device tree.
> 
> Signed-off-by: Rafa? Mi?ecki <zajec5@gmail.com>
> ---
> This is based on top of
> [PATCH v6] bcma: register bcma as device tree driver
> that I hope will reach wireless-next git tree.
> 
> V2: Describe axi chilren and make gpio a child of chipcommon core.
> V3: Make chipcommon a GPIO controller (avoid extra sub-child)
>     Speed up finding OF node in driver_gpio.c
> V4: Simplify setting of_node in driver_gpio.c
> 

Acked-by: Arnd Bergmann <arnd@arndb.de>