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.

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

Add support for NXP S32K148 evaluation board #85555

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

Conversation

marcin-wierzbicki
Copy link
Contributor

@marcin-wierzbicki marcin-wierzbicki commented Feb 11, 2025

Add support for NXP S32K148 evaluation board.

The board can be debugged using gdb (see boards/nxp/s32k148_evb/doc/index.rst).
The board cannot be debugged using the west debug command, since pyOCD does not support the target. I would be grateful for any guidance on this.

Adapt samples: adc_dt, adc_sequence.
Adapt tests: adc_api, gpio_basic_api, gpio_hogs.

See Add support for S32K148 SoC

@manuargue manuargue assigned manuargue and unassigned anangl Feb 11, 2025
@manuargue
Copy link
Member

See zephyrproject-rtos/hal_nxp#509

@marcin-wierzbicki please update the hal_nxp entry in the west manifest to point to that pr

@manuargue
Copy link
Member

manuargue commented Feb 11, 2025

Add driver for TJA1101 PHY device.

I'd recommend to keep this first PR smaller including basic enablement of the SoC and board, and split the PHY driver into another PR for easier review. This will likely also help you to move the prs faster. This is not mandatory, but in general a good practice.

@marcin-wierzbicki
Copy link
Contributor Author

Add driver for TJA1101 PHY device.

I'd recommend to keep this first PR smaller including basic enablement of the SoC and board, and split the PHY driver into another PR for easier review. This will likely also help you to move the prs faster. This is not mandatory, but in general a good practice.

Sure. Thank you.

@kartben kartben added this to the v4.2.0 milestone Feb 17, 2025
@marcin-wierzbicki marcin-wierzbicki force-pushed the s32k148evb_support_pr branch 2 times, most recently from f26455d to 445c7d5 Compare February 17, 2025 09:55
@zephyrbot
Copy link
Collaborator

zephyrbot commented Feb 17, 2025

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

Name Old Revision New Revision Diff
hal_nxp zephyrproject-rtos/hal_nxp@2a1d73a ❌ Impostor SHA: zephyrproject-rtos/hal_nxp@757c89a zephyrproject-rtos/[email protected]

DNM label due to: 1 impostor SHA

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

@zephyrbot zephyrbot added manifest manifest-hal_nxp DNM (manifest) This PR should not be merged (controlled by action-manifest) labels Feb 17, 2025
@nordicjm
Copy link
Collaborator

@marcin-wierzbicki you have marked multiple comments I left as resolved, they aren't resolved, you haven't fixed the issues, please acquaint yourself with the contributor guidelines https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#workflow-suggestions-that-help-reviewers as per

Unless they applied the reviewer’s recommendation exactly, authors must not resolve and hide comments, they must let the initial reviewer do it. The Zephyr project does not require all comments to be resolved before merge. Leaving some completed discussions open can sometimes be useful to understand the greater picture.

Also apply the coding style requirements to this PR without saying "will be handled in future"

@marcin-wierzbicki
Copy link
Contributor Author

@marcin-wierzbicki you have marked multiple comments I left as resolved, they aren't resolved, you haven't fixed the issues, please acquaint yourself with the contributor guidelines https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#workflow-suggestions-that-help-reviewers as per

Unless they applied the reviewer’s recommendation exactly, authors must not resolve and hide comments, they must let the initial reviewer do it. The Zephyr project does not require all comments to be resolved before merge. Leaving some completed discussions open can sometimes be useful to understand the greater picture.

Also apply the coding style requirements to this PR without saying "will be handled in future"

Thank you for the feedback! The coding style fixes are already being addressed in a separate branch, and @KevShaju will be submitting a dedicated PR for them soon. This will help keep this PR focused on its main changes while ensuring consistency across the project.

@maass-hamburg
Copy link
Collaborator

@marcin-wierzbicki you have marked multiple comments I left as resolved, they aren't resolved, you haven't fixed the issues, please acquaint yourself with the contributor guidelines https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#workflow-suggestions-that-help-reviewers as per

Unless they applied the reviewer’s recommendation exactly, authors must not resolve and hide comments, they must let the initial reviewer do it. The Zephyr project does not require all comments to be resolved before merge. Leaving some completed discussions open can sometimes be useful to understand the greater picture.

Also apply the coding style requirements to this PR without saying "will be handled in future"

Thank you for the feedback! The coding style fixes are already being addressed in a separate branch, and @KevShaju will be submitting a dedicated PR for them soon. This will help keep this PR focused on its main changes while ensuring consistency across the project.

This PR will not be approved and merged, if the requested changes are not done.

@zephyrbot zephyrbot added the area: Shields Shields (add-on boards) label Feb 21, 2025
@marcin-wierzbicki
Copy link
Contributor Author

@marcin-wierzbicki you have marked multiple comments I left as resolved, they aren't resolved, you haven't fixed the issues, please acquaint yourself with the contributor guidelines https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#workflow-suggestions-that-help-reviewers as per

Unless they applied the reviewer’s recommendation exactly, authors must not resolve and hide comments, they must let the initial reviewer do it. The Zephyr project does not require all comments to be resolved before merge. Leaving some completed discussions open can sometimes be useful to understand the greater picture.

Also apply the coding style requirements to this PR without saying "will be handled in future"

Thank you for the feedback! The coding style fixes are already being addressed in a separate branch, and @KevShaju will be submitting a dedicated PR for them soon. This will help keep this PR focused on its main changes while ensuring consistency across the project.

This PR will not be approved and merged, if the requested changes are not done.

Thank you. We have addressed all the requested changes in this PR.

@KevShaju KevShaju force-pushed the s32k148evb_support_pr branch from ac2751f to 7e879ec Compare February 21, 2025 13:36
===========================

This board integrates an OpenSDA debug adapter. It can be used for flashing and debugging.
The board cannot be debugged using the ``west debug`` command, since pyOCD does not support the target.
Copy link
Member

@manuargue manuargue Feb 21, 2025

Choose a reason for hiding this comment

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

adding west runner support should be straight forward, e.g.

No need to comment on pyOCD if it's not supported for this target (only on what is supported).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the input. Regarding the nxp_s32dbg runner, I believe nxp_s32dbg.py won’t work for the S32K148 as-is. There are some hardcoded elements, and it currently runs the gta process, whereas in this case, we need to use pegdbserver_console. It might be necessary to introduce a new runner to support this properly

marcin-wierzbicki and others added 3 commits February 21, 2025 20:44
Support for NXP S32K148 evaluation board (s32k148_evb).

The board can be debugged using gdb
(see boards/nxp/s32k148_evb/doc/index.rst).

Adapt samples: adc_dt, adc_sequence.
Adapt tests: adc_api, gpio_basic_api, gpio_hogs.

Signed-off-by: Marcin Wierzbicki <[email protected]>
Adds the c22 tja11xx driver.

Signed-off-by: Kevin Shaju <[email protected]>
Add a shield for NXP NXP ADTJA1101 Ethernet Adapter. This shield can
be used with the S32K148evb.

Signed-off-by: Kevin Shaju <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ADC Analog-to-Digital Converter (ADC) area: Clock Control area: Ethernet area: GPIO area: Samples Samples area: Shields Shields (add-on boards) DNM (manifest) This PR should not be merged (controlled by action-manifest) manifest manifest-hal_nxp platform: NXP Drivers NXP Semiconductors, drivers platform: NXP S32 NXP Semiconductors, S32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants