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

[sashiko-bot@kernel.org]

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