Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
Date:   Fri, 6 Oct 2023 06:34:10 -0500
From:   Nishanth Menon <nm@ti.com>
To:     Keerthy <j-keerthy@ti.com>
CC:     <robh+dt@kernel.org>, <vigneshr@ti.com>, <conor+dt@kernel.org>,
        <kristo@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>,
        <u-kumar1@ti.com>, <linux-arm-kernel@lists.infradead.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v7 5/7] arm64: dts: ti: k3-j784s4-mcu: Add the mcu domain
 watchdog instances
Message-ID: <20231006113410.tibvxxkeuujqnbv4@evolution>
References: <20231006042901.6474-1-j-keerthy@ti.com>
 <20231006042901.6474-6-j-keerthy@ti.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20231006042901.6474-6-j-keerthy@ti.com>
Precedence: bulk

On 09:58-20231006, Keerthy wrote:
> There are totally 2 instances of watchdog module in MCU domain.
> These instances are coupled with the MCU domain R5F instances.

> Disabling them as they are not used by Linux.
Device tree is hardware description - not tied to how Linux uses it.

Reason these wdts are disabled by default is because they are tightly
coupled with R5Fs.

> 
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
>  .../boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi     | 24 +++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
> index a7b5c4cb7d3e..809a0b1cf038 100644
> --- a/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j784s4-mcu-wakeup.dtsi
> @@ -714,4 +714,28 @@
>  		ti,esm-pins = <63>;
>  		bootph-pre-ram;
>  	};
> +
> +	/*
> +	 * The 2 RTI instances are couple with MCU R5Fs so keeping them
> +	 * disabled as these will be used by their respective firmware
> +	 */
> +	mcu_watchdog0: watchdog@40600000 {
> +		compatible = "ti,j7-rti-wdt";
> +		reg = <0x00 0x40600000 0x00 0x100>;
> +		clocks = <&k3_clks 367 1>;
> +		power-domains = <&k3_pds 367 TI_SCI_PD_EXCLUSIVE>;
> +		assigned-clocks = <&k3_clks 367 0>;
> +		assigned-clock-parents = <&k3_clks 367 4>;
> +		status = "disabled";
> +	};
> +
> +	mcu_watchdog1: watchdog@40610000 {
> +		compatible = "ti,j7-rti-wdt";
> +		reg = <0x00 0x40610000 0x00 0x100>;
> +		clocks = <&k3_clks 368 1>;
> +		power-domains = <&k3_pds 368 TI_SCI_PD_EXCLUSIVE>;
> +		assigned-clocks = <&k3_clks 368 0>;
> +		assigned-clock-parents = <&k3_clks 368 4>;

Please DONOT ignore the review comments - I did ask the documentation in
dts as well. reason being that this is what people will see rather than
dig up the commit log. it should be intutive when reading the dts why
nodes are disabled by default Vs the standard of leaving it enabled by
default. Given esp that these peripherals do not have anything to do
with board semantics (pinmux or something similar) to be complete.

> +		status = "disabled";
> +	};
>  };
> -- 
> 2.17.1
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D