Re: [PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

From: Kevin Hilman <khilman@kernel.org>
To: Tony Lindgren <tony@atomide.com>
Cc: Nishanth Menon <nm@ti.com>, Vignesh Raghavendra <vigneshr@ti.com>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Dhruva Gole <d-gole@ti.com>
Subject: Re: [PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0
Date: Wed, 13 Dec 2023 07:41:44 -0800	[thread overview]
Message-ID: <7hcyvam7nr.fsf@baylibre.com> (raw)
In-Reply-To: <20231209035900.GW5169@atomide.com>

Tony Lindgren <tony@atomide.com> writes:

> * Kevin Hilman <khilman@kernel.org> [231208 17:13]:
>> Tony Lindgren <tony@atomide.com> writes:
>> 
>> > * Kevin Hilman <khilman@kernel.org> [231205 18:14]:
>> >> I'm a little confused why these power-domain and clocks stay here and
>> >> are not moved under the wkup_uart0 node... 
>> >
>> > The resources are also needed by the interconnect target module. It's the
>> > wrapper IP for the child device(s). In this case there's one chip 8250 IP
>> > instance. In some other devices there can be multiple child IP devices
>> > wired to one target module.
>> >
>> >> > -		clock-names = "fclk";
>> >> > -		status = "disabled";
>> >> > +		clock-names = "fck";
>> >> > +		#address-cells = <1>;
>> >> > +		#size-cells = <1>;
>> >> > +		ranges = <0 0 0x2b300000 0x100000>;
>> >> > +
>> >> > +		wkup_uart0: serial@2b300000 {
>> >> > +			compatible = "ti,am64-uart", "ti,am654-uart";
>> >> > +			reg = <0 0x100>;
>> >> > +			interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
>> >> > +			status = "disabled";
>> >> 
>> >> ...here.
>> >> 
>> >> The SCI device ID 114 is specifically for wkup_uart0[1], so it seems to
>> >> me those should be in the wkup_uart0 node.
>> >
>> > Those resources are also needed for the parent target module for revision
>> > detection, quirks, reset, idle register configuration, and to probe the
>> > child devices.
>> >
>> > Here the 8250 IP can be set to status = "reserved" when used by the
>> > firmware, and 8250 not touched by Linux. However, the parent interconnect
>> > target module still needs to be configured for idle registers and wake-up
>> > path register bit so the wake-up from deeper suspend states works.
>> 
>> OK, makes sense.  Thanks for the clarification.
>
> One more thing to clarify, there's nothing stopping also mapping the needed
> resources for the child IP too if needed by the driver or the dts binding.
> The calls for resources by the 8250 driver just won't do anything as the
> resources are already enabled by the parent.

OK, thanks.

>> In that case, shouldn't the same be done for the other wakeup sources
>> there (e.g. wkup_rtc0) ?
>
> Yes it should be done for devices with the wake-up path wired like
> wkup_rtc0.

OK, in that case, this all makes sense to me.

Reviewed-by: Kevin Hilman <khilman@baylibre.com>

Kevin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel