[RFC PATCH v1 13/30] platform/x86: wmi: use dynamic debug to print data about events

[RFC PATCH v1 13/30] platform/x86: wmi: use dynamic debug to print data about events

[Barnabás Pőcze]

The dynamic debug framework provides a more flexible
way to configure debugging messages emitted by the kernel
than module options. Use `dev_dbg()` in `acpi_wmi_notify_handler()`
to print the event identifier and device name (which is the GUID).

Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
---
 drivers/platform/x86/wmi.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 90ba75247d7f..8aad8f080c64 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -1313,8 +1313,7 @@ static void acpi_wmi_notify_handler(acpi_handle handle, u32 event,
 		wblock->handler(event, wblock->handler_data);
 	}

-	if (debug_event)
-		pr_info("DEBUG Event GUID: %pUL\n", wblock->gblock.guid);
+	dev_dbg(&wblock->dev.dev, "event 0x%02X\n", event);

 	acpi_bus_generate_netlink_event(
 		wblock->acpi_device->pnp.device_class,
--
2.33.0

Re: [RFC PATCH v1 13/30] platform/x86: wmi: use dynamic debug to print data about events

[Hans de Goede]

Hi,

On 9/4/21 7:55 PM, Barnabás Pőcze wrote:
> The dynamic debug framework provides a more flexible
> way to configure debugging messages emitted by the kernel
> than module options. Use `dev_dbg()` in `acpi_wmi_notify_handler()`
> to print the event identifier and device name (which is the GUID).
> 
> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
> ---
>  drivers/platform/x86/wmi.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index 90ba75247d7f..8aad8f080c64 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -1313,8 +1313,7 @@ static void acpi_wmi_notify_handler(acpi_handle handle, u32 event,
>  		wblock->handler(event, wblock->handler_data);
>  	}
> 
> -	if (debug_event)
> -		pr_info("DEBUG Event GUID: %pUL\n", wblock->gblock.guid);
> +	dev_dbg(&wblock->dev.dev, "event 0x%02X\n", event);

The debug_event value gets set by a module-parameter and several WMI related
howto-s and forum threads on the web refer to this. At one point in time even:
https://wiki.ubuntu.com/Hotkeys/Troubleshooting

Used to refer to this, but they seem to have dropped this.

Either way this changes makes users have to also deal with dyndbg stuff to
get the same info which before they could get with just the debug_event module
param, which makes debugging harder, so I'm going to drop this patch from the
series.

Regards,

Hans


> 
>  	acpi_bus_generate_netlink_event(
>  		wblock->acpi_device->pnp.device_class,
> --
> 2.33.0
> 
> 

Re: [RFC PATCH v1 13/30] platform/x86: wmi: use dynamic debug to print data about events

[Barnabás Pőcze]

Hi


2021. szeptember 13., hétfő 11:43 keltezéssel, Hans de Goede írta:
> > -	if (debug_event)
> > -		pr_info("DEBUG Event GUID: %pUL\n", wblock->gblock.guid);
> > +	dev_dbg(&wblock->dev.dev, "event 0x%02X\n", event);
>
> The debug_event value gets set by a module-parameter and several WMI related
> howto-s and forum threads on the web refer to this. At one point in time even:
> https://wiki.ubuntu.com/Hotkeys/Troubleshooting
>
> Used to refer to this, but they seem to have dropped this.
>
> Either way this changes makes users have to also deal with dyndbg stuff to
> get the same info which before they could get with just the debug_event module
> param, which makes debugging harder, so I'm going to drop this patch from the
> series.

Would you consider accepting a patch that changes it to:

  if (debug_event)
    dev_info(&wblock->dev.dev, "event 0x%02X\n", event);

?


Regards,
Barnabás Pőcze

Re: [RFC PATCH v1 13/30] platform/x86: wmi: use dynamic debug to print data about events

[Hans de Goede]

Hi Barnabás,

On 9/13/21 12:09 PM, Barnabás Pőcze wrote:
> Hi
> 
> 
> 2021. szeptember 13., hétfő 11:43 keltezéssel, Hans de Goede írta:
>>> -	if (debug_event)
>>> -		pr_info("DEBUG Event GUID: %pUL\n", wblock->gblock.guid);
>>> +	dev_dbg(&wblock->dev.dev, "event 0x%02X\n", event);
>>
>> The debug_event value gets set by a module-parameter and several WMI related
>> howto-s and forum threads on the web refer to this. At one point in time even:
>> https://wiki.ubuntu.com/Hotkeys/Troubleshooting
>>
>> Used to refer to this, but they seem to have dropped this.
>>
>> Either way this changes makes users have to also deal with dyndbg stuff to
>> get the same info which before they could get with just the debug_event module
>> param, which makes debugging harder, so I'm going to drop this patch from the
>> series.
> 
> Would you consider accepting a patch that changes it to:
> 
>   if (debug_event)
>     dev_info(&wblock->dev.dev, "event 0x%02X\n", event);
> 
> ?

So I've just finished reviewing the series and I've pushed it out to
my review-hans branch minus this patch and I've also dropped patch 17 as
you requested.

I've added the "event 0x%02X\n", event bit to the existing pr_info, because I agree
that printing the event is useful.

I've squashed this change into the:

"[RFC PATCH v1 23/30] platform/x86: wmi: improve debug messages"

patch since that was making the same change for the wmi_notify_debug() code.

If you want to send out a follow-up patch/series on top of my current review-hans
switching to dev_info(), then that would be a welcome improvement, but in that
case please replace all pr_info() calls with dev_info(), not just this one.

Regards,

Hans