| Message ID | 20230518140224.2248782-1-robimarko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RESEND,1/2] firmware: qcom: scm: Add SDI disable support | expand |
On 5/18/2023 7:32 PM, Robert Marko wrote: > Some SoC-s like IPQ5018 require SDI(Secure Debug Image) to be disabled > before trying to reboot, otherwise board will just hang after reboot has > been issued via PSCI. > > So, provide a call to SCM that allows disabling it. > > Signed-off-by: Robert Marko <robimarko@gmail.com> This scm call support indeed needed for reboot cases, but i am not sure about target specific check in the later patch. Acked-by: Mukesh Ojha <quic_mojha@quicinc.com> -- Mukesh > --- > drivers/firmware/qcom_scm.c | 23 +++++++++++++++++++++++ > drivers/firmware/qcom_scm.h | 1 + > 2 files changed, 24 insertions(+) > > diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c > index fde33acd46b7..bdc9324d4e62 100644 > --- a/drivers/firmware/qcom_scm.c > +++ b/drivers/firmware/qcom_scm.c > @@ -407,6 +407,29 @@ int qcom_scm_set_remote_state(u32 state, u32 id) > } > EXPORT_SYMBOL(qcom_scm_set_remote_state); > > +static int qcom_scm_disable_sdi(void) > +{ > + int ret; > + struct qcom_scm_desc desc = { > + .svc = QCOM_SCM_SVC_BOOT, > + .cmd = QCOM_SCM_BOOT_SDI_CONFIG, > + .args[0] = 1, /* Disable watchdog debug */ > + .args[1] = 0, /* Disable SDI */ > + .arginfo = QCOM_SCM_ARGS(2), > + .owner = ARM_SMCCC_OWNER_SIP, > + }; > + struct qcom_scm_res res; > + > + ret = qcom_scm_clk_enable(); > + if (ret) > + return ret; > + ret = qcom_scm_call(__scm->dev, &desc, &res); > + > + qcom_scm_clk_disable(); > + > + return ret ? : res.result[0]; > +} > + > static int __qcom_scm_set_dload_mode(struct device *dev, bool enable) > { > struct qcom_scm_desc desc = { > diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h > index e6e512bd57d1..7b68fa820495 100644 > --- a/drivers/firmware/qcom_scm.h > +++ b/drivers/firmware/qcom_scm.h > @@ -80,6 +80,7 @@ extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc, > #define QCOM_SCM_SVC_BOOT 0x01 > #define QCOM_SCM_BOOT_SET_ADDR 0x01 > #define QCOM_SCM_BOOT_TERMINATE_PC 0x02 > +#define QCOM_SCM_BOOT_SDI_CONFIG 0x09 > #define QCOM_SCM_BOOT_SET_DLOAD_MODE 0x10 > #define QCOM_SCM_BOOT_SET_ADDR_MC 0x11 > #define QCOM_SCM_BOOT_SET_REMOTE_STATE 0x0a
On Thu, 18 May 2023 at 16:25, Mukesh Ojha <quic_mojha@quicinc.com> wrote: > > > > On 5/18/2023 7:32 PM, Robert Marko wrote: > > Some SoC-s like IPQ5018 require SDI(Secure Debug Image) to be disabled > > before trying to reboot, otherwise board will just hang after reboot has > > been issued via PSCI. > > > > So, provide a call to SCM that allows disabling it. > > > > Signed-off-by: Robert Marko <robimarko@gmail.com> > > This scm call support indeed needed for reboot cases, but i am not sure > about target specific check in the later patch. I am not really sure where to put it, maybe as part of qcom_scm_shutdown? Yesterday I found out that in OpenWrt we also have a Google IPQ4019 board that has been needing SDI to be disabled as well. Regards, Robert > > Acked-by: Mukesh Ojha <quic_mojha@quicinc.com> > > -- Mukesh > > > --- > > drivers/firmware/qcom_scm.c | 23 +++++++++++++++++++++++ > > drivers/firmware/qcom_scm.h | 1 + > > 2 files changed, 24 insertions(+) > > > > diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c > > index fde33acd46b7..bdc9324d4e62 100644 > > --- a/drivers/firmware/qcom_scm.c > > +++ b/drivers/firmware/qcom_scm.c > > @@ -407,6 +407,29 @@ int qcom_scm_set_remote_state(u32 state, u32 id) > > } > > EXPORT_SYMBOL(qcom_scm_set_remote_state); > > > > +static int qcom_scm_disable_sdi(void) > > +{ > > + int ret; > > + struct qcom_scm_desc desc = { > > + .svc = QCOM_SCM_SVC_BOOT, > > + .cmd = QCOM_SCM_BOOT_SDI_CONFIG, > > + .args[0] = 1, /* Disable watchdog debug */ > > + .args[1] = 0, /* Disable SDI */ > > + .arginfo = QCOM_SCM_ARGS(2), > > + .owner = ARM_SMCCC_OWNER_SIP, > > + }; > > + struct qcom_scm_res res; > > + > > + ret = qcom_scm_clk_enable(); > > + if (ret) > > + return ret; > > + ret = qcom_scm_call(__scm->dev, &desc, &res); > > + > > + qcom_scm_clk_disable(); > > + > > + return ret ? : res.result[0]; > > +} > > + > > static int __qcom_scm_set_dload_mode(struct device *dev, bool enable) > > { > > struct qcom_scm_desc desc = { > > diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h > > index e6e512bd57d1..7b68fa820495 100644 > > --- a/drivers/firmware/qcom_scm.h > > +++ b/drivers/firmware/qcom_scm.h > > @@ -80,6 +80,7 @@ extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc, > > #define QCOM_SCM_SVC_BOOT 0x01 > > #define QCOM_SCM_BOOT_SET_ADDR 0x01 > > #define QCOM_SCM_BOOT_TERMINATE_PC 0x02 > > +#define QCOM_SCM_BOOT_SDI_CONFIG 0x09 > > #define QCOM_SCM_BOOT_SET_DLOAD_MODE 0x10 > > #define QCOM_SCM_BOOT_SET_ADDR_MC 0x11 > > #define QCOM_SCM_BOOT_SET_REMOTE_STATE 0x0a
On Fri, May 19, 2023 at 01:16:56PM +0200, Robert Marko wrote: > On Thu, 18 May 2023 at 16:25, Mukesh Ojha <quic_mojha@quicinc.com> wrote: > > On 5/18/2023 7:32 PM, Robert Marko wrote: > > > Some SoC-s like IPQ5018 require SDI(Secure Debug Image) to be disabled > > > before trying to reboot, otherwise board will just hang after reboot has > > > been issued via PSCI. > > > > > > So, provide a call to SCM that allows disabling it. > > > > > > Signed-off-by: Robert Marko <robimarko@gmail.com> > > > > This scm call support indeed needed for reboot cases, but i am not sure > > about target specific check in the later patch. > > I am not really sure where to put it, maybe as part of qcom_scm_shutdown? I would suggest not waiting until shutdown. In fact, I suggest the opposite -- that this needs to be done as early as possible. Any delay leaves room for hanging the system -- because a watchdog reset, for instance, might not be able to trigger an shutdown handler. And I also imagine that's not the complain Mukesh had; I expect his reservation was about using the SoC compatible property. But I'd prefer having that conversation on patch 2. > Yesterday I found out that in OpenWrt we also have a Google IPQ4019 board that > has been needing SDI to be disabled as well. That's me [1] :) Feel free to CC me on such occasions. I would have loved to have engaged with this earlier. Anyway, I replied directly to patch 2, since I think that's a more appropriate place for the discussion. Brian [1] Subject: [RFC PATCH] firmware: qcom_scm: disable SDI at boot https://lore.kernel.org/all/20200721080054.2803881-1-computersforpeace@gmail.com/
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c index fde33acd46b7..bdc9324d4e62 100644 --- a/drivers/firmware/qcom_scm.c +++ b/drivers/firmware/qcom_scm.c @@ -407,6 +407,29 @@ int qcom_scm_set_remote_state(u32 state, u32 id) } EXPORT_SYMBOL(qcom_scm_set_remote_state); +static int qcom_scm_disable_sdi(void) +{ + int ret; + struct qcom_scm_desc desc = { + .svc = QCOM_SCM_SVC_BOOT, + .cmd = QCOM_SCM_BOOT_SDI_CONFIG, + .args[0] = 1, /* Disable watchdog debug */ + .args[1] = 0, /* Disable SDI */ + .arginfo = QCOM_SCM_ARGS(2), + .owner = ARM_SMCCC_OWNER_SIP, + }; + struct qcom_scm_res res; + + ret = qcom_scm_clk_enable(); + if (ret) + return ret; + ret = qcom_scm_call(__scm->dev, &desc, &res); + + qcom_scm_clk_disable(); + + return ret ? : res.result[0]; +} + static int __qcom_scm_set_dload_mode(struct device *dev, bool enable) { struct qcom_scm_desc desc = { diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h index e6e512bd57d1..7b68fa820495 100644 --- a/drivers/firmware/qcom_scm.h +++ b/drivers/firmware/qcom_scm.h @@ -80,6 +80,7 @@ extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc, #define QCOM_SCM_SVC_BOOT 0x01 #define QCOM_SCM_BOOT_SET_ADDR 0x01 #define QCOM_SCM_BOOT_TERMINATE_PC 0x02 +#define QCOM_SCM_BOOT_SDI_CONFIG 0x09 #define QCOM_SCM_BOOT_SET_DLOAD_MODE 0x10 #define QCOM_SCM_BOOT_SET_ADDR_MC 0x11 #define QCOM_SCM_BOOT_SET_REMOTE_STATE 0x0a
Some SoC-s like IPQ5018 require SDI(Secure Debug Image) to be disabled before trying to reboot, otherwise board will just hang after reboot has been issued via PSCI. So, provide a call to SCM that allows disabling it. Signed-off-by: Robert Marko <robimarko@gmail.com> --- drivers/firmware/qcom_scm.c | 23 +++++++++++++++++++++++ drivers/firmware/qcom_scm.h | 1 + 2 files changed, 24 insertions(+)