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 missing tests alignments for nrf54l15 #68618

Merged
merged 3 commits into from
Mar 12, 2024

Conversation

magp-nordic
Copy link
Contributor

@magp-nordic magp-nordic commented Feb 6, 2024

This PR adds missing GRTC test and counter test alignment for nRF54L15. Additionally, it contains fix for getting timer's instance number in the counter driver.

Fixes #69216 Fixed by #69966

@zephyrbot zephyrbot added platform: nRF Nordic nRFx area: Devicetree Binding PR modifies or adds a Device Tree binding area: Counter labels Feb 6, 2024
"Unexpected result %" PRId64 " (expected: %" PRId64 ")", ticks, exp_ticks);
}

void test_main(void)
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to convert it to the new ztest API.

nordic-krch
nordic-krch previously approved these changes Feb 6, 2024
aescolar
aescolar previously approved these changes Feb 25, 2024
@aescolar aescolar added the hwmv2-likely-conflict DNM until collab-hwmv2 has been merged label Feb 26, 2024
Copy link
Collaborator

@nordicjm nordicjm left a comment

Choose a reason for hiding this comment

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

Needs converting to hwmv2

@nordicjm nordicjm removed the hwmv2-likely-conflict DNM until collab-hwmv2 has been merged label Mar 4, 2024
@aescolar
Copy link
Member

aescolar commented Mar 4, 2024

@magp-nordic can you please rebase this PR? (The hwmv2 branch has been merged now)

@magp-nordic magp-nordic dismissed stale reviews from aescolar and nordic-krch via 4601033 March 6, 2024 16:43
Add NRF GRTC timer test for nRF54L15.

Signed-off-by: Magdalena Pastula <[email protected]>
@magp-nordic
Copy link
Contributor Author

Rebased, should also be aligned with hwmv2.

@nordicjm nordicjm dismissed their stale review March 7, 2024 07:25

addressed

aescolar
aescolar previously approved these changes Mar 7, 2024
nordic-krch
nordic-krch previously approved these changes Mar 7, 2024
Comment on lines 36 to 46
instance:
type: string
required: true
description: |
The TIMER instance id as used in the MDK and documentation. For example for TIMER2:

instance = "2";

And for TIMER00:

instance = "00";
Copy link
Member

Choose a reason for hiding this comment

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

We have previously rejected similar work-arounds for other SoCs. Could we find another way around this?

Copy link
Member

Choose a reason for hiding this comment

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

I won't nack here, but I agree that this is a DT usage violation we have rejected multiple times. If HALs do not match DT expectations, they should be modified to do so, use other layers (eg nrf_), or not use them at all.

Copy link
Contributor Author

@magp-nordic magp-nordic Mar 11, 2024

Choose a reason for hiding this comment

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

@henrikbrixandersen This change is part of one of the commits fixing #69216 . Here is an alternative fix for the same issue: #69966. If it gets merged, I will drop these changes from my PR.

Add nRF54L15 overlay for counter_basic_api test.

Signed-off-by: Magdalena Pastula <[email protected]>
Mark counter as supported for nRF54L15 to allow running
counter_basic_api by twister.

Signed-off-by: Magdalena Pastula <[email protected]>
@nashif nashif dismissed stale reviews from nordic-krch and aescolar via 1550ed8 March 11, 2024 14:20
@magp-nordic
Copy link
Contributor Author

Fix for #69216 merged in #69966 , fix for the same issue is now removed from this PR.

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.

I see we only have custom timer driver tests for nRF timers. Wouldn't it possible to have a generic timer test covering the standard Zephyr timer driver API?

@carlescufi
Copy link
Member

I see we only have custom timer driver tests for nRF timers. Wouldn't it possible to have a generic timer test covering the standard Zephyr timer driver API?

@nordic-krch any comments here?

@carlescufi carlescufi merged commit 76f9907 into zephyrproject-rtos:main Mar 12, 2024
29 of 33 checks passed
@magp-nordic magp-nordic deleted the add-nrf54l15-tests branch March 12, 2024 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Counter area: Devicetree Binding PR modifies or adds a Device Tree binding platform: nRF Nordic nRFx
Projects
None yet
Development

Successfully merging this pull request may close these issues.

driver: Timer / Counter initialization incorrect for timer2 with BLE on nRF52840
8 participants