[PATCH v2 07/12] drm: bridge: samsung-dsim: Add module init, exit

[Marek Szyprowski]

On 04.05.2022 13:40, Jagan Teki wrote:
> Add module init and exit functions for the bridge to register
> and unregister dsi_driver.
>
> Exynos drm driver stack will register the platform_driver separately
> in the common of it's exynos_drm_drv.c including dsi_driver.
>
> Register again would return -EBUSY, so return 0 for such cases as
> dsi_driver is already registered.
>
> v2, v1:
> * none
>
> Signed-off-by: Jagan Teki <jagan at amarulasolutions.com>
> ---
>   drivers/gpu/drm/bridge/samsung-dsim.c | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> index 8f9ae16d45bc..b618e52d0ee3 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -1740,6 +1740,28 @@ struct platform_driver dsi_driver = {
>   	},
>   };
>   
> +static int __init samsung_mipi_dsim_init(void)
> +{
> +	int ret;
> +
> +	ret = platform_driver_register(&dsi_driver);
> +
> +	/**
> +	 * Exynos drm driver stack will register the platform_driver
> +	 * separately in the common of it's exynos_drm_drv.c including
> +	 * dsi_driver. Register again would return -EBUSY, so return 0
> +	 * for such cases as dsi_driver is already registered.
> +	 */
> +	return ret == -EBUSY ? 0 : ret;
> +}
> +module_init(samsung_mipi_dsim_init);

I've just noticed this. The above approach is really a bad pattern: 
registering the same driver 2 times and relying on the error.

This gives the following error on Exynos boards:

Error: Driver 'samsung-dsim' is already registered, aborting...

which a bit misleading, because it is assumed that this will be ok.

This will also break if one compile it as modules, because the driver 
operation will depend on the order of module loading (and Exynos DSI 
won't be able to load as a second 'driver').

However the most important issue with such pattern is lack of 
multi-platform support (used usually by generic distros). One would not 
be able to compile a kernel with both Exynos and IMX support built-in. 
New drivers should really follow the multi-platform friendly patterns.


> +
> +static void __exit samsung_mipi_dsim_exit(void)
> +{
> +	platform_driver_unregister(&dsi_driver);
> +}
> +module_exit(samsung_mipi_dsim_exit);
> +
>   MODULE_AUTHOR("Jagan Teki <jagan at amarulasolutions.com>");
>   MODULE_DESCRIPTION("Samsung MIPI DSIM controller bridge");
>   MODULE_LICENSE("GPL");

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland