[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.