Skip to content
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

feat!(boards): Migrate from seeeduino_xiao_ble to xiao_ble definition #2749

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

caksoylar
Copy link
Contributor

Bring xiao_ble board from Zephyr to parity with seeeduino_xiao_ble board maintained in ZMK's Zephyr fork. Then switch the metadata to that, so that becomes the officially used version while maintaining backwards compatibility for users still using seeeduino_xiao_ble. This should let us remove seeeduino_xiao_ble definition in the future, and make it easier to migrate across Zephyr versions.

I did my best to use the overlay and conf to make the resulting builds equivalent, by comparing zephyr.dts and .config files. I did this for three variants:

  1. Regular build: board={seeeduino_,}xiao_ble west build -d build/$board -b $board -- -DSHIELD=hummingbird
  2. Logging build: board={seeeduino_,}xiao_ble west build -d build/$board -b $board -S zmk-usb-logging -- -DSHIELD=hummingbird
  3. Studio build: board={seeeduino_,}xiao_ble west build -d build/$board -b $board -S studio-rpc-usb-uart -- -DSHIELD=hummingbird -DCONFIG_ZMK_STUDIO=y

There are still a few (potentially significant) differences:

  • xiao_ble uses HV mode, whereas seeeduino_xiao_ble doesn't (this is a good change?)
  • &i2c0 and &i2c1 are swapped, but &xiao_i2c alias is swapped alongside it
  • xiao_ble sets CONFIG_USB_DEVICE_INITIALIZE_AT_BOOT=y
    • Overriding this in xiao_ble.conf didn't work, it was turned on by USB_DEVICE_STACK
    • Turning on the latter in Zephyr seems to be unusual, and it is removed in later updates

Additional notes

The CDC ACM node addition is also unusual for Zephyr, and we delete it back in the overlay. This is removed upstream in the same changeset above.

Final diff between two board DTS (for the logging build, omitting phandle changes):

diff --git a/build/seeeduino_xiao_ble_debug/zephyr/zephyr.dts b/build/xiao_ble_adj_debug/zephyr/zephyr.dts
index 8b5bab35..66cb60f6 100644
--- a/build/seeeduino_xiao_ble_debug/zephyr/zephyr.dts
+++ b/build/xiao_ble_adj_debug/zephyr/zephyr.dts
@@ -3,8 +3,8 @@
 / {
 	#address-cells = < 0x1 >;
 	#size-cells = < 0x1 >;
-	model = "Seeeduino XIAO BLE";
-	compatible = "seeed,xiao_ble";
+	model = "Seeed XIAO BLE";
+	compatible = "seeed,xiao-ble";
 	chosen {
 		zephyr,entropy = &rng;
 		zephyr,flash-controller = &flash_controller;
@@ -16,6 +16,7 @@
 		zephyr,sram = &sram0;
 		zephyr,flash = &flash0;
 		zephyr,code-partition = &code_partition;
+		zephyr,ieee802154 = &ieee802154;
 		zmk,battery = &vbatt;
 		zmk,kscan = &kscan0;
 		zmk,physical-layout = &physical_layout0;
@@ -24,6 +25,11 @@
 		led0 = &led0;
 		led1 = &led1;
 		led2 = &led2;
+		pwm-led0 = &pwm_led0;
+		bootloader-led0 = &led0;
+		mcuboot-led0 = &led0;
+		watchdog0 = &wdt0;
+		spi-flash0 = &p25q16h;
 	};
 	soc {
 		#address-cells = < 0x1 >;
@@ -54,6 +60,7 @@
 			compatible = "nordic,nrf-uicr";
 			reg = < 0x10001000 0x1000 >;
 			status = "okay";
+			gpio-as-nreset;
 		};
 		sram0: memory@20000000 {
 			compatible = "mmio-sram";
@@ -102,7 +109,7 @@
 			};
 		};
 		uart0: xiao_serial: uart@40002000 {
-			compatible = "nordic,nrf-uart";
+			compatible = "nordic,nrf-uarte";
 			reg = < 0x40002000 0x1000 >;
 			interrupts = < 0x2 0x1 >;
 			status = "disabled";
@@ -111,8 +118,8 @@
 			pinctrl-1 = < &uart0_sleep >;
 			pinctrl-names = "default", "sleep";
 		};
-		i2c0: xiao_i2c: i2c@40003000 {
-			compatible = "nordic,nrf-twi";
+		i2c0: i2c@40003000 {
+			compatible = "nordic,nrf-twim";
 			#address-cells = < 0x1 >;
 			#size-cells = < 0x0 >;
 			reg = < 0x40003000 0x1000 >;
@@ -120,9 +127,6 @@
 			interrupts = < 0x3 0x1 >;
 			easydma-maxcnt-bits = < 0x10 >;
 			status = "disabled";
-			pinctrl-0 = < &i2c0_default >;
-			pinctrl-1 = < &i2c0_sleep >;
-			pinctrl-names = "default", "sleep";
 		};
 		spi0: spi@40003000 {
 			compatible = "nordic,nrf-spim";
@@ -134,8 +138,8 @@
 			easydma-maxcnt-bits = < 0x10 >;
 			status = "disabled";
 		};
-		i2c1: i2c@40004000 {
-			compatible = "nordic,nrf-twim";
+		i2c1: xiao_i2c: i2c@40004000 {
+			compatible = "nordic,nrf-twi";
 			#address-cells = < 0x1 >;
 			#size-cells = < 0x0 >;
 			reg = < 0x40004000 0x1000 >;
@@ -143,6 +147,9 @@
 			interrupts = < 0x4 0x1 >;
 			easydma-maxcnt-bits = < 0x10 >;
 			status = "disabled";
+			pinctrl-0 = < &i2c1_default >;
+			pinctrl-1 = < &i2c1_sleep >;
+			pinctrl-names = "default", "sleep";
 		};
 		spi1: spi@40004000 {
 			compatible = "nordic,nrf-spim";
@@ -324,6 +331,10 @@
 			interrupts = < 0x1c 0x1 >;
 			status = "disabled";
 			#pwm-cells = < 0x3 >;
+			pinctrl-0 = < &pwm0_default >;
+			pinctrl-1 = < &pwm0_sleep >;
+			pinctrl-names = "default", "sleep";
+			phandle = < 0xf >;
 		};
 		pdm0: pdm@4001d000 {
 			compatible = "nordic,nrf-pdm";
@@ -353,7 +364,7 @@
 					#size-cells = < 0x1 >;
 					sd_partition: partition@0 {
 						label = "softdevice";
-						reg = < 0x0 0x26000 >;
+						reg = < 0x0 0x27000 >;
 					};
 					code_partition: partition@27000 {
 						label = "code_partition";
@@ -455,19 +466,6 @@
 			pinctrl-0 = < &qspi_default >;
 			pinctrl-1 = < &qspi_sleep >;
 			pinctrl-names = "default", "sleep";
-			gd25q16: gd25q16@0 {
-				compatible = "nordic,qspi-nor";
-				reg = < 0x0 >;
-				writeoc = "pp2o";
-				readoc = "read2io";
-				sck-frequency = < 0xf42400 >;
-				label = "GD25Q16";
-				jedec-id = [ C8 40 15 ];
-				size = < 0x200000 >;
-				has-dpd;
-				t-enter-dpd = < 0x4e20 >;
-				t-exit-dpd = < 0x4e20 >;
-			};
 			p25q16h: p25q16h@0 {
 				compatible = "nordic,qspi-nor";
 				reg = < 0x0 >;
@@ -634,11 +663,18 @@
 		};
 		led1: led_1 {
 			gpios = < &gpio0 0x1e 0x1 >;
-			label = "Blue LED";
+			label = "Green LED";
 		};
 		led2: led_2 {
 			gpios = < &gpio0 0x6 0x1 >;
-			label = "Green LED";
+			label = "Blue LED";
+		};
+	};
+	pwmleds {
+		compatible = "pwm-leds";
+		status = "disabled";
+		pwm_led0: pwm_led_0 {
+			pwms = < &pwm0 0x0 0x1312d00 0x1 >;
 		};
 	};
 	vbatt: vbatt {

Final Kconfig diff between two board DTS (for the logging build):

diff --git a/build/seeeduino_xiao_ble_debug/zephyr/.config b/build/xiao_ble_adj_debug/zephyr/.config
index 8d7a7592..9586fc22 100644
--- a/build/seeeduino_xiao_ble_debug/zephyr/.config
+++ b/build/xiao_ble_adj_debug/zephyr/.config
@@ -183,8 +183,10 @@ CONFIG_SHIELD_HUMMINGBIRD=y
 # CONFIG_I2C is not set
 CONFIG_LV_DPI_DEF=130
 CONFIG_SETTINGS=y
-CONFIG_BOARD="seeeduino_xiao_ble"
+CONFIG_BOARD="xiao_ble"
 CONFIG_BT_CTLR=y
+CONFIG_UART_CONSOLE=y
+CONFIG_USB_DEVICE_INITIALIZE_AT_BOOT=y
 # CONFIG_SPI is not set
 # CONFIG_INPUT is not set
 # CONFIG_WIFI is not set
@@ -472,9 +474,9 @@ CONFIG_NRFX_QSPI=y
 # CONFIG_NRFX_TIMER2 is not set
 # CONFIG_NRFX_TIMER3 is not set
 # CONFIG_NRFX_TIMER4 is not set
-# CONFIG_NRFX_TWI0 is not set
-# CONFIG_NRFX_TWIM1 is not set
-# CONFIG_NRFX_UART0 is not set
+# CONFIG_NRFX_TWI1 is not set
+# CONFIG_NRFX_TWIM0 is not set
+# CONFIG_NRFX_UARTE0 is not set
 # CONFIG_NRFX_UARTE1 is not set
 CONFIG_NRFX_USBD=y
 CONFIG_NRFX_USBD_ISO_IN_ZLP=y
@@ -939,12 +941,13 @@ CONFIG_TINYCRYPT_AES_CMAC=y
 
 CONFIG_BOARD_REVISION=""
 # CONFIG_NET_DRIVERS is not set
-CONFIG_BOARD_SEEEDUINO_XIAO_BLE=y
+CONFIG_BOARD_XIAO_BLE=y
 
 #
 # Board Options
 #
 CONFIG_BOARD_ENABLE_DCDC=y
+CONFIG_BOARD_ENABLE_DCDC_HV=y
 # end of Board Options
 
 # CONFIG_SOC_SERIES_APOLLO4X is not set
@@ -1129,6 +1132,7 @@ CONFIG_SOC_NRF52840=y
 # CONFIG_SOC_NRF52840_QFAA is not set
 CONFIG_SOC_NRF52840_QIAA=y
 CONFIG_SOC_DCDC_NRF52X=y
+CONFIG_SOC_DCDC_NRF52X_HV=y
 # CONFIG_GPIO_AS_PINRESET is not set
 CONFIG_NRF_ENABLE_ICACHE=y
 CONFIG_NRF_RTC_TIMER_USER_CHAN_COUNT=0
@@ -1469,7 +1473,6 @@ CONFIG_CONSOLE_INPUT_MAX_LINE_LEN=128
 CONFIG_CONSOLE_HAS_DRIVER=y
 # CONFIG_CONSOLE_HANDLER is not set
 CONFIG_CONSOLE_INIT_PRIORITY=60
-CONFIG_UART_CONSOLE=y
 # CONFIG_UART_CONSOLE_DEBUG_SERVER_HOOKS is not set
 # CONFIG_UART_CONSOLE_MCUMGR is not set
 # CONFIG_RAM_CONSOLE is not set
@@ -3235,7 +3238,6 @@ CONFIG_USB_MAX_POWER=50
 CONFIG_USB_WORKQUEUE=y
 CONFIG_USB_WORKQUEUE_STACK_SIZE=1024
 CONFIG_USB_WORKQUEUE_PRIORITY=-1
-# CONFIG_USB_DEVICE_INITIALIZE_AT_BOOT is not set
 
 #
 # USB CDC ACM Class support

(Currently pending testing on keyboards, hence the draft state.)

PR check-list

  • Branch has a clean commit history
  • Additional tests are included, if changing behaviors/core code that is testable.
  • Proper Copyright + License headers added to applicable files (Generally, we stick to "The ZMK Contributors" for copyrights to help avoid churn when files get edited)
  • Pre-commit used to check formatting of files, commit messages, etc.
  • Includes any necessary documentation changes.

@caksoylar caksoylar added the board PRs and issues related to boards. label Jan 4, 2025
@Nick-Munnich
Copy link
Contributor

The xiao does not use HV mode for the nrf52. Looking at mainline Zephyr's board definition they don't use HV there either. I think the board definition that we have in our version of Zephyr has an error.

@caksoylar
Copy link
Contributor Author

Yeah that's a bit weird, I guess no one noticed it in the initial PR? I can't find where exactly it gets removed, seems like it might be the hardware v2 migration.

If so, with all the issues with xiao_ble definition this PR is not very useful. We can maybe switch to the new board along with Zephyr 3.7 etc. Perhaps the overlay tweaks here will come in useful at that point.

@Nick-Munnich
Copy link
Contributor

It looks like that was fixed in 4.0, actually, so we may keep ours around until we upgrade beyond 3.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
board PRs and issues related to boards.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants