Re: [PATCH v2 2/4] firmware: scm: Modify only the DLOAD bit in TCSR register for download mode

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

 





On 3/1/2023 4:09 PM, Dmitry Baryshkov wrote:
On 01/03/2023 11:55, Mukesh Ojha wrote:
CrashDump collection is based on the DLOAD bit of TCSR register.
To retain other bits, we read the register and modify only the
DLOAD bit as the other bits have their own significance.

Originally-by: Poovendhan Selvaraj <quic_poovendh@xxxxxxxxxxx>
Signed-off-by: Mukesh Ojha <quic_mojha@xxxxxxxxxxx>
---
Changes in v2:
  - Addressed comment made by Bjorn.
  - Added download mask from patch 3 to this.

  drivers/firmware/qcom_scm.c | 17 +++++++++++++++--
  1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 51eb853..c9f1fad 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -27,6 +27,8 @@ module_param(download_mode, bool, 0);
  #define SCM_HAS_IFACE_CLK    BIT(1)
  #define SCM_HAS_BUS_CLK        BIT(2)
+#define QCOM_DOWNLOAD_MODE_MASK 0x30
+
  struct qcom_scm {
      struct device *dev;
      struct clk *core_clk;
@@ -419,6 +421,7 @@ static void qcom_scm_set_download_mode(bool enable)
  {
      bool avail;
      int ret = 0;
+    u32 val;
      avail = __qcom_scm_is_call_available(__scm->dev,
                           QCOM_SCM_SVC_BOOT,
@@ -426,8 +429,18 @@ static void qcom_scm_set_download_mode(bool enable)
      if (avail) {
          ret = __qcom_scm_set_dload_mode(__scm->dev, enable);
      } else if (__scm->dload_mode_addr) {
-        ret = qcom_scm_io_writel(__scm->dload_mode_addr,
-                enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0);
+        ret = qcom_scm_io_readl(__scm->dload_mode_addr, &val);
+        if (ret) {
+            dev_err(__scm->dev,
+                "failed to read dload mode address value: %d\n", ret);
+            return;
+        }
+
+        val &= ~QCOM_DOWNLOAD_MODE_MASK;
+        if (enable)
+            val |= QCOM_SCM_BOOT_SET_DLOAD_MODE;
+
+        ret = qcom_scm_io_writel(__scm->dload_mode_addr, val);

Any locking for this RMW?

While you ask this, i thought about who all are the user of this function. Only, multiple calls to module param callback where
this race could be possible.

I am doubtful, if introducing global mutex lock will be allowed to handle this. Any comments.


-Mukesh

      } else {
          dev_err(__scm->dev,
              "No available mechanism for setting download mode\n");




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux