| Message ID | 20230815140030.1068590-1-robimarko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/5] dt-bindings: firmware: qcom,scm: Document SDI disable | expand |
On 15/08/2023 15:59, 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 should be disabled. > > Signed-off-by: Robert Marko <robimarko@gmail.com> > --- > 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..bf753192498a 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-disable: The property should describe rather current hardware/firmware state, instead of expressing your intention for OS what to do. Therefore rather: qcom,sdi-enabled or qcom,secure-debug-image Best regards, Krzysztof
On Wed, Aug 16, 2023 at 08:15:54AM +0200, Krzysztof Kozlowski wrote: > On 15/08/2023 15:59, 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 should be disabled. > > > > Signed-off-by: Robert Marko <robimarko@gmail.com> > > --- > > 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..bf753192498a 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-disable: > > The property should describe rather current hardware/firmware state, > instead of expressing your intention for OS what to do. Therefore rather: > qcom,sdi-enabled > or > qcom,secure-debug-image Why can't you just disable SDI unconditionally when going into debug mode? Is doing that when not enabled going to crash the system or something? Rob
On Mon, 21 Aug 2023 at 21:31, Rob Herring <robh@kernel.org> wrote: > > On Wed, Aug 16, 2023 at 08:15:54AM +0200, Krzysztof Kozlowski wrote: > > On 15/08/2023 15:59, 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 should be disabled. > > > > > > Signed-off-by: Robert Marko <robimarko@gmail.com> > > > --- > > > 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..bf753192498a 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-disable: > > > > The property should describe rather current hardware/firmware state, > > instead of expressing your intention for OS what to do. Therefore rather: > > qcom,sdi-enabled > > or > > qcom,secure-debug-image > > Why can't you just disable SDI unconditionally when going into debug > mode? Is doing that when not enabled going to crash the system or > something? Because if not disabled you will enter the Secure Debug mode even on a regular reboot and then you have to pull the power in order to boot again. Even according to QCA docs they intended for the Linux to disable SDI as TZ/QSEE will always enable it as part of booting. Regards, Robert > > Rob >
On Mon, Aug 21, 2023 at 12:35 PM Robert Marko <robimarko@gmail.com> wrote: > On Mon, 21 Aug 2023 at 21:31, Rob Herring <robh@kernel.org> wrote: > > Why can't you just disable SDI unconditionally when going into debug > > mode? Is doing that when not enabled going to crash the system or > > something? I asked the same, to resounding silence: https://lore.kernel.org/all/20200721080054.2803881-1-computersforpeace@gmail.com/ https://lore.kernel.org/all/ZNlhSdh0qDMieTAS@localhost/ > Because if not disabled you will enter the Secure Debug mode even on a > regular reboot and then you have to pull the power in order to boot again. > Even according to QCA docs they intended for the Linux to disable SDI as > TZ/QSEE will always enable it as part of booting. NB: I've never read such docs. Presumably they're internal/private to Qualcomm and/or its direct partners? I'd love to see them. But, I think you (robinmarko) are not really answering the same question that Rob (robh) is asking. Rob is asking why you ever *don't* want to disable SDI. You're answering why we ever need to disable it at all. I don't think the latter question is controversial. FWIW, your description of those docs sounds like we should unconditionally *disable* SDI (like my first RFC above), which would answer Rob's question, and would agree with my RFC above :) And as a bonus, no Device Tree change would be required. Brian
On Mon, 21 Aug 2023 at 21:44, Brian Norris <computersforpeace@gmail.com> wrote: > > On Mon, Aug 21, 2023 at 12:35 PM Robert Marko <robimarko@gmail.com> wrote: > > On Mon, 21 Aug 2023 at 21:31, Rob Herring <robh@kernel.org> wrote: > > > Why can't you just disable SDI unconditionally when going into debug > > > mode? Is doing that when not enabled going to crash the system or > > > something? > > I asked the same, to resounding silence: > > https://lore.kernel.org/all/20200721080054.2803881-1-computersforpeace@gmail.com/ > https://lore.kernel.org/all/ZNlhSdh0qDMieTAS@localhost/ > > > Because if not disabled you will enter the Secure Debug mode even on a > > regular reboot and then you have to pull the power in order to boot again. > > Even according to QCA docs they intended for the Linux to disable SDI as > > TZ/QSEE will always enable it as part of booting. > > NB: I've never read such docs. Presumably they're internal/private to > Qualcomm and/or its direct partners? I'd love to see them. Sadly they are all behind the NDA. > > But, I think you (robinmarko) are not really answering the same > question that Rob (robh) is asking. Rob is asking why you ever *don't* > want to disable SDI. You're answering why we ever need to disable it > at all. I don't think the latter question is controversial. I understood his question differently, hence my answer. > > FWIW, your description of those docs sounds like we should > unconditionally *disable* SDI (like my first RFC above), which would > answer Rob's question, and would agree with my RFC above :) And as a > bonus, no Device Tree change would be required. Well, the thing is that I only have docs for some of the IPQ chips, and with the insane variety of SoC-s that use SCM and TZ/QSEE but completely different FW base or version something would break for sure so I would prefer to opt-in if its really required as SDI was something that was until IPQ5018 came along, always disabled by default, except for the weird Google WiFI board. Regards, Robert > > Brian
diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml index 4233ea839bfc..bf753192498a 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-disable: + 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:
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 should be disabled. Signed-off-by: Robert Marko <robimarko@gmail.com> --- Documentation/devicetree/bindings/firmware/qcom,scm.yaml | 8 ++++++++ 1 file changed, 8 insertions(+)