Messages in this thread | | | Date | Wed, 2 Nov 2022 15:17:41 -0600 | Subject | Re: [RFC PATCH v2 2/3] accel: add dedicated minor for accelerator devices | From | Jeffrey Hugo <> |
| |
On 11/2/2022 2:34 PM, Oded Gabbay wrote: > @@ -24,16 +33,6 @@ static char *accel_devnode(struct device *dev, umode_t *mode) > > static CLASS_ATTR_STRING(accel_version, 0444, "accel 1.0.0 20221018"); > > -/** > - * accel_sysfs_init - initialize sysfs helpers > - * > - * This is used to create the ACCEL class, which is the implicit parent of any > - * other top-level ACCEL sysfs objects. > - * > - * You must call accel_sysfs_destroy() to release the allocated resources. > - * > - * Return: 0 on success, negative error code on failure. > - */
Why are we removing this?
> static int accel_sysfs_init(void) > { > int err; > @@ -54,11 +53,6 @@ static int accel_sysfs_init(void) > return 0; > } > > -/** > - * accel_sysfs_destroy - destroys ACCEL class > - * > - * Destroy the ACCEL device class. > - */
Again, why remove this? Adding it in one patch than immediately removing it in the next patch seems wasteful.
> static void accel_sysfs_destroy(void) > { > if (IS_ERR_OR_NULL(accel_class)) > @@ -68,11 +62,185 @@ static void accel_sysfs_destroy(void) > accel_class = NULL; > } > > +static void accel_minor_release(struct drm_minor *minor) > +{ > + drm_dev_put(minor->dev); > +} > + > +/** > + * accel_open - open method for ACCEL file > + * @inode: device inode > + * @filp: file pointer. > + * > + * This function must be used by drivers as their &file_operations.open method.
Feels like it would be helpful to have an accel version of DEFINE_DRM_GEM_FOPS() which helps accel drivers to get this right
> + * It looks up the correct ACCEL device and instantiates all the per-file > + * resources for it. It also calls the &drm_driver.open driver callback. > + * > + * Return: 0 on success or negative errno value on failure. > + */ > +int accel_open(struct inode *inode, struct file *filp) > +{ > + struct drm_device *dev; > + struct drm_minor *minor; > + int retcode; > + > + minor = accel_minor_acquire(iminor(inode)); > + if (IS_ERR(minor)) > + return PTR_ERR(minor); > + > + dev = minor->dev; > + > + atomic_fetch_inc(&dev->open_count); > + > + /* share address_space across all char-devs of a single device */ > + filp->f_mapping = dev->anon_inode->i_mapping; > + > + retcode = drm_open_helper(filp, minor); > + if (retcode) > + goto err_undo; > + > + return 0; > + > +err_undo: > + atomic_dec(&dev->open_count); > + accel_minor_release(minor); > + return retcode; > +} > +EXPORT_SYMBOL_GPL(accel_open);
| |