search

zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.83k stars 6.6k forks source link

HAL Module Request: hal_telink #34282

Closed yurvyn closed 3 years ago

yurvyn commented 3 years ago

Origin

Zephyr HAL Telink project. Sources to be added: http://wiki.telink-semi.cn/tools_and_sdk/Driver/hal_telink.zip

Purpose

This software provides Hardware Abstraction Layer drivers support for Telink TLSR9 family SoCs.

Mode of integration

Please integrate this project as a module according to the generic Zephyr HAL modules structure.

Pull Request

N/A (new project)

Description

This project provides Telink TLSR9 family peripheral drivers support (such as GPIO, UART, PWM, I2C, SPI, RF etc..), and it is going to be used by Telink B91 SoC and TLSR9518ADK80D board in Zephyr.

No other projects available, no better options.

Dependencies

None.

Revision

http://wiki.telink-semi.cn/tools_and_sdk/Driver/hal_telink.zip

License

3-Clause BSD

galak commented 3 years ago

Please fill out the full form for a new external source repo:

https://github.com/zephyrproject-rtos/zephyr/issues/new?assignees=&labels=TSC&template=ext-source.md&title=

yurvyn commented 3 years ago

Done, please review.

carlescufi commented 3 years ago 
          4. This software, with or without modification, must only be used with a
           TELINK integrated circuit. All other usages are subject to written permission
           from TELINK and different commercial license may apply.

I am sorry, but this clause violates the OSI open source definition, which means that this is not an OSI-approved license. This makes it impossible to be integrated with upstream zephyr, as described here. The only viable paths forward for you are:

yurvyn commented 3 years ago

Thanks for the info. Let's keep this ticket open and I will came back with updates as soon as possible.

carlescufi commented 3 years ago

@yurvyn sounds good. Thanks.

yurvyn commented 3 years ago

In case if Telink relicense this HAL (to eg. 3-clause BSD), it still contains "libB91.a" library. Is the library usage acceptable?

carlescufi commented 3 years ago

In case if Telink relicense this HAL (to eg. 3-clause BSD), it still contains "libB91.a" library. Is the library usage acceptable?

Zephyr doesn't currently accept precompiled binaries directly, but you can check with @sylvioalves how Espressif solved this problem for the ESP32 platform.

yurvyn commented 3 years ago

In case if Telink relicense this HAL (to eg. 3-clause BSD), it still contains "libB91.a" library. Is the library usage acceptable?

Zephyr doesn't currently accept precompiled binaries directly, but you can check with @sylvioalves how Espressif solved this problem for the ESP32 platform.

As I see, "libhal.a" library is directly linked in "hal_espressif/components/xtensa/CMakeLists.txt". It is the same what I am doing in "hal_telink/B91_Driver_v1.5.0/CMakeLists.txt" (libB91.a).

So, maybe, precompiled binaries are allowed for vendors HALs. @sylvioalves am I right?

carlescufi commented 3 years ago

So, maybe, precompiled binaries are allowed for vendors HALs. @sylvioalves am I right?

They are not allowed in a repository hosted by the zephyrproject-rtos GitHub organization, so you will need to pull them in via a command or a Git submodule.

sylvioalves commented 3 years ago

As I see, "libhal.a" library is directly linked in "hal_espressif/components/xtensa/CMakeLists.txt".

@yurvyn, yes, the binary is linked in the CMakeLists but the file itself cannot be hosted by the zephyrproject-rtos.
The proposed solutions by the maintainers are:

  1. Put the binaries in a submodule that is linked into your hal module repo. Then, on west.yml, you add the submodules: true so that it can be pulled by west update automatically.
  2. Create a west command that resides in hal_telink module. The west command will perform the binary download with some specific command as such as west telink get-binaries.
carlescufi commented 3 years ago

Thanks @sylvioalves. Could you describe what option Espressif took and where this is implemented, for @yurvyn's information?

yurvyn commented 3 years ago

The "libhal.a" file is directly included in hal_espressif, it is not like git submodule.

I will try to resolve mentioned issues for hal_telink and will come back once I have updates. Thanks for the support.

yurvyn commented 3 years ago

@galak, @carlescufi Before making decisions, I have two questions.

  1. Is the next case acceptable? hal_telin contains only cmake and module yaml files, the driver files are added as a git submodule with the existing Telink License and precompiled binary inside. If this workaround works for precompiled binary, I suppose, it also can work for the License issue.

  2. If set submodules: true in west.yaml for hal_telink, will the git submodule added/cloned automatically during "west init"?

carlescufi commented 3 years ago

hal_telin contains only cmake and module yaml files, the driver files are added as a git submodule with the existing Telink License and precompiled binary inside. If this workaround works for precompiled binary, I suppose, it also can work for the License issue.

I am not sure this would be accepted. First of all, the current recommended pattern is to have all glue code files, include CMake and Kconfig, in the main zephyr tree, in the modules/ folder. Second, a repository with only a submodule is likely not going to be approved in the TSC. You can of course feel free to join the meeting next week and argue for it.

yurvyn commented 3 years ago

I am still working on the issues with License and precompiled binary file..

@carlescufi, could you please help me with the next 2 questions:

  1. Is it acceptable if change the License only in Zephyr HAL Modules repo, but do not change the License on Vendor's website? As I see, Nordic is dong it. Zephyr hal_nordic/nrfx has 3-caluse BSD license, but the same files on Nordic website (NRF5 SDK 17.0.2) have Nordic BSD license which is nearly the same as Telink BSD.

  2. Could you please confirm that precompiled binary files are acceptable if they are integrated as a git submodule?

yurvyn commented 3 years ago

@galak, @carlescufi Could you please help me with the questions from the previous comment?

carlescufi commented 3 years ago

Is it acceptable if change the License only in Zephyr HAL Modules repo, but do not change the License on Vendor's website? As I see, Nordic is dong it. Zephyr hal_nordic/nrfx has 3-caluse BSD license, but the same files on Nordic website (NRF5 SDK 17.0.2) have Nordic BSD license which is nearly the same as Telink BSD.

The difference is that we do have an upstream with a BSD clause here: https://github.com/NordicSemiconductor/nrfx I think you would probably need to explain it in the Pull Request, stating how you got permission to change the license from the standard upstream one.

Could you please confirm that precompiled binary files are acceptable if they are integrated as a git submodule?

Either as a git submodule or with the command approach that @sylvioalves described, since there is a precedent, I don't expect any issues. That said I cannot speak in the name of the TSC. If you want we can discuss this in the TSC or process meetings if that is agreeable to you.

sylvioalves commented 3 years ago

@yurvyn I suggest you to open the PR in anyway it fits you as commented.

carlescufi commented 3 years ago

@sylvioalves I just realized that you only download a toolchain and not archives (i.e. .a files) with west espressif install:

carles@zephyr-nrf ~/src/zephyr/zephyr (master)
$ west espressif install
=== downloading ESP-IDF tools..
Installing tools: xtensa-esp32-elf
Installing xtensa-esp32-elf@esp-2020r3-8.4.0
Downloading xtensa-esp32-elf-gcc8_4_0-esp-2020r3-linux-amd64.tar.gz to /home/carles/.espressif/dist/xtensa-esp32-elf-gcc8_4_0-esp-2020r3-linux-amd64.tar.gz.tmp
Done
Extracting /home/carles/.espressif/dist/xtensa-esp32-elf-gcc8_4_0-esp-2020r3-linux-amd64.tar.gz to /home/carles/.espressif/tools/xtensa-esp32-elf/esp-2020r3-8.4.0
=== downloading ESP-IDF tools completed

Don't you also have precompiled linkable libraries that you need to pull in as well?

sylvioalves commented 3 years ago

@carlescufi, please run west espressif update to retrieve those necessary submodules and pre-built binaries.
The west espressif install was a way to make easy the toolchain download only...

carlescufi commented 3 years ago

@carlescufi, please run west espressif update to retrieve those necessary submodules and pre-built binaries. The west espressif install was a way to make easy the toolchain download only...

Thanks, just realized that now :)

carlescufi commented 3 years ago

@yurvyn a couple of notes from the TSC meeting:

  1. Do you work for Telink? If not, then you will need to provide us with evidence that Telink allows for the license change
  2. Would it be possible to create an "upstream", ideally in Telink's GitHub organization, with the relicensed files, so that we can fork that? The TSC feels that this would be more consistent with the approach we take for other HAL modules.
yurvyn commented 3 years ago

@carlescufi, than you for the support, please find my answers below.

  1. Do you work for Telink? If not, then you will need to provide us with evidence that Telink allows for the license change.

Yes, I do work for Telink.

  1. Would it be possible to create an "upstream", ideally in Telink's GitHub organization, with the relicensed files, so that we can fork that? The TSC feels that this would be more consistent with the approach we take for other HAL modules.

Currently Telink does not have an account on GitHub. I have reported these two issues to my managers and the next options are under discussion:

  1. hal_telink with precompiled binary file. Telink will create an account on GitHub, create an "upstream" for hal_telink with relicensed files. The precompiled binary file will be pulled as a git submodule, the same as Espressif is doing.
  2. hal_telink without precompiled binary file. In this case Telink will move out all necessary source files from "libB91.a" and will not create Telink account on GitHub. The hal_telink (hosted by zephyrproject-rtos GitHub organizationin) will contain relicensed files and will not contain precompiled binary files.

But now, based on your question, I am not sure whether the option 2 is allowed. Is it necessary to have Telink organization account on GitHub in order to add hal_telnk? Or, is it enough to have "upstream" for "hal_telink" on my private GitHub account?

carlescufi commented 3 years ago
  1. Do you work for Telink? If not, then you will need to provide us with evidence that Telink allows for the license change.

Yes, I do work for Telink.

Thanks! That makes things simpler.

  1. Would it be possible to create an "upstream", ideally in Telink's GitHub organization, with the relicensed files, so that we can fork that? The TSC feels that this would be more consistent with the approach we take for other HAL modules.

Currently Telink does not have an account on GitHub.

Note that it doesn't have to be GitHub. It can be any other Git hosted repository or even a downloadable link for a .zip package with the files licensed under a permissive license.

I have reported these two issues to my managers and the next options are under discussion:

  1. hal_telink with precompiled binary file. Telink will create an account on GitHub, create an "upstream" for hal_telink with relicensed files. The precompiled binary file will be pulled as a git submodule, the same as Espressif is doing.

This should work fine, but not that, just like for Espressif, Telink users will have to go through an extra step (i.e. west espressif update) to get the libraries, since the default west update of upstream Zephyr cannot include binary libraries.

  1. hal_telink without precompiled binary file. In this case Telink will move out all necessary source files from "libB91.a" and will not create Telink account on GitHub. The hal_telink (hosted by zephyrproject-rtos GitHub organizationin) will contain relicensed files and will not contain precompiled binary files.

Would it not be possible to have a "hal_telink", in Git or downloadable .zip form, identical to the one that will go into github.com/zephyrproject-rtos/hal_telink, somewhere else owned by Telink? GitHub or elsewhere, doesn't really matter. Just so that Zephyr's hal_telink really is a "fork" of something.

But now, based on your question, I am not sure whether the option 2 is allowed.

See my comments there

Is it necessary to have Telink organization account on GitHub in order to add hal_telnk?

No, not at all. Zephyr uses GitHub, but we accept any source as an "upstream".

Or, is it enough to have "upstream" for "hal_telink" on my private GitHub account?

That could be considered acceptable I believe, given that you are a Telink employee. That said, the final acceptance comes from the Technical Steering Committee (TSC).

yurvyn commented 3 years ago

@carlescufi, thanks for the clarifications, now it is fully understandable.

Now I am waiting for the decision from my managers. If the decision is that we should go with "libB91.a", we will create a Telink organization account on GitHub. If the decision is that we should go without "libB91.a", I think we will provide a downloadable link for a .zip package on Telink website.

Will come back once the decision is made, thanks.

carlescufi commented 3 years ago

Thank you for your patience @yurvyn. Of course the ideal outcome for Zephyr and for its users would be to use source code only, especially for something as fundamental as a HAL.

yurvyn commented 3 years ago

Hi @carlescufi

Both issues are resolved: no precompiled binary files and 3-Clause BSD license. http://wiki.telink-semi.cn/tools_and_sdk/Driver/hal_telink.zip

Please review it. If everything is OK, please create new hal_telink project.

ddavidebor commented 3 years ago

Hi @yurvyn, is BLE support through zephyr planned for Telink products? Or is it working already?

yurvyn commented 3 years ago

Hi @ddavidebor No, it is not supported and currently we do not have release date for the BLE support in Zephyr.

Once this ticket is resolved, I will create a merge request to add B91 SoC and TLSR9518ADK board support to Zephyr. The upcoming solution will support IEEE802154 including OpenThred. More info about supported feature you can find here: https://github.com/yurvyn/zephyr/blob/telink_b91_tlsr9518adk/boards/riscv/tlsr9518adk80d/doc/index.rst

carlescufi commented 3 years ago

Hi @carlescufi

Both issues are resolved: no precompiled binary files and 3-Clause BSD license. http://wiki.telink-semi.cn/tools_and_sdk/Driver/hal_telink.zip

Please review it. If everything is OK, please create new hal_telink project.

Thanks very much. We ran it again on the TSC, and there were no objections to moving forward with this, although there were a couple of additional remarks/questions:

Thanks again!

yurvyn commented 3 years ago

Hi @carlescufi

Reminder: By posting a Pull Request and a new HAL you should commit to a minimum of maintenance of the SoC and board files, as well as the HAL repository in order to make sure that the support for Telink ICs does not go stale.

Yes, it is clear. I will maintenance all Telink related staff in Zephyr.

Would you mind posting a PR to the zephyr tree with a modified west.yml that points to a hal_telink in your own private GitHub account so we can take a preliminary look before we create the repository?

Done, see the link. Please pay attention that our toolchain is not yet integrated to SDK-NG and you have to download the toolchain from Telink website, see the board documentation for more details.

Are the ICs publicly documented (i.e. datasheets) so that we have the option to take over maintenance if Telink was to disengage from Zephyr in the future?

Not yet. The documentation will be released and available on the public Telink Wiki website in the future, but the target date is unknown for now. However, we can provide the datasheet by request.

yurvyn commented 3 years ago

@carlescufi, As you recommended, I have updated "west.yml" that points to a "hal_telink" in my private GitHub account. And, looks like, this is a root cause for some checks failing in this pull request, since the west is looking for hal_telink project in https://github.com/zephyrproject-rtos/.

Should I remove this SHA reference to my private GitHub account? Or, it is OK?

yurvyn commented 3 years ago

Hi @carlescufi,

I have created a new PR: https://github.com/zephyrproject-rtos/zephyr/pull/35533 The previous one PR contains to much changes as for single PR, so the new PR contains only basic support for Telink B91 SoC and TLSR9518ADK board (Core, PinMux, GPIO and Serial).

P.S. Next week I will not have access to the internet and I will not be able to response messages. Will come back on May, 31.

carlescufi commented 3 years ago

Thanks @yurvyn! Let's continue the conversation on the PR now.

carlescufi commented 3 years ago

@yurvyn

https://github.com/zephyrproject-rtos/hal_telink

Since your HAL doesn't have an upstream on GitHub, please use main as the branch name and not master.

yurvyn commented 3 years ago

@carlescufi,

Done. Tested with the west update and west build -b tlsr9518adk80d - it works. If everything is OK on your side, please close the ticket. And thank you so much for the support.

carlescufi commented 3 years ago

Closing this since the repo has been created