On 10/25/2023 5:05 AM, Mukesh Ojha wrote: > Crashdump collection is done based on DLOAD bits of TCSR register. > To retain other bits, scm driver need to read the register and > modify only the DLOAD bits, as other bits in TCSR may 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> > Tested-by: Kathiravan Thirumoorthy <quic_kathirav@xxxxxxxxxxx> # IPQ9574 and IPQ5332 > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > --- > drivers/firmware/qcom/qcom_scm.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c > index 25549178a30f..f1c4a9f9a53f 100644 > --- a/drivers/firmware/qcom/qcom_scm.c > +++ b/drivers/firmware/qcom/qcom_scm.c > @@ -4,6 +4,8 @@ > */ > > #include <linux/arm-smccc.h> > +#include <linux/bitfield.h> > +#include <linux/bits.h> > #include <linux/clk.h> > #include <linux/completion.h> > #include <linux/cpumask.h> > @@ -117,6 +119,10 @@ static const u8 qcom_scm_cpu_warm_bits[QCOM_SCM_BOOT_MAX_CPUS] = { > #define QCOM_SMC_WAITQ_FLAG_WAKE_ONE BIT(0) > #define QCOM_SMC_WAITQ_FLAG_WAKE_ALL BIT(1) > > +#define QCOM_DLOAD_MASK GENMASK(5, 4) > +#define QCOM_DLOAD_FULLDUMP 0x1 > +#define QCOM_DLOAD_NODUMP 0x0 > + Enum would be better here for related constants. diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c index f1c4a9f9a53f..95f73a8c51d7 100644 --- a/drivers/firmware/qcom/qcom_scm.c +++ b/drivers/firmware/qcom/qcom_scm.c @@ -122,4 +122,6 @@ static const u8 qcom_scm_cpu_warm_bits[QCOM_SCM_BOOT_MAX_CPUS] = { #define QCOM_DLOAD_MASK GENMASK(5, 4) -#define QCOM_DLOAD_FULLDUMP 0x1 -#define QCOM_DLOAD_NODUMP 0x0 +enum qcom_dload_mode { + QCOM_DLOAD_NODUMP = 0, + QCOM_DLOAD_FULLDUMP = 1, +}; > static const char * const qcom_scm_convention_names[] = { > [SMC_CONVENTION_UNKNOWN] = "unknown", > [SMC_CONVENTION_ARM_32] = "smc arm 32", > @@ -523,6 +529,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; > > @@ -532,8 +539,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_rmw(__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"); - Elliot