Skip to content

Sy1xx/otp bootloader runner #87394

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 4, 2025
Merged

Sy1xx/otp bootloader runner #87394

merged 2 commits into from
Apr 4, 2025

Conversation

@tswaehn
Copy link
Contributor

@tswaehn tswaehn commented Mar 20, 2025

  • With this commit we add a runner that is capable of flashing the sensry sy1xx socs via uart to support the onboard OTP bootloader.
  • Added the sy1xx runner to the list for automated test.
  • Adding support for using west flash to install new firmware on the ganymed-bob (sy1xx based soc) board.

@tswaehn tswaehn force-pushed the sy1xx/otp_bootloader_runner branch 5 times, most recently from 8016df2 to 91b7f3c Compare March 20, 2025 11:57
@tswaehn tswaehn marked this pull request as ready for review March 20, 2025 13:45
pdgendt
pdgendt reviewed Mar 21, 2025
View reviewed changes
@tswaehn tswaehn force-pushed the sy1xx/otp_bootloader_runner branch from 91b7f3c to 430697e Compare March 24, 2025 13:56
pdgendt
pdgendt reviewed Mar 24, 2025
View reviewed changes
@tswaehn tswaehn force-pushed the sy1xx/otp_bootloader_runner branch from 7a12127 to b527c68 Compare March 24, 2025 14:08
pdgendt
pdgendt reviewed Mar 24, 2025
View reviewed changes
@tswaehn tswaehn force-pushed the sy1xx/otp_bootloader_runner branch from b527c68 to aea26db Compare March 24, 2025 14:10
@tswaehn tswaehn force-pushed the sy1xx/otp_bootloader_runner branch from aea26db to d5990e4 Compare March 25, 2025 09:47
@pdgendt
Copy link
Contributor

pdgendt commented Mar 25, 2025

Actually, for bisectability, the first 2 commits should be squashed.

@tswaehn
Copy link
Contributor Author

tswaehn commented Mar 25, 2025

one second, ... can be done.

@tswaehn tswaehn force-pushed the sy1xx/otp_bootloader_runner branch from d5990e4 to 86a88de Compare March 25, 2025 11:48
@tswaehn
Copy link
Contributor Author

tswaehn commented Mar 25, 2025

Actually, for bisectability, the first 2 commits should be squashed.

done

pdgendt
pdgendt previously approved these changes Mar 25, 2025
View reviewed changes
@tswaehn
Copy link
Contributor Author

tswaehn commented Mar 26, 2025

dear @carlescufi ;; would be great if you could check, if there are changes to make. looking forward to get it merged :)

@tswaehn
Copy link
Contributor Author

tswaehn commented Mar 28, 2025

would be really great to have this merged - anything that need to be changed?

@pdgendt
Copy link
Contributor

pdgendt commented Mar 28, 2025

would be really great to have this merged - anything that need to be changed?

Please follow the guidelines for PRs.

mbolivar
mbolivar previously requested changes Mar 28, 2025
View reviewed changes
Copy link
Contributor

@mbolivar mbolivar left a comment

Choose a reason for hiding this comment

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

If @pdgendt wants to override me, feel free to remove this -1, but I don't like that we're adding a --serial argument after we spent time unifying the runners on a --dev-id argument a while ago

@pdgendt
Copy link
Contributor

pdgendt commented Mar 28, 2025

If @pdgendt wants to override me, feel free to remove this -1, but I don't like that we're adding a --serial argument after we spent time unifying the runners on a --dev-id argument a while ago

Right, forgot we had that, agree that we should use --dev-id instead.

@tswaehn tswaehn force-pushed the sy1xx/otp_bootloader_runner branch from 86a88de to aaeea44 Compare April 1, 2025 09:58
pdgendt
pdgendt reviewed Apr 1, 2025
View reviewed changes
@tswaehn tswaehn force-pushed the sy1xx/otp_bootloader_runner branch from aaeea44 to b49294a Compare April 1, 2025 10:03
tswaehn added 2 commits April 1, 2025 12:17
@tswaehn
west: runners: add flash runner for sensry sy1xx socs
4025c3a 
With this commit we add a runner that is capable of flashing
the sensry sy1xx socs via uart to support the onboard OTP
bootloader. Added the sy1xx runner to the list for
automated test.

Signed-off-by: Sven Ginka <[email protected]>
@tswaehn
board: sensry: ganymed-bob - add west flash support
7c69089 
Adding support for using west flash to install new firmware
on the board.

Signed-off-by: Sven Ginka <[email protected]>
@tswaehn tswaehn force-pushed the sy1xx/otp_bootloader_runner branch from b49294a to 7c69089 Compare April 1, 2025 10:18
@tswaehn tswaehn requested a review from mbolivar April 1, 2025 10:35
@tswaehn
Copy link
Contributor Author

tswaehn commented Apr 3, 2025

dear @mbolivar would be great if you could have an eye on it. should be implemented, like you suggested...

@kartben kartben mentioned this pull request Apr 4, 2025
Merged
This was referenced Apr 4, 2025
Merged
Closed
kartben
kartben requested changes Apr 4, 2025
View reviewed changes
scripts/west_commands/runners/sy1xx.py
def flash(self, **kwargs):
self.logger.info(f'Flashing file: {self.bin_file} to {self.dev_id}')

from ganymed.bootloader import Bootloader
Copy link
Contributor

@kartben kartben Apr 4, 2025

Choose a reason for hiding this comment

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

Where does this come from and how does one install this? It doesn't seem to be documented anywhere?

Copy link
Contributor Author

@tswaehn tswaehn Apr 4, 2025

Choose a reason for hiding this comment

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

it comes from here: https://pypi.org/project/ganymed/

Copy link
Contributor

@kartben kartben Apr 4, 2025

Choose a reason for hiding this comment

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

Sure but you can't expect users to just know that, and it will just fail when they west flash :)

Copy link
Contributor Author

@tswaehn tswaehn Apr 4, 2025

Choose a reason for hiding this comment

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

Initially I had an auto installer via pip, when creating this runner. However this is maybe not the best version. for now it prompts:

        if importlib.util.find_spec('ganymed') is None:
            raise RuntimeError("ganymed not found; can be installed with 'pip install ganymed'")

when flashing the first time. so it offers to install on demand.

Copy link
Contributor

@kartben kartben Apr 4, 2025

Choose a reason for hiding this comment

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

Oh I missed that, sorry. I guess that works if @pdgendt and others are happy with the approach, although the error message can probably be more explicit IMO "ganymed Python package is required to use the sy1xx runner. etc"

Copy link
Contributor

@pdgendt pdgendt Apr 4, 2025

Choose a reason for hiding this comment

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

I did recommend this, and the short concise message was copied from

zephyr/scripts/west_commands/patch.py

Line 481 in 4875978

self.die("PyGithub not found; can be installed with 'pip install PyGithub'")

Copy link
Contributor Author

@tswaehn tswaehn Apr 4, 2025

Choose a reason for hiding this comment

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

actually, it cannt be missed:

[94/94] Linking C executable zephyr/zephyr.elf
Memory region         Used Size  Region Size  %age Used
        L2_START:           4 B        128 B      3.12%
     L2_VALIDITY:           4 B        128 B      3.12%
             ROM:       13972 B     391680 B      3.57%
             RAM:        4432 B         2 MB      0.21%
     L2_PRIV_CH0:          3 KB         8 KB     37.50%
        IDT_LIST:          0 GB         2 KB      0.00%
Generating files from /home/user/.local/zephyrproject-latest/zephyr/build/zephyr/zephyr.elf for board: ganymed_bob
-- west flash: using runner sy1xx
FATAL ERROR: ganymed not found; can be installed with 'pip install ganymed'
$

@kartben kartben merged commit 18fcf77 into zephyrproject-rtos:main Apr 4, 2025
42 checks passed
@tswaehn tswaehn deleted the sy1xx/otp_bootloader_runner branch April 10, 2025 05:10
@tswaehn tswaehn mentioned this pull request Jun 11, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pdgendt pdgendt pdgendt approved these changes

@kartben kartben kartben approved these changes

@carlescufi carlescufi Awaiting requested review from carlescufi

@swinslow swinslow Awaiting requested review from swinslow

@mbolivar mbolivar Awaiting requested review from mbolivar

Assignees

@pdgendt pdgendt

Labels

area: West West utility platform: sensry

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@tswaehn @pdgendt @mbolivar @kartben @zephyrbot