Re: [PATCH v3 4/6] usb: gadget: f_fs: add a device reference of usb_gadget->dev

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 3/29/2023 4:21 PM, Greg Kroah-Hartman wrote:
On Wed, Mar 29, 2023 at 03:46:45PM +0800, Linyu Yuan wrote:
On 3/29/2023 3:31 PM, Greg Kroah-Hartman wrote:
On Wed, Mar 29, 2023 at 03:00:54PM +0800, Linyu Yuan wrote:
On 3/29/2023 2:53 PM, Greg Kroah-Hartman wrote:
On Mon, Mar 27, 2023 at 06:12:18PM +0800, Linyu Yuan wrote:
It is known that dev_vdbg() macro can accept NULL or non-NULL dev pointer.

Add a struct device *dev member in struct ffs_data, set it to NULL before
binding or after unbinding to a usb_gadget, set it reference of usb_gadget
->dev when bind success.

Then it can help replace private pr_vdebug() to dev_vdbg() consistently.

Signed-off-by: Linyu Yuan <quic_linyyuan@xxxxxxxxxxx>
---
v3: new patch in this version

    drivers/usb/gadget/function/f_fs.c | 3 +++
    drivers/usb/gadget/function/u_fs.h | 1 +
    2 files changed, 4 insertions(+)

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index a4051c8..25461f1 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -1722,6 +1722,7 @@ static struct ffs_data *ffs_data_new(const char *dev_name)
    		return NULL;
    	}
+	ffs->dev = NULL;
    	refcount_set(&ffs->ref, 1);
    	atomic_set(&ffs->opened, 0);
    	ffs->state = FFS_READ_DESCRIPTORS;
@@ -1831,6 +1832,7 @@ static int functionfs_bind(struct ffs_data *ffs, struct usb_composite_dev *cdev)
    	}
    	ffs->gadget = cdev->gadget;
+	ffs->dev = &cdev->gadget->dev;
    	ffs_data_get(ffs);
    	return 0;
    }
@@ -1843,6 +1845,7 @@ static void functionfs_unbind(struct ffs_data *ffs)
    		mutex_lock(&ffs->mutex);
    		usb_ep_free_request(ffs->gadget->ep0, ffs->ep0req);
    		ffs->ep0req = NULL;
+		ffs->dev = NULL;
    		ffs->gadget = NULL;
    		clear_bit(FFS_FL_BOUND, &ffs->flags);
    		mutex_unlock(&ffs->mutex);
diff --git a/drivers/usb/gadget/function/u_fs.h b/drivers/usb/gadget/function/u_fs.h
index 4b3365f..c5f6167 100644
--- a/drivers/usb/gadget/function/u_fs.h
+++ b/drivers/usb/gadget/function/u_fs.h
@@ -146,6 +146,7 @@ enum ffs_setup_state {
    struct ffs_data {
    	struct usb_gadget		*gadget;
+	struct device			*dev;
No, sorry, this is not correct.

You already have a struct device right there in the struct usb_gadget.
Use that one instead, as you are just setting this pointer to the same
value (see above where you set it.)
just want to use consistent dev_(v)dbg() related macro, to avoid reference
usb_gadget->dev

when usb_gadget is NULL.
When will usb_gadget be NULL when you want to print out logging
messages?  You shouldn't be printing out anything during that time
anyway, right?

when usb_gadget is NULL, there could be debug message because user space
application

can start configure the ffs instance (like adb ...) for USB
interface/endpoint/string descriptor.
But there is a "real" device down there somewhere as there would not be
any way for the driver to be able to talk to the hardware.  So please
use that.


no real device before bind, it only has a file.



as dev_dbg related macro is safe to accept NULL, there is no need find out
when will

usb_gadget is NULL and when will it a valid pointer.
Don't abuse dev_dbg() like this, that's not going to help out much
overall, and makes no sense to convert to something that is going to
print out incorrect messages (again, there is a device there.)

thanks,

greg k-h



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux