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
Hi Arend,
On Tue, May 16, 2017 at 12:05 AM, Arend Van Spriel
<[email protected]> 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 <[email protected]>
thank you!
@Rob: maybe you can ACK this as well if you're fine with this patch?
>> Signed-off-by: Martin Blumenstingl <[email protected]>
>> ---
>> 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/[email protected]/msg14678.html
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 <[email protected]>
---
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
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 <[email protected]>
> Signed-off-by: Martin Blumenstingl <[email protected]>
> ---
> 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>;
>
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 <[email protected]>
> ---
> Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Applied.
Rob
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 <[email protected]>
> ---
> 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)
Hi,
Am 16.05.2017 um 21:56 schrieb Martin Blumenstingl:
> On Tue, May 16, 2017 at 12:05 AM, Arend Van Spriel
> <[email protected]> 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)
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
>> <[email protected]> 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