diff mbox series

[RFC,v3,2/9] dt-bindings: board: Introduce board-id

Message ID 20240522-board-ids-v4-2-a173277987f5@quicinc.com (mailing list archive)
State New
Headers show
Series None | expand

Commit Message

Elliot Berman May 22, 2024, 11:54 p.m. UTC
Device manufcturers frequently ship multiple boards or SKUs under a
single softwre package. These software packages ship multiple devicetree
blobs and require some mechanims to pick the correct DTB for the boards
that use the software package. This patch introduces a common language
for adding board identifiers to devicetrees.

Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
 .../devicetree/bindings/board/board-id.yaml        | 71 ++++++++++++++++++++++
 1 file changed, 71 insertions(+)

Comments

Rob Herring (Arm) May 23, 2024, 1:23 a.m. UTC | #1
On Wed, 22 May 2024 16:54:23 -0700, Elliot Berman wrote:
> Device manufcturers frequently ship multiple boards or SKUs under a
> single softwre package. These software packages ship multiple devicetree
> blobs and require some mechanims to pick the correct DTB for the boards
> that use the software package. This patch introduces a common language
> for adding board identifiers to devicetrees.
> 
> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> ---
>  .../devicetree/bindings/board/board-id.yaml        | 71 ++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2.example.dtb: opp-table-0: opp-1200000000:opp-microvolt-slow:0: [915000, 900000, 925000, 925000, 910000, 935000] is too long
	from schema $id: http://devicetree.org/schemas/opp/opp-v2.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2.example.dtb: opp-table-0: opp-1200000000:opp-microvolt-fast:0: [975000, 970000, 985000, 965000, 960000, 975000] is too long
	from schema $id: http://devicetree.org/schemas/opp/opp-v2.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2.example.dtb: opp-table-0: Unevaluated properties are not allowed ('opp-1000000000', 'opp-1200000000', 'opp-shared' were unexpected)
	from schema $id: http://devicetree.org/schemas/opp/opp-v2.yaml#
compress: size (5) error for type uint32-matrix
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.example.dtb: uimage@100000: compress: b'lzma\x00' is not of type 'object', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
marvell,pad-type: size (11) error for type uint32-matrix
marvell,pad-type: size (3) error for type uint32-matrix
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb: mmc@aa0000: marvell,pad-type: b'fixed-1-8v\x00' is not of type 'array'
	from schema $id: http://devicetree.org/schemas/mmc/marvell,xenon-sdhci.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb: mmc@aa0000: marvell,pad-type: b'fixed-1-8v\x00' is not of type 'array'
	from schema $id: http://devicetree.org/schemas/mmc/marvell,xenon-sdhci.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb: mmc@aa0000: Unevaluated properties are not allowed ('marvell,pad-type' was unexpected)
	from schema $id: http://devicetree.org/schemas/mmc/marvell,xenon-sdhci.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb: mmc@aa0000: marvell,pad-type: b'fixed-1-8v\x00' is not of type 'object', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb: mmc@ab0000: marvell,pad-type: b'sd\x00' is not of type 'array'
	from schema $id: http://devicetree.org/schemas/mmc/marvell,xenon-sdhci.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb: mmc@ab0000: marvell,pad-type: b'sd\x00' is not of type 'array'
	from schema $id: http://devicetree.org/schemas/mmc/marvell,xenon-sdhci.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb: mmc@ab0000: Unevaluated properties are not allowed ('marvell,pad-type' was unexpected)
	from schema $id: http://devicetree.org/schemas/mmc/marvell,xenon-sdhci.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.example.dtb: mmc@ab0000: marvell,pad-type: b'sd\x00' is not of type 'object', 'array', 'boolean', 'null'
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/supply/sc27xx-fg.example.dtb: battery: ocv-capacity-table-0:0: [4185000, 100, 4113000, 95, 4066000, 90, 4022000, 85, 3983000, 80, 3949000, 75, 3917000, 70, 3889000, 65, 3864000, 60, 3835000, 55, 3805000, 50, 3787000, 45, 3777000, 40, 3773000, 35, 3770000, 30, 3765000, 25, 3752000, 20, 3724000, 15, 3680000, 10, 3605000, 5, 3400000, 0] is too long
	from schema $id: http://devicetree.org/schemas/power/supply/battery.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/supply/battery.example.dtb: battery: ocv-capacity-table-0:0: [4185000, 100, 4113000, 95, 4066000, 90] is too long
	from schema $id: http://devicetree.org/schemas/power/supply/battery.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/supply/battery.example.dtb: battery: ocv-capacity-table-1:0: [4200000, 100, 4185000, 95, 4113000, 90] is too long
	from schema $id: http://devicetree.org/schemas/power/supply/battery.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/supply/battery.example.dtb: battery: ocv-capacity-table-2:0: [4250000, 100, 4200000, 95, 4185000, 90] is too long
	from schema $id: http://devicetree.org/schemas/power/supply/battery.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/supply/battery.example.dtb: battery: ocv-capacity-celsius: 'anyOf' conditional failed, one must be fixed:
	[4294967286, 0, 10] is too long
	4294967286 is greater than the maximum of 2147483647
	from schema $id: http://devicetree.org/schemas/property-units.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/supply/battery.example.dtb: battery: operating-range-celsius: 'anyOf' conditional failed, one must be fixed:
	[4294967266, 50] is too long
	4294967266 is greater than the maximum of 2147483647
	from schema $id: http://devicetree.org/schemas/property-units.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/power/supply/battery.example.dtb: battery: ambient-celsius: 'anyOf' conditional failed, one must be fixed:
	[4294967291, 50] is too long
	4294967291 is greater than the maximum of 2147483647
	from schema $id: http://devicetree.org/schemas/property-units.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/afe/temperature-transducer.example.dtb: temperature-sensor-0: sense-offset-millicelsius: 'anyOf' conditional failed, one must be fixed:
	4294694146 is greater than the maximum of 2147483647
	from schema $id: http://devicetree.org/schemas/property-units.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/afe/temperature-transducer.example.dtb: temperature-sensor-1: sense-offset-millicelsius: 'anyOf' conditional failed, one must be fixed:
	4294694146 is greater than the maximum of 2147483647
	from schema $id: http://devicetree.org/schemas/property-units.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-tsa.example.dtb: tsa@ae0: tdm@0:fsl,tx-ts-routes:0: [2, 0, 24, 3, 1, 0, 5, 2] is too long
	from schema $id: http://devicetree.org/schemas/soc/fsl/cpm_qe/fsl,cpm1-tsa.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/fsl/cpm_qe/fsl,cpm1-tsa.example.dtb: tsa@ae0: tdm@0:fsl,rx-ts-routes:0: [2, 0, 24, 3, 1, 0, 5, 2] is too long
	from schema $id: http://devicetree.org/schemas/soc/fsl/cpm_qe/fsl,cpm1-tsa.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/board/board-id.example.dtb: /: 'model' is a required property
	from schema $id: http://devicetree.org/schemas/root-node.yaml#
Documentation/devicetree/bindings/board/board-id.example.dtb: /: failed to match any schema with compatible: ['example']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240522-board-ids-v4-2-a173277987f5@quicinc.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Conor Dooley May 25, 2024, 4:54 p.m. UTC | #2
On Wed, May 22, 2024 at 04:54:23PM -0700, Elliot Berman wrote:
> Device manufcturers frequently ship multiple boards or SKUs under a
> single softwre package. These software packages ship multiple devicetree
> blobs and require some mechanims to pick the correct DTB for the boards
> that use the software package. This patch introduces a common language
> for adding board identifiers to devicetrees.
> 
> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> ---
>  .../devicetree/bindings/board/board-id.yaml        | 71 ++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/board/board-id.yaml b/Documentation/devicetree/bindings/board/board-id.yaml
> new file mode 100644
> index 000000000000..894c1e310cbd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/board/board-id.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/board/board-id.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Board identifiers
> +description: |
> +  This node contains a list of identifier values for the board(s) supported by
> +  this devicetree. Identifier values are either N-tuples of integers or a
> +  string. The number of items for an N-tuple identifer is determined by the
> +  property name. String identifiers must be suffixed with "-string".
> +
> +  Every identifier in the devicetree must have a matching value from the board
> +  to be considered a valid devicetree for the board. In other words: if
> +  multiple identifiers are present in the board-id and one identifier doesn't
> +  match against the board, then the devicetree is not applicable. Note this is
> +  not the case where the the board can provide more identifiers than the
> +  devicetree describes: those additional identifers can be ignored.
> +
> +  Identifiers in the devicetree can describe multiple possible valid values,
> +  such as revision 1 and revision 2.
> +
> +maintainers:
> +  - Elliot Berman <quic_eberman@quicinc.com>
> +
> +properties:
> +  $nodename:
> +    const: '/'
> +  board-id:


Does this need to be
properties:
  $nodename:
    const: board-id
? That's the pattern I see for all top level nodes.

> +    type: object
> +    patternProperties:
> +      "^.*(?<!-string)$":

At least this regex now actually works :)

> +        $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +        description: |
> +          List of values that match boards this devicetree applies to.
> +          A bootloader checks whether any of the values in this list
> +          match against the board's value.
> +
> +          The number of items per tuple is determined by the property name,
> +          see the vendor-specific board-id bindings.
> +      "^.*-string$":
> +        $ref: /schemas/types.yaml#/definitions/string-array

Your description above doesn't match a string-array, just a single
string. That said I'm far from sold on the "thou shalt have -string"
edict. If every vendor is expected to go and define their own set of
properties (and provide their own callback in your libfdt PoC) there's
little to no reason to inflict property naming on them, AFAICT all that
is gained is a being able to share
	if (string) {
		return fdt_stringlist_contains(prop->data,
					       fdt32_to_cpu(prop->len),
					       data);
	} else {
		// exact data comparison. data_len is the size of each entry
		if (fdt32_to_cpu(prop->len) % data_len || data_len % 4)
			return -FDT_ERR_BADVALUE;

		for (int i = 0; i < fdt32_to_cpu(prop->len); i += data_len) {
			if (!memcmp(&prop->data[i], data, data_len))
				return 1;
		}

		return 0;
	}
in the libfdt PoC? I'd be expecting that a common mechanism would use
the same "callback" for boards shipped by both Qualcomm and
$other_vendor. Every vendor having different properties and only sharing
the board-id node name seems a wee bit like paying lip-service to a
common mechanism to me. What am I missing?

Thanks,
Conor.



> +        description: |
> +          List of strings that match boards this devicetree applies to.
> +          A bootloader checks whether any of the strings in this list
> +          match against the board's string representation.
> +
> +          String-based matching requires properties to be suffixed with
> +          -string so that a bootloader can match values without otherwise
> +          knowing the meaning of the identifier.
> +
> +examples:
> +  - |
> +    / {
> +      #address-cells = <1>;
> +      #size-cells = <1>;
> +      compatible = "example";
> +      board-id {
> +        // this works with any boards where:
> +        // some-hw-id = 1, other-hw-id = 1, some-id-string = "cat"
> +        // some-hw-id = 1, other-hw-id = 1, some-id-string = "kitten"
> +        // some-hw-id = 1, other-hw-id = 2, some-id-string = "cat"
> +        // some-hw-id = 1, other-hw-id = 2, some-id-string = "kitten"
> +        some-hw-id = <1>; // some-hw-id must be "1"
> +        other-hw-id = <1>, <2>; // other-hw-id must be "1" or "2"
> +        some-id-string = "cat", "kitten"; // some-id-string must be "cat" or "kitten"
> +      };
> +    };
> +
> +additionalProperties: true
> 
> -- 
> 2.34.1
Elliot Berman May 29, 2024, 3:43 p.m. UTC | #3
On Sat, May 25, 2024 at 05:54:52PM +0100, Conor Dooley wrote:
> On Wed, May 22, 2024 at 04:54:23PM -0700, Elliot Berman wrote:
> > Device manufcturers frequently ship multiple boards or SKUs under a
> > single softwre package. These software packages ship multiple devicetree
> > blobs and require some mechanims to pick the correct DTB for the boards
> > that use the software package. This patch introduces a common language
> > for adding board identifiers to devicetrees.
> > 
> > Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> > ---
> >  .../devicetree/bindings/board/board-id.yaml        | 71 ++++++++++++++++++++++
> >  1 file changed, 71 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/board/board-id.yaml b/Documentation/devicetree/bindings/board/board-id.yaml
> > new file mode 100644
> > index 000000000000..894c1e310cbd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/board/board-id.yaml
> > @@ -0,0 +1,71 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/board/board-id.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Board identifiers
> > +description: |
> > +  This node contains a list of identifier values for the board(s) supported by
> > +  this devicetree. Identifier values are either N-tuples of integers or a
> > +  string. The number of items for an N-tuple identifer is determined by the
> > +  property name. String identifiers must be suffixed with "-string".
> > +
> > +  Every identifier in the devicetree must have a matching value from the board
> > +  to be considered a valid devicetree for the board. In other words: if
> > +  multiple identifiers are present in the board-id and one identifier doesn't
> > +  match against the board, then the devicetree is not applicable. Note this is
> > +  not the case where the the board can provide more identifiers than the
> > +  devicetree describes: those additional identifers can be ignored.
> > +
> > +  Identifiers in the devicetree can describe multiple possible valid values,
> > +  such as revision 1 and revision 2.
> > +
> > +maintainers:
> > +  - Elliot Berman <quic_eberman@quicinc.com>
> > +
> > +properties:
> > +  $nodename:
> > +    const: '/'
> > +  board-id:
> 
> 
> Does this need to be
> properties:
>   $nodename:
>     const: board-id
> ? That's the pattern I see for all top level nodes.
> 
> > +    type: object
> > +    patternProperties:
> > +      "^.*(?<!-string)$":
> 
> At least this regex now actually works :)
> 
> > +        $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > +        description: |
> > +          List of values that match boards this devicetree applies to.
> > +          A bootloader checks whether any of the values in this list
> > +          match against the board's value.
> > +
> > +          The number of items per tuple is determined by the property name,
> > +          see the vendor-specific board-id bindings.
> > +      "^.*-string$":
> > +        $ref: /schemas/types.yaml#/definitions/string-array
> 
> Your description above doesn't match a string-array, just a single
> string. That said I'm far from sold on the "thou shalt have -string"
> edict. If every vendor is expected to go and define their own set of
> properties (and provide their own callback in your libfdt PoC) there's
> little to no reason to inflict property naming on them, AFAICT all that
> is gained is a being able to share
> 	if (string) {
> 		return fdt_stringlist_contains(prop->data,
> 					       fdt32_to_cpu(prop->len),
> 					       data);
> 	} else {
> 		// exact data comparison. data_len is the size of each entry
> 		if (fdt32_to_cpu(prop->len) % data_len || data_len % 4)
> 			return -FDT_ERR_BADVALUE;
> 
> 		for (int i = 0; i < fdt32_to_cpu(prop->len); i += data_len) {
> 			if (!memcmp(&prop->data[i], data, data_len))
> 				return 1;
> 		}
> 
> 		return 0;
> 	}
> in the libfdt PoC? I'd be expecting that a common mechanism would use
> the same "callback" for boards shipped by both Qualcomm and
> $other_vendor. Every vendor having different properties and only sharing
> the board-id node name seems a wee bit like paying lip-service to a
> common mechanism to me. What am I missing?

One way I thought to get the real board-id values from firmware to OS
loader is via DT itself. A firmware-provided DT provides the real
board-id values. In this case, firmware doesn't have any way to say the
board-id property is a string or a number, so I put that info in the DT
property name.

Another way I thought to get the real board-id values from firmware is
via a UEFI protocol. In that case, we could easily share whether the
value is a string or number and we can drop the "-string" suffix bit.

Thanks,
Elliot
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/board/board-id.yaml b/Documentation/devicetree/bindings/board/board-id.yaml
new file mode 100644
index 000000000000..894c1e310cbd
--- /dev/null
+++ b/Documentation/devicetree/bindings/board/board-id.yaml
@@ -0,0 +1,71 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/board/board-id.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Board identifiers
+description: |
+  This node contains a list of identifier values for the board(s) supported by
+  this devicetree. Identifier values are either N-tuples of integers or a
+  string. The number of items for an N-tuple identifer is determined by the
+  property name. String identifiers must be suffixed with "-string".
+
+  Every identifier in the devicetree must have a matching value from the board
+  to be considered a valid devicetree for the board. In other words: if
+  multiple identifiers are present in the board-id and one identifier doesn't
+  match against the board, then the devicetree is not applicable. Note this is
+  not the case where the the board can provide more identifiers than the
+  devicetree describes: those additional identifers can be ignored.
+
+  Identifiers in the devicetree can describe multiple possible valid values,
+  such as revision 1 and revision 2.
+
+maintainers:
+  - Elliot Berman <quic_eberman@quicinc.com>
+
+properties:
+  $nodename:
+    const: '/'
+  board-id:
+    type: object
+    patternProperties:
+      "^.*(?<!-string)$":
+        $ref: /schemas/types.yaml#/definitions/uint32-matrix
+        description: |
+          List of values that match boards this devicetree applies to.
+          A bootloader checks whether any of the values in this list
+          match against the board's value.
+
+          The number of items per tuple is determined by the property name,
+          see the vendor-specific board-id bindings.
+      "^.*-string$":
+        $ref: /schemas/types.yaml#/definitions/string-array
+        description: |
+          List of strings that match boards this devicetree applies to.
+          A bootloader checks whether any of the strings in this list
+          match against the board's string representation.
+
+          String-based matching requires properties to be suffixed with
+          -string so that a bootloader can match values without otherwise
+          knowing the meaning of the identifier.
+
+examples:
+  - |
+    / {
+      #address-cells = <1>;
+      #size-cells = <1>;
+      compatible = "example";
+      board-id {
+        // this works with any boards where:
+        // some-hw-id = 1, other-hw-id = 1, some-id-string = "cat"
+        // some-hw-id = 1, other-hw-id = 1, some-id-string = "kitten"
+        // some-hw-id = 1, other-hw-id = 2, some-id-string = "cat"
+        // some-hw-id = 1, other-hw-id = 2, some-id-string = "kitten"
+        some-hw-id = <1>; // some-hw-id must be "1"
+        other-hw-id = <1>, <2>; // other-hw-id must be "1" or "2"
+        some-id-string = "cat", "kitten"; // some-id-string must be "cat" or "kitten"
+      };
+    };
+
+additionalProperties: true