Skip to content

boards: Add two KW41Z-based boards#9934

Closed
pyropeter wants to merge 3 commits intoRIOT-OS:masterfrom
beduino-project:pr/board-common-kw41z
Closed

boards: Add two KW41Z-based boards#9934
pyropeter wants to merge 3 commits intoRIOT-OS:masterfrom
beduino-project:pr/board-common-kw41z

Conversation

@pyropeter
Copy link
Copy Markdown
Contributor

@pyropeter pyropeter commented Sep 14, 2018

Contribution description

This PR refactors the FRDM-KW41Z boards support to allow for code sharing with other KW41Z-based boards and then adds two of them:

  • kw41z-mini: https://openlabs.co/store/kw41z-mini (Basically a KW41Z breakout board with quarzes, reset button, one LED, and a 1.27mm 2x5 ARM Cortex-M debug connector)
  • Beduino Pro Mini: Our kw41z-mini clone with integrated DAPLink programmer and a second LED (To be published)

The first board is commercially available (for now, at least) and the second one is completely unpublished at the moment. We finished producing the first 15 prototype boards by hand yesterday and we will now look into commercial manufacture (and acquiring funds). We are aiming to publish our KICAD source files under a permissive license. I would not be insulted if the limited availability of the boards was reason to reject (parts of) this pull request.

This is the first time I introduced a boards/common directory, so I probably made some errors.

TODO

  • Add documentation for both boards

pyropeter
pyropeter commented Sep 14, 2018
View reviewed changes
boards/beduino-pro-mini/include/board.h
#define XTIMER_OVERHEAD (4)
#define XTIMER_HZ (32768ul)
#endif
/** @} */
Copy link
Copy Markdown
Contributor Author

@pyropeter pyropeter Sep 14, 2018

Choose a reason for hiding this comment

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

@gebart If I understand the KW41Z and xtimer correctly, the XTIMER_* defines should be the same for all KW41Z-based boards, right? If true, this should probably also be moved to common/kw41z, right? Is there some convention on how to do this?

pyropeter
pyropeter commented Sep 14, 2018
View reviewed changes
@pyropeter pyropeter mentioned this pull request Sep 14, 2018
Closed
@pyropeter pyropeter force-pushed the pr/board-common-kw41z branch from f7fe147 to bf39b30 Compare September 15, 2018 00:39
@jnohlgard
Copy link
Copy Markdown
Member

jnohlgard commented Sep 15, 2018

It does not make sense to share the periph_conf.h and adc_params.h between different boards unless they are completely pin compatible. But if they are so similar it doesn't make any sense to have a separate configuration at all. Are they identical to the frdm-kw41z in the schematic? If they are, why do you need a separate board config to begin with?

@pyropeter
Copy link
Copy Markdown
Contributor Author

pyropeter commented Sep 17, 2018

While the boards indeed are similar, they are not the same. The schematics are obviously not the same... e.g. the FRDM-KW41Z has on-board peripherals like the flash chip and the accelerometer and more LEDs. It also uses a different CPU and a different DEBUG_ADAPTER.

I too was unsure if two new board configurations are actually needed, but I couldn't think of any viable alternatives. What would you propose?

smlng
smlng suggested changes Sep 20, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@smlng smlng left a comment

Choose a reason for hiding this comment

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

IIRC we do not merge unavailable (ie. cannot buy (anymore)) boards into master. Hence, I'd suggest to make a PR for the open labs mini only and maintain your custom board in you own fork/branch.

In general one PR per board is a good rule, even if they share lots of stuff. And with respect to this PR: we do have the open labs here for testing, and are keen on getting this into RIOT. For the other: not possible for obvious reasons 😉

@smlng smlng added the Area: boards Area: Board ports label Sep 20, 2018
@smlng smlng self-assigned this Sep 20, 2018
@x3ro
Copy link
Copy Markdown
Contributor

x3ro commented Sep 27, 2018

I just tried out the kw41z-mini and the basics seem to be working fine, that is hello-world, default, lua_basics. Didn't have time to test any more right now, except gnrc_networking which currently panics in gnrc_rpl_auto_init.c:32:

#if (GNRC_NETIF_NUMOF == 1)
    gnrc_netif_t *netif = gnrc_netif_iter(NULL);
    assert(netif != NULL);  // <------

Did networking work well for you?

@x3ro
Copy link
Copy Markdown
Contributor

x3ro commented Sep 27, 2018

Oh and I also had to add this to Makefile.include be able to flash with a JLink:

ifeq (jlink,$(PROGRAMMER))
  export JLINK_DEVICE := MKW41Z256XXX4
  include $(RIOTMAKE)/tools/jlink.inc.mk
endif

To be honest I'm not sure where would best put this, though. Mostly putting it here so I don't forget :D

@jnohlgard
Copy link
Copy Markdown
Member

jnohlgard commented Sep 27, 2018

@x3ro you need #7107 for a radio driver for the kw41z.

@jowlo jowlo mentioned this pull request Oct 10, 2018
Closed
@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Dec 21, 2018

Looks like this PR was missed when usb-kw41z and phynode-kw41z were recently added. @pyropeter would mind split this PR: one introducing a common place for kw41z based boards (the most important!), one adding support for the kw41z-mini ?
I'm with @smlng to not provide a board support for a board that is not (yet) available on the market.

@benemorius benemorius mentioned this pull request Jan 23, 2019
Merged
7 tasks
@stale
Copy link
Copy Markdown

stale bot commented Aug 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Aug 10, 2019
@stale stale bot closed this Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@smlng smlng smlng requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

@smlng smlng

Labels

Area: boards Area: Board ports State: stale State: The issue / PR has no activity for >185 days

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@pyropeter @jnohlgard @x3ro @aabadie @smlng