[PATCH v2 00/10] staging: media: zoran: fusion in one module

[PATCH v2 00/10] staging: media: zoran: fusion in one module

[Corentin Labbe]

Hello

The main change of this serie is to fusion all zoran related modules in
one.
This fixes the load order problem when everything is built-in.

Regards

Changes since v1:
- add missing debugfs cleaning
- clean some remaining module_get/put functions which made impossible to
  remove the zoran module
- added the two latest patchs

Corentin Labbe (10):
  staging: media: zoran: move module parameter checks to zoran_probe
  staging: media: zoran: use module_pci_driver
  staging: media: zoran: rename debug module parameter
  staging: media: zoran: add debugfs
  staging: media: zoran: videocode: remove procfs
  staging: media: zoran: fusion all modules
  staging: media: zoran: remove vidmem
  staging: media: zoran: move videodev alloc
  staging: media: zoran: move config select on primary kconfig
  staging: media: zoran: introduce zoran_i2c_init

 drivers/staging/media/zoran/Kconfig        |  46 +--
 drivers/staging/media/zoran/Makefile       |   8 +-
 drivers/staging/media/zoran/videocodec.c   |  68 +----
 drivers/staging/media/zoran/videocodec.h   |   6 +-
 drivers/staging/media/zoran/zoran.h        |   6 +-
 drivers/staging/media/zoran/zoran_card.c   | 328 ++++++++++++++-------
 drivers/staging/media/zoran/zoran_driver.c |   5 +-
 drivers/staging/media/zoran/zr36016.c      |  24 +-
 drivers/staging/media/zoran/zr36016.h      |   2 +
 drivers/staging/media/zoran/zr36050.c      |  21 +-
 drivers/staging/media/zoran/zr36050.h      |   2 +
 drivers/staging/media/zoran/zr36060.c      |  21 +-
 drivers/staging/media/zoran/zr36060.h      |   2 +
 13 files changed, 291 insertions(+), 248 deletions(-)

-- 
2.32.0

[PATCH v2 01/10] staging: media: zoran: move module parameter checks to zoran_probe

[Corentin Labbe]

We need to empty zoran_init() for removing it later.
Furthermore, this permit to use pci_xxx instead of pr_xxx for prettier
printing.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 drivers/staging/media/zoran/zoran_card.c | 64 ++++++++++++------------
 1 file changed, 33 insertions(+), 31 deletions(-)

diff --git a/drivers/staging/media/zoran/zoran_card.c b/drivers/staging/media/zoran/zoran_card.c
index f259585b0689..3bc0e64f1007 100644
--- a/drivers/staging/media/zoran/zoran_card.c
+++ b/drivers/staging/media/zoran/zoran_card.c
@@ -1067,6 +1067,39 @@ static int zoran_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	unsigned int nr;
 	int err;
 
+	pci_info(pdev, "Zoran MJPEG board driver version %s\n", ZORAN_VERSION);
+
+	/* check the parameters we have been given, adjust if necessary */
+	if (v4l_nbufs < 2)
+		v4l_nbufs = 2;
+	if (v4l_nbufs > VIDEO_MAX_FRAME)
+		v4l_nbufs = VIDEO_MAX_FRAME;
+	/* The user specifies the in KB, we want them in byte (and page aligned) */
+	v4l_bufsize = PAGE_ALIGN(v4l_bufsize * 1024);
+	if (v4l_bufsize < 32768)
+		v4l_bufsize = 32768;
+	/* 2 MB is arbitrary but sufficient for the maximum possible images */
+	if (v4l_bufsize > 2048 * 1024)
+		v4l_bufsize = 2048 * 1024;
+	if (jpg_nbufs < 4)
+		jpg_nbufs = 4;
+	if (jpg_nbufs > BUZ_MAX_FRAME)
+		jpg_nbufs = BUZ_MAX_FRAME;
+	jpg_bufsize = PAGE_ALIGN(jpg_bufsize * 1024);
+	if (jpg_bufsize < 8192)
+		jpg_bufsize = 8192;
+	if (jpg_bufsize > (512 * 1024))
+		jpg_bufsize = 512 * 1024;
+	/* Use parameter for vidmem or try to find a video card */
+	if (vidmem)
+		pci_info(pdev, "%s: Using supplied video memory base address @ 0x%lx\n",
+			 ZORAN_NAME, vidmem);
+
+	/* some mainboards might not do PCI-PCI data transfer well */
+	if (pci_pci_problems & (PCIPCI_FAIL | PCIAGP_FAIL | PCIPCI_ALIMAGIK))
+		pci_warn(pdev, "%s: chipset does not support reliable PCI-PCI DMA\n",
+			 ZORAN_NAME);
+
 	err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
 	if (err)
 		return -ENODEV;
@@ -1285,37 +1318,6 @@ static int __init zoran_init(void)
 {
 	int res;
 
-	pr_info("Zoran MJPEG board driver version %s\n", ZORAN_VERSION);
-
-	/* check the parameters we have been given, adjust if necessary */
-	if (v4l_nbufs < 2)
-		v4l_nbufs = 2;
-	if (v4l_nbufs > VIDEO_MAX_FRAME)
-		v4l_nbufs = VIDEO_MAX_FRAME;
-	/* The user specifies the in KB, we want them in byte (and page aligned) */
-	v4l_bufsize = PAGE_ALIGN(v4l_bufsize * 1024);
-	if (v4l_bufsize < 32768)
-		v4l_bufsize = 32768;
-	/* 2 MB is arbitrary but sufficient for the maximum possible images */
-	if (v4l_bufsize > 2048 * 1024)
-		v4l_bufsize = 2048 * 1024;
-	if (jpg_nbufs < 4)
-		jpg_nbufs = 4;
-	if (jpg_nbufs > BUZ_MAX_FRAME)
-		jpg_nbufs = BUZ_MAX_FRAME;
-	jpg_bufsize = PAGE_ALIGN(jpg_bufsize * 1024);
-	if (jpg_bufsize < 8192)
-		jpg_bufsize = 8192;
-	if (jpg_bufsize > (512 * 1024))
-		jpg_bufsize = 512 * 1024;
-	/* Use parameter for vidmem or try to find a video card */
-	if (vidmem)
-		pr_info("%s: Using supplied video memory base address @ 0x%lx\n", ZORAN_NAME, vidmem);
-
-	/* some mainboards might not do PCI-PCI data transfer well */
-	if (pci_pci_problems & (PCIPCI_FAIL | PCIAGP_FAIL | PCIPCI_ALIMAGIK))
-		pr_warn("%s: chipset does not support reliable PCI-PCI DMA\n", ZORAN_NAME);
-
 	res = pci_register_driver(&zoran_driver);
 	if (res) {
 		pr_err("Unable to register ZR36057 driver\n");
-- 
2.32.0

[PATCH v2 02/10] staging: media: zoran: use module_pci_driver

[Corentin Labbe]

Simplify code by using module_pci_driver()

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 drivers/staging/media/zoran/zoran_card.c | 21 +--------------------
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/staging/media/zoran/zoran_card.c b/drivers/staging/media/zoran/zoran_card.c
index 3bc0e64f1007..f1465fbf98af 100644
--- a/drivers/staging/media/zoran/zoran_card.c
+++ b/drivers/staging/media/zoran/zoran_card.c
@@ -1314,23 +1314,4 @@ static struct pci_driver zoran_driver = {
 	.remove = zoran_remove,
 };
 
-static int __init zoran_init(void)
-{
-	int res;
-
-	res = pci_register_driver(&zoran_driver);
-	if (res) {
-		pr_err("Unable to register ZR36057 driver\n");
-		return res;
-	}
-
-	return 0;
-}
-
-static void __exit zoran_exit(void)
-{
-	pci_unregister_driver(&zoran_driver);
-}
-
-module_init(zoran_init);
-module_exit(zoran_exit);
+module_pci_driver(zoran_driver);
-- 
2.32.0

[PATCH v2 03/10] staging: media: zoran: rename debug module parameter

[Corentin Labbe]

All zoran module will be merged, so to prevent conflict, the debug
module parameter need to be renamed

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 drivers/staging/media/zoran/videocodec.c |  8 ++++----
 drivers/staging/media/zoran/zr36016.c    | 12 ++++++------
 drivers/staging/media/zoran/zr36050.c    |  8 ++++----
 drivers/staging/media/zoran/zr36060.c    |  9 ++++-----
 4 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/media/zoran/videocodec.c b/drivers/staging/media/zoran/videocodec.c
index 28031d3fd757..31019b5f377e 100644
--- a/drivers/staging/media/zoran/videocodec.c
+++ b/drivers/staging/media/zoran/videocodec.c
@@ -26,13 +26,13 @@
 
 #include "videocodec.h"
 
-static int debug;
-module_param(debug, int, 0);
-MODULE_PARM_DESC(debug, "Debug level (0-4)");
+static int videocodec_debug;
+module_param(videocodec_debug, int, 0);
+MODULE_PARM_DESC(videocodec_debug, "Debug level (0-4)");
 
 #define dprintk(num, format, args...) \
 	do { \
-		if (debug >= num) \
+		if (videocodec_debug >= num) \
 			printk(format, ##args); \
 	} while (0)
 
diff --git a/drivers/staging/media/zoran/zr36016.c b/drivers/staging/media/zoran/zr36016.c
index 9b350a885879..50605460a44b 100644
--- a/drivers/staging/media/zoran/zr36016.c
+++ b/drivers/staging/media/zoran/zr36016.c
@@ -22,14 +22,14 @@
 /* amount of chips attached via this driver */
 static int zr36016_codecs;
 
-/* debugging is available via module parameter */
-static int debug;
-module_param(debug, int, 0);
-MODULE_PARM_DESC(debug, "Debug level (0-4)");
+static int zr36016_debug;
+module_param(zr36016_debug, int, 0);
+MODULE_PARM_DESC(zr36016_debug, "Debug level (0-4)");
+
 
 #define dprintk(num, format, args...) \
 	do { \
-		if (debug >= num) \
+		if (zr36016_debug >= num) \
 			printk(format, ##args); \
 	} while (0)
 
@@ -120,7 +120,7 @@ static u8 zr36016_read_version(struct zr36016 *ptr)
 
 static int zr36016_basic_test(struct zr36016 *ptr)
 {
-	if (debug) {
+	if (zr36016_debug) {
 		int i;
 
 		zr36016_writei(ptr, ZR016I_PAX_LO, 0x55);
diff --git a/drivers/staging/media/zoran/zr36050.c b/drivers/staging/media/zoran/zr36050.c
index c62af27f2683..4dc7927fefc3 100644
--- a/drivers/staging/media/zoran/zr36050.c
+++ b/drivers/staging/media/zoran/zr36050.c
@@ -32,13 +32,13 @@
 static int zr36050_codecs;
 
 /* debugging is available via module parameter */
-static int debug;
-module_param(debug, int, 0);
-MODULE_PARM_DESC(debug, "Debug level (0-4)");
+static int zr36050_debug;
+module_param(zr36050_debug, int, 0);
+MODULE_PARM_DESC(zr36050_debug, "Debug level (0-4)");
 
 #define dprintk(num, format, args...) \
 	do { \
-		if (debug >= num) \
+		if (zr36050_debug >= num) \
 			printk(format, ##args); \
 	} while (0)
 
diff --git a/drivers/staging/media/zoran/zr36060.c b/drivers/staging/media/zoran/zr36060.c
index 1c3af11b5f24..7904d5b1f402 100644
--- a/drivers/staging/media/zoran/zr36060.c
+++ b/drivers/staging/media/zoran/zr36060.c
@@ -34,14 +34,13 @@ static bool low_bitrate;
 module_param(low_bitrate, bool, 0);
 MODULE_PARM_DESC(low_bitrate, "Buz compatibility option, halves bitrate");
 
-/* debugging is available via module parameter */
-static int debug;
-module_param(debug, int, 0);
-MODULE_PARM_DESC(debug, "Debug level (0-4)");
+static int zr36060_debug;
+module_param(zr36060_debug, int, 0);
+MODULE_PARM_DESC(zr36060_debug, "Debug level (0-4)");
 
 #define dprintk(num, format, args...) \
 	do { \
-		if (debug >= num) \
+		if (zr36060_debug >= num) \
 			printk(format, ##args); \
 	} while (0)
 
-- 
2.32.0

[PATCH v2 04/10] staging: media: zoran: add debugfs

[Corentin Labbe]

Add debugfs for displaying zoran debug and stats information.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 drivers/staging/media/zoran/Kconfig      |  9 ++++++
 drivers/staging/media/zoran/zoran.h      |  4 +++
 drivers/staging/media/zoran/zoran_card.c | 41 ++++++++++++++++++++++++
 3 files changed, 54 insertions(+)

diff --git a/drivers/staging/media/zoran/Kconfig b/drivers/staging/media/zoran/Kconfig
index 7874842033ca..06f79b91cda7 100644
--- a/drivers/staging/media/zoran/Kconfig
+++ b/drivers/staging/media/zoran/Kconfig
@@ -74,3 +74,12 @@ config VIDEO_ZORAN_AVS6EYES
 	select VIDEO_KS0127 if MEDIA_SUBDRV_AUTOSELECT
 	help
 	  Support for the AverMedia 6 Eyes video surveillance card.
+
+config VIDEO_ZORAN_DEBUG
+	bool "Enable zoran debugfs"
+	depends on VIDEO_ZORAN
+	depends on DEBUG_FS
+	help
+	  Say y to enable zoran debug file.
+	  This will create /sys/kernel/debug/CARD_NAME/debug for displaying
+	  stats and debug information.
diff --git a/drivers/staging/media/zoran/zoran.h b/drivers/staging/media/zoran/zoran.h
index b1ad2a2b914c..c37d064ff11d 100644
--- a/drivers/staging/media/zoran/zoran.h
+++ b/drivers/staging/media/zoran/zoran.h
@@ -18,6 +18,7 @@
 #ifndef _BUZ_H_
 #define _BUZ_H_
 
+#include <linux/debugfs.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-ctrls.h>
 #include <media/videobuf2-core.h>
@@ -295,6 +296,9 @@ struct zoran {
 	struct list_head queued_bufs;
 	spinlock_t queued_bufs_lock; /* Protects queued_bufs */
 	struct zr_buffer *inuse[BUZ_NUM_STAT_COM * 2];
+#ifdef CONFIG_VIDEO_ZORAN_DEBUG
+	struct dentry *dbgfs_dir;
+#endif
 };
 
 static inline struct zoran *to_zoran(struct v4l2_device *v4l2_dev)
diff --git a/drivers/staging/media/zoran/zoran_card.c b/drivers/staging/media/zoran/zoran_card.c
index f1465fbf98af..6f29986a3fc2 100644
--- a/drivers/staging/media/zoran/zoran_card.c
+++ b/drivers/staging/media/zoran/zoran_card.c
@@ -945,6 +945,8 @@ static void zoran_remove(struct pci_dev *pdev)
 	if (!zr->initialized)
 		goto exit_free;
 
+	debugfs_remove_recursive(zr->dbgfs_dir);
+
 	zoran_queue_exit(zr);
 
 	/* unregister videocodec bus */
@@ -1051,6 +1053,39 @@ static const struct v4l2_ctrl_ops zoran_video_ctrl_ops = {
 	.s_ctrl = zoran_video_set_ctrl,
 };
 
+#ifdef CONFIG_VIDEO_ZORAN_DEBUG
+static int zoran_debugfs_show(struct seq_file *seq, void *v)
+{
+	struct zoran *zr = seq->private;
+
+	seq_printf(seq, "Running mode %x\n", zr->running);
+	seq_printf(seq, "Codec mode %x\n", zr->codec_mode);
+	seq_printf(seq, "Norm %llx\n", zr->norm);
+	seq_printf(seq, "Input %d\n", zr->input);
+	seq_printf(seq, "Buffersize %d\n", zr->buffer_size);
+
+	seq_printf(seq, "V4L width %dx%d\n", zr->v4l_settings.width, zr->v4l_settings.height);
+	seq_printf(seq, "V4L bytesperline %d\n", zr->v4l_settings.bytesperline);
+
+	seq_printf(seq, "JPG decimation %u\n", zr->jpg_settings.decimation);
+	seq_printf(seq, "JPG hor_dcm %u\n", zr->jpg_settings.hor_dcm);
+	seq_printf(seq, "JPG ver_dcm %u\n", zr->jpg_settings.ver_dcm);
+	seq_printf(seq, "JPG tmp_dcm %u\n", zr->jpg_settings.tmp_dcm);
+	seq_printf(seq, "JPG odd_even %u\n", zr->jpg_settings.odd_even);
+	seq_printf(seq, "JPG crop %dx%d %d %d\n",
+		zr->jpg_settings.img_x,
+		zr->jpg_settings.img_y,
+		zr->jpg_settings.img_width,
+		zr->jpg_settings.img_height);
+
+	seq_printf(seq, "Prepared %u\n", zr->prepared);
+	seq_printf(seq, "Queued %u\n", zr->queued);
+	return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(zoran_debugfs);
+#endif
+
 /*
  *   Scan for a Buz card (actually for the PCI controller ZR36057),
  *   request the irq and map the io memory
@@ -1286,6 +1321,12 @@ static int zoran_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	zr->map_mode = ZORAN_MAP_MODE_RAW;
 
+#ifdef CONFIG_VIDEO_ZORAN_DEBUG
+	zr->dbgfs_dir = debugfs_create_dir(ZR_DEVNAME(zr), NULL);
+	debugfs_create_file("debug", 0444,
+					      zr->dbgfs_dir, zr,
+					      &zoran_debugfs_fops);
+#endif
 	return 0;
 
 zr_detach_vfe:
-- 
2.32.0

Re: [PATCH v2 04/10] staging: media: zoran: add debugfs

[Dan Carpenter]

On Wed, Oct 13, 2021 at 06:58:06PM +0000, Corentin Labbe wrote:
> +config VIDEO_ZORAN_DEBUG
> +	bool "Enable zoran debugfs"
> +	depends on VIDEO_ZORAN
> +	depends on DEBUG_FS
> +	help
> +	  Say y to enable zoran debug file.
> +	  This will create /sys/kernel/debug/CARD_NAME/debug for displaying
> +	  stats and debug information.

Why bother with a CONFIG?  Just make it always on?

> @@ -1286,6 +1321,12 @@ static int zoran_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  	zr->map_mode = ZORAN_MAP_MODE_RAW;
>  
> +#ifdef CONFIG_VIDEO_ZORAN_DEBUG
> +	zr->dbgfs_dir = debugfs_create_dir(ZR_DEVNAME(zr), NULL);
> +	debugfs_create_file("debug", 0444,
> +					      zr->dbgfs_dir, zr,
> +					      &zoran_debugfs_fops);

This whitespace is weird.

regards,
dan carpenter

Re: [PATCH v2 04/10] staging: media: zoran: add debugfs

[LABBE Corentin]

Le Thu, Oct 14, 2021 at 10:37:52AM +0300, Dan Carpenter a écrit :
> On Wed, Oct 13, 2021 at 06:58:06PM +0000, Corentin Labbe wrote:
> > +config VIDEO_ZORAN_DEBUG
> > +	bool "Enable zoran debugfs"
> > +	depends on VIDEO_ZORAN
> > +	depends on DEBUG_FS
> > +	help
> > +	  Say y to enable zoran debug file.
> > +	  This will create /sys/kernel/debug/CARD_NAME/debug for displaying
> > +	  stats and debug information.
> 
> Why bother with a CONFIG?  Just make it always on?
> 

Hello

I love to provides choice to user (and so avoid a dep on DEBUG_FS), even if I think I am the only one remaining user.

> > @@ -1286,6 +1321,12 @@ static int zoran_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >  
> >  	zr->map_mode = ZORAN_MAP_MODE_RAW;
> >  
> > +#ifdef CONFIG_VIDEO_ZORAN_DEBUG
> > +	zr->dbgfs_dir = debugfs_create_dir(ZR_DEVNAME(zr), NULL);
> > +	debugfs_create_file("debug", 0444,
> > +					      zr->dbgfs_dir, zr,
> > +					      &zoran_debugfs_fops);
> 
> This whitespace is weird.

Definitively Yes, fixed!

Thanks
Regards

Re: [PATCH v2 04/10] staging: media: zoran: add debugfs

[Dan Carpenter]

On Sun, Oct 17, 2021 at 10:05:06PM +0200, LABBE Corentin wrote:
> Le Thu, Oct 14, 2021 at 10:37:52AM +0300, Dan Carpenter a écrit :
> > On Wed, Oct 13, 2021 at 06:58:06PM +0000, Corentin Labbe wrote:
> > > +config VIDEO_ZORAN_DEBUG
> > > +	bool "Enable zoran debugfs"
> > > +	depends on VIDEO_ZORAN
> > > +	depends on DEBUG_FS
> > > +	help
> > > +	  Say y to enable zoran debug file.
> > > +	  This will create /sys/kernel/debug/CARD_NAME/debug for displaying
> > > +	  stats and debug information.
> > 
> > Why bother with a CONFIG?  Just make it always on?
> > 
> 
> Hello
> 
> I love to provides choice to user (and so avoid a dep on DEBUG_FS), even if I think I am the only one remaining user.

Sorry, for the delay, I was on vacation.

No, there is no depends on DEBUG_FS in the method that I am describing.

How that works is when DEBUG_FS is turned on then it's on for everything,
but when it's disabled it's disabled for everything.  You do not need
the "depends on DEBUG_FS" and if you make this an option the it feels
like it should be a selects DEBUG_FS instead.

How this normally works is that when you have debugfs disabled, there
are dummy files in the debugfs .h files.  I bet the compiler can tell
most of those are empty and removes them.  So if you have DEBUG_FS then
it doesn't use that much more memory than when VIDEO_ZORAN_DEBUG is
disabled.

I don't know if I'm being clear at all #jetlag.

It should be easy to check.  Just remove the "depends on DEBUG_FS" and
enable VIDEO_ZORAN_DEBUG.  Disable DEBUG_FS.  It should still build fine
because of the dummy functions.  That's build 1.  Then disable
VIDEO_ZORAN_DEBUG and that's build 2.  See how much memory difference
there is between 1 and 2.

regards,
dan carpenter

Re: [PATCH v2 04/10] staging: media: zoran: add debugfs

[LABBE Corentin]

Le Tue, Nov 02, 2021 at 08:40:28PM +0300, Dan Carpenter a écrit :
> On Sun, Oct 17, 2021 at 10:05:06PM +0200, LABBE Corentin wrote:
> > Le Thu, Oct 14, 2021 at 10:37:52AM +0300, Dan Carpenter a écrit :
> > > On Wed, Oct 13, 2021 at 06:58:06PM +0000, Corentin Labbe wrote:
> > > > +config VIDEO_ZORAN_DEBUG
> > > > +	bool "Enable zoran debugfs"
> > > > +	depends on VIDEO_ZORAN
> > > > +	depends on DEBUG_FS
> > > > +	help
> > > > +	  Say y to enable zoran debug file.
> > > > +	  This will create /sys/kernel/debug/CARD_NAME/debug for displaying
> > > > +	  stats and debug information.
> > > 
> > > Why bother with a CONFIG?  Just make it always on?
> > > 
> > 
> > Hello
> > 
> > I love to provides choice to user (and so avoid a dep on DEBUG_FS), even if I think I am the only one remaining user.
> 
> Sorry, for the delay, I was on vacation.
> 
> No, there is no depends on DEBUG_FS in the method that I am describing.
> 
> How that works is when DEBUG_FS is turned on then it's on for everything,
> but when it's disabled it's disabled for everything.  You do not need
> the "depends on DEBUG_FS" and if you make this an option the it feels
> like it should be a selects DEBUG_FS instead.
> 
> How this normally works is that when you have debugfs disabled, there
> are dummy files in the debugfs .h files.  I bet the compiler can tell
> most of those are empty and removes them.  So if you have DEBUG_FS then
> it doesn't use that much more memory than when VIDEO_ZORAN_DEBUG is
> disabled.
> 
> I don't know if I'm being clear at all #jetlag.
> 
> It should be easy to check.  Just remove the "depends on DEBUG_FS" and
> enable VIDEO_ZORAN_DEBUG.  Disable DEBUG_FS.  It should still build fine
> because of the dummy functions.  That's build 1.  Then disable
> VIDEO_ZORAN_DEBUG and that's build 2.  See how much memory difference
> there is between 1 and 2.
> 

No worry for the delay.
Anyway, I have removed VIDEO_ZORAN_DEBUG in v3 since Hans Verkuil also asked for its removing.

Regards

Re: [PATCH v2 04/10] staging: media: zoran: add debugfs

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 21047 bytes --]

Hi Corentin,

I love your patch! Perhaps something to improve:

[auto build test WARNING on staging/staging-testing]

url:    https://github.com/0day-ci/linux/commits/Corentin-Labbe/staging-media-zoran-fusion-in-one-module/20211014-025945
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 6ac113f741a7674e4268eea3eb13972732d83571
config: x86_64-randconfig-a016-20211014 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6c76d0101193aa4eb891a6954ff047eda2f9cf71)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/74fc116256f23b2c65d0c813f1d90b617ce9c97d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Corentin-Labbe/staging-media-zoran-fusion-in-one-module/20211014-025945
        git checkout 74fc116256f23b2c65d0c813f1d90b617ce9c97d
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/staging/media/zoran/zoran_card.c:948:31: error: no member named 'dbgfs_dir' in 'struct zoran'
           debugfs_remove_recursive(zr->dbgfs_dir);
                                    ~~  ^
>> drivers/staging/media/zoran/zoran_card.c:1141:46: warning: implicit conversion from 'unsigned long long' to 'unsigned int' changes value from 18446744073709551615 to 4294967295 [-Wconstant-conversion]
           vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~             ^~~~~~~~~~~~~~~~
   include/linux/dma-mapping.h:76:40: note: expanded from macro 'DMA_BIT_MASK'
   #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
                                          ^~~~~
   1 warning and 1 error generated.


vim +1141 drivers/staging/media/zoran/zoran_card.c

74fc116256f23b Corentin Labbe 2021-10-13  1088  
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1089  /*
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1090   *   Scan for a Buz card (actually for the PCI controller ZR36057),
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1091   *   request the irq and map the io memory
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1092   */
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1093  static int zoran_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1094  {
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1095  	unsigned char latency, need_latency;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1096  	struct zoran *zr;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1097  	int result;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1098  	struct videocodec_master *master_vfe = NULL;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1099  	struct videocodec_master *master_codec = NULL;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1100  	int card_num;
d61c7451fcb712 Corentin Labbe 2020-09-25  1101  	const char *codec_name, *vfe_name;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1102  	unsigned int nr;
d4ae3689226e56 Corentin Labbe 2020-09-25  1103  	int err;
d4ae3689226e56 Corentin Labbe 2020-09-25  1104  
26edeeecea59d6 Corentin Labbe 2021-10-13  1105  	pci_info(pdev, "Zoran MJPEG board driver version %s\n", ZORAN_VERSION);
26edeeecea59d6 Corentin Labbe 2021-10-13  1106  
26edeeecea59d6 Corentin Labbe 2021-10-13  1107  	/* check the parameters we have been given, adjust if necessary */
26edeeecea59d6 Corentin Labbe 2021-10-13  1108  	if (v4l_nbufs < 2)
26edeeecea59d6 Corentin Labbe 2021-10-13  1109  		v4l_nbufs = 2;
26edeeecea59d6 Corentin Labbe 2021-10-13  1110  	if (v4l_nbufs > VIDEO_MAX_FRAME)
26edeeecea59d6 Corentin Labbe 2021-10-13  1111  		v4l_nbufs = VIDEO_MAX_FRAME;
26edeeecea59d6 Corentin Labbe 2021-10-13  1112  	/* The user specifies the in KB, we want them in byte (and page aligned) */
26edeeecea59d6 Corentin Labbe 2021-10-13  1113  	v4l_bufsize = PAGE_ALIGN(v4l_bufsize * 1024);
26edeeecea59d6 Corentin Labbe 2021-10-13  1114  	if (v4l_bufsize < 32768)
26edeeecea59d6 Corentin Labbe 2021-10-13  1115  		v4l_bufsize = 32768;
26edeeecea59d6 Corentin Labbe 2021-10-13  1116  	/* 2 MB is arbitrary but sufficient for the maximum possible images */
26edeeecea59d6 Corentin Labbe 2021-10-13  1117  	if (v4l_bufsize > 2048 * 1024)
26edeeecea59d6 Corentin Labbe 2021-10-13  1118  		v4l_bufsize = 2048 * 1024;
26edeeecea59d6 Corentin Labbe 2021-10-13  1119  	if (jpg_nbufs < 4)
26edeeecea59d6 Corentin Labbe 2021-10-13  1120  		jpg_nbufs = 4;
26edeeecea59d6 Corentin Labbe 2021-10-13  1121  	if (jpg_nbufs > BUZ_MAX_FRAME)
26edeeecea59d6 Corentin Labbe 2021-10-13  1122  		jpg_nbufs = BUZ_MAX_FRAME;
26edeeecea59d6 Corentin Labbe 2021-10-13  1123  	jpg_bufsize = PAGE_ALIGN(jpg_bufsize * 1024);
26edeeecea59d6 Corentin Labbe 2021-10-13  1124  	if (jpg_bufsize < 8192)
26edeeecea59d6 Corentin Labbe 2021-10-13  1125  		jpg_bufsize = 8192;
26edeeecea59d6 Corentin Labbe 2021-10-13  1126  	if (jpg_bufsize > (512 * 1024))
26edeeecea59d6 Corentin Labbe 2021-10-13  1127  		jpg_bufsize = 512 * 1024;
26edeeecea59d6 Corentin Labbe 2021-10-13  1128  	/* Use parameter for vidmem or try to find a video card */
26edeeecea59d6 Corentin Labbe 2021-10-13  1129  	if (vidmem)
26edeeecea59d6 Corentin Labbe 2021-10-13  1130  		pci_info(pdev, "%s: Using supplied video memory base address @ 0x%lx\n",
26edeeecea59d6 Corentin Labbe 2021-10-13  1131  			 ZORAN_NAME, vidmem);
26edeeecea59d6 Corentin Labbe 2021-10-13  1132  
26edeeecea59d6 Corentin Labbe 2021-10-13  1133  	/* some mainboards might not do PCI-PCI data transfer well */
26edeeecea59d6 Corentin Labbe 2021-10-13  1134  	if (pci_pci_problems & (PCIPCI_FAIL | PCIAGP_FAIL | PCIPCI_ALIMAGIK))
26edeeecea59d6 Corentin Labbe 2021-10-13  1135  		pci_warn(pdev, "%s: chipset does not support reliable PCI-PCI DMA\n",
26edeeecea59d6 Corentin Labbe 2021-10-13  1136  			 ZORAN_NAME);
26edeeecea59d6 Corentin Labbe 2021-10-13  1137  
d4ae3689226e56 Corentin Labbe 2020-09-25  1138  	err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
d4ae3689226e56 Corentin Labbe 2020-09-25  1139  	if (err)
d4ae3689226e56 Corentin Labbe 2020-09-25  1140  		return -ENODEV;
d4ae3689226e56 Corentin Labbe 2020-09-25 @1141  	vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1142  
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1143  	nr = zoran_num++;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1144  	if (nr >= BUZ_MAX) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25  1145  		pci_err(pdev, "driver limited to %d card(s) maximum\n", BUZ_MAX);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1146  		return -ENOENT;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1147  	}
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1148  
6d1d9ba2c4396f Corentin Labbe 2020-09-25  1149  	zr = devm_kzalloc(&pdev->dev, sizeof(*zr), GFP_KERNEL);
5e195bbddabdd9 Corentin Labbe 2020-09-25  1150  	if (!zr)
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1151  		return -ENOMEM;
5e195bbddabdd9 Corentin Labbe 2020-09-25  1152  
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1153  	zr->v4l2_dev.notify = zoran_subdev_notify;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1154  	if (v4l2_device_register(&pdev->dev, &zr->v4l2_dev))
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1155  		goto zr_free_mem;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1156  	zr->pci_dev = pdev;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1157  	zr->id = nr;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1158  	snprintf(ZR_DEVNAME(zr), sizeof(ZR_DEVNAME(zr)), "MJPEG[%u]", zr->id);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1159  	if (v4l2_ctrl_handler_init(&zr->hdl, 10))
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1160  		goto zr_unreg;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1161  	zr->v4l2_dev.ctrl_handler = &zr->hdl;
8cb356d4eaae11 Corentin Labbe 2020-09-25  1162  	v4l2_ctrl_new_std(&zr->hdl, &zoran_video_ctrl_ops,
8cb356d4eaae11 Corentin Labbe 2020-09-25  1163  			  V4L2_CID_JPEG_COMPRESSION_QUALITY, 0,
8cb356d4eaae11 Corentin Labbe 2020-09-25  1164  			  100, 1, 50);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1165  	spin_lock_init(&zr->spinlock);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1166  	mutex_init(&zr->lock);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1167  	if (pci_enable_device(pdev))
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1168  		goto zr_unreg;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1169  	zr->revision = zr->pci_dev->revision;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1170  
9bb2720293a04f Corentin Labbe 2020-09-25  1171  	pci_info(zr->pci_dev, "Zoran ZR360%c7 (rev %d), irq: %d, memory: 0x%08llx\n",
9bb2720293a04f Corentin Labbe 2020-09-25  1172  		 zr->revision < 2 ? '5' : '6', zr->revision,
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1173  		 zr->pci_dev->irq, (uint64_t)pci_resource_start(zr->pci_dev, 0));
9bb2720293a04f Corentin Labbe 2020-09-25  1174  	if (zr->revision >= 2)
9bb2720293a04f Corentin Labbe 2020-09-25  1175  		pci_info(zr->pci_dev, "Subsystem vendor=0x%04x id=0x%04x\n",
9bb2720293a04f Corentin Labbe 2020-09-25  1176  			 zr->pci_dev->subsystem_vendor, zr->pci_dev->subsystem_device);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1177  
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1178  	/* Use auto-detected card type? */
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1179  	if (card[nr] == -1) {
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1180  		if (zr->revision < 2) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25  1181  			pci_err(pdev, "No card type specified, please use the card=X module parameter\n");
b7c3b2bb9db412 Corentin Labbe 2020-09-25  1182  			pci_err(pdev, "It is not possible to auto-detect ZR36057 based cards\n");
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1183  			goto zr_unreg;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1184  		}
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1185  
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1186  		card_num = ent->driver_data;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1187  		if (card_num >= NUM_CARDS) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25  1188  			pci_err(pdev, "Unknown card, try specifying card=X module parameter\n");
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1189  			goto zr_unreg;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1190  		}
daae1da762c1e3 Corentin Labbe 2020-09-25  1191  		pci_info(zr->pci_dev, "%s() - card %s detected\n", __func__, zoran_cards[card_num].name);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1192  	} else {
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1193  		card_num = card[nr];
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1194  		if (card_num >= NUM_CARDS || card_num < 0) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25  1195  			pci_err(pdev, "User specified card type %d out of range (0 .. %d)\n",
b7c3b2bb9db412 Corentin Labbe 2020-09-25  1196  				card_num, NUM_CARDS - 1);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1197  			goto zr_unreg;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1198  		}
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1199  	}
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1200  
5e195bbddabdd9 Corentin Labbe 2020-09-25  1201  	/*
5e195bbddabdd9 Corentin Labbe 2020-09-25  1202  	 * even though we make this a non pointer and thus
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1203  	 * theoretically allow for making changes to this struct
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1204  	 * on a per-individual card basis at runtime, this is
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1205  	 * strongly discouraged. This structure is intended to
5e195bbddabdd9 Corentin Labbe 2020-09-25  1206  	 * keep general card information, no settings or anything
5e195bbddabdd9 Corentin Labbe 2020-09-25  1207  	 */
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1208  	zr->card = zoran_cards[card_num];
5e195bbddabdd9 Corentin Labbe 2020-09-25  1209  	snprintf(ZR_DEVNAME(zr), sizeof(ZR_DEVNAME(zr)), "%s[%u]",
5e195bbddabdd9 Corentin Labbe 2020-09-25  1210  		 zr->card.name, zr->id);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1211  
845556fd8027b8 Corentin Labbe 2020-09-25  1212  	err = pci_request_regions(pdev, ZR_DEVNAME(zr));
845556fd8027b8 Corentin Labbe 2020-09-25  1213  	if (err)
845556fd8027b8 Corentin Labbe 2020-09-25  1214  		goto zr_unreg;
845556fd8027b8 Corentin Labbe 2020-09-25  1215  
e83bf68b5827e0 Corentin Labbe 2020-09-25  1216  	zr->zr36057_mem = devm_ioremap(&pdev->dev, pci_resource_start(pdev, 0), pci_resource_len(pdev, 0));
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1217  	if (!zr->zr36057_mem) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25  1218  		pci_err(pdev, "%s() - ioremap failed\n", __func__);
845556fd8027b8 Corentin Labbe 2020-09-25  1219  		goto zr_pci_release;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1220  	}
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1221  
ce72671d5d2d93 Corentin Labbe 2020-09-25  1222  	result = pci_request_irq(pdev, 0, zoran_irq, NULL, zr, ZR_DEVNAME(zr));
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1223  	if (result < 0) {
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1224  		if (result == -EINVAL) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25  1225  			pci_err(pdev, "%s - bad IRQ number or handler\n", __func__);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1226  		} else if (result == -EBUSY) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25  1227  			pci_err(pdev, "%s - IRQ %d busy, change your PnP config in BIOS\n",
b7c3b2bb9db412 Corentin Labbe 2020-09-25  1228  				__func__, zr->pci_dev->irq);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1229  		} else {
b7c3b2bb9db412 Corentin Labbe 2020-09-25  1230  			pci_err(pdev, "%s - cannot assign IRQ, error code %d\n", __func__, result);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1231  		}
e83bf68b5827e0 Corentin Labbe 2020-09-25  1232  		goto zr_pci_release;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1233  	}
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1234  
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1235  	/* set PCI latency timer */
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1236  	pci_read_config_byte(zr->pci_dev, PCI_LATENCY_TIMER,
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1237  			     &latency);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1238  	need_latency = zr->revision > 1 ? 32 : 48;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1239  	if (latency != need_latency) {
9bb2720293a04f Corentin Labbe 2020-09-25  1240  		pci_info(zr->pci_dev, "Changing PCI latency from %d to %d\n", latency, need_latency);
5e195bbddabdd9 Corentin Labbe 2020-09-25  1241  		pci_write_config_byte(zr->pci_dev, PCI_LATENCY_TIMER, need_latency);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1242  	}
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1243  
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1244  	zr36057_restart(zr);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1245  	/* i2c */
9bb2720293a04f Corentin Labbe 2020-09-25  1246  	pci_info(zr->pci_dev, "Initializing i2c bus...\n");
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1247  
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1248  	if (zoran_register_i2c(zr) < 0) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25  1249  		pci_err(pdev, "%s - can't initialize i2c bus\n", __func__);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1250  		goto zr_free_irq;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1251  	}
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1252  
5e195bbddabdd9 Corentin Labbe 2020-09-25  1253  	zr->decoder = v4l2_i2c_new_subdev(&zr->v4l2_dev, &zr->i2c_adapter,
5e195bbddabdd9 Corentin Labbe 2020-09-25  1254  					  zr->card.i2c_decoder, 0,
5e195bbddabdd9 Corentin Labbe 2020-09-25  1255  					  zr->card.addrs_decoder);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1256  
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1257  	if (zr->card.i2c_encoder)
5e195bbddabdd9 Corentin Labbe 2020-09-25  1258  		zr->encoder = v4l2_i2c_new_subdev(&zr->v4l2_dev, &zr->i2c_adapter,
5e195bbddabdd9 Corentin Labbe 2020-09-25  1259  						  zr->card.i2c_encoder, 0,
5e195bbddabdd9 Corentin Labbe 2020-09-25  1260  						  zr->card.addrs_encoder);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1261  
9bb2720293a04f Corentin Labbe 2020-09-25  1262  	pci_info(zr->pci_dev, "Initializing videocodec bus...\n");
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1263  
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1264  	if (zr->card.video_codec) {
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1265  		codec_name = codecid_to_modulename(zr->card.video_codec);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1266  		if (codec_name) {
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1267  			result = request_module(codec_name);
b7c3b2bb9db412 Corentin Labbe 2020-09-25  1268  			if (result)
b7c3b2bb9db412 Corentin Labbe 2020-09-25  1269  				pci_err(pdev, "failed to load modules %s: %d\n", codec_name, result);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1270  		}
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1271  	}
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1272  	if (zr->card.video_vfe) {
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1273  		vfe_name = codecid_to_modulename(zr->card.video_vfe);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1274  		if (vfe_name) {
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1275  			result = request_module(vfe_name);
b7c3b2bb9db412 Corentin Labbe 2020-09-25  1276  			if (result < 0)
b7c3b2bb9db412 Corentin Labbe 2020-09-25  1277  				pci_err(pdev, "failed to load modules %s: %d\n", vfe_name, result);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1278  		}
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1279  	}
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1280  
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1281  	/* reset JPEG codec */
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1282  	jpeg_codec_sleep(zr, 1);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1283  	jpeg_codec_reset(zr);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1284  	/* video bus enabled */
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1285  	/* display codec revision */
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1286  	if (zr->card.video_codec != 0) {
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1287  		master_codec = zoran_setup_videocodec(zr, zr->card.video_codec);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1288  		if (!master_codec)
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1289  			goto zr_unreg_i2c;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1290  		zr->codec = videocodec_attach(master_codec);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1291  		if (!zr->codec) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25  1292  			pci_err(pdev, "%s - no codec found\n", __func__);
4bae5db2f28d64 Corentin Labbe 2020-09-25  1293  			goto zr_unreg_i2c;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1294  		}
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1295  		if (zr->codec->type != zr->card.video_codec) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25  1296  			pci_err(pdev, "%s - wrong codec\n", __func__);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1297  			goto zr_detach_codec;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1298  		}
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1299  	}
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1300  	if (zr->card.video_vfe != 0) {
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1301  		master_vfe = zoran_setup_videocodec(zr, zr->card.video_vfe);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1302  		if (!master_vfe)
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1303  			goto zr_detach_codec;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1304  		zr->vfe = videocodec_attach(master_vfe);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1305  		if (!zr->vfe) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25  1306  			pci_err(pdev, "%s - no VFE found\n", __func__);
4bae5db2f28d64 Corentin Labbe 2020-09-25  1307  			goto zr_detach_codec;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1308  		}
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1309  		if (zr->vfe->type != zr->card.video_vfe) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25  1310  			pci_err(pdev, "%s = wrong VFE\n", __func__);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1311  			goto zr_detach_vfe;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1312  		}
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1313  	}
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1314  
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1315  	/* take care of Natoma chipset and a revision 1 zr36057 */
83f89a8bcbc3c5 Corentin Labbe 2020-09-25  1316  	if ((pci_pci_problems & PCIPCI_NATOMA) && zr->revision <= 1)
9bb2720293a04f Corentin Labbe 2020-09-25  1317  		pci_info(zr->pci_dev, "ZR36057/Natoma bug, max. buffer size is 128K\n");
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1318  
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1319  	if (zr36057_init(zr) < 0)
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1320  		goto zr_detach_vfe;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25  1321  
b564cb6e0bd587 Corentin Labbe 2020-09-25  1322  	zr->map_mode = ZORAN_MAP_MODE_RAW;
b564cb6e0bd587 Corentin Labbe 2020-09-25  1323  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 40477 bytes --]

[PATCH v2 05/10] staging: media: zoran: videocode: remove procfs

[Corentin Labbe]

Now we have a debugfs, we can remove all PROCFS stuff.
We keep videocodec_debugfs_show(), it will be used later

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 drivers/staging/media/zoran/videocodec.c | 24 ++----------------------
 drivers/staging/media/zoran/videocodec.h |  5 +++++
 2 files changed, 7 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/media/zoran/videocodec.c b/drivers/staging/media/zoran/videocodec.c
index 31019b5f377e..3d5a83a07e07 100644
--- a/drivers/staging/media/zoran/videocodec.c
+++ b/drivers/staging/media/zoran/videocodec.c
@@ -16,14 +16,6 @@
 #include <linux/types.h>
 #include <linux/slab.h>
 
-// kernel config is here (procfs flag)
-
-#ifdef CONFIG_PROC_FS
-#include <linux/proc_fs.h>
-#include <linux/seq_file.h>
-#include <linux/uaccess.h>
-#endif
-
 #include "videocodec.h"
 
 static int videocodec_debug;
@@ -265,8 +257,8 @@ int videocodec_unregister(const struct videocodec *codec)
 }
 EXPORT_SYMBOL(videocodec_unregister);
 
-#ifdef CONFIG_PROC_FS
-static int proc_videocodecs_show(struct seq_file *m, void *v)
+#ifdef CONFIG_VIDEO_ZORAN_DEBUG
+int videocodec_debugfs_show(struct seq_file *m)
 {
 	struct codec_list *h = codeclist_top;
 	struct attached_list *a;
@@ -300,25 +292,13 @@ static int proc_videocodecs_show(struct seq_file *m, void *v)
 /* ===================== */
 static int __init videocodec_init(void)
 {
-#ifdef CONFIG_PROC_FS
-	static struct proc_dir_entry *videocodec_proc_entry;
-#endif
-
 	pr_info("Linux video codec intermediate layer: %s\n", VIDEOCODEC_VERSION);
 
-#ifdef CONFIG_PROC_FS
-	videocodec_proc_entry = proc_create_single("videocodecs", 0, NULL, proc_videocodecs_show);
-	if (!videocodec_proc_entry)
-		pr_err("videocodec: can't init procfs.\n");
-#endif
 	return 0;
 }
 
 static void __exit videocodec_exit(void)
 {
-#ifdef CONFIG_PROC_FS
-	remove_proc_entry("videocodecs", NULL);
-#endif
 }
 
 module_init(videocodec_init);
diff --git a/drivers/staging/media/zoran/videocodec.h b/drivers/staging/media/zoran/videocodec.h
index 8a5003dda9f4..f2e17566795e 100644
--- a/drivers/staging/media/zoran/videocodec.h
+++ b/drivers/staging/media/zoran/videocodec.h
@@ -123,6 +123,7 @@ M                       zr36055[1] 0001 0000c001 00000000 (zr36050[1])
 #ifndef __LINUX_VIDEOCODEC_H
 #define __LINUX_VIDEOCODEC_H
 
+#include <linux/debugfs.h>
 #include <linux/videodev2.h>
 
 #define CODEC_DO_COMPRESSION 0
@@ -305,4 +306,8 @@ extern int videocodec_unregister(const struct videocodec *);
 
 /* the other calls are directly done via the videocodec structure! */
 
+#ifdef CONFIG_VIDEO_ZORAN_DEBUG
+int videocodec_debugfs_show(struct seq_file *m);
+#endif
+
 #endif				/*ifndef __LINUX_VIDEOCODEC_H */
-- 
2.32.0

[PATCH v2 06/10] staging: media: zoran: fusion all modules

[Corentin Labbe]

The zoran driver is split in many modules, but this lead to some
problems.
One of them is that load order is incorrect when everything is built-in.

Having more than one module is useless, so fusion all zoran modules in
one.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 drivers/staging/media/zoran/Kconfig      | 14 +++----
 drivers/staging/media/zoran/Makefile     |  8 ++--
 drivers/staging/media/zoran/videocodec.c | 36 +-----------------
 drivers/staging/media/zoran/videocodec.h |  1 -
 drivers/staging/media/zoran/zoran_card.c | 48 +++++++++++++++++++++---
 drivers/staging/media/zoran/zr36016.c    | 12 +-----
 drivers/staging/media/zoran/zr36016.h    |  2 +
 drivers/staging/media/zoran/zr36050.c    | 13 +------
 drivers/staging/media/zoran/zr36050.h    |  2 +
 drivers/staging/media/zoran/zr36060.c    | 12 +-----
 drivers/staging/media/zoran/zr36060.h    |  2 +
 11 files changed, 67 insertions(+), 83 deletions(-)

diff --git a/drivers/staging/media/zoran/Kconfig b/drivers/staging/media/zoran/Kconfig
index 06f79b91cda7..b5a3fc6e98f6 100644
--- a/drivers/staging/media/zoran/Kconfig
+++ b/drivers/staging/media/zoran/Kconfig
@@ -14,7 +14,7 @@ config VIDEO_ZORAN
 	  module will be called zr36067.
 
 config VIDEO_ZORAN_DC30
-	tristate "Pinnacle/Miro DC30(+) support"
+	bool "Pinnacle/Miro DC30(+) support"
 	depends on VIDEO_ZORAN
 	select VIDEO_ADV7175 if MEDIA_SUBDRV_AUTOSELECT
 	select VIDEO_VPX3220 if MEDIA_SUBDRV_AUTOSELECT
@@ -24,7 +24,7 @@ config VIDEO_ZORAN_DC30
 	  zr36050 MJPEG codec and zr36016 VFE.
 
 config VIDEO_ZORAN_ZR36060
-	tristate "Zoran ZR36060"
+	bool "Zoran ZR36060"
 	depends on VIDEO_ZORAN
 	help
 	  Say Y to support Zoran boards based on 36060 chips.
@@ -32,7 +32,7 @@ config VIDEO_ZORAN_ZR36060
 	  and 33 R10 and AverMedia 6 boards.
 
 config VIDEO_ZORAN_BUZ
-	tristate "Iomega Buz support"
+	bool "Iomega Buz support"
 	depends on VIDEO_ZORAN_ZR36060
 	select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECT
 	select VIDEO_SAA7185 if MEDIA_SUBDRV_AUTOSELECT
@@ -40,7 +40,7 @@ config VIDEO_ZORAN_BUZ
 	  Support for the Iomega Buz MJPEG capture/playback card.
 
 config VIDEO_ZORAN_DC10
-	tristate "Pinnacle/Miro DC10(+) support"
+	bool "Pinnacle/Miro DC10(+) support"
 	depends on VIDEO_ZORAN_ZR36060
 	select VIDEO_SAA7110 if MEDIA_SUBDRV_AUTOSELECT
 	select VIDEO_ADV7175 if MEDIA_SUBDRV_AUTOSELECT
@@ -49,7 +49,7 @@ config VIDEO_ZORAN_DC10
 	  card.
 
 config VIDEO_ZORAN_LML33
-	tristate "Linux Media Labs LML33 support"
+	bool "Linux Media Labs LML33 support"
 	depends on VIDEO_ZORAN_ZR36060
 	select VIDEO_BT819 if MEDIA_SUBDRV_AUTOSELECT
 	select VIDEO_BT856 if MEDIA_SUBDRV_AUTOSELECT
@@ -58,7 +58,7 @@ config VIDEO_ZORAN_LML33
 	  card.
 
 config VIDEO_ZORAN_LML33R10
-	tristate "Linux Media Labs LML33R10 support"
+	bool "Linux Media Labs LML33R10 support"
 	depends on VIDEO_ZORAN_ZR36060
 	select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECT
 	select VIDEO_ADV7170 if MEDIA_SUBDRV_AUTOSELECT
@@ -67,7 +67,7 @@ config VIDEO_ZORAN_LML33R10
 	  card.
 
 config VIDEO_ZORAN_AVS6EYES
-	tristate "AverMedia 6 Eyes support"
+	bool "AverMedia 6 Eyes support"
 	depends on VIDEO_ZORAN_ZR36060
 	select VIDEO_BT856 if MEDIA_SUBDRV_AUTOSELECT
 	select VIDEO_BT866 if MEDIA_SUBDRV_AUTOSELECT
diff --git a/drivers/staging/media/zoran/Makefile b/drivers/staging/media/zoran/Makefile
index 7023158e3892..9603bac0195c 100644
--- a/drivers/staging/media/zoran/Makefile
+++ b/drivers/staging/media/zoran/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 zr36067-objs	:=	zoran_device.o \
-			zoran_driver.o zoran_card.o
+			zoran_driver.o zoran_card.o videocodec.o
 
-obj-$(CONFIG_VIDEO_ZORAN) += zr36067.o videocodec.o
-obj-$(CONFIG_VIDEO_ZORAN_DC30) += zr36050.o zr36016.o
-obj-$(CONFIG_VIDEO_ZORAN_ZR36060) += zr36060.o
+obj-$(CONFIG_VIDEO_ZORAN) += zr36067.o
+zr36067-$(CONFIG_VIDEO_ZORAN_DC30) += zr36050.o zr36016.o
+zr36067-$(CONFIG_VIDEO_ZORAN_ZR36060) += zr36060.o
diff --git a/drivers/staging/media/zoran/videocodec.c b/drivers/staging/media/zoran/videocodec.c
index 3d5a83a07e07..7390fddd0279 100644
--- a/drivers/staging/media/zoran/videocodec.c
+++ b/drivers/staging/media/zoran/videocodec.c
@@ -8,8 +8,6 @@
  * (c) 2002 Wolfgang Scherr <scherr@net4you.at>
  */
 
-#define VIDEOCODEC_VERSION "v0.2"
-
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -72,12 +70,9 @@ struct videocodec *videocodec_attach(struct videocodec_master *master)
 		if ((master->flags & h->codec->flags) == master->flags) {
 			dprintk(4, "%s: try '%s'\n", __func__, h->codec->name);
 
-			if (!try_module_get(h->codec->owner))
-				return NULL;
-
 			codec = kmemdup(h->codec, sizeof(struct videocodec), GFP_KERNEL);
 			if (!codec)
-				goto out_module_put;
+				goto out_kfree;
 
 			res = strlen(codec->name);
 			snprintf(codec->name + res, sizeof(codec->name) - res, "[%d]", h->attached);
@@ -113,13 +108,10 @@ struct videocodec *videocodec_attach(struct videocodec_master *master)
 	pr_err("%s: no codec found!\n", __func__);
 	return NULL;
 
- out_module_put:
-	module_put(h->codec->owner);
  out_kfree:
 	kfree(codec);
 	return NULL;
 }
-EXPORT_SYMBOL(videocodec_attach);
 
 int videocodec_detach(struct videocodec *codec)
 {
@@ -160,7 +152,6 @@ int videocodec_detach(struct videocodec *codec)
 					prev->next = a->next;
 					dprintk(4, "videocodec: delete middle\n");
 				}
-				module_put(a->codec->owner);
 				kfree(a->codec);
 				kfree(a);
 				h->attached -= 1;
@@ -175,7 +166,6 @@ int videocodec_detach(struct videocodec *codec)
 	pr_err("%s: given codec not found!\n", __func__);
 	return -EINVAL;
 }
-EXPORT_SYMBOL(videocodec_detach);
 
 int videocodec_register(const struct videocodec *codec)
 {
@@ -208,7 +198,6 @@ int videocodec_register(const struct videocodec *codec)
 
 	return 0;
 }
-EXPORT_SYMBOL(videocodec_register);
 
 int videocodec_unregister(const struct videocodec *codec)
 {
@@ -255,7 +244,6 @@ int videocodec_unregister(const struct videocodec *codec)
 	pr_err("%s: given codec not found!\n", __func__);
 	return -EINVAL;
 }
-EXPORT_SYMBOL(videocodec_unregister);
 
 #ifdef CONFIG_VIDEO_ZORAN_DEBUG
 int videocodec_debugfs_show(struct seq_file *m)
@@ -286,25 +274,3 @@ int videocodec_debugfs_show(struct seq_file *m)
 	return 0;
 }
 #endif
-
-/* ===================== */
-/* hook in driver module */
-/* ===================== */
-static int __init videocodec_init(void)
-{
-	pr_info("Linux video codec intermediate layer: %s\n", VIDEOCODEC_VERSION);
-
-	return 0;
-}
-
-static void __exit videocodec_exit(void)
-{
-}
-
-module_init(videocodec_init);
-module_exit(videocodec_exit);
-
-MODULE_AUTHOR("Wolfgang Scherr <scherr@net4you.at>");
-MODULE_DESCRIPTION("Intermediate API module for video codecs "
-		   VIDEOCODEC_VERSION);
-MODULE_LICENSE("GPL");
diff --git a/drivers/staging/media/zoran/videocodec.h b/drivers/staging/media/zoran/videocodec.h
index f2e17566795e..7d0575554cca 100644
--- a/drivers/staging/media/zoran/videocodec.h
+++ b/drivers/staging/media/zoran/videocodec.h
@@ -234,7 +234,6 @@ struct jpeg_app_marker {
 };
 
 struct videocodec {
-	struct module *owner;
 	/* -- filled in by slave device during register -- */
 	char name[32];
 	unsigned long magic;	/* may be used for client<->master attaching */
diff --git a/drivers/staging/media/zoran/zoran_card.c b/drivers/staging/media/zoran/zoran_card.c
index 6f29986a3fc2..a0739d3462bb 100644
--- a/drivers/staging/media/zoran/zoran_card.c
+++ b/drivers/staging/media/zoran/zoran_card.c
@@ -29,6 +29,9 @@
 #include "zoran.h"
 #include "zoran_card.h"
 #include "zoran_device.h"
+#include "zr36016.h"
+#include "zr36050.h"
+#include "zr36060.h"
 
 extern const struct zoran_format zoran_formats[];
 
@@ -266,6 +269,39 @@ static const char *codecid_to_modulename(u16 codecid)
 	return name;
 }
 
+static int load_codec(struct zoran *zr, u16 codecid)
+{
+	switch (codecid) {
+	case CODEC_TYPE_ZR36060:
+#ifdef CONFIG_VIDEO_ZORAN_ZR36060
+		return zr36060_init_module();
+#else
+		pci_err(zr->pci_dev, "ZR36060 support is not enabled\n");
+		return -EINVAL;
+#endif
+		break;
+	case CODEC_TYPE_ZR36050:
+#ifdef CONFIG_VIDEO_ZORAN_DC30
+		return zr36050_init_module();
+#else
+		pci_err(zr->pci_dev, "ZR36050 support is not enabled\n");
+		return -EINVAL;
+#endif
+		break;
+	case CODEC_TYPE_ZR36016:
+#ifdef CONFIG_VIDEO_ZORAN_DC30
+		return zr36016_init_module();
+#else
+		pci_err(zr->pci_dev, "ZR36016 support is not enabled\n");
+		return -EINVAL;
+#endif
+		break;
+	}
+
+	pci_err(zr->pci_dev, "unknown codec id %x\n", codecid);
+	return -EINVAL;
+}
+
 // struct tvnorm {
 //      u16 wt, wa, h_start, h_sync_start, ht, ha, v_start;
 // };
@@ -1080,6 +1116,8 @@ static int zoran_debugfs_show(struct seq_file *seq, void *v)
 
 	seq_printf(seq, "Prepared %u\n", zr->prepared);
 	seq_printf(seq, "Queued %u\n", zr->queued);
+
+	videocodec_debugfs_show(seq);
 	return 0;
 }
 
@@ -1264,17 +1302,17 @@ static int zoran_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (zr->card.video_codec) {
 		codec_name = codecid_to_modulename(zr->card.video_codec);
 		if (codec_name) {
-			result = request_module(codec_name);
-			if (result)
-				pci_err(pdev, "failed to load modules %s: %d\n", codec_name, result);
+			result = load_codec(zr, zr->card.video_codec);
+			if (result < 0)
+				pci_err(pdev, "failed to load codec %s: %d\n", codec_name, result);
 		}
 	}
 	if (zr->card.video_vfe) {
 		vfe_name = codecid_to_modulename(zr->card.video_vfe);
 		if (vfe_name) {
-			result = request_module(vfe_name);
+			result = load_codec(zr, zr->card.video_vfe);
 			if (result < 0)
-				pci_err(pdev, "failed to load modules %s: %d\n", vfe_name, result);
+				pci_err(pdev, "failed to load codec %s: %d\n", vfe_name, result);
 		}
 	}
 
diff --git a/drivers/staging/media/zoran/zr36016.c b/drivers/staging/media/zoran/zr36016.c
index 50605460a44b..e3fda82fa479 100644
--- a/drivers/staging/media/zoran/zr36016.c
+++ b/drivers/staging/media/zoran/zr36016.c
@@ -390,7 +390,6 @@ static int zr36016_setup(struct videocodec *codec)
 }
 
 static const struct videocodec zr36016_codec = {
-	.owner = THIS_MODULE,
 	.name = "zr36016",
 	.magic = 0L,		/* magic not used */
 	.flags =
@@ -409,14 +408,14 @@ static const struct videocodec zr36016_codec = {
    HOOK IN DRIVER AS KERNEL MODULE
    ========================================================================= */
 
-static int __init zr36016_init_module(void)
+int zr36016_init_module(void)
 {
 	//dprintk(1, "ZR36016 driver %s\n",ZR016_VERSION);
 	zr36016_codecs = 0;
 	return videocodec_register(&zr36016_codec);
 }
 
-static void __exit zr36016_cleanup_module(void)
+void zr36016_cleanup_module(void)
 {
 	if (zr36016_codecs) {
 		dprintk(1,
@@ -425,10 +424,3 @@ static void __exit zr36016_cleanup_module(void)
 	}
 	videocodec_unregister(&zr36016_codec);
 }
-
-module_init(zr36016_init_module);
-module_exit(zr36016_cleanup_module);
-
-MODULE_AUTHOR("Wolfgang Scherr <scherr@net4you.at>");
-MODULE_DESCRIPTION("Driver module for ZR36016 video frontends");
-MODULE_LICENSE("GPL");
diff --git a/drivers/staging/media/zoran/zr36016.h b/drivers/staging/media/zoran/zr36016.h
index 1475f971cc24..04afba35669d 100644
--- a/drivers/staging/media/zoran/zr36016.h
+++ b/drivers/staging/media/zoran/zr36016.h
@@ -89,4 +89,6 @@ struct zr36016 {
 #define ZR016_SIGN           0x02
 #define ZR016_YMCS           0x01
 
+int zr36016_init_module(void);
+void zr36016_cleanup_module(void);
 #endif				/*fndef ZR36016_H */
diff --git a/drivers/staging/media/zoran/zr36050.c b/drivers/staging/media/zoran/zr36050.c
index 4dc7927fefc3..45d0eda69c7f 100644
--- a/drivers/staging/media/zoran/zr36050.c
+++ b/drivers/staging/media/zoran/zr36050.c
@@ -798,7 +798,6 @@ static int zr36050_setup(struct videocodec *codec)
 }
 
 static const struct videocodec zr36050_codec = {
-	.owner = THIS_MODULE,
 	.name = "zr36050",
 	.magic = 0L,		// magic not used
 	.flags =
@@ -817,14 +816,14 @@ static const struct videocodec zr36050_codec = {
    HOOK IN DRIVER AS KERNEL MODULE
    ========================================================================= */
 
-static int __init zr36050_init_module(void)
+int zr36050_init_module(void)
 {
 	//dprintk(1, "ZR36050 driver %s\n",ZR050_VERSION);
 	zr36050_codecs = 0;
 	return videocodec_register(&zr36050_codec);
 }
 
-static void __exit zr36050_cleanup_module(void)
+void zr36050_cleanup_module(void)
 {
 	if (zr36050_codecs) {
 		dprintk(1,
@@ -833,11 +832,3 @@ static void __exit zr36050_cleanup_module(void)
 	}
 	videocodec_unregister(&zr36050_codec);
 }
-
-module_init(zr36050_init_module);
-module_exit(zr36050_cleanup_module);
-
-MODULE_AUTHOR("Wolfgang Scherr <scherr@net4you.at>");
-MODULE_DESCRIPTION("Driver module for ZR36050 jpeg processors "
-		   ZR050_VERSION);
-MODULE_LICENSE("GPL");
diff --git a/drivers/staging/media/zoran/zr36050.h b/drivers/staging/media/zoran/zr36050.h
index 8f972d045b58..f9b58f4c77b9 100644
--- a/drivers/staging/media/zoran/zr36050.h
+++ b/drivers/staging/media/zoran/zr36050.h
@@ -160,4 +160,6 @@ struct zr36050 {
 #define ZR050_U_COMPONENT         1
 #define ZR050_V_COMPONENT         2
 
+int zr36050_init_module(void);
+void zr36050_cleanup_module(void);
 #endif				/*fndef ZR36050_H */
diff --git a/drivers/staging/media/zoran/zr36060.c b/drivers/staging/media/zoran/zr36060.c
index 7904d5b1f402..afbb5624e98f 100644
--- a/drivers/staging/media/zoran/zr36060.c
+++ b/drivers/staging/media/zoran/zr36060.c
@@ -831,7 +831,6 @@ static int zr36060_setup(struct videocodec *codec)
 }
 
 static const struct videocodec zr36060_codec = {
-	.owner = THIS_MODULE,
 	.name = "zr36060",
 	.magic = 0L,		// magic not used
 	.flags =
@@ -846,13 +845,13 @@ static const struct videocodec zr36060_codec = {
 	// others are not used
 };
 
-static int __init zr36060_init_module(void)
+int zr36060_init_module(void)
 {
 	zr36060_codecs = 0;
 	return videocodec_register(&zr36060_codec);
 }
 
-static void __exit zr36060_cleanup_module(void)
+void zr36060_cleanup_module(void)
 {
 	if (zr36060_codecs) {
 		dprintk(1,
@@ -863,10 +862,3 @@ static void __exit zr36060_cleanup_module(void)
 	/* however, we can't just stay alive */
 	videocodec_unregister(&zr36060_codec);
 }
-
-module_init(zr36060_init_module);
-module_exit(zr36060_cleanup_module);
-
-MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@skynet.be>");
-MODULE_DESCRIPTION("Driver module for ZR36060 jpeg processors " ZR060_VERSION);
-MODULE_LICENSE("GPL");
diff --git a/drivers/staging/media/zoran/zr36060.h b/drivers/staging/media/zoran/zr36060.h
index d2cdc26bf625..fbf5429534ac 100644
--- a/drivers/staging/media/zoran/zr36060.h
+++ b/drivers/staging/media/zoran/zr36060.h
@@ -198,4 +198,6 @@ struct zr36060 {
 #define ZR060_SR_H_SCALE2		 BIT(0)
 #define ZR060_SR_H_SCALE4		(2 << 0)
 
+int zr36060_init_module(void);
+void zr36060_cleanup_module(void);
 #endif				/*fndef ZR36060_H */
-- 
2.32.0

Re: [PATCH v2 06/10] staging: media: zoran: fusion all modules

[Dan Carpenter]

On Wed, Oct 13, 2021 at 06:58:08PM +0000, Corentin Labbe wrote:
> +static int load_codec(struct zoran *zr, u16 codecid)
> +{
> +	switch (codecid) {
> +	case CODEC_TYPE_ZR36060:
> +#ifdef CONFIG_VIDEO_ZORAN_ZR36060
> +		return zr36060_init_module();
> +#else
> +		pci_err(zr->pci_dev, "ZR36060 support is not enabled\n");
> +		return -EINVAL;
> +#endif
> +		break;
> +	case CODEC_TYPE_ZR36050:
> +#ifdef CONFIG_VIDEO_ZORAN_DC30
> +		return zr36050_init_module();
> +#else
> +		pci_err(zr->pci_dev, "ZR36050 support is not enabled\n");
> +		return -EINVAL;
> +#endif
> +		break;
> +	case CODEC_TYPE_ZR36016:
> +#ifdef CONFIG_VIDEO_ZORAN_DC30
> +		return zr36016_init_module();
> +#else
> +		pci_err(zr->pci_dev, "ZR36016 support is not enabled\n");
> +		return -EINVAL;
> +#endif

The caller already prints an error message.  Can you look through the
dmesg and make sure were not printing a bunch of duplicate stuff?  Also
if load_codec() fails, the probe function still does
zoran_setup_videocodec() on the failed codec.

These would be better in a .h file.

#ifdef CONFIG_VIDEO_ZORAN_ZR36060
int zr36060_init_module(void);
#else
int zr36060_init_module(void) { return -EINVAL; }
#endif

regards,
dan carpenter

Re: [PATCH v2 06/10] staging: media: zoran: fusion all modules

[Dan Carpenter]

On Wed, Oct 13, 2021 at 06:58:08PM +0000, Corentin Labbe wrote:
> The zoran driver is split in many modules, but this lead to some
> problems.
> One of them is that load order is incorrect when everything is built-in.
> 
> Having more than one module is useless, so fusion all zoran modules in
                                             ^^^^^^
"fusion" isn't the right word.  It should be "fuse" or even better
"merge".  Same in the subject.

> one.
> 
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>

[ snip ]

> +static int load_codec(struct zoran *zr, u16 codecid)
> +{
> +	switch (codecid) {
> +	case CODEC_TYPE_ZR36060:
> +#ifdef CONFIG_VIDEO_ZORAN_ZR36060
> +		return zr36060_init_module();
> +#else
> +		pci_err(zr->pci_dev, "ZR36060 support is not enabled\n");
> +		return -EINVAL;
> +#endif
> +		break;
> +	case CODEC_TYPE_ZR36050:
> +#ifdef CONFIG_VIDEO_ZORAN_DC30
> +		return zr36050_init_module();
> +#else
> +		pci_err(zr->pci_dev, "ZR36050 support is not enabled\n");
> +		return -EINVAL;
> +#endif
> +		break;
> +	case CODEC_TYPE_ZR36016:
> +#ifdef CONFIG_VIDEO_ZORAN_DC30
> +		return zr36016_init_module();
> +#else
> +		pci_err(zr->pci_dev, "ZR36016 support is not enabled\n");
> +		return -EINVAL;
> +#endif
> +		break;
> +	}

Not related to your patch but if load_codec() fails, the probe function
still calls zoran_setup_videocodec() on the failed codec.  It might be
better to set the codec to zero?

		result = load_codec(zr, zr->card.video_codec);
		if (result < 0) {
			pci_err(pdev, "failed to load codec %s: %d\n", codec_name, result);
			zr->card.video_codec = 0;
		}

regards,
dan carpenter

Re: [PATCH v2 06/10] staging: media: zoran: fusion all modules

[LABBE Corentin]

Le Thu, Oct 14, 2021 at 11:01:55AM +0300, Dan Carpenter a écrit :
> On Wed, Oct 13, 2021 at 06:58:08PM +0000, Corentin Labbe wrote:
> > The zoran driver is split in many modules, but this lead to some
> > problems.
> > One of them is that load order is incorrect when everything is built-in.
> > 
> > Having more than one module is useless, so fusion all zoran modules in
>                                              ^^^^^^
> "fusion" isn't the right word.  It should be "fuse" or even better
> "merge".  Same in the subject.
> 

Hello

I will use merge, thanks for the suggestion.

> > +static int load_codec(struct zoran *zr, u16 codecid)
> > +{
> > +	switch (codecid) {
> > +	case CODEC_TYPE_ZR36060:
> > +#ifdef CONFIG_VIDEO_ZORAN_ZR36060
> > +		return zr36060_init_module();
> > +#else
> > +		pci_err(zr->pci_dev, "ZR36060 support is not enabled\n");
> > +		return -EINVAL;
> > +#endif
> > +		break;
> > +	case CODEC_TYPE_ZR36050:
> > +#ifdef CONFIG_VIDEO_ZORAN_DC30
> > +		return zr36050_init_module();
> > +#else
> > +		pci_err(zr->pci_dev, "ZR36050 support is not enabled\n");
> > +		return -EINVAL;
> > +#endif
> > +		break;
> > +	case CODEC_TYPE_ZR36016:
> > +#ifdef CONFIG_VIDEO_ZORAN_DC30
> > +		return zr36016_init_module();
> > +#else
> > +		pci_err(zr->pci_dev, "ZR36016 support is not enabled\n");
> > +		return -EINVAL;
> > +#endif
> > +		break;
> > +	}
> 
> Not related to your patch but if load_codec() fails, the probe function
> still calls zoran_setup_videocodec() on the failed codec.  It might be
> better to set the codec to zero?
> 
> 		result = load_codec(zr, zr->card.video_codec);
> 		if (result < 0) {
> 			pci_err(pdev, "failed to load codec %s: %d\n", codec_name, result);
> 			zr->card.video_codec = 0;
> 		}
> 

I will rework by adding a video_codec_init/exit, it will help tracking error (current behavour to ignore error is bad).
Furthermore, my patch forgot to add call to all "old module" exits, so dedicated function will be better.

Thanks for the review
Regards

[PATCH v2 07/10] staging: media: zoran: remove vidmem

[Corentin Labbe]

The vidmem parameter is no longer necessary since we removed framebuffer
support.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 drivers/staging/media/zoran/zoran_card.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/drivers/staging/media/zoran/zoran_card.c b/drivers/staging/media/zoran/zoran_card.c
index a0739d3462bb..a079fb6e27a3 100644
--- a/drivers/staging/media/zoran/zoran_card.c
+++ b/drivers/staging/media/zoran/zoran_card.c
@@ -39,17 +39,6 @@ static int card[BUZ_MAX] = { [0 ... (BUZ_MAX - 1)] = -1 };
 module_param_array(card, int, NULL, 0444);
 MODULE_PARM_DESC(card, "Card type");
 
-/*
- * The video mem address of the video card. The driver has a little database
- * for some videocards to determine it from there. If your video card is not
- * in there you have either to give it to the driver as a parameter or set
- * in a VIDIOCSFBUF ioctl
- */
-
-static unsigned long vidmem;	/* default = 0 - Video memory base address */
-module_param_hw(vidmem, ulong, iomem, 0444);
-MODULE_PARM_DESC(vidmem, "Default video memory base address");
-
 /* Default input and video norm at startup of the driver. */
 
 static unsigned int default_input;	/* default 0 = Composite, 1 = S-Video */
@@ -1163,10 +1152,6 @@ static int zoran_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		jpg_bufsize = 8192;
 	if (jpg_bufsize > (512 * 1024))
 		jpg_bufsize = 512 * 1024;
-	/* Use parameter for vidmem or try to find a video card */
-	if (vidmem)
-		pci_info(pdev, "%s: Using supplied video memory base address @ 0x%lx\n",
-			 ZORAN_NAME, vidmem);
 
 	/* some mainboards might not do PCI-PCI data transfer well */
 	if (pci_pci_problems & (PCIPCI_FAIL | PCIAGP_FAIL | PCIPCI_ALIMAGIK))
-- 
2.32.0

[PATCH v2 08/10] staging: media: zoran: move videodev alloc

[Corentin Labbe]

Move some code out of zr36057_init() and create new functions for handling zr->video_dev.
This permit to ease code reading and fix a zr->video_dev memory leak.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 drivers/staging/media/zoran/zoran.h        |  2 +-
 drivers/staging/media/zoran/zoran_card.c   | 80 ++++++++++++++--------
 drivers/staging/media/zoran/zoran_driver.c |  5 +-
 3 files changed, 54 insertions(+), 33 deletions(-)

diff --git a/drivers/staging/media/zoran/zoran.h b/drivers/staging/media/zoran/zoran.h
index c37d064ff11d..31bc577f5df5 100644
--- a/drivers/staging/media/zoran/zoran.h
+++ b/drivers/staging/media/zoran/zoran.h
@@ -317,6 +317,6 @@ static inline struct zoran *to_zoran(struct v4l2_device *v4l2_dev)
 
 #endif
 
-int zoran_queue_init(struct zoran *zr, struct vb2_queue *vq);
+int zoran_queue_init(struct zoran *zr, struct vb2_queue *vq, int dir);
 void zoran_queue_exit(struct zoran *zr);
 int zr_set_buf(struct zoran *zr);
diff --git a/drivers/staging/media/zoran/zoran_card.c b/drivers/staging/media/zoran/zoran_card.c
index a079fb6e27a3..9bc5af34d909 100644
--- a/drivers/staging/media/zoran/zoran_card.c
+++ b/drivers/staging/media/zoran/zoran_card.c
@@ -828,6 +828,52 @@ int zoran_check_jpg_settings(struct zoran *zr,
 	return 0;
 }
 
+static int zoran_init_video_device(struct zoran *zr, struct video_device *video_dev, int dir)
+{
+	int err;
+
+	/* Now add the template and register the device unit. */
+	*video_dev = zoran_template;
+	video_dev->v4l2_dev = &zr->v4l2_dev;
+	video_dev->lock = &zr->lock;
+	video_dev->device_caps = V4L2_CAP_STREAMING | dir;
+
+	strscpy(video_dev->name, ZR_DEVNAME(zr), sizeof(video_dev->name));
+	/*
+	 * It's not a mem2mem device, but you can both capture and output from one and the same
+	 * device. This should really be split up into two device nodes, but that's a job for
+	 * another day.
+	 */
+	video_dev->vfl_dir = VFL_DIR_M2M;
+	zoran_queue_init(zr, &zr->vq, V4L2_BUF_TYPE_VIDEO_CAPTURE);
+
+	err = video_register_device(video_dev, VFL_TYPE_VIDEO, video_nr[zr->id]);
+	if (err < 0)
+		return err;
+	video_set_drvdata(video_dev, zr);
+	return 0;
+}
+
+static void zoran_exit_video_devices(struct zoran *zr)
+{
+	video_unregister_device(zr->video_dev);
+	kfree(zr->video_dev);
+}
+
+static int zoran_init_video_devices(struct zoran *zr)
+{
+	int err;
+
+	zr->video_dev = video_device_alloc();
+	if (!zr->video_dev)
+		return -ENOMEM;
+
+	err = zoran_init_video_device(zr, zr->video_dev, V4L2_CAP_VIDEO_CAPTURE);
+	if (err)
+		kfree(zr->video_dev);
+	return err;
+}
+
 void zoran_open_init_params(struct zoran *zr)
 {
 	int i;
@@ -899,17 +945,11 @@ static int zr36057_init(struct zoran *zr)
 	zoran_open_init_params(zr);
 
 	/* allocate memory *before* doing anything to the hardware in case allocation fails */
-	zr->video_dev = video_device_alloc();
-	if (!zr->video_dev) {
-		err = -ENOMEM;
-		goto exit;
-	}
 	zr->stat_com = dma_alloc_coherent(&zr->pci_dev->dev,
 					  BUZ_NUM_STAT_COM * sizeof(u32),
 					  &zr->p_sc, GFP_KERNEL);
 	if (!zr->stat_com) {
-		err = -ENOMEM;
-		goto exit_video;
+		return -ENOMEM;
 	}
 	for (j = 0; j < BUZ_NUM_STAT_COM; j++)
 		zr->stat_com[j] = cpu_to_le32(1); /* mark as unavailable to zr36057 */
@@ -922,26 +962,9 @@ static int zr36057_init(struct zoran *zr)
 		goto exit_statcom;
 	}
 
-	/* Now add the template and register the device unit. */
-	*zr->video_dev = zoran_template;
-	zr->video_dev->v4l2_dev = &zr->v4l2_dev;
-	zr->video_dev->lock = &zr->lock;
-	zr->video_dev->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_CAPTURE;
-
-	strscpy(zr->video_dev->name, ZR_DEVNAME(zr), sizeof(zr->video_dev->name));
-	/*
-	 * It's not a mem2mem device, but you can both capture and output from one and the same
-	 * device. This should really be split up into two device nodes, but that's a job for
-	 * another day.
-	 */
-	zr->video_dev->vfl_dir = VFL_DIR_M2M;
-
-	zoran_queue_init(zr, &zr->vq);
-
-	err = video_register_device(zr->video_dev, VFL_TYPE_VIDEO, video_nr[zr->id]);
-	if (err < 0)
+	err = zoran_init_video_devices(zr);
+	if (err)
 		goto exit_statcomb;
-	video_set_drvdata(zr->video_dev, zr);
 
 	zoran_init_hardware(zr);
 	if (!pass_through) {
@@ -956,9 +979,6 @@ static int zr36057_init(struct zoran *zr)
 	dma_free_coherent(&zr->pci_dev->dev, BUZ_NUM_STAT_COM * sizeof(u32) * 2, zr->stat_comb, zr->p_scb);
 exit_statcom:
 	dma_free_coherent(&zr->pci_dev->dev, BUZ_NUM_STAT_COM * sizeof(u32), zr->stat_com, zr->p_sc);
-exit_video:
-	kfree(zr->video_dev);
-exit:
 	return err;
 }
 
@@ -992,7 +1012,7 @@ static void zoran_remove(struct pci_dev *pdev)
 	dma_free_coherent(&zr->pci_dev->dev, BUZ_NUM_STAT_COM * sizeof(u32) * 2, zr->stat_comb, zr->p_scb);
 	pci_release_regions(pdev);
 	pci_disable_device(zr->pci_dev);
-	video_unregister_device(zr->video_dev);
+	zoran_exit_video_devices(zr);
 exit_free:
 	v4l2_ctrl_handler_free(&zr->hdl);
 	v4l2_device_unregister(&zr->v4l2_dev);
diff --git a/drivers/staging/media/zoran/zoran_driver.c b/drivers/staging/media/zoran/zoran_driver.c
index 46382e43f1bf..551db338c7f7 100644
--- a/drivers/staging/media/zoran/zoran_driver.c
+++ b/drivers/staging/media/zoran/zoran_driver.c
@@ -1008,7 +1008,7 @@ static const struct vb2_ops zr_video_qops = {
 	.wait_finish            = vb2_ops_wait_finish,
 };
 
-int zoran_queue_init(struct zoran *zr, struct vb2_queue *vq)
+int zoran_queue_init(struct zoran *zr, struct vb2_queue *vq, int dir)
 {
 	int err;
 
@@ -1016,7 +1016,8 @@ int zoran_queue_init(struct zoran *zr, struct vb2_queue *vq)
 	INIT_LIST_HEAD(&zr->queued_bufs);
 
 	vq->dev = &zr->pci_dev->dev;
-	vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	vq->type = dir;
+
 	vq->io_modes = VB2_USERPTR | VB2_DMABUF | VB2_MMAP | VB2_READ | VB2_WRITE;
 	vq->drv_priv = zr;
 	vq->buf_struct_size = sizeof(struct zr_buffer);
-- 
2.32.0

[PATCH v2 09/10] staging: media: zoran: move config select on primary kconfig

[Corentin Labbe]

Since all kconfigs for card selection are bool, this causes all selected modules
to be always built-in.
Prevent this by moving selects to the main tristate kconfig.

By doing this, remove also all "if MEDIA_SUBDRV_AUTOSELECT" which are
wrong, since zoran always need them to work.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 drivers/staging/media/zoran/Kconfig | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/media/zoran/Kconfig b/drivers/staging/media/zoran/Kconfig
index b5a3fc6e98f6..0a9c1ab19016 100644
--- a/drivers/staging/media/zoran/Kconfig
+++ b/drivers/staging/media/zoran/Kconfig
@@ -3,6 +3,16 @@ config VIDEO_ZORAN
 	depends on PCI && I2C_ALGOBIT && VIDEO_V4L2
 	depends on !ALPHA
 	select VIDEOBUF2_DMA_CONTIG
+	select VIDEO_ADV7170 if VIDEO_ZORAN_LML33R10
+	select VIDEO_ADV7175 if VIDEO_ZORAN_DC10 || VIDEO_ZORAN_DC30
+	select VIDEO_BT819 if VIDEO_ZORAN_LML33
+	select VIDEO_BT856 if VIDEO_ZORAN_LML33 || VIDEO_ZORAN_AVS6EYES
+	select VIDEO_BT866 if VIDEO_ZORAN_AVS6EYES
+	select VIDEO_KS0127 if VIDEO_ZORAN_AVS6EYES
+	select VIDEO_SAA711X if VIDEO_ZORAN_BUZ || VIDEO_ZORAN_LML33R10
+	select VIDEO_SAA7110 if VIDEO_ZORAN_DC10
+	select VIDEO_SAA7185 if VIDEO_ZORAN_BUZ
+	select VIDEO_VPX3220 if VIDEO_ZORAN_DC30
 	help
 	  Say Y for support for MJPEG capture cards based on the Zoran
 	  36057/36067 PCI controller chipset. This includes the Iomega
@@ -16,8 +26,6 @@ config VIDEO_ZORAN
 config VIDEO_ZORAN_DC30
 	bool "Pinnacle/Miro DC30(+) support"
 	depends on VIDEO_ZORAN
-	select VIDEO_ADV7175 if MEDIA_SUBDRV_AUTOSELECT
-	select VIDEO_VPX3220 if MEDIA_SUBDRV_AUTOSELECT
 	help
 	  Support for the Pinnacle/Miro DC30(+) MJPEG capture/playback
 	  card. This also supports really old DC10 cards based on the
@@ -34,16 +42,12 @@ config VIDEO_ZORAN_ZR36060
 config VIDEO_ZORAN_BUZ
 	bool "Iomega Buz support"
 	depends on VIDEO_ZORAN_ZR36060
-	select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECT
-	select VIDEO_SAA7185 if MEDIA_SUBDRV_AUTOSELECT
 	help
 	  Support for the Iomega Buz MJPEG capture/playback card.
 
 config VIDEO_ZORAN_DC10
 	bool "Pinnacle/Miro DC10(+) support"
 	depends on VIDEO_ZORAN_ZR36060
-	select VIDEO_SAA7110 if MEDIA_SUBDRV_AUTOSELECT
-	select VIDEO_ADV7175 if MEDIA_SUBDRV_AUTOSELECT
 	help
 	  Support for the Pinnacle/Miro DC10(+) MJPEG capture/playback
 	  card.
@@ -51,8 +55,6 @@ config VIDEO_ZORAN_DC10
 config VIDEO_ZORAN_LML33
 	bool "Linux Media Labs LML33 support"
 	depends on VIDEO_ZORAN_ZR36060
-	select VIDEO_BT819 if MEDIA_SUBDRV_AUTOSELECT
-	select VIDEO_BT856 if MEDIA_SUBDRV_AUTOSELECT
 	help
 	  Support for the Linux Media Labs LML33 MJPEG capture/playback
 	  card.
@@ -60,8 +62,6 @@ config VIDEO_ZORAN_LML33
 config VIDEO_ZORAN_LML33R10
 	bool "Linux Media Labs LML33R10 support"
 	depends on VIDEO_ZORAN_ZR36060
-	select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECT
-	select VIDEO_ADV7170 if MEDIA_SUBDRV_AUTOSELECT
 	help
 	  support for the Linux Media Labs LML33R10 MJPEG capture/playback
 	  card.
@@ -69,9 +69,6 @@ config VIDEO_ZORAN_LML33R10
 config VIDEO_ZORAN_AVS6EYES
 	bool "AverMedia 6 Eyes support"
 	depends on VIDEO_ZORAN_ZR36060
-	select VIDEO_BT856 if MEDIA_SUBDRV_AUTOSELECT
-	select VIDEO_BT866 if MEDIA_SUBDRV_AUTOSELECT
-	select VIDEO_KS0127 if MEDIA_SUBDRV_AUTOSELECT
 	help
 	  Support for the AverMedia 6 Eyes video surveillance card.
 
-- 
2.32.0

[PATCH v2 10/10] staging: media: zoran: introduce zoran_i2c_init

[Corentin Labbe]

Reduces the size of the probe function by adding zoran_i2c_init/zoran_i2c_exit
functions.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 drivers/staging/media/zoran/zoran_card.c | 67 ++++++++++++++++++------
 1 file changed, 51 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/media/zoran/zoran_card.c b/drivers/staging/media/zoran/zoran_card.c
index 9bc5af34d909..fe4d867bf341 100644
--- a/drivers/staging/media/zoran/zoran_card.c
+++ b/drivers/staging/media/zoran/zoran_card.c
@@ -874,6 +874,53 @@ static int zoran_init_video_devices(struct zoran *zr)
 	return err;
 }
 
+/*
+ * v4l2_device_unregister() will care about removing zr->encoder/zr->decoder
+ * via v4l2_i2c_subdev_unregister()
+ */
+static int zoran_i2c_init(struct zoran *zr)
+{
+	int err;
+
+	pci_info(zr->pci_dev, "Initializing i2c bus...\n");
+
+	err = zoran_register_i2c(zr);
+	if (err) {
+		pci_err(zr->pci_dev, "%s - cannot initialize i2c bus\n", __func__);
+		return err;
+	}
+
+	zr->decoder = v4l2_i2c_new_subdev(&zr->v4l2_dev, &zr->i2c_adapter,
+					  zr->card.i2c_decoder, 0,
+					  zr->card.addrs_decoder);
+	if (!zr->decoder) {
+		pci_err(zr->pci_dev, "Fail to get decoder\n");
+		err = -EINVAL;
+		goto error_decoder;
+	}
+
+	if (zr->card.i2c_encoder) {
+		zr->encoder = v4l2_i2c_new_subdev(&zr->v4l2_dev, &zr->i2c_adapter,
+						  zr->card.i2c_encoder, 0,
+						  zr->card.addrs_encoder);
+		if (!zr->encoder) {
+			pci_err(zr->pci_dev, "Fail to get encoder\n");
+			err = -EINVAL;
+			goto error_decoder;
+		}
+	}
+	return 0;
+
+error_decoder:
+	zoran_unregister_i2c(zr);
+	return err;
+}
+
+static void zoran_i2c_exit(struct zoran *zr)
+{
+	zoran_unregister_i2c(zr);
+}
+
 void zoran_open_init_params(struct zoran *zr)
 {
 	int i;
@@ -1001,7 +1048,7 @@ static void zoran_remove(struct pci_dev *pdev)
 		videocodec_detach(zr->vfe);
 
 	/* unregister i2c bus */
-	zoran_unregister_i2c(zr);
+	zoran_i2c_exit(zr);
 	/* disable PCI bus-mastering */
 	zoran_set_pci_master(zr, 0);
 	/* put chip into reset */
@@ -1285,22 +1332,10 @@ static int zoran_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	}
 
 	zr36057_restart(zr);
-	/* i2c */
-	pci_info(zr->pci_dev, "Initializing i2c bus...\n");
 
-	if (zoran_register_i2c(zr) < 0) {
-		pci_err(pdev, "%s - can't initialize i2c bus\n", __func__);
+	err = zoran_i2c_init(zr);
+	if (err)
 		goto zr_free_irq;
-	}
-
-	zr->decoder = v4l2_i2c_new_subdev(&zr->v4l2_dev, &zr->i2c_adapter,
-					  zr->card.i2c_decoder, 0,
-					  zr->card.addrs_decoder);
-
-	if (zr->card.i2c_encoder)
-		zr->encoder = v4l2_i2c_new_subdev(&zr->v4l2_dev, &zr->i2c_adapter,
-						  zr->card.i2c_encoder, 0,
-						  zr->card.addrs_encoder);
 
 	pci_info(zr->pci_dev, "Initializing videocodec bus...\n");
 
@@ -1377,7 +1412,7 @@ static int zoran_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 zr_detach_codec:
 	videocodec_detach(zr->codec);
 zr_unreg_i2c:
-	zoran_unregister_i2c(zr);
+	zoran_i2c_exit(zr);
 zr_free_irq:
 	btwrite(0, ZR36057_SPGPPCR);
 	pci_free_irq(zr->pci_dev, 0, zr);
-- 
2.32.0

Re: [PATCH v2 00/10] staging: media: zoran: fusion in one module

[Hans Verkuil]

Hi Corentin,

I noticed some code review comments from Dan and a kernel test robot issue.
Can you post a v3 fixing those by the end of the week? Next week I will have
access again to my zoran board, so then I can test the v3 series.

BTW, I agree with Dan, just drop the 'Enable zoran debugfs' config option. It's
not worth the additional complexity. Instead, just #ifdef CONFIG_DEBUG_FS
where necessary (in most cases you shouldn't even have to do that since the
since you have dummy debug_fs_* functions if CONFIG_DEBUG_FS isn't set).

Regards,

	Hans

On 13/10/2021 20:58, Corentin Labbe wrote:
> Hello
> 
> The main change of this serie is to fusion all zoran related modules in
> one.
> This fixes the load order problem when everything is built-in.
> 
> Regards
> 
> Changes since v1:
> - add missing debugfs cleaning
> - clean some remaining module_get/put functions which made impossible to
>   remove the zoran module
> - added the two latest patchs
> 
> Corentin Labbe (10):
>   staging: media: zoran: move module parameter checks to zoran_probe
>   staging: media: zoran: use module_pci_driver
>   staging: media: zoran: rename debug module parameter
>   staging: media: zoran: add debugfs
>   staging: media: zoran: videocode: remove procfs
>   staging: media: zoran: fusion all modules
>   staging: media: zoran: remove vidmem
>   staging: media: zoran: move videodev alloc
>   staging: media: zoran: move config select on primary kconfig
>   staging: media: zoran: introduce zoran_i2c_init
> 
>  drivers/staging/media/zoran/Kconfig        |  46 +--
>  drivers/staging/media/zoran/Makefile       |   8 +-
>  drivers/staging/media/zoran/videocodec.c   |  68 +----
>  drivers/staging/media/zoran/videocodec.h   |   6 +-
>  drivers/staging/media/zoran/zoran.h        |   6 +-
>  drivers/staging/media/zoran/zoran_card.c   | 328 ++++++++++++++-------
>  drivers/staging/media/zoran/zoran_driver.c |   5 +-
>  drivers/staging/media/zoran/zr36016.c      |  24 +-
>  drivers/staging/media/zoran/zr36016.h      |   2 +
>  drivers/staging/media/zoran/zr36050.c      |  21 +-
>  drivers/staging/media/zoran/zr36050.h      |   2 +
>  drivers/staging/media/zoran/zr36060.c      |  21 +-
>  drivers/staging/media/zoran/zr36060.h      |   2 +
>  13 files changed, 291 insertions(+), 248 deletions(-)
> 

Re: [PATCH v2 00/10] staging: media: zoran: fusion in one module

[LABBE Corentin]

Le Mon, Oct 18, 2021 at 11:55:40AM +0200, Hans Verkuil a écrit :
> Hi Corentin,
> 
> I noticed some code review comments from Dan and a kernel test robot issue.
> Can you post a v3 fixing those by the end of the week? Next week I will have
> access again to my zoran board, so then I can test the v3 series.
> 
> BTW, I agree with Dan, just drop the 'Enable zoran debugfs' config option. It's
> not worth the additional complexity. Instead, just #ifdef CONFIG_DEBUG_FS
> where necessary (in most cases you shouldn't even have to do that since the
> since you have dummy debug_fs_* functions if CONFIG_DEBUG_FS isn't set).
> 

Hello

Ok I started fixing issues and will send V3 this week.

Regards

Re: [PATCH v2 00/10] staging: media: zoran: fusion in one module

[Hans Verkuil]

Hi Corentin,

On 13/10/2021 20:58, Corentin Labbe wrote:
> Hello
> 
> The main change of this serie is to fusion all zoran related modules in
> one.
> This fixes the load order problem when everything is built-in.
> 
> Regards
> 
> Changes since v1:
> - add missing debugfs cleaning
> - clean some remaining module_get/put functions which made impossible to
>   remove the zoran module
> - added the two latest patchs

Something weird is wrong with this series. I have a DC30, but loading this with:

modprobe zr36067 card=3

results in this error message in the kernel log:

[   58.645557] zr36067: module is from the staging directory, the quality is unknown, you have been warned.
[   58.646658] zr36067 0000:03:00.0: Zoran MJPEG board driver version 0.10.1
[   58.646793] zr36067 0000:03:00.0: Zoran ZR36057 (rev 1), irq: 18, memory: 0xf4000000
[   58.648821] zr36067 0000:03:00.0: Initializing i2c bus...
[   58.662420] vpx3220 22-0047: vpx3216b found @ 0x8e (DC30[0])
[   58.737445] zr36067 0000:03:00.0: Fail to get encoder

This works before, so why this is now failing is not clear to me.

It does work with 'card=0', but I really have a DC30.

If I test with 'card=0' then the rmmod issue is now solved.

Regards,

	Hans

> 
> Corentin Labbe (10):
>   staging: media: zoran: move module parameter checks to zoran_probe
>   staging: media: zoran: use module_pci_driver
>   staging: media: zoran: rename debug module parameter
>   staging: media: zoran: add debugfs
>   staging: media: zoran: videocode: remove procfs
>   staging: media: zoran: fusion all modules
>   staging: media: zoran: remove vidmem
>   staging: media: zoran: move videodev alloc
>   staging: media: zoran: move config select on primary kconfig
>   staging: media: zoran: introduce zoran_i2c_init
> 
>  drivers/staging/media/zoran/Kconfig        |  46 +--
>  drivers/staging/media/zoran/Makefile       |   8 +-
>  drivers/staging/media/zoran/videocodec.c   |  68 +----
>  drivers/staging/media/zoran/videocodec.h   |   6 +-
>  drivers/staging/media/zoran/zoran.h        |   6 +-
>  drivers/staging/media/zoran/zoran_card.c   | 328 ++++++++++++++-------
>  drivers/staging/media/zoran/zoran_driver.c |   5 +-
>  drivers/staging/media/zoran/zr36016.c      |  24 +-
>  drivers/staging/media/zoran/zr36016.h      |   2 +
>  drivers/staging/media/zoran/zr36050.c      |  21 +-
>  drivers/staging/media/zoran/zr36050.h      |   2 +
>  drivers/staging/media/zoran/zr36060.c      |  21 +-
>  drivers/staging/media/zoran/zr36060.h      |   2 +
>  13 files changed, 291 insertions(+), 248 deletions(-)
> 

Re: [PATCH v2 00/10] staging: media: zoran: fusion in one module

[LABBE Corentin]

Le Mon, Oct 25, 2021 at 02:45:02PM +0200, Hans Verkuil a écrit :
> Hi Corentin,
> 
> On 13/10/2021 20:58, Corentin Labbe wrote:
> > Hello
> > 
> > The main change of this serie is to fusion all zoran related modules in
> > one.
> > This fixes the load order problem when everything is built-in.
> > 
> > Regards
> > 
> > Changes since v1:
> > - add missing debugfs cleaning
> > - clean some remaining module_get/put functions which made impossible to
> >   remove the zoran module
> > - added the two latest patchs
> 
> Something weird is wrong with this series. I have a DC30, but loading this with:
> 
> modprobe zr36067 card=3
> 
> results in this error message in the kernel log:
> 
> [   58.645557] zr36067: module is from the staging directory, the quality is unknown, you have been warned.
> [   58.646658] zr36067 0000:03:00.0: Zoran MJPEG board driver version 0.10.1
> [   58.646793] zr36067 0000:03:00.0: Zoran ZR36057 (rev 1), irq: 18, memory: 0xf4000000
> [   58.648821] zr36067 0000:03:00.0: Initializing i2c bus...
> [   58.662420] vpx3220 22-0047: vpx3216b found @ 0x8e (DC30[0])
> [   58.737445] zr36067 0000:03:00.0: Fail to get encoder
> 
> This works before, so why this is now failing is not clear to me.
> 
> It does work with 'card=0', but I really have a DC30.
> 
> If I test with 'card=0' then the rmmod issue is now solved.

Everything normal, since card 0 does not have encoder.
Could you check that adv7175 is compiled ?

I got the same problem with my DC10+ where saa7110 was not compiled.
This issue was reproduced randomly and I have no explanation. (kconfig problem ?)

Regards

Re: [PATCH v2 00/10] staging: media: zoran: fusion in one module

[Hans Verkuil]

On 25/10/2021 16:21, LABBE Corentin wrote:
> Le Mon, Oct 25, 2021 at 02:45:02PM +0200, Hans Verkuil a écrit :
>> Hi Corentin,
>>
>> On 13/10/2021 20:58, Corentin Labbe wrote:
>>> Hello
>>>
>>> The main change of this serie is to fusion all zoran related modules in
>>> one.
>>> This fixes the load order problem when everything is built-in.
>>>
>>> Regards
>>>
>>> Changes since v1:
>>> - add missing debugfs cleaning
>>> - clean some remaining module_get/put functions which made impossible to
>>>   remove the zoran module
>>> - added the two latest patchs
>>
>> Something weird is wrong with this series. I have a DC30, but loading this with:
>>
>> modprobe zr36067 card=3
>>
>> results in this error message in the kernel log:
>>
>> [   58.645557] zr36067: module is from the staging directory, the quality is unknown, you have been warned.
>> [   58.646658] zr36067 0000:03:00.0: Zoran MJPEG board driver version 0.10.1
>> [   58.646793] zr36067 0000:03:00.0: Zoran ZR36057 (rev 1), irq: 18, memory: 0xf4000000
>> [   58.648821] zr36067 0000:03:00.0: Initializing i2c bus...
>> [   58.662420] vpx3220 22-0047: vpx3216b found @ 0x8e (DC30[0])
>> [   58.737445] zr36067 0000:03:00.0: Fail to get encoder
>>
>> This works before, so why this is now failing is not clear to me.
>>
>> It does work with 'card=0', but I really have a DC30.
>>
>> If I test with 'card=0' then the rmmod issue is now solved.
> 
> Everything normal, since card 0 does not have encoder.
> Could you check that adv7175 is compiled ?

Yes, and it loaded as well (I see it with lsmod).

However, there is no adv7175 on my board, instead it appears to have an ITT MSE3000.
There is no driver for this one (and I don't even think it is an i2c device), so
I suspect that before the driver just continued without encoder support, whereas now
it fails when it can't load the encoder.

Could that be the reason? In the absence of an encoder, I think it should just
continue, esp. since the driver doesn't use the encoder anyway.

Regards,

	Hans

> 
> I got the same problem with my DC10+ where saa7110 was not compiled.
> This issue was reproduced randomly and I have no explanation. (kconfig problem ?)
> 
> Regards
> 

Re: [PATCH v2 00/10] staging: media: zoran: fusion in one module

[LABBE Corentin]

Le Mon, Oct 25, 2021 at 05:13:04PM +0200, Hans Verkuil a écrit :
> On 25/10/2021 16:21, LABBE Corentin wrote:
> > Le Mon, Oct 25, 2021 at 02:45:02PM +0200, Hans Verkuil a écrit :
> >> Hi Corentin,
> >>
> >> On 13/10/2021 20:58, Corentin Labbe wrote:
> >>> Hello
> >>>
> >>> The main change of this serie is to fusion all zoran related modules in
> >>> one.
> >>> This fixes the load order problem when everything is built-in.
> >>>
> >>> Regards
> >>>
> >>> Changes since v1:
> >>> - add missing debugfs cleaning
> >>> - clean some remaining module_get/put functions which made impossible to
> >>>   remove the zoran module
> >>> - added the two latest patchs
> >>
> >> Something weird is wrong with this series. I have a DC30, but loading this with:
> >>
> >> modprobe zr36067 card=3
> >>
> >> results in this error message in the kernel log:
> >>
> >> [   58.645557] zr36067: module is from the staging directory, the quality is unknown, you have been warned.
> >> [   58.646658] zr36067 0000:03:00.0: Zoran MJPEG board driver version 0.10.1
> >> [   58.646793] zr36067 0000:03:00.0: Zoran ZR36057 (rev 1), irq: 18, memory: 0xf4000000
> >> [   58.648821] zr36067 0000:03:00.0: Initializing i2c bus...
> >> [   58.662420] vpx3220 22-0047: vpx3216b found @ 0x8e (DC30[0])
> >> [   58.737445] zr36067 0000:03:00.0: Fail to get encoder
> >>
> >> This works before, so why this is now failing is not clear to me.
> >>
> >> It does work with 'card=0', but I really have a DC30.
> >>
> >> If I test with 'card=0' then the rmmod issue is now solved.
> > 
> > Everything normal, since card 0 does not have encoder.
> > Could you check that adv7175 is compiled ?
> 
> Yes, and it loaded as well (I see it with lsmod).
> 
> However, there is no adv7175 on my board, instead it appears to have an ITT MSE3000.
> There is no driver for this one (and I don't even think it is an i2c device), so
> I suspect that before the driver just continued without encoder support, whereas now
> it fails when it can't load the encoder.
> 
> Could that be the reason? In the absence of an encoder, I think it should just
> continue, esp. since the driver doesn't use the encoder anyway.
> 

So probably the card list is wrong against DC30.
I checked high resolution photo of DC30 on internet, and it confirms the fact that DC30 does not have adv7175.

Since DC30 and DC30+ are identical in the card list, perhaps it is a very old copy/paste error.

So I will add a patch removing adv7175 from DC30.

Thanks for the report
Regards