[PATCH RFC v3 0/9] dt-bindings: hwinfo: Introduce board-id

[PATCH RFC v3 0/9] dt-bindings: hwinfo: Introduce board-id

[Elliot Berman]

Device manufacturers frequently ship multiple boards or SKUs under a
single software package. These software packages will ship multiple
devicetree blobs and require some mechanism to pick the correct DTB for
the board the software package was deployed. Introduce a common
definition for adding board identifiers to device trees. board-id
provides a mechanism for bootloaders to select the appropriate DTB which
is vendor/OEM-agnostic.

This series is based off a talk I gave at EOSS NA 2024 [1]. There is
some further discussion about how to do devicetree selection in the
boot-architecture mailing list [2].

[1]: https://sched.co/1aBFy
[2]: https://lists.linaro.org/archives/list/boot-architecture@lists.linaro.org/thread/DZCZSOCRH5BN7YOXEL2OQKSDIY7DCW2M/

Quick summary
-------------
This series introduces a new subnode in the root:
/ {
	board-id {
		some-hw-id = <value>;
		other-hw-id = <val1>, <val2>;
	};
};

Firmware provides a mechanism to fetch the values of "some-hw-id" and
"other-hw-id" based on the property name. I'd like to leave exact
mechanism data out of the scope of this proposal to keep this proposal 
flexible because it seems architecture specific, although I think we we
should discuss possible approaches. A DTB matches if firmware can
provide a matching value for every one of the properties under
/board-id. In the above example, val1 and val2 are both valid values and
firmware only provides the one that actually describes the board. 

It's expected that devicetree's board-id don't describe all the
properties firmware could provide. For instance, a devicetree overlay
may only care about "other-hw-id" and not "some-hw-id". Thus, it need 
only mention "other-hw-id" in its board-id node.

Isn't that what the compatible property is for?
-----------------------------------------------
The compatible property can be used for board matching, but requires
bootloaders and/or firmware to maintain a database of possible strings
to match against or implement complex compatible string matching.
Compatible string matching becomes complicated when there are multiple
versions of board: the device tree selector should recognize a DTB that
cares to distinguish between v1/v2 and a DTB that doesn't make the
distinction.  An eeprom either needs to store the compatible strings
that could match against the board or the bootloader needs to have
vendor-specific decoding logic for the compatible string. Neither
increasing eeprom storage nor adding vendor-specific decoding logic is
desirable.

How is this better than Qualcomm's qcom,msm-id/qcom,board-id?
-------------------------------------------------------------
The selection process for devicetrees was Qualcomm-specific and not
useful for other devices and bootloaders that were not developed by
Qualcomm because a complex algorithm was used to implement. Board-ids
provide a matching solution that can be implemented by bootloaders
without introducing vendor-specific code. Qualcomm uses three
devicetree properties: msm-id (interchangeably: soc-id), board-id, and
pmic-id.  This does not scale well for use casese which use identifiers,
for example, to distinguish between a display panel. For a display
panel, an approach could be to add a new property: display-id, but now
bootloaders need to be updated to also read this property. We want to
avoid requiring to update bootloaders with new hardware identifiers: a
bootloader need only recognize the identifiers it can handle.

Notes about the patches
-----------------------
In my opinion, most of the patches in this series should be submitted to
libfdt and/or dtschema project. I've made them apply on the kernel tree
to be easier for other folks to pick them up and play with them. As the
patches evolve, I can send them to the appropriate projects.

Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
Changes in v3:
 - Follow new "/board-id {}" approach, which uses key-value pairs
 - Add match algorithm in libfdt and some examples to demo how the
   selection could work in tools/board-id

Changes in V2:
 - Addressed few comments related to board-id, and DDR type.
 - Link to V2:  https://lore.kernel.org/all/a930a3d6-0846-a709-8fe9-44335fec92ca@quicinc.com/#r

---
Amrit Anand (1):
      dt-bindings: arm: qcom: Update Devicetree identifiers

Elliot Berman (8):
      libfdt: board-id: Implement board-id scoring
      dt-bindings: board: Introduce board-id
      fdt-select-board: Add test tool for selecting dtbs based on board-id
      dt-bindings: board: Document board-ids for Qualcomm devices
      arm64: boot: dts: sm8650: Add board-id
      arm64: boot: dts: qcom: Use phandles for thermal_zones
      arm64: boot: dts: qcom: sm8550: Split into overlays
      tools: board-id: Add test suite

 .../devicetree/bindings/board/board-id.yaml        |  24 ++++
 .../devicetree/bindings/board/qcom,board-id.yaml   | 144 ++++++++++++++++++++
 arch/arm64/boot/dts/qcom/Makefile                  |   4 +
 arch/arm64/boot/dts/qcom/pm8010.dtsi               |  62 ++++-----
 arch/arm64/boot/dts/qcom/pm8550.dtsi               |  32 ++---
 arch/arm64/boot/dts/qcom/pm8550b.dtsi              |  36 +++--
 arch/arm64/boot/dts/qcom/pm8550ve.dtsi             |  38 +++---
 arch/arm64/boot/dts/qcom/pm8550vs.dtsi             | 128 +++++++++--------
 arch/arm64/boot/dts/qcom/pmr735d_a.dtsi            |  38 +++---
 arch/arm64/boot/dts/qcom/pmr735d_b.dtsi            |  38 +++---
 .../dts/qcom/{sm8550-mtp.dts => sm8550-mtp.dtso}   |  24 +++-
 .../dts/qcom/{sm8550-qrd.dts => sm8550-qrd.dtso}   |  22 ++-
 .../boot/dts/qcom/{sm8550.dtsi => sm8550.dts}      |  10 +-
 arch/arm64/boot/dts/qcom/sm8650-mtp.dts            |   6 +
 arch/arm64/boot/dts/qcom/sm8650-qrd.dts            |   6 +
 arch/arm64/boot/dts/qcom/sm8650.dtsi               |   2 +-
 include/dt-bindings/arm/qcom,ids.h                 |  86 ++++++++++--
 scripts/dtc/.gitignore                             |   1 +
 scripts/dtc/Makefile                               |   3 +-
 scripts/dtc/fdt-select-board.c                     | 126 +++++++++++++++++
 scripts/dtc/libfdt/fdt_ro.c                        |  76 +++++++++++
 scripts/dtc/libfdt/libfdt.h                        |  54 ++++++++
 tools/board-id/test.py                             | 151 +++++++++++++++++++++
 23 files changed, 901 insertions(+), 210 deletions(-)
---
base-commit: e8f897f4afef0031fe618a8e94127a0934896aba
change-id: 20240112-board-ids-809ff0281ee5

Best regards,
-- 
Elliot Berman <quic_eberman@quicinc.com>

[PATCH RFC v3 1/9] libfdt: board-id: Implement board-id scoring

[Elliot Berman]

The devicetree spec introduced a mechanism to match devicetree blobs to
boards using firmware-provided identifiers. Although the matching can be
implemented by DTB loaders, having a canonical implementation makes it
easier to integrate and ensure consistent behavior across ecosystems.

I've not yet investigated swig/python support for the new functions; I
would work on that before submitting the patch to libfdt.

Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
 scripts/dtc/libfdt/fdt_ro.c | 76 +++++++++++++++++++++++++++++++++++++++++++++
 scripts/dtc/libfdt/libfdt.h | 54 ++++++++++++++++++++++++++++++++
 2 files changed, 130 insertions(+)

diff --git a/scripts/dtc/libfdt/fdt_ro.c b/scripts/dtc/libfdt/fdt_ro.c
index 9f6c551a22c2..b19b17127399 100644
--- a/scripts/dtc/libfdt/fdt_ro.c
+++ b/scripts/dtc/libfdt/fdt_ro.c
@@ -857,3 +857,79 @@ int fdt_node_offset_by_compatible(const void *fdt, int startoffset,
 
 	return offset; /* error from fdt_next_node() */
 }
+
+int fdt_board_id_prop_matches(const void *fdt,
+			      const struct fdt_property *prop,
+			      fdt_get_board_id_t get_board_id_cb,
+			      void *ctx)
+{
+	int data_len;
+	const void *data;
+	const char *name;
+	int name_len;
+	int string = 0;
+
+	name = fdt_get_string(fdt, fdt32_to_cpu(prop->nameoff), &name_len);
+	if (!name)
+		return -FDT_ERR_BADOFFSET;
+
+	if (name_len > 4 && !strcmp(name + name_len - 4, "_str"))
+		string = 1;
+
+	data = get_board_id_cb(ctx, name, &data_len);
+	if (!data)
+		return -FDT_ERR_NOTFOUND;
+
+	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;
+	}
+
+	return 0;
+}
+
+int fdt_board_id_score(const void *fdt, fdt_get_board_id_t get_board_id_cb,
+		       void *ctx)
+{
+	const struct fdt_property *prop;
+	int node, property, ret, score = 0;
+	const char *name;
+
+	node = fdt_path_offset(fdt, "/board-id");
+	if (node < 0)
+		return node;
+
+	fdt_for_each_property_offset(property, fdt, node) {
+		prop = fdt_get_property_by_offset(fdt, property, NULL);
+		if (!prop)
+			return -FDT_ERR_BADOFFSET;
+
+		name = fdt_get_string(fdt, fdt32_to_cpu(prop->nameoff), NULL);
+		if (!name)
+			return -FDT_ERR_BADOFFSET;
+
+		if (!strcmp(name, "phandle") || !strcmp(name, "linux,phandle"))
+			continue;
+
+		ret = fdt_board_id_prop_matches(fdt, prop, get_board_id_cb,
+						ctx);
+		if (ret == 1)
+			score++;
+		else
+			return ret;
+	}
+
+	return score;
+}
diff --git a/scripts/dtc/libfdt/libfdt.h b/scripts/dtc/libfdt/libfdt.h
index 77ccff19911e..de1cbc339315 100644
--- a/scripts/dtc/libfdt/libfdt.h
+++ b/scripts/dtc/libfdt/libfdt.h
@@ -1230,6 +1230,60 @@ int fdt_address_cells(const void *fdt, int nodeoffset);
  */
 int fdt_size_cells(const void *fdt, int nodeoffset);
 
+/**********************************************************************/
+/* Read-only functions (board-id related)                             */
+/**********************************************************************/
+
+/**
+ * fdt_get_board_id_t - callback to retrieve the board id for the platform
+ * @ctx: Context data passed from fdt_board_id_prop_matches()
+ * @name: board id name
+ * @data: Callback sets pointer to the board id data
+ * @datalen: Callback sets length of the board id data
+ *
+ * returns:
+ *	Pointer to the board id data, NULL if the data doesn't exist
+ */
+typedef const void *(*fdt_get_board_id_t)(void *ctx, const char *name,
+					   int *datalen);
+
+/**
+ * fdt_board_id_prop_matches - check if the property matches the platform's board id
+ * @fdt: pointer to the device tree blob
+ * @prop_offset: Offset of the property to match
+ * @get_board_id_cb: Function pointer to retrieve the platform's board id
+ *
+ * returns:
+ *	1: the property matches
+ *	0: the property doesn't match
+ *	-FDT_ERR_BADMAGIC,
+ *	-FDT_ERR_BADVERSION,
+ *	-FDT_ERR_BADSTATE,
+ *	-FDT_ERR_BADSTRUCTURE,
+ *	-FDT_ERR_TRUNCATED, standard meanings
+ */
+int fdt_board_id_prop_matches(const void *fdt,
+			      const struct fdt_property *prop,
+			      fdt_get_board_id_t get_board_id_cb,
+			      void *ctx);
+
+/**
+ * fdt_board_id_score - calculate score of the fdt's board_id
+ * @fdt: pointer to the device tree blob
+ * @get_board_id_cb: Function pointer to retrieve the platform's board id
+ *
+ * returns:
+ *	>0 for the number of board-id properties that were matched
+ *	0 if there was a mismatch in any of the board-id properties
+ *	-FDT_ERR_BADMAGIC,
+ *	-FDT_ERR_BADVERSION,
+ *	-FDT_ERR_BADSTATE,
+ *	-FDT_ERR_BADSTRUCTURE,
+ *	-FDT_ERR_TRUNCATED, standard meanings
+ */
+int fdt_board_id_score(const void *fdt, fdt_get_board_id_t get_board_id_cb,
+		       void *ctx);
+
 
 /**********************************************************************/
 /* Write-in-place functions                                           */

-- 
2.34.1

Re: [PATCH RFC v3 1/9] libfdt: board-id: Implement board-id scoring

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 756 bytes --]

On Tue, May 21, 2024 at 11:37:58AM -0700, Elliot Berman wrote:
> The devicetree spec introduced a mechanism to match devicetree blobs to
> boards using firmware-provided identifiers.

Can you share a link to where the devicetree spec introduced this
mechanism? I don't recall seeing a PR to dt-schema for it nor did a
quick check of the devicetree specification repo show a PR adding it.

What am I missing?

Thanks,
Conor.

> Although the matching can be
> implemented by DTB loaders, having a canonical implementation makes it
> easier to integrate and ensure consistent behavior across ecosystems.
> 
> I've not yet investigated swig/python support for the new functions; I
> would work on that before submitting the patch to libfdt.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH RFC v3 1/9] libfdt: board-id: Implement board-id scoring

[Elliot Berman]

On Tue, May 21, 2024 at 08:28:01PM +0100, Conor Dooley wrote:
> On Tue, May 21, 2024 at 11:37:58AM -0700, Elliot Berman wrote:
> > The devicetree spec introduced a mechanism to match devicetree blobs to
> > boards using firmware-provided identifiers.
> 
> Can you share a link to where the devicetree spec introduced this
> mechanism? I don't recall seeing a PR to dt-schema for it nor did a
> quick check of the devicetree specification repo show a PR adding it.
> 
> What am I missing?

My thinking is that the next patch would probably go to dt-schema or
devicetree specification repo.

Thanks,
Elliot

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

[Elliot Berman]

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        | 24 ++++++++++++++++++++++
 1 file changed, 24 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..99514aef9718
--- /dev/null
+++ b/Documentation/devicetree/bindings/board/board-id.yaml
@@ -0,0 +1,24 @@
+# SPDX-License-Identifier: 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: Common property for board-id subnode
+
+maintainers:
+  - Elliot Berman <quic_eberman@quicinc.com>
+
+properties:
+  $nodename:
+    const: '/'
+  board-id:
+    type: object
+    patternProperties:
+      "^.*(?!_str)$":
+        $ref: /schemas/types.yaml#/definitions/uint32-matrix
+      "^.*_str$":
+        $ref: /schemas/types.yaml#/definitions/string-array
+
+additionalProperties: true

-- 
2.34.1

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

[Rob Herring (Arm)]

On Tue, 21 May 2024 11:37:59 -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        | 24 ++++++++++++++++++++++
>  1 file changed, 24 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#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240521-board-ids-v3-2-e6c71d05f4d2@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.

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2852 bytes --]

On Tue, May 21, 2024 at 11:37:59AM -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.

Okay, you've got the problem statement here, nice.

> This patch introduces a common language
> for adding board identifiers to devicetrees.

But then a completely useless remainder of the commit message.
I open this patch, see the regexes, say "wtf", look at the commit
message and there is absolutely no explanation of what these properties
are for. That's quite frankly just not good enough - even for an RFC.

> 
> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> ---
>  .../devicetree/bindings/board/board-id.yaml        | 24 ++++++++++++++++++++++
>  1 file changed, 24 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..99514aef9718
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/board/board-id.yaml
> @@ -0,0 +1,24 @@
> +# SPDX-License-Identifier: 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: Common property for board-id subnode

s/property/properties/

> +
> +maintainers:
> +  - Elliot Berman <quic_eberman@quicinc.com>
> +
> +properties:
> +  $nodename:
> +    const: '/'
> +  board-id:
> +    type: object
> +    patternProperties:
> +      "^.*(?!_str)$":

Does this regex even work? Take "foo_str" as an example - doesn't "^.*"
consume all of the string, leaving the negative lookahead with nothing
to object to? I didn't properly test this with an example and the dt
tooling, but I lazily threw it into regex101 and both the python and
emcascript versions agree with me. Did you test this?

And while I am here, no underscores in property names please. And if
"str" means string, I suggest not saving 3 characters.

> +        $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +      "^.*_str$":
> +        $ref: /schemas/types.yaml#/definitions/string-array

Why do we even need two methods? Commit message tells me nothing and
there's no description at all... Why do we need regexes here, rather
than explicitly defined properties? Your commit message should explain
the justification for that and the property descriptions (as comments if
needs be for patternProperties) should explain why this is intended to
be used.

How is anyone supposed to look at this binding and understand how it
should be used?

Utterly lost,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3176 bytes --]

On Tue, May 21, 2024 at 08:21:45PM +0100, Conor Dooley wrote:
> On Tue, May 21, 2024 at 11:37:59AM -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.
> 
> Okay, you've got the problem statement here, nice.
> 
> > This patch introduces a common language
> > for adding board identifiers to devicetrees.
> 
> But then a completely useless remainder of the commit message.
> I open this patch, see the regexes, say "wtf", look at the commit
> message and there is absolutely no explanation of what these properties
> are for. That's quite frankly just not good enough - even for an RFC.
> 
> > 
> > Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> > ---
> >  .../devicetree/bindings/board/board-id.yaml        | 24 ++++++++++++++++++++++
> >  1 file changed, 24 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..99514aef9718
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/board/board-id.yaml
> > @@ -0,0 +1,24 @@
> > +# SPDX-License-Identifier: 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: Common property for board-id subnode
> 
> s/property/properties/
> 
> > +
> > +maintainers:
> > +  - Elliot Berman <quic_eberman@quicinc.com>
> > +
> > +properties:
> > +  $nodename:
> > +    const: '/'
> > +  board-id:
> > +    type: object
> > +    patternProperties:
> > +      "^.*(?!_str)$":
> 
> Does this regex even work? Take "foo_str" as an example - doesn't "^.*"
> consume all of the string, leaving the negative lookahead with nothing
> to object to? I didn't properly test this with an example and the dt
> tooling, but I lazily threw it into regex101 and both the python and
> emcascript versions agree with me. Did you test this?
> 
> And while I am here, no underscores in property names please. And if
> "str" means string, I suggest not saving 3 characters.
> 
> > +        $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > +      "^.*_str$":
> > +        $ref: /schemas/types.yaml#/definitions/string-array
> 
> Why do we even need two methods? Commit message tells me nothing and
> there's no description at all... Why do we need regexes here, rather
> than explicitly defined properties? Your commit message should explain
> the justification for that and the property descriptions (as comments if
> needs be for patternProperties) should explain why this is intended to
> be used.
> 
> How is anyone supposed to look at this binding and understand how it
> should be used?

Also, please do not CC private mailing lists on your postings, I do not
want to get spammed by linaro's mailman :(

Thanks,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Rob Herring]

On Tue, May 21, 2024 at 2:25 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Tue, May 21, 2024 at 08:21:45PM +0100, Conor Dooley wrote:
> > On Tue, May 21, 2024 at 11:37:59AM -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.
> >
> > Okay, you've got the problem statement here, nice.
> >
> > > This patch introduces a common language
> > > for adding board identifiers to devicetrees.
> >
> > But then a completely useless remainder of the commit message.
> > I open this patch, see the regexes, say "wtf", look at the commit
> > message and there is absolutely no explanation of what these properties
> > are for. That's quite frankly just not good enough - even for an RFC.
> >
> > >
> > > Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> > > ---
> > >  .../devicetree/bindings/board/board-id.yaml        | 24 ++++++++++++++++++++++
> > >  1 file changed, 24 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..99514aef9718
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/board/board-id.yaml
> > > @@ -0,0 +1,24 @@
> > > +# SPDX-License-Identifier: 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: Common property for board-id subnode
> >
> > s/property/properties/
> >
> > > +
> > > +maintainers:
> > > +  - Elliot Berman <quic_eberman@quicinc.com>
> > > +
> > > +properties:
> > > +  $nodename:
> > > +    const: '/'
> > > +  board-id:
> > > +    type: object
> > > +    patternProperties:
> > > +      "^.*(?!_str)$":
> >
> > Does this regex even work? Take "foo_str" as an example - doesn't "^.*"
> > consume all of the string, leaving the negative lookahead with nothing
> > to object to? I didn't properly test this with an example and the dt
> > tooling, but I lazily threw it into regex101 and both the python and
> > emcascript versions agree with me. Did you test this?
> >
> > And while I am here, no underscores in property names please. And if
> > "str" means string, I suggest not saving 3 characters.
> >
> > > +        $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > +      "^.*_str$":
> > > +        $ref: /schemas/types.yaml#/definitions/string-array
> >
> > Why do we even need two methods? Commit message tells me nothing and
> > there's no description at all... Why do we need regexes here, rather
> > than explicitly defined properties? Your commit message should explain
> > the justification for that and the property descriptions (as comments if
> > needs be for patternProperties) should explain why this is intended to
> > be used.
> >
> > How is anyone supposed to look at this binding and understand how it
> > should be used?
>
> Also, please do not CC private mailing lists on your postings, I do not
> want to get spammed by linaro's mailman :(

boot-architecture is not private[0]. It is where EBBR gets discussed
amongst other things. This came up in a thread there[1].

Rob

[0] https://lists.linaro.org/mailman3/lists/boot-architecture.lists.linaro.org/
[1] https://lists.linaro.org/archives/list/boot-architecture@lists.linaro.org/thread/DZCZSOCRH5BN7YOXEL2OQKSDIY7DCW2M/

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 651 bytes --]

On Tue, May 21, 2024 at 04:32:16PM -0500, Rob Herring wrote:

> boot-architecture is not private[0]. It is where EBBR gets discussed
> amongst other things. This came up in a thread there[1].

I dunno man, in my book any mailing list that rejects non-member mails
is private. Or outright broken, take your pick :)

I don't care if a private list's subscribers miss part of the discussion,
but it's list owner probably should, whoever that may be.

> [0] https://lists.linaro.org/mailman3/lists/boot-architecture.lists.linaro.org/
> [1] https://lists.linaro.org/archives/list/boot-architecture@lists.linaro.org/thread/DZCZSOCRH5BN7YOXEL2OQKSDIY7DCW2M/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Elliot Berman]

Hi Conor,

Thanks for taking the time to look at the patch.

On Tue, May 21, 2024 at 08:21:45PM +0100, Conor Dooley wrote:
> On Tue, May 21, 2024 at 11:37:59AM -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.
> 
> Okay, you've got the problem statement here, nice.
> 
> > This patch introduces a common language
> > for adding board identifiers to devicetrees.
> 
> But then a completely useless remainder of the commit message.
> I open this patch, see the regexes, say "wtf", look at the commit
> message and there is absolutely no explanation of what these properties
> are for. That's quite frankly just not good enough - even for an RFC.
> 

Understood, I've been trying to walk the line of getting the idea across
to have conversation about the board-ids, while not getting into too
much of the weeds. I was hoping the example and the matching code in the
first patch would get enough of the idea across, but I totally
empathize that might not be enough. I'll reply here shortly with a
version of this patch which adds more details.

> > 
> > Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> > ---
> >  .../devicetree/bindings/board/board-id.yaml        | 24 ++++++++++++++++++++++
> >  1 file changed, 24 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..99514aef9718
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/board/board-id.yaml
> > @@ -0,0 +1,24 @@
> > +# SPDX-License-Identifier: 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: Common property for board-id subnode
> 
> s/property/properties/
> 
> > +
> > +maintainers:
> > +  - Elliot Berman <quic_eberman@quicinc.com>
> > +
> > +properties:
> > +  $nodename:
> > +    const: '/'
> > +  board-id:
> > +    type: object
> > +    patternProperties:
> > +      "^.*(?!_str)$":
> 
> Does this regex even work? Take "foo_str" as an example - doesn't "^.*"
> consume all of the string, leaving the negative lookahead with nothing
> to object to? I didn't properly test this with an example and the dt
> tooling, but I lazily threw it into regex101 and both the python and
> emcascript versions agree with me. Did you test this?

Right, it should be a lookbehind, not a lookahead.

> 
> And while I am here, no underscores in property names please. And if
> "str" means string, I suggest not saving 3 characters.
> 
> > +        $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > +      "^.*_str$":
> > +        $ref: /schemas/types.yaml#/definitions/string-array
> 
> Why do we even need two methods? Commit message tells me nothing and
> there's no description at all... Why do we need regexes here, rather
> than explicitly defined properties? Your commit message should explain
> the justification for that and the property descriptions (as comments if
> needs be for patternProperties) should explain why this is intended to
> be used.
> 
> How is anyone supposed to look at this binding and understand how it
> should be used?

I was thinking that firmware may only provide the data without being
able to provide the context whether the value is a number or a string.
I think this is posisble if firmware provides the device's board
identifier in the format of a DT itself. It seems natural to me in the
EBBR flow. There is example of this in example in patches 3
(fdt-select-board) and 9 (the test suite). DTB doesn't inherently
provide instruction on how to interpret a property's value, so I created
a rule that strings have to be suffixed with "-string".

One other note -- I (QCOM) don't currently have a need for board-ids to
be strings. I thought it was likely that someone might want that though.

Thanks,
Elliot

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

[Elliot Berman]

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:
+    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

-- 
2.34.1

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

[Rob Herring (Arm)]

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.

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 5393 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Elliot Berman]

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

[PATCH RFC v3 3/9] fdt-select-board: Add test tool for selecting dtbs based on board-id

[Elliot Berman]

Introduce a tool which scores devicetrees based on their board-id and a
supplied reference devicetree. This mechanism would be most similar to
an proposal to EBBR where firmware provides a reference devicetree which
contains the actual board identifier values, and an OS loader can
choose to replace (or overlay) the firmware-provided devicetree.

Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
 scripts/dtc/.gitignore         |   1 +
 scripts/dtc/Makefile           |   3 +-
 scripts/dtc/fdt-select-board.c | 126 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 129 insertions(+), 1 deletion(-)

diff --git a/scripts/dtc/.gitignore b/scripts/dtc/.gitignore
index e0b5c1d2464a..7f6d5202c0ba 100644
--- a/scripts/dtc/.gitignore
+++ b/scripts/dtc/.gitignore
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
 /dtc
 /fdtoverlay
+/fdt-select-board
diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
index 4d32b9497da9..a331f07091b3 100644
--- a/scripts/dtc/Makefile
+++ b/scripts/dtc/Makefile
@@ -2,7 +2,7 @@
 # scripts/dtc makefile
 
 # *** Also keep .gitignore in sync when changing ***
-hostprogs-always-$(CONFIG_DTC)		+= dtc fdtoverlay
+hostprogs-always-$(CONFIG_DTC)		+= dtc fdtoverlay fdt-select-board
 hostprogs-always-$(CHECK_DT_BINDING)	+= dtc
 
 dtc-objs	:= dtc.o flattree.o fstree.o data.o livetree.o treesource.o \
@@ -14,6 +14,7 @@ dtc-objs	+= dtc-lexer.lex.o dtc-parser.tab.o
 libfdt-objs	:= fdt.o fdt_ro.o fdt_wip.o fdt_sw.o fdt_rw.o fdt_strerror.o fdt_empty_tree.o fdt_addresses.o fdt_overlay.o
 libfdt		= $(addprefix libfdt/,$(libfdt-objs))
 fdtoverlay-objs	:= $(libfdt) fdtoverlay.o util.o
+fdt-select-board-objs := $(libfdt) fdt-select-board.o util.o
 
 # Source files need to get at the userspace version of libfdt_env.h to compile
 HOST_EXTRACFLAGS += -I $(srctree)/$(src)/libfdt
diff --git a/scripts/dtc/fdt-select-board.c b/scripts/dtc/fdt-select-board.c
new file mode 100644
index 000000000000..a7f3dc715ed1
--- /dev/null
+++ b/scripts/dtc/fdt-select-board.c
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+#include <ctype.h>
+#include <getopt.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <inttypes.h>
+
+#include <libfdt.h>
+
+#include "util.h"
+
+static const char usage_synopsis[] =
+	"find the best matching device tree from a reference board\n"
+	"	fdt-select-board <options> [<candidate.dtb>]\n"
+	"\n";
+static const char usage_short_opts[] = "r:av" USAGE_COMMON_SHORT_OPTS;
+static const struct option usage_long_opts[] = {
+	{"reference", required_argument, NULL, 'r'},
+	{"verbose",	    no_argument, NULL, 'v'},
+	{"all",		    no_argument, NULL, 'a'},
+	USAGE_COMMON_LONG_OPTS
+};
+
+static const char * const usage_opts_help[] = {
+	"Reference DTB",
+	"Verbose messages",
+	"List all matches",
+	USAGE_COMMON_OPTS_HELP
+};
+
+int verbose = 0;
+
+struct context {
+	const void *fdt;
+	int node;
+};
+
+static const void *get_board_id(void *ctx, const char *name, int *datalen)
+{
+	struct context *c = ctx;
+
+	return fdt_getprop(c->fdt, c->node, name, datalen);
+}
+
+int main(int argc, char *argv[])
+{
+	int opt;
+	char *input_filename = NULL;
+	const void *fdt;
+	const void *ref;
+	int ref_node;
+	int max_score = -1;
+	const char *best_match = NULL;
+	struct context ctx;
+	int all = 0;
+
+	while ((opt = util_getopt_long()) != EOF) {
+		switch (opt) {
+		case_USAGE_COMMON_FLAGS
+
+		case 'a':
+			all = 1;
+			break;
+		case 'r':
+			input_filename = optarg;
+			break;
+		case 'v':
+			verbose = 1;
+			break;
+		}
+	}
+
+	if (!input_filename)
+		usage("missing reference file");
+
+	argv += optind;
+	argc -= optind;
+
+	if (argc <= 0)
+		usage("missing candidate dtbs");
+
+	ref = utilfdt_read(input_filename, NULL);
+	if (!ref) {
+		fprintf(stderr, "failed to read reference %s\n", input_filename);
+		return -1;
+	}
+
+	ref_node = fdt_path_offset(ref, "/board-id");
+	if (ref_node < 0) {
+		fprintf(stderr, "reference blob doesn't have a board-id\n");
+		return -1;
+	}
+
+	ctx.fdt = ref;
+	ctx.node = ref_node;
+
+	for (; argc > 0; --argc, ++argv) {
+		int score;
+
+		fdt = utilfdt_read(*argv, NULL);
+		if (!fdt) {
+			fprintf(stderr, "failed to read %s\n", *argv);
+			return -1;
+		}
+
+		score = fdt_board_id_score(fdt, get_board_id, &ctx);
+		if (verbose || (score > 0 && all))
+			printf("%s: %d\n", *argv, score);
+
+		if (score > max_score) {
+			max_score = score;
+			best_match = *argv;
+		}
+
+		free(fdt);
+	}
+
+	if (best_match && !all)
+		printf("%s\n", best_match);
+
+	return 0;
+}

-- 
2.34.1

[PATCH RFC v3 4/9] dt-bindings: arm: qcom: Update Devicetree identifiers

[Elliot Berman]

From: Amrit Anand <quic_amrianan@quicinc.com>

Update existing documentation for qcom,msm-id (interchangeably:
qcom,soc-id) and qcom,board-id. Add support for qcom,pmic-id, qcom,oem-id
to support multi-DTB selection on Qualcomm's boards.

"qcom,soc-id", "qcom,board-id" and "qcom,pmic-id" are tuples of two 32-bit
values. The "qcom,oem-id" is a tuple of one 32-bit value.
Introduce macros to help generate SOC, board, PMIC and OEM identifiers.
QCOM_SOC_ID and QCOM_SOC_REVISION can be used to generate qcom,msm-id.
QCOM_BOARD_ID and QCOM_BOARD_SUBTYPE can be used to generate qcom,board-id.
QCOM_PMIC_SID and QCOM_PMIC_MODEL can be used to generate qcom,pmic-id.
QCOM_OEM_ID can be used to generate qcom,oem-id.

Add entries for different types of SoC, boards, DDR type, Boot device
type which are currently used by Qualcomm based bootloader.

Signed-off-by: Amrit Anand <quic_amrianan@quicinc.com>
Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
 include/dt-bindings/arm/qcom,ids.h | 86 ++++++++++++++++++++++++++++++++++----
 1 file changed, 77 insertions(+), 9 deletions(-)

diff --git a/include/dt-bindings/arm/qcom,ids.h b/include/dt-bindings/arm/qcom,ids.h
index 51e0f6059410..0cb521f0c0e7 100644
--- a/include/dt-bindings/arm/qcom,ids.h
+++ b/include/dt-bindings/arm/qcom,ids.h
@@ -8,9 +8,14 @@
 #define _DT_BINDINGS_ARM_QCOM_IDS_H
 
 /*
- * The MSM chipset and hardware revision used by Qualcomm bootloaders, DTS for
- * older chipsets (qcom,msm-id) and in socinfo driver:
+ * The MSM chipset ID (soc-id) used by Qualcomm bootloaders,
+ * and in socinfo driver:
+ * where, "a" indicates Qualcomm supported chipsets, example MSM8260, MSM8660 etc
  */
+
+#define QCOM_SOC_ID(a)  ((QCOM_ID_##a) && 0xffff)
+
+
 #define QCOM_ID_MSM8260			70
 #define QCOM_ID_MSM8660			71
 #define QCOM_ID_APQ8060			86
@@ -267,16 +272,79 @@
 #define QCOM_ID_IPQ5302			595
 #define QCOM_ID_IPQ5300			624
 
+ /* The SOC revision used by Qualcomm bootloaders (soc-revision) */
+
+#define QCOM_SOC_REVISION(a)		(a & 0xff)
+
 /*
- * The board type and revision information, used by Qualcomm bootloaders and
- * DTS for older chipsets (qcom,board-id):
+ * The board type and revision information (board-id), used by Qualcomm bootloaders
+ * where, "a" indicates board type which can be either MTP, QRD etc
  */
+
 #define QCOM_BOARD_ID(a, major, minor) \
-	(((major & 0xff) << 16) | ((minor & 0xff) << 8) | QCOM_BOARD_ID_##a)
+	(((major & 0xff) << 16) | ((minor & 0xff) << 8) | ((QCOM_BOARD_ID_##a) & 0xff))
+
+#define QCOM_BOARD_ID_MTP		0x8
+#define QCOM_BOARD_ID_LIQUID		0x9
+#define QCOM_BOARD_ID_DRAGONBOARD	0xA
+#define QCOM_BOARD_ID_QRD		0x11
+#define QCOM_BOARD_ID_ADP		0x19
+#define QCOM_BOARD_ID_HDK		0x1F
+#define QCOM_BOARD_ID_ATP		0x21
+#define QCOM_BOARD_ID_IDP		0x22
+#define QCOM_BOARD_ID_SBC		0x24
+#define QCOM_BOARD_ID_QXR		0x26
+#define QCOM_BOARD_ID_X100		0x26
+#define QCOM_BOARD_ID_CRD		0x28
+
+/*
+ * The platform subtype is used by Qualcomm bootloaders and
+ * DTS (board-subtype)
+ * where, "a" indicates boot device type, it can be EMMC,
+ * UFS, NAND or OTHER (which can be used for default).
+ * "b" indicates DDR type which can be 128MB, 256MB,
+ * 512MB, 1024MB, 2048MB, 3072MB, 4096MB or ANY 
+ * (which can be used for default).
+ */
+#define QCOM_BOARD_SUBTYPE(a, b, SUBTYPE) \
+	(((QCOM_BOARD_BOOT_##a & 0xf) << 16) | ((QCOM_BOARD_DDRTYPE_##b & 0x7) << 8) | \
+	(SUBTYPE & 0xff))
+
+/* Board DDR Type where each value indicates higher limit */
+#define QCOM_BOARD_DDRTYPE_ANY		0x0
+#define QCOM_BOARD_DDRTYPE_128M		0x1
+#define QCOM_BOARD_DDRTYPE_256M		0x2
+#define QCOM_BOARD_DDRTYPE_512M		0x3
+#define QCOM_BOARD_DDRTYPE_1024M	0x4
+#define QCOM_BOARD_DDRTYPE_2048M	0x5
+#define QCOM_BOARD_DDRTYPE_3072M	0x6
+#define QCOM_BOARD_DDRTYPE_4096M	0x7
 
-#define QCOM_BOARD_ID_MTP			8
-#define QCOM_BOARD_ID_DRAGONBOARD		10
-#define QCOM_BOARD_ID_QRD			11
-#define QCOM_BOARD_ID_SBC			24
+/* Board Boot Device Type */
+#define QCOM_BOARD_BOOT_EMMC		0x0
+#define QCOM_BOARD_BOOT_UFS		0x1
+#define QCOM_BOARD_BOOT_NAND		0x2
+#define QCOM_BOARD_BOOT_OTHER		0x3
+
+/*
+ * The PMIC slave id is used by Qualcomm bootloaders to
+ * indicates which PMIC is attached (pmic-sid)
+ */
+
+#define QCOM_PMIC_SID(a)		(a & 0xff)
+
+/*
+ * The PMIC ID is used by Qualcomm bootloaders to describe the ID
+ * of PMIC attached to bus described by SID (pmic-model)
+ */
+
+#define QCOM_PMIC_MODEL(ID, major, minor) \
+	(((major & 0xff) << 16) | ((minor & 0xff) << 8) | (ID & 0xff))
+
+/*
+ * The OEM ID consists of 32 bit value to support OEM boards where they
+ * have slight differences on top of Qualcomm's standard boards
+ */
+#define QCOM_OEM_ID(a)		(a & 0xffffffff)
 
 #endif /* _DT_BINDINGS_ARM_QCOM_IDS_H */

-- 
2.34.1

Re: [PATCH RFC v3 4/9] dt-bindings: arm: qcom: Update Devicetree identifiers

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 416 bytes --]

On Tue, May 21, 2024 at 11:38:01AM -0700, Elliot Berman wrote:
>  #define QCOM_BOARD_ID(a, major, minor) \
> -	(((major & 0xff) << 16) | ((minor & 0xff) << 8) | QCOM_BOARD_ID_##a)
> +	(((major & 0xff) << 16) | ((minor & 0xff) << 8) | ((QCOM_BOARD_ID_##a) & 0xff))

I assume there's no devices that have a >8 bit QCOM_BOARD_ID that would
end up with a different value in their dtb due to this change?

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH RFC v3 4/9] dt-bindings: arm: qcom: Update Devicetree identifiers

[Elliot Berman]

On Sat, May 25, 2024 at 06:21:32PM +0100, Conor Dooley wrote:
> On Tue, May 21, 2024 at 11:38:01AM -0700, Elliot Berman wrote:
> >  #define QCOM_BOARD_ID(a, major, minor) \
> > -	(((major & 0xff) << 16) | ((minor & 0xff) << 8) | QCOM_BOARD_ID_##a)
> > +	(((major & 0xff) << 16) | ((minor & 0xff) << 8) | ((QCOM_BOARD_ID_##a) & 0xff))
> 
> I assume there's no devices that have a >8 bit QCOM_BOARD_ID that would
> end up with a different value in their dtb due to this change?

That's correct.

[PATCH RFC v3 5/9] dt-bindings: board: Document board-ids for Qualcomm devices

[Elliot Berman]

Document board identifiers for devices from Qualcomm Technologies, Inc.
These platforms are described with two mechanisms: the hardware SoC
registers and the "CDT" which is in a RO storage.

The hardware SoC registers describe both the SoC (e.g. SM8650, SC7180)
as well as revision. Add qcom,soc to describe only the SoC itself and
qcom,soc-version when the devicetree only works with a certain revision.

The CDT describes all other information about the board/platform.
Besides the platform type (e.g. MTP, ADP, CRD), there are 3 further
levels of versioning as well as additional fields to describe the PMIC
and boot storage device attached. The 3 levels of versioning are a
subtype, major, and minor version of the platform. Support describing
just the platform type (qcom,platform), the platform type and subtype
(qcom,platform-type), and all 4 numbers (qcom,platform-version).

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

diff --git a/Documentation/devicetree/bindings/board/qcom,board-id.yaml b/Documentation/devicetree/bindings/board/qcom,board-id.yaml
new file mode 100644
index 000000000000..53ba7acab4c3
--- /dev/null
+++ b/Documentation/devicetree/bindings/board/qcom,board-id.yaml
@@ -0,0 +1,144 @@
+# SPDX-License-Identifier: BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/board/qcom,board-id.yaml
+$schema: http://devicetree.org/meta-schemas/core.yaml
+
+title: Board identifiers for devices from Qualcomm Technologies, Inc.
+description: Board identifiers for devices from Qualcomm Technologies, Inc.
+
+maintainers:
+  - Elliot Berman <quic_eberman@quicinc.com>
+
+properties:
+  $nodename:
+    const: 'board-id'
+
+  qcom,soc:
+    description:
+      List of Qualcomm SoCs this devicetree is applicable to.
+
+  qcom,soc-version:
+    items:
+      items:
+        - description: Qualcomm SoC identifier
+        - description: SoC version
+
+  qcom,platform:
+    description:
+      List of Qualcomm platforms this devicetree is applicable to.
+
+  qcom,platform-type:
+    items:
+      items:
+        - description: Qualcomm platform type identifier
+        - description: Qualcomm platform subtype
+
+  qcom,platform-version:
+    items:
+      items:
+        - description: Qualcomm platform type identifier
+        - description: Qualcomm platform subtype
+        - description: Qualcomm platform major and minor version.
+
+  qcom,boot-device:
+    description:
+      Boot device type
+
+  qcom,pmic:
+    description:
+      List of Qualcomm PMIC attaches
+
+  qcom,pmic-id:
+    items:
+      items:
+        - description: Qualcomm PMIC identifier
+        - description: Qualcomm PMIC revision
+
+allOf:
+  # either describe soc or soc-version; it's confusing to have both
+  - if:
+      properties:
+        qcom,soc: true
+    then:
+      properties:
+        qcom,soc-version: false
+  - if:
+      properties:
+        qcom,soc-version: true
+    then:
+      properties:
+        qcom,soc: false
+
+  # describe one of platform, platform-type, or platform-version; it's confusing to have multiple
+  - if:
+    properties:
+      qcom,platform: true
+    then:
+      properties:
+        qcom,platform-type: false
+        qcom,platform-version: false
+  - if:
+    properties:
+      qcom,platform-type: true
+    then:
+      properties:
+        qcom,platform: false
+        qcom,platform-version: false
+  - if:
+    properties:
+      qcom,platform: true
+    then:
+      properties:
+        qcom,platform: false
+        qcom,platform-type: false
+
+  # either describe pmic or pmic-id; it's confusing to have both
+  - if:
+    properties:
+      qcom,pmic: true
+    then:
+      properties:
+        qcom,pmic-id: false
+  - if:
+    properties:
+      qcom,pmic-id: true
+    then:
+      properties:
+        qcom,pmic: false
+
+additionalProperties: true
+
+examples:
+  - |
+    #include <dt-bindings/arm/qcom,ids.h>
+    / {
+      compatible = "qcom,sm8650";
+      board-id {
+        qcom,soc = <QCOM_ID_SM8650>;
+        qcom,platform = <QCOM_BOARD_ID_MTP>;
+      };
+    };
+
+  - |
+    #include <dt-bindings/arm/qcom,ids.h>
+    / {
+      compatible = "qcom,sm8650";
+      board-id {
+        qcom,soc-version = <QCOM_ID_SM8650 QCOM_SOC_REVISION(1)>,
+                           <QCOM_ID_SM8650 QCOM_SOC_REVISION(2)>;
+        qcom,platform-type = <QCOM_BOARD_ID_MTP 0>, <QCOM_BOARD_ID_MTP 1>;
+      };
+    };
+
+  - |
+    #include <dt-bindings/arm/qcom,ids.h>
+    / {
+      compatible = "qcom,sm8650";
+      board-id {
+        qcom,soc = <QCOM_ID_SM8650>;
+        qcom,platform-version = <QCOM_BOARD_ID(MTP, 0, 1, 0)>,
+                                <QCOM_BOARD_ID(MTP, 0, 1, 1)>;
+        qcom,boot-device = <QCOM_BOARD_BOOT_UFS>;
+      };
+    };

-- 
2.34.1

Re: [PATCH RFC v3 5/9] dt-bindings: board: Document board-ids for Qualcomm devices

[Rob Herring (Arm)]

On Tue, 21 May 2024 11:38:02 -0700, Elliot Berman wrote:
> Document board identifiers for devices from Qualcomm Technologies, Inc.
> These platforms are described with two mechanisms: the hardware SoC
> registers and the "CDT" which is in a RO storage.
> 
> The hardware SoC registers describe both the SoC (e.g. SM8650, SC7180)
> as well as revision. Add qcom,soc to describe only the SoC itself and
> qcom,soc-version when the devicetree only works with a certain revision.
> 
> The CDT describes all other information about the board/platform.
> Besides the platform type (e.g. MTP, ADP, CRD), there are 3 further
> levels of versioning as well as additional fields to describe the PMIC
> and boot storage device attached. The 3 levels of versioning are a
> subtype, major, and minor version of the platform. Support describing
> just the platform type (qcom,platform), the platform type and subtype
> (qcom,platform-type), and all 4 numbers (qcom,platform-version).
> 
> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> ---
>  .../devicetree/bindings/board/qcom,board-id.yaml   | 144 +++++++++++++++++++++
>  1 file changed, 144 insertions(+)
> 

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

yamllint warnings/errors:
./Documentation/devicetree/bindings/board/qcom,board-id.yaml:15:12: [error] string value is redundantly quoted with any quotes (quoted-strings)
./Documentation/devicetree/bindings/board/qcom,board-id.yaml:74:8: [error] empty value in block mapping (empty-values)
./Documentation/devicetree/bindings/board/qcom,board-id.yaml:81:8: [error] empty value in block mapping (empty-values)
./Documentation/devicetree/bindings/board/qcom,board-id.yaml:88:8: [error] empty value in block mapping (empty-values)
./Documentation/devicetree/bindings/board/qcom,board-id.yaml:97:8: [error] empty value in block mapping (empty-values)
./Documentation/devicetree/bindings/board/qcom,board-id.yaml:103:8: [error] empty value in block mapping (empty-values)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/board/qcom,board-id.yaml: allOf:2:if: None is not of type 'object', 'boolean'
	from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/board/qcom,board-id.yaml: allOf:3:if: None is not of type 'object', 'boolean'
	from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/board/qcom,board-id.yaml: allOf:4:if: None is not of type 'object', 'boolean'
	from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/board/qcom,board-id.yaml: allOf:5:if: None is not of type 'object', 'boolean'
	from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/board/qcom,board-id.yaml: allOf:6:if: None is not of type 'object', 'boolean'
	from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/board/qcom,board-id.yaml: $id: 'http://devicetree.org/schemas/board/qcom,board-id.yaml' does not match 'http://devicetree.org/schemas/.*\\.yaml#'
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/board/qcom,board-id.yaml: $schema: 'http://devicetree.org/meta-schemas/core.yaml' is not one of ['http://devicetree.org/meta-schemas/core.yaml#', 'http://devicetree.org/meta-schemas/base.yaml#']
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/board/qcom,board-id.yaml: ignoring, error in schema: allOf: 2: if
Documentation/devicetree/bindings/board/qcom,board-id.example.dts:26:56: error: macro "QCOM_BOARD_ID" passed 4 arguments, but takes just 3
   26 |     qcom,platform-version = <QCOM_BOARD_ID(MTP, 0, 1, 0)>,
      |                                                        ^
In file included from Documentation/devicetree/bindings/board/qcom,board-id.example.dts:4:
./scripts/dtc/include-prefixes/dt-bindings/arm/qcom,ids.h:279: note: macro "QCOM_BOARD_ID" defined here
  279 | #define QCOM_BOARD_ID(a, major, minor) \
      | 
Documentation/devicetree/bindings/board/qcom,board-id.example.dts:27:56: error: macro "QCOM_BOARD_ID" passed 4 arguments, but takes just 3
   27 |                             <QCOM_BOARD_ID(MTP, 0, 1, 1)>;
      |                                                        ^
./scripts/dtc/include-prefixes/dt-bindings/arm/qcom,ids.h:279: note: macro "QCOM_BOARD_ID" defined here
  279 | #define QCOM_BOARD_ID(a, major, minor) \
      | 
make[2]: *** [scripts/Makefile.lib:427: Documentation/devicetree/bindings/board/qcom,board-id.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1430: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240521-board-ids-v3-5-e6c71d05f4d2@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.

Re: [PATCH RFC v3 5/9] dt-bindings: board: Document board-ids for Qualcomm devices

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 5962 bytes --]

On Tue, May 21, 2024 at 11:38:02AM -0700, Elliot Berman wrote:
> Document board identifiers for devices from Qualcomm Technologies, Inc.
> These platforms are described with two mechanisms: the hardware SoC
> registers and the "CDT" which is in a RO storage.
> 
> The hardware SoC registers describe both the SoC (e.g. SM8650, SC7180)
> as well as revision. Add qcom,soc to describe only the SoC itself and
> qcom,soc-version when the devicetree only works with a certain revision.
> 
> The CDT describes all other information about the board/platform.
> Besides the platform type (e.g. MTP, ADP, CRD), there are 3 further
> levels of versioning as well as additional fields to describe the PMIC
> and boot storage device attached. The 3 levels of versioning are a
> subtype, major, and minor version of the platform. Support describing
> just the platform type (qcom,platform), the platform type and subtype
> (qcom,platform-type), and all 4 numbers (qcom,platform-version).
> 
> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> ---
>  .../devicetree/bindings/board/qcom,board-id.yaml   | 144 +++++++++++++++++++++
>  1 file changed, 144 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/board/qcom,board-id.yaml b/Documentation/devicetree/bindings/board/qcom,board-id.yaml
> new file mode 100644
> index 000000000000..53ba7acab4c3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/board/qcom,board-id.yaml
> @@ -0,0 +1,144 @@
> +# SPDX-License-Identifier: BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/board/qcom,board-id.yaml
> +$schema: http://devicetree.org/meta-schemas/core.yaml
> +
> +title: Board identifiers for devices from Qualcomm Technologies, Inc.
> +description: Board identifiers for devices from Qualcomm Technologies, Inc.
> +
> +maintainers:
> +  - Elliot Berman <quic_eberman@quicinc.com>
> +
> +properties:
> +  $nodename:
> +    const: 'board-id'
> +
> +  qcom,soc:
> +    description:
> +      List of Qualcomm SoCs this devicetree is applicable to.
> +
> +  qcom,soc-version:
> +    items:
> +      items:
> +        - description: Qualcomm SoC identifier
> +        - description: SoC version
> +
> +  qcom,platform:
> +    description:
> +      List of Qualcomm platforms this devicetree is applicable to.
> +
> +  qcom,platform-type:
> +    items:
> +      items:
> +        - description: Qualcomm platform type identifier
> +        - description: Qualcomm platform subtype
> +
> +  qcom,platform-version:
> +    items:
> +      items:
> +        - description: Qualcomm platform type identifier
> +        - description: Qualcomm platform subtype
> +        - description: Qualcomm platform major and minor version.
> +
> +  qcom,boot-device:
> +    description:
> +      Boot device type
> +
> +  qcom,pmic:
> +    description:
> +      List of Qualcomm PMIC attaches
> +
> +  qcom,pmic-id:
> +    items:
> +      items:
> +        - description: Qualcomm PMIC identifier
> +        - description: Qualcomm PMIC revision
> +
> +allOf:
> +  # either describe soc or soc-version; it's confusing to have both

Why not just use the one that has the most information and discard the
others? If your dtb picker for this platform doesn't care about the soc
version, then just don't look at that cell?

Likewise for platform and PMIC, why can't you ignore the cells you don't
care about, rather than having a new property for each variant? Nothing
in this patch explains why multiple variants are required rather than
just dealing with the most informational.

Thanks,
Conor.

> +  - if:
> +      properties:
> +        qcom,soc: true
> +    then:
> +      properties:
> +        qcom,soc-version: false
> +  - if:
> +      properties:
> +        qcom,soc-version: true
> +    then:
> +      properties:
> +        qcom,soc: false
> +
> +  # describe one of platform, platform-type, or platform-version; it's confusing to have multiple
> +  - if:
> +    properties:
> +      qcom,platform: true
> +    then:
> +      properties:
> +        qcom,platform-type: false
> +        qcom,platform-version: false
> +  - if:
> +    properties:
> +      qcom,platform-type: true
> +    then:
> +      properties:
> +        qcom,platform: false
> +        qcom,platform-version: false
> +  - if:
> +    properties:
> +      qcom,platform: true
> +    then:
> +      properties:
> +        qcom,platform: false
> +        qcom,platform-type: false
> +
> +  # either describe pmic or pmic-id; it's confusing to have both
> +  - if:
> +    properties:
> +      qcom,pmic: true
> +    then:
> +      properties:
> +        qcom,pmic-id: false
> +  - if:
> +    properties:
> +      qcom,pmic-id: true
> +    then:
> +      properties:
> +        qcom,pmic: false
> +
> +additionalProperties: true
> +
> +examples:
> +  - |
> +    #include <dt-bindings/arm/qcom,ids.h>
> +    / {
> +      compatible = "qcom,sm8650";
> +      board-id {
> +        qcom,soc = <QCOM_ID_SM8650>;
> +        qcom,platform = <QCOM_BOARD_ID_MTP>;
> +      };
> +    };
> +
> +  - |
> +    #include <dt-bindings/arm/qcom,ids.h>
> +    / {
> +      compatible = "qcom,sm8650";
> +      board-id {
> +        qcom,soc-version = <QCOM_ID_SM8650 QCOM_SOC_REVISION(1)>,
> +                           <QCOM_ID_SM8650 QCOM_SOC_REVISION(2)>;
> +        qcom,platform-type = <QCOM_BOARD_ID_MTP 0>, <QCOM_BOARD_ID_MTP 1>;
> +      };
> +    };
> +
> +  - |
> +    #include <dt-bindings/arm/qcom,ids.h>
> +    / {
> +      compatible = "qcom,sm8650";
> +      board-id {
> +        qcom,soc = <QCOM_ID_SM8650>;
> +        qcom,platform-version = <QCOM_BOARD_ID(MTP, 0, 1, 0)>,
> +                                <QCOM_BOARD_ID(MTP, 0, 1, 1)>;
> +        qcom,boot-device = <QCOM_BOARD_BOOT_UFS>;
> +      };
> +    };
> 
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH RFC v3 5/9] dt-bindings: board: Document board-ids for Qualcomm devices

[Elliot Berman]

On Sat, May 25, 2024 at 06:08:46PM +0100, Conor Dooley wrote:
> On Tue, May 21, 2024 at 11:38:02AM -0700, Elliot Berman wrote:
> > +
> > +allOf:
> > +  # either describe soc or soc-version; it's confusing to have both
> 
> Why not just use the one that has the most information and discard the
> others? If your dtb picker for this platform doesn't care about the soc
> version, then just don't look at that cell?

The dtb picker for the platform doesn't know whether to care about the
SoC version/platform version/whatever. That's a property of the DTB
itself and I don't think it makes much sense to bake that into the DTB
picker which would otherwise be unaware of this.

> 
> Likewise for platform and PMIC, why can't you ignore the cells you don't
> care about, rather than having a new property for each variant? Nothing
> in this patch explains why multiple variants are required rather than
> just dealing with the most informational.
>

Sure, I will explain in future revision.

- Elliot

[PATCH RFC v3 6/9] arm64: boot: dts: sm8650: Add board-id

[Elliot Berman]

Add board-id to match sm8650 MTPs and QRDs.

Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sm8650-mtp.dts | 6 ++++++
 arch/arm64/boot/dts/qcom/sm8650-qrd.dts | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8650-mtp.dts b/arch/arm64/boot/dts/qcom/sm8650-mtp.dts
index be133a3d5cbe..ceaf7cc270af 100644
--- a/arch/arm64/boot/dts/qcom/sm8650-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sm8650-mtp.dts
@@ -5,6 +5,7 @@
 
 /dts-v1/;
 
+#include <dt-bindings/arm/qcom,ids.h>
 #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
 #include "sm8650.dtsi"
 #include "pm8010.dtsi"
@@ -28,6 +29,11 @@ chosen {
 		stdout-path = "serial0:115200n8";
 	};
 
+	board-id {
+		qcom,soc = <QCOM_ID_SM8650>;
+		qcom,platform = <QCOM_BOARD_ID_MTP>;
+	};
+
 	pmic-glink {
 		compatible = "qcom,sm8650-pmic-glink",
 			     "qcom,sm8550-pmic-glink",
diff --git a/arch/arm64/boot/dts/qcom/sm8650-qrd.dts b/arch/arm64/boot/dts/qcom/sm8650-qrd.dts
index b9151c2ddf2e..672ffcd0eaf0 100644
--- a/arch/arm64/boot/dts/qcom/sm8650-qrd.dts
+++ b/arch/arm64/boot/dts/qcom/sm8650-qrd.dts
@@ -5,6 +5,7 @@
 
 /dts-v1/;
 
+#include <dt-bindings/arm/qcom,ids.h>
 #include <dt-bindings/leds/common.h>
 #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
 #include "sm8650.dtsi"
@@ -26,6 +27,11 @@ aliases {
 		serial1 = &uart14;
 	};
 
+	board-id {
+		qcom,soc = <QCOM_ID_SM8650>;
+		qcom,platform = <QCOM_BOARD_ID_QRD>;
+	};
+
 	chosen {
 		stdout-path = "serial0:115200n8";
 	};

-- 
2.34.1

Re: [PATCH RFC v3 6/9] arm64: boot: dts: sm8650: Add board-id

[Krzysztof Kozlowski]

On 21/05/2024 20:38, Elliot Berman wrote:
> Add board-id to match sm8650 MTPs and QRDs.
> 
> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

> ---
>  arch/arm64/boot/dts/qcom/sm8650-mtp.dts | 6 ++++++
>  arch/arm64/boot/dts/qcom/sm8650-qrd.dts | 6 ++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8650-mtp.dts b/arch/arm64/boot/dts/qcom/sm8650-mtp.dts
> index be133a3d5cbe..ceaf7cc270af 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8650-mtp.dts
> @@ -5,6 +5,7 @@
>  
>  /dts-v1/;
>  
> +#include <dt-bindings/arm/qcom,ids.h>
>  #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
>  #include "sm8650.dtsi"
>  #include "pm8010.dtsi"
> @@ -28,6 +29,11 @@ chosen {
>  		stdout-path = "serial0:115200n8";
>  	};
>  
> +	board-id {
> +		qcom,soc = <QCOM_ID_SM8650>;
> +		qcom,platform = <QCOM_BOARD_ID_MTP>;

I don't see a single benefit of this. This duplicates compatible and
brings absolutely no new information for the bootloaders.


Best regards,
Krzysztof

[PATCH RFC v3 7/9] arm64: boot: dts: qcom: Use phandles for thermal_zones

[Elliot Berman]

In preparation for converting sm8550 to use devicetree overlays, use
phandles for thermal_zones.

Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
 arch/arm64/boot/dts/qcom/pm8010.dtsi    |  62 ++++++++--------
 arch/arm64/boot/dts/qcom/pm8550.dtsi    |  32 ++++----
 arch/arm64/boot/dts/qcom/pm8550b.dtsi   |  36 +++++----
 arch/arm64/boot/dts/qcom/pm8550ve.dtsi  |  38 +++++-----
 arch/arm64/boot/dts/qcom/pm8550vs.dtsi  | 128 ++++++++++++++++----------------
 arch/arm64/boot/dts/qcom/pmr735d_a.dtsi |  38 +++++-----
 arch/arm64/boot/dts/qcom/pmr735d_b.dtsi |  38 +++++-----
 arch/arm64/boot/dts/qcom/sm8550.dtsi    |   2 +-
 arch/arm64/boot/dts/qcom/sm8650.dtsi    |   2 +-
 9 files changed, 181 insertions(+), 195 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/pm8010.dtsi b/arch/arm64/boot/dts/qcom/pm8010.dtsi
index 0ea641e12209..a889df2f2f25 100644
--- a/arch/arm64/boot/dts/qcom/pm8010.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8010.dtsi
@@ -6,47 +6,45 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/spmi/spmi.h>
 
-/ {
-	thermal-zones {
-		pm8010-m-thermal {
-			polling-delay-passive = <100>;
-			polling-delay = <0>;
+&thermal_zones {
+	pm8010-m-thermal {
+		polling-delay-passive = <100>;
+		polling-delay = <0>;
 
-			thermal-sensors = <&pm8010_m_temp_alarm>;
+		thermal-sensors = <&pm8010_m_temp_alarm>;
 
-			trips {
-				trip0 {
-					temperature = <95000>;
-					hysteresis = <0>;
-					type = "passive";
-				};
+		trips {
+			trip0 {
+				temperature = <95000>;
+				hysteresis = <0>;
+				type = "passive";
+			};
 
-				trip1 {
-					temperature = <115000>;
-					hysteresis = <0>;
-					type = "hot";
-				};
+			trip1 {
+				temperature = <115000>;
+				hysteresis = <0>;
+				type = "hot";
 			};
 		};
+	};
 
-		pm8010-n-thermal {
-			polling-delay-passive = <100>;
-			polling-delay = <0>;
+	pm8010-n-thermal {
+		polling-delay-passive = <100>;
+		polling-delay = <0>;
 
-			thermal-sensors = <&pm8010_n_temp_alarm>;
+		thermal-sensors = <&pm8010_n_temp_alarm>;
 
-			trips {
-				trip0 {
-					temperature = <95000>;
-					hysteresis = <0>;
-					type = "passive";
-				};
+		trips {
+			trip0 {
+				temperature = <95000>;
+				hysteresis = <0>;
+				type = "passive";
+			};
 
-				trip1 {
-					temperature = <115000>;
-					hysteresis = <0>;
-					type = "hot";
-				};
+			trip1 {
+				temperature = <115000>;
+				hysteresis = <0>;
+				type = "hot";
 			};
 		};
 	};
diff --git a/arch/arm64/boot/dts/qcom/pm8550.dtsi b/arch/arm64/boot/dts/qcom/pm8550.dtsi
index 797a18c249a4..cb5e70b28445 100644
--- a/arch/arm64/boot/dts/qcom/pm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8550.dtsi
@@ -6,26 +6,24 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/spmi/spmi.h>
 
-/ {
-	thermal-zones {
-		pm8550-thermal {
-			polling-delay-passive = <100>;
-			polling-delay = <0>;
+&thermal_zones {
+	pm8550-thermal {
+		polling-delay-passive = <100>;
+		polling-delay = <0>;
 
-			thermal-sensors = <&pm8550_temp_alarm>;
+		thermal-sensors = <&pm8550_temp_alarm>;
 
-			trips {
-				trip0 {
-					temperature = <95000>;
-					hysteresis = <0>;
-					type = "passive";
-				};
+		trips {
+			trip0 {
+				temperature = <95000>;
+				hysteresis = <0>;
+				type = "passive";
+			};
 
-				trip1 {
-					temperature = <115000>;
-					hysteresis = <0>;
-					type = "hot";
-				};
+			trip1 {
+				temperature = <115000>;
+				hysteresis = <0>;
+				type = "hot";
 			};
 		};
 	};
diff --git a/arch/arm64/boot/dts/qcom/pm8550b.dtsi b/arch/arm64/boot/dts/qcom/pm8550b.dtsi
index 72609f31c890..b3cfa030679a 100644
--- a/arch/arm64/boot/dts/qcom/pm8550b.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8550b.dtsi
@@ -6,26 +6,24 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/spmi/spmi.h>
 
-/ {
-	thermal-zones {
-		pm8550b-thermal {
-			polling-delay-passive = <100>;
-			polling-delay = <0>;
-
-			thermal-sensors = <&pm8550b_temp_alarm>;
-
-			trips {
-				trip0 {
-					temperature = <95000>;
-					hysteresis = <0>;
-					type = "passive";
-				};
+&thermal_zones {
+	pm8550b-thermal {
+		polling-delay-passive = <100>;
+		polling-delay = <0>;
+
+		thermal-sensors = <&pm8550b_temp_alarm>;
+
+		trips {
+			trip0 {
+				temperature = <95000>;
+				hysteresis = <0>;
+				type = "passive";
+			};
 
-				trip1 {
-					temperature = <115000>;
-					hysteresis = <0>;
-					type = "hot";
-				};
+			trip1 {
+				temperature = <115000>;
+				hysteresis = <0>;
+				type = "hot";
 			};
 		};
 	};
diff --git a/arch/arm64/boot/dts/qcom/pm8550ve.dtsi b/arch/arm64/boot/dts/qcom/pm8550ve.dtsi
index 4dc1f03ab2c7..8ef57a51b5cd 100644
--- a/arch/arm64/boot/dts/qcom/pm8550ve.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8550ve.dtsi
@@ -6,26 +6,24 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/spmi/spmi.h>
 
-/ {
-	thermal-zones {
-		pm8550ve-thermal {
-			polling-delay-passive = <100>;
-			polling-delay = <0>;
-
-			thermal-sensors = <&pm8550ve_temp_alarm>;
-
-			trips {
-				trip0 {
-					temperature = <95000>;
-					hysteresis = <0>;
-					type = "passive";
-				};
-
-				trip1 {
-					temperature = <115000>;
-					hysteresis = <0>;
-					type = "hot";
-				};
+&thermal_zones {
+	pm8550ve-thermal {
+		polling-delay-passive = <100>;
+		polling-delay = <0>;
+
+		thermal-sensors = <&pm8550ve_temp_alarm>;
+
+		trips {
+			trip0 {
+				temperature = <95000>;
+				hysteresis = <0>;
+				type = "passive";
+			};
+
+			trip1 {
+				temperature = <115000>;
+				hysteresis = <0>;
+				type = "hot";
 			};
 		};
 	};
diff --git a/arch/arm64/boot/dts/qcom/pm8550vs.dtsi b/arch/arm64/boot/dts/qcom/pm8550vs.dtsi
index 97b1c18aa7d8..258526abcc21 100644
--- a/arch/arm64/boot/dts/qcom/pm8550vs.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8550vs.dtsi
@@ -6,89 +6,87 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/spmi/spmi.h>
 
-/ {
-	thermal-zones {
-		pm8550vs-c-thermal {
-			polling-delay-passive = <100>;
-			polling-delay = <0>;
-
-			thermal-sensors = <&pm8550vs_c_temp_alarm>;
-
-			trips {
-				trip0 {
-					temperature = <95000>;
-					hysteresis = <0>;
-					type = "passive";
-				};
-
-				trip1 {
-					temperature = <115000>;
-					hysteresis = <0>;
-					type = "hot";
-				};
+&thermal_zones {
+	pm8550vs-c-thermal {
+		polling-delay-passive = <100>;
+		polling-delay = <0>;
+
+		thermal-sensors = <&pm8550vs_c_temp_alarm>;
+
+		trips {
+			trip0 {
+				temperature = <95000>;
+				hysteresis = <0>;
+				type = "passive";
+			};
+
+			trip1 {
+				temperature = <115000>;
+				hysteresis = <0>;
+				type = "hot";
 			};
 		};
+	};
 
-		pm8550vs-d-thermal {
-			polling-delay-passive = <100>;
-			polling-delay = <0>;
+	pm8550vs-d-thermal {
+		polling-delay-passive = <100>;
+		polling-delay = <0>;
 
-			thermal-sensors = <&pm8550vs_d_temp_alarm>;
+		thermal-sensors = <&pm8550vs_d_temp_alarm>;
 
-			trips {
-				trip0 {
-					temperature = <95000>;
-					hysteresis = <0>;
-					type = "passive";
-				};
+		trips {
+			trip0 {
+				temperature = <95000>;
+				hysteresis = <0>;
+				type = "passive";
+			};
 
-				trip1 {
-					temperature = <115000>;
-					hysteresis = <0>;
-					type = "hot";
-				};
+			trip1 {
+				temperature = <115000>;
+				hysteresis = <0>;
+				type = "hot";
 			};
 		};
+	};
 
-		pm8550vs-e-thermal {
-			polling-delay-passive = <100>;
-			polling-delay = <0>;
+	pm8550vs-e-thermal {
+		polling-delay-passive = <100>;
+		polling-delay = <0>;
 
-			thermal-sensors = <&pm8550vs_e_temp_alarm>;
+		thermal-sensors = <&pm8550vs_e_temp_alarm>;
 
-			trips {
-				trip0 {
-					temperature = <95000>;
-					hysteresis = <0>;
-					type = "passive";
-				};
+		trips {
+			trip0 {
+				temperature = <95000>;
+				hysteresis = <0>;
+				type = "passive";
+			};
 
-				trip1 {
-					temperature = <115000>;
-					hysteresis = <0>;
-					type = "hot";
-				};
+			trip1 {
+				temperature = <115000>;
+				hysteresis = <0>;
+				type = "hot";
 			};
 		};
+	};
 
-		pm8550vs-g-thermal {
-			polling-delay-passive = <100>;
-			polling-delay = <0>;
+	pm8550vs-g-thermal {
+		polling-delay-passive = <100>;
+		polling-delay = <0>;
 
-			thermal-sensors = <&pm8550vs_g_temp_alarm>;
+		thermal-sensors = <&pm8550vs_g_temp_alarm>;
 
-			trips {
-				trip0 {
-					temperature = <95000>;
-					hysteresis = <0>;
-					type = "passive";
-				};
+		trips {
+			trip0 {
+				temperature = <95000>;
+				hysteresis = <0>;
+				type = "passive";
+			};
 
-				trip1 {
-					temperature = <115000>;
-					hysteresis = <0>;
-					type = "hot";
-				};
+			trip1 {
+				temperature = <115000>;
+				hysteresis = <0>;
+				type = "hot";
 			};
 		};
 	};
diff --git a/arch/arm64/boot/dts/qcom/pmr735d_a.dtsi b/arch/arm64/boot/dts/qcom/pmr735d_a.dtsi
index 37daaefe3431..251a16424d84 100644
--- a/arch/arm64/boot/dts/qcom/pmr735d_a.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmr735d_a.dtsi
@@ -6,26 +6,24 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/spmi/spmi.h>
 
-/ {
-	thermal-zones {
-		pmr735d-k-thermal {
-			polling-delay-passive = <100>;
-			polling-delay = <0>;
-
-			thermal-sensors = <&pmr735d_k_temp_alarm>;
-
-			trips {
-				trip0 {
-					temperature = <95000>;
-					hysteresis = <0>;
-					type = "passive";
-				};
-
-				trip1 {
-					temperature = <115000>;
-					hysteresis = <0>;
-					type = "hot";
-				};
+&thermal_zones {
+	pmr735d-k-thermal {
+		polling-delay-passive = <100>;
+		polling-delay = <0>;
+
+		thermal-sensors = <&pmr735d_k_temp_alarm>;
+
+		trips {
+			trip0 {
+				temperature = <95000>;
+				hysteresis = <0>;
+				type = "passive";
+			};
+
+			trip1 {
+				temperature = <115000>;
+				hysteresis = <0>;
+				type = "hot";
 			};
 		};
 	};
diff --git a/arch/arm64/boot/dts/qcom/pmr735d_b.dtsi b/arch/arm64/boot/dts/qcom/pmr735d_b.dtsi
index 3b470f6ac46f..dbcfeb53d8ec 100644
--- a/arch/arm64/boot/dts/qcom/pmr735d_b.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmr735d_b.dtsi
@@ -6,26 +6,24 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/spmi/spmi.h>
 
-/ {
-	thermal-zones {
-		pmr735d-l-thermal {
-			polling-delay-passive = <100>;
-			polling-delay = <0>;
-
-			thermal-sensors = <&pmr735d_l_temp_alarm>;
-
-			trips {
-				trip0 {
-					temperature = <95000>;
-					hysteresis = <0>;
-					type = "passive";
-				};
-
-				trip1 {
-					temperature = <115000>;
-					hysteresis = <0>;
-					type = "hot";
-				};
+&thermal_zones {
+	pmr735d-l-thermal {
+		polling-delay-passive = <100>;
+		polling-delay = <0>;
+
+		thermal-sensors = <&pmr735d_l_temp_alarm>;
+
+		trips {
+			trip0 {
+				temperature = <95000>;
+				hysteresis = <0>;
+				type = "passive";
+			};
+
+			trip1 {
+				temperature = <115000>;
+				hysteresis = <0>;
+				type = "hot";
 			};
 		};
 	};
diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index ee1ba5a8c8fc..c68e08747b6f 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -4452,7 +4452,7 @@ compute-cb@8 {
 		};
 	};
 
-	thermal-zones {
+	thermal_zones: thermal-zones {
 		aoss0-thermal {
 			polling-delay-passive = <0>;
 			polling-delay = <0>;
diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
index 2df77123a8c7..32198bf3cf7c 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -5030,7 +5030,7 @@ compute-cb@8 {
 		};
 	};
 
-	thermal-zones {
+	thermal_zones: thermal-zones {
 		aoss0-thermal {
 			polling-delay-passive = <0>;
 			polling-delay = <0>;

-- 
2.34.1

[PATCH RFC v3 8/9] arm64: boot: dts: qcom: sm8550: Split into overlays

[Elliot Berman]

Generic sm8550 devicetree is split into a dtsi. Move it into its own DTB
and preserve the boards as overlays.

When not using overlays, 264 KB needed to store the sm8550-mtp.dtb and
sm8550-qrd.dtb. When using overlays, 188 KB is needed to store
sm8550.dtb, sm8550-mtp.dtbo, and sm8550-qrd.dtbo; where the overlays are
~36 KB.

Also add the board-ids for these DTBs.

This change is not intended to be merged, it breaks aliases and I doubt
it boots correct. The intent here is to show how board-id could be used
with a DTB and DTBO.

Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
 arch/arm64/boot/dts/qcom/Makefile                  |  4 ++++
 .../dts/qcom/{sm8550-mtp.dts => sm8550-mtp.dtso}   | 24 ++++++++++++++++++++--
 .../dts/qcom/{sm8550-qrd.dts => sm8550-qrd.dtso}   | 22 +++++++++++++++++---
 .../boot/dts/qcom/{sm8550.dtsi => sm8550.dts}      |  8 ++++++++
 4 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 39889d5f8e12..7f137f274d8c 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -233,6 +233,10 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sm8450-hdk.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sm8450-qrd.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sm8450-sony-xperia-nagara-pdx223.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sm8450-sony-xperia-nagara-pdx224.dtb
+
+sm8550-mtp-dtbs		:= sm8550.dtb sm8550-mtp.dtbo
+sm8550-qrd-dtbs		:= sm8550.dtb sm8550-qrd.dtbo
+
 dtb-$(CONFIG_ARCH_QCOM)	+= sm8550-mtp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sm8550-qrd.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sm8650-mtp.dtb
diff --git a/arch/arm64/boot/dts/qcom/sm8550-mtp.dts b/arch/arm64/boot/dts/qcom/sm8550-mtp.dtso
similarity index 98%
rename from arch/arm64/boot/dts/qcom/sm8550-mtp.dts
rename to arch/arm64/boot/dts/qcom/sm8550-mtp.dtso
index c1135ad5fa69..0ee4614719ca 100644
--- a/arch/arm64/boot/dts/qcom/sm8550-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sm8550-mtp.dtso
@@ -4,9 +4,12 @@
  */
 
 /dts-v1/;
+/plugin/;
 
+#include <dt-bindings/arm/qcom,ids.h>
+#include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
-#include "sm8550.dtsi"
+#include <dt-bindings/sound/qcom,q6dsp-lpass-ports.h>
 #include "pm8010.dtsi"
 #include "pm8550.dtsi"
 #include "pm8550b.dtsi"
@@ -17,13 +20,30 @@
 #include "pmr735d_a.dtsi"
 #include "pmr735d_b.dtsi"
 
+#define BOARD_ID	qcom,soc = <QCOM_ID_SM8550>; \
+			qcom,platform-type = <QCOM_BOARD_ID_MTP 0>
+
 / {
+	board-id {
+		BOARD_ID;
+	};
+};
+
+&{/} {
 	model = "Qualcomm Technologies, Inc. SM8550 MTP";
 	compatible = "qcom,sm8550-mtp", "qcom,sm8550";
 	chassis-type = "handset";
 
+	/**
+	 * Redefine the overlay in the DTBO so the sm8550-mtp.dtb that Kbuild
+	 * generates has accurate board-id.
+	 */
+	board-id {
+		BOARD_ID;
+	};
+
 	aliases {
-		serial0 = &uart7;
+		// serial0 = &uart7;
 	};
 
 	wcd938x: audio-codec {
diff --git a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts b/arch/arm64/boot/dts/qcom/sm8550-qrd.dtso
similarity index 98%
rename from arch/arm64/boot/dts/qcom/sm8550-qrd.dts
rename to arch/arm64/boot/dts/qcom/sm8550-qrd.dtso
index d401d63e5c4d..f756c50a80b9 100644
--- a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
+++ b/arch/arm64/boot/dts/qcom/sm8550-qrd.dtso
@@ -4,10 +4,14 @@
  */
 
 /dts-v1/;
+/plugin/;
 
+#include <dt-bindings/arm/qcom,ids.h>
+#include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/leds/common.h>
+#include <dt-bindings/phy/phy-qcom-qmp.h>
 #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
-#include "sm8550.dtsi"
+#include <dt-bindings/sound/qcom,q6dsp-lpass-ports.h>
 #include "pm8010.dtsi"
 #include "pm8550.dtsi"
 #include "pm8550b.dtsi"
@@ -19,13 +23,25 @@
 #include "pmr735d_b.dtsi"
 
 / {
+	board-id {
+		qcom,soc = <QCOM_ID_SM8550>;
+		qcom,platform = <QCOM_BOARD_ID_QRD>;
+	};
+};
+
+&{/} {
 	model = "Qualcomm Technologies, Inc. SM8550 QRD";
 	compatible = "qcom,sm8550-qrd", "qcom,sm8550";
 	chassis-type = "handset";
 
+	board-id {
+		qcom,soc = <QCOM_ID_SM8550>;
+		qcom,platform = <QCOM_BOARD_ID_QRD>;
+	};
+
 	aliases {
-		serial0 = &uart7;
-		serial1 = &uart14;
+		// serial0 = &uart7;
+		// serial1 = &uart14;
 	};
 
 	wcd938x: audio-codec {
diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dts
similarity index 99%
rename from arch/arm64/boot/dts/qcom/sm8550.dtsi
rename to arch/arm64/boot/dts/qcom/sm8550.dts
index c68e08747b6f..3546ea4b96f1 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dts
@@ -3,6 +3,9 @@
  * Copyright (c) 2022, Linaro Limited
  */
 
+/dts-v1/;
+
+#include <dt-bindings/arm/qcom,ids.h>
 #include <dt-bindings/clock/qcom,rpmh.h>
 #include <dt-bindings/clock/qcom,sm8450-videocc.h>
 #include <dt-bindings/clock/qcom,sm8550-camcc.h>
@@ -32,6 +35,11 @@ / {
 
 	chosen { };
 
+	board_id: board-id {
+		qcom,soc-version = <QCOM_ID_SM8550 QCOM_SOC_REVISION(1)>,
+				   <QCOM_ID_SM8550 QCOM_SOC_REVISION(2)>;
+	};
+
 	clocks {
 		xo_board: xo-board {
 			compatible = "fixed-clock";

-- 
2.34.1

Re: [PATCH RFC v3 8/9] arm64: boot: dts: qcom: sm8550: Split into overlays

[Krzysztof Kozlowski]

On 21/05/2024 20:38, Elliot Berman wrote:
>  
> +/dts-v1/;
> +
> +#include <dt-bindings/arm/qcom,ids.h>
>  #include <dt-bindings/clock/qcom,rpmh.h>
>  #include <dt-bindings/clock/qcom,sm8450-videocc.h>
>  #include <dt-bindings/clock/qcom,sm8550-camcc.h>
> @@ -32,6 +35,11 @@ / {
>  
>  	chosen { };
>  
> +	board_id: board-id {
> +		qcom,soc-version = <QCOM_ID_SM8550 QCOM_SOC_REVISION(1)>,
> +				   <QCOM_ID_SM8550 QCOM_SOC_REVISION(2)>;
> +	};


I don't see how does it help to understand usage of board-id. You list
all possible revisions, right? So this is entirely redundant.

Best regards,
Krzysztof

[PATCH RFC v3 9/9] tools: board-id: Add test suite

[Elliot Berman]

Add a short test suite to demonstrate board-id selection and scoring.
This patch isn't intended to be merged here.

After compiling the kernel (esp. arch/arm64/boot/dts/qcom DTBs), run
tools/board-id/test.py.

The test cases provide a hypothetical firmware-provied board-id and
compares expected output for which DTBs gets matched.

Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
 tools/board-id/test.py | 151 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 151 insertions(+)

diff --git a/tools/board-id/test.py b/tools/board-id/test.py
new file mode 100644
index 000000000000..687b31ad73d2
--- /dev/null
+++ b/tools/board-id/test.py
@@ -0,0 +1,151 @@
+from collections import namedtuple
+import glob
+import os
+import subprocess
+from tempfile import NamedTemporaryFile
+import unittest
+
+
+LINUX_ROOT = os.path.abspath(os.path.join(__file__, "..", "..", ".."))
+ENV_WITH_DTC = {
+    "PATH": os.path.join(LINUX_ROOT, "scripts", "dtc") + os.pathsep + os.environ["PATH"]
+}
+
+
+TestCase = namedtuple("TestCase", ["score_all", "board_id", "output"])
+
+test_cases = [
+    TestCase(
+        # A board_id that could be provided by firmware
+        board_id="""
+        qcom,soc = <QCOM_ID_SM8650>;
+        qcom,soc-version = <QCOM_ID_SM8650 QCOM_SOC_REVISION(1)>;
+        qcom,platform = <QCOM_BOARD_ID_MTP>;
+        qcom,platform-type = <QCOM_BOARD_ID_MTP 0>;
+        qcom,platform-version = <QCOM_BOARD_ID_MTP 0 0x0100>;
+        qcom,boot-device = <QCOM_BOARD_BOOT_UFS>;
+        """,
+        score_all=False,
+        output="""
+        qcom/sm8650-mtp.dtb
+        """,
+    ),
+    TestCase(
+        # A board_id that could be provided by firmware
+        board_id="""
+        qcom,soc = <QCOM_ID_SM8550>;
+        qcom,soc-version = <QCOM_ID_SM8550 QCOM_SOC_REVISION(1)>;
+        qcom,platform = <QCOM_BOARD_ID_MTP>;
+        qcom,platform-type = <QCOM_BOARD_ID_MTP 0>;
+        qcom,platform-version = <QCOM_BOARD_ID_MTP 0 0x0100>;
+        qcom,boot-device = <QCOM_BOARD_BOOT_UFS>;
+        """,
+        score_all=True,
+        output="""
+        qcom/sm8550.dtb: 1
+        qcom/sm8550-mtp.dtb: 3
+        qcom/sm8550-mtp.dtbo: 2
+        """,
+    ),
+]
+
+
+def compile_board_id(board_id: str):
+    dts = f"""
+        /dts-v1/;
+
+        #include <dt-bindings/arm/qcom,ids.h>
+
+        / {{
+            compatible = "linux,dummy";
+            board-id {{
+                {board_id}
+            }};
+        }};
+        """
+    dts_processed = subprocess.run(
+        [
+            "gcc",
+            "-E",
+            "-nostdinc",
+            f"-I{os.path.join(LINUX_ROOT, 'scripts', 'dtc', 'include-prefixes')}",
+            "-undef",
+            "-D__DTS__",
+            "-x",
+            "assembler-with-cpp",
+            "-o" "-",
+            "-",
+        ],
+        stdout=subprocess.PIPE,
+        input=dts.encode("utf-8"),
+        check=True,
+    )
+    dtc = subprocess.run(
+        ["dtc", "-I", "dts", "-O", "dtb", "-o", "-", "-"],
+        stdout=subprocess.PIPE,
+        input=dts_processed.stdout,
+        env=ENV_WITH_DTC,
+    )
+    return dtc.stdout
+
+
+def select_boards(score_all, fwdtb):
+    with NamedTemporaryFile() as fwdtb_file:
+        fwdtb_file.write(fwdtb)
+        fwdtb_file.flush()
+        root_dir = os.path.join(LINUX_ROOT, "arch", "arm64", "boot", "dts")
+        return subprocess.run(
+            filter(
+                bool,
+                [
+                    "fdt-select-board",
+                    "-a" if score_all else None,
+                    "-r",
+                    fwdtb_file.name,
+                    *glob.glob(
+                        "qcom/*.dtb*",
+                        root_dir=root_dir,
+                    ),
+                ],
+            ),
+            stdout=subprocess.PIPE,
+            text=True,
+            cwd=root_dir,
+            env=ENV_WITH_DTC,
+            stderr=subprocess.STDOUT,
+        )
+
+
+def fixup_lines(s):
+    return '\n'.join(filter(bool, sorted(_s.strip() for _s in s.split('\n'))))
+
+
+class TestBoardIds(unittest.TestCase):
+    def __init__(self, index: int, args: TestCase) -> None:
+        super().__init__()
+        self.args = args
+        self.index = index
+
+    def runTest(self):
+        fwdtb = compile_board_id(self.args.board_id)
+        output = select_boards(self.args.score_all, fwdtb)
+        if output.stderr:
+            self.assertMultiLineEqual(output.stderr, "")
+        expected = fixup_lines(self.args.output)
+        actual = fixup_lines(output.stdout)
+        self.assertMultiLineEqual(expected, actual)
+
+    def __str__(self):
+        return f"Test case {self.index}"
+
+
+def suite():
+    suite = unittest.TestSuite()
+    for idx, test in enumerate(test_cases):
+        suite.addTest(TestBoardIds(idx + 1, test))
+    return suite
+
+
+if __name__ == "__main__":
+    runner = unittest.TextTestRunner()
+    runner.run(suite())

-- 
2.34.1

Re: [PATCH RFC v3 0/9] dt-bindings: hwinfo: Introduce board-id

[Dmitry Baryshkov]

Hi Elliot,

On Tue, 21 May 2024 at 21:41, Elliot Berman <quic_eberman@quicinc.com> wrote:
>
> Device manufacturers frequently ship multiple boards or SKUs under a
> single software package. These software packages will ship multiple
> devicetree blobs and require some mechanism to pick the correct DTB for
> the board the software package was deployed. Introduce a common
> definition for adding board identifiers to device trees. board-id
> provides a mechanism for bootloaders to select the appropriate DTB which
> is vendor/OEM-agnostic.

This is a v3 of the RFC, however it is still a qcom-only series. Might
I suggest gaining an actual interest from any other hardware vendor
(in the form of the patches) before posting v4? Otherwise it might
still end up being a Qualcomm solution which is not supported and/or
used by other hardware vendors.

>
> This series is based off a talk I gave at EOSS NA 2024 [1]. There is
> some further discussion about how to do devicetree selection in the
> boot-architecture mailing list [2].
>
> [1]: https://sched.co/1aBFy
> [2]: https://lists.linaro.org/archives/list/boot-architecture@lists.linaro.org/thread/DZCZSOCRH5BN7YOXEL2OQKSDIY7DCW2M/
>

-- 
With best wishes
Dmitry

Re: [PATCH RFC v3 0/9] dt-bindings: hwinfo: Introduce board-id

[Konrad Dybcio]

On 21.05.2024 9:00 PM, Dmitry Baryshkov wrote:
> Hi Elliot,
> 
> On Tue, 21 May 2024 at 21:41, Elliot Berman <quic_eberman@quicinc.com> wrote:
>>
>> Device manufacturers frequently ship multiple boards or SKUs under a
>> single software package. These software packages will ship multiple
>> devicetree blobs and require some mechanism to pick the correct DTB for
>> the board the software package was deployed. Introduce a common
>> definition for adding board identifiers to device trees. board-id
>> provides a mechanism for bootloaders to select the appropriate DTB which
>> is vendor/OEM-agnostic.
> 
> This is a v3 of the RFC, however it is still a qcom-only series. Might
> I suggest gaining an actual interest from any other hardware vendor
> (in the form of the patches) before posting v4? Otherwise it might
> still end up being a Qualcomm solution which is not supported and/or
> used by other hardware vendors.

AMD should be onboard [1].

Konrad

[1] https://resources.linaro.org/en/resource/q7U3Rr7m3ZbZmXzYK7A9u3

Re: [PATCH RFC v3 0/9] dt-bindings: hwinfo: Introduce board-id

[Michal Simek]

Hi,

thanks for CCing me.

On 5/24/24 17:51, Konrad Dybcio wrote:
> On 21.05.2024 9:00 PM, Dmitry Baryshkov wrote:
>> Hi Elliot,
>>
>> On Tue, 21 May 2024 at 21:41, Elliot Berman <quic_eberman@quicinc.com> wrote:
>>>
>>> Device manufacturers frequently ship multiple boards or SKUs under a
>>> single software package. These software packages will ship multiple
>>> devicetree blobs and require some mechanism to pick the correct DTB for
>>> the board the software package was deployed. Introduce a common
>>> definition for adding board identifiers to device trees. board-id
>>> provides a mechanism for bootloaders to select the appropriate DTB which
>>> is vendor/OEM-agnostic.
>>
>> This is a v3 of the RFC, however it is still a qcom-only series. Might
>> I suggest gaining an actual interest from any other hardware vendor
>> (in the form of the patches) before posting v4? Otherwise it might
>> still end up being a Qualcomm solution which is not supported and/or
>> used by other hardware vendors.
> 
> AMD should be onboard [1].
> 
> Konrad
> 
> [1] https://resources.linaro.org/en/resource/q7U3Rr7m3ZbZmXzYK7A9u3

I am trying to wrap my head around this and I have also looked at that EOSS 
presentation.
I don't think I fully understand your case.
There are multiple components which you need to detect. SOC - I expect reading 
by some regs, board - I expect you have any eeprom, OTP, adc, gpio, etc way how 
to detect board ID and revision.
And then you mentioned displays - how do you detect them?

In our Kria platform we have eeproms on SOM and CC cards (or FMC/extension 
cards) which we read and decode and based on information from it we are 
composing "unique" string. And then we are having DTBs in FIT image where 
description of configuration it taken as regular expression. That's why it is up 
to you how you want to combine them.
Currently we are merging them offline and we are not applying any DT overlay at 
run time but can be done (we are missing one missing piece in U-Boot for it).

In presentation you mentioned also that applying overlay can fail  but not sure 
how you can reach it. Because Linux kernel has the whole infrastructure to cover 
all combinations with base DT + overlays. It means if you cover all working 
combinations there you should see if they don't apply properly.

Also do you really need to detect everything from firmware side? Or isn't it 
enough to have just "some" devices and then load the rest where you are in OS?
I think that's pretty much another way to go to have bare minimum functionality 
provided by firmware and deal with the rest in OS.

Thanks,
Michal

Re: [PATCH RFC v3 0/9] dt-bindings: hwinfo: Introduce board-id

[Elliot Berman]

On Mon, May 27, 2024 at 09:19:59AM +0200, Michal Simek wrote:
> Hi,
> 
> thanks for CCing me.
> 
> On 5/24/24 17:51, Konrad Dybcio wrote:
> > On 21.05.2024 9:00 PM, Dmitry Baryshkov wrote:
> > > Hi Elliot,
> > > 
> > > On Tue, 21 May 2024 at 21:41, Elliot Berman <quic_eberman@quicinc.com> wrote:
> > > > 
> > > > Device manufacturers frequently ship multiple boards or SKUs under a
> > > > single software package. These software packages will ship multiple
> > > > devicetree blobs and require some mechanism to pick the correct DTB for
> > > > the board the software package was deployed. Introduce a common
> > > > definition for adding board identifiers to device trees. board-id
> > > > provides a mechanism for bootloaders to select the appropriate DTB which
> > > > is vendor/OEM-agnostic.
> > > 
> > > This is a v3 of the RFC, however it is still a qcom-only series. Might
> > > I suggest gaining an actual interest from any other hardware vendor
> > > (in the form of the patches) before posting v4? Otherwise it might
> > > still end up being a Qualcomm solution which is not supported and/or
> > > used by other hardware vendors.
> > 
> > AMD should be onboard [1].
> > 
> > Konrad
> > 
> > [1] https://resources.linaro.org/en/resource/q7U3Rr7m3ZbZmXzYK7A9u3
> 
> I am trying to wrap my head around this and I have also looked at that EOSS
> presentation.
> I don't think I fully understand your case.
> There are multiple components which you need to detect. SOC - I expect
> reading by some regs, board - I expect you have any eeprom, OTP, adc, gpio,
> etc way how to detect board ID and revision.
> And then you mentioned displays - how do you detect them?

We have a similar mechanism to what you mention below: we have a ROM
which encodes information about the platform and that can be read by
firmware/bootloader/OS.

> 
> In our Kria platform we have eeproms on SOM and CC cards (or FMC/extension
> cards) which we read and decode and based on information from it we are
> composing "unique" string. And then we are having DTBs in FIT image where
> description of configuration it taken as regular expression. That's why it
> is up to you how you want to combine them.

I don't think this is a fundamentally different approach from my
proposal. Instead of composing a "unique" string and using regex to
match, I'm proposing that the information (bytes) that are in your
eeprom can be matched without going through regex/string conversion.
Instead of firmware/bootloader doing a conversion to the strings, it
provides the values via board-id. I have concerns about having
bootloaders to contain a regex library -- probably easily addressed by
standardizing what terms the regex processor needs to support. I'm also
not sure if regex strings are an appropriate use of compatible strings.
Using strings limits the usefulness of comaptible strings to the
consumers of DTB, since the compatible string has to describe only the
boards the DTB is applicable to, you can't mention compatible strings
"this board is like" such as some generic SoC compatible.

> Currently we are merging them offline and we are not applying any DT overlay
> at run time but can be done (we are missing one missing piece in U-Boot for
> it).
> 
> In presentation you mentioned also that applying overlay can fail  but not
> sure how you can reach it. Because Linux kernel has the whole infrastructure
> to cover all combinations with base DT + overlays. It means if you cover all
> working combinations there you should see if they don't apply properly.

Mostly, I was referring to a situation where firmware provides an
overlay. Firmware doesn't know the DTB that OS has and I don't see any
way to gaurantee that firmware knows how to fix up the OS DTB.

> 
> Also do you really need to detect everything from firmware side? Or isn't it
> enough to have just "some" devices and then load the rest where you are in
> OS?
> I think that's pretty much another way to go to have bare minimum
> functionality provided by firmware and deal with the rest in OS.

Agreed, although not all devices can be loaded once you are in the OS.
All nondiscoverable devices would need to be desribed in the DT.

Thanks,
Elliot

Re: [PATCH RFC v3 0/9] dt-bindings: hwinfo: Introduce board-id

[Michal Simek]

On 5/29/24 17:32, Elliot Berman wrote:
> On Mon, May 27, 2024 at 09:19:59AM +0200, Michal Simek wrote:
>> Hi,
>>
>> thanks for CCing me.
>>
>> On 5/24/24 17:51, Konrad Dybcio wrote:
>>> On 21.05.2024 9:00 PM, Dmitry Baryshkov wrote:
>>>> Hi Elliot,
>>>>
>>>> On Tue, 21 May 2024 at 21:41, Elliot Berman <quic_eberman@quicinc.com> wrote:
>>>>>
>>>>> Device manufacturers frequently ship multiple boards or SKUs under a
>>>>> single software package. These software packages will ship multiple
>>>>> devicetree blobs and require some mechanism to pick the correct DTB for
>>>>> the board the software package was deployed. Introduce a common
>>>>> definition for adding board identifiers to device trees. board-id
>>>>> provides a mechanism for bootloaders to select the appropriate DTB which
>>>>> is vendor/OEM-agnostic.
>>>>
>>>> This is a v3 of the RFC, however it is still a qcom-only series. Might
>>>> I suggest gaining an actual interest from any other hardware vendor
>>>> (in the form of the patches) before posting v4? Otherwise it might
>>>> still end up being a Qualcomm solution which is not supported and/or
>>>> used by other hardware vendors.
>>>
>>> AMD should be onboard [1].
>>>
>>> Konrad
>>>
>>> [1] https://resources.linaro.org/en/resource/q7U3Rr7m3ZbZmXzYK7A9u3
>>
>> I am trying to wrap my head around this and I have also looked at that EOSS
>> presentation.
>> I don't think I fully understand your case.
>> There are multiple components which you need to detect. SOC - I expect
>> reading by some regs, board - I expect you have any eeprom, OTP, adc, gpio,
>> etc way how to detect board ID and revision.
>> And then you mentioned displays - how do you detect them?
> 
> We have a similar mechanism to what you mention below: we have a ROM
> which encodes information about the platform and that can be read by
> firmware/bootloader/OS.
> 
>>
>> In our Kria platform we have eeproms on SOM and CC cards (or FMC/extension
>> cards) which we read and decode and based on information from it we are
>> composing "unique" string. And then we are having DTBs in FIT image where
>> description of configuration it taken as regular expression. That's why it
>> is up to you how you want to combine them.
> 
> I don't think this is a fundamentally different approach from my
> proposal. Instead of composing a "unique" string and using regex to
> match, I'm proposing that the information (bytes) that are in your
> eeprom can be matched without going through regex/string conversion.
> Instead of firmware/bootloader doing a conversion to the strings, it
> provides the values via board-id. I have concerns about having
> bootloaders to contain a regex library -- probably easily addressed by
> standardizing what terms the regex processor needs to support. I'm also
> not sure if regex strings are an appropriate use of compatible strings.
> Using strings limits the usefulness of comaptible strings to the
> consumers of DTB, since the compatible string has to describe only the
> boards the DTB is applicable to, you can't mention compatible strings
> "this board is like" such as some generic SoC compatible.

We use regular expression to match fit images configuration description not 
actual DT itself. And because we have base DT for SOM and overlays for CC we are 
just using compatible string which is coming from CC only.
That's because fdtoverlay works like that (pretty much when compose them 
together compatible and model are rewritten based on information from overlay).

In past when we used applying DT overlay at run time via OS origin 
compatible/model strings were used.

I don't think we have any need to try to look for DTs provided by generic 
distributions (generated mostly via make dtbs_install) and we tend to provide DT 
directly by firmware as is required by SR IR.

And DTs for programmable logic are coupled with bitstream itself and for it at 
least on SOM OS is fully aware about base board and it's revision and chip size 
that user space loader know exactly where it runs and which bitstreams are 
compatible with that combination.

> 
>> Currently we are merging them offline and we are not applying any DT overlay
>> at run time but can be done (we are missing one missing piece in U-Boot for
>> it).
>>
>> In presentation you mentioned also that applying overlay can fail  but not
>> sure how you can reach it. Because Linux kernel has the whole infrastructure
>> to cover all combinations with base DT + overlays. It means if you cover all
>> working combinations there you should see if they don't apply properly.
> 
> Mostly, I was referring to a situation where firmware provides an
> overlay. Firmware doesn't know the DTB that OS has and I don't see any
> way to gaurantee that firmware knows how to fix up the OS DTB.

In our case firmware is providing DTB to OS (own or via bootstript).
99% of our DTSes are generated via our Device Tree Generator with connection to 
HW design which is used. And it is pretty much unique based on used 
configuration that's why I don't think we get to situation that OS will provide 
correct DT for us.
That also means that we are not doing fixups in firmware because that fixups are 
coming to device tree generator aligned with particular releases.

> 
>>
>> Also do you really need to detect everything from firmware side? Or isn't it
>> enough to have just "some" devices and then load the rest where you are in
>> OS?
>> I think that's pretty much another way to go to have bare minimum
>> functionality provided by firmware and deal with the rest in OS.
> 
> Agreed, although not all devices can be loaded once you are in the OS.
> All nondiscoverable devices would need to be desribed in the DT.

 From my perspective IDs you are talking about can be find via different 
channels then just reading it via DT. They can be passed but we simply just read 
our EEPROMs again and also chipid via nvmem in OS and used that information for 
dealing with DT overlays.

I don't think that we have the same need as you have but it is pretty much 
because our programmable logic needs to be handled differently.

Thanks,
Michal

Re: [PATCH RFC v3 0/9] dt-bindings: hwinfo: Introduce board-id

[Simon Glass]

Hi Elliot,

On Tue, 21 May 2024 at 12:38, Elliot Berman <quic_eberman@quicinc.com> wrote:
>
> Device manufacturers frequently ship multiple boards or SKUs under a
> single software package. These software packages will ship multiple
> devicetree blobs and require some mechanism to pick the correct DTB for
> the board the software package was deployed. Introduce a common
> definition for adding board identifiers to device trees. board-id
> provides a mechanism for bootloaders to select the appropriate DTB which
> is vendor/OEM-agnostic.
>
> This series is based off a talk I gave at EOSS NA 2024 [1]. There is
> some further discussion about how to do devicetree selection in the
> boot-architecture mailing list [2].
>
> [1]: https://sched.co/1aBFy
> [2]: https://lists.linaro.org/archives/list/boot-architecture@lists.linaro.org/thread/DZCZSOCRH5BN7YOXEL2OQKSDIY7DCW2M/
>
> Quick summary
> -------------
> This series introduces a new subnode in the root:
> / {
>         board-id {
>                 some-hw-id = <value>;
>                 other-hw-id = <val1>, <val2>;
>         };
> };
>
> Firmware provides a mechanism to fetch the values of "some-hw-id" and
> "other-hw-id" based on the property name. I'd like to leave exact
> mechanism data out of the scope of this proposal to keep this proposal
> flexible because it seems architecture specific, although I think we we
> should discuss possible approaches. A DTB matches if firmware can
> provide a matching value for every one of the properties under
> /board-id. In the above example, val1 and val2 are both valid values and
> firmware only provides the one that actually describes the board.
>
> It's expected that devicetree's board-id don't describe all the
> properties firmware could provide. For instance, a devicetree overlay
> may only care about "other-hw-id" and not "some-hw-id". Thus, it need
> only mention "other-hw-id" in its board-id node.
>
> Isn't that what the compatible property is for?
> -----------------------------------------------
> The compatible property can be used for board matching, but requires
> bootloaders and/or firmware to maintain a database of possible strings
> to match against or implement complex compatible string matching.
> Compatible string matching becomes complicated when there are multiple
> versions of board: the device tree selector should recognize a DTB that
> cares to distinguish between v1/v2 and a DTB that doesn't make the
> distinction.  An eeprom either needs to store the compatible strings
> that could match against the board or the bootloader needs to have
> vendor-specific decoding logic for the compatible string. Neither
> increasing eeprom storage nor adding vendor-specific decoding logic is
> desirable.

That is not necessary, though. The compatible string should be enough.

>
> How is this better than Qualcomm's qcom,msm-id/qcom,board-id?
> -------------------------------------------------------------
> The selection process for devicetrees was Qualcomm-specific and not
> useful for other devices and bootloaders that were not developed by
> Qualcomm because a complex algorithm was used to implement. Board-ids
> provide a matching solution that can be implemented by bootloaders
> without introducing vendor-specific code. Qualcomm uses three
> devicetree properties: msm-id (interchangeably: soc-id), board-id, and
> pmic-id.  This does not scale well for use casese which use identifiers,
> for example, to distinguish between a display panel. For a display
> panel, an approach could be to add a new property: display-id, but now
> bootloaders need to be updated to also read this property. We want to
> avoid requiring to update bootloaders with new hardware identifiers: a
> bootloader need only recognize the identifiers it can handle.
>
> Notes about the patches
> -----------------------
> In my opinion, most of the patches in this series should be submitted to
> libfdt and/or dtschema project. I've made them apply on the kernel tree
> to be easier for other folks to pick them up and play with them. As the
> patches evolve, I can send them to the appropriate projects.
>
> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> ---
> Changes in v3:
>  - Follow new "/board-id {}" approach, which uses key-value pairs
>  - Add match algorithm in libfdt and some examples to demo how the
>    selection could work in tools/board-id
>
> Changes in V2:
>  - Addressed few comments related to board-id, and DDR type.
>  - Link to V2:  https://lore.kernel.org/all/a930a3d6-0846-a709-8fe9-44335fec92ca@quicinc.com/#r
>
> ---
> Amrit Anand (1):
>       dt-bindings: arm: qcom: Update Devicetree identifiers
>
> Elliot Berman (8):
>       libfdt: board-id: Implement board-id scoring
>       dt-bindings: board: Introduce board-id
>       fdt-select-board: Add test tool for selecting dtbs based on board-id
>       dt-bindings: board: Document board-ids for Qualcomm devices
>       arm64: boot: dts: sm8650: Add board-id
>       arm64: boot: dts: qcom: Use phandles for thermal_zones
>       arm64: boot: dts: qcom: sm8550: Split into overlays
>       tools: board-id: Add test suite
>
>  .../devicetree/bindings/board/board-id.yaml        |  24 ++++
>  .../devicetree/bindings/board/qcom,board-id.yaml   | 144 ++++++++++++++++++++
>  arch/arm64/boot/dts/qcom/Makefile                  |   4 +
>  arch/arm64/boot/dts/qcom/pm8010.dtsi               |  62 ++++-----
>  arch/arm64/boot/dts/qcom/pm8550.dtsi               |  32 ++---
>  arch/arm64/boot/dts/qcom/pm8550b.dtsi              |  36 +++--
>  arch/arm64/boot/dts/qcom/pm8550ve.dtsi             |  38 +++---
>  arch/arm64/boot/dts/qcom/pm8550vs.dtsi             | 128 +++++++++--------
>  arch/arm64/boot/dts/qcom/pmr735d_a.dtsi            |  38 +++---
>  arch/arm64/boot/dts/qcom/pmr735d_b.dtsi            |  38 +++---
>  .../dts/qcom/{sm8550-mtp.dts => sm8550-mtp.dtso}   |  24 +++-
>  .../dts/qcom/{sm8550-qrd.dts => sm8550-qrd.dtso}   |  22 ++-
>  .../boot/dts/qcom/{sm8550.dtsi => sm8550.dts}      |  10 +-
>  arch/arm64/boot/dts/qcom/sm8650-mtp.dts            |   6 +
>  arch/arm64/boot/dts/qcom/sm8650-qrd.dts            |   6 +
>  arch/arm64/boot/dts/qcom/sm8650.dtsi               |   2 +-
>  include/dt-bindings/arm/qcom,ids.h                 |  86 ++++++++++--
>  scripts/dtc/.gitignore                             |   1 +
>  scripts/dtc/Makefile                               |   3 +-
>  scripts/dtc/fdt-select-board.c                     | 126 +++++++++++++++++
>  scripts/dtc/libfdt/fdt_ro.c                        |  76 +++++++++++
>  scripts/dtc/libfdt/libfdt.h                        |  54 ++++++++
>  tools/board-id/test.py                             | 151 +++++++++++++++++++++
>  23 files changed, 901 insertions(+), 210 deletions(-)
> ---
> base-commit: e8f897f4afef0031fe618a8e94127a0934896aba
> change-id: 20240112-board-ids-809ff0281ee5
>
> Best regards,
> --
> Elliot Berman <quic_eberman@quicinc.com>
>

I am just picking up the discussion here, which was started on another thread.

I can't see why this new feature is needed. We should be able to use
compatible strings, as we do now. I added a 'usage' section to the FIT
spec [1] which might help. I also incorporated the board revision and
variant information and some notes on how to add to the available
suffixes.

Does that handle your use case?

Regards,
Simon

[1] https://github.com/open-source-firmware/flat-image-tree/blob/main/source/chapter3-usage.rst

Re: [PATCH RFC v3 0/9] dt-bindings: hwinfo: Introduce board-id

[Elliot Berman]

On Wed, Jun 05, 2024 at 07:17:35AM -0600, Simon Glass wrote:
> Hi Elliot,
> 
> I am just picking up the discussion here, which was started on another thread.
> 
> I can't see why this new feature is needed. We should be able to use
> compatible strings, as we do now. I added a 'usage' section to the FIT
> spec [1] which might help. I also incorporated the board revision and
> variant information and some notes on how to add to the available
> suffixes.
> 
> Does that handle your use case?

-rev and -sku don't fit the versioning scheme for QTI devices, so this
isn't a generic enough approach. Patch 5 in this series describes the
versioning scheme for us.

In the other thread, we had talked about using some regex based approach
for matching the root node compatible. I haven't had chance to work on
that proposal and will try to get to it in the next couple weeks.

Thanks,
Elliot

Re: [PATCH RFC v3 0/9] dt-bindings: hwinfo: Introduce board-id

[Simon Glass]

Hi Elliot,

On Wed, 5 Jun 2024 at 11:17, Elliot Berman <quic_eberman@quicinc.com> wrote:
>
> On Wed, Jun 05, 2024 at 07:17:35AM -0600, Simon Glass wrote:
> > Hi Elliot,
> >
> > I am just picking up the discussion here, which was started on another thread.
> >
> > I can't see why this new feature is needed. We should be able to use
> > compatible strings, as we do now. I added a 'usage' section to the FIT
> > spec [1] which might help. I also incorporated the board revision and
> > variant information and some notes on how to add to the available
> > suffixes.
> >
> > Does that handle your use case?
>
> -rev and -sku don't fit the versioning scheme for QTI devices, so this
> isn't a generic enough approach. Patch 5 in this series describes the
> versioning scheme for us.
>
> In the other thread, we had talked about using some regex based approach
> for matching the root node compatible. I haven't had chance to work on
> that proposal and will try to get to it in the next couple weeks.

OK, I look forward to it. Please do check the FIT best match approach
and see how it might be extended to handle your requirements. So far I
have not seen a need for regexes, but it is certainly a possibility.

Regards,
Simon

Re: [PATCH RFC v3 0/9] dt-bindings: hwinfo: Introduce board-id

[Elliot Berman]

Hi Simon,

On Thu, Jun 06, 2024 at 10:00:54AM -0600, Simon Glass wrote:
> On Wed, 5 Jun 2024 at 11:17, Elliot Berman <quic_eberman@quicinc.com> wrote:
> > On Wed, Jun 05, 2024 at 07:17:35AM -0600, Simon Glass wrote:
> > > Hi Elliot,
> > >
> > > I am just picking up the discussion here, which was started on another thread.
> > >
> > > I can't see why this new feature is needed. We should be able to use
> > > compatible strings, as we do now. I added a 'usage' section to the FIT
> > > spec [1] which might help. I also incorporated the board revision and
> > > variant information and some notes on how to add to the available
> > > suffixes.
> > >
> > > Does that handle your use case?
> >
> > -rev and -sku don't fit the versioning scheme for QTI devices, so this
> > isn't a generic enough approach. Patch 5 in this series describes the
> > versioning scheme for us.
> >
> > In the other thread, we had talked about using some regex based approach
> > for matching the root node compatible. I haven't had chance to work on
> > that proposal and will try to get to it in the next couple weeks.
> 
> OK, I look forward to it. Please do check the FIT best match approach
> and see how it might be extended to handle your requirements. So far I
> have not seen a need for regexes, but it is certainly a possibility.
> 

I spent some time collecting feedback from the team on using compatible
strings + regex-style approach and we're not able to add a regex library
into firmware, so this approach unfortunately won't work for us.
Because we have more axes of board identification than chromebook, using
FIT's compatible strings isn't a scalable solution for us. I don't think
we have incompatible problems, we only have more than 2-3 axes of
information.

Thanks,
Elliot

Re: [PATCH RFC v3 0/9] dt-bindings: hwinfo: Introduce board-id

[Dmitry Baryshkov]

On Fri, Jun 21, 2024 at 03:40:20PM GMT, Elliot Berman wrote:
> Hi Simon,
> 
> On Thu, Jun 06, 2024 at 10:00:54AM -0600, Simon Glass wrote:
> > On Wed, 5 Jun 2024 at 11:17, Elliot Berman <quic_eberman@quicinc.com> wrote:
> > > On Wed, Jun 05, 2024 at 07:17:35AM -0600, Simon Glass wrote:
> > > > Hi Elliot,
> > > >
> > > > I am just picking up the discussion here, which was started on another thread.
> > > >
> > > > I can't see why this new feature is needed. We should be able to use
> > > > compatible strings, as we do now. I added a 'usage' section to the FIT
> > > > spec [1] which might help. I also incorporated the board revision and
> > > > variant information and some notes on how to add to the available
> > > > suffixes.
> > > >
> > > > Does that handle your use case?
> > >
> > > -rev and -sku don't fit the versioning scheme for QTI devices, so this
> > > isn't a generic enough approach. Patch 5 in this series describes the
> > > versioning scheme for us.
> > >
> > > In the other thread, we had talked about using some regex based approach
> > > for matching the root node compatible. I haven't had chance to work on
> > > that proposal and will try to get to it in the next couple weeks.
> > 
> > OK, I look forward to it. Please do check the FIT best match approach
> > and see how it might be extended to handle your requirements. So far I
> > have not seen a need for regexes, but it is certainly a possibility.
> > 
> 
> I spent some time collecting feedback from the team on using compatible
> strings + regex-style approach and we're not able to add a regex library
> into firmware, so this approach unfortunately won't work for us.

Why? What is the size growth caused by the RegularExpressionDxe ?

> Because we have more axes of board identification than chromebook, using
> FIT's compatible strings isn't a scalable solution for us. I don't think
> we have incompatible problems, we only have more than 2-3 axes of
> information.

Well, not using compatibles / strings results in most of the phone
vendors having just the 'MTP' as their platform id. It makes then
impossible to have an image with several DTB files targeting different
phone families from several vendors. What looks like a nice feature for
MTP vs QRD vs HDK becomes useless with the end-user devices.

-- 
With best wishes
Dmitry

Re: [PATCH RFC v3 0/9] dt-bindings: hwinfo: Introduce board-id

[Simon Glass]

Hi Elliot,

On Fri, 21 Jun 2024 at 23:40, Elliot Berman <quic_eberman@quicinc.com> wrote:
>
> Hi Simon,
>
> On Thu, Jun 06, 2024 at 10:00:54AM -0600, Simon Glass wrote:
> > On Wed, 5 Jun 2024 at 11:17, Elliot Berman <quic_eberman@quicinc.com> wrote:
> > > On Wed, Jun 05, 2024 at 07:17:35AM -0600, Simon Glass wrote:
> > > > Hi Elliot,
> > > >
> > > > I am just picking up the discussion here, which was started on another thread.
> > > >
> > > > I can't see why this new feature is needed. We should be able to use
> > > > compatible strings, as we do now. I added a 'usage' section to the FIT
> > > > spec [1] which might help. I also incorporated the board revision and
> > > > variant information and some notes on how to add to the available
> > > > suffixes.
> > > >
> > > > Does that handle your use case?
> > >
> > > -rev and -sku don't fit the versioning scheme for QTI devices, so this
> > > isn't a generic enough approach. Patch 5 in this series describes the
> > > versioning scheme for us.
> > >
> > > In the other thread, we had talked about using some regex based approach
> > > for matching the root node compatible. I haven't had chance to work on
> > > that proposal and will try to get to it in the next couple weeks.
> >
> > OK, I look forward to it. Please do check the FIT best match approach
> > and see how it might be extended to handle your requirements. So far I
> > have not seen a need for regexes, but it is certainly a possibility.
> >
>
> I spent some time collecting feedback from the team on using compatible
> strings + regex-style approach and we're not able to add a regex library
> into firmware, so this approach unfortunately won't work for us.
> Because we have more axes of board identification than chromebook, using
> FIT's compatible strings isn't a scalable solution for us. I don't think
> we have incompatible problems, we only have more than 2-3 axes of
> information.

I understand that. I assume that you have a lot of devices that use
the same SoC but different PMICs, displays, etc. Some of these can be
handled in the bootloader, e.g. by detecting hardware and applying an
overlay, or enabling/disabling a node in the DT. It can be useful to
have a hardware-readable ID to indicate things which cannot be probed,
e.g. with GPIOs or ADC + resistors.

Another option is to give names to your projects, so that machines
with the same SoC but major hardware differences end up with a
different name (see rk3399-xx.dts for examples).

I'm sure you are already doing some/all of these things. I still feel
(so far) that you don't need to invent something new here.

Re "FIT's compatible strings isn't a scalable solution for us", how is
what you are doing different from other vendors? Is it the sheer
number of variations, or something else?

Regards,
Simon

Re: [PATCH RFC v3 0/9] dt-bindings: hwinfo: Introduce board-id

[Simon Glass]

On Fri, 28 Jun 2024 at 08:33, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Elliot,
>
> On Fri, 21 Jun 2024 at 23:40, Elliot Berman <quic_eberman@quicinc.com> wrote:
> >
> > Hi Simon,
> >
> > On Thu, Jun 06, 2024 at 10:00:54AM -0600, Simon Glass wrote:
> > > On Wed, 5 Jun 2024 at 11:17, Elliot Berman <quic_eberman@quicinc.com> wrote:
> > > > On Wed, Jun 05, 2024 at 07:17:35AM -0600, Simon Glass wrote:
> > > > > Hi Elliot,
> > > > >
> > > > > I am just picking up the discussion here, which was started on another thread.
> > > > >
> > > > > I can't see why this new feature is needed. We should be able to use
> > > > > compatible strings, as we do now. I added a 'usage' section to the FIT
> > > > > spec [1] which might help. I also incorporated the board revision and
> > > > > variant information and some notes on how to add to the available
> > > > > suffixes.
> > > > >
> > > > > Does that handle your use case?
> > > >
> > > > -rev and -sku don't fit the versioning scheme for QTI devices, so this
> > > > isn't a generic enough approach. Patch 5 in this series describes the
> > > > versioning scheme for us.
> > > >
> > > > In the other thread, we had talked about using some regex based approach
> > > > for matching the root node compatible. I haven't had chance to work on
> > > > that proposal and will try to get to it in the next couple weeks.
> > >
> > > OK, I look forward to it. Please do check the FIT best match approach
> > > and see how it might be extended to handle your requirements. So far I
> > > have not seen a need for regexes, but it is certainly a possibility.
> > >
> >
> > I spent some time collecting feedback from the team on using compatible
> > strings + regex-style approach and we're not able to add a regex library
> > into firmware, so this approach unfortunately won't work for us.
> > Because we have more axes of board identification than chromebook, using
> > FIT's compatible strings isn't a scalable solution for us. I don't think
> > we have incompatible problems, we only have more than 2-3 axes of
> > information.
>
> I understand that. I assume that you have a lot of devices that use
> the same SoC but different PMICs, displays, etc. Some of these can be
> handled in the bootloader, e.g. by detecting hardware and applying an
> overlay, or enabling/disabling a node in the DT. It can be useful to
> have a hardware-readable ID to indicate things which cannot be probed,
> e.g. with GPIOs or ADC + resistors.
>
> Another option is to give names to your projects, so that machines
> with the same SoC but major hardware differences end up with a
> different name (see rk3399-xx.dts for examples).
>
> I'm sure you are already doing some/all of these things. I still feel
> (so far) that you don't need to invent something new here.
>
> Re "FIT's compatible strings isn't a scalable solution for us", how is
> what you are doing different from other vendors? Is it the sheer
> number of variations, or something else?

Here I am referring to the actual number of separate boards which
appear in the wild, not the multiplication of the number of axes which
produced them.