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.

Sign up for GitHub

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

Jump to bottom

FatFs: upgrade to R0.14b #14911

Merged
merged 2 commits into from
Jul 30, 2021
Merged

FatFs: upgrade to R0.14b #14911

merged 2 commits into from
Jul 30, 2021

Conversation

ndrs-pst
Copy link
Contributor

@ndrs-pst ndrs-pst commented Jul 10, 2021 •
edited by Patater
Loading

Summary of changes

Current upstream version of FatFs was now R0.14b with a lot of fixed since R0.13a. LINK
So we migrated Mbed-OS modification in R0.13a back to R0.14b and change some that affect in FATFileSystem.cpp

Impact of changes

FatFs version upgrade that may introduce new regression from upstream.

Migration actions required

FatFs has been upgraded to R0.14b. This comes with the following API changes requiring migration for direct users of FatFs. Users of the Mbed OS C++ class FATFileSystem do not need to change their use of FATFileSystem.

f_mkfs() now takes a MKFS_PARAM *opt instead of a BYTE opt and DWORD au.
f_fdisk() now takes an LBA_t ptbl[] instead of a DWORD *szt.

Documentation

None

Pull request type 

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[X] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results 

[] No Tests required for this change (E.g docs only update)
[X] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@rajkan01 @michalpasztamobica

@ndrs-pst
Update FatFs from R0.13a to R0.14b by manual migration Mbed-OS modifi…
9e5f21b 
…cation in R0.13a to R0.14b

Since a lot of fixed from upstream (0.13b, 0.13c, 0.14, 0.14a and 0.14b)
@see http:https://elm-chan.org/fsw/ff/updates.txt
@mergify mergify bot added the needs: work label Jul 10, 2021
@ciarmcom ciarmcom requested review from rajkan01 and a team July 10, 2021 17:30
@ciarmcom
Copy link
Member

@ndrs-pst, thank you for your changes.
@rajkan01 @ARMmbed/mbed-os-core @ARMmbed/mbed-os-maintainers please review.

@ciarmcom
Copy link
Member

@ndrs-pst, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

@ciarmcom ciarmcom added needs: review stale Stale Pull Request labels Jul 10, 2021
@ciarmcom
Copy link
Member

This pull request has automatically been marked as stale because it has had no recent activity. @rajkan01, @ARMmbed/mbed-os-maintainers, @ARMmbed/mbed-os-core, please complete review of the changes to move the PR forward. Thank you for your contributions.

@LDong-Arm LDong-Arm added this to In progress (8) in Mbed Core via automation Jul 15, 2021
@LDong-Arm LDong-Arm moved this from In progress (8) to Inbox in Mbed Core Jul 15, 2021
@ciarmcom ciarmcom added stale Stale Pull Request and removed stale Stale Pull Request labels Jul 16, 2021
@0xc0170
Copy link
Contributor

0xc0170 commented Jul 21, 2021

AStyle job failed with just one line, please fix.

@ciarmcom ciarmcom removed the stale Stale Pull Request label Jul 21, 2021
@ndrs-pst
Copy link
Contributor Author

@0xc0170 Fixed AStyle job already 🚀

@0xc0170
Copy link
Contributor

0xc0170 commented Jul 21, 2021

We had earlier update that was later closed from @ladislas

@rajkan01 @michalpasztamobica

Please review

@ciarmcom ciarmcom added the stale Stale Pull Request label Jul 26, 2021
Mbed Core automation moved this from Inbox to Reviewer approved & awaiting CI & merge (3) Jul 27, 2021
@mergify mergify bot added needs: CI and removed needs: review labels Jul 27, 2021
@0xc0170
Copy link
Contributor

0xc0170 commented Jul 27, 2021

Ci started

@0xc0170 0xc0170 removed the stale Stale Pull Request label Jul 27, 2021
@mbed-ci
Copy link

mbed-ci commented Jul 27, 2021

Jenkins CI Test : ✔️ SUCCESS

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_greentea-test ✔️

@0xc0170
Copy link
Contributor

0xc0170 commented Jul 27, 2021

Back to review. I would like @ARMmbed/mbed-os-core to approve

@Patater
Copy link
Contributor

Patater commented Jul 29, 2021 •
edited
Loading

Are there any "Migration actions required" for users of FatFs APIs as a result of this upgrade? Maybe we can point to FatFs's release notes about any API changes.

I see the release notes say:
Changed some API functions, f_mkfs() and f_fdisk(). but, that's not really enough to tell users what to change, just that they need to change.

Any other migrations/changes needed?

@ndrs-pst
Copy link
Contributor Author

As far as I checked, the significant of API change was f_mkfs() that the author allow more parameters to pass to.
So, the migration that preserve same behavior would be in FATFileSystem.cpp FATFileSystem::format() which specified zero value to opt.n_fat, opt.align, opt.n_root and let FatFs to determine default value inside.

Left R0.13c vs Right R0.14b
fatfs_r0 13c_to_r0 14
f_expand() : Just typo fixed.
f_fdisk() : Just changing the argument notation to be array of LBA_t instead of pointer to DWORD.

Regards.

@Patater
Copy link
Contributor

Patater commented Jul 30, 2021

I've added a migration section to the PR description, which will end up in the next Mbed OS release notes.

@0xc0170 0xc0170 merged commit 334be7b into ARMmbed:master Jul 30, 2021
Mbed Core automation moved this from Reviewer approved & awaiting CI & merge (3) to Done Jul 30, 2021
@mergify mergify bot removed the ready for merge label Jul 30, 2021
@Patater Patater moved this from Done to Reported in Mbed Core Aug 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Patater Patater Patater approved these changes

@0xc0170 0xc0170 0xc0170 approved these changes

@rajkan01 rajkan01 Awaiting requested review from rajkan01

Assignees
No one assigned
Labels
component: core release-version: 6.14.0
Projects
No open projects
Mbed Core
  
Reported
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@ndrs-pst @ciarmcom @0xc0170 @mbed-ci @Patater @mbedmain