| Message ID | 1441801293-1440-3-git-send-email-dh.herrmann@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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; } /**
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> --- 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(-)