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/[email protected]/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 <[email protected]>
---
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/[email protected]/#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 <[email protected]>
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 <[email protected]>
---
.../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 <[email protected]>
+
+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
In preparation for converting sm8550 to use devicetree overlays, use
phandles for thermal_zones.
Signed-off-by: Elliot Berman <[email protected]>
---
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
Hi Elliot,
On Tue, 21 May 2024 at 21:41, Elliot Berman <[email protected]> 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/[email protected]/thread/DZCZSOCRH5BN7YOXEL2OQKSDIY7DCW2M/
>
--
With best wishes
Dmitry
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 <[email protected]>
> ---
> .../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/[email protected]
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.
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 <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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.
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 <[email protected]>
> > ---
> > .../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 <[email protected]>
> > +
> > +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.
On Tue, May 21, 2024 at 2:25 PM Conor Dooley <[email protected]> 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 <[email protected]>
> > > ---
> > > .../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 <[email protected]>
> > > +
> > > +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/[email protected]/thread/DZCZSOCRH5BN7YOXEL2OQKSDIY7DCW2M/
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/[email protected]/thread/DZCZSOCRH5BN7YOXEL2OQKSDIY7DCW2M/
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 <[email protected]>
> > ---
> > .../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 <[email protected]>
> > +
> > +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
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 <[email protected]>
---
.../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 <[email protected]>
+
+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
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 <[email protected]>
> ---
> .../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/[email protected]
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.
On 21.05.2024 9:00 PM, Dmitry Baryshkov wrote:
> Hi Elliot,
>
> On Tue, 21 May 2024 at 21:41, Elliot Berman <[email protected]> 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
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 <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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
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 <[email protected]> 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
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 <[email protected]> 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
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 <[email protected]>
> > ---
> > .../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 <[email protected]>
> > +
> > +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
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 <[email protected]> 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
Hi Elliot,
On Tue, 21 May 2024 at 12:38, Elliot Berman <[email protected]> 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/[email protected]/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 <[email protected]>
> ---
> 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/[email protected]/#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 <[email protected]>
>
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
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
Hi Elliot,
On Wed, 5 Jun 2024 at 11:17, Elliot Berman <[email protected]> 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