All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
@ 2022-07-21  0:06 ` William Zhang
  0 siblings, 0 replies; 28+ messages in thread
From: William Zhang @ 2022-07-21  0:06 UTC (permalink / raw)
  To: Linux ARM List
  Cc: joel.peshkin, dan.beygelman, William Zhang, Rob Herring,
	Krzysztof Kozlowski, Anand Gore, Kursad Oney, Florian Fainelli,
	Rafał Miłecki, Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

[-- Attachment #1: Type: text/plain, Size: 1209 bytes --]

Append "brcm,bcmbca" to BCM4908 chip family compatible strings. Add
generic 4908 board entry.

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

 Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml b/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
index 6a64afa95918..4494a2c58c7f 100644
--- a/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
+++ b/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
@@ -36,18 +36,22 @@ properties:
               - tplink,archer-c2300-v1
           - const: brcm,bcm4906
           - const: brcm,bcm4908
+          - const: brcm,bcmbca
 
       - description: BCM4908 based boards
         items:
           - enum:
               - asus,gt-ac5300
               - netgear,raxe500
+              - brcm,bcm94908
           - const: brcm,bcm4908
+          - const: brcm,bcmbca
 
       - description: BCM49408 based boards
         items:
           - const: brcm,bcm49408
           - const: brcm,bcm4908
+          - const: brcm,bcmbca
 
       - description: BCM4912 based boards
         items:
-- 
2.34.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

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

* [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
@ 2022-07-21  0:06 ` William Zhang
  0 siblings, 0 replies; 28+ messages in thread
From: William Zhang @ 2022-07-21  0:06 UTC (permalink / raw)
  To: Linux ARM List
  Cc: joel.peshkin, dan.beygelman, William Zhang, Rob Herring,
	Krzysztof Kozlowski, Anand Gore, Kursad Oney, Florian Fainelli,
	Rafał Miłecki, Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list


[-- Attachment #1.1: Type: text/plain, Size: 1209 bytes --]

Append "brcm,bcmbca" to BCM4908 chip family compatible strings. Add
generic 4908 board entry.

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

 Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml b/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
index 6a64afa95918..4494a2c58c7f 100644
--- a/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
+++ b/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
@@ -36,18 +36,22 @@ properties:
               - tplink,archer-c2300-v1
           - const: brcm,bcm4906
           - const: brcm,bcm4908
+          - const: brcm,bcmbca
 
       - description: BCM4908 based boards
         items:
           - enum:
               - asus,gt-ac5300
               - netgear,raxe500
+              - brcm,bcm94908
           - const: brcm,bcm4908
+          - const: brcm,bcmbca
 
       - description: BCM49408 based boards
         items:
           - const: brcm,bcm49408
           - const: brcm,bcm4908
+          - const: brcm,bcmbca
 
       - description: BCM4912 based boards
         items:
-- 
2.34.1


[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
  2022-07-21  0:06 ` William Zhang
@ 2022-07-21  6:11   ` Rafał Miłecki
  -1 siblings, 0 replies; 28+ messages in thread
From: Rafał Miłecki @ 2022-07-21  6:11 UTC (permalink / raw)
  To: William Zhang
  Cc: Linux ARM List, joel.peshkin, dan.beygelman, Rob Herring,
	Krzysztof Kozlowski, Anand Gore, Kursad Oney, Florian Fainelli,
	Broadcom internal kernel review list, devicetree, linux-kernel

On 2022-07-21 02:06, William Zhang wrote:
> Append "brcm,bcmbca" to BCM4908 chip family compatible strings. Add
> generic 4908 board entry.
> 
> Signed-off-by: William Zhang <william.zhang@broadcom.com>

Acked-by: Rafał Miłecki <rafal@milecki.pl>

(with one minor comment below)

> ---
> 
>  Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git
> a/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
> b/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
> index 6a64afa95918..4494a2c58c7f 100644
> --- a/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
> +++ b/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
> @@ -36,18 +36,22 @@ properties:
>                - tplink,archer-c2300-v1
>            - const: brcm,bcm4906
>            - const: brcm,bcm4908
> +          - const: brcm,bcmbca
> 
>        - description: BCM4908 based boards
>          items:
>            - enum:
>                - asus,gt-ac5300
>                - netgear,raxe500
> +              - brcm,bcm94908

I think it would be nice to keep enum entries sorted


>            - const: brcm,bcm4908
> +          - const: brcm,bcmbca
> 
>        - description: BCM49408 based boards
>          items:
>            - const: brcm,bcm49408
>            - const: brcm,bcm4908
> +          - const: brcm,bcmbca
> 
>        - description: BCM4912 based boards
>          items:

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
@ 2022-07-21  6:11   ` Rafał Miłecki
  0 siblings, 0 replies; 28+ messages in thread
From: Rafał Miłecki @ 2022-07-21  6:11 UTC (permalink / raw)
  To: William Zhang
  Cc: Linux ARM List, joel.peshkin, dan.beygelman, Rob Herring,
	Krzysztof Kozlowski, Anand Gore, Kursad Oney, Florian Fainelli,
	Broadcom internal kernel review list, devicetree, linux-kernel

On 2022-07-21 02:06, William Zhang wrote:
> Append "brcm,bcmbca" to BCM4908 chip family compatible strings. Add
> generic 4908 board entry.
> 
> Signed-off-by: William Zhang <william.zhang@broadcom.com>

Acked-by: Rafał Miłecki <rafal@milecki.pl>

(with one minor comment below)

> ---
> 
>  Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git
> a/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
> b/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
> index 6a64afa95918..4494a2c58c7f 100644
> --- a/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
> +++ b/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
> @@ -36,18 +36,22 @@ properties:
>                - tplink,archer-c2300-v1
>            - const: brcm,bcm4906
>            - const: brcm,bcm4908
> +          - const: brcm,bcmbca
> 
>        - description: BCM4908 based boards
>          items:
>            - enum:
>                - asus,gt-ac5300
>                - netgear,raxe500
> +              - brcm,bcm94908

I think it would be nice to keep enum entries sorted


>            - const: brcm,bcm4908
> +          - const: brcm,bcmbca
> 
>        - description: BCM49408 based boards
>          items:
>            - const: brcm,bcm49408
>            - const: brcm,bcm4908
> +          - const: brcm,bcmbca
> 
>        - description: BCM4912 based boards
>          items:

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
  2022-07-21  0:06 ` William Zhang
@ 2022-07-21  6:44   ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-21  6:44 UTC (permalink / raw)
  To: William Zhang, Linux ARM List
  Cc: joel.peshkin, dan.beygelman, Rob Herring, Krzysztof Kozlowski,
	Anand Gore, Kursad Oney, Florian Fainelli,
	Rafał Miłecki, Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 21/07/2022 02:06, William Zhang wrote:
> Append "brcm,bcmbca" to BCM4908 chip family compatible strings. Add
> generic 4908 board entry.

This does not explain at all why you are doing it. Improve your commit
messages.

The explanation you gave here:
https://lore.kernel.org/all/b8eda882-6838-ab7d-6e2e-131e3125b16f@broadcom.com/
is also not really sufficient (and is not in commit msg). Moving things
around in Linux Kconfig does not justify adding some new compatibles.

To be - clear - this is not a review, so you cannot add Rb tag. If you
insist on a tag, then it as counted as NAK.

Best regards,
Krzysztof

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
@ 2022-07-21  6:44   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-21  6:44 UTC (permalink / raw)
  To: William Zhang, Linux ARM List
  Cc: joel.peshkin, dan.beygelman, Rob Herring, Krzysztof Kozlowski,
	Anand Gore, Kursad Oney, Florian Fainelli,
	Rafał Miłecki, Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 21/07/2022 02:06, William Zhang wrote:
> Append "brcm,bcmbca" to BCM4908 chip family compatible strings. Add
> generic 4908 board entry.

This does not explain at all why you are doing it. Improve your commit
messages.

The explanation you gave here:
https://lore.kernel.org/all/b8eda882-6838-ab7d-6e2e-131e3125b16f@broadcom.com/
is also not really sufficient (and is not in commit msg). Moving things
around in Linux Kconfig does not justify adding some new compatibles.

To be - clear - this is not a review, so you cannot add Rb tag. If you
insist on a tag, then it as counted as NAK.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
  2022-07-21  6:44   ` Krzysztof Kozlowski
@ 2022-07-21  6:51     ` Rafał Miłecki
  -1 siblings, 0 replies; 28+ messages in thread
From: Rafał Miłecki @ 2022-07-21  6:51 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: William Zhang, Linux ARM List, joel.peshkin, dan.beygelman,
	Rob Herring, Krzysztof Kozlowski, Anand Gore, Kursad Oney,
	Florian Fainelli, Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 2022-07-21 08:44, Krzysztof Kozlowski wrote:
> On 21/07/2022 02:06, William Zhang wrote:
>> Append "brcm,bcmbca" to BCM4908 chip family compatible strings. Add
>> generic 4908 board entry.
> 
> This does not explain at all why you are doing it. Improve your commit
> messages.

To clarify it from my side (and maybe help a bit):

1. As I understand it BCMBCA is a one big family of SoCs.
2. BCM4908 is a subset of that family (a subfamily?) designed for a
    specific group of devices.

If that's correct I think William it's what you should describe in your
commit message. That would make binding more accurate and should be a
good argument for your change (I believe).

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
@ 2022-07-21  6:51     ` Rafał Miłecki
  0 siblings, 0 replies; 28+ messages in thread
From: Rafał Miłecki @ 2022-07-21  6:51 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: William Zhang, Linux ARM List, joel.peshkin, dan.beygelman,
	Rob Herring, Krzysztof Kozlowski, Anand Gore, Kursad Oney,
	Florian Fainelli, Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 2022-07-21 08:44, Krzysztof Kozlowski wrote:
> On 21/07/2022 02:06, William Zhang wrote:
>> Append "brcm,bcmbca" to BCM4908 chip family compatible strings. Add
>> generic 4908 board entry.
> 
> This does not explain at all why you are doing it. Improve your commit
> messages.

To clarify it from my side (and maybe help a bit):

1. As I understand it BCMBCA is a one big family of SoCs.
2. BCM4908 is a subset of that family (a subfamily?) designed for a
    specific group of devices.

If that's correct I think William it's what you should describe in your
commit message. That would make binding more accurate and should be a
good argument for your change (I believe).

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
  2022-07-21  6:51     ` Rafał Miłecki
@ 2022-07-21  7:01       ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-21  7:01 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: William Zhang, Linux ARM List, joel.peshkin, dan.beygelman,
	Rob Herring, Krzysztof Kozlowski, Anand Gore, Kursad Oney,
	Florian Fainelli, Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 21/07/2022 08:51, Rafał Miłecki wrote:
> On 2022-07-21 08:44, Krzysztof Kozlowski wrote:
>> On 21/07/2022 02:06, William Zhang wrote:
>>> Append "brcm,bcmbca" to BCM4908 chip family compatible strings. Add
>>> generic 4908 board entry.
>>
>> This does not explain at all why you are doing it. Improve your commit
>> messages.
> 
> To clarify it from my side (and maybe help a bit):
> 
> 1. As I understand it BCMBCA is a one big family of SoCs.
> 2. BCM4908 is a subset of that family (a subfamily?) designed for a
>     specific group of devices.
> 
> If that's correct I think William it's what you should describe in your
> commit message. That would make binding more accurate and should be a
> good argument for your change (I believe).

That's better argument. But what's the benefit of adding generic
compatible? Devices cannot bind to it (it is too generic). Does it
describe the device anyhow? Imagine someone adding compatible
"brcm,all-soc-of-broadcom" - does it make any sense?

Best regards,
Krzysztof

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
@ 2022-07-21  7:01       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-21  7:01 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: William Zhang, Linux ARM List, joel.peshkin, dan.beygelman,
	Rob Herring, Krzysztof Kozlowski, Anand Gore, Kursad Oney,
	Florian Fainelli, Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 21/07/2022 08:51, Rafał Miłecki wrote:
> On 2022-07-21 08:44, Krzysztof Kozlowski wrote:
>> On 21/07/2022 02:06, William Zhang wrote:
>>> Append "brcm,bcmbca" to BCM4908 chip family compatible strings. Add
>>> generic 4908 board entry.
>>
>> This does not explain at all why you are doing it. Improve your commit
>> messages.
> 
> To clarify it from my side (and maybe help a bit):
> 
> 1. As I understand it BCMBCA is a one big family of SoCs.
> 2. BCM4908 is a subset of that family (a subfamily?) designed for a
>     specific group of devices.
> 
> If that's correct I think William it's what you should describe in your
> commit message. That would make binding more accurate and should be a
> good argument for your change (I believe).

That's better argument. But what's the benefit of adding generic
compatible? Devices cannot bind to it (it is too generic). Does it
describe the device anyhow? Imagine someone adding compatible
"brcm,all-soc-of-broadcom" - does it make any sense?

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
  2022-07-21  7:01       ` Krzysztof Kozlowski
@ 2022-07-21  7:13         ` Rafał Miłecki
  -1 siblings, 0 replies; 28+ messages in thread
From: Rafał Miłecki @ 2022-07-21  7:13 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: William Zhang, Linux ARM List, joel.peshkin, dan.beygelman,
	Rob Herring, Krzysztof Kozlowski, Anand Gore, Kursad Oney,
	Florian Fainelli, Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 2022-07-21 09:01, Krzysztof Kozlowski wrote:
> On 21/07/2022 08:51, Rafał Miłecki wrote:
>> On 2022-07-21 08:44, Krzysztof Kozlowski wrote:
>>> On 21/07/2022 02:06, William Zhang wrote:
>>>> Append "brcm,bcmbca" to BCM4908 chip family compatible strings. Add
>>>> generic 4908 board entry.
>>> 
>>> This does not explain at all why you are doing it. Improve your 
>>> commit
>>> messages.
>> 
>> To clarify it from my side (and maybe help a bit):
>> 
>> 1. As I understand it BCMBCA is a one big family of SoCs.
>> 2. BCM4908 is a subset of that family (a subfamily?) designed for a
>>     specific group of devices.
>> 
>> If that's correct I think William it's what you should describe in 
>> your
>> commit message. That would make binding more accurate and should be a
>> good argument for your change (I believe).
> 
> That's better argument. But what's the benefit of adding generic
> compatible? Devices cannot bind to it (it is too generic). Does it
> describe the device anyhow? Imagine someone adding compatible
> "brcm,all-soc-of-broadcom" - does it make any sense?

OK, I see it now. I can't think of any case of handling all devices
covered with suc a wide brcm,bcmbca binding.

This leads me to another question if we should actually totally drop
brcm,bcmbca from other SoCs bindings, see linux-next's
Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
@ 2022-07-21  7:13         ` Rafał Miłecki
  0 siblings, 0 replies; 28+ messages in thread
From: Rafał Miłecki @ 2022-07-21  7:13 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: William Zhang, Linux ARM List, joel.peshkin, dan.beygelman,
	Rob Herring, Krzysztof Kozlowski, Anand Gore, Kursad Oney,
	Florian Fainelli, Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 2022-07-21 09:01, Krzysztof Kozlowski wrote:
> On 21/07/2022 08:51, Rafał Miłecki wrote:
>> On 2022-07-21 08:44, Krzysztof Kozlowski wrote:
>>> On 21/07/2022 02:06, William Zhang wrote:
>>>> Append "brcm,bcmbca" to BCM4908 chip family compatible strings. Add
>>>> generic 4908 board entry.
>>> 
>>> This does not explain at all why you are doing it. Improve your 
>>> commit
>>> messages.
>> 
>> To clarify it from my side (and maybe help a bit):
>> 
>> 1. As I understand it BCMBCA is a one big family of SoCs.
>> 2. BCM4908 is a subset of that family (a subfamily?) designed for a
>>     specific group of devices.
>> 
>> If that's correct I think William it's what you should describe in 
>> your
>> commit message. That would make binding more accurate and should be a
>> good argument for your change (I believe).
> 
> That's better argument. But what's the benefit of adding generic
> compatible? Devices cannot bind to it (it is too generic). Does it
> describe the device anyhow? Imagine someone adding compatible
> "brcm,all-soc-of-broadcom" - does it make any sense?

OK, I see it now. I can't think of any case of handling all devices
covered with suc a wide brcm,bcmbca binding.

This leads me to another question if we should actually totally drop
brcm,bcmbca from other SoCs bindings, see linux-next's
Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
  2022-07-21  7:13         ` Rafał Miłecki
@ 2022-07-21  7:36           ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-21  7:36 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: William Zhang, Linux ARM List, joel.peshkin, dan.beygelman,
	Rob Herring, Krzysztof Kozlowski, Anand Gore, Kursad Oney,
	Florian Fainelli, Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 21/07/2022 09:13, Rafał Miłecki wrote:
>> That's better argument. But what's the benefit of adding generic
>> compatible? Devices cannot bind to it (it is too generic). Does it
>> describe the device anyhow? Imagine someone adding compatible
>> "brcm,all-soc-of-broadcom" - does it make any sense?
> 
> OK, I see it now. I can't think of any case of handling all devices
> covered with suc a wide brcm,bcmbca binding.

Maybe there is some common part of a SoC which that generic compatible
would express?

Most archs don't use soc-wide generic compatible, because of reasons I
mentioned - no actual benefits for anyone from such compatible.

But there are exceptions. I fouun socfpga and apple. The apple sounds as
mistake to me, because the generic "apple,arm-platform" compatible looks
like covering all possible Apple ARM platforms. I think Apple ARM
designs in 20 years will not be compatible at all with current design,
so such broad compatible is not useful... but that's only my opinion.

> 
> This leads me to another question if we should actually totally drop
> brcm,bcmbca from other SoCs bindings, see linux-next's
> Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml

This would be tricky as it was already accepted, unless all sit in
linux-next and did not make to v5.19-rc1.

Best regards,
Krzysztof

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
@ 2022-07-21  7:36           ` Krzysztof Kozlowski
  0 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-21  7:36 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: William Zhang, Linux ARM List, joel.peshkin, dan.beygelman,
	Rob Herring, Krzysztof Kozlowski, Anand Gore, Kursad Oney,
	Florian Fainelli, Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 21/07/2022 09:13, Rafał Miłecki wrote:
>> That's better argument. But what's the benefit of adding generic
>> compatible? Devices cannot bind to it (it is too generic). Does it
>> describe the device anyhow? Imagine someone adding compatible
>> "brcm,all-soc-of-broadcom" - does it make any sense?
> 
> OK, I see it now. I can't think of any case of handling all devices
> covered with suc a wide brcm,bcmbca binding.

Maybe there is some common part of a SoC which that generic compatible
would express?

Most archs don't use soc-wide generic compatible, because of reasons I
mentioned - no actual benefits for anyone from such compatible.

But there are exceptions. I fouun socfpga and apple. The apple sounds as
mistake to me, because the generic "apple,arm-platform" compatible looks
like covering all possible Apple ARM platforms. I think Apple ARM
designs in 20 years will not be compatible at all with current design,
so such broad compatible is not useful... but that's only my opinion.

> 
> This leads me to another question if we should actually totally drop
> brcm,bcmbca from other SoCs bindings, see linux-next's
> Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml

This would be tricky as it was already accepted, unless all sit in
linux-next and did not make to v5.19-rc1.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
  2022-07-21  7:36           ` Krzysztof Kozlowski
@ 2022-07-21  7:50             ` Rafał Miłecki
  -1 siblings, 0 replies; 28+ messages in thread
From: Rafał Miłecki @ 2022-07-21  7:50 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: William Zhang, Linux ARM List, joel.peshkin, dan.beygelman,
	Rob Herring, Krzysztof Kozlowski, Anand Gore, Kursad Oney,
	Florian Fainelli, Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 2022-07-21 09:36, Krzysztof Kozlowski wrote:
> On 21/07/2022 09:13, Rafał Miłecki wrote:
>>> That's better argument. But what's the benefit of adding generic
>>> compatible? Devices cannot bind to it (it is too generic). Does it
>>> describe the device anyhow? Imagine someone adding compatible
>>> "brcm,all-soc-of-broadcom" - does it make any sense?
>> 
>> OK, I see it now. I can't think of any case of handling all devices
>> covered with suc a wide brcm,bcmbca binding.
> 
> Maybe there is some common part of a SoC which that generic compatible
> would express?
> 
> Most archs don't use soc-wide generic compatible, because of reasons I
> mentioned - no actual benefits for anyone from such compatible.
> 
> But there are exceptions. I fouun socfpga and apple. The apple sounds 
> as
> mistake to me, because the generic "apple,arm-platform" compatible 
> looks
> like covering all possible Apple ARM platforms. I think Apple ARM
> designs in 20 years will not be compatible at all with current design,
> so such broad compatible is not useful... but that's only my opinion.

Let's see if William / Broadcom guys can provide a valid argument for
the brcm,bcmbca.


>> This leads me to another question if we should actually totally drop
>> brcm,bcmbca from other SoCs bindings, see linux-next's
>> Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
> 
> This would be tricky as it was already accepted, unless all sit in
> linux-next and did not make to v5.19-rc1.

5.19-rc7 has only 1 case with brcm,bcmbca, see ff6992735ade7
("Linux 5.19-rc7"):
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml?id=ff6992735ade75aae3e35d16b17da1008d753d28

So we can still clean it up for the 5.20-rc1 or 5.20-rc2.

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
@ 2022-07-21  7:50             ` Rafał Miłecki
  0 siblings, 0 replies; 28+ messages in thread
From: Rafał Miłecki @ 2022-07-21  7:50 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: William Zhang, Linux ARM List, joel.peshkin, dan.beygelman,
	Rob Herring, Krzysztof Kozlowski, Anand Gore, Kursad Oney,
	Florian Fainelli, Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 2022-07-21 09:36, Krzysztof Kozlowski wrote:
> On 21/07/2022 09:13, Rafał Miłecki wrote:
>>> That's better argument. But what's the benefit of adding generic
>>> compatible? Devices cannot bind to it (it is too generic). Does it
>>> describe the device anyhow? Imagine someone adding compatible
>>> "brcm,all-soc-of-broadcom" - does it make any sense?
>> 
>> OK, I see it now. I can't think of any case of handling all devices
>> covered with suc a wide brcm,bcmbca binding.
> 
> Maybe there is some common part of a SoC which that generic compatible
> would express?
> 
> Most archs don't use soc-wide generic compatible, because of reasons I
> mentioned - no actual benefits for anyone from such compatible.
> 
> But there are exceptions. I fouun socfpga and apple. The apple sounds 
> as
> mistake to me, because the generic "apple,arm-platform" compatible 
> looks
> like covering all possible Apple ARM platforms. I think Apple ARM
> designs in 20 years will not be compatible at all with current design,
> so such broad compatible is not useful... but that's only my opinion.

Let's see if William / Broadcom guys can provide a valid argument for
the brcm,bcmbca.


>> This leads me to another question if we should actually totally drop
>> brcm,bcmbca from other SoCs bindings, see linux-next's
>> Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
> 
> This would be tricky as it was already accepted, unless all sit in
> linux-next and did not make to v5.19-rc1.

5.19-rc7 has only 1 case with brcm,bcmbca, see ff6992735ade7
("Linux 5.19-rc7"):
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml?id=ff6992735ade75aae3e35d16b17da1008d753d28

So we can still clean it up for the 5.20-rc1 or 5.20-rc2.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
  2022-07-21  7:50             ` Rafał Miłecki
@ 2022-07-21 16:43               ` Florian Fainelli
  -1 siblings, 0 replies; 28+ messages in thread
From: Florian Fainelli @ 2022-07-21 16:43 UTC (permalink / raw)
  To: Rafał Miłecki, Krzysztof Kozlowski
  Cc: William Zhang, Linux ARM List, joel.peshkin, dan.beygelman,
	Rob Herring, Krzysztof Kozlowski, Anand Gore, Kursad Oney,
	Florian Fainelli, Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 7/21/22 00:50, Rafał Miłecki wrote:
> On 2022-07-21 09:36, Krzysztof Kozlowski wrote:
>> On 21/07/2022 09:13, Rafał Miłecki wrote:
>>>> That's better argument. But what's the benefit of adding generic
>>>> compatible? Devices cannot bind to it (it is too generic). Does it
>>>> describe the device anyhow? Imagine someone adding compatible
>>>> "brcm,all-soc-of-broadcom" - does it make any sense?
>>>
>>> OK, I see it now. I can't think of any case of handling all devices
>>> covered with suc a wide brcm,bcmbca binding.
>>
>> Maybe there is some common part of a SoC which that generic compatible
>> would express?
>>
>> Most archs don't use soc-wide generic compatible, because of reasons I
>> mentioned - no actual benefits for anyone from such compatible.
>>
>> But there are exceptions. I fouun socfpga and apple. The apple sounds as
>> mistake to me, because the generic "apple,arm-platform" compatible looks
>> like covering all possible Apple ARM platforms. I think Apple ARM
>> designs in 20 years will not be compatible at all with current design,
>> so such broad compatible is not useful... but that's only my opinion.
> 
> Let's see if William / Broadcom guys can provide a valid argument for
> the brcm,bcmbca.

It is common practice to provide a generic fallback compatible string for a given chip family/architecture and all of our existing in-tree (and out of tree for that matter) DTSes for Broadcom SoCs do that:

- Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt
- Documentation/devicetree/bindings/arm/bcm/brcm,ns2.yaml
- Documentation/devicetree/bindings/arm/bcm/brcm,nsp.yaml
- Documentation/devicetree/bindings/arm/bcm/brcm,cygnus.yaml

list goes on and on, of course the counter examples are bcm2835, bcm4708 etc. although those are both chip and families technically, so I suppose the conflation is appropriate in that case. So the pattern is simple:

- outside of Broadcom contributors used convention
- inside of Broadcom contributors used another

so if nothing else, we ought to be consistent within ourselves as Broadcom insiders, which we are doing here.

While the generic fallback may not be in use, it still serves a purpose:

- Broadcom likes to create a gazillion of part numbers that are hard to untangle from their original SoC architecture unless you happen to work there so it serves as documentation for others to identify what family they belong to, and what to expect from that

- you never know when you might want to be matching on just the generic compatible string of a family and putting it in there costs nothing at all

The point of William's patch series is to right a number of wrongs on Broadcom's side:

- lack of appropriate involvements at the time Rafal submitted the 4908 support as a "standalone" family, I am to take the blame for suggesting that name in the first place, though I did not know at the time that William and others would ever be contributing upstream

- avoid the proliferation of "sub" families within a larger family (BCMBCA) since that serves no purpose other than to make it harder on users to select what they should be selecting in their kernel *and* it makes us inconsistent with arch/arm64/Kconfig.platforms that attempts to reduce those

I would conclude by asking you: why is this such a big issue? What *actual* problem does it causes, except maybe setting a precedent that you do not like, but for which you practically should have no reason to care as long as the binding is enforced.

> 
> 
>>> This leads me to another question if we should actually totally drop
>>> brcm,bcmbca from other SoCs bindings, see linux-next's
>>> Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
>>
>> This would be tricky as it was already accepted, unless all sit in
>> linux-next and did not make to v5.19-rc1.
> 
> 5.19-rc7 has only 1 case with brcm,bcmbca, see ff6992735ade7
> ("Linux 5.19-rc7"):
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml?id=ff6992735ade75aae3e35d16b17da1008d753d28
> 
> So we can still clean it up for the 5.20-rc1 or 5.20-rc2.

Not so fast.
-- 
Florian

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
@ 2022-07-21 16:43               ` Florian Fainelli
  0 siblings, 0 replies; 28+ messages in thread
From: Florian Fainelli @ 2022-07-21 16:43 UTC (permalink / raw)
  To: Rafał Miłecki, Krzysztof Kozlowski
  Cc: William Zhang, Linux ARM List, joel.peshkin, dan.beygelman,
	Rob Herring, Krzysztof Kozlowski, Anand Gore, Kursad Oney,
	Florian Fainelli, Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 7/21/22 00:50, Rafał Miłecki wrote:
> On 2022-07-21 09:36, Krzysztof Kozlowski wrote:
>> On 21/07/2022 09:13, Rafał Miłecki wrote:
>>>> That's better argument. But what's the benefit of adding generic
>>>> compatible? Devices cannot bind to it (it is too generic). Does it
>>>> describe the device anyhow? Imagine someone adding compatible
>>>> "brcm,all-soc-of-broadcom" - does it make any sense?
>>>
>>> OK, I see it now. I can't think of any case of handling all devices
>>> covered with suc a wide brcm,bcmbca binding.
>>
>> Maybe there is some common part of a SoC which that generic compatible
>> would express?
>>
>> Most archs don't use soc-wide generic compatible, because of reasons I
>> mentioned - no actual benefits for anyone from such compatible.
>>
>> But there are exceptions. I fouun socfpga and apple. The apple sounds as
>> mistake to me, because the generic "apple,arm-platform" compatible looks
>> like covering all possible Apple ARM platforms. I think Apple ARM
>> designs in 20 years will not be compatible at all with current design,
>> so such broad compatible is not useful... but that's only my opinion.
> 
> Let's see if William / Broadcom guys can provide a valid argument for
> the brcm,bcmbca.

It is common practice to provide a generic fallback compatible string for a given chip family/architecture and all of our existing in-tree (and out of tree for that matter) DTSes for Broadcom SoCs do that:

- Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt
- Documentation/devicetree/bindings/arm/bcm/brcm,ns2.yaml
- Documentation/devicetree/bindings/arm/bcm/brcm,nsp.yaml
- Documentation/devicetree/bindings/arm/bcm/brcm,cygnus.yaml

list goes on and on, of course the counter examples are bcm2835, bcm4708 etc. although those are both chip and families technically, so I suppose the conflation is appropriate in that case. So the pattern is simple:

- outside of Broadcom contributors used convention
- inside of Broadcom contributors used another

so if nothing else, we ought to be consistent within ourselves as Broadcom insiders, which we are doing here.

While the generic fallback may not be in use, it still serves a purpose:

- Broadcom likes to create a gazillion of part numbers that are hard to untangle from their original SoC architecture unless you happen to work there so it serves as documentation for others to identify what family they belong to, and what to expect from that

- you never know when you might want to be matching on just the generic compatible string of a family and putting it in there costs nothing at all

The point of William's patch series is to right a number of wrongs on Broadcom's side:

- lack of appropriate involvements at the time Rafal submitted the 4908 support as a "standalone" family, I am to take the blame for suggesting that name in the first place, though I did not know at the time that William and others would ever be contributing upstream

- avoid the proliferation of "sub" families within a larger family (BCMBCA) since that serves no purpose other than to make it harder on users to select what they should be selecting in their kernel *and* it makes us inconsistent with arch/arm64/Kconfig.platforms that attempts to reduce those

I would conclude by asking you: why is this such a big issue? What *actual* problem does it causes, except maybe setting a precedent that you do not like, but for which you practically should have no reason to care as long as the binding is enforced.

> 
> 
>>> This leads me to another question if we should actually totally drop
>>> brcm,bcmbca from other SoCs bindings, see linux-next's
>>> Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
>>
>> This would be tricky as it was already accepted, unless all sit in
>> linux-next and did not make to v5.19-rc1.
> 
> 5.19-rc7 has only 1 case with brcm,bcmbca, see ff6992735ade7
> ("Linux 5.19-rc7"):
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml?id=ff6992735ade75aae3e35d16b17da1008d753d28
> 
> So we can still clean it up for the 5.20-rc1 or 5.20-rc2.

Not so fast.
-- 
Florian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
  2022-07-21 16:43               ` Florian Fainelli
@ 2022-07-21 19:35                 ` William Zhang
  -1 siblings, 0 replies; 28+ messages in thread
From: William Zhang @ 2022-07-21 19:35 UTC (permalink / raw)
  To: Florian Fainelli, Rafał Miłecki, Krzysztof Kozlowski
  Cc: Linux ARM List, joel.peshkin, dan.beygelman, Rob Herring,
	Krzysztof Kozlowski, Anand Gore, Kursad Oney,
	Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

[-- Attachment #1: Type: text/plain, Size: 5574 bytes --]



On 07/21/2022 09:43 AM, Florian Fainelli wrote:
> On 7/21/22 00:50, Rafał Miłecki wrote:
>> On 2022-07-21 09:36, Krzysztof Kozlowski wrote:
>>> On 21/07/2022 09:13, Rafał Miłecki wrote:
>>>>> That's better argument. But what's the benefit of adding generic
>>>>> compatible? Devices cannot bind to it (it is too generic). Does it
>>>>> describe the device anyhow? Imagine someone adding compatible
>>>>> "brcm,all-soc-of-broadcom" - does it make any sense?


>>>>
>>>> OK, I see it now. I can't think of any case of handling all devices
>>>> covered with suc a wide brcm,bcmbca binding.
>>>
>>> Maybe there is some common part of a SoC which that generic compatible
>>> would express?
>>>
>>> Most archs don't use soc-wide generic compatible, because of reasons I
>>> mentioned - no actual benefits for anyone from such compatible.
>>>
>>> But there are exceptions. I fouun socfpga and apple. The apple sounds as
>>> mistake to me, because the generic "apple,arm-platform" compatible looks
>>> like covering all possible Apple ARM platforms. I think Apple ARM
>>> designs in 20 years will not be compatible at all with current design,
>>> so such broad compatible is not useful... but that's only my opinion.
>>
>> Let's see if William / Broadcom guys can provide a valid argument for
>> the brcm,bcmbca.
> 
> It is common practice to provide a generic fallback compatible string for a given chip family/architecture and all of our existing in-tree (and out of tree for that matter) DTSes for Broadcom SoCs do that:
> 
> - Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt
> - Documentation/devicetree/bindings/arm/bcm/brcm,ns2.yaml
> - Documentation/devicetree/bindings/arm/bcm/brcm,nsp.yaml
> - Documentation/devicetree/bindings/arm/bcm/brcm,cygnus.yaml
> 
> list goes on and on, of course the counter examples are bcm2835, bcm4708 etc. although those are both chip and families technically, so I suppose the conflation is appropriate in that case. So the pattern is simple:
> 
> - outside of Broadcom contributors used convention
> - inside of Broadcom contributors used another
> 
> so if nothing else, we ought to be consistent within ourselves as Broadcom insiders, which we are doing here.
> 
> While the generic fallback may not be in use, it still serves a purpose:
> 
> - Broadcom likes to create a gazillion of part numbers that are hard to untangle from their original SoC architecture unless you happen to work there so it serves as documentation for others to identify what family they belong to, and what to expect from that
> 
> - you never know when you might want to be matching on just the generic compatible string of a family and putting it in there costs nothing at all
> 
> The point of William's patch series is to right a number of wrongs on Broadcom's side:
> 
> - lack of appropriate involvements at the time Rafal submitted the 4908 support as a "standalone" family, I am to take the blame for suggesting that name in the first place, though I did not know at the time that William and others would ever be contributing upstream
> 
> - avoid the proliferation of "sub" families within a larger family (BCMBCA) since that serves no purpose other than to make it harder on users to select what they should be selecting in their kernel *and* it makes us inconsistent with arch/arm64/Kconfig.platforms that attempts to reduce those
> 
> I would conclude by asking you: why is this such a big issue? What *actual* problem does it causes, except maybe setting a precedent that you do not like, but for which you practically should have no reason to care as long as the binding is enforced.
> 
Totally agreed.  Just want to emphasize that the main reason of this 
change to merge all the bcmbca family chips into the same group for 
better organization and reduce the clutter to the kernel source. Think 
about 18 bcmbca devices with different changes to yaml, kconfig, 
makefile, sub-directory while they actually share many common blocks, 
drivers and dts entries.

While nobody would bind a device to brcm,bcmbca (and it can not be 
binded because bcmbca,yaml require to prefix with a specific chip), it 
is a great way to make it easy for people to understand what device they 
are working and look for the bcmbca common driver and other support code 
as well. Wouldn't this be good thing to have?

And don't forget we introduced bcmbca awhile back with first SoC 47622 
and have set a clear goal for the purpose we are discussing here today:
Please see this patch series:
http://lists.infradead.org/pipermail/linux-arm-kernel/2022-April/732867.html
"This change introduces Broadcom's ARCH_BCMBCA architecture for
armv7 and armv8 based Broadband SoCs. We expect to send additional
patches for each SoC in the near future."

And Krzysztof acked that yaml for brcm,bcabca here :
http://lists.infradead.org/pipermail/linux-arm-kernel/2022-April/732867.html


>>
>>
>>>> This leads me to another question if we should actually totally drop
>>>> brcm,bcmbca from other SoCs bindings, see linux-next's
>>>> Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
>>>
>>> This would be tricky as it was already accepted, unless all sit in
>>> linux-next and did not make to v5.19-rc1.
>>
>> 5.19-rc7 has only 1 case with brcm,bcmbca, see ff6992735ade7
>> ("Linux 5.19-rc7"):
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml?id=ff6992735ade75aae3e35d16b17da1008d753d28
>>
>> So we can still clean it up for the 5.20-rc1 or 5.20-rc2.
> 
> Not so fast.
> 

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
@ 2022-07-21 19:35                 ` William Zhang
  0 siblings, 0 replies; 28+ messages in thread
From: William Zhang @ 2022-07-21 19:35 UTC (permalink / raw)
  To: Florian Fainelli, Rafał Miłecki, Krzysztof Kozlowski
  Cc: Linux ARM List, joel.peshkin, dan.beygelman, Rob Herring,
	Krzysztof Kozlowski, Anand Gore, Kursad Oney,
	Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list


[-- Attachment #1.1: Type: text/plain, Size: 5574 bytes --]



On 07/21/2022 09:43 AM, Florian Fainelli wrote:
> On 7/21/22 00:50, Rafał Miłecki wrote:
>> On 2022-07-21 09:36, Krzysztof Kozlowski wrote:
>>> On 21/07/2022 09:13, Rafał Miłecki wrote:
>>>>> That's better argument. But what's the benefit of adding generic
>>>>> compatible? Devices cannot bind to it (it is too generic). Does it
>>>>> describe the device anyhow? Imagine someone adding compatible
>>>>> "brcm,all-soc-of-broadcom" - does it make any sense?


>>>>
>>>> OK, I see it now. I can't think of any case of handling all devices
>>>> covered with suc a wide brcm,bcmbca binding.
>>>
>>> Maybe there is some common part of a SoC which that generic compatible
>>> would express?
>>>
>>> Most archs don't use soc-wide generic compatible, because of reasons I
>>> mentioned - no actual benefits for anyone from such compatible.
>>>
>>> But there are exceptions. I fouun socfpga and apple. The apple sounds as
>>> mistake to me, because the generic "apple,arm-platform" compatible looks
>>> like covering all possible Apple ARM platforms. I think Apple ARM
>>> designs in 20 years will not be compatible at all with current design,
>>> so such broad compatible is not useful... but that's only my opinion.
>>
>> Let's see if William / Broadcom guys can provide a valid argument for
>> the brcm,bcmbca.
> 
> It is common practice to provide a generic fallback compatible string for a given chip family/architecture and all of our existing in-tree (and out of tree for that matter) DTSes for Broadcom SoCs do that:
> 
> - Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt
> - Documentation/devicetree/bindings/arm/bcm/brcm,ns2.yaml
> - Documentation/devicetree/bindings/arm/bcm/brcm,nsp.yaml
> - Documentation/devicetree/bindings/arm/bcm/brcm,cygnus.yaml
> 
> list goes on and on, of course the counter examples are bcm2835, bcm4708 etc. although those are both chip and families technically, so I suppose the conflation is appropriate in that case. So the pattern is simple:
> 
> - outside of Broadcom contributors used convention
> - inside of Broadcom contributors used another
> 
> so if nothing else, we ought to be consistent within ourselves as Broadcom insiders, which we are doing here.
> 
> While the generic fallback may not be in use, it still serves a purpose:
> 
> - Broadcom likes to create a gazillion of part numbers that are hard to untangle from their original SoC architecture unless you happen to work there so it serves as documentation for others to identify what family they belong to, and what to expect from that
> 
> - you never know when you might want to be matching on just the generic compatible string of a family and putting it in there costs nothing at all
> 
> The point of William's patch series is to right a number of wrongs on Broadcom's side:
> 
> - lack of appropriate involvements at the time Rafal submitted the 4908 support as a "standalone" family, I am to take the blame for suggesting that name in the first place, though I did not know at the time that William and others would ever be contributing upstream
> 
> - avoid the proliferation of "sub" families within a larger family (BCMBCA) since that serves no purpose other than to make it harder on users to select what they should be selecting in their kernel *and* it makes us inconsistent with arch/arm64/Kconfig.platforms that attempts to reduce those
> 
> I would conclude by asking you: why is this such a big issue? What *actual* problem does it causes, except maybe setting a precedent that you do not like, but for which you practically should have no reason to care as long as the binding is enforced.
> 
Totally agreed.  Just want to emphasize that the main reason of this 
change to merge all the bcmbca family chips into the same group for 
better organization and reduce the clutter to the kernel source. Think 
about 18 bcmbca devices with different changes to yaml, kconfig, 
makefile, sub-directory while they actually share many common blocks, 
drivers and dts entries.

While nobody would bind a device to brcm,bcmbca (and it can not be 
binded because bcmbca,yaml require to prefix with a specific chip), it 
is a great way to make it easy for people to understand what device they 
are working and look for the bcmbca common driver and other support code 
as well. Wouldn't this be good thing to have?

And don't forget we introduced bcmbca awhile back with first SoC 47622 
and have set a clear goal for the purpose we are discussing here today:
Please see this patch series:
http://lists.infradead.org/pipermail/linux-arm-kernel/2022-April/732867.html
"This change introduces Broadcom's ARCH_BCMBCA architecture for
armv7 and armv8 based Broadband SoCs. We expect to send additional
patches for each SoC in the near future."

And Krzysztof acked that yaml for brcm,bcabca here :
http://lists.infradead.org/pipermail/linux-arm-kernel/2022-April/732867.html


>>
>>
>>>> This leads me to another question if we should actually totally drop
>>>> brcm,bcmbca from other SoCs bindings, see linux-next's
>>>> Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml
>>>
>>> This would be tricky as it was already accepted, unless all sit in
>>> linux-next and did not make to v5.19-rc1.
>>
>> 5.19-rc7 has only 1 case with brcm,bcmbca, see ff6992735ade7
>> ("Linux 5.19-rc7"):
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/arm/bcm/brcm,bcmbca.yaml?id=ff6992735ade75aae3e35d16b17da1008d753d28
>>
>> So we can still clean it up for the 5.20-rc1 or 5.20-rc2.
> 
> Not so fast.
> 

[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
  2022-07-21  7:01       ` Krzysztof Kozlowski
@ 2022-07-22  0:07         ` William Zhang
  -1 siblings, 0 replies; 28+ messages in thread
From: William Zhang @ 2022-07-22  0:07 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rafał Miłecki
  Cc: Linux ARM List, joel.peshkin, dan.beygelman, Rob Herring,
	Krzysztof Kozlowski, Anand Gore, Kursad Oney, Florian Fainelli,
	Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list



On 07/21/2022 12:01 AM, Krzysztof Kozlowski wrote:
> On 21/07/2022 08:51, Rafał Miłecki wrote:
>> On 2022-07-21 08:44, Krzysztof Kozlowski wrote:
>>> On 21/07/2022 02:06, William Zhang wrote:
>>>> Append "brcm,bcmbca" to BCM4908 chip family compatible strings. Add
>>>> generic 4908 board entry.
>>>
>>> This does not explain at all why you are doing it. Improve your commit
>>> messages.
>>
>> To clarify it from my side (and maybe help a bit):
>>
>> 1. As I understand it BCMBCA is a one big family of SoCs.
>> 2. BCM4908 is a subset of that family (a subfamily?) designed for a
>>      specific group of devices.
>>
>> If that's correct I think William it's what you should describe in your
>> commit message. That would make binding more accurate and should be a
>> good argument for your change (I believe).
> 
> That's better argument. But what's the benefit of adding generic
> compatible? Devices cannot bind to it (it is too generic). Does it
> describe the device anyhow? Imagine someone adding compatible
> "brcm,all-soc-of-broadcom" - does it make any sense?
>
In case you were also referring the generic 4908 board compatible string 
brcm,bcm94908, this is for a bare bone 4908 board dts that only enables 
ARM cpu subsystem, memory and uart. It can be used on all 4908 based 
Broadcom reference boards and customer board. It is especially useful 
for initial board bring up and one can load this generic board and start 
work and debug from the console. Also would be helpful to do a quick 
verification of new kernel version when there is cpu subsystem related 
change.

I guess my mindset already assume people are now familiar with this 
model of bcmbca binding addition for a new SoC since we introduced the 
bcmbca arch with first soc 47622 and 10+ other socs late. But sure I 
agree and I will update the commit message with more details in addition 
to what the cover letter says.

> Best regards,
> Krzysztof
> 


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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
@ 2022-07-22  0:07         ` William Zhang
  0 siblings, 0 replies; 28+ messages in thread
From: William Zhang @ 2022-07-22  0:07 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rafał Miłecki
  Cc: Linux ARM List, joel.peshkin, dan.beygelman, Rob Herring,
	Krzysztof Kozlowski, Anand Gore, Kursad Oney, Florian Fainelli,
	Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list



On 07/21/2022 12:01 AM, Krzysztof Kozlowski wrote:
> On 21/07/2022 08:51, Rafał Miłecki wrote:
>> On 2022-07-21 08:44, Krzysztof Kozlowski wrote:
>>> On 21/07/2022 02:06, William Zhang wrote:
>>>> Append "brcm,bcmbca" to BCM4908 chip family compatible strings. Add
>>>> generic 4908 board entry.
>>>
>>> This does not explain at all why you are doing it. Improve your commit
>>> messages.
>>
>> To clarify it from my side (and maybe help a bit):
>>
>> 1. As I understand it BCMBCA is a one big family of SoCs.
>> 2. BCM4908 is a subset of that family (a subfamily?) designed for a
>>      specific group of devices.
>>
>> If that's correct I think William it's what you should describe in your
>> commit message. That would make binding more accurate and should be a
>> good argument for your change (I believe).
> 
> That's better argument. But what's the benefit of adding generic
> compatible? Devices cannot bind to it (it is too generic). Does it
> describe the device anyhow? Imagine someone adding compatible
> "brcm,all-soc-of-broadcom" - does it make any sense?
>
In case you were also referring the generic 4908 board compatible string 
brcm,bcm94908, this is for a bare bone 4908 board dts that only enables 
ARM cpu subsystem, memory and uart. It can be used on all 4908 based 
Broadcom reference boards and customer board. It is especially useful 
for initial board bring up and one can load this generic board and start 
work and debug from the console. Also would be helpful to do a quick 
verification of new kernel version when there is cpu subsystem related 
change.

I guess my mindset already assume people are now familiar with this 
model of bcmbca binding addition for a new SoC since we introduced the 
bcmbca arch with first soc 47622 and 10+ other socs late. But sure I 
agree and I will update the commit message with more details in addition 
to what the cover letter says.

> Best regards,
> Krzysztof
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
  2022-07-21 19:35                 ` William Zhang
@ 2022-07-22 18:22                   ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-22 18:22 UTC (permalink / raw)
  To: William Zhang, Florian Fainelli, Rafał Miłecki
  Cc: Linux ARM List, joel.peshkin, dan.beygelman, Rob Herring,
	Krzysztof Kozlowski, Anand Gore, Kursad Oney,
	Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 21/07/2022 21:35, William Zhang wrote:
> 
> 
> On 07/21/2022 09:43 AM, Florian Fainelli wrote:
>> On 7/21/22 00:50, Rafał Miłecki wrote:
>>> On 2022-07-21 09:36, Krzysztof Kozlowski wrote:
>>>> On 21/07/2022 09:13, Rafał Miłecki wrote:
>>>>>> That's better argument. But what's the benefit of adding generic
>>>>>> compatible? Devices cannot bind to it (it is too generic). Does it
>>>>>> describe the device anyhow? Imagine someone adding compatible
>>>>>> "brcm,all-soc-of-broadcom" - does it make any sense?
> 
> 
>>>>>
>>>>> OK, I see it now. I can't think of any case of handling all devices
>>>>> covered with suc a wide brcm,bcmbca binding.
>>>>
>>>> Maybe there is some common part of a SoC which that generic compatible
>>>> would express?
>>>>
>>>> Most archs don't use soc-wide generic compatible, because of reasons I
>>>> mentioned - no actual benefits for anyone from such compatible.
>>>>
>>>> But there are exceptions. I fouun socfpga and apple. The apple sounds as
>>>> mistake to me, because the generic "apple,arm-platform" compatible looks
>>>> like covering all possible Apple ARM platforms. I think Apple ARM
>>>> designs in 20 years will not be compatible at all with current design,
>>>> so such broad compatible is not useful... but that's only my opinion.
>>>
>>> Let's see if William / Broadcom guys can provide a valid argument for
>>> the brcm,bcmbca.
>>
>> It is common practice to provide a generic fallback compatible string for a given chip family/architecture and all of our existing in-tree (and out of tree for that matter) DTSes for Broadcom SoCs do that:
>>
>> - Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt
>> - Documentation/devicetree/bindings/arm/bcm/brcm,ns2.yaml
>> - Documentation/devicetree/bindings/arm/bcm/brcm,nsp.yaml
>> - Documentation/devicetree/bindings/arm/bcm/brcm,cygnus.yaml
>>
>> list goes on and on, of course the counter examples are bcm2835, bcm4708 etc. although those are both chip and families technically, so I suppose the conflation is appropriate in that case. So the pattern is simple:
>>
>> - outside of Broadcom contributors used convention
>> - inside of Broadcom contributors used another
>>
>> so if nothing else, we ought to be consistent within ourselves as Broadcom insiders, which we are doing here.
>>
>> While the generic fallback may not be in use, it still serves a purpose:
>>
>> - Broadcom likes to create a gazillion of part numbers that are hard to untangle from their original SoC architecture unless you happen to work there so it serves as documentation for others to identify what family they belong to, and what to expect from that
>>
>> - you never know when you might want to be matching on just the generic compatible string of a family and putting it in there costs nothing at all
>>
>> The point of William's patch series is to right a number of wrongs on Broadcom's side:
>>
>> - lack of appropriate involvements at the time Rafal submitted the 4908 support as a "standalone" family, I am to take the blame for suggesting that name in the first place, though I did not know at the time that William and others would ever be contributing upstream
>>
>> - avoid the proliferation of "sub" families within a larger family (BCMBCA) since that serves no purpose other than to make it harder on users to select what they should be selecting in their kernel *and* it makes us inconsistent with arch/arm64/Kconfig.platforms that attempts to reduce those
>>
>> I would conclude by asking you: why is this such a big issue? What *actual* problem does it causes, except maybe setting a precedent that you do not like, but for which you practically should have no reason to care as long as the binding is enforced.
>>
> Totally agreed.  Just want to emphasize that the main reason of this 
> change to merge all the bcmbca family chips into the same group for 
> better organization and reduce the clutter to the kernel source. Think 
> about 18 bcmbca devices with different changes to yaml, kconfig, 
> makefile, sub-directory while they actually share many common blocks, 
> drivers and dts entries.

Your commit does not explain why you are doing it at all. We have all
this discussion, you put so many arguments, but why none of it is in the
commit description?

> 
> While nobody would bind a device to brcm,bcmbca (and it can not be 
> binded because bcmbca,yaml require to prefix with a specific chip), it 
> is a great way to make it easy for people to understand what device they 
> are working and look for the bcmbca common driver and other support code 
> as well. Wouldn't this be good thing to have?
> 
> And don't forget we introduced bcmbca awhile back with first SoC 47622 
> and have set a clear goal for the purpose we are discussing here today:
> Please see this patch series:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2022-April/732867.html
> "This change introduces Broadcom's ARCH_BCMBCA architecture for
> armv7 and armv8 based Broadband SoCs. We expect to send additional
> patches for each SoC in the near future."
> 
> And Krzysztof acked that yaml for brcm,bcabca here :
> http://lists.infradead.org/pipermail/linux-arm-kernel/2022-April/732867.html

and I keep discussing, so bringing it up proves what? That new comments
appeared?

> 
> 


Best regards,
Krzysztof

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
@ 2022-07-22 18:22                   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-22 18:22 UTC (permalink / raw)
  To: William Zhang, Florian Fainelli, Rafał Miłecki
  Cc: Linux ARM List, joel.peshkin, dan.beygelman, Rob Herring,
	Krzysztof Kozlowski, Anand Gore, Kursad Oney,
	Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 21/07/2022 21:35, William Zhang wrote:
> 
> 
> On 07/21/2022 09:43 AM, Florian Fainelli wrote:
>> On 7/21/22 00:50, Rafał Miłecki wrote:
>>> On 2022-07-21 09:36, Krzysztof Kozlowski wrote:
>>>> On 21/07/2022 09:13, Rafał Miłecki wrote:
>>>>>> That's better argument. But what's the benefit of adding generic
>>>>>> compatible? Devices cannot bind to it (it is too generic). Does it
>>>>>> describe the device anyhow? Imagine someone adding compatible
>>>>>> "brcm,all-soc-of-broadcom" - does it make any sense?
> 
> 
>>>>>
>>>>> OK, I see it now. I can't think of any case of handling all devices
>>>>> covered with suc a wide brcm,bcmbca binding.
>>>>
>>>> Maybe there is some common part of a SoC which that generic compatible
>>>> would express?
>>>>
>>>> Most archs don't use soc-wide generic compatible, because of reasons I
>>>> mentioned - no actual benefits for anyone from such compatible.
>>>>
>>>> But there are exceptions. I fouun socfpga and apple. The apple sounds as
>>>> mistake to me, because the generic "apple,arm-platform" compatible looks
>>>> like covering all possible Apple ARM platforms. I think Apple ARM
>>>> designs in 20 years will not be compatible at all with current design,
>>>> so such broad compatible is not useful... but that's only my opinion.
>>>
>>> Let's see if William / Broadcom guys can provide a valid argument for
>>> the brcm,bcmbca.
>>
>> It is common practice to provide a generic fallback compatible string for a given chip family/architecture and all of our existing in-tree (and out of tree for that matter) DTSes for Broadcom SoCs do that:
>>
>> - Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt
>> - Documentation/devicetree/bindings/arm/bcm/brcm,ns2.yaml
>> - Documentation/devicetree/bindings/arm/bcm/brcm,nsp.yaml
>> - Documentation/devicetree/bindings/arm/bcm/brcm,cygnus.yaml
>>
>> list goes on and on, of course the counter examples are bcm2835, bcm4708 etc. although those are both chip and families technically, so I suppose the conflation is appropriate in that case. So the pattern is simple:
>>
>> - outside of Broadcom contributors used convention
>> - inside of Broadcom contributors used another
>>
>> so if nothing else, we ought to be consistent within ourselves as Broadcom insiders, which we are doing here.
>>
>> While the generic fallback may not be in use, it still serves a purpose:
>>
>> - Broadcom likes to create a gazillion of part numbers that are hard to untangle from their original SoC architecture unless you happen to work there so it serves as documentation for others to identify what family they belong to, and what to expect from that
>>
>> - you never know when you might want to be matching on just the generic compatible string of a family and putting it in there costs nothing at all
>>
>> The point of William's patch series is to right a number of wrongs on Broadcom's side:
>>
>> - lack of appropriate involvements at the time Rafal submitted the 4908 support as a "standalone" family, I am to take the blame for suggesting that name in the first place, though I did not know at the time that William and others would ever be contributing upstream
>>
>> - avoid the proliferation of "sub" families within a larger family (BCMBCA) since that serves no purpose other than to make it harder on users to select what they should be selecting in their kernel *and* it makes us inconsistent with arch/arm64/Kconfig.platforms that attempts to reduce those
>>
>> I would conclude by asking you: why is this such a big issue? What *actual* problem does it causes, except maybe setting a precedent that you do not like, but for which you practically should have no reason to care as long as the binding is enforced.
>>
> Totally agreed.  Just want to emphasize that the main reason of this 
> change to merge all the bcmbca family chips into the same group for 
> better organization and reduce the clutter to the kernel source. Think 
> about 18 bcmbca devices with different changes to yaml, kconfig, 
> makefile, sub-directory while they actually share many common blocks, 
> drivers and dts entries.

Your commit does not explain why you are doing it at all. We have all
this discussion, you put so many arguments, but why none of it is in the
commit description?

> 
> While nobody would bind a device to brcm,bcmbca (and it can not be 
> binded because bcmbca,yaml require to prefix with a specific chip), it 
> is a great way to make it easy for people to understand what device they 
> are working and look for the bcmbca common driver and other support code 
> as well. Wouldn't this be good thing to have?
> 
> And don't forget we introduced bcmbca awhile back with first SoC 47622 
> and have set a clear goal for the purpose we are discussing here today:
> Please see this patch series:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2022-April/732867.html
> "This change introduces Broadcom's ARCH_BCMBCA architecture for
> armv7 and armv8 based Broadband SoCs. We expect to send additional
> patches for each SoC in the near future."
> 
> And Krzysztof acked that yaml for brcm,bcabca here :
> http://lists.infradead.org/pipermail/linux-arm-kernel/2022-April/732867.html

and I keep discussing, so bringing it up proves what? That new comments
appeared?

> 
> 


Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
  2022-07-22  0:07         ` William Zhang
@ 2022-07-22 18:24           ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-22 18:24 UTC (permalink / raw)
  To: William Zhang, Rafał Miłecki
  Cc: Linux ARM List, joel.peshkin, dan.beygelman, Rob Herring,
	Krzysztof Kozlowski, Anand Gore, Kursad Oney, Florian Fainelli,
	Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 22/07/2022 02:07, William Zhang wrote:
> 
> 
> On 07/21/2022 12:01 AM, Krzysztof Kozlowski wrote:
>> On 21/07/2022 08:51, Rafał Miłecki wrote:
>>> On 2022-07-21 08:44, Krzysztof Kozlowski wrote:
>>>> On 21/07/2022 02:06, William Zhang wrote:
>>>>> Append "brcm,bcmbca" to BCM4908 chip family compatible strings. Add
>>>>> generic 4908 board entry.
>>>>
>>>> This does not explain at all why you are doing it. Improve your commit
>>>> messages.
>>>
>>> To clarify it from my side (and maybe help a bit):
>>>
>>> 1. As I understand it BCMBCA is a one big family of SoCs.
>>> 2. BCM4908 is a subset of that family (a subfamily?) designed for a
>>>      specific group of devices.
>>>
>>> If that's correct I think William it's what you should describe in your
>>> commit message. That would make binding more accurate and should be a
>>> good argument for your change (I believe).
>>
>> That's better argument. But what's the benefit of adding generic
>> compatible? Devices cannot bind to it (it is too generic). Does it
>> describe the device anyhow? Imagine someone adding compatible
>> "brcm,all-soc-of-broadcom" - does it make any sense?
>>
> In case you were also referring the generic 4908 board compatible string 
> brcm,bcm94908, this is for a bare bone 4908 board dts that only enables 

No, we refer to the contents of the patch, so bcmbca compatible.

I did not see you introducing here bcm4908 compatible.

> ARM cpu subsystem, memory and uart. It can be used on all 4908 based 
> Broadcom reference boards and customer board. It is especially useful 
> for initial board bring up and one can load this generic board and start 
> work and debug from the console. Also would be helpful to do a quick 
> verification of new kernel version when there is cpu subsystem related 
> change.
> 
> I guess my mindset already assume people are now familiar with this 
> model of bcmbca binding addition for a new SoC since we introduced the 
> bcmbca arch with first soc 47622 and 10+ other socs late. But sure I 
> agree and I will update the commit message with more details in addition 
> to what the cover letter says.



Best regards,
Krzysztof

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
@ 2022-07-22 18:24           ` Krzysztof Kozlowski
  0 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-22 18:24 UTC (permalink / raw)
  To: William Zhang, Rafał Miłecki
  Cc: Linux ARM List, joel.peshkin, dan.beygelman, Rob Herring,
	Krzysztof Kozlowski, Anand Gore, Kursad Oney, Florian Fainelli,
	Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 22/07/2022 02:07, William Zhang wrote:
> 
> 
> On 07/21/2022 12:01 AM, Krzysztof Kozlowski wrote:
>> On 21/07/2022 08:51, Rafał Miłecki wrote:
>>> On 2022-07-21 08:44, Krzysztof Kozlowski wrote:
>>>> On 21/07/2022 02:06, William Zhang wrote:
>>>>> Append "brcm,bcmbca" to BCM4908 chip family compatible strings. Add
>>>>> generic 4908 board entry.
>>>>
>>>> This does not explain at all why you are doing it. Improve your commit
>>>> messages.
>>>
>>> To clarify it from my side (and maybe help a bit):
>>>
>>> 1. As I understand it BCMBCA is a one big family of SoCs.
>>> 2. BCM4908 is a subset of that family (a subfamily?) designed for a
>>>      specific group of devices.
>>>
>>> If that's correct I think William it's what you should describe in your
>>> commit message. That would make binding more accurate and should be a
>>> good argument for your change (I believe).
>>
>> That's better argument. But what's the benefit of adding generic
>> compatible? Devices cannot bind to it (it is too generic). Does it
>> describe the device anyhow? Imagine someone adding compatible
>> "brcm,all-soc-of-broadcom" - does it make any sense?
>>
> In case you were also referring the generic 4908 board compatible string 
> brcm,bcm94908, this is for a bare bone 4908 board dts that only enables 

No, we refer to the contents of the patch, so bcmbca compatible.

I did not see you introducing here bcm4908 compatible.

> ARM cpu subsystem, memory and uart. It can be used on all 4908 based 
> Broadcom reference boards and customer board. It is especially useful 
> for initial board bring up and one can load this generic board and start 
> work and debug from the console. Also would be helpful to do a quick 
> verification of new kernel version when there is cpu subsystem related 
> change.
> 
> I guess my mindset already assume people are now familiar with this 
> model of bcmbca binding addition for a new SoC since we introduced the 
> bcmbca arch with first soc 47622 and 10+ other socs late. But sure I 
> agree and I will update the commit message with more details in addition 
> to what the cover letter says.



Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
  2022-07-22 18:22                   ` Krzysztof Kozlowski
@ 2022-07-25  6:00                     ` William Zhang
  -1 siblings, 0 replies; 28+ messages in thread
From: William Zhang @ 2022-07-25  6:00 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Florian Fainelli, Rafał Miłecki
  Cc: Linux ARM List, joel.peshkin, dan.beygelman, Rob Herring,
	Krzysztof Kozlowski, Anand Gore, Kursad Oney,
	Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

[-- Attachment #1: Type: text/plain, Size: 5811 bytes --]



On 07/22/2022 11:22 AM, Krzysztof Kozlowski wrote:
> On 21/07/2022 21:35, William Zhang wrote:
>>
>>
>> On 07/21/2022 09:43 AM, Florian Fainelli wrote:
>>> On 7/21/22 00:50, Rafał Miłecki wrote:
>>>> On 2022-07-21 09:36, Krzysztof Kozlowski wrote:
>>>>> On 21/07/2022 09:13, Rafał Miłecki wrote:
>>>>>>> That's better argument. But what's the benefit of adding generic
>>>>>>> compatible? Devices cannot bind to it (it is too generic). Does it
>>>>>>> describe the device anyhow? Imagine someone adding compatible
>>>>>>> "brcm,all-soc-of-broadcom" - does it make any sense?
>>
>>
>>>>>>
>>>>>> OK, I see it now. I can't think of any case of handling all devices
>>>>>> covered with suc a wide brcm,bcmbca binding.
>>>>>
>>>>> Maybe there is some common part of a SoC which that generic compatible
>>>>> would express?
>>>>>
>>>>> Most archs don't use soc-wide generic compatible, because of reasons I
>>>>> mentioned - no actual benefits for anyone from such compatible.
>>>>>
>>>>> But there are exceptions. I fouun socfpga and apple. The apple sounds as
>>>>> mistake to me, because the generic "apple,arm-platform" compatible looks
>>>>> like covering all possible Apple ARM platforms. I think Apple ARM
>>>>> designs in 20 years will not be compatible at all with current design,
>>>>> so such broad compatible is not useful... but that's only my opinion.
>>>>
>>>> Let's see if William / Broadcom guys can provide a valid argument for
>>>> the brcm,bcmbca.
>>>
>>> It is common practice to provide a generic fallback compatible string for a given chip family/architecture and all of our existing in-tree (and out of tree for that matter) DTSes for Broadcom SoCs do that:
>>>
>>> - Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt
>>> - Documentation/devicetree/bindings/arm/bcm/brcm,ns2.yaml
>>> - Documentation/devicetree/bindings/arm/bcm/brcm,nsp.yaml
>>> - Documentation/devicetree/bindings/arm/bcm/brcm,cygnus.yaml
>>>
>>> list goes on and on, of course the counter examples are bcm2835, bcm4708 etc. although those are both chip and families technically, so I suppose the conflation is appropriate in that case. So the pattern is simple:
>>>
>>> - outside of Broadcom contributors used convention
>>> - inside of Broadcom contributors used another
>>>
>>> so if nothing else, we ought to be consistent within ourselves as Broadcom insiders, which we are doing here.
>>>
>>> While the generic fallback may not be in use, it still serves a purpose:
>>>
>>> - Broadcom likes to create a gazillion of part numbers that are hard to untangle from their original SoC architecture unless you happen to work there so it serves as documentation for others to identify what family they belong to, and what to expect from that
>>>
>>> - you never know when you might want to be matching on just the generic compatible string of a family and putting it in there costs nothing at all
>>>
>>> The point of William's patch series is to right a number of wrongs on Broadcom's side:
>>>
>>> - lack of appropriate involvements at the time Rafal submitted the 4908 support as a "standalone" family, I am to take the blame for suggesting that name in the first place, though I did not know at the time that William and others would ever be contributing upstream
>>>
>>> - avoid the proliferation of "sub" families within a larger family (BCMBCA) since that serves no purpose other than to make it harder on users to select what they should be selecting in their kernel *and* it makes us inconsistent with arch/arm64/Kconfig.platforms that attempts to reduce those
>>>
>>> I would conclude by asking you: why is this such a big issue? What *actual* problem does it causes, except maybe setting a precedent that you do not like, but for which you practically should have no reason to care as long as the binding is enforced.
>>>
>> Totally agreed.  Just want to emphasize that the main reason of this
>> change to merge all the bcmbca family chips into the same group for
>> better organization and reduce the clutter to the kernel source. Think
>> about 18 bcmbca devices with different changes to yaml, kconfig,
>> makefile, sub-directory while they actually share many common blocks,
>> drivers and dts entries.
> 
> Your commit does not explain why you are doing it at all. We have all
> this discussion, you put so many arguments, but why none of it is in the
> commit description?
> 
The purpose of the discussion was to explain to why we think appending
brcm,bcmbca is necessary and helpful and the history of this string when
we first upstream the bcmbca SoC. And I also said I totally agree to the
improvement of commit message. I just posted the v2 of this patch series
with all the requests accommodated from this discussion.

>>
>> While nobody would bind a device to brcm,bcmbca (and it can not be
>> binded because bcmbca,yaml require to prefix with a specific chip), it
>> is a great way to make it easy for people to understand what device they
>> are working and look for the bcmbca common driver and other support code
>> as well. Wouldn't this be good thing to have?
>>
>> And don't forget we introduced bcmbca awhile back with first SoC 47622
>> and have set a clear goal for the purpose we are discussing here today:
>> Please see this patch series:
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2022-April/732867.html
>> "This change introduces Broadcom's ARCH_BCMBCA architecture for
>> armv7 and armv8 based Broadband SoCs. We expect to send additional
>> patches for each SoC in the near future."
>>
>> And Krzysztof acked that yaml for brcm,bcabca here :
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2022-April/732867.html
> 
> and I keep discussing, so bringing it up proves what? That new comments
> appeared?
> 
>>
>>
> 
> 
> Best regards,
> Krzysztof
> 

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

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

* Re: [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description
@ 2022-07-25  6:00                     ` William Zhang
  0 siblings, 0 replies; 28+ messages in thread
From: William Zhang @ 2022-07-25  6:00 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Florian Fainelli, Rafał Miłecki
  Cc: Linux ARM List, joel.peshkin, dan.beygelman, Rob Herring,
	Krzysztof Kozlowski, Anand Gore, Kursad Oney,
	Broadcom internal kernel review list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list


[-- Attachment #1.1: Type: text/plain, Size: 5811 bytes --]



On 07/22/2022 11:22 AM, Krzysztof Kozlowski wrote:
> On 21/07/2022 21:35, William Zhang wrote:
>>
>>
>> On 07/21/2022 09:43 AM, Florian Fainelli wrote:
>>> On 7/21/22 00:50, Rafał Miłecki wrote:
>>>> On 2022-07-21 09:36, Krzysztof Kozlowski wrote:
>>>>> On 21/07/2022 09:13, Rafał Miłecki wrote:
>>>>>>> That's better argument. But what's the benefit of adding generic
>>>>>>> compatible? Devices cannot bind to it (it is too generic). Does it
>>>>>>> describe the device anyhow? Imagine someone adding compatible
>>>>>>> "brcm,all-soc-of-broadcom" - does it make any sense?
>>
>>
>>>>>>
>>>>>> OK, I see it now. I can't think of any case of handling all devices
>>>>>> covered with suc a wide brcm,bcmbca binding.
>>>>>
>>>>> Maybe there is some common part of a SoC which that generic compatible
>>>>> would express?
>>>>>
>>>>> Most archs don't use soc-wide generic compatible, because of reasons I
>>>>> mentioned - no actual benefits for anyone from such compatible.
>>>>>
>>>>> But there are exceptions. I fouun socfpga and apple. The apple sounds as
>>>>> mistake to me, because the generic "apple,arm-platform" compatible looks
>>>>> like covering all possible Apple ARM platforms. I think Apple ARM
>>>>> designs in 20 years will not be compatible at all with current design,
>>>>> so such broad compatible is not useful... but that's only my opinion.
>>>>
>>>> Let's see if William / Broadcom guys can provide a valid argument for
>>>> the brcm,bcmbca.
>>>
>>> It is common practice to provide a generic fallback compatible string for a given chip family/architecture and all of our existing in-tree (and out of tree for that matter) DTSes for Broadcom SoCs do that:
>>>
>>> - Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt
>>> - Documentation/devicetree/bindings/arm/bcm/brcm,ns2.yaml
>>> - Documentation/devicetree/bindings/arm/bcm/brcm,nsp.yaml
>>> - Documentation/devicetree/bindings/arm/bcm/brcm,cygnus.yaml
>>>
>>> list goes on and on, of course the counter examples are bcm2835, bcm4708 etc. although those are both chip and families technically, so I suppose the conflation is appropriate in that case. So the pattern is simple:
>>>
>>> - outside of Broadcom contributors used convention
>>> - inside of Broadcom contributors used another
>>>
>>> so if nothing else, we ought to be consistent within ourselves as Broadcom insiders, which we are doing here.
>>>
>>> While the generic fallback may not be in use, it still serves a purpose:
>>>
>>> - Broadcom likes to create a gazillion of part numbers that are hard to untangle from their original SoC architecture unless you happen to work there so it serves as documentation for others to identify what family they belong to, and what to expect from that
>>>
>>> - you never know when you might want to be matching on just the generic compatible string of a family and putting it in there costs nothing at all
>>>
>>> The point of William's patch series is to right a number of wrongs on Broadcom's side:
>>>
>>> - lack of appropriate involvements at the time Rafal submitted the 4908 support as a "standalone" family, I am to take the blame for suggesting that name in the first place, though I did not know at the time that William and others would ever be contributing upstream
>>>
>>> - avoid the proliferation of "sub" families within a larger family (BCMBCA) since that serves no purpose other than to make it harder on users to select what they should be selecting in their kernel *and* it makes us inconsistent with arch/arm64/Kconfig.platforms that attempts to reduce those
>>>
>>> I would conclude by asking you: why is this such a big issue? What *actual* problem does it causes, except maybe setting a precedent that you do not like, but for which you practically should have no reason to care as long as the binding is enforced.
>>>
>> Totally agreed.  Just want to emphasize that the main reason of this
>> change to merge all the bcmbca family chips into the same group for
>> better organization and reduce the clutter to the kernel source. Think
>> about 18 bcmbca devices with different changes to yaml, kconfig,
>> makefile, sub-directory while they actually share many common blocks,
>> drivers and dts entries.
> 
> Your commit does not explain why you are doing it at all. We have all
> this discussion, you put so many arguments, but why none of it is in the
> commit description?
> 
The purpose of the discussion was to explain to why we think appending
brcm,bcmbca is necessary and helpful and the history of this string when
we first upstream the bcmbca SoC. And I also said I totally agree to the
improvement of commit message. I just posted the v2 of this patch series
with all the requests accommodated from this discussion.

>>
>> While nobody would bind a device to brcm,bcmbca (and it can not be
>> binded because bcmbca,yaml require to prefix with a specific chip), it
>> is a great way to make it easy for people to understand what device they
>> are working and look for the bcmbca common driver and other support code
>> as well. Wouldn't this be good thing to have?
>>
>> And don't forget we introduced bcmbca awhile back with first SoC 47622
>> and have set a clear goal for the purpose we are discussing here today:
>> Please see this patch series:
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2022-April/732867.html
>> "This change introduces Broadcom's ARCH_BCMBCA architecture for
>> armv7 and armv8 based Broadband SoCs. We expect to send additional
>> patches for each SoC in the near future."
>>
>> And Krzysztof acked that yaml for brcm,bcabca here :
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2022-April/732867.html
> 
> and I keep discussing, so bringing it up proves what? That new comments
> appeared?
> 
>>
>>
> 
> 
> Best regards,
> Krzysztof
> 

[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2022-07-25  6:02 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-21  0:06 [RESEND PATCH 2/9] dt-bindings: arm64: bcmbca: Update BCM4908 description William Zhang
2022-07-21  0:06 ` William Zhang
2022-07-21  6:11 ` Rafał Miłecki
2022-07-21  6:11   ` Rafał Miłecki
2022-07-21  6:44 ` Krzysztof Kozlowski
2022-07-21  6:44   ` Krzysztof Kozlowski
2022-07-21  6:51   ` Rafał Miłecki
2022-07-21  6:51     ` Rafał Miłecki
2022-07-21  7:01     ` Krzysztof Kozlowski
2022-07-21  7:01       ` Krzysztof Kozlowski
2022-07-21  7:13       ` Rafał Miłecki
2022-07-21  7:13         ` Rafał Miłecki
2022-07-21  7:36         ` Krzysztof Kozlowski
2022-07-21  7:36           ` Krzysztof Kozlowski
2022-07-21  7:50           ` Rafał Miłecki
2022-07-21  7:50             ` Rafał Miłecki
2022-07-21 16:43             ` Florian Fainelli
2022-07-21 16:43               ` Florian Fainelli
2022-07-21 19:35               ` William Zhang
2022-07-21 19:35                 ` William Zhang
2022-07-22 18:22                 ` Krzysztof Kozlowski
2022-07-22 18:22                   ` Krzysztof Kozlowski
2022-07-25  6:00                   ` William Zhang
2022-07-25  6:00                     ` William Zhang
2022-07-22  0:07       ` William Zhang
2022-07-22  0:07         ` William Zhang
2022-07-22 18:24         ` Krzysztof Kozlowski
2022-07-22 18:24           ` Krzysztof Kozlowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.