diff mbox series

[RFC,3/3] arm64: dts: bcmbca: update bcm4808 board dts file

Message ID 20220712021144.7068-4-william.zhang@broadcom.com (mailing list archive)
State New, archived
Headers show
Series arm: bcmbca: Update BCM4908 dts for BCMBCA merge | expand

Commit Message

William Zhang July 12, 2022, 2:11 a.m. UTC
Update compatible string based on the new bcmbca binding rule
for BCM4908 famliy based boards

Signed-off-by: William Zhang <william.zhang@broadcom.com>

---

 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

Krzysztof Kozlowski July 12, 2022, 7:47 a.m. UTC | #1
On 12/07/2022 04:11, William Zhang wrote:
> Update compatible string based on the new bcmbca binding rule
> for BCM4908 famliy based boards

Typo - family

Please explain why breaking the ABI (and users of these DTS_ is acceptable.

> 
> Signed-off-by: William Zhang <william.zhang@broadcom.com>
> 
> ---
> 

Best regards,
Krzysztof
Florian Fainelli July 12, 2022, 3:36 p.m. UTC | #2
On 7/12/22 00:47, Krzysztof Kozlowski wrote:
> On 12/07/2022 04:11, William Zhang wrote:
>> Update compatible string based on the new bcmbca binding rule
>> for BCM4908 famliy based boards
> 
> Typo - family
> 
> Please explain why breaking the ABI (and users of these DTS_ is acceptable.

This will be largely targeted towards Rafal who supports these kinds of 
devices with an upstream kernel. My understanding is that this is OK 
because we will always ship a DTB matching the Linux kernel, and I 
believe this is true for both the way that William and his group support 
these devices, as well as how OpenWrt, buildroot or other build systems 
envision to support these devices.

Rafal, does that sound about right?
Krzysztof Kozlowski July 12, 2022, 3:50 p.m. UTC | #3
On 12/07/2022 17:36, Florian Fainelli wrote:
> On 7/12/22 00:47, Krzysztof Kozlowski wrote:
>> On 12/07/2022 04:11, William Zhang wrote:
>>> Update compatible string based on the new bcmbca binding rule
>>> for BCM4908 famliy based boards
>>
>> Typo - family
>>
>> Please explain why breaking the ABI (and users of these DTS_ is acceptable.
> 
> This will be largely targeted towards Rafal who supports these kinds of 
> devices with an upstream kernel. My understanding is that this is OK 
> because we will always ship a DTB matching the Linux kernel, and I 
> believe this is true for both the way that William and his group support 
> these devices, as well as how OpenWrt, buildroot or other build systems 
> envision to support these devices.
> 
> Rafal, does that sound about right?

I am fine, just maybe mention it in the commit because it literally
breaks the DTSes.

I assume you considered all possible uses outside of Linux like U-Boot,
BSD etc?

Best regards,
Krzysztof
William Zhang July 12, 2022, 5:48 p.m. UTC | #4
On 7/12/22 08:50, Krzysztof Kozlowski wrote:
> On 12/07/2022 17:36, Florian Fainelli wrote:
>> On 7/12/22 00:47, Krzysztof Kozlowski wrote:
>>> On 12/07/2022 04:11, William Zhang wrote:
>>>> Update compatible string based on the new bcmbca binding rule
>>>> for BCM4908 famliy based boards
>>>
>>> Typo - family
>>>
>>> Please explain why breaking the ABI (and users of these DTS_ is acceptable.
>>
>> This will be largely targeted towards Rafal who supports these kinds of
>> devices with an upstream kernel. My understanding is that this is OK
>> because we will always ship a DTB matching the Linux kernel, and I
>> believe this is true for both the way that William and his group support
>> these devices, as well as how OpenWrt, buildroot or other build systems
>> envision to support these devices.
Yes that is correct for bca group.

>>
>> Rafal, does that sound about right?
> 
> I am fine, just maybe mention it in the commit because it literally
> breaks the DTSes.
> 
> I assume you considered all possible uses outside of Linux like U-Boot,
> BSD etc?
> 
> Best regards,
> Krzysztof

The reason for this patch is to keep the bcmbca board dts in the same 
format and keep everything in the same yaml file. Understand 4908 was 
already upstream but luckily there is no driver in linux and u-boot that 
uses these 4908 compatible strings. They are only used in the board dts 
as far as I can see.  So it does not really break anything in the end, 
unless someone use them in any driver but never upstream their code...

Rafal,  please let us know if this is okay with you or any 
concern/possible break of existing system.
Krzysztof Kozlowski July 12, 2022, 6:20 p.m. UTC | #5
On 12/07/2022 19:48, William Zhang wrote:
>>
>> Best regards,
>> Krzysztof
> 
> The reason for this patch is to keep the bcmbca board dts in the same 
> format and keep everything in the same yaml file. 

Not a good reason to change compatibles. You can have the same format
and keep everything in same YAML file without replacing compatibles.

> Understand 4908 was 
> already upstream but luckily there is no driver in linux and u-boot that 
> uses these 4908 compatible strings. They are only used in the board dts 
> as far as I can see.  So it does not really break anything in the end, 
> unless someone use them in any driver but never upstream their code...

So maybe just briefly mention it in the commit msg?

Best regards,
Krzysztof
William Zhang July 13, 2022, 12:59 a.m. UTC | #6
On 7/12/22 11:20, Krzysztof Kozlowski wrote:
> On 12/07/2022 19:48, William Zhang wrote:
>>>
>>> Best regards,
>>> Krzysztof
>>
>> The reason for this patch is to keep the bcmbca board dts in the same
>> format and keep everything in the same yaml file.
> 
> Not a good reason to change compatibles. You can have the same format
> and keep everything in same YAML file without replacing compatibles.
> 
Well the existing 4908 compatible string is not the same format as we 
are proposing here: "board variant", "chip variant", "brcm, bcmbca"

>> Understand 4908 was
>> already upstream but luckily there is no driver in linux and u-boot that
>> uses these 4908 compatible strings. They are only used in the board dts
>> as far as I can see.  So it does not really break anything in the end,
>> unless someone use them in any driver but never upstream their code...
> 
> So maybe just briefly mention it in the commit msg?
> 
I can do that for sure.

> Best regards,
> Krzysztof
Rafał Miłecki July 13, 2022, 10:55 a.m. UTC | #7
On 2022-07-12 17:36, Florian Fainelli wrote:
> On 7/12/22 00:47, Krzysztof Kozlowski wrote:
>> On 12/07/2022 04:11, William Zhang wrote:
>>> Update compatible string based on the new bcmbca binding rule
>>> for BCM4908 famliy based boards
>> 
>> Typo - family
>> 
>> Please explain why breaking the ABI (and users of these DTS_ is 
>> acceptable.
> 
> This will be largely targeted towards Rafal who supports these kinds
> of devices with an upstream kernel. My understanding is that this is
> OK because we will always ship a DTB matching the Linux kernel, and I
> believe this is true for both the way that William and his group
> support these devices, as well as how OpenWrt, buildroot or other
> build systems envision to support these devices.
> 
> Rafal, does that sound about right?

Right - in all cases I'm aware of - Linux gets shipped with DTB files.
So such change won't actually break anything in real world.
Krzysztof Kozlowski July 13, 2022, 11:09 a.m. UTC | #8
On 13/07/2022 12:55, Rafał Miłecki wrote:
> On 2022-07-12 17:36, Florian Fainelli wrote:
>> On 7/12/22 00:47, Krzysztof Kozlowski wrote:
>>> On 12/07/2022 04:11, William Zhang wrote:
>>>> Update compatible string based on the new bcmbca binding rule
>>>> for BCM4908 famliy based boards
>>>
>>> Typo - family
>>>
>>> Please explain why breaking the ABI (and users of these DTS_ is 
>>> acceptable.
>>
>> This will be largely targeted towards Rafal who supports these kinds
>> of devices with an upstream kernel. My understanding is that this is
>> OK because we will always ship a DTB matching the Linux kernel, and I
>> believe this is true for both the way that William and his group
>> support these devices, as well as how OpenWrt, buildroot or other
>> build systems envision to support these devices.
>>
>> Rafal, does that sound about right?
> 
> Right - in all cases I'm aware of - Linux gets shipped with DTB files.
> So such change won't actually break anything in real world.

We don't really talk here about Linux, but other projects, like
bootloaders or *BSD...

Best regards,
Krzysztof
Rafał Miłecki July 13, 2022, noon UTC | #9
On 2022-07-13 13:09, Krzysztof Kozlowski wrote:
> On 13/07/2022 12:55, Rafał Miłecki wrote:
>> On 2022-07-12 17:36, Florian Fainelli wrote:
>>> On 7/12/22 00:47, Krzysztof Kozlowski wrote:
>>>> On 12/07/2022 04:11, William Zhang wrote:
>>>>> Update compatible string based on the new bcmbca binding rule
>>>>> for BCM4908 famliy based boards
>>>> 
>>>> Typo - family
>>>> 
>>>> Please explain why breaking the ABI (and users of these DTS_ is
>>>> acceptable.
>>> 
>>> This will be largely targeted towards Rafal who supports these kinds
>>> of devices with an upstream kernel. My understanding is that this is
>>> OK because we will always ship a DTB matching the Linux kernel, and I
>>> believe this is true for both the way that William and his group
>>> support these devices, as well as how OpenWrt, buildroot or other
>>> build systems envision to support these devices.
>>> 
>>> Rafal, does that sound about right?
>> 
>> Right - in all cases I'm aware of - Linux gets shipped with DTB files.
>> So such change won't actually break anything in real world.
> 
> We don't really talk here about Linux, but other projects, like
> bootloaders or *BSD...

Right, let me more specific.

BCM4908 uses pkgtb firmware images. Those images contain:
1. bootfs (atf, u-boot, kernel, DTB files)
2. rootfs (filesystem)

So when you flash BCM4908 firmware it always contains:
1. U-Boot and DTB for it
2. Kernel and DTB for it
(+ more stuff)

There isn't any on-flash DTB file that doesn't get updated when flashing
a new image.
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..d1e9531f8bfe 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,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..8d91d0a475e3 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,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 {