Message-ID: <ef311715-3e61-4bf5-bdae-58fd87a3d5e7@163.com>
Date: Tue, 8 Apr 2025 20:19:41 +0800
From: Hans Zhang <18255117159@....com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: lpieralisi@...nel.org, bhelgaas@...gle.com, kw@...ux.com,
 manivannan.sadhasivam@...aro.org, robh@...nel.org, jingoohan1@...il.com,
 thomas.richard@...tlin.com, linux-pci@...r.kernel.org,
 LKML <linux-kernel@...r.kernel.org>
Subject: Re: [v7 2/5] PCI: Refactor capability search functions to eliminate
 code duplication



On 2025/4/8 01:03, Ilpo Järvinen wrote:
>> Hi Ilpo,
>>
>> The [v9 3/6]patch I plan to submit is as follows, please review it.
>>
>>  From 6da415d130e76b57ecf401f14bf0b66f20407839 Mon Sep 17 00:00:00 2001
>> From: Hans Zhang<18255117159@....com>
>> Date: Fri, 4 Apr 2025 00:20:29 +0800
>> Subject: [v9 3/6] PCI: Refactor capability search into common macros
>>
>> - Capability search is done both in PCI core and some controller drivers.
>> - PCI core's cap search func requires PCI device and bus structs exist.
>> - Controller drivers cannot use PCI core's cap search func as they
>>    need to find capabilities before they instantiated the PCI device & bus
>>    structs.
>>
>> - Move capability search into a macro so it can be reused where normal
>>    PCI config space accessors cannot yet be used due to lack of the
>>    instantiated PCI dev.
>> - Instead, give the config space reading function as an argument to the
>>    new macro.
>> - Convert PCI core to use the new macro.
> None of these bullets are true lists so please write them as normal
> English paragraphs. Also please extend some of shortened words lke "cap"
> --> "Capability", "PCI dev" -> PCI Device (for terms, the capitalization
> of the first letter, you should follow what the PCI specs use).
> 

Dear Ilpo,

Thank you very much for your reply. Is it OK to modify it like this?

The PCI Capability search functionality is duplicated across the PCI 
core and several controller drivers.  The core's current implementation
requires fully initialized PCI device and bus structures, which prevents
controller drivers from using it during early initialization phases 
before these structures are available.

Move the Capability search logic into a header-based macro that accepts 
a config space accessor function as an argument.  This enables 
controller drivers to perform Capability discovery using their early 
access mechanisms prior to full device initialization while maintaining 
the original search behavior.

Convert the existing PCI core Capability search implementation to use 
this new macro, eliminating code duplication. The refactoring preserves 
the original functionality without behavioral changes, while allowing 
both the core and drivers to share common Capability discovery logic.

Best regards,
Hans

>> The macros now implement, parameterized by the config access method. The
>> PCI core functions are converted to utilize these macros with the standard
>> pci_bus_read_config accessors. Controller drivers can later use the same
>> macros with their early access mechanisms while maintaining the existing
>> protection against infinite loops through preserved TTL checks.
>>
>> The ttl parameter was originally an additional safeguard to prevent
>> infinite loops in corrupted config space.  However, the
>> PCI_FIND_NEXT_CAP_TTL macro already enforces a TTL limit internally.
>> Removing redundant ttl handling simplifies the interface while maintaining
>> the safety guarantee. This aligns with the macro's design intent of
>> encapsulating TTL management.
>>
>> Signed-off-by: Hans Zhang<18255117159@....com>
>> ---
>>   drivers/pci/pci.c | 70 +++++---------------------------------
>>   drivers/pci/pci.h | 86 +++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 95 insertions(+), 61 deletions(-)

Powered by blists - more mailing lists