Open drensber opened 10 months ago
dpgeorge
commented
10 months ago Thanks for this! I'll have to take a good look.
Regarding the license: did you pick Apache 2 to match Zephyr, or because you took files from Zephyr that were already Apache 2? In other words, could it be licensed as MIT instead?
drensber
commented
10 months ago Damien,
The reason that I licensed as Apache 2 was because we were originally planning to submit this to the Zephyr Project, and that’s their license. Since it’s now going to be part of your repository, I /think/ Beechwoods Software and I would be fine with it being MIT license, but I need to check with Brad who owns Beechwoods to see what he thinks. We didn’t really take any files from Zephyr, and they already support integrating with “modules” that aren’t Apache-2 (the RPi Pico SDK, for example), so I don’ think that they believe that extensions to Zephyr need to be unless they’re being submitted to the Zephyr repository itself.
—Dave
On Nov 29, 2023, at 6:33 PM, Damien George @.***> wrote:
Thanks for this! I'll have to take a good look.
Regarding the license: did you pick Apache 2 to match Zephyr, or because you took files from Zephyr that were already Apache 2? In other words, could it be licensed as MIT instead?
— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.
dpgeorge
commented
10 months ago OK, thanks for the explanation. In principal Apache 2 is fine, but MIT is a bit simpler (although licensing is a tricky thing and "simple" is subjective!).
peterharperuk
commented
10 months ago We may want to consider maintaining this as a separate repository
Yes please. I don't think users of cyw43-driver will want all that stuff?
dpgeorge
commented
10 months ago Yes please. I don't think users of cyw43-driver will want all that stuff?
@peterharperuk do you have any specific concerns about merging this PR to this repo? There's no decision about it yet, and would be good to hear opinions. Text files are small and having them in a separate directory shouldn't affect other users of this driver.
peterharperuk
commented
10 months ago My comment was just referring to having to have a "zephyr_build" folder in this repo which just looks a bit odd. pico-sdk has a pico_cyw43_driver component and I wouldn't expect to add that to this repo. Having said that - I've not looked in detail at it.
drensber
commented
10 months ago My comment was just referring to having to have a "zephyr_build" folder in this repo which just looks a bit odd. pico-sdk has a pico_cyw43_driver component and I wouldn't expect to add that to this repo. Having said that - I've not looked in detail at it.
Well, I was originally thinking of it as an alternative to LWIP as the IP stack (and the LWIP interface /is/ in cyw43-driver). ...but this is more than just an IP stack (it's an OS binding too) and it's much bigger than lwip.c. The code I've submitted here /will/ get somewhat smaller, because it currently carries some things that just aren't merged into the zephyrproject mainline yet, but it will always be at least a dozen files, I think. If you guys would be willing to create a separate repository for the Zephyr support, I think that would be great. I do think it would be beneficial if it's part of /your/ GitHub rather than Beechwoods, because it is a companion to cyw43-driver, and I think people would be much more likely to find it and use it if it's part of your GitHub.
peterharperuk
commented
10 months ago I just see this as a driver for some infineon wifi chips. Really lwip support should not be in here either. It does look like it should live elsewhere but that's just my opinion.
drensber
commented
10 months ago I just see this as a driver for some infineon wifi chips. Really lwip support should not be in here either. It does look like it should live elsewhere but that's just my opinion.
I agree. When I was doing this Zephyr port, it was really a pleasant surprise to see how well you'd separated the generic driver from the parts that were specific to the RPi SDK or the IP stack (even though LWIP is included in the repo, it can be configured completely out). It was really great to find that I could integrate this driver with Zephyr without modifying any of the core cyw43-driver code at all. I think that separation is important to maintain, which was why I put the Zephyr specific stuff all in a separate directory, but a separate repository would probably be even better.
drensber
commented
10 months ago I talked the licensing issue over with some people during our internal meeting today. We're perfectly fine with MIT license if that's what you prefer.
drensber
commented
8 months ago We've published the Zephyr binding for your driver on Beechwoods's GitHub. It just pulls your driver in as a submodule. We'd still be happy to publish this on your GitHub instead if you'd like.
Our repo is: https://github.com/beechwoods-software/zephyr-cyw43-driver.git
I guess this PR can be closed, since I think that we all agree that adding it into the cyw43-driver repo doesn't make sense.
Hi, I suspect there will be a bit of back and forth here, but it has to start somewhere. This is my code to integrate the driver with the Zephyr OS and Zephyr's native IP stack.
The file zephyr_build/README.md explains how to do an out-of-tree build that will produce a Zephyr image with WiFi support. The example "app" is pretty minimal... it really just demonstrates which configuration items need to be in the prj.conf file to include the zephyr driver. Most interaction with the driver can be accomplished through Zephyr's network shell CLI.
We may want to consider maintaining this as a separate repository that pulls in georerobotics/cyw43-driver as a git submodule rather than as a subdirectory in main driver repository. That would be a little bit cleaner from a Zephyr perspective (it's the way that external Zephyr modules with out-of-tree builds are normally managed), but maintaining it as a subdirectory works too.
It currently only supports Raspberry Pi Pico W hardware, but the README in the zephyr_build directory explains how to provide support for other boards if someone wants to (although there may be limited interest in this, since Infineon has already provided their own driver to the Zephyr Project for boards that can use a regular SDIO interface).
I realize you'll need some time to review and try this, but please let me know as soon as you can if this at least looks good at a high level.