-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
overwrite grstctl on edpt_disable #1454
Conversation
From #1453
This looks reasonable/fine to me in light of that (unless it's somehow possible to get to this code before all the bits self-clear). If you look at my comments on PRs/Issues over the years, you'll find that I had a number of bugs where I do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for late response, and thank for your PR. This somehow falls off my radar. The code is spot-on, though I think we can actually set the fifo number, and flush it at the same time. Let me know if you have time to test that out, otherwise, I will merge this as it is and test the combined code later on.
@@ -796,7 +796,7 @@ static void dcd_edpt_disable (uint8_t rhport, uint8_t ep_addr, bool stall) | |||
} | |||
|
|||
// Flush the FIFO, and wait until we have confirmed it cleared. | |||
dwc2->grstctl |= (epnum << GRSTCTL_TXFNUM_Pos); | |||
dwc2->grstctl = (epnum << GRSTCTL_TXFNUM_Pos); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can actually merge them into 1 line as
dwc2->grstctl = (epnum << GRSTCTL_TXFNUM_Pos) | GRSTCTL_TXFFLSH;
from linux dwc2
Hi, @hathach,
unfortunately I currently don't have the test-equipment on desk any more currently, so no further possibility to test.
I'll do further tests with this software in the 2nd half of the year and also might test that change then, but I'd propose to first merge the current solution.
Regards Pascal
Am 26.05.22 um 16:34 schrieb Ha Thach:
…
***@***.**** approved this pull request.
Sorry for late response, and thank for your PR. This somehow falls off my radar. The code is spot-on, though I think we can actually set the fifo number, and flush it at the same time. Let me know
if you have time to test that out, otherwise, I will merge this as it is and test the combined code later on.
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
In src/portable/synopsys/dwc2/dcd_dwc2.c <#1454 (comment)>:
> @@ -796,7 +796,7 @@ static void dcd_edpt_disable (uint8_t rhport, uint8_t ep_addr, bool stall)
}
// Flush the FIFO, and wait until we have confirmed it cleared.
- dwc2->grstctl |= (epnum << GRSTCTL_TXFNUM_Pos);
+ dwc2->grstctl = (epnum << GRSTCTL_TXFNUM_Pos);
I think we can actually merge them into 1 line as
dwc2->grstctl = (epnum << GRSTCTL_TXFNUM_Pos) | GRSTCTL_TXFFLSH;
—
Reply to this email directly, view it on GitHub <#1454 (review)>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AF6RRCUMZ5HQTWMUT5R4IM3VL6DWVANCNFSM5U6EYHUQ>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
[ { ***@***.***": "http://schema.org", ***@***.***": "EmailMessage", "potentialAction": { ***@***.***": "ViewAction", "target": "#1454 (review)","url":
"#1454 (review)", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { ***@***.***": "Organization",
"name": "GitHub", "url": "https://github.com" } } ]
--
Mit freundlichen Grüßen
Pascal Speck
______________________________________________
IKTEK - Informations und Kommunikationstechnik
Oranienstrasse 49
35716 Dietzhölztal
Tel: 02774/9236918
Fax: 02774/9236919
Mail: ***@***.***
|
No problems, I think it is safe to do so when observing code from linux driver. I will make the change to pr and merge asap. |
Solves #1453