devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Marek Vasut <marex@denx.de>
To: Arnaud POULIQUEN <arnaud.pouliquen@st.com>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Cc: "devicetree@vger.kernel.org" <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" 
	<linux-stm32@st-md-mailman.stormreply.com>,
	"kernel@dh-electronics.com" <kernel@dh-electronics.com>
Subject: Re: [Linux-stm32] [PATCH 1/5] ARM: dts: stm32: Add missing detach mailbox for emtrion emSBC-Argon
Date: Sat, 10 Jun 2023 15:46:21 +0200	[thread overview]
Message-ID: <b65288f6-1d3d-424f-5df5-98e86cc51dec@denx.de> (raw)
In-Reply-To: <PAXPR10MB4718E8CE58A881DAE3983A27F153A@PAXPR10MB4718.EURPRD10.PROD.OUTLOOK.COM>

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.

  reply	other threads:[~2023-06-10 13:46 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-18  1:12 [PATCH 1/5] ARM: dts: stm32: Add missing detach mailbox for emtrion emSBC-Argon Marek Vasut
2023-05-18  1:12 ` [PATCH 2/5] ARM: dts: stm32: Add missing detach mailbox for Odyssey SoM Marek Vasut
2023-05-18  1:12 ` [PATCH 3/5] ARM: dts: stm32: Add missing detach mailbox for DHCOM SoM Marek Vasut
2023-05-18  1:12 ` [PATCH 4/5] ARM: dts: stm32: Add missing detach mailbox for DHCOR SoM Marek Vasut
2023-07-11  2:05   ` Marek Vasut
2023-07-11 13:37     ` Alexandre TORGUE
2023-07-11 13:40       ` Marek Vasut
2023-05-18  1:12 ` [PATCH 5/5] ARM: dts: stm32: Deduplicate rproc mboxes and IRQs Marek Vasut
2023-05-30  8:51   ` [Linux-stm32] " Arnaud POULIQUEN
2023-05-30  8:43 ` [Linux-stm32] [PATCH 1/5] ARM: dts: stm32: Add missing detach mailbox for emtrion emSBC-Argon Arnaud POULIQUEN
2023-05-30 11:50   ` Marek Vasut
2023-06-01 12:56     ` Arnaud POULIQUEN
2023-06-02  2:35       ` Marek Vasut
2023-06-06 16:21         ` Arnaud POULIQUEN
2023-06-06 17:28           ` Marek Vasut
2023-06-07  9:53             ` Arnaud POULIQUEN
2023-06-10 13:46               ` Marek Vasut [this message]
2023-06-12  8:26                 ` Arnaud POULIQUEN
2023-06-12  9:13                   ` Marek Vasut
2023-06-12 12:34                     ` Arnaud POULIQUEN
2023-06-17 14:34                       ` Marek Vasut

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=b65288f6-1d3d-424f-5df5-98e86cc51dec@denx.de \
    --to=marex@denx.de \
    --cc=arnaud.pouliquen@st.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=kernel@dh-electronics.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-stm32@st-md-mailman.stormreply.com \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=richardcochran@gmail.com \
    --cc=robh+dt@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).