[PATCH v2 2/8] dt-bindings: marvell: a38x: add solidrun armada 385 clearfog gtr boards

[PATCH v2 2/8] dt-bindings: marvell: a38x: add solidrun armada 385 clearfog gtr boards

[Josua Mayer]

Add DT compatible for SolidRun Armada-385 based Clearfog GTR L8 and S4
boards.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 .../devicetree/bindings/arm/marvell/armada-38x.yaml       | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
index 5af222e6db18..5bc69c4fe34d 100644
--- a/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
+++ b/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
@@ -30,4 +30,12 @@ properties:
           - const: marvell,armada385
           - const: marvell,armada380
 
+      - description: SolidRun Armada 385 based single-board computers
+        items:
+          - enum:
+             - solidrun,clearfog-gtr-l8
+             - solidrun,clearfog-gtr-s4
+          - const: marvell,armada385
+          - const: marvell,armada380
+
 additionalProperties: true
-- 
2.35.3


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

[PATCH v2 3/8] dt-bindings: marvell: a38x: add solidrun armada 388 clearfog boards

[Josua Mayer]

Add DT compatible for SolidRun Armada-388 based Clearfog Base and Pro
boards.

While they are just two variants, we have 3 device-tree:
- armada-388-clearfog.dts
- armada-388-clearfog-base.dts
- armada-388-clearfog-pro.dts

While the first one is identical to the pro variant, the filename is
not, and neither are the compatible strings.

The first is compatible to armada-380, 385, 388 and
solidrun,clearfog-a1.
The other two are compatible to the same, plus an additional string
including the pro/base as suffix.

Add two bindings, one for the explicit pro and base variants using a
total of 5 compatibles, and one for the generic name using just 4.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 .../bindings/arm/marvell/armada-38x.yaml      | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
index 5bc69c4fe34d..25c29b466094 100644
--- a/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
+++ b/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
@@ -38,4 +38,24 @@ properties:
           - const: marvell,armada385
           - const: marvell,armada380
 
+      - description: |
+          SolidRun Armada 388 SoM based clearfog board. This is
+          equivalent to clearfog pro, but lacks the "pro" label
+          in dtb filename and compatible strings.
+        items:
+          - const: solidrun,clearfog-a1
+          - const: marvell,armada388
+          - const: marvell,armada385
+          - const: marvell,armada380
+
+      - description: SolidRun Armada 388 SoM based clearfog board variants
+        items:
+          - enum:
+             - solidrun,clearfog-base-a1
+             - solidrun,clearfog-pro-a1
+          - const: solidrun,clearfog-a1
+          - const: marvell,armada388
+          - const: marvell,armada385
+          - const: marvell,armada380
+
 additionalProperties: true
-- 
2.35.3


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

Re: [PATCH v2 3/8] dt-bindings: marvell: a38x: add solidrun armada 388 clearfog boards

[Conor Dooley]

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

On Sun, Dec 24, 2023 at 03:38:45PM +0100, Josua Mayer wrote:
> Add DT compatible for SolidRun Armada-388 based Clearfog Base and Pro
> boards.
> 
> While they are just two variants, we have 3 device-tree:
> - armada-388-clearfog.dts
> - armada-388-clearfog-base.dts
> - armada-388-clearfog-pro.dts
> 
> While the first one is identical to the pro variant, the filename is
> not, and neither are the compatible strings.
> 
> The first is compatible to armada-380, 385, 388 and
> solidrun,clearfog-a1.
> The other two are compatible to the same, plus an additional string
> including the pro/base as suffix.
> 
> Add two bindings, one for the explicit pro and base variants using a
> total of 5 compatibles, and one for the generic name using just 4.
> 
> Signed-off-by: Josua Mayer <josua@solid-run.com>

There's nothing here that explains to me _why_ there are three sets of
compatibles for only two boards. Why is this the case?

Also, binding patches need to be sent to the devicetree mailing list.
get_maintainer.pl should have told you that, no?


> ---
>  .../bindings/arm/marvell/armada-38x.yaml      | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
> index 5bc69c4fe34d..25c29b466094 100644
> --- a/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
> +++ b/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
> @@ -38,4 +38,24 @@ properties:
>            - const: marvell,armada385
>            - const: marvell,armada380
>  
> +      - description: |

This isn't needed, there's no formatting to preserve.

Cheers,
Conor.

> +          SolidRun Armada 388 SoM based clearfog board. This is
> +          equivalent to clearfog pro, but lacks the "pro" label
> +          in dtb filename and compatible strings.
> +        items:
> +          - const: solidrun,clearfog-a1
> +          - const: marvell,armada388
> +          - const: marvell,armada385
> +          - const: marvell,armada380
> +
> +      - description: SolidRun Armada 388 SoM based clearfog board variants
> +        items:
> +          - enum:
> +             - solidrun,clearfog-base-a1
> +             - solidrun,clearfog-pro-a1
> +          - const: solidrun,clearfog-a1
> +          - const: marvell,armada388
> +          - const: marvell,armada385
> +          - const: marvell,armada380
> +
>  additionalProperties: true
> -- 
> 2.35.3
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 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

Re: [PATCH v2 3/8] dt-bindings: marvell: a38x: add solidrun armada 388 clearfog boards

[Conor Dooley]

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

On Sun, Dec 24, 2023 at 03:58:14PM +0000, Josua Mayer wrote:
> Am 24.12.23 um 15:42 schrieb Conor Dooley:
> 
> On Sun, Dec 24, 2023 at 03:38:45PM +0100, Josua Mayer wrote:
> 
> 
> Add DT compatible for SolidRun Armada-388 based Clearfog Base and Pro
> boards.
> 
> While they are just two variants, we have 3 device-tree:
> - armada-388-clearfog.dts
> - armada-388-clearfog-base.dts
> - armada-388-clearfog-pro.dts
> 
> While the first one is identical to the pro variant, the filename is
> not, and neither are the compatible strings.
> 
> The first is compatible to armada-380, 385, 388 and
> solidrun,clearfog-a1.
> The other two are compatible to the same, plus an additional string
> including the pro/base as suffix.
> 
> Add two bindings, one for the explicit pro and base variants using a
> total of 5 compatibles, and one for the generic name using just 4.
> 
> Signed-off-by: Josua Mayer <josua@solid-run.com><mailto:josua@solid-run.com>
> 
> 
> 
> There's nothing here that explains to me _why_ there are three sets of
> compatibles for only two boards. Why is this the case?
> 
> I have no idea how we ended up with these 3 combinations:
> clearfog-a1, armada388, ...
> clearfog-base-a1, clearfog-base-a1, armada388, ...
> clearfog-pro-a1, clearfog-base-a1, armada388, ...
> I can only assume it is because these boards are so similar they can boot
> from each others device-tree without large functional impact.

I think Krzysztof has dug more into this, but please just create two
combinations that make sense and have common elements (which they
already do to be fair). It doesn't really matter if some combination
that doesn't make sense is used out of tree.

Cheers,
Conor.

> 
> Note we did end up with 3 individual filenames because the Pro was supported first,
> without a suffix - and then both were supported with specific suffix.
> 
> 
> Also, binding patches need to be sent to the devicetree mailing list.
> get_maintainer.pl should have told you that, no?
> 
> I will fix it in v3, or do a resend.
> It did tell me, but I wrongly decided to pick only linux-arm-kernel.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 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

Re: [PATCH v2 3/8] dt-bindings: marvell: a38x: add solidrun armada 388 clearfog boards

[Josua Mayer]

Am 26.12.23 um 13:10 schrieb Conor Dooley:
> On Sun, Dec 24, 2023 at 03:58:14PM +0000, Josua Mayer wrote:
>> Am 24.12.23 um 15:42 schrieb Conor Dooley:
>>
>> On Sun, Dec 24, 2023 at 03:38:45PM +0100, Josua Mayer wrote:
>>
>>
>> Add DT compatible for SolidRun Armada-388 based Clearfog Base and Pro
>> boards.
>>
>> While they are just two variants, we have 3 device-tree:
>> - armada-388-clearfog.dts
>> - armada-388-clearfog-base.dts
>> - armada-388-clearfog-pro.dts
>>
>> While the first one is identical to the pro variant, the filename is
>> not, and neither are the compatible strings.
>>
>> The first is compatible to armada-380, 385, 388 and
>> solidrun,clearfog-a1.
>> The other two are compatible to the same, plus an additional string
>> including the pro/base as suffix.
>>
>> Add two bindings, one for the explicit pro and base variants using a
>> total of 5 compatibles, and one for the generic name using just 4.
>>
>> Signed-off-by: Josua Mayer <josua@solid-run.com><mailto:josua@solid-run.com>
>>
>>
>>
>> There's nothing here that explains to me _why_ there are three sets of
>> compatibles for only two boards. Why is this the case?
>>
>> I have no idea how we ended up with these 3 combinations:
>> clearfog-a1, armada388, ...
>> clearfog-base-a1, clearfog-base-a1, armada388, ...
>> clearfog-pro-a1, clearfog-base-a1, armada388, ...
>> I can only assume it is because these boards are so similar they can boot
>> from each others device-tree without large functional impact.
> I think Krzysztof has dug more into this, but please just create two
> combinations that make sense and have common elements (which they
> already do to be fair). It doesn't really matter if some combination
> that doesn't make sense is used out of tree.

Okay, thanks.
I will create another revision and rewrite descriptions to be more clear,
especially considering questions raised by Krzysztof.

>
> Cheers,
> Conor.
>
>> Note we did end up with 3 individual filenames because the Pro was supported first,
>> without a suffix - and then both were supported with specific suffix.
>>
>>
>> Also, binding patches need to be sent to the devicetree mailing list.
>> get_maintainer.pl should have told you that, no?
>>
>> I will fix it in v3, or do a resend.
>> It did tell me, but I wrongly decided to pick only linux-arm-kernel.


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

Re: [PATCH v2 3/8] dt-bindings: marvell: a38x: add solidrun armada 388 clearfog boards

[Krzysztof Kozlowski]

On 24/12/2023 15:38, Josua Mayer wrote:
> Add DT compatible for SolidRun Armada-388 based Clearfog Base and Pro
> boards.
> 
> While they are just two variants, we have 3 device-tree:
> - armada-388-clearfog.dts
> - armada-388-clearfog-base.dts
> - armada-388-clearfog-pro.dts
> 
> While the first one is identical to the pro variant, the filename is
> not, and neither are the compatible strings.
> 
> The first is compatible to armada-380, 385, 388 and
> solidrun,clearfog-a1.
> The other two are compatible to the same, plus an additional string
> including the pro/base as suffix.
> 
> Add two bindings, one for the explicit pro and base variants using a
> total of 5 compatibles, and one for the generic name using just 4.
> 
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
>  .../bindings/arm/marvell/armada-38x.yaml      | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
> index 5bc69c4fe34d..25c29b466094 100644
> --- a/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
> +++ b/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
> @@ -38,4 +38,24 @@ properties:
>            - const: marvell,armada385
>            - const: marvell,armada380
>  
> +      - description: |
> +          SolidRun Armada 388 SoM based clearfog board. This is
> +          equivalent to clearfog pro, but lacks the "pro" label
> +          in dtb filename and compatible strings.
> +        items:
> +          - const: solidrun,clearfog-a1

SoM cannot be alone.

> +          - const: marvell,armada388
> +          - const: marvell,armada385
> +          - const: marvell,armada380
> +
> +      - description: SolidRun Armada 388 SoM based clearfog board variants
> +        items:
> +          - enum:
> +             - solidrun,clearfog-base-a1

Board has name "base"?

Still not tested...

Best regards,
Krzysztof


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

Re: [PATCH v2 3/8] dt-bindings: marvell: a38x: add solidrun armada 388 clearfog boards

[Josua Mayer]

Am 24.12.23 um 15:55 schrieb Krzysztof Kozlowski:
> On 24/12/2023 15:38, Josua Mayer wrote:
>> Add DT compatible for SolidRun Armada-388 based Clearfog Base and Pro
>> boards.
>>
>> While they are just two variants, we have 3 device-tree:
>> - armada-388-clearfog.dts
>> - armada-388-clearfog-base.dts
>> - armada-388-clearfog-pro.dts
>>
>> While the first one is identical to the pro variant, the filename is
>> not, and neither are the compatible strings.
>>
>> The first is compatible to armada-380, 385, 388 and
>> solidrun,clearfog-a1.
>> The other two are compatible to the same, plus an additional string
>> including the pro/base as suffix.
>>
>> Add two bindings, one for the explicit pro and base variants using a
>> total of 5 compatibles, and one for the generic name using just 4.
>>
>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>> ---
>>   .../bindings/arm/marvell/armada-38x.yaml      | 20 +++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
>> index 5bc69c4fe34d..25c29b466094 100644
>> --- a/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
>> +++ b/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
>> @@ -38,4 +38,24 @@ properties:
>>             - const: marvell,armada385
>>             - const: marvell,armada380
>>   
>> +      - description: |
>> +          SolidRun Armada 388 SoM based clearfog board. This is
>> +          equivalent to clearfog pro, but lacks the "pro" label
>> +          in dtb filename and compatible strings.
>> +        items:
>> +          - const: solidrun,clearfog-a1
> SoM cannot be alone.

Please confirm, I think my wording above was not clear.

I meant to say "clearfog" devices based on a SoM,.
Armada 388 Clearfog Base / Pro are combinations of SoM and carrier.
Current device-trees do not mention a som-specific compatible string though.

This is in contrast to "Clearfog GTR" which
a) uses aramda-385
b) solders SoC to the board.

>
>> +          - const: marvell,armada388
>> +          - const: marvell,armada385
>> +          - const: marvell,armada380
>> +
>> +      - description: SolidRun Armada 388 SoM based clearfog board variants
>> +        items:
>> +          - enum:
>> +             - solidrun,clearfog-base-a1
> Board has name "base"?
Yes.
In marketing and in device-tree model field the board names are
Clearfog Base, Clearfog Pro
and historically just Clearfog, for the Pro.
>
> Still not tested...
>
> Best regards,
> Krzysztof
>

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

Re: [PATCH v2 3/8] dt-bindings: marvell: a38x: add solidrun armada 388 clearfog boards

[Krzysztof Kozlowski]

On 24/12/2023 17:02, Josua Mayer wrote:
> Am 24.12.23 um 15:55 schrieb Krzysztof Kozlowski:
>> On 24/12/2023 15:38, Josua Mayer wrote:
>>> Add DT compatible for SolidRun Armada-388 based Clearfog Base and Pro
>>> boards.
>>>
>>> While they are just two variants, we have 3 device-tree:
>>> - armada-388-clearfog.dts
>>> - armada-388-clearfog-base.dts
>>> - armada-388-clearfog-pro.dts
>>>
>>> While the first one is identical to the pro variant, the filename is
>>> not, and neither are the compatible strings.
>>>
>>> The first is compatible to armada-380, 385, 388 and
>>> solidrun,clearfog-a1.
>>> The other two are compatible to the same, plus an additional string
>>> including the pro/base as suffix.
>>>
>>> Add two bindings, one for the explicit pro and base variants using a
>>> total of 5 compatibles, and one for the generic name using just 4.
>>>
>>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>>> ---
>>>   .../bindings/arm/marvell/armada-38x.yaml      | 20 +++++++++++++++++++
>>>   1 file changed, 20 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
>>> index 5bc69c4fe34d..25c29b466094 100644
>>> --- a/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
>>> +++ b/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
>>> @@ -38,4 +38,24 @@ properties:
>>>             - const: marvell,armada385
>>>             - const: marvell,armada380
>>>   
>>> +      - description: |
>>> +          SolidRun Armada 388 SoM based clearfog board. This is
>>> +          equivalent to clearfog pro, but lacks the "pro" label
>>> +          in dtb filename and compatible strings.
>>> +        items:
>>> +          - const: solidrun,clearfog-a1
>> SoM cannot be alone.
> 
> Please confirm, I think my wording above was not clear.
> 
> I meant to say "clearfog" devices based on a SoM,.
> Armada 388 Clearfog Base / Pro are combinations of SoM and carrier.
> Current device-trees do not mention a som-specific compatible string though.

So Clearfog Base is a board, then what is Clearfog A1?

> 
> This is in contrast to "Clearfog GTR" which
> a) uses aramda-385
> b) solders SoC to the board.
> 
>>
>>> +          - const: marvell,armada388
>>> +          - const: marvell,armada385
>>> +          - const: marvell,armada380
>>> +
>>> +      - description: SolidRun Armada 388 SoM based clearfog board variants
>>> +        items:
>>> +          - enum:
>>> +             - solidrun,clearfog-base-a1
>> Board has name "base"?
> Yes.
> In marketing and in device-tree model field the board names are
> Clearfog Base, Clearfog Pro

Then clearfog-base or clearfog-a1-base, but why a1 is at the end? Or is
it the case that Clearfog Base can have different SoMs? Think what you
have there and choose appropriate compatibles. See Renesas SMARC for one
choice and iMX Toradex boards for different example

> 

Best regards,
Krzysztof


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

Re: [PATCH v2 3/8] dt-bindings: marvell: a38x: add solidrun armada 388 clearfog boards

[Josua Mayer]

Am 24.12.23 um 17:09 schrieb Krzysztof Kozlowski:
> On 24/12/2023 17:02, Josua Mayer wrote:
>> Am 24.12.23 um 15:55 schrieb Krzysztof Kozlowski:
>>> On 24/12/2023 15:38, Josua Mayer wrote:
>>>> Add DT compatible for SolidRun Armada-388 based Clearfog Base and Pro
>>>> boards.
>>>>
>>>> While they are just two variants, we have 3 device-tree:
>>>> - armada-388-clearfog.dts
>>>> - armada-388-clearfog-base.dts
>>>> - armada-388-clearfog-pro.dts
>>>>
>>>> While the first one is identical to the pro variant, the filename is
>>>> not, and neither are the compatible strings.
>>>>
>>>> The first is compatible to armada-380, 385, 388 and
>>>> solidrun,clearfog-a1.
>>>> The other two are compatible to the same, plus an additional string
>>>> including the pro/base as suffix.
>>>>
>>>> Add two bindings, one for the explicit pro and base variants using a
>>>> total of 5 compatibles, and one for the generic name using just 4.
>>>>
>>>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>>>> ---
>>>>    .../bindings/arm/marvell/armada-38x.yaml      | 20 +++++++++++++++++++
>>>>    1 file changed, 20 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
>>>> index 5bc69c4fe34d..25c29b466094 100644
>>>> --- a/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
>>>> +++ b/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
>>>> @@ -38,4 +38,24 @@ properties:
>>>>              - const: marvell,armada385
>>>>              - const: marvell,armada380
>>>>    
>>>> +      - description: |
>>>> +          SolidRun Armada 388 SoM based clearfog board. This is
>>>> +          equivalent to clearfog pro, but lacks the "pro" label
>>>> +          in dtb filename and compatible strings.
>>>> +        items:
>>>> +          - const: solidrun,clearfog-a1
>>> SoM cannot be alone.
>> Please confirm, I think my wording above was not clear.
>>
>> I meant to say "clearfog" devices based on a SoM,.
>> Armada 388 Clearfog Base / Pro are combinations of SoM and carrier.
>> Current device-trees do not mention a som-specific compatible string though.
> So Clearfog Base is a board, then what is Clearfog A1?
"Clearfog A1" is printed on the silk screen of a Clearfog Pro revision 
2.0 or earlier.
I don't have a later revision to look at.
So Clearfog A1 and Clearfog Pro  and Clearfog Pro A1 all mean the same 
board,
Clearfog Base (+-A1) mean the other board.

"-a1" of the compatible string is already used by openwrt to differentiate.

>
>> This is in contrast to "Clearfog GTR" which
>> a) uses aramda-385
>> b) solders SoC to the board.
>>
>>>> +          - const: marvell,armada388
>>>> +          - const: marvell,armada385
>>>> +          - const: marvell,armada380
>>>> +
>>>> +      - description: SolidRun Armada 388 SoM based clearfog board variants
>>>> +        items:
>>>> +          - enum:
>>>> +             - solidrun,clearfog-base-a1
>>> Board has name "base"?
>> Yes.
>> In marketing and in device-tree model field the board names are
>> Clearfog Base, Clearfog Pro
> Then clearfog-base or clearfog-a1-base, but why a1 is at the end? Or is
> it the case that Clearfog Base can have different SoMs? Think what you
> have there and choose appropriate compatibles. See Renesas SMARC for one
> choice and iMX Toradex boards for different example
Conceptually at this point, is it worth changing the compatibles of 
existing boards to make sense?
Or better to describe them in yaml as they are now (ensuring language is 
understood)?

If I try really hard, I can add a new armada-388 som compatible, but 
that opens another can.
There are other boards also based on the solidrun armada-388 SoMs, e.g. 
helios4 - and
there is no shared -som dtsi.
Effectively I guess they are all just individual boards, maybe not worth 
mentioning "SoM".

>
> Best regards,
> Krzysztof
>

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

Re: [PATCH v2 3/8] dt-bindings: marvell: a38x: add solidrun armada 388 clearfog boards

[Krzysztof Kozlowski]

On 24/12/2023 17:29, Josua Mayer wrote:

>>>>> +      - description: |
>>>>> +          SolidRun Armada 388 SoM based clearfog board. This is
>>>>> +          equivalent to clearfog pro, but lacks the "pro" label
>>>>> +          in dtb filename and compatible strings.
>>>>> +        items:
>>>>> +          - const: solidrun,clearfog-a1
>>>> SoM cannot be alone.
>>> Please confirm, I think my wording above was not clear.
>>>
>>> I meant to say "clearfog" devices based on a SoM,.
>>> Armada 388 Clearfog Base / Pro are combinations of SoM and carrier.
>>> Current device-trees do not mention a som-specific compatible string though.
>> So Clearfog Base is a board, then what is Clearfog A1?
> "Clearfog A1" is printed on the silk screen of a Clearfog Pro revision 
> 2.0 or earlier.
> I don't have a later revision to look at.
> So Clearfog A1 and Clearfog Pro  and Clearfog Pro A1 all mean the same 
> board,
> Clearfog Base (+-A1) mean the other board.

Then if A1 and Pro A1 are the same, why do you have A1 here and Pro A1
second time in the entry below?

> 
> "-a1" of the compatible string is already used by openwrt to differentiate.

Does not matter, we do not document out of tree users.

> 
>>
>>> This is in contrast to "Clearfog GTR" which
>>> a) uses aramda-385
>>> b) solders SoC to the board.
>>>
>>>>> +          - const: marvell,armada388
>>>>> +          - const: marvell,armada385
>>>>> +          - const: marvell,armada380
>>>>> +
>>>>> +      - description: SolidRun Armada 388 SoM based clearfog board variants
>>>>> +        items:
>>>>> +          - enum:
>>>>> +             - solidrun,clearfog-base-a1
>>>> Board has name "base"?
>>> Yes.
>>> In marketing and in device-tree model field the board names are
>>> Clearfog Base, Clearfog Pro
>> Then clearfog-base or clearfog-a1-base, but why a1 is at the end? Or is
>> it the case that Clearfog Base can have different SoMs? Think what you
>> have there and choose appropriate compatibles. See Renesas SMARC for one
>> choice and iMX Toradex boards for different example
> Conceptually at this point, is it worth changing the compatibles of 
> existing boards to make sense?

There are no existing boards - you add them here. I have doubts whether
you described it properly, thus my questions.


> Or better to describe them in yaml as they are now (ensuring language is 
> understood)?
> 
> If I try really hard, I can add a new armada-388 som compatible, but 
> that opens another can.
> There are other boards also based on the solidrun armada-388 SoMs, e.g. 
> helios4 - and
> there is no shared -som dtsi.
> Effectively I guess they are all just individual boards, maybe not worth 
> mentioning "SoM".
> 

Depends, is this SoM? If yes, usually you should have SoM DTSI and SoM
compatible.

Best regards,
Krzysztof


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

[PATCH v2 4/8] arm: dts: marvell: clearfog-gtr: add board-specific compatible strings

[Josua Mayer]

Most arm board have a board-specific compatible string that allows e.g.
userspace to match specific firmware variants or apply specific
policies.

Add board-specific properties to both variants of the Clearfog GTR:
- solidrun,clearfog-gtr-l8
- solidrun,clearfog-gtr-s4

Introduction of a common parent (e.g. "solidrun,clearfog-gtr") is
omitted for brevity.
Since announcement of the two products no additional variants were added
it is assumed that there will always be just two.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 arch/arm/boot/dts/marvell/armada-385-clearfog-gtr-l8.dts | 2 ++
 arch/arm/boot/dts/marvell/armada-385-clearfog-gtr-s4.dts | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr-l8.dts b/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr-l8.dts
index 1990f7d0cc79..cd5b070f1218 100644
--- a/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr-l8.dts
+++ b/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr-l8.dts
@@ -4,6 +4,8 @@
 
 / {
 	model = "SolidRun Clearfog GTR L8";
+	compatible = "solidrun,clearfog-gtr-l8", "marvell,armada385",
+		     "marvell,armada380";
 };
 
 &mdio {
diff --git a/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr-s4.dts b/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr-s4.dts
index b795ad573891..4a9736ec99b6 100644
--- a/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr-s4.dts
+++ b/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr-s4.dts
@@ -4,6 +4,8 @@
 
 / {
 	model = "SolidRun Clearfog GTR S4";
+	compatible = "solidrun,clearfog-gtr-s4", "marvell,armada385",
+		     "marvell,armada380";
 };
 
 &sfp0 {
-- 
2.35.3


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

[PATCH v2 5/8] arm: dts: marvell: clearfog-gtr: sort pinctrl nodes alphabetically

[Josua Mayer]

Cosmetic change to increase future patches readability when adding new
pinctrl nodes.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 .../dts/marvell/armada-385-clearfog-gtr.dtsi  | 40 +++++++++----------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr.dtsi b/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr.dtsi
index d1452a04e904..8eabb60765b0 100644
--- a/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr.dtsi
+++ b/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr.dtsi
@@ -141,18 +141,13 @@ i2c@11100 { /* SFP (CON5/CON6) */
 			};
 
 			pinctrl@18000 {
-				cf_gtr_switch_reset_pins: cf-gtr-switch-reset-pins {
-					marvell,pins = "mpp18";
-					marvell,function = "gpio";
-				};
-
-				cf_gtr_usb3_con_vbus: cf-gtr-usb3-con-vbus {
-					marvell,pins = "mpp22";
+				cf_gtr_fan_pwm: cf-gtr-fan-pwm {
+					marvell,pins = "mpp23";
 					marvell,function = "gpio";
 				};
 
-				cf_gtr_fan_pwm: cf-gtr-fan-pwm {
-					marvell,pins = "mpp23";
+				cf_gtr_front_button_pins: cf-gtr-front-button-pins {
+					marvell,pins = "mpp53";
 					marvell,function = "gpio";
 				};
 
@@ -162,13 +157,6 @@ cf_gtr_i2c1_pins: i2c1-pins {
 					marvell,function = "i2c1";
 				};
 
-				cf_gtr_sdhci_pins: cf-gtr-sdhci-pins {
-					marvell,pins = "mpp21", "mpp28",
-						       "mpp37", "mpp38",
-						       "mpp39", "mpp40";
-					marvell,function = "sd0";
-				};
-
 				cf_gtr_isolation_pins: cf-gtr-isolation-pins {
 					marvell,pins = "mpp47";
 					marvell,function = "gpio";
@@ -179,18 +167,30 @@ cf_gtr_poe_reset_pins: cf-gtr-poe-reset-pins {
 					marvell,function = "gpio";
 				};
 
+				cf_gtr_rear_button_pins: cf-gtr-rear-button-pins {
+					marvell,pins = "mpp36";
+					marvell,function = "gpio";
+				};
+
+				cf_gtr_sdhci_pins: cf-gtr-sdhci-pins {
+					marvell,pins = "mpp21", "mpp28",
+						       "mpp37", "mpp38",
+						       "mpp39", "mpp40";
+					marvell,function = "sd0";
+				};
+
 				cf_gtr_spi1_cs_pins: spi1-cs-pins {
 					marvell,pins = "mpp59";
 					marvell,function = "spi1";
 				};
 
-				cf_gtr_front_button_pins: cf-gtr-front-button-pins {
-					marvell,pins = "mpp53";
+				cf_gtr_switch_reset_pins: cf-gtr-switch-reset-pins {
+					marvell,pins = "mpp18";
 					marvell,function = "gpio";
 				};
 
-				cf_gtr_rear_button_pins: cf-gtr-rear-button-pins {
-					marvell,pins = "mpp36";
+				cf_gtr_usb3_con_vbus: cf-gtr-usb3-con-vbus {
+					marvell,pins = "mpp22";
 					marvell,function = "gpio";
 				};
 			};
-- 
2.35.3


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

[PATCH v2 6/8] arm: dts: marvell: clearfog-gtr: add missing pinctrl for all used gpios

[Josua Mayer]

Various control signals such as sfp module-absence, pci-e reset or led
gpios were missing pinctrl nodes, leaving any u-boot choices in place.

Since U-Boot is shared between multiple board variants, i.e. a388
clearfog pro / base, clearfog gtr l / 4, it is better to explicitly
configure functions.
Add explicit pinctrl entries for all gpios currently in use.

Additionally the loss-of-signal gpio specified is invalid, in fact los
only has a pull-up on the board but no gpio connection to the cpu.
Remove this stray reference.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 .../dts/marvell/armada-385-clearfog-gtr.dtsi  | 42 +++++++++++++++++--
 1 file changed, 39 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr.dtsi b/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr.dtsi
index 8eabb60765b0..39ac97edb463 100644
--- a/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr.dtsi
+++ b/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr.dtsi
@@ -162,6 +162,22 @@ cf_gtr_isolation_pins: cf-gtr-isolation-pins {
 					marvell,function = "gpio";
 				};
 
+				cf_gtr_led_pins: led-pins {
+					marvell,pins = "mpp42", "mpp52";
+					marvell,function = "gpio";
+				};
+
+				cf_gtr_lte_disable_pins: lte-disable-pins {
+					marvell,pins = "mpp34";
+					marvell,function = "gpio";
+				};
+
+				cf_gtr_pci_pins: pci-pins {
+					// pci reset
+					marvell,pins = "mpp33", "mpp35", "mpp44";
+					marvell,function = "gpio";
+				};
+
 				cf_gtr_poe_reset_pins: cf-gtr-poe-reset-pins {
 					marvell,pins = "mpp48";
 					marvell,function = "gpio";
@@ -179,6 +195,12 @@ cf_gtr_sdhci_pins: cf-gtr-sdhci-pins {
 					marvell,function = "sd0";
 				};
 
+				cf_gtr_sfp0_pins: sfp0-pins {
+					/* sfp modabs, txdisable */
+					marvell,pins = "mpp25", "mpp46";
+					marvell,function = "gpio";
+				};
+
 				cf_gtr_spi1_cs_pins: spi1-cs-pins {
 					marvell,pins = "mpp59";
 					marvell,function = "spi1";
@@ -193,6 +215,11 @@ cf_gtr_usb3_con_vbus: cf-gtr-usb3-con-vbus {
 					marvell,pins = "mpp22";
 					marvell,function = "gpio";
 				};
+
+				cf_gtr_wifi_disable_pins: wifi-disable-pins {
+					marvell,pins = "mpp30", "mpp31";
+					marvell,function = "gpio";
+				};
 			};
 
 			sdhci@d8000 {
@@ -221,21 +248,26 @@ usb3@f8000 {
 		};
 
 		pcie {
+			pinctrl-0 = <&cf_gtr_pci_pins>;
+			pinctrl-names = "default";
 			status = "okay";
 			/*
 			 * The PCIe units are accessible through
 			 * the mini-PCIe connectors on the board.
 			 */
+			/* CON3 - serdes 0 */
 			pcie@1,0 {
 				reset-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
 				status = "okay";
 			};
 
+			/* CON4 - serdes 2 */
 			pcie@2,0 {
 				reset-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
 				status = "okay";
 			};
 
+			/* CON2 - serdes 4 */
 			pcie@3,0 {
 				reset-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
 				status = "okay";
@@ -243,10 +275,12 @@ pcie@3,0 {
 		};
 	};
 
+	/* CON5 */
 	sfp0: sfp {
 		compatible = "sff,sfp";
+		pinctrl-0 = <&cf_gtr_sfp0_pins>;
+		pinctrl-names = "default";
 		i2c-bus = <&i2c1>;
-		los-gpio = <&gpio1 22 GPIO_ACTIVE_HIGH>;
 		mod-def0-gpio = <&gpio0 25 GPIO_ACTIVE_LOW>;
 		tx-disable-gpio = <&gpio1 14 GPIO_ACTIVE_HIGH>;
 	};
@@ -273,6 +307,8 @@ button-1 {
 
 	gpio-leds {
 		compatible = "gpio-leds";
+		pinctrl-0 = <&cf_gtr_led_pins>;
+		pinctrl-names = "default";
 
 		led1 {
 			function = LED_FUNCTION_CPU;
@@ -408,7 +444,7 @@ &ahci1 {
 };
 
 &gpio0 {
-	pinctrl-0 = <&cf_gtr_fan_pwm>;
+	pinctrl-0 = <&cf_gtr_fan_pwm &cf_gtr_wifi_disable_pins>;
 	pinctrl-names = "default";
 
 	wifi-disable {
@@ -420,7 +456,7 @@ wifi-disable {
 };
 
 &gpio1 {
-	pinctrl-0 = <&cf_gtr_isolation_pins &cf_gtr_poe_reset_pins>;
+	pinctrl-0 = <&cf_gtr_isolation_pins &cf_gtr_poe_reset_pins &cf_gtr_lte_disable_pins>;
 	pinctrl-names = "default";
 
 	lte-disable {
-- 
2.35.3


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

[PATCH v2 7/8] arm: dts: marvell: clearfog-gtr-l8: add support for second sfp connector

[Josua Mayer]

Clearfog GTR L8 has an extra SFP connector on the managed switch port 9.
Add descriptions for both entities along with pinctrl.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 .../marvell/armada-385-clearfog-gtr-l8.dts    | 20 ++++++++++++++++++-
 .../dts/marvell/armada-385-clearfog-gtr.dtsi  |  8 +++++++-
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr-l8.dts b/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr-l8.dts
index cd5b070f1218..2df388f222ec 100644
--- a/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr-l8.dts
+++ b/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr-l8.dts
@@ -6,6 +6,16 @@ / {
 	model = "SolidRun Clearfog GTR L8";
 	compatible = "solidrun,clearfog-gtr-l8", "marvell,armada385",
 		     "marvell,armada380";
+
+	/* CON25 */
+	sfp1: sfp-1 {
+		compatible = "sff,sfp";
+		pinctrl-0 = <&cf_gtr_sfp1_pins>;
+		pinctrl-names = "default";
+		i2c-bus = <&i2c0>;
+		mod-def0-gpio = <&gpio0 24 GPIO_ACTIVE_LOW>;
+		tx-disable-gpio = <&gpio1 22 GPIO_ACTIVE_HIGH>;
+	};
 };
 
 &mdio {
@@ -68,11 +78,19 @@ port@8 {
 				phy-handle = <&switch0phy7>;
 			};
 
+			port@9 {
+				reg = <9>;
+				label = "lan-sfp";
+				phy-mode = "sgmii";
+				sfp = <&sfp1>;
+				managed = "in-band-status";
+			};
+
 			port@10 {
 				reg = <10>;
 				phy-mode = "2500base-x";
-
 				ethernet = <&eth1>;
+
 				fixed-link {
 					speed = <2500>;
 					full-duplex;
diff --git a/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr.dtsi b/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr.dtsi
index 39ac97edb463..f3a3cb6ac311 100644
--- a/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr.dtsi
+++ b/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr.dtsi
@@ -201,6 +201,12 @@ cf_gtr_sfp0_pins: sfp0-pins {
 					marvell,function = "gpio";
 				};
 
+				cf_gtr_sfp1_pins: sfp1-pins {
+					/* sfp modabs, txdisable */
+					marvell,pins = "mpp24", "mpp54";
+					marvell,function = "gpio";
+				};
+
 				cf_gtr_spi1_cs_pins: spi1-cs-pins {
 					marvell,pins = "mpp59";
 					marvell,function = "spi1";
@@ -276,7 +282,7 @@ pcie@3,0 {
 	};
 
 	/* CON5 */
-	sfp0: sfp {
+	sfp0: sfp-0 {
 		compatible = "sff,sfp";
 		pinctrl-0 = <&cf_gtr_sfp0_pins>;
 		pinctrl-names = "default";
-- 
2.35.3


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

[PATCH v2 8/8] arm: dts: marvell: clearfog-gtr-l8: align port numbers with enclosure

[Josua Mayer]

Clearfog GTR has an official enclosure with labels for all interfaces.
The "lan" ports on the 8-port switch in device-tree were numbered  in
reverse wrt. enclosure.

Update all device-tree labels to match.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 .../dts/marvell/armada-385-clearfog-gtr-l8.dts   | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr-l8.dts b/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr-l8.dts
index 2df388f222ec..da6981677b03 100644
--- a/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr-l8.dts
+++ b/arch/arm/boot/dts/marvell/armada-385-clearfog-gtr-l8.dts
@@ -32,49 +32,49 @@ ports {
 
 			port@1 {
 				reg = <1>;
-				label = "lan8";
+				label = "lan1";
 				phy-handle = <&switch0phy0>;
 			};
 
 			port@2 {
 				reg = <2>;
-				label = "lan7";
+				label = "lan2";
 				phy-handle = <&switch0phy1>;
 			};
 
 			port@3 {
 				reg = <3>;
-				label = "lan6";
+				label = "lan3";
 				phy-handle = <&switch0phy2>;
 			};
 
 			port@4 {
 				reg = <4>;
-				label = "lan5";
+				label = "lan4";
 				phy-handle = <&switch0phy3>;
 			};
 
 			port@5 {
 				reg = <5>;
-				label = "lan4";
+				label = "lan5";
 				phy-handle = <&switch0phy4>;
 			};
 
 			port@6 {
 				reg = <6>;
-				label = "lan3";
+				label = "lan6";
 				phy-handle = <&switch0phy5>;
 			};
 
 			port@7 {
 				reg = <7>;
-				label = "lan2";
+				label = "lan7";
 				phy-handle = <&switch0phy6>;
 			};
 
 			port@8 {
 				reg = <8>;
-				label = "lan1";
+				label = "lan8";
 				phy-handle = <&switch0phy7>;
 			};
 
-- 
2.35.3


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

Re: [PATCH v2 2/8] dt-bindings: marvell: a38x: add solidrun armada 385 clearfog gtr boards

[Krzysztof Kozlowski]

On 24/12/2023 15:38, Josua Mayer wrote:
> Add DT compatible for SolidRun Armada-385 based Clearfog GTR L8 and S4
> boards.
> 
> Signed-off-by: Josua Mayer <josua@solid-run.com>

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time, thus I will skip this patch entirely till you follow
the process allowing the patch to be tested.

Please kindly resend and include all necessary To/Cc entries.

> ---
>  .../devicetree/bindings/arm/marvell/armada-38x.yaml       | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
> index 5af222e6db18..5bc69c4fe34d 100644
> --- a/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
> +++ b/Documentation/devicetree/bindings/arm/marvell/armada-38x.yaml
> @@ -30,4 +30,12 @@ properties:
>            - const: marvell,armada385
>            - const: marvell,armada380
>  
> +      - description: SolidRun Armada 385 based single-board computers
> +        items:
> +          - enum:
> +             - solidrun,clearfog-gtr-l8
> +             - solidrun,clearfog-gtr-s4

Not tested. This will fail and if you bothered to Cc lists, at least
automation would test it for you. Now, neither you nor automation will
tell it does not work :(


Best regards,
Krzysztof


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