[PATCH v3 0/3] usb: gadget: configfs: add some trace event

[PATCH v3 0/3] usb: gadget: configfs: add some trace event

[Linyu Yuan]

this series make some minor change to gadget configfs
which allow common trace from configfs layer.

follow suggestion from Felipe Balbi in link below,
https://lore.kernel.org/linux-usb/1629777281-30188-1-git-send-email-quic_linyyuan@quicinc.com/

v2: fix two issue Reported-by: kernel test robot <lkp@intel.com>
v3: do not move private structure to configfs.h

Linyu Yuan (3):
  usb: gadget: configfs: avoid list move operation of usb_function
  usb: gadget: configfs: add gadget_info for config group
  usb: gadget: configfs: add some trace event

 drivers/usb/gadget/configfs.c | 114 +++++++++++++++++++++++++++++++++---------
 1 file changed, 89 insertions(+), 25 deletions(-)

-- 
2.7.4

[PATCH v3 1/3] usb: gadget: configfs: avoid list move operation of usb_function

[Linyu Yuan]

add a new list which link all usb_function at configfs layers,
it means that after link a function a configuration,
from configfs layer, we can still found all functions,
it will allow trace all functions from configfs.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
---
v2: fix unused cfg variable warning
v3: add struct inside configfs.c

 drivers/usb/gadget/configfs.c | 52 ++++++++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 477e72a..88d6bcc 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -58,6 +58,11 @@ static inline struct gadget_info *to_gadget_info(struct config_item *item)
 	return container_of(to_config_group(item), struct gadget_info, group);
 }
 
+struct config_usb_function {
+	struct list_head list;
+	struct usb_function *f;
+};
+
 struct config_usb_cfg {
 	struct config_group group;
 	struct config_group strings_group;
@@ -420,7 +425,7 @@ static int config_usb_cfg_link(
 	struct usb_function_instance *fi = container_of(group,
 			struct usb_function_instance, group);
 	struct usb_function_instance *a_fi;
-	struct usb_function *f;
+	struct config_usb_function *cf;
 	int ret;
 
 	mutex_lock(&gi->lock);
@@ -438,21 +443,29 @@ static int config_usb_cfg_link(
 		goto out;
 	}
 
-	list_for_each_entry(f, &cfg->func_list, list) {
-		if (f->fi == fi) {
+	list_for_each_entry(cf, &cfg->func_list, list) {
+		if (cf->f->fi == fi) {
 			ret = -EEXIST;
 			goto out;
 		}
 	}
 
-	f = usb_get_function(fi);
-	if (IS_ERR(f)) {
-		ret = PTR_ERR(f);
+	cf = kzalloc(sizeof(*cf), GFP_KERNEL);
+	if (!cf) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	INIT_LIST_HEAD(&cf->list);
+
+	cf->f = usb_get_function(fi);
+	if (IS_ERR(cf->f)) {
+		ret = PTR_ERR(cf->f);
+		kfree(cf);
 		goto out;
 	}
 
 	/* stash the function until we bind it to the gadget */
-	list_add_tail(&f->list, &cfg->func_list);
+	list_add_tail(&cf->list, &cfg->func_list);
 	ret = 0;
 out:
 	mutex_unlock(&gi->lock);
@@ -470,7 +483,7 @@ static void config_usb_cfg_unlink(
 	struct config_group *group = to_config_group(usb_func_ci);
 	struct usb_function_instance *fi = container_of(group,
 			struct usb_function_instance, group);
-	struct usb_function *f;
+	struct config_usb_function *cf;
 
 	/*
 	 * ideally I would like to forbid to unlink functions while a gadget is
@@ -483,10 +496,11 @@ static void config_usb_cfg_unlink(
 		unregister_gadget(gi);
 	WARN_ON(gi->composite.gadget_driver.udc_name);
 
-	list_for_each_entry(f, &cfg->func_list, list) {
-		if (f->fi == fi) {
-			list_del(&f->list);
-			usb_put_function(f);
+	list_for_each_entry(cf, &cfg->func_list, list) {
+		if (cf->f->fi == fi) {
+			list_del(&cf->list);
+			usb_put_function(cf->f);
+			kfree(cf);
 			mutex_unlock(&gi->lock);
 			return;
 		}
@@ -1263,7 +1277,7 @@ static void purge_configs_funcs(struct gadget_info *gi)
 
 		list_for_each_entry_safe_reverse(f, tmp, &c->functions, list) {
 
-			list_move(&f->list, &cfg->func_list);
+			list_del(&f->list);
 			if (f->unbind) {
 				dev_dbg(&gi->cdev.gadget->dev,
 					"unbind function '%s'/%p\n",
@@ -1371,8 +1385,7 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
 	/* Go through all configs, attach all functions */
 	list_for_each_entry(c, &gi->cdev.configs, list) {
 		struct config_usb_cfg *cfg;
-		struct usb_function *f;
-		struct usb_function *tmp;
+		struct config_usb_function *cf, *tmp;
 		struct gadget_config_name *cn;
 
 		if (gadget_is_otg(gadget))
@@ -1396,13 +1409,10 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
 			c->iConfiguration = s[0].id;
 		}
 
-		list_for_each_entry_safe(f, tmp, &cfg->func_list, list) {
-			list_del(&f->list);
-			ret = usb_add_function(c, f);
-			if (ret) {
-				list_add(&f->list, &cfg->func_list);
+		list_for_each_entry_safe(cf, tmp, &cfg->func_list, list) {
+			ret = usb_add_function(c, cf->f);
+			if (ret)
 				goto err_purge_funcs;
-			}
 		}
 		ret = usb_gadget_check_config(cdev->gadget);
 		if (ret)
-- 
2.7.4

[PATCH v3 2/3] usb: gadget: configfs: add gadget_info for config group

[Linyu Yuan]

add gadget_info pointer in struct config_usb_cfg
to allow common gadget info trace from configfs layer.

Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
---
v2: no change
v3: change struct inside configfs.c

 drivers/usb/gadget/configfs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 88d6bcc..f766756 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -70,6 +70,7 @@ struct config_usb_cfg {
 	struct usb_configuration c;
 	struct list_head func_list;
 	struct usb_gadget_strings *gstrings[MAX_USB_STRING_LANGS + 1];
+	struct gadget_info *gi;
 };
 
 static inline struct config_usb_cfg *to_config_usb_cfg(struct config_item *item)
@@ -418,8 +419,7 @@ static int config_usb_cfg_link(
 	struct config_item *usb_func_ci)
 {
 	struct config_usb_cfg *cfg = to_config_usb_cfg(usb_cfg_ci);
-	struct usb_composite_dev *cdev = cfg->c.cdev;
-	struct gadget_info *gi = container_of(cdev, struct gadget_info, cdev);
+	struct gadget_info *gi = cfg->gi;
 
 	struct config_group *group = to_config_group(usb_func_ci);
 	struct usb_function_instance *fi = container_of(group,
@@ -477,8 +477,7 @@ static void config_usb_cfg_unlink(
 	struct config_item *usb_func_ci)
 {
 	struct config_usb_cfg *cfg = to_config_usb_cfg(usb_cfg_ci);
-	struct usb_composite_dev *cdev = cfg->c.cdev;
-	struct gadget_info *gi = container_of(cdev, struct gadget_info, cdev);
+	struct gadget_info *gi = cfg->gi;
 
 	struct config_group *group = to_config_group(usb_func_ci);
 	struct usb_function_instance *fi = container_of(group,
@@ -717,6 +716,7 @@ static struct config_group *config_desc_make(
 	cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
 	if (!cfg)
 		return ERR_PTR(-ENOMEM);
+	cfg->gi = gi;
 	cfg->c.label = kstrdup(buf, GFP_KERNEL);
 	if (!cfg->c.label) {
 		ret = -ENOMEM;
-- 
2.7.4

[PATCH v3 3/3] usb: gadget: configfs: add some trace event

[Linyu Yuan]

add UDC, cfg link/unlink and some attributes trace
to better trace gadget issues.

Suggested-by: Felipe Balbi <balbi@kernel.org>
Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
---
v3: build trace inside configfs.c

 drivers/usb/gadget/configfs.c | 54 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index f766756..509daf5 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -103,6 +103,42 @@ struct gadget_config_name {
 	struct list_head list;
 };
 
+#define MAX_CONFIGURAITON_STR_LEN	512
+
+static char *config_trace_string(struct gadget_info *gi)
+{
+	struct usb_configuration *uc;
+	struct config_usb_cfg *cfg;
+	struct config_usb_function *cf;
+	static char trs[MAX_CONFIGURAITON_STR_LEN];
+	size_t len = MAX_CONFIGURAITON_STR_LEN;
+	int n = 0;
+
+	trs[0] = '\0';
+
+	list_for_each_entry(uc, &gi->cdev.configs, list) {
+		cfg = container_of(uc, struct config_usb_cfg, c);
+
+		n += scnprintf(trs + n, len - n,
+			"group:%s,bConfigurationValue:%d,bmAttributes:%d,"
+			"MaxPower:%d,",
+			config_item_name(&cfg->group.cg_item),
+			uc->bConfigurationValue,
+			uc->bmAttributes,
+			uc->MaxPower);
+
+		n += scnprintf(trs + n, len - n, "function:[");
+		list_for_each_entry(cf, &cfg->func_list, list)
+			n += scnprintf(trs + n, len - n, "%s", cf->f->name);
+		n += scnprintf(trs + n, len - n, "},");
+	}
+
+	return trs;
+}
+
+#define CREATE_TRACE_POINTS
+#include "configfs_trace.h"
+
 #define USB_MAX_STRING_WITH_NULL_LEN	(USB_MAX_STRING_LEN+1)
 
 static int usb_string_copy(const char *s, char **s_copy)
@@ -210,6 +246,7 @@ static ssize_t gadget_dev_desc_bcdDevice_store(struct config_item *item,
 	if (ret)
 		return ret;
 
+	trace_gadget_dev_desc_bcdDevice_store(to_gadget_info(item));
 	to_gadget_info(item)->cdev.desc.bcdDevice = cpu_to_le16(bcdDevice);
 	return len;
 }
@@ -228,6 +265,7 @@ static ssize_t gadget_dev_desc_bcdUSB_store(struct config_item *item,
 		return ret;
 
 	to_gadget_info(item)->cdev.desc.bcdUSB = cpu_to_le16(bcdUSB);
+	trace_gadget_dev_desc_bcdUSB_store(to_gadget_info(item));
 	return len;
 }
 
@@ -240,6 +278,7 @@ static ssize_t gadget_dev_desc_UDC_show(struct config_item *item, char *page)
 	mutex_lock(&gi->lock);
 	udc_name = gi->composite.gadget_driver.udc_name;
 	ret = sprintf(page, "%s\n", udc_name ?: "");
+	trace_gadget_dev_desc_UDC_show(gi);
 	mutex_unlock(&gi->lock);
 
 	return ret;
@@ -249,6 +288,7 @@ static int unregister_gadget(struct gadget_info *gi)
 {
 	int ret;
 
+	trace_unregister_gadget(gi);
 	if (!gi->composite.gadget_driver.udc_name)
 		return -ENODEV;
 
@@ -276,6 +316,8 @@ static ssize_t gadget_dev_desc_UDC_store(struct config_item *item,
 	if (name[len - 1] == '\n')
 		name[len - 1] = '\0';
 
+	trace_gadget_dev_desc_UDC_store(gi);
+
 	mutex_lock(&gi->lock);
 
 	if (!strlen(name)) {
@@ -296,6 +338,8 @@ static ssize_t gadget_dev_desc_UDC_store(struct config_item *item,
 		}
 	}
 	mutex_unlock(&gi->lock);
+
+	trace_gadget_dev_desc_UDC_store(gi);
 	return len;
 err:
 	kfree(name);
@@ -308,6 +352,7 @@ static ssize_t gadget_dev_desc_max_speed_show(struct config_item *item,
 {
 	enum usb_device_speed speed = to_gadget_info(item)->composite.max_speed;
 
+	trace_gadget_dev_desc_max_speed_show(to_gadget_info(item));
 	return sprintf(page, "%s\n", usb_speed_string(speed));
 }
 
@@ -337,6 +382,8 @@ static ssize_t gadget_dev_desc_max_speed_store(struct config_item *item,
 
 	gi->composite.gadget_driver.max_speed = gi->composite.max_speed;
 
+	trace_gadget_dev_desc_max_speed_store(gi);
+
 	mutex_unlock(&gi->lock);
 	return len;
 err:
@@ -468,6 +515,7 @@ static int config_usb_cfg_link(
 	list_add_tail(&cf->list, &cfg->func_list);
 	ret = 0;
 out:
+	trace_config_usb_cfg_link(gi);
 	mutex_unlock(&gi->lock);
 	return ret;
 }
@@ -500,10 +548,12 @@ static void config_usb_cfg_unlink(
 			list_del(&cf->list);
 			usb_put_function(cf->f);
 			kfree(cf);
+			trace_config_usb_cfg_unlink(gi);
 			mutex_unlock(&gi->lock);
 			return;
 		}
 	}
+	trace_config_usb_cfg_unlink(gi);
 	mutex_unlock(&gi->lock);
 	WARN(1, "Unable to locate function to unbind\n");
 }
@@ -518,6 +568,7 @@ static struct configfs_item_operations gadget_config_item_ops = {
 static ssize_t gadget_config_desc_MaxPower_show(struct config_item *item,
 		char *page)
 {
+	trace_gadget_config_desc_MaxPower_show(to_config_usb_cfg(item)->gi);
 	return sprintf(page, "%u\n", to_config_usb_cfg(item)->c.MaxPower);
 }
 
@@ -532,12 +583,14 @@ static ssize_t gadget_config_desc_MaxPower_store(struct config_item *item,
 	if (DIV_ROUND_UP(val, 8) > 0xff)
 		return -ERANGE;
 	to_config_usb_cfg(item)->c.MaxPower = val;
+	trace_gadget_config_desc_MaxPower_store(to_config_usb_cfg(item)->gi);
 	return len;
 }
 
 static ssize_t gadget_config_desc_bmAttributes_show(struct config_item *item,
 		char *page)
 {
+	trace_gadget_config_desc_bmAttributes_show(to_config_usb_cfg(item)->gi);
 	return sprintf(page, "0x%02x\n",
 		to_config_usb_cfg(item)->c.bmAttributes);
 }
@@ -556,6 +609,7 @@ static ssize_t gadget_config_desc_bmAttributes_store(struct config_item *item,
 				USB_CONFIG_ATT_WAKEUP))
 		return -EINVAL;
 	to_config_usb_cfg(item)->c.bmAttributes = val;
+	trace_gadget_config_desc_bmAttributes_store(to_config_usb_cfg(item)->gi);
 	return len;
 }
 
-- 
2.7.4