-
Notifications
You must be signed in to change notification settings - Fork 2k
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
pkg/lwip: add support for HAL radios that require IRQ offloading #18465
Conversation
Why can't LWIP use |
It definitely can, I just wanted to speed up the migration process of the HAL. I was planning anyway to migrate all pending IPC based IRQ offloaders at some point. |
This needs a rebase |
rebased! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me - there is now a leftover kw2xrf_setup()
in kw2xrf.h
- want to get rid of that while you're at it?
kw2xrf_init(&kw2xrf_devs[i], &kw2xrf_params[i], &kw2xrf_netdev[i].submac.dev, | ||
bhp_msg_isr_cb, &netif[i].bhp); | ||
|
||
netdev_register(&kw2xrf_netdev[i].dev.netdev, NETDEV_KW2XRF, i); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this not done by kw2xrf_init()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because that would add a dependency to netdev
that does not exist in the current Radio HAL drivers.
sure, good catch. I can remove it |
done! |
Probably a mutex can do the trick, but still, this should probably be handled in a separate PR, as it affects essentially all network devices |
@maribu just recently stumbled across this too. |
It uses only only uses "one thread" (with quotes because it actually allocates one for every network device), everything else runs in the context of the caller (for I just implemented a version with IMO the solutions are either:
Any preference? |
I think it is possible to use a single thread to handle both lwip events as well as ISRs. With But having an rmutex until then sounds good to me. |
This needs a rebase, but then it should be good to merge, right? |
rebased! and yes, it should be ready to merge |
CI is unhappy |
Unrelated error |
I don't think so |
I really hope I fixed all the unrelated error issues... |
try building locally |
I think I saw the wrong output, because sure, it's definitely a problem of this PR |
it was a rebase error. Now it should be fixed |
Contribution description
This PR utilizes #18464 to run radio HAL based devices with LWIP. This also allows the usage of other non-netdev devices that require ISR offloading.
As a result of this PR, I also push support for KW2XRF radios on LWIP.
Testing procedure
Murdock compile output should be fine
Test LWIP with any KW2XRF based radio.
Issues/PRs references
Depends on #18464Now it depends on #18479