Messages in this thread | | | Date | Mon, 24 Jul 2023 12:05:45 -0700 | Subject | Re: [PATCH V5 3/3] firmware: scm: Modify only the download bits in TCSR register | From | Elliot Berman <> |
| |
On 7/20/2023 12:04 AM, Kathiravan T wrote: > From: Mukesh Ojha <quic_mojha@quicinc.com> > > 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> > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com> > --- > 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.
> 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");
| |