From a2bbca1428efc334243e1ac474244d667d5854bd Mon Sep 17 00:00:00 2001 From: RetiredWizard Date: Thu, 9 Feb 2023 01:14:01 -0500 Subject: [PATCH 01/12] Broadcom Raspberry Pi Zero2W neopixel timing fix These changes result in working neopixel functionality. I've tested on both the zero2w and the pi4b (The 4b didn't exhibit the original issue) and the boards now behave properly with 1 to 30 pixels and the board hanging no longer occurs. Remove mod that didn't help during testing Restoring back to original structure Replace 2 microsecond delay w/deterministic loop Remove unneded check for empty queue Put transmit delay outside loop so Queue is used Make sure last transmission is complete --- .../common-hal/neopixel_write/__init__.c | 40 +++++++++++++++---- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/ports/broadcom/common-hal/neopixel_write/__init__.c b/ports/broadcom/common-hal/neopixel_write/__init__.c index 0cd76ebca9..822c61e36e 100644 --- a/ports/broadcom/common-hal/neopixel_write/__init__.c +++ b/ports/broadcom/common-hal/neopixel_write/__init__.c @@ -45,7 +45,11 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, uint8_t *pixels, uint32_t num_bytes) { // Wait to make sure we don't append onto the last transmission. This should only be a tick or // two. - while (port_get_raw_ticks(NULL) < next_start_raw_ticks) { + int icnt; + while ((port_get_raw_ticks(NULL) < next_start_raw_ticks) & + (next_start_raw_ticks-port_get_raw_ticks(NULL) < 100)) { + + RUN_BACKGROUND_TASKS; } BP_Function_Enum alt_function = GPIO_FUNCTION_OUTPUT; @@ -92,7 +96,8 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, // Wait for the clock to start up. COMPLETE_MEMORY_READS; - while (CM_PWM->CS_b.BUSY == 0) { + icnt = 0; + while ((CM_PWM->CS_b.BUSY == 0) & (icnt++ < 1000)) { } } @@ -134,22 +139,41 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, expanded |= 0x80000000; } } - while (pwm->STA_b.FULL1 == 1) { - RUN_BACKGROUND_TASKS; - } if (channel == 1) { + icnt=0; + while ((pwm->STA_b.FULL1 == 1) & (icnt++ < 150)) { + RUN_BACKGROUND_TASKS; + } // Dummy value for the first channel. pwm->FIF1 = 0x000000; } + icnt=0; + while ((pwm->STA_b.FULL1 == 1) & (icnt++ < 150)) { + RUN_BACKGROUND_TASKS; + } pwm->FIF1 = expanded; if (channel == 0) { + icnt=0; + while ((pwm->STA_b.FULL1 == 1) & (icnt++ < 150)) { + RUN_BACKGROUND_TASKS; + } // Dummy value for the second channel. pwm->FIF1 = 0x000000; } } - // Wait just a little bit so that transmission can start. - common_hal_mcu_delay_us(2); - while (pwm->STA_b.STA1 == 1) { + + icnt = 0; + while ((pwm->STA_b.EMPT1 == 0) & (icnt++ < 2500)) { + RUN_BACKGROUND_TASKS; + } + // Wait for transmission to start. + icnt = 0; + while (((pwm->STA_b.STA1 ==0) & (pwm->STA_b.STA2 == 0)) & (icnt++ < 150)) { + RUN_BACKGROUND_TASKS; + } + // Wait for transmission to complete. + icnt = 0; + while (((pwm->STA_b.STA1 == 1) | (pwm->STA_b.STA2 == 1)) & (icnt++ < 150)) { RUN_BACKGROUND_TASKS; } From a462a316bddb3860c3bc5e352084a03d31d08650 Mon Sep 17 00:00:00 2001 From: RetiredWizard Date: Sat, 11 Feb 2023 12:10:03 -0500 Subject: [PATCH 02/12] Fix pre-commit formatting --- ports/broadcom/common-hal/neopixel_write/__init__.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ports/broadcom/common-hal/neopixel_write/__init__.c b/ports/broadcom/common-hal/neopixel_write/__init__.c index 822c61e36e..60b288eb28 100644 --- a/ports/broadcom/common-hal/neopixel_write/__init__.c +++ b/ports/broadcom/common-hal/neopixel_write/__init__.c @@ -47,7 +47,7 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, // two. int icnt; while ((port_get_raw_ticks(NULL) < next_start_raw_ticks) & - (next_start_raw_ticks-port_get_raw_ticks(NULL) < 100)) { + (next_start_raw_ticks - port_get_raw_ticks(NULL) < 100)) { RUN_BACKGROUND_TASKS; } @@ -140,20 +140,20 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, } } if (channel == 1) { - icnt=0; + icnt = 0; while ((pwm->STA_b.FULL1 == 1) & (icnt++ < 150)) { RUN_BACKGROUND_TASKS; } // Dummy value for the first channel. pwm->FIF1 = 0x000000; } - icnt=0; + icnt = 0; while ((pwm->STA_b.FULL1 == 1) & (icnt++ < 150)) { RUN_BACKGROUND_TASKS; } pwm->FIF1 = expanded; if (channel == 0) { - icnt=0; + icnt = 0; while ((pwm->STA_b.FULL1 == 1) & (icnt++ < 150)) { RUN_BACKGROUND_TASKS; } @@ -168,7 +168,7 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, } // Wait for transmission to start. icnt = 0; - while (((pwm->STA_b.STA1 ==0) & (pwm->STA_b.STA2 == 0)) & (icnt++ < 150)) { + while (((pwm->STA_b.STA1 == 0) & (pwm->STA_b.STA2 == 0)) & (icnt++ < 150)) { RUN_BACKGROUND_TASKS; } // Wait for transmission to complete. From f29cd4a836eb97cb589818932b55b002468e964e Mon Sep 17 00:00:00 2001 From: RetiredWizard Date: Sat, 11 Feb 2023 12:13:34 -0500 Subject: [PATCH 03/12] Pre-commit fix trim trailing whitespace --- ports/broadcom/common-hal/neopixel_write/__init__.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ports/broadcom/common-hal/neopixel_write/__init__.c b/ports/broadcom/common-hal/neopixel_write/__init__.c index 60b288eb28..023feb995f 100644 --- a/ports/broadcom/common-hal/neopixel_write/__init__.c +++ b/ports/broadcom/common-hal/neopixel_write/__init__.c @@ -46,7 +46,7 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, // Wait to make sure we don't append onto the last transmission. This should only be a tick or // two. int icnt; - while ((port_get_raw_ticks(NULL) < next_start_raw_ticks) & + while ((port_get_raw_ticks(NULL) < next_start_raw_ticks) & (next_start_raw_ticks - port_get_raw_ticks(NULL) < 100)) { RUN_BACKGROUND_TASKS; From 2104708c58c881a7ecf0b972245cc3ffcb48901d Mon Sep 17 00:00:00 2001 From: RetiredWizard Date: Sat, 11 Feb 2023 12:17:55 -0500 Subject: [PATCH 04/12] missed a trailing space --- ports/broadcom/common-hal/neopixel_write/__init__.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ports/broadcom/common-hal/neopixel_write/__init__.c b/ports/broadcom/common-hal/neopixel_write/__init__.c index 023feb995f..38519a0232 100644 --- a/ports/broadcom/common-hal/neopixel_write/__init__.c +++ b/ports/broadcom/common-hal/neopixel_write/__init__.c @@ -48,7 +48,7 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, int icnt; while ((port_get_raw_ticks(NULL) < next_start_raw_ticks) & (next_start_raw_ticks - port_get_raw_ticks(NULL) < 100)) { - + RUN_BACKGROUND_TASKS; } From 359a27e166617260d75dc6ef6769114ebac03d19 Mon Sep 17 00:00:00 2001 From: RetiredWizard Date: Sat, 11 Feb 2023 12:19:21 -0500 Subject: [PATCH 05/12] Pre-commit is stubborn --- ports/broadcom/common-hal/neopixel_write/__init__.c | 1 - 1 file changed, 1 deletion(-) diff --git a/ports/broadcom/common-hal/neopixel_write/__init__.c b/ports/broadcom/common-hal/neopixel_write/__init__.c index 38519a0232..5629c97425 100644 --- a/ports/broadcom/common-hal/neopixel_write/__init__.c +++ b/ports/broadcom/common-hal/neopixel_write/__init__.c @@ -48,7 +48,6 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, int icnt; while ((port_get_raw_ticks(NULL) < next_start_raw_ticks) & (next_start_raw_ticks - port_get_raw_ticks(NULL) < 100)) { - RUN_BACKGROUND_TASKS; } From 1679790481a80270e6d79bb7fe069cd71fe46488 Mon Sep 17 00:00:00 2001 From: RetiredWizard Date: Sat, 11 Feb 2023 20:21:25 -0500 Subject: [PATCH 06/12] Tweaked to run without delays on zero w --- ports/broadcom/common-hal/neopixel_write/__init__.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ports/broadcom/common-hal/neopixel_write/__init__.c b/ports/broadcom/common-hal/neopixel_write/__init__.c index 5629c97425..d685ac53ce 100644 --- a/ports/broadcom/common-hal/neopixel_write/__init__.c +++ b/ports/broadcom/common-hal/neopixel_write/__init__.c @@ -97,6 +97,7 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, COMPLETE_MEMORY_READS; icnt = 0; while ((CM_PWM->CS_b.BUSY == 0) & (icnt++ < 1000)) { + COMPLETE_MEMORY_READS; } } @@ -142,6 +143,7 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, icnt = 0; while ((pwm->STA_b.FULL1 == 1) & (icnt++ < 150)) { RUN_BACKGROUND_TASKS; + COMPLETE_MEMORY_READS; } // Dummy value for the first channel. pwm->FIF1 = 0x000000; @@ -149,12 +151,14 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, icnt = 0; while ((pwm->STA_b.FULL1 == 1) & (icnt++ < 150)) { RUN_BACKGROUND_TASKS; + COMPLETE_MEMORY_READS; } pwm->FIF1 = expanded; if (channel == 0) { icnt = 0; while ((pwm->STA_b.FULL1 == 1) & (icnt++ < 150)) { RUN_BACKGROUND_TASKS; + COMPLETE_MEMORY_READS; } // Dummy value for the second channel. pwm->FIF1 = 0x000000; @@ -164,17 +168,23 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, icnt = 0; while ((pwm->STA_b.EMPT1 == 0) & (icnt++ < 2500)) { RUN_BACKGROUND_TASKS; + COMPLETE_MEMORY_READS; } // Wait for transmission to start. icnt = 0; while (((pwm->STA_b.STA1 == 0) & (pwm->STA_b.STA2 == 0)) & (icnt++ < 150)) { RUN_BACKGROUND_TASKS; + COMPLETE_MEMORY_READS; } // Wait for transmission to complete. icnt = 0; while (((pwm->STA_b.STA1 == 1) | (pwm->STA_b.STA2 == 1)) & (icnt++ < 150)) { RUN_BACKGROUND_TASKS; + COMPLETE_MEMORY_READS; } + // Shouldn't be anything left in queue but clear it so the clock doesn't crash if there is + pwm->CTL = PWM0_CTL_CLRF1_Msk; + COMPLETE_MEMORY_READS; gpio_set_function(digitalinout->pin->number, GPIO_FUNCTION_OUTPUT); From 575f177dd09f0f518f7e2c8514b20e9b12c19af8 Mon Sep 17 00:00:00 2001 From: RetiredWizard Date: Mon, 13 Feb 2023 14:44:46 -0500 Subject: [PATCH 07/12] Update ports/broadcom/common-hal/neopixel_write/__init__.c Co-authored-by: Scott Shawcroft --- ports/broadcom/common-hal/neopixel_write/__init__.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ports/broadcom/common-hal/neopixel_write/__init__.c b/ports/broadcom/common-hal/neopixel_write/__init__.c index d685ac53ce..38b1028bb3 100644 --- a/ports/broadcom/common-hal/neopixel_write/__init__.c +++ b/ports/broadcom/common-hal/neopixel_write/__init__.c @@ -96,7 +96,7 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, // Wait for the clock to start up. COMPLETE_MEMORY_READS; icnt = 0; - while ((CM_PWM->CS_b.BUSY == 0) & (icnt++ < 1000)) { + while ((CM_PWM->CS_b.BUSY == 0) && (icnt++ < 1000)) { COMPLETE_MEMORY_READS; } } From 791aefd3880d8c0f58baa2e9cd7cc40815d05059 Mon Sep 17 00:00:00 2001 From: RetiredWizard Date: Mon, 13 Feb 2023 14:44:54 -0500 Subject: [PATCH 08/12] Update ports/broadcom/common-hal/neopixel_write/__init__.c Co-authored-by: Scott Shawcroft --- ports/broadcom/common-hal/neopixel_write/__init__.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ports/broadcom/common-hal/neopixel_write/__init__.c b/ports/broadcom/common-hal/neopixel_write/__init__.c index 38b1028bb3..b631953c50 100644 --- a/ports/broadcom/common-hal/neopixel_write/__init__.c +++ b/ports/broadcom/common-hal/neopixel_write/__init__.c @@ -46,7 +46,7 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, // Wait to make sure we don't append onto the last transmission. This should only be a tick or // two. int icnt; - while ((port_get_raw_ticks(NULL) < next_start_raw_ticks) & + while ((port_get_raw_ticks(NULL) < next_start_raw_ticks) && (next_start_raw_ticks - port_get_raw_ticks(NULL) < 100)) { RUN_BACKGROUND_TASKS; } From dcb6955fa5a8619e222109a334bdd31f377fd337 Mon Sep 17 00:00:00 2001 From: RetiredWizard Date: Mon, 13 Feb 2023 14:58:37 -0500 Subject: [PATCH 09/12] use boolean and symbols --- ports/broadcom/common-hal/neopixel_write/__init__.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ports/broadcom/common-hal/neopixel_write/__init__.c b/ports/broadcom/common-hal/neopixel_write/__init__.c index b631953c50..b42bfb9de8 100644 --- a/ports/broadcom/common-hal/neopixel_write/__init__.c +++ b/ports/broadcom/common-hal/neopixel_write/__init__.c @@ -141,7 +141,7 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, } if (channel == 1) { icnt = 0; - while ((pwm->STA_b.FULL1 == 1) & (icnt++ < 150)) { + while ((pwm->STA_b.FULL1 == 1) && (icnt++ < 150)) { RUN_BACKGROUND_TASKS; COMPLETE_MEMORY_READS; } @@ -149,14 +149,14 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, pwm->FIF1 = 0x000000; } icnt = 0; - while ((pwm->STA_b.FULL1 == 1) & (icnt++ < 150)) { + while ((pwm->STA_b.FULL1 == 1) && (icnt++ < 150)) { RUN_BACKGROUND_TASKS; COMPLETE_MEMORY_READS; } pwm->FIF1 = expanded; if (channel == 0) { icnt = 0; - while ((pwm->STA_b.FULL1 == 1) & (icnt++ < 150)) { + while ((pwm->STA_b.FULL1 == 1) && (icnt++ < 150)) { RUN_BACKGROUND_TASKS; COMPLETE_MEMORY_READS; } @@ -166,19 +166,19 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, } icnt = 0; - while ((pwm->STA_b.EMPT1 == 0) & (icnt++ < 2500)) { + while ((pwm->STA_b.EMPT1 == 0) && (icnt++ < 2500)) { RUN_BACKGROUND_TASKS; COMPLETE_MEMORY_READS; } // Wait for transmission to start. icnt = 0; - while (((pwm->STA_b.STA1 == 0) & (pwm->STA_b.STA2 == 0)) & (icnt++ < 150)) { + while (((pwm->STA_b.STA1 == 0) && (pwm->STA_b.STA2 == 0)) && (icnt++ < 150)) { RUN_BACKGROUND_TASKS; COMPLETE_MEMORY_READS; } // Wait for transmission to complete. icnt = 0; - while (((pwm->STA_b.STA1 == 1) | (pwm->STA_b.STA2 == 1)) & (icnt++ < 150)) { + while (((pwm->STA_b.STA1 == 1) | (pwm->STA_b.STA2 == 1)) && (icnt++ < 150)) { RUN_BACKGROUND_TASKS; COMPLETE_MEMORY_READS; } From 39d3d97ea4a0805dd12294728f16bca4a6301fbd Mon Sep 17 00:00:00 2001 From: RetiredWizard Date: Mon, 13 Feb 2023 19:46:35 -0500 Subject: [PATCH 10/12] Remove unnecessary memory barriers --- ports/broadcom/common-hal/neopixel_write/__init__.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/ports/broadcom/common-hal/neopixel_write/__init__.c b/ports/broadcom/common-hal/neopixel_write/__init__.c index b42bfb9de8..f5c3c245f6 100644 --- a/ports/broadcom/common-hal/neopixel_write/__init__.c +++ b/ports/broadcom/common-hal/neopixel_write/__init__.c @@ -96,9 +96,7 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, // Wait for the clock to start up. COMPLETE_MEMORY_READS; icnt = 0; - while ((CM_PWM->CS_b.BUSY == 0) && (icnt++ < 1000)) { - COMPLETE_MEMORY_READS; - } + while ((CM_PWM->CS_b.BUSY == 0) && (icnt++ < 1000)) {} } PWM0_Type *pwm = PWM0; @@ -143,7 +141,6 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, icnt = 0; while ((pwm->STA_b.FULL1 == 1) && (icnt++ < 150)) { RUN_BACKGROUND_TASKS; - COMPLETE_MEMORY_READS; } // Dummy value for the first channel. pwm->FIF1 = 0x000000; @@ -151,14 +148,12 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, icnt = 0; while ((pwm->STA_b.FULL1 == 1) && (icnt++ < 150)) { RUN_BACKGROUND_TASKS; - COMPLETE_MEMORY_READS; } pwm->FIF1 = expanded; if (channel == 0) { icnt = 0; while ((pwm->STA_b.FULL1 == 1) && (icnt++ < 150)) { RUN_BACKGROUND_TASKS; - COMPLETE_MEMORY_READS; } // Dummy value for the second channel. pwm->FIF1 = 0x000000; @@ -168,23 +163,19 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, icnt = 0; while ((pwm->STA_b.EMPT1 == 0) && (icnt++ < 2500)) { RUN_BACKGROUND_TASKS; - COMPLETE_MEMORY_READS; } // Wait for transmission to start. icnt = 0; while (((pwm->STA_b.STA1 == 0) && (pwm->STA_b.STA2 == 0)) && (icnt++ < 150)) { RUN_BACKGROUND_TASKS; - COMPLETE_MEMORY_READS; } // Wait for transmission to complete. icnt = 0; while (((pwm->STA_b.STA1 == 1) | (pwm->STA_b.STA2 == 1)) && (icnt++ < 150)) { RUN_BACKGROUND_TASKS; - COMPLETE_MEMORY_READS; } // Shouldn't be anything left in queue but clear it so the clock doesn't crash if there is pwm->CTL = PWM0_CTL_CLRF1_Msk; - COMPLETE_MEMORY_READS; gpio_set_function(digitalinout->pin->number, GPIO_FUNCTION_OUTPUT); From 09ccf2988d78a387f19ff0a9db7413fa8b62a926 Mon Sep 17 00:00:00 2001 From: RetiredWizard Date: Mon, 13 Feb 2023 21:04:11 -0500 Subject: [PATCH 11/12] Replace bitwise or with boolean or --- ports/broadcom/common-hal/neopixel_write/__init__.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ports/broadcom/common-hal/neopixel_write/__init__.c b/ports/broadcom/common-hal/neopixel_write/__init__.c index f5c3c245f6..aa256c7413 100644 --- a/ports/broadcom/common-hal/neopixel_write/__init__.c +++ b/ports/broadcom/common-hal/neopixel_write/__init__.c @@ -171,7 +171,7 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, } // Wait for transmission to complete. icnt = 0; - while (((pwm->STA_b.STA1 == 1) | (pwm->STA_b.STA2 == 1)) && (icnt++ < 150)) { + while (((pwm->STA_b.STA1 == 1) || (pwm->STA_b.STA2 == 1)) && (icnt++ < 150)) { RUN_BACKGROUND_TASKS; } // Shouldn't be anything left in queue but clear it so the clock doesn't crash if there is From 6ebb911a4dde0801c9fd5f56da7ee43c51375fd5 Mon Sep 17 00:00:00 2001 From: RetiredWizard Date: Mon, 13 Feb 2023 21:06:47 -0500 Subject: [PATCH 12/12] pre-commit formatting fix --- ports/broadcom/common-hal/neopixel_write/__init__.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ports/broadcom/common-hal/neopixel_write/__init__.c b/ports/broadcom/common-hal/neopixel_write/__init__.c index aa256c7413..245244614f 100644 --- a/ports/broadcom/common-hal/neopixel_write/__init__.c +++ b/ports/broadcom/common-hal/neopixel_write/__init__.c @@ -96,7 +96,8 @@ void common_hal_neopixel_write(const digitalio_digitalinout_obj_t *digitalinout, // Wait for the clock to start up. COMPLETE_MEMORY_READS; icnt = 0; - while ((CM_PWM->CS_b.BUSY == 0) && (icnt++ < 1000)) {} + while ((CM_PWM->CS_b.BUSY == 0) && (icnt++ < 1000)) { + } } PWM0_Type *pwm = PWM0;