Re: [Linux-kernel-mentees] modprobing 'vidtv' doesn't really do anything

[Mauro Carvalho Chehab <mchehab@kernel.org>]

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