[PATCH v5 0/2] SCM: Support latest version of waitq-aware firmware

[PATCH v5 0/2] SCM: Support latest version of waitq-aware firmware

[Unnathi Chalicheemala]

This series adds support for the latest improvements made in SCM
firmware that allow for multiple wait-queues in firmware.

To support multi VM synchronization when VMs make SMC calls on same CPU,
waitqueue mechanism is added in firmware which runs at EL2 & EL3 exception
levels.

P.S. While at Qualcomm, Guru Das Srinagesh authored the initial version of
these patches.
Thanks Guru!

---
Changes in v5:
- Use GIC_SPI and GIC_ESPI macros from dt-bindings instead of redefining
- Modified qcom_scm_query_waitq_count to take struct qcom_scm as
argument; scm is anyway stored to global struct __scm after
smp_store_and_release(). 
- Tested on SM8650 which has multi-waitq support and SM8550, which
doesn't. No error logs are seen.
-Link to v4: https://lore.kernel.org/all/cover.1730742637.git.quic_uchalich@quicinc.com/

Changes in v4:
- Moving back to redefining GIC_IRQ_TYPE_SPI and GIC_IRQ_TYPE_ESPI macros
in qcom_scm as seeing compilation issues in linux/irq.h when including
arm-gic header. Will send a fixes patch and move to dt-bindings in next patchset.
- Fixed a few compilation errors.
- Link to v3: https://lore.kernel.org/all/cover.1730735881.git.quic_uchalich@quicinc.com/

Changes in v3:
- Use GIC_SPI and GIC_ESPI macros from dt-bindings instead of redefining
- Prettified qcom_scm_fill_irq_fwspec_params()
- Moved waitq initialization before smp_store_release()
- There is no Gunyah hypercall API that can be used to fetch IRQ information hence
introducing new SCM call.
- Link to v2: https://lore.kernel.org/all/cover.1724968351.git.quic_uchalich@quicinc.com/

Changes in v2:
- Dropped "Initialize waitq before setting global __scm" as it was merged here:
https://lore.kernel.org/r/1711034642-22860-4-git-send-email-quic_mojha@quicinc.com
- Decoupled "Remove QCOM_SMC_WAITQ_FLAG_WAKE_ALL" from series
- Converted xarray to a statically sized array
- Initialize waitq array in probe function
- Remove reinit of waitq completion struct in scm_get_completion()
- Introduced new APIs to get no. of waitqueue contexts and waitqueue IRQ no.
directly from firmware.
- Link to v1: https://lore.kernel.org/all/20240228-multi_waitq-v1-0-ccb096419af0@quicinc.com/

Unnathi Chalicheemala (2):
  firmware: qcom_scm: Add API to get waitqueue IRQ info
  firmware: qcom_scm: Support multiple waitq contexts

 drivers/firmware/qcom/qcom_scm.c | 122 ++++++++++++++++++++++++++-----
 drivers/firmware/qcom/qcom_scm.h |   1 +
 2 files changed, 104 insertions(+), 19 deletions(-)

--
2.34.1

---
Unnathi Chalicheemala (2):
      firmware: qcom_scm: Add API to get waitqueue IRQ info
      firmware: qcom_scm: Support multiple waitq contexts

 drivers/firmware/qcom/qcom_scm.c | 127 +++++++++++++++++++++++++++++++++------
 drivers/firmware/qcom/qcom_scm.h |   1 +
 2 files changed, 109 insertions(+), 19 deletions(-)
---
base-commit: 1e15510b71c99c6e49134d756df91069f7d18141
change-id: 20250227-multi_waitq_scm-5bf05480b7b1

Best regards,
-- 
Unnathi Chalicheemala <unnathi.chalicheemala@oss.qualcomm.com>

[PATCH v5 1/2] firmware: qcom_scm: Add API to get waitqueue IRQ info

[Unnathi Chalicheemala]

Bootloader and firmware for SM8650 and older chipsets expect node
name as "qcom_scm", in order to patch the wait queue IRQ information.
However, DeviceTree uses node name "scm" and this mismatch prevents
firmware from correctly identifying waitqueue IRQ information. Waitqueue
IRQ is used for signaling between secure and non-secure worlds.

To resolve this, introduce qcom_scm_get_waitq_irq() that'll get the
hardware IRQ number to be used from firmware instead of relying on data
provided by devicetree, thereby bypassing the DeviceTree node name
mismatch.

This hardware IRQ number is converted to a Linux IRQ number using newly
defined fill_irq_fwspec_params(). This Linux IRQ number is then supplied
to the threaded_irq call.

Signed-off-by: Unnathi Chalicheemala <unnathi.chalicheemala@oss.qualcomm.com>
---
 drivers/firmware/qcom/qcom_scm.c | 60 +++++++++++++++++++++++++++++++++++++++-
 drivers/firmware/qcom/qcom_scm.h |  1 +
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index f0569bb9411f7175c97a4ede50b3775fece330cf..1aa42685640da8a14191557896fbb49423697a10 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -29,12 +29,18 @@
 #include <linux/reset-controller.h>
 #include <linux/sizes.h>
 #include <linux/types.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
 
 #include "qcom_scm.h"
 #include "qcom_tzmem.h"
 
 static u32 download_mode;
 
+#define GIC_SPI_BASE        32
+#define GIC_MAX_SPI       1019  // SPIs in GICv3 spec range from 32..1019
+#define GIC_ESPI_BASE     4096
+#define GIC_MAX_ESPI      5119 // ESPIs in GICv3 spec range from 4096..5119
+
 struct qcom_scm {
 	struct device *dev;
 	struct clk *core_clk;
@@ -2094,6 +2100,55 @@ bool qcom_scm_is_available(void)
 }
 EXPORT_SYMBOL_GPL(qcom_scm_is_available);
 
+static int qcom_scm_fill_irq_fwspec_params(struct irq_fwspec *fwspec, u32 virq)
+{
+	if (virq >= GIC_SPI_BASE && virq <= GIC_MAX_SPI) {
+		fwspec->param[0] = GIC_SPI;
+		fwspec->param[1] = virq - GIC_SPI_BASE;
+	} else if (virq >= GIC_ESPI_BASE && virq <= GIC_MAX_ESPI) {
+		fwspec->param[0] = GIC_ESPI;
+		fwspec->param[1] = virq - GIC_ESPI_BASE;
+	} else {
+		WARN(1, "Unexpected virq: %d\n", virq);
+		return -ENXIO;
+	}
+	fwspec->param[2] = IRQ_TYPE_EDGE_RISING;
+	fwspec->param_count = 3;
+
+	return 0;
+}
+
+static int qcom_scm_get_waitq_irq(void)
+{
+	int ret;
+	u32 hwirq;
+	struct qcom_scm_desc desc = {
+		.svc = QCOM_SCM_SVC_WAITQ,
+		.cmd = QCOM_SCM_WAITQ_GET_INFO,
+		.owner = ARM_SMCCC_OWNER_SIP
+	};
+	struct qcom_scm_res res;
+	struct irq_fwspec fwspec;
+	struct device_node *parent_irq_node;
+
+	ret = qcom_scm_call_atomic(__scm->dev, &desc, &res);
+	if (ret)
+		return ret;
+
+	hwirq = res.result[1] & GENMASK(15, 0);
+
+	ret = qcom_scm_fill_irq_fwspec_params(&fwspec, hwirq);
+	if (ret)
+		return ret;
+	parent_irq_node = of_irq_find_parent(__scm->dev->of_node);
+
+	fwspec.fwnode = of_node_to_fwnode(parent_irq_node);
+
+	ret = irq_create_fwspec_mapping(&fwspec);
+
+	return ret;
+}
+
 static int qcom_scm_assert_valid_wq_ctx(u32 wq_ctx)
 {
 	/* FW currently only supports a single wq_ctx (zero).
@@ -2250,7 +2305,10 @@ static int qcom_scm_probe(struct platform_device *pdev)
 	/* Paired with smp_load_acquire() in qcom_scm_is_available(). */
 	smp_store_release(&__scm, scm);
 
-	irq = platform_get_irq_optional(pdev, 0);
+	irq = qcom_scm_get_waitq_irq();
+	if (irq < 0)
+		irq = platform_get_irq_optional(pdev, 0);
+
 	if (irq < 0) {
 		if (irq != -ENXIO) {
 			ret = irq;
diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h
index 097369d38b84efbce5d672c4f36cc87373aac24b..7c6cb3154b394ab910bf7775a5ae07a28e0b57a5 100644
--- a/drivers/firmware/qcom/qcom_scm.h
+++ b/drivers/firmware/qcom/qcom_scm.h
@@ -148,6 +148,7 @@ struct qcom_tzmem_pool *qcom_scm_get_tzmem_pool(void);
 #define QCOM_SCM_SVC_WAITQ			0x24
 #define QCOM_SCM_WAITQ_RESUME			0x02
 #define QCOM_SCM_WAITQ_GET_WQ_CTX		0x03
+#define QCOM_SCM_WAITQ_GET_INFO		0x04
 
 #define QCOM_SCM_SVC_GPU			0x28
 #define QCOM_SCM_SVC_GPU_INIT_REGS		0x01

-- 
2.34.1

[PATCH v5 2/2] firmware: qcom_scm: Support multiple waitq contexts

[Unnathi Chalicheemala]

Currently, only a single waitqueue context exists, with waitqueue id zero.
Multi-waitqueue mechanism is added in firmware to support the case when
multiple VMs make SMC calls or single VM making multiple calls on same CPU.

When VMs make SMC call, firmware will allocate waitqueue context assuming
the SMC call to be a blocking call. SMC calls that cannot acquire resources
are returned to sleep in the calling VM. When resource is available, VM
will be notified to wake sleeping thread and resume SMC call.
SM8650 firmware can allocate two such waitq contexts so create these two
waitqueue contexts.

Unique waitqueue contexts are supported by a dynamically sized array where
each unique wq_ctx is associated with a struct completion variable for easy
lookup. To get the number of waitqueue contexts directly from firmware,
qcom_scm_query_waitq_cnt() is introduced. On older targets which support
only a single waitqueue, wq_cnt is set to 1 as SCM call for
query_waitq_cnt() is not implemented for single waitqueue case.

Signed-off-by: Unnathi Chalicheemala <unnathi.chalicheemala@oss.qualcomm.com>
---
 drivers/firmware/qcom/qcom_scm.c | 75 ++++++++++++++++++++++++++++------------
 1 file changed, 53 insertions(+), 22 deletions(-)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 1aa42685640da8a14191557896fbb49423697a10..ec139380ce5ba6d11f1023258e1d36edcf3d9d45 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -47,7 +47,7 @@ struct qcom_scm {
 	struct clk *iface_clk;
 	struct clk *bus_clk;
 	struct icc_path *path;
-	struct completion waitq_comp;
+	struct completion *waitq;
 	struct reset_controller_dev reset;
 
 	/* control access to the interconnect path */
@@ -57,6 +57,7 @@ struct qcom_scm {
 	u64 dload_mode_addr;
 
 	struct qcom_tzmem_pool *mempool;
+	unsigned int wq_cnt;
 };
 
 struct qcom_scm_current_perm_info {
@@ -2118,6 +2119,25 @@ static int qcom_scm_fill_irq_fwspec_params(struct irq_fwspec *fwspec, u32 virq)
 	return 0;
 }
 
+static int qcom_scm_query_waitq_count(struct qcom_scm *scm)
+{
+	int ret;
+	struct qcom_scm_desc desc = {
+		.svc = QCOM_SCM_SVC_WAITQ,
+		.cmd = QCOM_SCM_WAITQ_GET_INFO,
+		.owner = ARM_SMCCC_OWNER_SIP
+	};
+	struct qcom_scm_res res;
+
+	ret = qcom_scm_call_atomic(scm->dev, &desc, &res);
+	if (ret) {
+		dev_err(scm->dev, "Multi-waitqueue support unavailable\n");
+		return 1;
+	}
+
+	return res.result[0] & GENMASK(7, 0);
+}
+
 static int qcom_scm_get_waitq_irq(void)
 {
 	int ret;
@@ -2149,42 +2169,40 @@ static int qcom_scm_get_waitq_irq(void)
 	return ret;
 }
 
-static int qcom_scm_assert_valid_wq_ctx(u32 wq_ctx)
+static struct completion *qcom_scm_get_completion(u32 wq_ctx)
 {
-	/* FW currently only supports a single wq_ctx (zero).
-	 * TODO: Update this logic to include dynamic allocation and lookup of
-	 * completion structs when FW supports more wq_ctx values.
-	 */
-	if (wq_ctx != 0) {
-		dev_err(__scm->dev, "Firmware unexpectedly passed non-zero wq_ctx\n");
-		return -EINVAL;
-	}
+	struct completion *wq;
 
-	return 0;
+	if (WARN_ON_ONCE(wq_ctx >= __scm->wq_cnt))
+		return ERR_PTR(-EINVAL);
+
+	wq = &__scm->waitq[wq_ctx];
+
+	return wq;
 }
 
 int qcom_scm_wait_for_wq_completion(u32 wq_ctx)
 {
-	int ret;
+	struct completion *wq;
 
-	ret = qcom_scm_assert_valid_wq_ctx(wq_ctx);
-	if (ret)
-		return ret;
+	wq = qcom_scm_get_completion(wq_ctx);
+	if (IS_ERR(wq))
+		return PTR_ERR(wq);
 
-	wait_for_completion(&__scm->waitq_comp);
+	wait_for_completion(wq);
 
 	return 0;
 }
 
 static int qcom_scm_waitq_wakeup(unsigned int wq_ctx)
 {
-	int ret;
+	struct completion *wq;
 
-	ret = qcom_scm_assert_valid_wq_ctx(wq_ctx);
-	if (ret)
-		return ret;
+	wq = qcom_scm_get_completion(wq_ctx);
+	if (IS_ERR(wq))
+		return PTR_ERR(wq);
 
-	complete(&__scm->waitq_comp);
+	complete(wq);
 
 	return 0;
 }
@@ -2260,6 +2278,7 @@ static int qcom_scm_probe(struct platform_device *pdev)
 	struct qcom_tzmem_pool_config pool_config;
 	struct qcom_scm *scm;
 	int irq, ret;
+	int i;
 
 	scm = devm_kzalloc(&pdev->dev, sizeof(*scm), GFP_KERNEL);
 	if (!scm)
@@ -2270,7 +2289,19 @@ static int qcom_scm_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	init_completion(&scm->waitq_comp);
+	ret = qcom_scm_query_waitq_count(scm);
+	if (ret < 0)
+		return ret;
+
+	scm->wq_cnt = ret;
+
+	scm->waitq = devm_kcalloc(&pdev->dev, scm->wq_cnt, sizeof(*scm->waitq), GFP_KERNEL);
+	if (!scm->waitq)
+		return -ENOMEM;
+
+	for (i = 0; i < scm->wq_cnt; i++)
+		init_completion(&scm->waitq[i]);
+
 	mutex_init(&scm->scm_bw_lock);
 
 	scm->path = devm_of_icc_get(&pdev->dev, NULL);

-- 
2.34.1

Re: [PATCH v5 2/2] firmware: qcom_scm: Support multiple waitq contexts

[Bartosz Golaszewski]

On Fri, Feb 28, 2025 at 6:40 AM Unnathi Chalicheemala
<unnathi.chalicheemala@oss.qualcomm.com> wrote:
>
> Currently, only a single waitqueue context exists, with waitqueue id zero.
> Multi-waitqueue mechanism is added in firmware to support the case when
> multiple VMs make SMC calls or single VM making multiple calls on same CPU.
>
> When VMs make SMC call, firmware will allocate waitqueue context assuming
> the SMC call to be a blocking call. SMC calls that cannot acquire resources
> are returned to sleep in the calling VM. When resource is available, VM
> will be notified to wake sleeping thread and resume SMC call.
> SM8650 firmware can allocate two such waitq contexts so create these two
> waitqueue contexts.
>
> Unique waitqueue contexts are supported by a dynamically sized array where
> each unique wq_ctx is associated with a struct completion variable for easy
> lookup. To get the number of waitqueue contexts directly from firmware,
> qcom_scm_query_waitq_cnt() is introduced. On older targets which support

Seems like it's actually called qcom_scm_query_waitq_count

> only a single waitqueue, wq_cnt is set to 1 as SCM call for
> query_waitq_cnt() is not implemented for single waitqueue case.
>
> Signed-off-by: Unnathi Chalicheemala <unnathi.chalicheemala@oss.qualcomm.com>
> ---
>  drivers/firmware/qcom/qcom_scm.c | 75 ++++++++++++++++++++++++++++------------
>  1 file changed, 53 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 1aa42685640da8a14191557896fbb49423697a10..ec139380ce5ba6d11f1023258e1d36edcf3d9d45 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -47,7 +47,7 @@ struct qcom_scm {
>         struct clk *iface_clk;
>         struct clk *bus_clk;
>         struct icc_path *path;
> -       struct completion waitq_comp;
> +       struct completion *waitq;
>         struct reset_controller_dev reset;
>
>         /* control access to the interconnect path */
> @@ -57,6 +57,7 @@ struct qcom_scm {
>         u64 dload_mode_addr;
>
>         struct qcom_tzmem_pool *mempool;
> +       unsigned int wq_cnt;
>  };
>
>  struct qcom_scm_current_perm_info {
> @@ -2118,6 +2119,25 @@ static int qcom_scm_fill_irq_fwspec_params(struct irq_fwspec *fwspec, u32 virq)
>         return 0;
>  }
>
> +static int qcom_scm_query_waitq_count(struct qcom_scm *scm)
> +{
> +       int ret;
> +       struct qcom_scm_desc desc = {
> +               .svc = QCOM_SCM_SVC_WAITQ,
> +               .cmd = QCOM_SCM_WAITQ_GET_INFO,
> +               .owner = ARM_SMCCC_OWNER_SIP
> +       };
> +       struct qcom_scm_res res;
> +
> +       ret = qcom_scm_call_atomic(scm->dev, &desc, &res);

This can fail for a multitude of reasons - some of which we may want
to propagate to the caller, how about being more fine-grained and
using __qcom_scm_is_call_available() to check if
QCOM_SCM_WAITQ_GET_INFO is available first?

> +       if (ret) {
> +               dev_err(scm->dev, "Multi-waitqueue support unavailable\n");

Is this an error though? From the commit message it seems it's normal
operation on older platforms?

Bartosz


> +               return 1;
> +       }
> +
> +       return res.result[0] & GENMASK(7, 0);
> +}
> +
>  static int qcom_scm_get_waitq_irq(void)
>  {
>         int ret;
> @@ -2149,42 +2169,40 @@ static int qcom_scm_get_waitq_irq(void)
>         return ret;
>  }
>
> -static int qcom_scm_assert_valid_wq_ctx(u32 wq_ctx)
> +static struct completion *qcom_scm_get_completion(u32 wq_ctx)
>  {
> -       /* FW currently only supports a single wq_ctx (zero).
> -        * TODO: Update this logic to include dynamic allocation and lookup of
> -        * completion structs when FW supports more wq_ctx values.
> -        */
> -       if (wq_ctx != 0) {
> -               dev_err(__scm->dev, "Firmware unexpectedly passed non-zero wq_ctx\n");
> -               return -EINVAL;
> -       }
> +       struct completion *wq;
>
> -       return 0;
> +       if (WARN_ON_ONCE(wq_ctx >= __scm->wq_cnt))
> +               return ERR_PTR(-EINVAL);
> +
> +       wq = &__scm->waitq[wq_ctx];
> +
> +       return wq;
>  }
>
>  int qcom_scm_wait_for_wq_completion(u32 wq_ctx)
>  {
> -       int ret;
> +       struct completion *wq;
>
> -       ret = qcom_scm_assert_valid_wq_ctx(wq_ctx);
> -       if (ret)
> -               return ret;
> +       wq = qcom_scm_get_completion(wq_ctx);
> +       if (IS_ERR(wq))
> +               return PTR_ERR(wq);
>
> -       wait_for_completion(&__scm->waitq_comp);
> +       wait_for_completion(wq);
>
>         return 0;
>  }
>
>  static int qcom_scm_waitq_wakeup(unsigned int wq_ctx)
>  {
> -       int ret;
> +       struct completion *wq;
>
> -       ret = qcom_scm_assert_valid_wq_ctx(wq_ctx);
> -       if (ret)
> -               return ret;
> +       wq = qcom_scm_get_completion(wq_ctx);
> +       if (IS_ERR(wq))
> +               return PTR_ERR(wq);
>
> -       complete(&__scm->waitq_comp);
> +       complete(wq);
>
>         return 0;
>  }
> @@ -2260,6 +2278,7 @@ static int qcom_scm_probe(struct platform_device *pdev)
>         struct qcom_tzmem_pool_config pool_config;
>         struct qcom_scm *scm;
>         int irq, ret;
> +       int i;
>
>         scm = devm_kzalloc(&pdev->dev, sizeof(*scm), GFP_KERNEL);
>         if (!scm)
> @@ -2270,7 +2289,19 @@ static int qcom_scm_probe(struct platform_device *pdev)
>         if (ret < 0)
>                 return ret;
>
> -       init_completion(&scm->waitq_comp);
> +       ret = qcom_scm_query_waitq_count(scm);
> +       if (ret < 0)
> +               return ret;
> +
> +       scm->wq_cnt = ret;
> +
> +       scm->waitq = devm_kcalloc(&pdev->dev, scm->wq_cnt, sizeof(*scm->waitq), GFP_KERNEL);
> +       if (!scm->waitq)
> +               return -ENOMEM;
> +
> +       for (i = 0; i < scm->wq_cnt; i++)
> +               init_completion(&scm->waitq[i]);
> +
>         mutex_init(&scm->scm_bw_lock);
>
>         scm->path = devm_of_icc_get(&pdev->dev, NULL);
>
> --
> 2.34.1
>
>

Re: [PATCH v5 1/2] firmware: qcom_scm: Add API to get waitqueue IRQ info

[Bartosz Golaszewski]

On Fri, Feb 28, 2025 at 6:40 AM Unnathi Chalicheemala
<unnathi.chalicheemala@oss.qualcomm.com> wrote:
>
> Bootloader and firmware for SM8650 and older chipsets expect node
> name as "qcom_scm", in order to patch the wait queue IRQ information.
> However, DeviceTree uses node name "scm" and this mismatch prevents
> firmware from correctly identifying waitqueue IRQ information. Waitqueue
> IRQ is used for signaling between secure and non-secure worlds.
>
> To resolve this, introduce qcom_scm_get_waitq_irq() that'll get the
> hardware IRQ number to be used from firmware instead of relying on data
> provided by devicetree, thereby bypassing the DeviceTree node name
> mismatch.
>
> This hardware IRQ number is converted to a Linux IRQ number using newly
> defined fill_irq_fwspec_params(). This Linux IRQ number is then supplied
> to the threaded_irq call.
>
> Signed-off-by: Unnathi Chalicheemala <unnathi.chalicheemala@oss.qualcomm.com>
> ---

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Re: [PATCH v5 2/2] firmware: qcom_scm: Support multiple waitq contexts

[Unnathi Chalicheemala]

On 3/4/2025 4:49 AM, Bartosz Golaszewski wrote:
> On Fri, Feb 28, 2025 at 6:40 AM Unnathi Chalicheemala
> <unnathi.chalicheemala@oss.qualcomm.com> wrote:
>>
>> Currently, only a single waitqueue context exists, with waitqueue id zero.
>> Multi-waitqueue mechanism is added in firmware to support the case when
>> multiple VMs make SMC calls or single VM making multiple calls on same CPU.
>>
>> When VMs make SMC call, firmware will allocate waitqueue context assuming
>> the SMC call to be a blocking call. SMC calls that cannot acquire resources
>> are returned to sleep in the calling VM. When resource is available, VM
>> will be notified to wake sleeping thread and resume SMC call.
>> SM8650 firmware can allocate two such waitq contexts so create these two
>> waitqueue contexts.
>>
>> Unique waitqueue contexts are supported by a dynamically sized array where
>> each unique wq_ctx is associated with a struct completion variable for easy
>> lookup. To get the number of waitqueue contexts directly from firmware,
>> qcom_scm_query_waitq_cnt() is introduced. On older targets which support
> 
> Seems like it's actually called qcom_scm_query_waitq_count
> 

Yes my bad. Will correct this in next series.

>> only a single waitqueue, wq_cnt is set to 1 as SCM call for
>> query_waitq_cnt() is not implemented for single waitqueue case.
>>
>> Signed-off-by: Unnathi Chalicheemala <unnathi.chalicheemala@oss.qualcomm.com>
>> ---
>>  drivers/firmware/qcom/qcom_scm.c | 75 ++++++++++++++++++++++++++++------------
>>  1 file changed, 53 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
>> index 1aa42685640da8a14191557896fbb49423697a10..ec139380ce5ba6d11f1023258e1d36edcf3d9d45 100644
>> --- a/drivers/firmware/qcom/qcom_scm.c
>> +++ b/drivers/firmware/qcom/qcom_scm.c
>> @@ -47,7 +47,7 @@ struct qcom_scm {
>>         struct clk *iface_clk;
>>         struct clk *bus_clk;
>>         struct icc_path *path;
>> -       struct completion waitq_comp;
>> +       struct completion *waitq;
>>         struct reset_controller_dev reset;
>>
>>         /* control access to the interconnect path */
>> @@ -57,6 +57,7 @@ struct qcom_scm {
>>         u64 dload_mode_addr;
>>
>>         struct qcom_tzmem_pool *mempool;
>> +       unsigned int wq_cnt;
>>  };
>>
>>  struct qcom_scm_current_perm_info {
>> @@ -2118,6 +2119,25 @@ static int qcom_scm_fill_irq_fwspec_params(struct irq_fwspec *fwspec, u32 virq)
>>         return 0;
>>  }
>>
>> +static int qcom_scm_query_waitq_count(struct qcom_scm *scm)
>> +{
>> +       int ret;
>> +       struct qcom_scm_desc desc = {
>> +               .svc = QCOM_SCM_SVC_WAITQ,
>> +               .cmd = QCOM_SCM_WAITQ_GET_INFO,
>> +               .owner = ARM_SMCCC_OWNER_SIP
>> +       };
>> +       struct qcom_scm_res res;
>> +
>> +       ret = qcom_scm_call_atomic(scm->dev, &desc, &res);
> 
> This can fail for a multitude of reasons - some of which we may want
> to propagate to the caller, how about being more fine-grained and
> using __qcom_scm_is_call_available() to check if
> QCOM_SCM_WAITQ_GET_INFO is available first?
> 

I agree, will return 1 in the case call is unavailable.

Thanks for your review Bartosz!

>> +       if (ret) {
>> +               dev_err(scm->dev, "Multi-waitqueue support unavailable\n");
> 
> Is this an error though? From the commit message it seems it's normal
> operation on older platforms?
> 
> Bartosz
> 
> 
>> +               return 1;
>> +       }
>> +
>> +       return res.result[0] & GENMASK(7, 0);
>> +}
>> +
>>  static int qcom_scm_get_waitq_irq(void)
>>  {
>>         int ret;
>> @@ -2149,42 +2169,40 @@ static int qcom_scm_get_waitq_irq(void)
>>         return ret;
>>  }
>>
>> -static int qcom_scm_assert_valid_wq_ctx(u32 wq_ctx)
>> +static struct completion *qcom_scm_get_completion(u32 wq_ctx)
>>  {
>> -       /* FW currently only supports a single wq_ctx (zero).
>> -        * TODO: Update this logic to include dynamic allocation and lookup of
>> -        * completion structs when FW supports more wq_ctx values.
>> -        */
>> -       if (wq_ctx != 0) {
>> -               dev_err(__scm->dev, "Firmware unexpectedly passed non-zero wq_ctx\n");
>> -               return -EINVAL;
>> -       }
>> +       struct completion *wq;
>>
>> -       return 0;
>> +       if (WARN_ON_ONCE(wq_ctx >= __scm->wq_cnt))
>> +               return ERR_PTR(-EINVAL);
>> +
>> +       wq = &__scm->waitq[wq_ctx];
>> +
>> +       return wq;
>>  }
>>
>>  int qcom_scm_wait_for_wq_completion(u32 wq_ctx)
>>  {
>> -       int ret;
>> +       struct completion *wq;
>>
>> -       ret = qcom_scm_assert_valid_wq_ctx(wq_ctx);
>> -       if (ret)
>> -               return ret;
>> +       wq = qcom_scm_get_completion(wq_ctx);
>> +       if (IS_ERR(wq))
>> +               return PTR_ERR(wq);
>>
>> -       wait_for_completion(&__scm->waitq_comp);
>> +       wait_for_completion(wq);
>>
>>         return 0;
>>  }
>>
>>  static int qcom_scm_waitq_wakeup(unsigned int wq_ctx)
>>  {
>> -       int ret;
>> +       struct completion *wq;
>>
>> -       ret = qcom_scm_assert_valid_wq_ctx(wq_ctx);
>> -       if (ret)
>> -               return ret;
>> +       wq = qcom_scm_get_completion(wq_ctx);
>> +       if (IS_ERR(wq))
>> +               return PTR_ERR(wq);
>>
>> -       complete(&__scm->waitq_comp);
>> +       complete(wq);
>>
>>         return 0;
>>  }
>> @@ -2260,6 +2278,7 @@ static int qcom_scm_probe(struct platform_device *pdev)
>>         struct qcom_tzmem_pool_config pool_config;
>>         struct qcom_scm *scm;
>>         int irq, ret;
>> +       int i;
>>
>>         scm = devm_kzalloc(&pdev->dev, sizeof(*scm), GFP_KERNEL);
>>         if (!scm)
>> @@ -2270,7 +2289,19 @@ static int qcom_scm_probe(struct platform_device *pdev)
>>         if (ret < 0)
>>                 return ret;
>>
>> -       init_completion(&scm->waitq_comp);
>> +       ret = qcom_scm_query_waitq_count(scm);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       scm->wq_cnt = ret;
>> +
>> +       scm->waitq = devm_kcalloc(&pdev->dev, scm->wq_cnt, sizeof(*scm->waitq), GFP_KERNEL);
>> +       if (!scm->waitq)
>> +               return -ENOMEM;
>> +
>> +       for (i = 0; i < scm->wq_cnt; i++)
>> +               init_completion(&scm->waitq[i]);
>> +
>>         mutex_init(&scm->scm_bw_lock);
>>
>>         scm->path = devm_of_icc_get(&pdev->dev, NULL);
>>
>> --
>> 2.34.1
>>
>>