staging: comedi: refactor sysfs files in comedi_fops.c

PATCH In review H Hartley Sweeten 2 participants 3 messages May 9, 2012 – May 10, 2012 Staging

Share & tools

H Hartley Sweeten

author

2012-05-09 16:20 UTC #0 lore raw

PATCH v2 1/1 staging: comedi: refactor sysfs files in comedi_fops.c

Refactor the sysfs attributes and functions to remove the need for the forward declarations and use the DEVICE_ATTR macro to define them.

Instead of individually creating sysfs device attribute files, wrap them in an attribute_group and use the sysfs_create_group function to create them.

Signed-off-by H Hartley Sweeten <[email protected]>

Cc Ian Abbott <[email protected]>

Cc Mori Hess <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

---

v2: fix a copy-paste error in comedi_alloc_subdevice_minor

drivers/staging/comedi/comedi_fops.c +294 ~~-365~~

index 06fc656..44ca1fe 100644
@@ -129,15 +129,295 @@ static int do_cancel(struct comedi_device *dev, struct comedi_subdevice *s);
 static int comedi_fasync(int fd, struct file *file, int on);
 
 static int is_device_busy(struct comedi_device *dev);
+
 static int resize_async_buffer(struct comedi_device *dev,
 			       struct comedi_subdevice *s,
-			       struct comedi_async *async, unsigned new_size);
+			       struct comedi_async *async, unsigned new_size)
+{
+	int retval;
 
-/* declarations for sysfs attribute files */
-static struct device_attribute dev_attr_max_read_buffer_kb;
-static struct device_attribute dev_attr_read_buffer_kb;
-static struct device_attribute dev_attr_max_write_buffer_kb;
-static struct device_attribute dev_attr_write_buffer_kb;
+	if (new_size > async->max_bufsize)
+		return -EPERM;
+
+	if (s->busy) {
+		DPRINTK("subdevice is busy, cannot resize buffer\n");
+		return -EBUSY;
+	}
+	if (async->mmap_count) {
+		DPRINTK("subdevice is mmapped, cannot resize buffer\n");
+		return -EBUSY;
+	}
+
+	if (!async->prealloc_buf)
+		return -EINVAL;
+
+	/* make sure buffer is an integral number of pages
+	 * (we round up) */
+	new_size = (new_size + PAGE_SIZE - 1) & PAGE_MASK;
+
+	retval = comedi_buf_alloc(dev, s, new_size);
+	if (retval < 0)
+		return retval;
+
+	if (s->buf_change) {
+		retval = s->buf_change(dev, s, new_size);
+		if (retval < 0)
+			return retval;
+	}
+
+	DPRINTK("comedi%i subd %d buffer resized to %i bytes\n",
+		dev->minor, (int)(s - dev->subdevices), async->prealloc_bufsz);
+	return 0;
+}
+
+/* sysfs attribute files */
+
+static const unsigned bytes_per_kibi = 1024;
+
+static ssize_t show_max_read_buffer_kb(struct device *dev,
+				       struct device_attribute *attr, char *buf)
+{
+	ssize_t retval;
+	struct comedi_device_file_info *info = dev_get_drvdata(dev);
+	unsigned max_buffer_size_kb = 0;
+	struct comedi_subdevice *const read_subdevice =
+	    comedi_get_read_subdevice(info);
+
+	mutex_lock(&info->device->mutex);
+	if (read_subdevice &&
+	    (read_subdevice->subdev_flags & SDF_CMD_READ) &&
+	    read_subdevice->async) {
+		max_buffer_size_kb = read_subdevice->async->max_bufsize /
+		    bytes_per_kibi;
+	}
+	retval = snprintf(buf, PAGE_SIZE, "%i\n", max_buffer_size_kb);
+	mutex_unlock(&info->device->mutex);
+
+	return retval;
+}
+
+static ssize_t store_max_read_buffer_kb(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t count)
+{
+	struct comedi_device_file_info *info = dev_get_drvdata(dev);
+	unsigned int new_max_size_kb;
+	unsigned int new_max_size;
+	int ret;
+	struct comedi_subdevice *const read_subdevice =
+	    comedi_get_read_subdevice(info);
+
+	ret = kstrtouint(buf, 10, &new_max_size_kb);
+	if (ret)
+		return ret;
+	if (new_max_size_kb > (UINT_MAX / bytes_per_kibi))
+		return -EINVAL;
+	new_max_size = new_max_size_kb * bytes_per_kibi;
+
+	mutex_lock(&info->device->mutex);
+	if (read_subdevice == NULL ||
+	    (read_subdevice->subdev_flags & SDF_CMD_READ) == 0 ||
+	    read_subdevice->async == NULL) {
+		mutex_unlock(&info->device->mutex);
+		return -EINVAL;
+	}
+	read_subdevice->async->max_bufsize = new_max_size;
+	mutex_unlock(&info->device->mutex);
+
+	return count;
+}
+
+static DEVICE_ATTR(max_read_buffer_kb, S_IRUGO | S_IWUSR,
+		show_max_read_buffer_kb, store_max_read_buffer_kb);
+
+static ssize_t show_read_buffer_kb(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	ssize_t retval;
+	struct comedi_device_file_info *info = dev_get_drvdata(dev);
+	unsigned buffer_size_kb = 0;
+	struct comedi_subdevice *const read_subdevice =
+	    comedi_get_read_subdevice(info);
+
+	mutex_lock(&info->device->mutex);
+	if (read_subdevice &&
+	    (read_subdevice->subdev_flags & SDF_CMD_READ) &&
+	    read_subdevice->async) {
+		buffer_size_kb = read_subdevice->async->prealloc_bufsz /
+		    bytes_per_kibi;
+	}
+	retval = snprintf(buf, PAGE_SIZE, "%i\n", buffer_size_kb);
+	mutex_unlock(&info->device->mutex);
+
+	return retval;
+}
+
+static ssize_t store_read_buffer_kb(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t count)
+{
+	struct comedi_device_file_info *info = dev_get_drvdata(dev);
+	unsigned int new_size_kb;
+	unsigned int new_size;
+	int retval;
+	int ret;
+	struct comedi_subdevice *const read_subdevice =
+	    comedi_get_read_subdevice(info);
+
+	ret = kstrtouint(buf, 10, &new_size_kb);
+	if (ret)
+		return ret;
+	if (new_size_kb > (UINT_MAX / bytes_per_kibi))
+		return -EINVAL;
+	new_size = new_size_kb * bytes_per_kibi;
+
+	mutex_lock(&info->device->mutex);
+	if (read_subdevice == NULL ||
+	    (read_subdevice->subdev_flags & SDF_CMD_READ) == 0 ||
+	    read_subdevice->async == NULL) {
+		mutex_unlock(&info->device->mutex);
+		return -EINVAL;
+	}
+	retval = resize_async_buffer(info->device, read_subdevice,
+				     read_subdevice->async, new_size);
+	mutex_unlock(&info->device->mutex);
+
+	if (retval < 0)
+		return retval;
+	return count;
+}
+
+static DEVICE_ATTR(read_buffer_kb, S_IRUGO | S_IWUSR | S_IWGRP,
+		show_read_buffer_kb, store_read_buffer_kb);
+
+static ssize_t show_max_write_buffer_kb(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	ssize_t retval;
+	struct comedi_device_file_info *info = dev_get_drvdata(dev);
+	unsigned max_buffer_size_kb = 0;
+	struct comedi_subdevice *const write_subdevice =
+	    comedi_get_write_subdevice(info);
+
+	mutex_lock(&info->device->mutex);
+	if (write_subdevice &&
+	    (write_subdevice->subdev_flags & SDF_CMD_WRITE) &&
+	    write_subdevice->async) {
+		max_buffer_size_kb = write_subdevice->async->max_bufsize /
+		    bytes_per_kibi;
+	}
+	retval = snprintf(buf, PAGE_SIZE, "%i\n", max_buffer_size_kb);
+	mutex_unlock(&info->device->mutex);
+
+	return retval;
+}
+
+static ssize_t store_max_write_buffer_kb(struct device *dev,
+					 struct device_attribute *attr,
+					 const char *buf, size_t count)
+{
+	struct comedi_device_file_info *info = dev_get_drvdata(dev);
+	unsigned int new_max_size_kb;
+	unsigned int new_max_size;
+	int ret;
+	struct comedi_subdevice *const write_subdevice =
+	    comedi_get_write_subdevice(info);
+
+	ret = kstrtouint(buf, 10, &new_max_size_kb);
+	if (ret)
+		return ret;
+	if (new_max_size_kb > (UINT_MAX / bytes_per_kibi))
+		return -EINVAL;
+	new_max_size = new_max_size_kb * bytes_per_kibi;
+
+	mutex_lock(&info->device->mutex);
+	if (write_subdevice == NULL ||
+	    (write_subdevice->subdev_flags & SDF_CMD_WRITE) == 0 ||
+	    write_subdevice->async == NULL) {
+		mutex_unlock(&info->device->mutex);
+		return -EINVAL;
+	}
+	write_subdevice->async->max_bufsize = new_max_size;
+	mutex_unlock(&info->device->mutex);
+
+	return count;
+}
+
+static DEVICE_ATTR(max_write_buffer_kb, S_IRUGO | S_IWUSR,
+		show_max_write_buffer_kb, store_max_write_buffer_kb);
+
+static ssize_t show_write_buffer_kb(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	ssize_t retval;
+	struct comedi_device_file_info *info = dev_get_drvdata(dev);
+	unsigned buffer_size_kb = 0;
+	struct comedi_subdevice *const write_subdevice =
+	    comedi_get_write_subdevice(info);
+
+	mutex_lock(&info->device->mutex);
+	if (write_subdevice &&
+	    (write_subdevice->subdev_flags & SDF_CMD_WRITE) &&
+	    write_subdevice->async) {
+		buffer_size_kb = write_subdevice->async->prealloc_bufsz /
+		    bytes_per_kibi;
+	}
+	retval = snprintf(buf, PAGE_SIZE, "%i\n", buffer_size_kb);
+	mutex_unlock(&info->device->mutex);
+
+	return retval;
+}
+
+static ssize_t store_write_buffer_kb(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct comedi_device_file_info *info = dev_get_drvdata(dev);
+	unsigned int new_size_kb;
+	unsigned int new_size;
+	int retval;
+	int ret;
+	struct comedi_subdevice *const write_subdevice =
+	    comedi_get_write_subdevice(info);
+
+	ret = kstrtouint(buf, 10, &new_size_kb);
+	if (ret)
+		return ret;
+	if (new_size_kb > (UINT_MAX / bytes_per_kibi))
+		return -EINVAL;
+	new_size = ((uint64_t) new_size_kb) * bytes_per_kibi;
+
+	mutex_lock(&info->device->mutex);
+	if (write_subdevice == NULL ||
+	    (write_subdevice->subdev_flags & SDF_CMD_WRITE) == 0 ||
+	    write_subdevice->async == NULL) {
+		mutex_unlock(&info->device->mutex);
+		return -EINVAL;
+	}
+	retval = resize_async_buffer(info->device, write_subdevice,
+				     write_subdevice->async, new_size);
+	mutex_unlock(&info->device->mutex);
+
+	if (retval < 0)
+		return retval;
+	return count;
+}
+
+static DEVICE_ATTR(write_buffer_kb, S_IRUGO | S_IWUSR | S_IWGRP,
+		show_write_buffer_kb, store_write_buffer_kb);
+
+static struct attribute *comedi_attrs[] = {
+	&dev_attr_max_read_buffer_kb.attr,
+	&dev_attr_read_buffer_kb.attr,
+	&dev_attr_max_write_buffer_kb.attr,
+	&dev_attr_write_buffer_kb.attr,
+	NULL
+};
+
+static const struct attribute_group comedi_sysfs_files = {
+	.attrs	= comedi_attrs,
+};
 
 static long comedi_unlocked_ioctl(struct file *file, unsigned int cmd,
 				  unsigned long arg)

@@ -2251,42 +2531,15 @@ int comedi_alloc_board_minor(struct device *hardware_device)
 	if (!IS_ERR(csdev))
 		info->device->class_dev = csdev;
 	dev_set_drvdata(csdev, info);
-	retval = device_create_file(csdev, &dev_attr_max_read_buffer_kb);
+
+	retval = sysfs_create_group(&csdev->kobj, &comedi_sysfs_files);
 	if (retval) {
 		printk(KERN_ERR
-		       "comedi: "
-		       "failed to create sysfs attribute file \"%s\".\n",
-		       dev_attr_max_read_buffer_kb.attr.name);
-		comedi_free_board_minor(i);
-		return retval;
-	}
-	retval = device_create_file(csdev, &dev_attr_read_buffer_kb);
-	if (retval) {
-		printk(KERN_ERR
-		       "comedi: "
-		       "failed to create sysfs attribute file \"%s\".\n",
-		       dev_attr_read_buffer_kb.attr.name);
-		comedi_free_board_minor(i);
-		return retval;
-	}
-	retval = device_create_file(csdev, &dev_attr_max_write_buffer_kb);
-	if (retval) {
-		printk(KERN_ERR
-		       "comedi: "
-		       "failed to create sysfs attribute file \"%s\".\n",
-		       dev_attr_max_write_buffer_kb.attr.name);
-		comedi_free_board_minor(i);
-		return retval;
-	}
-	retval = device_create_file(csdev, &dev_attr_write_buffer_kb);
-	if (retval) {
-		printk(KERN_ERR
-		       "comedi: "
-		       "failed to create sysfs attribute file \"%s\".\n",
-		       dev_attr_write_buffer_kb.attr.name);
+		       "comedi: failed to create sysfs attribute files\n");
 		comedi_free_board_minor(i);
 		return retval;
 	}
+
 	return i;
 }

@@ -2367,42 +2620,15 @@ int comedi_alloc_subdevice_minor(struct comedi_device *dev,
 	if (!IS_ERR(csdev))
 		s->class_dev = csdev;
 	dev_set_drvdata(csdev, info);
-	retval = device_create_file(csdev, &dev_attr_max_read_buffer_kb);
+
+	retval = sysfs_create_group(&csdev->kobj, &comedi_sysfs_files);
 	if (retval) {
 		printk(KERN_ERR
-		       "comedi: "
-		       "failed to create sysfs attribute file \"%s\".\n",
-		       dev_attr_max_read_buffer_kb.attr.name);
-		comedi_free_subdevice_minor(s);
-		return retval;
-	}
-	retval = device_create_file(csdev, &dev_attr_read_buffer_kb);
-	if (retval) {
-		printk(KERN_ERR
-		       "comedi: "
-		       "failed to create sysfs attribute file \"%s\".\n",
-		       dev_attr_read_buffer_kb.attr.name);
-		comedi_free_subdevice_minor(s);
-		return retval;
-	}
-	retval = device_create_file(csdev, &dev_attr_max_write_buffer_kb);
-	if (retval) {
-		printk(KERN_ERR
-		       "comedi: "
-		       "failed to create sysfs attribute file \"%s\".\n",
-		       dev_attr_max_write_buffer_kb.attr.name);
-		comedi_free_subdevice_minor(s);
-		return retval;
-	}
-	retval = device_create_file(csdev, &dev_attr_write_buffer_kb);
-	if (retval) {
-		printk(KERN_ERR
-		       "comedi: "
-		       "failed to create sysfs attribute file \"%s\".\n",
-		       dev_attr_write_buffer_kb.attr.name);
+		       "comedi: failed to create sysfs attribute files\n");
 		comedi_free_subdevice_minor(s);
 		return retval;
 	}
+
 	return i;
 }

@@ -2441,300 +2667,3 @@ struct comedi_device_file_info *comedi_get_device_file_info(unsigned minor)
 	return info;
 }
 EXPORT_SYMBOL_GPL(comedi_get_device_file_info);
-
-static int resize_async_buffer(struct comedi_device *dev,
-			       struct comedi_subdevice *s,
-			       struct comedi_async *async, unsigned new_size)
-{
-	int retval;
-
-	if (new_size > async->max_bufsize)
-		return -EPERM;
-
-	if (s->busy) {
-		DPRINTK("subdevice is busy, cannot resize buffer\n");
-		return -EBUSY;
-	}
-	if (async->mmap_count) {
-		DPRINTK("subdevice is mmapped, cannot resize buffer\n");
-		return -EBUSY;
-	}
-
-	if (!async->prealloc_buf)
-		return -EINVAL;
-
-	/* make sure buffer is an integral number of pages
-	 * (we round up) */
-	new_size = (new_size + PAGE_SIZE - 1) & PAGE_MASK;
-
-	retval = comedi_buf_alloc(dev, s, new_size);
-	if (retval < 0)
-		return retval;
-
-	if (s->buf_change) {
-		retval = s->buf_change(dev, s, new_size);
-		if (retval < 0)
-			return retval;
-	}
-
-	DPRINTK("comedi%i subd %d buffer resized to %i bytes\n",
-		dev->minor, (int)(s - dev->subdevices), async->prealloc_bufsz);
-	return 0;
-}
-
-/* sysfs attribute files */
-
-static const unsigned bytes_per_kibi = 1024;
-
-static ssize_t show_max_read_buffer_kb(struct device *dev,
-				       struct device_attribute *attr, char *buf)
-{
-	ssize_t retval;
-	struct comedi_device_file_info *info = dev_get_drvdata(dev);
-	unsigned max_buffer_size_kb = 0;
-	struct comedi_subdevice *const read_subdevice =
-	    comedi_get_read_subdevice(info);
-
-	mutex_lock(&info->device->mutex);
-	if (read_subdevice &&
-	    (read_subdevice->subdev_flags & SDF_CMD_READ) &&
-	    read_subdevice->async) {
-		max_buffer_size_kb = read_subdevice->async->max_bufsize /
-		    bytes_per_kibi;
-	}
-	retval = snprintf(buf, PAGE_SIZE, "%i\n", max_buffer_size_kb);
-	mutex_unlock(&info->device->mutex);
-
-	return retval;
-}
-
-static ssize_t store_max_read_buffer_kb(struct device *dev,
-					struct device_attribute *attr,
-					const char *buf, size_t count)
-{
-	struct comedi_device_file_info *info = dev_get_drvdata(dev);
-	unsigned int new_max_size_kb;
-	unsigned int new_max_size;
-	int ret;
-	struct comedi_subdevice *const read_subdevice =
-	    comedi_get_read_subdevice(info);
-
-	ret = kstrtouint(buf, 10, &new_max_size_kb);
-	if (ret)
-		return ret;
-	if (new_max_size_kb > (UINT_MAX / bytes_per_kibi))
-		return -EINVAL;
-	new_max_size = new_max_size_kb * bytes_per_kibi;
-
-	mutex_lock(&info->device->mutex);
-	if (read_subdevice == NULL ||
-	    (read_subdevice->subdev_flags & SDF_CMD_READ) == 0 ||
-	    read_subdevice->async == NULL) {
-		mutex_unlock(&info->device->mutex);
-		return -EINVAL;
-	}
-	read_subdevice->async->max_bufsize = new_max_size;
-	mutex_unlock(&info->device->mutex);
-
-	return count;
-}
-
-static struct device_attribute dev_attr_max_read_buffer_kb = {
-	.attr = {
-		 .name = "max_read_buffer_kb",
-		 .mode = S_IRUGO | S_IWUSR},
-	.show = &show_max_read_buffer_kb,
-	.store = &store_max_read_buffer_kb
-};
-
-static ssize_t show_read_buffer_kb(struct device *dev,
-				   struct device_attribute *attr, char *buf)
-{
-	ssize_t retval;
-	struct comedi_device_file_info *info = dev_get_drvdata(dev);
-	unsigned buffer_size_kb = 0;
-	struct comedi_subdevice *const read_subdevice =
-	    comedi_get_read_subdevice(info);
-
-	mutex_lock(&info->device->mutex);
-	if (read_subdevice &&
-	    (read_subdevice->subdev_flags & SDF_CMD_READ) &&
-	    read_subdevice->async) {
-		buffer_size_kb = read_subdevice->async->prealloc_bufsz /
-		    bytes_per_kibi;
-	}
-	retval = snprintf(buf, PAGE_SIZE, "%i\n", buffer_size_kb);
-	mutex_unlock(&info->device->mutex);
-
-	return retval;
-}
-
-static ssize_t store_read_buffer_kb(struct device *dev,
-				    struct device_attribute *attr,
-				    const char *buf, size_t count)
-{
-	struct comedi_device_file_info *info = dev_get_drvdata(dev);
-	unsigned int new_size_kb;
-	unsigned int new_size;
-	int retval;
-	int ret;
-	struct comedi_subdevice *const read_subdevice =
-	    comedi_get_read_subdevice(info);
-
-	ret = kstrtouint(buf, 10, &new_size_kb);
-	if (ret)
-		return ret;
-	if (new_size_kb > (UINT_MAX / bytes_per_kibi))
-		return -EINVAL;
-	new_size = new_size_kb * bytes_per_kibi;
-
-	mutex_lock(&info->device->mutex);
-	if (read_subdevice == NULL ||
-	    (read_subdevice->subdev_flags & SDF_CMD_READ) == 0 ||
-	    read_subdevice->async == NULL) {
-		mutex_unlock(&info->device->mutex);
-		return -EINVAL;
-	}
-	retval = resize_async_buffer(info->device, read_subdevice,
-				     read_subdevice->async, new_size);
-	mutex_unlock(&info->device->mutex);
-
-	if (retval < 0)
-		return retval;
-	return count;
-}
-
-static struct device_attribute dev_attr_read_buffer_kb = {
-	.attr = {
-		 .name = "read_buffer_kb",
-		 .mode = S_IRUGO | S_IWUSR | S_IWGRP},
-	.show = &show_read_buffer_kb,
-	.store = &store_read_buffer_kb
-};
-
-static ssize_t show_max_write_buffer_kb(struct device *dev,
-					struct device_attribute *attr,
-					char *buf)
-{
-	ssize_t retval;
-	struct comedi_device_file_info *info = dev_get_drvdata(dev);
-	unsigned max_buffer_size_kb = 0;
-	struct comedi_subdevice *const write_subdevice =
-	    comedi_get_write_subdevice(info);
-
-	mutex_lock(&info->device->mutex);
-	if (write_subdevice &&
-	    (write_subdevice->subdev_flags & SDF_CMD_WRITE) &&
-	    write_subdevice->async) {
-		max_buffer_size_kb = write_subdevice->async->max_bufsize /
-		    bytes_per_kibi;
-	}
-	retval = snprintf(buf, PAGE_SIZE, "%i\n", max_buffer_size_kb);
-	mutex_unlock(&info->device->mutex);
-
-	return retval;
-}
-
-static ssize_t store_max_write_buffer_kb(struct device *dev,
-					 struct device_attribute *attr,
-					 const char *buf, size_t count)
-{
-	struct comedi_device_file_info *info = dev_get_drvdata(dev);
-	unsigned int new_max_size_kb;
-	unsigned int new_max_size;
-	int ret;
-	struct comedi_subdevice *const write_subdevice =
-	    comedi_get_write_subdevice(info);
-
-	ret = kstrtouint(buf, 10, &new_max_size_kb);
-	if (ret)
-		return ret;
-	if (new_max_size_kb > (UINT_MAX / bytes_per_kibi))
-		return -EINVAL;
-	new_max_size = new_max_size_kb * bytes_per_kibi;
-
-	mutex_lock(&info->device->mutex);
-	if (write_subdevice == NULL ||
-	    (write_subdevice->subdev_flags & SDF_CMD_WRITE) == 0 ||
-	    write_subdevice->async == NULL) {
-		mutex_unlock(&info->device->mutex);
-		return -EINVAL;
-	}
-	write_subdevice->async->max_bufsize = new_max_size;
-	mutex_unlock(&info->device->mutex);
-
-	return count;
-}
-
-static struct device_attribute dev_attr_max_write_buffer_kb = {
-	.attr = {
-		 .name = "max_write_buffer_kb",
-		 .mode = S_IRUGO | S_IWUSR},
-	.show = &show_max_write_buffer_kb,
-	.store = &store_max_write_buffer_kb
-};
-
-static ssize_t show_write_buffer_kb(struct device *dev,
-				    struct device_attribute *attr, char *buf)
-{
-	ssize_t retval;
-	struct comedi_device_file_info *info = dev_get_drvdata(dev);
-	unsigned buffer_size_kb = 0;
-	struct comedi_subdevice *const write_subdevice =
-	    comedi_get_write_subdevice(info);
-
-	mutex_lock(&info->device->mutex);
-	if (write_subdevice &&
-	    (write_subdevice->subdev_flags & SDF_CMD_WRITE) &&
-	    write_subdevice->async) {
-		buffer_size_kb = write_subdevice->async->prealloc_bufsz /
-		    bytes_per_kibi;
-	}
-	retval = snprintf(buf, PAGE_SIZE, "%i\n", buffer_size_kb);
-	mutex_unlock(&info->device->mutex);
-
-	return retval;
-}
-
-static ssize_t store_write_buffer_kb(struct device *dev,
-				     struct device_attribute *attr,
-				     const char *buf, size_t count)
-{
-	struct comedi_device_file_info *info = dev_get_drvdata(dev);
-	unsigned int new_size_kb;
-	unsigned int new_size;
-	int retval;
-	int ret;
-	struct comedi_subdevice *const write_subdevice =
-	    comedi_get_write_subdevice(info);
-
-	ret = kstrtouint(buf, 10, &new_size_kb);
-	if (ret)
-		return ret;
-	if (new_size_kb > (UINT_MAX / bytes_per_kibi))
-		return -EINVAL;
-	new_size = ((uint64_t) new_size_kb) * bytes_per_kibi;
-
-	mutex_lock(&info->device->mutex);
-	if (write_subdevice == NULL ||
-	    (write_subdevice->subdev_flags & SDF_CMD_WRITE) == 0 ||
-	    write_subdevice->async == NULL) {
-		mutex_unlock(&info->device->mutex);
-		return -EINVAL;
-	}
-	retval = resize_async_buffer(info->device, write_subdevice,
-				     write_subdevice->async, new_size);
-	mutex_unlock(&info->device->mutex);
-
-	if (retval < 0)
-		return retval;
-	return count;
-}
-
-static struct device_attribute dev_attr_write_buffer_kb = {
-	.attr = {
-		 .name = "write_buffer_kb",
-		 .mode = S_IRUGO | S_IWUSR | S_IWGRP},
-	.show = &show_write_buffer_kb,
-	.store = &store_write_buffer_kb
-};

Greg KH

maintainer

2012-05-09 20:39 UTC #1 lore raw

Quoted 7 lines · H Hartley Sweeten

Refactor the sysfs attributes and functions to remove the need for the forward declarations and use the DEVICE_ATTR macro to define them.
Instead of individually creating sysfs device attribute files, wrap them in an attribute_group and use the sysfs_create_group function to create them.

This is great, and needed to be done, but you need to take this one step further. This attribute group needs to be registered by the driver core, not the comedi core, so userspace doesn't get an add event before the files are created. To do that, assign this attribute group to the class and the core will handle it for you.

I've taken this patch, but will you make a follow-on patch that makes this change, which will remove this race? It should be a net removal of code in the comedi core overall.

thanks,

greg k-h

H Hartley Sweeten

author

2012-05-10 22:31 UTC #2 lore raw

Quoted 15 lines · Greg KH

Quoted 7 lines · H Hartley Sweeten
Refactor the sysfs attributes and functions to remove the need for the forward declarations and use the DEVICE_ATTR macro to define them.
Instead of individually creating sysfs device attribute files, wrap them in an attribute_group and use the sysfs_create_group function to create them.
This is great, and needed to be done, but you need to take this one step further. This attribute group needs to be registered by the driver core, not the comedi core, so userspace doesn't get an add event before the files are created. To do that, assign this attribute group to the class and the core will handle it for you.

Quoted 4 lines

I've taken this patch, but will you make a follow-on patch that makes this change, which will remove this race? It should be a net removal of code in the comedi core overall.

I think I worked this out. Compile testing it now and will post the patch shortly.

Regards,
Hartley