diff mbox series

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

Message ID 20220820102925.29476-1-pali@kernel.org
State Superseded
Headers show
Series [1/3] dt-bindings: reset: syscon-reboot: Add priority property | expand

Commit Message

Pali Rohár Aug. 20, 2022, 10:29 a.m. UTC
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 <pali@kernel.org>
---
 .../devicetree/bindings/power/reset/syscon-reboot.yaml        | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Rob Herring Aug. 21, 2022, 8:21 p.m. UTC | #1
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 <pali@kernel.org>
> ---
>  .../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.
Pali Rohár Aug. 22, 2022, 1:50 p.m. UTC | #2
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 <pali@kernel.org>
> > ---
> >  .../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.
Krzysztof Kozlowski Sept. 7, 2022, 12:27 p.m. UTC | #3
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
Krzysztof Kozlowski Dec. 26, 2022, 12:02 p.m. UTC | #4
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 <pali@kernel.org>
> 
> ---
> Changes in v4:
> * Use restart-handler.yaml

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

Best regards,
Krzysztof
Rob Herring Dec. 26, 2022, 6:23 p.m. UTC | #5
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 <pali@kernel.org>
> 
> ---
> 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/20221226114513.4569-1-pali@kernel.org

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.
diff mbox series

Patch

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:
+    $ref: /schemas/types.yaml#/definitions/sint32
+    description: Priority level of this syscon reset device. Default 192.
+
 required:
   - compatible
   - offset