drivers: stepper: introduce step-dir functions in stepper api to facilitate implementing motion controllers as device drivers #91979

jilaypandya · 2025-06-21T22:30:40Z

This PR aims to facilitate implementation of stepper motion controllers as device drivers. Hence two new functions as added, namely step and dir.

Note:

This is an intermediate step. The next step would be to create a stepper_step_dir_api with functions (enable/disable, step/dir, set_microstep_resolution) as mentioned in the RFC.
Test suite has been updated
1. Multiple Stepper drivers have been dropped from the stepper api test suite, since the concern is to test zephyr_stepper_motion_controller with stepper_api which consumes the "step-dir" interface in the future.

Side effects:

A lot of common code is refactored out and the stepper step-dir or h-bridge based drivers only do what the ic supports. i.e. enable/disable, set/get us resolution, step/dir.
Drv84xx handles fault events as of now, which is good. However, handling of fault event has to be refactored as mentioned in the RFC Split Stepper Motion Controller <-> Stepper Driver #89786 .
Introduce set_fault_event_callback, remove fault_event from the current stepper_event enum and rename stepper_event_callback to stepper_motion_control_event_callback.

This will have to be rebased on main, once this PR #91909 is merged.

faxe1008

Small little pass, will do some in depth review later on :^)

faxe1008 · 2025-06-22T12:59:59Z

drivers/stepper/adi_tmc/tmc22xx.c


 static int tmc22xx_stepper_enable(const struct device *dev)
 {
 	const struct tmc22xx_config *config = dev->config;

 	LOG_DBG("Enabling Stepper motor controller %s", dev->name);
-	return gpio_pin_set_dt(&config->enable_pin, 1);
+	return gpio_pin_set_dt(&config->enable_pin, 0);


Generally you would set this to 1 and change the actual output to be active low via devicetree

Yeah that makes sense :) Thanks :^)

Ping @andre-stefanov

yes this is exactly what i came up with in my implementation in the end. It is a bit inconvenient because the user of e.g. tmc2209 has to know this inverted EN logic during DTS definition instead of just saying "i define the pins, the driver knows which level has to be used for enable/disable". But unfortunately i could not provide gpio port/pin references without output level flag in DTS so i left it as faxe is proposing.

faxe1008 · 2025-06-22T13:00:13Z

drivers/stepper/adi_tmc/tmc22xx.c

 }

 static int tmc22xx_stepper_disable(const struct device *dev)
 {
 	const struct tmc22xx_config *config = dev->config;

 	LOG_DBG("Disabling Stepper motor controller %s", dev->name);
-	return gpio_pin_set_dt(&config->enable_pin, 0);
+	return gpio_pin_set_dt(&config->enable_pin, 1);


faxe1008 · 2025-06-22T13:01:56Z

drivers/stepper/allegro/a4979.c

@@ -121,7 +120,6 @@ static int a4979_stepper_set_micro_step_res(const struct device *dev,
 	case STEPPER_MICRO_STEP_4:
 		m0_value = 0;
 		m1_value = 1;
-		break;


I guess this was an accident

faxe1008 · 2025-06-22T13:05:07Z

drivers/stepper/gpio_stepper_driver.c

+	const struct gpio_stepper_config *config = dev->config;
+
+	LOG_DBG("Initializing %s gpio_stepper with %d pin", dev->name, NUM_CONTROL_PINS);
+	for (uint8_t n_pin = 0; n_pin < NUM_CONTROL_PINS; n_pin++) {


Checking readyness via gpio_ready_dt

dipakgmx · 2025-06-22T14:22:06Z

drivers/stepper/gpio_stepper_driver.c

+#include <zephyr/sys/__assert.h>
+
+#include <zephyr/logging/log.h>
+LOG_MODULE_REGISTER(gpio_stepper_motor_controller, CONFIG_STEPPER_LOG_LEVEL);


Nit: gpio_stepper_motor_controller, just a bit too long :)

andre-stefanov · 2025-06-22T17:34:49Z

dts/bindings/stepper/stepper.yaml

since the step/dir interface is extracted as a reusable component by now, we could also extract the corresponding bindings for gpio phandles into zephyr,stepper-step-dir.yaml

that would be en-pin, step-pin, dir-pin, m0/m1/m2-gpios and maybe fault pin as well. Hmm sounds fair. Is it okay if we do this in an upcoming patch, since I am going to separate the relevant functions in stepper-step-dir-api :)?

we might need a patch in between for tmc22xx as well, since it handles the mix-gpios in a slight different pattern, but we can do it.

andre-stefanov · 2025-06-22T17:39:33Z

drivers/stepper/stepper_motion_controllers/zephyr_stepper_motion_controller.c

+	update_remaining_steps(dev);
+	stepper_step(config->stepper);


call of stepper_step should be executed as early as possible during the ISR callback to avoid jitter due to different calculation overhead of update_remaining_steps.

Suggested change

update_remaining_steps(dev);

stepper_step(config->stepper);

stepper_step(config->stepper);

update_remaining_steps(dev);

andre-stefanov · 2025-06-22T18:07:21Z

drivers/stepper/stepper_motion_controllers/zephyr_stepper_motion_controller.c

+
+	data->dev = dev;
+
+	if (config->timing_source->init) {


we should return an error if config->timing_source->init evaluates to false because in this case the controller wont be usable (and we have the worker timing source as fallback for that reason).

andre-stefanov · 2025-06-22T18:13:27Z

dts/bindings/stepper/adi/adi,tmc50xx.yaml

@@ -85,7 +85,7 @@ properties:

 child-binding:
  include:
-    - name: stepper-controller.yaml
+    - name: stepper.yaml


i think only en-gpios and invert-direction should be allowed for this driver. Looking into the specs i dont see other features available over gpio (microstep config, step/dir etc)

you can use the tmc50xx with step-dir as well.
and micro step config doesn't necessarily have to be via gpios :). For instance, in h-bridge drivers we kinda control It using the half-step-lut.

oh ok. probably the model i checked was an exception and did not have those pins exposed

andre-stefanov · 2025-06-22T18:22:47Z

include/zephyr/drivers/stepper.h

+ * Counting the number of steps is the responsibility of the stepper motion controller,
+ * which will call this function for each step.


Suggested change

* Counting the number of steps is the responsibility of the stepper motion controller,

* which will call this function for each step.

* This function will not increment/decrement any position related data. Doing so

* is responsibility of the caller (e.g. stepper motion controller).

your text sounds better :)

andre-stefanov · 2025-06-22T18:27:52Z

include/zephyr/drivers/stepper.h

+ * @retval -ENOSYS If not implemented by device driver
+ * @retval 0 Success
+ */
+__syscall int stepper_set_fault_callback(const struct device *dev,


how is this different from set_event_callback ? also i don't see this API being implemented or used anywhere.

I have implemented in drv84xx, so quite a few step drivers have fault pin, this is to accommodate that particular use-case for now, we can maybe handle this more gracefully in the next patch :)

would that really be a separate callback then or can we define just another stepper event for this? (similar to endstops, stallguard etc)

This is what I am aiming for :) #89786 (comment)

andre-stefanov · 2025-06-22T18:28:52Z

samples/drivers/stepper/generic/boards/nucleo_g071rb.overlay

@@ -14,4 +22,12 @@
 			<&gpiob 0 GPIO_ACTIVE_HIGH>,	/* D10 */
 			<&gpioa 7 GPIO_ACTIVE_HIGH>;	/* D11 */
 	};
+
+	stepper_motion_control: stepper_motion_control {


Suggested change

stepper_motion_control: stepper_motion_control {

gpio_stepper_motion_control: stepper_motion_control {

because it toggles step pin?

it's just because it is a motion controller for the gpio stepper. if you have multiple steppers in a project, you will have to distinguish them. so i proposed to name them after the stepper they control.

I will be naming it as the driver itself zephyr_stepper_motion_control.

andre-stefanov · 2025-06-22T18:29:01Z

samples/drivers/stepper/generic/boards/nucleo_g071rb.overlay

@@ -1,11 +1,19 @@
 / {
 	aliases {
 		stepper = &gpio_stepper;
+		stepper-motion-control = &stepper_motion_control;


Suggested change

stepper-motion-control = &stepper_motion_control;

stepper-motion-control = &gpio_stepper_motion_control;

I will be naming it as zephyr_stepper_motion_control to reflect the device driver.

With the introduction of these two functions, now motion controllers can be directly implemented on the device driver level. This allows different motion controllers to consume various stepper drivers allowing for further upstream as well as downstream stepper motion controllers to be implemented Signed-off-by: Jilay Pandya <[email protected]>

Add fault handling in drv84xx using a fault cb mechanism Signed-off-by: Jilay Pandya <[email protected]>

Removing tests which are already covered in stepper api test suite or are outdated and have no relevance in accordance to the recent changes in the api Signed-off-by: Jilay Pandya <[email protected]>

sonarqubecloud · 2025-06-23T06:24:40Z

Quality Gate passed

Issues
3 New issues
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

jilaypandya force-pushed the feature/introduce-step-dir-functions branch from 0429ffb to 5c9c369 Compare June 21, 2025 22:35

jilaypandya assigned faxe1008 Jun 21, 2025

jilaypandya changed the title ~~wip~~ drivers: stepper: introduce step-dir functions in stepper api to facilitate implementing motion controllers as device drivers Jun 21, 2025

jilaypandya force-pushed the feature/introduce-step-dir-functions branch 9 times, most recently from 676d5fd to 1d51595 Compare June 22, 2025 12:25

faxe1008 reviewed Jun 22, 2025

View reviewed changes

jilaypandya force-pushed the feature/introduce-step-dir-functions branch 2 times, most recently from 9ebf11f to d3c27b5 Compare June 22, 2025 13:17

jilaypandya added this to the v4.3.0 milestone Jun 22, 2025

jilaypandya force-pushed the feature/introduce-step-dir-functions branch 2 times, most recently from 3b94e61 to 703d030 Compare June 22, 2025 14:16

dipakgmx reviewed Jun 22, 2025

View reviewed changes

jilaypandya force-pushed the feature/introduce-step-dir-functions branch 4 times, most recently from 4a60ace to e4b6c2d Compare June 22, 2025 15:31

jilaypandya mentioned this pull request Jun 22, 2025

Split Stepper Motion Controller <-> Stepper Driver #89786

Open

jilaypandya force-pushed the feature/introduce-step-dir-functions branch 4 times, most recently from 84392f5 to 060b707 Compare June 22, 2025 17:35

andre-stefanov suggested changes Jun 22, 2025

View reviewed changes

jilaypandya force-pushed the feature/introduce-step-dir-functions branch from 060b707 to e2dd143 Compare June 22, 2025 19:16

jilaypandya marked this pull request as ready for review June 22, 2025 19:21

github-actions bot added area: Devicetree area: Samples Samples area: Stepper platform: ADI Analog Devices, Inc. labels Jun 22, 2025

github-actions bot requested review from bjarki-andreasen, decsny, fabiobaltieri, galak, jbehrensnx, kartben, MaureenHelm, nashif, ozersa, rruuaanng and ttmut June 22, 2025 19:22

jilaypandya force-pushed the feature/introduce-step-dir-functions branch 2 times, most recently from 24fc13e to ba6069e Compare June 22, 2025 20:44

jilaypandya added 2 commits June 22, 2025 23:22

drivers: stepper: add fault handling in drv84xx

89f019b

Add fault handling in drv84xx using a fault cb mechanism Signed-off-by: Jilay Pandya <[email protected]>

jilaypandya force-pushed the feature/introduce-step-dir-functions branch from ba6069e to 89f019b Compare June 22, 2025 21:23

tests: stepper: drb84xx: deleting outdated/duplicated tests

2dfce36

Removing tests which are already covered in stepper api test suite or are outdated and have no relevance in accordance to the recent changes in the api Signed-off-by: Jilay Pandya <[email protected]>

		* Counting the number of steps is the responsibility of the stepper motion controller,
		* which will call this function for each step.

	stepper_motion_control: stepper_motion_control {
	gpio_stepper_motion_control: stepper_motion_control {

	stepper-motion-control = &stepper_motion_control;
	stepper-motion-control = &gpio_stepper_motion_control;

drivers: stepper: introduce step-dir functions in stepper api to facilitate implementing motion controllers as device drivers #91979

Are you sure you want to change the base?

drivers: stepper: introduce step-dir functions in stepper api to facilitate implementing motion controllers as device drivers #91979

Conversation

jilaypandya commented Jun 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

faxe1008 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jun 23, 2025

Quality Gate passed

Uh oh!

Uh oh!

jilaypandya commented Jun 21, 2025 •

edited

Loading