Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <5a601e85-cd6c-2d42-a2db-d706a64718c9@zhaoxin.com>
Date:   Fri, 28 Oct 2022 19:36:38 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.0
Subject: Re: [PATCH 1/5] ACPI/APEI: Add apei_hest_parse_aer()
To:     Bjorn Helgaas <helgaas@kernel.org>
CC:     <rafael@kernel.org>, <lenb@kernel.org>, <james.morse@arm.com>,
        <tony.luck@intel.com>, <bp@alien8.de>, <robert.moore@intel.com>,
        <ying.huang@intel.com>, <rdunlap@infradead.org>,
        <bhelgaas@google.com>, <linux-acpi@vger.kernel.org>,
        <linux-pci@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <devel@acpica.org>, <CobeChen@zhaoxin.com>,
        <TonyWWang@zhaoxin.com>, <ErosZhang@zhaoxin.com>
References: <20221027220746.GA844491@bhelgaas>
From:   LeoLiuoc <LeoLiu-oc@zhaoxin.com>
In-Reply-To: <20221027220746.GA844491@bhelgaas>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


在 2022/10/28 6:07, Bjorn Helgaas 写道:
> On Thu, Oct 27, 2022 at 11:15:18AM +0800, LeoLiu-oc wrote:
>> From: leoliu-oc <leoliu-oc@zhaoxin.com>
>>
>> apei_hest_parse_aer() is used to parse and record the PCI Express AER
>> Structure in the HEST Table.
>>
>> Signed-off-by: leoliu-oc <leoliu-oc@zhaoxin.com>
>> ---
>>   drivers/acpi/apei/hest.c | 119 ++++++++++++++++++++++++++++++++++++++-
>>   include/acpi/actbl1.h    |  69 +++++++++++++++++++++++
>>   include/acpi/apei.h      |   7 +++
>>   3 files changed, 194 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/acpi/apei/hest.c b/drivers/acpi/apei/hest.c
>> index 6aef1ee5e1bd..0bfdc18758f5 100644
>> --- a/drivers/acpi/apei/hest.c
>> +++ b/drivers/acpi/apei/hest.c
>> @@ -25,6 +25,7 @@
>>   #include <linux/platform_device.h>
>>   #include <acpi/apei.h>
>>   #include <acpi/ghes.h>
>> +#include <linux/pci.h>
>>   
>>   #include "apei-internal.h"
>>   
>> @@ -86,7 +87,48 @@ static int hest_esrc_len(struct acpi_hest_header *hest_hdr)
>>   	return len;
>>   };
>>   
>> -typedef int (*apei_hest_func_t)(struct acpi_hest_header *hest_hdr, void *data);
>> +static inline bool hest_source_is_pcie_aer(struct acpi_hest_header *hest_hdr)
> 
> Drop "inline" here and below.  This is not a performance path, so it's
> more clutter than it's worth.

Yes, I agree with it. Will fix it in the next version.

Thanks
leoliu-oc

>> +static inline bool hest_match_pci(struct acpi_hest_header *hest_hdr,
>> +		struct acpi_hest_aer_common *p, struct pci_dev *pci)
>> +{
>> +	if (hest_match_type(hest_hdr, pci))
>> +		return(hest_match_pci_devfn(p, pci));
> 
> No need for parens around hest_match_pci_devfn().
> 
>> +	else
> 
> You can drop the else, too.
> 

Yes, Agree with it. I will also fix it in the next version.

Thanks
leoliu-oc
>> +		return false;
>> +}
> 
>> + * apei_hest_parse_aer - Find the AER structure in the HEST Table and
>> + * match it with the PCI device.
>> + *
>> + * @hest_hdr: To save the acpi aer error source in hest table
>> + *
>> + * Return 1 if the pci dev matched with the acpi aer error source in
>> + * hest table, else return 0.
> 
> In comments and commit logs,
> 
>    s/pci/PCI/
>    s/aer/AER/
>    s/acpi/ACPI/
>    s/hest/HEST/
>    s/HEST Table/HEST/ (since the "T" in "HEST" stands for "Table")
> 
>> +/* HEST Sub-structure for PCIE EndPoint Structure (6) */
> 
> PCIe Root Port, if I'm following this correctly.
> 
>> +/* HEST Sub-structure for PCIE EndPoint Structure (7) */
> 
> PCIe Endpoint.
> 
>> +/* HEST Sub-structure for PCIE/PCI Bridge Structure (8) */
> 
> PCIe/PCI-X Bridge

Got it, will modify it in the next version.

Thanks
leoliu-oc