linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/1] fix node name in the brcm,bcm43xx-fmac.txt example
@ 2017-05-15 20:13 Martin Blumenstingl
  2017-05-15 20:13 ` [PATCH 1/1] dt-binding: net: wireless: fix node name in the BCM43xx example Martin Blumenstingl
  0 siblings, 1 reply; 8+ messages in thread
From: Martin Blumenstingl @ 2017-05-15 20:13 UTC (permalink / raw)
  To: kvalo, robh+dt, mark.rutland, devicetree
  Cc: linux-wireless, netdev, Martin Blumenstingl

recently there were some negative comments about the quality of
code-reviews for new .dts additions. one issue that came up was that the
node for the Broadcom FullMAC wireless SDIO devices was named "brcmf"
instead of "wifi".

This patch tries to fix (one of) the root cause(s), which is that .dts
authors copy the example from the documentation.
unfortunately there are still many .dts files out there which use
"brmcf" as node name - so any new addition of a Broadcom FullMAC SDIO
wireless device should be reviewed carefully regarding the node name
(just in case a .dts author copied from another .dts which still uses
the "wrong" node name).

Martin Blumenstingl (1):
  dt-binding: net: wireless: fix node name in the BCM43xx example

 Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.13.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/1] dt-binding: net: wireless: fix node name in the BCM43xx example
  2017-05-15 20:13 [PATCH 0/1] fix node name in the brcm,bcm43xx-fmac.txt example Martin Blumenstingl
@ 2017-05-15 20:13 ` Martin Blumenstingl
  2017-05-15 22:05   ` Arend Van Spriel
                     ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Martin Blumenstingl @ 2017-05-15 20:13 UTC (permalink / raw)
  To: kvalo, robh+dt, mark.rutland, devicetree
  Cc: linux-wireless, netdev, Martin Blumenstingl

The example in the BCM43xx documentation uses "brcmf" as node name.
However, wireless devices should be named "wifi" instead. Fix this to
make sure that .dts authors can simply use the documentation as
reference (or simply copy the node from the documentation and then
adjust only the board specific bits).

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
index 5dbf169cd81c..590f622188de 100644
--- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
+++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
@@ -31,7 +31,7 @@ mmc3: mmc@01c12000 {
 	non-removable;
 	status = "okay";
 
-	brcmf: bcrmf@1 {
+	brcmf: wifi@1 {
 		reg = <1>;
 		compatible = "brcm,bcm4329-fmac";
 		interrupt-parent = <&pio>;
-- 
2.13.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] dt-binding: net: wireless: fix node name in the BCM43xx example
  2017-05-15 20:13 ` [PATCH 1/1] dt-binding: net: wireless: fix node name in the BCM43xx example Martin Blumenstingl
@ 2017-05-15 22:05   ` Arend Van Spriel
  2017-05-16 19:56     ` Martin Blumenstingl
  2017-05-19  1:56   ` Rob Herring
  2017-05-21 14:54   ` Andreas Färber
  2 siblings, 1 reply; 8+ messages in thread
From: Arend Van Spriel @ 2017-05-15 22:05 UTC (permalink / raw)
  To: Martin Blumenstingl, kvalo, robh+dt, mark.rutland, devicetree
  Cc: linux-wireless, netdev

On 15-5-2017 22:13, Martin Blumenstingl wrote:
> The example in the BCM43xx documentation uses "brcmf" as node name.
> However, wireless devices should be named "wifi" instead. Fix this to

Hi Martin,

Since when is that a rule. I never got the memo and the DTC did not ever
complain to me about the naming. That being said I do not really care
and I suppose it is for the sake of consistency only.

> make sure that .dts authors can simply use the documentation as
> reference (or simply copy the node from the documentation and then
> adjust only the board specific bits).

Please feel free to add my...

Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> index 5dbf169cd81c..590f622188de 100644
> --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> @@ -31,7 +31,7 @@ mmc3: mmc@01c12000 {
>  	non-removable;
>  	status = "okay";
>  
> -	brcmf: bcrmf@1 {
> +	brcmf: wifi@1 {
>  		reg = <1>;
>  		compatible = "brcm,bcm4329-fmac";
>  		interrupt-parent = <&pio>;
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] dt-binding: net: wireless: fix node name in the BCM43xx example
  2017-05-15 22:05   ` Arend Van Spriel
@ 2017-05-16 19:56     ` Martin Blumenstingl
  2017-05-21 14:19       ` Andreas Färber
  0 siblings, 1 reply; 8+ messages in thread
From: Martin Blumenstingl @ 2017-05-16 19:56 UTC (permalink / raw)
  To: Arend Van Spriel, robh+dt
  Cc: kvalo, mark.rutland, devicetree, linux-wireless, netdev

Hi Arend,

On Tue, May 16, 2017 at 12:05 AM, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 15-5-2017 22:13, Martin Blumenstingl wrote:
>> The example in the BCM43xx documentation uses "brcmf" as node name.
>> However, wireless devices should be named "wifi" instead. Fix this to
>
> Hi Martin,
>
> Since when is that a rule. I never got the memo and the DTC did not ever
> complain to me about the naming. That being said I do not really care
> and I suppose it is for the sake of consistency only.
I'm not sure if it's actually a rule or (as you already noted) just
for consistency. back when I added devicetree support to ath9k Rob
pointed out that the node should be named "wifi" (instead of "ath9k"),
see [0]

>> make sure that .dts authors can simply use the documentation as
>> reference (or simply copy the node from the documentation and then
>> adjust only the board specific bits).
>
> Please feel free to add my...
>
> Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
thank you!

@Rob: maybe you can ACK this as well if you're fine with this patch?

>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> ---
>>  Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>> index 5dbf169cd81c..590f622188de 100644
>> --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
>> @@ -31,7 +31,7 @@ mmc3: mmc@01c12000 {
>>       non-removable;
>>       status = "okay";
>>
>> -     brcmf: bcrmf@1 {
>> +     brcmf: wifi@1 {
>>               reg = <1>;
>>               compatible = "brcm,bcm4329-fmac";
>>               interrupt-parent = <&pio>;
>>

[0] http://www.mail-archive.com/ath9k-devel@lists.ath9k.org/msg14678.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] dt-binding: net: wireless: fix node name in the BCM43xx example
  2017-05-15 20:13 ` [PATCH 1/1] dt-binding: net: wireless: fix node name in the BCM43xx example Martin Blumenstingl
  2017-05-15 22:05   ` Arend Van Spriel
@ 2017-05-19  1:56   ` Rob Herring
  2017-05-21 14:54   ` Andreas Färber
  2 siblings, 0 replies; 8+ messages in thread
From: Rob Herring @ 2017-05-19  1:56 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: kvalo, mark.rutland, devicetree, linux-wireless, netdev

On Mon, May 15, 2017 at 10:13:56PM +0200, Martin Blumenstingl wrote:
> The example in the BCM43xx documentation uses "brcmf" as node name.
> However, wireless devices should be named "wifi" instead. Fix this to
> make sure that .dts authors can simply use the documentation as
> reference (or simply copy the node from the documentation and then
> adjust only the board specific bits).
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied.

Rob

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] dt-binding: net: wireless: fix node name in the BCM43xx example
  2017-05-16 19:56     ` Martin Blumenstingl
@ 2017-05-21 14:19       ` Andreas Färber
  2017-05-22  9:37         ` Arend van Spriel
  0 siblings, 1 reply; 8+ messages in thread
From: Andreas Färber @ 2017-05-21 14:19 UTC (permalink / raw)
  To: Martin Blumenstingl, Arend Van Spriel
  Cc: robh+dt, kvalo, mark.rutland, devicetree, linux-wireless, netdev

Hi,

Am 16.05.2017 um 21:56 schrieb Martin Blumenstingl:
> On Tue, May 16, 2017 at 12:05 AM, Arend Van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>> On 15-5-2017 22:13, Martin Blumenstingl wrote:
>>> The example in the BCM43xx documentation uses "brcmf" as node name.
>>> However, wireless devices should be named "wifi" instead. Fix this to
>>
>> Since when is that a rule. I never got the memo and the DTC did not ever
>> complain to me about the naming.

How do you expect it to? Maintain a blacklist of every device model
someone might use, including all typo variations?

> That being said I do not really care
>> and I suppose it is for the sake of consistency only.
> I'm not sure if it's actually a rule or (as you already noted) just
> for consistency. back when I added devicetree support to ath9k Rob
> pointed out that the node should be named "wifi" (instead of "ath9k"),
> see [0]

The general rule is that the node name should be the type of the device,
not duplicate its compatible string.

For consistency Rob was asking we use "wifi" as node name.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] dt-binding: net: wireless: fix node name in the BCM43xx example
  2017-05-15 20:13 ` [PATCH 1/1] dt-binding: net: wireless: fix node name in the BCM43xx example Martin Blumenstingl
  2017-05-15 22:05   ` Arend Van Spriel
  2017-05-19  1:56   ` Rob Herring
@ 2017-05-21 14:54   ` Andreas Färber
  2 siblings, 0 replies; 8+ messages in thread
From: Andreas Färber @ 2017-05-21 14:54 UTC (permalink / raw)
  To: Martin Blumenstingl, kvalo, robh+dt, mark.rutland, devicetree
  Cc: linux-wireless, netdev, linux-arm-kernel, Maxime Ripard,
	Chen-Yu Tsai, Shawn Guo, Sascha Hauer, Fabio Estevam

Am 15.05.2017 um 22:13 schrieb Martin Blumenstingl:
> The example in the BCM43xx documentation uses "brcmf" as node name.

No, it doesn't, it uses "bcrmf".

This typo has spread to all ARM device trees:

$ git grep bcrmf -- arch/arm/boot/dts/
arch/arm/boot/dts/imx6sx-nitrogen6sx.dts:       brcmf: bcrmf@1 {
arch/arm/boot/dts/imx6ul-opos6ul.dtsi:  brcmf: bcrmf@1 {
arch/arm/boot/dts/sun6i-a31s-sinovoip-bpi-m2.dts:       brcmf: bcrmf@1 {
arch/arm/boot/dts/sun7i-a20-bananapi-m1-plus.dts:       brcmf: bcrmf@1 {
arch/arm/boot/dts/sun7i-a20-bananapro.dts:      brcmf: bcrmf@1 {
arch/arm/boot/dts/sun7i-a20-cubietruck.dts:     brcmf: bcrmf@1 {
arch/arm/boot/dts/sun7i-a20-i12-tvbox.dts:      brcmf: bcrmf@1 {
arch/arm/boot/dts/sun7i-a20-wits-pro-a20-dkt.dts:       brcmf: bcrmf@1 {
arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts:        brcmf: bcrmf@1 {

For arch/arm64/boot/dts/amlogic/* I've already fixed it, originally by
changing to "brcmf", in a second step "wifi" was requested.

> However, wireless devices should be named "wifi" instead. Fix this to
> make sure that .dts authors can simply use the documentation as
> reference (or simply copy the node from the documentation and then
> adjust only the board specific bits).
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> index 5dbf169cd81c..590f622188de 100644
> --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> @@ -31,7 +31,7 @@ mmc3: mmc@01c12000 {
>  	non-removable;
>  	status = "okay";
>  
> -	brcmf: bcrmf@1 {
> +	brcmf: wifi@1 {
>  		reg = <1>;
>  		compatible = "brcm,bcm4329-fmac";
>  		interrupt-parent = <&pio>;

Thanks for fixing this at its source.

Hopefully the maintainers in CC can make sure that at least it doesn't
spread further into new DTs.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] dt-binding: net: wireless: fix node name in the BCM43xx example
  2017-05-21 14:19       ` Andreas Färber
@ 2017-05-22  9:37         ` Arend van Spriel
  0 siblings, 0 replies; 8+ messages in thread
From: Arend van Spriel @ 2017-05-22  9:37 UTC (permalink / raw)
  To: Andreas Färber, Martin Blumenstingl
  Cc: robh+dt, kvalo, mark.rutland, devicetree, linux-wireless, netdev

On 5/21/2017 4:19 PM, Andreas Färber wrote:
> Hi,
> 
> Am 16.05.2017 um 21:56 schrieb Martin Blumenstingl:
>> On Tue, May 16, 2017 at 12:05 AM, Arend Van Spriel
>> <arend.vanspriel@broadcom.com> wrote:
>>> On 15-5-2017 22:13, Martin Blumenstingl wrote:
>>>> The example in the BCM43xx documentation uses "brcmf" as node name.
>>>> However, wireless devices should be named "wifi" instead. Fix this to
>>>
>>> Since when is that a rule. I never got the memo and the DTC did not ever
>>> complain to me about the naming.
> 
> How do you expect it to? Maintain a blacklist of every device model
> someone might use, including all typo variations?

Not really why I was asking it. Just saying the node name is trivial as 
I don't think there is different kernel behaviour depending on the node 
name.

>> That being said I do not really care
>>> and I suppose it is for the sake of consistency only.
>> I'm not sure if it's actually a rule or (as you already noted) just
>> for consistency. back when I added devicetree support to ath9k Rob
>> pointed out that the node should be named "wifi" (instead of "ath9k"),
>> see [0]
> 
> The general rule is that the node name should be the type of the device,
> not duplicate its compatible string.
> 
> For consistency Rob was asking we use "wifi" as node name.

Fine with that. Not sure how long ago it was that I added this binding, 
but DT folks were involved back than. I never looked back so I should 
not be surprised with new consistency rules. I was just curious about 
the story behind it.

Thanks,
Arend

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-05-22  9:38 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-15 20:13 [PATCH 0/1] fix node name in the brcm,bcm43xx-fmac.txt example Martin Blumenstingl
2017-05-15 20:13 ` [PATCH 1/1] dt-binding: net: wireless: fix node name in the BCM43xx example Martin Blumenstingl
2017-05-15 22:05   ` Arend Van Spriel
2017-05-16 19:56     ` Martin Blumenstingl
2017-05-21 14:19       ` Andreas Färber
2017-05-22  9:37         ` Arend van Spriel
2017-05-19  1:56   ` Rob Herring
2017-05-21 14:54   ` Andreas Färber

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).