2023-07-17 16:57:26

by Marco Felsch

[permalink] [raw]
Subject: [PATCH v2 2/4] dt-bindings: arm: fsl: fix DEBIX binding

The current imx8mp-debix-model-a.dts uses all three compatibles. Fix the
corresponding bindings by adding an own entry for it. Mark
polyhex,imx8mp-debix as deprecated but keep it within the dts file since
we already have a user for it [1].

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

Signed-off-by: Marco Felsch <[email protected]>
---
Changelog:

v2:
- deprecate polyhex,imx8mp-debix

Documentation/devicetree/bindings/arm/fsl.yaml | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
index 15d4110840654..b29974e3c30b3 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,14 @@ 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
+ deprecated: true
+ - const: fsl,imx8mp
+
- description: Toradex Boards with Verdin iMX8M Plus Modules
items:
- enum:
--
2.39.2



2023-07-17 17:14:23

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: arm: fsl: fix DEBIX binding

On 17/07/2023 18:51, 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. Mark
> polyhex,imx8mp-debix as deprecated but keep it within the dts file since
> we already have a user for it [1].
>
> [1] https://elixir.bootlin.com/barebox/v2023.07.1/source/arch/arm/ \
> boards/polyhex-debix/board.c#L38
>
> Signed-off-by: Marco Felsch <[email protected]>

Subject: fix is too generic and binding is redundant. You already state
this is binding in your prefix. Describe more precise what you are doing.

> ---
> Changelog:
>
> v2:
> - deprecate polyhex,imx8mp-debix
>
> Documentation/devicetree/bindings/arm/fsl.yaml | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> index 15d4110840654..b29974e3c30b3 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,14 @@ 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
> + deprecated: true
> + - const: fsl,imx8mp

That's not how it works and it does not look like you tested the DTS
against bindings. Please run `make dtbs_check` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

Don't deprecate some piece of entire compatible list, but entire list.

The commit which deprecates compatible should bring a proper one.
Otherwise at this point we have kernel only with deprecated compatibles.


Best regards,
Krzysztof


2023-07-17 17:27:20

by Ahmad Fatoum

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: arm: fsl: fix DEBIX binding

On 17.07.23 18:51, 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. Mark
> polyhex,imx8mp-debix as deprecated but keep it within the dts file since
> we already have a user for it [1].
>
> [1] https://elixir.bootlin.com/barebox/v2023.07.1/source/arch/arm/ \
> boards/polyhex-debix/board.c#L38
>
> Signed-off-by: Marco Felsch <[email protected]>
> ---
> Changelog:
>
> v2:
> - deprecate polyhex,imx8mp-debix
>
> Documentation/devicetree/bindings/arm/fsl.yaml | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> index 15d4110840654..b29974e3c30b3 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,14 @@ 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
> + deprecated: true

I don't see why you need to deprecate this. Can't you just change the comment
to read "Polyhex i.MX8MP SBCs" or similar?

Cheers,
Ahmad

> + - const: fsl,imx8mp
> +
> - description: Toradex Boards with Verdin iMX8M Plus Modules
> items:
> - enum:

--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |


2023-07-17 17:27:30

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: arm: fsl: fix DEBIX binding

On 17/07/2023 18:51, 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. Mark
> polyhex,imx8mp-debix as deprecated but keep it within the dts file since
> we already have a user for it [1].
>
> [1] https://elixir.bootlin.com/barebox/v2023.07.1/source/arch/arm/ \
> boards/polyhex-debix/board.c#L38
>
> Signed-off-by: Marco Felsch <[email protected]>
> ---
> Changelog:
>
> v2:
> - deprecate polyhex,imx8mp-debix
>
> Documentation/devicetree/bindings/arm/fsl.yaml | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> index 15d4110840654..b29974e3c30b3 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,14 @@ 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

I cannot find patches which add new compatible to the binding and which
fix the DTS. :/

Best regards,
Krzysztof


2023-07-17 17:28:43

by Marco Felsch

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: arm: fsl: fix DEBIX binding

On 23-07-17, Krzysztof Kozlowski wrote:
> On 17/07/2023 18:51, 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. Mark
> > polyhex,imx8mp-debix as deprecated but keep it within the dts file since
> > we already have a user for it [1].
> >
> > [1] https://elixir.bootlin.com/barebox/v2023.07.1/source/arch/arm/ \
> > boards/polyhex-debix/board.c#L38
> >
> > Signed-off-by: Marco Felsch <[email protected]>
> > ---
> > Changelog:
> >
> > v2:
> > - deprecate polyhex,imx8mp-debix
> >
> > Documentation/devicetree/bindings/arm/fsl.yaml | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> > index 15d4110840654..b29974e3c30b3 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,14 @@ 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
>
> I cannot find patches which add new compatible to the binding and which
> fix the DTS. :/

Please see my commit message, we can't remove the compatible since we
already have one user of this compatible.

Regards,
Marco

2023-07-17 17:28:58

by Marco Felsch

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: arm: fsl: fix DEBIX binding

On 23-07-17, Krzysztof Kozlowski wrote:
> On 17/07/2023 18:51, 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. Mark
> > polyhex,imx8mp-debix as deprecated but keep it within the dts file since
> > we already have a user for it [1].
> >
> > [1] https://elixir.bootlin.com/barebox/v2023.07.1/source/arch/arm/ \
> > boards/polyhex-debix/board.c#L38
> >
> > Signed-off-by: Marco Felsch <[email protected]>
>
> Subject: fix is too generic and binding is redundant. You already state
> this is binding in your prefix. Describe more precise what you are doing.
>
> > ---
> > Changelog:
> >
> > v2:
> > - deprecate polyhex,imx8mp-debix
> >
> > Documentation/devicetree/bindings/arm/fsl.yaml | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> > index 15d4110840654..b29974e3c30b3 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,14 @@ 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
> > + deprecated: true
> > + - const: fsl,imx8mp
>
> That's not how it works and it does not look like you tested the DTS
> against bindings. Please run `make dtbs_check` (see
> Documentation/devicetree/bindings/writing-schema.rst or
> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
> for instructions).
>
> Don't deprecate some piece of entire compatible list, but entire list.
>
> The commit which deprecates compatible should bring a proper one.

What did you mean by that?

Regards,
Marco

> Otherwise at this point we have kernel only with deprecated compatibles.
>
>
> Best regards,
> Krzysztof
>
>
>

2023-07-17 18:03:24

by Ahmad Fatoum

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: arm: fsl: fix DEBIX binding

On 17.07.23 19:24, Marco Felsch wrote:
> On 23-07-17, Ahmad Fatoum wrote:
>> On 17.07.23 18:51, 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. Mark
>>> polyhex,imx8mp-debix as deprecated but keep it within the dts file since
>>> we already have a user for it [1].
>>>
>>> [1] https://elixir.bootlin.com/barebox/v2023.07.1/source/arch/arm/ \
>>> boards/polyhex-debix/board.c#L38
>>>
>>> Signed-off-by: Marco Felsch <[email protected]>
>>> ---
>>> Changelog:
>>>
>>> v2:
>>> - deprecate polyhex,imx8mp-debix
>>>
>>> Documentation/devicetree/bindings/arm/fsl.yaml | 10 ++++++++--
>>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
>>> index 15d4110840654..b29974e3c30b3 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,14 @@ 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
>>> + deprecated: true
>>
>> I don't see why you need to deprecate this. Can't you just change the comment
>> to read "Polyhex i.MX8MP SBCs" or similar?
>
> This was suggested by Krzysztof, since polyhex,imx8mp-debix was to
> generic. I can keep it without the deprecation notice and just change
> the comment since we need to keep dts compatible anyway.

I agree that using it as compatible for both SBC and SoMs, when the boards
aren't based on the SoM isn't useful. I still think it's useful to have
a compatible for "Debix i.MX8MP SBCs" that spans current lineup of Model A,
Model B, B SE and possible future compatibles.

Cheers,
Ahmad

>
> Regards,
> Marco
>

--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |


2023-07-17 18:03:32

by Marco Felsch

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: arm: fsl: fix DEBIX binding

On 23-07-17, Ahmad Fatoum wrote:
> On 17.07.23 18:51, 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. Mark
> > polyhex,imx8mp-debix as deprecated but keep it within the dts file since
> > we already have a user for it [1].
> >
> > [1] https://elixir.bootlin.com/barebox/v2023.07.1/source/arch/arm/ \
> > boards/polyhex-debix/board.c#L38
> >
> > Signed-off-by: Marco Felsch <[email protected]>
> > ---
> > Changelog:
> >
> > v2:
> > - deprecate polyhex,imx8mp-debix
> >
> > Documentation/devicetree/bindings/arm/fsl.yaml | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> > index 15d4110840654..b29974e3c30b3 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,14 @@ 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
> > + deprecated: true
>
> I don't see why you need to deprecate this. Can't you just change the comment
> to read "Polyhex i.MX8MP SBCs" or similar?

This was suggested by Krzysztof, since polyhex,imx8mp-debix was to
generic. I can keep it without the deprecation notice and just change
the comment since we need to keep dts compatible anyway.

Regards,
Marco

2023-07-17 20:13:41

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: arm: fsl: fix DEBIX binding

On 17/07/2023 19:14, Marco Felsch wrote:
> On 23-07-17, Krzysztof Kozlowski wrote:
>> On 17/07/2023 18:51, 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. Mark
>>> polyhex,imx8mp-debix as deprecated but keep it within the dts file since
>>> we already have a user for it [1].
>>>
>>> [1] https://elixir.bootlin.com/barebox/v2023.07.1/source/arch/arm/ \
>>> boards/polyhex-debix/board.c#L38
>>>
>>> Signed-off-by: Marco Felsch <[email protected]>
>>
>> Subject: fix is too generic and binding is redundant. You already state
>> this is binding in your prefix. Describe more precise what you are doing.
>>
>>> ---
>>> Changelog:
>>>
>>> v2:
>>> - deprecate polyhex,imx8mp-debix
>>>
>>> Documentation/devicetree/bindings/arm/fsl.yaml | 10 ++++++++--
>>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
>>> index 15d4110840654..b29974e3c30b3 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,14 @@ 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
>>> + deprecated: true
>>> + - const: fsl,imx8mp
>>
>> That's not how it works and it does not look like you tested the DTS

Actually this could pass the test because this is used by DTS. Quite
surprising.

>> against bindings. Please run `make dtbs_check` (see
>> Documentation/devicetree/bindings/writing-schema.rst or
>> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
>> for instructions).
>>
>> Don't deprecate some piece of entire compatible list, but entire list.
>>
>> The commit which deprecates compatible should bring a proper one.
>
> What did you mean by that?

Exactly what I wrote below:

>
> Regards,
> Marco
>
>> Otherwise at this point we have kernel only with deprecated compatibles.

You now have kernel without proper compatible, because it is deprecated.



Best regards,
Krzysztof


2023-07-17 20:13:43

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: arm: fsl: fix DEBIX binding

On 17/07/2023 19:12, Marco Felsch wrote:
> On 23-07-17, Krzysztof Kozlowski wrote:
>> On 17/07/2023 18:51, 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. Mark
>>> polyhex,imx8mp-debix as deprecated but keep it within the dts file since
>>> we already have a user for it [1].
>>>
>>> [1] https://elixir.bootlin.com/barebox/v2023.07.1/source/arch/arm/ \
>>> boards/polyhex-debix/board.c#L38

Don't wrap links, they are not clickable.

>>>
>>> Signed-off-by: Marco Felsch <[email protected]>
>>> ---
>>> Changelog:
>>>
>>> v2:
>>> - deprecate polyhex,imx8mp-debix
>>>
>>> Documentation/devicetree/bindings/arm/fsl.yaml | 10 ++++++++--
>>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
>>> index 15d4110840654..b29974e3c30b3 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,14 @@ 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
>>
>> I cannot find patches which add new compatible to the binding and which
>> fix the DTS. :/
>
> Please see my commit message, we can't remove the compatible since we
> already have one user of this compatible.


Indeed. I wonder then what is the goal of deprecating this compatible
and what is the plan for dealing with it? There is no cover letter which
would point me to it.

Best regards,
Krzysztof


2023-07-19 10:04:09

by Marco Felsch

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: arm: fsl: fix DEBIX binding

Hi Ahmad, Krzysztof,

On 23-07-17, Ahmad Fatoum wrote:
> On 17.07.23 19:24, Marco Felsch wrote:
> > On 23-07-17, Ahmad Fatoum wrote:
> >> On 17.07.23 18:51, 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. Mark
> >>> polyhex,imx8mp-debix as deprecated but keep it within the dts file since
> >>> we already have a user for it [1].
> >>>
> >>> [1] https://elixir.bootlin.com/barebox/v2023.07.1/source/arch/arm/ \
> >>> boards/polyhex-debix/board.c#L38
> >>>
> >>> Signed-off-by: Marco Felsch <[email protected]>
> >>> ---
> >>> Changelog:
> >>>
> >>> v2:
> >>> - deprecate polyhex,imx8mp-debix
> >>>
> >>> Documentation/devicetree/bindings/arm/fsl.yaml | 10 ++++++++--
> >>> 1 file changed, 8 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> >>> index 15d4110840654..b29974e3c30b3 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,14 @@ 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
> >>> + deprecated: true
> >>
> >> I don't see why you need to deprecate this. Can't you just change the comment
> >> to read "Polyhex i.MX8MP SBCs" or similar?
> >
> > This was suggested by Krzysztof, since polyhex,imx8mp-debix was to
> > generic. I can keep it without the deprecation notice and just change
> > the comment since we need to keep dts compatible anyway.
>
> I agree that using it as compatible for both SBC and SoMs, when the boards
> aren't based on the SoM isn't useful. I still think it's useful to have
> a compatible for "Debix i.MX8MP SBCs" that spans current lineup of Model A,
> Model B, B SE and possible future compatibles.

Thanks for the input. @Krzysztof how do we proceed on this topic? I can
adapt the comment and drop the deprecated status.

Regards,
Marco