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

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@amarulasolutions.com>
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

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@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@amarulasolutions.com>");
>   MODULE_DESCRIPTION("Samsung MIPI DSIM controller bridge");
>   MODULE_LICENSE("GPL");

Best regards

On Mon, May 9, 2022 at 5:35 PM Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
>
> 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@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.

If it tries to register 2nd time, then it returns EBUSY so we are
returning 0 for that case. not sure why it registers 2nd time again.

Jagan.