| Message ID | 1674562755-5378-1-git-send-email-quic_mojha@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | firmware: qcom_scm: modify qcom_scm_set_download_mode() | expand |
Friendly reminder .. -Mukesh On 1/24/2023 5:49 PM, Mukesh Ojha wrote: > Modify qcom_scm_set_download_mode() such that it can support > multiple modes. There is no functional change with this change. > > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> > --- > drivers/firmware/qcom_scm.c | 17 ++++++++--------- > include/linux/qcom_scm.h | 5 +++++ > 2 files changed, 13 insertions(+), 9 deletions(-) > > diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c > index cdbfe54..712bb03 100644 > --- a/drivers/firmware/qcom_scm.c > +++ b/drivers/firmware/qcom_scm.c > @@ -400,22 +400,22 @@ int qcom_scm_set_remote_state(u32 state, u32 id) > } > EXPORT_SYMBOL(qcom_scm_set_remote_state); > > -static int __qcom_scm_set_dload_mode(struct device *dev, bool enable) > +static int __qcom_scm_set_dload_mode(struct device *dev, enum qcom_download_mode mode) > { > struct qcom_scm_desc desc = { > .svc = QCOM_SCM_SVC_BOOT, > .cmd = QCOM_SCM_BOOT_SET_DLOAD_MODE, > .arginfo = QCOM_SCM_ARGS(2), > - .args[0] = QCOM_SCM_BOOT_SET_DLOAD_MODE, > + .args[0] = mode, > .owner = ARM_SMCCC_OWNER_SIP, > }; > > - desc.args[1] = enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0; > + desc.args[1] = mode; > > 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(enum qcom_download_mode mode) > { > bool avail; > int ret = 0; > @@ -424,10 +424,9 @@ static void qcom_scm_set_download_mode(bool enable) > QCOM_SCM_SVC_BOOT, > QCOM_SCM_BOOT_SET_DLOAD_MODE); > if (avail) { > - ret = __qcom_scm_set_dload_mode(__scm->dev, enable); > + ret = __qcom_scm_set_dload_mode(__scm->dev, mode); > } 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_writel(__scm->dload_mode_addr, mode); > } else { > dev_err(__scm->dev, > "No available mechanism for setting download mode\n"); > @@ -1410,7 +1409,7 @@ static int qcom_scm_probe(struct platform_device *pdev) > * disabled below by a clean shutdown/reboot. > */ > if (download_mode) > - qcom_scm_set_download_mode(true); > + qcom_scm_set_download_mode(QCOM_DOWNLOAD_FULLDUMP); > > return 0; > } > @@ -1419,7 +1418,7 @@ static void qcom_scm_shutdown(struct platform_device *pdev) > { > /* Clean shutdown, disable download mode to allow normal restart */ > if (download_mode) > - qcom_scm_set_download_mode(false); > + qcom_scm_set_download_mode(QCOM_DOWNLOAD_NODUMP); > } > > static const struct of_device_id qcom_scm_dt_match[] = { > diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h > index f833564..f9bc84e 100644 > --- a/include/linux/qcom_scm.h > +++ b/include/linux/qcom_scm.h > @@ -14,6 +14,11 @@ > #define QCOM_SCM_CPU_PWR_DOWN_L2_OFF 0x1 > #define QCOM_SCM_HDCP_MAX_REQ_CNT 5 > > +enum qcom_download_mode { > + QCOM_DOWNLOAD_NODUMP = 0x00, > + QCOM_DOWNLOAD_FULLDUMP = 0x10, > +}; > + > struct qcom_scm_hdcp_req { > u32 addr; > u32 val;
On Tue, 24 Jan 2023 at 14:19, Mukesh Ojha <quic_mojha@quicinc.com> wrote: > > Modify qcom_scm_set_download_mode() such that it can support > multiple modes. There is no functional change with this change. Please describe usecases for such a change. Without them it's just an unused complication. > > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> > --- > drivers/firmware/qcom_scm.c | 17 ++++++++--------- > include/linux/qcom_scm.h | 5 +++++ > 2 files changed, 13 insertions(+), 9 deletions(-) > > diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c > index cdbfe54..712bb03 100644 > --- a/drivers/firmware/qcom_scm.c > +++ b/drivers/firmware/qcom_scm.c > @@ -400,22 +400,22 @@ int qcom_scm_set_remote_state(u32 state, u32 id) > } > EXPORT_SYMBOL(qcom_scm_set_remote_state); > > -static int __qcom_scm_set_dload_mode(struct device *dev, bool enable) > +static int __qcom_scm_set_dload_mode(struct device *dev, enum qcom_download_mode mode) > { > struct qcom_scm_desc desc = { > .svc = QCOM_SCM_SVC_BOOT, > .cmd = QCOM_SCM_BOOT_SET_DLOAD_MODE, > .arginfo = QCOM_SCM_ARGS(2), > - .args[0] = QCOM_SCM_BOOT_SET_DLOAD_MODE, > + .args[0] = mode, > .owner = ARM_SMCCC_OWNER_SIP, > }; > > - desc.args[1] = enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0; > + desc.args[1] = mode; > > 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(enum qcom_download_mode mode) > { > bool avail; > int ret = 0; > @@ -424,10 +424,9 @@ static void qcom_scm_set_download_mode(bool enable) > QCOM_SCM_SVC_BOOT, > QCOM_SCM_BOOT_SET_DLOAD_MODE); > if (avail) { > - ret = __qcom_scm_set_dload_mode(__scm->dev, enable); > + ret = __qcom_scm_set_dload_mode(__scm->dev, mode); > } 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_writel(__scm->dload_mode_addr, mode); > } else { > dev_err(__scm->dev, > "No available mechanism for setting download mode\n"); > @@ -1410,7 +1409,7 @@ static int qcom_scm_probe(struct platform_device *pdev) > * disabled below by a clean shutdown/reboot. > */ > if (download_mode) > - qcom_scm_set_download_mode(true); > + qcom_scm_set_download_mode(QCOM_DOWNLOAD_FULLDUMP); > > return 0; > } > @@ -1419,7 +1418,7 @@ static void qcom_scm_shutdown(struct platform_device *pdev) > { > /* Clean shutdown, disable download mode to allow normal restart */ > if (download_mode) > - qcom_scm_set_download_mode(false); > + qcom_scm_set_download_mode(QCOM_DOWNLOAD_NODUMP); > } > > static const struct of_device_id qcom_scm_dt_match[] = { > diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h > index f833564..f9bc84e 100644 > --- a/include/linux/qcom_scm.h > +++ b/include/linux/qcom_scm.h > @@ -14,6 +14,11 @@ > #define QCOM_SCM_CPU_PWR_DOWN_L2_OFF 0x1 > #define QCOM_SCM_HDCP_MAX_REQ_CNT 5 > > +enum qcom_download_mode { > + QCOM_DOWNLOAD_NODUMP = 0x00, > + QCOM_DOWNLOAD_FULLDUMP = 0x10, > +}; > + > struct qcom_scm_hdcp_req { > u32 addr; > u32 val; > -- > 2.7.4 >
On 2/2/2023 3:42 PM, Dmitry Baryshkov wrote: > On Tue, 24 Jan 2023 at 14:19, Mukesh Ojha <quic_mojha@quicinc.com> wrote: >> >> Modify qcom_scm_set_download_mode() such that it can support >> multiple modes. There is no functional change with this change. > > Please describe usecases for such a change. Without them it's just an > unused complication. I already mentioned the change is to support more download modes and i am working on the patches to add some other download modes, for which this may be needed as a preparation and since this change could be independent. so, sent it. -Mukesh > >> >> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> >> --- >> drivers/firmware/qcom_scm.c | 17 ++++++++--------- >> include/linux/qcom_scm.h | 5 +++++ >> 2 files changed, 13 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c >> index cdbfe54..712bb03 100644 >> --- a/drivers/firmware/qcom_scm.c >> +++ b/drivers/firmware/qcom_scm.c >> @@ -400,22 +400,22 @@ int qcom_scm_set_remote_state(u32 state, u32 id) >> } >> EXPORT_SYMBOL(qcom_scm_set_remote_state); >> >> -static int __qcom_scm_set_dload_mode(struct device *dev, bool enable) >> +static int __qcom_scm_set_dload_mode(struct device *dev, enum qcom_download_mode mode) >> { >> struct qcom_scm_desc desc = { >> .svc = QCOM_SCM_SVC_BOOT, >> .cmd = QCOM_SCM_BOOT_SET_DLOAD_MODE, >> .arginfo = QCOM_SCM_ARGS(2), >> - .args[0] = QCOM_SCM_BOOT_SET_DLOAD_MODE, >> + .args[0] = mode, >> .owner = ARM_SMCCC_OWNER_SIP, >> }; >> >> - desc.args[1] = enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0; >> + desc.args[1] = mode; >> >> 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(enum qcom_download_mode mode) >> { >> bool avail; >> int ret = 0; >> @@ -424,10 +424,9 @@ static void qcom_scm_set_download_mode(bool enable) >> QCOM_SCM_SVC_BOOT, >> QCOM_SCM_BOOT_SET_DLOAD_MODE); >> if (avail) { >> - ret = __qcom_scm_set_dload_mode(__scm->dev, enable); >> + ret = __qcom_scm_set_dload_mode(__scm->dev, mode); >> } 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_writel(__scm->dload_mode_addr, mode); >> } else { >> dev_err(__scm->dev, >> "No available mechanism for setting download mode\n"); >> @@ -1410,7 +1409,7 @@ static int qcom_scm_probe(struct platform_device *pdev) >> * disabled below by a clean shutdown/reboot. >> */ >> if (download_mode) >> - qcom_scm_set_download_mode(true); >> + qcom_scm_set_download_mode(QCOM_DOWNLOAD_FULLDUMP); >> >> return 0; >> } >> @@ -1419,7 +1418,7 @@ static void qcom_scm_shutdown(struct platform_device *pdev) >> { >> /* Clean shutdown, disable download mode to allow normal restart */ >> if (download_mode) >> - qcom_scm_set_download_mode(false); >> + qcom_scm_set_download_mode(QCOM_DOWNLOAD_NODUMP); >> } >> >> static const struct of_device_id qcom_scm_dt_match[] = { >> diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h >> index f833564..f9bc84e 100644 >> --- a/include/linux/qcom_scm.h >> +++ b/include/linux/qcom_scm.h >> @@ -14,6 +14,11 @@ >> #define QCOM_SCM_CPU_PWR_DOWN_L2_OFF 0x1 >> #define QCOM_SCM_HDCP_MAX_REQ_CNT 5 >> >> +enum qcom_download_mode { >> + QCOM_DOWNLOAD_NODUMP = 0x00, >> + QCOM_DOWNLOAD_FULLDUMP = 0x10, >> +}; >> + >> struct qcom_scm_hdcp_req { >> u32 addr; >> u32 val; >> -- >> 2.7.4 >> > >
On 24/01/2023 12:19, Mukesh Ojha wrote: > -static int __qcom_scm_set_dload_mode(struct device *dev, bool enable) > +static int __qcom_scm_set_dload_mode(struct device *dev, enum qcom_download_mode mode) > { > struct qcom_scm_desc desc = { > .svc = QCOM_SCM_SVC_BOOT, > .cmd = QCOM_SCM_BOOT_SET_DLOAD_MODE, > .arginfo = QCOM_SCM_ARGS(2), > - .args[0] = QCOM_SCM_BOOT_SET_DLOAD_MODE, > + .args[0] = mode, Is this a bug fix? why are we changing arg[0]? --srini > .owner = ARM_SMCCC_OWNER_SIP, > }; > > - desc.args[1] = enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0; > + desc.args[1] = mode; > > return qcom_scm_call_atomic(__scm->dev, &desc, NULL); > }
On 2/2/2023 4:47 PM, Srinivas Kandagatla wrote: > > > On 24/01/2023 12:19, Mukesh Ojha wrote: >> -static int __qcom_scm_set_dload_mode(struct device *dev, bool enable) >> +static int __qcom_scm_set_dload_mode(struct device *dev, enum >> qcom_download_mode mode) >> { >> struct qcom_scm_desc desc = { >> .svc = QCOM_SCM_SVC_BOOT, >> .cmd = QCOM_SCM_BOOT_SET_DLOAD_MODE, >> .arginfo = QCOM_SCM_ARGS(2), >> - .args[0] = QCOM_SCM_BOOT_SET_DLOAD_MODE, >> + .args[0] = mode, > > Is this a bug fix? why are we changing arg[0]? Thanks. This is legacy scm call and it is deprecated. Let's not change this. I will fix this in v2. -Mukesh > > --srini >> .owner = ARM_SMCCC_OWNER_SIP, >> }; >> - desc.args[1] = enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0; >> + desc.args[1] = mode; >> return qcom_scm_call_atomic(__scm->dev, &desc, NULL); >> }
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c index cdbfe54..712bb03 100644 --- a/drivers/firmware/qcom_scm.c +++ b/drivers/firmware/qcom_scm.c @@ -400,22 +400,22 @@ int qcom_scm_set_remote_state(u32 state, u32 id) } EXPORT_SYMBOL(qcom_scm_set_remote_state); -static int __qcom_scm_set_dload_mode(struct device *dev, bool enable) +static int __qcom_scm_set_dload_mode(struct device *dev, enum qcom_download_mode mode) { struct qcom_scm_desc desc = { .svc = QCOM_SCM_SVC_BOOT, .cmd = QCOM_SCM_BOOT_SET_DLOAD_MODE, .arginfo = QCOM_SCM_ARGS(2), - .args[0] = QCOM_SCM_BOOT_SET_DLOAD_MODE, + .args[0] = mode, .owner = ARM_SMCCC_OWNER_SIP, }; - desc.args[1] = enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0; + desc.args[1] = mode; 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(enum qcom_download_mode mode) { bool avail; int ret = 0; @@ -424,10 +424,9 @@ static void qcom_scm_set_download_mode(bool enable) QCOM_SCM_SVC_BOOT, QCOM_SCM_BOOT_SET_DLOAD_MODE); if (avail) { - ret = __qcom_scm_set_dload_mode(__scm->dev, enable); + ret = __qcom_scm_set_dload_mode(__scm->dev, mode); } 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_writel(__scm->dload_mode_addr, mode); } else { dev_err(__scm->dev, "No available mechanism for setting download mode\n"); @@ -1410,7 +1409,7 @@ static int qcom_scm_probe(struct platform_device *pdev) * disabled below by a clean shutdown/reboot. */ if (download_mode) - qcom_scm_set_download_mode(true); + qcom_scm_set_download_mode(QCOM_DOWNLOAD_FULLDUMP); return 0; } @@ -1419,7 +1418,7 @@ static void qcom_scm_shutdown(struct platform_device *pdev) { /* Clean shutdown, disable download mode to allow normal restart */ if (download_mode) - qcom_scm_set_download_mode(false); + qcom_scm_set_download_mode(QCOM_DOWNLOAD_NODUMP); } static const struct of_device_id qcom_scm_dt_match[] = { diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h index f833564..f9bc84e 100644 --- a/include/linux/qcom_scm.h +++ b/include/linux/qcom_scm.h @@ -14,6 +14,11 @@ #define QCOM_SCM_CPU_PWR_DOWN_L2_OFF 0x1 #define QCOM_SCM_HDCP_MAX_REQ_CNT 5 +enum qcom_download_mode { + QCOM_DOWNLOAD_NODUMP = 0x00, + QCOM_DOWNLOAD_FULLDUMP = 0x10, +}; + struct qcom_scm_hdcp_req { u32 addr; u32 val;
Modify qcom_scm_set_download_mode() such that it can support multiple modes. There is no functional change with this change. Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> --- drivers/firmware/qcom_scm.c | 17 ++++++++--------- include/linux/qcom_scm.h | 5 +++++ 2 files changed, 13 insertions(+), 9 deletions(-)