From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1763415AbdJQRNg (ORCPT ); Tue, 17 Oct 2017 13:13:36 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:49800 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757954AbdJQRNe (ORCPT ); Tue, 17 Oct 2017 13:13:34 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org E1879607CC Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=tbaicar@codeaurora.org Subject: Re: [PATCH] PCI/AER: update AER status string print to match other AER logs To: David Laight , "bhelgaas@google.com" , "helgaas@kernel.org" , "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" References: <1508254922-30925-1-git-send-email-tbaicar@codeaurora.org> <063D6719AE5E284EB5DD2968C1650D6DD0099999@AcuExch.aculab.com> From: Tyler Baicar Message-ID: Date: Tue, 17 Oct 2017 13:13:31 -0400 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD0099999@AcuExch.aculab.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/17/2017 12:00 PM, David Laight wrote: > From: Tyler Baicar >> Sent: 17 October 2017 16:42 >> Currently the AER driver uses cper_print_bits() to print the AER status >> string. This causes the status string to not include the proper PCI device >> name prefix that the other AER prints include. Also, it has a different >> print level than all the other AER prints. >> >> Update the AER driver to print the AER status string with the proper string >> prefix and proper print level. >> >> Previous log example: >> >> e1000e 0003:01:00.1: aer_status: 0x00000041, aer_mask: 0x00000000 >> Receiver Error, Bad TLP > ... >> New log: >> >> e1000e 0003:01:00.1: aer_status: 0x00000041, aer_mask: 0x00000000 >> e1000e 0003:01:00.1: Receiver Error >> e1000e 0003:01:00.1: Bad TLP > Wouldn't it be better to manage to print the above all on 1 line? Hello David, I broke them up into separate lines to simplify the code. If you look at cper_print_bits(), it is not a clean solution and involves some hard coded values to try to limit the lines to 80 characters. http://elixir.free-electrons.com/linux/v4.14-rc5/source/drivers/firmware/efi/cper.c#L85 I think printing one error per line in this case is a better solution since the code is much cleaner. If you would like me to add this code to print them in a list and limit the lines to 80 characters I can add that in though. > > ... >> index 54c4b69..b718daa 100644 >> --- a/drivers/pci/pcie/aer/aerdrv_errprint.c >> +++ b/drivers/pci/pcie/aer/aerdrv_errprint.c >> @@ -206,6 +206,19 @@ void aer_print_port_info(struct pci_dev *dev, struct aer_err_info *info) >> } >> >> #ifdef CONFIG_ACPI_APEI_PCIEAER >> +void dev_print_bits(struct pci_dev *dev, unsigned int bits, >> + const char * const strs[], unsigned int strs_size) > static and rename to aer_print_bits since this isn't a generic 'dev' > function. Will do. Thanks, Tyler -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.