Re: [PATCH V5 3/3] firmware: scm: Modify only the download bits in TCSR register

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

 




On 7/25/2023 12:35 AM, Elliot Berman wrote:


On 7/20/2023 12:04 AM, Kathiravan T wrote:
From: Mukesh Ojha <quic_mojha@xxxxxxxxxxx>

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.

Co-developed-by: Poovendhan Selvaraj <quic_poovendh@xxxxxxxxxxx>
Signed-off-by: Poovendhan Selvaraj <quic_poovendh@xxxxxxxxxxx>
Signed-off-by: Mukesh Ojha <quic_mojha@xxxxxxxxxxx>
Signed-off-by: Kathiravan T <quic_kathirav@xxxxxxxxxxx>
---
Changes in V5:
    - Added the Signed-off-by tag for user Poovendhan
    - Dropped the macro QCOM_DOWNLOAD_MODE_SHIFT in the favor of
      PREP_FIELD

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

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 104d86e49b97..3830dcf14326 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -30,6 +30,10 @@ module_param(download_mode, bool, 0);
  #define SCM_HAS_IFACE_CLK    BIT(1)
  #define SCM_HAS_BUS_CLK        BIT(2)
  +#define QCOM_DOWNLOAD_FULLDUMP        0x1
+#define QCOM_DOWNLOAD_NODUMP        0x0
+#define QCOM_DOWNLOAD_MODE_MASK        BIT(4)
+

Can you update __qcom_scm_set_dload_mode to use the FIELD_PREP bits as well? Ideally, you should be able to have no duplicate logic in __qcom_scm_set_dload_mode and in qcom_scm_set_download_mode. Before your patch, it was duplicated and we probably should've had it de-duplicated. With this patch, the logic and constants used have diverged when they don't need to.


Sure, will check this.



  struct qcom_scm {
      struct device *dev;
      struct clk *core_clk;
@@ -440,6 +444,7 @@ static int __qcom_scm_set_dload_mode(struct device *dev, bool enable)
  static void qcom_scm_set_download_mode(bool enable)
  {
      bool avail;
+    int val;
      int ret = 0;
        avail = __qcom_scm_is_call_available(__scm->dev,
@@ -448,8 +453,10 @@ 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);
+        val = enable ? QCOM_DOWNLOAD_FULLDUMP : QCOM_DOWNLOAD_NODUMP;
+        ret = qcom_scm_io_update_field(__scm->dload_mode_addr,
+                           QCOM_DOWNLOAD_MODE_MASK,
+                           FIELD_PREP(QCOM_DOWNLOAD_MODE_MASK, val));
      } else {
          dev_err(__scm->dev,
              "No available mechanism for setting download mode\n");



[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