All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/3] usb: gadget: f_fs: remove ENTER() macro
@ 2023-03-24  6:10 Linyu Yuan
  2023-03-24  6:10 ` [PATCH v2 2/3] usb: gadget: f_fs: add more function with struct ffs_data *ffs parameter Linyu Yuan
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Linyu Yuan @ 2023-03-24  6:10 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-usb, Linyu Yuan

ENTER() used to show function name which called during runtime, ftrace can
be used to get same information, let's remove it.

Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
---
v2: split to several changes according to v1 comments
v1: https://lore.kernel.org/linux-usb/1679481369-30094-1-git-send-email-quic_linyyuan@quicinc.com/

 drivers/usb/gadget/function/f_fs.c | 94 --------------------------------------
 drivers/usb/gadget/function/u_fs.h |  2 -
 2 files changed, 96 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index a277c70..8830847 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -335,8 +335,6 @@ static ssize_t ffs_ep0_write(struct file *file, const char __user *buf,
 	ssize_t ret;
 	char *data;
 
-	ENTER();
-
 	/* Fast check if setup was canceled */
 	if (ffs_setup_state_clear_cancelled(ffs) == FFS_SETUP_CANCELLED)
 		return -EIDRM;
@@ -512,8 +510,6 @@ static ssize_t ffs_ep0_read(struct file *file, char __user *buf,
 	size_t n;
 	int ret;
 
-	ENTER();
-
 	/* Fast check if setup was canceled */
 	if (ffs_setup_state_clear_cancelled(ffs) == FFS_SETUP_CANCELLED)
 		return -EIDRM;
@@ -612,8 +608,6 @@ static int ffs_ep0_open(struct inode *inode, struct file *file)
 {
 	struct ffs_data *ffs = inode->i_private;
 
-	ENTER();
-
 	if (ffs->state == FFS_CLOSING)
 		return -EBUSY;
 
@@ -627,8 +621,6 @@ static int ffs_ep0_release(struct inode *inode, struct file *file)
 {
 	struct ffs_data *ffs = file->private_data;
 
-	ENTER();
-
 	ffs_data_closed(ffs);
 
 	return 0;
@@ -640,8 +632,6 @@ static long ffs_ep0_ioctl(struct file *file, unsigned code, unsigned long value)
 	struct usb_gadget *gadget = ffs->gadget;
 	long ret;
 
-	ENTER();
-
 	if (code == FUNCTIONFS_INTERFACE_REVMAP) {
 		struct ffs_function *func = ffs->func;
 		ret = func ? ffs_func_revmap_intf(func, value) : -ENODEV;
@@ -715,7 +705,6 @@ static void ffs_epfile_io_complete(struct usb_ep *_ep, struct usb_request *req)
 {
 	struct ffs_io_data *io_data = req->context;
 
-	ENTER();
 	if (req->status)
 		io_data->status = req->status;
 	else
@@ -856,8 +845,6 @@ static void ffs_epfile_async_io_complete(struct usb_ep *_ep,
 	struct ffs_io_data *io_data = req->context;
 	struct ffs_data *ffs = io_data->ffs;
 
-	ENTER();
-
 	io_data->status = req->status ? req->status : req->actual;
 	usb_ep_free_request(_ep, req);
 
@@ -1161,8 +1148,6 @@ ffs_epfile_open(struct inode *inode, struct file *file)
 {
 	struct ffs_epfile *epfile = inode->i_private;
 
-	ENTER();
-
 	if (WARN_ON(epfile->ffs->state != FFS_ACTIVE))
 		return -ENODEV;
 
@@ -1179,8 +1164,6 @@ static int ffs_aio_cancel(struct kiocb *kiocb)
 	unsigned long flags;
 	int value;
 
-	ENTER();
-
 	spin_lock_irqsave(&epfile->ffs->eps_lock, flags);
 
 	if (io_data && io_data->ep && io_data->req)
@@ -1198,8 +1181,6 @@ static ssize_t ffs_epfile_write_iter(struct kiocb *kiocb, struct iov_iter *from)
 	struct ffs_io_data io_data, *p = &io_data;
 	ssize_t res;
 
-	ENTER();
-
 	if (!is_sync_kiocb(kiocb)) {
 		p = kzalloc(sizeof(io_data), GFP_KERNEL);
 		if (!p)
@@ -1235,8 +1216,6 @@ static ssize_t ffs_epfile_read_iter(struct kiocb *kiocb, struct iov_iter *to)
 	struct ffs_io_data io_data, *p = &io_data;
 	ssize_t res;
 
-	ENTER();
-
 	if (!is_sync_kiocb(kiocb)) {
 		p = kzalloc(sizeof(io_data), GFP_KERNEL);
 		if (!p)
@@ -1284,8 +1263,6 @@ ffs_epfile_release(struct inode *inode, struct file *file)
 {
 	struct ffs_epfile *epfile = inode->i_private;
 
-	ENTER();
-
 	__ffs_epfile_read_buffer_free(epfile);
 	ffs_data_closed(epfile->ffs);
 
@@ -1299,8 +1276,6 @@ static long ffs_epfile_ioctl(struct file *file, unsigned code,
 	struct ffs_ep *ep;
 	int ret;
 
-	ENTER();
-
 	if (WARN_ON(epfile->ffs->state != FFS_ACTIVE))
 		return -ENODEV;
 
@@ -1399,8 +1374,6 @@ ffs_sb_make_inode(struct super_block *sb, void *data,
 {
 	struct inode *inode;
 
-	ENTER();
-
 	inode = new_inode(sb);
 
 	if (inode) {
@@ -1432,8 +1405,6 @@ static struct dentry *ffs_sb_create_file(struct super_block *sb,
 	struct dentry	*dentry;
 	struct inode	*inode;
 
-	ENTER();
-
 	dentry = d_alloc_name(sb->s_root, name);
 	if (!dentry)
 		return NULL;
@@ -1468,8 +1439,6 @@ static int ffs_sb_fill(struct super_block *sb, struct fs_context *fc)
 	struct inode	*inode;
 	struct ffs_data	*ffs = data->ffs_data;
 
-	ENTER();
-
 	ffs->sb              = sb;
 	data->ffs_data       = NULL;
 	sb->s_fs_info        = ffs;
@@ -1521,8 +1490,6 @@ static int ffs_fs_parse_param(struct fs_context *fc, struct fs_parameter *param)
 	struct fs_parse_result result;
 	int opt;
 
-	ENTER();
-
 	opt = fs_parse(fc, ffs_fs_fs_parameters, param, &result);
 	if (opt < 0)
 		return opt;
@@ -1572,8 +1539,6 @@ static int ffs_fs_get_tree(struct fs_context *fc)
 	struct ffs_data	*ffs;
 	int ret;
 
-	ENTER();
-
 	if (!fc->source)
 		return invalf(fc, "No source specified");
 
@@ -1640,8 +1605,6 @@ static int ffs_fs_init_fs_context(struct fs_context *fc)
 static void
 ffs_fs_kill_sb(struct super_block *sb)
 {
-	ENTER();
-
 	kill_litter_super(sb);
 	if (sb->s_fs_info)
 		ffs_data_closed(sb->s_fs_info);
@@ -1663,8 +1626,6 @@ static int functionfs_init(void)
 {
 	int ret;
 
-	ENTER();
-
 	ret = register_filesystem(&ffs_fs_type);
 	if (!ret)
 		pr_info("file system registered\n");
@@ -1676,8 +1637,6 @@ static int functionfs_init(void)
 
 static void functionfs_cleanup(void)
 {
-	ENTER();
-
 	pr_info("unloading\n");
 	unregister_filesystem(&ffs_fs_type);
 }
@@ -1690,15 +1649,11 @@ static void ffs_data_reset(struct ffs_data *ffs);
 
 static void ffs_data_get(struct ffs_data *ffs)
 {
-	ENTER();
-
 	refcount_inc(&ffs->ref);
 }
 
 static void ffs_data_opened(struct ffs_data *ffs)
 {
-	ENTER();
-
 	refcount_inc(&ffs->ref);
 	if (atomic_add_return(1, &ffs->opened) == 1 &&
 			ffs->state == FFS_DEACTIVATED) {
@@ -1709,8 +1664,6 @@ static void ffs_data_opened(struct ffs_data *ffs)
 
 static void ffs_data_put(struct ffs_data *ffs)
 {
-	ENTER();
-
 	if (refcount_dec_and_test(&ffs->ref)) {
 		pr_info("%s(): freeing\n", __func__);
 		ffs_data_clear(ffs);
@@ -1729,8 +1682,6 @@ static void ffs_data_closed(struct ffs_data *ffs)
 	struct ffs_epfile *epfiles;
 	unsigned long flags;
 
-	ENTER();
-
 	if (atomic_dec_and_test(&ffs->opened)) {
 		if (ffs->no_disconnect) {
 			ffs->state = FFS_DEACTIVATED;
@@ -1765,8 +1716,6 @@ static struct ffs_data *ffs_data_new(const char *dev_name)
 	if (!ffs)
 		return NULL;
 
-	ENTER();
-
 	ffs->io_completion_wq = alloc_ordered_workqueue("%s", 0, dev_name);
 	if (!ffs->io_completion_wq) {
 		kfree(ffs);
@@ -1793,8 +1742,6 @@ static void ffs_data_clear(struct ffs_data *ffs)
 	struct ffs_epfile *epfiles;
 	unsigned long flags;
 
-	ENTER();
-
 	ffs_closed(ffs);
 
 	BUG_ON(ffs->gadget);
@@ -1826,8 +1773,6 @@ static void ffs_data_clear(struct ffs_data *ffs)
 
 static void ffs_data_reset(struct ffs_data *ffs)
 {
-	ENTER();
-
 	ffs_data_clear(ffs);
 
 	ffs->raw_descs_data = NULL;
@@ -1861,8 +1806,6 @@ static int functionfs_bind(struct ffs_data *ffs, struct usb_composite_dev *cdev)
 	struct usb_gadget_strings **lang;
 	int first_id;
 
-	ENTER();
-
 	if (WARN_ON(ffs->state != FFS_ACTIVE
 		 || test_and_set_bit(FFS_FL_BOUND, &ffs->flags)))
 		return -EBADFD;
@@ -1894,8 +1837,6 @@ static int functionfs_bind(struct ffs_data *ffs, struct usb_composite_dev *cdev)
 
 static void functionfs_unbind(struct ffs_data *ffs)
 {
-	ENTER();
-
 	if (!WARN_ON(!ffs->gadget)) {
 		/* dequeue before freeing ep0req */
 		usb_ep_dequeue(ffs->gadget->ep0, ffs->ep0req);
@@ -1914,8 +1855,6 @@ static int ffs_epfiles_create(struct ffs_data *ffs)
 	struct ffs_epfile *epfile, *epfiles;
 	unsigned i, count;
 
-	ENTER();
-
 	count = ffs->eps_count;
 	epfiles = kcalloc(count, sizeof(*epfiles), GFP_KERNEL);
 	if (!epfiles)
@@ -1946,8 +1885,6 @@ static void ffs_epfiles_destroy(struct ffs_epfile *epfiles, unsigned count)
 {
 	struct ffs_epfile *epfile = epfiles;
 
-	ENTER();
-
 	for (; count; --count, ++epfile) {
 		BUG_ON(mutex_is_locked(&epfile->mutex));
 		if (epfile->dentry) {
@@ -2064,8 +2001,6 @@ static int __must_check ffs_do_single_desc(char *data, unsigned len,
 	u8 length;
 	int ret;
 
-	ENTER();
-
 	/* At least two bytes are required: length and type */
 	if (len < 2) {
 		pr_vdebug("descriptor too short\n");
@@ -2204,8 +2139,6 @@ static int __must_check ffs_do_descs(unsigned count, char *data, unsigned len,
 	unsigned long num = 0;
 	int current_class = -1;
 
-	ENTER();
-
 	for (;;) {
 		int ret;
 
@@ -2243,8 +2176,6 @@ static int __ffs_data_do_entity(enum ffs_entity_type type,
 	struct ffs_desc_helper *helper = priv;
 	struct usb_endpoint_descriptor *d;
 
-	ENTER();
-
 	switch (type) {
 	case FFS_DESCRIPTOR:
 		break;
@@ -2329,8 +2260,6 @@ static int __must_check ffs_do_single_os_desc(char *data, unsigned len,
 	int ret;
 	const unsigned _len = len;
 
-	ENTER();
-
 	/* loop over all ext compat/ext prop descriptors */
 	while (feature_count--) {
 		ret = entity(type, h, data, len, priv);
@@ -2352,8 +2281,6 @@ static int __must_check ffs_do_os_descs(unsigned count,
 	const unsigned _len = len;
 	unsigned long num = 0;
 
-	ENTER();
-
 	for (num = 0; num < count; ++num) {
 		int ret;
 		enum ffs_os_desc_type type;
@@ -2416,8 +2343,6 @@ static int __ffs_data_do_os_desc(enum ffs_os_desc_type type,
 	struct ffs_data *ffs = priv;
 	u8 length;
 
-	ENTER();
-
 	switch (type) {
 	case FFS_OS_DESC_EXT_COMPAT: {
 		struct usb_ext_compat_desc *d = data;
@@ -2493,8 +2418,6 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
 	int ret = -EINVAL, i;
 	struct ffs_desc_helper helper;
 
-	ENTER();
-
 	if (get_unaligned_le32(data + 4) != len)
 		goto error;
 
@@ -2625,8 +2548,6 @@ static int __ffs_data_got_strings(struct ffs_data *ffs,
 	const char *data = _data;
 	struct usb_string *s;
 
-	ENTER();
-
 	if (len < 16 ||
 	    get_unaligned_le32(data) != FUNCTIONFS_STRINGS_MAGIC ||
 	    get_unaligned_le32(data + 4) != len)
@@ -3085,8 +3006,6 @@ static inline struct f_fs_opts *ffs_do_functionfs_bind(struct usb_function *f,
 	struct ffs_data *ffs_data;
 	int ret;
 
-	ENTER();
-
 	/*
 	 * Legacy gadget triggers binding in functionfs_ready_callback,
 	 * which already uses locking; taking the same lock here would
@@ -3163,8 +3082,6 @@ static int _ffs_func_bind(struct usb_configuration *c,
 	vla_item_with_sz(d, char, raw_descs, ffs->raw_descs_length);
 	char *vlabuf;
 
-	ENTER();
-
 	/* Has descriptors only for speeds gadget does not support */
 	if (!(full | high | super))
 		return -ENOTSUPP;
@@ -3368,8 +3285,6 @@ static int ffs_func_setup(struct usb_function *f,
 	unsigned long flags;
 	int ret;
 
-	ENTER();
-
 	pr_vdebug("creq->bRequestType = %02x\n", creq->bRequestType);
 	pr_vdebug("creq->bRequest     = %02x\n", creq->bRequest);
 	pr_vdebug("creq->wValue       = %04x\n", le16_to_cpu(creq->wValue));
@@ -3444,13 +3359,11 @@ static bool ffs_func_req_match(struct usb_function *f,
 
 static void ffs_func_suspend(struct usb_function *f)
 {
-	ENTER();
 	ffs_event_add(ffs_func_from_usb(f)->ffs, FUNCTIONFS_SUSPEND);
 }
 
 static void ffs_func_resume(struct usb_function *f)
 {
-	ENTER();
 	ffs_event_add(ffs_func_from_usb(f)->ffs, FUNCTIONFS_RESUME);
 }
 
@@ -3614,7 +3527,6 @@ static void ffs_func_unbind(struct usb_configuration *c,
 	unsigned count = ffs->eps_count;
 	unsigned long flags;
 
-	ENTER();
 	if (ffs->func == func) {
 		ffs_func_eps_disable(func);
 		ffs->func = NULL;
@@ -3654,8 +3566,6 @@ static struct usb_function *ffs_alloc(struct usb_function_instance *fi)
 {
 	struct ffs_function *func;
 
-	ENTER();
-
 	func = kzalloc(sizeof(*func), GFP_KERNEL);
 	if (!func)
 		return ERR_PTR(-ENOMEM);
@@ -3756,7 +3666,6 @@ static int ffs_acquire_dev(const char *dev_name, struct ffs_data *ffs_data)
 	int ret = 0;
 	struct ffs_dev *ffs_dev;
 
-	ENTER();
 	ffs_dev_lock();
 
 	ffs_dev = _ffs_find_dev(dev_name);
@@ -3779,7 +3688,6 @@ static int ffs_acquire_dev(const char *dev_name, struct ffs_data *ffs_data)
 
 static void ffs_release_dev(struct ffs_dev *ffs_dev)
 {
-	ENTER();
 	ffs_dev_lock();
 
 	if (ffs_dev && ffs_dev->mounted) {
@@ -3801,7 +3709,6 @@ static int ffs_ready(struct ffs_data *ffs)
 	struct ffs_dev *ffs_obj;
 	int ret = 0;
 
-	ENTER();
 	ffs_dev_lock();
 
 	ffs_obj = ffs->private_data;
@@ -3834,7 +3741,6 @@ static void ffs_closed(struct ffs_data *ffs)
 	struct f_fs_opts *opts;
 	struct config_item *ci;
 
-	ENTER();
 	ffs_dev_lock();
 
 	ffs_obj = ffs->private_data;
diff --git a/drivers/usb/gadget/function/u_fs.h b/drivers/usb/gadget/function/u_fs.h
index f102ec2..4b3365f 100644
--- a/drivers/usb/gadget/function/u_fs.h
+++ b/drivers/usb/gadget/function/u_fs.h
@@ -32,8 +32,6 @@
 #  define ffs_dump_mem(prefix, ptr, len) do { } while (0)
 #endif /* VERBOSE_DEBUG */
 
-#define ENTER()    pr_vdebug("%s()\n", __func__)
-
 struct f_fs_opts;
 
 struct ffs_dev {
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 2/3] usb: gadget: f_fs: add more function with struct ffs_data *ffs parameter
  2023-03-24  6:10 [PATCH v2 1/3] usb: gadget: f_fs: remove ENTER() macro Linyu Yuan
@ 2023-03-24  6:10 ` Linyu Yuan
  2023-03-25  9:01   ` Greg Kroah-Hartman
  2023-03-24  6:10 ` [PATCH v2 3/3] usb: gadget: f_fs: add dev name as prefix for pr_vdebug() Linyu Yuan
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Linyu Yuan @ 2023-03-24  6:10 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-usb, Linyu Yuan

It prepare to improve pr_vdebug() which will show dev_name when multiple
f_fs instance exist.

Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
---
v2: split to several changes according to v1 comments
v1: https://lore.kernel.org/linux-usb/1679481369-30094-1-git-send-email-quic_linyyuan@quicinc.com/

 drivers/usb/gadget/function/f_fs.c | 78 +++++++++++++++++++-------------------
 1 file changed, 38 insertions(+), 40 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 8830847..a4051c8 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -231,7 +231,6 @@ struct ffs_io_data {
 };
 
 struct ffs_desc_helper {
-	struct ffs_data *ffs;
 	unsigned interfaces_count;
 	unsigned eps_count;
 };
@@ -260,7 +259,7 @@ static void ffs_closed(struct ffs_data *ffs);
 
 static int ffs_mutex_lock(struct mutex *mutex, unsigned nonblock)
 	__attribute__((warn_unused_result, nonnull));
-static char *ffs_prepare_buffer(const char __user *buf, size_t len)
+static char *ffs_prepare_buffer(struct ffs_data *ffs, const char __user *buf, size_t len)
 	__attribute__((warn_unused_result, nonnull));
 
 
@@ -354,7 +353,7 @@ static ssize_t ffs_ep0_write(struct file *file, const char __user *buf,
 			break;
 		}
 
-		data = ffs_prepare_buffer(buf, len);
+		data = ffs_prepare_buffer(ffs, buf, len);
 		if (IS_ERR(data)) {
 			ret = PTR_ERR(data);
 			break;
@@ -426,7 +425,7 @@ static ssize_t ffs_ep0_write(struct file *file, const char __user *buf,
 
 		spin_unlock_irq(&ffs->ev.waitq.lock);
 
-		data = ffs_prepare_buffer(buf, len);
+		data = ffs_prepare_buffer(ffs, buf, len);
 		if (IS_ERR(data)) {
 			ret = PTR_ERR(data);
 			break;
@@ -713,7 +712,8 @@ static void ffs_epfile_io_complete(struct usb_ep *_ep, struct usb_request *req)
 	complete(&io_data->done);
 }
 
-static ssize_t ffs_copy_to_iter(void *data, int data_len, struct iov_iter *iter)
+static ssize_t ffs_copy_to_iter(struct ffs_data *ffs, void *data, int data_len,
+				struct iov_iter *iter)
 {
 	ssize_t ret = copy_to_iter(data, data_len, iter);
 	if (ret == data_len)
@@ -824,7 +824,7 @@ static void ffs_user_copy_worker(struct work_struct *work)
 
 	if (io_data->read && ret > 0) {
 		kthread_use_mm(io_data->mm);
-		ret = ffs_copy_to_iter(io_data->buf, ret, &io_data->data);
+		ret = ffs_copy_to_iter(io_data->ffs, io_data->buf, ret, &io_data->data);
 		kthread_unuse_mm(io_data->mm);
 	}
 
@@ -1984,16 +1984,16 @@ enum ffs_os_desc_type {
 	FFS_OS_DESC, FFS_OS_DESC_EXT_COMPAT, FFS_OS_DESC_EXT_PROP
 };
 
-typedef int (*ffs_entity_callback)(enum ffs_entity_type entity,
+typedef int (*ffs_entity_callback)(struct ffs_data *ffs, enum ffs_entity_type entity,
 				   u8 *valuep,
 				   struct usb_descriptor_header *desc,
 				   void *priv);
 
-typedef int (*ffs_os_desc_callback)(enum ffs_os_desc_type entity,
+typedef int (*ffs_os_desc_callback)(struct ffs_data *ffs, enum ffs_os_desc_type entity,
 				    struct usb_os_desc_header *h, void *data,
 				    unsigned len, void *priv);
 
-static int __must_check ffs_do_single_desc(char *data, unsigned len,
+static int __must_check ffs_do_single_desc(struct ffs_data *ffs, char *data, unsigned len,
 					   ffs_entity_callback entity,
 					   void *priv, int *current_class)
 {
@@ -2023,7 +2023,7 @@ static int __must_check ffs_do_single_desc(char *data, unsigned len,
 			pr_vdebug("invalid entity's value\n");		\
 			return -EINVAL;					\
 		}							\
-		ret = entity(FFS_ ##type, &val, _ds, priv);		\
+		ret = entity(ffs, FFS_ ##type, &val, _ds, priv);		\
 		if (ret < 0) {						\
 			pr_debug("entity " #type "(%02x); ret = %d\n",	\
 				 (val), ret);				\
@@ -2132,7 +2132,7 @@ static int __must_check ffs_do_single_desc(char *data, unsigned len,
 	return length;
 }
 
-static int __must_check ffs_do_descs(unsigned count, char *data, unsigned len,
+static int __must_check ffs_do_descs(struct ffs_data *ffs, unsigned count, char *data, unsigned len,
 				     ffs_entity_callback entity, void *priv)
 {
 	const unsigned _len = len;
@@ -2146,7 +2146,7 @@ static int __must_check ffs_do_descs(unsigned count, char *data, unsigned len,
 			data = NULL;
 
 		/* Record "descriptor" entity */
-		ret = entity(FFS_DESCRIPTOR, (u8 *)num, (void *)data, priv);
+		ret = entity(ffs, FFS_DESCRIPTOR, (u8 *)num, (void *)data, priv);
 		if (ret < 0) {
 			pr_debug("entity DESCRIPTOR(%02lx); ret = %d\n",
 				 num, ret);
@@ -2156,7 +2156,7 @@ static int __must_check ffs_do_descs(unsigned count, char *data, unsigned len,
 		if (!data)
 			return _len - len;
 
-		ret = ffs_do_single_desc(data, len, entity, priv,
+		ret = ffs_do_single_desc(ffs, data, len, entity, priv,
 			&current_class);
 		if (ret < 0) {
 			pr_debug("%s returns %d\n", __func__, ret);
@@ -2169,7 +2169,7 @@ static int __must_check ffs_do_descs(unsigned count, char *data, unsigned len,
 	}
 }
 
-static int __ffs_data_do_entity(enum ffs_entity_type type,
+static int __ffs_data_do_entity(struct ffs_data *ffs, enum ffs_entity_type type,
 				u8 *valuep, struct usb_descriptor_header *desc,
 				void *priv)
 {
@@ -2195,8 +2195,8 @@ static int __ffs_data_do_entity(enum ffs_entity_type type,
 		 * Strings are indexed from 1 (0 is reserved
 		 * for languages list)
 		 */
-		if (*valuep > helper->ffs->strings_count)
-			helper->ffs->strings_count = *valuep;
+		if (*valuep > ffs->strings_count)
+			ffs->strings_count = *valuep;
 		break;
 
 	case FFS_ENDPOINT:
@@ -2205,10 +2205,10 @@ static int __ffs_data_do_entity(enum ffs_entity_type type,
 		if (helper->eps_count >= FFS_MAX_EPS_COUNT)
 			return -EINVAL;
 		/* Check if descriptors for any speed were already parsed */
-		if (!helper->ffs->eps_count && !helper->ffs->interfaces_count)
-			helper->ffs->eps_addrmap[helper->eps_count] =
+		if (!ffs->eps_count && !ffs->interfaces_count)
+			ffs->eps_addrmap[helper->eps_count] =
 				d->bEndpointAddress;
-		else if (helper->ffs->eps_addrmap[helper->eps_count] !=
+		else if (ffs->eps_addrmap[helper->eps_count] !=
 				d->bEndpointAddress)
 			return -EINVAL;
 		break;
@@ -2217,7 +2217,7 @@ static int __ffs_data_do_entity(enum ffs_entity_type type,
 	return 0;
 }
 
-static int __ffs_do_os_desc_header(enum ffs_os_desc_type *next_type,
+static int __ffs_do_os_desc_header(struct ffs_data *ffs, enum ffs_os_desc_type *next_type,
 				   struct usb_os_desc_header *desc)
 {
 	u16 bcd_version = le16_to_cpu(desc->bcdVersion);
@@ -2250,7 +2250,7 @@ static int __ffs_do_os_desc_header(enum ffs_os_desc_type *next_type,
  * Process all extended compatibility/extended property descriptors
  * of a feature descriptor
  */
-static int __must_check ffs_do_single_os_desc(char *data, unsigned len,
+static int __must_check ffs_do_single_os_desc(struct ffs_data *ffs, char *data, unsigned len,
 					      enum ffs_os_desc_type type,
 					      u16 feature_count,
 					      ffs_os_desc_callback entity,
@@ -2262,7 +2262,7 @@ static int __must_check ffs_do_single_os_desc(char *data, unsigned len,
 
 	/* loop over all ext compat/ext prop descriptors */
 	while (feature_count--) {
-		ret = entity(type, h, data, len, priv);
+		ret = entity(ffs, type, h, data, len, priv);
 		if (ret < 0) {
 			pr_debug("bad OS descriptor, type: %d\n", type);
 			return ret;
@@ -2274,7 +2274,7 @@ static int __must_check ffs_do_single_os_desc(char *data, unsigned len,
 }
 
 /* Process a number of complete Feature Descriptors (Ext Compat or Ext Prop) */
-static int __must_check ffs_do_os_descs(unsigned count,
+static int __must_check ffs_do_os_descs(struct ffs_data *ffs, unsigned count,
 					char *data, unsigned len,
 					ffs_os_desc_callback entity, void *priv)
 {
@@ -2300,7 +2300,7 @@ static int __must_check ffs_do_os_descs(unsigned count,
 		if (le32_to_cpu(desc->dwLength) > len)
 			return -EINVAL;
 
-		ret = __ffs_do_os_desc_header(&type, desc);
+		ret = __ffs_do_os_desc_header(ffs, &type, desc);
 		if (ret < 0) {
 			pr_debug("entity OS_DESCRIPTOR(%02lx); ret = %d\n",
 				 num, ret);
@@ -2320,7 +2320,7 @@ static int __must_check ffs_do_os_descs(unsigned count,
 		 * Process all function/property descriptors
 		 * of this Feature Descriptor
 		 */
-		ret = ffs_do_single_os_desc(data, len, type,
+		ret = ffs_do_single_os_desc(ffs, data, len, type,
 					    feature_count, entity, priv, desc);
 		if (ret < 0) {
 			pr_debug("%s returns %d\n", __func__, ret);
@@ -2336,11 +2336,10 @@ static int __must_check ffs_do_os_descs(unsigned count,
 /*
  * Validate contents of the buffer from userspace related to OS descriptors.
  */
-static int __ffs_data_do_os_desc(enum ffs_os_desc_type type,
+static int __ffs_data_do_os_desc(struct ffs_data *ffs, enum ffs_os_desc_type type,
 				 struct usb_os_desc_header *h, void *data,
 				 unsigned len, void *priv)
 {
-	struct ffs_data *ffs = priv;
 	u8 length;
 
 	switch (type) {
@@ -2485,13 +2484,12 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
 
 	/* Read descriptors */
 	raw_descs = data;
-	helper.ffs = ffs;
 	for (i = 0; i < 3; ++i) {
 		if (!counts[i])
 			continue;
 		helper.interfaces_count = 0;
 		helper.eps_count = 0;
-		ret = ffs_do_descs(counts[i], data, len,
+		ret = ffs_do_descs(ffs, counts[i], data, len,
 				   __ffs_data_do_entity, &helper);
 		if (ret < 0)
 			goto error;
@@ -2512,8 +2510,8 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
 		len  -= ret;
 	}
 	if (os_descs_count) {
-		ret = ffs_do_os_descs(os_descs_count, data, len,
-				      __ffs_data_do_os_desc, ffs);
+		ret = ffs_do_os_descs(ffs, os_descs_count, data, len,
+				      __ffs_data_do_os_desc, NULL);
 		if (ret < 0)
 			goto error;
 		data += ret;
@@ -2763,7 +2761,7 @@ static int ffs_ep_addr2idx(struct ffs_data *ffs, u8 endpoint_address)
 	return -ENOENT;
 }
 
-static int __ffs_func_bind_do_descs(enum ffs_entity_type type, u8 *valuep,
+static int __ffs_func_bind_do_descs(struct ffs_data *ffs, enum ffs_entity_type type, u8 *valuep,
 				    struct usb_descriptor_header *desc,
 				    void *priv)
 {
@@ -2863,7 +2861,7 @@ static int __ffs_func_bind_do_descs(enum ffs_entity_type type, u8 *valuep,
 	return 0;
 }
 
-static int __ffs_func_bind_do_nums(enum ffs_entity_type type, u8 *valuep,
+static int __ffs_func_bind_do_nums(struct ffs_data *ffs, enum ffs_entity_type type, u8 *valuep,
 				   struct usb_descriptor_header *desc,
 				   void *priv)
 {
@@ -2918,7 +2916,7 @@ static int __ffs_func_bind_do_nums(enum ffs_entity_type type, u8 *valuep,
 	return 0;
 }
 
-static int __ffs_func_bind_do_os_desc(enum ffs_os_desc_type type,
+static int __ffs_func_bind_do_os_desc(struct ffs_data *ffs, enum ffs_os_desc_type type,
 				      struct usb_os_desc_header *h, void *data,
 				      unsigned len, void *priv)
 {
@@ -3119,7 +3117,7 @@ static int _ffs_func_bind(struct usb_configuration *c,
 	 */
 	if (full) {
 		func->function.fs_descriptors = vla_ptr(vlabuf, d, fs_descs);
-		fs_len = ffs_do_descs(ffs->fs_descs_count,
+		fs_len = ffs_do_descs(ffs, ffs->fs_descs_count,
 				      vla_ptr(vlabuf, d, raw_descs),
 				      d_raw_descs__sz,
 				      __ffs_func_bind_do_descs, func);
@@ -3133,7 +3131,7 @@ static int _ffs_func_bind(struct usb_configuration *c,
 
 	if (high) {
 		func->function.hs_descriptors = vla_ptr(vlabuf, d, hs_descs);
-		hs_len = ffs_do_descs(ffs->hs_descs_count,
+		hs_len = ffs_do_descs(ffs, ffs->hs_descs_count,
 				      vla_ptr(vlabuf, d, raw_descs) + fs_len,
 				      d_raw_descs__sz - fs_len,
 				      __ffs_func_bind_do_descs, func);
@@ -3148,7 +3146,7 @@ static int _ffs_func_bind(struct usb_configuration *c,
 	if (super) {
 		func->function.ss_descriptors = func->function.ssp_descriptors =
 			vla_ptr(vlabuf, d, ss_descs);
-		ss_len = ffs_do_descs(ffs->ss_descs_count,
+		ss_len = ffs_do_descs(ffs, ffs->ss_descs_count,
 				vla_ptr(vlabuf, d, raw_descs) + fs_len + hs_len,
 				d_raw_descs__sz - fs_len - hs_len,
 				__ffs_func_bind_do_descs, func);
@@ -3165,7 +3163,7 @@ static int _ffs_func_bind(struct usb_configuration *c,
 	 * endpoint numbers rewriting.  We can do that in one go
 	 * now.
 	 */
-	ret = ffs_do_descs(ffs->fs_descs_count +
+	ret = ffs_do_descs(ffs, ffs->fs_descs_count +
 			   (high ? ffs->hs_descs_count : 0) +
 			   (super ? ffs->ss_descs_count : 0),
 			   vla_ptr(vlabuf, d, raw_descs), d_raw_descs__sz,
@@ -3185,7 +3183,7 @@ static int _ffs_func_bind(struct usb_configuration *c,
 				vla_ptr(vlabuf, d, ext_compat) + i * 16;
 			INIT_LIST_HEAD(&desc->ext_prop);
 		}
-		ret = ffs_do_os_descs(ffs->ms_os_descs_count,
+		ret = ffs_do_os_descs(ffs, ffs->ms_os_descs_count,
 				      vla_ptr(vlabuf, d, raw_descs) +
 				      fs_len + hs_len + ss_len,
 				      d_raw_descs__sz - fs_len - hs_len -
@@ -3781,7 +3779,7 @@ static int ffs_mutex_lock(struct mutex *mutex, unsigned nonblock)
 		: mutex_lock_interruptible(mutex);
 }
 
-static char *ffs_prepare_buffer(const char __user *buf, size_t len)
+static char *ffs_prepare_buffer(struct ffs_data *ffs, const char __user *buf, size_t len)
 {
 	char *data;
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 3/3] usb: gadget: f_fs: add dev name as prefix for pr_vdebug()
  2023-03-24  6:10 [PATCH v2 1/3] usb: gadget: f_fs: remove ENTER() macro Linyu Yuan
  2023-03-24  6:10 ` [PATCH v2 2/3] usb: gadget: f_fs: add more function with struct ffs_data *ffs parameter Linyu Yuan
@ 2023-03-24  6:10 ` Linyu Yuan
  2023-03-25  9:00   ` Greg Kroah-Hartman
  2023-03-24 16:03 ` [PATCH v2 1/3] usb: gadget: f_fs: remove ENTER() macro kernel test robot
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Linyu Yuan @ 2023-03-24  6:10 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-usb, Linyu Yuan

when multiple instances in use, the debug message is hard to understand
as there is no instance name show.

this change will show each instance name for debug messages.

Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
---
v2: split to several changes according to v1 comments
v1: https://lore.kernel.org/linux-usb/1679481369-30094-1-git-send-email-quic_linyyuan@quicinc.com/

 drivers/usb/gadget/function/f_fs.c | 136 +++++++++++++++++++------------------
 1 file changed, 69 insertions(+), 67 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index a4051c8..df67ab5 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -317,12 +317,12 @@ static int __ffs_ep0_queue_wait(struct ffs_data *ffs, char *data, size_t len)
 static int __ffs_ep0_stall(struct ffs_data *ffs)
 {
 	if (ffs->ev.can_stall) {
-		pr_vdebug("ep0 stall\n");
+		pr_vdebug("%s: ep0 stall\n", ffs->dev_name);
 		usb_ep_set_halt(ffs->gadget->ep0);
 		ffs->setup_state = FFS_NO_SETUP;
 		return -EL2HLT;
 	} else {
-		pr_debug("bogus ep0 stall!\n");
+		pr_debug("%s: bogus ep0 stall!\n", ffs->dev_name);
 		return -ESRCH;
 	}
 }
@@ -361,7 +361,7 @@ static ssize_t ffs_ep0_write(struct file *file, const char __user *buf,
 
 		/* Handle data */
 		if (ffs->state == FFS_READ_DESCRIPTORS) {
-			pr_info("read descriptors\n");
+			pr_info("%s: read descriptors\n", ffs->dev_name);
 			ret = __ffs_data_got_descs(ffs, data, len);
 			if (ret < 0)
 				break;
@@ -369,7 +369,7 @@ static ssize_t ffs_ep0_write(struct file *file, const char __user *buf,
 			ffs->state = FFS_READ_STRINGS;
 			ret = len;
 		} else {
-			pr_info("read strings\n");
+			pr_info("%s: read strings\n", ffs->dev_name);
 			ret = __ffs_data_got_strings(ffs, data, len);
 			if (ret < 0)
 				break;
@@ -749,9 +749,9 @@ static ssize_t ffs_copy_to_iter(struct ffs_data *ffs, void *data, int data_len,
 	 * aio_read(2) etc. system calls.  Writing data to an IN endpoint is not
 	 * affected.
 	 */
-	pr_err("functionfs read size %d > requested size %zd, dropping excess data. "
+	pr_err("%s: functionfs read size %d > requested size %zd, dropping excess data. "
 	       "Align read buffer size to max packet size to avoid the problem.\n",
-	       data_len, ret);
+	       ffs->dev_name, data_len, ret);
 
 	return ret;
 }
@@ -911,8 +911,8 @@ static ssize_t __ffs_epfile_read_data(struct ffs_epfile *epfile,
 		return -EFAULT;
 
 	/* See ffs_copy_to_iter for more context. */
-	pr_warn("functionfs read size %d > requested size %zd, splitting request into multiple reads.",
-		data_len, ret);
+	pr_warn("%s: functionfs read size %d > requested size %zd, splitting request into multiple reads.",
+		epfile->ffs->dev_name, data_len, ret);
 
 	data_len -= ret;
 	buf = kmalloc(struct_size(buf, storage, data_len), GFP_KERNEL);
@@ -1043,7 +1043,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
 		 * For such reason, we're adding this redundant sanity check
 		 * here.
 		 */
-		WARN(1, "%s: data_len == -EINVAL\n", __func__);
+		WARN(1, "%s: %s(): data_len == -EINVAL\n", epfile->ffs->dev_name, __func__);
 		ret = -EINVAL;
 	} else if (!io_data->aio) {
 		bool interrupted = false;
@@ -1665,7 +1665,7 @@ static void ffs_data_opened(struct ffs_data *ffs)
 static void ffs_data_put(struct ffs_data *ffs)
 {
 	if (refcount_dec_and_test(&ffs->ref)) {
-		pr_info("%s(): freeing\n", __func__);
+		pr_info("%s: %s(): freeing\n", ffs->dev_name, __func__);
 		ffs_data_clear(ffs);
 		ffs_release_dev(ffs->private_data);
 		BUG_ON(waitqueue_active(&ffs->ev.waitq) ||
@@ -1942,8 +1942,8 @@ static int ffs_func_eps_enable(struct ffs_function *func)
 
 		ret = config_ep_by_speed(func->gadget, &func->function, ep->ep);
 		if (ret) {
-			pr_err("%s: config_ep_by_speed(%s) returned %d\n",
-					__func__, ep->ep->name, ret);
+			pr_err("%s: %s(): config_ep_by_speed(%s) returned %d\n",
+					ffs->dev_name, __func__, ep->ep->name, ret);
 			break;
 		}
 
@@ -2003,14 +2003,14 @@ static int __must_check ffs_do_single_desc(struct ffs_data *ffs, char *data, uns
 
 	/* At least two bytes are required: length and type */
 	if (len < 2) {
-		pr_vdebug("descriptor too short\n");
+		pr_vdebug("%s: descriptor too short\n", ffs->dev_name);
 		return -EINVAL;
 	}
 
 	/* If we have at least as many bytes as the descriptor takes? */
 	length = _ds->bLength;
 	if (len < length) {
-		pr_vdebug("descriptor longer then available data\n");
+		pr_vdebug("%s: descriptor longer then available data\n", ffs->dev_name);
 		return -EINVAL;
 	}
 
@@ -2018,15 +2018,15 @@ static int __must_check ffs_do_single_desc(struct ffs_data *ffs, char *data, uns
 #define __entity_check_STRING(val)     (val)
 #define __entity_check_ENDPOINT(val)   ((val) & USB_ENDPOINT_NUMBER_MASK)
 #define __entity(type, val) do {					\
-		pr_vdebug("entity " #type "(%02x)\n", (val));		\
+		pr_vdebug("%s: entity " #type "(%02x)\n", ffs->dev_name, (val));	\
 		if (!__entity_check_ ##type(val)) {			\
-			pr_vdebug("invalid entity's value\n");		\
+			pr_vdebug("%s: invalid entity's value\n", ffs->dev_name);	\
 			return -EINVAL;					\
 		}							\
-		ret = entity(ffs, FFS_ ##type, &val, _ds, priv);		\
+		ret = entity(ffs, FFS_ ##type, &val, _ds, priv);	\
 		if (ret < 0) {						\
-			pr_debug("entity " #type "(%02x); ret = %d\n",	\
-				 (val), ret);				\
+			pr_debug("%s: entity " #type "(%02x); ret = %d\n",	\
+				 ffs->dev_name, (val), ret);		\
 			return ret;					\
 		}							\
 	} while (0)
@@ -2038,13 +2038,13 @@ static int __must_check ffs_do_single_desc(struct ffs_data *ffs, char *data, uns
 	case USB_DT_STRING:
 	case USB_DT_DEVICE_QUALIFIER:
 		/* function can't have any of those */
-		pr_vdebug("descriptor reserved for gadget: %d\n",
-		      _ds->bDescriptorType);
+		pr_vdebug("%s: descriptor reserved for gadget: %d\n",
+		      ffs->dev_name, _ds->bDescriptorType);
 		return -EINVAL;
 
 	case USB_DT_INTERFACE: {
 		struct usb_interface_descriptor *ds = (void *)_ds;
-		pr_vdebug("interface descriptor\n");
+		pr_vdebug("%s: interface descriptor\n", ffs->dev_name);
 		if (length != sizeof *ds)
 			goto inv_length;
 
@@ -2057,7 +2057,7 @@ static int __must_check ffs_do_single_desc(struct ffs_data *ffs, char *data, uns
 
 	case USB_DT_ENDPOINT: {
 		struct usb_endpoint_descriptor *ds = (void *)_ds;
-		pr_vdebug("endpoint descriptor\n");
+		pr_vdebug("%s: endpoint descriptor\n", ffs->dev_name);
 		if (length != USB_DT_ENDPOINT_SIZE &&
 		    length != USB_DT_ENDPOINT_AUDIO_SIZE)
 			goto inv_length;
@@ -2067,18 +2067,18 @@ static int __must_check ffs_do_single_desc(struct ffs_data *ffs, char *data, uns
 
 	case USB_TYPE_CLASS | 0x01:
 		if (*current_class == USB_INTERFACE_CLASS_HID) {
-			pr_vdebug("hid descriptor\n");
+			pr_vdebug("%s: hid descriptor\n", ffs->dev_name);
 			if (length != sizeof(struct hid_descriptor))
 				goto inv_length;
 			break;
 		} else if (*current_class == USB_INTERFACE_CLASS_CCID) {
-			pr_vdebug("ccid descriptor\n");
+			pr_vdebug("%s: ccid descriptor\n", ffs->dev_name);
 			if (length != sizeof(struct ccid_descriptor))
 				goto inv_length;
 			break;
 		} else {
-			pr_vdebug("unknown descriptor: %d for class %d\n",
-			      _ds->bDescriptorType, *current_class);
+			pr_vdebug("%s: unknown descriptor: %d for class %d\n",
+			      ffs->dev_name, _ds->bDescriptorType, *current_class);
 			return -EINVAL;
 		}
 
@@ -2089,7 +2089,7 @@ static int __must_check ffs_do_single_desc(struct ffs_data *ffs, char *data, uns
 
 	case USB_DT_INTERFACE_ASSOCIATION: {
 		struct usb_interface_assoc_descriptor *ds = (void *)_ds;
-		pr_vdebug("interface association descriptor\n");
+		pr_vdebug("%s: interface association descriptor\n", ffs->dev_name);
 		if (length != sizeof *ds)
 			goto inv_length;
 		if (ds->iFunction)
@@ -2098,7 +2098,7 @@ static int __must_check ffs_do_single_desc(struct ffs_data *ffs, char *data, uns
 		break;
 
 	case USB_DT_SS_ENDPOINT_COMP:
-		pr_vdebug("EP SS companion descriptor\n");
+		pr_vdebug("%s: EP SS companion descriptor\n", ffs->dev_name);
 		if (length != sizeof(struct usb_ss_ep_comp_descriptor))
 			goto inv_length;
 		break;
@@ -2109,17 +2109,18 @@ static int __must_check ffs_do_single_desc(struct ffs_data *ffs, char *data, uns
 	case USB_DT_SECURITY:
 	case USB_DT_CS_RADIO_CONTROL:
 		/* TODO */
-		pr_vdebug("unimplemented descriptor: %d\n", _ds->bDescriptorType);
+		pr_vdebug("%s: unimplemented descriptor: %d\n", ffs->dev_name,
+			_ds->bDescriptorType);
 		return -EINVAL;
 
 	default:
 		/* We should never be here */
-		pr_vdebug("unknown descriptor: %d\n", _ds->bDescriptorType);
+		pr_vdebug("%s: unknown descriptor: %d\n", ffs->dev_name, _ds->bDescriptorType);
 		return -EINVAL;
 
 inv_length:
-		pr_vdebug("invalid length: %d (descriptor %d)\n",
-			  _ds->bLength, _ds->bDescriptorType);
+		pr_vdebug("%s: invalid length: %d (descriptor %d)\n",
+			  ffs->dev_name, _ds->bLength, _ds->bDescriptorType);
 		return -EINVAL;
 	}
 
@@ -2148,8 +2149,8 @@ static int __must_check ffs_do_descs(struct ffs_data *ffs, unsigned count, char
 		/* Record "descriptor" entity */
 		ret = entity(ffs, FFS_DESCRIPTOR, (u8 *)num, (void *)data, priv);
 		if (ret < 0) {
-			pr_debug("entity DESCRIPTOR(%02lx); ret = %d\n",
-				 num, ret);
+			pr_debug("%s: entity DESCRIPTOR(%02lx); ret = %d\n",
+				 ffs->dev_name, num, ret);
 			return ret;
 		}
 
@@ -2159,7 +2160,7 @@ static int __must_check ffs_do_descs(struct ffs_data *ffs, unsigned count, char
 		ret = ffs_do_single_desc(ffs, data, len, entity, priv,
 			&current_class);
 		if (ret < 0) {
-			pr_debug("%s returns %d\n", __func__, ret);
+			pr_debug("%s: %s(): returns %d\n", ffs->dev_name, __func__, ret);
 			return ret;
 		}
 
@@ -2224,11 +2225,12 @@ static int __ffs_do_os_desc_header(struct ffs_data *ffs, enum ffs_os_desc_type *
 	u16 w_index = le16_to_cpu(desc->wIndex);
 
 	if (bcd_version == 0x1) {
-		pr_warn("bcdVersion must be 0x0100, stored in Little Endian order. "
-			"Userspace driver should be fixed, accepting 0x0001 for compatibility.\n");
+		pr_warn("%s: bcdVersion must be 0x0100, stored in Little Endian order. "
+			"Userspace driver should be fixed, accepting 0x0001 for compatibility.\n",
+			ffs->dev_name);
 	} else if (bcd_version != 0x100) {
-		pr_vdebug("unsupported os descriptors version: 0x%x\n",
-			  bcd_version);
+		pr_vdebug("%s: unsupported os descriptors version: 0x%x\n",
+			  ffs->dev_name, bcd_version);
 		return -EINVAL;
 	}
 	switch (w_index) {
@@ -2239,7 +2241,7 @@ static int __ffs_do_os_desc_header(struct ffs_data *ffs, enum ffs_os_desc_type *
 		*next_type = FFS_OS_DESC_EXT_PROP;
 		break;
 	default:
-		pr_vdebug("unsupported os descriptor type: %d", w_index);
+		pr_vdebug("%s: unsupported os descriptor type: %d", ffs->dev_name, w_index);
 		return -EINVAL;
 	}
 
@@ -2264,7 +2266,7 @@ static int __must_check ffs_do_single_os_desc(struct ffs_data *ffs, char *data,
 	while (feature_count--) {
 		ret = entity(ffs, type, h, data, len, priv);
 		if (ret < 0) {
-			pr_debug("bad OS descriptor, type: %d\n", type);
+			pr_debug("%s: bad OS descriptor, type: %d\n", ffs->dev_name, type);
 			return ret;
 		}
 		data += ret;
@@ -2302,8 +2304,8 @@ static int __must_check ffs_do_os_descs(struct ffs_data *ffs, unsigned count,
 
 		ret = __ffs_do_os_desc_header(ffs, &type, desc);
 		if (ret < 0) {
-			pr_debug("entity OS_DESCRIPTOR(%02lx); ret = %d\n",
-				 num, ret);
+			pr_debug("%s: entity OS_DESCRIPTOR(%02lx); ret = %d\n",
+				 ffs->dev_name, num, ret);
 			return ret;
 		}
 		/*
@@ -2323,7 +2325,7 @@ static int __must_check ffs_do_os_descs(struct ffs_data *ffs, unsigned count,
 		ret = ffs_do_single_os_desc(ffs, data, len, type,
 					    feature_count, entity, priv, desc);
 		if (ret < 0) {
-			pr_debug("%s returns %d\n", __func__, ret);
+			pr_debug("%s: %s() returns %d\n", ffs->dev_name, __func__, ret);
 			return ret;
 		}
 
@@ -2357,7 +2359,7 @@ static int __ffs_data_do_os_desc(struct ffs_data *ffs, enum ffs_os_desc_type typ
 			 * values.  We fix it here to avoid returning EINVAL
 			 * in response to values we used to accept.
 			 */
-			pr_debug("usb_ext_compat_desc::Reserved1 forced to 1\n");
+			pr_debug("%s: usb_ext_compat_desc::Reserved1 forced to 1\n", ffs->dev_name);
 			d->Reserved1 = 1;
 		}
 		for (i = 0; i < ARRAY_SIZE(d->Reserved2); ++i)
@@ -2380,20 +2382,20 @@ static int __ffs_data_do_os_desc(struct ffs_data *ffs, enum ffs_os_desc_type typ
 		type = le32_to_cpu(d->dwPropertyDataType);
 		if (type < USB_EXT_PROP_UNICODE ||
 		    type > USB_EXT_PROP_UNICODE_MULTI) {
-			pr_vdebug("unsupported os descriptor property type: %d",
-				  type);
+			pr_vdebug("%s: unsupported os descriptor property type: %d",
+				  ffs->dev_name, type);
 			return -EINVAL;
 		}
 		pnl = le16_to_cpu(d->wPropertyNameLength);
 		if (length < 14 + pnl) {
-			pr_vdebug("invalid os descriptor length: %d pnl:%d (descriptor %d)\n",
-				  length, pnl, type);
+			pr_vdebug("%s: invalid os descriptor length: %d pnl:%d (%d)\n",
+				  ffs->dev_name, length, pnl, type);
 			return -EINVAL;
 		}
 		pdl = le32_to_cpu(*(__le32 *)((u8 *)data + 10 + pnl));
 		if (length != 14 + pnl + pdl) {
-			pr_vdebug("invalid os descriptor length: %d pnl:%d pdl:%d (descriptor %d)\n",
-				  length, pnl, pdl, type);
+			pr_vdebug("%s: invalid os descriptor length: %d pnl:%d pdl:%d (%d)\n",
+				ffs->dev_name, length, pnl, pdl, type);
 			return -EINVAL;
 		}
 		++ffs->ms_os_descs_ext_prop_count;
@@ -2403,7 +2405,7 @@ static int __ffs_data_do_os_desc(struct ffs_data *ffs, enum ffs_os_desc_type typ
 	}
 		break;
 	default:
-		pr_vdebug("unknown descriptor: %d\n", type);
+		pr_vdebug("%s: unknown descriptor: %d\n", ffs->dev_name, type);
 		return -EINVAL;
 	}
 	return length;
@@ -2718,7 +2720,7 @@ static void __ffs_event_add(struct ffs_data *ffs,
 		break;
 
 	default:
-		WARN(1, "%d: unknown event, this should not happen\n", type);
+		WARN(1, "%s: %d: unknown event, this should not happen\n", ffs->dev_name, type);
 		return;
 	}
 
@@ -2729,11 +2731,11 @@ static void __ffs_event_add(struct ffs_data *ffs,
 			if ((*ev == rem_type1 || *ev == rem_type2) == neg)
 				*out++ = *ev;
 			else
-				pr_vdebug("purging event %d\n", *ev);
+				pr_vdebug("%s: purging event %d\n", ffs->dev_name, *ev);
 		ffs->ev.count = out - ffs->ev.types;
 	}
 
-	pr_vdebug("adding event %d\n", type);
+	pr_vdebug("%s: adding event %d\n", ffs->dev_name, type);
 	ffs->ev.types[ffs->ev.count++] = type;
 	wake_up_locked(&ffs->ev.waitq);
 	if (ffs->ffs_eventfd)
@@ -2802,8 +2804,8 @@ static int __ffs_func_bind_do_descs(struct ffs_data *ffs, enum ffs_entity_type t
 	ffs_ep = func->eps + idx;
 
 	if (ffs_ep->descs[ep_desc_id]) {
-		pr_err("two %sspeed descriptors for EP %d\n",
-			  speed_names[ep_desc_id],
+		pr_err("%s: two %sspeed descriptors for EP %d\n",
+			  ffs->dev_name, speed_names[ep_desc_id],
 			  ds->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK);
 		return -EINVAL;
 	}
@@ -2830,7 +2832,7 @@ static int __ffs_func_bind_do_descs(struct ffs_data *ffs, enum ffs_entity_type t
 		 * endpoint descriptors as if they were full speed.
 		 */
 		wMaxPacketSize = ds->wMaxPacketSize;
-		pr_vdebug("autoconfig\n");
+		pr_vdebug("%s: autoconfig\n", ffs->dev_name);
 		ep = usb_ep_autoconfig(func->gadget, ds);
 		if (!ep)
 			return -ENOTSUPP;
@@ -2911,7 +2913,7 @@ static int __ffs_func_bind_do_nums(struct ffs_data *ffs, enum ffs_entity_type ty
 		break;
 	}
 
-	pr_vdebug("%02x -> %02x\n", *valuep, newValue);
+	pr_vdebug("%s: %02x -> %02x\n", ffs->dev_name, *valuep, newValue);
 	*valuep = newValue;
 	return 0;
 }
@@ -2989,7 +2991,7 @@ static int __ffs_func_bind_do_os_desc(struct ffs_data *ffs, enum ffs_os_desc_typ
 	}
 		break;
 	default:
-		pr_vdebug("unknown descriptor: %d\n", type);
+		pr_vdebug("%s: unknown descriptor: %d\n", ffs->dev_name, type);
 	}
 
 	return length;
@@ -3283,11 +3285,11 @@ static int ffs_func_setup(struct usb_function *f,
 	unsigned long flags;
 	int ret;
 
-	pr_vdebug("creq->bRequestType = %02x\n", creq->bRequestType);
-	pr_vdebug("creq->bRequest     = %02x\n", creq->bRequest);
-	pr_vdebug("creq->wValue       = %04x\n", le16_to_cpu(creq->wValue));
-	pr_vdebug("creq->wIndex       = %04x\n", le16_to_cpu(creq->wIndex));
-	pr_vdebug("creq->wLength      = %04x\n", le16_to_cpu(creq->wLength));
+	pr_vdebug("%s: creq->bRequestType = %02x\n", ffs->dev_name, creq->bRequestType);
+	pr_vdebug("%s: creq->bRequest     = %02x\n", ffs->dev_name, creq->bRequest);
+	pr_vdebug("%s: creq->wValue       = %04x\n", ffs->dev_name, le16_to_cpu(creq->wValue));
+	pr_vdebug("%s: creq->wIndex       = %04x\n", ffs->dev_name, le16_to_cpu(creq->wIndex));
+	pr_vdebug("%s: creq->wLength      = %04x\n", ffs->dev_name, le16_to_cpu(creq->wLength));
 
 	/*
 	 * Most requests directed to interface go through here
@@ -3790,7 +3792,7 @@ static char *ffs_prepare_buffer(struct ffs_data *ffs, const char __user *buf, si
 	if (IS_ERR(data))
 		return data;
 
-	pr_vdebug("Buffer from user space:\n");
+	pr_vdebug("%s: Buffer from user space:\n", ffs->dev_name);
 	ffs_dump_mem("", data, len);
 
 	return data;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/3] usb: gadget: f_fs: remove ENTER() macro
  2023-03-24  6:10 [PATCH v2 1/3] usb: gadget: f_fs: remove ENTER() macro Linyu Yuan
  2023-03-24  6:10 ` [PATCH v2 2/3] usb: gadget: f_fs: add more function with struct ffs_data *ffs parameter Linyu Yuan
  2023-03-24  6:10 ` [PATCH v2 3/3] usb: gadget: f_fs: add dev name as prefix for pr_vdebug() Linyu Yuan
@ 2023-03-24 16:03 ` kernel test robot
  2023-03-24 16:03 ` kernel test robot
  2023-03-25  9:02 ` Greg Kroah-Hartman
  4 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2023-03-24 16:03 UTC (permalink / raw)
  To: Linyu Yuan, Greg Kroah-Hartman; +Cc: llvm, oe-kbuild-all, linux-usb, Linyu Yuan

Hi Linyu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus westeri-thunderbolt/next linus/master v6.3-rc3 next-20230324]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Linyu-Yuan/usb-gadget-f_fs-add-more-function-with-struct-ffs_data-ffs-parameter/20230324-141223
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/1679638227-20496-1-git-send-email-quic_linyyuan%40quicinc.com
patch subject: [PATCH v2 1/3] usb: gadget: f_fs: remove ENTER() macro
config: x86_64-randconfig-a012 (https://download.01.org/0day-ci/archive/20230324/202303242346.LgEJTJJR-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/f4623ea5e9a18bfb3e96bc6566afe46ebdefff33
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Linyu-Yuan/usb-gadget-f_fs-add-more-function-with-struct-ffs_data-ffs-parameter/20230324-141223
        git checkout f4623ea5e9a18bfb3e96bc6566afe46ebdefff33
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/usb/gadget/legacy/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303242346.LgEJTJJR-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/usb/gadget/legacy/g_ffs.c:183:2: error: implicit declaration of function 'ENTER' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
           ENTER();
           ^
   drivers/usb/gadget/legacy/g_ffs.c:245:2: error: implicit declaration of function 'ENTER' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
           ENTER();
           ^
   drivers/usb/gadget/legacy/g_ffs.c:319:2: error: implicit declaration of function 'ENTER' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
           ENTER();
           ^
   drivers/usb/gadget/legacy/g_ffs.c:448:2: error: implicit declaration of function 'ENTER' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
           ENTER();
           ^
   4 errors generated.


vim +/ENTER +183 drivers/usb/gadget/legacy/g_ffs.c

c6c560085172c1 drivers/usb/gadget/g_ffs.c Michal Nazarewicz     2010-05-05  176  
8545e6031a7196 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-03-12  177  static int __init gfs_init(void)
c6c560085172c1 drivers/usb/gadget/g_ffs.c Michal Nazarewicz     2010-05-05  178  {
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  179  	struct f_fs_opts *opts;
581791f5c7a480 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-05-14  180  	int i;
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  181  	int ret = 0;
581791f5c7a480 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-05-14  182  
c6c560085172c1 drivers/usb/gadget/g_ffs.c Michal Nazarewicz     2010-05-05 @183  	ENTER();
c6c560085172c1 drivers/usb/gadget/g_ffs.c Michal Nazarewicz     2010-05-05  184  
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  185  	if (func_num < 2) {
581791f5c7a480 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-05-14  186  		gfs_single_func = true;
581791f5c7a480 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-05-14  187  		func_num = 1;
581791f5c7a480 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-05-14  188  	}
581791f5c7a480 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-05-14  189  
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  190  	/*
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  191  	 * Allocate in one chunk for easier maintenance
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  192  	 */
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  193  	f_ffs[0] = kcalloc(func_num * N_CONF, sizeof(*f_ffs), GFP_KERNEL);
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  194  	if (!f_ffs[0]) {
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  195  		ret = -ENOMEM;
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  196  		goto no_func;
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  197  	}
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  198  	for (i = 1; i < N_CONF; ++i)
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  199  		f_ffs[i] = f_ffs[0] + i * func_num;
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  200  
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  201  	fi_ffs = kcalloc(func_num, sizeof(*fi_ffs), GFP_KERNEL);
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  202  	if (!fi_ffs) {
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  203  		ret = -ENOMEM;
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  204  		goto no_func;
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  205  	}
581791f5c7a480 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-05-14  206  
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  207  	for (i = 0; i < func_num; i++) {
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  208  		fi_ffs[i] = usb_get_function_instance("ffs");
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  209  		if (IS_ERR(fi_ffs[i])) {
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  210  			ret = PTR_ERR(fi_ffs[i]);
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  211  			--i;
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  212  			goto no_dev;
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  213  		}
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  214  		opts = to_f_fs_opts(fi_ffs[i]);
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  215  		if (gfs_single_func)
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  216  			ret = ffs_single_dev(opts->dev);
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  217  		else
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  218  			ret = ffs_name_dev(opts->dev, func_names[i]);
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  219  		if (ret)
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  220  			goto no_dev;
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  221  		opts->dev->ffs_ready_callback = functionfs_ready_callback;
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  222  		opts->dev->ffs_closed_callback = functionfs_closed_callback;
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  223  		opts->dev->ffs_acquire_dev_callback = functionfs_acquire_dev;
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  224  		opts->dev->ffs_release_dev_callback = functionfs_release_dev;
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  225  		opts->no_configfs = true;
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  226  	}
581791f5c7a480 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-05-14  227  
581791f5c7a480 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-05-14  228  	missing_funcs = func_num;
581791f5c7a480 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-05-14  229  
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  230  	return 0;
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  231  no_dev:
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  232  	while (i >= 0)
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  233  		usb_put_function_instance(fi_ffs[i--]);
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  234  	kfree(fi_ffs);
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  235  no_func:
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  236  	kfree(f_ffs[0]);
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  237  	return ret;
c6c560085172c1 drivers/usb/gadget/g_ffs.c Michal Nazarewicz     2010-05-05  238  }
c6c560085172c1 drivers/usb/gadget/g_ffs.c Michal Nazarewicz     2010-05-05  239  module_init(gfs_init);
c6c560085172c1 drivers/usb/gadget/g_ffs.c Michal Nazarewicz     2010-05-05  240  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/3] usb: gadget: f_fs: remove ENTER() macro
  2023-03-24  6:10 [PATCH v2 1/3] usb: gadget: f_fs: remove ENTER() macro Linyu Yuan
                   ` (2 preceding siblings ...)
  2023-03-24 16:03 ` [PATCH v2 1/3] usb: gadget: f_fs: remove ENTER() macro kernel test robot
@ 2023-03-24 16:03 ` kernel test robot
  2023-03-25  9:02 ` Greg Kroah-Hartman
  4 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2023-03-24 16:03 UTC (permalink / raw)
  To: Linyu Yuan, Greg Kroah-Hartman; +Cc: llvm, oe-kbuild-all, linux-usb, Linyu Yuan

Hi Linyu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus westeri-thunderbolt/next linus/master v6.3-rc3 next-20230324]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Linyu-Yuan/usb-gadget-f_fs-add-more-function-with-struct-ffs_data-ffs-parameter/20230324-141223
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/1679638227-20496-1-git-send-email-quic_linyyuan%40quicinc.com
patch subject: [PATCH v2 1/3] usb: gadget: f_fs: remove ENTER() macro
config: i386-randconfig-a015 (https://download.01.org/0day-ci/archive/20230324/202303242317.aIQQrrX1-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/f4623ea5e9a18bfb3e96bc6566afe46ebdefff33
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Linyu-Yuan/usb-gadget-f_fs-add-more-function-with-struct-ffs_data-ffs-parameter/20230324-141223
        git checkout f4623ea5e9a18bfb3e96bc6566afe46ebdefff33
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/usb/gadget/legacy/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303242317.aIQQrrX1-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/usb/gadget/legacy/g_ffs.c:183:2: error: implicit declaration of function 'ENTER' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
           ENTER();
           ^
   drivers/usb/gadget/legacy/g_ffs.c:245:2: error: implicit declaration of function 'ENTER' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
           ENTER();
           ^
   drivers/usb/gadget/legacy/g_ffs.c:319:2: error: implicit declaration of function 'ENTER' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
           ENTER();
           ^
   drivers/usb/gadget/legacy/g_ffs.c:448:2: error: implicit declaration of function 'ENTER' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
           ENTER();
           ^
   4 errors generated.


vim +/ENTER +183 drivers/usb/gadget/legacy/g_ffs.c

c6c560085172c1 drivers/usb/gadget/g_ffs.c Michal Nazarewicz     2010-05-05  176  
8545e6031a7196 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-03-12  177  static int __init gfs_init(void)
c6c560085172c1 drivers/usb/gadget/g_ffs.c Michal Nazarewicz     2010-05-05  178  {
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  179  	struct f_fs_opts *opts;
581791f5c7a480 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-05-14  180  	int i;
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  181  	int ret = 0;
581791f5c7a480 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-05-14  182  
c6c560085172c1 drivers/usb/gadget/g_ffs.c Michal Nazarewicz     2010-05-05 @183  	ENTER();
c6c560085172c1 drivers/usb/gadget/g_ffs.c Michal Nazarewicz     2010-05-05  184  
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  185  	if (func_num < 2) {
581791f5c7a480 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-05-14  186  		gfs_single_func = true;
581791f5c7a480 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-05-14  187  		func_num = 1;
581791f5c7a480 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-05-14  188  	}
581791f5c7a480 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-05-14  189  
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  190  	/*
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  191  	 * Allocate in one chunk for easier maintenance
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  192  	 */
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  193  	f_ffs[0] = kcalloc(func_num * N_CONF, sizeof(*f_ffs), GFP_KERNEL);
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  194  	if (!f_ffs[0]) {
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  195  		ret = -ENOMEM;
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  196  		goto no_func;
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  197  	}
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  198  	for (i = 1; i < N_CONF; ++i)
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  199  		f_ffs[i] = f_ffs[0] + i * func_num;
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  200  
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  201  	fi_ffs = kcalloc(func_num, sizeof(*fi_ffs), GFP_KERNEL);
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  202  	if (!fi_ffs) {
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  203  		ret = -ENOMEM;
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  204  		goto no_func;
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  205  	}
581791f5c7a480 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-05-14  206  
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  207  	for (i = 0; i < func_num; i++) {
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  208  		fi_ffs[i] = usb_get_function_instance("ffs");
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  209  		if (IS_ERR(fi_ffs[i])) {
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  210  			ret = PTR_ERR(fi_ffs[i]);
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  211  			--i;
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  212  			goto no_dev;
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  213  		}
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  214  		opts = to_f_fs_opts(fi_ffs[i]);
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  215  		if (gfs_single_func)
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  216  			ret = ffs_single_dev(opts->dev);
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  217  		else
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  218  			ret = ffs_name_dev(opts->dev, func_names[i]);
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  219  		if (ret)
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  220  			goto no_dev;
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  221  		opts->dev->ffs_ready_callback = functionfs_ready_callback;
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  222  		opts->dev->ffs_closed_callback = functionfs_closed_callback;
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  223  		opts->dev->ffs_acquire_dev_callback = functionfs_acquire_dev;
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  224  		opts->dev->ffs_release_dev_callback = functionfs_release_dev;
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  225  		opts->no_configfs = true;
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  226  	}
581791f5c7a480 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-05-14  227  
581791f5c7a480 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-05-14  228  	missing_funcs = func_num;
581791f5c7a480 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2012-05-14  229  
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  230  	return 0;
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  231  no_dev:
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  232  	while (i >= 0)
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  233  		usb_put_function_instance(fi_ffs[i--]);
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  234  	kfree(fi_ffs);
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  235  no_func:
6f823cd5305c78 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  236  	kfree(f_ffs[0]);
4b187fceec3c73 drivers/usb/gadget/g_ffs.c Andrzej Pietrasiewicz 2013-12-03  237  	return ret;
c6c560085172c1 drivers/usb/gadget/g_ffs.c Michal Nazarewicz     2010-05-05  238  }
c6c560085172c1 drivers/usb/gadget/g_ffs.c Michal Nazarewicz     2010-05-05  239  module_init(gfs_init);
c6c560085172c1 drivers/usb/gadget/g_ffs.c Michal Nazarewicz     2010-05-05  240  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] usb: gadget: f_fs: add dev name as prefix for pr_vdebug()
  2023-03-24  6:10 ` [PATCH v2 3/3] usb: gadget: f_fs: add dev name as prefix for pr_vdebug() Linyu Yuan
@ 2023-03-25  9:00   ` Greg Kroah-Hartman
  2023-03-26  2:55     ` Linyu Yuan
  0 siblings, 1 reply; 13+ messages in thread
From: Greg Kroah-Hartman @ 2023-03-25  9:00 UTC (permalink / raw)
  To: Linyu Yuan; +Cc: linux-usb

On Fri, Mar 24, 2023 at 02:10:27PM +0800, Linyu Yuan wrote:
> when multiple instances in use, the debug message is hard to understand
> as there is no instance name show.
> 
> this change will show each instance name for debug messages.
> 
> Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
> ---
> v2: split to several changes according to v1 comments
> v1: https://lore.kernel.org/linux-usb/1679481369-30094-1-git-send-email-quic_linyyuan@quicinc.com/
> 
>  drivers/usb/gadget/function/f_fs.c | 136 +++++++++++++++++++------------------
>  1 file changed, 69 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
> index a4051c8..df67ab5 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -317,12 +317,12 @@ static int __ffs_ep0_queue_wait(struct ffs_data *ffs, char *data, size_t len)
>  static int __ffs_ep0_stall(struct ffs_data *ffs)
>  {
>  	if (ffs->ev.can_stall) {
> -		pr_vdebug("ep0 stall\n");
> +		pr_vdebug("%s: ep0 stall\n", ffs->dev_name);

Again, no, please use dev_dbg() instead.  Do NOT roll your own debugging
macros.  You have access to a struct device pointer for this device that
the driver is controlling, so please always use that instead.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/3] usb: gadget: f_fs: add more function with struct ffs_data *ffs parameter
  2023-03-24  6:10 ` [PATCH v2 2/3] usb: gadget: f_fs: add more function with struct ffs_data *ffs parameter Linyu Yuan
@ 2023-03-25  9:01   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 13+ messages in thread
From: Greg Kroah-Hartman @ 2023-03-25  9:01 UTC (permalink / raw)
  To: Linyu Yuan; +Cc: linux-usb

On Fri, Mar 24, 2023 at 02:10:26PM +0800, Linyu Yuan wrote:
> It prepare to improve pr_vdebug() which will show dev_name when multiple
> f_fs instance exist.

I am sorry, but I can not parse this.  Please describe why you are doing
this better next time.

> 
> Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
> ---
> v2: split to several changes according to v1 comments
> v1: https://lore.kernel.org/linux-usb/1679481369-30094-1-git-send-email-quic_linyyuan@quicinc.com/
> 
>  drivers/usb/gadget/function/f_fs.c | 78 +++++++++++++++++++-------------------
>  1 file changed, 38 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
> index 8830847..a4051c8 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -231,7 +231,6 @@ struct ffs_io_data {
>  };
>  
>  struct ffs_desc_helper {
> -	struct ffs_data *ffs;

Your subject line says you are adding more function, but you are really
removing the structure pointer here?  Why?

Shouldn't removing this pointer be a separate commit?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/3] usb: gadget: f_fs: remove ENTER() macro
  2023-03-24  6:10 [PATCH v2 1/3] usb: gadget: f_fs: remove ENTER() macro Linyu Yuan
                   ` (3 preceding siblings ...)
  2023-03-24 16:03 ` kernel test robot
@ 2023-03-25  9:02 ` Greg Kroah-Hartman
  2023-03-26  2:48   ` Linyu Yuan
  4 siblings, 1 reply; 13+ messages in thread
From: Greg Kroah-Hartman @ 2023-03-25  9:02 UTC (permalink / raw)
  To: Linyu Yuan; +Cc: linux-usb

On Fri, Mar 24, 2023 at 02:10:25PM +0800, Linyu Yuan wrote:
> ENTER() used to show function name which called during runtime, ftrace can
> be used to get same information, let's remove it.
> 
> Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
> ---
> v2: split to several changes according to v1 comments
> v1: https://lore.kernel.org/linux-usb/1679481369-30094-1-git-send-email-quic_linyyuan@quicinc.com/
> 
>  drivers/usb/gadget/function/f_fs.c | 94 --------------------------------------
>  drivers/usb/gadget/function/u_fs.h |  2 -
>  2 files changed, 96 deletions(-)

As the kernel-test-bot points out, this patch does not build :(


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/3] usb: gadget: f_fs: remove ENTER() macro
  2023-03-25  9:02 ` Greg Kroah-Hartman
@ 2023-03-26  2:48   ` Linyu Yuan
  0 siblings, 0 replies; 13+ messages in thread
From: Linyu Yuan @ 2023-03-26  2:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-usb


On 3/25/2023 5:02 PM, Greg Kroah-Hartman wrote:
> On Fri, Mar 24, 2023 at 02:10:25PM +0800, Linyu Yuan wrote:
>> ENTER() used to show function name which called during runtime, ftrace can
>> be used to get same information, let's remove it.
>>
>> Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
>> ---
>> v2: split to several changes according to v1 comments
>> v1: https://lore.kernel.org/linux-usb/1679481369-30094-1-git-send-email-quic_linyyuan@quicinc.com/
>>
>>   drivers/usb/gadget/function/f_fs.c | 94 --------------------------------------
>>   drivers/usb/gadget/function/u_fs.h |  2 -
>>   2 files changed, 96 deletions(-)
> As the kernel-test-bot points out, this patch does not build :(

it is truth that only build and test f_fs.c, didn't know there is 
another legacy driver use u_fs.h.


>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] usb: gadget: f_fs: add dev name as prefix for pr_vdebug()
  2023-03-25  9:00   ` Greg Kroah-Hartman
@ 2023-03-26  2:55     ` Linyu Yuan
  2023-03-26  6:29       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 13+ messages in thread
From: Linyu Yuan @ 2023-03-26  2:55 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-usb


On 3/25/2023 5:00 PM, Greg Kroah-Hartman wrote:
> On Fri, Mar 24, 2023 at 02:10:27PM +0800, Linyu Yuan wrote:
>> when multiple instances in use, the debug message is hard to understand
>> as there is no instance name show.
>>
>> this change will show each instance name for debug messages.
>>
>> Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
>> ---
>> v2: split to several changes according to v1 comments
>> v1: https://lore.kernel.org/linux-usb/1679481369-30094-1-git-send-email-quic_linyyuan@quicinc.com/
>>
>>   drivers/usb/gadget/function/f_fs.c | 136 +++++++++++++++++++------------------
>>   1 file changed, 69 insertions(+), 67 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
>> index a4051c8..df67ab5 100644
>> --- a/drivers/usb/gadget/function/f_fs.c
>> +++ b/drivers/usb/gadget/function/f_fs.c
>> @@ -317,12 +317,12 @@ static int __ffs_ep0_queue_wait(struct ffs_data *ffs, char *data, size_t len)
>>   static int __ffs_ep0_stall(struct ffs_data *ffs)
>>   {
>>   	if (ffs->ev.can_stall) {
>> -		pr_vdebug("ep0 stall\n");
>> +		pr_vdebug("%s: ep0 stall\n", ffs->dev_name);
> Again, no, please use dev_dbg() instead.  Do NOT roll your own debugging
> macros.  You have access to a struct device pointer for this device that
> the driver is controlling, so please always use that instead.


thanks for your suggestion, i didn't know dev_dbg can accept NULL dev 
pointer

as this driver have no real struct device.


will change to dev_dbg with NULL dev pointer.



>
> thanks,
>
> greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] usb: gadget: f_fs: add dev name as prefix for pr_vdebug()
  2023-03-26  2:55     ` Linyu Yuan
@ 2023-03-26  6:29       ` Greg Kroah-Hartman
  2023-03-27  1:54         ` Linyu Yuan
  0 siblings, 1 reply; 13+ messages in thread
From: Greg Kroah-Hartman @ 2023-03-26  6:29 UTC (permalink / raw)
  To: Linyu Yuan; +Cc: linux-usb

On Sun, Mar 26, 2023 at 10:55:18AM +0800, Linyu Yuan wrote:
> 
> On 3/25/2023 5:00 PM, Greg Kroah-Hartman wrote:
> > On Fri, Mar 24, 2023 at 02:10:27PM +0800, Linyu Yuan wrote:
> > > when multiple instances in use, the debug message is hard to understand
> > > as there is no instance name show.
> > > 
> > > this change will show each instance name for debug messages.
> > > 
> > > Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
> > > ---
> > > v2: split to several changes according to v1 comments
> > > v1: https://lore.kernel.org/linux-usb/1679481369-30094-1-git-send-email-quic_linyyuan@quicinc.com/
> > > 
> > >   drivers/usb/gadget/function/f_fs.c | 136 +++++++++++++++++++------------------
> > >   1 file changed, 69 insertions(+), 67 deletions(-)
> > > 
> > > diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
> > > index a4051c8..df67ab5 100644
> > > --- a/drivers/usb/gadget/function/f_fs.c
> > > +++ b/drivers/usb/gadget/function/f_fs.c
> > > @@ -317,12 +317,12 @@ static int __ffs_ep0_queue_wait(struct ffs_data *ffs, char *data, size_t len)
> > >   static int __ffs_ep0_stall(struct ffs_data *ffs)
> > >   {
> > >   	if (ffs->ev.can_stall) {
> > > -		pr_vdebug("ep0 stall\n");
> > > +		pr_vdebug("%s: ep0 stall\n", ffs->dev_name);
> > Again, no, please use dev_dbg() instead.  Do NOT roll your own debugging
> > macros.  You have access to a struct device pointer for this device that
> > the driver is controlling, so please always use that instead.
> 
> 
> thanks for your suggestion, i didn't know dev_dbg can accept NULL dev
> pointer
> 
> as this driver have no real struct device.

That is not true, you have access to a struct usb_gadget, which is a
struct device.  Use that please.

> will change to dev_dbg with NULL dev pointer.

No, that is pointless, please do not do that.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] usb: gadget: f_fs: add dev name as prefix for pr_vdebug()
  2023-03-26  6:29       ` Greg Kroah-Hartman
@ 2023-03-27  1:54         ` Linyu Yuan
  2023-03-27  5:46           ` Greg Kroah-Hartman
  0 siblings, 1 reply; 13+ messages in thread
From: Linyu Yuan @ 2023-03-27  1:54 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-usb


On 3/26/2023 2:29 PM, Greg Kroah-Hartman wrote:
> On Sun, Mar 26, 2023 at 10:55:18AM +0800, Linyu Yuan wrote:
>> On 3/25/2023 5:00 PM, Greg Kroah-Hartman wrote:
>>> On Fri, Mar 24, 2023 at 02:10:27PM +0800, Linyu Yuan wrote:
>>>> when multiple instances in use, the debug message is hard to understand
>>>> as there is no instance name show.
>>>>
>>>> this change will show each instance name for debug messages.
>>>>
>>>> Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
>>>> ---
>>>> v2: split to several changes according to v1 comments
>>>> v1: https://lore.kernel.org/linux-usb/1679481369-30094-1-git-send-email-quic_linyyuan@quicinc.com/
>>>>
>>>>    drivers/usb/gadget/function/f_fs.c | 136 +++++++++++++++++++------------------
>>>>    1 file changed, 69 insertions(+), 67 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
>>>> index a4051c8..df67ab5 100644
>>>> --- a/drivers/usb/gadget/function/f_fs.c
>>>> +++ b/drivers/usb/gadget/function/f_fs.c
>>>> @@ -317,12 +317,12 @@ static int __ffs_ep0_queue_wait(struct ffs_data *ffs, char *data, size_t len)
>>>>    static int __ffs_ep0_stall(struct ffs_data *ffs)
>>>>    {
>>>>    	if (ffs->ev.can_stall) {
>>>> -		pr_vdebug("ep0 stall\n");
>>>> +		pr_vdebug("%s: ep0 stall\n", ffs->dev_name);
>>> Again, no, please use dev_dbg() instead.  Do NOT roll your own debugging
>>> macros.  You have access to a struct device pointer for this device that
>>> the driver is controlling, so please always use that instead.
>>
>> thanks for your suggestion, i didn't know dev_dbg can accept NULL dev
>> pointer
>>
>> as this driver have no real struct device.
> That is not true, you have access to a struct usb_gadget, which is a
> struct device.  Use that please.

but this is not good, as for gadget/udc, from my view, there are two layer,

one configfs driver, one lower UDC, only after bind, there will be a 
reference to usb_gadget.

but configuration to driver can happen before bind, if so the usb_gadget 
will be NULL.


>
>> will change to dev_dbg with NULL dev pointer.
> No, that is pointless, please do not do that.
>
> thanks,
>
> greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] usb: gadget: f_fs: add dev name as prefix for pr_vdebug()
  2023-03-27  1:54         ` Linyu Yuan
@ 2023-03-27  5:46           ` Greg Kroah-Hartman
  0 siblings, 0 replies; 13+ messages in thread
From: Greg Kroah-Hartman @ 2023-03-27  5:46 UTC (permalink / raw)
  To: Linyu Yuan; +Cc: linux-usb

On Mon, Mar 27, 2023 at 09:54:11AM +0800, Linyu Yuan wrote:
> 
> On 3/26/2023 2:29 PM, Greg Kroah-Hartman wrote:
> > On Sun, Mar 26, 2023 at 10:55:18AM +0800, Linyu Yuan wrote:
> > > On 3/25/2023 5:00 PM, Greg Kroah-Hartman wrote:
> > > > On Fri, Mar 24, 2023 at 02:10:27PM +0800, Linyu Yuan wrote:
> > > > > when multiple instances in use, the debug message is hard to understand
> > > > > as there is no instance name show.
> > > > > 
> > > > > this change will show each instance name for debug messages.
> > > > > 
> > > > > Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
> > > > > ---
> > > > > v2: split to several changes according to v1 comments
> > > > > v1: https://lore.kernel.org/linux-usb/1679481369-30094-1-git-send-email-quic_linyyuan@quicinc.com/
> > > > > 
> > > > >    drivers/usb/gadget/function/f_fs.c | 136 +++++++++++++++++++------------------
> > > > >    1 file changed, 69 insertions(+), 67 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
> > > > > index a4051c8..df67ab5 100644
> > > > > --- a/drivers/usb/gadget/function/f_fs.c
> > > > > +++ b/drivers/usb/gadget/function/f_fs.c
> > > > > @@ -317,12 +317,12 @@ static int __ffs_ep0_queue_wait(struct ffs_data *ffs, char *data, size_t len)
> > > > >    static int __ffs_ep0_stall(struct ffs_data *ffs)
> > > > >    {
> > > > >    	if (ffs->ev.can_stall) {
> > > > > -		pr_vdebug("ep0 stall\n");
> > > > > +		pr_vdebug("%s: ep0 stall\n", ffs->dev_name);
> > > > Again, no, please use dev_dbg() instead.  Do NOT roll your own debugging
> > > > macros.  You have access to a struct device pointer for this device that
> > > > the driver is controlling, so please always use that instead.
> > > 
> > > thanks for your suggestion, i didn't know dev_dbg can accept NULL dev
> > > pointer
> > > 
> > > as this driver have no real struct device.
> > That is not true, you have access to a struct usb_gadget, which is a
> > struct device.  Use that please.
> 
> but this is not good, as for gadget/udc, from my view, there are two layer,
> 
> one configfs driver, one lower UDC, only after bind, there will be a
> reference to usb_gadget.
> 
> but configuration to driver can happen before bind, if so the usb_gadget
> will be NULL.

But is that where this debug message is being called?  This is a
function while the driver is bound to a device, so you have a usb_gadget
pointer here.

If you have messages that could happen before the device is bound, sure,
don't use the device there.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-03-27  5:46 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-24  6:10 [PATCH v2 1/3] usb: gadget: f_fs: remove ENTER() macro Linyu Yuan
2023-03-24  6:10 ` [PATCH v2 2/3] usb: gadget: f_fs: add more function with struct ffs_data *ffs parameter Linyu Yuan
2023-03-25  9:01   ` Greg Kroah-Hartman
2023-03-24  6:10 ` [PATCH v2 3/3] usb: gadget: f_fs: add dev name as prefix for pr_vdebug() Linyu Yuan
2023-03-25  9:00   ` Greg Kroah-Hartman
2023-03-26  2:55     ` Linyu Yuan
2023-03-26  6:29       ` Greg Kroah-Hartman
2023-03-27  1:54         ` Linyu Yuan
2023-03-27  5:46           ` Greg Kroah-Hartman
2023-03-24 16:03 ` [PATCH v2 1/3] usb: gadget: f_fs: remove ENTER() macro kernel test robot
2023-03-24 16:03 ` kernel test robot
2023-03-25  9:02 ` Greg Kroah-Hartman
2023-03-26  2:48   ` Linyu Yuan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.