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

refactor: SD card SPI HAL #3957

Merged
merged 14 commits into from
Sep 23, 2023
Merged

refactor: SD card SPI HAL #3957

merged 14 commits into from
Sep 23, 2023

Conversation

raphaelcoeffic
Copy link
Member

@raphaelcoeffic raphaelcoeffic commented Aug 20, 2023

Summary of changes:

  • Moved SPI SD card to HAL/LL (ported from RIOT-OS)
  • New generic diskio API (similar to ST's middleware)

@richardclli
Copy link
Collaborator

@raphaelcoeffic I think you may need to consider this PR as well. Change in SD card specific functions may breaks Virtual FS.

#3962

@raphaelcoeffic
Copy link
Member Author

I think you may need to consider #3962. Change in SD card specific functions may breaks Virtual FS.

Probably, yes. I will need to rebase / port on top of this PR as soon as I have tested and fixed it. The plan we had with @gagarinlg was to adopt a more generic FatFs driver as the first layer similar to this: https://github.com/STMicroelectronics/stm32_mw_fatfs/blob/master/src/ff_gen_drv.h

@raphaelcoeffic raphaelcoeffic force-pushed the sdcard-spi-hal branch 4 times, most recently from 51620bf to a27b1be Compare August 30, 2023 09:04
@3djc
Copy link
Collaborator

3djc commented Aug 30, 2023

Did run some test in T20 (because of the SD chip instead of card)

  • no issue found at all
  • write speed is about 10/15% faster than main
  • read speed is much faster than main
  • tested both while in bootloader or while running firmware, with similar results

@3djc
Copy link
Collaborator

3djc commented Aug 30, 2023

Also tested on X7 (for the F2 processor, SD is a no name 32Gb class 10 card)

no issue found at all
write speed is about 10/15% faster than main
read speed is faster than main (but not as good as T20)
tested both while in bootloader or while running firmware, with similar results

@3djc
Copy link
Collaborator

3djc commented Aug 30, 2023

Tested now on X7 with 256MB card supplied with another radio:

  • full of errors
  • impossible to format card from PC when in BL mode (not tried in firmware)

SAME card directly connected to PC with a SDCard adapter:

  • no errors
  • formatting is fine

Errors are mysteriously gone in radio after the sd card reformat, something strange is at play since something similar happened to Raph

@raphaelcoeffic raphaelcoeffic marked this pull request as ready for review September 1, 2023 06:01
@3djc
Copy link
Collaborator

3djc commented Sep 2, 2023

Test on X7 (F2) : all ok, despite using the dreadful no name sd (tested mass storage and BL mode)
Test on TX12MKII (F4) : all ok, despite using the dreadful no name sd (tested mass storage and BL mode)
Test on T20 (F4, internal SD chip) : all ok (and faster !)
Test on TX16S (should not be affected, but just checking anyway) : FAIL. Card cannot read on this branch (both main and 2.9.0 work fine on exact same hardware setup)

@3djc
Copy link
Collaborator

3djc commented Sep 3, 2023

Ok now :)

@pfeerick pfeerick self-requested a review September 23, 2023 08:37
@pfeerick pfeerick added this to the 2.10 milestone Sep 23, 2023
@pfeerick
Copy link
Member

X9D+2019: Flashed FW and BL, twice. Navigating SD, renaming, deleting fine. Models and radio settings intact. Able to copy files and make changes via USB when radio in BL or running main firmware. Able to update ISRM via SD. Able to run standalone lua from SD.

BetaFPV LR3: Same as above (except for ISRM update obviously).

EL18: DFU flashed firmware as one trim hat borked with repeated bootloader flashes 🤭, otherwise everything seems to be working as above.

@pfeerick pfeerick merged commit 87df8a4 into main Sep 23, 2023
37 checks passed
@pfeerick pfeerick deleted the sdcard-spi-hal branch September 23, 2023 09:32
@philmoz
Copy link
Collaborator

philmoz commented Sep 24, 2023

This PR breaks audio playback on my EL18 - no sounds can be played from the SOUNDS folder.

No startup sound played, no sounds played by scripts or special functions and sound does not play if manually selected from the file browser.

TX16 is not affected, only broken on EL18.

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.

5 participants