| Message ID | 1675419435-30726-1-git-send-email-quic_mojha@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2] firmware: qcom_scm: modify qcom_scm_set_download_mode() | expand |
On 03/02/2023 10:17, 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> > --- > Changes in v2: > - Stop changing legacy scm id for dload mode. > > drivers/firmware/qcom_scm.c | 15 +++++++-------- > include/linux/qcom_scm.h | 5 +++++ > 2 files changed, 12 insertions(+), 8 deletions(-) > > diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c > index cdbfe54..6245b97 100644 > --- a/drivers/firmware/qcom_scm.c > +++ b/drivers/firmware/qcom_scm.c > @@ -400,7 +400,7 @@ 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, > @@ -410,12 +410,12 @@ static int __qcom_scm_set_dload_mode(struct device *dev, bool enable) > .owner = ARM_SMCCC_OWNER_SIP, > }; > > - desc.args[1] = enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0; > + desc.args[1] = mode ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0; I think this line should be: desc.args[1] = mode; --srini > > 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 2/3/2023 8:21 PM, Srinivas Kandagatla wrote: > > > On 03/02/2023 10:17, 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> >> --- >> Changes in v2: >> - Stop changing legacy scm id for dload mode. >> >> drivers/firmware/qcom_scm.c | 15 +++++++-------- >> include/linux/qcom_scm.h | 5 +++++ >> 2 files changed, 12 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c >> index cdbfe54..6245b97 100644 >> --- a/drivers/firmware/qcom_scm.c >> +++ b/drivers/firmware/qcom_scm.c >> @@ -400,7 +400,7 @@ 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, >> @@ -410,12 +410,12 @@ static int __qcom_scm_set_dload_mode(struct >> device *dev, bool enable) >> .owner = ARM_SMCCC_OWNER_SIP, >> }; >> - desc.args[1] = enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0; >> + desc.args[1] = mode ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0; > > I think this line should be: > > desc.args[1] = mode; > Should be fine..for backward compatibility as we want to avoid what is passed to trust zone without check and since this is legacy code, i would like to avoid. -Mukesh > > --srini > >> 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 Fri, 3 Feb 2023 at 16:53, Mukesh Ojha <quic_mojha@quicinc.com> wrote: > > > > On 2/3/2023 8:21 PM, Srinivas Kandagatla wrote: > > > > > > On 03/02/2023 10:17, 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> > >> --- > >> Changes in v2: > >> - Stop changing legacy scm id for dload mode. > >> > >> drivers/firmware/qcom_scm.c | 15 +++++++-------- > >> include/linux/qcom_scm.h | 5 +++++ > >> 2 files changed, 12 insertions(+), 8 deletions(-) > >> > >> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c > >> index cdbfe54..6245b97 100644 > >> --- a/drivers/firmware/qcom_scm.c > >> +++ b/drivers/firmware/qcom_scm.c > >> @@ -400,7 +400,7 @@ 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, > >> @@ -410,12 +410,12 @@ static int __qcom_scm_set_dload_mode(struct > >> device *dev, bool enable) > >> .owner = ARM_SMCCC_OWNER_SIP, > >> }; > >> - desc.args[1] = enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0; > >> + desc.args[1] = mode ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0; > > > > I think this line should be: > > > > desc.args[1] = mode; > > > > Should be fine..for backward compatibility as we want to avoid what is > passed to trust zone without check and since this is legacy code, i > would like to avoid. I'd second Srini here. Please remove the ternary operator and pass mode directly. If you'd like to limit the 'mode' argument, do so directly (and return an error if the mode is not supported). However there still exists a bigger problem in my opinion. You are changing an API. Please provide a user for this API. 'The user will be provided separately/later/whatever' is usually not enough.
On Fri, Feb 03, 2023 at 08:23:29PM +0530, Mukesh Ojha wrote: > > > On 2/3/2023 8:21 PM, Srinivas Kandagatla wrote: > > > > > > On 03/02/2023 10:17, 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> > > > --- > > > Changes in v2: > > > - Stop changing legacy scm id for dload mode. > > > > > > drivers/firmware/qcom_scm.c | 15 +++++++-------- > > > include/linux/qcom_scm.h | 5 +++++ > > > 2 files changed, 12 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c > > > index cdbfe54..6245b97 100644 > > > --- a/drivers/firmware/qcom_scm.c > > > +++ b/drivers/firmware/qcom_scm.c > > > @@ -400,7 +400,7 @@ 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, > > > @@ -410,12 +410,12 @@ static int __qcom_scm_set_dload_mode(struct > > > device *dev, bool enable) > > > .owner = ARM_SMCCC_OWNER_SIP, > > > }; > > > - desc.args[1] = enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0; > > > + desc.args[1] = mode ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0; > > > > I think this line should be: > > > > desc.args[1] = mode; > > > > Should be fine..for backward compatibility as we want to avoid what is > passed to trust zone without check and since this is legacy code, i would > like to avoid. > Afaict you touch every caller of this function, so there is no concerns about backwards compatibility. With some use of imagination, this patch is likely intended to be followed by some other patch which adds more values to the enum, at which point the state you leave this in would be wrong. > -Mukesh > > > > --srini > > > > > 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 { This is not an enumeration, it's a set of constants. Please use #define. > > > + QCOM_DOWNLOAD_NODUMP = 0x00, > > > + QCOM_DOWNLOAD_FULLDUMP = 0x10, > > > +}; Regards, Bjorn > > > + > > > struct qcom_scm_hdcp_req { > > > u32 addr; > > > u32 val;
On Fri, Feb 03, 2023 at 03:47:15PM +0530, Mukesh Ojha wrote: > Modify qcom_scm_set_download_mode() such that it can support > multiple modes. There is no functional change with this change. > As Dmitry said, you argue for added flexibility, but doesn't provide a user of that flexibility. I will drop this patch from the queue for now. Please include this together with the patch(es) that benefit from such flexibility. > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> > --- > Changes in v2: > - Stop changing legacy scm id for dload mode. > > drivers/firmware/qcom_scm.c | 15 +++++++-------- > include/linux/qcom_scm.h | 5 +++++ > 2 files changed, 12 insertions(+), 8 deletions(-) > > diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c > index cdbfe54..6245b97 100644 > --- a/drivers/firmware/qcom_scm.c > +++ b/drivers/firmware/qcom_scm.c > @@ -400,7 +400,7 @@ 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, > @@ -410,12 +410,12 @@ static int __qcom_scm_set_dload_mode(struct device *dev, bool enable) > .owner = ARM_SMCCC_OWNER_SIP, > }; > > - desc.args[1] = enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0; > + desc.args[1] = mode ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0; > > 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) PS. Wouldn't it make sense, if !download_mode to set NODUMP? Regards, Bjorn > - 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/4/2023 12:32 AM, Bjorn Andersson wrote: > On Fri, Feb 03, 2023 at 03:47:15PM +0530, Mukesh Ojha wrote: >> Modify qcom_scm_set_download_mode() such that it can support >> multiple modes. There is no functional change with this change. >> > > As Dmitry said, you argue for added flexibility, but doesn't provide a > user of that flexibility. I will drop this patch from the queue for now. > > Please include this together with the patch(es) that benefit from such > flexibility. Sure, will add this along with patches which benefit from this change. > >> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> >> --- >> Changes in v2: >> - Stop changing legacy scm id for dload mode. >> >> drivers/firmware/qcom_scm.c | 15 +++++++-------- >> include/linux/qcom_scm.h | 5 +++++ >> 2 files changed, 12 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c >> index cdbfe54..6245b97 100644 >> --- a/drivers/firmware/qcom_scm.c >> +++ b/drivers/firmware/qcom_scm.c >> @@ -400,7 +400,7 @@ 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, >> @@ -410,12 +410,12 @@ static int __qcom_scm_set_dload_mode(struct device *dev, bool enable) >> .owner = ARM_SMCCC_OWNER_SIP, >> }; >> >> - desc.args[1] = enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0; >> + desc.args[1] = mode ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0; >> >> 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) > > PS. Wouldn't it make sense, if !download_mode to set NODUMP? IMO, it does not need even a check, since our intention is to disable download mode during reboot/restart. -Mukesh > > Regards, > Bjorn > >> - 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 >>
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c index cdbfe54..6245b97 100644 --- a/drivers/firmware/qcom_scm.c +++ b/drivers/firmware/qcom_scm.c @@ -400,7 +400,7 @@ 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, @@ -410,12 +410,12 @@ static int __qcom_scm_set_dload_mode(struct device *dev, bool enable) .owner = ARM_SMCCC_OWNER_SIP, }; - desc.args[1] = enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0; + desc.args[1] = mode ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0; 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> --- Changes in v2: - Stop changing legacy scm id for dload mode. drivers/firmware/qcom_scm.c | 15 +++++++-------- include/linux/qcom_scm.h | 5 +++++ 2 files changed, 12 insertions(+), 8 deletions(-)