Re: [PATCH v5 4/8] PCI/AER: Rename aer_print_port_info() to aer_printrp_info()

[Karolina Stolarek <karolina.stolarek@oracle.com>]

On 21/03/2025 02:58, Jon Pan-Doh wrote:
> Update function/param names to be more descriptive. This is a
> preparatory patch for when source devices are iterated through
> to institue rate limits.

This commit description doesn't provide enough information on why and 
how we make the names more descriptive. With this change, we want to 
make it clear that this function logs information about the Root Port 
that received an error message, not just a generic PCIe Port.

Also, there's a typo in the subject:
s/aer_printrp_info()/aer_print_rp_info()/

I'm fine with the code changes but I'd like to see the new commit 
description before giving the r-b.

All the best,
Karolina

> 
> Signed-off-by: Jon Pan-Doh <pandoh@google.com>
> Reported-by: Sargun Dhillon <sargun@meta.com>
> ---
>   drivers/pci/pcie/aer.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index e5db1fdd8421..3c63a6963608 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -727,15 +727,15 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info,
>   			info->severity, info->tlp_header_valid, &info->tlp);
>   }
>   
> -static void aer_print_port_info(struct pci_dev *dev, struct aer_err_info *info)
> +static void aer_print_rp_info(struct pci_dev *rp, struct aer_err_info *info)
>   {
>   	u8 bus = info->id >> 8;
>   	u8 devfn = info->id & 0xff;
>   
> -	pci_info(dev, "%s%s error message received from %04x:%02x:%02x.%d\n",
> +	pci_info(rp, "%s%s error message received from %04x:%02x:%02x.%d\n",
>   		 info->multi_error_valid ? "Multiple " : "",
>   		 aer_error_severity_string[info->severity],
> -		 pci_domain_nr(dev->bus), bus, PCI_SLOT(devfn),
> +		 pci_domain_nr(rp->bus), bus, PCI_SLOT(devfn),
>   		 PCI_FUNC(devfn));
>   }
>   
> @@ -1299,7 +1299,7 @@ static void aer_isr_one_error(struct aer_rpc *rpc,
>   			e_info.multi_error_valid = 1;
>   		else
>   			e_info.multi_error_valid = 0;
> -		aer_print_port_info(pdev, &e_info);
> +		aer_print_rp_info(pdev, &e_info);
>   
>   		if (find_source_device(pdev, &e_info))
>   			aer_process_err_devices(&e_info, KERN_WARNING);
> @@ -1318,7 +1318,7 @@ static void aer_isr_one_error(struct aer_rpc *rpc,
>   		else
>   			e_info.multi_error_valid = 0;
>   
> -		aer_print_port_info(pdev, &e_info);
> +		aer_print_rp_info(pdev, &e_info);
>   
>   		if (find_source_device(pdev, &e_info))
>   			aer_process_err_devices(&e_info, KERN_ERR);