initial PR for SDIOBlockdevice implementation #1

JojoS62 · 2018-11-09T22:06:30Z

This is the component that was not merged into mbed-os.

add SDIOBlockdevice implementation for STM32F4

added some description

almost the same as for STM32F4, but in F7 SDIO device is renamed to SDMMC SDMMC1 is used for DISCO_F746NG updated readme.md for new target

0xc0170 · 2018-11-12T17:31:10Z

@ARMmbed/mbed-os-storage Can you review?

dannybenor · 2018-11-12T17:50:04Z

@0xc0170 will do after 5.11 release

screamerbg · 2018-11-22T10:39:22Z

@dannybenor We need a review sooner. This is blocking new Pelion DM boards by ST. Could you please help?

dannybenor · 2018-11-22T11:23:41Z

blockdevice should be a component and not a feature
This needs review from HAL team

offirko · 2018-11-22T11:19:11Z

SDIOBlockDevice.cpp

+#define SD_DBG 0       /*!< 1 - Enable debugging */
+#define SD_CMD_TRACE 0 /*!< 1 - Enable SD command tracing */
+
+#define SD_BLOCK_DEVICE_ERROR_WOULD_BLOCK -5001           /*!< operation would block */


shouldn't those be in H file?

the debug settings are used private in the cpp source, they are not relevant for other modules. I have taken this approach from the SDBlockdevice.

Those return errors should be exposed by the h file (possibly for SDBlockDevice as well)
An even better suggestion is to use the errors at mbed_error.h

https://github.com/ARMmbed/mbed-os/blob/master/platform/mbed_error.h

UNKNOWN 256 Unknown error INVALID_ARGUMENT 257 Invalid Argument INVALID_DATA 258 Invalid data INVALID_FORMAT 259 Invalid format INVALID_INDEX 260 Invalid Index ...

SDIOBlockDevice.cpp

offirko · 2018-11-22T13:08:06Z

SDIOBlockDevice.h

+     *  @param buffer   Buffer to write blocks to
+     *  @param addr     Address of block to begin reading from
+     *  @param size     Size to read in bytes, must be a multiple of read block size
+     *  @return         0 on success, negative error code on failure


Please try to elaborate possible error values for each API

SDIOBlockDevice.h

offirko · 2018-11-22T13:24:40Z

@JojoS62 - thanks for submitting this important BlockDevice implementation. I've left several questions and remarks - thanks!

JojoS62 · 2018-11-22T14:17:31Z

I have prepared some modifications as requested, but I will run the tests again first. Will push the commits tomorrow.

- removed comments - renamed short variable names

screamerbg · 2018-12-13T10:41:42Z

@JojoS62 @offirko Any update on this? We have 7 ST boards using SDIO, but we're forced to use external SD (SPI) shield.

dannybenor · 2018-12-13T11:18:11Z

@screamerbg this requires HAL to go into mbedOS master. Please create a preq and ad HAL team

deepikabhavnani · 2018-12-13T15:56:29Z

@dannybenor - This PR was moved to external repo for the same reason. HAL changes might take time as they are not planned, but that should not stop users to use SDIO interface.

HAL changes will be needed to integrate with Mbed OS, but in parallel this driver can be polished and used as external. Also other partners will have reference to add to SDIO changes which will be helpful in finalizing HAL API's.

dannybenor · 2018-12-16T10:02:36Z

@deepikabhavnani Yes, you are right. I missed this point. LGTM

offirko · 2018-12-16T10:10:56Z

SDIOBlockDevice.cpp

+        while (SD_DMA_ReadPending() != SD_TRANSFER_OK)
+        {
+            uint32_t tickstart = HAL_GetTick();
+            if ((HAL_GetTick() - tickstart) >= MBED_CONF_SD_TIMEOUT)


This requires a fix in order to measure Timeout correctly.

offirko · 2018-12-16T10:12:28Z

@JojoS62 - thanks for making adjustments!
I believe the Timeout issue still requires a fix.

tickstart was inside checking loop removed wait_ms() calls added timeout checks for all while loops

jeromecoutant · 2019-01-15T15:49:11Z

Hi

Any update ?

Thx

screamerbg · 2019-02-07T10:43:10Z

@dannybenor @offirko Any update on this?

JojoS62 · 2019-02-07T10:58:12Z

sorry for the delay, at the moment I'm very busy with my work.
The component is working, I've applied the most fixes. Only the error codes could be improved, but the mbed errror handling has become also complex and at the moment I have no time to update this. I have used similar codes like in SDBlockDevice, so this component is usable already.
Please review and check if there are major issues that prevent using this PR.

offirko

LGTM

deepikabhavnani

@JojoS62 - Thanks for adding this

deepikabhavnani · 2019-02-11T14:09:39Z

@ARMmbed/mbed-os-maintainers - Please review and merge

0xc0170 · 2019-02-11T14:16:11Z

@ARMmbed/mbed-os-maintainers - Please review and merge

we can review but merging is up to @ARMmbed/mbed-os-storage team I believe

0xc0170 · 2019-02-11T14:35:57Z

util/fsfat_test.h

+/** @file fsfat_test.h
+ *
+ * mbed Microcontroller Library
+ * Copyright (c) 2006-2016 ARM Limited


this should be 2019 (new file or at least 2018-2019)

New files should contain SPDX identifier (look at platform header files).

0xc0170

Small license fix, rest looks fine

deepikabhavnani · 2019-02-11T14:44:28Z

we can review but merging is up to @ARMmbed/mbed-os-storage team I believe

Sounds good, not sure why I thought only maintainers have merge permissions on this repo

0xc0170 · 2019-02-19T12:28:04Z

@ARMmbed/mbed-os-storage Can we progress? there's new PR that depends also on this one

jeromecoutant · 2019-03-06T16:44:54Z

Why this PR is not progressing ?

adbridge · 2019-03-06T17:16:40Z

Why this PR is not progressing ?

@ARMmbed/mbed-os-storage @dannybenor Could you please answer @jeromecoutant ?

maclobdell · 2019-03-15T20:58:51Z

Just curious - would this better to now have have a generic SDIOBlockDevice in the mbed-os/components/storage/blockdevice/COMPONENT_SDIO and then specific implementations under platform-specific target code at mbed-os/targets/TARGET_/.../TARGET_/

Or alternatively, if there isn't a HAL abstraction to be used by a generic block device driver, perhaps fully target-specific SDIO blockdevice implementations could live in the target code.

Can we move forward with one of these approaches, or perhaps another approach?

- ARMmbed#1 is based on [JojoS62/sdio-driver/master](JojoS62@c03489c). - ARMmbed#2 is based on [NXPmicro/sdio-driver/Add_SDIO_LPC55S69](nxp-archive@370c51a)

jeromecoutant · 2020-08-13T07:40:22Z

@MarceloSalazar @adbridge don't forget this feature :-)

adbridge · 2020-08-13T10:29:09Z

@jeromecoutant we don't deal with this repository and it doesn't form part of our mbed-os releases. @MarceloSalazar @0xc0170 do you know anything about how this repo is used/controlled ?

JojoS62 · 2020-08-13T11:58:27Z

It seems I missed some notifications about updates here. I will try to implement the changes soon.
@adbridge This component was not complying the rules for an integration into Mbed-os, so it was offloaded here. It would be nice if the master branch could be filled.

@irfanafa is this issue still pending? I don't have a NUCLEO_F746ZG, so I can check it only in theorie.

irfanafa · 2020-08-13T12:05:03Z

It's good for the STM32F746. Thankyou

…

On Thu, 13 Aug 2020, 14:03 JojoS, ***@***.***> wrote: It seems I missed some notifications about updates here. I will try to implement the changes soon. @adbridge <https://github.com/adbridge> This component was not complying the rules for an integration into Mbed-os, so it was offloaded here. It would be nice if the master branch could be filled. @irfanafa <https://github.com/irfanafa> is this issue still pending? I don't have a NUCLEO_F746ZG, so I can check it only in theorie. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKVTDIBKONA5QCQAIPNJBGTSAPI75ANCNFSM4GC6LHLA> .

mbed file structure has changed again

jeromecoutant

Didn't test, but I assume @JojoS62 did !
Thx

JojoS62 · 2021-06-09T16:56:12Z

in the past month, I reworked this component for the F7. I added also support for the DISCO_F769NI, which uses the 2nd SDMMC interface. I still need to clean up experimental stuff and run the filesystem tests again.
I experienced some problems with the data caching and added some cache flush functions. The difficult thing is, that it depends on the memory what is used. Mbed uses a 'one linker script fits all', but this is difficult to keep when F7 with D-cache and DMA is used. Same problem when I used DMA2D for graphics. I can open an issue for that subject, I want to mention it only here because it can drive you nuts...

JojoS62 added 6 commits November 9, 2018 22:36

initial commit

a9deb13

add SDIOBlockdevice implementation for STM32F4

Update README.md

d6beb4d

added some description

add uitl folder, necessary for tests

e59a952

add implementation for STM32F7

73b18fc

almost the same as for STM32F4, but in F7 SDIO device is renamed to SDMMC SDMMC1 is used for DISCO_F746NG updated readme.md for new target

changed copyright notice to standard mBed

8717513

changed copyright to mBed standard

2d8e3d1

offirko reviewed Nov 22, 2018

View reviewed changes

code cleanup

016abcc

- removed comments - renamed short variable names

offirko suggested changes Dec 16, 2018

View reviewed changes

fiixed timeout checking

c03489c

tickstart was inside checking loop removed wait_ms() calls added timeout checks for all while loops

offirko approved these changes Feb 11, 2019

View reviewed changes

deepikabhavnani approved these changes Feb 11, 2019

View reviewed changes

0xc0170 reviewed Feb 11, 2019

View reviewed changes

0xc0170 requested changes Feb 11, 2019

View reviewed changes

offirko mentioned this pull request Feb 19, 2019

Add sdio driver #2

Open

0xc0170 mentioned this pull request Feb 28, 2019

Is SDIO host mode enabled? ARMmbed/mbed-os#9881

Closed

orenc17 mentioned this pull request Mar 26, 2019

SDIO support - WIP ARMmbed/mbed-os#10235

Closed

jeromecoutant mentioned this pull request Jun 27, 2019

mbed 5.12.4 integration ARMmbed/mbed-os#10909

Closed

Fix order order of parameters

06289dd

mathias-arm mentioned this pull request Aug 9, 2019

Unified implementation for TARGET_STM and TARGET_NXP #3

Closed

JojoS62 added 2 commits December 19, 2019 17:27

added get_type() function

e2ade76

fixed printf warning

f1b70f2

JojoS62 added 2 commits August 16, 2020 12:51

remove path from include

55d1557

mbed file structure has changed again

add SDMM2 for F7 with 2 infterfaces

4985bb9

jeromecoutant approved these changes Jun 9, 2021

View reviewed changes

0xc0170 self-requested a review June 28, 2021 15:05

JojoS62 added 4 commits October 31, 2022 19:22

update for mbed-os 6

dd0c09c

add H7

a61242c

change lib type to static

d132129

add sleep in wait for data Todo: use event_flags

e4ea1df

initial PR for SDIOBlockdevice implementation #1

Are you sure you want to change the base?

initial PR for SDIOBlockdevice implementation #1

Conversation

JojoS62 commented Nov 9, 2018 • edited Loading

0xc0170 commented Nov 12, 2018

dannybenor commented Nov 12, 2018

screamerbg commented Nov 22, 2018

dannybenor commented Nov 22, 2018

offirko Nov 22, 2018

Choose a reason for hiding this comment

JojoS62 Nov 22, 2018

Choose a reason for hiding this comment

offirko Nov 22, 2018

Choose a reason for hiding this comment

offirko Nov 22, 2018

Choose a reason for hiding this comment

offirko commented Nov 22, 2018

JojoS62 commented Nov 22, 2018

screamerbg commented Dec 13, 2018

dannybenor commented Dec 13, 2018

deepikabhavnani commented Dec 13, 2018

dannybenor commented Dec 16, 2018

offirko Dec 16, 2018

Choose a reason for hiding this comment

offirko commented Dec 16, 2018

jeromecoutant commented Jan 15, 2019

screamerbg commented Feb 7, 2019

JojoS62 commented Feb 7, 2019

offirko left a comment

Choose a reason for hiding this comment

deepikabhavnani left a comment

Choose a reason for hiding this comment

deepikabhavnani commented Feb 11, 2019

0xc0170 commented Feb 11, 2019

0xc0170 Feb 11, 2019

Choose a reason for hiding this comment

0xc0170 left a comment

Choose a reason for hiding this comment

deepikabhavnani commented Feb 11, 2019

0xc0170 commented Feb 19, 2019

jeromecoutant commented Mar 6, 2019

adbridge commented Mar 6, 2019

maclobdell commented Mar 15, 2019 • edited Loading

jeromecoutant commented Aug 13, 2020

adbridge commented Aug 13, 2020

JojoS62 commented Aug 13, 2020 • edited Loading

irfanafa commented Aug 13, 2020 via email

jeromecoutant left a comment

Choose a reason for hiding this comment

JojoS62 commented Jun 9, 2021

JojoS62 commented Nov 9, 2018 •

edited

Loading

maclobdell commented Mar 15, 2019 •

edited

Loading

JojoS62 commented Aug 13, 2020 •

edited

Loading