2019-09-25 22:03:14

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH] dt-bindings: at24: convert the binding document to yaml

From: Bartosz Golaszewski <[email protected]>

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 <[email protected]>
---
.../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 <[email protected]>
+
+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 <[email protected]>
L: [email protected]
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


2019-09-25 23:47:01

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: at24: convert the binding document to yaml

On Mon, Sep 23, 2019 at 12:52 PM Bartosz Golaszewski <[email protected]> wrote:
>
> From: Bartosz Golaszewski <[email protected]>
>
> 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 <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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 <[email protected]>
> L: [email protected]
> 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
>

2019-09-26 00:57:09

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: at24: convert the binding document to yaml

pon., 23 wrz 2019 o 22:38 Rob Herring <[email protected]> napisaƂ(a):
>
> On Mon, Sep 23, 2019 at 12:52 PM Bartosz Golaszewski <[email protected]> wrote:
> >
> > From: Bartosz Golaszewski <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > .../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 <[email protected]>
> > +
> > +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 <[email protected]>
> > L: [email protected]
> > 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
> >