Skip to content
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

stm32f4: USB support for STM32F446, STMF32469 #681

Closed
wants to merge 2 commits into from

Conversation

davids5
Copy link
Contributor

@davids5 davids5 commented Aug 9, 2016

This supersedes #676

This has been tested on STM32F469 and STM32F427

@davids5
Copy link
Contributor Author

davids5 commented Aug 9, 2016

@karlp, @ChuckM

First let me say thank you to @ChuckM - RCC fixed are vetted. Please merge #677 then this PR.

@kuldeepdhaka
Copy link
Contributor

kuldeepdhaka commented Aug 9, 2016

@davids5 can you please confirm if this work for you?
https://gist.github.com/kuldeepdhaka/1a2f810dd47d611fd731b46a2774bb52
just repackaged your fix in something (imo) more useful.

also, the code should work on previously working hardware.

@davids5
Copy link
Contributor Author

davids5 commented Aug 9, 2016

I do not agree with violating the reserved bit, Also the semantic of bit 21 are different. Hence the CID change

F427

1280/1731 DocID018909 Rev 11

image

F446

1144/1321 DocID026976 Rev 1

image

@kuldeepdhaka
Copy link
Contributor

yes, if i see as per F469 ds, it is violating.
but if i see as per F427, we ain't.

@kuldeepdhaka
Copy link
Contributor

also, the code good isnt because it handle the bit 21 very casually instead of handling it carefuly.
but should work for both target. in future, we should think of providing some way to let user choose.

@davids5
Copy link
Contributor Author

davids5 commented Aug 10, 2016

@karlp - Will this be merged to master?

@karlp
Copy link
Member

karlp commented Aug 10, 2016

My basic opinion is that it's not ideal, but it's probably the best short term solution until the driver layer can get refactored a bit to make more of this optional and api, rather than just hard coded.

@davids5
Copy link
Contributor Author

davids5 commented Aug 10, 2016

@karlp - Agreed - https://en.wikipedia.org/wiki/Perfect_is_the_enemy_of_good

So what's holding up you from merging it and the RCC changes @ChuckM did?

I'm not sure if you realize it but, my original code and ChuckM changes fix the library violating the PLL configurations on the exiting F4 procs. It's critical to get in. It was setting reserved bits to 0 Which is legal value for the F4.

@davids5
Copy link
Contributor Author

davids5 commented Aug 13, 2016

@karlp - Please merge this and #677

@karlp
Copy link
Member

karlp commented Aug 13, 2016

slakaðu á aðeins

@ChuckM
Copy link
Contributor

ChuckM commented Aug 13, 2016

Ekki að hjálpa Karli

@kuldeepdhaka
Copy link
Contributor

kuldeepdhaka commented Aug 15, 2016

@davids5 i have a made changes that you would love to have.
https://github.com/kuldeepdhaka/unicore-mx/tree/stm32-cid-2.0
if you can verify that the code work for you, that would be great! :D we can then merge it in unicore-mx.
also, it would be fairly easy to merge it in libopencm3. (but someone has to do it)

@davids5
Copy link
Contributor Author

davids5 commented Aug 15, 2016

@kuldeepdhaka

Hi,

How does unicore-mx relate to libopencm3? - If it is not drop-in replacement - I have no way to test it for you. If you want to fork PX4/Bootloader and rework it to build with unicore-mx. I will test it for you.

David

@davids5
Copy link
Contributor Author

davids5 commented Aug 18, 2016

@karlp - now that the RCC changes are in. Would you please merge these 8 lines of code to master.

@davids5
Copy link
Contributor Author

davids5 commented Aug 26, 2016

@karlp @esden - Would you please commit these changes.

@karlp
Copy link
Member

karlp commented Aug 26, 2016

Yep, it's on my list.

@davids5
Copy link
Contributor Author

davids5 commented Sep 6, 2016

@karlp any ETA?

@karlp
Copy link
Member

karlp commented Sep 6, 2016

I've made excellent progress on my own work this week, best in weeks, so it's looking good.

@davids5
Copy link
Contributor Author

davids5 commented Sep 8, 2016

@karlp That is great to hear! Does that mean this PR will be this week?

@davids5
Copy link
Contributor Author

davids5 commented Oct 5, 2016

@karlp - I am running out of runway - do you have an ETA on merging this?

@karlp
Copy link
Member

karlp commented Oct 5, 2016

It's still my understanding that this has a very short and well established workaround in user application code, namely,

   OTG_FS_GCCFG = (1 << 21) | (1 << 16);
    OTG_FS_DCTL = 0;

after calling usbd_init. This is why I've not been treating it as any sort of critical priority. I still think I'd rather get driver private data/driver parameters, to handle both this difference, and critical problems like working with the wrong number of endpoints, or failing to handle control requests, that sort of thing.

This affects L4 usb FS as well, I just checked it has CID = 0x2000 there too.

@davids5
Copy link
Contributor Author

davids5 commented Oct 5, 2016

If that will work. It sounds like a least resistance solution. FWIW I just finished fixing an L4 USB device driver in Nuttx. The fifio PKTSTS was not the same sequencing as the F4 or the F7. After an Out setup, the next in setup never triggered a setup completed PKTSTS or an EP setup IRQ

@karlp
Copy link
Member

karlp commented Oct 5, 2016

It's what's being used here pretty well https://github.com/ChuckM/stm32f469i/tree/master/demos/usb_cdcacm clearly the dwc_otg driver needs more refactoring for not just f469. The in/out sequencing sounds like another issue though.

@davids5
Copy link
Contributor Author

davids5 commented Oct 6, 2016

Thank you @karlp. I recently found the solution to the lost FIFO words (where the lib had that delay) - In my case it was related to disabling NAK in the SETUP received PKTSTS service and it corrupted the FIFO which would loose a word - Delaying the CNAK change to the SETUP DONE PKTSTS fixed the issue on the F7 and F4 for the NuttX USB device drivers. But It did not work on the L4. I really worry that the Synopsys USB core microcode is unstable. It seems far from stable and there are many race conditions. I have never seen a clear set of documentation on how to interact with it to avoid the races.

@karlp
Copy link
Member

karlp commented Oct 13, 2016

kuldeep doesn't like this, quoting from irc:

11:17 < kuldeep> karlp, DO NOT merge this PR https://github.com/libopencm3/libopencm3/pull/681
11:17 < kuldeep> this will break stuff in future
11:17 < kuldeep> see 
https://github.com/insane-adding-machines/unicore-mx/commit/f38ac888eaf7551aa0c7dedb61c3d97fb977b150
11:19 < kuldeep> karlp, the "==" comparision will haunt peoples.

Addressing :
kuldeep doesn't like this, quoting from irc:
11:17 < kuldeep> karlp, DO NOT merge this PR libopencm3#681
11:17 < kuldeep> this will break stuff in future
11:17 < kuldeep> see 
insane-adding-machines/unicore-mx@f38ac88
11:19 < kuldeep> karlp, the "==" comparision will haunt peoples.
@davids5
Copy link
Contributor Author

davids5 commented Oct 13, 2016

@karlp, @kuldeepdhaka

Per your request is now >=

@davids5 davids5 closed this Jan 6, 2017
@karlp
Copy link
Member

karlp commented Jan 7, 2017

was there any particular reason to close this that I missed? Do you mind if I reopen it to keep track of it?

@karlp karlp reopened this Jan 7, 2017
@karlp
Copy link
Member

karlp commented Jan 10, 2017

NVM, I see that it should always have been after the mode force

@karlp
Copy link
Member

karlp commented Jan 10, 2017

merged in cf80e2b

@karlp karlp closed this Jan 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants