All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Markus Schneider-Pargmann" <msp@baylibre.com>
To: "Nishanth Menon" <nm@ti.com>,
	"Markus Schneider-Pargmann (TI)" <msp@baylibre.com>
Cc: Rob Herring <robh@kernel.org>, Conor Dooley <conor+dt@kernel.org>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Dhruva Gole <d-gole@ti.com>, Akashdeep Kaur <a-kaur@ti.com>,
	Kevin Hilman <khilman@baylibre.com>,
	Bjorn Andersson <andersson@kernel.org>,
	linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org,
	Kendall Willis <k-willis@ti.com>,
	devicetree@vger.kernel.org, Vishal Mahaveer <vishalm@ti.com>,
	Sebin Francis <sebin.francis@ti.com>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Tero Kristo <kristo@kernel.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4 1/4] arm64: dts: ti: k3-am62a7-sk: Split r5f memory region
Date: Thu, 07 May 2026 15:14:25 +0200	[thread overview]
Message-ID: <DICGZTCMDWHK.HMQMCB8M32ZV@baylibre.com> (raw)
In-Reply-To: <20260505124121.hffywentvo5pusfx@glowing>

[-- Attachment #1: Type: text/plain, Size: 3495 bytes --]

Hi Nishanth,

On Tue May 5, 2026 at 2:41 PM CEST, Nishanth Menon wrote:
> On 15:22-20260429, Markus Schneider-Pargmann (TI) wrote:
>> Split the firmware memory region in more specific parts so it is better
>> described where to find which information. Specifically the LPM metadata
>> region is important as bootloader software like U-Boot has to know where
>> that data is to be able to read that data.
>> 
>> Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
>> ---
>>  arch/arm64/boot/dts/ti/k3-am62a7-sk.dts | 40 +++++++++++++++++++++++++++++++--
>>  1 file changed, 38 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>> index c1e9067b3bdd5ab0591541d4685bb17a5dac4f65..6f2ee93c7be141ee5ae3f1e3324d3a060db069f6 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>> +++ b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>> @@ -59,9 +59,33 @@ wkup_r5fss0_core0_dma_memory_region: memory@9c800000 {
>>  			no-map;
>>  		};
>>  
>> -		wkup_r5fss0_core0_memory_region: memory@9c900000 {
>> +		wkup_r5fss0_core0_ipc_region: memory@9c900000 {
>
> Looks like you have'nt addressed Vignesh's comments from previous
> revision.
>
> https://lore.kernel.org/all/DHS46FH9ZYZB.3BG6HVH832NAE@baylibre.com/

Sorry, I was trying to understand the issue in that thread but I didn't
and didn't get a response so I couldn't really address it.

>
> We dropped wkup_r5fss0_core0_memory_region here..
>
>>  #include "k3-am62a-ti-ipc-firmware.dtsi"
> In this file:
> https://git.kernel.org/pub/scm/linux/kernel/git/ti/linux.git/tree/arch/arm64/boot/dts/ti/k3-am62a-ti-ipc-firmware.dtsi#n27

This file line 27 is
	mcu_r5fss0_core0_memory_region: memory@9b900000 {
		compatible = "shared-dma-pool";
		reg = <0x00 0x9b900000 0x00 0xf00000>;
		no-map;
	};

But my patch is removing wkup_r5fss0_core0_memory_region and not
touching the mcu_* definitions.

> https://git.kernel.org/pub/scm/linux/kernel/git/ti/linux.git/tree/arch/arm64/boot/dts/ti/k3-am62a-ti-ipc-firmware.dtsi#n68
>
> I am not clear where wkup_r5fss0_core0_memory_region is now defined.

It is not defined anymore for k3-am62a7-sk.

> BUT, in the patch, we introduce:
>
>> +
>> +&wkup_r5fss0_core0 {
>> +	memory-region = <&wkup_r5fss0_core0_dma_memory_region>,
>> +			<&wkup_r5fss0_core0_ipc_region>,
>> +			<&wkup_r5fss0_core0_lpm_fs_stub_region>,
>> +			<&wkup_r5fss0_core0_lpm_metadata_region>,
>> +			<&wkup_r5fss0_core0_lpm_rest_region>,
>> +			<&wkup_r5fss0_core0_dm_region>;
>> +	memory-region-names = "dma", "ipc", "lpm-stub",
>> +			      "lpm-metadata", "lpm-context",
>> +			      "dm-firmware";
>> +};
>
>
> So we go ahead an override the definitions of ipc-firmware.dtsi for
> wkup_r5fss0_core0 here - explaining why the build does'nt fail. I am
> confused why the ipc firmware dtsi was'nt updated instead? is this
> something different firmware dtsi now? if so, we should split the
> ipc-firmware.dtsi accordingly. Commit message does'nt mention the same
> either. This right solution is to make up our minds if ipc-firmware.dtsi
> is meant for LPM mode support or not. if not, split the dtsi, if yes, do
> the mods in the ipc.dtsi

We can update the firmware definitions as well. I currently only did
this for am62a and am62p as these require the new format so IO+DDR
works. But if you prefer I can make the changes to firmware.dtsi
instead.

Best
Markus

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 289 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: "Markus Schneider-Pargmann" <msp@baylibre.com>
To: "Nishanth Menon" <nm@ti.com>,
	"Markus Schneider-Pargmann (TI)" <msp@baylibre.com>
Cc: "Bjorn Andersson" <andersson@kernel.org>,
	"Mathieu Poirier" <mathieu.poirier@linaro.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Suman Anna" <s-anna@ti.com>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	"Tero Kristo" <kristo@kernel.org>,
	"Vishal Mahaveer" <vishalm@ti.com>,
	"Kevin Hilman" <khilman@baylibre.com>,
	"Dhruva Gole" <d-gole@ti.com>,
	"Sebin Francis" <sebin.francis@ti.com>,
	"Kendall Willis" <k-willis@ti.com>,
	"Akashdeep Kaur" <a-kaur@ti.com>,
	<linux-remoteproc@vger.kernel.org>, <devicetree@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v4 1/4] arm64: dts: ti: k3-am62a7-sk: Split r5f memory region
Date: Thu, 07 May 2026 15:14:25 +0200	[thread overview]
Message-ID: <DICGZTCMDWHK.HMQMCB8M32ZV@baylibre.com> (raw)
In-Reply-To: <20260505124121.hffywentvo5pusfx@glowing>

[-- Attachment #1: Type: text/plain, Size: 3495 bytes --]

Hi Nishanth,

On Tue May 5, 2026 at 2:41 PM CEST, Nishanth Menon wrote:
> On 15:22-20260429, Markus Schneider-Pargmann (TI) wrote:
>> Split the firmware memory region in more specific parts so it is better
>> described where to find which information. Specifically the LPM metadata
>> region is important as bootloader software like U-Boot has to know where
>> that data is to be able to read that data.
>> 
>> Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
>> ---
>>  arch/arm64/boot/dts/ti/k3-am62a7-sk.dts | 40 +++++++++++++++++++++++++++++++--
>>  1 file changed, 38 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>> index c1e9067b3bdd5ab0591541d4685bb17a5dac4f65..6f2ee93c7be141ee5ae3f1e3324d3a060db069f6 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>> +++ b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>> @@ -59,9 +59,33 @@ wkup_r5fss0_core0_dma_memory_region: memory@9c800000 {
>>  			no-map;
>>  		};
>>  
>> -		wkup_r5fss0_core0_memory_region: memory@9c900000 {
>> +		wkup_r5fss0_core0_ipc_region: memory@9c900000 {
>
> Looks like you have'nt addressed Vignesh's comments from previous
> revision.
>
> https://lore.kernel.org/all/DHS46FH9ZYZB.3BG6HVH832NAE@baylibre.com/

Sorry, I was trying to understand the issue in that thread but I didn't
and didn't get a response so I couldn't really address it.

>
> We dropped wkup_r5fss0_core0_memory_region here..
>
>>  #include "k3-am62a-ti-ipc-firmware.dtsi"
> In this file:
> https://git.kernel.org/pub/scm/linux/kernel/git/ti/linux.git/tree/arch/arm64/boot/dts/ti/k3-am62a-ti-ipc-firmware.dtsi#n27

This file line 27 is
	mcu_r5fss0_core0_memory_region: memory@9b900000 {
		compatible = "shared-dma-pool";
		reg = <0x00 0x9b900000 0x00 0xf00000>;
		no-map;
	};

But my patch is removing wkup_r5fss0_core0_memory_region and not
touching the mcu_* definitions.

> https://git.kernel.org/pub/scm/linux/kernel/git/ti/linux.git/tree/arch/arm64/boot/dts/ti/k3-am62a-ti-ipc-firmware.dtsi#n68
>
> I am not clear where wkup_r5fss0_core0_memory_region is now defined.

It is not defined anymore for k3-am62a7-sk.

> BUT, in the patch, we introduce:
>
>> +
>> +&wkup_r5fss0_core0 {
>> +	memory-region = <&wkup_r5fss0_core0_dma_memory_region>,
>> +			<&wkup_r5fss0_core0_ipc_region>,
>> +			<&wkup_r5fss0_core0_lpm_fs_stub_region>,
>> +			<&wkup_r5fss0_core0_lpm_metadata_region>,
>> +			<&wkup_r5fss0_core0_lpm_rest_region>,
>> +			<&wkup_r5fss0_core0_dm_region>;
>> +	memory-region-names = "dma", "ipc", "lpm-stub",
>> +			      "lpm-metadata", "lpm-context",
>> +			      "dm-firmware";
>> +};
>
>
> So we go ahead an override the definitions of ipc-firmware.dtsi for
> wkup_r5fss0_core0 here - explaining why the build does'nt fail. I am
> confused why the ipc firmware dtsi was'nt updated instead? is this
> something different firmware dtsi now? if so, we should split the
> ipc-firmware.dtsi accordingly. Commit message does'nt mention the same
> either. This right solution is to make up our minds if ipc-firmware.dtsi
> is meant for LPM mode support or not. if not, split the dtsi, if yes, do
> the mods in the ipc.dtsi

We can update the firmware definitions as well. I currently only did
this for am62a and am62p as these require the new format so IO+DDR
works. But if you prefer I can make the changes to firmware.dtsi
instead.

Best
Markus

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 289 bytes --]

  reply	other threads:[~2026-05-07 13:14 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-29 13:22 [PATCH v4 0/4] arm64: dts: ti: k3-am62a7-sk: Split r5f memory region Markus Schneider-Pargmann (TI)
2026-04-29 13:22 ` [PATCH v4 1/4] " Markus Schneider-Pargmann (TI)
2026-05-05 12:41   ` Nishanth Menon
2026-05-05 12:41     ` Nishanth Menon
2026-05-07 13:14     ` Markus Schneider-Pargmann [this message]
2026-05-07 13:14       ` Markus Schneider-Pargmann
2026-05-07 15:57       ` Nishanth Menon
2026-05-07 15:57         ` Nishanth Menon
2026-04-29 13:22 ` [PATCH v4 2/4] arm64: dts: ti: k3-am62p5-sk: " Markus Schneider-Pargmann (TI)
2026-04-29 13:22 ` [PATCH v4 3/4] arm64: dts: ti: k3-am62a7-sk: Add r5f nodes to pre-ram bootphase Markus Schneider-Pargmann (TI)
2026-04-29 13:22 ` [PATCH v4 4/4] arm64: dts: ti: k3-am62p5-sk: " Markus Schneider-Pargmann (TI)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DICGZTCMDWHK.HMQMCB8M32ZV@baylibre.com \
    --to=msp@baylibre.com \
    --cc=a-kaur@ti.com \
    --cc=andersson@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=d-gole@ti.com \
    --cc=devicetree@vger.kernel.org \
    --cc=k-willis@ti.com \
    --cc=khilman@baylibre.com \
    --cc=kristo@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=mathieu.poirier@linaro.org \
    --cc=nm@ti.com \
    --cc=robh@kernel.org \
    --cc=sebin.francis@ti.com \
    --cc=vigneshr@ti.com \
    --cc=vishalm@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.