Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <90cbb65b-389e-95b5-26d8-39bc33f88df6@ti.com>
Date:   Wed, 19 Oct 2022 12:26:31 -0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [PATCH v2 3/4] arm64: dts: ti: Add initial support for J784S4 SoC
Content-Language: en-US
To:     Apurva Nandan <a-nandan@ti.com>, Nishanth Menon <nm@ti.com>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        Tero Kristo <kristo@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Linus Walleij <linus.walleij@linaro.org>,
        <linux-arm-kernel@lists.infradead.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-gpio@vger.kernel.org>
CC:     Hari Nagalla <hnagalla@ti.com>
References: <20221014082314.118361-1-a-nandan@ti.com>
 <20221014082314.118361-4-a-nandan@ti.com>
From:   Andrew Davis <afd@ti.com>
In-Reply-To: <20221014082314.118361-4-a-nandan@ti.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 10/14/22 3:23 AM, Apurva Nandan wrote:
> The J784S4 SoC belongs to the K3 Multicore SoC architecture
> platform, providing advanced system integration in automotive,
> ADAS and industrial applications requiring AI at the network edge.
> This SoC extends the K3 Jacinto 7 family of SoCs with focus on
> raising performance and integration while providing interfaces,
> memory architecture and compute performance for multi-sensor, high
> concurrency applications.
> 

[...]

> +
> +		hwspinlock: hwlock@30e00000 {
> +			compatible = "ti,am654-hwspinlock";
> +			reg = <0x00 0x30e00000 0x00 0x1000>;
> +			#hwlock-cells = <1>;
> +			status = "disabled";

Why is this disabled? The node is complete and usable.

I do not know if we settled on a set of rules for disabling nodes
by default in the dtsi, I couldn't find one if we did, but how does
this sound,

If a node in a dtsi file is incomplete, or the described hardware
unusable, without additional information provided by board-level
additions, then the node may be disabled. Otherwise it must be
left in the default enabled state.

Without something like that, we will end up with the same problem
we are trying to fix here, but in reverse, one will have to
enable nodes in board files that would not otherwise need to
refrenced. Worse that sounds like a configuration setting, not a
hardware description, so it would not belong in the device tree.

Same for main_ringacc, main_udmap, cpts, and mcu_navss below.

> +		};
> +

[...]

> +
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/pinctrl/k3.h>
> +#include <dt-bindings/soc/ti,sci_pm_domain.h>
> +
> +/ {
> +

Extra newline not needed.

Andrew

> +	model = "Texas Instruments K3 J784S4 SoC";