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@xxxxxxxxxxxxxxx>"); >>>> +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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel