Fix: GPIO Stepper Driver

MAINTAINERS.yml

@@ -2077,6 +2077,11 @@ Release Notes:
   status: maintained
   maintainers:
     - jilaypandya
+  collaborators:
+    - bjarki-andreasen
+    - dipakgmx
+    - fabiobaltieri
+    - faxe1008
   files:
     - drivers/stepper/
     - include/zephyr/drivers/stepper.h

doc/hardware/peripherals/stepper.rst

@@ -24,6 +24,7 @@ Control Stepper
 - Run continuously with a **constant velocity** in a specific direction until
   a stop is detected using :c:func:`stepper_enable_constant_velocity_mode`.
 - Check if the stepper is **moving** using :c:func:`stepper_is_moving`.
+- Register an **event callback** using :c:func:`stepper_set_event_callback`.
 
 Device Tree
 ===========
@@ -36,6 +37,7 @@ be used in a boards devicetree to configure a stepper driver to its initial stat
 
 See examples in:
 
+- :dtcompatible:`zephyr,gpio-stepper`
 - :dtcompatible:`adi,tmc5041`
 
 Discord

drivers/stepper/Kconfig.gpio

@@ -6,7 +6,7 @@ menu "GPIO stepper driver"
 
 config GPIO_STEPPER
 	bool "Activate driver for gpio stepper control"
-	depends on DT_HAS_ZEPHYR_GPIO_STEPPERS_ENABLED
+	depends on DT_HAS_ZEPHYR_GPIO_STEPPER_ENABLED
 	default y
 	help
 	  GPIO Stepper driver for stepper motor control with darlington arrays or dual H-bridge.

drivers/stepper/gpio_stepper_controller.c

@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-#define DT_DRV_COMPAT zephyr_gpio_steppers
+#define DT_DRV_COMPAT zephyr_gpio_stepper
 
 #include <zephyr/drivers/gpio.h>
 #include <zephyr/kernel.h>
@@ -77,6 +77,21 @@
 	}
 }
 
+static int power_down_coils(const struct device *dev)
+{
+	const struct gpio_stepper_config *config = dev->config;
+
+	for (int i = 0; i < NUM_CONTROL_PINS; i++) {
+		const int err = gpio_pin_set_dt(&config->control_pins[i], 0u);
+
+		if (err != 0) {
+			LOG_ERR("Failed to power down coil %d", i);
+			return err;
+		}
+	}
+	return 0;
+}
+
 static void update_coil_charge(const struct device *dev)
 {
 	const struct gpio_stepper_config *config = dev->config;
@@ -313,67 +328,60 @@
 			(void)k_work_reschedule(&data->stepper_dwork, K_NO_WAIT);
 		} else {
 			(void)k_work_cancel_delayable(&data->stepper_dwork);
+			const int err = power_down_coils(dev);
+
+			if (err != 0) {
+				return -EIO;
+			}
 		}
 	}
 	return 0;
 }
 
-static int gpio_stepper_motor_controller_init(const struct device *dev)
+static int gpio_stepper_init(const struct device *dev)
 {
 	struct gpio_stepper_data *data = dev->data;
 	const struct gpio_stepper_config *config = dev->config;
 
 	data->dev = dev;
-	LOG_DBG("Initializing %s gpio_stepper_motor_controller with %d pin", dev->name,
-		NUM_CONTROL_PINS);
+	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++) {
 		(void)gpio_pin_configure_dt(&config->control_pins[n_pin], GPIO_OUTPUT_INACTIVE);
 	}
 	k_work_init_delayable(&data->stepper_dwork, stepper_work_step_handler);
 	return 0;
 }
 
-#define GPIO_STEPPER_DEVICE_DATA_DEFINE(child)                                                     \
-	static struct gpio_stepper_data gpio_stepper_data_##child = {                              \
-		.step_gap = MAX_MICRO_STEP_RES >> (DT_PROP(child, micro_step_res) - 1),            \
-	};                                                                                         \
-	BUILD_ASSERT(DT_PROP(child, micro_step_res) <= STEPPER_MICRO_STEP_2,                       \
-		     "gpio_stepper_controller driver supports up to 2 micro steps");
-
-#define GPIO_STEPPER_DEVICE_CONFIG_DEFINE(child)                                                   \
-	static const struct gpio_dt_spec gpio_stepper_motor_control_pins_##child[] = {             \
-		DT_FOREACH_PROP_ELEM_SEP(child, gpios, GPIO_DT_SPEC_GET_BY_IDX, (,)),              \
-	};                                                                                         \
-	BUILD_ASSERT(                                                                              \
-		ARRAY_SIZE(gpio_stepper_motor_control_pins_##child) == 4,                          \
-		"gpio_stepper_controller driver currently supports only 4 wire configuration");    \
-	static const struct gpio_stepper_config gpio_stepper_config_##child = {                    \
-		.invert_direction = DT_PROP(child, invert_direction),                              \
-		.control_pins = gpio_stepper_motor_control_pins_##child};
-
-#define GPIO_STEPPER_API_DEFINE(child)                                                             \
-	static const struct stepper_driver_api gpio_stepper_api_##child = {                        \
-		.enable = gpio_stepper_enable,                                                     \
-		.move = gpio_stepper_move,                                                         \
-		.is_moving = gpio_stepper_is_moving,                                               \
-		.set_actual_position = gpio_stepper_set_actual_position,                           \
-		.get_actual_position = gpio_stepper_get_actual_position,                           \
-		.set_target_position = gpio_stepper_set_target_position,                           \
-		.set_max_velocity = gpio_stepper_set_max_velocity,                                 \
-		.enable_constant_velocity_mode = gpio_stepper_enable_constant_velocity_mode,       \
-		.set_micro_step_res = gpio_stepper_set_micro_step_res,                             \
-		.get_micro_step_res = gpio_stepper_get_micro_step_res,                             \
-		.set_event_callback = gpio_stepper_set_event_callback, };
-
-#define GPIO_STEPPER_DEVICE_DEFINE(child)                                                          \
-	DEVICE_DT_DEFINE(child, gpio_stepper_motor_controller_init, NULL,                          \
-			 &gpio_stepper_data_##child, &gpio_stepper_config_##child, POST_KERNEL,    \
-			 CONFIG_STEPPER_INIT_PRIORITY, &gpio_stepper_api_##child);
-
-#define GPIO_STEPPER_CONTROLLER_DEFINE(inst)                                                       \
-	DT_INST_FOREACH_CHILD(inst, GPIO_STEPPER_DEVICE_CONFIG_DEFINE);                            \
-	DT_INST_FOREACH_CHILD(inst, GPIO_STEPPER_DEVICE_DATA_DEFINE);                              \
-	DT_INST_FOREACH_CHILD(inst, GPIO_STEPPER_API_DEFINE);                                      \
-	DT_INST_FOREACH_CHILD(inst, GPIO_STEPPER_DEVICE_DEFINE);
-
-DT_INST_FOREACH_STATUS_OKAY(GPIO_STEPPER_CONTROLLER_DEFINE)
+static const struct stepper_driver_api gpio_stepper_api = {
+	.enable = gpio_stepper_enable,
+	.move = gpio_stepper_move,
+	.is_moving = gpio_stepper_is_moving,
+	.set_actual_position = gpio_stepper_set_actual_position,
+	.get_actual_position = gpio_stepper_get_actual_position,
+	.set_target_position = gpio_stepper_set_target_position,
+	.set_max_velocity = gpio_stepper_set_max_velocity,
+	.enable_constant_velocity_mode = gpio_stepper_enable_constant_velocity_mode,
+	.set_micro_step_res = gpio_stepper_set_micro_step_res,
+	.get_micro_step_res = gpio_stepper_get_micro_step_res,
+	.set_event_callback = gpio_stepper_set_event_callback,
+};
+
+#define GPIO_STEPPER_DEFINE(inst)								\
+	static const struct gpio_dt_spec gpio_stepper_motor_control_pins_##inst[] = {		\
+		DT_INST_FOREACH_PROP_ELEM_SEP(inst, gpios, GPIO_DT_SPEC_GET_BY_IDX, (,)),	\
+	};											\
+	BUILD_ASSERT(ARRAY_SIZE(gpio_stepper_motor_control_pins_##inst) == 4,			\
+		"gpio_stepper_controller driver currently supports only 4 wire configuration");	\
+	static const struct gpio_stepper_config gpio_stepper_config_##inst = {			\
+		.invert_direction = DT_INST_PROP(inst, invert_direction),			\
+		.control_pins = gpio_stepper_motor_control_pins_##inst};			\
+	static struct gpio_stepper_data gpio_stepper_data_##inst = {				\
+		.step_gap = MAX_MICRO_STEP_RES >> (DT_INST_PROP(inst, micro_step_res) - 1),	\
+	};											\
+	BUILD_ASSERT(DT_INST_PROP(inst, micro_step_res) <= STEPPER_MICRO_STEP_2,		\
+		     "gpio_stepper_controller driver supports up to 2 micro steps");		\
+	DEVICE_DT_INST_DEFINE(inst, gpio_stepper_init, NULL, &gpio_stepper_data_##inst,		\
+			      &gpio_stepper_config_##inst, POST_KERNEL,				\
+			      CONFIG_STEPPER_INIT_PRIORITY, &gpio_stepper_api);
+
+DT_INST_FOREACH_STATUS_OKAY(GPIO_STEPPER_DEFINE)

drivers/stepper/stepper_shell.c

@@ -44,6 +44,9 @@ static void print_callback(const struct device *dev, const enum stepper_event ev
 			   void *user_data)
 {
 	const struct shell *sh = user_data;
+	if (!sh) {
+		return;
+	}
 
 	switch (event) {
 	case STEPPER_EVENT_STEPS_COMPLETED:
@@ -203,7 +206,7 @@ static int cmd_stepper_move(const struct shell *sh, size_t argc, char **argv)
 		return err;
 	}
 
-	err = stepper_set_callback(dev, print_callback, (void *)sh);
+	err = stepper_set_event_callback(dev, print_callback, (void *)sh);
 	if (err != 0) {
 		shell_error(sh, "Failed to set callback: %d", err);
 	}
@@ -350,7 +353,7 @@ static int cmd_stepper_set_target_position(const struct shell *sh, size_t argc,
 		return err;
 	}
 
-	err = stepper_set_callback(dev, print_callback, NULL);
+	err = stepper_set_event_callback(dev, print_callback, (void *)sh);
 	if (err != 0) {
 		shell_error(sh, "Failed to set callback: %d", err);
 	}
@@ -393,7 +396,7 @@ static int cmd_stepper_enable_constant_velocity_mode(const struct shell *sh, siz
 		return err;
 	}
 
-	err = stepper_set_callback(dev, print_callback, NULL);
+	err = stepper_set_event_callback(dev, print_callback, (void *)sh);
 	if (err != 0) {
 		shell_error(sh, "Failed to set callback: %d", err);
 	}

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

@@ -89,13 +89,10 @@ properties:
 
 child-binding:
   include:
+    - name: stepper-controller.yaml
     - name: base.yaml
       property-allowlist:
         - reg
-    - name: stepper-controller.yaml
-      property-allowlist:
-        - invert-direction
-        - micro-step-res
     - name: adi,trinamic-ramp-generator.yaml
       property-allowlist:
         - vstart

dts/bindings/stepper/stepper-controller.yaml

@@ -3,8 +3,6 @@
 
 description: Stepper Controller
 
-include: base.yaml
-
 properties:
   invert-direction:
     type: boolean

dts/bindings/stepper/zephyr,gpio-stepper.yaml

@@ -3,33 +3,25 @@
 # SPDX-License-Identifier: Apache-2.0
 
 description: |
-  GPIO Stepper Controller cluster for darlington transistor arrays or dual H-bridge
+  GPIO Stepper Controller for darlington transistor arrays or dual H-bridge
 
   Example:
     /* Lead A is connected Lead C and Lead B is connected to Lead D*/
-    stepper {
-        compatible = "zephyr,gpio-steppers";
-        motor: motor {
-            gpios = <&gpioa 9 GPIO_ACTIVE_HIGH>,  /* Lead A1/A */
-                    <&gpioc 7 GPIO_ACTIVE_HIGH>,  /* Lead B1/B */
-                    <&gpiob 0 GPIO_ACTIVE_HIGH>,  /* Lead A2/C */
-                    <&gpioa 7 GPIO_ACTIVE_HIGH>;  /* Lead B2/D */
-        };
+    stepper: stepper {
+        compatible = "zephyr,gpio-stepper";
+        gpios = <&gpioa 9 GPIO_ACTIVE_HIGH>,  /* Lead A1/A */
+                <&gpioc 7 GPIO_ACTIVE_HIGH>,  /* Lead B1/B */
+                <&gpiob 0 GPIO_ACTIVE_HIGH>,  /* Lead A2/C */
+                <&gpioa 7 GPIO_ACTIVE_HIGH>;  /* Lead B2/D */
     };
 
-compatible: "zephyr,gpio-steppers"
+compatible: "zephyr,gpio-stepper"
 
-child-binding:
-  description: GPIO Controller for stepper motor
-  include:
-    - name: stepper-controller.yaml
-      property-allowlist:
-        - micro-step-res
-        - invert-direction
+include: stepper-controller.yaml
 
-  properties:
-    gpios:
-      type: phandle-array
-      required: true
-      description: |
-        The gpio pin array on which the stepper inputs are to be connected
+properties:
+  gpios:
+    type: phandle-array
+    required: true
+    description: |
+      The gpio pin array on which the stepper inputs are to be connected

include/zephyr/drivers/stepper.h

@@ -180,7 +180,7 @@ typedef void (*stepper_event_callback_t)(const struct device *dev, const enum st
 /**
  * @brief Set the callback function to be called when a stepper event occurs
  *
- * @see stepper_set_callback() for details.
+ * @see stepper_set_event_callback() for details.
  */
 typedef int (*stepper_set_event_callback_t)(const struct device *dev,
 					    stepper_event_callback_t callback, void *user_data);
@@ -449,11 +449,12 @@ static inline int z_impl_stepper_enable_constant_velocity_mode(
  * @retval -ENOSYS If not implemented by device driver
  * @retval 0 Success
  */
-__syscall int stepper_set_callback(const struct device *dev, stepper_event_callback_t callback,
-				   void *user_data);
+__syscall int stepper_set_event_callback(const struct device *dev,
+					 stepper_event_callback_t callback, void *user_data);
 
-static inline int z_impl_stepper_set_callback(const struct device *dev,
-					      stepper_event_callback_t callback, void *user_data)
+static inline int z_impl_stepper_set_event_callback(const struct device *dev,
+						    stepper_event_callback_t callback,
+						    void *user_data)
 {
 	const struct stepper_driver_api *api = (const struct stepper_driver_api *)dev->api;
 

tests/drivers/stepper/stepper_api/CMakeLists.txt

@@ -1,10 +1,8 @@
-# Copyright (c) 2024 Jilay Sandeep Pandya
+# SPDX-FileCopyrightText: Copyright (c) 2024 Jilay Sandeep Pandya
 # SPDX-License-Identifier: Apache-2.0
 
 cmake_minimum_required(VERSION 3.20.0)
-
 find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE})
 project(stepper_api)
-target_sources(app PRIVATE
-	src/main.c
-)
+
+target_sources(app PRIVATE src/main.c)

tests/drivers/stepper/stepper_api/boards/nucleo_g071rb.conf

@@ -1,4 +1,4 @@
-# Copyright (c) 2024 Jilay Sandeep Pandya
+# SPDX-FileCopyrightText: Copyright (c) 2024 Jilay Sandeep Pandya
 # SPDX-License-Identifier: Apache-2.0
 
 CONFIG_GPIO=y

tests/drivers/stepper/stepper_api/boards/nucleo_g071rb.overlay

@@ -4,15 +4,13 @@
  */
 
 / {
-	uln2003_motor: uln2003_1 {
-		compatible = "zephyr,gpio-steppers";
+	motor_1: motor_1 {
+		compatible = "zephyr,gpio-stepper";
 		status = "okay";
-		motor_1: motor_1 {
-			micro-step-res = <1>;
-			gpios = <&gpioa 9 GPIO_ACTIVE_HIGH>,
-				<&gpioc 7 GPIO_ACTIVE_HIGH>,
-				<&gpiob 0 GPIO_ACTIVE_HIGH>,
-				<&gpioa 7 GPIO_ACTIVE_HIGH>;
-		};
+		micro-step-res = <1>;
+		gpios = <&gpioa 9 GPIO_ACTIVE_HIGH>,
+			<&gpioc 7 GPIO_ACTIVE_HIGH>,
+			<&gpiob 0 GPIO_ACTIVE_HIGH>,
+			<&gpioa 7 GPIO_ACTIVE_HIGH>;
 	};
 };

tests/drivers/stepper/stepper_api/boards/qemu_x86_64.conf

@@ -1,4 +1,4 @@
-# Copyright (c) 2024 Jilay Sandeep Pandya
+# SPDX-FileCopyrightText: Copyright (c) 2024 Jilay Sandeep Pandya
 # SPDX-License-Identifier: Apache-2.0
 
 CONFIG_GPIO=y

tests/drivers/stepper/stepper_api/boards/qemu_x86_64.overlay

@@ -19,16 +19,13 @@
 };
 
 / {
-	test_uln2003_motor_cluster: uln2003_motor_cluster {
-		compatible = "zephyr,gpio-steppers";
+	motor_1: motor_1 {
+		compatible = "zephyr,gpio-stepper";
 		status = "okay";
-
-		motor_1: motor_1 {
-			micro-step-res = <1>;
-			gpios = <&test_gpio 0 0>,
-				<&test_gpio 0 0>,
-				<&test_gpio 0 0>,
-				<&test_gpio 0 0>;
-		};
+		micro-step-res = <1>;
+		gpios = <&test_gpio 0 0>,
+			<&test_gpio 0 0>,
+			<&test_gpio 0 0>,
+			<&test_gpio 0 0>;
 	};
 };

tests/drivers/stepper/stepper_api/prj.conf

@@ -1,4 +1,4 @@
-# Copyright (c) 2024 Jilay Sandeep Pandya
+# SPDX-FileCopyrightText: Copyright (c) 2024 Jilay Sandeep Pandya
 # SPDX-License-Identifier: Apache-2.0
 
 CONFIG_ZTEST=y