mbox series

[0/7] arm64: dts: ti: k3-am65: dtbs_check warnings fixups

Message ID 20230424144949.244135-1-nm@ti.com
Headers show
Series arm64: dts: ti: k3-am65: dtbs_check warnings fixups | expand

Message

Nishanth Menon April 24, 2023, 2:49 p.m. UTC
Hi,

One round of long pending dtbs_checks warnings fixups for am65.

Bootlog: https://gist.github.com/nmenon/605142cb4a4ffb2b9d05bea944f8fd32#file-am654-evm-patchset2-txt

Pending warnings: https://gist.github.com/nmenon/605142cb4a4ffb2b9d05bea944f8fd32#file-pending-dtbs_check_warnings

Nishanth Menon (7):
  arm64: dts: ti: k3-am65-main: Remove "syscon" nodes added for
    pcieX_ctrl
  arm64: dts: ti: k3-am65-main: Fix mux controller node name
  arm64: dts: ti: k3-am65: Switch to "ti,j721e-system-controller"
    compatible
  arm64: dts: ti: k3-am65-main: Fix mcan node name
  arm64: dts: ti: k3-am65-main: Drop deprecated ti,otap-del-sel property
  arm64: dts: ti: k3-am65-mcu: Fix fss node
  arm64: dts: ti: k3-am65-iot2050-common: Rename rtc8564 nodename

 .../boot/dts/ti/k3-am65-iot2050-common.dtsi   |  2 +-
 arch/arm64/boot/dts/ti/k3-am65-main.dtsi      | 32 +++++--------------
 arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi       |  9 +++---
 3 files changed, 14 insertions(+), 29 deletions(-)

Comments

Andrew Davis April 24, 2023, 5:24 p.m. UTC | #1
On 4/24/23 9:49 AM, Nishanth Menon wrote:
> Switch scm-conf to "ti,j721e-system-controller" compatible to be more
> specific.
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>   arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 2 +-
>   arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi  | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> index 227573773b26..40fa631f2f3d 100644
> --- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> @@ -475,7 +475,7 @@ sdhci1: mmc@4fa0000 {
>   	};
>   
>   	scm_conf: scm-conf@100000 {
> -		compatible = "syscon", "simple-mfd";
> +		compatible = "ti,j721e-system-controller", "syscon", "simple-mfd";
>   		reg = <0 0x00100000 0 0x1c000>;
>   		#address-cells = <1>;
>   		#size-cells = <1>;
> diff --git a/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi b/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi
> index 5dfa31840e9c..566dc584d3f3 100644
> --- a/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am65-mcu.dtsi
> @@ -7,7 +7,7 @@
>   
>   &cbass_mcu {
>   	mcu_conf: scm-conf@40f00000 {
> -		compatible = "syscon", "simple-mfd";
> +		compatible = "ti,j721e-system-controller", "syscon", "simple-mfd";

This node is not a "j721e-system-controller". Only the one in main could be
said to be one, but even it is different enough that this is not correct IMHO.
It almost seems like you are using "ti,j721e-system-controller" as a workaround
for the restriction on raw "syscon", "simple-mfd" nodes. And just replacing all
instance of those with something that avoids the warning.

What we should do here is turn both of these nodes into "simple-bus". The sub-nodes
themselves would describe what they are. This is the normal DT way vs having
all our device nodes pointing into one big "syscon" node with various offsets (which
makes it hard to see all users of a node and near impossible to work out the real
memory map in these "system-controller" nodes).

Worse, if the parent is a "syscon" then the whole memory region gets one big regmap
over it, and any child nodes that also build a regmap for their smaller sub-range
leads to having two regmaps pointing to the same memory area. This breaks some
assumptions around atomic access and reg caching.

Taking a quick look I see some of our sub-node drivers expecting the parent to
always be a syscon, others turn themselves into a syscon, and others still do the
normal "reg" mapping. What a mess..

To unwind this I'd suggest we do this:

  * Add support for these sub-node drivers to use the normal "reg" property by
    default if available, falling back to expecting the parent to be a syscon only
    for backwards compatibility.

  * Add "reg" properties to the sub-nodes.

  * Remove "syscon" from our system-controller nodes and instead use "simple-bus".
    Which more accurately describes what these regions are, and prevents issues like
    having a regmap over gaps (as these system-controller have gaps in between the
    sub device memory regions)

We would still have to add simple compatibles for the efuse and pcie mode/id regions,
but that is much more correct than hiding them in the device's node like done in patch
1/7 of this series.

Andrew

>   		reg = <0x0 0x40f00000 0x0 0x20000>;
>   		#address-cells = <1>;
>   		#size-cells = <1>;