On 08/02/2017 03:14 PM, Laurent Pinchart wrote:
Hi Hans,
On Wednesday 02 Aug 2017 08:47:23 Hans Verkuil wrote:
On 08/02/2017 12:32 AM, Laurent Pinchart wrote:
- cec_register_cec_notifier(cec->adap, cec->notify);
- return 0;
+}
+static int dw_hdmi_cec_remove(struct platform_device *pdev) +{
- struct dw_hdmi_cec *cec = platform_get_drvdata(pdev);
- cec_unregister_adapter(cec->adap);
- cec_notifier_put(cec->notify);
- return 0;
+}
+static struct platform_driver dw_hdmi_cec_driver = {
- .probe = dw_hdmi_cec_probe,
- .remove = dw_hdmi_cec_remove,
- .driver = {
.name = "dw-hdmi-cec",- },
+}; +module_platform_driver(dw_hdmi_cec_driver);
Is there a particular reason why this has to be a separate module instead of simply calling the CEC init/cleanup functions directly from the main dw-hdmi driver ?
Not all SoCs that use dw-hdmi also use the dw-hdmi CEC implementation. Some use their own implementation (amlogic).
Lovely. Of course we need to reinvent the wheel every time, where would the fun be otherwise ?
So by implementing the cec-notifier in the dw-hdmi driver and keeping dw- hdmi CEC separate you can easily choose whether or not you want to use this CEC driver or another SoC CEC driver.
I'm certainly fine with such a split, but I don't think it requires a separate platform_driver. We could use a similar approach as with the HDMI PHY that can also differ between SoCs. The PHY is identified at runtime when possible, and the SoC-specific glue code can override that with a few data fields and function pointers.
The HDMI CEC is in the Synopsys IP like the I2S or AHB Audio, and is optional. So it's legitimate to handle it in a similar fashion. Amlogic has a separate custom CEC implementation, having an optional driver makes it possible to avoid loading unnecessary code like the AHB audio driver for non-freescale platforms.
+MODULE_AUTHOR("Russell King rmk+kernel@armlinux.org.uk"); +MODULE_DESCRIPTION("Synopsys Designware HDMI CEC driver for i.MX"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS(PLATFORM_MODULE_PREFIX "dw-hdmi-cec");
Yes let's get this merged !
Neil