diff mbox series

[v2,3/9] arm64: dts: bcmbca: update BCM4908 board dts files

Message ID 20220725055402.6013-4-william.zhang@broadcom.com (mailing list archive)
State New, archived
Headers show
Series arm64: bcmbca: Move BCM4908 SoC support under ARCH_BCMBCA | expand

Commit Message

William Zhang July 25, 2022, 5:53 a.m. UTC 
Append "brcm,bcmbca" to compatible strings based on the new bcmbca
binding rule for BCM4908 family based boards. This will break drivers
that use the old compatible string for binding. Fortunately there is no
such usage in linux and u-boot.

Signed-off-by: William Zhang <william.zhang@broadcom.com>
Acked-by: Rafał Miłecki <rafal@milecki.pl>

---

Changes in v2:
- Add Acked-by tag

 arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-netgear-r8000p.dts | 2 +-
 .../dts/broadcom/bcm4908/bcm4906-tplink-archer-c2300-v1.dts     | 2 +-
 arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-asus-gt-ac5300.dts | 2 +-
 .../arm64/boot/dts/broadcom/bcm4908/bcm4908-netgear-raxe500.dts | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

Comments

Rob Herring (Arm) July 25, 2022, 11:32 p.m. UTC | #1 
On Sun, Jul 24, 2022 at 10:53:56PM -0700, William Zhang wrote:
> Append "brcm,bcmbca" to compatible strings based on the new bcmbca
> binding rule for BCM4908 family based boards. This will break drivers
> that use the old compatible string for binding. Fortunately there is no
> such usage in linux and u-boot.

How does adding an additional compatible break things?

> 
> Signed-off-by: William Zhang <william.zhang@broadcom.com>
> Acked-by: Rafał Miłecki <rafal@milecki.pl>
> 
> ---
> 
> Changes in v2:
> - Add Acked-by tag
> 
>  arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-netgear-r8000p.dts | 2 +-
>  .../dts/broadcom/bcm4908/bcm4906-tplink-archer-c2300-v1.dts     | 2 +-
>  arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-asus-gt-ac5300.dts | 2 +-
>  .../arm64/boot/dts/broadcom/bcm4908/bcm4908-netgear-raxe500.dts | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-netgear-r8000p.dts b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-netgear-r8000p.dts
> index 2dd028438c22..d8b60575eb4f 100644
> --- a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-netgear-r8000p.dts
> +++ b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-netgear-r8000p.dts
> @@ -7,7 +7,7 @@
>  #include "bcm4906.dtsi"
>  
>  / {
> -	compatible = "netgear,r8000p", "brcm,bcm4906", "brcm,bcm4908";
> +	compatible = "netgear,r8000p", "brcm,bcm4906", "brcm,bcm4908", "brcm,bcmbca";
>  	model = "Netgear R8000P";
>  
>  	memory@0 {
> diff --git a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-tplink-archer-c2300-v1.dts b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-tplink-archer-c2300-v1.dts
> index 064f7f549665..296393d4aaab 100644
> --- a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-tplink-archer-c2300-v1.dts
> +++ b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-tplink-archer-c2300-v1.dts
> @@ -7,7 +7,7 @@
>  #include "bcm4906.dtsi"
>  
>  / {
> -	compatible = "tplink,archer-c2300-v1", "brcm,bcm4906", "brcm,bcm4908";
> +	compatible = "tplink,archer-c2300-v1", "brcm,bcm4906", "brcm,bcm4908", "brcm,bcmbca";
>  	model = "TP-Link Archer C2300 V1";
>  
>  	memory@0 {
> diff --git a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-asus-gt-ac5300.dts b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-asus-gt-ac5300.dts
> index 04f8524b5335..787c7ddf9102 100644
> --- a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-asus-gt-ac5300.dts
> +++ b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-asus-gt-ac5300.dts
> @@ -6,7 +6,7 @@
>  #include "bcm4908.dtsi"
>  
>  / {
> -	compatible = "asus,gt-ac5300", "brcm,bcm4908";
> +	compatible = "asus,gt-ac5300", "brcm,bcm4908", "brcm,bcmbca";
>  	model = "Asus GT-AC5300";
>  
>  	memory@0 {
> diff --git a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-netgear-raxe500.dts b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-netgear-raxe500.dts
> index 3c2cf2d238b6..23b96c663239 100644
> --- a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-netgear-raxe500.dts
> +++ b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-netgear-raxe500.dts
> @@ -3,7 +3,7 @@
>  #include "bcm4908.dtsi"
>  
>  / {
> -	compatible = "netgear,raxe500", "brcm,bcm4908";
> +	compatible = "netgear,raxe500", "brcm,bcm4908", "brcm,bcmbca";
>  	model = "Netgear RAXE500";
>  
>  	memory@0 {
> -- 
> 2.34.1
>
William Zhang July 26, 2022, 1:09 a.m. UTC | #2 
Hi Rob,

On 07/25/2022 04:32 PM, Rob Herring wrote:
> On Sun, Jul 24, 2022 at 10:53:56PM -0700, William Zhang wrote:
>> Append "brcm,bcmbca" to compatible strings based on the new bcmbca
>> binding rule for BCM4908 family based boards. This will break drivers
>> that use the old compatible string for binding. Fortunately there is no
>> such usage in linux and u-boot.
> 
> How does adding an additional compatible break things?
> In theory when some crazy code tries to match the entire string. But not 
in linux, u-boot code and hopefully not in other bootloader and Os does 
that. But this does change an existing compatible string so Krzysztof 
suggested to add comment about the breakage in the commit message. I can 
remove this and send v3 if you guys think it is necessary.

>>
>> Signed-off-by: William Zhang <william.zhang@broadcom.com>
>> Acked-by: Rafał Miłecki <rafal@milecki.pl>
>>
>> ---
>>
>> Changes in v2:
>> - Add Acked-by tag
>>
>>   arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-netgear-r8000p.dts | 2 +-
>>   .../dts/broadcom/bcm4908/bcm4906-tplink-archer-c2300-v1.dts     | 2 +-
>>   arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-asus-gt-ac5300.dts | 2 +-
>>   .../arm64/boot/dts/broadcom/bcm4908/bcm4908-netgear-raxe500.dts | 2 +-
>>   4 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-netgear-r8000p.dts b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-netgear-r8000p.dts
>> index 2dd028438c22..d8b60575eb4f 100644
>> --- a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-netgear-r8000p.dts
>> +++ b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-netgear-r8000p.dts
>> @@ -7,7 +7,7 @@
>>   #include "bcm4906.dtsi"
>>   
>>   / {
>> -	compatible = "netgear,r8000p", "brcm,bcm4906", "brcm,bcm4908";
>> +	compatible = "netgear,r8000p", "brcm,bcm4906", "brcm,bcm4908", "brcm,bcmbca";
>>   	model = "Netgear R8000P";
>>   
>>   	memory@0 {
>> diff --git a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-tplink-archer-c2300-v1.dts b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-tplink-archer-c2300-v1.dts
>> index 064f7f549665..296393d4aaab 100644
>> --- a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-tplink-archer-c2300-v1.dts
>> +++ b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-tplink-archer-c2300-v1.dts
>> @@ -7,7 +7,7 @@
>>   #include "bcm4906.dtsi"
>>   
>>   / {
>> -	compatible = "tplink,archer-c2300-v1", "brcm,bcm4906", "brcm,bcm4908";
>> +	compatible = "tplink,archer-c2300-v1", "brcm,bcm4906", "brcm,bcm4908", "brcm,bcmbca";
>>   	model = "TP-Link Archer C2300 V1";
>>   
>>   	memory@0 {
>> diff --git a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-asus-gt-ac5300.dts b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-asus-gt-ac5300.dts
>> index 04f8524b5335..787c7ddf9102 100644
>> --- a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-asus-gt-ac5300.dts
>> +++ b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-asus-gt-ac5300.dts
>> @@ -6,7 +6,7 @@
>>   #include "bcm4908.dtsi"
>>   
>>   / {
>> -	compatible = "asus,gt-ac5300", "brcm,bcm4908";
>> +	compatible = "asus,gt-ac5300", "brcm,bcm4908", "brcm,bcmbca";
>>   	model = "Asus GT-AC5300";
>>   
>>   	memory@0 {
>> diff --git a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-netgear-raxe500.dts b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-netgear-raxe500.dts
>> index 3c2cf2d238b6..23b96c663239 100644
>> --- a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-netgear-raxe500.dts
>> +++ b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-netgear-raxe500.dts
>> @@ -3,7 +3,7 @@
>>   #include "bcm4908.dtsi"
>>   
>>   / {
>> -	compatible = "netgear,raxe500", "brcm,bcm4908";
>> +	compatible = "netgear,raxe500", "brcm,bcm4908", "brcm,bcmbca";
>>   	model = "Netgear RAXE500";
>>   
>>   	memory@0 {
>> -- 
>> 2.34.1
>>
> 
>
Rafał Miłecki July 27, 2022, 10:39 a.m. UTC | #3 
On 2022-07-26 03:09, William Zhang wrote:
> On 07/25/2022 04:32 PM, Rob Herring wrote:
>> On Sun, Jul 24, 2022 at 10:53:56PM -0700, William Zhang wrote:
>>> Append "brcm,bcmbca" to compatible strings based on the new bcmbca
>>> binding rule for BCM4908 family based boards. This will break drivers
>>> that use the old compatible string for binding. Fortunately there is 
>>> no
>>> such usage in linux and u-boot.
>> 
>> How does adding an additional compatible break things?
>> In theory when some crazy code tries to match the entire string. But 
>> not
> in linux, u-boot code and hopefully not in other bootloader and Os
> does that. But this does change an existing compatible string so
> Krzysztof suggested to add comment about the breakage in the commit
> message. I can remove this and send v3 if you guys think it is
> necessary.

Krzysztof commented on ABI breakage [1] when you tried removing
"brcm,bcm4908" from the "compatible" list in your patch
[RFC PATCH 3/3] arm64: dts: bcmbca: update bcm4808 board dts file [2]

In this version of your patch you don't remove "brcm,bcm4908" anymore so
this change doesn't break anything. Adding a new "compatible" string
doesn't break things. You can remove that info from the commit message.

[1] 
https://lore.kernel.org/linux-arm-kernel/d93e55fa-3359-2609-aad5-c80eca78f380@linaro.org/
[2] 
https://lore.kernel.org/linux-arm-kernel/20220712021144.7068-4-william.zhang@broadcom.com/
Krzysztof Kozlowski July 27, 2022, 12:12 p.m. UTC | #4 
On 27/07/2022 12:39, Rafał Miłecki wrote:
> On 2022-07-26 03:09, William Zhang wrote:
>> On 07/25/2022 04:32 PM, Rob Herring wrote:
>>> On Sun, Jul 24, 2022 at 10:53:56PM -0700, William Zhang wrote:
>>>> Append "brcm,bcmbca" to compatible strings based on the new bcmbca
>>>> binding rule for BCM4908 family based boards. This will break drivers
>>>> that use the old compatible string for binding. Fortunately there is 
>>>> no
>>>> such usage in linux and u-boot.
>>>
>>> How does adding an additional compatible break things?
>>> In theory when some crazy code tries to match the entire string. But 
>>> not
>> in linux, u-boot code and hopefully not in other bootloader and Os
>> does that. But this does change an existing compatible string so
>> Krzysztof suggested to add comment about the breakage in the commit
>> message. I can remove this and send v3 if you guys think it is
>> necessary.
> 
> Krzysztof commented on ABI breakage [1] when you tried removing
> "brcm,bcm4908" from the "compatible" list in your patch
> [RFC PATCH 3/3] arm64: dts: bcmbca: update bcm4808 board dts file [2]
> 
> In this version of your patch you don't remove "brcm,bcm4908" anymore so
> this change doesn't break anything. Adding a new "compatible" string
> doesn't break things. You can remove that info from the commit message.

Thanks... It is second thing (after not existing Reviewed-by) attributed
to me by William, although here probably by misunderstanding... So for
clarity (obvious stuff is not always obvious to everyone):
1. Removal of compatible is an ABI break.
2. Add of compatible is not an ABI break.

See also:
https://elixir.bootlin.com/linux/v5.19-rc8/source/Documentation/devicetree/bindings/ABI.rst#L26

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-netgear-r8000p.dts b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-netgear-r8000p.dts
index 2dd028438c22..d8b60575eb4f 100644
--- a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-netgear-r8000p.dts
+++ b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-netgear-r8000p.dts
@@ -7,7 +7,7 @@ 
 #include "bcm4906.dtsi"
 
 / {
-	compatible = "netgear,r8000p", "brcm,bcm4906", "brcm,bcm4908";
+	compatible = "netgear,r8000p", "brcm,bcm4906", "brcm,bcm4908", "brcm,bcmbca";
 	model = "Netgear R8000P";
 
 	memory@0 {
diff --git a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-tplink-archer-c2300-v1.dts b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-tplink-archer-c2300-v1.dts
index 064f7f549665..296393d4aaab 100644
--- a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-tplink-archer-c2300-v1.dts
+++ b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4906-tplink-archer-c2300-v1.dts
@@ -7,7 +7,7 @@ 
 #include "bcm4906.dtsi"
 
 / {
-	compatible = "tplink,archer-c2300-v1", "brcm,bcm4906", "brcm,bcm4908";
+	compatible = "tplink,archer-c2300-v1", "brcm,bcm4906", "brcm,bcm4908", "brcm,bcmbca";
 	model = "TP-Link Archer C2300 V1";
 
 	memory@0 {
diff --git a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-asus-gt-ac5300.dts b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-asus-gt-ac5300.dts
index 04f8524b5335..787c7ddf9102 100644
--- a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-asus-gt-ac5300.dts
+++ b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-asus-gt-ac5300.dts
@@ -6,7 +6,7 @@ 
 #include "bcm4908.dtsi"
 
 / {
-	compatible = "asus,gt-ac5300", "brcm,bcm4908";
+	compatible = "asus,gt-ac5300", "brcm,bcm4908", "brcm,bcmbca";
 	model = "Asus GT-AC5300";
 
 	memory@0 {
diff --git a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-netgear-raxe500.dts b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-netgear-raxe500.dts
index 3c2cf2d238b6..23b96c663239 100644
--- a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-netgear-raxe500.dts
+++ b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908-netgear-raxe500.dts
@@ -3,7 +3,7 @@ 
 #include "bcm4908.dtsi"
 
 / {
-	compatible = "netgear,raxe500", "brcm,bcm4908";
+	compatible = "netgear,raxe500", "brcm,bcm4908", "brcm,bcmbca";
 	model = "Netgear RAXE500";
 
 	memory@0 {