linux-kernel-mentees.lists.linuxfoundation.org archive mirror
 help / color / mirror / Atom feed
* [Linux-kernel-mentees] modprobing 'vidtv' doesn't really do anything
@ 2020-05-16 22:09 Daniel W. S. Almeida
  2020-05-16 23:29 ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel W. S. Almeida @ 2020-05-16 22:09 UTC (permalink / raw)
  To: mchehab; +Cc: Linux Media Mailing List, linux-kernel-mentees

Hi Mauro!

I am trying to iron out the bugs in the virtual DVB driver I have been
working on for the past few months.

modprobing vidtv_bridge apparently works, i.e. 'vidtv_bridge' gets
listed in the output of 'lsmod' and there's a message on dmesg warning
against loading out of tree modules.

It does not seem like the probe function actually runs, though. I have
added a printk call in vidtv_bridge_i2c_probe() but it does not output
anything to dmesg.

Also, I am using QEMU + KGDB and the debugger does not break into this
function (or into any other function in vidtv_bridg.c) at all, although
it can actually get the addresses for them, i.e.

4       breakpoint     keep y   0xffffffffc0000025 in
vidtv_bridge_i2c_probe at drivers/media/test-drivers/vidtv/vidtv_bridg.c:373
5       breakpoint     keep y   0xffffffffc0000523 in
vidtv_bridge_i2c_remove at drivers/media/test-drivers/vidtv/vidtv_bridg.c:416

It can't find these, though:
2       breakpoint     keep y   <PENDING>          drivers/media/test-drivers/vidtv/vidtv_bridg.c:vidtv_bridge_driver_exit
3       breakpoint     keep y   <PENDING>          drivers/media/test-drivers/vidtv/vidtv_bridg.c:vidtv_bridge_driver_init


>The best is to start using dvbv5 tools inside v4l-utils.
>The first step to check if the demod code was properly loaded is to run:
>$ dvb-fe-tool 

I tried this, but to no avail.
>WARNING device dvb0.frontend0 not found

I am stuck, can I get some help with this issue?

Thanks!

-Daniel
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] modprobing 'vidtv' doesn't really do anything
  2020-05-16 22:09 [Linux-kernel-mentees] modprobing 'vidtv' doesn't really do anything Daniel W. S. Almeida
@ 2020-05-16 23:29 ` Mauro Carvalho Chehab
  2020-05-16 23:42   ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2020-05-16 23:29 UTC (permalink / raw)
  To: Daniel W. S. Almeida
  Cc: linux-kernel-mentees@lists.linuxfoundation.org"
	<linux-kernel-mentees@lists.linuxfoundation.org>,
	Linux Media Mailing List

Em Sat, 16 May 2020 19:09:35 -0300
"Daniel W. S. Almeida" <dwlsalmeida@gmail.com> escreveu:

> Hi Mauro!
> 
> I am trying to iron out the bugs in the virtual DVB driver I have been
> working on for the past few months.
> 
> modprobing vidtv_bridge apparently works, i.e. 'vidtv_bridge' gets
> listed in the output of 'lsmod' and there's a message on dmesg warning
> against loading out of tree modules.

The warning is probably due to that:

WARNING: modpost: missing MODULE_LICENSE() in drivers/media/test-drivers/vidtv//vidtv_demod.o
WARNING: modpost: missing MODULE_LICENSE() in drivers/media/test-drivers/vidtv//vidtv_bridge.o

You forgot to add MODULE_LICENSE() macro somewhere.

With is weird, as those are there:

	drivers/media/test-drivers/vidtv/vidtv_bridge.c:MODULE_AUTHOR("Daniel W. S. Almeida");
	drivers/media/test-drivers/vidtv/vidtv_bridge.c:MODULE_LICENSE("GPL");
	drivers/media/test-drivers/vidtv/vidtv_bridge.c:MODULE_DEVICE_TABLE(i2c, vidtv_bridge_id_table);

(yet, a good practice nowadays is to place all those three at the end of
a file - not at the beginning)

This is weird:

	drivers/media/test-drivers/vidtv/vidtv_bridge.c:module_i2c_driver(vidtv_bridge_driver);
	drivers/media/test-drivers/vidtv/vidtv_demod.c:module_i2c_driver(vidtv_demod_i2c_driver);
	drivers/media/test-drivers/vidtv/vidtv_tuner.c:module_i2c_driver(vidtv_tuner_i2c_driver);

With the above, none of the three files will be initialized, as they
would wait for some other driver to bind them on some I2C bus.

See, the way the Kernel works is that each bus has some sort
of code that initializes the driver. For buses like PCI and USB,
this happens when an USB ID or PCI ID matches their device tables.
For normal[1] I2C devices, the driver which creates an I2C adapter
should either manually bind new I2C devices or ask the I2C core
to scan.

[1] ACPI devices using I2C buses use a different logic.

So, basically, the module_i2c_driver() tells the driver code
that those devices will be available when some other driver
would need to bind them. This is the right thing to do for the
tuner and demod, but the bridge driver should do something else.

I would be expecting at the bridge driver something like:

	static struct platform_driver vidtv_bridge_driver = {
		.probe		= vidtv_bridge_probe,
		.remove		= vidtv_bridge_remove,
		.driver		= {
			.name	= "vidtv-bridge",
		},
	};
	module_platform_driver(vidtv_bridge_driver);

Note: the other media virtual drivers don't use the 
"module_platform_driver" macro. 

While I don't test this for a while, but looking at the code
differences, they also declare a "platform_driver". So, they use 
a more verbose syntax (see at the end). I suspect that this is
needed for those devices to be probed unconditionally[2].

[1] a real platform driver is probed via DT or some other
mechanism, like ACPI.

Thanks,
Mauro

-

This is what vim2m.c does, for example:


static struct platform_driver vim2m_pdrv = {
	.probe		= vim2m_probe,
	.remove		= vim2m_remove,
	.driver		= {
		.name	= MEM2MEM_NAME,
	},
};

static void __exit vim2m_exit(void)
{
	platform_driver_unregister(&vim2m_pdrv);
	platform_device_unregister(&vim2m_pdev);
}

static int __init vim2m_init(void)
{
	int ret;

	ret = platform_device_register(&vim2m_pdev);
	if (ret)
		return ret;

	ret = platform_driver_register(&vim2m_pdrv);
	if (ret)
		platform_device_unregister(&vim2m_pdev);

	return ret;
}

module_init(vim2m_init);
module_exit(vim2m_exit);
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] modprobing 'vidtv' doesn't really do anything
  2020-05-16 23:29 ` Mauro Carvalho Chehab
@ 2020-05-16 23:42   ` Mauro Carvalho Chehab
  2020-05-17  0:04     ` Daniel W. S. Almeida
  2020-05-17  0:17     ` [Linux-kernel-mentees] [PATCH] media: vidtv: fix issues preventing it to build and load Mauro Carvalho Chehab
  0 siblings, 2 replies; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2020-05-16 23:42 UTC (permalink / raw)
  To: Daniel W. S. Almeida
  Cc: linux-kernel-mentees@lists.linuxfoundation.org"
	<linux-kernel-mentees@lists.linuxfoundation.org>,
	Linux Media Mailing List

Em Sun, 17 May 2020 01:29:35 +0200
Mauro Carvalho Chehab <mchehab@kernel.org> escreveu:

> Em Sat, 16 May 2020 19:09:35 -0300
> "Daniel W. S. Almeida" <dwlsalmeida@gmail.com> escreveu:
> 
> > Hi Mauro!
> > 
> > I am trying to iron out the bugs in the virtual DVB driver I have been
> > working on for the past few months.
> > 
> > modprobing vidtv_bridge apparently works, i.e. 'vidtv_bridge' gets
> > listed in the output of 'lsmod' and there's a message on dmesg warning
> > against loading out of tree modules.
> 
> The warning is probably due to that:
> 
> WARNING: modpost: missing MODULE_LICENSE() in drivers/media/test-drivers/vidtv//vidtv_demod.o
> WARNING: modpost: missing MODULE_LICENSE() in drivers/media/test-drivers/vidtv//vidtv_bridge.o
> 
> You forgot to add MODULE_LICENSE() macro somewhere.
> 
> With is weird, as those are there:
> 
> 	drivers/media/test-drivers/vidtv/vidtv_bridge.c:MODULE_AUTHOR("Daniel W. S. Almeida");
> 	drivers/media/test-drivers/vidtv/vidtv_bridge.c:MODULE_LICENSE("GPL");
> 	drivers/media/test-drivers/vidtv/vidtv_bridge.c:MODULE_DEVICE_TABLE(i2c, vidtv_bridge_id_table);
> 
> (yet, a good practice nowadays is to place all those three at the end of
> a file - not at the beginning)
> 
> This is weird:
> 
> 	drivers/media/test-drivers/vidtv/vidtv_bridge.c:module_i2c_driver(vidtv_bridge_driver);
> 	drivers/media/test-drivers/vidtv/vidtv_demod.c:module_i2c_driver(vidtv_demod_i2c_driver);
> 	drivers/media/test-drivers/vidtv/vidtv_tuner.c:module_i2c_driver(vidtv_tuner_i2c_driver);
> 
> With the above, none of the three files will be initialized, as they
> would wait for some other driver to bind them on some I2C bus.
> 
> See, the way the Kernel works is that each bus has some sort
> of code that initializes the driver. For buses like PCI and USB,
> this happens when an USB ID or PCI ID matches their device tables.
> For normal[1] I2C devices, the driver which creates an I2C adapter
> should either manually bind new I2C devices or ask the I2C core
> to scan.
> 
> [1] ACPI devices using I2C buses use a different logic.
> 
> So, basically, the module_i2c_driver() tells the driver code
> that those devices will be available when some other driver
> would need to bind them. This is the right thing to do for the
> tuner and demod, but the bridge driver should do something else.
> 
> I would be expecting at the bridge driver something like:
> 
> 	static struct platform_driver vidtv_bridge_driver = {
> 		.probe		= vidtv_bridge_probe,
> 		.remove		= vidtv_bridge_remove,
> 		.driver		= {
> 			.name	= "vidtv-bridge",
> 		},
> 	};
> 	module_platform_driver(vidtv_bridge_driver);
> 
> Note: the other media virtual drivers don't use the 
> "module_platform_driver" macro. 
> 
> While I don't test this for a while, but looking at the code
> differences, they also declare a "platform_driver". So, they use 
> a more verbose syntax (see at the end). I suspect that this is
> needed for those devices to be probed unconditionally[2].
> 
> [1] a real platform driver is probed via DT or some other
> mechanism, like ACPI.
> 
> Thanks,
> Mauro
> 
> -
> 
> This is what vim2m.c does, for example:
> 
> 
> static struct platform_driver vim2m_pdrv = {
> 	.probe		= vim2m_probe,
> 	.remove		= vim2m_remove,
> 	.driver		= {
> 		.name	= MEM2MEM_NAME,
> 	},
> };
> 
> static void __exit vim2m_exit(void)
> {
> 	platform_driver_unregister(&vim2m_pdrv);
> 	platform_device_unregister(&vim2m_pdev);
> }
> 
> static int __init vim2m_init(void)
> {
> 	int ret;
> 
> 	ret = platform_device_register(&vim2m_pdev);
> 	if (ret)
> 		return ret;
> 
> 	ret = platform_driver_register(&vim2m_pdrv);
> 	if (ret)
> 		platform_device_unregister(&vim2m_pdev);
> 
> 	return ret;
> }
> 
> module_init(vim2m_init);
> module_exit(vim2m_exit);

There's also a problem at the Makefile. The way it was defined, the
vidtv-bridge driver won't be built.

The enclosed patch should fix the issues. The bridge driver doesn't 
build, though.

Thanks,
Mauro

---

diff --git a/drivers/media/test-drivers/vidtv/Makefile b/drivers/media/test-drivers/vidtv/Makefile
index a1d29001fffe..a56ad8bce0cb 100644
--- a/drivers/media/test-drivers/vidtv/Makefile
+++ b/drivers/media/test-drivers/vidtv/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
 vidtv_demod-objs := vidtv_common.o
-vidtv_bridge-objs := vidtv_common.o vidtv_ts.o vidtv_psi.o vidtv_pes.o \
-		     vidtv_s302m.o vidtv_channel.o vidtv_mux.o
+vidtv-objs := vidtv_common.o vidtv_ts.o vidtv_psi.o vidtv_pes.o \
+	      vidtv_s302m.o vidtv_channel.o vidtv_mux.o vidtv_bridge.o
 
 obj-$(CONFIG_DVB_VIDTV)	+= vidtv_tuner.o vidtv_demod.o vidtv_bridge.o
diff --git a/drivers/media/test-drivers/vidtv/vidtv_bridge.c b/drivers/media/test-drivers/vidtv/vidtv_bridge.c
index c9876372fdeb..762abf45dda0 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_bridge.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_bridge.c
@@ -20,9 +20,6 @@
 #define TUNER_DEFAULT_ADDR 0x68
 #define DEMOD_DEFAULT_ADDR 0x60
 
-MODULE_AUTHOR("Daniel W. S. Almeida");
-MODULE_LICENSE("GPL");
-
 static unsigned int drop_tslock_prob_on_low_snr;
 module_param(drop_tslock_prob_on_low_snr, uint, 0644);
 MODULE_PARM_DESC(drop_tslock_prob_on_low_snr,
@@ -422,21 +419,44 @@ static int vidtv_bridge_i2c_remove(struct i2c_client *client)
 	return 0;
 }
 
-static const struct i2c_device_id vidtv_bridge_id_table[] = {
-	{"vidtv_bridge", 0},
-	{}
+static struct platform_device vidtv_bridge_dev = {
+	.name		= "vidtv_bridge",
+	.dev.release	= vidtv_bridge_dev_release,
 };
 
-MODULE_DEVICE_TABLE(i2c, vidtv_bridge_id_table);
-
-static struct i2c_driver vidtv_bridge_driver = {
+static struct platform_driver vidtv_bridge_driver = {
 	.driver = {
 		.name                = "vidtv_bridge",
 		.suppress_bind_attrs = true,
 	},
 	.probe    = vidtv_bridge_i2c_probe,
 	.remove   = vidtv_bridge_i2c_remove,
-	.id_table = vidtv_bridge_id_table,
 };
 
-module_i2c_driver(vidtv_bridge_driver);
+static void __exit vidtv_bridge_exit(void)
+{
+	platform_driver_unregister(&vidtv_bridge_dev);
+	platform_device_unregister(&vidtv_bridge_driver);
+}
+
+static int __init vidtv_bridge_init(void)
+{
+	int ret;
+
+	ret = platform_device_register(&vidtv_bridge_dev);
+	if (ret)
+		return ret;
+
+	ret = platform_driver_register(&vidtv_bridge_driver);
+	if (ret)
+		platform_device_unregister(&vidtv_bridge_pdev);
+
+	return ret;
+}
+
+module_init(vidtv_bridge_init);
+module_exit(vidtv_bridge_exit);
+
+MODULE_DESCRIPTION("Virtual Digital TV Test Driver");
+MODULE_AUTHOR("Daniel W. S. Almeida");
+MODULE_LICENSE("GPL");
diff --git a/drivers/media/test-drivers/vidtv/vidtv_demod.c b/drivers/media/test-drivers/vidtv/vidtv_demod.c
index 15436e565a7b..01cd4a93b0ec 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_demod.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_demod.c
@@ -21,10 +21,6 @@
 #include "vidtv_demod.h"
 #include "vidtv_config.h"
 
-MODULE_DESCRIPTION("Virtual DVB Demodulator Driver");
-MODULE_AUTHOR("Daniel W. S. Almeida");
-MODULE_LICENSE("GPL");
-
 struct vidtv_demod_cnr_to_qual_s vidtv_demod_c_cnr_2_qual[] = {
 	/* from libdvbv5 source code, in milli db */
 	{ QAM_256, FEC_NONE,  34000, 38000},
@@ -492,3 +488,7 @@ static struct i2c_driver vidtv_demod_i2c_driver = {
 };
 
 module_i2c_driver(vidtv_demod_i2c_driver);
+
+MODULE_DESCRIPTION("Virtual DVB Demodulator Driver");
+MODULE_AUTHOR("Daniel W. S. Almeida");
+MODULE_LICENSE("GPL");

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] modprobing 'vidtv' doesn't really do anything
  2020-05-16 23:42   ` Mauro Carvalho Chehab
@ 2020-05-17  0:04     ` Daniel W. S. Almeida
  2020-05-17  0:17     ` [Linux-kernel-mentees] [PATCH] media: vidtv: fix issues preventing it to build and load Mauro Carvalho Chehab
  1 sibling, 0 replies; 5+ messages in thread
From: Daniel W. S. Almeida @ 2020-05-17  0:04 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Media Mailing List,
	linux-kernel-mentees@lists.linuxfoundation.org"

Yeah the Makefile was broken, although I corrected it in the last
revision, so this warning should be gone already?

I guess you're looking at v4, in which case I have sent in new code that
hopefully addresses the things you pointed out in the last code review :)

I will try what you said regarding the platform bus and report back.

- Daniel.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* [Linux-kernel-mentees] [PATCH] media: vidtv: fix issues preventing it to build and load
  2020-05-16 23:42   ` Mauro Carvalho Chehab
  2020-05-17  0:04     ` Daniel W. S. Almeida
@ 2020-05-17  0:17     ` Mauro Carvalho Chehab
  1 sibling, 0 replies; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2020-05-17  0:17 UTC (permalink / raw)
  To: Daniel W. S. Almeida
  Cc: linux-kernel-mentees@lists.linuxfoundation.org"
	<linux-kernel-mentees@lists.linuxfoundation.org>,
	Linux Media Mailing List

There are several issues that were preventing it to properly
build and load:

- Makefile was not actually compiling everything;
- The bridge driver should be a platform driver;
- There are lots of warnings and other errors produced
  by the driver.

Address them.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

Please notice that there are still several issues reported by
sparse. I didn't check them:

$ make W=1 C=1 M=drivers/media/test-drivers/vidtv 
SPARSE:drivers/media/test-drivers/vidtv/vidtv_channel.c drivers/media/test-drivers/vidtv/vidtv_channel.c:54:62:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_psi.c drivers/media/test-drivers/vidtv/vidtv_psi.c:71:43:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_psi.c drivers/media/test-drivers/vidtv/vidtv_psi.c:174:43:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_psi.c drivers/media/test-drivers/vidtv/vidtv_psi.c:319:43:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_psi.c drivers/media/test-drivers/vidtv/vidtv_psi.c:359:43:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_psi.c drivers/media/test-drivers/vidtv/vidtv_psi.c:574:50:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_psi.c drivers/media/test-drivers/vidtv/vidtv_psi.c:575:50:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_psi.c drivers/media/test-drivers/vidtv/vidtv_psi.c:576:50:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_psi.c drivers/media/test-drivers/vidtv/vidtv_psi.c:770:44:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_psi.c drivers/media/test-drivers/vidtv/vidtv_psi.c:771:44:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_psi.c drivers/media/test-drivers/vidtv/vidtv_psi.c:772:44:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_psi.c drivers/media/test-drivers/vidtv/vidtv_psi.c:773:44:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_psi.c drivers/media/test-drivers/vidtv/vidtv_psi.c:938:44:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_psi.c drivers/media/test-drivers/vidtv/vidtv_psi.c:939:44:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_psi.c drivers/media/test-drivers/vidtv/vidtv_psi.c:940:44:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_psi.c drivers/media/test-drivers/vidtv/vidtv_psi.c:941:44:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_mux.c drivers/media/test-drivers/vidtv/vidtv_mux.c:155:39:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_mux.c drivers/media/test-drivers/vidtv/vidtv_mux.c:197:39:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_mux.c drivers/media/test-drivers/vidtv/vidtv_mux.c:259:47:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_pes.c drivers/media/test-drivers/vidtv/vidtv_pes.c:80:54:  warning: missing braces around initializer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_pes.c drivers/media/test-drivers/vidtv/vidtv_pes.c:81:54:  warning: missing braces around initializer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_pes.c drivers/media/test-drivers/vidtv/vidtv_pes.c:146:54:  warning: missing braces around initializer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_pes.c drivers/media/test-drivers/vidtv/vidtv_pes.c:147:54:  warning: missing braces around initializer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_pes.c drivers/media/test-drivers/vidtv/vidtv_pes.c:311:59:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_pes.c drivers/media/test-drivers/vidtv/vidtv_pes.c:312:59:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_s302m.c drivers/media/test-drivers/vidtv/vidtv_s302m.c:374:42:  warning: missing braces around initializer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_s302m.c drivers/media/test-drivers/vidtv/vidtv_s302m.c:430:36:  warning: incorrect type in assignment (different base types)
SPARSE:drivers/media/test-drivers/vidtv/vidtv_s302m.c drivers/media/test-drivers/vidtv/vidtv_s302m.c:430:36:     expected signed short [usertype] sample
SPARSE:drivers/media/test-drivers/vidtv/vidtv_s302m.c drivers/media/test-drivers/vidtv/vidtv_s302m.c:430:36:     got restricted __le16 [usertype]
SPARSE:drivers/media/test-drivers/vidtv/vidtv_bridge.c drivers/media/test-drivers/vidtv/vidtv_bridge.c:252:42:  warning: Using plain integer as NULL pointer
SPARSE:drivers/media/test-drivers/vidtv/vidtv_bridge.c drivers/media/test-drivers/vidtv/vidtv_bridge.c:270:42:  warning: Using plain integer as NULL pointer


 drivers/media/test-drivers/vidtv/Makefile     | 11 ++-
 .../media/test-drivers/vidtv/vidtv_bridge.c   | 88 ++++++++++++-------
 .../media/test-drivers/vidtv/vidtv_common.c   |  6 +-
 .../media/test-drivers/vidtv/vidtv_demod.c    | 39 ++++----
 .../media/test-drivers/vidtv/vidtv_s302m.c    |  2 -
 .../media/test-drivers/vidtv/vidtv_tuner.c    | 10 +--
 6 files changed, 93 insertions(+), 63 deletions(-)

diff --git a/drivers/media/test-drivers/vidtv/Makefile b/drivers/media/test-drivers/vidtv/Makefile
index a1d29001fffe..cc3b67fd1c9f 100644
--- a/drivers/media/test-drivers/vidtv/Makefile
+++ b/drivers/media/test-drivers/vidtv/Makefile
@@ -1,7 +1,10 @@
 # SPDX-License-Identifier: GPL-2.0
 
-vidtv_demod-objs := vidtv_common.o
-vidtv_bridge-objs := vidtv_common.o vidtv_ts.o vidtv_psi.o vidtv_pes.o \
-		     vidtv_s302m.o vidtv_channel.o vidtv_mux.o
+dvb-vidtv-tuner-objs := vidtv_tuner.o
+dvb-vidtv-demod-objs := vidtv_common.o vidtv_demod.o
+dvb-vidtv-bridge-objs := vidtv_common.o vidtv_ts.o vidtv_psi.o vidtv_pes.o \
+		         vidtv_s302m.o vidtv_channel.o vidtv_mux.o \
+		         vidtv_bridge.o
 
-obj-$(CONFIG_DVB_VIDTV)	+= vidtv_tuner.o vidtv_demod.o vidtv_bridge.o
+obj-$(CONFIG_DVB_VIDTV)	+= dvb-vidtv-tuner.o dvb-vidtv-demod.o \
+			   dvb-vidtv-bridge.o
diff --git a/drivers/media/test-drivers/vidtv/vidtv_bridge.c b/drivers/media/test-drivers/vidtv/vidtv_bridge.c
index c9876372fdeb..26681ecacfc0 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_bridge.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_bridge.c
@@ -7,22 +7,22 @@
  * Written by Daniel W. S. Almeida <dwlsalmeida@gmail.com>
  */
 
-#include <linux/types.h>
 #include <linux/moduleparam.h>
 #include <linux/mutex.h>
-#include <linux/workqueue.h>
+#include <linux/platform_device.h>
 #include <linux/time.h>
+#include <linux/types.h>
+#include <linux/workqueue.h>
+
 #include "vidtv_bridge.h"
-#include "vidtv_ts.h"
+#include "vidtv_config.h"
 #include "vidtv_mux.h"
+#include "vidtv_ts.h"
 
 #define TS_BUF_MAX_SZ (128 * TS_PACKET_LEN)
 #define TUNER_DEFAULT_ADDR 0x68
 #define DEMOD_DEFAULT_ADDR 0x60
 
-MODULE_AUTHOR("Daniel W. S. Almeida");
-MODULE_LICENSE("GPL");
-
 static unsigned int drop_tslock_prob_on_low_snr;
 module_param(drop_tslock_prob_on_low_snr, uint, 0644);
 MODULE_PARM_DESC(drop_tslock_prob_on_low_snr,
@@ -87,7 +87,7 @@ static unsigned int mux_rate_kbytes_sec = 4096;
 module_param(mux_rate_kbytes_sec, uint, 0644);
 MODULE_PARM_DESC(mux_rate_kbytes_sec, "Optional mux rate: will pad stream if below");
 
-static unsigned int pcr_pid = 0x200
+static unsigned int pcr_pid = 0x200;
 module_param(pcr_pid, uint, 0644);
 MODULE_PARM_DESC(pcr_pid, "Optional PCR PID for all channels: defaults to 0x200");
 
@@ -97,11 +97,13 @@ static bool vidtv_bridge_check_demod_lock(struct vidtv_dvb *dvb, u32 n)
 
 	dvb->fe[n]->ops.read_status(dvb->fe[n], &status);
 
-	return status == FE_HAS_SIGNAL  |
-			 FE_HAS_CARRIER |
-			 FE_HAS_VITERBI |
-			 FE_HAS_SYNC    |
-			 FE_HAS_LOCK;
+	status = FE_HAS_SIGNAL  |
+		 FE_HAS_CARRIER |
+		 FE_HAS_VITERBI |
+		 FE_HAS_SYNC    |
+		 FE_HAS_LOCK;
+
+	return status;
 }
 
 static void
@@ -188,7 +190,7 @@ static u32 vidtv_i2c_func(struct i2c_adapter *adapter)
 	return I2C_FUNC_I2C;
 }
 
-struct i2c_algorithm vidtv_i2c_algorithm = {
+static const struct i2c_algorithm vidtv_i2c_algorithm = {
 	.master_xfer   = vidtv_master_xfer,
 	.functionality = vidtv_i2c_func,
 };
@@ -358,8 +360,7 @@ static int vidtv_bridge_dvb_init(struct vidtv_dvb *dvb)
 	return ret;
 }
 
-static int vidtv_bridge_i2c_probe(struct i2c_client *client,
-				  const struct i2c_device_id *id)
+static int vidtv_bridge_probe(struct platform_device *pdev)
 {
 	int ret;
 	struct vidtv_dvb *dvb;
@@ -378,14 +379,14 @@ static int vidtv_bridge_i2c_probe(struct i2c_client *client,
 	mux_args.mux_rate_kbytes_sec = mux_rate_kbytes_sec;
 	mux_args.on_new_packets_available_cb = vidtv_bridge_on_new_pkts_avail;
 	mux_args.ts_buf_sz = ts_buf_sz;
-	mux_args.pcr_period_usecs = pcr_period_msecs * 1000;
-	mux_args.si_period_usecs = si_period_msecs * 1000;
+	mux_args.pcr_period_usecs = pcr_period_msec * 1000;
+	mux_args.si_period_usecs = si_period_msec * 1000;
 	mux_args.pcr_pid = pcr_pid;
 	mux_args.priv = dvb;
 
 	dvb->mux = vidtv_mux_init(mux_args);
 
-	i2c_set_clientdata(client, dvb);
+	platform_set_drvdata(pdev, dvb);
 
 	return ret;
 
@@ -394,12 +395,12 @@ static int vidtv_bridge_i2c_probe(struct i2c_client *client,
 	return ret;
 }
 
-static int vidtv_bridge_i2c_remove(struct i2c_client *client)
+static int vidtv_bridge_remove(struct platform_device *pdev)
 {
 	struct vidtv_dvb *dvb;
 	u32 i;
 
-	dvb = i2c_get_clientdata(client);
+	dvb = platform_get_drvdata(pdev);
 
 	vidtv_mux_destroy(dvb->mux);
 
@@ -422,21 +423,48 @@ static int vidtv_bridge_i2c_remove(struct i2c_client *client)
 	return 0;
 }
 
-static const struct i2c_device_id vidtv_bridge_id_table[] = {
-	{"vidtv_bridge", 0},
-	{}
-};
+static void vidtv_bridge_dev_release(struct device *dev)
+{
+}
 
-MODULE_DEVICE_TABLE(i2c, vidtv_bridge_id_table);
+static struct platform_device vidtv_bridge_dev = {
+	.name		= "vidtv_bridge",
+	.dev.release	= vidtv_bridge_dev_release,
+};
 
-static struct i2c_driver vidtv_bridge_driver = {
+static struct platform_driver vidtv_bridge_driver = {
 	.driver = {
 		.name                = "vidtv_bridge",
 		.suppress_bind_attrs = true,
 	},
-	.probe    = vidtv_bridge_i2c_probe,
-	.remove   = vidtv_bridge_i2c_remove,
-	.id_table = vidtv_bridge_id_table,
+	.probe    = vidtv_bridge_probe,
+	.remove   = vidtv_bridge_remove,
 };
 
-module_i2c_driver(vidtv_bridge_driver);
+static void __exit vidtv_bridge_exit(void)
+{
+	platform_driver_unregister(&vidtv_bridge_driver);
+	platform_device_unregister(&vidtv_bridge_dev);
+}
+
+static int __init vidtv_bridge_init(void)
+{
+	int ret;
+
+	ret = platform_device_register(&vidtv_bridge_dev);
+	if (ret)
+		return ret;
+
+	ret = platform_driver_register(&vidtv_bridge_driver);
+	if (ret)
+		platform_device_unregister(&vidtv_bridge_dev);
+
+	return ret;
+}
+
+module_init(vidtv_bridge_init);
+module_exit(vidtv_bridge_exit);
+
+MODULE_DESCRIPTION("Virtual Digital TV Test Driver");
+MODULE_AUTHOR("Daniel W. S. Almeida");
+MODULE_LICENSE("GPL");
diff --git a/drivers/media/test-drivers/vidtv/vidtv_common.c b/drivers/media/test-drivers/vidtv/vidtv_common.c
index b1412b497e1e..3a2358d4fe69 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_common.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_common.c
@@ -7,9 +7,11 @@
  * Written by Daniel W. S. Almeida <dwlsalmeida@gmail.com>
  */
 
-#include <linux/types.h>
-#include <linux/string.h>
 #include <linux/printk.h>
+#include <linux/string.h>
+#include <linux/types.h>
+
+#include "vidtv_common.h"
 
 u32 vidtv_memcpy(void *to,
 		 const void *from,
diff --git a/drivers/media/test-drivers/vidtv/vidtv_demod.c b/drivers/media/test-drivers/vidtv/vidtv_demod.c
index 15436e565a7b..7d3aa464d6fc 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_demod.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_demod.c
@@ -8,30 +8,27 @@
  * Based on the example driver written by Emard <emard@softhome.net>
  */
 
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/init.h>
-#include <linux/string.h>
+#include <linux/random.h>
 #include <linux/slab.h>
+#include <linux/string.h>
 #include <linux/workqueue.h>
-#include <linux/random.h>
-#include <linux/errno.h>
-#include <linux/i2c.h>
 #include <media/dvb_frontend.h>
-#include "vidtv_demod.h"
-#include "vidtv_config.h"
 
-MODULE_DESCRIPTION("Virtual DVB Demodulator Driver");
-MODULE_AUTHOR("Daniel W. S. Almeida");
-MODULE_LICENSE("GPL");
+#include "vidtv_config.h"
+#include "vidtv_demod.h"
 
-struct vidtv_demod_cnr_to_qual_s vidtv_demod_c_cnr_2_qual[] = {
+static const struct vidtv_demod_cnr_to_qual_s vidtv_demod_c_cnr_2_qual[] = {
 	/* from libdvbv5 source code, in milli db */
 	{ QAM_256, FEC_NONE,  34000, 38000},
 	{ QAM_64,  FEC_NONE,  30000, 34000},
 };
 
-struct vidtv_demod_cnr_to_qual_s vidtv_demod_s_cnr_2_qual[] = {
+static const struct vidtv_demod_cnr_to_qual_s vidtv_demod_s_cnr_2_qual[] = {
 	/* from libdvbv5 source code, in milli db */
 	{ QPSK, FEC_1_2,  7000, 10000},
 	{ QPSK, FEC_2_3,  9000, 12000},
@@ -40,7 +37,7 @@ struct vidtv_demod_cnr_to_qual_s vidtv_demod_s_cnr_2_qual[] = {
 	{ QPSK, FEC_7_8, 12000, 15000},
 };
 
-struct vidtv_demod_cnr_to_qual_s vidtv_demod_s2_cnr_2_qual[] = {
+static const struct vidtv_demod_cnr_to_qual_s vidtv_demod_s2_cnr_2_qual[] = {
 	/* from libdvbv5 source code, in milli db */
 	{ QPSK,  FEC_1_2,   9000,  12000},
 	{ QPSK,  FEC_2_3,  11000,  14000},
@@ -54,7 +51,7 @@ struct vidtv_demod_cnr_to_qual_s vidtv_demod_s2_cnr_2_qual[] = {
 	{ PSK_8, FEC_8_9,  19000,  22000},
 };
 
-static struct vidtv_demod_cnr_to_qual_s vidtv_demod_t_cnr_2_qual[] = {
+static const struct vidtv_demod_cnr_to_qual_s vidtv_demod_t_cnr_2_qual[] = {
 	/* from libdvbv5 source code, in milli db*/
 	{   QPSK, FEC_1_2,  4100,  5900},
 	{   QPSK, FEC_2_3,  6100,  9600},
@@ -75,11 +72,11 @@ static struct vidtv_demod_cnr_to_qual_s vidtv_demod_t_cnr_2_qual[] = {
 	{ QAM_64, FEC_7_8, 22000, 24000},
 };
 
-static struct vidtv_demod_cnr_to_qual_s
+static const struct vidtv_demod_cnr_to_qual_s
 *vidtv_match_cnr_s(struct dvb_frontend *fe)
 {
 	struct dtv_frontend_properties *c;
-	struct vidtv_demod_cnr_to_qual_s *cnr2qual = NULL;
+	const struct vidtv_demod_cnr_to_qual_s *cnr2qual = NULL;
 	u32    array_size                          = 0;
 	u32 i;
 
@@ -129,13 +126,11 @@ static void vidtv_demod_poll_snr_handler(struct work_struct *work)
 	 * lose the TS lock if it dips too low
 	 */
 	struct vidtv_demod_state *state;
-	struct dtv_frontend_properties *c;
-	struct vidtv_demod_cnr_to_qual_s *cnr2qual = NULL;
+	const struct vidtv_demod_cnr_to_qual_s *cnr2qual = NULL;
 	struct vidtv_demod_config *config;
 	u16 snr = 0;
 
 	state  = container_of(work, struct vidtv_demod_state, poll_snr.work);
-	c      = &state->frontend.dtv_property_cache;
 	config = &state->config;
 
 	if (!state->frontend.ops.tuner_ops.get_rf_strength)
@@ -214,7 +209,7 @@ static int vidtv_demod_get_frontend(struct dvb_frontend *fe,
 static int vidtv_demod_set_frontend(struct dvb_frontend *fe)
 {
 	struct vidtv_demod_state *state            = fe->demodulator_priv;
-	struct vidtv_demod_cnr_to_qual_s *cnr2qual = NULL;
+	const struct vidtv_demod_cnr_to_qual_s *cnr2qual = NULL;
 	u32    tuner_status                        = 0;
 
 	if (fe->ops.tuner_ops.set_params) {
@@ -492,3 +487,7 @@ static struct i2c_driver vidtv_demod_i2c_driver = {
 };
 
 module_i2c_driver(vidtv_demod_i2c_driver);
+
+MODULE_DESCRIPTION("Virtual DVB Demodulator Driver");
+MODULE_AUTHOR("Daniel W. S. Almeida");
+MODULE_LICENSE("GPL");
diff --git a/drivers/media/test-drivers/vidtv/vidtv_s302m.c b/drivers/media/test-drivers/vidtv/vidtv_s302m.c
index b08bfff7b8f2..1ffb1da1e124 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_s302m.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_s302m.c
@@ -200,14 +200,12 @@ vidtv_s302m_compute_sample_count_v(struct vidtv_encoder *e)
 	u32 vau_duration_usecs;
 	u32 sample_duration_usecs;
 	u32 i;
-	u32 sample_count;
 	u32 s;
 
 	vau_duration_usecs    = USEC_PER_SEC / e->sync->sampling_rate_hz;
 	sample_duration_usecs = USEC_PER_SEC / e->sampling_rate_hz;
 
 	for (i = 0; i < e->sync->nunits; ++i) {
-		sample_count = e->samples_per_unit[i];
 		s = DIV_ROUND_UP(vau_duration_usecs, sample_duration_usecs);
 		e->samples_per_unit[i] = s;
 	}
diff --git a/drivers/media/test-drivers/vidtv/vidtv_tuner.c b/drivers/media/test-drivers/vidtv/vidtv_tuner.c
index ece4a94b0c3a..882a1fec2ce5 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_tuner.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_tuner.c
@@ -30,13 +30,13 @@ struct vidtv_tuner_cnr_to_qual_s {
 	u32 cnr_ok, cnr_good;
 };
 
-struct vidtv_tuner_cnr_to_qual_s vidtv_tuner_c_cnr_2_qual[] = {
+static const struct vidtv_tuner_cnr_to_qual_s vidtv_tuner_c_cnr_2_qual[] = {
 	/* from libdvbv5 source code, in milli db */
 	{ QAM_256, FEC_NONE,  34000, 38000},
 	{ QAM_64,  FEC_NONE,  30000, 34000},
 };
 
-struct vidtv_tuner_cnr_to_qual_s vidtv_tuner_s_cnr_2_qual[] = {
+static const struct vidtv_tuner_cnr_to_qual_s vidtv_tuner_s_cnr_2_qual[] = {
 	/* from libdvbv5 source code, in milli db */
 	{ QPSK, FEC_1_2,  7000, 10000},
 
@@ -47,7 +47,7 @@ struct vidtv_tuner_cnr_to_qual_s vidtv_tuner_s_cnr_2_qual[] = {
 	{ QPSK, FEC_7_8, 12000, 15000},
 };
 
-struct vidtv_tuner_cnr_to_qual_s vidtv_tuner_s2_cnr_2_qual[] = {
+static const struct vidtv_tuner_cnr_to_qual_s vidtv_tuner_s2_cnr_2_qual[] = {
 	/* from libdvbv5 source code, in milli db */
 	{ QPSK,  FEC_1_2,   9000,  12000},
 	{ QPSK,  FEC_2_3,  11000,  14000},
@@ -61,7 +61,7 @@ struct vidtv_tuner_cnr_to_qual_s vidtv_tuner_s2_cnr_2_qual[] = {
 	{ PSK_8, FEC_8_9,  19000,  22000},
 };
 
-static struct vidtv_tuner_cnr_to_qual_s vidtv_tuner_t_cnr_2_qual[] = {
+static const struct vidtv_tuner_cnr_to_qual_s vidtv_tuner_t_cnr_2_qual[] = {
 	/* from libdvbv5 source code, in milli db*/
 	{   QPSK, FEC_1_2,  4100,  5900},
 	{   QPSK, FEC_2_3,  6100,  9600},
@@ -159,7 +159,7 @@ static int
 vidtv_tuner_get_signal_strength(struct dvb_frontend *fe, u16 *strength)
 {
 	struct dtv_frontend_properties *c          = &fe->dtv_property_cache;
-	struct vidtv_tuner_cnr_to_qual_s *cnr2qual = NULL;
+	const struct vidtv_tuner_cnr_to_qual_s *cnr2qual = NULL;
 	u32    array_size                          = 0;
 	s32 shift;
 	u32 i;
-- 
2.26.2



Thanks,
Mauro
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

end of thread, other threads:[~2020-05-20 12:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-16 22:09 [Linux-kernel-mentees] modprobing 'vidtv' doesn't really do anything Daniel W. S. Almeida
2020-05-16 23:29 ` Mauro Carvalho Chehab
2020-05-16 23:42   ` Mauro Carvalho Chehab
2020-05-17  0:04     ` Daniel W. S. Almeida
2020-05-17  0:17     ` [Linux-kernel-mentees] [PATCH] media: vidtv: fix issues preventing it to build and load Mauro Carvalho Chehab

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).