From: Marek Szyprowski <m.szyprowski@samsung.com>
To: Minas Harutyunyan <minas.harutyunyan@synopsys.com>,
Dan Carpenter <dan.carpenter@oracle.com>,
Maynard CABIENTE <maynard.cabiente@raritan.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Felipe Balbi <felipe.balbi@linux.intel.com>,
Geert Uytterhoeven <geert@linux-m68k.org>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Subject: Re: [PATCH] usb: dwc2: Revert "usb: dwc2: Disable all EP's on disconnect"
Date: Thu, 6 Dec 2018 16:03:53 +0100 [thread overview]
Message-ID: <2b48a7b5-f806-d761-97f6-885ae6c69a7e@samsung.com> (raw)
In-Reply-To: <410670D7E743164D87FA6160E7907A56013A7B19C2@am04wembxa.internal.synopsys.com>
Dear Minas,
On 2018-12-04 13:34, Minas Harutyunyan wrote:
> On 11/23/2018 6:43 PM, Dan Carpenter wrote:
>> Ugh... We also had a long thread about the v2 patch but it turns out
>> the list was not CC'd. I should have checked for that.
>>
>> You have to pass a flag to say if the caller holds the lock or not...
>>
>> regards,
>> dan carpenter
>>
> Hi Dan, Marek, Maynard,
>
> Could you please apply bellow patch over follow patches:
>
> dccf1bad4be7 usb: dwc2: Disable all EP's on disconnect
> 6f774b501928 usb: dwc2: Fix ep disable spinlock flow.
>
> Please review and test. Feedback is appreciated :-)
Okay, I finally managed to find some time to check this. Your diff is
mangled, so I had to manually apply it. Frankly, it is very similar to
the revert I proposed. I've checked it on my test machines and it fixes
the issues. I'm not very happy about the unlock/lock design, but it
should be safe in this case and doesn't make the code even more complex.
Feel free to add a following tag to the final patch:
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 8eb25e3597b0..db438d13c36a 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -3165,8 +3165,6 @@ static void kill_all_requests(struct dwc2_hsotg
> *hsotg,
> dwc2_hsotg_txfifo_flush(hsotg, ep->fifo_index);
> }
>
> -static int dwc2_hsotg_ep_disable(struct usb_ep *ep);
> -
> /**
> * dwc2_hsotg_disconnect - disconnect service
> * @hsotg: The device state.
> @@ -3185,12 +3183,13 @@ void dwc2_hsotg_disconnect(struct dwc2_hsotg *hsotg)
> hsotg->connected = 0;
> hsotg->test_mode = 0;
>
> - /* all endpoints should be shutdown */
> for (ep = 0; ep < hsotg->num_of_eps; ep++) {
> if (hsotg->eps_in[ep])
> - dwc2_hsotg_ep_disable(&hsotg->eps_in[ep]->ep);
> + kill_all_requests(hsotg, hsotg->eps_in[ep],
> + -ESHUTDOWN);
> if (hsotg->eps_out[ep])
> - dwc2_hsotg_ep_disable(&hsotg->eps_out[ep]->ep);
> + kill_all_requests(hsotg, hsotg->eps_out[ep],
> + -ESHUTDOWN);
> }
>
> call_gadget(hsotg, disconnect);
> @@ -3234,6 +3233,8 @@ static void dwc2_hsotg_irq_fifoempty(struct
> dwc2_hsotg *hsotg, bool periodic)
> GINTSTS_PTXFEMP | \
> GINTSTS_RXFLVL)
>
> +static int dwc2_hsotg_ep_disable(struct usb_ep *ep);
> +
> /**
> * dwc2_hsotg_core_init - issue softreset to the core
> * @hsotg: The device state
> @@ -3258,12 +3259,14 @@ void dwc2_hsotg_core_init_disconnected(struct
> dwc2_hsotg *hsotg,
> return;
> } else {
> /* all endpoints should be shutdown */
> + spin_unlock(&hsotg->lock);
> for (ep = 1; ep < hsotg->num_of_eps; ep++) {
> if (hsotg->eps_in[ep])
>
> dwc2_hsotg_ep_disable(&hsotg->eps_in[ep]->ep);
> if (hsotg->eps_out[ep])
>
> dwc2_hsotg_ep_disable(&hsotg->eps_out[ep]->ep);
> }
> + spin_lock(&hsotg->lock);
> }
>
> /*
> @@ -4072,7 +4075,6 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep)
> unsigned long flags;
> u32 epctrl_reg;
> u32 ctrl;
> - int locked;
>
> dev_dbg(hsotg->dev, "%s(ep %p)\n", __func__, ep);
>
> @@ -4088,7 +4090,7 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep)
>
> epctrl_reg = dir_in ? DIEPCTL(index) : DOEPCTL(index);
>
> - locked = spin_trylock_irqsave(&hsotg->lock, flags);
> + spin_lock_irqsave(&hsotg->lock, flags);
>
> ctrl = dwc2_readl(hsotg, epctrl_reg);
>
> @@ -4112,8 +4114,7 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep)
> hs_ep->fifo_index = 0;
> hs_ep->fifo_size = 0;
>
> - if (locked)
> - spin_unlock_irqrestore(&hsotg->lock, flags);
> + spin_unlock_irqrestore(&hsotg->lock, flags);
>
> return 0;
> }
>
> Thanks,
> Minas
>
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
next prev parent reply other threads:[~2018-12-06 15:04 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20181121154518eucas1p2cc7ffd4d071ff420c747e1d0c387ee5d@eucas1p2.samsung.com>
2018-11-21 15:45 ` [PATCH] usb: dwc2: Revert "usb: dwc2: Disable all EP's on disconnect" Marek Szyprowski
2018-11-22 6:53 ` Minas Harutyunyan
2018-11-23 9:49 ` Marek Szyprowski
2018-11-23 14:43 ` Dan Carpenter
2018-12-04 12:34 ` Minas Harutyunyan
2018-12-04 13:29 ` Dan Carpenter
2018-12-05 12:52 ` Minas Harutyunyan
2018-12-07 10:15 ` Dan Carpenter
2018-12-07 11:20 ` Minas Harutyunyan
2018-12-07 14:13 ` Minas Harutyunyan
2018-12-07 14:40 ` Dan Carpenter
2018-12-06 15:03 ` Marek Szyprowski [this message]
2018-12-07 9:06 ` Minas Harutyunyan
2018-12-07 9:50 ` Felipe Balbi
2018-12-07 9:58 ` Minas Harutyunyan
2018-12-07 10:11 ` Felipe Balbi
2018-12-06 23:09 ` Maynard CABIENTE
2018-12-07 9:06 ` Minas Harutyunyan
2018-12-06 14:52 ` Dan Carpenter
2018-12-06 15:07 ` Marek Szyprowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2b48a7b5-f806-d761-97f6-885ae6c69a7e@samsung.com \
--to=m.szyprowski@samsung.com \
--cc=b.zolnierkie@samsung.com \
--cc=dan.carpenter@oracle.com \
--cc=felipe.balbi@linux.intel.com \
--cc=geert@linux-m68k.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=maynard.cabiente@raritan.com \
--cc=minas.harutyunyan@synopsys.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).