Message-ID: <CAJZ5v0h3ZggWKAuKDbFnPbhWpcwnDVk+OrBPRsahM87_H=ob1A@mail.gmail.com>
Date:   Fri, 6 Oct 2023 17:36:57 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     "Wilczynski, Michal" <michal.wilczynski@...el.com>
Cc:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
        nvdimm@...ts.linux.dev, rafael.j.wysocki@...el.com,
        andriy.shevchenko@...el.com, lenb@...nel.org,
        dan.j.williams@...el.com, vishal.l.verma@...el.com,
        ira.weiny@...el.com, rui.zhang@...el.com,
        Elena Reshetova <elena.reshetova@...el.com>
Subject: Re: [PATCH v1 2/9] docs: firmware-guide: ACPI: Clarify ACPI bus concepts

On Thu, Oct 5, 2023 at 10:39 PM Wilczynski, Michal
<michal.wilczynski@...el.com> wrote:
>
>
>
> On 10/5/2023 8:28 PM, Wilczynski, Michal wrote:
> >
> > On 10/5/2023 7:57 PM, Rafael J. Wysocki wrote:
> >> On Monday, September 25, 2023 4:48:35 PM CEST Michal Wilczynski wrote:
> >>> Some devices implement ACPI driver as a way to manage devices
> >>> enumerated by the ACPI. This might be confusing as a preferred way to
> >>> implement a driver for devices not connected to any bus is a platform
> >>> driver, as stated in the documentation. Clarify relationships between
> >>> ACPI device, platform device and ACPI entries.
> >>>
> >>> Suggested-by: Elena Reshetova <elena.reshetova@...el.com>
> >>> Signed-off-by: Michal Wilczynski <michal.wilczynski@...el.com>
> >>> ---
> >>>  Documentation/firmware-guide/acpi/enumeration.rst | 13 +++++++++++++
> >>>  1 file changed, 13 insertions(+)
> >>>
> >>> diff --git a/Documentation/firmware-guide/acpi/enumeration.rst b/Documentation/firmware-guide/acpi/enumeration.rst
> >>> index 56d9913a3370..f56cc79a9e83 100644
> >>> --- a/Documentation/firmware-guide/acpi/enumeration.rst
> >>> +++ b/Documentation/firmware-guide/acpi/enumeration.rst
> >>> @@ -64,6 +64,19 @@ If the driver needs to perform more complex initialization like getting and
> >>>  configuring GPIOs it can get its ACPI handle and extract this information
> >>>  from ACPI tables.
> >>>
> >>> +ACPI bus
> >>> +====================
> >>> +
> >>> +Historically some devices not connected to any bus were represented as ACPI
> >>> +devices, and had to implement ACPI driver. This is not a preferred way for new
> >>> +drivers. As explained above devices not connected to any bus should implement
> >>> +platform driver. ACPI device would be created during enumeration nonetheless,
> >>> +and would be accessible through ACPI_COMPANION() macro, and the ACPI handle would
> >>> +be accessible through ACPI_HANDLE() macro. ACPI device is meant to describe
> >>> +information related to ACPI entry e.g. handle of the ACPI entry. Think -
> >>> +ACPI device interfaces with the FW, and the platform device with the rest of
> >>> +the system.
> >>> +
> >>>  DMA support
> >>>  ===========
> >> I rewrote the above entirely, so here's a new patch to replace this one:
> >>
> >> ---
> >> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> >> Subject: [PATCH v2 2/9] ACPI: docs: enumeration: Clarify ACPI bus concepts
> >>
> >> In some cases, ACPI drivers are implemented as a way to manage devices
> >> enumerated with the help of the platform firmware through ACPI.
> >>
> >> This might be confusing, since the preferred way to implement a driver
> >> for a device that cannot be enumerated natively, is a platform
> >> driver, as stated in the documentation.
> >>
> >> Clarify relationships between ACPI device objects, platform devices and
> >> ACPI Namespace entries.
> >>
> >> Suggested-by: Elena Reshetova <elena.reshetova@...el.com>
> >> Co-developed-by: Michal Wilczynski <michal.wilczynski@...el.com>
> >> Signed-off-by: Michal Wilczynski <michal.wilczynski@...el.com>
> >> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> >> ---
> >>  Documentation/firmware-guide/acpi/enumeration.rst |   43 ++++++++++++++++++++++
> >>  1 file changed, 43 insertions(+)
> >>
> >> Index: linux-pm/Documentation/firmware-guide/acpi/enumeration.rst
> >> ===================================================================
> >> --- linux-pm.orig/Documentation/firmware-guide/acpi/enumeration.rst
> >> +++ linux-pm/Documentation/firmware-guide/acpi/enumeration.rst
> >> @@ -64,6 +64,49 @@ If the driver needs to perform more comp
> >>  configuring GPIOs it can get its ACPI handle and extract this information
> >>  from ACPI tables.
> >>
> >> +ACPI device objects
> >> +===================
> >> +
> >> +Generally speaking, there are two categories of devices in a system in which
> >> +ACPI is used as an interface between the platform firmware and the OS: Devices
> >> +that can be discovered and enumerated natively, through a protocol defined for
> >> +the specific bus that they are on (for example, configuration space in PCI),
> >> +without the platform firmware assistance, and devices that need to be described
> >> +by the platform firmware so that they can be discovered.  Still, for any device
> >> +known to the platform firmware, regardless of which category it falls into,
> >> +there can be a corresponding ACPI device object in the ACPI Namespace in which
> >> +case the Linux kernel will create a struct acpi_device object based on it for
> >> +that device.
> >> +
> >> +Those struct acpi_device objects are never used for binding drivers to natively
> >> +discoverable devices, because they are represented by other types of device
> >> +objects (for example, struct pci_dev for PCI devices) that are bound to by
> >> +device drivers (the corresponding struct acpi_device object is then used as
> >> +an additional source of information on the configuration of the given device).
> >> +Moreover, the core ACPI device enumeration code creates struct platform_device
> >> +objects for the majority of devices that are discovered and enumerated with the
> >> +help of the platform firmware and those platform device objects can be bound to
> >> +by platform drivers in direct analogy with the natively enumerable devices
> >> +case.  Therefore it is logically inconsistent and so generally invalid to bind
> >> +drivers to struct acpi_device objects, including drivers for devices that are
> >> +discovered with the help of the platform firmware.
> >> +
> >> +Historically, ACPI drivers that bound directly to struct acpi_device objects
> >> +were implemented for some devices enumerated with the help of the platform
> >> +firmware, but this is not recommended for any new drivers.  As explained above,
> >> +platform device objects are created for those devices as a rule (with a few
> >> +exceptions that are not relevant here) and so platform drivers should be used
> >> +for handling them, even though the corresponding ACPI device objects are the
> >> +only source of device configuration information in that case.
> >> +
> >> +For every device having a corresponding struct acpi_device object, the pointer
> >> +to it is returned by the ACPI_COMPANION() macro, so it is always possible to
> >> +get to the device configuration information stored in the ACPI device object
> >> +this way.  Accordingly, struct acpi_device can be regarded as a part of the
> >> +interface between the kernel and the ACPI Namespace, whereas device objects of
> >> +other types (for example, struct pci_dev or struct platform_device) are used
> >> +for interacting with the rest of the system.
> >> +
> >>  DMA support
> >>  ===========
> > Thanks a lot !
> > Looks very good, will include this in next revision.
> >
> > Michał
>
> Aww, forgot that you can also just apply it yourself, so I can just fetch and
> rebase. Whichever version you prefer is fine with me :-)

So I went ahead and queued up my versions of patches [1-2/9].  They
are present in the acpi-bus branch in linux-pm.git (based on 6.6-rc4)
and in the bleeding-edge branch (I'll merge acpi-bus into linux-next
next week if all goes well).

Powered by blists - more mailing lists