Re: [PATCH 1/2] dt-bindings: arm64: marvell: add solidrun cn9130 clearfog boards

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 22/03/2024 11:08, Josua Mayer wrote:
> Am 21.03.24 um 22:47 schrieb Josua Mayer:
>> Add bindings for SolidRun Clearfog boards, using a new SoM based on
>> CN9130 SoC.
>> The carrier boards are identical to the older Armada 388 based Clearfog
>> boards. For consistency the carrier part of compatible strings are
>> copied, including the established "-a1" suffix.
>>
>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>> ---
>>  .../devicetree/bindings/arm/marvell/armada-7k-8k.yaml        | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
>> index 16d2e132d3d1..36bdfd1bedd9 100644
>> --- a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
>> +++ b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
>> @@ -82,4 +82,16 @@ properties:
>>            - const: marvell,armada-ap807-quad
>>            - const: marvell,armada-ap807
>>  
>> +      - description:
>> +          SolidRun CN9130 clearfog family single-board computers
>> +        items:
>> +          - enum:
>> +              - solidrun,clearfog-base-a1
>> +              - solidrun,clearfog-pro-a1
>> +          - const: solidrun,clearfog-a1
>> +          - const: solidrun,cn9130-sr-som
>> +          - const: marvell,cn9130
>> +          - const: marvell,armada-ap807-quad
>> +          - const: marvell,armada-ap807
>> +
>>  additionalProperties: true
> 
> Before merging I would like some feedback about adding
> another product later, to ensure the compatibles above
> are adequate? In particular:
> - sequence of soc, cp, carrier compatibles
> - name of som compatible
> 
> Draft for future bindings:
>       - description:
>           SolidRun CN9130 SoM based single-board computers
>           with 1 external CP on the Carrier.
>         items:
>           - enum:
>               - solidrun,cn9131-solidwan
>           - const: marvell,cn9131
>           - const: solidrun,cn9130-sr-som

This does not look correct. cn9131 is not compatible with your som.

>           - const: marvell,cn9130

SoCs are compatible only in some cases, e.g. one is a subset of another
like stripped out of modem. Are you sure this is your case?


>           - const: marvell,armada-ap807-quad
>           - const: marvell,armada-ap807

Anyway, 6 compatibles is beyond useful amount. What are you expressing
here? Why is this even armada ap807?

Best regards,
Krzysztof