diff mbox series

dt-bindings: at24: convert the binding document to yaml

Message ID 20190923175211.2060-1-brgl@bgdev.pl
State Superseded
Headers show
Series dt-bindings: at24: convert the binding document to yaml | expand

Commit Message

Bartosz Golaszewski Sept. 23, 2019, 5:52 p.m. UTC
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Convert the binding document for at24 EEPROMs from txt to yaml. The
compatible property uses a regex pattern to address all the possible
combinations of "vendor,model" strings.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 .../devicetree/bindings/eeprom/at24.txt       |  90 +--------------
 .../devicetree/bindings/eeprom/at24.yaml      | 107 ++++++++++++++++++
 MAINTAINERS                                   |   2 +-
 3 files changed, 109 insertions(+), 90 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/eeprom/at24.yaml

Comments

Peter Rosin Sept. 23, 2019, 6:30 p.m. UTC | #1
On 2019-09-23 19:52, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Convert the binding document for at24 EEPROMs from txt to yaml. The
> compatible property uses a regex pattern to address all the possible
> combinations of "vendor,model" strings.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  .../devicetree/bindings/eeprom/at24.txt       |  90 +--------------
>  .../devicetree/bindings/eeprom/at24.yaml      | 107 ++++++++++++++++++
>  MAINTAINERS                                   |   2 +-
>  3 files changed, 109 insertions(+), 90 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/eeprom/at24.yaml
> 
> diff --git a/Documentation/devicetree/bindings/eeprom/at24.txt b/Documentation/devicetree/bindings/eeprom/at24.txt
> index 22aead844d0f..c94acbb8cb0c 100644
> --- a/Documentation/devicetree/bindings/eeprom/at24.txt
> +++ b/Documentation/devicetree/bindings/eeprom/at24.txt
> @@ -1,89 +1 @@
> -EEPROMs (I2C)
> -
> -Required properties:
> -
> -  - compatible: Must be a "<manufacturer>,<model>" pair. The following <model>
> -                values are supported (assuming "atmel" as manufacturer):
> -
> -                "atmel,24c00",
> -                "atmel,24c01",
> -                "atmel,24cs01",
> -                "atmel,24c02",
> -                "atmel,24cs02",
> -                "atmel,24mac402",
> -                "atmel,24mac602",
> -                "atmel,spd",
> -                "atmel,24c04",
> -                "atmel,24cs04",
> -                "atmel,24c08",
> -                "atmel,24cs08",
> -                "atmel,24c16",
> -                "atmel,24cs16",
> -                "atmel,24c32",
> -                "atmel,24cs32",
> -                "atmel,24c64",
> -                "atmel,24cs64",
> -                "atmel,24c128",
> -                "atmel,24c256",
> -                "atmel,24c512",
> -                "atmel,24c1024",
> -                "atmel,24c2048",
> -
> -                If <manufacturer> is not "atmel", then a fallback must be used
> -                with the same <model> and "atmel" as manufacturer.
> -
> -                Example:
> -                        compatible = "microchip,24c128", "atmel,24c128";
> -
> -                Supported manufacturers are:
> -
> -                "catalyst",
> -                "microchip",
> -                "nxp",
> -                "ramtron",
> -                "renesas",
> -                "rohm",
> -                "st",
> -
> -                Some vendors use different model names for chips which are just
> -                variants of the above. Known such exceptions are listed below:
> -
> -                "nxp,se97b" - the fallback is "atmel,24c02",
> -                "renesas,r1ex24002" - the fallback is "atmel,24c02"
> -                "renesas,r1ex24016" - the fallback is "atmel,24c16"
> -                "renesas,r1ex24128" - the fallback is "atmel,24c128"
> -                "rohm,br24t01" - the fallback is "atmel,24c01"
> -
> -  - reg: The I2C address of the EEPROM.
> -
> -Optional properties:
> -
> -  - pagesize: The length of the pagesize for writing. Please consult the
> -              manual of your device, that value varies a lot. A wrong value
> -              may result in data loss! If not specified, a safety value of
> -              '1' is used which will be very slow.
> -
> -  - read-only: This parameterless property disables writes to the eeprom.
> -
> -  - size: Total eeprom size in bytes.
> -
> -  - no-read-rollover: This parameterless property indicates that the
> -                      multi-address eeprom does not automatically roll over
> -                      reads to the next slave address. Please consult the
> -                      manual of your device.
> -
> -  - wp-gpios: GPIO to which the write-protect pin of the chip is connected.
> -
> -  - address-width: number of address bits (one of 8, 16).
> -
> -  - num-addresses: total number of i2c slave addresses this device takes
> -
> -Example:
> -
> -eeprom@52 {
> -	compatible = "atmel,24c32";
> -	reg = <0x52>;
> -	pagesize = <32>;
> -	wp-gpios = <&gpio1 3 0>;
> -	num-addresses = <8>;
> -};
> +This file has been moved to at24.yaml.
> diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
> new file mode 100644
> index 000000000000..28c8b068c8a1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
> @@ -0,0 +1,107 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright 2019 BayLibre SAS
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/eeprom/at24.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: I2C EEPROMs compatible with Atmel's AT24
> +
> +maintainers:
> +  - Bartosz Golaszewski <bgolaszewski@baylibre.com>
> +
> +properties:
> +  compatible:
> +    additionalItems: true
> +    maxItems: 2
> +    pattern: "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),24(c|cs|mac)[0-9]+$"
> +    oneOf:
> +      - const: nxp,se97b
> +      - const: renesas,r1ex24002
> +      - const: renesas,r1ex24016
> +      - const: renesas,r1ex24128
> +      - const: rohm,br24t01
> +    contains:
> +      enum:
> +        - atmel,24c00
> +        - atmel,24c01
> +        - atmel,24cs01
> +        - atmel,24c02
> +        - atmel,24cs02
> +        - atmel,24mac402
> +        - atmel,24mac602
> +        - atmel,spd
> +        - atmel,24c04
> +        - atmel,24cs04
> +        - atmel,24c08
> +        - atmel,24cs08
> +        - atmel,24c16
> +        - atmel,24cs16
> +        - atmel,24c32
> +        - atmel,24cs32
> +        - atmel,24c64
> +        - atmel,24cs64
> +        - atmel,24c128
> +        - atmel,24c256
> +        - atmel,24c512
> +        - atmel,24c1024
> +        - atmel,24c2048

The previous binding allows more e.g.

	compatible = "nxp,spd", "atmel,spd";

which is no longer allowed. That might be a problem? The previous binding
also allows less e.g.

	compatible = "atmel,24c00", "renesas,24mac402";

which of course is nonsense but AFAIU now allowed.

The new formal rules are therefore not "right", and it might be impossible
to express the subtleties of this weird binding with the current spec so
there might be little to do about it? But either way, these issues are not
mentioned neither in the binding nor the commit message. Should they be
mentioned?

Cheers,
Peter

> +
> +  reg:
> +    description:
> +      The I2C slave address of the EEPROM.
> +    maxItems: 1
> +
> +  pagesize:
> +    description:
> +      The length of the pagesize for writing. Please consult the
> +      manual of your device, that value varies a lot. A wrong value
> +      may result in data loss! If not specified, a safety value of
> +      '1' is used which will be very slow.
> +    type: integer
> +
> +  read-only:
> +    description:
> +      This parameterless property disables writes to the eeprom.
> +    type: boolean
> +
> +  size:
> +    description:
> +      Total eeprom size in bytes.
> +    type: integer
> +
> +  no-read-rollover:
> +    description:
> +      This parameterless property indicates that the multi-address
> +      eeprom does not automatically roll over reads to the next slave
> +      address. Please consult the manual of your device.
> +    type: boolean
> +
> +  wp-gpios:
> +    description:
> +      GPIO to which the write-protect pin of the chip is connected.
> +    maxItems: 1
> +
> +  address-width:
> +    description:
> +      Number of address bits (one of 8, 16).
> +    type: integer
> +
> +  num-addresses:
> +    description:
> +      Total number of i2c slave addresses this device takes.
> +    type: integer
> +
> +required:
> +  - compatible
> +  - reg
> +
> +examples:
> +  - |
> +    eeprom@52 {
> +        compatible = "microchip,24c32", "atmel,24c32";
> +        reg = <0x52>;
> +        pagesize = <32>;
> +        wp-gpios = <&gpio1 3 0>;
> +        num-addresses = <8>;
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a400af0501c9..3c7ced686966 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2698,7 +2698,7 @@ M:	Bartosz Golaszewski <bgolaszewski@baylibre.com>
>  L:	linux-i2c@vger.kernel.org
>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git
>  S:	Maintained
> -F:	Documentation/devicetree/bindings/eeprom/at24.txt
> +F:	Documentation/devicetree/bindings/eeprom/at24.yaml
>  F:	drivers/misc/eeprom/at24.c
>  
>  ATA OVER ETHERNET (AOE) DRIVER
>
Bartosz Golaszewski Sept. 23, 2019, 6:34 p.m. UTC | #2
pon., 23 wrz 2019 o 20:30 Peter Rosin <peda@axentia.se> napisał(a):
>
> On 2019-09-23 19:52, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Convert the binding document for at24 EEPROMs from txt to yaml. The
> > compatible property uses a regex pattern to address all the possible
> > combinations of "vendor,model" strings.
> >
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > ---
> >  .../devicetree/bindings/eeprom/at24.txt       |  90 +--------------
> >  .../devicetree/bindings/eeprom/at24.yaml      | 107 ++++++++++++++++++
> >  MAINTAINERS                                   |   2 +-
> >  3 files changed, 109 insertions(+), 90 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/eeprom/at24.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/eeprom/at24.txt b/Documentation/devicetree/bindings/eeprom/at24.txt
> > index 22aead844d0f..c94acbb8cb0c 100644
> > --- a/Documentation/devicetree/bindings/eeprom/at24.txt
> > +++ b/Documentation/devicetree/bindings/eeprom/at24.txt
> > @@ -1,89 +1 @@
> > -EEPROMs (I2C)
> > -
> > -Required properties:
> > -
> > -  - compatible: Must be a "<manufacturer>,<model>" pair. The following <model>
> > -                values are supported (assuming "atmel" as manufacturer):
> > -
> > -                "atmel,24c00",
> > -                "atmel,24c01",
> > -                "atmel,24cs01",
> > -                "atmel,24c02",
> > -                "atmel,24cs02",
> > -                "atmel,24mac402",
> > -                "atmel,24mac602",
> > -                "atmel,spd",
> > -                "atmel,24c04",
> > -                "atmel,24cs04",
> > -                "atmel,24c08",
> > -                "atmel,24cs08",
> > -                "atmel,24c16",
> > -                "atmel,24cs16",
> > -                "atmel,24c32",
> > -                "atmel,24cs32",
> > -                "atmel,24c64",
> > -                "atmel,24cs64",
> > -                "atmel,24c128",
> > -                "atmel,24c256",
> > -                "atmel,24c512",
> > -                "atmel,24c1024",
> > -                "atmel,24c2048",
> > -
> > -                If <manufacturer> is not "atmel", then a fallback must be used
> > -                with the same <model> and "atmel" as manufacturer.
> > -
> > -                Example:
> > -                        compatible = "microchip,24c128", "atmel,24c128";
> > -
> > -                Supported manufacturers are:
> > -
> > -                "catalyst",
> > -                "microchip",
> > -                "nxp",
> > -                "ramtron",
> > -                "renesas",
> > -                "rohm",
> > -                "st",
> > -
> > -                Some vendors use different model names for chips which are just
> > -                variants of the above. Known such exceptions are listed below:
> > -
> > -                "nxp,se97b" - the fallback is "atmel,24c02",
> > -                "renesas,r1ex24002" - the fallback is "atmel,24c02"
> > -                "renesas,r1ex24016" - the fallback is "atmel,24c16"
> > -                "renesas,r1ex24128" - the fallback is "atmel,24c128"
> > -                "rohm,br24t01" - the fallback is "atmel,24c01"
> > -
> > -  - reg: The I2C address of the EEPROM.
> > -
> > -Optional properties:
> > -
> > -  - pagesize: The length of the pagesize for writing. Please consult the
> > -              manual of your device, that value varies a lot. A wrong value
> > -              may result in data loss! If not specified, a safety value of
> > -              '1' is used which will be very slow.
> > -
> > -  - read-only: This parameterless property disables writes to the eeprom.
> > -
> > -  - size: Total eeprom size in bytes.
> > -
> > -  - no-read-rollover: This parameterless property indicates that the
> > -                      multi-address eeprom does not automatically roll over
> > -                      reads to the next slave address. Please consult the
> > -                      manual of your device.
> > -
> > -  - wp-gpios: GPIO to which the write-protect pin of the chip is connected.
> > -
> > -  - address-width: number of address bits (one of 8, 16).
> > -
> > -  - num-addresses: total number of i2c slave addresses this device takes
> > -
> > -Example:
> > -
> > -eeprom@52 {
> > -     compatible = "atmel,24c32";
> > -     reg = <0x52>;
> > -     pagesize = <32>;
> > -     wp-gpios = <&gpio1 3 0>;
> > -     num-addresses = <8>;
> > -};
> > +This file has been moved to at24.yaml.
> > diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
> > new file mode 100644
> > index 000000000000..28c8b068c8a1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
> > @@ -0,0 +1,107 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright 2019 BayLibre SAS
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/eeprom/at24.yaml#"
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > +
> > +title: I2C EEPROMs compatible with Atmel's AT24
> > +
> > +maintainers:
> > +  - Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > +
> > +properties:
> > +  compatible:
> > +    additionalItems: true
> > +    maxItems: 2
> > +    pattern: "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),24(c|cs|mac)[0-9]+$"
> > +    oneOf:
> > +      - const: nxp,se97b
> > +      - const: renesas,r1ex24002
> > +      - const: renesas,r1ex24016
> > +      - const: renesas,r1ex24128
> > +      - const: rohm,br24t01
> > +    contains:
> > +      enum:
> > +        - atmel,24c00
> > +        - atmel,24c01
> > +        - atmel,24cs01
> > +        - atmel,24c02
> > +        - atmel,24cs02
> > +        - atmel,24mac402
> > +        - atmel,24mac602
> > +        - atmel,spd
> > +        - atmel,24c04
> > +        - atmel,24cs04
> > +        - atmel,24c08
> > +        - atmel,24cs08
> > +        - atmel,24c16
> > +        - atmel,24cs16
> > +        - atmel,24c32
> > +        - atmel,24cs32
> > +        - atmel,24c64
> > +        - atmel,24cs64
> > +        - atmel,24c128
> > +        - atmel,24c256
> > +        - atmel,24c512
> > +        - atmel,24c1024
> > +        - atmel,24c2048
>
> The previous binding allows more e.g.
>
>         compatible = "nxp,spd", "atmel,spd";
>

Ugh, I was thinking about spd and then forgot it anyway. :(

> which is no longer allowed. That might be a problem? The previous binding
> also allows less e.g.
>
>         compatible = "atmel,24c00", "renesas,24mac402";
>

Right, but I'm not really sure how to express fallbacks in yaml. Any hint?

Bart

> which of course is nonsense but AFAIU now allowed.
>
> The new formal rules are therefore not "right", and it might be impossible
> to express the subtleties of this weird binding with the current spec so
> there might be little to do about it? But either way, these issues are not
> mentioned neither in the binding nor the commit message. Should they be
> mentioned?
>
> Cheers,
> Peter
>
> > +
> > +  reg:
> > +    description:
> > +      The I2C slave address of the EEPROM.
> > +    maxItems: 1
> > +
> > +  pagesize:
> > +    description:
> > +      The length of the pagesize for writing. Please consult the
> > +      manual of your device, that value varies a lot. A wrong value
> > +      may result in data loss! If not specified, a safety value of
> > +      '1' is used which will be very slow.
> > +    type: integer
> > +
> > +  read-only:
> > +    description:
> > +      This parameterless property disables writes to the eeprom.
> > +    type: boolean
> > +
> > +  size:
> > +    description:
> > +      Total eeprom size in bytes.
> > +    type: integer
> > +
> > +  no-read-rollover:
> > +    description:
> > +      This parameterless property indicates that the multi-address
> > +      eeprom does not automatically roll over reads to the next slave
> > +      address. Please consult the manual of your device.
> > +    type: boolean
> > +
> > +  wp-gpios:
> > +    description:
> > +      GPIO to which the write-protect pin of the chip is connected.
> > +    maxItems: 1
> > +
> > +  address-width:
> > +    description:
> > +      Number of address bits (one of 8, 16).
> > +    type: integer
> > +
> > +  num-addresses:
> > +    description:
> > +      Total number of i2c slave addresses this device takes.
> > +    type: integer
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +examples:
> > +  - |
> > +    eeprom@52 {
> > +        compatible = "microchip,24c32", "atmel,24c32";
> > +        reg = <0x52>;
> > +        pagesize = <32>;
> > +        wp-gpios = <&gpio1 3 0>;
> > +        num-addresses = <8>;
> > +    };
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index a400af0501c9..3c7ced686966 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -2698,7 +2698,7 @@ M:      Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >  L:   linux-i2c@vger.kernel.org
> >  T:   git git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git
> >  S:   Maintained
> > -F:   Documentation/devicetree/bindings/eeprom/at24.txt
> > +F:   Documentation/devicetree/bindings/eeprom/at24.yaml
> >  F:   drivers/misc/eeprom/at24.c
> >
> >  ATA OVER ETHERNET (AOE) DRIVER
> >
>
Peter Rosin Sept. 23, 2019, 6:39 p.m. UTC | #3
On 2019-09-23 20:34, Bartosz Golaszewski wrote:
> pon., 23 wrz 2019 o 20:30 Peter Rosin <peda@axentia.se> napisał(a):
>>
>> which is no longer allowed. That might be a problem? The previous binding
>> also allows less e.g.
>>
>>         compatible = "atmel,24c00", "renesas,24mac402";
>>
> 
> Right, but I'm not really sure how to express fallbacks in yaml. Any hint?

Absolutely no idea what-so-ever. Sorry...

Cheers,
Peter
Rob Herring Sept. 23, 2019, 8:38 p.m. UTC | #4
On Mon, Sep 23, 2019 at 12:52 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Convert the binding document for at24 EEPROMs from txt to yaml. The
> compatible property uses a regex pattern to address all the possible
> combinations of "vendor,model" strings.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  .../devicetree/bindings/eeprom/at24.txt       |  90 +--------------
>  .../devicetree/bindings/eeprom/at24.yaml      | 107 ++++++++++++++++++
>  MAINTAINERS                                   |   2 +-
>  3 files changed, 109 insertions(+), 90 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/eeprom/at24.yaml

[...]

> diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
> new file mode 100644
> index 000000000000..28c8b068c8a1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
> @@ -0,0 +1,107 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright 2019 BayLibre SAS
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/eeprom/at24.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: I2C EEPROMs compatible with Atmel's AT24
> +
> +maintainers:
> +  - Bartosz Golaszewski <bgolaszewski@baylibre.com>
> +
> +properties:
> +  compatible:

Did you run this thru 'make dt_bindings_check' and is dt-schema up to
date? I don't think it will pass and if it does I want to fix that.

> +    additionalItems: true

We pretty much never allow this.

> +    maxItems: 2

This applies to arrays...

> +    pattern: "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),24(c|cs|mac)[0-9]+$"

And this to strings which is non-sense. What you want is something like this:

minItems: 1
maxItems: 2
items:
  - pattern: "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),24(c|cs|mac)[0-9]+$"
  - pattern: "^atmel,24(c|cs|mac)[0-9]+$"

This would allow 'atmel' twice, but entries have to be unique already.
It doesn't enforce the part numbers matching though. For that, you'd
need either a bunch of these under a oneOf instead:

items:
  - pattern: "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),24c00$"
  - const: atmel,24c00

Or just add this to the above with an 'allOf':

items:
  pattern: "(c00|c01|mac402|...)$"

Note the lack of '-' under items. That means the schema applies to all entries.

> +    oneOf:
> +      - const: nxp,se97b
> +      - const: renesas,r1ex24002
> +      - const: renesas,r1ex24016
> +      - const: renesas,r1ex24128
> +      - const: rohm,br24t01

For this part, you probably want:

oneOf:
  - items:
      - const: nxp,se97b
      - const: atmel,24c02
  - items:
      ...

And for the spd cases...

> +    contains:
> +      enum:

allOf:
  - oneOf:
      # all the above stuff
  - contains:
      enum:

> +        - atmel,24c00
> +        - atmel,24c01
> +        - atmel,24cs01
> +        - atmel,24c02
> +        - atmel,24cs02
> +        - atmel,24mac402
> +        - atmel,24mac602
> +        - atmel,spd
> +        - atmel,24c04
> +        - atmel,24cs04
> +        - atmel,24c08
> +        - atmel,24cs08
> +        - atmel,24c16
> +        - atmel,24cs16
> +        - atmel,24c32
> +        - atmel,24cs32
> +        - atmel,24c64
> +        - atmel,24cs64
> +        - atmel,24c128
> +        - atmel,24c256
> +        - atmel,24c512
> +        - atmel,24c1024
> +        - atmel,24c2048
> +
> +  reg:
> +    description:
> +      The I2C slave address of the EEPROM.
> +    maxItems: 1
> +
> +  pagesize:
> +    description:
> +      The length of the pagesize for writing. Please consult the
> +      manual of your device, that value varies a lot. A wrong value
> +      may result in data loss! If not specified, a safety value of
> +      '1' is used which will be very slow.
> +    type: integer

Other than boolean, you need to reference a type in types.yaml.

Does it really vary too much to list out possible values?

> +
> +  read-only:
> +    description:
> +      This parameterless property disables writes to the eeprom.
> +    type: boolean
> +
> +  size:
> +    description:
> +      Total eeprom size in bytes.
> +    type: integer
> +
> +  no-read-rollover:
> +    description:
> +      This parameterless property indicates that the multi-address
> +      eeprom does not automatically roll over reads to the next slave
> +      address. Please consult the manual of your device.
> +    type: boolean
> +
> +  wp-gpios:
> +    description:
> +      GPIO to which the write-protect pin of the chip is connected.
> +    maxItems: 1
> +
> +  address-width:
> +    description:
> +      Number of address bits (one of 8, 16).

Sounds like a constraint...

> +    type: integer
> +
> +  num-addresses:
> +    description:
> +      Total number of i2c slave addresses this device takes.

2^32 addresses okay?

> +    type: integer
> +
> +required:
> +  - compatible
> +  - reg
> +
> +examples:
> +  - |
> +    eeprom@52 {
> +        compatible = "microchip,24c32", "atmel,24c32";
> +        reg = <0x52>;
> +        pagesize = <32>;
> +        wp-gpios = <&gpio1 3 0>;
> +        num-addresses = <8>;
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a400af0501c9..3c7ced686966 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2698,7 +2698,7 @@ M:        Bartosz Golaszewski <bgolaszewski@baylibre.com>
>  L:     linux-i2c@vger.kernel.org
>  T:     git git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git
>  S:     Maintained
> -F:     Documentation/devicetree/bindings/eeprom/at24.txt
> +F:     Documentation/devicetree/bindings/eeprom/at24.yaml
>  F:     drivers/misc/eeprom/at24.c
>
>  ATA OVER ETHERNET (AOE) DRIVER
> --
> 2.23.0
>
Bartosz Golaszewski Sept. 24, 2019, 9:20 a.m. UTC | #5
pon., 23 wrz 2019 o 22:38 Rob Herring <robh+dt@kernel.org> napisał(a):
>
> On Mon, Sep 23, 2019 at 12:52 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Convert the binding document for at24 EEPROMs from txt to yaml. The
> > compatible property uses a regex pattern to address all the possible
> > combinations of "vendor,model" strings.
> >
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > ---
> >  .../devicetree/bindings/eeprom/at24.txt       |  90 +--------------
> >  .../devicetree/bindings/eeprom/at24.yaml      | 107 ++++++++++++++++++
> >  MAINTAINERS                                   |   2 +-
> >  3 files changed, 109 insertions(+), 90 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/eeprom/at24.yaml
>
> [...]
>
> > diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
> > new file mode 100644
> > index 000000000000..28c8b068c8a1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
> > @@ -0,0 +1,107 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright 2019 BayLibre SAS
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/eeprom/at24.yaml#"
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > +
> > +title: I2C EEPROMs compatible with Atmel's AT24
> > +
> > +maintainers:
> > +  - Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > +
> > +properties:
> > +  compatible:
>
> Did you run this thru 'make dt_bindings_check' and is dt-schema up to
> date? I don't think it will pass and if it does I want to fix that.
>

I couldn't get the dt_binding_check target to work, but I ran it
through dt-doc-validate directly until it didn't complain.

> > +    additionalItems: true
>
> We pretty much never allow this.
>
> > +    maxItems: 2
>
> This applies to arrays...
>
> > +    pattern: "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),24(c|cs|mac)[0-9]+$"
>
> And this to strings which is non-sense. What you want is something like this:
>
> minItems: 1
> maxItems: 2
> items:
>   - pattern: "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),24(c|cs|mac)[0-9]+$"
>   - pattern: "^atmel,24(c|cs|mac)[0-9]+$"
>
> This would allow 'atmel' twice, but entries have to be unique already.
> It doesn't enforce the part numbers matching though. For that, you'd
> need either a bunch of these under a oneOf instead:
>
> items:
>   - pattern: "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),24c00$"
>   - const: atmel,24c00
>
> Or just add this to the above with an 'allOf':
>
> items:
>   pattern: "(c00|c01|mac402|...)$"
>

I'm lost here - what do you mean add this to the above with an
'allOf'? I can't really imagine an example for that.

> Note the lack of '-' under items. That means the schema applies to all entries.
>
> > +    oneOf:
> > +      - const: nxp,se97b
> > +      - const: renesas,r1ex24002
> > +      - const: renesas,r1ex24016
> > +      - const: renesas,r1ex24128
> > +      - const: rohm,br24t01
>
> For this part, you probably want:
>
> oneOf:
>   - items:
>       - const: nxp,se97b
>       - const: atmel,24c02
>   - items:
>       ...
>
> And for the spd cases...
>
> > +    contains:
> > +      enum:
>
> allOf:
>   - oneOf:
>       # all the above stuff
>   - contains:
>       enum:
>

I'm not sure I follow the entire thing. I'll try to prepare a v2 but I
don't really expect it to be right already.

> > +        - atmel,24c00
> > +        - atmel,24c01
> > +        - atmel,24cs01
> > +        - atmel,24c02
> > +        - atmel,24cs02
> > +        - atmel,24mac402
> > +        - atmel,24mac602
> > +        - atmel,spd
> > +        - atmel,24c04
> > +        - atmel,24cs04
> > +        - atmel,24c08
> > +        - atmel,24cs08
> > +        - atmel,24c16
> > +        - atmel,24cs16
> > +        - atmel,24c32
> > +        - atmel,24cs32
> > +        - atmel,24c64
> > +        - atmel,24cs64
> > +        - atmel,24c128
> > +        - atmel,24c256
> > +        - atmel,24c512
> > +        - atmel,24c1024
> > +        - atmel,24c2048
> > +
> > +  reg:
> > +    description:
> > +      The I2C slave address of the EEPROM.
> > +    maxItems: 1
> > +
> > +  pagesize:
> > +    description:
> > +      The length of the pagesize for writing. Please consult the
> > +      manual of your device, that value varies a lot. A wrong value
> > +      may result in data loss! If not specified, a safety value of
> > +      '1' is used which will be very slow.
> > +    type: integer
>
> Other than boolean, you need to reference a type in types.yaml.
>
> Does it really vary too much to list out possible values?
>

Nobody is using anything other than 1, 8, 16, 32 and 64, so I guess we
can limit ourselves to those for now.

> > +
> > +  read-only:
> > +    description:
> > +      This parameterless property disables writes to the eeprom.
> > +    type: boolean
> > +
> > +  size:
> > +    description:
> > +      Total eeprom size in bytes.
> > +    type: integer
> > +
> > +  no-read-rollover:
> > +    description:
> > +      This parameterless property indicates that the multi-address
> > +      eeprom does not automatically roll over reads to the next slave
> > +      address. Please consult the manual of your device.
> > +    type: boolean
> > +
> > +  wp-gpios:
> > +    description:
> > +      GPIO to which the write-protect pin of the chip is connected.
> > +    maxItems: 1
> > +
> > +  address-width:
> > +    description:
> > +      Number of address bits (one of 8, 16).
>
> Sounds like a constraint...

Right.

>
> > +    type: integer
> > +
> > +  num-addresses:
> > +    description:
> > +      Total number of i2c slave addresses this device takes.
>
> 2^32 addresses okay?
>

Nope, I'll fix it.

Thanks for the review!

Bart

> > +    type: integer
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +examples:
> > +  - |
> > +    eeprom@52 {
> > +        compatible = "microchip,24c32", "atmel,24c32";
> > +        reg = <0x52>;
> > +        pagesize = <32>;
> > +        wp-gpios = <&gpio1 3 0>;
> > +        num-addresses = <8>;
> > +    };
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index a400af0501c9..3c7ced686966 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -2698,7 +2698,7 @@ M:        Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >  L:     linux-i2c@vger.kernel.org
> >  T:     git git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git
> >  S:     Maintained
> > -F:     Documentation/devicetree/bindings/eeprom/at24.txt
> > +F:     Documentation/devicetree/bindings/eeprom/at24.yaml
> >  F:     drivers/misc/eeprom/at24.c
> >
> >  ATA OVER ETHERNET (AOE) DRIVER
> > --
> > 2.23.0
> >
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/eeprom/at24.txt b/Documentation/devicetree/bindings/eeprom/at24.txt
index 22aead844d0f..c94acbb8cb0c 100644
--- a/Documentation/devicetree/bindings/eeprom/at24.txt
+++ b/Documentation/devicetree/bindings/eeprom/at24.txt
@@ -1,89 +1 @@ 
-EEPROMs (I2C)
-
-Required properties:
-
-  - compatible: Must be a "<manufacturer>,<model>" pair. The following <model>
-                values are supported (assuming "atmel" as manufacturer):
-
-                "atmel,24c00",
-                "atmel,24c01",
-                "atmel,24cs01",
-                "atmel,24c02",
-                "atmel,24cs02",
-                "atmel,24mac402",
-                "atmel,24mac602",
-                "atmel,spd",
-                "atmel,24c04",
-                "atmel,24cs04",
-                "atmel,24c08",
-                "atmel,24cs08",
-                "atmel,24c16",
-                "atmel,24cs16",
-                "atmel,24c32",
-                "atmel,24cs32",
-                "atmel,24c64",
-                "atmel,24cs64",
-                "atmel,24c128",
-                "atmel,24c256",
-                "atmel,24c512",
-                "atmel,24c1024",
-                "atmel,24c2048",
-
-                If <manufacturer> is not "atmel", then a fallback must be used
-                with the same <model> and "atmel" as manufacturer.
-
-                Example:
-                        compatible = "microchip,24c128", "atmel,24c128";
-
-                Supported manufacturers are:
-
-                "catalyst",
-                "microchip",
-                "nxp",
-                "ramtron",
-                "renesas",
-                "rohm",
-                "st",
-
-                Some vendors use different model names for chips which are just
-                variants of the above. Known such exceptions are listed below:
-
-                "nxp,se97b" - the fallback is "atmel,24c02",
-                "renesas,r1ex24002" - the fallback is "atmel,24c02"
-                "renesas,r1ex24016" - the fallback is "atmel,24c16"
-                "renesas,r1ex24128" - the fallback is "atmel,24c128"
-                "rohm,br24t01" - the fallback is "atmel,24c01"
-
-  - reg: The I2C address of the EEPROM.
-
-Optional properties:
-
-  - pagesize: The length of the pagesize for writing. Please consult the
-              manual of your device, that value varies a lot. A wrong value
-              may result in data loss! If not specified, a safety value of
-              '1' is used which will be very slow.
-
-  - read-only: This parameterless property disables writes to the eeprom.
-
-  - size: Total eeprom size in bytes.
-
-  - no-read-rollover: This parameterless property indicates that the
-                      multi-address eeprom does not automatically roll over
-                      reads to the next slave address. Please consult the
-                      manual of your device.
-
-  - wp-gpios: GPIO to which the write-protect pin of the chip is connected.
-
-  - address-width: number of address bits (one of 8, 16).
-
-  - num-addresses: total number of i2c slave addresses this device takes
-
-Example:
-
-eeprom@52 {
-	compatible = "atmel,24c32";
-	reg = <0x52>;
-	pagesize = <32>;
-	wp-gpios = <&gpio1 3 0>;
-	num-addresses = <8>;
-};
+This file has been moved to at24.yaml.
diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
new file mode 100644
index 000000000000..28c8b068c8a1
--- /dev/null
+++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
@@ -0,0 +1,107 @@ 
+# SPDX-License-Identifier: GPL-2.0
+# Copyright 2019 BayLibre SAS
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/eeprom/at24.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: I2C EEPROMs compatible with Atmel's AT24
+
+maintainers:
+  - Bartosz Golaszewski <bgolaszewski@baylibre.com>
+
+properties:
+  compatible:
+    additionalItems: true
+    maxItems: 2
+    pattern: "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),24(c|cs|mac)[0-9]+$"
+    oneOf:
+      - const: nxp,se97b
+      - const: renesas,r1ex24002
+      - const: renesas,r1ex24016
+      - const: renesas,r1ex24128
+      - const: rohm,br24t01
+    contains:
+      enum:
+        - atmel,24c00
+        - atmel,24c01
+        - atmel,24cs01
+        - atmel,24c02
+        - atmel,24cs02
+        - atmel,24mac402
+        - atmel,24mac602
+        - atmel,spd
+        - atmel,24c04
+        - atmel,24cs04
+        - atmel,24c08
+        - atmel,24cs08
+        - atmel,24c16
+        - atmel,24cs16
+        - atmel,24c32
+        - atmel,24cs32
+        - atmel,24c64
+        - atmel,24cs64
+        - atmel,24c128
+        - atmel,24c256
+        - atmel,24c512
+        - atmel,24c1024
+        - atmel,24c2048
+
+  reg:
+    description:
+      The I2C slave address of the EEPROM.
+    maxItems: 1
+
+  pagesize:
+    description:
+      The length of the pagesize for writing. Please consult the
+      manual of your device, that value varies a lot. A wrong value
+      may result in data loss! If not specified, a safety value of
+      '1' is used which will be very slow.
+    type: integer
+
+  read-only:
+    description:
+      This parameterless property disables writes to the eeprom.
+    type: boolean
+
+  size:
+    description:
+      Total eeprom size in bytes.
+    type: integer
+
+  no-read-rollover:
+    description:
+      This parameterless property indicates that the multi-address
+      eeprom does not automatically roll over reads to the next slave
+      address. Please consult the manual of your device.
+    type: boolean
+
+  wp-gpios:
+    description:
+      GPIO to which the write-protect pin of the chip is connected.
+    maxItems: 1
+
+  address-width:
+    description:
+      Number of address bits (one of 8, 16).
+    type: integer
+
+  num-addresses:
+    description:
+      Total number of i2c slave addresses this device takes.
+    type: integer
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    eeprom@52 {
+        compatible = "microchip,24c32", "atmel,24c32";
+        reg = <0x52>;
+        pagesize = <32>;
+        wp-gpios = <&gpio1 3 0>;
+        num-addresses = <8>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index a400af0501c9..3c7ced686966 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2698,7 +2698,7 @@  M:	Bartosz Golaszewski <bgolaszewski@baylibre.com>
 L:	linux-i2c@vger.kernel.org
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git
 S:	Maintained
-F:	Documentation/devicetree/bindings/eeprom/at24.txt
+F:	Documentation/devicetree/bindings/eeprom/at24.yaml
 F:	drivers/misc/eeprom/at24.c
 
 ATA OVER ETHERNET (AOE) DRIVER