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

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

[Peter Korsgaard]

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);
+}
+
+CONFIGFS_ATTR_RO(f_fs_opts_, ready);
+
+static struct configfs_attribute *ffs_attrs[] = {
+	&f_fs_opts_attr_ready,
+	NULL,
+};
+
 static void ffs_attr_release(struct config_item *item)
 {
 	struct f_fs_opts *opts = to_ffs_opts(item);
@@ -3459,6 +3473,7 @@ static struct configfs_item_operations ffs_item_ops = {
 
 static const struct config_item_type ffs_func_type = {
 	.ct_item_ops	= &ffs_item_ops,
+	.ct_attrs	= ffs_attrs,
 	.ct_owner	= THIS_MODULE,
 };
 
-- 
2.39.2

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

[Andrzej Pietrasiewicz]

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().

Regards,

Andrzej

> +}
> +
> +CONFIGFS_ATTR_RO(f_fs_opts_, ready);
> +
> +static struct configfs_attribute *ffs_attrs[] = {
> +	&f_fs_opts_attr_ready,
> +	NULL,
> +};
> +
>   static void ffs_attr_release(struct config_item *item)
>   {
>   	struct f_fs_opts *opts = to_ffs_opts(item);
> @@ -3459,6 +3473,7 @@ static struct configfs_item_operations ffs_item_ops = {
>   
>   static const struct config_item_type ffs_func_type = {
>   	.ct_item_ops	= &ffs_item_ops,
> +	.ct_attrs	= ffs_attrs,
>   	.ct_owner	= THIS_MODULE,
>   };
>   

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

[Peter Korsgaard]

>>>>> "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