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

doc: kernel: document general policy for Zephyr without threads #29338

Closed
wants to merge 1 commit into from

Conversation

pabigot
Copy link
Collaborator

@pabigot pabigot commented Oct 19, 2020

This provides the documentation of scope required as a stage towards removing deprecation for CONFIG_MULTITHREADING=n. The specific lists of what does work will follow as the code base is inspected and updated.

Addresses a gating item in #27415.

Copy link
Member

@carlescufi carlescufi left a comment

Choose a reason for hiding this comment

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

This looks like a good starting point. I don't miss anything in here, and my comments are mostly nits.


* Non-sleeping delays e.g. :c:func:`k_busy_wait`

* The system clock including :c:func:`k_uptime_get`
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be worthwhile mentioning SYS_CLOCK_EXISTS here, because when no MT is often combined with no system clock

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, I'll have to look at that. Looks like SYS_CLOCK_EXISTS=n means nothing related to ticks is supported, including k_uptime_get(). So the only way to detect passage of time is k_cycle_get_32().

I guess this one goes too. The value of this feature to me is diminishing rapidly....

* Applications that select :option:`CONFIG_UART_INTERRUPT_DRIVEN` may
work, depending on driver implementation.

* Applications that select :option:`CONFIG_UART_ASYNC_API` may
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure about this one. I always thought this actually required at least k_timer:
https://github.com/zephyrproject-rtos/zephyr/blob/master/drivers/serial/uart_nrfx_uarte.c#L830

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's unfortunate. I wouldn't expect other SoC's to have that problem. I suppose we don't support that API, then.

@carlescufi
Copy link
Member

@nashif @andyross @andrewboie this addresses the documentation of the scope of CONFIG_MULTITHREADING as agreed in #27415. I would be grateful if you could take a look and see if you are in line with this scope.

@pabigot
Copy link
Collaborator Author

pabigot commented Jan 11, 2021

I've tweaked the text a little, and added more structure.

I've also explicitly noted the requirement that CONFIG_SYS_CLOCK_EXISTS=y be present in order to ensure that support for detecting the passage of time is present. This is open for debate.

Note that all this is preliminary, intended to capture high level requirements before reviewing the code to determine what can actually be supported based on existing use and target-specific capabilities.

Subsystem Behavior Without Thread Support
*****************************************

The seconds below list driver and functional subsystems that are
Copy link
Member

Choose a reason for hiding this comment

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

The seconds below?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

s/seconds/sections/

Functionality that will not work with :option:`CONFIG_MULTITHREADING`
includes but is not limited to:

* :c:func:`k_sleep()` and any other API that causes a thread to
Copy link
Member

Choose a reason for hiding this comment

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

We should consider being explicit here, just like we are explicit above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Better would be to make progress on #18970 so we can just say "Any api that sleeps except if using the no-wait path".

Copy link
Member

Choose a reason for hiding this comment

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

Since we don't have that as of now, I'd rather you mention that explicitly as @nashif mentions so we can move this forward while we wait for progress in #18970.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure if the direction given was to list all API that meets the "sleeps" criteria (which I can't do until #18970 progresses to the point where we've identified that API), or to just refine the text to match what I said. So I did the latter.

This provides the documentation of scope required as a stage towards
removing deprecation for CONFIG_MULTITHREADING=n.  The specific lists
of what does work will follow as the code base is inspected and
updated.

Signed-off-by: Peter Bigot <[email protected]>
@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Mar 28, 2021
@github-actions github-actions bot closed this Apr 11, 2021
@nordic-krch
Copy link
Contributor

@pabigot if you are ok with that I would like to pick up this work and reopen PR as i would like to de-deprecate no multithreading support.

@pabigot
Copy link
Collaborator Author

pabigot commented Apr 14, 2021

@nordic-krch it's yours.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants