drivers: stepper: remove stop function wrapped in disable

Removing stop functionality wrapped in stepper_disable since
there is a dedicated function for it.

Signed-off-by: Jilay Pandya <jilay.pandya@outlook.com>

drivers/stepper/allegro/a4979.c

@@ -24,7 +24,6 @@ struct a4979_config {
 struct a4979_data {
 	const struct step_dir_stepper_common_data common;
 	enum stepper_micro_step_resolution micro_step_res;
-	bool enabled;
 };
 
 STEP_DIR_STEPPER_STRUCT_CHECK(struct a4979_config, struct a4979_data);
@@ -54,7 +53,6 @@ static int a4979_stepper_enable(const struct device *dev)
 {
 	int ret;
 	const struct a4979_config *config = dev->config;
-	struct a4979_data *data = dev->data;
 	bool has_enable_pin = config->en_pin.port != NULL;
 
 	/* Check availability of enable pin, as it might be hardwired. */
@@ -69,8 +67,6 @@ static int a4979_stepper_enable(const struct device *dev)
 		return ret;
 	}
 
-	data->enabled = true;
-
 	return 0;
 }
 
@@ -78,7 +74,6 @@ static int a4979_stepper_disable(const struct device *dev)
 {
 	int ret;
 	const struct a4979_config *config = dev->config;
-	struct a4979_data *data = dev->data;
 	bool has_enable_pin = config->en_pin.port != NULL;
 
 	/* Check availability of enable pin, as it might be hardwired. */
@@ -93,9 +88,6 @@ static int a4979_stepper_disable(const struct device *dev)
 		return ret;
 	}
 
-	config->common.timing_source->stop(dev);
-	data->enabled = false;
-
 	return 0;
 }
 
@@ -153,42 +145,6 @@ static int a4979_stepper_get_micro_step_res(const struct device *dev,
 	return 0;
 }
 
-static int a4979_move_to(const struct device *dev, int32_t target)
-{
-	struct a4979_data *data = dev->data;
-
-	if (!data->enabled) {
-		LOG_ERR("Failed to move to target position, device is not enabled");
-		return -ECANCELED;
-	}
-
-	return step_dir_stepper_common_move_to(dev, target);
-}
-
-static int a4979_stepper_move_by(const struct device *dev, const int32_t micro_steps)
-{
-	struct a4979_data *data = dev->data;
-
-	if (!data->enabled) {
-		LOG_ERR("Failed to move by delta, device is not enabled");
-		return -ECANCELED;
-	}
-
-	return step_dir_stepper_common_move_by(dev, micro_steps);
-}
-
-static int a4979_run(const struct device *dev, enum stepper_direction direction)
-{
-	struct a4979_data *data = dev->data;
-
-	if (!data->enabled) {
-		LOG_ERR("Failed to run stepper, device is not enabled");
-		return -ECANCELED;
-	}
-
-	return step_dir_stepper_common_run(dev, direction);
-}
-
 static int a4979_init(const struct device *dev)
 {
 	const struct a4979_config *config = dev->config;
@@ -269,13 +225,13 @@ static int a4979_init(const struct device *dev)
 static DEVICE_API(stepper, a4979_stepper_api) = {
 	.enable = a4979_stepper_enable,
 	.disable = a4979_stepper_disable,
-	.move_by = a4979_stepper_move_by,
-	.move_to = a4979_move_to,
+	.move_by = step_dir_stepper_common_move_by,
+	.move_to = step_dir_stepper_common_move_to,
 	.is_moving = step_dir_stepper_common_is_moving,
 	.set_reference_position = step_dir_stepper_common_set_reference_position,
 	.get_actual_position = step_dir_stepper_common_get_actual_position,
 	.set_microstep_interval = step_dir_stepper_common_set_microstep_interval,
-	.run = a4979_run,
+	.run = step_dir_stepper_common_run,
 	.stop = step_dir_stepper_common_stop,
 	.set_micro_step_res = a4979_stepper_set_micro_step_res,
 	.get_micro_step_res = a4979_stepper_get_micro_step_res,

drivers/stepper/gpio_stepper_controller.c

@@ -24,6 +24,7 @@ static const uint8_t
 		{0u, 0u, 1u, 1u}, {0u, 0u, 0u, 1u}, {1u, 0u, 0u, 1u}, {1u, 0u, 0u, 0u}};
 
 struct gpio_stepper_config {
+	const struct gpio_dt_spec en_pin;
 	const struct gpio_dt_spec *control_pins;
 	bool invert_direction;
 };
@@ -39,7 +40,6 @@ struct gpio_stepper_data {
 	int32_t actual_position;
 	uint64_t delay_in_ns;
 	int32_t step_count;
-	bool is_enabled;
 	stepper_event_callback_t callback;
 	void *event_cb_user_data;
 };
@@ -183,11 +183,6 @@ static int gpio_stepper_move_by(const struct device *dev, int32_t micro_steps)
 {
 	struct gpio_stepper_data *data = dev->data;
 
-	if (!data->is_enabled) {
-		LOG_ERR("Stepper motor is not enabled");
-		return -ECANCELED;
-	}
-
 	if (data->delay_in_ns == 0) {
 		LOG_ERR("Step interval not set or invalid step interval set");
 		return -EINVAL;
@@ -262,11 +257,6 @@ static int gpio_stepper_run(const struct device *dev, const enum stepper_directi
 {
 	struct gpio_stepper_data *data = dev->data;
 
-	if (!data->is_enabled) {
-		LOG_ERR("Stepper motor is not enabled");
-		return -ECANCELED;
-	}
-
 	K_SPINLOCK(&data->lock) {
 		data->run_mode = STEPPER_RUN_MODE_VELOCITY;
 		data->direction = direction;
@@ -317,18 +307,16 @@ static int gpio_stepper_set_event_callback(const struct device *dev,
 
 static int gpio_stepper_enable(const struct device *dev)
 {
+	const struct gpio_stepper_config *config = dev->config;
 	struct gpio_stepper_data *data = dev->data;
 	int err;
 
-	if (data->is_enabled) {
-		LOG_WRN("Stepper motor is already enabled");
-		return 0;
-	}
-
 	K_SPINLOCK(&data->lock) {
-		err = energize_coils(dev, true);
-		if (err == 0) {
-			data->is_enabled = true;
+		if (config->en_pin.port != NULL) {
+			err = gpio_pin_set_dt(&config->en_pin, 1);
+		} else {
+			LOG_DBG("No en_pin detected");
+			err = -ENOTSUP;
 		}
 	}
 	return err;
@@ -336,14 +324,18 @@ static int gpio_stepper_enable(const struct device *dev)
 
 static int gpio_stepper_disable(const struct device *dev)
 {
+	const struct gpio_stepper_config *config = dev->config;
 	struct gpio_stepper_data *data = dev->data;
 	int err;
 
 	K_SPINLOCK(&data->lock) {
-		(void)k_work_cancel_delayable(&data->stepper_dwork);
-		err = energize_coils(dev, false);
-		if (err == 0) {
-			data->is_enabled = false;
+		(void)energize_coils(dev, false);
+		if (config->en_pin.port != NULL) {
+			err = gpio_pin_set_dt(&config->en_pin, 0);
+		} else {
+			LOG_DBG("No en_pin detected, power stages will not be turned off if "
+				"stepper is in motion");
+			err = -ENOTSUP;
 		}
 	}
 	return err;

drivers/stepper/ti/drv84xx.c

@@ -49,7 +49,6 @@ struct drv84xx_pin_states {
 struct drv84xx_data {
 	const struct step_dir_stepper_common_data common;
 	const struct device *dev;
-	bool enabled;
 	struct drv84xx_pin_states pin_states;
 	enum stepper_micro_step_resolution ustep_res;
 	struct gpio_callback fault_cb_data;
@@ -220,8 +219,6 @@ static int drv84xx_enable(const struct device *dev)
 		}
 	}
 
-	data->enabled = true;
-
 	return ret;
 }
 
@@ -255,10 +252,6 @@ static int drv84xx_disable(const struct device *dev)
 		}
 	}
 
-	config->common.timing_source->stop(dev);
-
-	data->enabled = false;
-
 	return ret;
 }
 
@@ -351,42 +344,6 @@ static int drv84xx_get_micro_step_res(const struct device *dev,
 	return 0;
 }
 
-static int drv84xx_move_to(const struct device *dev, int32_t target)
-{
-	struct drv84xx_data *data = dev->data;
-
-	if (!data->enabled) {
-		LOG_ERR("Failed to move to target position, device is not enabled");
-		return -ECANCELED;
-	}
-
-	return step_dir_stepper_common_move_to(dev, target);
-}
-
-static int drv84xx_move_by(const struct device *dev, int32_t steps)
-{
-	struct drv84xx_data *data = dev->data;
-
-	if (!data->enabled) {
-		LOG_ERR("Failed to move by delta, device is not enabled");
-		return -ECANCELED;
-	}
-
-	return step_dir_stepper_common_move_by(dev, steps);
-}
-
-static int drv84xx_run(const struct device *dev, enum stepper_direction direction)
-{
-	struct drv84xx_data *data = dev->data;
-
-	if (!data->enabled) {
-		LOG_ERR("Failed to run stepper, device is not enabled");
-		return -ECANCELED;
-	}
-
-	return step_dir_stepper_common_run(dev, direction);
-}
-
 void fault_event(const struct device *dev, struct gpio_callback *cb, uint32_t pins)
 {
 	struct drv84xx_data *data = CONTAINER_OF(cb, struct drv84xx_data, fault_cb_data);
@@ -478,13 +435,13 @@ static int drv84xx_init(const struct device *dev)
 static DEVICE_API(stepper, drv84xx_stepper_api) = {
 	.enable = drv84xx_enable,
 	.disable = drv84xx_disable,
-	.move_by = drv84xx_move_by,
-	.move_to = drv84xx_move_to,
+	.move_by = step_dir_stepper_common_move_by,
+	.move_to = step_dir_stepper_common_move_to,
 	.is_moving = step_dir_stepper_common_is_moving,
 	.set_reference_position = step_dir_stepper_common_set_reference_position,
 	.get_actual_position = step_dir_stepper_common_get_actual_position,
 	.set_microstep_interval = step_dir_stepper_common_set_microstep_interval,
-	.run = drv84xx_run,
+	.run = step_dir_stepper_common_run,
 	.stop = step_dir_stepper_common_stop,
 	.set_micro_step_res = drv84xx_set_micro_step_res,
 	.get_micro_step_res = drv84xx_get_micro_step_res,

include/zephyr/drivers/stepper.h

@@ -262,6 +262,7 @@ static inline int z_impl_stepper_enable(const struct device *dev)
  *
  * @param dev pointer to the stepper driver instance
  *
+ * @retval  -ENOTSUP Disabling of driver is not supported.
  * @retval -EIO Error during Disabling
  * @retval 0 Success
  */
@@ -436,7 +437,6 @@ static inline int z_impl_stepper_set_microstep_interval(const struct device *dev
  * @param dev pointer to the stepper driver instance
  * @param micro_steps target micro-steps to be moved from the current position
  *
- * @retval -ECANCELED If the stepper is disabled
  * @retval -EIO General input / output error
  * @retval 0 Success
  */
@@ -458,7 +458,6 @@ static inline int z_impl_stepper_move_by(const struct device *dev, const int32_t
  * @param dev pointer to the stepper driver instance
  * @param micro_steps target position to set in micro-steps
  *
- * @retval -ECANCELED If the stepper is disabled
  * @retval -EIO General input / output error
  * @retval -ENOSYS If not implemented by device driver
  * @retval 0 Success
@@ -485,7 +484,6 @@ static inline int z_impl_stepper_move_to(const struct device *dev, const int32_t
  * @param dev pointer to the stepper driver instance
  * @param direction The direction to set
  *
- * @retval -ECANCELED If the stepper is disabled
  * @retval -EIO General input / output error
  * @retval -ENOSYS If not implemented by device driver
  * @retval 0 Success

tests/drivers/stepper/drv84xx/api/src/main.c

@@ -87,72 +87,6 @@ ZTEST_F(drv84xx_api, test_actual_position_set)
 	zassert_equal(pos, 100u, "Actual position should be %u but is %u", 100u, pos);
 }
 
-ZTEST_F(drv84xx_api, test_is_not_moving_when_disabled)
-{
-	int32_t steps = 100;
-	bool moving = true;
-
-	(void)stepper_enable(fixture->dev);
-	(void)stepper_set_microstep_interval(fixture->dev, 20000000);
-	(void)stepper_move_by(fixture->dev, steps);
-	(void)stepper_disable(fixture->dev);
-	(void)stepper_is_moving(fixture->dev, &moving);
-	zassert_false(moving, "Driver should not be in state is_moving after being disabled");
-}
-
-ZTEST_F(drv84xx_api, test_position_not_updating_when_disabled)
-{
-	int32_t steps = 1000;
-	int32_t position_1 = 0;
-	int32_t position_2 = 0;
-
-	(void)stepper_enable(fixture->dev);
-	(void)stepper_set_microstep_interval(fixture->dev, 20000000);
-	(void)stepper_move_by(fixture->dev, steps);
-	(void)stepper_disable(fixture->dev);
-	(void)stepper_get_actual_position(fixture->dev, &position_1);
-	k_msleep(100);
-	(void)stepper_get_actual_position(fixture->dev, &position_2);
-	zassert_equal(position_2, position_1,
-		      "Actual position should not have changed from %d but is %d", position_1,
-		      position_2);
-}
-
-ZTEST_F(drv84xx_api, test_is_not_moving_when_reenabled_after_movement)
-{
-	int32_t steps = 1000;
-	bool moving = true;
-
-	(void)stepper_enable(fixture->dev);
-	(void)stepper_set_microstep_interval(fixture->dev, 20000000);
-	(void)stepper_move_by(fixture->dev, steps);
-	(void)stepper_disable(fixture->dev);
-	(void)k_msleep(100);
-	(void)stepper_enable(fixture->dev);
-	(void)k_msleep(100);
-	(void)stepper_is_moving(fixture->dev, &moving);
-	zassert_false(moving, "Driver should not be in state is_moving after being reenabled");
-}
-ZTEST_F(drv84xx_api, test_position_not_updating_when_reenabled_after_movement)
-{
-	int32_t steps = 1000;
-	int32_t position_1 = 0;
-	int32_t position_2 = 0;
-
-	(void)stepper_enable(fixture->dev);
-	(void)stepper_set_microstep_interval(fixture->dev, 20000000);
-	(void)stepper_move_by(fixture->dev, steps);
-	(void)stepper_disable(fixture->dev);
-	(void)stepper_get_actual_position(fixture->dev, &position_1);
-	(void)k_msleep(100);
-	(void)stepper_enable(fixture->dev);
-	(void)k_msleep(100);
-	(void)stepper_get_actual_position(fixture->dev, &position_2);
-	zassert_equal(position_2, position_1,
-		      "Actual position should not have changed from %d but is %d", position_1,
-		      position_2);
-}
-
 ZTEST_F(drv84xx_api, test_move_to_positive_direction_movement)
 {
 	int32_t pos = 50;
@@ -247,23 +181,6 @@ ZTEST_F(drv84xx_api, test_move_to_is_moving_false_when_completed)
 	zassert_false(moving, "Driver should not be in state is_moving after finishing");
 }
 
-ZTEST_F(drv84xx_api, test_move_to_no_movement_when_disabled)
-{
-	int32_t pos = 50;
-	int32_t curr_pos = 50;
-	int32_t ret = 0;
-
-	(void)stepper_set_microstep_interval(fixture->dev, 20000000);
-	(void)stepper_disable(fixture->dev);
-
-	ret = stepper_move_to(fixture->dev, pos);
-	zassert_equal(ret, -ECANCELED, "Move_to should fail with error code %d but returned %d",
-		      -ECANCELED, ret);
-	(void)stepper_get_actual_position(fixture->dev, &curr_pos);
-	zassert_equal(curr_pos, 0, "Current position should not have changed from %d but is %d", 0,
-		      curr_pos);
-}
-
 ZTEST_F(drv84xx_api, test_move_by_zero_steps_no_movement)
 {
 	int32_t steps = 0;
@@ -284,22 +201,6 @@ ZTEST_F(drv84xx_api, test_move_by_zero_steps_no_movement)
 	zassert_equal(steps, 0, "Target position should be %d but is %d", 0, steps);
 }
 
-ZTEST_F(drv84xx_api, test_move_by_zero_step_interval)
-{
-	int32_t steps = 100;
-	int32_t ret = 0;
-	int32_t pos = 100;
-
-	(void)stepper_enable(fixture->dev);
-	(void)stepper_disable(fixture->dev);
-	ret = stepper_move_by(fixture->dev, steps);
-
-	zassert_not_equal(ret, 0, "Command should fail with an error code, but returned 0");
-	k_msleep(100);
-	(void)stepper_get_actual_position(fixture->dev, &pos);
-	zassert_equal(pos, 0, "Target position should not have changed from %d but is %d", 0, pos);
-}
-
 ZTEST_F(drv84xx_api, test_move_by_is_moving_true_while_moving)
 {
 	int32_t steps = 50;
@@ -334,23 +235,6 @@ ZTEST_F(drv84xx_api, test_move_by_is_moving_false_when_completed)
 	zassert_false(moving, "Driver should not be in state is_moving after completion");
 }
 
-ZTEST_F(drv84xx_api, test_move_by_no_movement_when_disabled)
-{
-	int32_t steps = 100;
-	int32_t curr_pos = 100;
-	int32_t ret = 0;
-
-	(void)stepper_set_microstep_interval(fixture->dev, 20000000);
-	(void)stepper_disable(fixture->dev);
-
-	ret = stepper_move_by(fixture->dev, steps);
-	zassert_equal(ret, -ECANCELED, "Move_by should fail with error code %d but returned %d",
-		      -ECANCELED, ret);
-	(void)stepper_get_actual_position(fixture->dev, &curr_pos);
-	zassert_equal(curr_pos, 0, "Current position should not have changed from %d but is %d", 0,
-		      curr_pos);
-}
-
 ZTEST_F(drv84xx_api, test_run_positive_direction_correct_position)
 {
 	uint64_t step_interval = 20000000;
@@ -408,21 +292,4 @@ ZTEST_F(drv84xx_api, test_run_is_moving_true_when_step_interval_greater_zero)
 	(void)stepper_disable(fixture->dev);
 }
 
-ZTEST_F(drv84xx_api, test_run_no_movement_when_disabled)
-{
-	uint64_t step_interval = 20000000;
-	int32_t steps = 50;
-	int32_t ret = 0;
-
-	(void)stepper_disable(fixture->dev);
-	(void)stepper_set_microstep_interval(fixture->dev, step_interval);
-
-	ret = stepper_run(fixture->dev, STEPPER_DIRECTION_POSITIVE);
-	zassert_equal(ret, -ECANCELED, "Run should fail with error code %d but returned %d",
-		      -ECANCELED, ret);
-	(void)stepper_get_actual_position(fixture->dev, &steps);
-	zassert_equal(steps, 0, "Current position should not have changed from %d but is %d", 0,
-		      steps);
-}
-
 ZTEST_SUITE(drv84xx_api, NULL, drv84xx_api_setup, drv84xx_api_before, drv84xx_api_after, NULL);