Re: [PATCH 1/2] dt-bindings: firmware: arm,scmi: Document arm,poll-transport property

[Marek Vasut <marek.vasut@mailbox.org>]

On 12/5/25 10:54 AM, Sudeep Holla wrote:

Hello Sudeep,

>>>>> Also IIUC, the irq request happens
>>>>> as part of channel startup and there are no explicit APIs for the mbox client
>>>>> driver to control that. SCMI is mbox client in this case.
>>>>
>>>> Sure, but the mailbox driver has to make sure it is correctly demuxing the
>>>> IRQs it handles and correctly sends received_data notifications to the right
>>>> channel(s) .
>>>>
>>>
>>> Agreed, but the concern is that if SCMI is forced to use polling when the
>>> channel is opened, and IRQs are enabled by default with no way for SCMI to
>>> disable them in polling mode, we could run into issues.
>>
>> This constellation seems odd -- if the channel can do IRQs, then this
>> property should not be present in DT.
>>
> 
> Yes, but there is no way to validate or check this and that is the root
> cause for all my worries.

Should a configuration like that even be considered valid and relevant ?

>>> I realise it’s a very
>>> specific corner case, but every time I’ve assumed such scenarios wouldn’t
>>> occur, we eventually ended up encountering them. So sorry if I am very
>>> pedantic, but I prefer to start smaller and restrictive and expand if and
>>> when necessary or required only.
>>
>> I don't think this case, where mailbox channel does IRQs and polling is
>> enabled, can/should even be considered valid. Either the channel does not do
>> IRQs and then it should do polling, or it does IRQs and then it should use
>> IRQs, but not both.
>>
> 
> Yes ideally, but having loose ends like this binding which allows someone
> to add it to their DT complicates though it is invalid. We have no way to
> detect and I don't want to work around such configs in the future.

If the DT is invalid, bad things happen, but I would argue that is then 
a DT bug and the DT should be fixed.

[...]

>>> Yes, that’s essential, because polling in an SMC context is meaningless in my
>>> opinion.
>>
>> Maybe the "a2p" IRQ is also used for notifications from longer running
>> operations ?
>>
> 
> Yes, it is some sort of work around some platforms implemented as by design
> when the SMC returns, the synchronous commands must complete and it is had
> to support async SCMI commands without platform specific interrupt(p2a). This
> a2p is sort of completion interrupt for synchronous command. I assume the
> platform may offload the task from secure f/w to something else otherwise
> secure side needs to be given CPU cycles to complete which complicates this.
> In short SMC is synchronous and if the execution returns from it in NS world,
> the command is complete.

Wouldn't polling still be useful for the async case , even in SMC setup?
Note that the SMC setup does use shmem, and therefore can do polling on 
the shmem.

>> [...]
>>
>>>>> Yes it can be minimalistic but not restrictive. As I already clearly mentioned
>>>>> I don't see it makes any sense to enable this for SMC/OPTEE. Lets start with
>>>>> just mailbox to start with and extend to other transports if and when needed.
>>>>> It would be good to impose that restriction in the binding as well but that
>>>>> is not a must IMO. I am fine if the bindings for whatever reasons(though I
>>>>> don't see the need) to apply for any transport.
>>>> So I should simply drop the smc.c changes , keep the rest, and send V2 ?
>>>
>>> Not just that. Unless DT maintainers oppose, I just want to keep this
>>> new property valid only for mailbox transport(i.e. "arm,scmi" compatible
>>> not otherwise) so that we can catch any other use in binding checks and
>>> interested parties must discuss on the list and expand that if they require.
>>>
>>> Also we can explore if we can parse and scan this in mailbox transport for
>>> now.
>> I feel that this only adds more implementation complexity and makes the
>> solution less generic, while it does win us very little in the end ? The
>> generic solution implementation is actually easier to implement.
> 
> Yes I want it less generic to start with. Why you want to start making
> this workaround on your platform a generic implementation just because
> the specification has provision for it ?

Because this is generic kernel code, it seems counterintuitive to 
introduce less generic solution which requires more complex implementation.

Since the DEN0056 specification states this mode of operation is 
supported, I also wouldn't call it a workaround.