[PATCH 0/2] firmware: arm_scmi: add notification completion channel

[PATCH 0/2] firmware: arm_scmi: add notification completion channel

[Peng Fan (OSS)]

Per spec:
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.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Peng Fan (2):
      dt-bindings: firmware: arm,scmi: Support notification completion channel
      firmware: arm_scmi: mailbox: support P2A channel completion

 .../devicetree/bindings/firmware/arm,scmi.yaml     | 12 +++--
 drivers/firmware/arm_scmi/common.h                 |  1 +
 drivers/firmware/arm_scmi/mailbox.c                | 51 +++++++++++++++++++---
 drivers/firmware/arm_scmi/shmem.c                  |  5 +++
 4 files changed, 59 insertions(+), 10 deletions(-)
---
base-commit: 9221b2819b8a4196eecf5476d66201be60fbcf29
change-id: 20240507-scmi-notify-07e87a8b9a23

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] dt-bindings: firmware: arm,scmi: Support notification completion channel

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Per System Control Management Interface specification:
" 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."
So add an optional mailbox channel for notification completion interrupts.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 Documentation/devicetree/bindings/firmware/arm,scmi.yaml | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index 7de2c29606e5..308af58180d1 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -72,14 +72,17 @@ properties:
           - const: tx
           - const: tx_reply
           - const: rx
+          - const: rx_reply
         minItems: 2
 
   mboxes:
     description:
       List of phandle and mailbox channel specifiers. It should contain
-      exactly one, two or three mailboxes; the first one or two for transmitting
-      messages ("tx") and another optional ("rx") for receiving notifications
-      and delayed responses, if supported by the platform.
+      exactly one, two, three or four mailboxes; the first one or two for
+      transmitting messages ("tx") and another optional ("rx") for receiving
+      notifications and delayed responses, if supported by the platform.
+      The optional ("rx_reply") is for notifications completion interrupt,
+      if supported by the platform.
       The number of mailboxes needed for transmitting messages depends on the
       type of channels exposed by the specific underlying mailbox controller;
       one single channel descriptor is enough if such channel is bidirectional,
@@ -92,9 +95,10 @@ properties:
        2 mbox / 2 shmem => SCMI TX and RX over 2 mailbox bidirectional channels
        2 mbox / 1 shmem => SCMI TX over 2 mailbox unidirectional channels
        3 mbox / 2 shmem => SCMI TX and RX over 3 mailbox unidirectional channels
+       4 mbox / 2 shmem => SCMI TX and RX over 4 mailbox unidirectional channels
       Any other combination of mboxes and shmem is invalid.
     minItems: 1
-    maxItems: 3
+    maxItems: 4
 
   shmem:
     description:

-- 
2.37.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] firmware: arm_scmi: mailbox: support P2A channel completion

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

i.MX95 System Manager firmware is fully interrupt driven. The notification
channel needs completion interrupt to drive its notification queue. Without
completion interrupt, the notification will work abnormal.

Add an optional mailbox channel. If the channel flag has INTR set, and
the completion interrupt channel is provided, issue the mbox message
to Platform after the channel is freed.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/firmware/arm_scmi/common.h  |  1 +
 drivers/firmware/arm_scmi/mailbox.c | 51 ++++++++++++++++++++++++++++++++-----
 drivers/firmware/arm_scmi/shmem.c   |  5 ++++
 3 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index b5ac25dbc1ca..4b8c5250cdb5 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -326,6 +326,7 @@ void shmem_clear_channel(struct scmi_shared_mem __iomem *shmem);
 bool shmem_poll_done(struct scmi_shared_mem __iomem *shmem,
 		     struct scmi_xfer *xfer);
 bool shmem_channel_free(struct scmi_shared_mem __iomem *shmem);
+bool shmem_channel_intr_enabled(struct scmi_shared_mem __iomem *shmem);
 
 /* declarations for message passing transports */
 struct scmi_msg_payld;
diff --git a/drivers/firmware/arm_scmi/mailbox.c b/drivers/firmware/arm_scmi/mailbox.c
index 615a3b2ad83d..59a3085ce366 100644
--- a/drivers/firmware/arm_scmi/mailbox.c
+++ b/drivers/firmware/arm_scmi/mailbox.c
@@ -21,6 +21,7 @@
  * @cl: Mailbox Client
  * @chan: Transmit/Receive mailbox uni/bi-directional channel
  * @chan_receiver: Optional Receiver mailbox unidirectional channel
+ * @chan_platform_receiver: Optional Platform Receiver mailbox unidirectional channel
  * @cinfo: SCMI channel info
  * @shmem: Transmit/Receive shared memory area
  */
@@ -28,6 +29,7 @@ struct scmi_mailbox {
 	struct mbox_client cl;
 	struct mbox_chan *chan;
 	struct mbox_chan *chan_receiver;
+	struct mbox_chan *chan_platform_receiver;
 	struct scmi_chan_info *cinfo;
 	struct scmi_shared_mem __iomem *shmem;
 };
@@ -91,6 +93,8 @@ static bool mailbox_chan_available(struct device_node *of_node, int idx)
  *		 for replies on the a2p channel. Set as zero if not present.
  * @p2a_chan: A reference to the optional p2a channel.
  *	      Set as zero if not present.
+ * @p2a_rx_chan: A reference to the optional p2a completion channel.
+ *	      Set as zero if not present.
  *
  * At first, validate the transport configuration as described in terms of
  * 'mboxes' and 'shmem', then determin which mailbox channel indexes are
@@ -98,8 +102,8 @@ static bool mailbox_chan_available(struct device_node *of_node, int idx)
  *
  * Return: 0 on Success or error
  */
-static int mailbox_chan_validate(struct device *cdev,
-				 int *a2p_rx_chan, int *p2a_chan)
+static int mailbox_chan_validate(struct device *cdev, int *a2p_rx_chan,
+				 int *p2a_chan, int *p2a_rx_chan)
 {
 	int num_mb, num_sh, ret = 0;
 	struct device_node *np = cdev->of_node;
@@ -109,8 +113,9 @@ static int mailbox_chan_validate(struct device *cdev,
 	dev_dbg(cdev, "Found %d mboxes and %d shmems !\n", num_mb, num_sh);
 
 	/* Bail out if mboxes and shmem descriptors are inconsistent */
-	if (num_mb <= 0 || num_sh <= 0 || num_sh > 2 || num_mb > 3 ||
-	    (num_mb == 1 && num_sh != 1) || (num_mb == 3 && num_sh != 2)) {
+	if (num_mb <= 0 || num_sh <= 0 || num_sh > 2 || num_mb > 4 ||
+	    (num_mb == 1 && num_sh != 1) || (num_mb == 3 && num_sh != 2) ||
+	    (num_mb == 4 && num_sh != 2)) {
 		dev_warn(cdev,
 			 "Invalid channel descriptor for '%s' - mbs:%d  shm:%d\n",
 			 of_node_full_name(np), num_mb, num_sh);
@@ -139,6 +144,7 @@ static int mailbox_chan_validate(struct device *cdev,
 		case 1:
 			*a2p_rx_chan = 0;
 			*p2a_chan = 0;
+			*p2a_rx_chan = 0;
 			break;
 		case 2:
 			if (num_sh == 2) {
@@ -148,10 +154,17 @@ static int mailbox_chan_validate(struct device *cdev,
 				*a2p_rx_chan = 1;
 				*p2a_chan = 0;
 			}
+			*p2a_rx_chan = 0;
 			break;
 		case 3:
 			*a2p_rx_chan = 1;
 			*p2a_chan = 2;
+			*p2a_rx_chan = 0;
+			break;
+		case 4:
+			*a2p_rx_chan = 1;
+			*p2a_chan = 2;
+			*p2a_rx_chan = 3;
 			break;
 		}
 	}
@@ -166,12 +179,12 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
 	struct device *cdev = cinfo->dev;
 	struct scmi_mailbox *smbox;
 	struct device_node *shmem;
-	int ret, a2p_rx_chan, p2a_chan, idx = tx ? 0 : 1;
+	int ret, a2p_rx_chan, p2a_chan, p2a_rx_chan, idx = tx ? 0 : 1;
 	struct mbox_client *cl;
 	resource_size_t size;
 	struct resource res;
 
-	ret = mailbox_chan_validate(cdev, &a2p_rx_chan, &p2a_chan);
+	ret = mailbox_chan_validate(cdev, &a2p_rx_chan, &p2a_chan, &p2a_rx_chan);
 	if (ret)
 		return ret;
 
@@ -229,6 +242,17 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
 		}
 	}
 
+	if (!tx && p2a_rx_chan) {
+		smbox->chan_platform_receiver = mbox_request_channel(cl, p2a_rx_chan);
+		if (IS_ERR(smbox->chan_platform_receiver)) {
+			ret = PTR_ERR(smbox->chan_platform_receiver);
+			if (ret != -EPROBE_DEFER)
+				dev_err(cdev, "failed to request SCMI P2A Receiver mailbox\n");
+			return ret;
+		}
+	}
+
+
 	cinfo->transport_info = smbox;
 	smbox->cinfo = cinfo;
 
@@ -243,9 +267,11 @@ static int mailbox_chan_free(int id, void *p, void *data)
 	if (smbox && !IS_ERR(smbox->chan)) {
 		mbox_free_channel(smbox->chan);
 		mbox_free_channel(smbox->chan_receiver);
+		mbox_free_channel(smbox->chan_platform_receiver);
 		cinfo->transport_info = NULL;
 		smbox->chan = NULL;
 		smbox->chan_receiver = NULL;
+		smbox->chan_platform_receiver = NULL;
 		smbox->cinfo = NULL;
 	}
 
@@ -300,8 +326,21 @@ static void mailbox_fetch_notification(struct scmi_chan_info *cinfo,
 static void mailbox_clear_channel(struct scmi_chan_info *cinfo)
 {
 	struct scmi_mailbox *smbox = cinfo->transport_info;
+	int ret;
 
 	shmem_clear_channel(smbox->shmem);
+
+	if (smbox->chan_platform_receiver) {
+		if (!shmem_channel_intr_enabled(smbox->shmem))
+			return;
+
+		ret = mbox_send_message(smbox->chan_platform_receiver, NULL);
+		/* mbox_send_message returns non-negative value on success, so reset */
+		if (ret > 0)
+			ret = 0;
+
+		mbox_client_txdone(smbox->chan_platform_receiver, ret);
+	}
 }
 
 static bool
diff --git a/drivers/firmware/arm_scmi/shmem.c b/drivers/firmware/arm_scmi/shmem.c
index 8bf495bcad09..b74e5a740f2c 100644
--- a/drivers/firmware/arm_scmi/shmem.c
+++ b/drivers/firmware/arm_scmi/shmem.c
@@ -128,3 +128,8 @@ bool shmem_channel_free(struct scmi_shared_mem __iomem *shmem)
 	return (ioread32(&shmem->channel_status) &
 			SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE);
 }
+
+bool shmem_channel_intr_enabled(struct scmi_shared_mem __iomem *shmem)
+{
+	return ioread32(&shmem->flags) & SCMI_SHMEM_FLAG_INTR_ENABLED;
+}

-- 
2.37.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] firmware: arm_scmi: add notification completion channel

[Cristian Marussi]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 0/2] firmware: arm_scmi: add notification completion channel

[Peng Fan]

Hi Cristian,

> Subject: Re: [PATCH 0/2] firmware: arm_scmi: add notification completion
> channel
> 
> 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)

Right. i.MX95 use unidirectional channels.

> 
> 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 for detailed explanation, it is clear enough. I will update to support
bidirectional channel in V2.

Thanks,
Peng.
> 
> Thanks,
> Cristian


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel