public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: Sudeep Holla <sudeep.holla@arm.com>
To: Marek Vasut <marek.vasut@mailbox.org>
Cc: Cristian Marussi <cristian.marussi@arm.com>,
	arm-scmi@vger.kernel.org, Conor Dooley <conor+dt@kernel.org>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Florian Fainelli <florian.fainelli@broadcom.com>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Rob Herring <robh@kernel.org>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH 1/2] dt-bindings: firmware: arm,scmi: Document arm,poll-transport property
Date: Wed, 3 Dec 2025 11:41:51 +0000	[thread overview]
Message-ID: <20251203-thick-didactic-cockatoo-deaa1d@sudeepholla> (raw)
In-Reply-To: <8d773671-5e2e-4e21-ade6-2bf9a3b75066@mailbox.org>

On Tue, Dec 02, 2025 at 08:25:11PM +0100, Marek Vasut wrote:
> On 12/2/25 7:55 PM, Sudeep Holla wrote:
> 
> Hello Sudeep,
> 
> > > How do you imagine this -- a transport shared with other components, one
> > > which does generate IRQs and one which does not -- would look like ? Can you
> > > think of an example ?
> > > 
> > 
> > Consider a system where a mailbox controller is present and one channel is
> > used for SCMI communication, while another channel is used for an unrelated
> > purpose. If both channels share the same interrupt line, and the other use
> > case enables interrupt mode on its channel, what would be the impact on the
> > SCMI-specific channel?
> 
> None, SCMI kernel driver and SCMI server side would still do polling on
> their respective SHMEM areas, while whatever kernel driver needs to receive
> the interrupt notifications would subscribe to them using request_irq(),
> right ?
> 

Fair enough. I was thinking if the controller manages to not call
mbox_chan_received_data() in that case. 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.

> > I am aware of systems that implement such sharing, which is why I prefer to be
> > explicit that this type of design is challenging to support within this
> > binding. The intent is to support only minimal, constrained cases - essentially
> > systems that are already somewhat broken. I do not see value in broadening the
> > binding to cover every conceivable scenario.
> > 
> > > > Clearly defining these constraints would be helpful. It may also be useful to
> > > > note that this is primarily intended for mailbox transports, if that’s
> > > > accurate. Alternatively, we could keep the DT binding definition broader but
> > > > emit warnings when a transport other than mailbox is used. That approach might
> > > > make it easier to move forward.
> > > 
> > > DEN0056F refers to this polling mode in Shared memory based transports, that
> > > can be other than mailbox transports, it includes e.g. SMC or OPTEE
> > > transports.
> > > 
> > 
> > However, polling does not make sense in the context of SMC. Once control
> > returns from an SMC call, the command has completed. What form of polling in
> > an SMC workflow do you have in mind?
> 
> I think the polling happens on the SHMEM and the SMC transport is capable of
> that too, see :
> 
> drivers/firmware/arm_scmi/transports/smc.c
> 
> 175         /*
> 176          * If there is an interrupt named "a2p", then the service and
> 177          * completion of a message is signaled by an interrupt rather
> than by
> 178          * the return of the SMC call.
> 179          */
> 180         scmi_info->irq = of_irq_get_byname(cdev->of_node, "a2p");
> 

Ah this one, is actually implemented to avoid sort of implicit polling
mode we get with any SMC/HVC. I don't know how the platform deals with it
but SMC/HVC is synchronous and doesn't need this polling. The irq introduced
here is again a sort of workaround to get some sort of async/non-polling
mode with SMC/HVC. So, to repeat polling mode make absolutely no sense
whatsoever for SMC/OPTEE(based on pure SMC) transports.

> > I believe the same applies to OP-TEE.
> > While OP-TEE now provides a notification mechanism that could, in theory,
> > allow synchronous commands to be treated in a quasi-asynchronous manner, I
> > strongly doubt that the current SCMI-over-OP-TEE implementation behaves this
> > way, given that it ultimately reaches the secure side via an SMC call.
> > 
> > > I don't think a warning is justified, if the behavior follows the
> > > specification. But I do agree the behavior is ... suboptimal.
> > > 
> > 
> > The specification does not address SMC or OP-TEE transports, placing them
> > outside its scope and likewise these DT bindings.
> 
> I believe the shmem transport includes the SMC and OPTEE ones, right ?
>

Yes, but the expectation when the SMC completes is to have the shmem to be
owned by the OS(except that irq workaround case). Again the OPTEE/SMC is
completely out of spec, but I agree the SHMEM behaviour must conform to the
specification.

> > Consequently, what we
> > decide here in this discussion effectively defines the expected behavior in
> > this context, in my view. So I would like to start with minimal possible
> > coverage, why do you think that is not a good idea here ?
> 
> I would argue the current implementation covers pretty much every transport
> which could ever need to do polling on shmem, so the implementation is
> generic and inline with the specification. Also, the current implementation
> is some 20 lines, so I think it is minimalistic?
> 
> What would you propose we do here ?
> 

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.

-- 
Regards,
Sudeep


  reply	other threads:[~2025-12-03 11:42 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-23 12:35 [PATCH 1/2] dt-bindings: firmware: arm,scmi: Document arm,poll-transport property Marek Vasut
2025-10-23 12:35 ` [PATCH 2/2] firmware: arm_scmi: Implement " Marek Vasut
2025-10-23 13:07 ` [PATCH 1/2] dt-bindings: firmware: arm,scmi: Document " Geert Uytterhoeven
2025-10-23 13:19   ` Marek Vasut
2025-10-23 13:16 ` Sudeep Holla
2025-10-23 13:42   ` Marek Vasut
2025-10-23 14:30     ` Cristian Marussi
2025-10-23 14:36       ` Marek Vasut
2025-10-23 14:36     ` Sudeep Holla
2025-10-23 14:47       ` Marek Vasut
2025-10-23 13:42   ` Sudeep Holla
2025-10-23 13:57     ` Marek Vasut
2025-10-23 13:45 ` Cristian Marussi
2025-10-23 14:00   ` Marek Vasut
2025-10-30  0:52     ` Marek Vasut
2025-11-13 11:03       ` Cristian Marussi
2025-11-13 11:34         ` Marek Vasut
2025-11-14  7:21         ` Wolfram Sang
2025-12-02 14:52         ` Sudeep Holla
2025-12-02 16:36           ` Marek Vasut
2025-12-02 18:55             ` Sudeep Holla
2025-12-02 19:25               ` Marek Vasut
2025-12-03 11:41                 ` Sudeep Holla [this message]
2025-12-03 22:53                   ` Marek Vasut
2025-12-04 12:33                     ` Sudeep Holla
2025-12-04 18:59                       ` Marek Vasut
2025-12-05  9:54                         ` Sudeep Holla
2025-12-07  9:16                           ` Marek Vasut
2025-12-08 16:30                             ` Sudeep Holla
2025-12-31 21:00                               ` 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=20251203-thick-didactic-cockatoo-deaa1d@sudeepholla \
    --to=sudeep.holla@arm.com \
    --cc=arm-scmi@vger.kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=cristian.marussi@arm.com \
    --cc=devicetree@vger.kernel.org \
    --cc=florian.fainelli@broadcom.com \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=marek.vasut@mailbox.org \
    --cc=robh@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