Re: [PATCH 2/5] drm: move drm_class into drm_sysfs.c

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Sep 09, 2015 at 02:21:30PM +0200, David Herrmann wrote:
> Right now, drm_sysfs_create() returns the newly allocated "struct class"
> to the caller (which is drm_core_init()), which then has to set the
> global variable 'drm_class'. During cleanup, though, we call
> drm_sysfs_destroy() which implicitly uses the global 'drm_class'. This is
> confusing, as ownership of the global 'drm_class' is non-obvious.
> 
> This patch changes drm_sysfs_create() to drm_sysfs_init() and makes it
> initialize the 'drm_class' object directly, rather than returning it.
> This way, both drm_sysfs_init() and drm_sysfs_destroy() work in a similar
> fashion and manage the global drm class.
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

First 2 patches applied to drm-misc, thanks.
-Daniel

> ---
>  drivers/gpu/drm/drm_drv.c      |  6 ++----
>  drivers/gpu/drm/drm_internal.h |  2 +-
>  drivers/gpu/drm/drm_sysfs.c    | 47 +++++++++++++++++++-----------------------
>  3 files changed, 24 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 53d09a1..58299f7 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -55,7 +55,6 @@ module_param_named(debug, drm_debug, int, 0600);
>  static DEFINE_SPINLOCK(drm_minor_lock);
>  static struct idr drm_minors_idr;
>  
> -struct class *drm_class;
>  static struct dentry *drm_debugfs_root;
>  
>  void drm_err(const char *format, ...)
> @@ -839,10 +838,9 @@ static int __init drm_core_init(void)
>  	if (register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops))
>  		goto err_p1;
>  
> -	drm_class = drm_sysfs_create(THIS_MODULE, "drm");
> -	if (IS_ERR(drm_class)) {
> +	ret = drm_sysfs_init();
> +	if (ret < 0) {
>  		printk(KERN_ERR "DRM: Error creating drm class.\n");
> -		ret = PTR_ERR(drm_class);
>  		goto err_p2;
>  	}
>  
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index 059af01..43cbda3 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -73,7 +73,7 @@ int drm_authmagic(struct drm_device *dev, void *data,
>  /* drm_sysfs.c */
>  extern struct class *drm_class;
>  
> -struct class *drm_sysfs_create(struct module *owner, char *name);
> +int drm_sysfs_init(void);
>  void drm_sysfs_destroy(void);
>  struct device *drm_sysfs_minor_alloc(struct drm_minor *minor);
>  int drm_sysfs_connector_add(struct drm_connector *connector);
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 3f66cb0..f08873f 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -30,6 +30,8 @@ static struct device_type drm_sysfs_device_minor = {
>  	.name = "drm_minor"
>  };
>  
> +struct class *drm_class;
> +
>  /**
>   * __drm_class_suspend - internal DRM class suspend routine
>   * @dev: Linux device to suspend
> @@ -112,41 +114,34 @@ static CLASS_ATTR_STRING(version, S_IRUGO,
>  		CORE_DATE);
>  
>  /**
> - * drm_sysfs_create - create a struct drm_sysfs_class structure
> - * @owner: pointer to the module that is to "own" this struct drm_sysfs_class
> - * @name: pointer to a string for the name of this class.
> + * drm_sysfs_init - initialize sysfs helpers
> + *
> + * This is used to create the DRM class, which is the implicit parent of any
> + * other top-level DRM sysfs objects.
>   *
> - * This is used to create DRM class pointer that can then be used
> - * in calls to drm_sysfs_device_add().
> + * You must call drm_sysfs_destroy() to release the allocated resources.
>   *
> - * Note, the pointer created here is to be destroyed when finished by making a
> - * call to drm_sysfs_destroy().
> + * Return: 0 on success, negative error code on failure.
>   */
> -struct class *drm_sysfs_create(struct module *owner, char *name)
> +int drm_sysfs_init(void)
>  {
> -	struct class *class;
>  	int err;
>  
> -	class = class_create(owner, name);
> -	if (IS_ERR(class)) {
> -		err = PTR_ERR(class);
> -		goto err_out;
> -	}
> -
> -	class->pm = &drm_class_dev_pm_ops;
> +	drm_class = class_create(THIS_MODULE, "drm");
> +	if (IS_ERR(drm_class))
> +		return PTR_ERR(drm_class);
>  
> -	err = class_create_file(class, &class_attr_version.attr);
> -	if (err)
> -		goto err_out_class;
> +	drm_class->pm = &drm_class_dev_pm_ops;
>  
> -	class->devnode = drm_devnode;
> -
> -	return class;
> +	err = class_create_file(drm_class, &class_attr_version.attr);
> +	if (err) {
> +		class_destroy(drm_class);
> +		drm_class = NULL;
> +		return err;
> +	}
>  
> -err_out_class:
> -	class_destroy(class);
> -err_out:
> -	return ERR_PTR(err);
> +	drm_class->devnode = drm_devnode;
> +	return 0;
>  }
>  
>  /**
> -- 
> 2.5.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel