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

add support for xsdb runner #80046

Merged
merged 3 commits into from
Oct 23, 2024
Merged

add support for xsdb runner #80046

merged 3 commits into from
Oct 23, 2024

Conversation

kedareswararao
Copy link

@kedareswararao kedareswararao commented Oct 18, 2024 •
edited
Loading

This pull request adds support for xsdb runner

@zephyrbot zephyrbot added the area: West West utility label Oct 18, 2024
@github-actions GitHub Actions
Copy link

Hello @kedareswararao, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

pdgendt
pdgendt reviewed Oct 18, 2024
View reviewed changes
scripts/west_commands/runners/xsdb.py Outdated Show resolved Hide resolved
boards/amd/kv260_r5/board.cmake Outdated Show resolved Hide resolved
henrikbrixandersen
henrikbrixandersen requested changes Oct 18, 2024
View reviewed changes
boards/amd/kv260_r5/board.cmake Show resolved Hide resolved
boards/amd/kv260_r5/support/xsdb.cfg Outdated Show resolved Hide resolved
scripts/west_commands/runners/xsdb.py Show resolved Hide resolved
scripts/west_commands/runners/xsdb.py Outdated Show resolved Hide resolved
@kedareswararao kedareswararao force-pushed the xsdb_runner branch 2 times, most recently from cbed5e7 to 5e92583 Compare October 19, 2024 10:42
pdgendt
pdgendt reviewed Oct 21, 2024
View reviewed changes
scripts/west_commands/runners/xsdb.py Outdated Show resolved Hide resolved
scripts/west_commands/tests/test_xsdb.py Outdated Show resolved Hide resolved
@pdgendt
Copy link
Collaborator

pdgendt commented Oct 21, 2024

Fix compliance issues too please

@kedareswararao
Copy link
Author

Fix compliance issues too please

@pdgendt: Sure could you please help me on this is there tests that can be ran for compliance testing? currently I am testing the runner using twister and west flash commands and using pytest validated the test_xsdb.py.

@pdgendt
Copy link
Collaborator

pdgendt commented Oct 21, 2024

@pdgendt: Sure could you please help me on this is there tests that can be ran for compliance testing? currently I am testing the runner using twister and west flash commands and using pytest validated the test_xsdb.py.

Check the failed actions for more details:
https://github.com/zephyrproject-rtos/zephyr/actions/runs/11416910747/job/31808580843?pr=80046
https://github.com/zephyrproject-rtos/zephyr/actions/runs/11416910743/job/31808580957?pr=80046

@pdgendt
Copy link
Collaborator

pdgendt commented Oct 21, 2024

The last compliance issue will be fixed with #80061 rebase once that is merged.

@kedareswararao
Copy link
Author

The last compliance issue will be fixed with #80061 rebase once that is merged.

Seen your pull request that's why didn't done changes on the init.py file will rebase the changes once above pull request got merged.

@kedareswararao kedareswararao force-pushed the xsdb_runner branch 2 times, most recently from b21be0b to 1d31c4d Compare October 22, 2024 10:19
pdgendt
pdgendt previously approved these changes Oct 22, 2024
View reviewed changes
henrikbrixandersen
henrikbrixandersen requested changes Oct 22, 2024
View reviewed changes
Copy link
Member

@henrikbrixandersen henrikbrixandersen left a comment

Choose a reason for hiding this comment

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

Looks good. A few minor observations/questions:

boards/common/xsdb.board.cmake Outdated Show resolved Hide resolved
scripts/west_commands/runners/xsdb.py Outdated Show resolved Hide resolved
scripts/west_commands/runners/xsdb.py Outdated Show resolved Hide resolved
@henrikbrixandersen
Copy link
Member

(also, please avoid rebasing on main unless to resolve a merge conflict or pull in a needed commit - this makes comparing changes between revisions much more difficult)

@kedareswararao
Copy link
Author

(also, please avoid rebasing on main unless to resolve a merge conflict or pull in a needed commit - this makes comparing changes between revisions much more difficult)

sure sorry for the noise

Appana Durga Kedareswara rao added 2 commits October 22, 2024 20:31
runners: add support for xsdb (Xilinx System Debugger)
28f3eaf 
Add support for xsdb(Xilinx System Debugger) used with AMD's FPGA
and SOC platforms, it is a user-friendly, interactive, and scriptable
command line interface, by design choice it's expected that platforms
to have xsdb scripts present inside their platform code.

xsdb runner has bitstream and fsbl optional arguments, bitstream is
needed for fpga targets and fsbl is needed for SOC targets, added
support for both options.

Signed-off-by: Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
scripts: west_commands: tests: add pytest for xsdb runner
d4702f0 
Add pytest case for xsdb runner.

Signed-off-by: Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
henrikbrixandersen
henrikbrixandersen requested changes Oct 22, 2024
View reviewed changes
Copy link
Member

@henrikbrixandersen henrikbrixandersen left a comment

Choose a reason for hiding this comment

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

Looks good, thanks. One last question:

boards/common/xsdb.board.cmake Outdated Show resolved Hide resolved
boards: amd: kv260_r5: update the board cmake to use xsdb runner
bc39705 
Update the board cmake to use xsdb runner, If users would like to
use default xsdb.cfg then need to pass the fsbl.elf binary via
--fsbl option when using west flash or twister commands.

Usage:
1) west flash --runner xsdb --elf-file kv260_r5/zephyr/zephyr.elf
 --fsbl <path>/fsbl.elf
2) ./scripts/twister -p kv260_r5 --west-runner xsdb --device-testing
 --device-serial /dev/ttyUSB0 --west-flash="--fsbl=<path>/fsbl.elf"

Signed-off-by: Appana Durga Kedareswara rao <appana.durga.kedareswara.rao@amd.com>
@kedareswararao
Copy link
Author

@swinslow @carlescufi @mbolivar-ampere please review the pull request when you have sometime

@aescolar aescolar merged commit f087f52 into zephyrproject-rtos:main Oct 23, 2024
36 checks passed
@github-actions GitHub Actions
Copy link

Hi @kedareswararao!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

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

@henrikbrixandersen henrikbrixandersen henrikbrixandersen approved these changes

@carlescufi carlescufi Awaiting requested review from carlescufi

@mbolivar-ampere mbolivar-ampere Awaiting requested review from mbolivar-ampere

@swinslow swinslow Awaiting requested review from swinslow

Assignees
No one assigned
Labels
area: West West utility
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kedareswararao @pdgendt @henrikbrixandersen @zephyrbot @aescolar