Re: [PATCH v11 1/4] firmware: qcom: scm: provide a read-modify-write function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 1/9/2024 10:34 AM, Pavan Kondeti wrote:
On Mon, Jan 08, 2024 at 08:57:31PM +0530, Mukesh Ojha wrote:
It was realized by Srinivas K. that there is a need of
read-modify-write scm exported function so that it can
be used by multiple clients.

Let's introduce qcom_scm_io_rmw() which masks out the bits
and write the passed value to that bit-offset.

Suggested-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
Signed-off-by: Mukesh Ojha <quic_mojha@xxxxxxxxxxx>
Tested-by: Kathiravan Thirumoorthy <quic_kathirav@xxxxxxxxxxx> # IPQ9574 and IPQ5332
---
  drivers/firmware/qcom/qcom_scm.c       | 26 ++++++++++++++++++++++++++
  include/linux/firmware/qcom/qcom_scm.h |  1 +
  2 files changed, 27 insertions(+)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 520de9b5633a..25549178a30f 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -19,6 +19,7 @@
  #include <linux/of_irq.h>
  #include <linux/of_platform.h>
  #include <linux/platform_device.h>
+#include <linux/spinlock.h>
  #include <linux/reset-controller.h>
  #include <linux/types.h>
@@ -41,6 +42,8 @@ struct qcom_scm {
  	int scm_vote_count;
u64 dload_mode_addr;
+	/* Atomic context only */
+	spinlock_t lock;

IMHO, this comment can be confusing later. one might think that
qcom_scm_call_atomic() needs to be called with this lock, but that does
not seems to be the intention here.

  };
struct qcom_scm_current_perm_info {
@@ -481,6 +484,28 @@ static int qcom_scm_disable_sdi(void)
  	return ret ? : res.result[0];
  }
+int qcom_scm_io_rmw(phys_addr_t addr, unsigned int mask, unsigned int val)
+{
+	unsigned int old, new;
+	int ret;
+
+	if (!__scm)
+		return -EINVAL;
+
+	spin_lock(&__scm->lock);

So, this function can't be called from hardirq context. If that ever
happens, with this new spinlock (without disabling interrupts), can
result in deadlock.

Ok, let's make it fully atomic with spin_lock_irqsave();

-Mukesh

+	ret = qcom_scm_io_readl(addr, &old);
+	if (ret)
+		goto unlock;
+
+	new = (old & ~mask) | (val & mask);
+
+	ret = qcom_scm_io_writel(addr, new);
+unlock:
+	spin_unlock(&__scm->lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(qcom_scm_io_rmw);

Thanks,
Pavan




[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux