Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
Message-ID: <065980bb-5890-48ba-b75b-86b3632fd826@intel.com>
Date:   Fri, 6 Oct 2023 19:29:06 +0200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v1 2/9] docs: firmware-guide: ACPI: Clarify ACPI bus
 concepts
Content-Language: en-US
To:     "Rafael J. Wysocki" <rafael@kernel.org>
CC:     "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        <linux-acpi@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <nvdimm@lists.linux.dev>, <rafael.j.wysocki@intel.com>,
        <andriy.shevchenko@intel.com>, <lenb@kernel.org>,
        <dan.j.williams@intel.com>, <vishal.l.verma@intel.com>,
        <ira.weiny@intel.com>, <rui.zhang@intel.com>,
        Elena Reshetova <elena.reshetova@intel.com>
References: <20230925144842.586829-1-michal.wilczynski@intel.com>
 <20230925144842.586829-3-michal.wilczynski@intel.com>
 <2725050.mvXUDI8C0e@kreacher>
 <edb1d4bc-a484-42ea-9a5c-4c77a860645a@intel.com>
 <04972447-91b5-4392-acf0-c942e02403c2@intel.com>
 <CAJZ5v0h3ZggWKAuKDbFnPbhWpcwnDVk+OrBPRsahM87_H=ob1A@mail.gmail.com>
From:   "Wilczynski, Michal" <michal.wilczynski@intel.com>
In-Reply-To: <CAJZ5v0h3ZggWKAuKDbFnPbhWpcwnDVk+OrBPRsahM87_H=ob1A@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SVJNNFdaeUl6QllkVHNDRTB5SVIyaVBOMUFodllkYzVDWlpCQkQyeEpMWVRa?=
 =?utf-8?B?K2d3UzByN2tvcmcvTTI3UWRkK0xtaHFJM3l3WHZxbHVDWHB6N0dkVkpZOEV3?=
 =?utf-8?B?bUM2Vm9qNmFSeUtISVNXeS95ZTRuM2RvTEp6VG5vMlc3aDZXWjdORHVIYkZU?=
 =?utf-8?B?RE9ZV3NoK3BkbGE5NHp3UHpiSkdxckxGSEFFOHpINDYzYUZHRXlBUElmNlZB?=
 =?utf-8?B?QWM0bkdSOUNBU3JUWEx2KzRkTkFSVkZMT2RZUzN5NkN1blViUllLdGx0QXFx?=
 =?utf-8?B?SDBmaDU1c3RuMzRRUDZmVTlkcDIxNnN4SDhlMUJTcU1lalFiNjV5VHNwdU1U?=
 =?utf-8?B?Q0JxTmZHMDBUWTVQYzFoSkdJcmpWOTIreDRXcWY2a1Z1YzBubXFjOC9QT3Uv?=
 =?utf-8?B?K25zN2s4NzlTMmlHeHVhSjAvVjZnY2NZZThwVzlRZDZsY3VrY1JwdVJOMWE4?=
 =?utf-8?B?QWIvT3FTMkZaMkZGVk96MUxIZVlkOEpKY0hCN3d4N3FDQWNmamVYZVFPbVFz?=
 =?utf-8?B?aml0bU9PajF6WWc1VnQvdFBCNlNJL0dLWEwxYkpjUnI0RlMyMHhFNlRUNEVr?=
 =?utf-8?B?VUdINXFpOTBHUDRuUlZwYzJqR2ZVNytnRmRZSFVzeGNtNGlpTFZ5QXJFb1ky?=
 =?utf-8?B?UW9RVk5Cek1ZVm9FWW9lcCtIeFJTcU8wWU1xQkhnNXZHUHNDWnZ5YlY0bkd0?=
 =?utf-8?B?S3BmK2hUWVRNcnk4UDhKb0xsOFZVZ3Axc2FWSGQvVE9VRHhWbGw4VkE0QVhz?=
 =?utf-8?B?VGFya0JyQTd3V0pTQm43RStnNjVKdVRKc2RhRWFzbUJFZjJGYlpOcUx2cTdX?=
 =?utf-8?B?WldZeXZvTHlZZ21oYzJDSkU2alg0bkdXdU14aHFsNjRZVUdvZWhaVWdmK0Fy?=
 =?utf-8?B?eXJRTDVyM1A3M1kyVWE5R2o2dFFoWWU1WnpOUXhqd3Bqc3gwK2dvRXE2U3pI?=
 =?utf-8?B?SmRjOXluSDIzNDhhVGhJeU8vS0FwSjZnZEp5Mk8xN1FoUW1Gek5TVmMyQ0ZI?=
 =?utf-8?B?clUrU0tldGJiVG1DN01YMm1Jc1pFRzJOQ1Q3am1iemZ6WnFWem5INGlIOW5S?=
 =?utf-8?B?dFpiVjYxTm9UN3N1NndneXYydFY3OGNTdjBmczFjSS8yOFVyeFkwOUxtNU9P?=
 =?utf-8?B?WHlvd3IrSmhRWGJYeHZRdnEvTW5QRW1acTFTUDN1VlJ6WkVxN05xNmhIVGVZ?=
 =?utf-8?B?WktSbDM2emxHNHBqcEJrelppbG1VQ0RkbGlTQzRIbU9WQi9yMG5MRzVDM3F2?=
 =?utf-8?B?cjBHcFY3ZHJVK2NveCtyRGtHaUR1UlRwRjg1L0J1UTlYK2NtRFRuazY3M2tS?=
 =?utf-8?B?TXh0VnZZeU9memJLODF6Wm5YSzZUYytCY2RTa3FsUWlWN2JBajJTTWt5T3U2?=
 =?utf-8?B?cTZhaXcvYUZ0MVI4a3J4bmxVYTZmVnpucW5EZUNrcWdRSDh2L2EvMHV6UzRK?=
 =?utf-8?B?YzRCa3Q2YlpuR3pYSnJiTGtEbVZsTUsxUG5Cb2w3dzBJMzRuOUFRUSs0WEJv?=
 =?utf-8?B?WDQyQnFqSFpqZjlzRk5UenNGcHgzU0d5SHk4cklldDh0MHNtc05xLzhySW1S?=
 =?utf-8?B?SldTK2tXdGNlTENhMzg4N0tlZ045dGFiWDNGYS9rZDB5UlpoREZkMXJwNVFW?=
 =?utf-8?B?U2ttNzN3c09oWkpzcUlnMzFoOE9mUE9abklISmwrN0RiZEliZmNQWUpocEsz?=
 =?utf-8?B?NUl4VlZYR0pYaWNuYVZLc1lwL0dKdU5YemhIblRoR0VtZ0I5MFhuM2hqWVRX?=
 =?utf-8?B?VFMraHpnQ3BWb2tScEhhLy9hVnV6a2IyVXEzNjRjNGdGSDk3dGlDYTk5b3Q4?=
 =?utf-8?B?MG1nKzFMMitTOGQvbFJpd3lveVBwL3Y0cU1lU3dPMXA4ZmFnWHdoemtRSEti?=
 =?utf-8?B?NmZtWEJ1Q2o5YUdqWU1HUzJrWTluUjVydThaVVYwUUhnRTY2UEtodm1ScUhZ?=
 =?utf-8?B?emEyaVZ4L1RKcThFWVV3cVZYMjU4eCt4eVJ3WTBSQkFoVnVnMEhiRWhMd2Rq?=
 =?utf-8?B?OWt0TzVSM2ljUVYxYzZCbDlIVmNwcHQvL01hd1dZZStWZFRCczNKaVZXWFVx?=
 =?utf-8?B?Qkp3VnlvRGVxTU5QNE9KdGJjZCtLOEFRaGVzNTBmRkRrZEFOUTRheldLZEpm?=
 =?utf-8?B?TTJhREE4WjBNbVBpanl1NUpOOXFRSSszSE9rYTcwRlBGYWZVNlU1RkVSSEd0?=
 =?utf-8?B?Tmc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 49999138-5bc5-4c10-e4e4-08dbc691c2f4
X-MS-Exchange-CrossTenant-AuthSource: CO6PR11MB5603.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Oct 2023 17:29:14.9654
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: LMiiqoafAaoMiWM8TshP2LtEUC2YQVmjQ1AWsTtyeZ+TTcii+B4xZGkSNtnvcho9PA9iSzsYd++Fe6QwxQ8pfqC74wV6GHquXjV1DxFS51M=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV3PR11MB8696
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=2.7 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,RCVD_IN_SBL_CSS,SPF_HELO_NONE,SPF_PASS autolearn=no
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on howler.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (howler.vger.email [0.0.0.0]); Fri, 06 Oct 2023 10:29:40 -0700 (PDT)
X-Spam-Level: **


On 10/6/2023 5:36 PM, Rafael J. Wysocki wrote:
> On Thu, Oct 5, 2023 at 10:39 PM Wilczynski, Michal
> <michal.wilczynski@intel.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@intel.com>
>>>>> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.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@intel.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@intel.com>
>>>> Co-developed-by: Michal Wilczynski <michal.wilczynski@intel.com>
>>>> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
>>>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.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).

Thanks, great !
Will re-send the rest of the patchset.

Michał

>