Received-SPF: pass (google.com: domain of linux-kernel+bounces-173591-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Wed, 8 May 2024 18:04:00 +0100
From: Cristian Marussi <cristian.marussi@arm.com>
To: "Peng Fan (OSS)" <peng.fan@oss.nxp.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, Peng Fan <peng.fan@nxp.com>
Subject: Re: [PATCH 0/2] firmware: arm_scmi: add notification completion
 channel
Message-ID: <ZjuwgLZ1Sh5jRBWn@pluto>
References: <20240507-scmi-notify-v1-0-1fdefc984d53@nxp.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240507-scmi-notify-v1-0-1fdefc984d53@nxp.com>

On Tue, May 07, 2024 at 11:34:59PM +0800, Peng Fan (OSS) wrote:
> Per spec:

Hi Peng,

thanks for doing this.

> Completion interrupts This transport supports polling or interrupt driven
> modes of communication. In interrupt mode, when the callee completes
> processing a message, it raises an interrupt to the caller. Hardware
> support for completion interrupts is optional.
> 
> i.MX95 SCMI firmware is fully interrupt driven, so Platform expects
> completion interrupt for Platform to Agent(P2A) notifictions.
> 
> Add another optional mailbox channel for Agent to notify Platform that
> notification message has been accepted
> 
> After notification channel status become freed, Agent will use the
> new mailbox channel to send completion interrupt to Platform.
> 
> Add shmem_channel_intr_enabled to check channel flags.

Glancing through the series I think the bindings and the code are
quite good and sensible to achieve the addition of the optional P2A 
completion interrupt.

Your current solution, though, addresses only the case of a mailbox
controller providing unidirectional channels. (which I suppose is the
one you are using on your platform)

We should take care to add such optional P2A completion interrupt support
also for the case of mailboxes sporting bidirectional channels.

For bidirectional channels we really have already the needed bindings...
..no changes there...you have just to also ring the doorbell on that same P2A
if completion IRQs are requested.

IOW, when the DT description is made by 2 mboxes + 2 shmem, means we have
both A2P and P2A provided via bidirectional channels...we currently
just use the P2A (RX) channel to receive notifs/dresp via rx_callback
(so using only the platform_to_agent direction)...now we should also ring
the P2A doorbell on the existing P2A channel if the FLAG_INTR_ENABLED is set
to signal the completion interrupt on the other direction (agent_to_platform)

Not sure if I have been clear or make it worst with my explanation :P

Thanks,
Cristian