mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v5 0/2] This series cleans up unused module parameters from the atomisp driver.
       [not found] <YOUR_ANDY_EMAIL_MESSAGE_ID_HERE>
@ 2026-07-22  5:46 ` Prajval Chaudhary
  2026-07-22  5:46   ` [PATCH v5 1/2] staging: media: atomisp: Remove pad_w and pad_h module parameters Prajval Chaudhary
                     ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Prajval Chaudhary @ 2026-07-22  5:46 UTC (permalink / raw)
  To: andriy.shevchenko
  Cc: hverkuil-cisco, mchehab, sakari.ailios, gregkh, linux-media,
	linux-kernel, linux-staging, Prajval Chaudhary

Patch 1 converts 'pad_w' and 'pad_h' module parameters into internal
driver defines (ATOMISP_PAD_W and ATOMISP_PAD_H).
Patch 2 removes 'dbg_level' and 'dbg_func' module parameters and updates
ia_css_debug_vdtrace() to filter trace logging to IA_CSS_DEBUG_WARNING
level or below to avoid potential dmesg log flooding.

Output Behavior Difference:
- Before: Trace logging depended on setting dbg_level via module
  parameters. Higher values flooded dmesg with verbose low-level trace
  messages.
- After: Trace logging is filtered at IA_CSS_DEBUG_WARNING via standard
  vprintk facilities, avoiding dmesg flooding while maintaining
  critical warnings.

Changes in v5:
 - Converted pad_w/pad_h to ATOMISP_PAD_* macro defines instead of global
   variables.
 - Preserved blank line in ia_css_debug.h and sorted <linux/printk.h>
   header inclusion.
 - Added explanation of default levels and parameter options in commit
   messages and cover letter.

Changes in v4:
 - Fixed macro constant name in ia_css_debug_vdtrace() to
   IA_CSS_DEBUG_WARNING.
 - Guarded vprintk() with level check to prevent log flooding.

Prajval Chaudhary (2):
  staging: media: atomisp: Remove pad_w and pad_h module parameters
  staging: media: atomisp: Remove dbg_level and dbg_func module
    parameters

 drivers/staging/media/atomisp/pci/atomisp_cmd.c    |  8 ++++----
 drivers/staging/media/atomisp/pci/atomisp_common.h |  6 ++----
 .../media/atomisp/pci/atomisp_compat_css20.c       | 12 +++---------
 drivers/staging/media/atomisp/pci/atomisp_ioctl.c  |  4 ++--
 drivers/staging/media/atomisp/pci/atomisp_v4l2.c   | 14 --------------
 .../pci/runtime/debug/interface/ia_css_debug.h     | 10 +++++-----
 .../atomisp/pci/runtime/debug/src/ia_css_debug.c   |  4 ++--
 7 files changed, 18 insertions(+), 40 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v5 1/2] staging: media: atomisp: Remove pad_w and pad_h module parameters
  2026-07-22  5:46 ` [PATCH v5 0/2] This series cleans up unused module parameters from the atomisp driver Prajval Chaudhary
@ 2026-07-22  5:46   ` Prajval Chaudhary
  2026-07-22  5:46   ` [PATCH v5 2/2] staging: media: atomisp: Remove dbg_level and dbg_func " Prajval Chaudhary
  2026-08-11 14:02   ` [PATCH v5 0/2] This series cleans up unused module parameters from the atomisp driver Andy Shevchenko
  2 siblings, 0 replies; 5+ messages in thread
From: Prajval Chaudhary @ 2026-07-22  5:46 UTC (permalink / raw)
  To: andriy.shevchenko
  Cc: hverkuil-cisco, mchehab, sakari.ailios, gregkh, linux-media,
	linux-kernel, linux-staging, Prajval Chaudhary

Remove the 'pad_w' and 'pad_h' module parameters to prevent userspace
from overriding internal ISP hardware padding boundaries. Convert them
into internal driver constants instead.

Signed-off-by: Prajval Chaudhary <prajvalchaudhari733@gmail.com>
---
 drivers/staging/media/atomisp/pci/atomisp_common.h | 4 ++--
 drivers/staging/media/atomisp/pci/atomisp_v4l2.c   | 8 ++------
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_common.h b/drivers/staging/media/atomisp/pci/atomisp_common.h
index a2462fc306fb..7c6c0124ff72 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_common.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_common.h
@@ -22,8 +22,8 @@
 
 extern int dbg_level;
 extern int dbg_func;
-extern int pad_w;
-extern int pad_h;
+extern const int pad_w;
+extern const int pad_h;
 
 /* Minimum padding requirements for ISP2400 (BYT) */
 #define ISP2400_MIN_PAD_W		12
diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index 812230397409..d117fea2860c 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -64,13 +64,9 @@ MODULE_PARM_DESC(dbg_func,
  * output res, and isp output res. however since we do not have this yet,
  * the chosen solution is the next best thing.
  */
-int pad_w = 16;
-module_param(pad_w, int, 0644);
-MODULE_PARM_DESC(pad_w, "extra data for ISP processing");
+const int pad_w = 16;
 
-int pad_h = 16;
-module_param(pad_h, int, 0644);
-MODULE_PARM_DESC(pad_h, "extra data for ISP processing");
+const int pad_h = 16;
 
 /*
  * FIXME: this is a hack to make easier to support ISP2401 variant.
-- 
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v5 2/2] staging: media: atomisp: Remove dbg_level and dbg_func module parameters
  2026-07-22  5:46 ` [PATCH v5 0/2] This series cleans up unused module parameters from the atomisp driver Prajval Chaudhary
  2026-07-22  5:46   ` [PATCH v5 1/2] staging: media: atomisp: Remove pad_w and pad_h module parameters Prajval Chaudhary
@ 2026-07-22  5:46   ` Prajval Chaudhary
  2026-09-30  9:18     ` Andy Shevchenko
  2026-08-11 14:02   ` [PATCH v5 0/2] This series cleans up unused module parameters from the atomisp driver Andy Shevchenko
  2 siblings, 1 reply; 5+ messages in thread
From: Prajval Chaudhary @ 2026-07-22  5:46 UTC (permalink / raw)
  To: andriy.shevchenko
  Cc: hverkuil-cisco, mchehab, sakari.ailios, gregkh, linux-media,
	linux-kernel, linux-staging, Prajval Chaudhary

Remove the 'dbg_level' and 'dbg_func' module parameters to align the
driver's debug logging with standard Linux kernel printk infrastructure.

Previously:
- 'dbg_level' controlled trace verbosity level (default 0, up to 9).
- 'dbg_func' selected whether CSS trace outputs routed to printk (1)
  or remained disabled/unrouted (0).

Since module-level custom parameters duplicate modern dynamic debug and
printk capabilities, remove them and filter trace messages in
ia_css_debug_vdtrace() to IA_CSS_DEBUG_WARNING or lower.

Signed-off-by: Prajval Chaudhary <prajvalchaudhari733@gmail.com>
---
 drivers/staging/media/atomisp/pci/atomisp_cmd.c      |  8 ++++----
 drivers/staging/media/atomisp/pci/atomisp_common.h   |  6 ++----
 .../staging/media/atomisp/pci/atomisp_compat_css20.c | 12 +++---------
 drivers/staging/media/atomisp/pci/atomisp_ioctl.c    |  4 ++--
 drivers/staging/media/atomisp/pci/atomisp_v4l2.c     | 10 ----------
 .../pci/runtime/debug/interface/ia_css_debug.h       | 10 +++++-----
 .../atomisp/pci/runtime/debug/src/ia_css_debug.c     |  4 ++--
 7 files changed, 18 insertions(+), 36 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index 6cd500d9fd26..0b40cd69cef0 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -3574,8 +3574,8 @@ void atomisp_get_padding(struct atomisp_device *isp, u32 width, u32 height,
 	struct v4l2_mbus_framefmt *sink;
 
 	if (!input->crop_support) {
-		*padding_w = pad_w;
-		*padding_h = pad_h;
+		*padding_w = ATOMISP_PAD_W;
+		*padding_h = ATOMISP_PAD_H;
 		return;
 	}
 
@@ -3588,8 +3588,8 @@ void atomisp_get_padding(struct atomisp_device *isp, u32 width, u32 height,
 		native_rect.height /= 2;
 	}
 
-	*padding_w = min_t(u32, (native_rect.width - width) & ~1, pad_w);
-	*padding_h = min_t(u32, (native_rect.height - height) & ~1, pad_h);
+	*padding_w = min_t(u32, (native_rect.width - width) & ~1, ATOMISP_PAD_W);
+	*padding_h = min_t(u32, (native_rect.height - height) & ~1, ATOMISP_PAD_H);
 
 	/* The below minimum padding requirements are for BYT / ISP2400 only */
 	if (IS_ISP2401)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_common.h b/drivers/staging/media/atomisp/pci/atomisp_common.h
index 7c6c0124ff72..fbc72975ec54 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_common.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_common.h
@@ -20,10 +20,8 @@
 
 #include "ia_css.h"
 
-extern int dbg_level;
-extern int dbg_func;
-extern const int pad_w;
-extern const int pad_h;
+#define ATOMISP_PAD_W	16
+#define ATOMISP_PAD_H	16
 
 /* Minimum padding requirements for ISP2400 (BYT) */
 #define ISP2400_MIN_PAD_W		12
diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
index ea49ebe2da5c..647f6e7e2455 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
@@ -784,7 +784,7 @@ int atomisp_css_load_firmware(struct atomisp_device *isp)
 	isp->css_env.isp_css_env.hw_access_env.load = atomisp_css2_hw_load;
 	isp->css_env.isp_css_env.hw_access_env.store = atomisp_css2_hw_store;
 
-	__set_css_print_env(isp, dbg_func);
+	__set_css_print_env(isp, 1);
 
 	isp->css_env.isp_css_env.print_env.error_print = atomisp_vprintk;
 
@@ -3372,18 +3372,12 @@ void atomisp_css_set_isp_config_applied_frame(struct atomisp_sub_device *asd,
 
 int atomisp_get_css_dbgfunc(void)
 {
-	return dbg_func;
+	return 1;
 }
 
 int atomisp_set_css_dbgfunc(struct atomisp_device *isp, int opt)
 {
-	int ret;
-
-	ret = __set_css_print_env(isp, opt);
-	if (ret == 0)
-		dbg_func = opt;
-
-	return ret;
+	return __set_css_print_env(isp, opt);
 }
 
 void atomisp_en_dz_capt_pipe(struct atomisp_sub_device *asd, bool enable)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index 50366bf10f32..b50d73aaf440 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -554,8 +554,8 @@ static int atomisp_enum_framesizes(struct file *file, void *priv,
 		return ret;
 
 	fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
-	fsize->discrete.width = fse.max_width - pad_w;
-	fsize->discrete.height = fse.max_height - pad_h;
+	fsize->discrete.width = fse.max_width - ATOMISP_PAD_W;
+	fsize->discrete.height = fse.max_height - ATOMISP_PAD_H;
 
 	return 0;
 }
diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index d117fea2860c..fe3b844a47bb 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -45,15 +45,8 @@
 #define CSTATE_EXIT_LATENCY_C1  1
 
 /* cross component debug message flag */
-int dbg_level;
-module_param(dbg_level, int, 0644);
-MODULE_PARM_DESC(dbg_level, "debug message level (default:0)");
 
 /* log function switch */
-int dbg_func = 1;
-module_param(dbg_func, int, 0644);
-MODULE_PARM_DESC(dbg_func,
-		 "log function switch non/printk (default:printk)");
 
 /*
  * Set to 16x16 since this is the amount of lines and pixels the sensor
@@ -64,9 +57,6 @@ MODULE_PARM_DESC(dbg_func,
  * output res, and isp output res. however since we do not have this yet,
  * the chosen solution is the next best thing.
  */
-const int pad_w = 16;
-
-const int pad_h = 16;
 
 /*
  * FIXME: this is a hack to make easier to support ISP2401 variant.
diff --git a/drivers/staging/media/atomisp/pci/runtime/debug/interface/ia_css_debug.h b/drivers/staging/media/atomisp/pci/runtime/debug/interface/ia_css_debug.h
index 2d0e906530af..87cf8ff7408e 100644
--- a/drivers/staging/media/atomisp/pci/runtime/debug/interface/ia_css_debug.h
+++ b/drivers/staging/media/atomisp/pci/runtime/debug/interface/ia_css_debug.h
@@ -9,9 +9,10 @@
 
 /*! \file */
 
-#include <type_support.h>
-#include <linux/stdarg.h>
 #include <linux/bits.h>
+#include <linux/printk.h>
+#include <linux/stdarg.h>
+#include <type_support.h>
 #include "ia_css_types.h"
 #include "ia_css_binary.h"
 #include "ia_css_frame_public.h"
@@ -39,7 +40,6 @@
 #define IA_CSS_DEBUG_INFO    9
 
 /* Global variable which controls the verbosity levels of the debug tracing */
-extern int dbg_level;
 
 /*! @brief Enum defining the different isp parameters to dump.
  *  Values can be combined to dump a combination of sets.
@@ -125,8 +125,8 @@ static inline void __printf(2, 0) ia_css_debug_vdtrace(unsigned int level,
 						       const char *fmt,
 						       va_list args)
 {
-	if (dbg_level >= level)
-		sh_css_vprint(fmt, args);
+	if (level <= IA_CSS_DEBUG_WARNING)
+		vprintk(fmt, args);
 }
 
 __printf(2, 3) void ia_css_debug_dtrace(unsigned int level,
diff --git a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
index 5113aa5973f3..86b2cca2f441 100644
--- a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
+++ b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
@@ -135,13 +135,13 @@ void ia_css_debug_dtrace(unsigned int level, const char *fmt, ...)
 
 void ia_css_debug_set_dtrace_level(const unsigned int trace_level)
 {
-	dbg_level = trace_level;
+	/* Trace level is now controlled dynamically via Linux kernel dynamic debug facility */
 	return;
 }
 
 unsigned int ia_css_debug_get_dtrace_level(void)
 {
-	return dbg_level;
+	return 0;
 }
 
 static const char *debug_stream_format2str(const enum atomisp_input_format
-- 
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5 0/2] This series cleans up unused module parameters from the atomisp driver.
  2026-07-22  5:46 ` [PATCH v5 0/2] This series cleans up unused module parameters from the atomisp driver Prajval Chaudhary
  2026-07-22  5:46   ` [PATCH v5 1/2] staging: media: atomisp: Remove pad_w and pad_h module parameters Prajval Chaudhary
  2026-07-22  5:46   ` [PATCH v5 2/2] staging: media: atomisp: Remove dbg_level and dbg_func " Prajval Chaudhary
@ 2026-08-11 14:02   ` Andy Shevchenko
  2 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2026-08-11 14:02 UTC (permalink / raw)
  To: Prajval Chaudhary
  Cc: hverkuil-cisco, mchehab, sakari.ailios, gregkh, linux-media,
	linux-kernel, linux-staging

On Wed, Jul 22, 2026 at 11:16:13AM +0530, Prajval Chaudhary wrote:
> Patch 1 converts 'pad_w' and 'pad_h' module parameters into internal
> driver defines (ATOMISP_PAD_W and ATOMISP_PAD_H).
> Patch 2 removes 'dbg_level' and 'dbg_func' module parameters and updates
> ia_css_debug_vdtrace() to filter trace logging to IA_CSS_DEBUG_WARNING
> level or below to avoid potential dmesg log flooding.
> 
> Output Behavior Difference:
> - Before: Trace logging depended on setting dbg_level via module
>   parameters. Higher values flooded dmesg with verbose low-level trace
>   messages.
> - After: Trace logging is filtered at IA_CSS_DEBUG_WARNING via standard
>   vprintk facilities, avoiding dmesg flooding while maintaining
>   critical warnings.

Why do you think this is good enough? I already pointed out that this
needs the report from the field, id est showing the before and after on
real hardware. Also I mentioned that printk() is not good for low-level
traces and we _have_ the mechanisms for that kind of messages, exempli
gratia trace events and trace points.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5 2/2] staging: media: atomisp: Remove dbg_level and dbg_func module parameters
  2026-07-22  5:46   ` [PATCH v5 2/2] staging: media: atomisp: Remove dbg_level and dbg_func " Prajval Chaudhary
@ 2026-09-30  9:18     ` Andy Shevchenko
  0 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2026-09-30  9:18 UTC (permalink / raw)
  To: Prajval Chaudhary
  Cc: hverkuil-cisco, mchehab, sakari.ailios, gregkh, linux-media,
	linux-kernel, linux-staging

On Wed, Jul 22, 2026 at 11:16:15AM +0530, Prajval Chaudhary wrote:
> Remove the 'dbg_level' and 'dbg_func' module parameters to align the
> driver's debug logging with standard Linux kernel printk infrastructure.
> 
> Previously:
> - 'dbg_level' controlled trace verbosity level (default 0, up to 9).
> - 'dbg_func' selected whether CSS trace outputs routed to printk (1)
>   or remained disabled/unrouted (0).
> 
> Since module-level custom parameters duplicate modern dynamic debug and
> printk capabilities, remove them and filter trace messages in
> ia_css_debug_vdtrace() to IA_CSS_DEBUG_WARNING or lower.

...

>  unsigned int ia_css_debug_get_dtrace_level(void)
>  {
> -	return dbg_level;
> +	return 0;
>  }

I believe this changes the flexibility of the tracing. But like I said, the
whole infrastructure should be switched to use trace events and trace points.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-30  9:18 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <YOUR_ANDY_EMAIL_MESSAGE_ID_HERE>
2026-07-22  5:46 ` [PATCH v5 0/2] This series cleans up unused module parameters from the atomisp driver Prajval Chaudhary
2026-07-22  5:46   ` [PATCH v5 1/2] staging: media: atomisp: Remove pad_w and pad_h module parameters Prajval Chaudhary
2026-07-22  5:46   ` [PATCH v5 2/2] staging: media: atomisp: Remove dbg_level and dbg_func " Prajval Chaudhary
2026-09-30  9:18     ` Andy Shevchenko
2026-08-11 14:02   ` [PATCH v5 0/2] This series cleans up unused module parameters from the atomisp driver Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®