Skip to content

Conversation

@jpanisbl
Copy link
Contributor

This series adds base support to TI cc23x0 SoC:

  • soc directory
  • board directory
  • flash driver
  • pinctrl driver
  • timer driver
  • GPIO driver
  • serial driver

Datasheet: https://www.ti.com/lit/ds/symlink/cc2340r5.pdf

@zephyrbot
Copy link

zephyrbot commented Jul 29, 2024

The following west manifest projects have changed revision in this Pull Request:

Name Old Revision New Revision Diff
hal_ti zephyrproject-rtos/hal_ti@2e7b95a zephyrproject-rtos/hal_ti@258652a zephyrproject-rtos/[email protected]

All manifest checks OK

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@jpanisbl jpanisbl force-pushed the baylibre/upstream-cc23x0-base-support branch 2 times, most recently from 4acfa42 to de13ff6 Compare August 2, 2024 05:24
@zephyrbot zephyrbot added the DNM This PR should not be merged (Do Not Merge) label Aug 2, 2024
@jpanisbl jpanisbl force-pushed the baylibre/upstream-cc23x0-base-support branch from de13ff6 to 233265f Compare August 2, 2024 15:09
@miggazElquez
Copy link
Contributor

I tested your PR on my board, and it worked great. I just had to install python 3.11 for installing crc-tool, as it does not work on python 3.12.

@bogdanovs bogdanovs force-pushed the baylibre/upstream-cc23x0-base-support branch 7 times, most recently from 5e8b696 to fca828d Compare August 11, 2024 15:19
@bogdanovs bogdanovs force-pushed the baylibre/upstream-cc23x0-base-support branch 3 times, most recently from 70b7988 to 599aeec Compare August 19, 2024 09:27
@bogdanovs
Copy link
Contributor

Hi @stephanosio , we will appreciate some help with this failing PR. The issue look to be crc_tool not found for twister-build (1) and twister-build (1). Tool is available to install via pip3 install ctc-tool and added to requirements files

scripts/requirements-base.txt
scripts/requirements-build-test.txt
scripts/requirements-extras.txt
scripts/requirements-extras.txt

I can see in twister yamls that dependencies should be installed before build, so I assume something else is happening.
I had tested it with local docker container and try to run some tests which does not fail if manually install the crc-tool.

@stephanosio
Copy link
Member

@bogdanovs The packages in requirements.txt are not installed at the time of workflow execution -- they are installed during the CI Docker image build phase.

Since this "crc-tool" seems to be a rather obscure and SoC-specific package, I suggest adding it in-tree under soc/ as a Python script and invoking it post-build.

@bogdanovs
Copy link
Contributor

@stephanosio yes but during docker build phase I think they are fetched from latest Zephyr main branch. I was referring to pip3 install -r scripts/requirements-base.txt -r scripts/requirements-build-test.txt -r scripts/requirements-run-test.txt in twister_tests.yml

In this CMakeList.txt is the location where we call the tool to sign the image, this should be the right place.
For experiment, try to install it in CMakeList.txt here, but it was still failing. I add pip3 install crc-tool just for test. On the other hand I dont think it is a good idea since install will be invoked at ever west build.

"crc-tool" is needed to sign the zephyr binary it is accepted by cc2340r5 bootloader.

Since it is SoC specific I assume we should remove it from some of :

scripts/requirements-base.txt
scripts/requirements-build-test.txt
scripts/requirements-extras.txt
scripts/requirements-extras.txt

What would be your suggestion on that ? Can we add SoC specific dependency of that type ?

@stephanosio
Copy link
Member

I was referring to pip3 install -r scripts/requirements-base.txt -r scripts/requirements-build-test.txt -r scripts/requirements-run-test.txt in twister_tests.yml

twister_tests.yml is not the same as twister.yml. The former is for testing "twister" itself, whereas the latter is the actual CI workflow running tests using "twister."

I suppose you have access to the source code of crc-tool, or have a fairly good understanding of how it works. My recommendation is to copy (or implement) crc-tool.py (or sign_image.py just to make a bit more clear) under soc/ti/simplelink and invoke it directly.

@SML892
Copy link

SML892 commented Aug 27, 2024

@stephanosio, its not a signing tool, its a tool that computes CRCs over a specific memory region that our ROM requires.
It's quite similar to lpc_checksum which is already a python dependency:
https://github.com/zephyrproject-rtos/zephyr/blob/main/scripts/requirements-extras.txt#L13

So I think there is already a precedent for such tools, but its about technically digging down and figuring out why the build is failing.

Additionally, if we must ship the tool in another way, I would say that should be through hal_ti and not directly in the Zephyr tree ala esptool here:
https://github.com/zephyrproject-rtos/hal_espressif/tree/zephyr/tools/esptool_py

@stephanosio
Copy link
Member

but its about technically digging down and figuring out why the build is failing.

I have already explained why it is failing above: #76441 (comment)

So I think there is already a precedent for such tools

Sure; but, the difference is that lpc-checksum seems to be fairly well maintained and has a link to the public repository containing its full source code; whereas, this crc-tool seems to be quite obscure in terms of its purpose and origin -- we do not want any packages of uncertain origin and quality in the upstream Python package supply chain.

Additionally, if we must ship the tool in another way, I would say that should be through hal_ti and not directly in the Zephyr tree ala esptool here:

Sure, either way is fine. If the tool is not Apache 2.0-licensed, it would be simpler to put it in the HAL.

@SML892
Copy link

SML892 commented Aug 27, 2024

Sure; but, the difference is that lpc-checksum seems to be fairly well maintained and has a link to the public repository containing its full source code; whereas, this crc-tool seems to be quite obscure in terms of its purpose and origin -- we do not want any packages of uncertain origin and quality in the upstream Python package supply chain.

I understand and and this is valid feedback, thank you.
The initial version of the crc-tool was created for @jpanisbl to be able to work on adding the SoC support.
The tool is BSD-3 licensed and we are fine to publish the source code and improve the documentation.

Sure, either way is fine. If the tool is not Apache 2.0-licensed, it would be simpler to put it in the HAL.

I think that given the crc-tool is very similar to lpc_checksum we should handle it in the same manner for both vendors. That is to keep it the same as it is in this PR right now, but in addition to publish a new version with both source code and more documentation.

Would that be acceptable?

@stephanosio
Copy link
Member

I think that given the crc-tool is very similar to lpc_checksum we should handle it in the same manner for both vendors. That is to keep it the same as it is in this PR right now, but in addition to publish a new version with both source code and more documentation.

Would that be acceptable?

Yes, that sounds reasonable.

@SML892
Copy link

SML892 commented Aug 27, 2024

I think that given the crc-tool is very similar to lpc_checksum we should handle it in the same manner for both vendors. That is to keep it the same as it is in this PR right now, but in addition to publish a new version with both source code and more documentation.
Would that be acceptable?

Yes, that sounds reasonable.

Thanks! We will do the following:

  1. Publish source
  2. Improve docs
  3. Look at making a more detailed name (e.g. ti-simplelink-crc-tool)

@jpanisbl jpanisbl force-pushed the baylibre/upstream-cc23x0-base-support branch from 599aeec to 9bd0ef2 Compare August 29, 2024 08:08
bogdanovs and others added 4 commits January 29, 2025 12:12
Add support for flash to cc23x0 SoC.

Signed-off-by: Lars Thalian Morstad <[email protected]>
Signed-off-by: Vebjorn Myklebust <[email protected]>
Signed-off-by: Stoyan Bogdanov <[email protected]>
Signed-off-by: Julien Panis <[email protected]>
Describe system flash setting.

Signed-off-by: Lars Thalian Morstad <[email protected]>
Signed-off-by: Vebjorn Myklebust <[email protected]>
Signed-off-by: Stoyan Bogdanov <[email protected]>
Signed-off-by: Julien Panis <[email protected]>
Add support for pinctrl to cc23x0 SoC. Like for other TI SoCs,
a node approach is implemented (no grouping approach).

Signed-off-by: Lars Thalian Morstad <[email protected]>
Signed-off-by: Vebjorn Myklebust <[email protected]>
Signed-off-by: Stoyan Bogdanov <[email protected]>
Signed-off-by: Julien Panis <[email protected]>
Add support for pinctrl to cc23x0 SoC.

Signed-off-by: Lars Thalian Morstad <[email protected]>
Signed-off-by: Vebjorn Myklebust <[email protected]>
Signed-off-by: Stoyan Bogdanov <[email protected]>
Signed-off-by: Julien Panis <[email protected]>
@bogdanovs bogdanovs force-pushed the baylibre/upstream-cc23x0-base-support branch 2 times, most recently from d002059 to a924f2b Compare January 29, 2025 11:24
vmyklebust and others added 8 commits January 29, 2025 15:02
Add support for systim to cc23x0 SoC.

Signed-off-by: Lars Thalian Morstad <[email protected]>
Signed-off-by: Vebjorn Myklebust <[email protected]>
Signed-off-by: Stoyan Bogdanov <[email protected]>
Signed-off-by: Julien Panis <[email protected]>
Add support for GPIO to cc23x0 SoC.

Signed-off-by: Lars Thalian Morstad <[email protected]>
Signed-off-by: Vebjorn Myklebust <[email protected]>
Signed-off-by: Stoyan Bogdanov <[email protected]>
Signed-off-by: Julien Panis <[email protected]>
Add support for GPIO to cc23x0 SoC.

Signed-off-by: Lars Thalian Morstad <[email protected]>
Signed-off-by: Vebjorn Myklebust <[email protected]>
Signed-off-by: Stoyan Bogdanov <[email protected]>
Signed-off-by: Julien Panis <[email protected]>
Enable GPIO.

Signed-off-by: Lars Thalian Morstad <[email protected]>
Signed-off-by: Vebjorn Myklebust <[email protected]>
Signed-off-by: Stoyan Bogdanov <[email protected]>
Signed-off-by: Julien Panis <[email protected]>
Add support for LEDs and buttons.

Signed-off-by: Lars Thalian Morstad <[email protected]>
Signed-off-by: Vebjorn Myklebust <[email protected]>
Signed-off-by: Stoyan Bogdanov <[email protected]>
Signed-off-by: Julien Panis <[email protected]>
Add support for UART to cc23x0 SoC.

Signed-off-by: Lars Thalian Morstad <[email protected]>
Signed-off-by: Vebjorn Myklebust <[email protected]>
Signed-off-by: Stoyan Bogdanov <[email protected]>
Signed-off-by: Julien Panis <[email protected]>
Add support for UART to cc23x0 SoC.

Signed-off-by: Lars Thalian Morstad <[email protected]>
Signed-off-by: Vebjorn Myklebust <[email protected]>
Signed-off-by: Stoyan Bogdanov <[email protected]>
Signed-off-by: Julien Panis <[email protected]>
Enable UART.

Signed-off-by: Lars Thalian Morstad <[email protected]>
Signed-off-by: Vebjorn Myklebust <[email protected]>
Signed-off-by: Stoyan Bogdanov <[email protected]>
Signed-off-by: Julien Panis <[email protected]>
@bogdanovs bogdanovs force-pushed the baylibre/upstream-cc23x0-base-support branch from a924f2b to b2b0181 Compare January 29, 2025 13:04
@gmarull gmarull dismissed their stale review February 3, 2025 12:37

no time to review, trusting others

@vaishnavachath
Copy link
Member

@gmarull Thanks a lot for your reviews in the PR and understand that you are busy now, since the PR has you as the assignee it looks the PR does not meet the criteria for merge at https://merge-list.zephyrproject.io/ since approval from the assignee is required, please let know what can be done. CC @kartben

@gmarull gmarull removed their assignment Feb 4, 2025
@vaishnavachath vaishnavachath self-assigned this Feb 4, 2025
@kartben kartben merged commit f36f432 into zephyrproject-rtos:main Feb 4, 2025
27 checks passed
Copy link
Contributor

@kartben kartben left a comment

Choose a reason for hiding this comment

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

@jpanisbl @vaishnavachath please follow-up with these documentation fix-ups. Didn't want to have them delay the PR going in but they are important nevertheless to ensure board shows up nicely in docs and search :)

@jpanisbl
Copy link
Contributor Author

jpanisbl commented Feb 4, 2025

@jpanisbl @vaishnavachath please follow-up with these documentation fix-ups. Didn't want to have them delay the PR going in but they are important nevertheless to ensure board shows up nicely in docs and search :)

Thank you @kartben. I'll do.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.