[1/3] dt-bindings: arm: fsl: fix DEBIX binding

Message ID 20230704184109.991104-1-m.felsch@pengutronix.de
State New
Headers
Series [1/3] dt-bindings: arm: fsl: fix DEBIX binding |

Commit Message

Marco Felsch July 4, 2023, 6:41 p.m. UTC
  The current imx8mp-debix-model-a.dts uses all three compatibles. Fix the
corresponding bindings by adding an own entry for it.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 Documentation/devicetree/bindings/arm/fsl.yaml | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
  

Comments

Krzysztof Kozlowski July 5, 2023, 6:35 a.m. UTC | #1
On 04/07/2023 20:41, Marco Felsch wrote:
> The current imx8mp-debix-model-a.dts uses all three compatibles. Fix the
> corresponding bindings by adding an own entry for it.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/arm/fsl.yaml | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> index 15d4110840654..d9e763ef932e5 100644
> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> @@ -1019,8 +1019,6 @@ properties:
>                - dmo,imx8mp-data-modul-edm-sbc # i.MX8MP eDM SBC
>                - fsl,imx8mp-evk            # i.MX8MP EVK Board
>                - gateworks,imx8mp-gw74xx   # i.MX8MP Gateworks Board
> -              - polyhex,imx8mp-debix      # Polyhex Debix boards
> -              - polyhex,imx8mp-debix-model-a # Polyhex Debix Model A Board
>                - toradex,verdin-imx8mp     # Verdin iMX8M Plus Modules
>                - toradex,verdin-imx8mp-nonwifi  # Verdin iMX8M Plus Modules without Wi-Fi / BT
>                - toradex,verdin-imx8mp-wifi  # Verdin iMX8M Plus Wi-Fi / BT Modules
> @@ -1054,6 +1052,13 @@ properties:
>            - const: phytec,imx8mp-phycore-som         # phyCORE-i.MX8MP SoM
>            - const: fsl,imx8mp
>  
> +      - description: Polyhex DEBIX i.MX8MP based SBCs
> +        items:
> +          - enum:
> +              - polyhex,imx8mp-debix-model-a        # Polyhex Debix Model A Board
> +          - const: polyhex,imx8mp-debix             # Polyhex Debix boards

Same comments as for patch #2. I think this should be rather deprecated
- not a good pattern.

Best regards,
Krzysztof
  
Marco Felsch July 5, 2023, 8:28 a.m. UTC | #2
Hi Krzysztof,

On 23-07-05, Krzysztof Kozlowski wrote:
> On 04/07/2023 20:41, Marco Felsch wrote:
> > The current imx8mp-debix-model-a.dts uses all three compatibles. Fix the
> > corresponding bindings by adding an own entry for it.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  Documentation/devicetree/bindings/arm/fsl.yaml | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> > index 15d4110840654..d9e763ef932e5 100644
> > --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > @@ -1019,8 +1019,6 @@ properties:
> >                - dmo,imx8mp-data-modul-edm-sbc # i.MX8MP eDM SBC
> >                - fsl,imx8mp-evk            # i.MX8MP EVK Board
> >                - gateworks,imx8mp-gw74xx   # i.MX8MP Gateworks Board
> > -              - polyhex,imx8mp-debix      # Polyhex Debix boards
> > -              - polyhex,imx8mp-debix-model-a # Polyhex Debix Model A Board
> >                - toradex,verdin-imx8mp     # Verdin iMX8M Plus Modules
> >                - toradex,verdin-imx8mp-nonwifi  # Verdin iMX8M Plus Modules without Wi-Fi / BT
> >                - toradex,verdin-imx8mp-wifi  # Verdin iMX8M Plus Wi-Fi / BT Modules
> > @@ -1054,6 +1052,13 @@ properties:
> >            - const: phytec,imx8mp-phycore-som         # phyCORE-i.MX8MP SoM
> >            - const: fsl,imx8mp
> >  
> > +      - description: Polyhex DEBIX i.MX8MP based SBCs
> > +        items:
> > +          - enum:
> > +              - polyhex,imx8mp-debix-model-a        # Polyhex Debix Model A Board
> > +          - const: polyhex,imx8mp-debix             # Polyhex Debix boards
> 
> Same comments as for patch #2. I think this should be rather deprecated
> - not a good pattern.

How can we deprecate a binding?

Regards,
  Marco
  
Krzysztof Kozlowski July 5, 2023, 9:15 a.m. UTC | #3
On 05/07/2023 10:28, Marco Felsch wrote:
> Hi Krzysztof,
> 
> On 23-07-05, Krzysztof Kozlowski wrote:
>> On 04/07/2023 20:41, Marco Felsch wrote:
>>> The current imx8mp-debix-model-a.dts uses all three compatibles. Fix the
>>> corresponding bindings by adding an own entry for it.
>>>
>>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
>>> ---
>>>  Documentation/devicetree/bindings/arm/fsl.yaml | 9 +++++++--
>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
>>> index 15d4110840654..d9e763ef932e5 100644
>>> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
>>> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
>>> @@ -1019,8 +1019,6 @@ properties:
>>>                - dmo,imx8mp-data-modul-edm-sbc # i.MX8MP eDM SBC
>>>                - fsl,imx8mp-evk            # i.MX8MP EVK Board
>>>                - gateworks,imx8mp-gw74xx   # i.MX8MP Gateworks Board
>>> -              - polyhex,imx8mp-debix      # Polyhex Debix boards
>>> -              - polyhex,imx8mp-debix-model-a # Polyhex Debix Model A Board
>>>                - toradex,verdin-imx8mp     # Verdin iMX8M Plus Modules
>>>                - toradex,verdin-imx8mp-nonwifi  # Verdin iMX8M Plus Modules without Wi-Fi / BT
>>>                - toradex,verdin-imx8mp-wifi  # Verdin iMX8M Plus Wi-Fi / BT Modules
>>> @@ -1054,6 +1052,13 @@ properties:
>>>            - const: phytec,imx8mp-phycore-som         # phyCORE-i.MX8MP SoM
>>>            - const: fsl,imx8mp
>>>  
>>> +      - description: Polyhex DEBIX i.MX8MP based SBCs
>>> +        items:
>>> +          - enum:
>>> +              - polyhex,imx8mp-debix-model-a        # Polyhex Debix Model A Board
>>> +          - const: polyhex,imx8mp-debix             # Polyhex Debix boards
>>
>> Same comments as for patch #2. I think this should be rather deprecated
>> - not a good pattern.
> 
> How can we deprecate a binding?

git grep "deprecated:" -- Documentation/devicetree/

Best regards,
Krzysztof
  
Ahmad Fatoum July 5, 2023, 9:28 a.m. UTC | #4
On 05.07.23 08:35, Krzysztof Kozlowski wrote:
> On 04/07/2023 20:41, Marco Felsch wrote:
>> The current imx8mp-debix-model-a.dts uses all three compatibles. Fix the
>> corresponding bindings by adding an own entry for it.
>>
>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
>> ---
>>  Documentation/devicetree/bindings/arm/fsl.yaml | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
>> index 15d4110840654..d9e763ef932e5 100644
>> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
>> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
>> @@ -1019,8 +1019,6 @@ properties:
>>                - dmo,imx8mp-data-modul-edm-sbc # i.MX8MP eDM SBC
>>                - fsl,imx8mp-evk            # i.MX8MP EVK Board
>>                - gateworks,imx8mp-gw74xx   # i.MX8MP Gateworks Board
>> -              - polyhex,imx8mp-debix      # Polyhex Debix boards
>> -              - polyhex,imx8mp-debix-model-a # Polyhex Debix Model A Board
>>                - toradex,verdin-imx8mp     # Verdin iMX8M Plus Modules
>>                - toradex,verdin-imx8mp-nonwifi  # Verdin iMX8M Plus Modules without Wi-Fi / BT
>>                - toradex,verdin-imx8mp-wifi  # Verdin iMX8M Plus Wi-Fi / BT Modules
>> @@ -1054,6 +1052,13 @@ properties:
>>            - const: phytec,imx8mp-phycore-som         # phyCORE-i.MX8MP SoM
>>            - const: fsl,imx8mp
>>  
>> +      - description: Polyhex DEBIX i.MX8MP based SBCs
>> +        items:
>> +          - enum:
>> +              - polyhex,imx8mp-debix-model-a        # Polyhex Debix Model A Board
>> +          - const: polyhex,imx8mp-debix             # Polyhex Debix boards
> 
> Same comments as for patch #2. I think this should be rather deprecated
> - not a good pattern.

The middle compatible was my suggestion, because there's also the Debix Model
B Standard and Model B SE, which is the same board, but different SoC variant:

 Model A:          Commercial Temperature Grade
 Model B Standard: Industrial Temperature Grade
 Model B SE:       Industrial Temperature Grate, but i.MX8MP Lite
                   (No Neural/Video/Image accelerators).

As everything outside the SoC is the same, I wanted a generic board
compatible that bootloaders can match against. The SoMs should probably
not reuse it, but I think it should be kept (perhaps renamed?) for the
SBCs that don't utilize the Debix SoM.

Cheers,
Ahmad

> 
> Best regards,
> Krzysztof
> 
> 
>
  
Krzysztof Kozlowski July 5, 2023, 9:37 a.m. UTC | #5
On 05/07/2023 11:28, Ahmad Fatoum wrote:
>>> +        items:
>>> +          - enum:
>>> +              - polyhex,imx8mp-debix-model-a        # Polyhex Debix Model A Board
>>> +          - const: polyhex,imx8mp-debix             # Polyhex Debix boards
>>
>> Same comments as for patch #2. I think this should be rather deprecated
>> - not a good pattern.
> 
> The middle compatible was my suggestion, because there's also the Debix Model
> B Standard and Model B SE, which is the same board, but different SoC variant:
> 
>  Model A:          Commercial Temperature Grade
>  Model B Standard: Industrial Temperature Grade
>  Model B SE:       Industrial Temperature Grate, but i.MX8MP Lite
>                    (No Neural/Video/Image accelerators).
> 
> As everything outside the SoC is the same, I wanted a generic board
> compatible that bootloaders can match against. The SoMs should probably
> not reuse it, but I think it should be kept (perhaps renamed?) for the
> SBCs that don't utilize the Debix SoM.

The order of compatibles in patch two does not really look correct,
although it is accepted in some cases (e.g. Renesas). But anyway "Debix"
sounds like a vendor - they even have website - so compatible for all
boards seems too generic. This should be compatible for one specific
board. I understand that one board can have different SoMs (it is
common, just look at Toradex or Variscite), but it does not mean that
board should be unspecific.

Best regards,
Krzysztof
  
Marco Felsch July 14, 2023, 3:16 p.m. UTC | #6
On 23-07-05, Krzysztof Kozlowski wrote:
> On 05/07/2023 11:28, Ahmad Fatoum wrote:
> >>> +        items:
> >>> +          - enum:
> >>> +              - polyhex,imx8mp-debix-model-a        # Polyhex Debix Model A Board
> >>> +          - const: polyhex,imx8mp-debix             # Polyhex Debix boards
> >>
> >> Same comments as for patch #2. I think this should be rather deprecated
> >> - not a good pattern.
> > 
> > The middle compatible was my suggestion, because there's also the Debix Model
> > B Standard and Model B SE, which is the same board, but different SoC variant:
> > 
> >  Model A:          Commercial Temperature Grade
> >  Model B Standard: Industrial Temperature Grade
> >  Model B SE:       Industrial Temperature Grate, but i.MX8MP Lite
> >                    (No Neural/Video/Image accelerators).
> > 
> > As everything outside the SoC is the same, I wanted a generic board
> > compatible that bootloaders can match against. The SoMs should probably
> > not reuse it, but I think it should be kept (perhaps renamed?) for the
> > SBCs that don't utilize the Debix SoM.

The SoM may come also in a 'Standard' and 'SE' edition.

> The order of compatibles in patch two does not really look correct,
> although it is accepted in some cases (e.g. Renesas). But anyway "Debix"
> sounds like a vendor - they even have website - so compatible for all
> boards seems too generic. This should be compatible for one specific
> board. I understand that one board can have different SoMs (it is
> common, just look at Toradex or Variscite), but it does not mean that
> board should be unspecific.

I reused the "polyhex,imx8mp-debix" compatible since we already have a
user [1] and there are no differences.

I can drop it for the SoM case but for the SBC case I can't since this
would break current users [1].

Regards,
  Marco

[1] https://elixir.bootlin.com/barebox/v2023.07.1/source/arch/arm/boards/polyhex-debix/board.c#L38
  

Patch

diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
index 15d4110840654..d9e763ef932e5 100644
--- a/Documentation/devicetree/bindings/arm/fsl.yaml
+++ b/Documentation/devicetree/bindings/arm/fsl.yaml
@@ -1019,8 +1019,6 @@  properties:
               - dmo,imx8mp-data-modul-edm-sbc # i.MX8MP eDM SBC
               - fsl,imx8mp-evk            # i.MX8MP EVK Board
               - gateworks,imx8mp-gw74xx   # i.MX8MP Gateworks Board
-              - polyhex,imx8mp-debix      # Polyhex Debix boards
-              - polyhex,imx8mp-debix-model-a # Polyhex Debix Model A Board
               - toradex,verdin-imx8mp     # Verdin iMX8M Plus Modules
               - toradex,verdin-imx8mp-nonwifi  # Verdin iMX8M Plus Modules without Wi-Fi / BT
               - toradex,verdin-imx8mp-wifi  # Verdin iMX8M Plus Wi-Fi / BT Modules
@@ -1054,6 +1052,13 @@  properties:
           - const: phytec,imx8mp-phycore-som         # phyCORE-i.MX8MP SoM
           - const: fsl,imx8mp
 
+      - description: Polyhex DEBIX i.MX8MP based SBCs
+        items:
+          - enum:
+              - polyhex,imx8mp-debix-model-a        # Polyhex Debix Model A Board
+          - const: polyhex,imx8mp-debix             # Polyhex Debix boards
+          - const: fsl,imx8mp
+
       - description: Toradex Boards with Verdin iMX8M Plus Modules
         items:
           - enum: