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.

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

BOARDIOC_SDCARD_SETNOTIFYCB callback from OS into application #6046

Closed
pkarashchenko opened this issue Apr 12, 2022 · 8 comments · Fixed by #7584 or #8243
Closed

BOARDIOC_SDCARD_SETNOTIFYCB callback from OS into application #6046

pkarashchenko opened this issue Apr 12, 2022 · 8 comments · Fixed by #7584 or #8243
Assignees
Labels
modularity Needed to support modular architecture

Comments

@pkarashchenko
Copy link
Contributor

The #6036 introduce BOARDIOC_SDCARD_SETNOTIFYCB that allows attaching application callback to be called from the OS.
From mailing list "FS automount: callback when FS is mounted":
@pkarashchenko : Is there a way to get a callback when file system is mounted and ready to be accessed?
@patacongo : No, there will never never be callbacks from the OS into application code. That is not the POSIX way. The POSIX way would use signals for things like this.

Examples of similar implementation are:

  • include/nuttx/buttons.h : BTNIOC_REGISTER
  • include/nuttx/fs/ioctl.h : FIOC_NOTIFY

The BOARDIOC_SDCARD_SETNOTIFYCB should be removed before next NuttX release.

@patacongo
Copy link
Contributor

patacongo commented Apr 12, 2022

There is a related problem in sched/signal/sig_notification.c. POSIX requires that signals be able to be delivered on a thread via a callback. If CONFIG_SIG_EVTHREAD is enabled, then the LP or HP work queue is used to perform the callback. But the work queues are within the OS and cannot call into applications.

There is at least some protection for this in sched/Kconfig:

1364 config SIG_EVTHREAD
1365         bool "Support SIGEV_THREAD"
1366         default n
1367         depends on !BUILD_KERNEL && SCHED_WORKQUEUE
1368         select LIBC_USRWORK if BUILD_PROTECTED
1369         ---help---
1370                 Built in support for the SIGEV_THREAD signal deliver method.

1371
1372 NOTE: The current implementation uses a work queue to notify the
1373 client. This, however, would only work in the FLAT build. A
1374 different mechanism would need to be developed to support this
1375 feature on the PROTECTED or KERNEL build.

The Kconfig selects the user work queue, but that is not used.

@pkarashchenko pkarashchenko added the modularity Needed to support modular architecture label May 24, 2022
@pkarashchenko
Copy link
Contributor Author

@SPRESENSE are there any plans to fix this hole in modularity? The automount signaling is already in place, so it would be good to fix this instead of keeping releasing with it.

@SPRESENSE
Copy link
Contributor

@pkarashchenko Still under consideration, so please leave it as is.

@pkarashchenko
Copy link
Contributor Author

@pkarashchenko Still under consideration, so please leave it as is.

Maybe you can share via e-mail an example app that use it so I can rework it by myself if you do not have capacity to do it?

@SPRESENSE
Copy link
Contributor

@pkarashchenko san
Thank you for your suggestion, it is very helpful.

An example application can be found here.
https://github.com/sonydevworld/spresense/tree/master/examples/dsc

To verify this application, the Spresense main board, extension board, camera board and LCD (ILI9341) are required. In the main function, BOARDIOC_SDCARD_SETNOTIFYCB is used to detect the insertion and removal of the SD card.
https://github.com/sonydevworld/spresense/blob/master/examples/dsc/dsc_main.cxx#L297

This application is using the Spresense SDK, but we can work as NuttX application with the following patches.

$ cp -r <spresense>/examples/dsc <your-nuttx>/apps/examples
diff --git a/examples/dsc/Make.defs b/examples/dsc/Make.defs
index d991f2baf..836c1c9dd 100644
--- a/examples/dsc/Make.defs
+++ b/examples/dsc/Make.defs
@@ -34,5 +34,5 @@
 ############################################################################

 ifneq ($(CONFIG_EXAMPLES_DSC),)
-CONFIGURED_APPS += dsc
+CONFIGURED_APPS += $(APPDIR)/examples/dsc
 endif
diff --git a/examples/dsc/Makefile b/examples/dsc/Makefile
index 50af61d9e..b69f80802 100644
--- a/examples/dsc/Makefile
+++ b/examples/dsc/Makefile
@@ -34,7 +34,6 @@
 ############################################################################

 include $(APPDIR)/Make.defs
-include $(SDKDIR)/Make.defs

 PROGNAME = $(CONFIG_EXAMPLES_DSC_PROGNAME)
 PRIORITY = $(CONFIG_EXAMPLES_DSC_PRIORITY)
diff --git a/examples/dsc/utils/font_draw.c b/examples/dsc/utils/font_draw.c
index 1336cc039..615b633c9 100644
--- a/examples/dsc/utils/font_draw.c
+++ b/examples/dsc/utils/font_draw.c
@@ -38,7 +38,6 @@
  ****************************************************************************/

 #include <nuttx/config.h>
-#include <sdk/config.h>
 #include <stdio.h>
 #include <string.h>
 #include <ctype.h>

You can use a defconfig with a modified spresense:example_camera.

diff --git a/boards/arm/cxd56xx/spresense/configs/example_camera/defconfig b/boards/arm/cxd56xx/spresense/configs/example_camera/defconfig
index 9e07ae6901..1cc7762872 100644
--- a/boards/arm/cxd56xx/spresense/configs/example_camera/defconfig
+++ b/boards/arm/cxd56xx/spresense/configs/example_camera/defconfig
@@ -20,6 +20,7 @@ CONFIG_ARCH_CHIP="cxd56xx"
 CONFIG_ARCH_CHIP_CXD56XX=y
 CONFIG_ARCH_STACKDUMP=y
 CONFIG_ARMV7M_USEBASEPRI=y
+CONFIG_BOARDCTL_IOCTL=y
 CONFIG_BOARD_LOOPSPERMSEC=5434
 CONFIG_BOOT_RUNFROMISRAM=y
 CONFIG_BUILTIN=y
@@ -37,6 +38,8 @@ CONFIG_DEBUG_SYMBOLS=y
 CONFIG_DRIVERS_VIDEO=y
 CONFIG_EXAMPLES_CAMERA=y
 CONFIG_EXAMPLES_CAMERA_OUTPUT_LCD=y
+CONFIG_EXAMPLES_DSC=y
+CONFIG_EXAMPLES_DSC_KEYBOARD_INPUT=y
 CONFIG_FAT_LCNAMES=y
 CONFIG_FAT_LFN=y
 CONFIG_FS_FAT=y

I need to keep the application compatible. Can you add the reworked code while leaving the existing code in Kconfig/ifdef?
I would like to remove the old code after all applications have been switched over to the reworked code.

Thanks.

@pkarashchenko
Copy link
Contributor Author

It seems to be not a low hanging fruit, but still I've done initial changes. I will open a PR, but will need you to test because I do not have the HW.

@pkarashchenko
Copy link
Contributor Author

pkarashchenko commented Nov 12, 2022

@SPRESENSE please take a look into #7584
I will keep application code until this PR is reviewed and approved.
I would appreciate if you can find some time to try the changes by enabling fs automount with NSH. The test case is pretty trivial. You should ls /mnt/sd0 and insert / eject the card. The approximate detection timeout is around 2 seconds, but can be tuned via parameter. If all will work as expected then I will push application code and then we can eliminate BOARDIOC_SDCARD_SETNOTIFYCB from the code.
The application code is also depending on #7583

@pkarashchenko
Copy link
Contributor Author

@SPRESENSE please review sonydevworld/spresense#536

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modularity Needed to support modular architecture
Projects
None yet
5 participants