[PATCH 07/23] usb-gadget/f_loopback: use per-attribute show and store methods

Subsystems: the rest, usb gadget/peripheral subsystem, usb subsystem

STALE4045d

4 messages, 3 authors, 2015-09-28 · open the first message on its own page

[PATCH 07/23] usb-gadget/f_loopback: use per-attribute show and store methods

From: Christoph Hellwig <hidden>
Date: 2015-09-25 13:49:44

Signed-off-by: Christoph Hellwig <redacted>
---
 drivers/usb/gadget/function/f_loopback.c | 32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/gadget/function/f_loopback.c b/drivers/usb/gadget/function/f_loopback.c
index 6e2fe63..d4ef421 100644
--- a/drivers/usb/gadget/function/f_loopback.c
+++ b/drivers/usb/gadget/function/f_loopback.c

@@ -413,9 +413,6 @@ static inline struct f_lb_opts *to_f_lb_opts(struct config_item *item)
 			    func_inst.group);
 }
 
-CONFIGFS_ATTR_STRUCT(f_lb_opts);
-CONFIGFS_ATTR_OPS(f_lb_opts);
-
 static void lb_attr_release(struct config_item *item)
 {
 	struct f_lb_opts *lb_opts = to_f_lb_opts(item);

@@ -425,12 +422,11 @@ static void lb_attr_release(struct config_item *item)
 
 static struct configfs_item_operations lb_item_ops = {
 	.release		= lb_attr_release,
-	.show_attribute		= f_lb_opts_attr_show,
-	.store_attribute	= f_lb_opts_attr_store,
 };
 
-static ssize_t f_lb_opts_qlen_show(struct f_lb_opts *opts, char *page)
+static ssize_t f_lb_opts_qlen_show(struct config_item *item, char *page)
 {
+	struct f_lb_opts *opts = to_f_lb_opts(item);
 	int result;
 
 	mutex_lock(&opts->lock);

@@ -440,9 +436,10 @@ static ssize_t f_lb_opts_qlen_show(struct f_lb_opts *opts, char *page)
 	return result;
 }
 
-static ssize_t f_lb_opts_qlen_store(struct f_lb_opts *opts,
+static ssize_t f_lb_opts_qlen_store(struct config_item *item,
 				    const char *page, size_t len)
 {
+	struct f_lb_opts *opts = to_f_lb_opts(item);
 	int ret;
 	u32 num;

@@ -463,13 +460,9 @@ end:
 	return ret;
 }
 
-static struct f_lb_opts_attribute f_lb_opts_qlen =
-	__CONFIGFS_ATTR(qlen, S_IRUGO | S_IWUSR,
-			f_lb_opts_qlen_show,
-			f_lb_opts_qlen_store);
-
-static ssize_t f_lb_opts_bulk_buflen_show(struct f_lb_opts *opts, char *page)
+static ssize_t f_lb_opts_bulk_buflen_show(struct config_item *item, char *page)
 {
+	struct f_lb_opts *opts = to_f_lb_opts(item);
 	int result;
 
 	mutex_lock(&opts->lock);

@@ -479,9 +472,10 @@ static ssize_t f_lb_opts_bulk_buflen_show(struct f_lb_opts *opts, char *page)
 	return result;
 }
 
-static ssize_t f_lb_opts_bulk_buflen_store(struct f_lb_opts *opts,
+static ssize_t f_lb_opts_bulk_buflen_store(struct config_item *item,
 				    const char *page, size_t len)
 {
+	struct f_lb_opts *opts = to_f_lb_opts(item);
 	int ret;
 	u32 num;

@@ -502,14 +496,12 @@ end:
 	return ret;
 }
 
-static struct f_lb_opts_attribute f_lb_opts_bulk_buflen =
-	__CONFIGFS_ATTR(buflen, S_IRUGO | S_IWUSR,
-			f_lb_opts_bulk_buflen_show,
-			f_lb_opts_bulk_buflen_store);
+CONFIGFS_ATTR(f_lb_opts_, qlen);
+CONFIGFS_ATTR(f_lb_opts_, bulk_buflen);
 
 static struct configfs_attribute *lb_attrs[] = {
-	&f_lb_opts_qlen.attr,
-	&f_lb_opts_bulk_buflen.attr,
+	&f_lb_opts_attr_qlen,
+	&f_lb_opts_attr_bulk_buflen,
 	NULL,
 };

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/23] usb-gadget/f_loopback: use per-attribute show and store methods

From: Andrzej Pietrasiewicz <hidden>
Date: 2015-09-28 11:47:01

Hi Christoph,

Please see comments inline.

With the issue addressed you can add

Reviewed-by: Andrzej Pietrasiewicz <redacted>

W dniu 25.09.2015 o 15:49, Christoph Hellwig pisze:

quoted hunk

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
  drivers/usb/gadget/function/f_loopback.c | 32 ++++++++++++--------------------
  1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/gadget/function/f_loopback.c b/drivers/usb/gadget/function/f_loopback.c
index 6e2fe63..d4ef421 100644
--- a/drivers/usb/gadget/function/f_loopback.c
+++ b/drivers/usb/gadget/function/f_loopback.c

@@ -413,9 +413,6 @@ static inline struct f_lb_opts *to_f_lb_opts(struct config_item *item)
  			    func_inst.group);
  }

-CONFIGFS_ATTR_STRUCT(f_lb_opts);
-CONFIGFS_ATTR_OPS(f_lb_opts);
-
  static void lb_attr_release(struct config_item *item)
  {
  	struct f_lb_opts *lb_opts = to_f_lb_opts(item);

@@ -425,12 +422,11 @@ static void lb_attr_release(struct config_item *item)

  static struct configfs_item_operations lb_item_ops = {
  	.release		= lb_attr_release,
-	.show_attribute		= f_lb_opts_attr_show,
-	.store_attribute	= f_lb_opts_attr_store,
  };

-static ssize_t f_lb_opts_qlen_show(struct f_lb_opts *opts, char *page)
+static ssize_t f_lb_opts_qlen_show(struct config_item *item, char *page)
  {
+	struct f_lb_opts *opts = to_f_lb_opts(item);
  	int result;

  	mutex_lock(&opts->lock);

@@ -440,9 +436,10 @@ static ssize_t f_lb_opts_qlen_show(struct f_lb_opts *opts, char *page)
  	return result;
  }

-static ssize_t f_lb_opts_qlen_store(struct f_lb_opts *opts,
+static ssize_t f_lb_opts_qlen_store(struct config_item *item,

15 more quoted lines

In my opinion the below line belongs here:

+CONFIGFS_ATTR(f_lb_opts_, qlen);

quoted hunk

-static ssize_t f_lb_opts_bulk_buflen_show(struct f_lb_opts *opts, char *page)
+static ssize_t f_lb_opts_bulk_buflen_show(struct config_item *item, char *page)
  {
+	struct f_lb_opts *opts = to_f_lb_opts(item);
  	int result;

  	mutex_lock(&opts->lock);

@@ -479,9 +472,10 @@ static ssize_t f_lb_opts_bulk_buflen_show(struct f_lb_opts *opts, char *page)
  	return result;
  }

-static ssize_t f_lb_opts_bulk_buflen_store(struct f_lb_opts *opts,
+static ssize_t f_lb_opts_bulk_buflen_store(struct config_item *item,
  				    const char *page, size_t len)
  {
+	struct f_lb_opts *opts = to_f_lb_opts(item);
  	int ret;
  	u32 num;
@@ -502,14 +496,12 @@ end:
  	return ret;
  }

-static struct f_lb_opts_attribute f_lb_opts_bulk_buflen =
-	__CONFIGFS_ATTR(buflen, S_IRUGO | S_IWUSR,
-			f_lb_opts_bulk_buflen_show,
-			f_lb_opts_bulk_buflen_store);
+CONFIGFS_ATTR(f_lb_opts_, qlen);
+CONFIGFS_ATTR(f_lb_opts_, bulk_buflen);

Why group CONFIGFS_ATTR invocations if in the original code their
corresponding __CONFIGFS_ATTR invocations were where they were?

quoted hunk

  static struct configfs_attribute *lb_attrs[] = {
-	&f_lb_opts_qlen.attr,
-	&f_lb_opts_bulk_buflen.attr,
+	&f_lb_opts_attr_qlen,
+	&f_lb_opts_attr_bulk_buflen,
  	NULL,
  };

Re: [PATCH 07/23] usb-gadget/f_loopback: use per-attribute show and store methods

From: Christoph Hellwig <hch@lst.de>
Date: 2015-09-28 13:41:01

On Mon, Sep 28, 2015 at 01:46:57PM +0200, Andrzej Pietrasiewicz wrote:

quoted

  }

-static struct f_lb_opts_attribute f_lb_opts_qlen =
-	__CONFIGFS_ATTR(qlen, S_IRUGO | S_IWUSR,
-			f_lb_opts_qlen_show,
-			f_lb_opts_qlen_store);
-

In my opinion the below line belongs here:

+CONFIGFS_ATTR(f_lb_opts_, qlen);

The idea is to keep all the attribute defintions near the attribute
array, similar to how most drivers define their sysfs attributes.

If you really don't like that way I'll move it back.

Re: [PATCH 07/23] usb-gadget/f_loopback: use per-attribute show and store methods

From: Andrzej Pietrasiewicz <hidden>
Date: 2015-09-28 13:52:50

Hi,

W dniu 28.09.2015 o 15:41, Christoph Hellwig pisze:

On Mon, Sep 28, 2015 at 01:46:57PM +0200, Andrzej Pietrasiewicz wrote:

quoted

   }

-static struct f_lb_opts_attribute f_lb_opts_qlen =
-	__CONFIGFS_ATTR(qlen, S_IRUGO | S_IWUSR,
-			f_lb_opts_qlen_show,
-			f_lb_opts_qlen_store);
-

In my opinion the below line belongs here:

+CONFIGFS_ATTR(f_lb_opts_, qlen);

The idea is to keep all the attribute defintions near the attribute
array, similar to how most drivers define their sysfs attributes.

You were not consistent with the approach, though: with some
patches CONFIGFS_ATTR() are where __CONFIGFS_ATTR were,
with some other they are grouped.

If you really don't like that way I'll move it back.

I think the change is more explicit/readable if the macro invocations
are where what they substitute used to be.

AP

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help