diff mbox series

[5/6] of: Use the standards compliant default address cells value for x86

Message ID 20241104172001.165640-6-herve.codina@bootlin.com (mailing list archive)
State New
Headers show
Series Add support for the root PCI bus device-tree node creation. | expand

Commit Message

Herve Codina Nov. 4, 2024, 5:19 p.m. UTC
The default address cells value is 1.

According to the devicetree specification and the OpenFirmware standard
(IEEE 1275-1994) this default value should be 2.

The device tree compiler already use 2 as default value and the powerpc
PROM code also use 2 as default value. Modern implementation should have
the #address-cells property set and should not rely on this default
value.

On x86, an empty root device-tree node is created but as the hardware
is not described by a device-tree passed by the bootloader, this root
device-tree remains empty and so the #address-cells default value is
used.

In preparation of the support for device-tree overlay on PCI devices
feature on x86 (i.e. the creation of the PCI root bus device-tree node),
this default value needs to be updated. Indeed, on x86_64, addresses are
on 64bits and the upper part of an address is needed for correct address
translations. On x86_32 having the default value updated does not lead
to issues while the uppert part of a 64bits address is zero.

Changing the default value for all architectures may break device-tree
compatibility. Indeed, existing dts file without the #address-cells
property set in the root node will not be compatible with this
modification. Restrict the modification to the x86 architecture.

Instead of having 1 for this default value, be consistent with standards
and set the default address cells value to 2 in case of x86.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/of/of_private.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Rob Herring Nov. 4, 2024, 8:07 p.m. UTC | #1
On Mon, Nov 4, 2024 at 11:20 AM Herve Codina <herve.codina@bootlin.com> wrote:
>
> The default address cells value is 1.
>
> According to the devicetree specification and the OpenFirmware standard
> (IEEE 1275-1994) this default value should be 2.
>
> The device tree compiler already use 2 as default value and the powerpc
> PROM code also use 2 as default value. Modern implementation should have
> the #address-cells property set and should not rely on this default
> value.

It's a mess.

Relying on defaults and inheriting from parent nodes in dtc has been a
warning as far back as I could trace (2005-2006).


> On x86, an empty root device-tree node is created but as the hardware
> is not described by a device-tree passed by the bootloader, this root
> device-tree remains empty and so the #address-cells default value is
> used.
>
> In preparation of the support for device-tree overlay on PCI devices
> feature on x86 (i.e. the creation of the PCI root bus device-tree node),
> this default value needs to be updated. Indeed, on x86_64, addresses are
> on 64bits and the upper part of an address is needed for correct address
> translations. On x86_32 having the default value updated does not lead
> to issues while the uppert part of a 64bits address is zero.
>
> Changing the default value for all architectures may break device-tree
> compatibility. Indeed, existing dts file without the #address-cells
> property set in the root node will not be compatible with this
> modification. Restrict the modification to the x86 architecture.
>
> Instead of having 1 for this default value, be consistent with standards
> and set the default address cells value to 2 in case of x86.

I prefer the default to just be wrong. My intention is to get rid of
the defaults (at least for all FDT, not OF) and walking up parent
nodes. My first step was to add warnings[1] and see who complained.

The base tree needs to be populated with #address-cells/#size-cells.
I'd be fine if we say those are always 2 to keep it simple.

Rob

[1] https://lore.kernel.org/all/20240530185049.2851617-1-robh@kernel.org/
diff mbox series

Patch

diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 04aa2a91f851..d8353f04af0a 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -29,7 +29,7 @@  struct alias_prop {
 	char stem[];
 };
 
-#if defined(CONFIG_SPARC)
+#if defined(CONFIG_SPARC) || defined(CONFIG_X86)
 #define OF_ROOT_NODE_ADDR_CELLS_DEFAULT 2
 #else
 #define OF_ROOT_NODE_ADDR_CELLS_DEFAULT 1