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;
Date:   Thu, 27 Oct 2022 17:07:46 -0500
From:   Bjorn Helgaas <helgaas@kernel.org>
To:     LeoLiu-oc <LeoLiu-oc@zhaoxin.com>
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
Subject: Re: [PATCH 1/5] ACPI/APEI: Add apei_hest_parse_aer()
Message-ID: <20221027220746.GA844491@bhelgaas>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221027031518.2855743-1-LeoLiu-oc@zhaoxin.com>
Precedence: bulk

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.

> +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.

> +		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