[PATCH 0/4] ARM: dts: aspeed: x570d4u: devicetree fixes and cleanups

[PATCH 0/4] ARM: dts: aspeed: x570d4u: devicetree fixes and cleanups

[Renze Nicolai]

A handful of independent fixes and cleanups for the ASRock X570D4U
BMC devicetree, found while working on OpenBMC support for the board:

 - Remove the w83773g temperature sensor node, which is only present
   on 10GbE variants of the board and should not be instantiated
   unconditionally.
 - Fix the tachometer channel assignments for FAN4-FAN6, which did
   not match the board's actual PWM wiring and produced incorrect
   fan speed readings.
 - Give the eth0/eth1 MAC address nvmem cells unique node names, since
   they previously both resolved to the same NVMEM cell name once the
   unit address is stripped.
 - Add gpio-line-names entries for a few GPIO lines (NMI, BIOS
   chip-select, LPC reset, hard power-off) that were previously
   unnamed.

Renze Nicolai (4):
  ARM: dts: aspeed: x570d4u: Remove w83773g temperature sensor
  ARM: dts: aspeed: x570d4u: Fix fan tach channel assignments
  ARM: dts: aspeed: x570d4u: Give unique names to MAC address nvmem
    cells
  ARM: dts: aspeed: x570d4u: Add some GPIO line names

 .../dts/aspeed/aspeed-bmc-asrock-x570d4u.dts  | 25 ++++++++-----------
 1 file changed, 10 insertions(+), 15 deletions(-)

-- 
2.56.0

[PATCH 1/4] ARM: dts: aspeed: x570d4u: Remove w83773g temperature sensor

[Renze Nicolai]

The w83773g temperature sensor is only available on variants
of the X570D4U with 10GBit ethernet and should only be loaded
after checking the board variant information in the FRU EEPROM.

Signed-off-by: Renze Nicolai <renze@rnplus.nl>
---
 arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
index e61a6cb43438..7d27833b761d 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
@@ -164,11 +164,6 @@ &i2c0 {
 &i2c1 {
 	/* Hardware monitoring SMBus */
 	status = "okay";
-
-	w83773g@4c {
-		compatible = "nuvoton,w83773g";
-		reg = <0x4c>;
-	};
 };
 
 &i2c2 {
-- 
2.56.0

[PATCH 2/4] ARM: dts: aspeed: x570d4u: Fix fan tach channel assignments

[Renze Nicolai]

The tachometer channel numbers for FAN4, FAN5 and FAN6 did not match
the board's actual PWM wiring, causing incorrect fan speed readings.
FAN6 is wired to PWM5 and FAN5 is wired to PWM6 (not PWM6/PWM5
respectively as previously assigned), and FAN4's tach channel was
off by one.

Signed-off-by: Renze Nicolai <renze@rnplus.nl>
---
 arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
index 7d27833b761d..6e7e3e7ec101 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
@@ -320,19 +320,19 @@ fan@2 {
 	fan@3 {
 		/* FAN4 (6-pin) */
 		reg = <0x03>;
-		aspeed,fan-tach-ch = /bits/ 8 <0x04 0x0b>;
+		aspeed,fan-tach-ch = /bits/ 8 <0x03 0x0b>;
 	};
 
 	fan@4 {
-		/* FAN6 (6-pin) */
+		/* FAN6 (6-pin), wired to PWM5 */
 		reg = <0x04>;
-		aspeed,fan-tach-ch = /bits/ 8 <0x06 0x0d>;
+		aspeed,fan-tach-ch = /bits/ 8 <0x05 0x0d>;
 	};
 
 	fan@5 {
-		/* FAN5 (6-pin) */
+		/* FAN5 (6-pin), wired to PWM6 */
 		reg = <0x05>;
-		aspeed,fan-tach-ch = /bits/ 8 <0x05 0x0c>;
+		aspeed,fan-tach-ch = /bits/ 8 <0x04 0x0c>;
 	};
 };
 
-- 
2.56.0

[PATCH 3/4] ARM: dts: aspeed: x570d4u: Give unique names to MAC address nvmem cells

[Renze Nicolai]

The eth0 and eth1 MAC address cells in the eeprom's nvmem-layout were
both named "macaddress" (differing only by unit address), so their
NVMEM cell names collided once the unit address is stripped. Rename
them to eth0_macaddress@3f80 and eth1_macaddress@3f88 to match their
labels and make each cell uniquely addressable by name.

Signed-off-by: Renze Nicolai <renze@rnplus.nl>
---
 arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
index 6e7e3e7ec101..fca14320829e 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
@@ -233,11 +233,11 @@ nvmem-layout {
 			#address-cells = <1>;
 			#size-cells = <1>;
 
-			eth0_macaddress: macaddress@3f80 {
+			eth0_macaddress: eth0_macaddress@3f80 {
 				reg = <0x3f80 6>;
 			};
 
-			eth1_macaddress: macaddress@3f88 {
+			eth1_macaddress: eth1_macaddress@3f88 {
 				reg = <0x3f88 6>;
 			};
 		};
-- 
2.56.0

[PATCH 4/4] ARM: dts: aspeed: x570d4u: Add some GPIO line names

[Renze Nicolai]

Several GPIO lines used on the board had no gpio-line-names entry.
Name the NMI button and control lines (GPIOA2/A3), the BIOS flash
chip-select and LPC reset outputs (GPIOJ1/J3), and the hard
power-off control line (GPIOAB3).

Signed-off-by: Renze Nicolai <renze@rnplus.nl>
---
 arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
index fca14320829e..0089845a6f0b 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
@@ -79,7 +79,7 @@ iio-hwmon {
 &gpio {
 	status = "okay";
 	gpio-line-names =
-	/*  A */ "input-locatorled-n", "", "", "", "", "", "", "",
+	/*  A */ "input-locatorled-n", "", "button-nmi-n", "control-nmi-n", "", "", "", "",
 	/*  B */ "input-bios-post-cmplt-n", "", "", "", "", "", "", "",
 	/*  C */ "", "", "", "", "", "", "control-locatorbutton-n", "",
 	/*  D */ "button-power-n", "control-power-n", "button-reset-n",
@@ -92,7 +92,7 @@ &gpio {
 	/*  H */ "", "", "", "", "input-mfg-mode-n", "",
 		 "led-heartbeat-n", "input-case-open-n",
 	/*  I */ "", "", "", "", "", "", "", "",
-	/*  J */ "output-bmc-ready-n", "", "", "", "", "", "", "",
+	/*  J */ "output-bmc-ready-n", "output-bios-cs-n", "", "output-lpc-reset", "", "", "", "",
 	/*  K */ "", "", "", "", "", "", "", "",
 	/*  L */ "", "", "", "", "", "", "", "",
 	/*  M */ "", "", "", "", "", "", "", "",
@@ -114,7 +114,7 @@ &gpio {
 		 "", "",
 	/* AA */ "input-cpu1-thermtrip-latch-n", "",
 		 "input-cpu1-prochot-n", "", "", "", "", "",
-	/* AB */ "", "input-power-good", "", "", "", "", "", "",
+	/* AB */ "", "input-power-good", "", "control-hard-power-off", "", "", "", "",
 	/* AC */ "", "", "", "", "", "", "", "";
 };
 
-- 
2.56.0

Re: [PATCH 3/4] ARM: dts: aspeed: x570d4u: Give unique names to MAC address nvmem cells

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] ARM: dts: aspeed: x570d4u: invalid underscores in nvmem cell node names
--

commit 48dfc9aa6b8fe87b8ac64b7c75b8d6bd5fea2a5f
Author: Renze Nicolai <renze@rnplus.nl>

ARM: dts: aspeed: x570d4u: Give unique names to MAC address nvmem cells

This commit renames the two MAC address nvmem-layout child nodes in
aspeed-bmc-asrock-x570d4u.dts from macaddress@3f80 and macaddress@3f88 to
eth0_macaddress@3f80 and eth1_macaddress@3f88.

> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
> index 6e7e3e7ec1019..fca14320829e1 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-asrock-x570d4u.dts
> @@ -233,11 +233,11 @@ nvmem-layout {
>  			#address-cells = <1>;
>  			#size-cells = <1>;
>  
> -			eth0_macaddress: macaddress@3f80 {
> +			eth0_macaddress: eth0_macaddress@3f80 {
>  				reg = <0x3f80 6>;
>  			};
>  
> -			eth1_macaddress: macaddress@3f88 {
> +			eth1_macaddress: eth1_macaddress@3f88 {

[Severity: Low]
Does renaming these nodes in nvmem-layout to use underscores introduce dtc
W=2 warnings and violate the Devicetree coding style?

Per Documentation/devicetree/bindings/dts-coding-style.rst, node names should
use lowercase letters, digits, and hyphens (-), whereas underscores (_) are
only meant for Devicetree labels. When building DTBs with W=2
(-Wnode_name_chars_strict), check_node_name_chars_strict() in
scripts/dtc/checks.c rejects underscores in node basenames:

	if (n < node->basenamelen)
		FAIL(c, dti, node, "Character '%c' not recommended in node name",
		     node->name[n]);

Also, does the name collision described in the commit message actually occur
for DT-defined cells? In drivers/nvmem/core.c, DT consumers resolve
nvmem-cells phandles via of_nvmem_cell_get(), which matches by device_node
pointer rather than cell->name:

	cell_entry = nvmem_find_cell_entry_by_node(nvmem, cell_np);

And in sysfs, nvmem_populate_sysfs_cells() includes the offset and bit_offset
in the attribute filename, producing macaddress@3f80,0 and macaddress@3f88,0:

		attrs[i].attr.name = devm_kasprintf(&nvmem->dev, GFP_KERNEL,
						    "%s@%x,%x", entry->name,
						    entry->offset,
						    entry->bit_offset);

If distinct node basenames are still desired here, should hyphens be used
instead (such as eth0-macaddress@3f80 and eth1-macaddress@3f88)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103203308.54789-1-renze@rnplus.nl?part=3