| Message ID | 20210113182016.166049-8-pobrn@protonmail.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | platform/x86: ideapad-laptop: cleanup, keyboard backlight and "always on USB charging" control support, reenable touchpad control | expand |
On Wed, Jan 13, 2021 at 8:22 PM Barnabás Pőcze <pobrn@protonmail.com> wrote: > > Having the device name in the log message makes it easier to determine in > the context of which device the message was printed, so utilize the > appropriate variants of dev_{err,warn,...} when printing log messages. This doesn't explain transitions like pr_err() -> dev_warn() or pr_info() -> dev_warn(). Care to elaborate in the commit message? > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > Reviewed-by: Hans de Goede <hdegoede@redhat.com> > > diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c > index 174edbfc52dc..b0d8e332b48a 100644 > --- a/drivers/platform/x86/ideapad-laptop.c > +++ b/drivers/platform/x86/ideapad-laptop.c > @@ -203,7 +203,7 @@ static int read_ec_data(acpi_handle handle, int cmd, unsigned long *data) > return 0; > } > } > - pr_err("timeout in %s\n", __func__); > + acpi_handle_err(handle, "timeout in %s\n", __func__); > return -1; > } > > @@ -225,7 +225,7 @@ static int write_ec_cmd(acpi_handle handle, int cmd, unsigned long data) > if (val == 0) > return 0; > } > - pr_err("timeout in %s\n", __func__); > + acpi_handle_err(handle, "timeout in %s\n", __func__); > return -1; > } > > @@ -696,13 +696,15 @@ static int ideapad_input_init(struct ideapad_private *priv) > > error = sparse_keymap_setup(inputdev, ideapad_keymap, NULL); > if (error) { > - pr_err("Unable to setup input device keymap\n"); > + dev_err(&priv->platform_device->dev, > + "Unable to setup input device keymap\n"); > goto err_free_dev; > } > > error = input_register_device(inputdev); > if (error) { > - pr_err("Unable to register input device\n"); > + dev_err(&priv->platform_device->dev, > + "Unable to register input device\n"); > goto err_free_dev; > } > > @@ -756,7 +758,8 @@ static void ideapad_check_special_buttons(struct ideapad_private *priv) > ideapad_input_report(priv, 64); > break; > default: > - pr_info("Unknown special button: %lu\n", bit); > + dev_warn(&priv->platform_device->dev, > + "Unknown special button: %lu\n", bit); > break; > } > } > @@ -822,7 +825,8 @@ static int ideapad_backlight_init(struct ideapad_private *priv) > &ideapad_backlight_ops, > &props); > if (IS_ERR(blightdev)) { > - pr_err("Could not register backlight device\n"); > + dev_warn(&priv->platform_device->dev, > + "Could not register backlight device\n"); > return PTR_ERR(blightdev); > } > > @@ -934,7 +938,8 @@ static void ideapad_acpi_notify(acpi_handle handle, u32 event, void *data) > */ > break; > default: > - pr_info("Unknown event: %lu\n", bit); > + dev_warn(&priv->platform_device->dev, > + "Unknown event: %lu\n", bit); > } > } > } > @@ -942,12 +947,15 @@ static void ideapad_acpi_notify(acpi_handle handle, u32 event, void *data) > #if IS_ENABLED(CONFIG_ACPI_WMI) > static void ideapad_wmi_notify(u32 value, void *context) > { > + struct ideapad_private *priv = context; > + > switch (value) { > case 128: > - ideapad_input_report(context, value); > + ideapad_input_report(priv, value); > break; > default: > - pr_info("Unknown WMI event %u\n", value); > + dev_warn(&priv->platform_device->dev, > + "Unknown WMI event: %u\n", value); > } > } > #endif > -- > 2.30.0 >
Hi 2021. január 16., szombat 20:46 keltezéssel, Andy Shevchenko írta: > On Wed, Jan 13, 2021 at 8:22 PM Barnabás Pőcze wrote: > > > > Having the device name in the log message makes it easier to determine in > > the context of which device the message was printed, so utilize the > > appropriate variants of dev_{err,warn,...} when printing log messages. > > This doesn't explain transitions like pr_err() -> dev_warn() or > pr_info() -> dev_warn(). > Care to elaborate in the commit message? > [...] Thanks for the review, and thanks for pointing this out. I don't recall intendeding to promote/demote any of the log messages when I was making these changes. I will revisit my them and modify the commit and commit message accordingly. Thanks, Barnabás Pőcze
On 1/17/21 4:34 AM, Barnabás Pőcze wrote: > Hi > > > 2021. január 16., szombat 20:46 keltezéssel, Andy Shevchenko írta: > >> On Wed, Jan 13, 2021 at 8:22 PM Barnabás Pőcze wrote: >>> >>> Having the device name in the log message makes it easier to determine in >>> the context of which device the message was printed, so utilize the >>> appropriate variants of dev_{err,warn,...} when printing log messages. >> >> This doesn't explain transitions like pr_err() -> dev_warn() or >> pr_info() -> dev_warn(). >> Care to elaborate in the commit message? >> [...] > > Thanks for the review, and thanks for pointing this out. I don't recall intendeding > to promote/demote any of the log messages when I was making these changes. I will > revisit my them and modify the commit and commit message accordingly. > > > Thanks, > Barnabás Pőcze > Hi, For unknown key/wmi event, it might come from latest model on new function. That's why it is pr_info. For backlight_init, this should not happen. that's why it is pr_err. I am ok if you have any though of promoting or demoting. -- Ike Panhc
diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c index 174edbfc52dc..b0d8e332b48a 100644 --- a/drivers/platform/x86/ideapad-laptop.c +++ b/drivers/platform/x86/ideapad-laptop.c @@ -203,7 +203,7 @@ static int read_ec_data(acpi_handle handle, int cmd, unsigned long *data) return 0; } } - pr_err("timeout in %s\n", __func__); + acpi_handle_err(handle, "timeout in %s\n", __func__); return -1; } @@ -225,7 +225,7 @@ static int write_ec_cmd(acpi_handle handle, int cmd, unsigned long data) if (val == 0) return 0; } - pr_err("timeout in %s\n", __func__); + acpi_handle_err(handle, "timeout in %s\n", __func__); return -1; } @@ -696,13 +696,15 @@ static int ideapad_input_init(struct ideapad_private *priv) error = sparse_keymap_setup(inputdev, ideapad_keymap, NULL); if (error) { - pr_err("Unable to setup input device keymap\n"); + dev_err(&priv->platform_device->dev, + "Unable to setup input device keymap\n"); goto err_free_dev; } error = input_register_device(inputdev); if (error) { - pr_err("Unable to register input device\n"); + dev_err(&priv->platform_device->dev, + "Unable to register input device\n"); goto err_free_dev; } @@ -756,7 +758,8 @@ static void ideapad_check_special_buttons(struct ideapad_private *priv) ideapad_input_report(priv, 64); break; default: - pr_info("Unknown special button: %lu\n", bit); + dev_warn(&priv->platform_device->dev, + "Unknown special button: %lu\n", bit); break; } } @@ -822,7 +825,8 @@ static int ideapad_backlight_init(struct ideapad_private *priv) &ideapad_backlight_ops, &props); if (IS_ERR(blightdev)) { - pr_err("Could not register backlight device\n"); + dev_warn(&priv->platform_device->dev, + "Could not register backlight device\n"); return PTR_ERR(blightdev); } @@ -934,7 +938,8 @@ static void ideapad_acpi_notify(acpi_handle handle, u32 event, void *data) */ break; default: - pr_info("Unknown event: %lu\n", bit); + dev_warn(&priv->platform_device->dev, + "Unknown event: %lu\n", bit); } } } @@ -942,12 +947,15 @@ static void ideapad_acpi_notify(acpi_handle handle, u32 event, void *data) #if IS_ENABLED(CONFIG_ACPI_WMI) static void ideapad_wmi_notify(u32 value, void *context) { + struct ideapad_private *priv = context; + switch (value) { case 128: - ideapad_input_report(context, value); + ideapad_input_report(priv, value); break; default: - pr_info("Unknown WMI event %u\n", value); + dev_warn(&priv->platform_device->dev, + "Unknown WMI event: %u\n", value); } } #endif
Having the device name in the log message makes it easier to determine in the context of which device the message was printed, so utilize the appropriate variants of dev_{err,warn,...} when printing log messages. Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> Reviewed-by: Hans de Goede <hdegoede@redhat.com>