diff mbox series

[v2,1/3] dt-bindings: watchdog: ti,rti-wdt: Add support for WDIOF_CARDRESET

Message ID 20230711091713.1113010-2-huaqian.li@siemens.com (mailing list archive)
State Superseded
Headers show
Series Add support for WDIOF_CARDRESET on TI AM65x | expand

Commit Message

Li, Hua Qian July 11, 2023, 9:17 a.m. UTC 
From: Li Hua Qian <huaqian.li@siemens.com>

TI RTI (Real Time Interrupt) Watchdog doesn't support to record the
watchdog cause. Add a reserved memory to know the last reboot was caused
by the watchdog card. In the reserved memory, some specific info will be
saved to indicate whether the watchdog reset was triggered in last
boot.

Signed-off-by: Li Hua Qian <huaqian.li@siemens.com>
---
 .../devicetree/bindings/watchdog/ti,rti-wdt.yaml    | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Krzysztof Kozlowski July 11, 2023, 9:23 a.m. UTC | #1 
On 11/07/2023 11:17, huaqian.li@siemens.com wrote:
> From: Li Hua Qian <huaqian.li@siemens.com>
> 
> TI RTI (Real Time Interrupt) Watchdog doesn't support to record the
> watchdog cause. Add a reserved memory to know the last reboot was caused
> by the watchdog card. In the reserved memory, some specific info will be
> saved to indicate whether the watchdog reset was triggered in last
> boot.
> 
> Signed-off-by: Li Hua Qian <huaqian.li@siemens.com>
> ---

Missing changelog.

>  .../devicetree/bindings/watchdog/ti,rti-wdt.yaml    | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml b/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
> index fc553211e42d..f227db08dc70 100644
> --- a/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
> @@ -26,7 +26,18 @@ properties:
>        - ti,j7-rti-wdt
>  
>    reg:
> -    maxItems: 1
> +    maxItems: 2

The expected syntax is in such case:
  items:
    - description: ...
    - description: ...

You will find plenty of examples for this.

> +      description:
> +	- Contains the address and the size of MCU RTI register.
> +	- Contains the address and the size of reserved memory, which

I don't think Conor suggested using reg of the device, but reg of
reserved memory. This is not device address space, but just some random
memory.

memory-region seems proper to me. We were just discussing totally
useless new property of size.

What's more - you did not test it... so usual template:

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

Best regards,
Krzysztof
Rob Herring (Arm) July 11, 2023, 10:13 a.m. UTC | #2 
On Tue, 11 Jul 2023 17:17:11 +0800, huaqian.li@siemens.com wrote:
> From: Li Hua Qian <huaqian.li@siemens.com>
> 
> TI RTI (Real Time Interrupt) Watchdog doesn't support to record the
> watchdog cause. Add a reserved memory to know the last reboot was caused
> by the watchdog card. In the reserved memory, some specific info will be
> saved to indicate whether the watchdog reset was triggered in last
> boot.
> 
> Signed-off-by: Li Hua Qian <huaqian.li@siemens.com>
> ---
>  .../devicetree/bindings/watchdog/ti,rti-wdt.yaml    | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml:30:18: [error] syntax error: mapping values are not allowed here (syntax)

dtschema/dtc warnings/errors:
make[2]: *** Deleting file 'Documentation/devicetree/bindings/watchdog/ti,rti-wdt.example.dts'
Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml:30:18: mapping values are not allowed in this context
make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/watchdog/ti,rti-wdt.example.dts] Error 1
make[2]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml:30:18: mapping values are not allowed in this context
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml: ignoring, error parsing file
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1500: dt_binding_check] Error 2
make: *** [Makefile:234: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230711091713.1113010-2-huaqian.li@siemens.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Li, Hua Qian July 12, 2023, 1:40 a.m. UTC | #3 
On Tue, 2023-07-11 at 11:23 +0200, Krzysztof Kozlowski wrote:
> On 11/07/2023 11:17, huaqian.li@siemens.com wrote:
> > From: Li Hua Qian <huaqian.li@siemens.com>
> > 
> > TI RTI (Real Time Interrupt) Watchdog doesn't support to record the
> > watchdog cause. Add a reserved memory to know the last reboot was
> > caused
> > by the watchdog card. In the reserved memory, some specific info
> > will be
> > saved to indicate whether the watchdog reset was triggered in last
> > boot.
> > 
> > Signed-off-by: Li Hua Qian <huaqian.li@siemens.com>
> > ---
> 
> Missing changelog.
Thanks for pointing out.
> 
> >  .../devicetree/bindings/watchdog/ti,rti-wdt.yaml    | 13
> > ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/watchdog/ti,rti-
> > wdt.yaml b/Documentation/devicetree/bindings/watchdog/ti,rti-
> > wdt.yaml
> > index fc553211e42d..f227db08dc70 100644
> > --- a/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
> > +++ b/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
> > @@ -26,7 +26,18 @@ properties:
> >        - ti,j7-rti-wdt
> >  
> >    reg:
> > -    maxItems: 1
> > +    maxItems: 2
> 
> The expected syntax is in such case:
>   items:
>     - description: ...
>     - description: ...
> 
> You will find plenty of examples for this.
> 
> > +      description:
> > +       - Contains the address and the size of MCU RTI register.
> > +       - Contains the address and the size of reserved memory,
> > which
> 
> I don't think Conor suggested using reg of the device, but reg of
> reserved memory. This is not device address space, but just some
> random
> memory.
> 
> memory-region seems proper to me. We were just discussing totally
> useless new property of size.
I guess I misunderstood what Conor suggested, I will change back to
memory-region and get size from reserved-memory. Does this look good
to you?
> 
> What's more - you did not test it... so usual template:
> 
> It does not look like you tested the DTS against bindings. Please run
> `make dtbs_check` (see
> Documentation/devicetree/bindings/writing-schema.rst or
> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
> for instructions).
> 
> Best regards,
> Krzysztof
> 
Sorry for this, I will make sure this is no problem in the next
version.

Best regards,
Li Hua Qian
Li, Hua Qian July 12, 2023, 1:42 a.m. UTC | #4 
On Tue, 2023-07-11 at 04:13 -0600, Rob Herring wrote:
> 
> On Tue, 11 Jul 2023 17:17:11 +0800, huaqian.li@siemens.com wrote:
> > From: Li Hua Qian <huaqian.li@siemens.com>
> > 
> > TI RTI (Real Time Interrupt) Watchdog doesn't support to record the
> > watchdog cause. Add a reserved memory to know the last reboot was
> > caused
> > by the watchdog card. In the reserved memory, some specific info
> > will be
> > saved to indicate whether the watchdog reset was triggered in last
> > boot.
> > 
> > Signed-off-by: Li Hua Qian <huaqian.li@siemens.com>
> > ---
> >  .../devicetree/bindings/watchdog/ti,rti-wdt.yaml    | 13
> > ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m
> dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml:30:18:
> [error] syntax error: mapping values are not allowed here (syntax)
> 
> dtschema/dtc warnings/errors:
> make[2]: *** Deleting file
> 'Documentation/devicetree/bindings/watchdog/ti,rti-wdt.example.dts'
> Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml:30:18:
> mapping values are not allowed in this context
> make[2]: *** [Documentation/devicetree/bindings/Makefile:26:
> Documentation/devicetree/bindings/watchdog/ti,rti-wdt.example.dts]
> Error 1
> make[2]: *** Waiting for unfinished jobs....
> ./Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml:30:18:
> mapping values are not allowed in this context
> /builds/robherring/dt-review-
> ci/linux/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml:
> ignoring, error parsing file
> make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1500:
> dt_binding_check] Error 2
> make: *** [Makefile:234: __sub-make] Error 2
> 
> doc reference errors (make refcheckdocs):
> 
> See
> https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230711091713.1113010-2-huaqian.li@siemens.com
> 
> The base for the series is generally the latest rc1. A different
> dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up
> to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself.
> Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up
> checking
> your schema. However, it must be unset to test all examples with your
> schema.
> 
Thanks for the test and your advice, I will make sure this is no
problem in the next version.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml b/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
index fc553211e42d..f227db08dc70 100644
--- a/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/ti,rti-wdt.yaml
@@ -26,7 +26,18 @@  properties:
       - ti,j7-rti-wdt
 
   reg:
-    maxItems: 1
+    maxItems: 2
+      description:
+	- Contains the address and the size of MCU RTI register.
+	- Contains the address and the size of reserved memory, which
+	  has the pre-stored watchdog reset cause as power-on reason. The
+	  second item is optional.
+	  In the reserved memory, the following values are needed at the
+	  first 12 bytes to tell that last boot was caused by watchdog
+	  reset.
+	  - PON_REASON_SOF_NUM:   0xBBBBCCCC
+	  - PON_REASON_MAGIC_NUM: 0xDDDDDDDD
+	  - PON_REASON_EOF_NUM:   0xCCCCBBBB
 
   clocks:
     maxItems: 1