[PATCH v3 1/2] ACPI: use %pe for better readability of errors while printing

[PATCH v3 1/2] ACPI: use %pe for better readability of errors while printing

[Onkarnarth]

From: Onkarnath <onkarnath.1@samsung.com>

As %pe is already introduced, it's better to use it in place of (%ld) for
printing errors in logs. It would enhance readability of logs.

Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
Signed-off-by: Onkarnath <onkarnath.1@samsung.com>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
v1 -> v2: Updated subject line as per file history & corrected spellings
in description.
v2 -> v3: Updated Reviewed-by tag.

 drivers/acpi/acpi_processor.c | 2 +-
 drivers/acpi/acpi_watchdog.c  | 2 +-
 drivers/acpi/pci_slot.c       | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 4fe2ef54088c..2ddd36a21850 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -161,7 +161,7 @@ static void cpufreq_add_device(const char *name)
 
 	pdev = platform_device_register_simple(name, PLATFORM_DEVID_NONE, NULL, 0);
 	if (IS_ERR(pdev))
-		pr_info("%s device creation failed: %ld\n", name, PTR_ERR(pdev));
+		pr_info("%s device creation failed: %pe\n", name, pdev);
 }
 
 #ifdef CONFIG_X86
diff --git a/drivers/acpi/acpi_watchdog.c b/drivers/acpi/acpi_watchdog.c
index 8e9e001da38f..14b24157799c 100644
--- a/drivers/acpi/acpi_watchdog.c
+++ b/drivers/acpi/acpi_watchdog.c
@@ -179,7 +179,7 @@ void __init acpi_watchdog_init(void)
 	pdev = platform_device_register_simple("wdat_wdt", PLATFORM_DEVID_NONE,
 					       resources, nresources);
 	if (IS_ERR(pdev))
-		pr_err("Device creation failed: %ld\n", PTR_ERR(pdev));
+		pr_err("Device creation failed: %pe\n", pdev);
 
 	kfree(resources);
 
diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c
index d6cb2c27a23b..741bcc9d6d6a 100644
--- a/drivers/acpi/pci_slot.c
+++ b/drivers/acpi/pci_slot.c
@@ -111,7 +111,7 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
 	snprintf(name, sizeof(name), "%llu", sun);
 	pci_slot = pci_create_slot(pci_bus, device, name, NULL);
 	if (IS_ERR(pci_slot)) {
-		pr_err("pci_create_slot returned %ld\n", PTR_ERR(pci_slot));
+		pr_err("pci_create_slot returned %pe\n", pci_slot);
 		kfree(slot);
 		return AE_OK;
 	}
-- 
2.25.1

[PATCH v3 2/2] cpufreq/schedutil: print errors with %pe for better readability of logs

[Onkarnarth]

From: Onkarnath <onkarnath.1@samsung.com>

Instead of printing errros as a number(%ld), it's better to print in string
format for better readability of logs.

Signed-off-by: Onkarnath <onkarnath.1@samsung.com>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Reviewed-by: Valentin Schneider <vschneid@redhat.com>
---
v1 -> v2: Updated subject as per file history.
v2 -> v3: No change in this patch, change done in PATCH v3 1/2.

 kernel/sched/cpufreq_schedutil.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
index eece6244f9d2..2c42eaa56fa3 100644
--- a/kernel/sched/cpufreq_schedutil.c
+++ b/kernel/sched/cpufreq_schedutil.c
@@ -671,7 +671,7 @@ static int sugov_kthread_create(struct sugov_policy *sg_policy)
 				"sugov:%d",
 				cpumask_first(policy->related_cpus));
 	if (IS_ERR(thread)) {
-		pr_err("failed to create sugov thread: %ld\n", PTR_ERR(thread));
+		pr_err("failed to create sugov thread: %pe\n", thread);
 		return PTR_ERR(thread);
 	}
 
-- 
2.25.1

Re: [PATCH v3 1/2] ACPI: use %pe for better readability of errors while printing

[Bjorn Helgaas]

On Tue, Feb 13, 2024 at 01:14:15PM +0530, Onkarnarth wrote:
> From: Onkarnath <onkarnath.1@samsung.com>
> 
> As %pe is already introduced, it's better to use it in place of (%ld) for
> printing errors in logs. It would enhance readability of logs.

Here are some more candidates that I see regularly:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/dd.c?id=v6.7#n590

Something like:

  git grep "\(_info(\|_warn(\).*%d"

finds a ton of them (plus a lot of unrelated hits, of course).  If you
were to do this for drivers/pci/, I would want them all for the whole
directory in a single patch, and I would take the opportunity to make
minor changes so the style is more consistent, e.g.,
"... failed (%pe)" or something.

Bjorn

Re: [PATCH v3 1/2] ACPI: use %pe for better readability of errors while printing

[Rafael J. Wysocki]

On Tue, Feb 13, 2024 at 9:20 AM Onkarnarth <onkarnath.1@samsung.com> wrote:
>
> From: Onkarnath <onkarnath.1@samsung.com>
>
> As %pe is already introduced, it's better to use it in place of (%ld) for
> printing errors in logs. It would enhance readability of logs.
>
> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>

What exactly is the role of this S-o-b?  Has the person helped you to
develop the patch or something else?

> Signed-off-by: Onkarnath <onkarnath.1@samsung.com>
> Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> ---
> v1 -> v2: Updated subject line as per file history & corrected spellings
> in description.
> v2 -> v3: Updated Reviewed-by tag.
>
>  drivers/acpi/acpi_processor.c | 2 +-
>  drivers/acpi/acpi_watchdog.c  | 2 +-
>  drivers/acpi/pci_slot.c       | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
> index 4fe2ef54088c..2ddd36a21850 100644
> --- a/drivers/acpi/acpi_processor.c
> +++ b/drivers/acpi/acpi_processor.c
> @@ -161,7 +161,7 @@ static void cpufreq_add_device(const char *name)
>
>         pdev = platform_device_register_simple(name, PLATFORM_DEVID_NONE, NULL, 0);
>         if (IS_ERR(pdev))
> -               pr_info("%s device creation failed: %ld\n", name, PTR_ERR(pdev));
> +               pr_info("%s device creation failed: %pe\n", name, pdev);
>  }
>
>  #ifdef CONFIG_X86
> diff --git a/drivers/acpi/acpi_watchdog.c b/drivers/acpi/acpi_watchdog.c
> index 8e9e001da38f..14b24157799c 100644
> --- a/drivers/acpi/acpi_watchdog.c
> +++ b/drivers/acpi/acpi_watchdog.c
> @@ -179,7 +179,7 @@ void __init acpi_watchdog_init(void)
>         pdev = platform_device_register_simple("wdat_wdt", PLATFORM_DEVID_NONE,
>                                                resources, nresources);
>         if (IS_ERR(pdev))
> -               pr_err("Device creation failed: %ld\n", PTR_ERR(pdev));
> +               pr_err("Device creation failed: %pe\n", pdev);
>
>         kfree(resources);
>
> diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c
> index d6cb2c27a23b..741bcc9d6d6a 100644
> --- a/drivers/acpi/pci_slot.c
> +++ b/drivers/acpi/pci_slot.c
> @@ -111,7 +111,7 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
>         snprintf(name, sizeof(name), "%llu", sun);
>         pci_slot = pci_create_slot(pci_bus, device, name, NULL);
>         if (IS_ERR(pci_slot)) {
> -               pr_err("pci_create_slot returned %ld\n", PTR_ERR(pci_slot));
> +               pr_err("pci_create_slot returned %pe\n", pci_slot);
>                 kfree(slot);
>                 return AE_OK;
>         }
> --

Re: [PATCH v3 1/2] ACPI: use %pe for better readability of errors while printing

[Onkarnath]

>On Tue, Feb 13, 2024 at 9:20 AM Onkarnarth <onkarnath.1@samsung.com> wrote:
>>
>> From: Onkarnath <onkarnath.1@samsung.com>
>>
>> As %pe is already introduced, it's better to use it in place of (%ld) for
>> printing errors in logs. It would enhance readability of logs.
>>
>> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
>
>What exactly is the role of this S-o-b?  Has the person helped you to
>develop the patch or something else?
>

Yes It was meant for Co-developed tag, Because We are working collectively for making errors more readable for our product kernel.(5.4)
And some part of this patch was made by him.

Then we checked that it is also suggested by open source to have %pe for printing errors:
https://lore.kernel.org/all/92972476-0b1f-4d0a-9951-af3fc8bc6e65@suswa.mountain/

So I prepared same changes for open source kernel, and because of smaller patch I kept it as normal signed-off tag only.
If it is needed I can resend with Co-developed tag.

Thanks,
Onkarnath

Re: [PATCH v3 1/2] ACPI: use %pe for better readability of errors while printing

[Rafael J. Wysocki]

On Fri, Feb 16, 2024 at 6:54 AM Onkarnath <onkarnath.1@samsung.com> wrote:
>
> >On Tue, Feb 13, 2024 at 9:20 AM Onkarnarth <onkarnath.1@samsung.com> wrote:
> >>
> >> From: Onkarnath <onkarnath.1@samsung.com>
> >>
> >> As %pe is already introduced, it's better to use it in place of (%ld) for
> >> printing errors in logs. It would enhance readability of logs.
> >>
> >> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
> >
> >What exactly is the role of this S-o-b?  Has the person helped you to
> >develop the patch or something else?
> >
>
> Yes It was meant for Co-developed tag, Because We are working collectively for making errors more readable for our product kernel.(5.4)
> And some part of this patch was made by him.
>
> Then we checked that it is also suggested by open source to have %pe for printing errors:
> https://lore.kernel.org/all/92972476-0b1f-4d0a-9951-af3fc8bc6e65@suswa.mountain/
>
> So I prepared same changes for open source kernel, and because of smaller patch I kept it as normal signed-off tag only.
> If it is needed I can resend with Co-developed tag.

No need, I can add it for you.  Thanks for the explanation!

Re: [PATCH v3 1/2] ACPI: use %pe for better readability of errors while printing

[Rafael J. Wysocki]

On Tue, Feb 13, 2024 at 9:20 AM Onkarnarth <onkarnath.1@samsung.com> wrote:
>
> From: Onkarnath <onkarnath.1@samsung.com>
>
> As %pe is already introduced, it's better to use it in place of (%ld) for
> printing errors in logs. It would enhance readability of logs.
>
> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
> Signed-off-by: Onkarnath <onkarnath.1@samsung.com>
> Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> ---
> v1 -> v2: Updated subject line as per file history & corrected spellings
> in description.
> v2 -> v3: Updated Reviewed-by tag.
>
>  drivers/acpi/acpi_processor.c | 2 +-
>  drivers/acpi/acpi_watchdog.c  | 2 +-
>  drivers/acpi/pci_slot.c       | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
> index 4fe2ef54088c..2ddd36a21850 100644
> --- a/drivers/acpi/acpi_processor.c
> +++ b/drivers/acpi/acpi_processor.c
> @@ -161,7 +161,7 @@ static void cpufreq_add_device(const char *name)
>
>         pdev = platform_device_register_simple(name, PLATFORM_DEVID_NONE, NULL, 0);
>         if (IS_ERR(pdev))
> -               pr_info("%s device creation failed: %ld\n", name, PTR_ERR(pdev));
> +               pr_info("%s device creation failed: %pe\n", name, pdev);
>  }
>
>  #ifdef CONFIG_X86
> diff --git a/drivers/acpi/acpi_watchdog.c b/drivers/acpi/acpi_watchdog.c
> index 8e9e001da38f..14b24157799c 100644
> --- a/drivers/acpi/acpi_watchdog.c
> +++ b/drivers/acpi/acpi_watchdog.c
> @@ -179,7 +179,7 @@ void __init acpi_watchdog_init(void)
>         pdev = platform_device_register_simple("wdat_wdt", PLATFORM_DEVID_NONE,
>                                                resources, nresources);
>         if (IS_ERR(pdev))
> -               pr_err("Device creation failed: %ld\n", PTR_ERR(pdev));
> +               pr_err("Device creation failed: %pe\n", pdev);
>
>         kfree(resources);
>
> diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c
> index d6cb2c27a23b..741bcc9d6d6a 100644
> --- a/drivers/acpi/pci_slot.c
> +++ b/drivers/acpi/pci_slot.c
> @@ -111,7 +111,7 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
>         snprintf(name, sizeof(name), "%llu", sun);
>         pci_slot = pci_create_slot(pci_bus, device, name, NULL);
>         if (IS_ERR(pci_slot)) {
> -               pr_err("pci_create_slot returned %ld\n", PTR_ERR(pci_slot));
> +               pr_err("pci_create_slot returned %pe\n", pci_slot);
>                 kfree(slot);
>                 return AE_OK;
>         }
> --

Applied as 6.9 material, thanks!