2022-07-12 02:14:54

by William Zhang

[permalink] [raw]
Subject: [RFC PATCH 2/3] dt-bindings: arm64: bcm4908: remove binding document

bcm4908 binding document are now merged into bcmbca.

Signed-off-by: William Zhang <[email protected]>
---

.../bindings/arm/bcm/brcm,bcm4908.yaml | 42 -------------------
1 file changed, 42 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/arm/bcm/brcm,bcm4908.yaml

diff --git a/Documentation/devicetree/bindings/arm/bcm/brcm,bcm4908.yaml b/Documentation/devicetree/bindings/arm/bcm/brcm,bcm4908.yaml
deleted file mode 100644
index 9b745531ff04..000000000000
--- a/Documentation/devicetree/bindings/arm/bcm/brcm,bcm4908.yaml
+++ /dev/null
@@ -1,42 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
-%YAML 1.2
----
-$id: http://devicetree.org/schemas/arm/bcm/brcm,bcm4908.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
-
-title: Broadcom BCM4908 device tree bindings
-
-description:
- Broadcom BCM4906 / BCM4908 / BCM49408 Wi-Fi/network SoCs with Brahma CPUs.
-
-maintainers:
- - Rafał Miłecki <[email protected]>
-
-properties:
- $nodename:
- const: '/'
- compatible:
- oneOf:
- - description: BCM4906 based boards
- items:
- - enum:
- - netgear,r8000p
- - tplink,archer-c2300-v1
- - const: brcm,bcm4906
- - const: brcm,bcm4908
-
- - description: BCM4908 based boards
- items:
- - enum:
- - asus,gt-ac5300
- - netgear,raxe500
- - const: brcm,bcm4908
-
- - description: BCM49408 based boards
- items:
- - const: brcm,bcm49408
- - const: brcm,bcm4908
-
-additionalProperties: true
-
-...
--
2.34.1


Attachments:
smime.p7s (4.11 kB)
S/MIME Cryptographic Signature

2022-07-12 08:02:06

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [RFC PATCH 2/3] dt-bindings: arm64: bcm4908: remove binding document

On 12/07/2022 04:11, William Zhang wrote:
> bcm4908 binding document are now merged into bcmbca.
>
> Signed-off-by: William Zhang <[email protected]>

This must be squashed with previous one.

Best regards,
Krzysztof

2022-07-12 08:10:19

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [RFC PATCH 1/3] dt-bindings: arm64: bcmbca: Merge BCM4908 into BCMBCA

On 12/07/2022 04:11, William Zhang wrote:
> Merge BCM4908 SoC device tree description into BCMBCA and combined
> all BCM4908 chip variants into the same BCM4908 chip family item.

Merge means you combine some entries, so I would expect to see the
removal here as well.

>
> Each compatible string represent the whole chip family. The board
> variants and chip varints go into the first and second enum in the
> compatible string item list.
>
> Signed-off-by: William Zhang <[email protected]>
> ---
>
> .../bindings/arm/bcm/brcm,bcmbca.yaml | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml b/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
> index d9dc4f22f4a5..906c3e1de372 100644
> --- a/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
> +++ b/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
> @@ -28,6 +28,23 @@ properties:
> - const: brcm,bcm47622
> - const: brcm,bcmbca
>
> + - description: BCM4908 Family based boards
> + items:
> + - enum:
> + # BCM4908 SoC based boards
> + - brcm,bcm94908
> + - asus,gt-ac5300
> + - netgear,raxe500
> + # BCM4906 SoC based boards
> + - brcm,bcm94906
> + - netgear,r8000p
> + - tplink,archer-c2300-v1
> + - enum:
> + - brcm,bcm4908
> + - brcm,bcm4906
> + - brcm,bcm49408

This is wrong. brcm,bcm94908 followed by brcm,bcm4906 does not look
like valid list of compatibles.

> + - const: brcm,bcmbca
> +
> - description: BCM4912 based boards
> items:
> - enum:


Best regards,
Krzysztof

2022-07-12 08:22:59

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [RFC PATCH 3/3] arm64: dts: bcmbca: update bcm4808 board dts file

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 <[email protected]>
>
> ---
>

Best regards,
Krzysztof

2022-07-12 15:56:42

by Florian Fainelli

[permalink] [raw]
Subject: Re: [RFC PATCH 3/3] arm64: dts: bcmbca: update bcm4808 board dts file

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?
--
Florian

2022-07-12 16:54:45

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [RFC PATCH 3/3] arm64: dts: bcmbca: update bcm4808 board dts file

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

2022-07-12 17:55:59

by William Zhang

[permalink] [raw]
Subject: Re: [RFC PATCH 1/3] dt-bindings: arm64: bcmbca: Merge BCM4908 into BCMBCA



On 7/12/22 00:45, Krzysztof Kozlowski wrote:
> On 12/07/2022 04:11, William Zhang wrote:
>> Merge BCM4908 SoC device tree description into BCMBCA and combined
>> all BCM4908 chip variants into the same BCM4908 chip family item.
>
> Merge means you combine some entries, so I would expect to see the
> removal here as well.
>
Will combine with the removal patch as you pointed out

>>
>> Each compatible string represent the whole chip family. The board
>> variants and chip varints go into the first and second enum in the
>> compatible string item list.
>>
>> Signed-off-by: William Zhang <[email protected]>
>> ---
>>
>> .../bindings/arm/bcm/brcm,bcmbca.yaml | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml b/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
>> index d9dc4f22f4a5..906c3e1de372 100644
>> --- a/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
>> +++ b/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
>> @@ -28,6 +28,23 @@ properties:
>> - const: brcm,bcm47622
>> - const: brcm,bcmbca
>>
>> + - description: BCM4908 Family based boards
>> + items:
>> + - enum:
>> + # BCM4908 SoC based boards
>> + - brcm,bcm94908
>> + - asus,gt-ac5300
>> + - netgear,raxe500
>> + # BCM4906 SoC based boards
>> + - brcm,bcm94906
>> + - netgear,r8000p
>> + - tplink,archer-c2300-v1
>> + - enum:
>> + - brcm,bcm4908
>> + - brcm,bcm4906
>> + - brcm,bcm49408
>
> This is wrong. brcm,bcm94908 followed by brcm,bcm4906 does not look
> like valid list of compatibles.
>
For 4908 board variant, it will need to be followed by 4908 chip. Sorry
for the basic question but is there any requirement to enforce this kind
of rule? I would assume dts writer know what he/she is doing and select
the right combination.

>> + - const: brcm,bcmbca
>> +
>> - description: BCM4912 based boards
>> items:
>> - enum:
>
>
> Best regards,
> Krzysztof


Attachments:
smime.p7s (4.11 kB)
S/MIME Cryptographic Signature

2022-07-12 18:04:46

by William Zhang

[permalink] [raw]
Subject: Re: [RFC PATCH 3/3] arm64: dts: bcmbca: update bcm4808 board dts file



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.


Attachments:
smime.p7s (4.11 kB)
S/MIME Cryptographic Signature

2022-07-12 18:23:30

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [RFC PATCH 1/3] dt-bindings: arm64: bcmbca: Merge BCM4908 into BCMBCA

On 12/07/2022 19:37, William Zhang wrote:
>>> + - description: BCM4908 Family based boards
>>> + items:
>>> + - enum:
>>> + # BCM4908 SoC based boards
>>> + - brcm,bcm94908
>>> + - asus,gt-ac5300
>>> + - netgear,raxe500
>>> + # BCM4906 SoC based boards
>>> + - brcm,bcm94906
>>> + - netgear,r8000p
>>> + - tplink,archer-c2300-v1
>>> + - enum:
>>> + - brcm,bcm4908
>>> + - brcm,bcm4906
>>> + - brcm,bcm49408
>>
>> This is wrong. brcm,bcm94908 followed by brcm,bcm4906 does not look
>> like valid list of compatibles.
>>
> For 4908 board variant, it will need to be followed by 4908 chip. Sorry
> for the basic question but is there any requirement to enforce this kind
> of rule? I would assume dts writer know what he/she is doing and select
> the right combination.

The entire point of DT schema is to validate DTS. Combination like above
prevents that goal.

Best regards,
Krzysztof

2022-07-12 18:34:34

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [RFC PATCH 3/3] arm64: dts: bcmbca: update bcm4808 board dts file

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

2022-07-13 01:12:47

by William Zhang

[permalink] [raw]
Subject: Re: [RFC PATCH 3/3] arm64: dts: bcmbca: update bcm4808 board dts file



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


Attachments:
smime.p7s (4.11 kB)
S/MIME Cryptographic Signature

2022-07-13 01:44:10

by William Zhang

[permalink] [raw]
Subject: Re: [RFC PATCH 1/3] dt-bindings: arm64: bcmbca: Merge BCM4908 into BCMBCA



On 7/12/22 11:18, Krzysztof Kozlowski wrote:
> On 12/07/2022 19:37, William Zhang wrote:
>>>> + - description: BCM4908 Family based boards
>>>> + items:
>>>> + - enum:
>>>> + # BCM4908 SoC based boards
>>>> + - brcm,bcm94908
>>>> + - asus,gt-ac5300
>>>> + - netgear,raxe500
>>>> + # BCM4906 SoC based boards
>>>> + - brcm,bcm94906
>>>> + - netgear,r8000p
>>>> + - tplink,archer-c2300-v1
>>>> + - enum:
>>>> + - brcm,bcm4908
>>>> + - brcm,bcm4906
>>>> + - brcm,bcm49408
>>>
>>> This is wrong. brcm,bcm94908 followed by brcm,bcm4906 does not look
>>> like valid list of compatibles.
>>>
>> For 4908 board variant, it will need to be followed by 4908 chip. Sorry
>> for the basic question but is there any requirement to enforce this kind
>> of rule? I would assume dts writer know what he/she is doing and select
>> the right combination.
>
> The entire point of DT schema is to validate DTS. Combination like above
> prevents that goal.
>
> Best regards,
> Krzysztof
Understand the DT schema purpose. But items property allows multiple
enums in the list which gives a lot of flexibility but make it hard to
validate. I am not familiar with DT schema, is there any directive to
specify one enum value depending on another so dts validation tool can
report error if combination is wrong?

This is our preferred format of all bcmbca compatible string especially
when we could have more than 10 chip variants for the same chip family
and we really want to work on the chip family id. We will make sure
they are in the right combination in our own patch and patch from other
contributors. Would this work? If not, I will probably have to revert
the change of 4908(maybe append brcm,bcmbca as this chip belongs to the
same bca group) and use "enum board variant", "const main chip id",
"brcm,bca" for all other chips as our secondary choice.


Attachments:
smime.p7s (4.11 kB)
S/MIME Cryptographic Signature

2022-07-13 11:04:14

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [RFC PATCH 3/3] arm64: dts: bcmbca: update bcm4808 board dts file

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.

2022-07-13 11:06:13

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [RFC PATCH 1/3] dt-bindings: arm64: bcmbca: Merge BCM4908 into BCMBCA

On 2022-07-13 02:57, William Zhang wrote:
> On 7/12/22 11:18, Krzysztof Kozlowski wrote:
>> On 12/07/2022 19:37, William Zhang wrote:
>>>>> + - description: BCM4908 Family based boards
>>>>> + items:
>>>>> + - enum:
>>>>> + # BCM4908 SoC based boards
>>>>> + - brcm,bcm94908
>>>>> + - asus,gt-ac5300
>>>>> + - netgear,raxe500
>>>>> + # BCM4906 SoC based boards
>>>>> + - brcm,bcm94906
>>>>> + - netgear,r8000p
>>>>> + - tplink,archer-c2300-v1
>>>>> + - enum:
>>>>> + - brcm,bcm4908
>>>>> + - brcm,bcm4906
>>>>> + - brcm,bcm49408
>>>>
>>>> This is wrong. brcm,bcm94908 followed by brcm,bcm4906 does not look
>>>> like valid list of compatibles.
>>>>
>>> For 4908 board variant, it will need to be followed by 4908 chip.
>>> Sorry
>>> for the basic question but is there any requirement to enforce this
>>> kind
>>> of rule? I would assume dts writer know what he/she is doing and
>>> select
>>> the right combination.
>>
>> The entire point of DT schema is to validate DTS. Combination like
>> above
>> prevents that goal.
>>
>> Best regards,
>> Krzysztof
> Understand the DT schema purpose. But items property allows multiple
> enums in the list which gives a lot of flexibility but make it hard to
> validate. I am not familiar with DT schema, is there any directive to
> specify one enum value depending on another so dts validation tool can
> report error if combination is wrong?
>
> This is our preferred format of all bcmbca compatible string
> especially when we could have more than 10 chip variants for the same
> chip family and we really want to work on the chip family id. We will
> make sure they are in the right combination in our own patch and patch
> from other contributors. Would this work? If not, I will probably have
> to revert the change of 4908(maybe append brcm,bcmbca as this chip
> belongs to the same bca group) and use "enum board variant", "const
> main chip id", "brcm,bca" for all other chips as our secondary choice.

I'm not sure why I didn't even receive 1/3 and half of discussion
e-mails.

You can't just put all strings into a single bag and allow mixing them
in any combos. Please check how it's properly handled in the current
existing binding:
Documentation/devicetree/bindings/arm/bcm/brcm,bcm4908.yaml

Above binding enforces that non-matching compatible strings are not used
together.

2022-07-13 11:26:59

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [RFC PATCH 1/3] dt-bindings: arm64: bcmbca: Merge BCM4908 into BCMBCA

On 2022-07-13 12:50, Rafał Miłecki wrote:
> On 2022-07-13 02:57, William Zhang wrote:
>> On 7/12/22 11:18, Krzysztof Kozlowski wrote:
>>> On 12/07/2022 19:37, William Zhang wrote:
>>>>>> + - description: BCM4908 Family based boards
>>>>>> + items:
>>>>>> + - enum:
>>>>>> + # BCM4908 SoC based boards
>>>>>> + - brcm,bcm94908
>>>>>> + - asus,gt-ac5300
>>>>>> + - netgear,raxe500
>>>>>> + # BCM4906 SoC based boards
>>>>>> + - brcm,bcm94906
>>>>>> + - netgear,r8000p
>>>>>> + - tplink,archer-c2300-v1
>>>>>> + - enum:
>>>>>> + - brcm,bcm4908
>>>>>> + - brcm,bcm4906
>>>>>> + - brcm,bcm49408
>>>>>
>>>>> This is wrong. brcm,bcm94908 followed by brcm,bcm4906 does not
>>>>> look
>>>>> like valid list of compatibles.
>>>>>
>>>> For 4908 board variant, it will need to be followed by 4908 chip.
>>>> Sorry
>>>> for the basic question but is there any requirement to enforce this
>>>> kind
>>>> of rule? I would assume dts writer know what he/she is doing and
>>>> select
>>>> the right combination.
>>>
>>> The entire point of DT schema is to validate DTS. Combination like
>>> above
>>> prevents that goal.
>>>
>>> Best regards,
>>> Krzysztof
>> Understand the DT schema purpose. But items property allows multiple
>> enums in the list which gives a lot of flexibility but make it hard to
>> validate. I am not familiar with DT schema, is there any directive to
>> specify one enum value depending on another so dts validation tool can
>> report error if combination is wrong?
>>
>> This is our preferred format of all bcmbca compatible string
>> especially when we could have more than 10 chip variants for the same
>> chip family and we really want to work on the chip family id. We will
>> make sure they are in the right combination in our own patch and patch
>> from other contributors. Would this work? If not, I will probably have
>> to revert the change of 4908(maybe append brcm,bcmbca as this chip
>> belongs to the same bca group) and use "enum board variant", "const
>> main chip id", "brcm,bca" for all other chips as our secondary choice.
>
> I'm not sure why I didn't even receive 1/3 and half of discussion
> e-mails.
>
> You can't just put all strings into a single bag and allow mixing them
> in any combos. Please check how it's properly handled in the current
> existing binding:
> Documentation/devicetree/bindings/arm/bcm/brcm,bcm4908.yaml
>
> Above binding enforces that non-matching compatible strings are not
> used
> together.

I just noticed you're actually removing brcm,bcm4908.yaml in the 2/3 so
you must be aware of that file.

So you see a cleanly working binding in the brcm,bcm4908.yaml but
instead copying it you decided to wrote your own one from scratch.
Incorrectly.

This smells of NIH (not invented here). Please just use that binding I
wrote and move if it needed.

2022-07-13 11:46:17

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [RFC PATCH 3/3] arm64: dts: bcmbca: update bcm4808 board dts file

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

2022-07-13 16:29:30

by Florian Fainelli

[permalink] [raw]
Subject: Re: [RFC PATCH 1/3] dt-bindings: arm64: bcmbca: Merge BCM4908 into BCMBCA

On 7/13/22 03:50, Rafał Miłecki wrote:
> On 2022-07-13 02:57, William Zhang wrote:
>> On 7/12/22 11:18, Krzysztof Kozlowski wrote:
>>> On 12/07/2022 19:37, William Zhang wrote:
>>>>>> +      - description: BCM4908 Family based boards
>>>>>> +        items:
>>>>>> +          - enum:
>>>>>> +              # BCM4908 SoC based boards
>>>>>> +              - brcm,bcm94908
>>>>>> +              - asus,gt-ac5300
>>>>>> +              - netgear,raxe500
>>>>>> +              # BCM4906 SoC based boards
>>>>>> +              - brcm,bcm94906
>>>>>> +              - netgear,r8000p
>>>>>> +              - tplink,archer-c2300-v1
>>>>>> +          - enum:
>>>>>> +              - brcm,bcm4908
>>>>>> +              - brcm,bcm4906
>>>>>> +              - brcm,bcm49408
>>>>>
>>>>> This is wrong.  brcm,bcm94908 followed by brcm,bcm4906 does not look
>>>>> like valid list of compatibles.
>>>>>
>>>> For 4908 board variant, it will need to be followed by 4908 chip. Sorry
>>>> for the basic question but is there any requirement to enforce this
>>>> kind
>>>> of rule?  I would assume dts writer know what he/she is doing and
>>>> select
>>>> the right combination.
>>>
>>> The entire point of DT schema is to validate DTS. Combination like above
>>> prevents that goal.
>>>
>>> Best regards,
>>> Krzysztof
>> Understand the DT schema purpose. But items property allows multiple
>> enums in the list which gives a lot of flexibility but make it hard to
>> validate. I am not familiar with DT schema, is there any directive to
>> specify one enum value depending on another so dts validation tool can
>> report error if combination is wrong?
>>
>> This is our preferred format of all bcmbca compatible string
>> especially when we could have more than 10 chip variants for the same
>> chip family and we really want to work on the chip family id.  We will
>> make sure they are in the right combination in our own patch and patch
>> from other contributors. Would this work? If not, I will probably have
>> to revert the change of 4908(maybe append brcm,bcmbca as this chip
>> belongs to the same bca group) and use "enum board variant", "const
>> main chip id", "brcm,bca" for all other chips as our secondary choice.
>
> I'm not sure why I didn't even receive 1/3 and half of discussion
> e-mails.

You are copied on all 4 emails (including cover letter).
--
Florian

2022-07-13 17:03:18

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [RFC PATCH 3/3] arm64: dts: bcmbca: update bcm4808 board dts file

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.

2022-07-13 19:07:15

by William Zhang

[permalink] [raw]
Subject: Re: [RFC PATCH 1/3] dt-bindings: arm64: bcmbca: Merge BCM4908 into BCMBCA

Hi Rafal,

On 7/13/22 03:58, Rafał Miłecki wrote:
> On 2022-07-13 12:50, Rafał Miłecki wrote:
>> On 2022-07-13 02:57, William Zhang wrote:
>>> On 7/12/22 11:18, Krzysztof Kozlowski wrote:
>>>> On 12/07/2022 19:37, William Zhang wrote:
>>>>>>> +      - description: BCM4908 Family based boards
>>>>>>> +        items:
>>>>>>> +          - enum:
>>>>>>> +              # BCM4908 SoC based boards
>>>>>>> +              - brcm,bcm94908
>>>>>>> +              - asus,gt-ac5300
>>>>>>> +              - netgear,raxe500
>>>>>>> +              # BCM4906 SoC based boards
>>>>>>> +              - brcm,bcm94906
>>>>>>> +              - netgear,r8000p
>>>>>>> +              - tplink,archer-c2300-v1
>>>>>>> +          - enum:
>>>>>>> +              - brcm,bcm4908
>>>>>>> +              - brcm,bcm4906
>>>>>>> +              - brcm,bcm49408
>>>>>>
>>>>>> This is wrong.  brcm,bcm94908 followed by brcm,bcm4906 does not look
>>>>>> like valid list of compatibles.
>>>>>>
>>>>> For 4908 board variant, it will need to be followed by 4908 chip.
>>>>> Sorry
>>>>> for the basic question but is there any requirement to enforce this
>>>>> kind
>>>>> of rule?  I would assume dts writer know what he/she is doing and
>>>>> select
>>>>> the right combination.
>>>>
>>>> The entire point of DT schema is to validate DTS. Combination like
>>>> above
>>>> prevents that goal.
>>>>
>>>> Best regards,
>>>> Krzysztof
>>> Understand the DT schema purpose. But items property allows multiple
>>> enums in the list which gives a lot of flexibility but make it hard to
>>> validate. I am not familiar with DT schema, is there any directive to
>>> specify one enum value depending on another so dts validation tool can
>>> report error if combination is wrong?
>>>
>>> This is our preferred format of all bcmbca compatible string
>>> especially when we could have more than 10 chip variants for the same
>>> chip family and we really want to work on the chip family id.  We will
>>> make sure they are in the right combination in our own patch and patch
>>> from other contributors. Would this work? If not, I will probably have
>>> to revert the change of 4908(maybe append brcm,bcmbca as this chip
>>> belongs to the same bca group) and use "enum board variant", "const
>>> main chip id", "brcm,bca" for all other chips as our secondary choice.
>>
>> I'm not sure why I didn't even receive 1/3 and half of discussion
>> e-mails.
>>
>> You can't just put all strings into a single bag and allow mixing them
>> in any combos. Please check how it's properly handled in the current
>> existing binding:
>> Documentation/devicetree/bindings/arm/bcm/brcm,bcm4908.yaml
>>
>> Above binding enforces that non-matching compatible strings are not used
>> together.
>
> I just noticed you're actually removing brcm,bcm4908.yaml in the 2/3 so
> you must be aware of that file.
>
> So you see a cleanly working binding in the brcm,bcm4908.yaml but
> instead copying it you decided to wrote your own one from scratch.
> Incorrectly.
>
> This smells of NIH (not invented here). Please just use that binding I
> wrote and move if it needed.

Not mean to discredit any of your work and I did copy over your binding
and combine them into one SoC entry to the new bcmbca.yaml and add you
as one of the maintainer to this file. As this change would certainly
concern you, that's why I sent RFC first. As I explained in the cover
letter, the purpose of the change is to reduce the number of compatible
strings and keep one entry for one chip family due to possible large
number of chip variants. But since there is no way to validate the
combination, I will copy the existing 4908 bindings as they are now but
I would propose to append "brcm, bcmbca" as it is part of bcmbca chip.
And for the other chips, we would just use enum "board variant", const
"main chip id", const "brcm,bca". Does that sound good to you?


Attachments:
smime.p7s (4.11 kB)
S/MIME Cryptographic Signature

2022-07-13 20:45:02

by William Zhang

[permalink] [raw]
Subject: Re: [RFC PATCH 1/3] dt-bindings: arm64: bcmbca: Merge BCM4908 into BCMBCA



On 7/13/22 13:23, Rafał Miłecki wrote:
> On 2022-07-13 20:37, William Zhang wrote:
>> Hi Rafal,
>>
>> On 7/13/22 03:58, Rafał Miłecki wrote:
>>> On 2022-07-13 12:50, Rafał Miłecki wrote:
>>>> On 2022-07-13 02:57, William Zhang wrote:
>>>>> On 7/12/22 11:18, Krzysztof Kozlowski wrote:
>>>>>> On 12/07/2022 19:37, William Zhang wrote:
>>>>>>>>> +      - description: BCM4908 Family based boards
>>>>>>>>> +        items:
>>>>>>>>> +          - enum:
>>>>>>>>> +              # BCM4908 SoC based boards
>>>>>>>>> +              - brcm,bcm94908
>>>>>>>>> +              - asus,gt-ac5300
>>>>>>>>> +              - netgear,raxe500
>>>>>>>>> +              # BCM4906 SoC based boards
>>>>>>>>> +              - brcm,bcm94906
>>>>>>>>> +              - netgear,r8000p
>>>>>>>>> +              - tplink,archer-c2300-v1
>>>>>>>>> +          - enum:
>>>>>>>>> +              - brcm,bcm4908
>>>>>>>>> +              - brcm,bcm4906
>>>>>>>>> +              - brcm,bcm49408
>>>>>>>>
>>>>>>>> This is wrong.  brcm,bcm94908 followed by brcm,bcm4906 does not
>>>>>>>> look
>>>>>>>> like valid list of compatibles.
>>>>>>>>
>>>>>>> For 4908 board variant, it will need to be followed by 4908 chip.
>>>>>>> Sorry
>>>>>>> for the basic question but is there any requirement to enforce
>>>>>>> this kind
>>>>>>> of rule?  I would assume dts writer know what he/she is doing and
>>>>>>> select
>>>>>>> the right combination.
>>>>>>
>>>>>> The entire point of DT schema is to validate DTS. Combination like
>>>>>> above
>>>>>> prevents that goal.
>>>>>>
>>>>>> Best regards,
>>>>>> Krzysztof
>>>>> Understand the DT schema purpose. But items property allows multiple
>>>>> enums in the list which gives a lot of flexibility but make it hard to
>>>>> validate. I am not familiar with DT schema, is there any directive to
>>>>> specify one enum value depending on another so dts validation tool can
>>>>> report error if combination is wrong?
>>>>>
>>>>> This is our preferred format of all bcmbca compatible string
>>>>> especially when we could have more than 10 chip variants for the same
>>>>> chip family and we really want to work on the chip family id.  We will
>>>>> make sure they are in the right combination in our own patch and patch
>>>>> from other contributors. Would this work? If not, I will probably have
>>>>> to revert the change of 4908(maybe append brcm,bcmbca as this chip
>>>>> belongs to the same bca group) and use "enum board variant", "const
>>>>> main chip id", "brcm,bca" for all other chips as our secondary choice.
>>>>
>>>> I'm not sure why I didn't even receive 1/3 and half of discussion
>>>> e-mails.
>>>>
>>>> You can't just put all strings into a single bag and allow mixing them
>>>> in any combos. Please check how it's properly handled in the current
>>>> existing binding:
>>>> Documentation/devicetree/bindings/arm/bcm/brcm,bcm4908.yaml
>>>>
>>>> Above binding enforces that non-matching compatible strings are not
>>>> used
>>>> together.
>>>
>>> I just noticed you're actually removing brcm,bcm4908.yaml in the 2/3 so
>>> you must be aware of that file.
>>>
>>> So you see a cleanly working binding in the brcm,bcm4908.yaml but
>>> instead copying it you decided to wrote your own one from scratch.
>>> Incorrectly.
>>>
>>> This smells of NIH (not invented here). Please just use that binding I
>>> wrote and move if it needed.
>>
>> Not mean to discredit any of your work and I did copy over your
>> binding and combine them into one SoC entry to the new bcmbca.yaml and
>> add you as one of the maintainer to this file. As this change would
>> certainly concern you, that's why I sent RFC first.  As I explained in
>> the cover letter, the purpose of the change is to reduce the number of
>> compatible strings and keep one entry for one chip family due to
>> possible large number of chip variants.  But since there is no way to
>> validate the combination, I will copy the existing 4908 bindings as
>> they are now
>
> Right. I believe we need that.
>
>
>> but I would propose to append "brcm, bcmbca" as it is
>> part of bcmbca chip. And for the other chips, we would just use enum
>> "board variant", const "main chip id", const "brcm,bca".  Does that
>> sound good to you?
>
> Nitpicking: you meant "brcm,bcmbca" (typo) but sounds absolutely fine!
Yup its a typo. Will append "brcm,bcmbca" and send out new patch.


Attachments:
smime.p7s (4.11 kB)
S/MIME Cryptographic Signature

2022-07-13 21:13:23

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [RFC PATCH 1/3] dt-bindings: arm64: bcmbca: Merge BCM4908 into BCMBCA

On 2022-07-13 20:37, William Zhang wrote:
> Hi Rafal,
>
> On 7/13/22 03:58, Rafał Miłecki wrote:
>> On 2022-07-13 12:50, Rafał Miłecki wrote:
>>> On 2022-07-13 02:57, William Zhang wrote:
>>>> On 7/12/22 11:18, Krzysztof Kozlowski wrote:
>>>>> On 12/07/2022 19:37, William Zhang wrote:
>>>>>>>> +      - description: BCM4908 Family based boards
>>>>>>>> +        items:
>>>>>>>> +          - enum:
>>>>>>>> +              # BCM4908 SoC based boards
>>>>>>>> +              - brcm,bcm94908
>>>>>>>> +              - asus,gt-ac5300
>>>>>>>> +              - netgear,raxe500
>>>>>>>> +              # BCM4906 SoC based boards
>>>>>>>> +              - brcm,bcm94906
>>>>>>>> +              - netgear,r8000p
>>>>>>>> +              - tplink,archer-c2300-v1
>>>>>>>> +          - enum:
>>>>>>>> +              - brcm,bcm4908
>>>>>>>> +              - brcm,bcm4906
>>>>>>>> +              - brcm,bcm49408
>>>>>>>
>>>>>>> This is wrong.  brcm,bcm94908 followed by brcm,bcm4906 does not
>>>>>>> look
>>>>>>> like valid list of compatibles.
>>>>>>>
>>>>>> For 4908 board variant, it will need to be followed by 4908 chip.
>>>>>> Sorry
>>>>>> for the basic question but is there any requirement to enforce
>>>>>> this kind
>>>>>> of rule?  I would assume dts writer know what he/she is doing and
>>>>>> select
>>>>>> the right combination.
>>>>>
>>>>> The entire point of DT schema is to validate DTS. Combination like
>>>>> above
>>>>> prevents that goal.
>>>>>
>>>>> Best regards,
>>>>> Krzysztof
>>>> Understand the DT schema purpose. But items property allows multiple
>>>> enums in the list which gives a lot of flexibility but make it hard
>>>> to
>>>> validate. I am not familiar with DT schema, is there any directive
>>>> to
>>>> specify one enum value depending on another so dts validation tool
>>>> can
>>>> report error if combination is wrong?
>>>>
>>>> This is our preferred format of all bcmbca compatible string
>>>> especially when we could have more than 10 chip variants for the
>>>> same
>>>> chip family and we really want to work on the chip family id.  We
>>>> will
>>>> make sure they are in the right combination in our own patch and
>>>> patch
>>>> from other contributors. Would this work? If not, I will probably
>>>> have
>>>> to revert the change of 4908(maybe append brcm,bcmbca as this chip
>>>> belongs to the same bca group) and use "enum board variant", "const
>>>> main chip id", "brcm,bca" for all other chips as our secondary
>>>> choice.
>>>
>>> I'm not sure why I didn't even receive 1/3 and half of discussion
>>> e-mails.
>>>
>>> You can't just put all strings into a single bag and allow mixing
>>> them
>>> in any combos. Please check how it's properly handled in the current
>>> existing binding:
>>> Documentation/devicetree/bindings/arm/bcm/brcm,bcm4908.yaml
>>>
>>> Above binding enforces that non-matching compatible strings are not
>>> used
>>> together.
>>
>> I just noticed you're actually removing brcm,bcm4908.yaml in the 2/3
>> so
>> you must be aware of that file.
>>
>> So you see a cleanly working binding in the brcm,bcm4908.yaml but
>> instead copying it you decided to wrote your own one from scratch.
>> Incorrectly.
>>
>> This smells of NIH (not invented here). Please just use that binding I
>> wrote and move if it needed.
>
> Not mean to discredit any of your work and I did copy over your
> binding and combine them into one SoC entry to the new bcmbca.yaml and
> add you as one of the maintainer to this file. As this change would
> certainly concern you, that's why I sent RFC first. As I explained in
> the cover letter, the purpose of the change is to reduce the number of
> compatible strings and keep one entry for one chip family due to
> possible large number of chip variants. But since there is no way to
> validate the combination, I will copy the existing 4908 bindings as
> they are now

Right. I believe we need that.


> but I would propose to append "brcm, bcmbca" as it is
> part of bcmbca chip. And for the other chips, we would just use enum
> "board variant", const "main chip id", const "brcm,bca". Does that
> sound good to you?

Nitpicking: you meant "brcm,bcmbca" (typo) but sounds absolutely fine!

2022-07-18 20:14:11

by Rob Herring

[permalink] [raw]
Subject: Re: [RFC PATCH 1/3] dt-bindings: arm64: bcmbca: Merge BCM4908 into BCMBCA

On Wed, Jul 13, 2022 at 11:37:18AM -0700, William Zhang wrote:
> Hi Rafal,
>
> On 7/13/22 03:58, Rafał Miłecki wrote:
> > On 2022-07-13 12:50, Rafał Miłecki wrote:
> > > On 2022-07-13 02:57, William Zhang wrote:
> > > > On 7/12/22 11:18, Krzysztof Kozlowski wrote:
> > > > > On 12/07/2022 19:37, William Zhang wrote:
> > > > > > > > +      - description: BCM4908 Family based boards
> > > > > > > > +        items:
> > > > > > > > +          - enum:
> > > > > > > > +              # BCM4908 SoC based boards
> > > > > > > > +              - brcm,bcm94908
> > > > > > > > +              - asus,gt-ac5300
> > > > > > > > +              - netgear,raxe500
> > > > > > > > +              # BCM4906 SoC based boards
> > > > > > > > +              - brcm,bcm94906
> > > > > > > > +              - netgear,r8000p
> > > > > > > > +              - tplink,archer-c2300-v1
> > > > > > > > +          - enum:
> > > > > > > > +              - brcm,bcm4908
> > > > > > > > +              - brcm,bcm4906
> > > > > > > > +              - brcm,bcm49408
> > > > > > >
> > > > > > > This is wrong.  brcm,bcm94908 followed by brcm,bcm4906 does not look
> > > > > > > like valid list of compatibles.
> > > > > > >
> > > > > > For 4908 board variant, it will need to be followed by
> > > > > > 4908 chip. Sorry
> > > > > > for the basic question but is there any requirement to
> > > > > > enforce this kind
> > > > > > of rule?  I would assume dts writer know what he/she is
> > > > > > doing and select
> > > > > > the right combination.
> > > > >
> > > > > The entire point of DT schema is to validate DTS.
> > > > > Combination like above
> > > > > prevents that goal.
> > > > >
> > > > > Best regards,
> > > > > Krzysztof
> > > > Understand the DT schema purpose. But items property allows multiple
> > > > enums in the list which gives a lot of flexibility but make it hard to
> > > > validate. I am not familiar with DT schema, is there any directive to
> > > > specify one enum value depending on another so dts validation tool can
> > > > report error if combination is wrong?
> > > >
> > > > This is our preferred format of all bcmbca compatible string
> > > > especially when we could have more than 10 chip variants for the same
> > > > chip family and we really want to work on the chip family id.  We will
> > > > make sure they are in the right combination in our own patch and patch
> > > > from other contributors. Would this work? If not, I will probably have
> > > > to revert the change of 4908(maybe append brcm,bcmbca as this chip
> > > > belongs to the same bca group) and use "enum board variant", "const
> > > > main chip id", "brcm,bca" for all other chips as our secondary choice.
> > >
> > > I'm not sure why I didn't even receive 1/3 and half of discussion
> > > e-mails.
> > >
> > > You can't just put all strings into a single bag and allow mixing them
> > > in any combos. Please check how it's properly handled in the current
> > > existing binding:
> > > Documentation/devicetree/bindings/arm/bcm/brcm,bcm4908.yaml
> > >
> > > Above binding enforces that non-matching compatible strings are not used
> > > together.
> >
> > I just noticed you're actually removing brcm,bcm4908.yaml in the 2/3 so
> > you must be aware of that file.
> >
> > So you see a cleanly working binding in the brcm,bcm4908.yaml but
> > instead copying it you decided to wrote your own one from scratch.
> > Incorrectly.
> >
> > This smells of NIH (not invented here). Please just use that binding I
> > wrote and move if it needed.
>
> Not mean to discredit any of your work and I did copy over your binding and
> combine them into one SoC entry to the new bcmbca.yaml and add you as one of
> the maintainer to this file. As this change would certainly concern you,
> that's why I sent RFC first. As I explained in the cover letter, the
> purpose of the change is to reduce the number of compatible strings and keep
> one entry for one chip family due to possible large number of chip variants.
> But since there is no way to validate the combination, I will copy the
> existing 4908 bindings as they are now but I would propose to append "brcm,
> bcmbca" as it is part of bcmbca chip. And for the other chips, we would just
> use enum "board variant", const "main chip id", const "brcm,bca". Does that
> sound good to you?

If you want fewer combinations of compatibles, adding a genericish
"brcm,bcmbca" is not going to help. Is there much value to adding it?
What can you do with that information (and nothing else) is the
question to ask.

Rob

2022-07-18 23:00:47

by William Zhang

[permalink] [raw]
Subject: Re: [RFC PATCH 1/3] dt-bindings: arm64: bcmbca: Merge BCM4908 into BCMBCA



On 07/18/2022 01:02 PM, Rob Herring wrote:
> On Wed, Jul 13, 2022 at 11:37:18AM -0700, William Zhang wrote:
>> Hi Rafal,
>>
>> On 7/13/22 03:58, Rafał Miłecki wrote:
>>> On 2022-07-13 12:50, Rafał Miłecki wrote:
>>>> On 2022-07-13 02:57, William Zhang wrote:
>>>>> On 7/12/22 11:18, Krzysztof Kozlowski wrote:
>>>>>> On 12/07/2022 19:37, William Zhang wrote:
>>>>>>>>> +      - description: BCM4908 Family based boards
>>>>>>>>> +        items:
>>>>>>>>> +          - enum:
>>>>>>>>> +              # BCM4908 SoC based boards
>>>>>>>>> +              - brcm,bcm94908
>>>>>>>>> +              - asus,gt-ac5300
>>>>>>>>> +              - netgear,raxe500
>>>>>>>>> +              # BCM4906 SoC based boards
>>>>>>>>> +              - brcm,bcm94906
>>>>>>>>> +              - netgear,r8000p
>>>>>>>>> +              - tplink,archer-c2300-v1
>>>>>>>>> +          - enum:
>>>>>>>>> +              - brcm,bcm4908
>>>>>>>>> +              - brcm,bcm4906
>>>>>>>>> +              - brcm,bcm49408
>>>>>>>>
>>>>>>>> This is wrong.  brcm,bcm94908 followed by brcm,bcm4906 does not look
>>>>>>>> like valid list of compatibles.
>>>>>>>>
>>>>>>> For 4908 board variant, it will need to be followed by
>>>>>>> 4908 chip. Sorry
>>>>>>> for the basic question but is there any requirement to
>>>>>>> enforce this kind
>>>>>>> of rule?  I would assume dts writer know what he/she is
>>>>>>> doing and select
>>>>>>> the right combination.
>>>>>>
>>>>>> The entire point of DT schema is to validate DTS.
>>>>>> Combination like above
>>>>>> prevents that goal.
>>>>>>
>>>>>> Best regards,
>>>>>> Krzysztof
>>>>> Understand the DT schema purpose. But items property allows multiple
>>>>> enums in the list which gives a lot of flexibility but make it hard to
>>>>> validate. I am not familiar with DT schema, is there any directive to
>>>>> specify one enum value depending on another so dts validation tool can
>>>>> report error if combination is wrong?
>>>>>
>>>>> This is our preferred format of all bcmbca compatible string
>>>>> especially when we could have more than 10 chip variants for the same
>>>>> chip family and we really want to work on the chip family id.  We will
>>>>> make sure they are in the right combination in our own patch and patch
>>>>> from other contributors. Would this work? If not, I will probably have
>>>>> to revert the change of 4908(maybe append brcm,bcmbca as this chip
>>>>> belongs to the same bca group) and use "enum board variant", "const
>>>>> main chip id", "brcm,bca" for all other chips as our secondary choice.
>>>>
>>>> I'm not sure why I didn't even receive 1/3 and half of discussion
>>>> e-mails.
>>>>
>>>> You can't just put all strings into a single bag and allow mixing them
>>>> in any combos. Please check how it's properly handled in the current
>>>> existing binding:
>>>> Documentation/devicetree/bindings/arm/bcm/brcm,bcm4908.yaml
>>>>
>>>> Above binding enforces that non-matching compatible strings are not used
>>>> together.
>>>
>>> I just noticed you're actually removing brcm,bcm4908.yaml in the 2/3 so
>>> you must be aware of that file.
>>>
>>> So you see a cleanly working binding in the brcm,bcm4908.yaml but
>>> instead copying it you decided to wrote your own one from scratch.
>>> Incorrectly.
>>>
>>> This smells of NIH (not invented here). Please just use that binding I
>>> wrote and move if it needed.
>>
>> Not mean to discredit any of your work and I did copy over your binding and
>> combine them into one SoC entry to the new bcmbca.yaml and add you as one of
>> the maintainer to this file. As this change would certainly concern you,
>> that's why I sent RFC first. As I explained in the cover letter, the
>> purpose of the change is to reduce the number of compatible strings and keep
>> one entry for one chip family due to possible large number of chip variants.
>> But since there is no way to validate the combination, I will copy the
>> existing 4908 bindings as they are now but I would propose to append "brcm,
>> bcmbca" as it is part of bcmbca chip. And for the other chips, we would just
>> use enum "board variant", const "main chip id", const "brcm,bca". Does that
>> sound good to you?
>
> If you want fewer combinations of compatibles, adding a genericish
> "brcm,bcmbca" is not going to help. Is there much value to adding it?
> What can you do with that information (and nothing else) is the
> question to ask.
>
Since we moved all the Broadcom Broadband BCA origin SoC to ARCH_BCMBCA
arch, I think it makes sense to have that as the last part of the
compatible string, not really for reducing the number. And for that
matter, we have agreed to not change anything to Rafal dts definitions
including the number of compat strings but just appending brcm,bca. So
we are all good now.

> Rob
>


Attachments:
smime.p7s (4.11 kB)
S/MIME Cryptographic Signature