| Message ID | 20241204204227.95757-4-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | platform/x86: x86-android-tablets: Add Bluetooth support for Vexia EDU ATLA 10 | expand |
On Wed, Dec 04, 2024 at 09:42:14PM +0100, Hans de Goede wrote: > dell_uart_bl_pdev_probe() calls get_serdev_controller() with the > serial_ctrl_uid parameter set to NULL. > > In case of errors this NULL parameter then gets passed to pr_err() > as argument matching a "%s" conversion specification. This leads to > compiler warnings when building with "make W=1". > > Check serial_ctrl_uid before passing it to pr_err() to avoid these. Reviewed-by: Andy Shevchenko <andy@kernel.org> ... > + serial_ctrl_hid, serial_ctrl_uid ?: "*"); Not sure about '*' as it would mean 'any', perhaps 'none', '-', or 'undefined' would be better, but since they are error messages, it's not so critical.
On Thu, 5 Dec 2024, Andy Shevchenko wrote: > On Wed, Dec 04, 2024 at 09:42:14PM +0100, Hans de Goede wrote: > > dell_uart_bl_pdev_probe() calls get_serdev_controller() with the > > serial_ctrl_uid parameter set to NULL. > > > > In case of errors this NULL parameter then gets passed to pr_err() > > as argument matching a "%s" conversion specification. This leads to > > compiler warnings when building with "make W=1". > > > > Check serial_ctrl_uid before passing it to pr_err() to avoid these. > > Reviewed-by: Andy Shevchenko <andy@kernel.org> > > ... > > > + serial_ctrl_hid, serial_ctrl_uid ?: "*"); > > Not sure about '*' as it would mean 'any', perhaps 'none', '-', or 'undefined' > would be better, but since they are error messages, it's not so critical. Isn't not checking _UID (in acpi_dev_get_first_match_dev()) same as "any" _UID?
On Thu, Dec 5, 2024 at 12:32 PM Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote: > On Thu, 5 Dec 2024, Andy Shevchenko wrote: > > On Wed, Dec 04, 2024 at 09:42:14PM +0100, Hans de Goede wrote: ... > > > + serial_ctrl_hid, serial_ctrl_uid ?: "*"); > > > > Not sure about '*' as it would mean 'any', perhaps 'none', '-', or 'undefined' > > would be better, but since they are error messages, it's not so critical. > > Isn't not checking _UID (in acpi_dev_get_first_match_dev()) same as "any" > _UID? Ah, good point!
diff --git a/drivers/platform/x86/serdev_helpers.h b/drivers/platform/x86/serdev_helpers.h index bcf3a0c356ea..3bc7fd8e1e19 100644 --- a/drivers/platform/x86/serdev_helpers.h +++ b/drivers/platform/x86/serdev_helpers.h @@ -35,7 +35,7 @@ get_serdev_controller(const char *serial_ctrl_hid, ctrl_adev = acpi_dev_get_first_match_dev(serial_ctrl_hid, serial_ctrl_uid, -1); if (!ctrl_adev) { pr_err("error could not get %s/%s serial-ctrl adev\n", - serial_ctrl_hid, serial_ctrl_uid); + serial_ctrl_hid, serial_ctrl_uid ?: "*"); return ERR_PTR(-ENODEV); } @@ -43,7 +43,7 @@ get_serdev_controller(const char *serial_ctrl_hid, ctrl_dev = get_device(acpi_get_first_physical_node(ctrl_adev)); if (!ctrl_dev) { pr_err("error could not get %s/%s serial-ctrl physical node\n", - serial_ctrl_hid, serial_ctrl_uid); + serial_ctrl_hid, serial_ctrl_uid ?: "*"); ctrl_dev = ERR_PTR(-ENODEV); goto put_ctrl_adev; }
dell_uart_bl_pdev_probe() calls get_serdev_controller() with the serial_ctrl_uid parameter set to NULL. In case of errors this NULL parameter then gets passed to pr_err() as argument matching a "%s" conversion specification. This leads to compiler warnings when building with "make W=1". Check serial_ctrl_uid before passing it to pr_err() to avoid these. Fixes: dc5afd720f84 ("platform/x86: Add new get_serdev_controller() helper") Cc: stable@vger.kernel.org Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/platform/x86/serdev_helpers.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)