Storage: Fix FATFileSystem unmount issue #14137
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rajkan01
changed the title
ciarmcom
added
the
release-type: patch
|
@rajkan01, thank you for your changes. |
0xc0170
changed the title
evedon
previously requested changes
Jan 12, 2021
| @@ -330,8 +330,14 @@ int FATFileSystem::unmount() | |||
| } | |||
|
|
|||
| FRESULT res = f_mount(NULL, _fsid, 0); | |||
| _ffs[_id] = NULL; | |||
| _id = -1; | |||
| if (!res) { | |||
There was a problem hiding this comment.
res is of type FRESULT not boolean, so check if res != FR_OK
There was a problem hiding this comment.
I have done the rework in this PR, please re-review
Yes, this is rather confusing. Even the documentation does not mention mount() would also initialize the blockdevice. I guess even those who wrote the examples just forgot about bd->deinit(); Actually i am for adding bd->deinit() to fs.unmount(). That way also bd->deinit() would happen automatically, after which we need to call fs.mount(bd); anyway if we want to access the file system once again. At least i can not think of any use case where binding bd->deinit() to fs.unmount() would break the code. |
rajkan01
force-pushed
the
fix_fatfilesytem_unmount
branch
from
a5ef032 to
1f94a5c
Compare
…y libary) API is called from both mount() and unmount() Mbed OS APIs. f_mount() is doing both initializing (via find_volume() -> disk_initialize() where disk_initialize api calls Mbed OS init() function) underlying block device and register filesystem but in case of "unmount" calls this f_mount is doing deregister filesystem and missed to a deinitializing block device. So added the Mbed OS "deint()" call in unmount API to deinitialize block device as f_mount API doesn't have a way to call deinit().
rajkan01
force-pushed
the
fix_fatfilesytem_unmount
branch
from
1f94a5c to
a29e3cd
Compare
rajkan01
changed the title
evedon
approved these changes
Jan 15, 2021
|
@0xc0170 This PR is ready for CI, could you please review and start the CI |
0xc0170
approved these changes
Jan 18, 2021
|
CI started |
Jenkins CI Test : ❌ FAILEDBuild Number: 3 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
|
@0xc0170 CI failure related to mbed deploy command failed with mbed os AWS example, please restart the CI |
|
Restarted |
Jenkins CI Test : ❌ FAILEDBuild Number: 4 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
|
CI restarted |
Jenkins CI Test : ✔️ SUCCESSBuild Number: 5 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
mbedmain
added
release-version: 6.7.0
Release-pending
and removed
release-type: patch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of changes
f_mount() (http://www.elm-chan.org/fsw/ff/00index_e.html - third party libary) API is called from both mount() and unmount() Mbed OS APIs.
f_mount() is doing both initializing (via find_volume() -> disk_initialize() where disk_initialize API calls Mbed OS block device init() function) underlying block device and register filesystem but in case of "unmount" calls, this f_mount is doing deregister filesystem and missed to a deinitializing block device.
So added the Mbed OS "deint()" call in unmount API to deinitialize block device as f_mount API doesn't have a way to call deinit().
fixes #12621
Impact of changes
With these changes, Unmount API will do both deregistering filesystem and deinitializing the underlaying block device.
Migration actions required
None.
Documentation
None.
Pull request type
Test results
Reviewers