On Wed, Mar 22, 2023 at 06:36:09PM +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 all most all debug messsage.
>
> Signed-off-by: Linyu Yuan <quic_linyyuan@xxxxxxxxxxx>
> ---
> drivers/usb/gadget/function/f_fs.c | 406 +++++++++++++++++++------------------
That's a lot of churn just for debugging messages.
But that's good, the debugging code here needs lots of work, so let's do
this properly!
> drivers/usb/gadget/function/u_fs.h | 2 +
> 2 files changed, 215 insertions(+), 193 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
> index a277c70..5abeb18 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -231,13 +231,13 @@ struct ffs_io_data {
> };
>
> struct ffs_desc_helper {
> - struct ffs_data *ffs;
> - unsigned interfaces_count;
> - unsigned eps_count;
> + unsigned int interfaces_count;
> + unsigned int eps_count;
Why did you change interfaces_count and eps_count for a debugging
message change?
This doesn't make me feel good about this...
> };
>
> static int __must_check ffs_epfiles_create(struct ffs_data *ffs);
> -static void ffs_epfiles_destroy(struct ffs_epfile *epfiles, unsigned count);
> +static void ffs_epfiles_destroy(struct ffs_data *ffs, struct ffs_epfile *epfiles,
> + unsigned int count);
>
> static struct dentry *
> ffs_sb_create_file(struct super_block *sb, const char *name, void *data,
> @@ -258,9 +258,9 @@ static void ffs_closed(struct ffs_data *ffs);
>
> /* Misc helper functions ****************************************************/
>
> -static int ffs_mutex_lock(struct mutex *mutex, unsigned nonblock)
> +static int ffs_mutex_lock(struct mutex *mutex, unsigned int 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));
>
>
> @@ -318,12 +318,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);
Please don't try to create a custom version of the widely used dev_dbg()
call like you are doing here. Just use dev_dbg(). Why can't you do
that?
> 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;
> }
> }
> @@ -335,7 +335,7 @@ static ssize_t ffs_ep0_write(struct file *file, const char __user *buf,
> ssize_t ret;
> char *data;
>
> - ENTER();
> + ENTER_FFS();
Why change the macro name if no options are changed?
And why is this macro needed at all anymore? You should just use
ftrace, and these macros should be deleted as they are nothing that we
would ever accept in new submissions.
So how about making this a patch series, one that removed the unneeded
tracing macros like ENTER(), and then one that uses the proper dev_dbg()
functions and then, if you really want it, one that fixes up the use of
"unsigned" which has nothing to do with debug messages.
thanks,
greg k-h
