| Message ID | 20230717165127.2882535-2-m.felsch@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/4] arm64: dts: imx8mp-debix: remove unused fec pinctrl node | expand |
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 <m.felsch@pengutronix.de> >> >> 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
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 <m.felsch@pengutronix.de> >>> --- >>> 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
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 <m.felsch@pengutronix.de> > >>> --- > >>> 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
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:
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 <m.felsch@pengutronix.de> --- Changelog: v2: - deprecate polyhex,imx8mp-debix Documentation/devicetree/bindings/arm/fsl.yaml | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)