[v2,1/2] dt-bindings: arm64: mediatek: add Kontron 3.5"-SBC-i1200

Message ID 20240219084456.1075445-1-mwalle@kernel.org
State New
Headers
Series [v2,1/2] dt-bindings: arm64: mediatek: add Kontron 3.5"-SBC-i1200 |

Commit Message

Michael Walle Feb. 19, 2024, 8:44 a.m. UTC
  Add the compatible string for the Kontron 3.5"-SBC-i1200 single board
computer.

Signed-off-by: Michael Walle <mwalle@kernel.org>
---
v2:
 - convert enum to const as there is only one specific board

 Documentation/devicetree/bindings/arm/mediatek.yaml | 5 +++++
 1 file changed, 5 insertions(+)
  

Comments

Krzysztof Kozlowski Feb. 19, 2024, 9:06 a.m. UTC | #1
On 19/02/2024 09:44, Michael Walle wrote:
> Add the compatible string for the Kontron 3.5"-SBC-i1200 single board
> computer.
> 
> Signed-off-by: Michael Walle <mwalle@kernel.org>
> ---
> v2:
>  - convert enum to const as there is only one specific board
> 
>  Documentation/devicetree/bindings/arm/mediatek.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml
> index 09f9ffd3ff7b..add167d8b8da 100644
> --- a/Documentation/devicetree/bindings/arm/mediatek.yaml
> +++ b/Documentation/devicetree/bindings/arm/mediatek.yaml
> @@ -357,6 +357,11 @@ properties:
>                - radxa,nio-12l
>            - const: mediatek,mt8395
>            - const: mediatek,mt8195
> +      - description: Kontron 3.5"-SBC-i1200
> +        items:
> +          - const: kontron,3-5-sbc-i1200

And then it should be part of earlier entry. Just take a look there in
the file...

Best regards,
Krzysztof
  
AngeloGioacchino Del Regno Feb. 19, 2024, 9:23 a.m. UTC | #2
Il 19/02/24 09:44, Michael Walle ha scritto:
> Add the compatible string for the Kontron 3.5"-SBC-i1200 single board
> computer.
> 
> Signed-off-by: Michael Walle <mwalle@kernel.org>
> ---
> v2:
>   - convert enum to const as there is only one specific board
> 
>   Documentation/devicetree/bindings/arm/mediatek.yaml | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml
> index 09f9ffd3ff7b..add167d8b8da 100644
> --- a/Documentation/devicetree/bindings/arm/mediatek.yaml
> +++ b/Documentation/devicetree/bindings/arm/mediatek.yaml
> @@ -357,6 +357,11 @@ properties:
>                 - radxa,nio-12l
>             - const: mediatek,mt8395
>             - const: mediatek,mt8195
> +      - description: Kontron 3.5"-SBC-i1200
> +        items:
> +          - const: kontron,3-5-sbc-i1200
> +          - const: mediatek,mt8395
> +          - const: mediatek,mt8195
>         - items:
>             - enum:
>                 - mediatek,mt8516-pumpkin

I understand that you took inspiration from the Google Chromebooks entries, but
those are separated only because they've got "a bunch of revisions".

For machines that don't have a billion compatible strings, I would suggest to
add the compatible under a single big entry: like this, the binding is (imo) a
bit more readable .. and cleaner.

Eventually, the machine name is in the devicetree so we're not losing any kind
of information anyway... :-)

       - items:
           - enum:
               - kontron,3-5-sbc-i1200
               - mediatek,mt8395-evk
               - radxa,nio-12l
           - const: mediatek,mt8395
           - const: mediatek,mt8195

Cheers,
Angelo
  
Michael Walle Feb. 19, 2024, 9:25 a.m. UTC | #3
On Mon Feb 19, 2024 at 10:06 AM CET, Krzysztof Kozlowski wrote:
> On 19/02/2024 09:44, Michael Walle wrote:
> > Add the compatible string for the Kontron 3.5"-SBC-i1200 single board
> > computer.
> > 
> > Signed-off-by: Michael Walle <mwalle@kernel.org>
> > ---
> > v2:
> >  - convert enum to const as there is only one specific board
> > 
> >  Documentation/devicetree/bindings/arm/mediatek.yaml | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml
> > index 09f9ffd3ff7b..add167d8b8da 100644
> > --- a/Documentation/devicetree/bindings/arm/mediatek.yaml
> > +++ b/Documentation/devicetree/bindings/arm/mediatek.yaml
> > @@ -357,6 +357,11 @@ properties:
> >                - radxa,nio-12l
> >            - const: mediatek,mt8395
> >            - const: mediatek,mt8195
> > +      - description: Kontron 3.5"-SBC-i1200
> > +        items:
> > +          - const: kontron,3-5-sbc-i1200
>
> And then it should be part of earlier entry. Just take a look there in
> the file...

FWIW I did and all the entries with descriptions are by there own
(that's also true for fsl.yaml and sunxi.yaml).

mediatek.yaml seems to have both, either boards without description
like the "radxa,nio-12l" which you are probably referring to. Or
boards which has the same sub compatibles but still have individual
entries, like google,burnet, google,cozmo, google,damu.

Therefore, what is the rule of thumb here? Drop the description?
If so, why it it there in the first place?

-michael
  
Michael Walle Feb. 19, 2024, 9:36 a.m. UTC | #4
On Mon Feb 19, 2024 at 10:23 AM CET, AngeloGioacchino Del Regno wrote:
> Il 19/02/24 09:44, Michael Walle ha scritto:
> > Add the compatible string for the Kontron 3.5"-SBC-i1200 single board
> > computer.
> > 
> > Signed-off-by: Michael Walle <mwalle@kernel.org>
> > ---
> > v2:
> >   - convert enum to const as there is only one specific board
> > 
> >   Documentation/devicetree/bindings/arm/mediatek.yaml | 5 +++++
> >   1 file changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml
> > index 09f9ffd3ff7b..add167d8b8da 100644
> > --- a/Documentation/devicetree/bindings/arm/mediatek.yaml
> > +++ b/Documentation/devicetree/bindings/arm/mediatek.yaml
> > @@ -357,6 +357,11 @@ properties:
> >                 - radxa,nio-12l
> >             - const: mediatek,mt8395
> >             - const: mediatek,mt8195
> > +      - description: Kontron 3.5"-SBC-i1200
> > +        items:
> > +          - const: kontron,3-5-sbc-i1200
> > +          - const: mediatek,mt8395
> > +          - const: mediatek,mt8195
> >         - items:
> >             - enum:
> >                 - mediatek,mt8516-pumpkin
>
> I understand that you took inspiration from the Google Chromebooks entries, but
> those are separated only because they've got "a bunch of revisions".
>

I don't really care about the description. It's just the way I've
done it in the past. And in this file, there is also google,burnet,
google,cozmo, google,damu, without any further revs.

> For machines that don't have a billion compatible strings, I would suggest to
> add the compatible under a single big entry: like this, the binding is (imo) a
> bit more readable .. and cleaner.
>
> Eventually, the machine name is in the devicetree so we're not losing any kind
> of information anyway... :-)
>
>        - items:
>            - enum:
>                - kontron,3-5-sbc-i1200
>                - mediatek,mt8395-evk
>                - radxa,nio-12l
>            - const: mediatek,mt8395
>            - const: mediatek,mt8195

Ack. I'll give you some time to look at the actual DTS before
posting a new version.

-michael
  
AngeloGioacchino Del Regno Feb. 19, 2024, 10:07 a.m. UTC | #5
Il 19/02/24 10:36, Michael Walle ha scritto:
> On Mon Feb 19, 2024 at 10:23 AM CET, AngeloGioacchino Del Regno wrote:
>> Il 19/02/24 09:44, Michael Walle ha scritto:
>>> Add the compatible string for the Kontron 3.5"-SBC-i1200 single board
>>> computer.
>>>
>>> Signed-off-by: Michael Walle <mwalle@kernel.org>
>>> ---
>>> v2:
>>>    - convert enum to const as there is only one specific board
>>>
>>>    Documentation/devicetree/bindings/arm/mediatek.yaml | 5 +++++
>>>    1 file changed, 5 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml
>>> index 09f9ffd3ff7b..add167d8b8da 100644
>>> --- a/Documentation/devicetree/bindings/arm/mediatek.yaml
>>> +++ b/Documentation/devicetree/bindings/arm/mediatek.yaml
>>> @@ -357,6 +357,11 @@ properties:
>>>                  - radxa,nio-12l
>>>              - const: mediatek,mt8395
>>>              - const: mediatek,mt8195
>>> +      - description: Kontron 3.5"-SBC-i1200
>>> +        items:
>>> +          - const: kontron,3-5-sbc-i1200
>>> +          - const: mediatek,mt8395
>>> +          - const: mediatek,mt8195
>>>          - items:
>>>              - enum:
>>>                  - mediatek,mt8516-pumpkin
>>
>> I understand that you took inspiration from the Google Chromebooks entries, but
>> those are separated only because they've got "a bunch of revisions".
>>
> 
> I don't really care about the description. It's just the way I've
> done it in the past. And in this file, there is also google,burnet,
> google,cozmo, google,damu, without any further revs.
> 

Yeah, I have to check why it is like this - as in - if there's any real reason for
those to be like that... otherwise I wouldn't mind cleaning 'em all up.

Whatever - that's not on you, and completely irrelevant to this specific patch,
so don't worry about the cleanup of the other entries.

>> For machines that don't have a billion compatible strings, I would suggest to
>> add the compatible under a single big entry: like this, the binding is (imo) a
>> bit more readable .. and cleaner.
>>
>> Eventually, the machine name is in the devicetree so we're not losing any kind
>> of information anyway... :-)
>>
>>         - items:
>>             - enum:
>>                 - kontron,3-5-sbc-i1200
>>                 - mediatek,mt8395-evk
>>                 - radxa,nio-12l
>>             - const: mediatek,mt8395
>>             - const: mediatek,mt8195
> 
> Ack. I'll give you some time to look at the actual DTS before
> posting a new version.
> 
> -michael
  

Patch

diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml
index 09f9ffd3ff7b..add167d8b8da 100644
--- a/Documentation/devicetree/bindings/arm/mediatek.yaml
+++ b/Documentation/devicetree/bindings/arm/mediatek.yaml
@@ -357,6 +357,11 @@  properties:
               - radxa,nio-12l
           - const: mediatek,mt8395
           - const: mediatek,mt8195
+      - description: Kontron 3.5"-SBC-i1200
+        items:
+          - const: kontron,3-5-sbc-i1200
+          - const: mediatek,mt8395
+          - const: mediatek,mt8195
       - items:
           - enum:
               - mediatek,mt8516-pumpkin