Message-ID: <CAJZ5v0gXGXOEJpYSAsFiO_L+ek1o9mKT0k2gJe1SSYz0p1=RaA@mail.gmail.com>
Date: Tue, 27 Jan 2026 21:04:26 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, huyuye <huyuye812@....com>, 
	Bjorn Helgaas <bhelgaas@...gle.com>, Sunil V L <sunilvl@...tanamicro.com>, 
	Paul Walmsley <pjw@...nel.org>, Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>, 
	Alexandre Ghiti <alex@...ti.fr>, linux-pci@...r.kernel.org, linux-acpi@...r.kernel.org, 
	linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	dai.hualiang@....com.cn, deng.weixian@....com.cn, guo.chang2@....com.cn, 
	liu.qingtao2@....com.cn, wu.jiabao@....com.cn, lin.yongchun@....com.cn, 
	hu.yuye@....com.cn, zhang.longxiang@....com.cn, zuo.jiang@....com.cn, 
	li.kunpeng@....com.cn
Subject: Re: [PATCH v2] ACPI: pci_root: Clear the acpi dependencies after PCI
 root bridge initialization on RISC-V

On Tue, Jan 27, 2026 at 8:46 PM Bjorn Helgaas <helgaas@...nel.org> wrote:
>
> On Tue, Jan 27, 2026 at 06:50:24PM +0100, Rafael J. Wysocki wrote:
> > On Tue, Jan 27, 2026 at 6:26 PM Bjorn Helgaas <helgaas@...nel.org> wrote:
> > > On Tue, Jan 27, 2026 at 04:00:49PM +0100, Rafael J. Wysocki wrote:
> > > > On Mon, Jan 12, 2026 at 3:17 PM huyuye <huyuye812@....com> wrote:
> > > > >
> > > > > Hi Rafael,
> > > > > Thank you for your thorough review and valuable comments on v1.
> > > > > I've updated the patch as follows:
> > > > > 1. Removed the redundant #ifdef CONFIG_ACPI and if (!acpi_disabled)
> > > > > guard as you pointed out. The entire code block indeed already depends
> > > > > on CONFIG_ACPI at a higher level, making the inner guard unnecessary.
> > > > > 2. Moved acpi_dev_clear_dependencies to RISC-V specific architecture
> > > > > code (driver/acpi/riscv/acpi_pci.c). This ensures that ACPI dependency
> > > > > clearing is handled within the appropriate architectural context.
> > > > >
> > > > > Best regards
> > > > > Signed-off-by: huyuye <huyuye812@....com>
> > > > > ---
> > > > >  drivers/acpi/pci_root.c       |  6 ++++++
> > > > >  drivers/acpi/riscv/Makefile   |  2 +-
> > > > >  drivers/acpi/riscv/acpi_pci.c | 11 +++++++++++
> > > > >  include/acpi/acpi_bus.h       |  1 +
> > > > >  4 files changed, 19 insertions(+), 1 deletion(-)
> > > > >  create mode 100644 drivers/acpi/riscv/acpi_pci.c
> > > > >
> > > > > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> > > > > index 9d7f85dadc48..a16eb9097cdc 100644
> > > > > --- a/drivers/acpi/pci_root.c
> > > > > +++ b/drivers/acpi/pci_root.c
> > > > > @@ -30,6 +30,11 @@ static int acpi_pci_root_add(struct acpi_device *device,
> > > > >                              const struct acpi_device_id *not_used);
> > > > >  static void acpi_pci_root_remove(struct acpi_device *device);
> > > > >
> > > > > +
> > > > > +void __weak arch_acpi_pci_root_add_clear_dep(struct acpi_device *device)
> > > > > +{
> > > > > +}
> > > > > +
> > > > >  static int acpi_pci_root_scan_dependent(struct acpi_device *adev)
> > > > >  {
> > > > >         acpiphp_check_host_bridge(adev);
> > > > > @@ -760,6 +765,7 @@ static int acpi_pci_root_add(struct acpi_device *device,
> > > > >         pci_lock_rescan_remove();
> > > > >         pci_bus_add_devices(root->bus);
> > > > >         pci_unlock_rescan_remove();
> > > > > +       arch_acpi_pci_root_add_clear_dep(device);
> > > >
> > > > Actually, this could be as simple as
> > > >
> > > >        if (IS_ENABLED(CONFIG_RISCV))
> > > >               acpi_dev_clear_dependencies(device);
> > > >
> > > > with a brief comment explaining why it is needed.
> > > >
> > > > Bjorn, any thoughts?
> > >
> > > The justification ("If a host bridge B depends on host bridge A (via
> > > _DEP), this call allows bridge B to proceed with enumeration after
> > > bridge A is fully initialized") doesn't sound specific to RISC-V.
> >
> > But there are no _DEP dependencies between host bridgers on other
> > architectures in practice.
> >
> > acpi_dev_clear_dependencies() could be called unconditionally here,
> > but it would be useless overhead if no such dependencies existed.
> >
> > > For that matter, it doesn't sound specific to host bridges either.
> >
> > No, it is not specific to host bridges.
> >
> > > The _DEP spec language is a bit vague.  ACPI r6.6, sec 6.5.8, says:
> > >
> > >   _DEP evaluates to a package and designates device objects that OSPM
> > >   should assign a higher priority in start ordering due to
> > >   dependencies between devices (for example, related to future
> > >   operation region accesses).
> > >
> > > I don't know what "device start" means.  It sounds like this alludes
> > > to the order in which OSPM runs some device start method?  _INI?
> > > Should acpi_dev_clear_dependencies() be done at the point where that
> > > device start method is run?
> >
> > Not really.
> >
> > acpi_dev_clear_dependencies() is related to the way Linux uses _DEP
> > which is to defer the enumeration of dependent devices until the
> > devices they depend on are ready.
> >
> > So by calling acpi_dev_clear_dependencies() the driver basically
> > allows other drivers to bind to devices.
>
> I assumed the dependency expressed by _DEP would be satisfied by the
> execution of some other ACPI method.  E.g., the dependency might be
> satisfied when a _REG method makes an opregion available (although the
> spec seems to suggest that's only one of the possible dependencies).
>
> But in this case it sounds like RISC-V is using _DEP not because of
> any ACPI-related ordering requirement, but simply to enforce the OS
> enumeration order (and therefore naming).  I guess this refers to PCI
> device naming, so I suppose that dependency is on
> pci_acpi_scan_root().
>
> I thought udev was supposed to be the real solution for consistent
> naming.  Is this sort of a workaround to accomplish the same end?

IIUC, the enumeration ordering enforcement on RISC-V is related to a
functional dependency, but admittedly I'm unfamiliar with the details.

> In any case, your IS_ENABLED(CONFIG_RISCV) proposal seems fine to me.
> I think it's nice if we can avoid adding another __weak function.

OK, thanks!

I would also like to get some feedback on this from the RISC-V side.
Specifically on the reasons why the _DEP in question is used in the
firmware.

Powered by blists - more mailing lists