LinuxLists.cc - [PATCH 1/3] dt-bindings: reset: syscon-reboot: Add priority property

[permalink] [raw]

Subject: [PATCH 2/3] power: reset: syscon-reboot: Add support for specifying priority

Read new optional device tree property priority for specifying priority
level of reset handler. Default value is 192 as before.

Signed-off-by: Pali Rohár <[email protected]>
---
drivers/power/reset/syscon-reboot.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/power/reset/syscon-reboot.c b/drivers/power/reset/syscon-reboot.c
index 510e363381ca..45e34e6885f7 100644
--- a/drivers/power/reset/syscon-reboot.c
+++ b/drivers/power/reset/syscon-reboot.c
@@ -44,6 +44,7 @@ static int syscon_reboot_probe(struct platform_device *pdev)
struct syscon_reboot_context *ctx;
struct device *dev = &pdev->dev;
int mask_err, value_err;
+ int priority;
int err;

ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL);
@@ -57,6 +58,9 @@ static int syscon_reboot_probe(struct platform_device *pdev)
return PTR_ERR(ctx->map);
}

+ if (of_property_read_s32(pdev->dev.of_node, "priority", &priority))
+ priority = 192;
+
if (of_property_read_u32(pdev->dev.of_node, "offset", &ctx->offset))
return -EINVAL;

@@ -77,7 +81,7 @@ static int syscon_reboot_probe(struct platform_device *pdev)
}

ctx->restart_handler.notifier_call = syscon_restart_handle;
- ctx->restart_handler.priority = 192;
+ ctx->restart_handler.priority = priority;
err = register_restart_handler(&ctx->restart_handler);
if (err)
dev_err(dev, "can't register restart notifier (err=%d)\n", err);
--
2.20.1

2022-08-20 11:11:04

[permalink] [raw]

Subject: [PATCH 3/3] powerpc: dts: turris1x.dts: Set lower priority for CPLD syscon-reboot

Due to CPLD firmware bugs, set CPLD syscon-reboot priority level to 64
(between rstcr and watchdog) to ensure that rstcr's global-utilities reset
method which is preferred stay as default one, and to ensure that CPLD
syscon-reboot is more preferred than watchdog reset method.

Fixes: 0531a4abd1c6 ("powerpc: dts: turris1x.dts: Add CPLD reboot node")
Signed-off-by: Pali Rohár <[email protected]>
---
arch/powerpc/boot/dts/turris1x.dts | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)

diff --git a/arch/powerpc/boot/dts/turris1x.dts b/arch/powerpc/boot/dts/turris1x.dts
index 69c38ed8a3a5..c189293d5a1e 100644
--- a/arch/powerpc/boot/dts/turris1x.dts
+++ b/arch/powerpc/boot/dts/turris1x.dts
@@ -353,11 +353,34 @@
};

reboot@d {
+ /*
+ * CPLD firmware which manages system reset and
+ * watchdog registers has bugs. It does not
+ * autoclear system reset register after change
+ * and watchdog ignores reset line on immediate
+ * succeeding reset cycle triggered by watchdog.
+ * These bugs have to be workarounded in U-Boot
+ * bootloader. So use system reset via syscon as
+ * a last resort because older U-Boot versions
+ * do not have workaround for watchdog.
+ *
+ * Reset method via rstcr's global-utilities
+ * (the preferred one) has priority level 128,
+ * watchdog has priority level 0 and default
+ * syscon-reboot priority level is 192.
+ *
+ * So define syscon-reboot with custom priority
+ * level 64 (between rstcr and watchdog) because
+ * rstcr should stay as default preferred reset
+ * method and reset via watchdog is more broken
+ * than system reset via syscon.
+ */
compatible = "syscon-reboot";
reg = <0x0d 0x01>;
offset = <0x0d>;
mask = <0x01>;
value = <0x01>;
+ priority = <64>;
};

led-controller@13 {
--
2.20.1

2022-08-21 20:22:22

[permalink] [raw]

Subject: Re: [PATCH 1/3] dt-bindings: reset: syscon-reboot: Add priority property

On Sat, 20 Aug 2022 12:29:23 +0200, Pali Rohár wrote:
> This new optional priority property allows to specify custom priority level
> of reset device. Default level was always 192.
>
> Signed-off-by: Pali Rohár <[email protected]>
> ---
> .../devicetree/bindings/power/reset/syscon-reboot.yaml | 4 ++++
> 1 file changed, 4 insertions(+)
>

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:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml: Unresolvable JSON pointer: 'definitions/sint32'

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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.

2022-08-22 12:57:05

[permalink] [raw]

Subject: Re: [PATCH 1/3] dt-bindings: reset: syscon-reboot: Add priority property

On Sat, Aug 20, 2022 at 12:29:23PM +0200, Pali Roh?r wrote:
> This new optional priority property allows to specify custom priority level
> of reset device. Default level was always 192.

Why do we need/want this? What problem does it solve?

> Signed-off-by: Pali Roh?r <[email protected]>
> ---
> .../devicetree/bindings/power/reset/syscon-reboot.yaml | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> index da2509724812..d905133aab27 100644
> --- a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> @@ -42,6 +42,10 @@ properties:
> $ref: /schemas/types.yaml#/definitions/uint32
> description: The reset value written to the reboot register (32 bit access).
>
> + priority:

A bit too generic for the name.

> + $ref: /schemas/types.yaml#/definitions/sint32
> + description: Priority level of this syscon reset device. Default 192.

default: 192

Though I'm not really sure about the whole concept of this in DT. Where
does 192 come from? Presumably if we have more than 1 reset device, then
'priority' is needed in multiple places. So you need a common schema
defining the property (as property types should be defined exactly
once) which this schema can reference.

Rob

2022-08-22 13:58:57

[permalink] [raw]

Subject: Re: [PATCH 1/3] dt-bindings: reset: syscon-reboot: Add priority property

On Monday 22 August 2022 07:47:28 Rob Herring wrote:
> On Sat, Aug 20, 2022 at 12:29:23PM +0200, Pali Rohár wrote:
> > This new optional priority property allows to specify custom priority level
> > of reset device. Default level was always 192.
>
> Why do we need/want this? What problem does it solve?

See patch 3/3.

> > Signed-off-by: Pali Rohár <[email protected]>
> > ---
> > .../devicetree/bindings/power/reset/syscon-reboot.yaml | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> > index da2509724812..d905133aab27 100644
> > --- a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> > +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> > @@ -42,6 +42,10 @@ properties:
> > $ref: /schemas/types.yaml#/definitions/uint32
> > description: The reset value written to the reboot register (32 bit access).
> >
> > + priority:
>
> A bit too generic for the name.
>
> > + $ref: /schemas/types.yaml#/definitions/sint32
> > + description: Priority level of this syscon reset device. Default 192.
>
> default: 192
>
>
> Though I'm not really sure about the whole concept of this in DT. Where
> does 192 come from?

Implicitly from the current implementation and how it is used.

> Presumably if we have more than 1 reset device, then
> 'priority' is needed in multiple places. So you need a common schema
> defining the property (as property types should be defined exactly
> once) which this schema can reference.
>
> Rob

Sorry, I do not understand.

2022-08-30 23:06:01

[permalink] [raw]

Subject: [PATCH v2 1/3] dt-bindings: reset: syscon-reboot: Add priority property

This new optional priority property allows to specify custom priority level
of reset device. Default level was always 192.

Signed-off-by: Pali Rohár <[email protected]>

---
Changes in v2:
* Change sint32 to int32
* Add default
---
.../devicetree/bindings/power/reset/syscon-reboot.yaml | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
index da2509724812..4c8b0d0a0111 100644
--- a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
+++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
@@ -42,6 +42,11 @@ properties:
$ref: /schemas/types.yaml#/definitions/uint32
description: The reset value written to the reboot register (32 bit access).

+ priority:
+ $ref: /schemas/types.yaml#/definitions/int32
+ description: Priority level of this syscon reset device.
+ default: 192
+
required:
- compatible
- offset
--
2.20.1

2022-08-30 23:18:07

[permalink] [raw]

Subject: [PATCH v2 2/3] power: reset: syscon-reboot: Add support for specifying priority

2022-08-30 23:18:14

[permalink] [raw]

Subject: [PATCH v2 3/3] powerpc: dts: turris1x.dts: Set lower priority for CPLD syscon-reboot

2022-08-31 07:47:01

[permalink] [raw]

Subject: Re: [PATCH v2 1/3] dt-bindings: reset: syscon-reboot: Add priority property

On 31/08/2022 02:00, Pali Rohár wrote:
> This new optional priority property allows to specify custom priority level
> of reset device. Default level was always 192.

You still did not explain why do we need this. You only explained what
you did here, which is obvious and visible from the diff. What you
should explain is why you are doing it, what problem you are solving.

Best regards,
Krzysztof

2022-08-31 08:04:22

[permalink] [raw]

Subject: Re: [PATCH v2 1/3] dt-bindings: reset: syscon-reboot: Add priority property

On 31/08/2022 10:36, Pali Rohár wrote:
> On Wednesday 31 August 2022 10:31:22 Krzysztof Kozlowski wrote:
>> On 31/08/2022 02:00, Pali Rohár wrote:
>>> This new optional priority property allows to specify custom priority level
>>> of reset device. Default level was always 192.
>>
>> You still did not explain why do we need this. You only explained what
>> you did here, which is obvious and visible from the diff. What you
>> should explain is why you are doing it, what problem you are solving.
>>
>> Best regards,
>> Krzysztof
>
> Look at patch 3/3, thanks.

This commit should explain it why you add new property. Some other
commits going via different trees/branches and ending up in entirely
different time/place in history do not really count.

Best regards,
Krzysztof

2022-08-31 08:04:56

[permalink] [raw]

Subject: Re: [PATCH v2 1/3] dt-bindings: reset: syscon-reboot: Add priority property

On Wednesday 31 August 2022 10:31:22 Krzysztof Kozlowski wrote:
> On 31/08/2022 02:00, Pali Rohár wrote:
> > This new optional priority property allows to specify custom priority level
> > of reset device. Default level was always 192.
>
> You still did not explain why do we need this. You only explained what
> you did here, which is obvious and visible from the diff. What you
> should explain is why you are doing it, what problem you are solving.
>
> Best regards,
> Krzysztof

Look at patch 3/3, thanks.

2022-08-31 08:37:49

[permalink] [raw]

Subject: [PATCH v3 1/3] dt-bindings: reset: syscon-reboot: Add priority property

This new optional priority property allows to specify custom priority level
of reset device. Prior this change priority level was hardcoded to 192 and
not possible to specify or change. Specifying other value is needed for
some boards. Default level when not specified stays at 192 as before.

Signed-off-by: Pali Rohár <[email protected]>

---
Changes in v3:
* Add explanation into commit message

Changes in v2:
* Change sint32 to int32
* Add default
---
.../devicetree/bindings/power/reset/syscon-reboot.yaml | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
index da2509724812..4c8b0d0a0111 100644
--- a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
+++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
@@ -42,6 +42,11 @@ properties:
$ref: /schemas/types.yaml#/definitions/uint32
description: The reset value written to the reboot register (32 bit access).

+ priority:
+ $ref: /schemas/types.yaml#/definitions/int32
+ description: Priority level of this syscon reset device.
+ default: 192
+
required:
- compatible
- offset
--
2.20.1

2022-08-31 09:20:23

[permalink] [raw]

Subject: [PATCH v3 3/3] powerpc: dts: turris1x.dts: Set lower priority for CPLD syscon-reboot

2022-08-31 09:23:17

[permalink] [raw]

Subject: [PATCH v3 2/3] power: reset: syscon-reboot: Add support for specifying priority

2022-09-02 20:44:51

[permalink] [raw]

Subject: Re: [PATCH 1/3] dt-bindings: reset: syscon-reboot: Add priority property

On Mon, Aug 22, 2022 at 03:50:50PM +0200, Pali Roh?r wrote:
> On Monday 22 August 2022 07:47:28 Rob Herring wrote:
> > On Sat, Aug 20, 2022 at 12:29:23PM +0200, Pali Roh?r wrote:
> > > This new optional priority property allows to specify custom priority level
> > > of reset device. Default level was always 192.
> >
> > Why do we need/want this? What problem does it solve?
>
> See patch 3/3.
>
> > > Signed-off-by: Pali Roh?r <[email protected]>
> > > ---
> > > .../devicetree/bindings/power/reset/syscon-reboot.yaml | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> > > index da2509724812..d905133aab27 100644
> > > --- a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> > > +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> > > @@ -42,6 +42,10 @@ properties:
> > > $ref: /schemas/types.yaml#/definitions/uint32
> > > description: The reset value written to the reboot register (32 bit access).
> > >
> > > + priority:
> >
> > A bit too generic for the name.
> >
> > > + $ref: /schemas/types.yaml#/definitions/sint32
> > > + description: Priority level of this syscon reset device. Default 192.
> >
> > default: 192
> >
> >
> > Though I'm not really sure about the whole concept of this in DT. Where
> > does 192 come from?
>
> Implicitly from the current implementation and how it is used.

Implementation of what? u-boot? BSD? robOS?

> >?Presumably if we have more than 1 reset device, then
> > 'priority' is needed in multiple places. So you need a common schema
> > defining the property (as property types should be defined exactly
> > once) which this schema can reference.
> >
> > Rob
>
> Sorry, I do not understand.

So just keep sending new versions instead?

syscon-reboot is not the only binding for a system reset device, right?
So those others reset devices will need 'priority' too. For a given
property, there should only be one schema definition defining the type
for the property. Otherwise, there might be conflicts. So you need a
common schema doing that. And here you would just have 'priority: true'
or possibly some binding specific constraints.

Rob

2022-09-07 12:35:52

[permalink] [raw]

Subject: Re: [PATCH 1/3] dt-bindings: reset: syscon-reboot: Add priority property

On 02/09/2022 22:37, Rob Herring wrote:
>>
>> Sorry, I do not understand.
>
> So just keep sending new versions instead?
>
> syscon-reboot is not the only binding for a system reset device, right?
> So those others reset devices will need 'priority' too. For a given
> property, there should only be one schema definition defining the type
> for the property. Otherwise, there might be conflicts. So you need a
> common schema doing that. And here you would just have 'priority: true'
> or possibly some binding specific constraints.

I'll propose a patch for this.

Best regards,
Krzysztof

2022-09-07 13:08:51

[permalink] [raw]

Subject: Re: [PATCH v3 1/3] dt-bindings: reset: syscon-reboot: Add priority property

On 31/08/2022 10:17, Pali Rohár wrote:
> This new optional priority property allows to specify custom priority level
> of reset device. Prior this change priority level was hardcoded to 192 and
> not possible to specify or change. Specifying other value is needed for
> some boards. Default level when not specified stays at 192 as before.
>
> Signed-off-by: Pali Rohár <[email protected]>

Thanks for the changes. Explanation looks good.

I sent a patch adding the common schema with priority. If it gets
ack/review from Rob and Sebastian, please kindly rebase on top of it and
use same way as I did for gpio-restart.yaml

Best regards,
Krzysztof

2022-09-07 16:42:57

[permalink] [raw]

Subject: Re: [PATCH v3 1/3] dt-bindings: reset: syscon-reboot: Add priority property

On Wednesday 07 September 2022 14:38:42 Krzysztof Kozlowski wrote:
> On 31/08/2022 10:17, Pali Rohár wrote:
> > This new optional priority property allows to specify custom priority level
> > of reset device. Prior this change priority level was hardcoded to 192 and
> > not possible to specify or change. Specifying other value is needed for
> > some boards. Default level when not specified stays at 192 as before.
> >
> > Signed-off-by: Pali Rohár <[email protected]>
>
> Thanks for the changes. Explanation looks good.
>
> I sent a patch adding the common schema with priority. If it gets
> ack/review from Rob and Sebastian, please kindly rebase on top of it and
> use same way as I did for gpio-restart.yaml
>
> Best regards,
> Krzysztof

Ok, so just by adding "allOf: - $ref: restart-handler.yaml#" right?

2022-09-08 07:43:42

[permalink] [raw]

Subject: Re: [PATCH v3 1/3] dt-bindings: reset: syscon-reboot: Add priority property

On 07/09/2022 18:33, Pali Rohár wrote:
> On Wednesday 07 September 2022 14:38:42 Krzysztof Kozlowski wrote:
>> On 31/08/2022 10:17, Pali Rohár wrote:
>>> This new optional priority property allows to specify custom priority level
>>> of reset device. Prior this change priority level was hardcoded to 192 and
>>> not possible to specify or change. Specifying other value is needed for
>>> some boards. Default level when not specified stays at 192 as before.
>>>
>>> Signed-off-by: Pali Rohár <[email protected]>
>>
>> Thanks for the changes. Explanation looks good.
>>
>> I sent a patch adding the common schema with priority. If it gets
>> ack/review from Rob and Sebastian, please kindly rebase on top of it and
>> use same way as I did for gpio-restart.yaml
>>
>> Best regards,
>> Krzysztof
>
> Ok, so just by adding "allOf: - $ref: restart-handler.yaml#" right?

Yes.

Best regards,
Krzysztof

2022-12-26 12:12:36

[permalink] [raw]

Subject: [PATCH v4 1/3] dt-bindings: reset: syscon-reboot: Add priority property

This new optional priority property allows to specify custom priority level
of reset device. Prior this change priority level was hardcoded to 192 and
not possible to specify or change. Specifying other value is needed for
some boards. Default level when not specified stays at 192 as before.

Signed-off-by: Pali Rohár <[email protected]>

---
Changes in v4:
* Use restart-handler.yaml

Changes in v3:
* Add explanation into commit message

Changes in v2:
* Change sint32 to int32
* Add default
---
.../devicetree/bindings/power/reset/syscon-reboot.yaml | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
index da2509724812..f721bdf63a98 100644
--- a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
+++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
@@ -19,6 +19,9 @@ description: |+
sub-node of a "syscon", "simple-mfd" node. Though the regmap property
pointing to the system controller node is also supported.

+allOf:
+ - $ref: restart-handler.yaml#
+
properties:
compatible:
const: syscon-reboot
@@ -42,6 +45,9 @@ properties:
$ref: /schemas/types.yaml#/definitions/uint32
description: The reset value written to the reboot register (32 bit access).

+ priority:
+ default: 192
+
required:
- compatible
- offset
--
2.20.1

2022-12-26 12:13:18

[permalink] [raw]

Subject: [PATCH v4 2/3] power: reset: syscon-reboot: Add support for specifying priority

2022-12-26 12:17:33

[permalink] [raw]

Subject: Re: [PATCH v4 1/3] dt-bindings: reset: syscon-reboot: Add priority property

On 26/12/2022 12:45, Pali Rohár wrote:
> This new optional priority property allows to specify custom priority level
> of reset device. Prior this change priority level was hardcoded to 192 and
> not possible to specify or change. Specifying other value is needed for
> some boards. Default level when not specified stays at 192 as before.
>
> Signed-off-by: Pali Rohár <[email protected]>
>
> ---
> Changes in v4:
> * Use restart-handler.yaml

Acked-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

2022-12-26 12:18:44

[permalink] [raw]

Subject: [PATCH v4 3/3] powerpc: dts: turris1x.dts: Set lower priority for CPLD syscon-reboot

Due to CPLD firmware bugs, set CPLD syscon-reboot priority level to 64
(between rstcr and watchdog) to ensure that rstcr's global-utilities reset
method which is preferred stay as default one, and to ensure that CPLD
syscon-reboot is more preferred than watchdog reset method.

Fixes: 0531a4abd1c6 ("powerpc: dts: turris1x.dts: Add CPLD reboot node")
Signed-off-by: Pali Rohár <[email protected]>
---
arch/powerpc/boot/dts/turris1x.dts | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)

diff --git a/arch/powerpc/boot/dts/turris1x.dts b/arch/powerpc/boot/dts/turris1x.dts
index e9cda34a140e..c9b619f6ed5c 100644
--- a/arch/powerpc/boot/dts/turris1x.dts
+++ b/arch/powerpc/boot/dts/turris1x.dts
@@ -367,11 +367,34 @@
};

reboot@d {
+ /*
+ * CPLD firmware which manages system reset and
+ * watchdog registers has bugs. It does not
+ * autoclear system reset register after change
+ * and watchdog ignores reset line on immediate
+ * succeeding reset cycle triggered by watchdog.
+ * These bugs have to be workarounded in U-Boot
+ * bootloader. So use system reset via syscon as
+ * a last resort because older U-Boot versions
+ * do not have workaround for watchdog.
+ *
+ * Reset method via rstcr's global-utilities
+ * (the preferred one) has priority level 128,
+ * watchdog has priority level 0 and default
+ * syscon-reboot priority level is 192.
+ *
+ * So define syscon-reboot with custom priority
+ * level 64 (between rstcr and watchdog) because
+ * rstcr should stay as default preferred reset
+ * method and reset via watchdog is more broken
+ * than system reset via syscon.
+ */
compatible = "syscon-reboot";
reg = <0x0d 0x01>;
offset = <0x0d>;
mask = <0x01>;
value = <0x01>;
+ priority = <64>;
};

led-controller@13 {
--
2.20.1

2022-12-26 22:51:21

[permalink] [raw]

Subject: Re: [PATCH v4 1/3] dt-bindings: reset: syscon-reboot: Add priority property

On Mon, 26 Dec 2022 12:45:11 +0100, Pali Rohár wrote:
> This new optional priority property allows to specify custom priority level
> of reset device. Prior this change priority level was hardcoded to 192 and
> not possible to specify or change. Specifying other value is needed for
> some boards. Default level when not specified stays at 192 as before.
>
> Signed-off-by: Pali Rohár <[email protected]>
>
> ---
> Changes in v4:
> * Use restart-handler.yaml
>
> Changes in v3:
> * Add explanation into commit message
>
> Changes in v2:
> * Change sint32 to int32
> * Add default
> ---
> .../devicetree/bindings/power/reset/syscon-reboot.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>

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/power/reset/syscon-reboot.yaml:57:1: [error] duplication of key "allOf" in mapping (key-duplicates)

dtschema/dtc warnings/errors:
make[1]: *** Deleting file 'Documentation/devicetree/bindings/power/reset/syscon-reboot.example.dts'
Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml:57:1: found duplicate key "allOf" with value "[]" (original value: "[]")
make[1]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/power/reset/syscon-reboot.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/soc/samsung/exynos-pmu.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/power/reset/syscon-reboot.yaml
./Documentation/devicetree/bindings/mfd/canaan,k210-sysctl.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/power/reset/syscon-reboot.yaml
./Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml:57:1: found duplicate key "allOf" with value "[]" (original value: "[]")
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml: ignoring, error parsing file
make: *** [Makefile:1508: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):
Documentation/cdrom/packet-writing.rst: Documentation/ABI/testing/sysfs-class-pktcdvd
Documentation/cdrom/packet-writing.rst: Documentation/ABI/testing/debugfs-pktcdvd
tools/power/cpupower/man/cpupower-powercap-info.1: Documentation/power/powercap/powercap.txt

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

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.

2023-01-22 12:16:11

[permalink] [raw]

Subject: Re: [PATCH v4 1/3] dt-bindings: reset: syscon-reboot: Add priority property

Reminder: Any plan how to handle these patches?

On Monday 26 December 2022 12:45:11 Pali Rohár wrote:
> This new optional priority property allows to specify custom priority level
> of reset device. Prior this change priority level was hardcoded to 192 and
> not possible to specify or change. Specifying other value is needed for
> some boards. Default level when not specified stays at 192 as before.
>
> Signed-off-by: Pali Rohár <[email protected]>
>
> ---
> Changes in v4:
> * Use restart-handler.yaml
>
> Changes in v3:
> * Add explanation into commit message
>
> Changes in v2:
> * Change sint32 to int32
> * Add default
> ---
> .../devicetree/bindings/power/reset/syscon-reboot.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> index da2509724812..f721bdf63a98 100644
> --- a/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> +++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> @@ -19,6 +19,9 @@ description: |+
> sub-node of a "syscon", "simple-mfd" node. Though the regmap property
> pointing to the system controller node is also supported.
>
> +allOf:
> + - $ref: restart-handler.yaml#
> +
> properties:
> compatible:
> const: syscon-reboot
> @@ -42,6 +45,9 @@ properties:
> $ref: /schemas/types.yaml#/definitions/uint32
> description: The reset value written to the reboot register (32 bit access).
>
> + priority:
> + default: 192
> +
> required:
> - compatible
> - offset
> --
> 2.20.1
>

2023-02-03 12:16:25

by Sebastian Reichel

[permalink] [raw]

Subject: Re: [PATCH v4 1/3] dt-bindings: reset: syscon-reboot: Add priority property

Hi,

On Sun, Jan 22, 2023 at 12:27:44PM +0100, Pali Roh?r wrote:
> On Monday 26 December 2022 12:45:11 Pali Roh?r wrote:
> > This new optional priority property allows to specify custom priority level
> > of reset device. Prior this change priority level was hardcoded to 192 and
> > not possible to specify or change. Specifying other value is needed for
> > some boards. Default level when not specified stays at 192 as before.
> >
> > Signed-off-by: Pali Roh?r <[email protected]>
>
> Reminder: Any plan how to handle these patches?

Please don't top-post :) Sorry, I'm a bit slow with processing
patches this cycle. Technically this was also throwing a DT
warning reported by the bot, so I expected a new version. Anyways,
I fixed the DT warning myself and applied patches 1-2.

Thanks,

-- Sebastian

Attachments:

(No filename) (804.00 B)
signature.asc (833.00 B)
Download all attachments

2023-02-03 12:35:34

[permalink] [raw]

Subject: Re: [PATCH v4 1/3] dt-bindings: reset: syscon-reboot: Add priority property

On 03/02/2023 13:16, Sebastian Reichel wrote:
> Hi,
>
> On Sun, Jan 22, 2023 at 12:27:44PM +0100, Pali Rohár wrote:
>> On Monday 26 December 2022 12:45:11 Pali Rohár wrote:
>>> This new optional priority property allows to specify custom priority level
>>> of reset device. Prior this change priority level was hardcoded to 192 and
>>> not possible to specify or change. Specifying other value is needed for
>>> some boards. Default level when not specified stays at 192 as before.
>>>
>>> Signed-off-by: Pali Rohár <[email protected]>
>>
>> Reminder: Any plan how to handle these patches?
>
> Please don't top-post :) Sorry, I'm a bit slow with processing
> patches this cycle. Technically this was also throwing a DT
> warning reported by the bot, so I expected a new version. Anyways,
> I fixed the DT warning myself and applied patches 1-2.

Everyone was expecting new version, so that checks pass. We should not
take known-broken code, because then the duty to check for other errors
is on you Sebastian. :)

Best regards,
Krzysztof

2023-02-09 00:10:31

[permalink] [raw]

Subject: Re: [PATCH v4 3/3] powerpc: dts: turris1x.dts: Set lower priority for CPLD syscon-reboot

On Monday 26 December 2022 12:45:13 Pali Rohár wrote:
> Due to CPLD firmware bugs, set CPLD syscon-reboot priority level to 64
> (between rstcr and watchdog) to ensure that rstcr's global-utilities reset
> method which is preferred stay as default one, and to ensure that CPLD
> syscon-reboot is more preferred than watchdog reset method.
>
> Fixes: 0531a4abd1c6 ("powerpc: dts: turris1x.dts: Add CPLD reboot node")
> Signed-off-by: Pali Rohár <[email protected]>

May I ask who can take this 3/3 patch? powersupply or powerpc tree?

> ---
> arch/powerpc/boot/dts/turris1x.dts | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/arch/powerpc/boot/dts/turris1x.dts b/arch/powerpc/boot/dts/turris1x.dts
> index e9cda34a140e..c9b619f6ed5c 100644
> --- a/arch/powerpc/boot/dts/turris1x.dts
> +++ b/arch/powerpc/boot/dts/turris1x.dts
> @@ -367,11 +367,34 @@
> };
>
> reboot@d {
> + /*
> + * CPLD firmware which manages system reset and
> + * watchdog registers has bugs. It does not
> + * autoclear system reset register after change
> + * and watchdog ignores reset line on immediate
> + * succeeding reset cycle triggered by watchdog.
> + * These bugs have to be workarounded in U-Boot
> + * bootloader. So use system reset via syscon as
> + * a last resort because older U-Boot versions
> + * do not have workaround for watchdog.
> + *
> + * Reset method via rstcr's global-utilities
> + * (the preferred one) has priority level 128,
> + * watchdog has priority level 0 and default
> + * syscon-reboot priority level is 192.
> + *
> + * So define syscon-reboot with custom priority
> + * level 64 (between rstcr and watchdog) because
> + * rstcr should stay as default preferred reset
> + * method and reset via watchdog is more broken
> + * than system reset via syscon.
> + */
> compatible = "syscon-reboot";
> reg = <0x0d 0x01>;
> offset = <0x0d>;
> mask = <0x01>;
> value = <0x01>;
> + priority = <64>;
> };
>
> led-controller@13 {
> --
> 2.20.1
>

2023-02-18 12:05:43