[v2,15/20] dt-bindings: nvmem: add YAML schema for the sl28 vpd layout

Add a schema for the NVMEM layout on Kontron's sl28 boards.

Signed-off-by: Michael Walle <michael@walle.cc>
---
changes since v1:
 - add custom select
 - add description
 - add "additionalProperties: false", I wasn't sure if all the
   subnodes needs it. I'd say yes, but the brcm,nvram binding
   doesn't have them neither.

 .../nvmem/layouts/kontron,sl28-vpd.yaml       | 67 +++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/layouts/kontron,sl28-vpd.yaml

On 02/09/2022 00:18, Michael Walle wrote:
> Add a schema for the NVMEM layout on Kontron's sl28 boards.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> changes since v1:
>  - add custom select
>  - add description
>  - add "additionalProperties: false", I wasn't sure if all the
>    subnodes needs it. I'd say yes, but the brcm,nvram binding
>    doesn't have them neither.
> 
>  .../nvmem/layouts/kontron,sl28-vpd.yaml       | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/nvmem/layouts/kontron,sl28-vpd.yaml
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/layouts/kontron,sl28-vpd.yaml b/Documentation/devicetree/bindings/nvmem/layouts/kontron,sl28-vpd.yaml
> new file mode 100644
> index 000000000000..0c180f29e880
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/layouts/kontron,sl28-vpd.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/nvmem/layouts/kontron,sl28-vpd.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NVMEM layout of the Kontron SMARC-sAL28 vital product data
> +
> +maintainers:
> +  - Michael Walle <michael@walle.cc>
> +
> +description:
> +  The vital product data (VPD) of the sl28 boards contains a serial
> +  number and a base MAC address. The actual MAC addresses for the
> +  on-board ethernet devices are derived from this base MAC address by
> +  adding an offset.
> +
> +# We need a select here so we don't match all nodes with 'user-otp'
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        const: kontron,sl28-vpd
> +  required:
> +    - compatible
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: kontron,sl28-vpd
> +      - const: user-otp
> +
> +  serial-number:

I will leave it for Rob to ack...

Best regards,
Krzysztof

On Fri, Sep 02, 2022 at 12:18:52AM +0200, Michael Walle wrote:
> Add a schema for the NVMEM layout on Kontron's sl28 boards.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> changes since v1:
>  - add custom select
>  - add description
>  - add "additionalProperties: false", I wasn't sure if all the
>    subnodes needs it. I'd say yes, but the brcm,nvram binding
>    doesn't have them neither.
> 
>  .../nvmem/layouts/kontron,sl28-vpd.yaml       | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/nvmem/layouts/kontron,sl28-vpd.yaml
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/layouts/kontron,sl28-vpd.yaml b/Documentation/devicetree/bindings/nvmem/layouts/kontron,sl28-vpd.yaml
> new file mode 100644
> index 000000000000..0c180f29e880
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/layouts/kontron,sl28-vpd.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/nvmem/layouts/kontron,sl28-vpd.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NVMEM layout of the Kontron SMARC-sAL28 vital product data
> +
> +maintainers:
> +  - Michael Walle <michael@walle.cc>
> +
> +description:
> +  The vital product data (VPD) of the sl28 boards contains a serial
> +  number and a base MAC address. The actual MAC addresses for the
> +  on-board ethernet devices are derived from this base MAC address by
> +  adding an offset.
> +
> +# We need a select here so we don't match all nodes with 'user-otp'
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        const: kontron,sl28-vpd
> +  required:
> +    - compatible
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: kontron,sl28-vpd
> +      - const: user-otp
> +
> +  serial-number:
> +    type: object
> +    description: The board's serial number
> +
> +  base-mac-address:
> +    type: object
> +    description:
> +      Base MAC address for all on-module network interfaces. The first
> +      argument of the phandle will be treated as an offset.
> +
> +    properties:
> +      "#nvmem-cell-cells":

You can't just add a new #.*-cells buried in a device binding. I'm fine 
with the concept though having more than 1 user would be nice.

Any case that doesn't match foos->#foo-cells or has a default # of 
cells if missing (as this does) has to be added to dtschema to decode it 
properly. It won't really matter until there's a user with 2 or more 
entries. I'm happy to do update the dtschema part, but I'd prefer to see 
the schema in dtschema rather than the kernel.

Rob

Am 2022-09-12 21:20, schrieb Rob Herring:

>> +  base-mac-address:
>> +    type: object
>> +    description:
>> +      Base MAC address for all on-module network interfaces. The 
>> first
>> +      argument of the phandle will be treated as an offset.
>> +
>> +    properties:
>> +      "#nvmem-cell-cells":
> 
> You can't just add a new #.*-cells buried in a device binding. I'm fine
> with the concept though having more than 1 user would be nice.

I was under the impression the tooling will handle it, but as you
pointed out below, this isn't the case for a missing default. The
statement above should only be to validate that there is one
additional argument if the base-mac-address node is used in a
phandle.

> Any case that doesn't match foos->#foo-cells or has a default # of
> cells if missing (as this does) has to be added to dtschema to decode 
> it
> properly. It won't really matter until there's a user with 2 or more
> entries. I'm happy to do update the dtschema part, but I'd prefer to 
> see
> the schema in dtschema rather than the kernel.

Ok, but I'm not sure I understand you correctly here. You will
update the dtschema tooling (I guess it's about fixup_phandles() in
dtb.py) and which schema should be in dtschema? nvmem.yaml
and/or nvmem-consumer.yaml? The entire schema or only a
subset of it?

-michael

Hello Rob & Krzysztof,

michael@walle.cc wrote on Tue, 13 Sep 2022 16:21:24 +0200:

> Am 2022-09-12 21:20, schrieb Rob Herring:
> 
> >> +  base-mac-address:
> >> +    type: object
> >> +    description:
> >> +      Base MAC address for all on-module network interfaces. The >> first
> >> +      argument of the phandle will be treated as an offset.
> >> +
> >> +    properties:
> >> +      "#nvmem-cell-cells":  
> > 
> > You can't just add a new #.*-cells buried in a device binding. I'm fine
> > with the concept though having more than 1 user would be nice.  
> 
> I was under the impression the tooling will handle it, but as you
> pointed out below, this isn't the case for a missing default. The
> statement above should only be to validate that there is one
> additional argument if the base-mac-address node is used in a
> phandle.
> 
> > Any case that doesn't match foos->#foo-cells or has a default # of
> > cells if missing (as this does) has to be added to dtschema to decode > it
> > properly. It won't really matter until there's a user with 2 or more
> > entries. I'm happy to do update the dtschema part, but I'd prefer to > see
> > the schema in dtschema rather than the kernel.  
> 
> Ok, but I'm not sure I understand you correctly here. You will
> update the dtschema tooling (I guess it's about fixup_phandles() in
> dtb.py) and which schema should be in dtschema? nvmem.yaml
> and/or nvmem-consumer.yaml? The entire schema or only a
> subset of it?

I currently see this as the main "blocking point", although Rob told he
was happy with the overall idea, so let's try to move forward together.

We discussed on IRC with Michael, I guess what's remaining is:
- Michael: Move #nvmem-cell-cells to nvmem.yaml in the core dtschema.
- Rob/Krzysztof: Add the necessary tooling to use this new
  property and enforce the right # of cells cells (may be added later
  as anyway for now we only have single consumer cases).

Is this what you meant?

Thanks,
Miquèl