| Message ID | 20210415101037.1465-13-alexandre.torgue@foss.st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: dts: stm32: fix "make dtbs_check W=1" round1 | expand |
Hi, On 15.04.21 12:10, Alexandre Torgue wrote: > Running "make dtbs_check W=1", some warnings are reported concerning > DSI. This patch reorder DSI nodes to avoid: > > soc/dsi@5a000000: unnecessary #address-cells/#size-cells without > "ranges" or child "reg" property This reverts parts of commit 9c32f980d9 ("ARM: dts: stm32: preset stm32mp15x video #address- and #size-cells"): The cell count for address and size is defined by the binding and not something a board would change. Avoid each board adding this boilerplate by having the cell size specification in the SoC DTSI. The DSI can have child nodes with a unit address (e.g. a panel) and ones without (ports { } container). ports is described in the dtsi, panels are described in the dts if available. Apparently, the checker is fine with ports { #address-cells = <1>; #size-cells = <0>; }; I think my rationale for the patch above was sound, so I think the checker taking offense at the DSI cells here should be considered a false positive. Thanks, Ahmad > > Signed-off-by: Alexandre Torgue <alexandre.torgue@foss.st.com> > > diff --git a/arch/arm/boot/dts/stm32mp157.dtsi b/arch/arm/boot/dts/stm32mp157.dtsi > index 54e73ccea446..c355fcf26ec3 100644 > --- a/arch/arm/boot/dts/stm32mp157.dtsi > +++ b/arch/arm/boot/dts/stm32mp157.dtsi > @@ -24,8 +24,6 @@ > clock-names = "pclk", "ref", "px_clk"; > resets = <&rcc DSI_R>; > reset-names = "apb"; > - #address-cells = <1>; > - #size-cells = <0>; > status = "disabled"; > > ports { > diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts b/arch/arm/boot/dts/stm32mp157c-dk2.dts > index 19ef475a48fc..763dde1dbbaf 100644 > --- a/arch/arm/boot/dts/stm32mp157c-dk2.dts > +++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts > @@ -36,6 +36,8 @@ > &dsi { > status = "okay"; > phy-dsi-supply = <®18>; > + #address-cells = <1>; > + #size-cells = <0>; > > ports { > port@0 { > diff --git a/arch/arm/boot/dts/stm32mp157c-ev1.dts b/arch/arm/boot/dts/stm32mp157c-ev1.dts > index 6fe5b0fee7c4..4625bb58cc6d 100644 > --- a/arch/arm/boot/dts/stm32mp157c-ev1.dts > +++ b/arch/arm/boot/dts/stm32mp157c-ev1.dts > @@ -102,6 +102,8 @@ > &dsi { > phy-dsi-supply = <®18>; > status = "okay"; > + #address-cells = <1>; > + #size-cells = <0>; > > ports { > port@0 { >
Hi Ahmad On 4/15/21 12:43 PM, Ahmad Fatoum wrote: > Hi, > > On 15.04.21 12:10, Alexandre Torgue wrote: >> Running "make dtbs_check W=1", some warnings are reported concerning >> DSI. This patch reorder DSI nodes to avoid: >> >> soc/dsi@5a000000: unnecessary #address-cells/#size-cells without >> "ranges" or child "reg" property > > This reverts parts of commit 9c32f980d9 ("ARM: dts: stm32: preset > stm32mp15x video #address- and #size-cells"): > > The cell count for address and size is defined by the binding and not > something a board would change. Avoid each board adding this > boilerplate by having the cell size specification in the SoC DTSI. > > > The DSI can have child nodes with a unit address (e.g. a panel) and ones > without (ports { } container). ports is described in the dtsi, panels are > described in the dts if available. > > Apparently, the checker is fine with > ports { > #address-cells = <1>; > #size-cells = <0>; > }; > > I think my rationale for the patch above was sound, so I think the checker > taking offense at the DSI cells here should be considered a false positive. If it's a "false positive" warning then we need to find a way to not print it out. Else, it'll be difficult to distinguish which warnings are "normal" and which are not. This question could also be applied to patch[3]. Arnd, Rob what is your feeling about this case ? thanks alex > Thanks, > Ahmad > >> >> Signed-off-by: Alexandre Torgue <alexandre.torgue@foss.st.com> >> >> diff --git a/arch/arm/boot/dts/stm32mp157.dtsi b/arch/arm/boot/dts/stm32mp157.dtsi >> index 54e73ccea446..c355fcf26ec3 100644 >> --- a/arch/arm/boot/dts/stm32mp157.dtsi >> +++ b/arch/arm/boot/dts/stm32mp157.dtsi >> @@ -24,8 +24,6 @@ >> clock-names = "pclk", "ref", "px_clk"; >> resets = <&rcc DSI_R>; >> reset-names = "apb"; >> - #address-cells = <1>; >> - #size-cells = <0>; >> status = "disabled"; >> >> ports { >> diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts b/arch/arm/boot/dts/stm32mp157c-dk2.dts >> index 19ef475a48fc..763dde1dbbaf 100644 >> --- a/arch/arm/boot/dts/stm32mp157c-dk2.dts >> +++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts >> @@ -36,6 +36,8 @@ >> &dsi { >> status = "okay"; >> phy-dsi-supply = <®18>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> >> ports { >> port@0 { >> diff --git a/arch/arm/boot/dts/stm32mp157c-ev1.dts b/arch/arm/boot/dts/stm32mp157c-ev1.dts >> index 6fe5b0fee7c4..4625bb58cc6d 100644 >> --- a/arch/arm/boot/dts/stm32mp157c-ev1.dts >> +++ b/arch/arm/boot/dts/stm32mp157c-ev1.dts >> @@ -102,6 +102,8 @@ >> &dsi { >> phy-dsi-supply = <®18>; >> status = "okay"; >> + #address-cells = <1>; >> + #size-cells = <0>; >> >> ports { >> port@0 { >> >
On Thu, Apr 15, 2021 at 2:23 PM Alexandre TORGUE <alexandre.torgue@foss.st.com> wrote: > On 4/15/21 12:43 PM, Ahmad Fatoum wrote: > > On 15.04.21 12:10, Alexandre Torgue wrote: > >> Running "make dtbs_check W=1", some warnings are reported concerning > >> DSI. This patch reorder DSI nodes to avoid: > >> > >> soc/dsi@5a000000: unnecessary #address-cells/#size-cells without > >> "ranges" or child "reg" property > > > > This reverts parts of commit 9c32f980d9 ("ARM: dts: stm32: preset > > stm32mp15x video #address- and #size-cells"): > > > > The cell count for address and size is defined by the binding and not > > something a board would change. Avoid each board adding this > > boilerplate by having the cell size specification in the SoC DTSI. > > > > > > The DSI can have child nodes with a unit address (e.g. a panel) and ones > > without (ports { } container). ports is described in the dtsi, panels are > > described in the dts if available. > > > > Apparently, the checker is fine with > > ports { > > #address-cells = <1>; > > #size-cells = <0>; > > }; > > > > I think my rationale for the patch above was sound, so I think the checker > > taking offense at the DSI cells here should be considered a false positive. > > If it's a "false positive" warning then we need to find a way to not > print it out. Else, it'll be difficult to distinguish which warnings are > "normal" and which are not. This question could also be applied to patch[3]. > > Arnd, Rob what is your feeling about this case ? I don't have a strong opinion on this either way, but I would just not apply this one for 5.13 in this case. Rob, Alexandre, please let me know if I should apply the other patches before the merge window, I usually don't mind taking bugfixes late before the merge window, but I still want some level of confidence that they are actually correct. Ahmad, if you feel strongly about this particular issue, would you like to suggest a patch for the checker? Arnd
On 4/19/21 3:57 PM, Arnd Bergmann wrote: > On Thu, Apr 15, 2021 at 2:23 PM Alexandre TORGUE > <alexandre.torgue@foss.st.com> wrote: >> On 4/15/21 12:43 PM, Ahmad Fatoum wrote: >>> On 15.04.21 12:10, Alexandre Torgue wrote: >>>> Running "make dtbs_check W=1", some warnings are reported concerning >>>> DSI. This patch reorder DSI nodes to avoid: >>>> >>>> soc/dsi@5a000000: unnecessary #address-cells/#size-cells without >>>> "ranges" or child "reg" property >>> >>> This reverts parts of commit 9c32f980d9 ("ARM: dts: stm32: preset >>> stm32mp15x video #address- and #size-cells"): >>> >>> The cell count for address and size is defined by the binding and not >>> something a board would change. Avoid each board adding this >>> boilerplate by having the cell size specification in the SoC DTSI. >>> >>> >>> The DSI can have child nodes with a unit address (e.g. a panel) and ones >>> without (ports { } container). ports is described in the dtsi, panels are >>> described in the dts if available. >>> >>> Apparently, the checker is fine with >>> ports { >>> #address-cells = <1>; >>> #size-cells = <0>; >>> }; >>> >>> I think my rationale for the patch above was sound, so I think the checker >>> taking offense at the DSI cells here should be considered a false positive. >> >> If it's a "false positive" warning then we need to find a way to not >> print it out. Else, it'll be difficult to distinguish which warnings are >> "normal" and which are not. This question could also be applied to patch[3]. >> >> Arnd, Rob what is your feeling about this case ? > > I don't have a strong opinion on this either way, but I would just > not apply this one for 5.13 in this case. Rob, Alexandre, please > let me know if I should apply the other patches before the > merge window, I usually don't mind taking bugfixes late before the > merge window, but I still want some level of confidence that they > are actually correct. For me, we can keep this series for the v5.14 cycle. regards alex > > Ahmad, if you feel strongly about this particular issue, would you like > to suggest a patch for the checker? > > Arnd >
Hello Arnd, On 19.04.21 15:57, Arnd Bergmann wrote: > On Thu, Apr 15, 2021 at 2:23 PM Alexandre TORGUE > <alexandre.torgue@foss.st.com> wrote: >> On 4/15/21 12:43 PM, Ahmad Fatoum wrote: >>> I think my rationale for the patch above was sound, so I think the checker >>> taking offense at the DSI cells here should be considered a false positive. >> >> If it's a "false positive" warning then we need to find a way to not >> print it out. Else, it'll be difficult to distinguish which warnings are >> "normal" and which are not. This question could also be applied to patch[3]. >> >> Arnd, Rob what is your feeling about this case ? > > I don't have a strong opinion on this either way, but I would just > not apply this one for 5.13 in this case. Rob, Alexandre, please > let me know if I should apply the other patches before the > merge window, I usually don't mind taking bugfixes late before the > merge window, but I still want some level of confidence that they > are actually correct. > > Ahmad, if you feel strongly about this particular issue, would you like > to suggest a patch for the checker? The check is certainly useful. If it's not feasible to fix the checker (e.g. because it analyzes standalone DTSIs), I am fine with reverting my commit with an indication that this is to avoid triggering a dt-validate false positive. I am not familiar with dt-validate's inner workings to offer a patch. Cheers, Ahmad > > Arnd >
diff --git a/arch/arm/boot/dts/stm32mp157.dtsi b/arch/arm/boot/dts/stm32mp157.dtsi index 54e73ccea446..c355fcf26ec3 100644 --- a/arch/arm/boot/dts/stm32mp157.dtsi +++ b/arch/arm/boot/dts/stm32mp157.dtsi @@ -24,8 +24,6 @@ clock-names = "pclk", "ref", "px_clk"; resets = <&rcc DSI_R>; reset-names = "apb"; - #address-cells = <1>; - #size-cells = <0>; status = "disabled"; ports { diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts b/arch/arm/boot/dts/stm32mp157c-dk2.dts index 19ef475a48fc..763dde1dbbaf 100644 --- a/arch/arm/boot/dts/stm32mp157c-dk2.dts +++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts @@ -36,6 +36,8 @@ &dsi { status = "okay"; phy-dsi-supply = <®18>; + #address-cells = <1>; + #size-cells = <0>; ports { port@0 { diff --git a/arch/arm/boot/dts/stm32mp157c-ev1.dts b/arch/arm/boot/dts/stm32mp157c-ev1.dts index 6fe5b0fee7c4..4625bb58cc6d 100644 --- a/arch/arm/boot/dts/stm32mp157c-ev1.dts +++ b/arch/arm/boot/dts/stm32mp157c-ev1.dts @@ -102,6 +102,8 @@ &dsi { phy-dsi-supply = <®18>; status = "okay"; + #address-cells = <1>; + #size-cells = <0>; ports { port@0 {
Running "make dtbs_check W=1", some warnings are reported concerning DSI. This patch reorder DSI nodes to avoid: soc/dsi@5a000000: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property Signed-off-by: Alexandre Torgue <alexandre.torgue@foss.st.com>