Re: [PATCH v3 1/2] dt-bindings: rng: bcm2835: document reset support
On Thu, Mar 4, 2021 at 6:07 AM Nicolas Saenz Julienne wrote: > > Hi Alvaro, > > On Tue, 2021-02-23 at 18:00 +0100, Álvaro Fernández Rojas wrote: > > Some devices may need to perform a reset before using the RNG, such as the > > BCM6368. > > > > Signed-off-by: Álvaro Fernández Rojas > > --- > > v3: make resets required if brcm,bcm6368-rng. > > v2: document reset support. > > > > .../devicetree/bindings/rng/brcm,bcm2835.yaml | 17 + > > 1 file changed, 17 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml > > b/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml > > index c147900f9041..11c23e1f6988 100644 > > --- a/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml > > +++ b/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml > > @@ -37,6 +37,21 @@ required: > > > > > > additionalProperties: false > > I can't claim I fully understand all the meta stuff in shemas, so I generally > just follow the patterns already available out there. That said, shoudln't > this > be at the end, just before the examples? Maybe the cause of that odd warning > you got there? Yes, 'resets' needs to be defined under 'properties' as additionalProperties can't 'see' into if/then schemas. The top level needs to define everything and then if/then schema just add additional constraints. > > > +if: > > + properties: > > +compatible: > > + enum: > > +- brcm,bcm6368-rng > > +then: > > + properties: > > +resets: > > + maxItems: 1 > > + required: > > +- resets > > I belive you can't really make a property required when the bindings for > 'brcm,bcm6368-rng' were already defined. This will break the schema for those > otherwise correct devicetrees. Right, unless not having the property meant the device never would have worked. Rob
Re: [PATCH v3 1/2] dt-bindings: rng: bcm2835: document reset support
Hi Nicolas,
> El 4 mar 2021, a las 14:30, Nicolas Saenz Julienne
> escribió:
>
> On Thu, 2021-03-04 at 13:18 +0100, Álvaro Fernández Rojas wrote:
>>
>>> El 4 mar 2021, a las 13:07, Nicolas Saenz Julienne
>>> escribió:
>>>
>>> Hi Alvaro,
>>>
>>> On Tue, 2021-02-23 at 18:00 +0100, Álvaro Fernández Rojas wrote:
Some devices may need to perform a reset before using the RNG, such as the
BCM6368.
Signed-off-by: Álvaro Fernández Rojas
---
v3: make resets required if brcm,bcm6368-rng.
v2: document reset support.
.../devicetree/bindings/rng/brcm,bcm2835.yaml | 17 +
1 file changed, 17 insertions(+)
diff --git a/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
b/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
index c147900f9041..11c23e1f6988 100644
--- a/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
+++ b/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
@@ -37,6 +37,21 @@ required:
additionalProperties: false
>>>
>>> I can't claim I fully understand all the meta stuff in shemas, so I
>>> generally
>>> just follow the patterns already available out there.
>>
>> Well, that makes two of us :).
>>
>>> That said, shoudln't this be at the end, just before the examples?
>>
>> I don’t know but I can move it there ¯\_(ツ)_/¯
>
> Yes please do. See commit 7f464532b05 ("dt-bindings: Add missing
> 'additionalProperties: false'") which expands on why it is needed, and why it
> should be at the end.
>
>>> Maybe the cause of that odd warning
>>> you got there?
>>
>> Which odd warning?
>
> The one pointed out by Rob Herring's script, which I can reproduce too BTW. On
> the other hand I can't really tell what's wrong right away.
Well, I can’t reproduce that locally and I don’t know what’s wrong either, I
think that the best option is to remove the full if block.
>
>> I don’t get any warnings when running (or at least warnings related to rig,
>> because I get warnings related to other yamls):
>> make dt_binding_check
>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
>>
>>>
+if:
+ properties:
+compatible:
+ enum:
+- brcm,bcm6368-rng
+then:
+ properties:
+resets:
+ maxItems: 1
+ required:
+- resets
>>>
>>> I belive you can't really make a property required when the bindings for
>>> 'brcm,bcm6368-rng' were already defined. This will break the schema for
>>> those
>>> otherwise correct devicetrees.
>>
>> Why not?
>> Wouldn’t just be required for brcm,bcm6368-rng?
>
> Well, do all 'brcm,bcm6368-rng' users absolutely need the reset controller? If
> so, the original binding is wrong, which should be mentioned and a 'Fixes:'
> tag
> should be added to the commit message. Otherwise, the requirement is optional.
I’m not sure about this...
>
> Regards,
> Nicolas
Best regards,
Álvaro.
Re: [PATCH v3 1/2] dt-bindings: rng: bcm2835: document reset support
On Thu, 2021-03-04 at 13:18 +0100, Álvaro Fernández Rojas wrote:
>
> > El 4 mar 2021, a las 13:07, Nicolas Saenz Julienne
> > escribió:
> >
> > Hi Alvaro,
> >
> > On Tue, 2021-02-23 at 18:00 +0100, Álvaro Fernández Rojas wrote:
> > > Some devices may need to perform a reset before using the RNG, such as the
> > > BCM6368.
> > >
> > > Signed-off-by: Álvaro Fernández Rojas
> > > ---
> > > v3: make resets required if brcm,bcm6368-rng.
> > > v2: document reset support.
> > >
> > > .../devicetree/bindings/rng/brcm,bcm2835.yaml | 17 +
> > > 1 file changed, 17 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
> > > b/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
> > > index c147900f9041..11c23e1f6988 100644
> > > --- a/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
> > > +++ b/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
> > > @@ -37,6 +37,21 @@ required:
> > >
> > >
> > >
> > > additionalProperties: false
> >
> > I can't claim I fully understand all the meta stuff in shemas, so I
> > generally
> > just follow the patterns already available out there.
>
> Well, that makes two of us :).
>
> > That said, shoudln't this be at the end, just before the examples?
>
> I don’t know but I can move it there ¯\_(ツ)_/¯
Yes please do. See commit 7f464532b05 ("dt-bindings: Add missing
'additionalProperties: false'") which expands on why it is needed, and why it
should be at the end.
> > Maybe the cause of that odd warning
> > you got there?
>
> Which odd warning?
The one pointed out by Rob Herring's script, which I can reproduce too BTW. On
the other hand I can't really tell what's wrong right away.
> I don’t get any warnings when running (or at least warnings related to rig,
> because I get warnings related to other yamls):
> make dt_binding_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
>
> >
> > > +if:
> > > + properties:
> > > +compatible:
> > > + enum:
> > > +- brcm,bcm6368-rng
> > > +then:
> > > + properties:
> > > +resets:
> > > + maxItems: 1
> > > + required:
> > > +- resets
> >
> > I belive you can't really make a property required when the bindings for
> > 'brcm,bcm6368-rng' were already defined. This will break the schema for
> > those
> > otherwise correct devicetrees.
>
> Why not?
> Wouldn’t just be required for brcm,bcm6368-rng?
Well, do all 'brcm,bcm6368-rng' users absolutely need the reset controller? If
so, the original binding is wrong, which should be mentioned and a 'Fixes:' tag
should be added to the commit message. Otherwise, the requirement is optional.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v3 1/2] dt-bindings: rng: bcm2835: document reset support
> El 4 mar 2021, a las 13:07, Nicolas Saenz Julienne
> escribió:
>
> Hi Alvaro,
>
> On Tue, 2021-02-23 at 18:00 +0100, Álvaro Fernández Rojas wrote:
>> Some devices may need to perform a reset before using the RNG, such as the
>> BCM6368.
>>
>> Signed-off-by: Álvaro Fernández Rojas
>> ---
>> v3: make resets required if brcm,bcm6368-rng.
>> v2: document reset support.
>>
>> .../devicetree/bindings/rng/brcm,bcm2835.yaml | 17 +
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
>> b/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
>> index c147900f9041..11c23e1f6988 100644
>> --- a/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
>> +++ b/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
>> @@ -37,6 +37,21 @@ required:
>>
>>
>> additionalProperties: false
>
> I can't claim I fully understand all the meta stuff in shemas, so I generally
> just follow the patterns already available out there.
Well, that makes two of us :).
> That said, shoudln't this be at the end, just before the examples?
I don’t know but I can move it there ¯\_(ツ)_/¯
> Maybe the cause of that odd warning
> you got there?
Which odd warning?
I don’t get any warnings when running (or at least warnings related to rig,
because I get warnings related to other yamls):
make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
>
>> +if:
>> + properties:
>> +compatible:
>> + enum:
>> +- brcm,bcm6368-rng
>> +then:
>> + properties:
>> +resets:
>> + maxItems: 1
>> + required:
>> +- resets
>
> I belive you can't really make a property required when the bindings for
> 'brcm,bcm6368-rng' were already defined. This will break the schema for those
> otherwise correct devicetrees.
Why not?
Wouldn’t just be required for brcm,bcm6368-rng?
Anyway, I can omit this, since it would be the same for clocks and those aren’t
required either.
>
>> +else:
>> + properties:
>> +resets: false
>> +
>> examples:
>>- |
>> rng@7e104000 {
>> @@ -58,4 +73,6 @@ examples:
>>
>>
>> clocks = <_clk 18>;
>> clock-names = "ipsec";
>> +
>> +resets = <_rst 4>;
>> };
>
> Regards,
> Nicolas
Best regards,
Álvaro.
Re: [PATCH v3 1/2] dt-bindings: rng: bcm2835: document reset support
Hi Alvaro,
On Tue, 2021-02-23 at 18:00 +0100, Álvaro Fernández Rojas wrote:
> Some devices may need to perform a reset before using the RNG, such as the
> BCM6368.
>
> Signed-off-by: Álvaro Fernández Rojas
> ---
> v3: make resets required if brcm,bcm6368-rng.
> v2: document reset support.
>
> .../devicetree/bindings/rng/brcm,bcm2835.yaml | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
> b/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
> index c147900f9041..11c23e1f6988 100644
> --- a/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
> +++ b/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
> @@ -37,6 +37,21 @@ required:
>
>
> additionalProperties: false
I can't claim I fully understand all the meta stuff in shemas, so I generally
just follow the patterns already available out there. That said, shoudln't this
be at the end, just before the examples? Maybe the cause of that odd warning
you got there?
> +if:
> + properties:
> +compatible:
> + enum:
> +- brcm,bcm6368-rng
> +then:
> + properties:
> +resets:
> + maxItems: 1
> + required:
> +- resets
I belive you can't really make a property required when the bindings for
'brcm,bcm6368-rng' were already defined. This will break the schema for those
otherwise correct devicetrees.
> +else:
> + properties:
> +resets: false
> +
> examples:
> - |
> rng@7e104000 {
> @@ -58,4 +73,6 @@ examples:
>
>
> clocks = <_clk 18>;
> clock-names = "ipsec";
> +
> +resets = <_rst 4>;
> };
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v3 1/2] dt-bindings: rng: bcm2835: document reset support
Hi Rob, > El 23 feb 2021, a las 20:34, Rob Herring escribió: > > On Tue, 23 Feb 2021 18:00:05 +0100, Álvaro Fernández Rojas wrote: >> Some devices may need to perform a reset before using the RNG, such as the >> BCM6368. >> >> Signed-off-by: Álvaro Fernández Rojas >> --- >> v3: make resets required if brcm,bcm6368-rng. >> v2: document reset support. >> >> .../devicetree/bindings/rng/brcm,bcm2835.yaml | 17 + >> 1 file changed, 17 insertions(+) >> > > My bot found errors running 'make dt_binding_check' on your patch: > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/rng/brcm,bcm2835.example.dt.yaml: > rng@10004180: 'resets' does not match any of the regexes: 'pinctrl-[0-9]+' > From schema: > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml I don’t get what’s wrong here... > > See https://patchwork.ozlabs.org/patch/1443582 > > 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. > Best regards, Álvaro.
Re: [PATCH v3 1/2] dt-bindings: rng: bcm2835: document reset support
On Tue, 23 Feb 2021 18:00:05 +0100, Ãlvaro Fernández Rojas wrote: > Some devices may need to perform a reset before using the RNG, such as the > BCM6368. > > Signed-off-by: Ãlvaro Fernández Rojas > --- > v3: make resets required if brcm,bcm6368-rng. > v2: document reset support. > > .../devicetree/bindings/rng/brcm,bcm2835.yaml | 17 + > 1 file changed, 17 insertions(+) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/rng/brcm,bcm2835.example.dt.yaml: rng@10004180: 'resets' does not match any of the regexes: 'pinctrl-[0-9]+' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml See https://patchwork.ozlabs.org/patch/1443582 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.
[PATCH v3 1/2] dt-bindings: rng: bcm2835: document reset support
Some devices may need to perform a reset before using the RNG, such as the
BCM6368.
Signed-off-by: Álvaro Fernández Rojas
---
v3: make resets required if brcm,bcm6368-rng.
v2: document reset support.
.../devicetree/bindings/rng/brcm,bcm2835.yaml | 17 +
1 file changed, 17 insertions(+)
diff --git a/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
b/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
index c147900f9041..11c23e1f6988 100644
--- a/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
+++ b/Documentation/devicetree/bindings/rng/brcm,bcm2835.yaml
@@ -37,6 +37,21 @@ required:
additionalProperties: false
+if:
+ properties:
+compatible:
+ enum:
+- brcm,bcm6368-rng
+then:
+ properties:
+resets:
+ maxItems: 1
+ required:
+- resets
+else:
+ properties:
+resets: false
+
examples:
- |
rng@7e104000 {
@@ -58,4 +73,6 @@ examples:
clocks = <_clk 18>;
clock-names = "ipsec";
+
+resets = <_rst 4>;
};
--
2.20.1
