| Message ID | 20210904175450.156801-14-pobrn@protonmail.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | platform/x86: wmi: minor improvements | expand |
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 > >
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
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
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,
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(-) -- 2.33.0