boards: add support for kw41z-mini #10846
Conversation
efb92fb to
54d5505
Compare
|
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. |
|
I'll push an update to this "soon". I forgot it had gotten outdated. |
| .type = KINETIS_LPUART, | ||
| }, | ||
| }; | ||
| #define UART_NUMOF (sizeof(uart_config) / sizeof(uart_config[0])) |
There was a problem hiding this comment.
You can use ARRAY_SIZE(uart_config) now.
| [6] = { .dev = ADC0, .pin = GPIO_UNDEF, .chan = 23, .avg = ADC_AVG_MAX }, | ||
| }; | ||
|
|
||
| #define ADC_NUMOF (sizeof(adc_config) / sizeof(adc_config[0])) |
There was a problem hiding this comment.
| #define ADC_NUMOF (sizeof(adc_config) / sizeof(adc_config[0])) | |
| #define ADC_NUMOF ARRAY_SIZE(adc_config) |
boards/kw41z-mini/Makefile.include
Outdated
| @@ -0,0 +1,6 @@ | |||
| # define the cpu used by the board | |||
| export CPU = kinetis | |||
| export CPU_MODEL = mkw41z256vht4 | |||
There was a problem hiding this comment.
Since this board doesn't come with a programmer, I'd suggest also adding
export JLINK_DEVICE = MKW41Z256xxx4 for convenience. The Pin header on the board is after all compatible with e.g. JLink Edu Mini.
| }, | ||
| }; | ||
|
|
||
| #define DAC_NUMOF (sizeof(dac_config) / sizeof(dac_config[0])) |
There was a problem hiding this comment.
| #define DAC_NUMOF (sizeof(dac_config) / sizeof(dac_config[0])) | |
| #define DAC_NUMOF ARRAY_SIZE(dac_config) |
| }, | ||
| }; | ||
|
|
||
| #define SPI_NUMOF (sizeof(spi_config) / sizeof(spi_config[0])) |
There was a problem hiding this comment.
| #define SPI_NUMOF (sizeof(spi_config) / sizeof(spi_config[0])) | |
| #define SPI_NUMOF ARRAY_SIZE(spi_config) |
| .sda_pcr = (PORT_PCR_MUX(3)), | ||
| }, | ||
| }; | ||
| #define I2C_NUMOF (sizeof(i2c_config) / sizeof(i2c_config[0])) |
There was a problem hiding this comment.
| #define I2C_NUMOF (sizeof(i2c_config) / sizeof(i2c_config[0])) | |
| #define I2C_NUMOF ARRAY_SIZE(i2c_config) |
| { | ||
| .name = "IO_7", | ||
| .pin = IO_7_PIN, | ||
| .mode = GPIO_IN, | ||
| }, | ||
| { | ||
| .name = "IO_8", | ||
| .pin = IO_8_PIN, | ||
| .mode = GPIO_IN, | ||
| }, |
There was a problem hiding this comment.
Those are also used for I2C.
I had to remove those for i2c_scan to work.
|
I tested I2C, it works well too. |
|
Very cool! The code seems to be in pretty good shape and I'll try to run some tests later today. Two small things I wondered while quickly scanning the PR:
|
|
@benpicco how did you connect/flash the board? The Makefile expects a CMSIS-DAP by default. I tried to connect manually via JLink (EDU) without success. It is likely that I missed something. |
|
I also used JLink EDU mini. Mind that JLink does not provide 3.3V to the board, so you have to power it separately. |
Ahja, forgot about that :-). So I attached a separate supply and got a little bit further but it seems there are some reset issues. Only executing So I tried to manually connect to the device which seems to have problems with the reset pin: Any suggestions? |
|
To actually get the build system to use JLink you also have to add And in JLinkExe, select SWD, not JTAG. If it still fails then, try rotating the connector 180° 😉 |
|
Thanks @benpicco!
|
|
Yes, I complied OpenOCD from source - that is also needed for same54 and many other chips released around or after 2017. |
|
Due to my flashing issues this morning I didn't manage to test that much. Right now I manually flashed the device using JLinkExe and so far I managed to:
Nice to see these things work! I would provide some more tests next week during Hack'n'ACK |
|
Can you please run the following: Then in the shell run the following Please share your shell output because I am not getting an address NACK My results |
|
ADC
I did not test lines 4 - 6, did you? DAC I had to figure out in the RM that the DAC connects to PTB18, so this seems to be a fixed setup right? Could you add a comment next to the configuration in periph_conf? GPIO
SPI
PWM I2C RTC & RTT Timer xtimer Seems ok, see results |
|
The I2C bug shouldn't be related to the board though, that's in The basic I2C config for this board is valid (when ignoring some pins doubling as saul gpio). |
|
@benemorius ping 😉 |
static int i2c_tx_addr(i2c_t dev, uint8_t byte)
{
I2C_Type *i2c = i2c_config[dev].i2c;
TRACE("i2c: txa: %02x\n", (unsigned)byte);
i2c->D = byte;
uint8_t S;
uint16_t timeout = UINT16_MAX;
do {
S = i2c->S;
} while (!(S & I2C_S_IICIF_MASK) && --timeout);
i2c_clear_irq_flags(i2c);
int res = 0;
if (S & I2C_S_ARBL_MASK) {
DEBUG("i2c: arbitration lost\n");
res = -EAGAIN;
}
else if (timeout == 0) {
/* slave stretches the clock for too long */
DEBUG("i2c: tx timeout\n");
res = -ETIMEDOUT;
}
else if (S & I2C_S_RXAK_MASK) {
DEBUG("i2c: NACK\n");
res = -ENXIO;
}
if (res < 0) {
bit_clear8(&i2c->C1, I2C_C1_MST_SHIFT);
i2c_state[dev].active = 0;
i2c_clear_irq_flags(i2c);
}
return res;
}
It seems like timeout happens on when the With the If this gets merged please raise an issue on this and I will attend to it when I have time. EDIT: |
|
My apologies, it seems like there is no internal pullups in the port. This means that the external pullups are needed (and not provided with the board). Maybe make a note on the board documentation or something because the requirement are different from the frdm-kw41z board. Either way I2C should pass. |
|
@MrKevinWeiss thanks for investigating on the I2C issue! @benemorius the PR looks good but there are some changes and checkups needed:
|
54d5505 to
b6585fd
Compare
|
The deal with the nonworking ADC input is that the kinetis ADC peripheral has an extra mux to switch between two sets of mostly-overlapping inputs. Currently we don't handle that configurability in the driver and it's just hardcoded to the B inputs whereas this board needs (and all 4 ADC inputs work with) the A inputs selected. Verified with this hack: I don't know the reason for the timer test failing. I know both timers work. I'll investigate. |
|
Murdock also found some issues. |
Thanks I expected it to I just needed to see how close I was getting. I'm working on addressing everything now. |
|
The reason kw41z doesn't have PWM yet is because most other kinetis use a FlexTimer peripheral for PWM whereas others like kw41z use a standard PWM peripheral and the driver only supports FTM not PWM peripherals. I have an implementation that supports PWM that I need to clean up and PR. |
|
The periph_timer test is failing on the second timer because it's an LPTIMER which can't run as fast as 1 MHz. The test has a list of boards needing 32 kHz timers that I can add this board to. It passes after that. |
9d968cf to
e1a7bb9
Compare
|
Just a few more things to address I think. |
2a45776 to
ee0ff40
Compare
|
I think that's everything addressed. Hopefully the notes in the doc file are adequate for the points that were brought up. Let me know what to improve otherwise. |
boards/openlabs-kw41z-mini/doc.txt
Outdated
| Raspbian has an old version of gcc-arm-none-eabi as of this writing | ||
| and it's recommended to install a newer version for compiling RIOT. |
There was a problem hiding this comment.
Well it's 2018-q2-6 which isn't that old but I just noticed it's older than what I'm using regularly so it seemed worth mentioning. In any case I think it's always good advice to use the latest version if it's a really modern cpu. I don't have a specific recommendation I just noticed it's old.
There was a problem hiding this comment.
Yes but please add the version number to the comment.
The code will still be there in 5 years and then raspbian hopefully ships a newer version ;)
There was a problem hiding this comment.
btw: Ubuntu ships the same version and I had no trouble compiling RIOT for any ARM controller with that yet.
Why would kw41z be special here - or rather, what issues are you experiencing?
There was a problem hiding this comment.
Oh cool thanks for mentioning that. I was just about to reply that I couldn't find any actual issues and I'm content to not have this remark.
My only reservation was that it's different from the configuration that I've generally tested. I think I'll wrap up testing and proceed to remove the remark.
35f14e1 to
ef6024c
Compare
I couldn't recreate any issues so I dropped it. |
|
Thanks very much for the review and testing! |
|
Are there two versions of the board? --- a/boards/openlabs-kw41z-mini/Makefile.features
+++ b/boards/openlabs-kw41z-mini/Makefile.features
@@ -1,5 +1,5 @@
CPU = kinetis
-CPU_MODEL = mkw41z512vht4
+CPU_MODEL = mkw41z256vht4
# Put defined MCU peripherals here (in alphabetical order)
FEATURES_PROVIDED += periph_adcfor it to print anything over UART. M41W 9VT4 1N16S CTHHQPM41W 8VT4 1N16S CTDFMD |
I'm sorry I thought I had reached out to everyone who got any of those to replace them. What happened is NXP reduced the price of the 512k chip and it didn't make sense to use the 256k chip anymore. So only the early boards have the 256k chip and rather than support them both I thought it easier to just find and replace the few boards that have 256k chips. But obviously I didn't manage to get them all back and I'm so very sorry for the confusion and trouble. If it's ok, can you email me a shipping address and tell me how many of the 256k boards you have so I can replace them with 512k boards? |
|
Don't worry, you've already sent me four boards when I ordered two (2x r2 and 2x r1.2 of which one is the 256k version. They had bent JTAG pins which I managed to unbent and then just used those as a had a fitting adapter - thank you for the freebies 😄) |
Contribution description
This adds support for kw41z-mini. It's much the same as frdm-kw41z except it has half the memory and no dcdc. Also no load caps on the crystals. I plan to be making them regularly unless the stm32wb ends up being a much better chip.
Testing procedure
Radio support needs #7107
Everything I've tested seems to be working. I haven't taken a close look at low power modes yet (with #7897) aside from getting down to about 5uA and being unable to wake afterwards. I didn't pursue it further but I plan to soonish.
I just realized I've never tested I2C and I don't have anything handy to test with, but if it works on frdm-kw41z then it must work on kw41z-mini because they use the same I2C pins and I copied and pasted the I2C configuration.
Tested:
Issues/PRs references
Continuing the discussion from #9934, I don't see much room for combining common kw41z files. The only things I see that could be picked out are timer and xtimer configurations and
spi_clk_config, but I don't know that it's worth it for that and there can be valid reasons for a board to change the timer.