fat_fs: Add reentrant zephyr support #51484

Ox11 · 2022-10-20T13:02:41Z

This commit enables zephyr to configure the FatFs FF_FS_REENTRANT
option and support fs actions from multiple threads.
CONFIG_FS_FATFS_REENTRANT enables the option and provides zephyr
mutex wrappers.

Signed-off-by: Nicola Ochsenbein [email protected]

de-nordic · 2022-10-21T09:26:32Z

Missing west.yml manifest update to include change in external module https://github.com/zephyrproject-rtos/fatfs/pull/13/files:
https://docs.zephyrproject.org/latest/develop/modules.html#changes-to-existing-module

zephyrbot · 2022-10-21T11:02:38Z

The following west manifest projects have been modified in this Pull Request:

Name	Old Revision	New Revision	Diff
fatfs	zephyrproject-rtos/fatfs@`89f53db` (master)	zephyrproject-rtos/fatfs#13	zephyrproject-rtos/fatfs#13/files

Note: This message is automatically posted and updated by the Manifest GitHub Action.

de-nordic · 2022-10-21T13:56:01Z

Is it possible to provide test scenarios, in Zephyr, to prove this works correctly?

Ox11 · 2022-10-27T09:31:52Z

Is it possible to provide test scenarios, in Zephyr, to prove this works correctly?

Yes, I added tests.

This commit enables zephyr to configure the FatFs FF_FS_REENTRANT option and support fs actions from multiple threads. CONFIG_FS_FATFS_REENTRANT enables the option and provides zephyr mutex wrappers. R2D2-8240 FS reentrant zephyrproject-rtos#51484 Signed-off-by: Nicola Ochsenbein <[email protected]> (cherry picked from commit 67e7e5b)

nordicjm

this can rebased now to use the 3.4 release note file

nordicjm · 2023-02-07T09:51:38Z

MAINTAINERS.yml

Make separate commit

Done.
But I am not quite sure if i should add me as a maintainer because I feel like I am just a collaborator. But I don't know if I can add a subsystem entry without a maintainer. Any ideas? @de-nordic @nordicjm

Ox11 · 2023-02-27T09:50:33Z

this can rebased now to use the 3.4 release note file

Done.

de-nordic · 2023-03-02T12:15:23Z

MAINTAINERS.yml

Rename this as "FatFs reentrant support"

de-nordic · 2023-03-02T12:16:04Z

MAINTAINERS.yml

I see no reason to pass Maintainership on this file, it is not even touched in this PR. Remove it from list.

Yes, you are right.

de-nordic · 2023-03-02T12:16:54Z

MAINTAINERS.yml

List is missing test_fat_file_reentrant.c file.

This commit enables zephyr to configure the FatFs FF_FS_REENTRANT option and support fs actions from multiple threads. CONFIG_FS_FATFS_REENTRANT enables the option and provides zephyr mutex wrappers. Signed-off-by: Nicola Ochsenbein <[email protected]>

This commit adds tests for the FatFs FF_FS_REENTRANT option. Signed-off-by: Nicola Ochsenbein <[email protected]>

ox11 is maintaining on FatFs. Signed-off-by: Nicola Ochsenbein <[email protected]>

Update release notes 3.4 with the new FatFs reentrant Kconfig option. Signed-off-by: Nicola Ochsenbein <[email protected]>

Ox11 · 2023-03-13T07:18:04Z

Could you please review this PR again @nordicjm @de-nordic ? Did I miss something that I have to do?

Ox11 · 2023-03-13T10:47:43Z

Thank you for your approves @nordicjm @de-nordic. Who can/will merge this PR? - I canot merge it, as I am not authorized.

stephanosio · 2023-03-13T11:10:58Z

MAINTAINERS.yml

+"Filesystems: FatFs reentrant support":
+  status: maintained
+  maintainers:
+    - ox11
+  files:
+    - modules/fatfs/zfs_ffsystem.c
+    - tests/subsys/fs/fat_fs_api/src/test_fat_file_reentrant.c
+  labels:
+    - "area: File System"


From the documentation:

We should keep the granularity of code maintainership at a manageable level

It does not make sense to have "FatFs reentrant support" as a dedicated area -- this is too much granularity.

Please revert this change or define the maintainership as "Filesystems: Fatfs."

I am not a maintainer of the entire FatFS module.
My suggestion would be to either create a new module like this

"Filesystems: FatFs": status: maintained maintainers: - de-nordic collaborators: - ox11 files: - modules/fatfs - tests/subsys/fs/common - tests/subsys/fs/fat_fs_api - tests/subsys/fs/fat_fs_dual_drive labels: - "area: File System"

or to add myself to the collaborators in the fs area

Filesystems: status: maintained maintainers: - de-nordic collaborators: - Laczen - nashif - ox11 files: - include/zephyr/fs/ - samples/subsys/fs/ - subsys/fs/ - tests/subsys/fs/ labels: - "area: File System"

The first option should IMHO be created by @de-nordic as he would be mentioned as maintainer. Additionally, is there anyone else that should be mentioned as collaborator?

This commit enables zephyr to configure the FatFs FF_FS_REENTRANT option and support fs actions from multiple threads. CONFIG_FS_FATFS_REENTRANT enables the option and provides zephyr mutex wrappers. R2D2-8240 FS reentrant zephyrproject-rtos#51484 Signed-off-by: Nicola Ochsenbein <[email protected]>

This commit enables zephyr to configure the FatFs FF_FS_REENTRANT option and support fs actions from multiple threads. CONFIG_FS_FATFS_REENTRANT enables the option and provides zephyr mutex wrappers. R2D2-8240 FS reentrant zephyrproject-rtos#51484 Signed-off-by: Nicola Ochsenbein <[email protected]> (cherry picked from commit 67e7e5b) (cherry picked from commit dd7f6e4)

Ox11 force-pushed the add_fs_fatfs_reentrant_zephyr_support branch 2 times, most recently from 83a179a to 95149bd Compare October 21, 2022 06:07

Ox11 changed the title ~~fat_fs: Add reentrant zephyr support~~ [RFC] fat_fs: Add reentrant zephyr support Oct 21, 2022

Ox11 marked this pull request as ready for review October 21, 2022 06:15

Ox11 requested a review from nashif as a code owner October 21, 2022 06:15

zephyrbot added the area: File System label Oct 21, 2022

zephyrbot requested review from Laczen and de-nordic October 21, 2022 06:16

zephyrbot assigned de-nordic Oct 21, 2022

Ox11 force-pushed the add_fs_fatfs_reentrant_zephyr_support branch from 95149bd to efab516 Compare October 21, 2022 11:01

zephyrbot added manifest west manifest-fatfs labels Oct 21, 2022

Ox11 force-pushed the add_fs_fatfs_reentrant_zephyr_support branch 6 times, most recently from 7dceddd to e967468 Compare October 27, 2022 09:28

Ox11 force-pushed the add_fs_fatfs_reentrant_zephyr_support branch from e967468 to 1549ef9 Compare October 27, 2022 09:38

Ox11 changed the title ~~[RFC] fat_fs: Add reentrant zephyr support~~ fat_fs: Add reentrant zephyr support Oct 27, 2022

stephanosio added area: West West utility and removed west labels Nov 2, 2022

Ox11 marked this pull request as draft November 17, 2022 09:00

Ox11 force-pushed the add_fs_fatfs_reentrant_zephyr_support branch from 1549ef9 to 8c63f7b Compare November 22, 2022 10:14

Ox11 force-pushed the add_fs_fatfs_reentrant_zephyr_support branch from 9253c71 to 5f5feda Compare February 7, 2023 08:37

de-nordic requested a review from arbrauns February 7, 2023 08:58

Ox11 force-pushed the add_fs_fatfs_reentrant_zephyr_support branch 2 times, most recently from 2789f9a to 129d355 Compare February 7, 2023 09:29

Ox11 force-pushed the add_fs_fatfs_reentrant_zephyr_support branch from 129d355 to 5a73b15 Compare February 17, 2023 13:22

zephyrbot requested review from kartben and removed request for arbrauns February 17, 2023 13:23

nordicjm reviewed Feb 20, 2023

View reviewed changes

Ox11 force-pushed the add_fs_fatfs_reentrant_zephyr_support branch 2 times, most recently from b59b6e4 to 93ba51a Compare February 27, 2023 09:47

Ox11 force-pushed the add_fs_fatfs_reentrant_zephyr_support branch from 93ba51a to 671d63c Compare February 27, 2023 11:02

nordicjm previously approved these changes Mar 2, 2023

View reviewed changes

de-nordic requested changes Mar 2, 2023

View reviewed changes

Ox11 dismissed nordicjm’s stale review via d65f540 March 2, 2023 13:25

Ox11 force-pushed the add_fs_fatfs_reentrant_zephyr_support branch from 671d63c to d65f540 Compare March 2, 2023 13:25

Ox11 added 4 commits March 2, 2023 14:32

test: fs: fat_fs_api: Add tests for reentrant zephyr support

8f5a889

This commit adds tests for the FatFs FF_FS_REENTRANT option. Signed-off-by: Nicola Ochsenbein <[email protected]>

MAINTAINERS: Add ox11 to FatFs

d75949e

ox11 is maintaining on FatFs. Signed-off-by: Nicola Ochsenbein <[email protected]>

doc: release-notes: Update release notes 3.4

c789f7b

Update release notes 3.4 with the new FatFs reentrant Kconfig option. Signed-off-by: Nicola Ochsenbein <[email protected]>

Ox11 force-pushed the add_fs_fatfs_reentrant_zephyr_support branch from d65f540 to c789f7b Compare March 2, 2023 13:33

nordicjm approved these changes Mar 13, 2023

View reviewed changes

de-nordic approved these changes Mar 13, 2023

View reviewed changes

carlescufi merged commit 516b673 into zephyrproject-rtos:main Mar 13, 2023

stephanosio reviewed Mar 13, 2023

View reviewed changes

Ox11 mentioned this pull request Sep 5, 2024

MAINTAINERS: remove maintainer FatFs #78035

Merged

fat_fs: Add reentrant zephyr support #51484

fat_fs: Add reentrant zephyr support #51484

Uh oh!

Conversation

Ox11 commented Oct 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

de-nordic commented Oct 21, 2022

Uh oh!

zephyrbot commented Oct 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

de-nordic commented Oct 21, 2022

Uh oh!

Ox11 commented Oct 27, 2022

Uh oh!

nordicjm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ox11 commented Feb 27, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ox11 commented Mar 13, 2023

Uh oh!

Ox11 commented Mar 13, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Ox11 commented Oct 20, 2022 •

edited

Loading

zephyrbot commented Oct 21, 2022 •

edited

Loading