On 29.11.2016 22:12, Jyri Sarha wrote:
> Store the module that provides the bridge and adjust its refcount
> accordingly. The bridge module unload should not be allowed while the
> bridge is attached.
Module refcounting as a way of preventing driver from unbinding
is quite popular, but as other developers said already is incorrect
solution - it does not really prevent from unbinding and is a hack.
>
> Signed-off-by: Jyri Sarha <jsarha@xxxxxx>
> ---
> drivers/gpu/drm/drm_bridge.c | 15 ++++++++++++---
> include/drm/drm_bridge.h | 11 ++++++++++-
> 2 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 0ee052b..36d427b 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -61,22 +61,26 @@
> static LIST_HEAD(bridge_list);
>
> /**
> - * drm_bridge_add - add the given bridge to the global bridge list
> + * __drm_bridge_add - add the given bridge to the global bridge list
> *
> * @bridge: bridge control structure
> + * @module: module that provides this bridge
> *
> * RETURNS:
> * Unconditionally returns Zero.
> */
> -int drm_bridge_add(struct drm_bridge *bridge)
> +int __drm_bridge_add(struct drm_bridge *bridge, struct module *module)
> {
> +#ifdef MODULE
> + bridge->module = module;
> +#endif
> mutex_lock(&bridge_lock);
> list_add_tail(&bridge->list, &bridge_list);
> mutex_unlock(&bridge_lock);
>
> return 0;
> }
> -EXPORT_SYMBOL(drm_bridge_add);
> +EXPORT_SYMBOL(__drm_bridge_add);
>
> /**
> * drm_bridge_remove - remove the given bridge from the global bridge list
> @@ -114,6 +118,9 @@ int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge)
> if (bridge->dev)
> return -EBUSY;
>
> + if (!try_module_get(bridge->module))
> + return -ENOENT;
> +
module field can be missing, will it compile in such case?
> bridge->dev = dev;
>
> if (bridge->funcs->attach)
> @@ -144,6 +151,8 @@ void drm_bridge_detach(struct drm_bridge *bridge)
> if (bridge->funcs->detach)
> bridge->funcs->detach(bridge);
>
> + module_put(bridge->module);
> +
> bridge->dev = NULL;
> }
> EXPORT_SYMBOL(drm_bridge_detach);
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 530a1d6..d60d5d2 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -25,6 +25,7 @@
>
> #include <linux/list.h>
> #include <linux/ctype.h>
> +#include <linux/module.h>
> #include <drm/drm_mode_object.h>
> #include <drm/drm_modes.h>
>
> @@ -192,13 +193,21 @@ struct drm_bridge {
> #ifdef CONFIG_OF
> struct device_node *of_node;
> #endif
> +#ifdef MODULE
> + struct module *module;
> +#endif
> struct list_head list;
>
> const struct drm_bridge_funcs *funcs;
> void *driver_private;
> };
>
> -int drm_bridge_add(struct drm_bridge *bridge);
> +int __drm_bridge_add(struct drm_bridge *bridge, struct module *module);
> +#ifdef MODULE
> +#define drm_bridge_add(bridge) __drm_bridge_add(bridge, THIS_MODULE)
> +#else
> +#define drm_bridge_add(bridge) __drm_bridge_add(bridge, NULL)
> +#endif
If I remember correctly THIS_MODULE is NULL if MODULE is undefined.
So the whole #ifdef here is unnecessary.
Regards
Andrzej
> void drm_bridge_remove(struct drm_bridge *bridge);
> struct drm_bridge *of_drm_find_bridge(struct device_node *np);
> int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge);
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
