* [PATCH 0/4] arm64: dts: ti: Cleanup mix of "okay" and "disabled" usage @ 2020-11-04 22:43 Nishanth Menon 2020-11-04 22:43 ` [PATCH 1/4] arm64: dts: ti: k3-am65*: Cleanup disabled nodes at SoC dtsi level Nishanth Menon ` (3 more replies) 0 siblings, 4 replies; 12+ messages in thread From: Nishanth Menon @ 2020-11-04 22:43 UTC (permalink / raw) To: Roger Quadros, Keerthy, Jyri Sarha, Tomi Valkeinen, Peter Ujfalusi, Lokesh Vutla, Rob Herring, Tony Lindgren, Tero Kristo Cc: linux-kernel, devicetree, linux-arm-kernel, Nishanth Menon Hi, This is hopefully a conclusion of the thread we had (online[1] and offline). There are few options one could take when dealing with SoC dtsi and board dts: a. SoC dtsi provide nodes as a super-set default (aka enabled) state and to prevent messy board files, when more boards are added per SoC, we optimize and disable commonly un-used nodes in board-common.dtsi b. SoC dtsi disables all hardware dependent nodes by default and board dts files enable nodes based on a need basis. c. Subjectively pick and choose which nodes we will disable by default in SoC dtsi and over the years we can optimize things and change default state depending on the need. What we have today is a bit of a mix of seemingly random set of choices, however, predominantly following (a) and a few intermittent cases of (b) and (c). While there are pros and cons on each of these approaches, the right thing to do will be to stick with device tree default (aka device tree standards) and work within those established rules. So, lets cleanup and follow what the vast majority of SoC platforms are doing and which also happens to be the path of least churn for TI dts nodes as well. Functionally the dtb output is same -> a) As of v5.10-rc1: am654: https://pastebin.ubuntu.com/p/G4P5vghpV3/ j7200: https://pastebin.ubuntu.com/p/SsG6JtPzR9/ j721e: https://pastebin.ubuntu.com/p/HWXmTwD6m8/ b) with this series applied: am654: https://pastebin.ubuntu.com/p/h7MmHPQpRx/ j7200: https://pastebin.ubuntu.com/p/VXjQHhQNgn/ j721e: https://pastebin.ubuntu.com/p/2JMgftd4Xx/ The actual diff between the two versions being: https://pastebin.ubuntu.com/p/4rwy5qRY84/ Which is equivalent as per device tree standards, but uses lesser redundant strings. Thanks Tony, for sticking to the guns and providing us clear guidance on this topic. [1] https://lore.kernel.org/linux-arm-kernel/20201027130701.GE5639@atomide.com/ Nishanth Menon (4): arm64: dts: ti: k3-am65*: Cleanup disabled nodes at SoC dtsi level arm64: dts: ti: k3-j721e*: Cleanup disabled nodes at SoC dtsi level arm64: dts: ti: am65/j721e: Fix up un-necessary status set to "okay" for crypto arm64: dts: ti: k3-am654-base-board: Fix up un-necessary status set to "okay" for USB arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 9 ---- .../arm64/boot/dts/ti/k3-am654-base-board.dts | 24 ++++++---- .../dts/ti/k3-j721e-common-proc-board.dts | 48 ++++++++++++++++++- arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 28 ----------- 4 files changed, 63 insertions(+), 46 deletions(-) -- 2.29.2 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/4] arm64: dts: ti: k3-am65*: Cleanup disabled nodes at SoC dtsi level 2020-11-04 22:43 [PATCH 0/4] arm64: dts: ti: Cleanup mix of "okay" and "disabled" usage Nishanth Menon @ 2020-11-04 22:43 ` Nishanth Menon 2020-11-05 7:25 ` Tomi Valkeinen 2020-11-04 22:43 ` [PATCH 2/4] arm64: dts: ti: k3-j721e*: " Nishanth Menon ` (2 subsequent siblings) 3 siblings, 1 reply; 12+ messages in thread From: Nishanth Menon @ 2020-11-04 22:43 UTC (permalink / raw) To: Roger Quadros, Keerthy, Jyri Sarha, Tomi Valkeinen, Peter Ujfalusi, Lokesh Vutla, Rob Herring, Tony Lindgren, Tero Kristo Cc: linux-kernel, devicetree, linux-arm-kernel, Nishanth Menon The device tree standard sets the default node behavior when status property as enabled. There are many reasons for doing the same, number of strings in device tree, default power management functionality etc are few of the reasons. In general, after a few rounds of discussions [1] there are few options one could take when dealing with SoC dtsi and board dts a. SoC dtsi provide nodes as a super-set default (aka enabled) state and to prevent messy board files, when more boards are added per SoC, we optimize and disable commonly un-used nodes in board-common.dtsi b. SoC dtsi disables all hardware dependent nodes by default and board dts files enable nodes based on a need basis. c. Subjectively pick and choose which nodes we will disable by default in SoC dtsi and over the years we can optimize things and change default state depending on the need. While there are pros and cons on each of these approaches, the right thing to do will be to stick with device tree default standards and work within those established rules. So, we choose to go with option (a). Lets cleanup defaults of am654 SoC dtsi before this gets more harder to cleanup later on and new SoCs are added. The dtb generated is identical with the patch and it is just cleanup to ensure we have a clean usage model [1] https://lore.kernel.org/linux-arm-kernel/20201027130701.GE5639@atomide.com/ Fixes: 9bcb631e9953 ("arm64: dts: ti: k3-am654-main: Add McASP nodes") Fixes: fc539b90eda2 ("arm64: dts: ti: am654: Add DSS node") Cc: Jyri Sarha <jsarha@ti.com> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> Cc: Peter Ujfalusi <peter.ujfalusi@ti.com> Cc: Tony Lindgren <tony@atomide.com> Signed-off-by: Nishanth Menon <nm@ti.com> --- arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 8 -------- arch/arm64/boot/dts/ti/k3-am654-base-board.dts | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi index 533525229a8d..21e50021dd83 100644 --- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi +++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi @@ -770,8 +770,6 @@ mcasp0: mcasp@2b00000 { clocks = <&k3_clks 104 0>; clock-names = "fck"; power-domains = <&k3_pds 104 TI_SCI_PD_EXCLUSIVE>; - - status = "disabled"; }; mcasp1: mcasp@2b10000 { @@ -789,8 +787,6 @@ mcasp1: mcasp@2b10000 { clocks = <&k3_clks 105 0>; clock-names = "fck"; power-domains = <&k3_pds 105 TI_SCI_PD_EXCLUSIVE>; - - status = "disabled"; }; mcasp2: mcasp@2b20000 { @@ -808,8 +804,6 @@ mcasp2: mcasp@2b20000 { clocks = <&k3_clks 106 0>; clock-names = "fck"; power-domains = <&k3_pds 106 TI_SCI_PD_EXCLUSIVE>; - - status = "disabled"; }; cal: cal@6f03000 { @@ -865,8 +859,6 @@ dss: dss@04a00000 { interrupts = <GIC_SPI 166 IRQ_TYPE_EDGE_RISING>; - status = "disabled"; - dss_ports: ports { #address-cells = <1>; #size-cells = <0>; diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts index d12dd89f3405..199c4d4e7539 100644 --- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts +++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts @@ -486,3 +486,19 @@ &cpsw_port1 { phy-mode = "rgmii-rxid"; phy-handle = <&phy0>; }; + +&mcasp0 { + status = "disabled"; +}; + +&mcasp1 { + status = "disabled"; +}; + +&mcasp2 { + status = "disabled"; +}; + +&dss { + status = "disabled"; +}; -- 2.29.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] arm64: dts: ti: k3-am65*: Cleanup disabled nodes at SoC dtsi level 2020-11-04 22:43 ` [PATCH 1/4] arm64: dts: ti: k3-am65*: Cleanup disabled nodes at SoC dtsi level Nishanth Menon @ 2020-11-05 7:25 ` Tomi Valkeinen 0 siblings, 0 replies; 12+ messages in thread From: Tomi Valkeinen @ 2020-11-05 7:25 UTC (permalink / raw) To: Nishanth Menon, Roger Quadros, Keerthy, Jyri Sarha, Peter Ujfalusi, Lokesh Vutla, Rob Herring, Tony Lindgren, Tero Kristo Cc: linux-kernel, devicetree, linux-arm-kernel On 05/11/2020 00:43, Nishanth Menon wrote: > The device tree standard sets the default node behavior when status > property as enabled. There are many reasons for doing the same, number > of strings in device tree, default power management functionality etc > are few of the reasons. > > In general, after a few rounds of discussions [1] there are few > options one could take when dealing with SoC dtsi and board dts > > a. SoC dtsi provide nodes as a super-set default (aka enabled) state and > to prevent messy board files, when more boards are added per SoC, we > optimize and disable commonly un-used nodes in board-common.dtsi > b. SoC dtsi disables all hardware dependent nodes by default and board > dts files enable nodes based on a need basis. > c. Subjectively pick and choose which nodes we will disable by default > in SoC dtsi and over the years we can optimize things and change > default state depending on the need. > > While there are pros and cons on each of these approaches, the right > thing to do will be to stick with device tree default standards and > work within those established rules. So, we choose to go with option > (a). > > Lets cleanup defaults of am654 SoC dtsi before this gets more harder > to cleanup later on and new SoCs are added. > > The dtb generated is identical with the patch and it is just cleanup to > ensure we have a clean usage model > > [1] https://lore.kernel.org/linux-arm-kernel/20201027130701.GE5639@atomide.com/ > > Fixes: 9bcb631e9953 ("arm64: dts: ti: k3-am654-main: Add McASP nodes") > Fixes: fc539b90eda2 ("arm64: dts: ti: am654: Add DSS node") > Cc: Jyri Sarha <jsarha@ti.com> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> > Cc: Peter Ujfalusi <peter.ujfalusi@ti.com> > Cc: Tony Lindgren <tony@atomide.com> > Signed-off-by: Nishanth Menon <nm@ti.com> > --- > arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 8 -------- > arch/arm64/boot/dts/ti/k3-am654-base-board.dts | 16 ++++++++++++++++ > 2 files changed, 16 insertions(+), 8 deletions(-) Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com> Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/4] arm64: dts: ti: k3-j721e*: Cleanup disabled nodes at SoC dtsi level 2020-11-04 22:43 [PATCH 0/4] arm64: dts: ti: Cleanup mix of "okay" and "disabled" usage Nishanth Menon 2020-11-04 22:43 ` [PATCH 1/4] arm64: dts: ti: k3-am65*: Cleanup disabled nodes at SoC dtsi level Nishanth Menon @ 2020-11-04 22:43 ` Nishanth Menon 2020-11-05 7:25 ` Tomi Valkeinen 2020-11-05 7:32 ` Peter Ujfalusi 2020-11-04 22:43 ` [PATCH 3/4] arm64: dts: ti: am65/j721e: Fix up un-necessary status set to "okay" for crypto Nishanth Menon 2020-11-04 22:43 ` [PATCH 4/4] arm64: dts: ti: k3-am654-base-board: Fix up un-necessary status set to "okay" for USB Nishanth Menon 3 siblings, 2 replies; 12+ messages in thread From: Nishanth Menon @ 2020-11-04 22:43 UTC (permalink / raw) To: Roger Quadros, Keerthy, Jyri Sarha, Tomi Valkeinen, Peter Ujfalusi, Lokesh Vutla, Rob Herring, Tony Lindgren, Tero Kristo Cc: linux-kernel, devicetree, linux-arm-kernel, Nishanth Menon The device tree standard sets the default node behavior when status property as enabled. There are many reasons for doing the same, number of strings in device tree, default power management functionality etc are few of the reasons. In general, after a few rounds of discussions [1] there are few options one could take when dealing with SoC dtsi and board dts a. SoC dtsi provide nodes as a super-set default (aka enabled) state and to prevent messy board files, when more boards are added per SoC, we optimize and disable commonly un-used nodes in board-common.dtsi b. SoC dtsi disables all hardware dependent nodes by default and board dts files enable nodes based on a need basis. c. Subjectively pick and choose which nodes we will disable by default in SoC dtsi and over the years we can optimize things and change default state depending on the need. While there are pros and cons on each of these approaches, the right thing to do will be to stick with device tree default standards and work within those established rules. So, we choose to go with option (a). Lets cleanup defaults of j721e SoC dtsi before this gets more harder to cleanup later on and new SoCs are added. The only functional difference between the dtb generated is status='okay' is no longer necessary for mcasp10 and depends on the default state. [1] https://lore.kernel.org/linux-arm-kernel/20201027130701.GE5639@atomide.com/ Fixes: 1c4d35265fb2 ("arm64: dts: ti: k3-j721e-main: Add McASP nodes") Fixes: 76921f15acc0 ("arm64: dts: ti: k3-j721e-main: Add DSS node") Cc: Jyri Sarha <jsarha@ti.com> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> Cc: Peter Ujfalusi <peter.ujfalusi@ti.com> Cc: Tony Lindgren <tony@atomide.com> Signed-off-by: Nishanth Menon <nm@ti.com> --- .../dts/ti/k3-j721e-common-proc-board.dts | 48 ++++++++++++++++++- arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 26 ---------- 2 files changed, 47 insertions(+), 27 deletions(-) diff --git a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts index 52e121155563..9416528caa8a 100644 --- a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts +++ b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts @@ -540,6 +540,46 @@ &dss { <&k3_clks 152 18>; /* PLL23_HSDIV0 */ }; +&mcasp0 { + status = "disabled"; +}; + +&mcasp1 { + status = "disabled"; +}; + +&mcasp2 { + status = "disabled"; +}; + +&mcasp3 { + status = "disabled"; +}; + +&mcasp4 { + status = "disabled"; +}; + +&mcasp5 { + status = "disabled"; +}; + +&mcasp6 { + status = "disabled"; +}; + +&mcasp7 { + status = "disabled"; +}; + +&mcasp8 { + status = "disabled"; +}; + +&mcasp9 { + status = "disabled"; +}; + &mcasp10 { #sound-dai-cells = <0>; @@ -556,8 +596,10 @@ &mcasp10 { >; tx-num-evt = <0>; rx-num-evt = <0>; +}; - status = "okay"; +&mcasp11 { + status = "disabled"; }; &serdes0 { @@ -639,3 +681,7 @@ &pcie3_rc { &pcie3_ep { status = "disabled"; }; + +&dss { + status = "disabled"; +}; diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi index e2a96b2c423c..b54332d6fdc5 100644 --- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi +++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi @@ -1327,8 +1327,6 @@ dss: dss@04a00000 { "common_s1", "common_s2"; - status = "disabled"; - dss_ports: ports { #address-cells = <1>; #size-cells = <0>; @@ -1350,8 +1348,6 @@ mcasp0: mcasp@2b00000 { clocks = <&k3_clks 174 1>; clock-names = "fck"; power-domains = <&k3_pds 174 TI_SCI_PD_EXCLUSIVE>; - - status = "disabled"; }; mcasp1: mcasp@2b10000 { @@ -1369,8 +1365,6 @@ mcasp1: mcasp@2b10000 { clocks = <&k3_clks 175 1>; clock-names = "fck"; power-domains = <&k3_pds 175 TI_SCI_PD_EXCLUSIVE>; - - status = "disabled"; }; mcasp2: mcasp@2b20000 { @@ -1388,8 +1382,6 @@ mcasp2: mcasp@2b20000 { clocks = <&k3_clks 176 1>; clock-names = "fck"; power-domains = <&k3_pds 176 TI_SCI_PD_EXCLUSIVE>; - - status = "disabled"; }; mcasp3: mcasp@2b30000 { @@ -1407,8 +1399,6 @@ mcasp3: mcasp@2b30000 { clocks = <&k3_clks 177 1>; clock-names = "fck"; power-domains = <&k3_pds 177 TI_SCI_PD_EXCLUSIVE>; - - status = "disabled"; }; mcasp4: mcasp@2b40000 { @@ -1426,8 +1416,6 @@ mcasp4: mcasp@2b40000 { clocks = <&k3_clks 178 1>; clock-names = "fck"; power-domains = <&k3_pds 178 TI_SCI_PD_EXCLUSIVE>; - - status = "disabled"; }; mcasp5: mcasp@2b50000 { @@ -1445,8 +1433,6 @@ mcasp5: mcasp@2b50000 { clocks = <&k3_clks 179 1>; clock-names = "fck"; power-domains = <&k3_pds 179 TI_SCI_PD_EXCLUSIVE>; - - status = "disabled"; }; mcasp6: mcasp@2b60000 { @@ -1464,8 +1450,6 @@ mcasp6: mcasp@2b60000 { clocks = <&k3_clks 180 1>; clock-names = "fck"; power-domains = <&k3_pds 180 TI_SCI_PD_EXCLUSIVE>; - - status = "disabled"; }; mcasp7: mcasp@2b70000 { @@ -1483,8 +1467,6 @@ mcasp7: mcasp@2b70000 { clocks = <&k3_clks 181 1>; clock-names = "fck"; power-domains = <&k3_pds 181 TI_SCI_PD_EXCLUSIVE>; - - status = "disabled"; }; mcasp8: mcasp@2b80000 { @@ -1502,8 +1484,6 @@ mcasp8: mcasp@2b80000 { clocks = <&k3_clks 182 1>; clock-names = "fck"; power-domains = <&k3_pds 182 TI_SCI_PD_EXCLUSIVE>; - - status = "disabled"; }; mcasp9: mcasp@2b90000 { @@ -1521,8 +1501,6 @@ mcasp9: mcasp@2b90000 { clocks = <&k3_clks 183 1>; clock-names = "fck"; power-domains = <&k3_pds 183 TI_SCI_PD_EXCLUSIVE>; - - status = "disabled"; }; mcasp10: mcasp@2ba0000 { @@ -1540,8 +1518,6 @@ mcasp10: mcasp@2ba0000 { clocks = <&k3_clks 184 1>; clock-names = "fck"; power-domains = <&k3_pds 184 TI_SCI_PD_EXCLUSIVE>; - - status = "disabled"; }; mcasp11: mcasp@2bb0000 { @@ -1559,8 +1535,6 @@ mcasp11: mcasp@2bb0000 { clocks = <&k3_clks 185 1>; clock-names = "fck"; power-domains = <&k3_pds 185 TI_SCI_PD_EXCLUSIVE>; - - status = "disabled"; }; watchdog0: watchdog@2200000 { -- 2.29.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] arm64: dts: ti: k3-j721e*: Cleanup disabled nodes at SoC dtsi level 2020-11-04 22:43 ` [PATCH 2/4] arm64: dts: ti: k3-j721e*: " Nishanth Menon @ 2020-11-05 7:25 ` Tomi Valkeinen 2020-11-05 7:32 ` Peter Ujfalusi 1 sibling, 0 replies; 12+ messages in thread From: Tomi Valkeinen @ 2020-11-05 7:25 UTC (permalink / raw) To: Nishanth Menon, Roger Quadros, Keerthy, Jyri Sarha, Peter Ujfalusi, Lokesh Vutla, Rob Herring, Tony Lindgren, Tero Kristo Cc: linux-kernel, devicetree, linux-arm-kernel On 05/11/2020 00:43, Nishanth Menon wrote: > The device tree standard sets the default node behavior when status > property as enabled. There are many reasons for doing the same, number > of strings in device tree, default power management functionality etc > are few of the reasons. > > In general, after a few rounds of discussions [1] there are few > options one could take when dealing with SoC dtsi and board dts > > a. SoC dtsi provide nodes as a super-set default (aka enabled) state and > to prevent messy board files, when more boards are added per SoC, we > optimize and disable commonly un-used nodes in board-common.dtsi > b. SoC dtsi disables all hardware dependent nodes by default and board > dts files enable nodes based on a need basis. > c. Subjectively pick and choose which nodes we will disable by default > in SoC dtsi and over the years we can optimize things and change > default state depending on the need. > > While there are pros and cons on each of these approaches, the right > thing to do will be to stick with device tree default standards and > work within those established rules. So, we choose to go with option > (a). > > Lets cleanup defaults of j721e SoC dtsi before this gets more harder > to cleanup later on and new SoCs are added. > > The only functional difference between the dtb generated is > status='okay' is no longer necessary for mcasp10 and depends on the > default state. > > [1] https://lore.kernel.org/linux-arm-kernel/20201027130701.GE5639@atomide.com/ > > Fixes: 1c4d35265fb2 ("arm64: dts: ti: k3-j721e-main: Add McASP nodes") > Fixes: 76921f15acc0 ("arm64: dts: ti: k3-j721e-main: Add DSS node") > Cc: Jyri Sarha <jsarha@ti.com> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> > Cc: Peter Ujfalusi <peter.ujfalusi@ti.com> > Cc: Tony Lindgren <tony@atomide.com> > Signed-off-by: Nishanth Menon <nm@ti.com> > --- > .../dts/ti/k3-j721e-common-proc-board.dts | 48 ++++++++++++++++++- > arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 26 ---------- > 2 files changed, 47 insertions(+), 27 deletions(-) Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com> Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] arm64: dts: ti: k3-j721e*: Cleanup disabled nodes at SoC dtsi level 2020-11-04 22:43 ` [PATCH 2/4] arm64: dts: ti: k3-j721e*: " Nishanth Menon 2020-11-05 7:25 ` Tomi Valkeinen @ 2020-11-05 7:32 ` Peter Ujfalusi 2020-11-05 14:08 ` Nishanth Menon 1 sibling, 1 reply; 12+ messages in thread From: Peter Ujfalusi @ 2020-11-05 7:32 UTC (permalink / raw) To: Nishanth Menon, Roger Quadros, Keerthy, Jyri Sarha, Tomi Valkeinen, Lokesh Vutla, Rob Herring, Tony Lindgren, Tero Kristo Cc: linux-kernel, devicetree, linux-arm-kernel Nishanth, On 05/11/2020 0.43, Nishanth Menon wrote: > The device tree standard sets the default node behavior when status > property as enabled. It should be: When the status property is not present under a node, the "okay' value is assumed. Note: the device tree specification does not document default value as such, see v0.3 (2.3.4, page 14). Yes, the "okay" is used in case the status property is missing (by Linux at least). > There are many reasons for doing the same, number > of strings in device tree, with expense of loc and readability. > default power management functionality etc Right, so how does that helps with devices present in the SoC, but no node at all? First thing which comes to mind is AASRC, we don't have Linux driver for it (and no DT binding document), but that does not mean that it is not present. How PM would take that into account? > are few of the reasons. > > In general, after a few rounds of discussions [1] there are few > options one could take when dealing with SoC dtsi and board dts > > a. SoC dtsi provide nodes as a super-set default (aka enabled) state and > to prevent messy board files, when more boards are added per SoC, we > optimize and disable commonly un-used nodes in board-common.dtsi > b. SoC dtsi disables all hardware dependent nodes by default and board > dts files enable nodes based on a need basis. > c. Subjectively pick and choose which nodes we will disable by default > in SoC dtsi and over the years we can optimize things and change > default state depending on the need. For the record: c was not really an option. There were no subjectivity, the reason was pragmatic. We are all familiar with the Devicetree specification, but let me quote from chapter 2.3.4: "okay" Indicates the device is operational. "disabled" Indicates that the device is not presently operational, but it might become operational in the future (for example, something is not plugged in, or switched off). Refer to the device binding for details on what disabled means for a given device. The reason why we kept McASP nodes (and dss) disabled in the soc dtsi file is that they are not operation in the form they present in there. They _need_ additional properties to be operational and those properties can only be added in the board dts file. This is not remotely a subjective view, this is the opposite of subjectivity. As for things not owned by the OS we have the "reserved" status. > While there are pros and cons on each of these approaches, the right > thing to do will be to stick with device tree default standards and > work within those established rules. So, we choose to go with option > (a). > > Lets cleanup defaults of j721e SoC dtsi before this gets more harder > to cleanup later on and new SoCs are added. > > The only functional difference between the dtb generated is > status='okay' is no longer necessary for mcasp10 and depends on the > default state. > > [1] https://lore.kernel.org/linux-arm-kernel/20201027130701.GE5639@atomide.com/ > > Fixes: 1c4d35265fb2 ("arm64: dts: ti: k3-j721e-main: Add McASP nodes") > Fixes: 76921f15acc0 ("arm64: dts: ti: k3-j721e-main: Add DSS node") > Cc: Jyri Sarha <jsarha@ti.com> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> > Cc: Peter Ujfalusi <peter.ujfalusi@ti.com> > Cc: Tony Lindgren <tony@atomide.com> > Signed-off-by: Nishanth Menon <nm@ti.com> > --- > .../dts/ti/k3-j721e-common-proc-board.dts | 48 ++++++++++++++++++- > arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 26 ---------- > 2 files changed, 47 insertions(+), 27 deletions(-) > > diff --git a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts > index 52e121155563..9416528caa8a 100644 > --- a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts > +++ b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts > @@ -540,6 +540,46 @@ &dss { > <&k3_clks 152 18>; /* PLL23_HSDIV0 */ > }; > > +&mcasp0 { > + status = "disabled"; > +}; > + > +&mcasp1 { > + status = "disabled"; > +}; > + > +&mcasp2 { > + status = "disabled"; > +}; > + > +&mcasp3 { > + status = "disabled"; > +}; > + > +&mcasp4 { > + status = "disabled"; > +}; > + > +&mcasp5 { > + status = "disabled"; > +}; > + > +&mcasp6 { > + status = "disabled"; > +}; > + > +&mcasp7 { > + status = "disabled"; > +}; > + > +&mcasp8 { > + status = "disabled"; > +}; > + > +&mcasp9 { > + status = "disabled"; > +}; > + > &mcasp10 { > #sound-dai-cells = <0>; > > @@ -556,8 +596,10 @@ &mcasp10 { > >; > tx-num-evt = <0>; > rx-num-evt = <0>; > +}; > > - status = "okay"; > +&mcasp11 { > + status = "disabled"; > }; Looks much better in this way. ? I always wondered what is _not_ used by the board... But it is not really about that, we need to disable these nodes as they are incomplete in dtsi, they are not operational... > &serdes0 { > @@ -639,3 +681,7 @@ &pcie3_rc { > &pcie3_ep { > status = "disabled"; > }; > + > +&dss { > + status = "disabled"; > +}; > diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi > index e2a96b2c423c..b54332d6fdc5 100644 > --- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi > +++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi > @@ -1327,8 +1327,6 @@ dss: dss@04a00000 { > "common_s1", > "common_s2"; > > - status = "disabled"; > - > dss_ports: ports { > #address-cells = <1>; > #size-cells = <0>; > @@ -1350,8 +1348,6 @@ mcasp0: mcasp@2b00000 { > clocks = <&k3_clks 174 1>; > clock-names = "fck"; > power-domains = <&k3_pds 174 TI_SCI_PD_EXCLUSIVE>; > - > - status = "disabled"; > }; > > mcasp1: mcasp@2b10000 { > @@ -1369,8 +1365,6 @@ mcasp1: mcasp@2b10000 { > clocks = <&k3_clks 175 1>; > clock-names = "fck"; > power-domains = <&k3_pds 175 TI_SCI_PD_EXCLUSIVE>; > - > - status = "disabled"; > }; > > mcasp2: mcasp@2b20000 { > @@ -1388,8 +1382,6 @@ mcasp2: mcasp@2b20000 { > clocks = <&k3_clks 176 1>; > clock-names = "fck"; > power-domains = <&k3_pds 176 TI_SCI_PD_EXCLUSIVE>; > - > - status = "disabled"; > }; > > mcasp3: mcasp@2b30000 { > @@ -1407,8 +1399,6 @@ mcasp3: mcasp@2b30000 { > clocks = <&k3_clks 177 1>; > clock-names = "fck"; > power-domains = <&k3_pds 177 TI_SCI_PD_EXCLUSIVE>; > - > - status = "disabled"; > }; > > mcasp4: mcasp@2b40000 { > @@ -1426,8 +1416,6 @@ mcasp4: mcasp@2b40000 { > clocks = <&k3_clks 178 1>; > clock-names = "fck"; > power-domains = <&k3_pds 178 TI_SCI_PD_EXCLUSIVE>; > - > - status = "disabled"; > }; > > mcasp5: mcasp@2b50000 { > @@ -1445,8 +1433,6 @@ mcasp5: mcasp@2b50000 { > clocks = <&k3_clks 179 1>; > clock-names = "fck"; > power-domains = <&k3_pds 179 TI_SCI_PD_EXCLUSIVE>; > - > - status = "disabled"; > }; > > mcasp6: mcasp@2b60000 { > @@ -1464,8 +1450,6 @@ mcasp6: mcasp@2b60000 { > clocks = <&k3_clks 180 1>; > clock-names = "fck"; > power-domains = <&k3_pds 180 TI_SCI_PD_EXCLUSIVE>; > - > - status = "disabled"; > }; > > mcasp7: mcasp@2b70000 { > @@ -1483,8 +1467,6 @@ mcasp7: mcasp@2b70000 { > clocks = <&k3_clks 181 1>; > clock-names = "fck"; > power-domains = <&k3_pds 181 TI_SCI_PD_EXCLUSIVE>; > - > - status = "disabled"; > }; > > mcasp8: mcasp@2b80000 { > @@ -1502,8 +1484,6 @@ mcasp8: mcasp@2b80000 { > clocks = <&k3_clks 182 1>; > clock-names = "fck"; > power-domains = <&k3_pds 182 TI_SCI_PD_EXCLUSIVE>; > - > - status = "disabled"; > }; > > mcasp9: mcasp@2b90000 { > @@ -1521,8 +1501,6 @@ mcasp9: mcasp@2b90000 { > clocks = <&k3_clks 183 1>; > clock-names = "fck"; > power-domains = <&k3_pds 183 TI_SCI_PD_EXCLUSIVE>; > - > - status = "disabled"; > }; > > mcasp10: mcasp@2ba0000 { > @@ -1540,8 +1518,6 @@ mcasp10: mcasp@2ba0000 { > clocks = <&k3_clks 184 1>; > clock-names = "fck"; > power-domains = <&k3_pds 184 TI_SCI_PD_EXCLUSIVE>; > - > - status = "disabled"; > }; > > mcasp11: mcasp@2bb0000 { > @@ -1559,8 +1535,6 @@ mcasp11: mcasp@2bb0000 { > clocks = <&k3_clks 185 1>; > clock-names = "fck"; > power-domains = <&k3_pds 185 TI_SCI_PD_EXCLUSIVE>; > - > - status = "disabled"; > }; > > watchdog0: watchdog@2200000 { > There is no such a tag, but: whatever-by: Peter Ujfalusi <peter.ujfalusi@ti.com> - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] arm64: dts: ti: k3-j721e*: Cleanup disabled nodes at SoC dtsi level 2020-11-05 7:32 ` Peter Ujfalusi @ 2020-11-05 14:08 ` Nishanth Menon 2020-11-06 11:32 ` Peter Ujfalusi 0 siblings, 1 reply; 12+ messages in thread From: Nishanth Menon @ 2020-11-05 14:08 UTC (permalink / raw) To: Peter Ujfalusi Cc: Roger Quadros, Keerthy, Jyri Sarha, Tomi Valkeinen, Lokesh Vutla, Rob Herring, Tony Lindgren, Tero Kristo, linux-kernel, devicetree, linux-arm-kernel On 09:32-20201105, Peter Ujfalusi wrote: > Nishanth, > > On 05/11/2020 0.43, Nishanth Menon wrote: > > The device tree standard sets the default node behavior when status > > property as enabled. > > It should be: > When the status property is not present under a node, the "okay' value > is assumed. Thanks.. will update. > > Note: the device tree specification does not document default value as > such, see v0.3 (2.3.4, page 14). > Yes, the "okay" is used in case the status property is missing (by Linux > at least). Maybe the spec update needs a formal release? Kumar's patch is merged: https://github.com/devicetree-org/devicetree-specification/pull/33 on that exact same section, which you can see https://github.com/devicetree-org/devicetree-specification/blob/master/source/chapter2-devicetree-basics.rst Brings it to sync to: https://elinux.org/Device_Tree_Linux#status_property > > > There are many reasons for doing the same, number > > of strings in device tree, > > with expense of loc and readability. The "readability" part is subjective a bit.. enabled and disabled both have verbosity problem lets see how we can optimize as new boards come in. > > > default power management functionality etc > > Right, so how does that helps with devices present in the SoC, but no > node at all? First thing which comes to mind is AASRC, we don't have > Linux driver for it (and no DT binding document), but that does not mean > that it is not present. How PM would take that into account? I think we are mixing topics here -> I was stating the motivation why devicetree chose such as default. Do we have a suggestion to improve the description in the commit? > > > are few of the reasons. > > > > In general, after a few rounds of discussions [1] there are few > > options one could take when dealing with SoC dtsi and board dts > > > > a. SoC dtsi provide nodes as a super-set default (aka enabled) state and > > to prevent messy board files, when more boards are added per SoC, we > > optimize and disable commonly un-used nodes in board-common.dtsi > > b. SoC dtsi disables all hardware dependent nodes by default and board > > dts files enable nodes based on a need basis. > > c. Subjectively pick and choose which nodes we will disable by default > > in SoC dtsi and over the years we can optimize things and change > > default state depending on the need. > > For the record: c was not really an option. There were no subjectivity, > the reason was pragmatic. (c) some examples where we did pick that option (fixes): https://lore.kernel.org/linux-arm-kernel/20201104224356.18040-4-nm@ti.com/ https://lore.kernel.org/linux-arm-kernel/20201104224356.18040-5-nm@ti.com/ > > The reason why we kept McASP nodes (and dss) disabled in the soc dtsi > file is that they are not operation in the form they present in there. > They _need_ additional properties to be operational and those properties > can only be added in the board dts file. I dont think we are changing anything in the output dtb files, we are just leaving the defaults as dt defaults and set the disable state in board dts OR common board dtsi. > > This is not remotely a subjective view, this is the opposite of > subjectivity. the usage of McASP was'nt meant as (c).. it is (b). is there a better way to describe this in a generic manner? > > As for things not owned by the OS we have the "reserved" status. Which is correct usage. I think your point with wkup_uart should be set as reserved? I might have missed doing that - am I correct? [...] > > > > - status = "okay"; > > +&mcasp11 { > > + status = "disabled"; > > }; > > Looks much better in this way. > ? > > I always wondered what is _not_ used by the board... > But it is not really about that, we need to disable these nodes as they > are incomplete in dtsi, they are not operational... Alright - what do we suggest we do? Tony, Rob - I need some guidance here. > > > &serdes0 { [...] > > > > watchdog0: watchdog@2200000 { > > > > There is no such a tag, but: > whatever-by: Peter Ujfalusi <peter.ujfalusi@ti.com> OK - I have no idea how B4 or patchworks pick that one as :D -- Regards, Nishanth Menon Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] arm64: dts: ti: k3-j721e*: Cleanup disabled nodes at SoC dtsi level 2020-11-05 14:08 ` Nishanth Menon @ 2020-11-06 11:32 ` Peter Ujfalusi 2020-11-06 21:46 ` Nishanth Menon 0 siblings, 1 reply; 12+ messages in thread From: Peter Ujfalusi @ 2020-11-06 11:32 UTC (permalink / raw) To: Nishanth Menon Cc: Roger Quadros, Keerthy, Jyri Sarha, Tomi Valkeinen, Lokesh Vutla, Rob Herring, Tony Lindgren, Tero Kristo, linux-kernel, devicetree, linux-arm-kernel Nishanth, On 05/11/2020 16.08, Nishanth Menon wrote: > On 09:32-20201105, Peter Ujfalusi wrote: >> Nishanth, >> >> On 05/11/2020 0.43, Nishanth Menon wrote: >>> The device tree standard sets the default node behavior when status >>> property as enabled. >> >> It should be: >> When the status property is not present under a node, the "okay' value >> is assumed. > > Thanks.. will update. > >> >> Note: the device tree specification does not document default value as >> such, see v0.3 (2.3.4, page 14). >> Yes, the "okay" is used in case the status property is missing (by Linux >> at least). > > Maybe the spec update needs a formal release? Kumar's patch is merged: > https://github.com/devicetree-org/devicetree-specification/pull/33 > > on that exact same section, which you can see > https://github.com/devicetree-org/devicetree-specification/blob/master/source/chapter2-devicetree-basics.rst I stand correct, I only checked the released version. > Brings it to sync to: > https://elinux.org/Device_Tree_Linux#status_property > >> >>> There are many reasons for doing the same, number >>> of strings in device tree, >> >> with expense of loc and readability. > > The "readability" part is subjective a bit.. enabled and disabled both > have verbosity problem lets see how we can optimize as new boards come > in. I agree. > >> >>> default power management functionality etc >> >> Right, so how does that helps with devices present in the SoC, but no >> node at all? First thing which comes to mind is AASRC, we don't have >> Linux driver for it (and no DT binding document), but that does not mean >> that it is not present. How PM would take that into account? > > I think we are mixing topics here -> I was stating the motivation why > devicetree chose such as default. I don't question the fact that 'okay' is the default status if it is not explicitly present. There is no better default than that. > Do we have a suggestion to improve > the description in the commit? A bit later on that. >> >>> are few of the reasons. >>> >>> In general, after a few rounds of discussions [1] there are few >>> options one could take when dealing with SoC dtsi and board dts >>> >>> a. SoC dtsi provide nodes as a super-set default (aka enabled) state and >>> to prevent messy board files, when more boards are added per SoC, we >>> optimize and disable commonly un-used nodes in board-common.dtsi >>> b. SoC dtsi disables all hardware dependent nodes by default and board >>> dts files enable nodes based on a need basis. >>> c. Subjectively pick and choose which nodes we will disable by default >>> in SoC dtsi and over the years we can optimize things and change >>> default state depending on the need. >> >> For the record: c was not really an option. There were no subjectivity, >> the reason was pragmatic. > > > (c) some examples where we did pick that option (fixes): > https://lore.kernel.org/linux-arm-kernel/20201104224356.18040-4-nm@ti.com/ > https://lore.kernel.org/linux-arm-kernel/20201104224356.18040-5-nm@ti.com/ this is different, these patches just removing the "status = 'okay';" lines where they are not needed and can be omitted to save few lines and it does help on readablity. >> The reason why we kept McASP nodes (and dss) disabled in the soc dtsi >> file is that they are not operation in the form they present in there. >> They _need_ additional properties to be operational and those properties >> can only be added in the board dts file. > > I dont think we are changing anything in the output dtb files, Correct, the resulted dtb is identical. If the developer for upcoming boards did check the schematics vs TRM vs dtsi and spot the things that is not configured. dtb check will complain when it is starting to check against the documentation, but McASP is not yet converted to yaml and to be honest I don't want to convert the current binding to be the binding. When it was done it just moved pdata variables to DT and that was wrong. This is off-topic a bit. > we are > just leaving the defaults as dt defaults and set the disable state in > board dts OR common board dtsi. Yes, we leave the non working/configured node 'okay' in dtsi and expect that the board file author will know which node must be disabled because it is incomplete. >> This is not remotely a subjective view, this is the opposite of >> subjectivity. > > the usage of McASP was'nt meant as (c).. it is (b). is there a better way > to describe this in a generic manner? I had my saying on that ever since I have been taking care of audio on TI SoCs ;) I used similar analogy in a private thread around this, but imho it fits the case neatly: car == McASP you don't put an 'okay' (as is ready, operational) stamp on the car in the middle of the production line when the engine is not even installed. >> As for things not owned by the OS we have the "reserved" status. > Which is correct usage. I think your point with wkup_uart should be set as > reserved? I might have missed doing that - am I correct? > > [...] >>> >>> - status = "okay"; >>> +&mcasp11 { >>> + status = "disabled"; >>> }; >> >> Looks much better in this way. >> ? >> >> I always wondered what is _not_ used by the board... >> But it is not really about that, we need to disable these nodes as they >> are incomplete in dtsi, they are not operational... > > Alright - what do we suggest we do? Not sure, I'm 'whatever' after [1] makes it to mainline or next. > Tony, Rob - I need some guidance here. I'm fine whatever way we take, but I think it is up to you to make the call as the maintainer of the TI dts files... ;) >> >>> &serdes0 { > [...] >>> >>> watchdog0: watchdog@2200000 { >>> >> >> There is no such a tag, but: >> whatever-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > > OK - I have no idea how B4 or patchworks pick that one as :D If we take this road, than I'm okay with it, but I'm going to take silent protest (not sending acked-by or revired-by). That should not stop you doing what you believe is best for the future! fwiw, McASP will have sane handling for the variations of 'okay': [1] https://lore.kernel.org/alsa-devel/20201106072551.689-1-peter.ujfalusi@ti.com/ - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] arm64: dts: ti: k3-j721e*: Cleanup disabled nodes at SoC dtsi level 2020-11-06 11:32 ` Peter Ujfalusi @ 2020-11-06 21:46 ` Nishanth Menon 2020-11-09 7:49 ` Peter Ujfalusi 0 siblings, 1 reply; 12+ messages in thread From: Nishanth Menon @ 2020-11-06 21:46 UTC (permalink / raw) To: Peter Ujfalusi Cc: Roger Quadros, Keerthy, Jyri Sarha, Tomi Valkeinen, Lokesh Vutla, Rob Herring, Tony Lindgren, Tero Kristo, linux-kernel, devicetree, linux-arm-kernel On 13:32-20201106, Peter Ujfalusi wrote: [...] > > > >> > >>> default power management functionality etc > >> > >> Right, so how does that helps with devices present in the SoC, but no > >> node at all? First thing which comes to mind is AASRC, we don't have > >> Linux driver for it (and no DT binding document), but that does not mean > >> that it is not present. How PM would take that into account? > > > > I think we are mixing topics here -> I was stating the motivation why > > devicetree chose such as default. > > I don't question the fact that 'okay' is the default status if it is not > explicitly present. There is no better default than that. ^^ -> Alright, that is all we are trying to do here: defaults in the SoC.dtsi and specific cleanups (firmware reserved / board unused disables) be done in a common board.dtsi (for now, there is no such specific need, I guess). [..] > > >> The reason why we kept McASP nodes (and dss) disabled in the soc dtsi > >> file is that they are not operation in the form they present in there. > >> They _need_ additional properties to be operational and those properties > >> can only be added in the board dts file. > > > > I dont think we are changing anything in the output dtb files, > > Correct, the resulted dtb is identical. If the developer for upcoming > boards did check the schematics vs TRM vs dtsi and spot the things that > is not configured. Yes. > > > we are > > just leaving the defaults as dt defaults and set the disable state in > > board dts OR common board dtsi. > > Yes, we leave the non working/configured node 'okay' in dtsi and expect > that the board file author will know which node must be disabled because > it is incomplete. Yes - I understand(and empathise) the implicit omission error risk we are incurring here. I will add that to the commit message as well. > >> This is not remotely a subjective view, this is the opposite of > >> subjectivity. > > > > the usage of McASP was'nt meant as (c).. it is (b). is there a better way > > to describe this in a generic manner? > > I had my saying on that ever since I have been taking care of audio on > TI SoCs ;) > > I used similar analogy in a private thread around this, but imho it fits > the case neatly: > car == McASP > > you don't put an 'okay' (as is ready, operational) stamp on the car in > the middle of the production line when the engine is not even installed. Completely agree with you. we are just insisting that this be done in either common board.dtsi OR board.dtsi where applicable. > [..] > > Alright - what do we suggest we do? > > Not sure, I'm 'whatever' after [1] makes it to mainline or next. [....] > [1] > https://lore.kernel.org/alsa-devel/20201106072551.689-1-peter.ujfalusi@ti.com/ I don't see the relationship between the series.. I think this series brings no change in dtb, hence with OR without your driver cleanup series, there is no practical regressions. > > > Tony, Rob - I need some guidance here. > > I'm fine whatever way we take, but I think it is up to you to make the > call as the maintainer of the TI dts files... ;) Yep - I have'nt seen a reason yet that must cause us to change from the Device tree default approach in our debates. As Tony already pointed out.. if we start seeing a lot more boards for an SoC.. Instead of (reverse issue- where we have a lot of places where people are doing "okay" problem): https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=12afc0cf81210969756daecd7eb48b307f08faed We should look at ways to consolidate in a common-board.dtsi > > >> > >>> &serdes0 { > > [...] > >>> > >>> watchdog0: watchdog@2200000 { > >>> > >> > >> There is no such a tag, but: > >> whatever-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > > > > OK - I have no idea how B4 or patchworks pick that one as :D > > If we take this road, than I'm okay with it, but I'm going to take > silent protest (not sending acked-by or revired-by). > That should not stop you doing what you believe is best for the future! OK - thanks for your review and the discussions, always appreciate getting our views out there. if there are no other comments, I will try and post a v2 over the weekend. -- Regards, Nishanth Menon Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] arm64: dts: ti: k3-j721e*: Cleanup disabled nodes at SoC dtsi level 2020-11-06 21:46 ` Nishanth Menon @ 2020-11-09 7:49 ` Peter Ujfalusi 0 siblings, 0 replies; 12+ messages in thread From: Peter Ujfalusi @ 2020-11-09 7:49 UTC (permalink / raw) To: Nishanth Menon Cc: Roger Quadros, Keerthy, Jyri Sarha, Tomi Valkeinen, Lokesh Vutla, Rob Herring, Tony Lindgren, Tero Kristo, linux-kernel, devicetree, linux-arm-kernel On 06/11/2020 23.46, Nishanth Menon wrote: > On 13:32-20201106, Peter Ujfalusi wrote: > [...] >>> >>>> >>>>> default power management functionality etc >>>> >>>> Right, so how does that helps with devices present in the SoC, but no >>>> node at all? First thing which comes to mind is AASRC, we don't have >>>> Linux driver for it (and no DT binding document), but that does not mean >>>> that it is not present. How PM would take that into account? >>> >>> I think we are mixing topics here -> I was stating the motivation why >>> devicetree chose such as default. >> >> I don't question the fact that 'okay' is the default status if it is not >> explicitly present. There is no better default than that. > > ^^ -> Alright, that is all we are trying to do here: defaults in the > SoC.dtsi and specific cleanups (firmware reserved / board unused > disables) be done in a common board.dtsi (for now, there is no such > specific need, I guess). The default is what it is: default choice which suits most of the nodes. If the node is not complete in it's present form then it is not in it's default state. imho. >>> Alright - what do we suggest we do? >> >> Not sure, I'm 'whatever' after [1] makes it to mainline or next. > [....] >> [1] >> https://lore.kernel.org/alsa-devel/20201106072551.689-1-peter.ujfalusi@ti.com/ > > > I don't see the relationship between the series.. I think this series > brings no change in dtb, hence with OR without your driver cleanup > series, there is no practical regressions. This series opens up the possibility of nodes leaking to dtb with known broken state and the driver should have a better strategy than 'works by luck' to handle it ;) >> >>> Tony, Rob - I need some guidance here. >> >> I'm fine whatever way we take, but I think it is up to you to make the >> call as the maintainer of the TI dts files... ;) > > Yep - I have'nt seen a reason yet that must cause us to change from the > Device tree default approach in our debates. Imho 'disabled' is the default for nodes like McASP as it is: "Indicates that the device is not presently operational, but it might become operational in the future" (for example, needed properties added to the node). >>>> There is no such a tag, but: >>>> whatever-by: Peter Ujfalusi <peter.ujfalusi@ti.com> >>> >>> OK - I have no idea how B4 or patchworks pick that one as :D >> >> If we take this road, than I'm okay with it, but I'm going to take >> silent protest (not sending acked-by or revired-by). >> That should not stop you doing what you believe is best for the future! > > OK - thanks for your review and the discussions, always appreciate > getting our views out there. > > if there are no other comments, I will try and post a v2 over the > weekend. OK - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/4] arm64: dts: ti: am65/j721e: Fix up un-necessary status set to "okay" for crypto 2020-11-04 22:43 [PATCH 0/4] arm64: dts: ti: Cleanup mix of "okay" and "disabled" usage Nishanth Menon 2020-11-04 22:43 ` [PATCH 1/4] arm64: dts: ti: k3-am65*: Cleanup disabled nodes at SoC dtsi level Nishanth Menon 2020-11-04 22:43 ` [PATCH 2/4] arm64: dts: ti: k3-j721e*: " Nishanth Menon @ 2020-11-04 22:43 ` Nishanth Menon 2020-11-04 22:43 ` [PATCH 4/4] arm64: dts: ti: k3-am654-base-board: Fix up un-necessary status set to "okay" for USB Nishanth Menon 3 siblings, 0 replies; 12+ messages in thread From: Nishanth Menon @ 2020-11-04 22:43 UTC (permalink / raw) To: Roger Quadros, Keerthy, Jyri Sarha, Tomi Valkeinen, Peter Ujfalusi, Lokesh Vutla, Rob Herring, Tony Lindgren, Tero Kristo Cc: linux-kernel, devicetree, linux-arm-kernel, Nishanth Menon The default state of a device tree node is "okay". There is no specific use of explicitly adding status = "okay" in the SoC dtsi. Fixes: 8ebcaaae8017 ("arm64: dts: ti: k3-j721e-main: Add crypto accelerator node") Fixes: b366b2409c97 ("arm64: dts: ti: k3-am6: Add crypto accelarator node") Cc: Keerthy <j-keerthy@ti.com> Signed-off-by: Nishanth Menon <nm@ti.com> --- arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 1 - arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 2 -- 2 files changed, 3 deletions(-) diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi index 21e50021dd83..2bd66a9e4b1d 100644 --- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi +++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi @@ -119,7 +119,6 @@ crypto: crypto@4e00000 { #address-cells = <2>; #size-cells = <2>; ranges = <0x0 0x04e00000 0x00 0x04e00000 0x0 0x30000>; - status = "okay"; dmas = <&main_udmap 0xc000>, <&main_udmap 0x4000>, <&main_udmap 0x4001>; diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi index b54332d6fdc5..9747c387385b 100644 --- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi +++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi @@ -345,8 +345,6 @@ main_crypto: crypto@4e00000 { #size-cells = <2>; ranges = <0x0 0x04e00000 0x00 0x04e00000 0x0 0x30000>; - status = "okay"; - dmas = <&main_udmap 0xc000>, <&main_udmap 0x4000>, <&main_udmap 0x4001>; dma-names = "tx", "rx1", "rx2"; -- 2.29.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/4] arm64: dts: ti: k3-am654-base-board: Fix up un-necessary status set to "okay" for USB 2020-11-04 22:43 [PATCH 0/4] arm64: dts: ti: Cleanup mix of "okay" and "disabled" usage Nishanth Menon ` (2 preceding siblings ...) 2020-11-04 22:43 ` [PATCH 3/4] arm64: dts: ti: am65/j721e: Fix up un-necessary status set to "okay" for crypto Nishanth Menon @ 2020-11-04 22:43 ` Nishanth Menon 3 siblings, 0 replies; 12+ messages in thread From: Nishanth Menon @ 2020-11-04 22:43 UTC (permalink / raw) To: Roger Quadros, Keerthy, Jyri Sarha, Tomi Valkeinen, Peter Ujfalusi, Lokesh Vutla, Rob Herring, Tony Lindgren, Tero Kristo Cc: linux-kernel, devicetree, linux-arm-kernel, Nishanth Menon The default state of a device tree node is "okay". There is no specific use of explicitly adding status = "okay" in the board dts. Fixes: 7e7e7dd51d06 ("arm64: dts: ti: k3-am654-base-board: enable USB1") Cc: Roger Quadros <rogerq@ti.com> Signed-off-by: Nishanth Menon <nm@ti.com> --- arch/arm64/boot/dts/ti/k3-am654-base-board.dts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts index 199c4d4e7539..744efcbb4f7f 100644 --- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts +++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts @@ -325,14 +325,6 @@ &sdhci1 { disable-wp; }; -&dwc3_1 { - status = "okay"; -}; - -&usb1_phy { - status = "okay"; -}; - &usb1 { pinctrl-names = "default"; pinctrl-0 = <&usb1_pins_default>; -- 2.29.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-11-09 7:48 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-04 22:43 [PATCH 0/4] arm64: dts: ti: Cleanup mix of "okay" and "disabled" usage Nishanth Menon 2020-11-04 22:43 ` [PATCH 1/4] arm64: dts: ti: k3-am65*: Cleanup disabled nodes at SoC dtsi level Nishanth Menon 2020-11-05 7:25 ` Tomi Valkeinen 2020-11-04 22:43 ` [PATCH 2/4] arm64: dts: ti: k3-j721e*: " Nishanth Menon 2020-11-05 7:25 ` Tomi Valkeinen 2020-11-05 7:32 ` Peter Ujfalusi 2020-11-05 14:08 ` Nishanth Menon 2020-11-06 11:32 ` Peter Ujfalusi 2020-11-06 21:46 ` Nishanth Menon 2020-11-09 7:49 ` Peter Ujfalusi 2020-11-04 22:43 ` [PATCH 3/4] arm64: dts: ti: am65/j721e: Fix up un-necessary status set to "okay" for crypto Nishanth Menon 2020-11-04 22:43 ` [PATCH 4/4] arm64: dts: ti: k3-am654-base-board: Fix up un-necessary status set to "okay" for USB Nishanth Menon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).