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

canbus: isotp: use new k_work API #34863

Merged
merged 1 commit into from May 5, 2021

Conversation

martinjaeger
Copy link
Member

Replace soon-to-be-deprecated call to start queue.

Delayed work is not used here.

Remark:
While implementing the work queue changes, I realized that the local work queue is actually not used in the ISO-TP implementation. Instead, the work is always submitted to the system work queue with k_work_submit. A trivial fix did not work, so I opened issue #34862 to leave it for a separate PR.

Replace soon-to-be-deprecated call to start queue.

Signed-off-by: Martin Jäger <martin@libre.solar>
Copy link
Collaborator

@pabigot pabigot left a comment

Choose a reason for hiding this comment

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

OK, but if it's not used maybe it should be removed.

@martinjaeger
Copy link
Member Author

OK, but if it's not used maybe it should be removed.

True, but there are also Kconfig symbols associated to it, which could preliminarily break existing firmware if they are removed.

I was wondering if we can't use the system work queue permanently. @pabigot Is there any rule/recommendation when to use a dedicated work queue instead of the system work queue? Looking through other drivers both approaches seem to be common.

The BK issues are unrelated to this PR, btw.

@pabigot
Copy link
Collaborator

pabigot commented May 5, 2021

@pabigot Is there any rule/recommendation when to use a dedicated work queue instead of the system work queue? Looking through other drivers both approaches seem to be common.

https://docs.zephyrproject.org/latest/reference/kernel/threads/workqueue.html#system-workqueue

Ideally if the work handler can sleep don't use the system work queue, but some existing uses run afoul of that expectation which is why USB and maybe other subsystems got their own (sometimes optionally).

@alexanderwachter
Copy link
Member

Danke @martinjaeger !

@galak
Copy link
Collaborator

galak commented May 5, 2021

Going to merge this and we can follow up cleanup / changes in a new PR.

@galak galak merged commit 08b067f into zephyrproject-rtos:master May 5, 2021
@martinjaeger martinjaeger deleted the isotp-work-queue branch May 5, 2021 18:33
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.

None yet

4 participants