diff mbox series

[v1,1/1] PCI/EDR: Change ACPI event message log level

Message ID 01afb4e01efbe455de0c445bef6cf3ffc59340d2.1586996350.git.sathyanarayanan.kuppuswamy@linux.intel.com
State New
Headers show
Series [v1,1/1] PCI/EDR: Change ACPI event message log level | expand

Commit Message

Kuppuswamy, Sathyanarayanan April 16, 2020, 12:38 a.m. UTC
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Currently we have pci_info() message in the beginning of
edr_handle_event() function, which will be printing
notification details every-time firmware sends ACPI SYSTEM
level events. This will pollute the dmesg logs for systems
that has lot for ACPI system level notifications. So change
the log-level to pci_dbg, and add a new info log for EDR
events.

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/pci/pcie/edr.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Bjorn Helgaas April 24, 2020, 11:10 p.m. UTC | #1
On Wed, Apr 15, 2020 at 05:38:32PM -0700, sathyanarayanan.kuppuswamy@linux.intel.com wrote:
> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> 
> Currently we have pci_info() message in the beginning of
> edr_handle_event() function, which will be printing
> notification details every-time firmware sends ACPI SYSTEM
> level events. This will pollute the dmesg logs for systems
> that has lot for ACPI system level notifications. So change
> the log-level to pci_dbg, and add a new info log for EDR
> events.
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> ---
>  drivers/pci/pcie/edr.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pcie/edr.c b/drivers/pci/pcie/edr.c
> index 594622a6cb16..e346c82559fa 100644
> --- a/drivers/pci/pcie/edr.c
> +++ b/drivers/pci/pcie/edr.c
> @@ -148,11 +148,13 @@ static void edr_handle_event(acpi_handle handle, u32 event, void *data)
>  	pci_ers_result_t estate = PCI_ERS_RESULT_DISCONNECT;
>  	u16 status;
>  
> -	pci_info(pdev, "ACPI event %#x received\n", event);
> +	pci_dbg(pdev, "ACPI event %#x received\n", event);

I agree this looks excessively verbose.  Do we even need a pci_dbg()
message here?  Maybe a message like that would belong in ACPI?
There is already an ACPI_DEBUG_PRINT() in
acpi_ev_queue_notify_request() that would serve a similar purpose.

>  	if (event != ACPI_NOTIFY_DISCONNECT_RECOVER)
>  		return;
>  
> +	pci_info(pdev, "EDR event received\n");
> +
>  	/* Locate the port which issued EDR event */
>  	edev = acpi_dpc_port_get(pdev);
>  	if (!edev) {
> -- 
> 2.17.1
>
Kuppuswamy, Sathyanarayanan April 24, 2020, 11:15 p.m. UTC | #2
Hi Bjorn,

On 4/24/20 4:10 PM, Bjorn Helgaas wrote:
> On Wed, Apr 15, 2020 at 05:38:32PM -0700, sathyanarayanan.kuppuswamy@linux.intel.com wrote:
>> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>>
>> Currently we have pci_info() message in the beginning of
>> edr_handle_event() function, which will be printing
>> notification details every-time firmware sends ACPI SYSTEM
>> level events. This will pollute the dmesg logs for systems
>> that has lot for ACPI system level notifications. So change
>> the log-level to pci_dbg, and add a new info log for EDR
>> events.
>>
>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>> ---
>>   drivers/pci/pcie/edr.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/pcie/edr.c b/drivers/pci/pcie/edr.c
>> index 594622a6cb16..e346c82559fa 100644
>> --- a/drivers/pci/pcie/edr.c
>> +++ b/drivers/pci/pcie/edr.c
>> @@ -148,11 +148,13 @@ static void edr_handle_event(acpi_handle handle, u32 event, void *data)
>>   	pci_ers_result_t estate = PCI_ERS_RESULT_DISCONNECT;
>>   	u16 status;
>>   
>> -	pci_info(pdev, "ACPI event %#x received\n", event);
>> +	pci_dbg(pdev, "ACPI event %#x received\n", event);
> 
> I agree this looks excessively verbose.  Do we even need a pci_dbg()
> message here?  Maybe a message like that would belong in ACPI?
> There is already an ACPI_DEBUG_PRINT() in
> acpi_ev_queue_notify_request() that would serve a similar purpose.
I tried to keep it here to simplify enabling debug logs  related to
EDR. But as you have mentioned we could get similar message from
ACPI. If you think its excessive we can remove it. You want me to
send next version with this change ?
> 
>>   	if (event != ACPI_NOTIFY_DISCONNECT_RECOVER)
>>   		return;
>>   
>> +	pci_info(pdev, "EDR event received\n");
>> +
>>   	/* Locate the port which issued EDR event */
>>   	edev = acpi_dpc_port_get(pdev);
>>   	if (!edev) {
>> -- 
>> 2.17.1
>>
Bjorn Helgaas April 24, 2020, 11:36 p.m. UTC | #3
On Wed, Apr 15, 2020 at 05:38:32PM -0700, sathyanarayanan.kuppuswamy@linux.intel.com wrote:
> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> 
> Currently we have pci_info() message in the beginning of
> edr_handle_event() function, which will be printing
> notification details every-time firmware sends ACPI SYSTEM
> level events. This will pollute the dmesg logs for systems
> that has lot for ACPI system level notifications. So change
> the log-level to pci_dbg, and add a new info log for EDR
> events.
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

I dropped the pci_dbg() of all events, since there's a way to get
those logs from ACPI already (see acpi_ev_queue_notify_request(); I
didn't say it was *easy* :)) and applied this to pci/error for v5.8,
thanks!

> ---
>  drivers/pci/pcie/edr.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pcie/edr.c b/drivers/pci/pcie/edr.c
> index 594622a6cb16..e346c82559fa 100644
> --- a/drivers/pci/pcie/edr.c
> +++ b/drivers/pci/pcie/edr.c
> @@ -148,11 +148,13 @@ static void edr_handle_event(acpi_handle handle, u32 event, void *data)
>  	pci_ers_result_t estate = PCI_ERS_RESULT_DISCONNECT;
>  	u16 status;
>  
> -	pci_info(pdev, "ACPI event %#x received\n", event);
> +	pci_dbg(pdev, "ACPI event %#x received\n", event);
>  
>  	if (event != ACPI_NOTIFY_DISCONNECT_RECOVER)
>  		return;
>  
> +	pci_info(pdev, "EDR event received\n");
> +
>  	/* Locate the port which issued EDR event */
>  	edev = acpi_dpc_port_get(pdev);
>  	if (!edev) {
> -- 
> 2.17.1
>
diff mbox series

Patch

diff --git a/drivers/pci/pcie/edr.c b/drivers/pci/pcie/edr.c
index 594622a6cb16..e346c82559fa 100644
--- a/drivers/pci/pcie/edr.c
+++ b/drivers/pci/pcie/edr.c
@@ -148,11 +148,13 @@  static void edr_handle_event(acpi_handle handle, u32 event, void *data)
 	pci_ers_result_t estate = PCI_ERS_RESULT_DISCONNECT;
 	u16 status;
 
-	pci_info(pdev, "ACPI event %#x received\n", event);
+	pci_dbg(pdev, "ACPI event %#x received\n", event);
 
 	if (event != ACPI_NOTIFY_DISCONNECT_RECOVER)
 		return;
 
+	pci_info(pdev, "EDR event received\n");
+
 	/* Locate the port which issued EDR event */
 	edev = acpi_dpc_port_get(pdev);
 	if (!edev) {