Re: [PATCH 2/4] firmware: scm: Modify only the DLOAD bit in TCSR register for download mode

From: Brian Masney
Date: Thu Feb 23 2023 - 06:49:17 EST


On Tue, Feb 21, 2023 at 08:09:39PM +0530, 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.
>
> Originally-by: Poovendhan Selvaraj <quic_poovendh@xxxxxxxxxxx>
> Signed-off-by: Mukesh Ojha <quic_mojha@xxxxxxxxxxx>
> ---
> drivers/firmware/qcom_scm.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index 51eb853..c376ba8 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -419,6 +419,7 @@ static void qcom_scm_set_download_mode(bool enable)
> {
> bool avail;
> int ret = 0;
> + u32 dload_addr_val;

Minor nit if you happen to need to send out a v2: put in reverse
Christmas tree order.

>
> avail = __qcom_scm_is_call_available(__scm->dev,
> QCOM_SCM_SVC_BOOT,
> @@ -426,8 +427,16 @@ 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_readl(__scm->dload_mode_addr, &dload_addr_val);
> + if (ret) {
> + dev_err(__scm->dev,
> + "failed to read dload mode address value: %d\n", ret);
> + return;
> + }
> +
> + ret = qcom_scm_io_writel(__scm->dload_mode_addr, enable ?
> + dload_addr_val | QCOM_SCM_BOOT_SET_DLOAD_MODE :
> + dload_addr_val & ~(QCOM_SCM_BOOT_SET_DLOAD_MODE));

Should the dload_mode_addr variable be renamed to something else? I'm
not sure what else is in that register.

Brian