[PATCH RESEND v2 0/4] stratix10: Add Interrupt support for asynchronous communication with SDM

[PATCH RESEND v2 0/4] stratix10: Add Interrupt support for asynchronous communication with SDM

[Mahesh Rao via B4 Relay]

This patch-set adds support for interrupt from
Secure Device Manager(SDM) to Stratix10 SoC for
asynchronous communication.

Following things are added in this patch-set:
- Add support for interrupt from SDM to Stratix10 SoC.
- Add documentation for SDM interrupt in Stratix10 SoC.
- Enable SDM interrupt in Stratix10 SoC.
- Enable SDM interrupt in Agilex SoC.

These patches were originally submitted within a
different patch set, but were requested to be
separated into their own series.
Reference: https://lore.kernel.org/all/df8db66b-89a4-40d9-bd44-6705fdbb4517@kernel.org/

Signed-off-by: Mahesh Rao <mahesh.rao@altera.com>
---
Changes in v2:
- EDITME: describe what is new in this series revision.
- EDITME: use bulletpoints and terse descriptions.
- Link to v1: https://lore.kernel.org/r/20250722-sip_svc_irq-v1-0-c5b9e02c1ce6@altera.com

---
Mahesh Rao (4):
      dt-bindings: firmware: Add interrupt specification for Intel Stratix 10 Service Layer.
      dts: stratix10: Add support for SDM mailbox interrupt for Intel Stratix10 SoC FPGA.
      dts: agilex: Add support for SDM mailbox interrupt for Intel Agilex SoC FPGA.
      firmware: stratix10-svc: Add for SDM mailbox doorbell interrupt

 .../bindings/firmware/intel,stratix10-svc.yaml     |  10 ++
 arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi  |   4 +
 arch/arm64/boot/dts/intel/socfpga_agilex.dtsi      |   2 +
 drivers/firmware/stratix10-svc.c                   | 117 +++++++++++++++++++--
 include/linux/firmware/intel/stratix10-smc.h       |  23 ++++
 5 files changed, 146 insertions(+), 10 deletions(-)
---
base-commit: 062b3e4a1f880f104a8d4b90b767788786aa7b78
change-id: 20250709-sip_svc_irq-8be44f46ad9c
prerequisite-change-id: 20250131-sip_svc_upstream-e78d8c9b4335:v5
prerequisite-patch-id: 6a4223bd2c01a0fd20925e597c906dc64e11ec2f
prerequisite-patch-id: 33ca4dbe8b8e18d3e51145c6bcaae55170878b22
prerequisite-patch-id: cc72b86440a07b6311c0ac8cf2212072d2a0c561
prerequisite-patch-id: 5c3dc9849364a5c6573b032b878459be252305e5
prerequisite-patch-id: 5db2eb9fe78188c2dbc453d0d2d6e45c8d8e1be7
prerequisite-patch-id: f7ffa416aef888c30d3fcf875e7b2f79e333dbfa
prerequisite-patch-id: b827c6490b3ebef5de4ad67e3150e318df79a08e

Best regards,
-- 
Mahesh Rao <mahesh.rao@altera.com>

[PATCH RESEND v2 1/4] dt-bindings: firmware: Add interrupt specification for Intel Stratix 10 Service Layer.

[Mahesh Rao via B4 Relay]

From: Mahesh Rao <mahesh.rao@altera.com>

Add interrupt specification for Intel Stratix10 Service
layer for asynchronous communication.

Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Mahesh Rao <mahesh.rao@altera.com>
---
 .../devicetree/bindings/firmware/intel,stratix10-svc.yaml      | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml b/Documentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml
index fac1e955852e4f9b966c991dcfac56222c5f7315..656cc50fd08217f270f95ae39010152423315ed1 100644
--- a/Documentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml
+++ b/Documentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml
@@ -54,6 +54,12 @@ properties:
       reserved memory region for the service layer driver to
       communicate with the secure device manager.
 
+  interrupts:
+    maxItems: 1
+    description:
+      This interrupt is used by the Secure Device Manager (SDM) to signal
+      completion of an asynchronous operation to service layer driver.
+
   fpga-mgr:
     $ref: /schemas/fpga/intel,stratix10-soc-fpga-mgr.yaml
     description: Optional child node for fpga manager to perform fabric configuration.
@@ -67,6 +73,8 @@ additionalProperties: false
 
 examples:
   - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
     reserved-memory {
       #address-cells = <2>;
       #size-cells = <2>;
@@ -84,6 +92,8 @@ examples:
         compatible = "intel,stratix10-svc";
         method = "smc";
         memory-region = <&service_reserved>;
+        interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-parent = <&intc0>;
 
         fpga-mgr {
           compatible = "intel,stratix10-soc-fpga-mgr";

-- 
2.35.3

[PATCH RESEND v2 2/4] dts: stratix10: Add support for SDM mailbox interrupt for Intel Stratix10 SoC FPGA.

[Mahesh Rao via B4 Relay]

From: Mahesh Rao <mahesh.rao@altera.com>

Add support for Secure Device Manager (SDM) mailbox
doorbell interrupt on Stratix10 SoC FPGA for
supporting asynchronous transactions.

Signed-off-by: Mahesh Rao <mahesh.rao@altera.com>
---
 arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
index effd242f6bf709a53659b4de2a2da728052d086f..9e8b3b07dde3ff57382044f528c34bd52f8df123 100644
--- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
+++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
@@ -1,11 +1,13 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright Altera Corporation (C) 2015. All rights reserved.
+ * Copyright (C) 2025, Altera Corporation
  */
 
 /dts-v1/;
 #include <dt-bindings/reset/altr,rst-mgr-s10.h>
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/clock/stratix10-clock.h>
 
 / {
@@ -74,6 +76,8 @@ svc {
 			compatible = "intel,stratix10-svc";
 			method = "smc";
 			memory-region = <&service_reserved>;
+			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-parent = <&intc>;
 
 			fpga_mgr: fpga-mgr {
 				compatible = "intel,stratix10-soc-fpga-mgr";

-- 
2.35.3

[PATCH RESEND v2 3/4] dts: agilex: Add support for SDM mailbox interrupt for Intel Agilex SoC FPGA.

[Mahesh Rao via B4 Relay]

From: Mahesh Rao <mahesh.rao@altera.com>

Add support for Secure Device Manager (SDM) mailbox
doorbell interrupt on Agilex SoC FPGA for supporting
asynchronous transactions.

Signed-off-by: Mahesh Rao <mahesh.rao@altera.com>
Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
---
 arch/arm64/boot/dts/intel/socfpga_agilex.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
index c1e66db0f4c524809d4334b86ef741f834a9b896..c3c72e8c1c10fcff818dcc934a36e2ffaa0bd347 100644
--- a/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
+++ b/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
@@ -65,6 +65,8 @@ svc {
 			compatible = "intel,agilex-svc";
 			method = "smc";
 			memory-region = <&service_reserved>;
+			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-parent = <&intc>;
 
 			fpga_mgr: fpga-mgr {
 				compatible = "intel,agilex-soc-fpga-mgr";

-- 
2.35.3

[PATCH RESEND v2 4/4] firmware: stratix10-svc: Add for SDM mailbox doorbell interrupt

[Mahesh Rao via B4 Relay]

From: Mahesh Rao <mahesh.rao@altera.com>

Add support for SDM (Secure Device Manager) mailbox
doorbell interrupt for async transactions. On interrupt,
a workqueue is triggered which polls the ATF for
pending responses and retrieves the bitmap of all
retrieved and unprocessed transaction ids of mailbox
responses from SDM. It then triggers the corresponding
registered callbacks.

Signed-off-by: Mahesh Rao <mahesh.rao@altera.com>
Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
---
 drivers/firmware/stratix10-svc.c             | 117 ++++++++++++++++++++++++---
 include/linux/firmware/intel/stratix10-smc.h |  23 ++++++
 2 files changed, 130 insertions(+), 10 deletions(-)

diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
index 491a8149033f975d515444f025723658c51aa1fe..a65c64c1be61d9f1fd27114d7f30d7a759e8201e 100644
--- a/drivers/firmware/stratix10-svc.c
+++ b/drivers/firmware/stratix10-svc.c
@@ -9,12 +9,14 @@
 #include <linux/delay.h>
 #include <linux/genalloc.h>
 #include <linux/hashtable.h>
+#include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/kfifo.h>
 #include <linux/kthread.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
@@ -22,6 +24,7 @@
 #include <linux/firmware/intel/stratix10-smc.h>
 #include <linux/firmware/intel/stratix10-svc-client.h>
 #include <linux/types.h>
+#include <linux/workqueue.h>
 
 /**
  * SVC_NUM_DATA_IN_FIFO - number of struct stratix10_svc_data in the FIFO
@@ -213,6 +216,7 @@ struct stratix10_async_chan {
  *                               asynchronous operations
  * @initialized: Flag indicating whether the control structure has
  *               been initialized
+ * @irq: Interrupt request number associated with the asynchronous control
  * @invoke_fn: Function pointer for invoking Stratix10 service calls
  *             to EL3 secure firmware
  * @async_id_pool: Pointer to the ID pool used for asynchronous
@@ -223,11 +227,13 @@ struct stratix10_async_chan {
  *                     structure
  * @trx_list_wr_lock: Spinlock for protecting the transaction list
  *                    write operations
+ * @async_work: Work structure for scheduling asynchronous work
  * @trx_list: Hash table for managing asynchronous transactions
  */
 
 struct stratix10_async_ctrl {
 	bool initialized;
+	int irq;
 	void (*invoke_fn)(struct stratix10_async_ctrl *actrl,
 			  const struct arm_smccc_1_2_regs *args,
 			  struct arm_smccc_1_2_regs *res);
@@ -236,6 +242,7 @@ struct stratix10_async_ctrl {
 	struct stratix10_async_chan *common_async_chan;
 	/* spinlock to protect the writes to trx_list hash table */
 	spinlock_t trx_list_wr_lock;
+	struct work_struct async_work;
 	DECLARE_HASHTABLE(trx_list, ASYNC_TRX_HASH_BITS);
 };
 
@@ -1709,14 +1716,81 @@ static inline void stratix10_smc_1_2(struct stratix10_async_ctrl *actrl,
 	arm_smccc_1_2_smc(args, res);
 }
 
+static irqreturn_t stratix10_svc_async_irq_handler(int irq, void *dev_id)
+{
+	struct stratix10_svc_controller *ctrl = dev_id;
+	struct stratix10_async_ctrl *actrl = &ctrl->actrl;
+
+	queue_work(system_bh_wq, &actrl->async_work);
+	disable_irq_nosync(actrl->irq);
+	return IRQ_HANDLED;
+}
+/**
+ * stratix10_async_workqueue_handler - Handler for the asynchronous
+ * workqueue in Stratix10 service controller.
+ * @work: Pointer to the work structure that contains the asynchronous
+ *        workqueue handler.
+ * This function is the handler for the asynchronous workqueue. It performs
+ * the following tasks:
+ * - Invokes the asynchronous polling on interrupt supervisory call.
+ * - On success,it retrieves the bitmap of pending transactions from mailbox
+ *   fifo in ATF.
+ * - It processes each pending transaction by calling the corresponding
+ *   callback function.
+ *
+ * The function ensures that the IRQ is enabled after processing the transactions
+ * and logs the total time taken to handle the transactions along with the number
+ * of transactions handled and the CPU on which the handler ran.
+ */
+static void stratix10_async_workqueue_handler(struct work_struct *work)
+{
+	struct stratix10_async_ctrl *actrl =
+		container_of(work, struct stratix10_async_ctrl, async_work);
+	struct arm_smccc_1_2_regs
+		args = { .a0 = INTEL_SIP_SMC_ASYNC_POLL_ON_IRQ }, res;
+	DECLARE_BITMAP(pend_on_irq, TOTAL_TRANSACTION_IDS);
+	struct stratix10_svc_async_handler *handler;
+	unsigned long transaction_id = 0;
+	u64 bitmap_array[4];
+
+	actrl->invoke_fn(actrl, &args, &res);
+	if (res.a0 == INTEL_SIP_SMC_STATUS_OK) {
+		bitmap_array[0] = res.a1;
+		bitmap_array[1] = res.a2;
+		bitmap_array[2] = res.a3;
+		bitmap_array[3] = res.a4;
+		bitmap_from_arr64(pend_on_irq, bitmap_array, TOTAL_TRANSACTION_IDS);
+		rcu_read_lock();
+		do {
+			transaction_id = find_next_bit(pend_on_irq,
+						       TOTAL_TRANSACTION_IDS,
+						       transaction_id);
+			if (transaction_id >= TOTAL_TRANSACTION_IDS)
+				break;
+			hash_for_each_possible_rcu_notrace(actrl->trx_list,
+							   handler, next,
+							   transaction_id) {
+				if (handler->transaction_id == transaction_id) {
+					handler->cb(handler->cb_arg);
+					break;
+				}
+			}
+			transaction_id++;
+		} while (transaction_id < TOTAL_TRANSACTION_IDS);
+		rcu_read_unlock();
+	}
+	enable_irq(actrl->irq);
+}
+
 /**
  * stratix10_svc_async_init - Initialize the Stratix10 service
  *                            controller for asynchronous operations.
  * @controller: Pointer to the Stratix10 service controller structure.
  *
  * This function initializes the asynchronous service controller by
- * setting up the necessary data structures and initializing the
- * transaction list.
+ * setting up the necessary data structures ,initializing the
+ * transaction list and registering the IRQ handler for asynchronous
+ * transactions.
  *
  * Return: 0 on success, -EINVAL if the controller is NULL or already
  *         initialized, -ENOMEM if memory allocation fails,
@@ -1728,7 +1802,7 @@ static int stratix10_svc_async_init(struct stratix10_svc_controller *controller)
 	struct stratix10_async_ctrl *actrl;
 	struct arm_smccc_res res;
 	struct device *dev;
-	int ret;
+	int ret, irq;
 
 	if (!controller)
 		return -EINVAL;
@@ -1775,6 +1849,22 @@ static int stratix10_svc_async_init(struct stratix10_svc_controller *controller)
 	hash_init(actrl->trx_list);
 	atomic_set(&actrl->common_achan_refcount, 0);
 
+	irq = of_irq_get(dev_of_node(dev), 0);
+	if (irq < 0) {
+		dev_warn(dev, "Failed to get IRQ, falling back to polling mode\n");
+	} else {
+		ret = devm_request_any_context_irq(dev, irq, stratix10_svc_async_irq_handler,
+						   IRQF_NO_AUTOEN, "stratix10_svc", controller);
+		if (ret == 0) {
+			dev_alert(dev,
+				  "Registered IRQ %d for sip async operations\n",
+				irq);
+			actrl->irq = irq;
+			INIT_WORK(&actrl->async_work, stratix10_async_workqueue_handler);
+			enable_irq(actrl->irq);
+		}
+	}
+
 	actrl->initialized = true;
 	return 0;
 }
@@ -1784,13 +1874,14 @@ static int stratix10_svc_async_init(struct stratix10_svc_controller *controller)
  *                            service controller
  * @ctrl: Pointer to the stratix10_svc_controller structure
  *
- * This function performs the necessary cleanup for the asynchronous
- * service controller. It checks if the controller is valid and if it
- * has been initialized. It then locks the transaction list and safely
- * removes and deallocates each handler in the list. The function also
- * removes any asynchronous clients associated with the controller's
- * channels and destroys the asynchronous ID pool. Finally, it resets
- * the asynchronous ID pool and invoke function pointers to NULL.
+ * This function performs the necessary cleanup for the asynchronous service
+ * controller. It checks if the controller is valid and if it has been
+ * initialized. Also If the controller has an IRQ assigned, it frees the IRQ
+ * and flushes any pending asynchronous work. It then locks the transaction
+ * list and safely removes and deallocates each handler in the list.
+ * The function also removes any asynchronous clients associated with the
+ * controller's channels and destroys the asynchronous ID pool. Finally, it
+ * resets the asynchronous ID pool and invoke function pointers to NULL.
  *
  * Return: 0 on success, -EINVAL if the controller is invalid or not
  *         initialized.
@@ -1812,6 +1903,12 @@ static int stratix10_svc_async_exit(struct stratix10_svc_controller *ctrl)
 
 	actrl->initialized = false;
 
+	if (actrl->irq > 0) {
+		free_irq(actrl->irq, ctrl);
+		flush_work(&actrl->async_work);
+		actrl->irq = 0;
+	}
+
 	spin_lock(&actrl->trx_list_wr_lock);
 	hash_for_each_safe(actrl->trx_list, i, tmp, handler, next) {
 		stratix10_deallocate_id(handler->achan->job_id_pool,
diff --git a/include/linux/firmware/intel/stratix10-smc.h b/include/linux/firmware/intel/stratix10-smc.h
index f87273af5e284b8912d87eb9d7179eb3d43e40e1..45e9dd4211f4994d67e5a6e00a5a817e96d42a8d 100644
--- a/include/linux/firmware/intel/stratix10-smc.h
+++ b/include/linux/firmware/intel/stratix10-smc.h
@@ -645,6 +645,29 @@ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_COMPLETED_WRITE)
 #define INTEL_SIP_SMC_ASYNC_POLL \
 	INTEL_SIP_SMC_ASYNC_VAL(INTEL_SIP_SMC_ASYNC_FUNC_ID_POLL)
 
+/**
+ * Request INTEL_SIP_SMC_ASYNC_POLL_ON_IRQ
+ * Async call used by service driver at EL1 to read response from SDM
+ * mailbox and to retrieve the transaction id's of the read response's.
+ *
+ * Call register usage:
+ * a0 INTEL_SIP_SMC_ASYNC_POLL_ON_IRQ
+ * a1 transaction job id
+ * a2-7 will be used to return the response data
+ *
+ * Return status
+ * a0 INTEL_SIP_SMC_STATUS_OK
+ * a1-a4 will contain bitmap of available responses's transaction id as set
+ *  bit position.
+ * a5-17 not used
+ * Or
+ * a0 INTEL_SIP_SMC_STATUS_NO_RESPONSE
+ * a1-17 not used
+ */
+#define INTEL_SIP_SMC_ASYNC_FUNC_ID_IRQ_POLL (0xC9)
+#define INTEL_SIP_SMC_ASYNC_POLL_ON_IRQ \
+	INTEL_SIP_SMC_ASYNC_VAL(INTEL_SIP_SMC_ASYNC_FUNC_ID_IRQ_POLL)
+
 /**
  * Request INTEL_SIP_SMC_ASYNC_RSU_GET_SPT
  * Async call to get RSU SPT from SDM.

-- 
2.35.3

Re: [PATCH RESEND v2 4/4] firmware: stratix10-svc: Add for SDM mailbox doorbell interrupt

[Dinh Nguyen]

On 8/12/25 07:59, Mahesh Rao via B4 Relay wrote:
> From: Mahesh Rao <mahesh.rao@altera.com>
> 
> Add support for SDM (Secure Device Manager) mailbox
> doorbell interrupt for async transactions. On interrupt,
> a workqueue is triggered which polls the ATF for
> pending responses and retrieves the bitmap of all
> retrieved and unprocessed transaction ids of mailbox
> responses from SDM. It then triggers the corresponding
> registered callbacks.

You should configure your editor to use a full 80-char width. Why stop
at ~50? When you're unsure, look at that other commit logs from other
developers. If yours doesn't look similar, its probably a problem. For
example:

"Add support for SDM (Secure Device Manager) mailbox doorbell interrupt
for async transactions. On interrupt, a workqueue is triggered which
polls the ATF for pending responses and retrieves the bitmap of all
retrieved and unprocessed transaction ids of mailbox responses from SDM.
It then triggers the corresponding registered callbacks."

> 
> Signed-off-by: Mahesh Rao <mahesh.rao@altera.com>
> Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
> ---
>   drivers/firmware/stratix10-svc.c             | 117 ++++++++++++++++++++++++---
>   include/linux/firmware/intel/stratix10-smc.h |  23 ++++++
>   2 files changed, 130 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
> index 491a8149033f975d515444f025723658c51aa1fe..a65c64c1be61d9f1fd27114d7f30d7a759e8201e 100644
> --- a/drivers/firmware/stratix10-svc.c
> +++ b/drivers/firmware/stratix10-svc.c
> @@ -9,12 +9,14 @@
>   #include <linux/delay.h>
>   #include <linux/genalloc.h>
>   #include <linux/hashtable.h>
> +#include <linux/interrupt.h>
>   #include <linux/io.h>
>   #include <linux/kfifo.h>
>   #include <linux/kthread.h>
>   #include <linux/module.h>
>   #include <linux/mutex.h>
>   #include <linux/of.h>
> +#include <linux/of_irq.h>
>   #include <linux/of_platform.h>
>   #include <linux/platform_device.h>
>   #include <linux/slab.h>
> @@ -22,6 +24,7 @@
>   #include <linux/firmware/intel/stratix10-smc.h>
>   #include <linux/firmware/intel/stratix10-svc-client.h>
>   #include <linux/types.h>
> +#include <linux/workqueue.h>
>   
>   /**
>    * SVC_NUM_DATA_IN_FIFO - number of struct stratix10_svc_data in the FIFO
> @@ -213,6 +216,7 @@ struct stratix10_async_chan {
>    *                               asynchronous operations
>    * @initialized: Flag indicating whether the control structure has
>    *               been initialized
> + * @irq: Interrupt request number associated with the asynchronous control
>    * @invoke_fn: Function pointer for invoking Stratix10 service calls
>    *             to EL3 secure firmware
>    * @async_id_pool: Pointer to the ID pool used for asynchronous
> @@ -223,11 +227,13 @@ struct stratix10_async_chan {
>    *                     structure
>    * @trx_list_wr_lock: Spinlock for protecting the transaction list
>    *                    write operations
> + * @async_work: Work structure for scheduling asynchronous work
>    * @trx_list: Hash table for managing asynchronous transactions
>    */
>   
>   struct stratix10_async_ctrl {
>   	bool initialized;
> +	int irq;
>   	void (*invoke_fn)(struct stratix10_async_ctrl *actrl,
>   			  const struct arm_smccc_1_2_regs *args,
>   			  struct arm_smccc_1_2_regs *res);
> @@ -236,6 +242,7 @@ struct stratix10_async_ctrl {
>   	struct stratix10_async_chan *common_async_chan;
>   	/* spinlock to protect the writes to trx_list hash table */
>   	spinlock_t trx_list_wr_lock;
> +	struct work_struct async_work;
>   	DECLARE_HASHTABLE(trx_list, ASYNC_TRX_HASH_BITS);
>   };
>   
> @@ -1709,14 +1716,81 @@ static inline void stratix10_smc_1_2(struct stratix10_async_ctrl *actrl,
>   	arm_smccc_1_2_smc(args, res);
>   }
>   
> +static irqreturn_t stratix10_svc_async_irq_handler(int irq, void *dev_id)
> +{
> +	struct stratix10_svc_controller *ctrl = dev_id;
> +	struct stratix10_async_ctrl *actrl = &ctrl->actrl;
> +
> +	queue_work(system_bh_wq, &actrl->async_work);
> +	disable_irq_nosync(actrl->irq);
> +	return IRQ_HANDLED;
> +}

Add a newline here.

> +/**
> + * stratix10_async_workqueue_handler - Handler for the asynchronous
> + * workqueue in Stratix10 service controller.
> + * @work: Pointer to the work structure that contains the asynchronous
> + *        workqueue handler.
> + * This function is the handler for the asynchronous workqueue. It performs
> + * the following tasks:
> + * - Invokes the asynchronous polling on interrupt supervisory call.
> + * - On success,it retrieves the bitmap of pending transactions from mailbox
> + *   fifo in ATF.
> + * - It processes each pending transaction by calling the corresponding
> + *   callback function.
> + *
> + * The function ensures that the IRQ is enabled after processing the transactions
> + * and logs the total time taken to handle the transactions along with the number
> + * of transactions handled and the CPU on which the handler ran.
> + */
> +static void stratix10_async_workqueue_handler(struct work_struct *work)
> +{
> +	struct stratix10_async_ctrl *actrl =
> +		container_of(work, struct stratix10_async_ctrl, async_work);
> +	struct arm_smccc_1_2_regs
> +		args = { .a0 = INTEL_SIP_SMC_ASYNC_POLL_ON_IRQ }, res;
> +	DECLARE_BITMAP(pend_on_irq, TOTAL_TRANSACTION_IDS);
> +	struct stratix10_svc_async_handler *handler;
> +	unsigned long transaction_id = 0;
> +	u64 bitmap_array[4];
> +
> +	actrl->invoke_fn(actrl, &args, &res);
> +	if (res.a0 == INTEL_SIP_SMC_STATUS_OK) {
> +		bitmap_array[0] = res.a1;
> +		bitmap_array[1] = res.a2;
> +		bitmap_array[2] = res.a3;
> +		bitmap_array[3] = res.a4;
> +		bitmap_from_arr64(pend_on_irq, bitmap_array, TOTAL_TRANSACTION_IDS);
> +		rcu_read_lock();
> +		do {
> +			transaction_id = find_next_bit(pend_on_irq,
> +						       TOTAL_TRANSACTION_IDS,
> +						       transaction_id);
> +			if (transaction_id >= TOTAL_TRANSACTION_IDS)
> +				break;
> +			hash_for_each_possible_rcu_notrace(actrl->trx_list,
> +							   handler, next,
> +							   transaction_id) {
> +				if (handler->transaction_id == transaction_id) {
> +					handler->cb(handler->cb_arg);
> +					break;
> +				}
> +			}
> +			transaction_id++;
> +		} while (transaction_id < TOTAL_TRANSACTION_IDS);
> +		rcu_read_unlock();
> +	}
> +	enable_irq(actrl->irq);
> +}
> +
>   /**
>    * stratix10_svc_async_init - Initialize the Stratix10 service
>    *                            controller for asynchronous operations.
>    * @controller: Pointer to the Stratix10 service controller structure.
>    *
>    * This function initializes the asynchronous service controller by
> - * setting up the necessary data structures and initializing the
> - * transaction list.
> + * setting up the necessary data structures ,initializing the
> + * transaction list and registering the IRQ handler for asynchronous
> + * transactions.
>    *
>    * Return: 0 on success, -EINVAL if the controller is NULL or already
>    *         initialized, -ENOMEM if memory allocation fails,
> @@ -1728,7 +1802,7 @@ static int stratix10_svc_async_init(struct stratix10_svc_controller *controller)
>   	struct stratix10_async_ctrl *actrl;
>   	struct arm_smccc_res res;
>   	struct device *dev;
> -	int ret;
> +	int ret, irq;
>   
>   	if (!controller)
>   		return -EINVAL;
> @@ -1775,6 +1849,22 @@ static int stratix10_svc_async_init(struct stratix10_svc_controller *controller)
>   	hash_init(actrl->trx_list);
>   	atomic_set(&actrl->common_achan_refcount, 0);
>   
> +	irq = of_irq_get(dev_of_node(dev), 0);
> +	if (irq < 0) {

0 is a failing value as well.

> +		dev_warn(dev, "Failed to get IRQ, falling back to polling mode\n");
> +	} else {
> +		ret = devm_request_any_context_irq(dev, irq, stratix10_svc_async_irq_handler,
> +						   IRQF_NO_AUTOEN, "stratix10_svc", controller);
> +		if (ret == 0) {
> +			dev_alert(dev,
> +				  "Registered IRQ %d for sip async operations\n",
> +				irq);
> +			actrl->irq = irq;
> +			INIT_WORK(&actrl->async_work, stratix10_async_workqueue_handler);
> +			enable_irq(actrl->irq);
> +		}
> +	}
> +
>   	actrl->initialized = true;
>   	return 0;
>   }
> @@ -1784,13 +1874,14 @@ static int stratix10_svc_async_init(struct stratix10_svc_controller *controller)
>    *                            service controller
>    * @ctrl: Pointer to the stratix10_svc_controller structure
>    *
> - * This function performs the necessary cleanup for the asynchronous
> - * service controller. It checks if the controller is valid and if it
> - * has been initialized. It then locks the transaction list and safely
> - * removes and deallocates each handler in the list. The function also
> - * removes any asynchronous clients associated with the controller's
> - * channels and destroys the asynchronous ID pool. Finally, it resets
> - * the asynchronous ID pool and invoke function pointers to NULL.
> + * This function performs the necessary cleanup for the asynchronous service
> + * controller. It checks if the controller is valid and if it has been
> + * initialized. Also If the controller has an IRQ assigned, it frees the IRQ
> + * and flushes any pending asynchronous work. It then locks the transaction
> + * list and safely removes and deallocates each handler in the list.
> + * The function also removes any asynchronous clients associated with the
> + * controller's channels and destroys the asynchronous ID pool. Finally, it
> + * resets the asynchronous ID pool and invoke function pointers to NULL.

Did you mean to repeat the same paragraph twice?

Dinh

Re: [PATCH RESEND v2 4/4] firmware: stratix10-svc: Add for SDM mailbox doorbell interrupt

[Dinh Nguyen]

On 9/1/25 10:16, Dinh Nguyen wrote:
> 
> 
> On 8/12/25 07:59, Mahesh Rao via B4 Relay wrote:
>> From: Mahesh Rao <mahesh.rao@altera.com>
>>
>> Add support for SDM (Secure Device Manager) mailbox
>> doorbell interrupt for async transactions. On interrupt,
>> a workqueue is triggered which polls the ATF for
>> pending responses and retrieves the bitmap of all
>> retrieved and unprocessed transaction ids of mailbox
>> responses from SDM. It then triggers the corresponding
>> registered callbacks.
> 
> You should configure your editor to use a full 80-char width. Why stop
> at ~50? When you're unsure, look at that other commit logs from other
> developers. If yours doesn't look similar, its probably a problem. For
> example:
> 
> "Add support for SDM (Secure Device Manager) mailbox doorbell interrupt
> for async transactions. On interrupt, a workqueue is triggered which
> polls the ATF for pending responses and retrieves the bitmap of all
> retrieved and unprocessed transaction ids of mailbox responses from SDM.
> It then triggers the corresponding registered callbacks."
> 
>>
>> Signed-off-by: Mahesh Rao <mahesh.rao@altera.com>
>> Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
>> ---

<snip>

>> @@ -1784,13 +1874,14 @@ static int stratix10_svc_async_init(struct 
>> stratix10_svc_controller *controller)
>>    *                            service controller
>>    * @ctrl: Pointer to the stratix10_svc_controller structure
>>    *
>> - * This function performs the necessary cleanup for the asynchronous
>> - * service controller. It checks if the controller is valid and if it
>> - * has been initialized. It then locks the transaction list and safely
>> - * removes and deallocates each handler in the list. The function also
>> - * removes any asynchronous clients associated with the controller's
>> - * channels and destroys the asynchronous ID pool. Finally, it resets
>> - * the asynchronous ID pool and invoke function pointers to NULL.
>> + * This function performs the necessary cleanup for the asynchronous 
>> service
>> + * controller. It checks if the controller is valid and if it has been
>> + * initialized. Also If the controller has an IRQ assigned, it frees 
>> the IRQ
>> + * and flushes any pending asynchronous work. It then locks the 
>> transaction
>> + * list and safely removes and deallocates each handler in the list.
>> + * The function also removes any asynchronous clients associated with 
>> the
>> + * controller's channels and destroys the asynchronous ID pool. 
>> Finally, it
>> + * resets the asynchronous ID pool and invoke function pointers to NULL.
> 
> Did you mean to repeat the same paragraph twice?
> 

Ignore this comment...

Re: [PATCH RESEND v2 4/4] firmware: stratix10-svc: Add for SDM mailbox doorbell interrupt

[Mahesh Rao]

Hi Dinh,

On 01-09-2025 08:46 pm, Dinh Nguyen wrote:
> 
> 
> On 8/12/25 07:59, Mahesh Rao via B4 Relay wrote:
>> From: Mahesh Rao <mahesh.rao@altera.com>
>>
>> Add support for SDM (Secure Device Manager) mailbox
>> doorbell interrupt for async transactions. On interrupt,
>> a workqueue is triggered which polls the ATF for
>> pending responses and retrieves the bitmap of all
>> retrieved and unprocessed transaction ids of mailbox
>> responses from SDM. It then triggers the corresponding
>> registered callbacks.
> 
> You should configure your editor to use a full 80-char width. Why stop
> at ~50? When you're unsure, look at that other commit logs from other
> developers. If yours doesn't look similar, its probably a problem. For
> example:
> 
> "Add support for SDM (Secure Device Manager) mailbox doorbell interrupt
> for async transactions. On interrupt, a workqueue is triggered which
> polls the ATF for pending responses and retrieves the bitmap of all
> retrieved and unprocessed transaction ids of mailbox responses from SDM.
> It then triggers the corresponding registered callbacks."

ok, will do.

> 
>>
>> Signed-off-by: Mahesh Rao <mahesh.rao@altera.com>
>> Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
>> ---
>>   drivers/firmware/stratix10-svc.c             | 117 +++++++++++++++++ 
>> +++++++---
>>   include/linux/firmware/intel/stratix10-smc.h |  23 ++++++
>>   2 files changed, 130 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/ 
>> stratix10-svc.c
>> index 


>> +    struct work_struct async_work;
>>       DECLARE_HASHTABLE(trx_list, ASYNC_TRX_HASH_BITS);
>>   };
>> @@ -1709,14 +1716,81 @@ static inline void stratix10_smc_1_2(struct 
>> stratix10_async_ctrl *actrl,
>>       arm_smccc_1_2_smc(args, res);
>>   }
>> +static irqreturn_t stratix10_svc_async_irq_handler(int irq, void 
>> *dev_id)
>> +{
>> +    struct stratix10_svc_controller *ctrl = dev_id;
>> +    struct stratix10_async_ctrl *actrl = &ctrl->actrl;
>> +
>> +    queue_work(system_bh_wq, &actrl->async_work);
>> +    disable_irq_nosync(actrl->irq);
>> +    return IRQ_HANDLED;
>> +}
> 
> Add a newline here.
ok, will make this change.

> 
>> +/**
>> + * stratix10_async_workqueue_handler - Handler for the asynchronous
>> + * workqueue in Stratix10 service controller.
>> + * @work: Pointer to the work structure that contains the asynchronous
>> + *        workqueue handler.
>> + * This function is the handler for the asynchronous workqueue. It 
>> performs
>> + * the following tasks:
>> + * - Invokes the asynchronous polling on interrupt supervisory call.
>> + * - On success,it retrieves the bitmap of pending transactions from 
>> mailbox
>> + *   fifo in ATF.
>> + * - It processes each pending transaction by calling the corresponding
>> + *   callback function.
>> + *
>> + * The function ensures that the IRQ is enabled after processing the 
>> transactions
>> + * and logs the total time taken to handle the transactions along 
>> with the number
>> + * of transactions handled and the CPU on which the handler ran.


>> -    int ret;
>> +    int ret, irq;
>>       if (!controller)
>>           return -EINVAL;
>> @@ -1775,6 +1849,22 @@ static int stratix10_svc_async_init(struct 
>> stratix10_svc_controller *controller)
>>       hash_init(actrl->trx_list);
>>       atomic_set(&actrl->common_achan_refcount, 0);
>> +    irq = of_irq_get(dev_of_node(dev), 0);
>> +    if (irq < 0) {
> 
> 0 is a failing value as well.

will add this.

> 
>> +        dev_warn(dev, "Failed to get IRQ, falling back to polling 
>> mode\n");
>> +    } else {


>> - * the asynchronous ID pool and invoke function pointers to NULL.
>> + * This function performs the necessary cleanup for the asynchronous 
>> service
>> + * controller. It checks if the controller is valid and if it has been
>> + * initialized. Also If the controller has an IRQ assigned, it frees 
>> the IRQ
>> + * and flushes any pending asynchronous work. It then locks the 
>> transaction
>> + * list and safely removes and deallocates each handler in the list.
>> + * The function also removes any asynchronous clients associated with 
>> the
>> + * controller's channels and destroys the asynchronous ID pool. 
>> Finally, it
>> + * resets the asynchronous ID pool and invoke function pointers to NULL.
> 
> Did you mean to repeat the same paragraph twice?
> 
> Dinh