Linux PCI subsystem development
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: huyuye <huyuye812@163.com>, Bjorn Helgaas <bhelgaas@google.com>,
	Sunil V L <sunilvl@ventanamicro.com>,
	Paul Walmsley <pjw@kernel.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Alexandre Ghiti <alex@ghiti.fr>,
	Robert Moore <robert.moore@intel.com>,
	linux-pci@vger.kernel.org, linux-acpi@vger.kernel.org,
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
	dai.hualiang@zte.com.cn, deng.weixian@zte.com.cn,
	guo.chang2@zte.com.cn, liu.qingtao2@zte.com.cn,
	wu.jiabao@zte.com.cn, lin.yongchun@zte.com.cn,
	hu.yuye@zte.com.cn, zhang.longxiang@zte.com.cn,
	zuo.jiang@zte.com.cn, li.kunpeng@zte.com.cn
Subject: Re: [PATCH v2] ACPI: pci_root: Clear the acpi dependencies after PCI root bridge initialization on RISC-V
Date: Tue, 27 Jan 2026 13:46:48 -0600	[thread overview]
Message-ID: <20260127194648.GA368841@bhelgaas> (raw)
In-Reply-To: <CAJZ5v0iZukqD0atbdfBnmZAxUzNB6sTcJ1YUeAo5a8NRn-emEg@mail.gmail.com>

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@kernel.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@163.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@163.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?

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.

Bjorn

  reply	other threads:[~2026-01-27 19:46 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-03 14:07 [PATCH] ACPI: pci_root: Clear the acpi dependencies after PCI root bridge initialization on RISC-V huyuye
2026-01-08 14:04 ` Rafael J. Wysocki
2026-01-12 14:16 ` [PATCH v2] " huyuye
2026-01-13 21:56   ` Bjorn Helgaas
2026-01-14 12:32   ` [PATCH v3] " huyuye
2026-01-29 14:42     ` [PATCH v4] " huyuye
2026-01-27 15:00   ` [PATCH v2] " Rafael J. Wysocki
2026-01-27 17:25     ` Bjorn Helgaas
2026-01-27 17:50       ` Rafael J. Wysocki
2026-01-27 19:46         ` Bjorn Helgaas [this message]
2026-01-27 20:04           ` Rafael J. Wysocki
2026-01-29 13:49           ` Sunil V L
2026-02-19 15:47             ` Lorenzo Pieralisi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260127194648.GA368841@bhelgaas \
    --to=helgaas@kernel.org \
    --cc=alex@ghiti.fr \
    --cc=aou@eecs.berkeley.edu \
    --cc=bhelgaas@google.com \
    --cc=dai.hualiang@zte.com.cn \
    --cc=deng.weixian@zte.com.cn \
    --cc=guo.chang2@zte.com.cn \
    --cc=hu.yuye@zte.com.cn \
    --cc=huyuye812@163.com \
    --cc=li.kunpeng@zte.com.cn \
    --cc=lin.yongchun@zte.com.cn \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=liu.qingtao2@zte.com.cn \
    --cc=palmer@dabbelt.com \
    --cc=pjw@kernel.org \
    --cc=rafael@kernel.org \
    --cc=robert.moore@intel.com \
    --cc=sunilvl@ventanamicro.com \
    --cc=wu.jiabao@zte.com.cn \
    --cc=zhang.longxiang@zte.com.cn \
    --cc=zuo.jiang@zte.com.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox