Add nrf52840dk #32

szczys · 2022-08-16T21:10:55Z

This PR adds overlay and pinmap for Nordic and NXP boards that have Arduino headers.

I used the following technique for mapping these headers:

Digital pins 0..13 for D0..13
Analog pins 0..5 for D14..D19
SDA/SCL pins for D20..D21

This is based on #31 which hasn't yet been merged. There is actually only one new commit in this PR: d0697ca

DhruvaG2000 · 2022-08-17T17:36:55Z

I think you need to merge the dev branch onto this branch rather than rebase...
As of now, there are repeat commits from dev in this branch making the changed files look more than there are

szczys · 2022-08-17T19:44:03Z

@DhruvaG2000 Sorry about that, I've cleaned up the PR.

DhruvaG2000 · 2022-08-24T18:08:05Z

I don't really have the hardware to test this PR, but if it builds and works on your end I don't see any issues.
Maybe if we could add the i2c nodes as well here it would be great for when the I2C PR finally merges

szczys · 2022-09-12T03:02:26Z

@DhruvaG2000 will you please review and approve this PR? Notes below, thanks!

Add nrf52840dk (this PR)

I have added i2c support and tested the nrf52840dk_nrf52840 and found it is working. This PR now represents all changes necessary to add that board as a variant.

New branch: Add mimxrt1060_evkb (not in this PR)

The mimxrt1060_evkb is not working in tests (scanning for i2c address returns a device on every address). The changes to add that board have been moved to a different branch so that this PR can be merged.

DhruvaG2000

running checkpatch.pl produces many warnings and errors, kindly fix them if possible:

└─❯ ./scripts/checkpatch.pl patches/0001-variants-add-nrf52840dk.patch
patches/0001-variants-add-nrf52840dk.patch:75: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#75: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:20:
+    GPIO_DT_SPEC_GET(DT_PATH(zephyr_user), d4_gpios);$

patches/0001-variants-add-nrf52840dk.patch:77: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#77: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:22:
+    GPIO_DT_SPEC_GET(DT_PATH(zephyr_user), d5_gpios);$

patches/0001-variants-add-nrf52840dk.patch:79: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#79: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:24:
+    GPIO_DT_SPEC_GET(DT_PATH(zephyr_user), d6_gpios);$

patches/0001-variants-add-nrf52840dk.patch:81: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#81: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:26:
+    GPIO_DT_SPEC_GET(DT_PATH(zephyr_user), d7_gpios);$

patches/0001-variants-add-nrf52840dk.patch:83: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#83: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:28:
+    GPIO_DT_SPEC_GET(DT_PATH(zephyr_user), d8_gpios);$

patches/0001-variants-add-nrf52840dk.patch:85: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#85: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:30:
+    GPIO_DT_SPEC_GET(DT_PATH(zephyr_user), d9_gpios);$

patches/0001-variants-add-nrf52840dk.patch:87: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#87: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:32:
+    GPIO_DT_SPEC_GET(DT_PATH(zephyr_user), d10_gpios);$

patches/0001-variants-add-nrf52840dk.patch:89: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#89: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:34:
+    GPIO_DT_SPEC_GET(DT_PATH(zephyr_user), d11_gpios);$

patches/0001-variants-add-nrf52840dk.patch:91: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#91: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:36:
+    GPIO_DT_SPEC_GET(DT_PATH(zephyr_user), d12_gpios);$

patches/0001-variants-add-nrf52840dk.patch:93: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#93: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:38:
+    GPIO_DT_SPEC_GET(DT_PATH(zephyr_user), d13_gpios);$

patches/0001-variants-add-nrf52840dk.patch:95: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#95: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:40:
+    GPIO_DT_SPEC_GET(DT_PATH(zephyr_user), d13_gpios);$

patches/0001-variants-add-nrf52840dk.patch:97: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#97: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:42:
+    GPIO_DT_SPEC_GET(DT_PATH(zephyr_user), d15_gpios);$

patches/0001-variants-add-nrf52840dk.patch:99: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#99: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:44:
+    GPIO_DT_SPEC_GET(DT_PATH(zephyr_user), d16_gpios);$

patches/0001-variants-add-nrf52840dk.patch:101: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#101: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:46:
+    GPIO_DT_SPEC_GET(DT_PATH(zephyr_user), d17_gpios);$

patches/0001-variants-add-nrf52840dk.patch:103: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#103: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:48:
+    GPIO_DT_SPEC_GET(DT_PATH(zephyr_user), d18_gpios);$

patches/0001-variants-add-nrf52840dk.patch:105: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#105: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:50:
+    GPIO_DT_SPEC_GET(DT_PATH(zephyr_user), d19_gpios);$

patches/0001-variants-add-nrf52840dk.patch:107: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#107: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:52:
+    GPIO_DT_SPEC_GET(DT_PATH(zephyr_user), d20_gpios);$

patches/0001-variants-add-nrf52840dk.patch:109: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#109: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:54:
+    GPIO_DT_SPEC_GET(DT_PATH(zephyr_user), d21_gpios);$

patches/0001-variants-add-nrf52840dk.patch:111: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#111: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:56:
+    GPIO_DT_SPEC_GET(DT_PATH(zephyr_user), d22_gpios); /* LED0 */$

patches/0001-variants-add-nrf52840dk.patch:114: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#114: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:59:
+    &d0,  &d1,  &d2,  &d3,  &d4,  &d5,  &d6,  &d7,  &d8,  &d9,  &d10,$

patches/0001-variants-add-nrf52840dk.patch:115: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#115: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:60:
+    &d11, &d12, &d13, &d14, &d15, &d16, &d17, &d18, &d19, &d20, &d21,$

patches/0001-variants-add-nrf52840dk.patch:116: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#116: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:61:
+    &d22};$

patches/0001-variants-add-nrf52840dk.patch:119: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#119: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:64:
+  D0,$

patches/0001-variants-add-nrf52840dk.patch:120: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#120: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:65:
+  D1,$

patches/0001-variants-add-nrf52840dk.patch:121: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#121: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:66:
+  D2,$

patches/0001-variants-add-nrf52840dk.patch:122: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#122: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:67:
+  D3,$

patches/0001-variants-add-nrf52840dk.patch:123: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#123: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:68:
+  D4,$

patches/0001-variants-add-nrf52840dk.patch:124: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#124: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:69:
+  D5,$

patches/0001-variants-add-nrf52840dk.patch:125: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#125: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:70:
+  D6,$

patches/0001-variants-add-nrf52840dk.patch:126: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#126: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:71:
+  D7,$

patches/0001-variants-add-nrf52840dk.patch:127: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#127: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:72:
+  D8,$

patches/0001-variants-add-nrf52840dk.patch:128: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#128: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:73:
+  D9,$

patches/0001-variants-add-nrf52840dk.patch:129: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#129: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:74:
+  D10,$

patches/0001-variants-add-nrf52840dk.patch:130: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#130: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:75:
+  D11,$

patches/0001-variants-add-nrf52840dk.patch:131: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#131: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:76:
+  D12,$

patches/0001-variants-add-nrf52840dk.patch:132: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#132: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:77:
+  D13,$

patches/0001-variants-add-nrf52840dk.patch:133: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#133: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:78:
+  D14,$

patches/0001-variants-add-nrf52840dk.patch:134: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#134: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:79:
+  D15,$

patches/0001-variants-add-nrf52840dk.patch:135: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#135: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:80:
+  D16,$

patches/0001-variants-add-nrf52840dk.patch:136: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#136: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:81:
+  D17,$

patches/0001-variants-add-nrf52840dk.patch:137: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#137: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:82:
+  D18,$

patches/0001-variants-add-nrf52840dk.patch:138: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#138: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:83:
+  D19,$

patches/0001-variants-add-nrf52840dk.patch:139: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#139: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:84:
+  D20,$

patches/0001-variants-add-nrf52840dk.patch:140: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#140: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:85:
+  D21,$

patches/0001-variants-add-nrf52840dk.patch:141: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#141: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:86:
+  D22   /* LED0 */$

patches/0001-variants-add-nrf52840dk.patch:155: ERROR:C99_COMMENTS: do not use C99 // comments
#155: FILE: variants/variants.h:9:
+#endif // CONFIG_BOARD_ARDUINO_NANO_33_IOT

patches/0001-variants-add-nrf52840dk.patch:158: ERROR:C99_COMMENTS: do not use C99 // comments
#158: FILE: variants/variants.h:12:
+#endif // CONFIG_BOARD_NRF52840DK_NRF52840

patches/0001-variants-add-nrf52840dk.patch total: 2 errors, 45 warnings, 125 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

patches/0001-variants-add-nrf52840dk.patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES C99_COMMENT_TOLERANCE COMPLEX_MACRO CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME DT_SCHEMA_BINDING_PATCH DT_SPLIT_BINDING_PATCH ENOSYS FILE_PATH_CHANGES IS_ENABLED_CONFIG MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE PREFER_KERNEL_TYPES PREFER_SECTION PRINTK_WITHOUT_KERN_LEVEL REPEATED_WORD SPDX_LICENSE_TAG SPLIT_STRING TRAILING_SEMICOLON UNDOCUMENTED_DT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

I know this project hasn't strictly followed coding guidelines so far, but I believe that it would be a good idea to start doing so from now.

DhruvaG2000 · 2022-09-12T14:15:32Z

variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h

@@ -0,0 +1,89 @@
+/*
+ * Copyright (c) 2022 Dhruva Gole


I believe the Copyright belongs to you @szczys ;)

DhruvaG2000 · 2022-09-12T14:16:13Z

variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h

+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/* All the pins that are 100 + x are gpio1 pins and < 100 are in gpio0 */


Not sure if this is a relevant comment anymore

I removed that. I also changed the pragma to an #ifdef which I think is the Zephry-preferred method for header includes.

szczys · 2022-09-12T17:16:23Z

@DhruvaG2000 thanks for catching the checkpatch issues. I think I've fixed them, however I do still get 3 warnings. Do you know if these are problems that need fixing?

#2: 
new file mode 100644

WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
#40: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:1:
+/*

WARNING: Misplaced SPDX-License-Identifier tag - use line 1 instead
#43: FILE: variants/nrf52840dk_nrf52840/nrf52840dk_nrf52840_pinmap.h:4:
+ * SPDX-License-Identifier: Apache-2.0

DhruvaG2000 · 2022-09-12T17:21:06Z

Imo we can safely ignore the SPDX warnings, because visually it LGTM.
Code formatting is mostly important and that seems to be fixed,
I'll try and build this again on my machine tomorrow just to be sure before approving and then I think we can merge this.

DhruvaG2000

Built on my system locally, and also ran checkpatch again, ended up with fewer errors this time:

patches/0001-variants-add-nrf52840dk.patch:162: ERROR:C99_COMMENTS: do not use C99 // comments
#162: FILE: variants/variants.h:9:
+#endif // CONFIG_BOARD_ARDUINO_NANO_33_IOT

patches/0001-variants-add-nrf52840dk.patch:165: ERROR:C99_COMMENTS: do not use C99 // comments
#165: FILE: variants/variants.h:12:
+#endif // CONFIG_BOARD_NRF52840DK_NRF52840

patches/0001-variants-add-nrf52840dk.patch total: 2 errors, 0 warnings, 132 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

patches/0001-variants-add-nrf52840dk.patch has style problems, please review.

DhruvaG2000

Replacing just the // comments with /* comments gets rid of any checkpatch errors.
We can create a separate PR to fix the rest of the comments in this file, but for now my suggestion is we follow /*.
However if in your opinion this will look inconsistent then I am okay with // as well.

variants/variants.h

szczys · 2022-09-15T22:44:14Z

Ah yes, I know that Zephyr doesn't allow // comments. Thanks for catching that!

DhruvaG2000 · 2022-09-15T23:51:20Z

@szczys Thank you so much for this contribution!
A final nit, please squash the redundant "update" commits.

Add varints folder and pinmapping for nRF52840dk Signed-off-by: Mike Szczys <mike@golioth.io>

szczys · 2022-09-16T00:40:14Z

Okay, I have squashed and also re-targeted this PR for main.

DhruvaG2000

Thanks for your invaluable time, effort and also for bearing with the multiple rounds of reviews @szczys !
PR LGTM :D

szczys requested a review from DhruvaG2000 August 16, 2022 21:10

szczys marked this pull request as draft August 16, 2022 21:13

beriberikix changed the title ~~Add nrf52840 and mimxrt1060_evkb variants~~ Add nrf52840dk and mimxrt1060_evkb variants Aug 17, 2022

szczys force-pushed the szczys/add-52840-1060evkb branch from d0697ca to e1a4b24 Compare August 17, 2022 14:52

szczys force-pushed the szczys/add-52840-1060evkb branch from e1a4b24 to f8e21be Compare August 17, 2022 19:42

szczys marked this pull request as ready for review August 18, 2022 15:55

DhruvaG2000 force-pushed the dev branch from c512ccd to bf7dcdf Compare September 4, 2022 02:14

DhruvaG2000 added the new variant Adding a new variant label Sep 4, 2022

szczys force-pushed the szczys/add-52840-1060evkb branch from f8e21be to b3a7ada Compare September 12, 2022 02:57

szczys changed the title ~~Add nrf52840dk and mimxrt1060_evkb variants~~ Add nrf52840dk Sep 12, 2022

DhruvaG2000 requested changes Sep 12, 2022

View reviewed changes

szczys force-pushed the szczys/add-52840-1060evkb branch 2 times, most recently from bfa2e1d to 92544ad Compare September 12, 2022 17:13

DhruvaG2000 self-requested a review September 13, 2022 16:07

DhruvaG2000 requested changes Sep 13, 2022

View reviewed changes

variants/variants.h Outdated Show resolved Hide resolved

variants/variants.h Outdated Show resolved Hide resolved

szczys requested a review from DhruvaG2000 September 15, 2022 22:44

variants: add nrf52840dk

950573d

Add varints folder and pinmapping for nRF52840dk Signed-off-by: Mike Szczys <mike@golioth.io>

szczys force-pushed the szczys/add-52840-1060evkb branch from ba216a7 to 950573d Compare September 16, 2022 00:36

szczys changed the base branch from dev to main September 16, 2022 00:39

DhruvaG2000 approved these changes Sep 16, 2022

View reviewed changes

szczys merged commit 8a28d3a into main Sep 16, 2022

szczys deleted the szczys/add-52840-1060evkb branch September 16, 2022 14:43

Add nrf52840dk #32

Add nrf52840dk #32

Uh oh!

Conversation

szczys commented Aug 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DhruvaG2000 commented Aug 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

szczys commented Aug 17, 2022

Uh oh!

DhruvaG2000 commented Aug 24, 2022

Uh oh!

szczys commented Sep 12, 2022

Add nrf52840dk (this PR)

New branch: Add mimxrt1060_evkb (not in this PR)

Uh oh!

DhruvaG2000 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DhruvaG2000 Sep 12, 2022

Choose a reason for hiding this comment

Uh oh!

szczys Sep 12, 2022

Choose a reason for hiding this comment

Uh oh!

DhruvaG2000 Sep 12, 2022

Choose a reason for hiding this comment

Uh oh!

szczys Sep 12, 2022

Choose a reason for hiding this comment

Uh oh!

szczys commented Sep 12, 2022

Uh oh!

DhruvaG2000 commented Sep 12, 2022

Uh oh!

DhruvaG2000 left a comment

Choose a reason for hiding this comment

Uh oh!

DhruvaG2000 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

szczys commented Sep 15, 2022

Uh oh!

DhruvaG2000 commented Sep 15, 2022

Uh oh!

szczys commented Sep 16, 2022

Uh oh!

DhruvaG2000 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

szczys commented Aug 16, 2022 •

edited

Loading

DhruvaG2000 commented Aug 17, 2022 •

edited

Loading

DhruvaG2000 left a comment •

edited

Loading