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 <[email protected]>
> ---
> 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 = <&periph_clk 18>;
> clock-names = "ipsec";
> +
> + resets = <&periph_rst 4>;
> };
Regards,
Nicolas
> El 4 mar 2021, a las 13:07, Nicolas Saenz Julienne <[email protected]> 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 <[email protected]>
>> ---
>> 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 = <&periph_clk 18>;
>> clock-names = "ipsec";
>> +
>> + resets = <&periph_rst 4>;
>> };
>
> Regards,
> Nicolas
Best regards,
Álvaro.
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 <[email protected]> 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 <[email protected]>
> > > ---
> > > 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