-
Notifications
You must be signed in to change notification settings - Fork 7k
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
arch: arm: support for KW2xD SiP and USB-KW24D512 #845
Conversation
jfischer-no
commented
Jul 19, 2017
- add KW2XD SiP support
- add USB-KW24D512 support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a bunch of k64 references that need cleaning up.
dts/arm/nxp/nxp_kw2xd.dtsi
Outdated
|
||
/ { | ||
cpus { | ||
cpu@0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update CPU node to match recent PR #809
}; | ||
|
||
soc { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add in I2C nodes - see PR #755
|
||
soc { | ||
|
||
mcg: clock-controller@40064000 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add ADC nodes - see PR #804
arch/arm/soc/nxp_kinetis/kw2xd/soc.h
Outdated
|
||
/* address bases */ | ||
|
||
#define PERIPH_ADDR_BASE_MPU 0x4000D000 /* Memory Protection Unit */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all these PERIPH_* defines aren't needed.
The USB-KW24D512 board is an IEEE 802.15.4 packet sniffer in a | ||
convenient USB dongle form factor. | ||
|
||
- Packet sniffers are essential for wireless network development allowing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would read better just as regular sentences (with the previous one) rather than two bullet points.
|
||
The KW2xD SoC has six UARTs. One is configured and can be used for the console, | ||
the remaining are not used. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're missing the "Programming and Debugging" section (with Flashing and Debugging information) listed in the board doc template (doc/templates/board.tmpl). Can you include something about this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have left it consciously 😄. KW24D512 is functional equal to the KW22D512, KW22D512 is supported by OpenOCD and for the KW24D512 I have a 3 lines patch. Phytec has the similar hardware, but with the KW22D512. And it is possible to flash it with jlink, so it would be a little longer chapter. Currently I do not know how I describe it best, so I would like to do it later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added the "Programming and Debugging" section in ad382b7
- Integrated PCB Folded F-type antenna | ||
- Cortex 10-pin (0.05”) JTAG debug port for target MCU | ||
|
||
For more information about the KW2xD SiP and USB-KW24D512 board: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggest spelling out SiP in this first use:
KW2xD System-in-Package (SiP)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some doc suggestions and ask for programming details found in the board.tmpl template.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks pretty good, just one question regarding soc series and a couple of minor edits. Could you please also update ext/hal/nxp/mcux/README
|
||
if SPI | ||
|
||
config SPI_MCUX |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be SPI_MCUX_DSPI
|
||
if RANDOM_GENERATOR | ||
|
||
config RANDOM_MCUX |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be RANDOM_MCUX_RNGA
|
||
config SOC | ||
string | ||
default mkw22d5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be different for KW22 and KW24. It's used to select which device header files get included from mcux. See ext/hal/nxp/mcux/Makefile
# SPDX-License-Identifier: Apache-2.0 | ||
# | ||
|
||
config SOC_SERIES_KINETIS_KW2XD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My original intent when I added KW41Z was that KW2xD would be part of the same series (e.g., the Kinetis W-series). That's why I didn't select a specific Cortex M core (M0+ vs. M4) with SOC_SERIES_KINETIS_KWX. Your thoughts on merging kw2xd into kwx series?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about it, but then I was not sure, I look at it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please review 7c4c1dc
@@ -0,0 +1,77 @@ | |||
# Kconfig - Kinetis K6X MCU line | |||
# | |||
# Copyright (c) 2016 Open-RnD Sp. z o.o. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the header
dts/arm/nxp/nxp_kw2xd.dtsi
Outdated
status = "disabled"; | ||
}; | ||
|
||
uart3: uart@4006d000 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This instance doesn't exist
@@ -0,0 +1,7 @@ | |||
FLASH_SCRIPT = openocd.sh | |||
DEBUG_SCRIPT = openocd.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does OpenOCD support this part upstream? I don't know offhand. Can you add jlink support?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OpenOCD supports KW21D and KW22D512 devices. As far as I know none uses the KW24D5 😄, it has the same functionality as KW22D5, but a different ID. For the KW24D5 I quickly made a patch (https://github.com/jfischer-phytec-iot/openocd/commit/2ec86a443d452135b5c2a1711f99f3dd423397f5). I do not have a jlink, we have built the similar device with KW22D512 and we use the mbed firmware for our debuggers and kits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to swap the mbed firmware with the jlink firmware.
Can you please add a link to your OpenOCD patch in the board doc?
Serial Port | ||
=========== | ||
|
||
The KW2xD SoC has six UARTs. One is configured and can be used for the console, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/six/three/
20a1931
to
f646570
Compare
@@ -43,14 +59,58 @@ config SOC_PART_NUMBER_MKW41Z256VHT4 | |||
config SOC_PART_NUMBER_MKW41Z512VHT4 | |||
bool | |||
|
|||
config SOC_PART_NUMBER_MKW22D512VHA5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Could you please move these new lines above the KW40 so they're in numerical order?
config SOC_PART_NUMBER_KINETIS_KWX | ||
string | ||
default "MKW40Z160VHT4" if SOC_PART_NUMBER_MKW40Z160VHT4 | ||
default "MKW41Z256VHT4" if SOC_PART_NUMBER_MKW41Z256VHT4 | ||
default "MKW41Z512VHT4" if SOC_PART_NUMBER_MKW41Z512VHT4 | ||
default "MKW22D512VHA5" if SOC_PART_NUMBER_MKW22D512VHA5 | ||
default "MKW24D512VHA5" if SOC_PART_NUMBER_MKW24D512VHA5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move these before the KW40
obj-$(CONFIG_SOC_MKW24D5) += wdog.o soc_kw2xd.o | ||
obj-$(CONFIG_SOC_MKW22D5) += wdog.o soc_kw2xd.o | ||
obj-$(CONFIG_SOC_MKW41Z4) += soc_kw4xz.o | ||
obj-$(CONFIG_SOC_MKW40Z4) += soc_kw4xz.o |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think eventually we should try to unify these into one soc.c file, but it's fine to have them separate for now. I'm planning to work on a more general MCG clock driver soon that should help.
PORTB->PCR[19] = (PORTB->PCR[19] & ~PORT_PCR_MUX_MASK) | | ||
PORT_PCR_MUX(0x01u); | ||
/* PORTC.0 as GPIO */ | ||
PORTC->PCR[0] = (PORTC->PCR[0] & ~PORT_PCR_MUX_MASK) | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move this to the board pinmux.c file? I like to have all the pinmuxing in one place because it will be a lot more obvious if there's a conflict.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is not possible since kw2xd_init is called before phynode_kw22d5_pinmux_init (*_INIT_PRIORITY 0 vs. 45). The part with the modem clock is not so nice, but it works so far everything right. I have successfully tested it with the MCR20A driver. Driver only needs to be modified so that it does not reinitialize MCR20A. USB driver also works so far.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will describe set_modem_clock better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is board specific, so why not move this to the board code and do something like:
clkInit(void)
{
CLOCK_SetSimSafeDivs();
early_board_clkInit();
...
}
__weak early_board_clkInit(void)
{
return;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not really board specific, if CLK_OUT from the transceiver is the clock source for the PLL, it will look the same for all boards (these pins are internally connected, not on the board)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, thanks
default usb_kw24d512 | ||
|
||
config OSC_XTAL0_FREQ | ||
default 4000000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be 32 MHz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the modem is initialized like in set_modem_clock, it is 4MHz (Table 5-10, CLOCK_OUT ist 4MHz if GPIO5=0 after reset, GPIO5 is connected to PTC0, CLOCK_OUT to EXTAL0)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Can you please add a comment here so we remember that we're not using the board crystal?
@@ -0,0 +1,7 @@ | |||
FLASH_SCRIPT = openocd.sh | |||
DEBUG_SCRIPT = openocd.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to swap the mbed firmware with the jlink firmware.
Can you please add a link to your OpenOCD patch in the board doc?
CONFIG_BOARD_USB_KW24D512=y | ||
CONFIG_CONSOLE=y | ||
CONFIG_UART_CONSOLE=y | ||
CONFIG_UART_CONSOLE_ON_DEV_NAME="UART_0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should come from dts instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed it and just added J-Link support. Once the support for kw24d5 is in pyOCD or OpenOCD mainline, I will expand the docu.
PORTB->PCR[19] = (PORTB->PCR[19] & ~PORT_PCR_MUX_MASK) | | ||
PORT_PCR_MUX(0x01u); | ||
/* PORTC.0 as GPIO */ | ||
PORTC->PCR[0] = (PORTC->PCR[0] & ~PORT_PCR_MUX_MASK) | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, thanks
arch/arm/soc/nxp_kinetis/kwx/soc.h
Outdated
#define IRQ_UART1_STATUS 33 | ||
#define IRQ_UART1_ERROR 34 | ||
#define IRQ_UART2_STATUS 35 | ||
#define IRQ_UART2_ERROR 36 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the UART IRQs. See #899
# | ||
|
||
config BOARD_USB_KW24D512 | ||
bool "Freescale USB-KW24D512" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/Freescale/NXP/
default usb_kw24d512 | ||
|
||
config OSC_XTAL0_FREQ | ||
default 4000000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Can you please add a comment here so we remember that we're not using the board crystal?
Serial Port | ||
=========== | ||
|
||
The KW2xD SoC has tree UARTs. One is configured and can be used for the console, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/tree/three/
$ cd <zephyr_root_path> | ||
$ . zephyr-env.sh | ||
$ cd samples/hello_world/ | ||
$ make BOARD=usb_kw24d512 DEBUG_SCRIPT=jlink.sh debug |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove DEBUG_SCRIPT=jlink.sh
since it's the default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some DT updates/changes.
dts/arm/nxp/nxp_kw2xd.dtsi
Outdated
}; | ||
}; | ||
|
||
sram0: memory { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sram0: memory@20000000
dts/arm/nxp/nxp_kw2xd.dtsi
Outdated
}; | ||
|
||
sram0: memory { | ||
compatible = "mmio-sram"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
device_type = "memory";
dts/arm/nxp/nxp_kw2xd.dtsi
Outdated
}; | ||
|
||
pwm1: pwm@40039000{ | ||
compatible = "nxp,k64f-pwm"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets go ahead and rename the compatible to:
compatible = "nxp,kinetis-ftm";
dts/arm/nxp/nxp_kw2xd.dtsi
Outdated
compatible = "nxp,k64f-pwm"; | ||
reg = <0x40039000 0x98>; | ||
prescaler = <2>; | ||
period = <1000>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets remove prescaler, period, & clock-source
|
||
In a second terminal, open telnet: | ||
|
||
.. code-block:: console |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The .. code-block shouldn't be indented
|
||
Continue program execution in GDB, then in the telnet terminal you should see: | ||
|
||
.. code-block:: console |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here... don't indent the .. code-block
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
formatting fix requested (.. code-block shouldn't be indented)
@dbkinder @galak @MaureenHelm please review changes |
- zephyr | ||
- gccarmemb | ||
supported: | ||
- ieee802154 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to add:
ram: 32
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you look at the failures when we do:
./scripts/sanitycheck -p usb_kw24d512
(after adding ram: 32, in usb_kw24d512.yaml) I see:
total complete: 0/ 3 0% failed: 0
usb_kw24d512 samples/nfc/nfc_hello/test FAILED: build_error
see: sanity-out/usb_kw24d512/samples/nfc/nfc_hello/test/build.log
total complete: 1/ 3 33% failed: 1
usb_kw24d512 tests/drivers/build_all/test_build_drivers FAILED: build_error
see: sanity-out/usb_kw24d512/tests/drivers/build_all/test_build_drivers/build.log
total complete: 2/ 3 66% failed: 2
usb_kw24d512 samples/sensor/sx9500/test FAILED: build_error
see: sanity-out/usb_kw24d512/samples/sensor/sx9500/test/build.log
Funny, it does not even go through (on Debian testing), I did not try it yet with Zephyr SDK. It fails each time on different test or samples.
|
That's a new one for me. |
It was my environment in the shell, USE_CCACHE=1 was defined. |
|
@galak any chance to get it in 1.9, I guess not 😄 |
Needs rebasing, but otherwise LGTM. I tested hello_world and basic/blinky samples on hw. Can you enable at least one of the net samples too? |
rebased, sample |
136d84a
to
02f2b6d
Compare
@galak Is there anything that prevents it from being accepted? |
@jfischer-phytec-iot need to look at the sanity failures and decide what to do about them. |
The problem is that you've enabled default drivers based on the SoC (which is the right thing to do), but not default driver instances based on the board. You can add default uart and i2c instances to the board defconfig to clean up the build warnings in these tests. |
6996bf6
to
88a3d1b
Compare
@galak @MaureenHelm I followed your suggestion also added options for ADC and FTM, needs review |
Origin: NXP MCUXpresso KSDK 2.2 URL: https://mcuxpresso.nxp.com/ Maintained-by: External Signed-off-by: Johann Fischer <[email protected]>
Jira: ZEP-1471 Signed-off-by: Johann Fischer <[email protected]>
Jira: ZEP-1472 Signed-off-by: Johann Fischer <[email protected]>
88a3d1b
to
dca154f
Compare