Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 26 Oct 2021 10:41:40 -0400
From:   Alan Stern <stern@rowland.harvard.edu>
To:     Wesley Cheng <quic_wcheng@quicinc.com>
Cc:     balbi@kernel.org, gregkh@linuxfoundation.org,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
        Wesley Cheng <wcheng@codeaurora.org>
Subject: Re: [PATCH] usb: gadget: f_mass_storage: Disable eps during
 disconnect
Message-ID: <20211026144140.GA1288435@rowland.harvard.edu>
References: <20211026004456.23054-1-quic_wcheng@quicinc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211026004456.23054-1-quic_wcheng@quicinc.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On Mon, Oct 25, 2021 at 05:44:56PM -0700, Wesley Cheng wrote:
> From: Wesley Cheng <wcheng@codeaurora.org>
> 
> When receiving a disconnect event from the UDC, the mass storage
> function driver currently runs the handle_exception() routine
> asynchronously.  For UDCs that support runtime PM, there is a
> possibility the UDC is already suspended by the time the
> do_set_interface() is executed.  This can lead to HW register access
> while the UDC is already suspended.
> 
> Signed-off-by: Wesley Cheng <wcheng@codeaurora.org>
> ---
>  drivers/usb/gadget/function/f_mass_storage.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
> index 3cabf7692ee1..752439690fda 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.c
> +++ b/drivers/usb/gadget/function/f_mass_storage.c
> @@ -2342,6 +2342,16 @@ static void fsg_disable(struct usb_function *f)
>  {
>  	struct fsg_dev *fsg = fsg_from_func(f);
>  
> +	/* Disable the endpoints */
> +	if (fsg->bulk_in_enabled) {
> +		usb_ep_disable(fsg->bulk_in);

According to the kerneldoc, this routine must be called in process 
context.

> +		fsg->bulk_in_enabled = 0;
> +	}
> +	if (fsg->bulk_out_enabled) {
> +		usb_ep_disable(fsg->bulk_out);
> +		fsg->bulk_out_enabled = 0;
> +	}
> +
>  	__raise_exception(fsg->common, FSG_STATE_CONFIG_CHANGE, NULL);
>  }

Looks like you'll have to find a different way to avoid the problem.  
For example, if an exception is pending then you might prevent the 
gadget from going into runtime suspend until the exception has been 
handled.

Alan Stern