drivers: stepper: refactor ISR safe event mechanism

[jilaypandya]

Refactor ISR safe event handling mechanism from step_dir common to a dedicated module .

[dipakgmx]

I am not sure about the context of the ISR callbacks, so had 2 questions:

1. Why was this check for callback made, if the callback was called in ISR
2. It seems, right now, we are locking the data in spinlock and calling the callback, i mean something like this:

K_SPINLOCK(lock) {
stepper_handle_timing_signal -> 
  position_mode_task ->  
    update_remaining_steps -> 
      stepper_trigger_callback -> 
        data->callback
}

This, am not sure is the right approach. Guess, the callbacks need to be called from some work, rather than cascading the calls.

[jilaypandya]

I am not sure about the context of the ISR callbacks, so had 2 questions:

1. Why was this check for callback made, if the callback was called in ISR
2. It seems, right now, we are locking the data in spinlock and calling the callback, i mean something like this:

K_SPINLOCK(lock) {
stepper_handle_timing_signal -> 
  position_mode_task ->  
    update_remaining_steps -> 
      stepper_trigger_callback -> 
        data->callback
}

This, am not sure is the right approach. Guess, the callbacks need to be called from some work, rather than cascading the calls.

Yes, you are right, i have refactored the event handling out to a seperate module called stepper_event_handler, so that other drivers can benefit from it as well. Could you give a second look at the way i am using Iterable Sections?

[Copilot]

Pull Request Overview

This PR extracts the ISR-safe event queuing logic from the step_dir common code into a dedicated event_handler module and updates all stepper drivers to use the new stepper_post_event API.

• Moved message‐queue and work handler logic into a standalone stepper_event_handler module
• Added stepper_post_event function and iterable‐section registrations for each device
• Updated all drivers and sample code to call stepper_post_event and use STEPPER_DEVICE_DT_DEFINE

Reviewed Changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
samples/drivers/stepper/generic/src/main.c	Added debug log for STEPPER_EVENT_STEPS_COMPLETED
include/zephyr/drivers/stepper.h	Introduced macros and structs for STEPPER_EVENT_HANDLER
drivers/stepper/ti/Kconfig.drv84xx	Removed old ISR-safe event select
drivers/stepper/ti/drv84xx.c	Switched to STEPPER_DEVICE_DT_INST_DEFINE
drivers/stepper/step_dir/step_dir_stepper_common.h	Removed inlined ISR-safe event fields
drivers/stepper/step_dir/step_dir_stepper_common.c	Replaced local ISR dispatch with stepper_post_event
drivers/stepper/step_dir/Kconfig	Dropped legacy event-template include
drivers/stepper/gpio_stepper_controller.c	Switched to stepper_post_event calls and DT define
drivers/stepper/fake_stepper_controller.c	Switched to STEPPER_DEVICE_DT_INST_DEFINE
drivers/stepper/allegro/a4979.c	Switched to STEPPER_DEVICE_DT_INST_DEFINE
drivers/stepper/adi_tmc/tmc22xx.c	Switched to STEPPER_DEVICE_DT_INST_DEFINE
drivers/stepper/event_handler/stepper_event_handler.h	New public header for event handler API
drivers/stepper/event_handler/stepper_event_handler.c	New implementation of queued ISR-safe event dispatch
drivers/stepper/event_handler/iterables.ld	Linker script for iterable sections
drivers/stepper/event_handler/Kconfig	Kconfig for enabling/configuring the event handler
drivers/stepper/event_handler/CMakeLists.txt	CMake rules to build the event handler module
drivers/stepper/Kconfig	Hooked in new event_handler Kconfig
drivers/stepper/CMakeLists.txt	Added subdirectory for event_handler

Comments suppressed due to low confidence (4)

drivers/stepper/ti/Kconfig.drv84xx:9

• After removing the old config, the TI driver no longer selects the new STEPPER_EVENT_HANDLER option. You should add select STEPPER_EVENT_HANDLER to ensure the event handler is enabled.

-	select STEPPER_STEP_DIR_GENERATE_ISR_SAFE_EVENTS

drivers/stepper/event_handler/stepper_event_handler.h:6

• The header guard does not match the directory hierarchy. It should be ZEPHYR_DRIVERS_STEPPER_EVENT_HANDLER_H_ to align with the include path.

#ifndef ZEPHYR_DRIVER_STEPPER_STEPPER_EVENT_HANDLER_H_

include/zephyr/drivers/stepper.h:543

• The macros use STRUCT_SECTION_ITERABLE but this header does not include <zephyr/sys/iterable_sections.h>, which can lead to compilation errors. Add the necessary include.

#ifdef CONFIG_STEPPER_EVENT_HANDLER

drivers/stepper/step_dir/step_dir_stepper_common.c:6

• [nitpick] Use a project‐style include path (e.g., <zephyr/drivers/stepper/event_handler/stepper_event_handler.h>) instead of a relative path for consistency.

#include "../event_handler/stepper_event_handler.h"

[dipakgmx]

Shouldn't this be initialized with the driver?

[jilaypandya]

do you mean the init_prio should be CONFIG_STEPPER_INIT_PRIORITY?

[dipakgmx]

The headers can be moved to an include folder and include via zephyr_library_include_directories, so that it can be include directly rather than the #include "../event_handler/stepper_event_handler.h" form.

[dipakgmx]

Any reason to do this as an iterable section?

[jilaypandya]

My idea is that the event handling data structure is same for all the drivers, hence it could be registered for all the drivers through STEPPER_DEVICE_DT_INST_DEFINE, sensors also does it, for some other purpose though :)

And then via SYS_INIT one could initialize the workq and msgq in terms of event handling for all the drivers.

[dipakgmx]

Any drawback of having the struct stepper_event_handler, as a data member of the stepper driver? Then the members could be initialised during the driver init and at the same time use the common function from stepper_event_handler.c.

[jilaypandya]

Yeah but then each and every driver would have to do this explicitly. And in this way it happens in the background.

Also with CONFIG_STEPPER_EVENT_HANDLER=n, you could switch off this functionality and then the callback would just simply be called without going via the event_handler

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[faxe1008]

👍 for moving out the event handling to another file, but I feel like having this inside of an iterable section is a bit too much complexity for few gains. The only real advantage that I see is that the user does not have to call the stepper_event_handler_init manually.

[faxe1008]

This feels off imo, the driver should be ready for use after POST_KERNEL init level has run. I get the idea behind it, that drivers do not have to explicitly call an init function but the reduction in complexity would be worth it.

[jilaypandya]

👍 for moving out the event handling to another file, but I feel like having this inside of an iterable section is a bit too much complexity for few gains. The only real advantage that I see is that the user does not have to call the stepper_event_handler_init manually.

👍 I'll close the PR :)