Em Thu, 28 Oct 2021 18:34:47 +0900
Tsuchiya Yuto [off-list ref] escreveu:

<Adding back people/list to Cc>

On Tue, 2021-10-26 at 09:32 +0100, Mauro Carvalho Chehab wrote:

quoted

Em Mon, 18 Oct 2021 01:19:54 +0900
Tsuchiya Yuto [off-list ref] escreveu:
  

quoted

(patch based on intel-aero kernel patch:
 https://github.com/intel-aero/meta-intel-aero-base/commit/26fc9fe5030b63bc9dcf0b5f32981948911ca272)

Here is the original commit message from the aforementioned patch:

	From 26fc9fe5030b63bc9dcf0b5f32981948911ca272 Mon Sep 17 00:00:00 2001
	From: Lucas De Marchi [off-list ref]
	Date: Fri, 7 Jul 2017 14:23:53 -0700
	Subject: [PATCH] linux-yocto: Remove remaining instance of call to
	 trace_printk

	It's not sufficient to leave trace_printk() out of "normal call chains" since
	the way trace infrastructure works is that it will allocate the trace_printk
	buffers if the symbol is there (by using a separate section for the function
	and checking if __start_* and __stop_* symbols are different.

	Therefore, even if the default value for the param tells the module to use
	printk(), just the fact that it can be changed to trace_printk() means the
	initialization code will be called.

The trace_printk() was replaced by pr_info() on commit 3d81099c75a6
("media: atomisp: Replace trace_printk by pr_info") for the upstreamed
atomisp, too. However, as the aforementioned commit message says, there
is still a remaining instance. This causes the "trace_printk() being
used" kernel warning message to still appear on the first driver load.

Based on the aforementioned patch, this patch removes the call to
ftrace_vprintk(). This removes that kernel warning.

In addition to this, this patch also removes the following now unused
things:

    - now empty atomisp_css2_dbg_ftrace_print()
    - trace_printk option from dbg_func kernel parameter  

This patch is incomplete. I mean, if the idea is to drop support for
trace, dbg_func parameter becomes obsolete, and a lot of code can be
dropped/cleaned.  

I left __set_css_print_env() and dbg_func module parameter intentionally
because there is still the option "non", which means do not use any
function to log. Yes, I don't know if it's really useful since the module
parameter dbg_level=0 (this is the default value) means no debug output.

However, they (those who originally wrote atomisp) added the option "non"
anyway. So, I thought there is a use case for this.

If everyone thinks it's useless, I can additionally remove them. In this
case, indeed a lot of code may be additionally removed (I haven't checked
yet).

quoted

That's said, I don't see a good reason to drop it, at least while
this driver is not 100%.  

This driver is not 100% yet, but at least it can capture images now
(at least on ISP2401). So, I thought this is the right time to remove
it when I wrote this patch.

But yes, we don't necessarily have to drop it. The other idea is hide
it by default using a macro, and still can be enabled by manual edit.
Here is the example from drivers/usb/dwc2/core.h:

        49-/*
        50- * Suggested defines for tracers:
        51- * - no_printk:    Disable tracing
        52- * - pr_info:      Print this info to the console
        53- * - trace_printk: Print this info to trace buffer (good for verbose logging)
        54- */
        55-
        56:#define DWC2_TRACE_SCHEDULER		no_printk
        57:#define DWC2_TRACE_SCHEDULER_VB		no_printk
        58-
        59-/* Detailed scheduler tracing, but won't overwhelm console */
        60-#define dwc2_sch_dbg(hsotg, fmt, ...)					\
        61:	DWC2_TRACE_SCHEDULER(pr_fmt("%s: SCH: " fmt),			\
        62-			     dev_name(hsotg->dev), ##__VA_ARGS__)
        63-
        64-/* Verbose scheduler tracing */
        65-#define dwc2_sch_vdbg(hsotg, fmt, ...)					\
        66:	DWC2_TRACE_SCHEDULER_VB(pr_fmt("%s: SCH: " fmt),		\
        67-				dev_name(hsotg->dev), ##__VA_ARGS__)

with using pr_info by default for atomisp (the parameter dbg_level is
0 by default, so still no output by default). If this sounds OK, I'd
like to try this way.

Let's just postpone this change. It sounds a little early to start
cleaning up debug stuff. This can be done any time later, but let's
first test it on our devices and ensure that it is stable enough.

Regards,
Mauro

quoted

quoted

Signed-off-by: Tsuchiya Yuto <redacted>
---
 .../staging/media/atomisp/pci/atomisp_compat_css20.c   | 10 ----------
 drivers/staging/media/atomisp/pci/atomisp_v4l2.c       |  4 ++--
 2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
index 99a632f33d2d..d81d55c6f1fa 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c

@@ -159,13 +159,6 @@ static void atomisp_css2_hw_load(hrt_address addr, void *to, uint32_t n)
 	spin_unlock_irqrestore(&mmio_lock, flags);
 }
 
-static int  __printf(1, 0) atomisp_css2_dbg_ftrace_print(const char *fmt,
-							 va_list args)
-{
-	ftrace_vprintk(fmt, args);
-	return 0;
-}
-
 static int  __printf(1, 0) atomisp_vprintk(const char *fmt, va_list args)
 {
 	vprintk(fmt, args);

@@ -860,9 +853,6 @@ static inline int __set_css_print_env(struct atomisp_device *isp, int opt)
 	if (opt == 0)
 		isp->css_env.isp_css_env.print_env.debug_print = NULL;
 	else if (opt == 1)
-		isp->css_env.isp_css_env.print_env.debug_print =
-		    atomisp_css2_dbg_ftrace_print;
-	else if (opt == 2)
 		isp->css_env.isp_css_env.print_env.debug_print = atomisp_vprintk;
 	else
 		ret = -EINVAL;

diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index f5362554638e..720963156d24 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c

@@ -87,10 +87,10 @@ module_param(dbg_level, int, 0644);
 MODULE_PARM_DESC(dbg_level, "debug message level (default:0)");
 
 /* log function switch */
-int dbg_func = 2;
+int dbg_func = 1;
 module_param(dbg_func, int, 0644);
 MODULE_PARM_DESC(dbg_func,
-		 "log function switch non/trace_printk/printk (default:printk)");
+		 "log function switch non/printk (default:printk)");
 
 int mipicsi_flag;
 module_param(mipicsi_flag, int, 0644);