diff mbox series

[v8,08/17] ACPI: pci_link: Clear the dependencies after probe

Message ID 20240812005929.113499-9-sunilvl@ventanamicro.com
State Accepted
Commit 2cb9155d116c4d9dd7610139a876d123b75924ef
Headers show
Series RISC-V: ACPI: Add external interrupt controller support | expand

Commit Message

Sunil V L Aug. 12, 2024, 12:59 a.m. UTC
RISC-V platforms need to use dependencies between PCI host bridge, Link
devices and the interrupt controllers to ensure probe order. The
dependency is like below.

Interrupt controller <-- Link Device <-- PCI Host bridge.

If there is no dependency between Link device and PCI Host Bridge,
then PCI devices may be probed prior to Link devices.  If a PCI
device is probed before its Link device, we won't be able to find
its INTx mapping.

So, add the link device's HID to dependency honor list and clear the
dependency after probe is done so that the dependent devices are
unblocked to probe.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
Tested-by: Björn Töpel <bjorn@rivosinc.com>
---
 drivers/acpi/pci_link.c | 2 ++
 drivers/acpi/scan.c     | 1 +
 2 files changed, 3 insertions(+)

Comments

Bjorn Helgaas Aug. 22, 2024, 9:44 p.m. UTC | #1
On Mon, Aug 12, 2024 at 06:29:20AM +0530, Sunil V L wrote:
> RISC-V platforms need to use dependencies between PCI host bridge, Link
> devices and the interrupt controllers to ensure probe order. The
> dependency is like below.
> 
> Interrupt controller <-- Link Device <-- PCI Host bridge.
> 
> If there is no dependency between Link device and PCI Host Bridge,
> then PCI devices may be probed prior to Link devices.  If a PCI
> device is probed before its Link device, we won't be able to find
> its INTx mapping.

This seems to explain why we want these dependencies, which is useful,
but *this* patch only removes the dependencies.

Maybe this description should be in the patch that *adds* the
dependencies, e.g., "ACPI: RISC-V: Implement function to add implicit
dependencies"?

> So, add the link device's HID to dependency honor list and clear the
> dependency after probe is done so that the dependent devices are
> unblocked to probe.

This still claims this patch adds HID, which I don't think it does.

> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> Tested-by: Björn Töpel <bjorn@rivosinc.com>
> ---
>  drivers/acpi/pci_link.c | 2 ++
>  drivers/acpi/scan.c     | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
> index aa1038b8aec4..b727db968f33 100644
> --- a/drivers/acpi/pci_link.c
> +++ b/drivers/acpi/pci_link.c
> @@ -748,6 +748,8 @@ static int acpi_pci_link_add(struct acpi_device *device,
>  	if (result)
>  		kfree(link);
>  
> +	acpi_dev_clear_dependencies(device);
> +
>  	return result < 0 ? result : 1;
>  }
>  
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 28a221f956d7..753539a1f26b 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -863,6 +863,7 @@ static const char * const acpi_honor_dep_ids[] = {
>  	"INTC10CF", /* IVSC (MTL) driver must be loaded to allow i2c access to camera sensors */
>  	"RSCV0001", /* RISC-V PLIC */
>  	"RSCV0002", /* RISC-V APLIC */
> +	"PNP0C0F",  /* PCI Link Device */
>  	NULL
>  };
>  
> -- 
> 2.43.0
>
Sunil V L Aug. 23, 2024, 6:33 a.m. UTC | #2
Hi Bjorn,

On Thu, Aug 22, 2024 at 04:44:15PM -0500, Bjorn Helgaas wrote:
> On Mon, Aug 12, 2024 at 06:29:20AM +0530, Sunil V L wrote:
> > RISC-V platforms need to use dependencies between PCI host bridge, Link
> > devices and the interrupt controllers to ensure probe order. The
> > dependency is like below.
> > 
> > Interrupt controller <-- Link Device <-- PCI Host bridge.
> > 
> > If there is no dependency between Link device and PCI Host Bridge,
> > then PCI devices may be probed prior to Link devices.  If a PCI
> > device is probed before its Link device, we won't be able to find
> > its INTx mapping.
> 
> This seems to explain why we want these dependencies, which is useful,
> but *this* patch only removes the dependencies.
> 
> Maybe this description should be in the patch that *adds* the
> dependencies, e.g., "ACPI: RISC-V: Implement function to add implicit
> dependencies"?
>
Okay. Let me move this to the patch you suggested.
 
> > So, add the link device's HID to dependency honor list and clear the
> > dependency after probe is done so that the dependent devices are
> > unblocked to probe.
> 
> This still claims this patch adds HID, which I don't think it does.
>
Please see below.
 
> > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> > Tested-by: Björn Töpel <bjorn@rivosinc.com>
> > ---
> >  drivers/acpi/pci_link.c | 2 ++
> >  drivers/acpi/scan.c     | 1 +
> >  2 files changed, 3 insertions(+)
> > 
> > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
> > index aa1038b8aec4..b727db968f33 100644
> > --- a/drivers/acpi/pci_link.c
> > +++ b/drivers/acpi/pci_link.c
> > @@ -748,6 +748,8 @@ static int acpi_pci_link_add(struct acpi_device *device,
> >  	if (result)
> >  		kfree(link);
> >  
> > +	acpi_dev_clear_dependencies(device);
> > +
> >  	return result < 0 ? result : 1;
> >  }
> >  
> > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > index 28a221f956d7..753539a1f26b 100644
> > --- a/drivers/acpi/scan.c
> > +++ b/drivers/acpi/scan.c
> > @@ -863,6 +863,7 @@ static const char * const acpi_honor_dep_ids[] = {
> >  	"INTC10CF", /* IVSC (MTL) driver must be loaded to allow i2c access to camera sensors */
> >  	"RSCV0001", /* RISC-V PLIC */
> >  	"RSCV0002", /* RISC-V APLIC */
> > +	"PNP0C0F",  /* PCI Link Device */
This is the change which I meant adding HID to the honor list. Do you
recommend to make this change separate patch so that it doesn't confuse
with adding a new HID to the probe match table?

Thanks!
Sunil
diff mbox series

Patch

diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
index aa1038b8aec4..b727db968f33 100644
--- a/drivers/acpi/pci_link.c
+++ b/drivers/acpi/pci_link.c
@@ -748,6 +748,8 @@  static int acpi_pci_link_add(struct acpi_device *device,
 	if (result)
 		kfree(link);
 
+	acpi_dev_clear_dependencies(device);
+
 	return result < 0 ? result : 1;
 }
 
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 28a221f956d7..753539a1f26b 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -863,6 +863,7 @@  static const char * const acpi_honor_dep_ids[] = {
 	"INTC10CF", /* IVSC (MTL) driver must be loaded to allow i2c access to camera sensors */
 	"RSCV0001", /* RISC-V PLIC */
 	"RSCV0002", /* RISC-V APLIC */
+	"PNP0C0F",  /* PCI Link Device */
 	NULL
 };