[PATCH] Revert "drm/sched: Define pr_fmt() for DRM using pr_*()"

[PATCH] Revert "drm/sched: Define pr_fmt() for DRM using pr_*()"

[Luben Tuikov]

This reverts commit 36245bd02e88e68ac5955c2958c968879d7b75a9.

Signed-off-by: Luben Tuikov <ltuikov89@gmail.com>
---
 include/drm/drm_print.h | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index e8fe60d0eb8783..a93a387f8a1a15 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -26,20 +26,6 @@
 #ifndef DRM_PRINT_H_
 #define DRM_PRINT_H_
 
-/* Define this before including linux/printk.h, so that the format
- * string in pr_*() macros is correctly set for DRM. If a file wants
- * to define this to something else, it should do so before including
- * this header file.
- *
- * It is encouraged code using pr_err() to prefix their format with
- * the string "*ERROR* ", to make it easier to scan kernel logs. For
- * instance,
- *   pr_err("*ERROR* <the rest of your format string here>", args).
- */
-#ifndef pr_fmt
-#define pr_fmt(fmt) "[drm] " fmt
-#endif
-
 #include <linux/compiler.h>
 #include <linux/printk.h>
 #include <linux/seq_file.h>

base-commit: 540527b1385fb203cc4513ca838b4de60bbbc49a
-- 
2.42.1

Re: [PATCH] Revert "drm/sched: Define pr_fmt() for DRM using pr_*()"

[Javier Martinez Canillas]

Luben Tuikov <ltuikov89@gmail.com> writes:

Hello Luben,

> This reverts commit 36245bd02e88e68ac5955c2958c968879d7b75a9.
>

You should include a commit message explaining why this commit should
be reverted. I noticed that was asked by Jani so I would include what
he mentioned in the other email thread.

> Signed-off-by: Luben Tuikov <ltuikov89@gmail.com>
> ---

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

[PATCH] Revert "drm/sched: Define pr_fmt() for DRM using pr_*()"

[Luben Tuikov]

From Jani:
The drm_print.[ch] facilities use very few pr_*() calls directly. The
users of pr_*() calls do not necessarily include <drm/drm_print.h> at
all, and really don't have to.

Even the ones that do include it, usually have <linux/...> includes
first, and <drm/...> includes next. Notably, <linux/kernel.h> includes
<linux/printk.h>.

And, of course, <linux/printk.h> defines pr_fmt() itself if not already
defined.

No, it's encouraged not to use pr_*() at all, and prefer drm device
based logging, or device based logging.

This reverts commit 36245bd02e88e68ac5955c2958c968879d7b75a9.

Signed-off-by: Luben Tuikov <ltuikov89@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/878r75wzm9.fsf@intel.com
---
 include/drm/drm_print.h | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index e8fe60d0eb8783..a93a387f8a1a15 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -26,20 +26,6 @@
 #ifndef DRM_PRINT_H_
 #define DRM_PRINT_H_
 
-/* Define this before including linux/printk.h, so that the format
- * string in pr_*() macros is correctly set for DRM. If a file wants
- * to define this to something else, it should do so before including
- * this header file.
- *
- * It is encouraged code using pr_err() to prefix their format with
- * the string "*ERROR* ", to make it easier to scan kernel logs. For
- * instance,
- *   pr_err("*ERROR* <the rest of your format string here>", args).
- */
-#ifndef pr_fmt
-#define pr_fmt(fmt) "[drm] " fmt
-#endif
-
 #include <linux/compiler.h>
 #include <linux/printk.h>
 #include <linux/seq_file.h>

base-commit: 540527b1385fb203cc4513ca838b4de60bbbc49a
-- 
2.42.1

Re: [PATCH] Revert "drm/sched: Define pr_fmt() for DRM using pr_*()"

[Javier Martinez Canillas]

Luben Tuikov <ltuikov89@gmail.com> writes:

Hello Luben,

> From Jani:
> The drm_print.[ch] facilities use very few pr_*() calls directly. The
> users of pr_*() calls do not necessarily include <drm/drm_print.h> at
> all, and really don't have to.
>
> Even the ones that do include it, usually have <linux/...> includes
> first, and <drm/...> includes next. Notably, <linux/kernel.h> includes
> <linux/printk.h>.
>
> And, of course, <linux/printk.h> defines pr_fmt() itself if not already
> defined.
>
> No, it's encouraged not to use pr_*() at all, and prefer drm device
> based logging, or device based logging.
>

Thanks for including the rationale from Jani for the revert.

> This reverts commit 36245bd02e88e68ac5955c2958c968879d7b75a9.
>
> Signed-off-by: Luben Tuikov <ltuikov89@gmail.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/878r75wzm9.fsf@intel.com
> ---

Acked-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH] Revert "drm/sched: Define pr_fmt() for DRM using pr_*()"

[Jani Nikula]

On Sat, 11 Nov 2023, Luben Tuikov <ltuikov89@gmail.com> wrote:
> From Jani:
> The drm_print.[ch] facilities use very few pr_*() calls directly. The
> users of pr_*() calls do not necessarily include <drm/drm_print.h> at
> all, and really don't have to.
>
> Even the ones that do include it, usually have <linux/...> includes
> first, and <drm/...> includes next. Notably, <linux/kernel.h> includes
> <linux/printk.h>.
>
> And, of course, <linux/printk.h> defines pr_fmt() itself if not already
> defined.
>
> No, it's encouraged not to use pr_*() at all, and prefer drm device
> based logging, or device based logging.
>
> This reverts commit 36245bd02e88e68ac5955c2958c968879d7b75a9.
>
> Signed-off-by: Luben Tuikov <ltuikov89@gmail.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/878r75wzm9.fsf@intel.com

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  include/drm/drm_print.h | 14 --------------
>  1 file changed, 14 deletions(-)
>
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index e8fe60d0eb8783..a93a387f8a1a15 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -26,20 +26,6 @@
>  #ifndef DRM_PRINT_H_
>  #define DRM_PRINT_H_
>  
> -/* Define this before including linux/printk.h, so that the format
> - * string in pr_*() macros is correctly set for DRM. If a file wants
> - * to define this to something else, it should do so before including
> - * this header file.
> - *
> - * It is encouraged code using pr_err() to prefix their format with
> - * the string "*ERROR* ", to make it easier to scan kernel logs. For
> - * instance,
> - *   pr_err("*ERROR* <the rest of your format string here>", args).
> - */
> -#ifndef pr_fmt
> -#define pr_fmt(fmt) "[drm] " fmt
> -#endif
> -
>  #include <linux/compiler.h>
>  #include <linux/printk.h>
>  #include <linux/seq_file.h>
>
> base-commit: 540527b1385fb203cc4513ca838b4de60bbbc49a

-- 
Jani Nikula, Intel

Re: [PATCH] Revert "drm/sched: Define pr_fmt() for DRM using pr_*()"

[Luben Tuikov]

[-- Attachment #1.1.1: Type: text/plain, Size: 2100 bytes --]

On 2023-11-11 06:33, Jani Nikula wrote:
> On Sat, 11 Nov 2023, Luben Tuikov <ltuikov89@gmail.com> wrote:
>> From Jani:
>> The drm_print.[ch] facilities use very few pr_*() calls directly. The
>> users of pr_*() calls do not necessarily include <drm/drm_print.h> at
>> all, and really don't have to.
>>
>> Even the ones that do include it, usually have <linux/...> includes
>> first, and <drm/...> includes next. Notably, <linux/kernel.h> includes
>> <linux/printk.h>.
>>
>> And, of course, <linux/printk.h> defines pr_fmt() itself if not already
>> defined.
>>
>> No, it's encouraged not to use pr_*() at all, and prefer drm device
>> based logging, or device based logging.
>>
>> This reverts commit 36245bd02e88e68ac5955c2958c968879d7b75a9.
>>
>> Signed-off-by: Luben Tuikov <ltuikov89@gmail.com>
>> Link: https://patchwork.freedesktop.org/patch/msgid/878r75wzm9.fsf@intel.com
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> 
>> ---
>>  include/drm/drm_print.h | 14 --------------
>>  1 file changed, 14 deletions(-)
>>
>> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
>> index e8fe60d0eb8783..a93a387f8a1a15 100644
>> --- a/include/drm/drm_print.h
>> +++ b/include/drm/drm_print.h
>> @@ -26,20 +26,6 @@
>>  #ifndef DRM_PRINT_H_
>>  #define DRM_PRINT_H_
>>  
>> -/* Define this before including linux/printk.h, so that the format
>> - * string in pr_*() macros is correctly set for DRM. If a file wants
>> - * to define this to something else, it should do so before including
>> - * this header file.
>> - *
>> - * It is encouraged code using pr_err() to prefix their format with
>> - * the string "*ERROR* ", to make it easier to scan kernel logs. For
>> - * instance,
>> - *   pr_err("*ERROR* <the rest of your format string here>", args).
>> - */
>> -#ifndef pr_fmt
>> -#define pr_fmt(fmt) "[drm] " fmt
>> -#endif
>> -
>>  #include <linux/compiler.h>
>>  #include <linux/printk.h>
>>  #include <linux/seq_file.h>
>>
>> base-commit: 540527b1385fb203cc4513ca838b4de60bbbc49a
> 

Pushed.
-- 
Regards,
Luben

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 677 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]