diff mbox series

[v3,1/4] dt-bindings: firmware: qcom,scm: support indicating SDI default state

Message ID 20230816164641.3371878-1-robimarko@gmail.com
State Accepted
Commit 92dab9ea5f389c12828283146c60054642453a91
Headers show
Series [v3,1/4] dt-bindings: firmware: qcom,scm: support indicating SDI default state | expand

Commit Message

Robert Marko Aug. 16, 2023, 4:45 p.m. UTC
IPQ5018 has SDI (Secure Debug Image) enabled by TZ by default, and that
means that WDT being asserted or just trying to reboot will hang the board
in the debug mode and only pulling the power and repowering will help.
Some IPQ4019 boards like Google WiFI have it enabled as well.

So, lets add a boolean property to indicate that SDI is enabled by default
and thus needs to be disabled by the kernel.

Signed-off-by: Robert Marko <robimarko@gmail.com>
---
Changes in v3:
* Change the property so it indicates that SDI has been enabled by default
---
 Documentation/devicetree/bindings/firmware/qcom,scm.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Guru Das Srinagesh Aug. 16, 2023, 5:02 p.m. UTC | #1
On Aug 16 2023 18:45, Robert Marko wrote:
> IPQ5018 has SDI (Secure Debug Image) enabled by TZ by default, and that
> means that WDT being asserted or just trying to reboot will hang the board
> in the debug mode and only pulling the power and repowering will help.
> Some IPQ4019 boards like Google WiFI have it enabled as well.
> 
> Luckily, SDI can be disabled via an SCM call.
> 
> So, lets use the boolean DT property to identify boards that have SDI
> enabled by default and use the SCM call to disable SDI during SCM probe.
> It is important to disable it as soon as possible as we might have a WDT
> assertion at any time which would then leave the board in debug mode,
> thus disabling it during SCM removal is not enough.
> 
> Signed-off-by: Robert Marko <robimarko@gmail.com>

Reviewed-by: Guru Das Srinagesh <quic_gurus@quicinc.com>
Konrad Dybcio Aug. 16, 2023, 5:33 p.m. UTC | #2
On 16.08.2023 18:45, Robert Marko wrote:
> IPQ5018 has SDI (Secure Debug Image) enabled by TZ by default, and that
> means that WDT being asserted or just trying to reboot will hang the board
> in the debug mode and only pulling the power and repowering will help.
> Some IPQ4019 boards like Google WiFI have it enabled as well.
> 
> Luckily, SDI can be disabled via an SCM call.
> 
> So, lets use the boolean DT property to identify boards that have SDI
> enabled by default and use the SCM call to disable SDI during SCM probe.
> It is important to disable it as soon as possible as we might have a WDT
> assertion at any time which would then leave the board in debug mode,
> thus disabling it during SCM removal is not enough.
> 
> Signed-off-by: Robert Marko <robimarko@gmail.com>
> ---
[...]


> +	/*
> +	 * Disable SDI if indicated by DT that it is enabled by default.
> +	 */
> +	if (of_property_read_bool(pdev->dev.of_node, "qcom,sdi-enabled"))
> +		qcom_scm_disable_sdi();
Should we care about the return value?

Konrad
Brian Norris Aug. 17, 2023, 3:35 a.m. UTC | #3
On Wed, Aug 16, 2023 at 06:45:38PM +0200, Robert Marko wrote:
> IPQ5018 has SDI (Secure Debug Image) enabled by TZ by default, and that
> means that WDT being asserted or just trying to reboot will hang the board
> in the debug mode and only pulling the power and repowering will help.
> Some IPQ4019 boards like Google WiFI have it enabled as well.
> 
> So, lets add a boolean property to indicate that SDI is enabled by default
> and thus needs to be disabled by the kernel.
> 
> Signed-off-by: Robert Marko <robimarko@gmail.com>

The series looks good to me. Thanks for doing this!

Reviewed-by: Brian Norris <computersforpeace@gmail.com>
Krzysztof Kozlowski Aug. 19, 2023, 2:04 p.m. UTC | #4
On 16/08/2023 18:45, Robert Marko wrote:
> IPQ5018 has SDI (Secure Debug Image) enabled by TZ by default, and that
> means that WDT being asserted or just trying to reboot will hang the board
> in the debug mode and only pulling the power and repowering will help.
> Some IPQ4019 boards like Google WiFI have it enabled as well.
> 
> So, lets add a boolean property to indicate that SDI is enabled by default
> and thus needs to be disabled by the kernel.
> 
> Signed-off-by: Robert Marko <robimarko@gmail.com>

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Mukesh Ojha Aug. 22, 2023, 4:55 p.m. UTC | #5
On 8/16/2023 10:15 PM, Robert Marko wrote:
> IPQ5018 has SDI (Secure Debug Image) enabled by TZ by default, and that
> means that WDT being asserted or just trying to reboot will hang the board
> in the debug mode and only pulling the power and repowering will help.
> Some IPQ4019 boards like Google WiFI have it enabled as well.
> 
> So, lets add a boolean property to indicate that SDI is enabled by default
> and thus needs to be disabled by the kernel.
> 
> Signed-off-by: Robert Marko <robimarko@gmail.com>

Acked-by: Mukesh Ojha <quic_mojha@quicinc.com>

-Mukesh

> ---
> Changes in v3:
> * Change the property so it indicates that SDI has been enabled by default
> ---
>   Documentation/devicetree/bindings/firmware/qcom,scm.yaml | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> index 4233ea839bfc..590bbbd61de5 100644
> --- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> @@ -89,6 +89,14 @@ properties:
>         protocol to handle sleeping SCM calls.
>       maxItems: 1
>   
> +  qcom,sdi-enabled:
> +    description:
> +      Indicates that the SDI (Secure Debug Image) has been enabled by TZ
> +      by default and it needs to be disabled.
> +      If not disabled WDT assertion or reboot will cause the board to hang
> +      in the debug mode.
> +    type: boolean
> +
>     qcom,dload-mode:
>       $ref: /schemas/types.yaml#/definitions/phandle-array
>       items:
Bjorn Andersson Sept. 20, 2023, 6:15 p.m. UTC | #6
On Wed, 16 Aug 2023 18:45:38 +0200, Robert Marko wrote:
> IPQ5018 has SDI (Secure Debug Image) enabled by TZ by default, and that
> means that WDT being asserted or just trying to reboot will hang the board
> in the debug mode and only pulling the power and repowering will help.
> Some IPQ4019 boards like Google WiFI have it enabled as well.
> 
> So, lets add a boolean property to indicate that SDI is enabled by default
> and thus needs to be disabled by the kernel.
> 
> [...]

Applied, thanks!

[4/4] arm64: dts: qcom: ipq5018: indicate that SDI should be disabled
      commit: 79796e87215db9587d6c66ec6f6781e091bc6464

Best regards,
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
index 4233ea839bfc..590bbbd61de5 100644
--- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
+++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
@@ -89,6 +89,14 @@  properties:
       protocol to handle sleeping SCM calls.
     maxItems: 1
 
+  qcom,sdi-enabled:
+    description:
+      Indicates that the SDI (Secure Debug Image) has been enabled by TZ
+      by default and it needs to be disabled.
+      If not disabled WDT assertion or reboot will cause the board to hang
+      in the debug mode.
+    type: boolean
+
   qcom,dload-mode:
     $ref: /schemas/types.yaml#/definitions/phandle-array
     items: