All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] usb: gadget: storage: Remove warning message
@ 2019-05-08  3:24 EJ Hsu
  2019-05-08  6:39 ` Greg KH
  2019-05-08  6:40 ` Greg KH
  0 siblings, 2 replies; 4+ messages in thread
From: EJ Hsu @ 2019-05-08  3:24 UTC (permalink / raw)
  To: balbi; +Cc: linux-usb, ejh

This change is to fix below warning message in following scenario:
usb_composite_setup_continue: Unexpected call

When system tried to enter suspend, the fsg_disable() will be called to
disable fsg driver and send a signal to fsg_main_thread. However, at
this point, the fsg_main_thread has already been frozen and can not
respond to this signal. So, this signal will be pended until
fsg_main_thread wakes up.

Once system resumes from suspend, fsg_main_thread will detect a signal
pended and do some corresponding action (in handle_exception()). Then,
host will send some setup requests (get descriptor, set configuration...)
to UDC driver trying to enumerate this device. During the handling of "set
configuration" request, it will try to sync up with fsg_main_thread by
sending a signal (which is the same as the signal sent by fsg_disable)
to it. In a similar manner, once the fsg_main_thread receives this
signal, it will call handle_exception() to handle the request.

However, if the fsg_main_thread wakes up from suspend a little late and
"set configuration" request from Host arrives a little earlier,
fsg_main_thread might come across the request from "set configuration"
when it handles the signal from fsg_disable(). In this case, it will
handle this request as well. So, when fsg_main_thread tries to handle
the signal sent from "set configuration" later, there will nothing left
to do and warning message "Unexpected call" is printed.

Signed-off-by: EJ Hsu <ejh@nvidia.com>
---
 drivers/usb/gadget/function/f_mass_storage.c | 18 +++++++++++++++---
 drivers/usb/gadget/function/storage_common.h |  4 ++++
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
index 043f97a..813083f 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -4,6 +4,7 @@
  *
  * Copyright (C) 2003-2008 Alan Stern
  * Copyright (C) 2009 Samsung Electronics
+ * Copyright (c) 2019, NVIDIA CORPORATION.  All rights reserved.
  *                    Author: Michal Nazarewicz <mina86@mina86.com>
  * All rights reserved.
  *
@@ -2294,7 +2295,7 @@ static void fsg_disable(struct usb_function *f)
 {
 	struct fsg_dev *fsg = fsg_from_func(f);
 	fsg->common->new_fsg = NULL;
-	raise_exception(fsg->common, FSG_STATE_CONFIG_CHANGE);
+	raise_exception(fsg->common, FSG_STATE_DISCONNECT);
 }
 
 
@@ -2307,6 +2308,7 @@ static void handle_exception(struct fsg_common *common)
 	enum fsg_state		old_state;
 	struct fsg_lun		*curlun;
 	unsigned int		exception_req_tag;
+	struct fsg_dev		*fsg;
 
 	/*
 	 * Clear the existing signals.  Anything but SIGUSR1 is converted
@@ -2413,9 +2415,19 @@ static void handle_exception(struct fsg_common *common)
 		break;
 
 	case FSG_STATE_CONFIG_CHANGE:
-		do_set_interface(common, common->new_fsg);
-		if (common->new_fsg)
+		fsg = common->new_fsg;
+		/*
+		 * Add a check here to double confirm if a disconnect event
+		 * occurs and common->new_fsg has been cleared.
+		 */
+		if (fsg) {
+			do_set_interface(common, fsg);
 			usb_composite_setup_continue(common->cdev);
+		}
+		break;
+
+	case FSG_STATE_DISCONNECT:
+		do_set_interface(common, NULL);
 		break;
 
 	case FSG_STATE_EXIT:
diff --git a/drivers/usb/gadget/function/storage_common.h b/drivers/usb/gadget/function/storage_common.h
index e5e3a25..405bf34 100644
--- a/drivers/usb/gadget/function/storage_common.h
+++ b/drivers/usb/gadget/function/storage_common.h
@@ -1,4 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2019, NVIDIA CORPORATION.  All rights reserved.
+ */
 #ifndef USB_STORAGE_COMMON_H
 #define USB_STORAGE_COMMON_H
 
@@ -161,6 +164,7 @@ enum fsg_state {
 	FSG_STATE_ABORT_BULK_OUT,
 	FSG_STATE_PROTOCOL_RESET,
 	FSG_STATE_CONFIG_CHANGE,
+	FSG_STATE_DISCONNECT,
 	FSG_STATE_EXIT,
 	FSG_STATE_TERMINATED
 };
-- 
2.1.4


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

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

* Re: [PATCH] usb: gadget: storage: Remove warning message
  2019-05-08  3:24 [PATCH] usb: gadget: storage: Remove warning message EJ Hsu
@ 2019-05-08  6:39 ` Greg KH
  2019-05-08  6:40 ` Greg KH
  1 sibling, 0 replies; 4+ messages in thread
From: Greg KH @ 2019-05-08  6:39 UTC (permalink / raw)
  To: EJ Hsu; +Cc: balbi, linux-usb

On Wed, May 08, 2019 at 11:24:00AM +0800, EJ Hsu wrote:
> This change is to fix below warning message in following scenario:
> usb_composite_setup_continue: Unexpected call
> 
> When system tried to enter suspend, the fsg_disable() will be called to
> disable fsg driver and send a signal to fsg_main_thread. However, at
> this point, the fsg_main_thread has already been frozen and can not
> respond to this signal. So, this signal will be pended until
> fsg_main_thread wakes up.
> 
> Once system resumes from suspend, fsg_main_thread will detect a signal
> pended and do some corresponding action (in handle_exception()). Then,
> host will send some setup requests (get descriptor, set configuration...)
> to UDC driver trying to enumerate this device. During the handling of "set
> configuration" request, it will try to sync up with fsg_main_thread by
> sending a signal (which is the same as the signal sent by fsg_disable)
> to it. In a similar manner, once the fsg_main_thread receives this
> signal, it will call handle_exception() to handle the request.
> 
> However, if the fsg_main_thread wakes up from suspend a little late and
> "set configuration" request from Host arrives a little earlier,
> fsg_main_thread might come across the request from "set configuration"
> when it handles the signal from fsg_disable(). In this case, it will
> handle this request as well. So, when fsg_main_thread tries to handle
> the signal sent from "set configuration" later, there will nothing left
> to do and warning message "Unexpected call" is printed.
> 
> Signed-off-by: EJ Hsu <ejh@nvidia.com>
> ---
>  drivers/usb/gadget/function/f_mass_storage.c | 18 +++++++++++++++---
>  drivers/usb/gadget/function/storage_common.h |  4 ++++
>  2 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
> index 043f97a..813083f 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.c
> +++ b/drivers/usb/gadget/function/f_mass_storage.c
> @@ -4,6 +4,7 @@
>   *
>   * Copyright (C) 2003-2008 Alan Stern
>   * Copyright (C) 2009 Samsung Electronics
> + * Copyright (c) 2019, NVIDIA CORPORATION.  All rights reserved.
>   *                    Author: Michal Nazarewicz <mina86@mina86.com>

If this is the only nvidia contribution to this file, then this new line
is not warrented, sorry.  Please contact your corporate lawyers to
understand why, I'm sure they will be glad to have you learn all you
ever wanted to know about copyright law :)

thanks,

greg k-h

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

* Re: [PATCH] usb: gadget: storage: Remove warning message
  2019-05-08  3:24 [PATCH] usb: gadget: storage: Remove warning message EJ Hsu
  2019-05-08  6:39 ` Greg KH
@ 2019-05-08  6:40 ` Greg KH
  2019-05-08  7:54   ` EJ Hsu
  1 sibling, 1 reply; 4+ messages in thread
From: Greg KH @ 2019-05-08  6:40 UTC (permalink / raw)
  To: EJ Hsu; +Cc: balbi, linux-usb

On Wed, May 08, 2019 at 11:24:00AM +0800, EJ Hsu wrote:
> --- a/drivers/usb/gadget/function/storage_common.h
> +++ b/drivers/usb/gadget/function/storage_common.h
> @@ -1,4 +1,7 @@
>  /* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2019, NVIDIA CORPORATION.  All rights reserved.
> + */

Same here, why are you claiming copyright for this whole file?

That is just flat out not acceptable at all, again, go talk to your
lawyers.  If you insist on this, I will insist on having one of your
lawyers on the signed-off-by: line as well so that they know what they
are agreeing with.

not ok.

greg k-h

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

* RE: [PATCH] usb: gadget: storage: Remove warning message
  2019-05-08  6:40 ` Greg KH
@ 2019-05-08  7:54   ` EJ Hsu
  0 siblings, 0 replies; 4+ messages in thread
From: EJ Hsu @ 2019-05-08  7:54 UTC (permalink / raw)
  To: Greg KH; +Cc: balbi, linux-usb

Thanks for the info. Will remove it in the V2.

Regards,
EJ

-----Original Message-----
From: Greg KH <gregkh@linuxfoundation.org> 
Sent: Wednesday, May 8, 2019 2:40 PM
To: EJ Hsu <ejh@nvidia.com>
Cc: balbi@kernel.org; linux-usb@vger.kernel.org
Subject: Re: [PATCH] usb: gadget: storage: Remove warning message

On Wed, May 08, 2019 at 11:24:00AM +0800, EJ Hsu wrote:
> --- a/drivers/usb/gadget/function/storage_common.h
> +++ b/drivers/usb/gadget/function/storage_common.h
> @@ -1,4 +1,7 @@
>  /* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2019, NVIDIA CORPORATION.  All rights reserved.
> + */

Same here, why are you claiming copyright for this whole file?

That is just flat out not acceptable at all, again, go talk to your lawyers.  If you insist on this, I will insist on having one of your lawyers on the signed-off-by: line as well so that they know what they are agreeing with.

not ok.

greg k-h
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

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

end of thread, other threads:[~2019-05-08  7:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-08  3:24 [PATCH] usb: gadget: storage: Remove warning message EJ Hsu
2019-05-08  6:39 ` Greg KH
2019-05-08  6:40 ` Greg KH
2019-05-08  7:54   ` EJ Hsu

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.