Skip to content

Conversation

@vanwinkeljan
Copy link
Member

@vanwinkeljan vanwinkeljan commented May 29, 2019

Add support to access native posix flash device via host file system.
The flash device is mapped into the host file system by means of the FUSE library.

These changesets where original part of PR #12503

edit: actually only access to FS partitions is provided

@vanwinkeljan vanwinkeljan added the area: native port Host native arch port (native_sim) label May 29, 2019
@vanwinkeljan vanwinkeljan requested review from aescolar, galak and nashif May 29, 2019 07:11
@zephyrbot
Copy link

zephyrbot commented May 29, 2019

All checks are passing now.

Review history of this comment for details about previous failed status.
Note that some checks might have not completed yet.

@vanwinkeljan vanwinkeljan force-pushed the fuse_flash_native_posix branch 2 times, most recently from fc7af16 to 2b2341a Compare May 29, 2019 08:45
Copy link
Member

@aescolar aescolar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move the code, kconfig and cmake into drivers/flash
The Kconfig can be into drivers/flash/Kconfig.native_posix
The cmake chunk into drivers/flash/CMakeLists.txt
The code could be just named drivers/flash/fuse_native_posix.c or so

@vanwinkeljan
Copy link
Member Author

Please move the code, kconfig and cmake into drivers/flash

I did try this but it is not an option as the compiler flags need to be changed to make use of FUSE and there is a conflict with the flags used in the zephyr flash library.

Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the documentation. I've got a few recommended edits.

@aescolar
Copy link
Member

there is a conflict with the flags used in the zephyr flash library.

@vanwinkeljan Would declaring the fuse part as a separate library do the trick? (even if it is in the same folder as the other flash drivers)

@aescolar aescolar requested a review from nvlsianpu June 7, 2019 12:57
@vanwinkeljan vanwinkeljan force-pushed the fuse_flash_native_posix branch from 2b2341a to 37192c7 Compare June 7, 2019 18:00
@vanwinkeljan
Copy link
Member Author

Would declaring the fuse part as a separate library do the trick? (even if it is in the same folder as the other flash drivers)

Yes it does, moved the code to subsys/fs

@zephyrbot zephyrbot added area: Boards area: API Changes to public APIs area: Samples Samples labels Jun 7, 2019
@vanwinkeljan
Copy link
Member Author

Renamed fuse_flash to fuse_fs_access as the code actually only provides access to the FS partitions

@aescolar aescolar requested a review from dbkinder June 7, 2019 18:24
@aescolar aescolar dismissed their stale review June 7, 2019 18:25

Addressed

@aescolar
Copy link
Member

aescolar commented Jun 7, 2019

@vanwinkeljan I guess you have tested that it works nicely?
You may want to add a line in CODEOWNERS for this new fuse file assigned to yourself.
If you commit to maintain/support this new functionality I think it should be ok to merge it, as there is very little risk, this being a new optional feature.
Regarding the new fs function, I hope @nvlsianpu has taken a look(?) and is ok with it

@aescolar
Copy link
Member

aescolar commented Jun 7, 2019

@vanwinkeljan Apart from those minor issues I'm ok with this, and can approve it after you fixed them

Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one fix to remove the doc-build warning...

@vanwinkeljan vanwinkeljan force-pushed the fuse_flash_native_posix branch from 37192c7 to 1349de5 Compare June 7, 2019 23:16
@vanwinkeljan
Copy link
Member Author

You may want to add a line in CODEOWNERS for this new fuse file assigned to yourself.

Done

If you commit to maintain/support this new functionality I think it should be ok to merge it,

No problem with that, I think it is an added value for zephyr

@vanwinkeljan vanwinkeljan requested a review from MaureenHelm June 7, 2019 23:24
@dbkinder
Copy link
Contributor

dbkinder commented Jun 8, 2019

The Makefile in the Zephyr root has this htmldocs-fast target.

@aescolar aescolar requested a review from dbkinder June 8, 2019 08:09
@nvlsianpu
Copy link
Contributor

Any chance that this will work on widowns?

@vanwinkeljan
Copy link
Member Author

@nvlsianpu

Any chance that this will work on widowns?

No currently this not possible due to 2 limitations:

  • Fuse is not suported on windows
  • The WSL layer does not support 32-bit application, and native_posix is a 32-bit appilcation

Now it looks that both issues will be solved by introduction of WSL2, an initial build of WSL2 would be available by end of June for people with access to WIndows Insider.
https://devblogs.microsoft.com/commandline/announcing-wsl-2/

Added API the read mount point names

Signed-off-by: Jan Van Winkel <jan.van_winkel@dxplore.eu>
Added support to access flash device partitions from host through
FUSE.

Signed-off-by: Jan Van Winkel <jan.van_winkel@dxplore.eu>
Added file system shell sample

Signed-off-by: Jan Van Winkel <jan.van_winkel@dxplore.eu>
@vanwinkeljan vanwinkeljan force-pushed the fuse_flash_native_posix branch from 1349de5 to 29fb346 Compare June 10, 2019 09:56
@vanwinkeljan vanwinkeljan requested a review from nvlsianpu June 10, 2019 10:44
Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docs LGTM, thanks!

@nashif nashif merged commit 265b195 into zephyrproject-rtos:master Jun 11, 2019
@vanwinkeljan vanwinkeljan deleted the fuse_flash_native_posix branch June 12, 2019 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: API Changes to public APIs area: Boards area: File System area: native port Host native arch port (native_sim) area: Samples Samples

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants