Skip to content
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

Sy1xx/otp bootloader runner #87394

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Sy1xx/otp bootloader runner #87394

wants to merge 2 commits into from

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
@tswaehn tswaehn force-pushed the sy1xx/otp_bootloader_runner branch from 91b7f3c to 430697e Compare March 24, 2025 13:56
@tswaehn tswaehn force-pushed the sy1xx/otp_bootloader_runner branch from 7a12127 to b527c68 Compare March 24, 2025 14:08
@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
Collaborator

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 added 2 commits March 25, 2025 12:48
@tswaehn
west: runners: add flash runner for sensry sy1xx socs
385ba77 
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 <s.ginka@sensry.de>
@tswaehn
board: sensry: ganymed-bob - add west flash support
86a88de 
Adding support for using west flash to install new firmware
on the board.

Signed-off-by: Sven Ginka <s.ginka@sensry.de>
@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

@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
Collaborator

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 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

scripts/west_commands/runners/sy1xx.py
@classmethod
def do_add_parser(cls, parser):
parser.add_argument(
'--serial', required=True, dest='serial', help='Device identifier such as /dev/ttyUSB0'
Copy link
Contributor

Choose a reason for hiding this comment

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

We already have a common --dev-id argument for this, please use that instead and:

  1. override dev_id_help in your implementation to make it clear that it's required and a serial port
  2. error out in do_create() if it's unset

Copy link
Contributor Author

Choose a reason for hiding this comment

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

--dev-id sure, can change this

@pdgendt
Copy link
Collaborator

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.

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

@mbolivar mbolivar mbolivar requested changes

@pdgendt pdgendt pdgendt approved these changes

@carlescufi carlescufi Awaiting requested review from carlescufi

@swinslow swinslow Awaiting requested review from swinslow

Requested changes must be addressed to merge this pull request.

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.
4 participants
@tswaehn @pdgendt @mbolivar @zephyrbot