diff mbox series

[REBASE,v5,15/17] firmware: scm: Modify only the download bits in TCSR register

Message ID 1694429639-21484-16-git-send-email-quic_mojha@quicinc.com
State Superseded
Headers show
Series Add Qualcomm Minidump kernel driver related support | expand

Commit Message

Mukesh Ojha Sept. 11, 2023, 10:53 a.m. UTC
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@quicinc.com>
Signed-off-by: Poovendhan Selvaraj <quic_poovendh@quicinc.com>
Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
Tested-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com> # IPQ9574 and IPQ5332
---
 drivers/firmware/qcom_scm.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Comments

Kathiravan Thirumoorthy Sept. 11, 2023, 3:07 p.m. UTC | #1
On 9/11/2023 4:23 PM, 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.
>
> Co-developed-by: Poovendhan Selvaraj <quic_poovendh@quicinc.com>
> Signed-off-by: Poovendhan Selvaraj <quic_poovendh@quicinc.com>
> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
> Tested-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com> # IPQ9574 and IPQ5332
> ---
>   drivers/firmware/qcom_scm.c | 16 ++++++++++++++--
>   1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index 321133f0950d..5cacae63ee2a 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -5,6 +5,8 @@
>   #include <linux/platform_device.h>
>   #include <linux/init.h>
>   #include <linux/interrupt.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
>   #include <linux/completion.h>
>   #include <linux/cpumask.h>
>   #include <linux/export.h>
> @@ -26,6 +28,14 @@
>   static bool download_mode = IS_ENABLED(CONFIG_QCOM_SCM_DOWNLOAD_MODE_DEFAULT);
>   module_param(download_mode, bool, 0);
>   
> +#define SCM_HAS_CORE_CLK	BIT(0)
> +#define SCM_HAS_IFACE_CLK	BIT(1)
> +#define SCM_HAS_BUS_CLK		BIT(2)


Is this intentional to add these macros back again?


> +
> +#define QCOM_DLOAD_MASK		GENMASK(5, 4)
> +#define QCOM_DLOAD_FULLDUMP	0x1
> +#define QCOM_DLOAD_NODUMP	0x0
> +
>   struct qcom_scm {
>   	struct device *dev;
>   	struct clk *core_clk;
> @@ -440,6 +450,7 @@ static int __qcom_scm_set_dload_mode(struct device *dev, bool enable)
>   
>   static void qcom_scm_set_download_mode(bool enable)
>   {
> +	u32 val = enable ? QCOM_DLOAD_FULLDUMP : QCOM_DLOAD_NODUMP;
>   	bool avail;
>   	int ret = 0;
>   
> @@ -449,8 +460,9 @@ 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_update_field(__scm->dload_mode_addr,
> +					       QCOM_DLOAD_MASK,
> +					       FIELD_PREP(QCOM_DLOAD_MASK, val));
>   	} else {
>   		dev_err(__scm->dev,
>   			"No available mechanism for setting download mode\n");
Mukesh Ojha Sept. 12, 2023, 9:09 a.m. UTC | #2
On 9/11/2023 8:37 PM, Kathiravan Thirumoorthy wrote:
> 
> On 9/11/2023 4:23 PM, 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.
>>
>> Co-developed-by: Poovendhan Selvaraj <quic_poovendh@quicinc.com>
>> Signed-off-by: Poovendhan Selvaraj <quic_poovendh@quicinc.com>
>> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
>> Tested-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com> # 
>> IPQ9574 and IPQ5332
>> ---
>>   drivers/firmware/qcom_scm.c | 16 ++++++++++++++--
>>   1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
>> index 321133f0950d..5cacae63ee2a 100644
>> --- a/drivers/firmware/qcom_scm.c
>> +++ b/drivers/firmware/qcom_scm.c
>> @@ -5,6 +5,8 @@
>>   #include <linux/platform_device.h>
>>   #include <linux/init.h>
>>   #include <linux/interrupt.h>
>> +#include <linux/bitfield.h>
>> +#include <linux/bits.h>
>>   #include <linux/completion.h>
>>   #include <linux/cpumask.h>
>>   #include <linux/export.h>
>> @@ -26,6 +28,14 @@
>>   static bool download_mode = 
>> IS_ENABLED(CONFIG_QCOM_SCM_DOWNLOAD_MODE_DEFAULT);
>>   module_param(download_mode, bool, 0);
>> +#define SCM_HAS_CORE_CLK    BIT(0)
>> +#define SCM_HAS_IFACE_CLK    BIT(1)
>> +#define SCM_HAS_BUS_CLK        BIT(2)
> 
> 
> Is this intentional to add these macros back again?

This is a mistake, thanks for letting me know.

-Mukesh
> 
> 
>> +
>> +#define QCOM_DLOAD_MASK        GENMASK(5, 4)
>> +#define QCOM_DLOAD_FULLDUMP    0x1
>> +#define QCOM_DLOAD_NODUMP    0x0
>> +
>>   struct qcom_scm {
>>       struct device *dev;
>>       struct clk *core_clk;
>> @@ -440,6 +450,7 @@ static int __qcom_scm_set_dload_mode(struct device 
>> *dev, bool enable)
>>   static void qcom_scm_set_download_mode(bool enable)
>>   {
>> +    u32 val = enable ? QCOM_DLOAD_FULLDUMP : QCOM_DLOAD_NODUMP;
>>       bool avail;
>>       int ret = 0;
>> @@ -449,8 +460,9 @@ 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_update_field(__scm->dload_mode_addr,
>> +                           QCOM_DLOAD_MASK,
>> +                           FIELD_PREP(QCOM_DLOAD_MASK, val));
>>       } else {
>>           dev_err(__scm->dev,
>>               "No available mechanism for setting download mode\n");
diff mbox series

Patch

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 321133f0950d..5cacae63ee2a 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -5,6 +5,8 @@ 
 #include <linux/platform_device.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
 #include <linux/completion.h>
 #include <linux/cpumask.h>
 #include <linux/export.h>
@@ -26,6 +28,14 @@ 
 static bool download_mode = IS_ENABLED(CONFIG_QCOM_SCM_DOWNLOAD_MODE_DEFAULT);
 module_param(download_mode, bool, 0);
 
+#define SCM_HAS_CORE_CLK	BIT(0)
+#define SCM_HAS_IFACE_CLK	BIT(1)
+#define SCM_HAS_BUS_CLK		BIT(2)
+
+#define QCOM_DLOAD_MASK		GENMASK(5, 4)
+#define QCOM_DLOAD_FULLDUMP	0x1
+#define QCOM_DLOAD_NODUMP	0x0
+
 struct qcom_scm {
 	struct device *dev;
 	struct clk *core_clk;
@@ -440,6 +450,7 @@  static int __qcom_scm_set_dload_mode(struct device *dev, bool enable)
 
 static void qcom_scm_set_download_mode(bool enable)
 {
+	u32 val = enable ? QCOM_DLOAD_FULLDUMP : QCOM_DLOAD_NODUMP;
 	bool avail;
 	int ret = 0;
 
@@ -449,8 +460,9 @@  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_update_field(__scm->dload_mode_addr,
+					       QCOM_DLOAD_MASK,
+					       FIELD_PREP(QCOM_DLOAD_MASK, val));
 	} else {
 		dev_err(__scm->dev,
 			"No available mechanism for setting download mode\n");