Add sdioblockdevice #8634
Add sdioblockdevice #8634
Conversation
intermediate layer between HAL and C++ API. Can be used for other targets to implement SDIO functions
removed also debug outputs.
Init uses a local SD_WideBus_Enable and not the HAL version. The HAL version has the problem with FindSCR(), this fails due to slow HAL_GetTick / ticker_read_us implementation (using many locks and checks for thread safety). Cardinfo struckture was defined in sdio_device to be independant from HAL functions in SDioblockdevice.cpp user API.
use private SD_Cardinfo_t struct for cardinfo.
after a read opteration, just check for DMA ready. Speeds up the operation a lot. Check if SD card ready before reading/writing. Leave check if SD ready aftere writing, DMA ready seems not to be sufficient for checking ready state. Removed unused member vars. Lock iis called on function entry to make multithreaded apps safer.
was wrong inside other loop
Please review and run astyle (using astylerc file) for non-target files. |
|
@bulislaw @donatieng I am not sure this work fits the current HAL/Driver structure that we have. Can you comment? |
|
This driver is different from the SD with SPI, for SPI there is an abstraction that is common for many targets. The SDIO is more special, I think the hardware implementation is very different in other targets and it will hard to find a common hardware interface only. The STM HAL offers a higher level interface that deals already with the necessary sequences of commands. So in the SDIOBlockdevice is less code and some more work is done in the underlying layer. |
|
@JojoS62 Thanks for the proposal, adding implementation with more tests 👍 Good point mentioned above about how to support various targets. This should not be in the block device, but rather including some common layer (HAL header would be a good candidate). Otherwise, the block device implementation contains target specific functions/definitions as in this case ( There's https://www.sdcard.org/developers/overview/sdio/index.html (defines simplified specifications) that could help. I only checked few targets if they have SDIO (nordic has simple read/write bytes, other API looks more transfer oriented - read/write blocks). Isn't this similar to QSPI where we have HAL defined? |
|
@0xc0170 Thanks for reviewing. |
remvove target specific path from include, add full path for feature blockdevice.h
There was a problem hiding this comment.
Thanks for contributing the code. Unfortunately the model you adopted doesn't fit our architecture. As some people already mentioned we have a HAL layer, that hides all the HW differences. I agree that sometimes it's challenging to come up with a generic API, but from our experience it make everyones life easier in the long run.
|
ok, no problem, I understand. I've learned already a lot by debugging and testing the component and reading about SDIO. The mBed HAL should only supply read/write for the command and data channel. Maybe a driver level can handle the general command - response sequence. Then the blockdriver has to implement the command sequences for the various operations. It is also possible that there are more common SD-SPI and SDIO code fragments, but I don't have this overview at the moment. |
|
@JojoS62 - Thanks for contribution and awesome work here. @bulislaw @0xc0170 - Shall we add new repo for SDIO driver just like sd-driver (https://github.com/ARMmbed/sd-driver) for now and later when HAL API's and common layer is designed we can integrate it to mbed-os repo? @JojoS62 - Will like to know your thoughts on having an independent repository for sdio driver? Also another option would be to have SDIO as abstract class and each target has its own implementation. Reference for this will be EMAC (https://github.com/ARMmbed/mbed-os/tree/master/features/netsocket/emac-drivers) |
that will be ok for me also. Some people asked already for SDIO support and this solution may satisfy for now, although not in the main repo. |
|
Oh this very cool! Thanks for creating a PR. We'll see if we can't find a home for this guy. In the meantime you can create your own repo similar to any of the old block device repos. This is actually how all of the other block devices in the components folder were started before they were completely supported: |
|
@JojoS62 We created repository for this driver: https://github.com/ARMmbed/sdio-driver, can you redirect this pull request there please? I am closing this PR, will move to the new one. Good example to follow |
Took the work done as part of PR ARMmbed/mbed-os#8634 Signed-off-by: Mahesh Mahadevan <[email protected]>
Took the work done as part of PR ARMmbed/mbed-os#8634 Signed-off-by: Mahesh Mahadevan <[email protected]>
Description
Adding a blockdevice for SD cards, using the 4 bit wide SDIO MCU interface. This interface is faster than the 1-bit SPI implementation. Some boards like the DISCO_F469NI have a SD slot onboard and it is connected to the SDIO pins. These cannot be configured as SPI device, so the SDIO implementation is neccessary to use this hardware.
The SDIO interface is present on the STM32F4 and other MCU series. This blockdevice was tested with the DISCO_F469NI board. It works also with a custom target with STM32F407 MCU.
This implementation uses the STM32 HAL, but the user API in the blockdevice class is kept free of HAL functions/structures. Adapting to other MCUs should be done by replacing the sdio_driver.c.
Tests for the use with FATFilesystem have been added. These tests differ from the SPI implementation only in the blockdevice definition. This is already configurable, so the tests maybe moved to a common level for SD/SDIOBlockdevice implementation. There are also common error defines that could be used in both blockevices.
This SDIO implementation uses DMA because there were problems with the polling version. The data fifo must be read/written very fast and the HAL implementation with timeout checking using the ticker_read_us() is too slow.
Pull request type