Re: [PATCH v3] usb: gadget: f_fs: expose ready state in configfs

[Peter Korsgaard <peter@korsgaard.com>]

>>>>> "Andrzej" == Andrzej Pietrasiewicz <andrzej.p@collabora.com> writes:

 > Hi Peter,
 > W dniu 18.01.2024 o 15:48, Peter Korsgaard pisze:
 >> When a USB gadget is configured through configfs with 1 or more f_fs
 >> functions, then the logic setting up the gadget configuration has to wait
 >> until the user space code (typically separate applications) responsible for
 >> those functions have written their descriptors before the gadget can be
 >> activated.
 >> The f_fs instance already knows if this has been done, so expose it
 >> through
 >> a "ready" attribute in configfs for easier synchronization.
 >> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
 >> ---
 >> Changes since v2:
 >> - Add ABI documentation as requested by Greg
 >> Changes since v1:
 >> - Add documentation snippet as requested by Greg.
 >> Documentation/ABI/testing/configfs-usb-gadget-ffs | 12
 >> ++++++++++--
 >> Documentation/usb/gadget-testing.rst              |  8 ++++++++
 >> drivers/usb/gadget/function/f_fs.c                | 15 +++++++++++++++
 >> 3 files changed, 33 insertions(+), 2 deletions(-)
 >> diff --git a/Documentation/ABI/testing/configfs-usb-gadget-ffs
 >> b/Documentation/ABI/testing/configfs-usb-gadget-ffs
 >> index e39b27653c65..bf8936ff6d38 100644
 >> --- a/Documentation/ABI/testing/configfs-usb-gadget-ffs
 >> +++ b/Documentation/ABI/testing/configfs-usb-gadget-ffs
 >> @@ -4,6 +4,14 @@ KernelVersion:	3.13
 >> Description:	The purpose of this directory is to create and remove it.
 >> A corresponding USB function instance is
 >> created/removed.
 >> -		There are no attributes here.
 >> -		All parameters are set through FunctionFS.
 >> +		All attributes are read only:
 >> +
 >> +		=============	============================================
 >> +		ready		1 if the function is ready to be used, E.G.
 >> +				if userspace has written descriptors and
 >> +				strings to ep0, so the gadget can be
 >> +				enabled - 0 otherwise.
 >> +		=============	============================================
 >> +
 >> +		All other parameters are set through FunctionFS.
 >> diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
 >> index 29072c166d23..fcbd8bb22db4 100644
 >> --- a/Documentation/usb/gadget-testing.rst
 >> +++ b/Documentation/usb/gadget-testing.rst
 >> @@ -206,6 +206,14 @@ the standard procedure for using FunctionFS (mount it, run the userspace
 >> process which implements the function proper). The gadget should be enabled
 >> by writing a suitable string to usb_gadget/<gadget>/UDC.
 >> +The FFS function provides just one attribute in its function
 >> directory:
 >> +
 >> +	ready
 >> +
 >> +The attribute is read-only and signals if the function is ready (1) to be
 >> +used, E.G. if userspace has written descriptors and strings to ep0, so
 >> +the gadget can be enabled.
 >> +
 >> Testing the FFS function
 >> ------------------------
 >> diff --git a/drivers/usb/gadget/function/f_fs.c
 >> b/drivers/usb/gadget/function/f_fs.c
 >> index fdd0fc7b8f25..ae44dd5f3a94 100644
 >> --- a/drivers/usb/gadget/function/f_fs.c
 >> +++ b/drivers/usb/gadget/function/f_fs.c
 >> @@ -3446,6 +3446,20 @@ static inline struct f_fs_opts *to_ffs_opts(struct config_item *item)
 >> func_inst.group);
 >> }
 >> +static ssize_t f_fs_opts_ready_show(struct config_item *item,
 >> char *page)
 >> +{
 >> +	struct f_fs_opts *opts = to_ffs_opts(item);
 >> +
 >> +	return sprintf(page, "%d\n", opts->dev->desc_ready);

 > Don't we need some locking here? "desc_ready" seems to be manipulated
 > always under ffs_dev_lock().

Ups, indeed. It is just a boolean, but the instance could disappear from
under us.

I'll send an update, thanks.

-- 
Bye, Peter Korsgaard