From 87476e5dc344a32f4f00bba101f0bb3fdabfa969 Mon Sep 17 00:00:00 2001 From: Erik Trinkle Date: Thu, 23 Jan 2025 15:36:11 -0500 Subject: [PATCH 1/7] add verification of startup prior to next step --- src/pmw3610.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/pmw3610.c b/src/pmw3610.c index f922cc3..90da123 100644 --- a/src/pmw3610.c +++ b/src/pmw3610.c @@ -231,7 +231,21 @@ static int pmw3610_async_init_power_up(const struct device *dev) { if (ret < 0) { return ret; } - return 0; + + /* Poll for sensor ready.*/ + for (int i = 0; i < 5000; i++) { + uint8_t product_id = 0; + ret = pmw3610_read_reg(dev, PMW3610_REG_PRODUCT_ID, &product_id); + if (ret == 0 && product_id == PMW3610_PRODUCT_ID) { + LOG_INF("PMW3610 awake, product ID confirmed: 0x%02x", product_id); + return 0; + } + k_sleep(K_MSEC(1)); + } + + /* If we never get the expected ID, fail out here. */ + LOG_ERR("PMW3610 did not respond with correct product ID after 5000 ms"); + return -EIO; } static int pmw3610_async_init_clear_ob1(const struct device *dev) { From 8950395ee05f268d08e729da41207c02b0d81495 Mon Sep 17 00:00:00 2001 From: Erik Trinkle Date: Sat, 25 Jan 2025 13:30:01 -0500 Subject: [PATCH 2/7] Revert "add verification of startup prior to next step" This reverts commit 87476e5dc344a32f4f00bba101f0bb3fdabfa969. --- src/pmw3610.c | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/src/pmw3610.c b/src/pmw3610.c index 90da123..f922cc3 100644 --- a/src/pmw3610.c +++ b/src/pmw3610.c @@ -231,21 +231,7 @@ static int pmw3610_async_init_power_up(const struct device *dev) { if (ret < 0) { return ret; } - - /* Poll for sensor ready.*/ - for (int i = 0; i < 5000; i++) { - uint8_t product_id = 0; - ret = pmw3610_read_reg(dev, PMW3610_REG_PRODUCT_ID, &product_id); - if (ret == 0 && product_id == PMW3610_PRODUCT_ID) { - LOG_INF("PMW3610 awake, product ID confirmed: 0x%02x", product_id); - return 0; - } - k_sleep(K_MSEC(1)); - } - - /* If we never get the expected ID, fail out here. */ - LOG_ERR("PMW3610 did not respond with correct product ID after 5000 ms"); - return -EIO; + return 0; } static int pmw3610_async_init_clear_ob1(const struct device *dev) { From 917acdbfafc96f003622629580dd2acfc3348470 Mon Sep 17 00:00:00 2001 From: Erik Trinkle Date: Sat, 25 Jan 2025 12:47:52 -0500 Subject: [PATCH 3/7] restart the whole initialization if it fails --- Kconfig | 13 +++++++++++++ src/pixart.h | 1 + src/pmw3610.c | 17 +++++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/Kconfig b/Kconfig index 85d7495..ef624f6 100644 --- a/Kconfig +++ b/Kconfig @@ -29,6 +29,19 @@ config PMW3610_INIT_POWER_UP_EXTRA_DELAY_MS Default minimum init power up delay is 10ms. Use this config to postpone init power up sequence if needs longer bootup time. +config PMW3610_INIT_RETRY_COUNT + int "Number of times to retry PMW3610 init" + default 10 + help + How many times the driver should attempt to re-initialize + the PMW3610 sensor if initialization fails. + +config PMW3610_INIT_RETRY_DELAY_MS + int "Delay between retries (ms)" + default 500 + help + How long the driver waits between retry attempts in milliseconds. + config PMW3610_REPORT_INTERVAL_MIN int "PMW3610's default minimum report rate" default 0 diff --git a/src/pixart.h b/src/pixart.h index d26f5a5..2388959 100644 --- a/src/pixart.h +++ b/src/pixart.h @@ -29,6 +29,7 @@ struct pixart_data { bool ready; // whether init is finished successfully bool last_read_burst; int err; // error code during async init + int retries; // number of retries of the initialization sequence }; // device config data structure diff --git a/src/pmw3610.c b/src/pmw3610.c index f922cc3..ad974b2 100644 --- a/src/pmw3610.c +++ b/src/pmw3610.c @@ -340,6 +340,23 @@ static void pmw3610_async_init(struct k_work *work) { data->err = async_init_fn[data->async_init_step](dev); if (data->err) { LOG_ERR("PMW3610 initialization failed in step %d", data->async_init_step); + + /* If we haven’t exceeded our retry budget, reset to step 0 and try again */ + if (data->retries < CONFIG_PMW3610_INIT_RETRY_COUNT) { + data->retries++; + data->async_init_step = 0; + LOG_WRN("Retrying PMW3610 init (attempt %d/%d) in %d ms...", + data->retries, + CONFIG_PMW3610_INIT_RETRY_COUNT, + CONFIG_PMW3610_INIT_RETRY_DELAY_MS); + + /* Schedule the next attempt after a delay */ + k_work_schedule(&data->init_work, + K_MSEC(CONFIG_PMW3610_INIT_RETRY_DELAY_MS)); + } else { + LOG_ERR("PMW3610 init failed after %d retries; giving up.", data->retries); + /* At this point, we do NOT re-schedule, so init stops. data->ready stays false */ + } } else { data->async_init_step++; From 1c99fe6d6ff703c842ff79892ae0b70ef14bfb59 Mon Sep 17 00:00:00 2001 From: Erik Trinkle Date: Sat, 25 Jan 2025 13:44:09 -0500 Subject: [PATCH 4/7] remove extra startup delay config retries and delays can be adjusted instead if more than 5 seconds (50 tries) are needed. --- Kconfig | 7 ------- README.md | 5 ----- src/pmw3610.c | 2 +- 3 files changed, 1 insertion(+), 13 deletions(-) diff --git a/Kconfig b/Kconfig index ef624f6..e32c29b 100644 --- a/Kconfig +++ b/Kconfig @@ -22,13 +22,6 @@ config PMW3610_INVERT_X config PMW3610_INVERT_Y bool "Invert the Y axis of PMW3610 sensor" -config PMW3610_INIT_POWER_UP_EXTRA_DELAY_MS - int "Extra power up init delay (ms)" - default 0 - help - Default minimum init power up delay is 10ms. - Use this config to postpone init power up sequence if needs longer bootup time. - config PMW3610_INIT_RETRY_COUNT int "Number of times to retry PMW3610 init" default 10 diff --git a/README.md b/README.md index 8b518f0..36b6045 100644 --- a/README.md +++ b/README.md @@ -99,9 +99,4 @@ CONFIG_PMW3610=y # CONFIG_PMW3610_INVERT_Y=y # CONFIG_PMW3610_REPORT_INTERVAL_MIN=12 # CONFIG_PMW3610_LOG_LEVEL_DBG=y -# CONFIG_PMW3610_INIT_POWER_UP_EXTRA_DELAY_MS=300 // <--see Troubleshooting ``` - -## Troubleshooting - -If you are getting `Incorrect product id 0xFF (expecting 0x3E)!` on `nice_nano_v2` board from the log, you'd want to apply `CONFIG_PMW3610_INIT_POWER_UP_EXTRA_DELAY_MS=1000` in your shield .conf/.overlay file. Due to this driver doesn't offer module dependancy setting, that would ensure external power (to enable VCC pin on board) is ready, the `CONFIG_PMW3610_INIT_POWER_UP_EXTRA_DELAY_MS` would use to add extra one second delay of power up. diff --git a/src/pmw3610.c b/src/pmw3610.c index ad974b2..f4a5ff6 100644 --- a/src/pmw3610.c +++ b/src/pmw3610.c @@ -32,7 +32,7 @@ enum pmw3610_init_step { // - Since MCU is not involved in the sensor init process, i is allowed to do other tasks. // Thus, k_sleep or delayed schedule can be used. static const int32_t async_init_delay[ASYNC_INIT_STEP_COUNT] = { - [ASYNC_INIT_STEP_POWER_UP] = 10 + CONFIG_PMW3610_INIT_POWER_UP_EXTRA_DELAY_MS, // >10ms needed + [ASYNC_INIT_STEP_POWER_UP] = 10, // >10ms needed [ASYNC_INIT_STEP_CLEAR_OB1] = 200, // 150 us required, test shows too short, // also power-up reset is added in this step, thus using 50 ms [ASYNC_INIT_STEP_CHECK_OB1] = 50, // 10 ms required in spec, From 22e73b8d7ccfb6f50080fbd35bc7fba4eb54d279 Mon Sep 17 00:00:00 2001 From: Erik Trinkle Date: Sat, 25 Jan 2025 13:45:55 -0500 Subject: [PATCH 5/7] shorter default initialization retry delay --- Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Kconfig b/Kconfig index e32c29b..a2e1a20 100644 --- a/Kconfig +++ b/Kconfig @@ -24,14 +24,14 @@ config PMW3610_INVERT_Y config PMW3610_INIT_RETRY_COUNT int "Number of times to retry PMW3610 init" - default 10 + default 50 help How many times the driver should attempt to re-initialize the PMW3610 sensor if initialization fails. config PMW3610_INIT_RETRY_DELAY_MS int "Delay between retries (ms)" - default 500 + default 100 help How long the driver waits between retry attempts in milliseconds. From 2433c783daa9979dee07bd084aaa1573fc9ca8cf Mon Sep 17 00:00:00 2001 From: Erik Trinkle Date: Sat, 25 Jan 2025 13:46:20 -0500 Subject: [PATCH 6/7] list new configuration options for initialization retries. --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 36b6045..ac4fcf7 100644 --- a/README.md +++ b/README.md @@ -99,4 +99,6 @@ CONFIG_PMW3610=y # CONFIG_PMW3610_INVERT_Y=y # CONFIG_PMW3610_REPORT_INTERVAL_MIN=12 # CONFIG_PMW3610_LOG_LEVEL_DBG=y +# CONFIG_PMW3610_INIT_RETRY_COUNT=50 +# CONFIG_PMW3610_INIT_RETRY_DELAY_MS=100 ``` From fa94a3dbfd2eb2ce73f417c687d6750804aecee3 Mon Sep 17 00:00:00 2001 From: Crush53 Date: Thu, 11 Dec 2025 09:18:34 -0800 Subject: [PATCH 7/7] fix: rename compatible string to avoid Zephyr 4.1 conflict --- dts/bindings/{pixart,pmw3610.yml => cyboard,pmw3610.yml} | 4 ++-- src/pmw3610.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) rename dts/bindings/{pixart,pmw3610.yml => cyboard,pmw3610.yml} (86%) diff --git a/dts/bindings/pixart,pmw3610.yml b/dts/bindings/cyboard,pmw3610.yml similarity index 86% rename from dts/bindings/pixart,pmw3610.yml rename to dts/bindings/cyboard,pmw3610.yml index 2400a47..c515fd4 100644 --- a/dts/bindings/pixart,pmw3610.yml +++ b/dts/bindings/cyboard,pmw3610.yml @@ -1,7 +1,7 @@ description: | - Sensor driver for the pixart PMW3610 mouse sensor + Sensor driver for the pixart PMW3610 mouse sensor -compatible: "pixart,pmw3610" +compatible: "cyboard,pmw3610" include: spi-device.yaml diff --git a/src/pmw3610.c b/src/pmw3610.c index 3ea0117..8242ade 100644 --- a/src/pmw3610.c +++ b/src/pmw3610.c @@ -4,7 +4,7 @@ * SPDX-License-Identifier: MIT */ -#define DT_DRV_COMPAT pixart_pmw3610 +#define DT_DRV_COMPAT cyboard_pmw3610 #include #include @@ -746,7 +746,7 @@ DT_INST_FOREACH_STATUS_OKAY(PMW3610_DEFINE) #define GET_PMW3610_DEV(node_id) DEVICE_DT_GET(node_id), static const struct device *pmw3610_devs[] = { - DT_FOREACH_STATUS_OKAY(pixart_pmw3610, GET_PMW3610_DEV) + DT_FOREACH_STATUS_OKAY(cyboard_pmw3610, GET_PMW3610_DEV) }; static int on_activity_state(const zmk_event_t *eh) {