[PATCH 1/5] ARM: dts: stm32: Add missing detach mailbox for emtrion emSBC-Argon

[PATCH 1/5] ARM: dts: stm32: Add missing detach mailbox for emtrion emSBC-Argon

[Marek Vasut]

Add missing "detach" mailbox to this board to permit the CPU to inform
the remote processor on a detach. This signal allows the remote processor
firmware to stop IPC communication and to reinitialize the resources for
a re-attach.

Without this mailbox, detach is not possible and kernel log contains the
following warning to, so make sure all the STM32MP15xx platform DTs are
in sync regarding the mailboxes to fix the detach issue and the warning:
"
stm32-rproc 10000000.m4: mbox_request_channel_byname() could not locate channel named "detach"
"

Fixes: 6257dfc1c412 ("ARM: dts: stm32: Add coprocessor detach mbox on stm32mp15x-dkx boards")
Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: kernel@dh-electronics.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-stm32@st-md-mailman.stormreply.com
---
 arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi b/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
index b01470a9a3d53..82061c9186338 100644
--- a/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
@@ -366,8 +366,8 @@ &iwdg2 {
 &m4_rproc {
 	memory-region = <&retram>, <&mcuram>, <&mcuram2>, <&vdev0vring0>,
 			<&vdev0vring1>, <&vdev0buffer>;
-	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>;
-	mbox-names = "vq0", "vq1", "shutdown";
+	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
+	mbox-names = "vq0", "vq1", "shutdown", "detach";
 	interrupt-parent = <&exti>;
 	interrupts = <68 1>;
 	interrupt-names = "wdg";
-- 
2.39.2

[PATCH 2/5] ARM: dts: stm32: Add missing detach mailbox for Odyssey SoM

[Marek Vasut]

Add missing "detach" mailbox to this board to permit the CPU to inform
the remote processor on a detach. This signal allows the remote processor
firmware to stop IPC communication and to reinitialize the resources for
a re-attach.

Without this mailbox, detach is not possible and kernel log contains the
following warning to, so make sure all the STM32MP15xx platform DTs are
in sync regarding the mailboxes to fix the detach issue and the warning:
"
stm32-rproc 10000000.m4: mbox_request_channel_byname() could not locate channel named "detach"
"

Fixes: 6257dfc1c412 ("ARM: dts: stm32: Add coprocessor detach mbox on stm32mp15x-dkx boards")
Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: kernel@dh-electronics.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-stm32@st-md-mailman.stormreply.com
---
 arch/arm/boot/dts/stm32mp157c-odyssey-som.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/stm32mp157c-odyssey-som.dtsi b/arch/arm/boot/dts/stm32mp157c-odyssey-som.dtsi
index e22871dc580c8..cf74852514906 100644
--- a/arch/arm/boot/dts/stm32mp157c-odyssey-som.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c-odyssey-som.dtsi
@@ -230,8 +230,8 @@ &iwdg2 {
 &m4_rproc {
 	memory-region = <&retram>, <&mcuram>, <&mcuram2>, <&vdev0vring0>,
 			<&vdev0vring1>, <&vdev0buffer>;
-	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>;
-	mbox-names = "vq0", "vq1", "shutdown";
+	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
+	mbox-names = "vq0", "vq1", "shutdown", "detach";
 	interrupt-parent = <&exti>;
 	interrupts = <68 1>;
 	status = "okay";
-- 
2.39.2

[PATCH 3/5] ARM: dts: stm32: Add missing detach mailbox for DHCOM SoM

[Marek Vasut]

Add missing "detach" mailbox to this board to permit the CPU to inform
the remote processor on a detach. This signal allows the remote processor
firmware to stop IPC communication and to reinitialize the resources for
a re-attach.

Without this mailbox, detach is not possible and kernel log contains the
following warning to, so make sure all the STM32MP15xx platform DTs are
in sync regarding the mailboxes to fix the detach issue and the warning:
"
stm32-rproc 10000000.m4: mbox_request_channel_byname() could not locate channel named "detach"
"

Fixes: 6257dfc1c412 ("ARM: dts: stm32: Add coprocessor detach mbox on stm32mp15x-dkx boards")
Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: kernel@dh-electronics.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-stm32@st-md-mailman.stormreply.com
---
 arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi b/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
index b2557bb718f52..7bf13183437c5 100644
--- a/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
+++ b/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
@@ -414,8 +414,8 @@ &iwdg2 {
 &m4_rproc {
 	memory-region = <&retram>, <&mcuram>, <&mcuram2>, <&vdev0vring0>,
 			<&vdev0vring1>, <&vdev0buffer>;
-	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>;
-	mbox-names = "vq0", "vq1", "shutdown";
+	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
+	mbox-names = "vq0", "vq1", "shutdown", "detach";
 	interrupt-parent = <&exti>;
 	interrupts = <68 1>;
 	status = "okay";
-- 
2.39.2

[PATCH 4/5] ARM: dts: stm32: Add missing detach mailbox for DHCOR SoM

[Marek Vasut]

Add missing "detach" mailbox to this board to permit the CPU to inform
the remote processor on a detach. This signal allows the remote processor
firmware to stop IPC communication and to reinitialize the resources for
a re-attach.

Without this mailbox, detach is not possible and kernel log contains the
following warning to, so make sure all the STM32MP15xx platform DTs are
in sync regarding the mailboxes to fix the detach issue and the warning:
"
stm32-rproc 10000000.m4: mbox_request_channel_byname() could not locate channel named "detach"
"

Fixes: 6257dfc1c412 ("ARM: dts: stm32: Add coprocessor detach mbox on stm32mp15x-dkx boards")
Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: kernel@dh-electronics.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-stm32@st-md-mailman.stormreply.com
---
 arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
index 864960387e634..f0351f599a508 100644
--- a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
+++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
@@ -227,8 +227,8 @@ &iwdg2 {
 &m4_rproc {
 	memory-region = <&retram>, <&mcuram>, <&mcuram2>, <&vdev0vring0>,
 			<&vdev0vring1>, <&vdev0buffer>;
-	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>;
-	mbox-names = "vq0", "vq1", "shutdown";
+	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
+	mbox-names = "vq0", "vq1", "shutdown", "detach";
 	interrupt-parent = <&exti>;
 	interrupts = <68 1>;
 	status = "okay";
-- 
2.39.2

Re: [PATCH 4/5] ARM: dts: stm32: Add missing detach mailbox for DHCOR SoM

[Marek Vasut]

On 5/18/23 03:12, Marek Vasut wrote:
> Add missing "detach" mailbox to this board to permit the CPU to inform
> the remote processor on a detach. This signal allows the remote processor
> firmware to stop IPC communication and to reinitialize the resources for
> a re-attach.
> 
> Without this mailbox, detach is not possible and kernel log contains the
> following warning to, so make sure all the STM32MP15xx platform DTs are
> in sync regarding the mailboxes to fix the detach issue and the warning:
> "
> stm32-rproc 10000000.m4: mbox_request_channel_byname() could not locate channel named "detach"
> "
> 
> Fixes: 6257dfc1c412 ("ARM: dts: stm32: Add coprocessor detach mbox on stm32mp15x-dkx boards")
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Richard Cochran <richardcochran@gmail.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: kernel@dh-electronics.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> ---
>   arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
> index 864960387e634..f0351f599a508 100644
> --- a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
> +++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
> @@ -227,8 +227,8 @@ &iwdg2 {
>   &m4_rproc {
>   	memory-region = <&retram>, <&mcuram>, <&mcuram2>, <&vdev0vring0>,
>   			<&vdev0vring1>, <&vdev0buffer>;
> -	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>;
> -	mbox-names = "vq0", "vq1", "shutdown";
> +	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
> +	mbox-names = "vq0", "vq1", "shutdown", "detach";
>   	interrupt-parent = <&exti>;
>   	interrupts = <68 1>;
>   	status = "okay";

Is anything blocking 1/5..4/5 (i.e. the duplication in each board DT) 
patches from being applied ?

Re: [PATCH 4/5] ARM: dts: stm32: Add missing detach mailbox for DHCOR SoM

[Alexandre TORGUE]

Hi Marek

On 7/11/23 04:05, Marek Vasut wrote:
> On 5/18/23 03:12, Marek Vasut wrote:
>> Add missing "detach" mailbox to this board to permit the CPU to inform
>> the remote processor on a detach. This signal allows the remote processor
>> firmware to stop IPC communication and to reinitialize the resources for
>> a re-attach.
>>
>> Without this mailbox, detach is not possible and kernel log contains the
>> following warning to, so make sure all the STM32MP15xx platform DTs are
>> in sync regarding the mailboxes to fix the detach issue and the warning:
>> "
>> stm32-rproc 10000000.m4: mbox_request_channel_byname() could not 
>> locate channel named "detach"
>> "
>>
>> Fixes: 6257dfc1c412 ("ARM: dts: stm32: Add coprocessor detach mbox on 
>> stm32mp15x-dkx boards")
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> ---
>> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
>> Cc: Conor Dooley <conor+dt@kernel.org>
>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
>> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>> Cc: Richard Cochran <richardcochran@gmail.com>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: devicetree@vger.kernel.org
>> Cc: kernel@dh-electronics.com
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-stm32@st-md-mailman.stormreply.com
>> ---
>>   arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi 
>> b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
>> index 864960387e634..f0351f599a508 100644
>> --- a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
>> +++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
>> @@ -227,8 +227,8 @@ &iwdg2 {
>>   &m4_rproc {
>>       memory-region = <&retram>, <&mcuram>, <&mcuram2>, <&vdev0vring0>,
>>               <&vdev0vring1>, <&vdev0buffer>;
>> -    mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>;
>> -    mbox-names = "vq0", "vq1", "shutdown";
>> +    mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
>> +    mbox-names = "vq0", "vq1", "shutdown", "detach";
>>       interrupt-parent = <&exti>;
>>       interrupts = <68 1>;
>>       status = "okay";
> 
> Is anything blocking 1/5..4/5 (i.e. the duplication in each board DT) 
> patches from being applied ?

Nothing. I was just waiting to discuss with you about patch 5 at Prague 
then merge windows.

So patch 1 to 4 applied on stm32-next.

Cheers
Alex

Re: [PATCH 4/5] ARM: dts: stm32: Add missing detach mailbox for DHCOR SoM

[Marek Vasut]

On 7/11/23 15:37, Alexandre TORGUE wrote:
> Hi Marek
> 
> On 7/11/23 04:05, Marek Vasut wrote:
>> On 5/18/23 03:12, Marek Vasut wrote:
>>> Add missing "detach" mailbox to this board to permit the CPU to inform
>>> the remote processor on a detach. This signal allows the remote 
>>> processor
>>> firmware to stop IPC communication and to reinitialize the resources for
>>> a re-attach.
>>>
>>> Without this mailbox, detach is not possible and kernel log contains the
>>> following warning to, so make sure all the STM32MP15xx platform DTs are
>>> in sync regarding the mailboxes to fix the detach issue and the warning:
>>> "
>>> stm32-rproc 10000000.m4: mbox_request_channel_byname() could not 
>>> locate channel named "detach"
>>> "
>>>
>>> Fixes: 6257dfc1c412 ("ARM: dts: stm32: Add coprocessor detach mbox on 
>>> stm32mp15x-dkx boards")
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> ---
>>> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
>>> Cc: Conor Dooley <conor+dt@kernel.org>
>>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
>>> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>>> Cc: Richard Cochran <richardcochran@gmail.com>
>>> Cc: Rob Herring <robh+dt@kernel.org>
>>> Cc: devicetree@vger.kernel.org
>>> Cc: kernel@dh-electronics.com
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: linux-stm32@st-md-mailman.stormreply.com
>>> ---
>>>   arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi 
>>> b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
>>> index 864960387e634..f0351f599a508 100644
>>> --- a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
>>> +++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
>>> @@ -227,8 +227,8 @@ &iwdg2 {
>>>   &m4_rproc {
>>>       memory-region = <&retram>, <&mcuram>, <&mcuram2>, <&vdev0vring0>,
>>>               <&vdev0vring1>, <&vdev0buffer>;
>>> -    mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>;
>>> -    mbox-names = "vq0", "vq1", "shutdown";
>>> +    mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
>>> +    mbox-names = "vq0", "vq1", "shutdown", "detach";
>>>       interrupt-parent = <&exti>;
>>>       interrupts = <68 1>;
>>>       status = "okay";
>>
>> Is anything blocking 1/5..4/5 (i.e. the duplication in each board DT) 
>> patches from being applied ?
> 
> Nothing. I was just waiting to discuss with you about patch 5 at Prague 
> then merge windows.
> 
> So patch 1 to 4 applied on stm32-next.

Thank you

[PATCH 5/5] ARM: dts: stm32: Deduplicate rproc mboxes and IRQs

[Marek Vasut]

Pull mboxes, mbox-names, interrupt-parent, interrupts properties of the
m4_rproc into stm32mp151.dtsi to deduplicate multiple copies of the same
in multiple board files. Worse, those copies were starting to get out of
sync, so this should prevent any such issues in the future.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: kernel@dh-electronics.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-stm32@st-md-mailman.stormreply.com
---
 arch/arm/boot/dts/stm32mp151.dtsi                        | 4 ++++
 arch/arm/boot/dts/stm32mp157c-ed1.dts                    | 4 ----
 arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi         | 4 ----
 arch/arm/boot/dts/stm32mp157c-odyssey-som.dtsi           | 4 ----
 arch/arm/boot/dts/stm32mp157c-phycore-stm32mp15-som.dtsi | 4 ----
 arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi             | 4 ----
 arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi             | 4 ----
 arch/arm/boot/dts/stm32mp15xx-dkx.dtsi                   | 4 ----
 arch/arm/boot/dts/stm32mp15xx-osd32.dtsi                 | 4 ----
 9 files changed, 4 insertions(+), 32 deletions(-)

diff --git a/arch/arm/boot/dts/stm32mp151.dtsi b/arch/arm/boot/dts/stm32mp151.dtsi
index accbeef4df6da..97d54bf0dcc30 100644
--- a/arch/arm/boot/dts/stm32mp151.dtsi
+++ b/arch/arm/boot/dts/stm32mp151.dtsi
@@ -1824,6 +1824,10 @@ m4_rproc: m4@10000000 {
 			reg = <0x10000000 0x40000>,
 			      <0x30000000 0x40000>,
 			      <0x38000000 0x10000>;
+			interrupt-parent = <&exti>;
+			interrupts = <68 IRQ_TYPE_EDGE_RISING>;
+			mbox-names = "vq0", "vq1", "shutdown", "detach";
+			mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
 			resets = <&rcc MCU_R>;
 			st,syscfg-holdboot = <&rcc 0x10C 0x1>;
 			st,syscfg-tz = <&rcc 0x000 0x1>;
diff --git a/arch/arm/boot/dts/stm32mp157c-ed1.dts b/arch/arm/boot/dts/stm32mp157c-ed1.dts
index 8beb901be5065..3b40c2d8c3d9e 100644
--- a/arch/arm/boot/dts/stm32mp157c-ed1.dts
+++ b/arch/arm/boot/dts/stm32mp157c-ed1.dts
@@ -304,10 +304,6 @@ &iwdg2 {
 &m4_rproc {
 	memory-region = <&retram>, <&mcuram>, <&mcuram2>, <&vdev0vring0>,
 			<&vdev0vring1>, <&vdev0buffer>;
-	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
-	mbox-names = "vq0", "vq1", "shutdown", "detach";
-	interrupt-parent = <&exti>;
-	interrupts = <68 1>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi b/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
index 82061c9186338..a5c86bba46aea 100644
--- a/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
@@ -366,10 +366,6 @@ &iwdg2 {
 &m4_rproc {
 	memory-region = <&retram>, <&mcuram>, <&mcuram2>, <&vdev0vring0>,
 			<&vdev0vring1>, <&vdev0buffer>;
-	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
-	mbox-names = "vq0", "vq1", "shutdown", "detach";
-	interrupt-parent = <&exti>;
-	interrupts = <68 1>;
 	interrupt-names = "wdg";
 	recovery;
 	status = "okay";
diff --git a/arch/arm/boot/dts/stm32mp157c-odyssey-som.dtsi b/arch/arm/boot/dts/stm32mp157c-odyssey-som.dtsi
index cf74852514906..31d7bfe8bf8c9 100644
--- a/arch/arm/boot/dts/stm32mp157c-odyssey-som.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c-odyssey-som.dtsi
@@ -230,10 +230,6 @@ &iwdg2 {
 &m4_rproc {
 	memory-region = <&retram>, <&mcuram>, <&mcuram2>, <&vdev0vring0>,
 			<&vdev0vring1>, <&vdev0buffer>;
-	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
-	mbox-names = "vq0", "vq1", "shutdown", "detach";
-	interrupt-parent = <&exti>;
-	interrupts = <68 1>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/stm32mp157c-phycore-stm32mp15-som.dtsi b/arch/arm/boot/dts/stm32mp157c-phycore-stm32mp15-som.dtsi
index 4e8b2d2b30c7a..f68aaf6aa9fb5 100644
--- a/arch/arm/boot/dts/stm32mp157c-phycore-stm32mp15-som.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c-phycore-stm32mp15-som.dtsi
@@ -405,10 +405,6 @@ &m_can2 {
 &m4_rproc {
 	memory-region = <&retram>, <&mcuram>, <&mcuram2>, <&vdev0vring0>,
 			<&vdev0vring1>, <&vdev0buffer>;
-	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
-	mbox-names = "vq0", "vq1", "shutdown", "detach";
-	interrupt-parent = <&exti>;
-	interrupts = <68 1>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi b/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
index 7bf13183437c5..a38009f8456b8 100644
--- a/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
+++ b/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
@@ -414,10 +414,6 @@ &iwdg2 {
 &m4_rproc {
 	memory-region = <&retram>, <&mcuram>, <&mcuram2>, <&vdev0vring0>,
 			<&vdev0vring1>, <&vdev0buffer>;
-	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
-	mbox-names = "vq0", "vq1", "shutdown", "detach";
-	interrupt-parent = <&exti>;
-	interrupts = <68 1>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
index f0351f599a508..8c30cecacaf86 100644
--- a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
+++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
@@ -227,10 +227,6 @@ &iwdg2 {
 &m4_rproc {
 	memory-region = <&retram>, <&mcuram>, <&mcuram2>, <&vdev0vring0>,
 			<&vdev0vring1>, <&vdev0buffer>;
-	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
-	mbox-names = "vq0", "vq1", "shutdown", "detach";
-	interrupt-parent = <&exti>;
-	interrupts = <68 1>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/stm32mp15xx-dkx.dtsi b/arch/arm/boot/dts/stm32mp15xx-dkx.dtsi
index 0f1110e42c939..cc3eb755663fd 100644
--- a/arch/arm/boot/dts/stm32mp15xx-dkx.dtsi
+++ b/arch/arm/boot/dts/stm32mp15xx-dkx.dtsi
@@ -467,10 +467,6 @@ ltdc_ep0_out: endpoint@0 {
 &m4_rproc {
 	memory-region = <&retram>, <&mcuram>, <&mcuram2>, <&vdev0vring0>,
 			<&vdev0vring1>, <&vdev0buffer>;
-	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
-	mbox-names = "vq0", "vq1", "shutdown", "detach";
-	interrupt-parent = <&exti>;
-	interrupts = <68 1>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/stm32mp15xx-osd32.dtsi b/arch/arm/boot/dts/stm32mp15xx-osd32.dtsi
index a43965c86fe8b..6532726502c32 100644
--- a/arch/arm/boot/dts/stm32mp15xx-osd32.dtsi
+++ b/arch/arm/boot/dts/stm32mp15xx-osd32.dtsi
@@ -210,10 +210,6 @@ &ipcc {
 &m4_rproc {
 	memory-region = <&retram>, <&mcuram>, <&mcuram2>, <&vdev0vring0>,
 			<&vdev0vring1>, <&vdev0buffer>;
-	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
-	mbox-names = "vq0", "vq1", "shutdown", "detach";
-	interrupt-parent = <&exti>;
-	interrupts = <68 1>;
 	status = "okay";
 };
 
-- 
2.39.2

RE: [Linux-stm32] [PATCH 5/5] ARM: dts: stm32: Deduplicate rproc mboxes and IRQs

[Arnaud POULIQUEN]

ST Restricted

> -----Original Message-----
> From: Linux-stm32 <linux-stm32-bounces@st-md-mailman.stormreply.com>
> On Behalf Of Marek Vasut
> Sent: Thursday, May 18, 2023 3:13 AM
> To: linux-arm-kernel@lists.infradead.org
> Cc: Marek Vasut <marex@denx.de>; devicetree@vger.kernel.org; Conor
> Dooley <conor+dt@kernel.org>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@linaro.org>; Richard Cochran
> <richardcochran@gmail.com>; Rob Herring <robh+dt@kernel.org>; Maxime
> Coquelin <mcoquelin.stm32@gmail.com>; linux-stm32@st-md-
> mailman.stormreply.com; kernel@dh-electronics.com
> Subject: [Linux-stm32] [PATCH 5/5] ARM: dts: stm32: Deduplicate rproc
> mboxes and IRQs
> 
> Pull mboxes, mbox-names, interrupt-parent, interrupts properties of the
> m4_rproc into stm32mp151.dtsi to deduplicate multiple copies of the same
> in multiple board files. Worse, those copies were starting to get out of sync,
> so this should prevent any such issues in the future.


Theses declarations depend on the coprocessor firmware more than the board itself.
All are optional:
- The IRQ is used for the Coprocessor Watchdog
- The mailboxes for the RPMsg communication and a graceful shutdown of the
coprocessor

So, except for the "detach" mailbox this proposal seems reasonable to me.

Thanks,
Arnaud


> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Richard Cochran <richardcochran@gmail.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: kernel@dh-electronics.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> ---
>  arch/arm/boot/dts/stm32mp151.dtsi                        | 4 ++++
>  arch/arm/boot/dts/stm32mp157c-ed1.dts                    | 4 ----
>  arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi         | 4 ----
>  arch/arm/boot/dts/stm32mp157c-odyssey-som.dtsi           | 4 ----
>  arch/arm/boot/dts/stm32mp157c-phycore-stm32mp15-som.dtsi | 4 ----
>  arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi             | 4 ----
>  arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi             | 4 ----
>  arch/arm/boot/dts/stm32mp15xx-dkx.dtsi                   | 4 ----
>  arch/arm/boot/dts/stm32mp15xx-osd32.dtsi                 | 4 ----
>  9 files changed, 4 insertions(+), 32 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/stm32mp151.dtsi
> b/arch/arm/boot/dts/stm32mp151.dtsi
> index accbeef4df6da..97d54bf0dcc30 100644
> --- a/arch/arm/boot/dts/stm32mp151.dtsi
> +++ b/arch/arm/boot/dts/stm32mp151.dtsi
> @@ -1824,6 +1824,10 @@ m4_rproc: m4@10000000 {
>  			reg = <0x10000000 0x40000>,
>  			      <0x30000000 0x40000>,
>  			      <0x38000000 0x10000>;
> +			interrupt-parent = <&exti>;
> +			interrupts = <68 IRQ_TYPE_EDGE_RISING>;
> +			mbox-names = "vq0", "vq1", "shutdown", "detach";
> +			mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc
> 3>;
>  			resets = <&rcc MCU_R>;
>  			st,syscfg-holdboot = <&rcc 0x10C 0x1>;
>  			st,syscfg-tz = <&rcc 0x000 0x1>;
> diff --git a/arch/arm/boot/dts/stm32mp157c-ed1.dts
> b/arch/arm/boot/dts/stm32mp157c-ed1.dts
> index 8beb901be5065..3b40c2d8c3d9e 100644
> --- a/arch/arm/boot/dts/stm32mp157c-ed1.dts
> +++ b/arch/arm/boot/dts/stm32mp157c-ed1.dts
> @@ -304,10 +304,6 @@ &iwdg2 {
>  &m4_rproc {
>  	memory-region = <&retram>, <&mcuram>, <&mcuram2>,
> <&vdev0vring0>,
>  			<&vdev0vring1>, <&vdev0buffer>;
> -	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
> -	mbox-names = "vq0", "vq1", "shutdown", "detach";
> -	interrupt-parent = <&exti>;
> -	interrupts = <68 1>;
>  	status = "okay";
>  };
> 
> diff --git a/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
> b/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
> index 82061c9186338..a5c86bba46aea 100644
> --- a/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
> +++ b/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
> @@ -366,10 +366,6 @@ &iwdg2 {
>  &m4_rproc {
>  	memory-region = <&retram>, <&mcuram>, <&mcuram2>,
> <&vdev0vring0>,
>  			<&vdev0vring1>, <&vdev0buffer>;
> -	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
> -	mbox-names = "vq0", "vq1", "shutdown", "detach";
> -	interrupt-parent = <&exti>;
> -	interrupts = <68 1>;
>  	interrupt-names = "wdg";
>  	recovery;
>  	status = "okay";
> diff --git a/arch/arm/boot/dts/stm32mp157c-odyssey-som.dtsi
> b/arch/arm/boot/dts/stm32mp157c-odyssey-som.dtsi
> index cf74852514906..31d7bfe8bf8c9 100644
> --- a/arch/arm/boot/dts/stm32mp157c-odyssey-som.dtsi
> +++ b/arch/arm/boot/dts/stm32mp157c-odyssey-som.dtsi
> @@ -230,10 +230,6 @@ &iwdg2 {
>  &m4_rproc {
>  	memory-region = <&retram>, <&mcuram>, <&mcuram2>,
> <&vdev0vring0>,
>  			<&vdev0vring1>, <&vdev0buffer>;
> -	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
> -	mbox-names = "vq0", "vq1", "shutdown", "detach";
> -	interrupt-parent = <&exti>;
> -	interrupts = <68 1>;
>  	status = "okay";
>  };
> 
> diff --git a/arch/arm/boot/dts/stm32mp157c-phycore-stm32mp15-som.dtsi
> b/arch/arm/boot/dts/stm32mp157c-phycore-stm32mp15-som.dtsi
> index 4e8b2d2b30c7a..f68aaf6aa9fb5 100644
> --- a/arch/arm/boot/dts/stm32mp157c-phycore-stm32mp15-som.dtsi
> +++ b/arch/arm/boot/dts/stm32mp157c-phycore-stm32mp15-som.dtsi
> @@ -405,10 +405,6 @@ &m_can2 {
>  &m4_rproc {
>  	memory-region = <&retram>, <&mcuram>, <&mcuram2>,
> <&vdev0vring0>,
>  			<&vdev0vring1>, <&vdev0buffer>;
> -	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
> -	mbox-names = "vq0", "vq1", "shutdown", "detach";
> -	interrupt-parent = <&exti>;
> -	interrupts = <68 1>;
>  	status = "okay";
>  };
> 
> diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
> b/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
> index 7bf13183437c5..a38009f8456b8 100644
> --- a/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
> +++ b/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
> @@ -414,10 +414,6 @@ &iwdg2 {
>  &m4_rproc {
>  	memory-region = <&retram>, <&mcuram>, <&mcuram2>,
> <&vdev0vring0>,
>  			<&vdev0vring1>, <&vdev0buffer>;
> -	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
> -	mbox-names = "vq0", "vq1", "shutdown", "detach";
> -	interrupt-parent = <&exti>;
> -	interrupts = <68 1>;
>  	status = "okay";
>  };
> 
> diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
> b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
> index f0351f599a508..8c30cecacaf86 100644
> --- a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
> +++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
> @@ -227,10 +227,6 @@ &iwdg2 {
>  &m4_rproc {
>  	memory-region = <&retram>, <&mcuram>, <&mcuram2>,
> <&vdev0vring0>,
>  			<&vdev0vring1>, <&vdev0buffer>;
> -	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
> -	mbox-names = "vq0", "vq1", "shutdown", "detach";
> -	interrupt-parent = <&exti>;
> -	interrupts = <68 1>;
>  	status = "okay";
>  };
> 
> diff --git a/arch/arm/boot/dts/stm32mp15xx-dkx.dtsi
> b/arch/arm/boot/dts/stm32mp15xx-dkx.dtsi
> index 0f1110e42c939..cc3eb755663fd 100644
> --- a/arch/arm/boot/dts/stm32mp15xx-dkx.dtsi
> +++ b/arch/arm/boot/dts/stm32mp15xx-dkx.dtsi
> @@ -467,10 +467,6 @@ ltdc_ep0_out: endpoint@0 {  &m4_rproc {
>  	memory-region = <&retram>, <&mcuram>, <&mcuram2>,
> <&vdev0vring0>,
>  			<&vdev0vring1>, <&vdev0buffer>;
> -	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
> -	mbox-names = "vq0", "vq1", "shutdown", "detach";
> -	interrupt-parent = <&exti>;
> -	interrupts = <68 1>;
>  	status = "okay";
>  };
> 
> diff --git a/arch/arm/boot/dts/stm32mp15xx-osd32.dtsi
> b/arch/arm/boot/dts/stm32mp15xx-osd32.dtsi
> index a43965c86fe8b..6532726502c32 100644
> --- a/arch/arm/boot/dts/stm32mp15xx-osd32.dtsi
> +++ b/arch/arm/boot/dts/stm32mp15xx-osd32.dtsi
> @@ -210,10 +210,6 @@ &ipcc {
>  &m4_rproc {
>  	memory-region = <&retram>, <&mcuram>, <&mcuram2>,
> <&vdev0vring0>,
>  			<&vdev0vring1>, <&vdev0buffer>;
> -	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
> -	mbox-names = "vq0", "vq1", "shutdown", "detach";
> -	interrupt-parent = <&exti>;
> -	interrupts = <68 1>;
>  	status = "okay";
>  };
> 
> --
> 2.39.2
> 
> _______________________________________________
> Linux-stm32 mailing list
> Linux-stm32@st-md-mailman.stormreply.com
> https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32

RE: [Linux-stm32] [PATCH 1/5] ARM: dts: stm32: Add missing detach mailbox for emtrion emSBC-Argon

[Arnaud POULIQUEN]

Hello Marek,


ST Restricted

> -----Original Message-----
> From: Linux-stm32 <linux-stm32-bounces@st-md-mailman.stormreply.com>
> On Behalf Of Marek Vasut
> Sent: Thursday, May 18, 2023 3:13 AM
> To: linux-arm-kernel@lists.infradead.org
> Cc: Marek Vasut <marex@denx.de>; devicetree@vger.kernel.org; Conor
> Dooley <conor+dt@kernel.org>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@linaro.org>; Richard Cochran
> <richardcochran@gmail.com>; Rob Herring <robh+dt@kernel.org>; Maxime
> Coquelin <mcoquelin.stm32@gmail.com>; linux-stm32@st-md-
> mailman.stormreply.com; kernel@dh-electronics.com
> Subject: [Linux-stm32] [PATCH 1/5] ARM: dts: stm32: Add missing detach
> mailbox for emtrion emSBC-Argon
> 
> Add missing "detach" mailbox to this board to permit the CPU to inform the
> remote processor on a detach. This signal allows the remote processor
> firmware to stop IPC communication and to reinitialize the resources for a
> re-attach.
> 
> Without this mailbox, detach is not possible and kernel log contains the
> following warning to, so make sure all the STM32MP15xx platform DTs are in
> sync regarding the mailboxes to fix the detach issue and the warning:
> "
> stm32-rproc 10000000.m4: mbox_request_channel_byname() could not
> locate channel named "detach"
> "
> 
> Fixes: 6257dfc1c412 ("ARM: dts: stm32: Add coprocessor detach mbox on
> stm32mp15x-dkx boards")
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Richard Cochran <richardcochran@gmail.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: kernel@dh-electronics.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> ---
>  arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
> b/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
> index b01470a9a3d53..82061c9186338 100644
> --- a/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
> +++ b/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
> @@ -366,8 +366,8 @@ &iwdg2 {
>  &m4_rproc {
>  	memory-region = <&retram>, <&mcuram>, <&mcuram2>,
> <&vdev0vring0>,
>  			<&vdev0vring1>, <&vdev0buffer>;
> -	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>;
> -	mbox-names = "vq0", "vq1", "shutdown";
> +	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
> +	mbox-names = "vq0", "vq1", "shutdown", "detach";

Why do you want to add the detach mailbox? 
It looks to me here that you want to clean the warning message, right?

The detach is used in a particular usecase where the main processor
is  shutdown while the coprocessor is still running.
I would prefer to not enable it by default as it need a specific
coprocessor Firmware.

Rather than adding unused optional mailbox, I will more in favor
of having a mbox_request_channel_byname_optional helper or
something similar

Regards
Arnaud 

 

>  	interrupt-parent = <&exti>;
>  	interrupts = <68 1>;
>  	interrupt-names = "wdg";
> --
> 2.39.2
> 
> _______________________________________________
> Linux-stm32 mailing list
> Linux-stm32@st-md-mailman.stormreply.com
> https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32

Re: [Linux-stm32] [PATCH 1/5] ARM: dts: stm32: Add missing detach mailbox for emtrion emSBC-Argon

[Marek Vasut]

On 5/30/23 10:43, Arnaud POULIQUEN wrote:
> Hello Marek,

Hi,

> ST Restricted
> 
>> -----Original Message-----
>> From: Linux-stm32 <linux-stm32-bounces@st-md-mailman.stormreply.com>
>> On Behalf Of Marek Vasut
>> Sent: Thursday, May 18, 2023 3:13 AM
>> To: linux-arm-kernel@lists.infradead.org
>> Cc: Marek Vasut <marex@denx.de>; devicetree@vger.kernel.org; Conor
>> Dooley <conor+dt@kernel.org>; Krzysztof Kozlowski
>> <krzysztof.kozlowski+dt@linaro.org>; Richard Cochran
>> <richardcochran@gmail.com>; Rob Herring <robh+dt@kernel.org>; Maxime
>> Coquelin <mcoquelin.stm32@gmail.com>; linux-stm32@st-md-
>> mailman.stormreply.com; kernel@dh-electronics.com
>> Subject: [Linux-stm32] [PATCH 1/5] ARM: dts: stm32: Add missing detach
>> mailbox for emtrion emSBC-Argon
>>
>> Add missing "detach" mailbox to this board to permit the CPU to inform the
>> remote processor on a detach. This signal allows the remote processor
>> firmware to stop IPC communication and to reinitialize the resources for a
>> re-attach.
>>
>> Without this mailbox, detach is not possible and kernel log contains the
>> following warning to, so make sure all the STM32MP15xx platform DTs are in
>> sync regarding the mailboxes to fix the detach issue and the warning:
>> "
>> stm32-rproc 10000000.m4: mbox_request_channel_byname() could not
>> locate channel named "detach"
>> "
>>
>> Fixes: 6257dfc1c412 ("ARM: dts: stm32: Add coprocessor detach mbox on
>> stm32mp15x-dkx boards")
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> ---
>> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
>> Cc: Conor Dooley <conor+dt@kernel.org>
>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
>> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>> Cc: Richard Cochran <richardcochran@gmail.com>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: devicetree@vger.kernel.org
>> Cc: kernel@dh-electronics.com
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-stm32@st-md-mailman.stormreply.com
>> ---
>>   arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
>> b/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
>> index b01470a9a3d53..82061c9186338 100644
>> --- a/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
>> +++ b/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
>> @@ -366,8 +366,8 @@ &iwdg2 {
>>   &m4_rproc {
>>   	memory-region = <&retram>, <&mcuram>, <&mcuram2>,
>> <&vdev0vring0>,
>>   			<&vdev0vring1>, <&vdev0buffer>;
>> -	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>;
>> -	mbox-names = "vq0", "vq1", "shutdown";
>> +	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
>> +	mbox-names = "vq0", "vq1", "shutdown", "detach";
> 
> Why do you want to add the detach mailbox?
> It looks to me here that you want to clean the warning message, right?

Yes

> The detach is used in a particular usecase where the main processor
> is  shutdown while the coprocessor is still running.
> I would prefer to not enable it by default as it need a specific
> coprocessor Firmware.

Why is it enabled by default on ST boards and left out on all other boards ?

Surely the ST evaluation boards can load and run both types of firmware, 
ones which do use the detach mailbox and ones which do not use the 
detach mailbox , right ?

I assume that if the firmware does not use the detach mailbox, then the 
detach mailbox is just ignored and unused, so there is no problem with 
having it described in the DT in any case ?

And if that's the case, then I would much rather prefer to have all the 
boards describe the same set of mailboxes, so they don't diverge . What 
do you think ?

> Rather than adding unused optional mailbox, I will more in favor
> of having a mbox_request_channel_byname_optional helper or
> something similar

See above, I think it is better to have the mailbox described in DT 
always and not use it (the user can always remove it), than to not have 
it described on some boards and have it described on other boards 
(inconsistency).

RE: [Linux-stm32] [PATCH 1/5] ARM: dts: stm32: Add missing detach mailbox for emtrion emSBC-Argon

[Arnaud POULIQUEN]

ST Restricted

> -----Original Message-----
> From: Marek Vasut <marex@denx.de>
> Sent: Tuesday, May 30, 2023 1:51 PM
> To: Arnaud POULIQUEN <arnaud.pouliquen@st.com>; linux-arm-
> kernel@lists.infradead.org
> Cc: devicetree@vger.kernel.org; Conor Dooley <conor+dt@kernel.org>;
> Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Richard Cochran
> <richardcochran@gmail.com>; Rob Herring <robh+dt@kernel.org>; Maxime
> Coquelin <mcoquelin.stm32@gmail.com>; linux-stm32@st-md-
> mailman.stormreply.com; kernel@dh-electronics.com
> Subject: Re: [Linux-stm32] [PATCH 1/5] ARM: dts: stm32: Add missing detach
> mailbox for emtrion emSBC-Argon
> 
> On 5/30/23 10:43, Arnaud POULIQUEN wrote:
> > Hello Marek,
> 
> Hi,
> 
> > ST Restricted
> >
> >> -----Original Message-----
> >> From: Linux-stm32 <linux-stm32-bounces@st-md-
> mailman.stormreply.com>
> >> On Behalf Of Marek Vasut
> >> Sent: Thursday, May 18, 2023 3:13 AM
> >> To: linux-arm-kernel@lists.infradead.org
> >> Cc: Marek Vasut <marex@denx.de>; devicetree@vger.kernel.org; Conor
> >> Dooley <conor+dt@kernel.org>; Krzysztof Kozlowski
> >> <krzysztof.kozlowski+dt@linaro.org>; Richard Cochran
> >> <richardcochran@gmail.com>; Rob Herring <robh+dt@kernel.org>;
> Maxime
> >> Coquelin <mcoquelin.stm32@gmail.com>; linux-stm32@st-md-
> >> mailman.stormreply.com; kernel@dh-electronics.com
> >> Subject: [Linux-stm32] [PATCH 1/5] ARM: dts: stm32: Add missing
> >> detach mailbox for emtrion emSBC-Argon
> >>
> >> Add missing "detach" mailbox to this board to permit the CPU to
> >> inform the remote processor on a detach. This signal allows the
> >> remote processor firmware to stop IPC communication and to
> >> reinitialize the resources for a re-attach.
> >>
> >> Without this mailbox, detach is not possible and kernel log contains
> >> the following warning to, so make sure all the STM32MP15xx platform
> >> DTs are in sync regarding the mailboxes to fix the detach issue and the
> warning:
> >> "
> >> stm32-rproc 10000000.m4: mbox_request_channel_byname() could not
> >> locate channel named "detach"
> >> "
> >>
> >> Fixes: 6257dfc1c412 ("ARM: dts: stm32: Add coprocessor detach mbox on
> >> stm32mp15x-dkx boards")
> >> Signed-off-by: Marek Vasut <marex@denx.de>
> >> ---
> >> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
> >> Cc: Conor Dooley <conor+dt@kernel.org>
> >> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> >> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> >> Cc: Richard Cochran <richardcochran@gmail.com>
> >> Cc: Rob Herring <robh+dt@kernel.org>
> >> Cc: devicetree@vger.kernel.org
> >> Cc: kernel@dh-electronics.com
> >> Cc: linux-arm-kernel@lists.infradead.org
> >> Cc: linux-stm32@st-md-mailman.stormreply.com
> >> ---
> >>   arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
> >> b/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
> >> index b01470a9a3d53..82061c9186338 100644
> >> --- a/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
> >> +++ b/arch/arm/boot/dts/stm32mp157c-emstamp-argon.dtsi
> >> @@ -366,8 +366,8 @@ &iwdg2 {
> >>   &m4_rproc {
> >>   	memory-region = <&retram>, <&mcuram>, <&mcuram2>,
> <&vdev0vring0>,
> >>   			<&vdev0vring1>, <&vdev0buffer>;
> >> -	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>;
> >> -	mbox-names = "vq0", "vq1", "shutdown";
> >> +	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>, <&ipcc 3>;
> >> +	mbox-names = "vq0", "vq1", "shutdown", "detach";
> >
> > Why do you want to add the detach mailbox?
> > It looks to me here that you want to clean the warning message, right?
> 
> Yes
> 
> > The detach is used in a particular usecase where the main processor is
> > shutdown while the coprocessor is still running.
> > I would prefer to not enable it by default as it need a specific
> > coprocessor Firmware.
> 
> Why is it enabled by default on ST boards and left out on all other boards ?

Mainly to be able to test the feature on the K2 board . And ensure that the generic
Implementation proposed was compatible. We do not provide demo in the ST
Distribution. Remove it could be an option, but this could ipact legacy some M4 firmware.
 
> 
> Surely the ST evaluation boards can load and run both types of firmware,
> ones which do use the detach mailbox and ones which do not use the detach
> mailbox , right ?

Yes,

> 
> I assume that if the firmware does not use the detach mailbox, then the
> detach mailbox is just ignored and unused, so there is no problem with
> having it described in the DT in any case ?

Yes, The aim of the ST evaluation board is to provide a DT  to a support
different firmwares for demo and tests.  But it is not the case of all boards...
If your boards provide demo using the "detach" it is justified.
If you just add it as a workaround to mask the warnings, you just mask the issue.

> 
> And if that's the case, then I would much rather prefer to have all the boards
> describe the same set of mailboxes, so they don't diverge . What do you
> think ?
> 

I would avoid this.  It is only a configuration by default for current demo.
The allocation depends on the firmware loaded on M4, so depend on the project.
For instance, a work has started in OpenAMP community to implement the vIrtio Services
For the IPC.  Each virtio services would be associated to one or several mailbox
Channels.  In this case we would need to arbitrate allocations.
The result could be that we propose a virtio channel for rpmsg + some other virtio.
More than that we probably manage the mailboxes in sub node
Here is an RFC on the topic (https://lore.kernel.org/lkml/20220920202201.GB1042164@p14s/)

That said, fixing rpmsg virqueue and the shutdown mailboxes in the  SoC dtsi, seems reasonable as it
provides the default expected implementation.
Do the same for the detach that is optional and mainly unused, I'm not fan.
Adding the detach mailbox in the DT to mask a warning issue, I'm rather against it

> > Rather than adding unused optional mailbox, I will more in favor of
> > having a mbox_request_channel_byname_optional helper or something
> > similar
> 
> See above, I think it is better to have the mailbox described in DT always and
> not use it (the user can always remove it), than to not have it described on
> some boards and have it described on other boards (inconsistency).

Adding it in the DT ( and especially in the Soc DTSI) can also be interpreted as
"it is defined so you must use it". I would expect that the Bindings already provide 
the information to help user to add it on need.

Regards,
Arnaud

Re: [Linux-stm32] [PATCH 1/5] ARM: dts: stm32: Add missing detach mailbox for emtrion emSBC-Argon

[Marek Vasut]

On 6/1/23 14:56, Arnaud POULIQUEN wrote:

Hi,

[...]

>> I assume that if the firmware does not use the detach mailbox, then the
>> detach mailbox is just ignored and unused, so there is no problem with
>> having it described in the DT in any case ?
> 
> Yes, The aim of the ST evaluation board is to provide a DT  to a support
> different firmwares for demo and tests.  But it is not the case of all boards...
> If your boards provide demo using the "detach" it is justified.
> If you just add it as a workaround to mask the warnings, you just mask the issue.

Then it seems there is no issue with the boards modified here, because 
as far as I can tell, those are all general purpose SoMs and evaluation 
boards. With such systems, you cannot predict what the user would like 
to use those for, that could include whatever ST demo.

>> And if that's the case, then I would much rather prefer to have all the boards
>> describe the same set of mailboxes, so they don't diverge . What do you
>> think ?
>>
> 
> I would avoid this.  It is only a configuration by default for current demo.

That current demo is restricted to ST produced boards only, or can it 
also be run on development kits manufactured by other vendors ? I think 
it is the later, and I don't see why those should be kept out.

> The allocation depends on the firmware loaded on M4, so depend on the project.
> For instance, a work has started in OpenAMP community to implement the vIrtio Services
> For the IPC.  Each virtio services would be associated to one or several mailbox
> Channels.  In this case we would need to arbitrate allocations.
> The result could be that we propose a virtio channel for rpmsg + some other virtio.
> More than that we probably manage the mailboxes in sub node
> Here is an RFC on the topic (https://lore.kernel.org/lkml/20220920202201.GB1042164@p14s/)
> 
> That said, fixing rpmsg virqueue and the shutdown mailboxes in the  SoC dtsi, seems reasonable as it
> provides the default expected implementation.
> Do the same for the detach that is optional and mainly unused, I'm not fan.
> Adding the detach mailbox in the DT to mask a warning issue, I'm rather against it

Removal of divergence.

>>> Rather than adding unused optional mailbox, I will more in favor of
>>> having a mbox_request_channel_byname_optional helper or something
>>> similar
>>
>> See above, I think it is better to have the mailbox described in DT always and
>> not use it (the user can always remove it), than to not have it described on
>> some boards and have it described on other boards (inconsistency).
> 
> Adding it in the DT ( and especially in the Soc DTSI) can also be interpreted as
> "it is defined so you must use it". I would expect that the Bindings already provide
> the information to help user to add it on need.

Why should every single board add it separately and duplicate the same 
stuff, if they can all start with it, and if anyone needs to tweak the 
mailbox allocation, then they can do that in the board DT ? I really 
don't like the duplication suggestion here.

RE: [Linux-stm32] [PATCH 1/5] ARM: dts: stm32: Add missing detach mailbox for emtrion emSBC-Argon

[Arnaud POULIQUEN]

Hi,

> -----Original Message-----
> From: Marek Vasut <marex@denx.de>
> Sent: Friday, June 2, 2023 4:36 AM
> To: Arnaud POULIQUEN <arnaud.pouliquen@st.com>; linux-arm-
> kernel@lists.infradead.org
> Cc: devicetree@vger.kernel.org; Conor Dooley <conor+dt@kernel.org>;
> Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Richard Cochran
> <richardcochran@gmail.com>; Rob Herring <robh+dt@kernel.org>; Maxime
> Coquelin <mcoquelin.stm32@gmail.com>; linux-stm32@st-md-
> mailman.stormreply.com; kernel@dh-electronics.com
> Subject: Re: [Linux-stm32] [PATCH 1/5] ARM: dts: stm32: Add missing detach
> mailbox for emtrion emSBC-Argon
> 
> On 6/1/23 14:56, Arnaud POULIQUEN wrote:
> 
> Hi,
> 
> [...]
> 
> >> I assume that if the firmware does not use the detach mailbox, then
> >> the detach mailbox is just ignored and unused, so there is no problem
> >> with having it described in the DT in any case ?
> >
> > Yes, The aim of the ST evaluation board is to provide a DT  to a
> > support different firmwares for demo and tests.  But it is not the case of all
> boards...
> > If your boards provide demo using the "detach" it is justified.
> > If you just add it as a workaround to mask the warnings, you just mask the
> issue.
> 
> Then it seems there is no issue with the boards modified here, because as far
> as I can tell, those are all general purpose SoMs and evaluation boards. With
> such systems, you cannot predict what the user would like to use those for,
> that could include whatever ST demo.
> 
> >> And if that's the case, then I would much rather prefer to have all
> >> the boards describe the same set of mailboxes, so they don't diverge
> >> . What do you think ?
> >>
> >
> > I would avoid this.  It is only a configuration by default for current demo.
> 
> That current demo is restricted to ST produced boards only, or can it also be
> run on development kits manufactured by other vendors ? I think it is the
> later, and I don't see why those should be kept out.[] 

ST Demos are boards dependent.

> 
> > The allocation depends on the firmware loaded on M4, so depend on the
> project.
> > For instance, a work has started in OpenAMP community to implement the
> > vIrtio Services For the IPC.  Each virtio services would be associated
> > to one or several mailbox Channels.  In this case we would need to
> arbitrate allocations.
> > The result could be that we propose a virtio channel for rpmsg + some
> other virtio.
> > More than that we probably manage the mailboxes in sub node Here is an
> > RFC on the topic
> > (https://lore.kernel.org/lkml/20220920202201.GB1042164@p14s/)
> >
> > That said, fixing rpmsg virqueue and the shutdown mailboxes in the
> > SoC dtsi, seems reasonable as it provides the default expected
> implementation.
> > Do the same for the detach that is optional and mainly unused, I'm not fan.
> > Adding the detach mailbox in the DT to mask a warning issue, I'm
> > rather against it
> 
> Removal of divergence.
> 
> >>> Rather than adding unused optional mailbox, I will more in favor of
> >>> having a mbox_request_channel_byname_optional helper or something
> >>> similar
> >>
> >> See above, I think it is better to have the mailbox described in DT
> >> always and not use it (the user can always remove it), than to not
> >> have it described on some boards and have it described on other boards
> (inconsistency).
> >
> > Adding it in the DT ( and especially in the Soc DTSI) can also be
> > interpreted as "it is defined so you must use it". I would expect that
> > the Bindings already provide the information to help user to add it on need.
> 
> Why should every single board add it separately and duplicate the same stuff,
> if they can all start with it, and if anyone needs to tweak the mailbox
> allocation, then they can do that in the board DT ? I really don't like the
> duplication suggestion here.

I was speaking about "detach mailbox. Here is what I would like to propose to
you

1)  move all the mailbox declaration in the DTSI except "detach"
2) don't declare "detach" in boards DTS ( except ST board for legacy compliance)
3) as a next step we will have to fix the unexpected warning on the 
   "detach" mailbox.

Regards,
Arnaud

Re: [Linux-stm32] [PATCH 1/5] ARM: dts: stm32: Add missing detach mailbox for emtrion emSBC-Argon

[Marek Vasut]

On 6/6/23 18:21, Arnaud POULIQUEN wrote:
> Hi,

Hi,

>>>> I assume that if the firmware does not use the detach mailbox, then
>>>> the detach mailbox is just ignored and unused, so there is no problem
>>>> with having it described in the DT in any case ?
>>>
>>> Yes, The aim of the ST evaluation board is to provide a DT  to a
>>> support different firmwares for demo and tests.  But it is not the case of all
>> boards...
>>> If your boards provide demo using the "detach" it is justified.
>>> If you just add it as a workaround to mask the warnings, you just mask the
>> issue.
>>
>> Then it seems there is no issue with the boards modified here, because as far
>> as I can tell, those are all general purpose SoMs and evaluation boards. With
>> such systems, you cannot predict what the user would like to use those for,
>> that could include whatever ST demo.
>>
>>>> And if that's the case, then I would much rather prefer to have all
>>>> the boards describe the same set of mailboxes, so they don't diverge
>>>> . What do you think ?
>>>>
>>>
>>> I would avoid this.  It is only a configuration by default for current demo.
>>
>> That current demo is restricted to ST produced boards only, or can it also be
>> run on development kits manufactured by other vendors ? I think it is the
>> later, and I don't see why those should be kept out.[]
> 
> ST Demos are boards dependent.

I was under the impression those demos can be built from this CubeMX 
stuff for any board, all you need is the CubeMX BSP ?

[...]

>>>>> Rather than adding unused optional mailbox, I will more in favor of
>>>>> having a mbox_request_channel_byname_optional helper or something
>>>>> similar
>>>>
>>>> See above, I think it is better to have the mailbox described in DT
>>>> always and not use it (the user can always remove it), than to not
>>>> have it described on some boards and have it described on other boards
>> (inconsistency).
>>>
>>> Adding it in the DT ( and especially in the Soc DTSI) can also be
>>> interpreted as "it is defined so you must use it". I would expect that
>>> the Bindings already provide the information to help user to add it on need.
>>
>> Why should every single board add it separately and duplicate the same stuff,
>> if they can all start with it, and if anyone needs to tweak the mailbox
>> allocation, then they can do that in the board DT ? I really don't like the
>> duplication suggestion here.
> 
> I was speaking about "detach mailbox. Here is what I would like to propose to
> you
> 
> 1)  move all the mailbox declaration in the DTSI except "detach"
> 2) don't declare "detach" in boards DTS ( except ST board for legacy compliance)
> 3) as a next step we will have to fix the unexpected warning on the
>     "detach" mailbox.

Why not make the mailbox available by default on all boards ?

As far as I can tell, if the software is not using the detach mailbox, 
there is no downside, it would just be unused. User can always remove it 
in their DT if really needed.

I believe once can build demos using the detach mailbox for boards with 
stm32mp15xx not manufactured by ST, correct ?

[...]

RE: [Linux-stm32] [PATCH 1/5] ARM: dts: stm32: Add missing detach mailbox for emtrion emSBC-Argon

[Arnaud POULIQUEN]

> -----Original Message-----
> From: Marek Vasut <marex@denx.de>
> Sent: Tuesday, June 6, 2023 7:28 PM
> To: Arnaud POULIQUEN <arnaud.pouliquen@st.com>; linux-arm-
> kernel@lists.infradead.org
> Cc: devicetree@vger.kernel.org; Conor Dooley <conor+dt@kernel.org>;
> Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Richard Cochran
> <richardcochran@gmail.com>; Rob Herring <robh+dt@kernel.org>; Maxime
> Coquelin <mcoquelin.stm32@gmail.com>; linux-stm32@st-md-
> mailman.stormreply.com; kernel@dh-electronics.com
> Subject: Re: [Linux-stm32] [PATCH 1/5] ARM: dts: stm32: Add missing detach
> mailbox for emtrion emSBC-Argon
> 
> On 6/6/23 18:21, Arnaud POULIQUEN wrote:
> > Hi,
> 
> Hi,
> 
> >>>> I assume that if the firmware does not use the detach mailbox, then
> >>>> the detach mailbox is just ignored and unused, so there is no
> >>>> problem with having it described in the DT in any case ?
> >>>
> >>> Yes, The aim of the ST evaluation board is to provide a DT  to a
> >>> support different firmwares for demo and tests.  But it is not the
> >>> case of all
> >> boards...
> >>> If your boards provide demo using the "detach" it is justified.
> >>> If you just add it as a workaround to mask the warnings, you just
> >>> mask the
> >> issue.
> >>
> >> Then it seems there is no issue with the boards modified here,
> >> because as far as I can tell, those are all general purpose SoMs and
> >> evaluation boards. With such systems, you cannot predict what the
> >> user would like to use those for, that could include whatever ST demo.
> >>
> >>>> And if that's the case, then I would much rather prefer to have all
> >>>> the boards describe the same set of mailboxes, so they don't
> >>>> diverge . What do you think ?
> >>>>
> >>>
> >>> I would avoid this.  It is only a configuration by default for current demo.
> >>
> >> That current demo is restricted to ST produced boards only, or can it
> >> also be run on development kits manufactured by other vendors ? I
> >> think it is the later, and I don't see why those should be kept
> >> out.[]
> >
> > ST Demos are boards dependent.
> 
> I was under the impression those demos can be built from this CubeMX stuff
> for any board, all you need is the CubeMX BSP ?
> 
> [...]
> 
> >>>>> Rather than adding unused optional mailbox, I will more in favor
> >>>>> of having a mbox_request_channel_byname_optional helper or
> >>>>> something similar
> >>>>
> >>>> See above, I think it is better to have the mailbox described in DT
> >>>> always and not use it (the user can always remove it), than to not
> >>>> have it described on some boards and have it described on other
> >>>> boards
> >> (inconsistency).
> >>>
> >>> Adding it in the DT ( and especially in the Soc DTSI) can also be
> >>> interpreted as "it is defined so you must use it". I would expect
> >>> that the Bindings already provide the information to help user to add it
> on need.
> >>
> >> Why should every single board add it separately and duplicate the
> >> same stuff, if they can all start with it, and if anyone needs to
> >> tweak the mailbox allocation, then they can do that in the board DT ?
> >> I really don't like the duplication suggestion here.
> >
> > I was speaking about "detach mailbox. Here is what I would like to
> > propose to you
> >
> > 1)  move all the mailbox declaration in the DTSI except "detach"
> > 2) don't declare "detach" in boards DTS ( except ST board for legacy
> > compliance)
> > 3) as a next step we will have to fix the unexpected warning on the
> >     "detach" mailbox.
> 
> Why not make the mailbox available by default on all boards ?

It has been introduced mainly to test the detach feature,
on a second platform to help remoteproc maintainers for the review
process. But the feature is not fully implemented on stm32mp1
( auto reboot of thye M4 on crash, appropriate resource table clean-up,...)
I would prefer to remove it in ST board DT than add it in the DTSI.
That said as mentioned for legacy support, better to keep for ST board.

> 
> As far as I can tell, if the software is not using the detach mailbox, there is no
> downside, it would just be unused. User can always remove it in their DT if
> really needed.

The inverse it true, User can add it in their DT if really need.

> 
> I believe once can build demos using the detach mailbox for boards with
> stm32mp15xx not manufactured by ST, correct ?[] 

Everything could be possible...
Once can want to replace the shutdown mailbox by the detach. 
Once can also build demo using the detach mailbox channel for another purpose.

I quite confuse why you insist to declare this detach mailbox in the DTSI?
Is there a strong constraint on your side?

Regards,
Arnaud

> 
> [...]

Re: [Linux-stm32] [PATCH 1/5] ARM: dts: stm32: Add missing detach mailbox for emtrion emSBC-Argon

[Marek Vasut]

On 6/7/23 11:53, Arnaud POULIQUEN wrote:

Hi,

[...]

>>>>>>> Rather than adding unused optional mailbox, I will more in favor
>>>>>>> of having a mbox_request_channel_byname_optional helper or
>>>>>>> something similar
>>>>>>
>>>>>> See above, I think it is better to have the mailbox described in DT
>>>>>> always and not use it (the user can always remove it), than to not
>>>>>> have it described on some boards and have it described on other
>>>>>> boards
>>>> (inconsistency).
>>>>>
>>>>> Adding it in the DT ( and especially in the Soc DTSI) can also be
>>>>> interpreted as "it is defined so you must use it". I would expect
>>>>> that the Bindings already provide the information to help user to add it
>> on need.
>>>>
>>>> Why should every single board add it separately and duplicate the
>>>> same stuff, if they can all start with it, and if anyone needs to
>>>> tweak the mailbox allocation, then they can do that in the board DT ?
>>>> I really don't like the duplication suggestion here.
>>>
>>> I was speaking about "detach mailbox. Here is what I would like to
>>> propose to you
>>>
>>> 1)  move all the mailbox declaration in the DTSI except "detach"
>>> 2) don't declare "detach" in boards DTS ( except ST board for legacy
>>> compliance)
>>> 3) as a next step we will have to fix the unexpected warning on the
>>>      "detach" mailbox.
>>
>> Why not make the mailbox available by default on all boards ?
> 
> It has been introduced mainly to test the detach feature,
> on a second platform to help remoteproc maintainers for the review
> process. But the feature is not fully implemented on stm32mp1
> ( auto reboot of thye M4 on crash, appropriate resource table clean-up,...)

This is a driver bug, unrelated to DT description, please just fix it.

> I would prefer to remove it in ST board DT than add it in the DTSI.
> That said as mentioned for legacy support, better to keep for ST board.

Why not remove it from ST boards if this was legacy test feature and is 
no longer needed ?

>> As far as I can tell, if the software is not using the detach mailbox, there is no
>> downside, it would just be unused. User can always remove it in their DT if
>> really needed.
> 
> The inverse it true, User can add it in their DT if really need.

Is there a downside if this is enabled by default, yes or no ?

>> I believe once can build demos using the detach mailbox for boards with
>> stm32mp15xx not manufactured by ST, correct ?[]
> 
> Everything could be possible...
> Once can want to replace the shutdown mailbox by the detach.
> Once can also build demo using the detach mailbox channel for another purpose.
> 
> I quite confuse why you insist to declare this detach mailbox in the DTSI?
> Is there a strong constraint on your side?

I just don't see any explanation why ST board(s) should be somehow 
special and have the detach mailbox described in DT by default, while 
other boards would require separate DT patch to use the detach mailbox 
first. That just reduces usability of non-ST-manufactured boards and 
increases fragmentation. I do not like that.

Re: [Linux-stm32] [PATCH 1/5] ARM: dts: stm32: Add missing detach mailbox for emtrion emSBC-Argon

[Arnaud POULIQUEN]

On 6/10/23 15:46, Marek Vasut wrote:
> On 6/7/23 11:53, Arnaud POULIQUEN wrote:
> 
> Hi,
> 
> [...]
> 
>>>>>>>> Rather than adding unused optional mailbox, I will more in favor
>>>>>>>> of having a mbox_request_channel_byname_optional helper or
>>>>>>>> something similar
>>>>>>>
>>>>>>> See above, I think it is better to have the mailbox described in DT
>>>>>>> always and not use it (the user can always remove it), than to not
>>>>>>> have it described on some boards and have it described on other
>>>>>>> boards
>>>>> (inconsistency).
>>>>>>
>>>>>> Adding it in the DT ( and especially in the Soc DTSI) can also be
>>>>>> interpreted as "it is defined so you must use it". I would expect
>>>>>> that the Bindings already provide the information to help user to add it
>>> on need.
>>>>>
>>>>> Why should every single board add it separately and duplicate the
>>>>> same stuff, if they can all start with it, and if anyone needs to
>>>>> tweak the mailbox allocation, then they can do that in the board DT ?
>>>>> I really don't like the duplication suggestion here.
>>>>
>>>> I was speaking about "detach mailbox. Here is what I would like to
>>>> propose to you
>>>>
>>>> 1)  move all the mailbox declaration in the DTSI except "detach"
>>>> 2) don't declare "detach" in boards DTS ( except ST board for legacy
>>>> compliance)
>>>> 3) as a next step we will have to fix the unexpected warning on the
>>>>      "detach" mailbox.
>>>
>>> Why not make the mailbox available by default on all boards ?
>>
>> It has been introduced mainly to test the detach feature,
>> on a second platform to help remoteproc maintainers for the review
>> process. But the feature is not fully implemented on stm32mp1
>> ( auto reboot of thye M4 on crash, appropriate resource table clean-up,...)
> 
> This is a driver bug, unrelated to DT description, please just fix it.

No, it is a system usecase that depends on Hardware, M4 firmware, A7
applications, ...
The detach/re-attach is a quite complex feature that needs to understand
the whole picture. As already mentioned above it as been introduced for
test on upstream.

> 
>> I would prefer to remove it in ST board DT than add it in the DTSI.
>> That said as mentioned for legacy support, better to keep for ST board.
> 
> Why not remove it from ST boards if this was legacy test feature and is no
> longer needed ?

If you can guaranty that this will not introduce regression on legacy, yes we can.

> 
>>> As far as I can tell, if the software is not using the detach mailbox, there
>>> is no
>>> downside, it would just be unused. User can always remove it in their DT if
>>> really needed.
>>
>> The inverse it true, User can add it in their DT if really need.
> 
> Is there a downside if this is enabled by default, yes or no ?

Yes, by doing this you impose that this channel is reserved for the detach.
making it no more optional.

> 
>>> I believe once can build demos using the detach mailbox for boards with
>>> stm32mp15xx not manufactured by ST, correct ?[]
>>
>> Everything could be possible...
>> Once can want to replace the shutdown mailbox by the detach.
>> Once can also build demo using the detach mailbox channel for another purpose.
>>
>> I quite confuse why you insist to declare this detach mailbox in the DTSI?
>> Is there a strong constraint on your side?
> 
> I just don't see any explanation why ST board(s) should be somehow special and
> have the detach mailbox described in DT by default, while other boards would
> require separate DT patch to use the detach mailbox first. That just reduces
> usability of non-ST-manufactured boards and increases fragmentation. I do not
> like that.

The "somehow special" should only be an internal M4 firmware  allowing to test
the feature to help remoteproc maintainers to verify it on demand.
But I can not know if someone in the community have another firmware using
detach on the ST boards.
Anyway what you propose here is that we impose it for all boards. Some boards
would require separate DT patch to not use it. We just inverse the things...
The difference is that I would not impose something optional.

In term of fragmentation I can only see a specificity for the ST boards.As
already said if you can ensure that this will not generate impact on legacy,
it can be removed from the ST DT.

@Alex: any opinion on that?

Regards
Arnaud



> _______________________________________________
> Linux-stm32 mailing list
> Linux-stm32@st-md-mailman.stormreply.com
> https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32

Re: [Linux-stm32] [PATCH 1/5] ARM: dts: stm32: Add missing detach mailbox for emtrion emSBC-Argon

[Marek Vasut]

On 6/12/23 10:26, Arnaud POULIQUEN wrote:
> 
> 
> On 6/10/23 15:46, Marek Vasut wrote:
>> On 6/7/23 11:53, Arnaud POULIQUEN wrote:
>>
>> Hi,
>>
>> [...]
>>
>>>>>>>>> Rather than adding unused optional mailbox, I will more in favor
>>>>>>>>> of having a mbox_request_channel_byname_optional helper or
>>>>>>>>> something similar
>>>>>>>>
>>>>>>>> See above, I think it is better to have the mailbox described in DT
>>>>>>>> always and not use it (the user can always remove it), than to not
>>>>>>>> have it described on some boards and have it described on other
>>>>>>>> boards
>>>>>> (inconsistency).
>>>>>>>
>>>>>>> Adding it in the DT ( and especially in the Soc DTSI) can also be
>>>>>>> interpreted as "it is defined so you must use it". I would expect
>>>>>>> that the Bindings already provide the information to help user to add it
>>>> on need.
>>>>>>
>>>>>> Why should every single board add it separately and duplicate the
>>>>>> same stuff, if they can all start with it, and if anyone needs to
>>>>>> tweak the mailbox allocation, then they can do that in the board DT ?
>>>>>> I really don't like the duplication suggestion here.
>>>>>
>>>>> I was speaking about "detach mailbox. Here is what I would like to
>>>>> propose to you
>>>>>
>>>>> 1)  move all the mailbox declaration in the DTSI except "detach"
>>>>> 2) don't declare "detach" in boards DTS ( except ST board for legacy
>>>>> compliance)
>>>>> 3) as a next step we will have to fix the unexpected warning on the
>>>>>       "detach" mailbox.
>>>>
>>>> Why not make the mailbox available by default on all boards ?
>>>
>>> It has been introduced mainly to test the detach feature,
>>> on a second platform to help remoteproc maintainers for the review
>>> process. But the feature is not fully implemented on stm32mp1
>>> ( auto reboot of thye M4 on crash, appropriate resource table clean-up,...)
>>
>> This is a driver bug, unrelated to DT description, please just fix it.
> 
> No, it is a system usecase that depends on Hardware, M4 firmware, A7
> applications, ...
> The detach/re-attach is a quite complex feature that needs to understand
> the whole picture. As already mentioned above it as been introduced for
> test on upstream.
> 
>>
>>> I would prefer to remove it in ST board DT than add it in the DTSI.
>>> That said as mentioned for legacy support, better to keep for ST board.
>>
>> Why not remove it from ST boards if this was legacy test feature and is no
>> longer needed ?
> 
> If you can guaranty that this will not introduce regression on legacy, yes we can.

Then clearly the only way to avoid this fragmentation is to add it on 
all boards.

>>>> As far as I can tell, if the software is not using the detach mailbox, there
>>>> is no
>>>> downside, it would just be unused. User can always remove it in their DT if
>>>> really needed.
>>>
>>> The inverse it true, User can add it in their DT if really need.
>>
>> Is there a downside if this is enabled by default, yes or no ?
> 
> Yes, by doing this you impose that this channel is reserved for the detach.
> making it no more optional.

Not really, the user can still define whatever channels they need for 
their custom setup in their DT. The SoC DTSI should be just a default.

>>>> I believe once can build demos using the detach mailbox for boards with
>>>> stm32mp15xx not manufactured by ST, correct ?[]
>>>
>>> Everything could be possible...
>>> Once can want to replace the shutdown mailbox by the detach.
>>> Once can also build demo using the detach mailbox channel for another purpose.
>>>
>>> I quite confuse why you insist to declare this detach mailbox in the DTSI?
>>> Is there a strong constraint on your side?
>>
>> I just don't see any explanation why ST board(s) should be somehow special and
>> have the detach mailbox described in DT by default, while other boards would
>> require separate DT patch to use the detach mailbox first. That just reduces
>> usability of non-ST-manufactured boards and increases fragmentation. I do not
>> like that.
> 
> The "somehow special" should only be an internal M4 firmware  allowing to test
> the feature to help remoteproc maintainers to verify it on demand.
> But I can not know if someone in the community have another firmware using
> detach on the ST boards.
> Anyway what you propose here is that we impose it for all boards. Some boards
> would require separate DT patch to not use it. We just inverse the things...
> The difference is that I would not impose something optional.

I believe it is better to have single capable consistent default and let 
users remove the capabilities for specific application if needed, than 
to have fragmented inconsistent board-specific configurations where 
users have to first determine whether they need to patch them to add 
missing capabilities, and then possibly patch them, that's just a mess.

It also puts non-ST-manufactured boards into worse position.

> In term of fragmentation I can only see a specificity for the ST boards.As
> already said if you can ensure that this will not generate impact on legacy,
> it can be removed from the ST DT.
> 
> @Alex: any opinion on that?

[...]

Re: [Linux-stm32] [PATCH 1/5] ARM: dts: stm32: Add missing detach mailbox for emtrion emSBC-Argon

[Arnaud POULIQUEN]

On 6/12/23 11:13, Marek Vasut wrote:
> On 6/12/23 10:26, Arnaud POULIQUEN wrote:
>>
>>
>> On 6/10/23 15:46, Marek Vasut wrote:
>>> On 6/7/23 11:53, Arnaud POULIQUEN wrote:
>>>
>>> Hi,
>>>
>>> [...]
>>>
>>>>>>>>>> Rather than adding unused optional mailbox, I will more in favor
>>>>>>>>>> of having a mbox_request_channel_byname_optional helper or
>>>>>>>>>> something similar
>>>>>>>>>
>>>>>>>>> See above, I think it is better to have the mailbox described in DT
>>>>>>>>> always and not use it (the user can always remove it), than to not
>>>>>>>>> have it described on some boards and have it described on other
>>>>>>>>> boards
>>>>>>> (inconsistency).
>>>>>>>>
>>>>>>>> Adding it in the DT ( and especially in the Soc DTSI) can also be
>>>>>>>> interpreted as "it is defined so you must use it". I would expect
>>>>>>>> that the Bindings already provide the information to help user to add it
>>>>> on need.
>>>>>>>
>>>>>>> Why should every single board add it separately and duplicate the
>>>>>>> same stuff, if they can all start with it, and if anyone needs to
>>>>>>> tweak the mailbox allocation, then they can do that in the board DT ?
>>>>>>> I really don't like the duplication suggestion here.
>>>>>>
>>>>>> I was speaking about "detach mailbox. Here is what I would like to
>>>>>> propose to you
>>>>>>
>>>>>> 1)  move all the mailbox declaration in the DTSI except "detach"
>>>>>> 2) don't declare "detach" in boards DTS ( except ST board for legacy
>>>>>> compliance)
>>>>>> 3) as a next step we will have to fix the unexpected warning on the
>>>>>>       "detach" mailbox.
>>>>>
>>>>> Why not make the mailbox available by default on all boards ?
>>>>
>>>> It has been introduced mainly to test the detach feature,
>>>> on a second platform to help remoteproc maintainers for the review
>>>> process. But the feature is not fully implemented on stm32mp1
>>>> ( auto reboot of thye M4 on crash, appropriate resource table clean-up,...)
>>>
>>> This is a driver bug, unrelated to DT description, please just fix it.
>>
>> No, it is a system usecase that depends on Hardware, M4 firmware, A7
>> applications, ...
>> The detach/re-attach is a quite complex feature that needs to understand
>> the whole picture. As already mentioned above it as been introduced for
>> test on upstream.
>>
>>>
>>>> I would prefer to remove it in ST board DT than add it in the DTSI.
>>>> That said as mentioned for legacy support, better to keep for ST board.
>>>
>>> Why not remove it from ST boards if this was legacy test feature and is no
>>> longer needed ?
>>
>> If you can guaranty that this will not introduce regression on legacy, yes we
>> can.
> 
> Then clearly the only way to avoid this fragmentation is to add it on all boards.

You can not avoid fragmentation here, the DT and the Cortex-M4 firmware are
dependent, the M4 firmware is board dependent.
For instance, if we introduce some new mailbox channels to support some virtio
services should we also add it on all boards that embedd stm32mp15 chip..?

For me no, as the M4 Firmware is board dependent.

> 
>>>>> As far as I can tell, if the software is not using the detach mailbox, there
>>>>> is no
>>>>> downside, it would just be unused. User can always remove it in their DT if
>>>>> really needed.
>>>>
>>>> The inverse it true, User can add it in their DT if really need.
>>>
>>> Is there a downside if this is enabled by default, yes or no ?
>>
>> Yes, by doing this you impose that this channel is reserved for the detach.
>> making it no more optional.
> 
> Not really, the user can still define whatever channels they need for their
> custom setup in their DT. The SoC DTSI should be just a default.
> 
>>>>> I believe once can build demos using the detach mailbox for boards with
>>>>> stm32mp15xx not manufactured by ST, correct ?[]
>>>>
>>>> Everything could be possible...
>>>> Once can want to replace the shutdown mailbox by the detach.
>>>> Once can also build demo using the detach mailbox channel for another purpose.
>>>>
>>>> I quite confuse why you insist to declare this detach mailbox in the DTSI?
>>>> Is there a strong constraint on your side?
>>>
>>> I just don't see any explanation why ST board(s) should be somehow special and
>>> have the detach mailbox described in DT by default, while other boards would
>>> require separate DT patch to use the detach mailbox first. That just reduces
>>> usability of non-ST-manufactured boards and increases fragmentation. I do not
>>> like that.
>>
>> The "somehow special" should only be an internal M4 firmware  allowing to test
>> the feature to help remoteproc maintainers to verify it on demand.
>> But I can not know if someone in the community have another firmware using
>> detach on the ST boards.
>> Anyway what you propose here is that we impose it for all boards. Some boards
>> would require separate DT patch to not use it. We just inverse the things...
>> The difference is that I would not impose something optional.
> 
> I believe it is better to have single capable consistent default and let users
> remove the capabilities for specific application if needed, than to have
> fragmented inconsistent board-specific configurations where users have to first
> determine whether they need to patch them to add missing capabilities, and then
> possibly patch them, that's just a mess.


Be aware that to manage a coprocessor firmware this not sufficient so in any
case user will have to patch the DT to assign peripherals to the Cortex-M4,
update he memory regions,...
It is a system usecase, not only the enable of a peripheral.That's why we have
specific DT in downstream for M4 examples, to be able to support more examples
and demos.

> 
> It also puts non-ST-manufactured boards into worse position.

What would you mean by worst position? As there is no example provided that
would take advantage of the "detach", I don't understand your point.

The only argument I can see for generic is that the  proposed settings allow
to run a Zephyr IPC sample, that could be cross-platform.
So I would say we should first extend the M4 zephyr sample to implement the
detach and then that might make sense.

Having said that, I'd rather not spend any more time on this subject.
I've given some arguments, you've given others.
I now propose to let Alex, as maintainer of stm32 DT, decide...

Regards,
Arnaud

> 
>> In term of fragmentation I can only see a specificity for the ST boards.As
>> already said if you can ensure that this will not generate impact on legacy,
>> it can be removed from the ST DT.
>>
>> @Alex: any opinion on that?
> 
> [...]

Re: [Linux-stm32] [PATCH 1/5] ARM: dts: stm32: Add missing detach mailbox for emtrion emSBC-Argon

[Marek Vasut]

On 6/12/23 14:34, Arnaud POULIQUEN wrote:

Hi,

[...]

>>>>> I would prefer to remove it in ST board DT than add it in the DTSI.
>>>>> That said as mentioned for legacy support, better to keep for ST board.
>>>>
>>>> Why not remove it from ST boards if this was legacy test feature and is no
>>>> longer needed ?
>>>
>>> If you can guaranty that this will not introduce regression on legacy, yes we
>>> can.
>>
>> Then clearly the only way to avoid this fragmentation is to add it on all boards.
> 
> You can not avoid fragmentation here, the DT and the Cortex-M4 firmware are
> dependent, the M4 firmware is board dependent.
> For instance, if we introduce some new mailbox channels to support some virtio
> services should we also add it on all boards that embedd stm32mp15 chip..?

Yes, I believe so, as long as one can use cubemx to generate bsp for 
non-ST board which uses that functionality.

> For me no, as the M4 Firmware is board dependent.

[...]

>>>>>> I believe once can build demos using the detach mailbox for boards with
>>>>>> stm32mp15xx not manufactured by ST, correct ?[]
>>>>>
>>>>> Everything could be possible...
>>>>> Once can want to replace the shutdown mailbox by the detach.
>>>>> Once can also build demo using the detach mailbox channel for another purpose.
>>>>>
>>>>> I quite confuse why you insist to declare this detach mailbox in the DTSI?
>>>>> Is there a strong constraint on your side?
>>>>
>>>> I just don't see any explanation why ST board(s) should be somehow special and
>>>> have the detach mailbox described in DT by default, while other boards would
>>>> require separate DT patch to use the detach mailbox first. That just reduces
>>>> usability of non-ST-manufactured boards and increases fragmentation. I do not
>>>> like that.
>>>
>>> The "somehow special" should only be an internal M4 firmware  allowing to test
>>> the feature to help remoteproc maintainers to verify it on demand.
>>> But I can not know if someone in the community have another firmware using
>>> detach on the ST boards.
>>> Anyway what you propose here is that we impose it for all boards. Some boards
>>> would require separate DT patch to not use it. We just inverse the things...
>>> The difference is that I would not impose something optional.
>>
>> I believe it is better to have single capable consistent default and let users
>> remove the capabilities for specific application if needed, than to have
>> fragmented inconsistent board-specific configurations where users have to first
>> determine whether they need to patch them to add missing capabilities, and then
>> possibly patch them, that's just a mess.
> 
> 
> Be aware that to manage a coprocessor firmware this not sufficient so in any
> case user will have to patch the DT to assign peripherals to the Cortex-M4,
> update he memory regions,...

Not necessarily, a lot of the cubemx examples do use the same memory 
layout. So making it easy for user to evaluate the CM4 usage is the goal 
here. And that includes non-ST produced boards.

> It is a system usecase, not only the enable of a peripheral.That's why we have
> specific DT in downstream for M4 examples, to be able to support more examples
> and demos.
> 
>>
>> It also puts non-ST-manufactured boards into worse position.
> 
> What would you mean by worst position? As there is no example provided that
> would take advantage of the "detach", I don't understand your point.
> 
> The only argument I can see for generic is that the  proposed settings allow
> to run a Zephyr IPC sample, that could be cross-platform.
> So I would say we should first extend the M4 zephyr sample to implement the
> detach and then that might make sense.
> 
> Having said that, I'd rather not spend any more time on this subject.
> I've given some arguments, you've given others.
> I now propose to let Alex, as maintainer of stm32 DT, decide...

I agree, let's wait for Alex.