linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] PCI: of: Downgrade error message on missing of_root node
@ 2025-11-05 18:33 Andrea della Porta
  2025-11-06  0:23 ` Bjorn Helgaas
  0 siblings, 1 reply; 10+ messages in thread
From: Andrea della Porta @ 2025-11-05 18:33 UTC (permalink / raw)
  To: Bjorn Helgaas, linux-pci, linux-kernel, mbrugger,
	guillaume.gardet, tiwai
  Cc: Andrea della Porta

When CONFIG_PCI_DYNAMIC_OF_NODES is enabled, an error message
is generated if no 'of_root' node is defined.

On DT-based systems, this cannot happen as a root DT node is
always present. On ACPI-based systems, this is not a true error
because a DT is not used.

Downgrade the pr_err() to pr_info() and reword the message text
to be less context specific.

Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
---
CHANGES in V2:

* message text reworded to be less context specific (Bjorn)
---
 drivers/pci/of.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 3579265f1198..872c36b195e3 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -775,7 +775,7 @@ void of_pci_make_host_bridge_node(struct pci_host_bridge *bridge)
 
 	/* Check if there is a DT root node to attach the created node */
 	if (!of_root) {
-		pr_err("of_root node is NULL, cannot create PCI host bridge node\n");
+		pr_info("Missing DeviceTree, cannot create PCI host bridge node\n");
 		return;
 	}
 
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] PCI: of: Downgrade error message on missing of_root node
  2025-11-05 18:33 [PATCH v2] PCI: of: Downgrade error message on missing of_root node Andrea della Porta
@ 2025-11-06  0:23 ` Bjorn Helgaas
  2025-11-06 11:04   ` Andrea della Porta
  0 siblings, 1 reply; 10+ messages in thread
From: Bjorn Helgaas @ 2025-11-06  0:23 UTC (permalink / raw)
  To: Andrea della Porta
  Cc: Bjorn Helgaas, linux-pci, linux-kernel, mbrugger,
	guillaume.gardet, tiwai, Lizhi Hou

[+cc Lizhi]

On Wed, Nov 05, 2025 at 07:33:40PM +0100, Andrea della Porta wrote:
> When CONFIG_PCI_DYNAMIC_OF_NODES is enabled, an error message
> is generated if no 'of_root' node is defined.
> 
> On DT-based systems, this cannot happen as a root DT node is
> always present. On ACPI-based systems, this is not a true error
> because a DT is not used.
> 
> Downgrade the pr_err() to pr_info() and reword the message text
> to be less context specific.

of_pci_make_host_bridge_node() is called in the very generic
pci_register_host_bridge() path.  Does that mean every boot of a
kernel with CONFIG_PCI_DYNAMIC_OF_NODES on a non-DT system will see
this message?

This message seems like something that will generate user questions.
Or is this really an error, and we were supposed to have created
of_root somewhere but it failed?  If so, I would expect a message
where the of_root creation failed.

I guess I'm confused about what the point of this message is.  If it's
just a hint that loading an overlay in the future will fail, I assume
we would emit a message at that time, connected with the user action
of trying to load the overlay.

What badness would ensue if we downgraded this message even further
and removed it completely?

> Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> ---
> CHANGES in V2:
> 
> * message text reworded to be less context specific (Bjorn)
> ---
>  drivers/pci/of.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index 3579265f1198..872c36b195e3 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -775,7 +775,7 @@ void of_pci_make_host_bridge_node(struct pci_host_bridge *bridge)
>  
>  	/* Check if there is a DT root node to attach the created node */
>  	if (!of_root) {
> -		pr_err("of_root node is NULL, cannot create PCI host bridge node\n");
> +		pr_info("Missing DeviceTree, cannot create PCI host bridge node\n");
>  		return;
>  	}
>  
> -- 
> 2.35.3
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] PCI: of: Downgrade error message on missing of_root node
  2025-11-06  0:23 ` Bjorn Helgaas
@ 2025-11-06 11:04   ` Andrea della Porta
  2025-11-06 12:18     ` Herve Codina
  0 siblings, 1 reply; 10+ messages in thread
From: Andrea della Porta @ 2025-11-06 11:04 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Andrea della Porta, Bjorn Helgaas, linux-pci, linux-kernel,
	mbrugger, guillaume.gardet, tiwai, Lizhi Hou, herve.codina

[+cc Herve]

Hi Bjorn,

On 18:23 Wed 05 Nov     , Bjorn Helgaas wrote:
> [+cc Lizhi]
> 
> On Wed, Nov 05, 2025 at 07:33:40PM +0100, Andrea della Porta wrote:
> > When CONFIG_PCI_DYNAMIC_OF_NODES is enabled, an error message
> > is generated if no 'of_root' node is defined.
> > 
> > On DT-based systems, this cannot happen as a root DT node is
> > always present. On ACPI-based systems, this is not a true error
> > because a DT is not used.
> > 
> > Downgrade the pr_err() to pr_info() and reword the message text
> > to be less context specific.
> 
> of_pci_make_host_bridge_node() is called in the very generic
> pci_register_host_bridge() path.  Does that mean every boot of a
> kernel with CONFIG_PCI_DYNAMIC_OF_NODES on a non-DT system will see
> this message?

This is the case, indeed. That's why downgrading to info seems sensible.

> 
> This message seems like something that will generate user questions.
> Or is this really an error, and we were supposed to have created
> of_root somewhere but it failed?  If so, I would expect a message
> where the of_root creation failed.

Not really an error per se: on ACPI system we usually don't have DT, so
this message just warns you that there will be no pci nodes created on it.
Which, again, should be of no importance on ACPI.

The only scenario in which this message is actually an error would be on
ACPI system that use DT as a complement to make runtime overlay work,
i.e. the overlay approach for RP1 on RPi5 with ACPI fw. AFAIK this fw is
more a PoC that something really widespread and currntly the overlay
approach is in stand-by anyway (meaning no one will use it unless some
major changes will be made to make it work). But there may be other
situations in which this scenario could arise, I'm thinking about Bootlin's
LAN966x driver which also uses runtime overlay to describe thw hw.
On ACPI system the root DT node is not created because unflatten_device_tree()
is not called.
One possible (easy) solution would be calling unflatten_device_tree() also
in case IS_ENABLED(PCI_DYNAMIC_OF_NODES), but this of course requires some
investigation against side effects.
In this case the roto DT node is always created (on both ACPI and non ACPI
systems) and the info message will not be printed.

> 
> I guess I'm confused about what the point of this message is.  If it's
> just a hint that loading an overlay in the future will fail, I assume
> we would emit a message at that time, connected with the user action
> of trying to load the overlay.

For a functional standpoint, it basically is, if we neglect the fact that
you won't have PCI nodes described in the DT, of course, which I guess it's
just informationali (but I may be mistaken on this point). Loading an overlay
in the future will fail anyway so no need to take further action there. Plus,
the systems on which this could happen (ACPI system + runtime overlay) would
be probably rare, if ever exist.

> 
> What badness would ensue if we downgraded this message even further
> and removed it completely?

As stated above, I would expect no major issue but other opinions
would be very welcome. 

Many thanks,
Andrea

> 
> > Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> > ---
> > CHANGES in V2:
> > 
> > * message text reworded to be less context specific (Bjorn)
> > ---
> >  drivers/pci/of.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > index 3579265f1198..872c36b195e3 100644
> > --- a/drivers/pci/of.c
> > +++ b/drivers/pci/of.c
> > @@ -775,7 +775,7 @@ void of_pci_make_host_bridge_node(struct pci_host_bridge *bridge)
> >  
> >  	/* Check if there is a DT root node to attach the created node */
> >  	if (!of_root) {
> > -		pr_err("of_root node is NULL, cannot create PCI host bridge node\n");
> > +		pr_info("Missing DeviceTree, cannot create PCI host bridge node\n");
> >  		return;
> >  	}
> >  
> > -- 
> > 2.35.3
> > 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] PCI: of: Downgrade error message on missing of_root node
  2025-11-06 11:04   ` Andrea della Porta
@ 2025-11-06 12:18     ` Herve Codina
  2025-11-06 15:21       ` Andrea della Porta
  0 siblings, 1 reply; 10+ messages in thread
From: Herve Codina @ 2025-11-06 12:18 UTC (permalink / raw)
  To: Andrea della Porta
  Cc: Bjorn Helgaas, Bjorn Helgaas, linux-pci, linux-kernel, mbrugger,
	guillaume.gardet, tiwai, Lizhi Hou

Hi, Andrea, Bjorn,

On Thu, 6 Nov 2025 12:04:07 +0100
Andrea della Porta <andrea.porta@suse.com> wrote:

> [+cc Herve]
> 
> Hi Bjorn,
> 
> On 18:23 Wed 05 Nov     , Bjorn Helgaas wrote:
> > [+cc Lizhi]
> > 
> > On Wed, Nov 05, 2025 at 07:33:40PM +0100, Andrea della Porta wrote:  
> > > When CONFIG_PCI_DYNAMIC_OF_NODES is enabled, an error message
> > > is generated if no 'of_root' node is defined.
> > > 
> > > On DT-based systems, this cannot happen as a root DT node is
> > > always present. On ACPI-based systems, this is not a true error
> > > because a DT is not used.
> > > 
> > > Downgrade the pr_err() to pr_info() and reword the message text
> > > to be less context specific.  
> > 
> > of_pci_make_host_bridge_node() is called in the very generic
> > pci_register_host_bridge() path.  Does that mean every boot of a
> > kernel with CONFIG_PCI_DYNAMIC_OF_NODES on a non-DT system will see
> > this message?  
> 
> This is the case, indeed. That's why downgrading to info seems sensible.
> 
> > 
> > This message seems like something that will generate user questions.
> > Or is this really an error, and we were supposed to have created
> > of_root somewhere but it failed?  If so, I would expect a message
> > where the of_root creation failed.  
> 
> Not really an error per se: on ACPI system we usually don't have DT, so
> this message just warns you that there will be no pci nodes created on it.
> Which, again, should be of no importance on ACPI.

I my last understanding, all architecture (even x86) have the DT root node
set. This node is empty on architectures that don't use DT to describe
hardware at boot (ACPI for instance).

This DT node is needed for PCI board that will be described by a DT overlay.
LAN966x for instance.

On v6.18-rc1 kernel, I successfully used my LAN966x board on a ACPI system.
This means that of_root DT node was present on my system.

> 
> The only scenario in which this message is actually an error would be on
> ACPI system that use DT as a complement to make runtime overlay work,

It is an error also if you use a PCI board that needs PCI DT nodes
(CONFIG_PCI_DYNAMIC_OF_NODES) Lan966x for instance.

> i.e. the overlay approach for RP1 on RPi5 with ACPI fw. AFAIK this fw is
> more a PoC that something really widespread and currntly the overlay
> approach is in stand-by anyway (meaning no one will use it unless some
> major changes will be made to make it work). But there may be other
> situations in which this scenario could arise, I'm thinking about Bootlin's
> LAN966x driver which also uses runtime overlay to describe thw hw.
> On ACPI system the root DT node is not created because unflatten_device_tree()
> is not called.

I am not so sure.
My LAN966x board is working on a x86 ACPI system.

I think, that if you don't want the kernel log, just set 
  CONFIG_PCI_DYNAMIC_OF_NODES = n

With CONFIG_PCI_DYNAMIC_OF_NODES = y, we need to create some nodes
and if cannot succeed to attach them to a DT tree, it is an error.
IMHO, pr_err() in that case is legit.


Best regards,
Hervé

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] PCI: of: Downgrade error message on missing of_root node
  2025-11-06 12:18     ` Herve Codina
@ 2025-11-06 15:21       ` Andrea della Porta
  2025-11-06 17:27         ` Herve Codina
  0 siblings, 1 reply; 10+ messages in thread
From: Andrea della Porta @ 2025-11-06 15:21 UTC (permalink / raw)
  To: Herve Codina
  Cc: Andrea della Porta, Bjorn Helgaas, Bjorn Helgaas, linux-pci,
	linux-kernel, mbrugger, guillaume.gardet, tiwai, Lizhi Hou

Hi Herve,

On 13:18 Thu 06 Nov     , Herve Codina wrote:
> Hi, Andrea, Bjorn,
> 
> On Thu, 6 Nov 2025 12:04:07 +0100
> Andrea della Porta <andrea.porta@suse.com> wrote:
> 
> > [+cc Herve]
> > 
> > Hi Bjorn,
> > 
> > On 18:23 Wed 05 Nov     , Bjorn Helgaas wrote:
> > > [+cc Lizhi]
> > > 
> > > On Wed, Nov 05, 2025 at 07:33:40PM +0100, Andrea della Porta wrote:  
> > > > When CONFIG_PCI_DYNAMIC_OF_NODES is enabled, an error message
> > > > is generated if no 'of_root' node is defined.
> > > > 
> > > > On DT-based systems, this cannot happen as a root DT node is
> > > > always present. On ACPI-based systems, this is not a true error
> > > > because a DT is not used.
> > > > 
> > > > Downgrade the pr_err() to pr_info() and reword the message text
> > > > to be less context specific.  
> > > 
> > > of_pci_make_host_bridge_node() is called in the very generic
> > > pci_register_host_bridge() path.  Does that mean every boot of a
> > > kernel with CONFIG_PCI_DYNAMIC_OF_NODES on a non-DT system will see
> > > this message?  
> > 
> > This is the case, indeed. That's why downgrading to info seems sensible.
> > 
> > > 
> > > This message seems like something that will generate user questions.
> > > Or is this really an error, and we were supposed to have created
> > > of_root somewhere but it failed?  If so, I would expect a message
> > > where the of_root creation failed.  
> > 
> > Not really an error per se: on ACPI system we usually don't have DT, so
> > this message just warns you that there will be no pci nodes created on it.
> > Which, again, should be of no importance on ACPI.
> 
> I my last understanding, all architecture (even x86) have the DT root node
> set. This node is empty on architectures that don't use DT to describe
> hardware at boot (ACPI for instance).

This does not seem to be the case for all arch, see below.

> 
> This DT node is needed for PCI board that will be described by a DT overlay.
> LAN966x for instance.
> 
> On v6.18-rc1 kernel, I successfully used my LAN966x board on a ACPI system.
> This means that of_root DT node was present on my system.
> 
> > 
> > The only scenario in which this message is actually an error would be on
> > ACPI system that use DT as a complement to make runtime overlay work,
> 
> It is an error also if you use a PCI board that needs PCI DT nodes
> (CONFIG_PCI_DYNAMIC_OF_NODES) Lan966x for instance.

Yes, I was referring exactly to that.

> 
> > i.e. the overlay approach for RP1 on RPi5 with ACPI fw. AFAIK this fw is
> > more a PoC that something really widespread and currntly the overlay
> > approach is in stand-by anyway (meaning no one will use it unless some
> > major changes will be made to make it work). But there may be other
> > situations in which this scenario could arise, I'm thinking about Bootlin's
> > LAN966x driver which also uses runtime overlay to describe thw hw.
> > On ACPI system the root DT node is not created because unflatten_device_tree()
> > is not called.
> 
> I am not so sure.
> My LAN966x board is working on a x86 ACPI system.

Indeed it depends on the architecture. On x86 an empty DT node is created,
provided you have CONFIG_OF_EARLY_FLATTREE defined (which I guess you have,
even if it's not in default config).

On arm64, ACPI and DT are mutually exclusive, unless the DT is basically empty
(i.e. only root node and chosen node). The DT root node is not automatically
created if not provided at boot, though. This reinforces my idea of providing
the only root node DT on arm as well, but I'm not entirely sure about 
possible side effects.

Many thanks,
Andrea

> 
> I think, that if you don't want the kernel log, just set 
>   CONFIG_PCI_DYNAMIC_OF_NODES = n
> 
> With CONFIG_PCI_DYNAMIC_OF_NODES = y, we need to create some nodes
> and if cannot succeed to attach them to a DT tree, it is an error.
> IMHO, pr_err() in that case is legit.
> 
> 
> Best regards,
> Hervé

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] PCI: of: Downgrade error message on missing of_root node
  2025-11-06 15:21       ` Andrea della Porta
@ 2025-11-06 17:27         ` Herve Codina
  2025-11-06 17:50           ` Bjorn Helgaas
  2025-11-07  9:26           ` Andrea della Porta
  0 siblings, 2 replies; 10+ messages in thread
From: Herve Codina @ 2025-11-06 17:27 UTC (permalink / raw)
  To: Andrea della Porta
  Cc: Bjorn Helgaas, Bjorn Helgaas, linux-pci, linux-kernel, mbrugger,
	guillaume.gardet, tiwai, Lizhi Hou, Rob Herring

Hi Andrea,

+ CC Rob

On Thu, 6 Nov 2025 16:21:47 +0100
Andrea della Porta <andrea.porta@suse.com> wrote:

> Hi Herve,
> 
> On 13:18 Thu 06 Nov     , Herve Codina wrote:
> > Hi, Andrea, Bjorn,
> > 
> > On Thu, 6 Nov 2025 12:04:07 +0100
> > Andrea della Porta <andrea.porta@suse.com> wrote:
> >   
> > > [+cc Herve]
> > > 
> > > Hi Bjorn,
> > > 
> > > On 18:23 Wed 05 Nov     , Bjorn Helgaas wrote:  
> > > > [+cc Lizhi]
> > > > 
> > > > On Wed, Nov 05, 2025 at 07:33:40PM +0100, Andrea della Porta wrote:    
> > > > > When CONFIG_PCI_DYNAMIC_OF_NODES is enabled, an error message
> > > > > is generated if no 'of_root' node is defined.
> > > > > 
> > > > > On DT-based systems, this cannot happen as a root DT node is
> > > > > always present. On ACPI-based systems, this is not a true error
> > > > > because a DT is not used.
> > > > > 
> > > > > Downgrade the pr_err() to pr_info() and reword the message text
> > > > > to be less context specific.    
> > > > 
> > > > of_pci_make_host_bridge_node() is called in the very generic
> > > > pci_register_host_bridge() path.  Does that mean every boot of a
> > > > kernel with CONFIG_PCI_DYNAMIC_OF_NODES on a non-DT system will see
> > > > this message?    
> > > 
> > > This is the case, indeed. That's why downgrading to info seems sensible.
> > >   
> > > > 
> > > > This message seems like something that will generate user questions.
> > > > Or is this really an error, and we were supposed to have created
> > > > of_root somewhere but it failed?  If so, I would expect a message
> > > > where the of_root creation failed.    
> > > 
> > > Not really an error per se: on ACPI system we usually don't have DT, so
> > > this message just warns you that there will be no pci nodes created on it.
> > > Which, again, should be of no importance on ACPI.  
> > 
> > I my last understanding, all architecture (even x86) have the DT root node
> > set. This node is empty on architectures that don't use DT to describe
> > hardware at boot (ACPI for instance).  
> 
> This does not seem to be the case for all arch, see below.
> 
> > 
> > This DT node is needed for PCI board that will be described by a DT overlay.
> > LAN966x for instance.
> > 
> > On v6.18-rc1 kernel, I successfully used my LAN966x board on a ACPI system.
> > This means that of_root DT node was present on my system.
> >   
> > > 
> > > The only scenario in which this message is actually an error would be on
> > > ACPI system that use DT as a complement to make runtime overlay work,  
> > 
> > It is an error also if you use a PCI board that needs PCI DT nodes
> > (CONFIG_PCI_DYNAMIC_OF_NODES) Lan966x for instance.  
> 
> Yes, I was referring exactly to that.
> 
> >   
> > > i.e. the overlay approach for RP1 on RPi5 with ACPI fw. AFAIK this fw is
> > > more a PoC that something really widespread and currntly the overlay
> > > approach is in stand-by anyway (meaning no one will use it unless some
> > > major changes will be made to make it work). But there may be other
> > > situations in which this scenario could arise, I'm thinking about Bootlin's
> > > LAN966x driver which also uses runtime overlay to describe thw hw.
> > > On ACPI system the root DT node is not created because unflatten_device_tree()
> > > is not called.  
> > 
> > I am not so sure.
> > My LAN966x board is working on a x86 ACPI system.  
> 
> Indeed it depends on the architecture. On x86 an empty DT node is created,
> provided you have CONFIG_OF_EARLY_FLATTREE defined (which I guess you have,
> even if it's not in default config).

Indeed, I have CONFIG_OF_EARLY_FLATTREE = y.

> 
> On arm64, ACPI and DT are mutually exclusive, unless the DT is basically empty
> (i.e. only root node and chosen node). The DT root node is not automatically
> created if not provided at boot, though. This reinforces my idea of providing
> the only root node DT on arm as well, but I'm not entirely sure about 
> possible side effects.
> 

Isn't it possible to have the same kind of operations on ARM64 ACPI and on x86?

In order to have CONFIG_PCI_DYNAMIC_OF_NODES working on ACPI, we need a DT
node, even empty.

ARM64 ACPI without an empty DT node means that no PCI boards using a DT
description will work on this system.

Best regards,
Hervé

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] PCI: of: Downgrade error message on missing of_root node
  2025-11-06 17:27         ` Herve Codina
@ 2025-11-06 17:50           ` Bjorn Helgaas
  2025-11-07  9:32             ` Andrea della Porta
  2025-11-07  9:26           ` Andrea della Porta
  1 sibling, 1 reply; 10+ messages in thread
From: Bjorn Helgaas @ 2025-11-06 17:50 UTC (permalink / raw)
  To: Herve Codina
  Cc: Andrea della Porta, Bjorn Helgaas, linux-pci, linux-kernel,
	mbrugger, guillaume.gardet, tiwai, Lizhi Hou, Rob Herring

On Thu, Nov 06, 2025 at 06:27:08PM +0100, Herve Codina wrote:
> On Thu, 6 Nov 2025 16:21:47 +0100
> Andrea della Porta <andrea.porta@suse.com> wrote:
> > On 13:18 Thu 06 Nov     , Herve Codina wrote:
> > > On Thu, 6 Nov 2025 12:04:07 +0100
> > > Andrea della Porta <andrea.porta@suse.com> wrote:
> > > > On 18:23 Wed 05 Nov     , Bjorn Helgaas wrote:  
> > > > > On Wed, Nov 05, 2025 at 07:33:40PM +0100, Andrea della Porta wrote:    
Patch at https://lore.kernel.org/r/955bc7a9b78678fad4b705c428e8b45aeb0cbf3c.1762367117.git.andrea.porta@suse.com,
added back for reference:

  diff --git a/drivers/pci/of.c b/drivers/pci/of.c
  index 3579265f1198..872c36b195e3 100644
  --- a/drivers/pci/of.c
  +++ b/drivers/pci/of.c
  @@ -775,7 +775,7 @@ void of_pci_make_host_bridge_node(struct pci_host_bridge *bridge)

	  /* Check if there is a DT root node to attach the created node */
	  if (!of_root) {
  -               pr_err("of_root node is NULL, cannot create PCI host bridge node\n");
  +               pr_info("Missing DeviceTree, cannot create PCI host bridge node\n");
		  return;
	  }

> > > > > > When CONFIG_PCI_DYNAMIC_OF_NODES is enabled, an error
> > > > > > message is generated if no 'of_root' node is defined.
> > > > > > 
> > > > > > On DT-based systems, this cannot happen as a root DT node
> > > > > > is always present. On ACPI-based systems, this is not a
> > > > > > true error because a DT is not used.
> > > > > > 
> > > > > > Downgrade the pr_err() to pr_info() and reword the message
> > > > > > text to be less context specific.    
> > > > > 
> > > > > of_pci_make_host_bridge_node() is called in the very generic
> > > > > pci_register_host_bridge() path.  Does that mean every boot
> > > > > of a kernel with CONFIG_PCI_DYNAMIC_OF_NODES on a non-DT
> > > > > system will see this message?    
> > > > 
> > > > This is the case, indeed. That's why downgrading to info seems
> > > > sensible.
> > > >   
> > > > > This message seems like something that will generate user
> > > > > questions.  Or is this really an error, and we were supposed
> > > > > to have created of_root somewhere but it failed?  If so, I
> > > > > would expect a message where the of_root creation failed.    

I think we should just remove the message completely.  I don't want
users to enable CONFIG_PCI_DYNAMIC_OF_NODES out of curiosity or
willingness to test, and then ask about this message.

"You can avoid the message by also enabling CONFIG_OF_EARLY_FLATTREE"
is not a very satisfactory answer.

A message at the point of *needing* this, i.e., when loading an
overlay fails for lack of this dynamic DT, is fine.

> > > > Not really an error per se: on ACPI system we usually don't
> > > > have DT, so this message just warns you that there will be no
> > > > pci nodes created on it.  Which, again, should be of no
> > > > importance on ACPI.  
> > > 
> > > I my last understanding, all architecture (even x86) have the DT
> > > root node set. This node is empty on architectures that don't
> > > use DT to describe hardware at boot (ACPI for instance).  
> > 
> > This does not seem to be the case for all arch, see below.
> > 
> > > This DT node is needed for PCI board that will be described by a
> > > DT overlay.  LAN966x for instance.
> > > 
> > > On v6.18-rc1 kernel, I successfully used my LAN966x board on a
> > > ACPI system.  This means that of_root DT node was present on my
> > > system.
> > >   
> > > > The only scenario in which this message is actually an error
> > > > would be on ACPI system that use DT as a complement to make
> > > > runtime overlay work,  
> > > 
> > > It is an error also if you use a PCI board that needs PCI DT
> > > nodes (CONFIG_PCI_DYNAMIC_OF_NODES) Lan966x for instance.  
> > 
> > Yes, I was referring exactly to that.
> > 
> > > > i.e. the overlay approach for RP1 on RPi5 with ACPI fw. AFAIK
> > > > this fw is more a PoC that something really widespread and
> > > > currntly the overlay approach is in stand-by anyway (meaning
> > > > no one will use it unless some major changes will be made to
> > > > make it work). But there may be other situations in which this
> > > > scenario could arise, I'm thinking about Bootlin's LAN966x
> > > > driver which also uses runtime overlay to describe thw hw.  On
> > > > ACPI system the root DT node is not created because
> > > > unflatten_device_tree() is not called.  
> > > 
> > > I am not so sure.  My LAN966x board is working on a x86 ACPI
> > > system.  
> > 
> > Indeed it depends on the architecture. On x86 an empty DT node is
> > created, provided you have CONFIG_OF_EARLY_FLATTREE defined (which
> > I guess you have, even if it's not in default config).
> 
> Indeed, I have CONFIG_OF_EARLY_FLATTREE = y.
> 
> > On arm64, ACPI and DT are mutually exclusive, unless the DT is
> > basically empty (i.e. only root node and chosen node). The DT root
> > node is not automatically created if not provided at boot, though.
> > This reinforces my idea of providing the only root node DT on arm
> > as well, but I'm not entirely sure about possible side effects.
> 
> Isn't it possible to have the same kind of operations on ARM64 ACPI
> and on x86?
> 
> In order to have CONFIG_PCI_DYNAMIC_OF_NODES working on ACPI, we
> need a DT node, even empty.
> 
> ARM64 ACPI without an empty DT node means that no PCI boards using a
> DT description will work on this system.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] PCI: of: Downgrade error message on missing of_root node
  2025-11-06 17:27         ` Herve Codina
  2025-11-06 17:50           ` Bjorn Helgaas
@ 2025-11-07  9:26           ` Andrea della Porta
  1 sibling, 0 replies; 10+ messages in thread
From: Andrea della Porta @ 2025-11-07  9:26 UTC (permalink / raw)
  To: Herve Codina
  Cc: Andrea della Porta, Bjorn Helgaas, Bjorn Helgaas, linux-pci,
	linux-kernel, mbrugger, guillaume.gardet, tiwai, Lizhi Hou,
	Rob Herring

Hi Herve,

On 18:27 Thu 06 Nov     , Herve Codina wrote:
> Hi Andrea,
> 
> + CC Rob
> 
> On Thu, 6 Nov 2025 16:21:47 +0100
> Andrea della Porta <andrea.porta@suse.com> wrote:
> 
> > Hi Herve,
> > 
> > On 13:18 Thu 06 Nov     , Herve Codina wrote:
> > > Hi, Andrea, Bjorn,
> > > 
> > > On Thu, 6 Nov 2025 12:04:07 +0100
> > > Andrea della Porta <andrea.porta@suse.com> wrote:
> > >   
> > > > [+cc Herve]
> > > > 
> > > > Hi Bjorn,
> > > > 
> > > > On 18:23 Wed 05 Nov     , Bjorn Helgaas wrote:  
> > > > > [+cc Lizhi]
> > > > > 
> > > > > On Wed, Nov 05, 2025 at 07:33:40PM +0100, Andrea della Porta wrote:    
> > > > > > When CONFIG_PCI_DYNAMIC_OF_NODES is enabled, an error message
> > > > > > is generated if no 'of_root' node is defined.
> > > > > > 
> > > > > > On DT-based systems, this cannot happen as a root DT node is
> > > > > > always present. On ACPI-based systems, this is not a true error
> > > > > > because a DT is not used.
> > > > > > 
> > > > > > Downgrade the pr_err() to pr_info() and reword the message text
> > > > > > to be less context specific.    
> > > > > 
> > > > > of_pci_make_host_bridge_node() is called in the very generic
> > > > > pci_register_host_bridge() path.  Does that mean every boot of a
> > > > > kernel with CONFIG_PCI_DYNAMIC_OF_NODES on a non-DT system will see
> > > > > this message?    
> > > > 
> > > > This is the case, indeed. That's why downgrading to info seems sensible.
> > > >   
> > > > > 
> > > > > This message seems like something that will generate user questions.
> > > > > Or is this really an error, and we were supposed to have created
> > > > > of_root somewhere but it failed?  If so, I would expect a message
> > > > > where the of_root creation failed.    
> > > > 
> > > > Not really an error per se: on ACPI system we usually don't have DT, so
> > > > this message just warns you that there will be no pci nodes created on it.
> > > > Which, again, should be of no importance on ACPI.  
> > > 
> > > I my last understanding, all architecture (even x86) have the DT root node
> > > set. This node is empty on architectures that don't use DT to describe
> > > hardware at boot (ACPI for instance).  
> > 
> > This does not seem to be the case for all arch, see below.
> > 
> > > 
> > > This DT node is needed for PCI board that will be described by a DT overlay.
> > > LAN966x for instance.
> > > 
> > > On v6.18-rc1 kernel, I successfully used my LAN966x board on a ACPI system.
> > > This means that of_root DT node was present on my system.
> > >   
> > > > 
> > > > The only scenario in which this message is actually an error would be on
> > > > ACPI system that use DT as a complement to make runtime overlay work,  
> > > 
> > > It is an error also if you use a PCI board that needs PCI DT nodes
> > > (CONFIG_PCI_DYNAMIC_OF_NODES) Lan966x for instance.  
> > 
> > Yes, I was referring exactly to that.
> > 
> > >   
> > > > i.e. the overlay approach for RP1 on RPi5 with ACPI fw. AFAIK this fw is
> > > > more a PoC that something really widespread and currntly the overlay
> > > > approach is in stand-by anyway (meaning no one will use it unless some
> > > > major changes will be made to make it work). But there may be other
> > > > situations in which this scenario could arise, I'm thinking about Bootlin's
> > > > LAN966x driver which also uses runtime overlay to describe thw hw.
> > > > On ACPI system the root DT node is not created because unflatten_device_tree()
> > > > is not called.  
> > > 
> > > I am not so sure.
> > > My LAN966x board is working on a x86 ACPI system.  
> > 
> > Indeed it depends on the architecture. On x86 an empty DT node is created,
> > provided you have CONFIG_OF_EARLY_FLATTREE defined (which I guess you have,
> > even if it's not in default config).
> 
> Indeed, I have CONFIG_OF_EARLY_FLATTREE = y.
> 
> > 
> > On arm64, ACPI and DT are mutually exclusive, unless the DT is basically empty
> > (i.e. only root node and chosen node). The DT root node is not automatically
> > created if not provided at boot, though. This reinforces my idea of providing
> > the only root node DT on arm as well, but I'm not entirely sure about 
> > possible side effects.
> > 
> 
> Isn't it possible to have the same kind of operations on ARM64 ACPI and on x86?
> 
> In order to have CONFIG_PCI_DYNAMIC_OF_NODES working on ACPI, we need a DT
> node, even empty.

That's a good point and it's what I'm proposing, indeed. Maybe it may worth just
standardizing that across all platforms, if it is possible. But let's get there
step by step, and arm64 could be a good starting point. If there is an empty DT
on x86 I think it would be logical to do the same on arm. But the if guard that calls
unflatten_device_tree() only if acpi is disabled makes me wonder if there is some
rationale behind it that I'm not aware of, that's why I'm asking if anyone knows
of any possible side effects. Maybe Rob could give guidance...

> 
> ARM64 ACPI without an empty DT node means that no PCI boards using a DT
> description will work on this system.

Indeed.

Regards,
Andrea

> 
> Best regards,
> Hervé

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] PCI: of: Downgrade error message on missing of_root node
  2025-11-06 17:50           ` Bjorn Helgaas
@ 2025-11-07  9:32             ` Andrea della Porta
  2025-11-07 11:58               ` Herve Codina
  0 siblings, 1 reply; 10+ messages in thread
From: Andrea della Porta @ 2025-11-07  9:32 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Herve Codina, Andrea della Porta, Bjorn Helgaas, linux-pci,
	linux-kernel, mbrugger, guillaume.gardet, tiwai, Lizhi Hou,
	Rob Herring

Hi Bjorn,

On 11:50 Thu 06 Nov     , Bjorn Helgaas wrote:
> On Thu, Nov 06, 2025 at 06:27:08PM +0100, Herve Codina wrote:
> > On Thu, 6 Nov 2025 16:21:47 +0100
> > Andrea della Porta <andrea.porta@suse.com> wrote:
> > > On 13:18 Thu 06 Nov     , Herve Codina wrote:
> > > > On Thu, 6 Nov 2025 12:04:07 +0100
> > > > Andrea della Porta <andrea.porta@suse.com> wrote:
> > > > > On 18:23 Wed 05 Nov     , Bjorn Helgaas wrote:  
> > > > > > On Wed, Nov 05, 2025 at 07:33:40PM +0100, Andrea della Porta wrote:    
> Patch at https://lore.kernel.org/r/955bc7a9b78678fad4b705c428e8b45aeb0cbf3c.1762367117.git.andrea.porta@suse.com,
> added back for reference:
> 
>   diff --git a/drivers/pci/of.c b/drivers/pci/of.c
>   index 3579265f1198..872c36b195e3 100644
>   --- a/drivers/pci/of.c
>   +++ b/drivers/pci/of.c
>   @@ -775,7 +775,7 @@ void of_pci_make_host_bridge_node(struct pci_host_bridge *bridge)
> 
> 	  /* Check if there is a DT root node to attach the created node */
> 	  if (!of_root) {
>   -               pr_err("of_root node is NULL, cannot create PCI host bridge node\n");
>   +               pr_info("Missing DeviceTree, cannot create PCI host bridge node\n");
> 		  return;
> 	  }
> 
> > > > > > > When CONFIG_PCI_DYNAMIC_OF_NODES is enabled, an error
> > > > > > > message is generated if no 'of_root' node is defined.
> > > > > > > 
> > > > > > > On DT-based systems, this cannot happen as a root DT node
> > > > > > > is always present. On ACPI-based systems, this is not a
> > > > > > > true error because a DT is not used.
> > > > > > > 
> > > > > > > Downgrade the pr_err() to pr_info() and reword the message
> > > > > > > text to be less context specific.    
> > > > > > 
> > > > > > of_pci_make_host_bridge_node() is called in the very generic
> > > > > > pci_register_host_bridge() path.  Does that mean every boot
> > > > > > of a kernel with CONFIG_PCI_DYNAMIC_OF_NODES on a non-DT
> > > > > > system will see this message?    
> > > > > 
> > > > > This is the case, indeed. That's why downgrading to info seems
> > > > > sensible.
> > > > >   
> > > > > > This message seems like something that will generate user
> > > > > > questions.  Or is this really an error, and we were supposed
> > > > > > to have created of_root somewhere but it failed?  If so, I
> > > > > > would expect a message where the of_root creation failed.    
> 
> I think we should just remove the message completely.  I don't want
> users to enable CONFIG_PCI_DYNAMIC_OF_NODES out of curiosity or
> willingness to test, and then ask about this message.

Agreed. This would be the easy solution, the other being creating the
empty DT so that the message will never be printed. But this require some
careful thought. The latter solution will be needed if we'll ever want to
make drivers like RP1 or lan96xx (which uses the runtime overlay) to work.

> 
> "You can avoid the message by also enabling CONFIG_OF_EARLY_FLATTREE"
> is not a very satisfactory answer.

Unfortunately this would work on x86, but not on arm. And who knows on
other platforms.

> 
> A message at the point of *needing* this, i.e., when loading an
> overlay fails for lack of this dynamic DT, is fine.

It seems fine to me.

Thanks,
Andrea

> 
> > > > > Not really an error per se: on ACPI system we usually don't
> > > > > have DT, so this message just warns you that there will be no
> > > > > pci nodes created on it.  Which, again, should be of no
> > > > > importance on ACPI.  
> > > > 
> > > > I my last understanding, all architecture (even x86) have the DT
> > > > root node set. This node is empty on architectures that don't
> > > > use DT to describe hardware at boot (ACPI for instance).  
> > > 
> > > This does not seem to be the case for all arch, see below.
> > > 
> > > > This DT node is needed for PCI board that will be described by a
> > > > DT overlay.  LAN966x for instance.
> > > > 
> > > > On v6.18-rc1 kernel, I successfully used my LAN966x board on a
> > > > ACPI system.  This means that of_root DT node was present on my
> > > > system.
> > > >   
> > > > > The only scenario in which this message is actually an error
> > > > > would be on ACPI system that use DT as a complement to make
> > > > > runtime overlay work,  
> > > > 
> > > > It is an error also if you use a PCI board that needs PCI DT
> > > > nodes (CONFIG_PCI_DYNAMIC_OF_NODES) Lan966x for instance.  
> > > 
> > > Yes, I was referring exactly to that.
> > > 
> > > > > i.e. the overlay approach for RP1 on RPi5 with ACPI fw. AFAIK
> > > > > this fw is more a PoC that something really widespread and
> > > > > currntly the overlay approach is in stand-by anyway (meaning
> > > > > no one will use it unless some major changes will be made to
> > > > > make it work). But there may be other situations in which this
> > > > > scenario could arise, I'm thinking about Bootlin's LAN966x
> > > > > driver which also uses runtime overlay to describe thw hw.  On
> > > > > ACPI system the root DT node is not created because
> > > > > unflatten_device_tree() is not called.  
> > > > 
> > > > I am not so sure.  My LAN966x board is working on a x86 ACPI
> > > > system.  
> > > 
> > > Indeed it depends on the architecture. On x86 an empty DT node is
> > > created, provided you have CONFIG_OF_EARLY_FLATTREE defined (which
> > > I guess you have, even if it's not in default config).
> > 
> > Indeed, I have CONFIG_OF_EARLY_FLATTREE = y.
> > 
> > > On arm64, ACPI and DT are mutually exclusive, unless the DT is
> > > basically empty (i.e. only root node and chosen node). The DT root
> > > node is not automatically created if not provided at boot, though.
> > > This reinforces my idea of providing the only root node DT on arm
> > > as well, but I'm not entirely sure about possible side effects.
> > 
> > Isn't it possible to have the same kind of operations on ARM64 ACPI
> > and on x86?
> > 
> > In order to have CONFIG_PCI_DYNAMIC_OF_NODES working on ACPI, we
> > need a DT node, even empty.
> > 
> > ARM64 ACPI without an empty DT node means that no PCI boards using a
> > DT description will work on this system.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] PCI: of: Downgrade error message on missing of_root node
  2025-11-07  9:32             ` Andrea della Porta
@ 2025-11-07 11:58               ` Herve Codina
  0 siblings, 0 replies; 10+ messages in thread
From: Herve Codina @ 2025-11-07 11:58 UTC (permalink / raw)
  To: Andrea della Porta
  Cc: Bjorn Helgaas, Bjorn Helgaas, linux-pci, linux-kernel, mbrugger,
	guillaume.gardet, tiwai, Lizhi Hou, Rob Herring

Hi Andrea, Bjorn,

On Fri, 7 Nov 2025 10:32:35 +0100
Andrea della Porta <andrea.porta@suse.com> wrote:

> Hi Bjorn,
> 
> On 11:50 Thu 06 Nov     , Bjorn Helgaas wrote:
> > On Thu, Nov 06, 2025 at 06:27:08PM +0100, Herve Codina wrote:  
> > > On Thu, 6 Nov 2025 16:21:47 +0100
> > > Andrea della Porta <andrea.porta@suse.com> wrote:  
> > > > On 13:18 Thu 06 Nov     , Herve Codina wrote:  
> > > > > On Thu, 6 Nov 2025 12:04:07 +0100
> > > > > Andrea della Porta <andrea.porta@suse.com> wrote:  
> > > > > > On 18:23 Wed 05 Nov     , Bjorn Helgaas wrote:    
> > > > > > > On Wed, Nov 05, 2025 at 07:33:40PM +0100, Andrea della Porta wrote:      
> > Patch at https://lore.kernel.org/r/955bc7a9b78678fad4b705c428e8b45aeb0cbf3c.1762367117.git.andrea.porta@suse.com,
> > added back for reference:
> > 
> >   diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> >   index 3579265f1198..872c36b195e3 100644
> >   --- a/drivers/pci/of.c
> >   +++ b/drivers/pci/of.c
> >   @@ -775,7 +775,7 @@ void of_pci_make_host_bridge_node(struct pci_host_bridge *bridge)
> > 
> > 	  /* Check if there is a DT root node to attach the created node */
> > 	  if (!of_root) {
> >   -               pr_err("of_root node is NULL, cannot create PCI host bridge node\n");
> >   +               pr_info("Missing DeviceTree, cannot create PCI host bridge node\n");
> > 		  return;
> > 	  }
> >   
> > > > > > > > When CONFIG_PCI_DYNAMIC_OF_NODES is enabled, an error
> > > > > > > > message is generated if no 'of_root' node is defined.
> > > > > > > > 
> > > > > > > > On DT-based systems, this cannot happen as a root DT node
> > > > > > > > is always present. On ACPI-based systems, this is not a
> > > > > > > > true error because a DT is not used.
> > > > > > > > 
> > > > > > > > Downgrade the pr_err() to pr_info() and reword the message
> > > > > > > > text to be less context specific.      
> > > > > > > 
> > > > > > > of_pci_make_host_bridge_node() is called in the very generic
> > > > > > > pci_register_host_bridge() path.  Does that mean every boot
> > > > > > > of a kernel with CONFIG_PCI_DYNAMIC_OF_NODES on a non-DT
> > > > > > > system will see this message?      
> > > > > > 
> > > > > > This is the case, indeed. That's why downgrading to info seems
> > > > > > sensible.
> > > > > >     
> > > > > > > This message seems like something that will generate user
> > > > > > > questions.  Or is this really an error, and we were supposed
> > > > > > > to have created of_root somewhere but it failed?  If so, I
> > > > > > > would expect a message where the of_root creation failed.      
> > 
> > I think we should just remove the message completely.  I don't want
> > users to enable CONFIG_PCI_DYNAMIC_OF_NODES out of curiosity or
> > willingness to test, and then ask about this message.  
> 
> Agreed. This would be the easy solution, the other being creating the
> empty DT so that the message will never be printed. But this require some
> careful thought. The latter solution will be needed if we'll ever want to
> make drivers like RP1 or lan96xx (which uses the runtime overlay) to work.
> 
> > 
> > "You can avoid the message by also enabling CONFIG_OF_EARLY_FLATTREE"
> > is not a very satisfactory answer.  
> 
> Unfortunately this would work on x86, but not on arm. And who knows on
> other platforms.
> 
> > 
> > A message at the point of *needing* this, i.e., when loading an
> > overlay fails for lack of this dynamic DT, is fine.  
> 
> It seems fine to me.
> 

Ok, even if I would prefer having an empty (or not) of_node available on all
platforms, what is proposed makes sense especially in regards to potential
users questions.

Best regards,
Hervé


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-11-07 11:58 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-05 18:33 [PATCH v2] PCI: of: Downgrade error message on missing of_root node Andrea della Porta
2025-11-06  0:23 ` Bjorn Helgaas
2025-11-06 11:04   ` Andrea della Porta
2025-11-06 12:18     ` Herve Codina
2025-11-06 15:21       ` Andrea della Porta
2025-11-06 17:27         ` Herve Codina
2025-11-06 17:50           ` Bjorn Helgaas
2025-11-07  9:32             ` Andrea della Porta
2025-11-07 11:58               ` Herve Codina
2025-11-07  9:26           ` Andrea della Porta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).