Skip to content

Commit

Permalink
drivers: stepper: gpio: simplify driver
Browse files Browse the repository at this point in the history
- Singularize instantiation of gpio-stepper driver
- Add missing SPDX headers
- Document set_event_callback function in steppers.rst
- Use the correct dtcompatible in release notes
- Rename tests from stepper_api to gpio_steppers

Signed-off-by: Jilay Pandya <[email protected]>
  • Loading branch information
jilaypandya committed Oct 30, 2024
1 parent cdf2f9a commit d1097a4
Show file tree
Hide file tree
Showing 16 changed files with 100 additions and 121 deletions.
2 changes: 2 additions & 0 deletions doc/hardware/peripherals/stepper.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
===========
Expand All @@ -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-steppers`
- :dtcompatible:`adi,tmc5041`

Discord
Expand Down
2 changes: 1 addition & 1 deletion doc/releases/release-notes-4.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ Drivers and Sensors
* Introduced stepper shell commands for controlling and configuring
stepper motors with :kconfig:option:`CONFIG_STEPPER_SHELL`
* Added support for ADI TMC5041 (:dtcompatible:`adi,tmc5041`)
* Added support for gpio-stepper-controller (:dtcompatible:`gpio-stepper-controller`)
* Added support for gpio-stepper-controller (:dtcompatible:`zephyr,gpio-stepper`)
* Added stepper api test-suite
* Added stepper shell test-suite

Expand Down
2 changes: 1 addition & 1 deletion drivers/stepper/Kconfig.gpio
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
84 changes: 36 additions & 48 deletions drivers/stepper/gpio_stepper_controller.c
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -318,62 +318,50 @@ static int gpio_stepper_enable(const struct device *dev, bool enable)
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);

Check notice on line 365 in drivers/stepper/gpio_stepper_controller.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

drivers/stepper/gpio_stepper_controller.c:365 -#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, \ +#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, \

DT_INST_FOREACH_STATUS_OKAY(GPIO_STEPPER_DEFINE)
6 changes: 3 additions & 3 deletions drivers/stepper/stepper_shell.c
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,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);
}
Expand Down Expand Up @@ -350,7 +350,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, NULL);
if (err != 0) {
shell_error(sh, "Failed to set callback: %d", err);
}
Expand Down Expand Up @@ -393,7 +393,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, NULL);
if (err != 0) {
shell_error(sh, "Failed to set callback: %d", err);
}
Expand Down
42 changes: 19 additions & 23 deletions dts/bindings/stepper/zephyr,gpio-stepper.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,29 @@
# 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:
- name: stepper-controller.yaml
property-allowlist:
- micro-step-res
- invert-direction

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
8 changes: 4 additions & 4 deletions include/zephyr/drivers/stepper.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -449,10 +449,10 @@ 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,
static inline int z_impl_stepper_set_event_callback(const struct device *dev,
stepper_event_callback_t callback, void *user_data)
{

Check notice on line 457 in include/zephyr/drivers/stepper.h

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

include/zephyr/drivers/stepper.h:457 - stepper_event_callback_t callback, void *user_data); + 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) + stepper_event_callback_t callback, + void *user_data)
const struct stepper_driver_api *api = (const struct stepper_driver_api *)dev->api;
Expand Down
Original file line number Diff line number Diff line change
@@ -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
)
project(gpio_stepper)

target_sources(app PRIVATE src/main.c)
Original file line number Diff line number Diff line change
@@ -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
Expand Down
16 changes: 16 additions & 0 deletions tests/drivers/stepper/gpio_stepper/boards/nucleo_g071rb.overlay
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2024 Jilay Sandeep Pandya
* SPDX-License-Identifier: Apache-2.0
*/

/ {
motor_1: motor_1 {
compatible = "zephyr,gpio-stepper";
status = "okay";
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>;
};
};
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
};
};
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2024 Jilay Sandeep Pandya
* SPDX-FileCopyrightText: Copyright (c) 2024 Jilay Sandeep Pandya
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand All @@ -17,7 +17,7 @@ struct k_poll_event stepper_event;
void *user_data_received;

static void stepper_print_event_callback(const struct device *dev, enum stepper_event event,
void *user_data)
void *user_data)
{
user_data_received = user_data;
switch (event) {
Expand Down Expand Up @@ -85,7 +85,7 @@ ZTEST_F(stepper, test_target_position)
(void)stepper_set_max_velocity(fixture->dev, 100u);

/* Pass the function name as user data */
(void)stepper_set_callback(fixture->dev, fixture->callback, &fixture);
(void)stepper_set_event_callback(fixture->dev, fixture->callback, &fixture);

(void)stepper_set_target_position(fixture->dev, pos);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) 2024 Jilay Sandeep Pandya
# SPDX-FileCopyrightText: Copyright (c) 2024 Jilay Sandeep Pandya
# SPDX-License-Identifier: Apache-2.0

tests:
Expand Down
18 changes: 0 additions & 18 deletions tests/drivers/stepper/stepper_api/boards/nucleo_g071rb.overlay

This file was deleted.

0 comments on commit d1097a4

Please sign in to comment.