| Message ID | 20231111083327.18607-2-ltuikov89@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Revert "drm/sched: Define pr_fmt() for DRM using pr_*()" | expand |
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>
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
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.
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>
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(-) base-commit: 540527b1385fb203cc4513ca838b4de60bbbc49a