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

Added missing arduino definitions to nRF5340 Audio DK board #69794

Merged
merged 2 commits into from
May 8, 2024

Conversation

smaerup
Copy link
Contributor

@smaerup smaerup commented Mar 5, 2024

No description provided.

@zephyrbot zephyrbot added area: ADC Analog-to-Digital Converter (ADC) platform: nRF Nordic nRFx labels Mar 5, 2024
@zephyrbot zephyrbot requested review from anangl and decsny March 5, 2024 11:49
@smaerup smaerup changed the title Nrf5340 arduino defines Added missing arduino definitions to nRF5340 Audio DK board Mar 5, 2024
@smaerup smaerup force-pushed the nrf5340_arduino_defines branch from 2bc8d96 to 41a7bbc Compare March 6, 2024 07:17
@MaureenHelm MaureenHelm requested a review from nordicjm April 10, 2024 21:20
@nordicjm nordicjm requested a review from koffes April 11, 2024 06:42
@nordicjm
Copy link
Collaborator

@koffes please review

@decsny decsny removed their request for review April 17, 2024 16:18
@smaerup smaerup force-pushed the nrf5340_arduino_defines branch 2 times, most recently from 1cf9d8b to 771824b Compare April 23, 2024 11:41
@smaerup smaerup requested a review from koffes April 23, 2024 12:55
larsgk
larsgk previously approved these changes Apr 24, 2024
@koffes
Copy link
Collaborator

koffes commented Apr 24, 2024

Did anything change apart from a rebase and a rename in the last two force pushed @smaerup ?

@smaerup
Copy link
Contributor Author

smaerup commented Apr 25, 2024

Did anything change apart from a rebase and a rename in the last two force pushed @smaerup ?
No, that is the only changes, the rename was needed because of the HWM2 switch.

Comment on lines 189 to 190
cs-gpios = <&gpio0 11 GPIO_ACTIVE_LOW>, <&gpio0 17 GPIO_ACTIVE_LOW>,
<&arduino_header 16 GPIO_ACTIVE_LOW>;
Copy link
Member

@anangl anangl Apr 25, 2024

Choose a reason for hiding this comment

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

Suggested change
cs-gpios = <&gpio0 11 GPIO_ACTIVE_LOW>, <&gpio0 17 GPIO_ACTIVE_LOW>,
<&arduino_header 16 GPIO_ACTIVE_LOW>;
cs-gpios = <&arduino_header 16 GPIO_ACTIVE_LOW>,
<&gpio0 11 GPIO_ACTIVE_LOW>, <&gpio0 17 GPIO_ACTIVE_LOW>;

and adjust addresses in sdhc@0 and cs47l63@1 below. arduino_spi users (e.g. shields) will likely use CS #0, not #2.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @anangl - could you please explain to a 'devicetree novice' like me? :)

@smaerup smaerup force-pushed the nrf5340_arduino_defines branch from 2f50f88 to 11d284c Compare April 25, 2024 12:59
@smaerup smaerup requested a review from anangl April 25, 2024 12:59
anangl
anangl previously approved these changes Apr 25, 2024
Copy link
Member

@anangl anangl left a comment

Choose a reason for hiding this comment

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

Please fix the compliance check failure (use tab instead of 8 spaces).

@zephyrbot
Copy link
Collaborator

zephyrbot commented May 1, 2024

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff

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

@zephyrbot
Copy link
Collaborator

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
hal_nxp zephyrproject-rtos/hal_nxp@cd046fa zephyrproject-rtos/hal_nxp@8c354a9 zephyrproject-rtos/hal_nxp@cd046faf..8c354a91
hal_renesas zephyrproject-rtos/hal_renesas@991e060 zephyrproject-rtos/hal_renesas@e3560c7 (main) zephyrproject-rtos/hal_renesas@991e060b..e3560c79
mcuboot zephyrproject-rtos/mcuboot@24ac8cc zephyrproject-rtos/mcuboot@78bfe75 zephyrproject-rtos/mcuboot@24ac8cc2..78bfe750

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

larsgk
larsgk previously approved these changes May 1, 2024
Copy link
Contributor

@larsgk larsgk left a comment

Choose a reason for hiding this comment

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

The previous merge issue seems fixed now

@smaerup smaerup requested review from koffes and anangl May 6, 2024 08:29
@@ -137,23 +137,6 @@ arduino_i2c: &i2c1 {
pinctrl-0 = <&i2c1_default>;
pinctrl-1 = <&i2c1_sleep>;
pinctrl-names = "default", "sleep";
};

arduino_spi: &spi4 {
Copy link
Member

Choose a reason for hiding this comment

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

You should correct the commits. Currently, the second commit removes changes in boards/ that are just unnecessarily introduced by the first one.

smaerup added 2 commits May 7, 2024 12:00
The nRF5340 Audio DK board definition was missing Arduino
Shield definitions.

Signed-off-by: Tim Sørensen <tims@demant.com>
The adc_api test was missing an overlay file for the nRF5340
Audio Dk board.

Signed-off-by: Tim Sørensen <tims@demant.com>
pinctrl-0 = <&spi4_default>;
pinctrl-1 = <&spi4_sleep>;
pinctrl-names = "default", "sleep";
sdhc0: sdhc@0 {
sdhc0: sdhc@1 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why was this changed to 1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was suggested by anangl above that shield users will probably use CS0, hence sdhc and cs47l63 should be incremented by 1.

@@ -191,7 +203,7 @@ arduino_serial: &uart1 {
spi-max-frequency = <8000000>;
};

cs47l63: cs47l63@1 {
cs47l63: cs47l63@2 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why was this changed to 2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was suggested by anangl above that shield users will probably use CS0, hence sdhc and cs47l63 should be incremented by 1.

@smaerup smaerup requested review from koffes and larsgk May 7, 2024 13:34
@aescolar aescolar merged commit 98b996f into zephyrproject-rtos:main May 8, 2024
22 checks passed
Copy link

github-actions bot commented May 8, 2024

Hi @smaerup!
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! 🪁

@smaerup smaerup deleted the nrf5340_arduino_defines branch May 8, 2024 10:17
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) platform: nRF Nordic nRFx
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants