Intention of this series to support multiple download mode and
only modify the required bits during setting tcsr register.
Other download modes are minidump, bothdump (full dump + minidump).
Latest minidump kernel driver patches has been sent here
https://lore.kernel.org/lkml/[email protected]/
Also, this series should be applied on
https://lore.kernel.org/lkml/[email protected]/
Changes in v5:
- Tried to fix the issue reported by kernel test robot
https://lore.kernel.org/lkml/[email protected]/
- Applied some of the improvement suggested by [Bjorn.andersson]
. Dropped 'both' instead support full,mini or mini,full for setting download
mode to collect both minidump and full dump.
. logging improvement.
Changes in v4: https://lore.kernel.org/lkml/[email protected]/
- val should be shifted within the function [srinivas.kandagatla]
i.e new = (old & ~mask) | (val << ffs(mask) - 1);
- Added Acked-by [linus.walleij] on pinctrl change.
Changes in v3 : https://lore.kernel.org/lkml/[email protected]/
- Removed [1] from the series and sent as a separate patch[2], although this series
should be applied on top [2].
[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/
- Introduce new exported symbol on suggestion from [srinivas.kandagatla]
- Use the symbol from drivers/pinctrl/qcom/pinctrl-msm.c.
- Addressed comment given by [dmitry.baryshkov]
- Converted non-standard Originally-by to Signed-off-by.
Changes in v2: https://lore.kernel.org/lkml/[email protected]/
- Addressed comment made by [bjorn]
- Added download mask.
- Passed download mode as parameter
- Accept human accepatable download mode string.
- enable = !!dload_mode
- Shifted module param callback to somewhere down in
the file so that it no longer need to know the
prototype of qcom_scm_set_download_mode()
- updated commit text.
Mukesh Ojha (5):
firmware: qcom_scm: provide a read-modify-write function
pinctrl: qcom: Use qcom_scm_io_update_field()
firmware: scm: Modify only the download bits in TCSR register
firmware: qcom_scm: Refactor code to support multiple download mode
firmware: qcom_scm: Add multiple download mode support
drivers/firmware/Kconfig | 11 -----
drivers/firmware/qcom_scm.c | 89 +++++++++++++++++++++++++++++++---
drivers/pinctrl/qcom/pinctrl-msm.c | 11 ++---
include/linux/firmware/qcom/qcom_scm.h | 2 +
4 files changed, 87 insertions(+), 26 deletions(-)
--
2.7.4
Use qcom_scm_io_update_field() exported function introduced
in last commit.
Acked-by: Linus Walleij <[email protected]>
Signed-off-by: Mukesh Ojha <[email protected]>
---
drivers/pinctrl/qcom/pinctrl-msm.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index daeb79a..6e1a5e4 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -1016,6 +1016,7 @@ static int msm_gpio_irq_set_type(struct irq_data *d, unsigned int type)
unsigned long flags;
bool was_enabled;
u32 val;
+ u32 mask;
if (msm_gpio_needs_dual_edge_parent_workaround(d, type)) {
set_bit(d->hwirq, pctrl->dual_edge_irqs);
@@ -1049,23 +1050,19 @@ static int msm_gpio_irq_set_type(struct irq_data *d, unsigned int type)
* With intr_target_use_scm interrupts are routed to
* application cpu using scm calls.
*/
+ mask = (7 << g->intr_target_bit);
if (pctrl->intr_target_use_scm) {
u32 addr = pctrl->phys_base[0] + g->intr_target_reg;
int ret;
- qcom_scm_io_readl(addr, &val);
-
- val &= ~(7 << g->intr_target_bit);
- val |= g->intr_target_kpss_val << g->intr_target_bit;
-
- ret = qcom_scm_io_writel(addr, val);
+ ret = qcom_scm_io_update_field(addr, mask, g->intr_target_kpss_val);
if (ret)
dev_err(pctrl->dev,
"Failed routing %lu interrupt to Apps proc",
d->hwirq);
} else {
val = msm_readl_intr_target(pctrl, g);
- val &= ~(7 << g->intr_target_bit);
+ val &= ~mask;
val |= g->intr_target_kpss_val << g->intr_target_bit;
msm_writel_intr_target(val, pctrl, g);
}
--
2.7.4
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.
Signed-off-by: Poovendhan Selvaraj <[email protected]>
Signed-off-by: Mukesh Ojha <[email protected]>
---
drivers/firmware/qcom_scm.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index cb0bc32..8e39b97 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -30,6 +30,9 @@ 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
+#define QCOM_DOWNLOAD_FULLDUMP 0x1
+
struct qcom_scm {
struct device *dev;
struct clk *core_clk;
@@ -448,8 +451,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_DOWNLOAD_MODE_MASK,
+ enable ? QCOM_DOWNLOAD_FULLDUMP : 0);
} else {
dev_err(__scm->dev,
"No available mechanism for setting download mode\n");
--
2.7.4
Currently on Qualcomm SoC, download_mode is enabled if
CONFIG_QCOM_SCM_DOWNLOAD_MODE_DEFAULT is selected.
Refactor the code such that it supports multiple download
modes and drop CONFIG_QCOM_SCM_DOWNLOAD_MODE_DEFAULT config
instead, give interface to set the download mode from
module parameter.
Signed-off-by: Mukesh Ojha <[email protected]>
---
drivers/firmware/Kconfig | 11 ---------
drivers/firmware/qcom_scm.c | 60 +++++++++++++++++++++++++++++++++++++++------
2 files changed, 52 insertions(+), 19 deletions(-)
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index b59e304..ff7e9f3 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -215,17 +215,6 @@ config MTK_ADSP_IPC
config QCOM_SCM
tristate
-config QCOM_SCM_DOWNLOAD_MODE_DEFAULT
- bool "Qualcomm download mode enabled by default"
- depends on QCOM_SCM
- help
- A device with "download mode" enabled will upon an unexpected
- warm-restart enter a special debug mode that allows the user to
- "download" memory content over USB for offline postmortem analysis.
- The feature can be enabled/disabled on the kernel command line.
-
- Say Y here to enable "download mode" by default.
-
config SYSFB
bool
select BOOT_VESA_SUPPORT
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 8e39b97..53ee301 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -20,11 +20,11 @@
#include <linux/clk.h>
#include <linux/reset-controller.h>
#include <linux/arm-smccc.h>
+#include <linux/kstrtox.h>
#include "qcom_scm.h"
-static bool download_mode = IS_ENABLED(CONFIG_QCOM_SCM_DOWNLOAD_MODE_DEFAULT);
-module_param(download_mode, bool, 0);
+static u32 download_mode;
#define SCM_HAS_CORE_CLK BIT(0)
#define SCM_HAS_IFACE_CLK BIT(1)
@@ -32,6 +32,7 @@ module_param(download_mode, bool, 0);
#define QCOM_DOWNLOAD_MODE_MASK 0x30
#define QCOM_DOWNLOAD_FULLDUMP 0x1
+#define QCOM_DOWNLOAD_NODUMP 0x0
struct qcom_scm {
struct device *dev;
@@ -440,8 +441,9 @@ static int __qcom_scm_set_dload_mode(struct device *dev, bool enable)
return qcom_scm_call_atomic(__scm->dev, &desc, NULL);
}
-static void qcom_scm_set_download_mode(bool enable)
+static void qcom_scm_set_download_mode(u32 download_mode)
{
+ bool enable = !!download_mode;
bool avail;
int ret = 0;
@@ -452,8 +454,7 @@ static void qcom_scm_set_download_mode(bool enable)
ret = __qcom_scm_set_dload_mode(__scm->dev, enable);
} else if (__scm->dload_mode_addr) {
ret = qcom_scm_io_update_field(__scm->dload_mode_addr,
- QCOM_DOWNLOAD_MODE_MASK,
- enable ? QCOM_DOWNLOAD_FULLDUMP : 0);
+ QCOM_DOWNLOAD_MODE_MASK, download_mode);
} else {
dev_err(__scm->dev,
"No available mechanism for setting download mode\n");
@@ -1419,6 +1420,49 @@ static irqreturn_t qcom_scm_irq_handler(int irq, void *data)
return IRQ_HANDLED;
}
+
+static int get_download_mode(char *buffer, const struct kernel_param *kp)
+{
+ int len = 0;
+
+ if (download_mode == QCOM_DOWNLOAD_FULLDUMP)
+ len = sysfs_emit(buffer, "full\n");
+ else if (download_mode == QCOM_DOWNLOAD_NODUMP)
+ len = sysfs_emit(buffer, "off\n");
+
+ return len;
+}
+
+static int set_download_mode(const char *val, const struct kernel_param *kp)
+{
+ u32 old = download_mode;
+
+ if (!strncmp(val, "full", strlen("full"))) {
+ download_mode = QCOM_DOWNLOAD_FULLDUMP;
+ } else if (!strncmp(val, "off", strlen("off"))) {
+ download_mode = QCOM_DOWNLOAD_NODUMP;
+ } else if (kstrtouint(val, 0, &download_mode) ||
+ !(download_mode == 0 || download_mode == 1)) {
+ download_mode = old;
+ pr_err("qcom_scm: unknown download mode: %s\n", val);
+ return -EINVAL;
+ }
+
+ if (__scm)
+ qcom_scm_set_download_mode(download_mode);
+
+ return 0;
+}
+
+static const struct kernel_param_ops download_mode_param_ops = {
+ .get = get_download_mode,
+ .set = set_download_mode,
+};
+
+module_param_cb(download_mode, &download_mode_param_ops, NULL, 0644);
+MODULE_PARM_DESC(download_mode,
+ "Download mode: off/full or 0/1 for existing users");
+
static int qcom_scm_probe(struct platform_device *pdev)
{
struct qcom_scm *scm;
@@ -1512,12 +1556,12 @@ static int qcom_scm_probe(struct platform_device *pdev)
__get_convention();
/*
- * If requested enable "download mode", from this point on warmboot
+ * If "download mode" is requested, from this point on warmboot
* will cause the boot stages to enter download mode, unless
* disabled below by a clean shutdown/reboot.
*/
if (download_mode)
- qcom_scm_set_download_mode(true);
+ qcom_scm_set_download_mode(download_mode);
return 0;
}
@@ -1525,7 +1569,7 @@ static int qcom_scm_probe(struct platform_device *pdev)
static void qcom_scm_shutdown(struct platform_device *pdev)
{
/* Clean shutdown, disable download mode to allow normal restart */
- qcom_scm_set_download_mode(false);
+ qcom_scm_set_download_mode(QCOM_DOWNLOAD_NODUMP);
}
static const struct of_device_id qcom_scm_dt_match[] = {
--
2.7.4
Currently, scm driver only supports full dump when download
mode is selected. Add support to enable minidump as well as
enable it along with fulldump.
Signed-off-by: Mukesh Ojha <[email protected]>
---
drivers/firmware/qcom_scm.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 53ee301..2a16431 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -32,6 +32,8 @@ static u32 download_mode;
#define QCOM_DOWNLOAD_MODE_MASK 0x30
#define QCOM_DOWNLOAD_FULLDUMP 0x1
+#define QCOM_DOWNLOAD_MINIDUMP 0x2
+#define QCOM_DOWNLOAD_BOTHDUMP (QCOM_DOWNLOAD_FULLDUMP | QCOM_DOWNLOAD_MINIDUMP)
#define QCOM_DOWNLOAD_NODUMP 0x0
struct qcom_scm {
@@ -1420,13 +1422,16 @@ static irqreturn_t qcom_scm_irq_handler(int irq, void *data)
return IRQ_HANDLED;
}
-
static int get_download_mode(char *buffer, const struct kernel_param *kp)
{
int len = 0;
if (download_mode == QCOM_DOWNLOAD_FULLDUMP)
len = sysfs_emit(buffer, "full\n");
+ else if (download_mode == QCOM_DOWNLOAD_MINIDUMP)
+ len = sysfs_emit(buffer, "mini\n");
+ else if (download_mode == QCOM_DOWNLOAD_BOTHDUMP)
+ len = sysfs_emit(buffer, "full,mini\n");
else if (download_mode == QCOM_DOWNLOAD_NODUMP)
len = sysfs_emit(buffer, "off\n");
@@ -1437,8 +1442,13 @@ static int set_download_mode(const char *val, const struct kernel_param *kp)
{
u32 old = download_mode;
- if (!strncmp(val, "full", strlen("full"))) {
+ if (!strncmp(val, "full,mini", strlen("full,mini")) ||
+ !strncmp(val, "mini,full", strlen("mini,full"))) {
+ download_mode = QCOM_DOWNLOAD_BOTHDUMP;
+ } else if (!strncmp(val, "full", strlen("full"))) {
download_mode = QCOM_DOWNLOAD_FULLDUMP;
+ } else if (!strncmp(val, "mini", strlen("mini"))) {
+ download_mode = QCOM_DOWNLOAD_MINIDUMP;
} else if (!strncmp(val, "off", strlen("off"))) {
download_mode = QCOM_DOWNLOAD_NODUMP;
} else if (kstrtouint(val, 0, &download_mode) ||
@@ -1461,7 +1471,7 @@ static const struct kernel_param_ops download_mode_param_ops = {
module_param_cb(download_mode, &download_mode_param_ops, NULL, 0644);
MODULE_PARM_DESC(download_mode,
- "Download mode: off/full or 0/1 for existing users");
+ "download mode: off/full/mini/full,mini or mini,full and 0/1 for existing users");
static int qcom_scm_probe(struct platform_device *pdev)
{
--
2.7.4
Please ignore this series as i have sent v6.
https://lore.kernel.org/lkml/[email protected]/
-Mukesh
On 3/28/2023 9:07 PM, Mukesh Ojha wrote:
> Intention of this series to support multiple download mode and
> only modify the required bits during setting tcsr register.
>
> Other download modes are minidump, bothdump (full dump + minidump).
>
> Latest minidump kernel driver patches has been sent here
> https://lore.kernel.org/lkml/[email protected]/
>
> Also, this series should be applied on
> https://lore.kernel.org/lkml/[email protected]/
>
> Changes in v5:
> - Tried to fix the issue reported by kernel test robot
> https://lore.kernel.org/lkml/[email protected]/
>
> - Applied some of the improvement suggested by [Bjorn.andersson]
>
> . Dropped 'both' instead support full,mini or mini,full for setting download
> mode to collect both minidump and full dump.
>
> . logging improvement.
>
>
> Changes in v4: https://lore.kernel.org/lkml/[email protected]/
> - val should be shifted within the function [srinivas.kandagatla]
> i.e new = (old & ~mask) | (val << ffs(mask) - 1);
> - Added Acked-by [linus.walleij] on pinctrl change.
>
> Changes in v3 : https://lore.kernel.org/lkml/[email protected]/
> - Removed [1] from the series and sent as a separate patch[2], although this series
> should be applied on top [2].
> [1] https://lore.kernel.org/lkml/[email protected]/
> [2] https://lore.kernel.org/lkml/[email protected]/
> - Introduce new exported symbol on suggestion from [srinivas.kandagatla]
> - Use the symbol from drivers/pinctrl/qcom/pinctrl-msm.c.
> - Addressed comment given by [dmitry.baryshkov]
> - Converted non-standard Originally-by to Signed-off-by.
>
> Changes in v2: https://lore.kernel.org/lkml/[email protected]/
> - Addressed comment made by [bjorn]
> - Added download mask.
> - Passed download mode as parameter
> - Accept human accepatable download mode string.
> - enable = !!dload_mode
> - Shifted module param callback to somewhere down in
> the file so that it no longer need to know the
> prototype of qcom_scm_set_download_mode()
> - updated commit text.
>
> Mukesh Ojha (5):
> firmware: qcom_scm: provide a read-modify-write function
> pinctrl: qcom: Use qcom_scm_io_update_field()
> firmware: scm: Modify only the download bits in TCSR register
> firmware: qcom_scm: Refactor code to support multiple download mode
> firmware: qcom_scm: Add multiple download mode support
>
> drivers/firmware/Kconfig | 11 -----
> drivers/firmware/qcom_scm.c | 89 +++++++++++++++++++++++++++++++---
> drivers/pinctrl/qcom/pinctrl-msm.c | 11 ++---
> include/linux/firmware/qcom/qcom_scm.h | 2 +
> 4 files changed, 87 insertions(+), 26 deletions(-)
>