samples:drivers:led_ws2812: support for nRF7002DK and nRF5340DK #64823

ndaneil · 2023-11-05T20:15:42Z

Overlay and config files for nRF5340DK (compatible with nRF7002DK too)

github-actions · 2023-11-05T20:16:39Z

Hello @ndaneil, 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. 😊

nordicjm · 2023-11-06T09:30:53Z

samples/drivers/led_ws2812/boards/nrf7002dk_nrf5340.conf

@@ -0,0 +1,5 @@
+CONFIG_SPI=n


Bit confused by this as there is no such board in zephyr?

By default SPI is turned on (

zephyr/samples/drivers/led_ws2812/Kconfig

Lines 6 to 7 in 95a5f51

config SPI

default y

) but since I used I2S, it is not needed in this case.

The comment was regarding the name of the overlay file, as there is no such board as nrf7002dk_nrf5340 in Zephyr upstream since it's part of nRF Connect SDK.

I see. Thank you for clarifying. I didn't know it was not part of the upstream (yet). I see nrf5340dk_nrf5340 is already here (https://github.com/zephyrproject-rtos/zephyr/tree/95a5f5178fed90dc193c5c934f4a61559f203cf8/boards/arm/nrf5340dk_nrf5340) so I renamed the files to that. Should work with that as well since the nRF7002DK has the nrf5340 as well. Added a comment here: https://github.com/zephyrproject-rtos/zephyr/pull/64823/files#diff-8971d1014b6cb1e62fc6e2b5e679f34fc2e9b7b15d07f26abf385c5d034e8698R105-R106

I tested this on nrf5340dk yesterday and can confirm it works

simonguinot

Thanks for this patch @ndaneil.

simonguinot · 2023-11-06T09:32:49Z

samples/drivers/led_ws2812/boards/nrf7002dk_nrf5340.overlay

+
+&gpio1 {
+	status = "okay";
+};


Why does gpio1 need to be set to okay ?

I remember I needed to set it for a project and I thought that since the I2S device is on gpio1, I needed it here as well. But I checked and it compiled without this so I removed these four lines.

nordicjm

Can you amend the original commits with the correct filenames instead of adding the nrf7002 filenames then renaming then in a later commit?

kartben · 2023-11-07T08:51:20Z

samples/drivers/led_ws2812/README.rst

@@ -102,6 +102,8 @@ This sample uses different drivers depending on the selected board:
 I2S driver:

 - thingy52_nrf52832
+- nrf5340dk_nrf5340 (1.8V logic level, make sure to use a logic level shifter!)
+    - works with nrf7002dk_nrf5340 too


Please remove as this is an out-of-tree board, or reword along the lines of "can be used [or "should work"] for other boards featuring an nRF5340 host processor" if that is the case.

Suggested change

- works with nrf7002dk_nrf5340 too

Updated the README as you suggested.

ndaneil · 2023-11-07T23:40:46Z

@nordicjm I squashed the commits.

nordicjm · 2023-11-08T07:59:11Z

samples/drivers/led_ws2812/README.rst

+- nrf5340dk_nrf5340 (1.8V logic level, make sure to use a logic level shifter!)
+    - should work for other boards featuring an nRF5340 host processor


This is incorrect, the nrf5340dk is 3.3v logic level

samples/drivers/led_ws2812/boards/nrf5340dk_nrf5340.overlay

simonguinot · 2023-11-08T09:37:45Z

samples/drivers/led_ws2812/boards/nrf5340dk_nrf5340.overlay

+	led_strip: ws2812 {
+		compatible = "worldsemi,ws2812-i2s";
+
+		i2s-dev = < &i2s_led >;


Since you'll have to push a new version and to be consistent with the rest of the file, please remove the empty spaces in the phandle above.

There is a misunderstanding here. I was not talking about the new line after the compatible (which was perfectly OK) but about the empty spaces around the I2S phandle just above:

i2s-dev = < &i2s_led >;

Should be:

i2s-dev = <&i2s_led>;

I see. Fixed it.

simonguinot · 2023-11-08T14:37:37Z

samples/drivers/led_ws2812/boards/nrf5340dk_nrf5340.overlay

+					LED_COLOR_ID_BLUE>;
+		reset-delay = <500>;
+	};
+


There is an unnecessary tab above. Sorry :)

New config files to support nrf5340-based boards Signed-off-by: Daniel Istvan Nemeth <nemeth.daniel.istvan@hallgato.ppke.hu>

simonguinot

Thanks :)

github-actions · 2023-11-13T08:46:30Z

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

ndaneil requested a review from nashif as a code owner November 5, 2023 20:15

zephyrbot added area: LED Label to identify LED subsystem area: Samples Samples labels Nov 5, 2023

zephyrbot requested review from bbilas, kartben, Mani-Sadhasivam and simonguinot November 5, 2023 20:16

zephyrbot assigned Mani-Sadhasivam Nov 5, 2023

ndaneil force-pushed the main branch 3 times, most recently from 09aaaa3 to 4141c05 Compare November 5, 2023 22:31

nordicjm self-requested a review November 6, 2023 07:57

nordicjm reviewed Nov 6, 2023

View reviewed changes

simonguinot reviewed Nov 6, 2023

View reviewed changes

simonguinot requested a review from mbolivar-ampere November 6, 2023 09:34

ndaneil changed the title ~~samples:drivers:led_ws2812: support for nRF7002DK~~ samples:drivers:led_ws2812: support for nRF7002DK and nRF5340DK Nov 7, 2023

nordicjm requested changes Nov 7, 2023

View reviewed changes

kartben requested changes Nov 7, 2023

View reviewed changes

ndaneil force-pushed the main branch from a8e596a to efda2cf Compare November 7, 2023 23:37

nordicjm reviewed Nov 8, 2023

View reviewed changes

simonguinot reviewed Nov 8, 2023

View reviewed changes

ndaneil force-pushed the main branch 2 times, most recently from 3992001 to eb391c1 Compare November 8, 2023 14:14

simonguinot reviewed Nov 8, 2023

View reviewed changes

samples: drivers: led_ws2812: nRF5340DK support

743ac3d

New config files to support nrf5340-based boards Signed-off-by: Daniel Istvan Nemeth <nemeth.daniel.istvan@hallgato.ppke.hu>

ndaneil force-pushed the main branch from eb391c1 to 743ac3d Compare November 8, 2023 14:40

simonguinot approved these changes Nov 8, 2023

View reviewed changes

kartben approved these changes Nov 8, 2023

View reviewed changes

ndaneil requested a review from nordicjm November 10, 2023 08:15

nordicjm approved these changes Nov 10, 2023

View reviewed changes

carlescufi merged commit e5dbb26 into zephyrproject-rtos:main Nov 13, 2023
15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

samples:drivers:led_ws2812: support for nRF7002DK and nRF5340DK #64823

samples:drivers:led_ws2812: support for nRF7002DK and nRF5340DK #64823

ndaneil commented Nov 5, 2023 •

edited

Loading

github-actions bot commented Nov 5, 2023

nordicjm Nov 6, 2023

ndaneil Nov 7, 2023 •

edited

Loading

kartben Nov 7, 2023

ndaneil Nov 7, 2023

nordicjm Nov 7, 2023

simonguinot left a comment

simonguinot Nov 6, 2023

ndaneil Nov 7, 2023

nordicjm left a comment

kartben Nov 7, 2023

ndaneil Nov 7, 2023

ndaneil commented Nov 7, 2023

nordicjm Nov 8, 2023

ndaneil Nov 8, 2023

simonguinot Nov 8, 2023

ndaneil Nov 8, 2023

simonguinot Nov 8, 2023

ndaneil Nov 8, 2023

simonguinot Nov 8, 2023

ndaneil Nov 8, 2023

simonguinot left a comment

github-actions bot commented Nov 13, 2023

		- nrf5340dk_nrf5340 (1.8V logic level, make sure to use a logic level shifter!)
		- should work for other boards featuring an nRF5340 host processor

samples:drivers:led_ws2812: support for nRF7002DK and nRF5340DK #64823

samples:drivers:led_ws2812: support for nRF7002DK and nRF5340DK #64823

Conversation

ndaneil commented Nov 5, 2023 • edited Loading

github-actions bot commented Nov 5, 2023

Choose a reason for hiding this comment

ndaneil Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonguinot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nordicjm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ndaneil commented Nov 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonguinot left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 13, 2023

ndaneil commented Nov 5, 2023 •

edited

Loading

ndaneil Nov 7, 2023 •

edited

Loading