Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers: stepper: add stepper driver for allegro a4979 #86620

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions drivers/stepper/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ zephyr_syscall_header(${ZEPHYR_BASE}/include/zephyr/drivers/stepper.h)

# zephyr-keep-sorted-start
add_subdirectory_ifdef(CONFIG_STEPPER_ADI_TMC adi_tmc)
add_subdirectory_ifdef(CONFIG_STEPPER_ALLEGRO allegro)
add_subdirectory_ifdef(CONFIG_STEPPER_TI ti)
add_subdirectory_ifdef(CONFIG_STEP_DIR_STEPPER step_dir)
# zephyr-keep-sorted-stop
Expand Down
1 change: 1 addition & 0 deletions drivers/stepper/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ comment "Stepper Drivers"
rsource "Kconfig.fake"
rsource "Kconfig.gpio"
rsource "adi_tmc/Kconfig"
rsource "allegro/Kconfig"
rsource "ti/Kconfig"
# zephyr-keep-sorted-stop

Expand Down
7 changes: 7 additions & 0 deletions drivers/stepper/allegro/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# SPDX-FileCopyrightText: Copyright (c) 2025 Carl Zeiss Meditec AG
# SPDX-License-Identifier: Apache-2.0

zephyr_library()
zephyr_library_property(ALLOW_EMPTY TRUE)

zephyr_library_sources_ifdef(CONFIG_A4979_STEPPER a4979.c)
19 changes: 19 additions & 0 deletions drivers/stepper/allegro/Kconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# SPDX-FileCopyrightText: Copyright (c) 2025 Carl Zeiss Meditec AG
# SPDX-License-Identifier: Apache-2.0

menuconfig STEPPER_ALLEGRO
bool "Allegro Stepper Controller"
depends on STEPPER
default y
help
Enable allegro stepper controller

if STEPPER_ALLEGRO

comment "Allegro Stepper Drivers"

# zephyr-keep-sorted-start
rsource "Kconfig.a4979"
# zephyr-keep-sorted-stop

endif # STEPPER_ALLEGRO
10 changes: 10 additions & 0 deletions drivers/stepper/allegro/Kconfig.a4979
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# SPDX-FileCopyrightText: Copyright (c) 2025 Carl Zeiss Meditec AG
# SPDX-License-Identifier: Apache-2.0

config A4979_STEPPER
bool "Activate allegro A4979 stepper driver"
default y
depends on DT_HAS_ALLEGRO_A4979_ENABLED
select STEP_DIR_STEPPER
help
Microstepping motor driver for stepper motors.
281 changes: 281 additions & 0 deletions drivers/stepper/allegro/a4979.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,281 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2025 Carl Zeiss Meditec AG
* SPDX-License-Identifier: Apache-2.0
*/

#define DT_DRV_COMPAT allegro_a4979

#include <zephyr/kernel.h>
#include <zephyr/drivers/stepper.h>
#include <zephyr/drivers/gpio.h>
#include "../step_dir/step_dir_stepper_common.h"

#include <zephyr/logging/log.h>
LOG_MODULE_REGISTER(a4979, CONFIG_STEPPER_LOG_LEVEL);

struct a4979_config {
const struct step_dir_stepper_common_config common;
const struct gpio_dt_spec en_pin;
const struct gpio_dt_spec reset_pin;
const struct gpio_dt_spec m0_pin;
const struct gpio_dt_spec m1_pin;
};

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);

static int a4979_set_microstep_pin(const struct device *dev, const struct gpio_dt_spec *pin,
int value)
{
int ret;

/* Reset microstep pin as it may have been disconnected. */
ret = gpio_pin_configure_dt(pin, GPIO_OUTPUT_INACTIVE);
if (ret != 0) {
LOG_ERR("Failed to configure microstep pin (error: %d)", ret);
return ret;
}

ret = gpio_pin_set_dt(pin, value);
if (ret != 0) {
LOG_ERR("Failed to set microstep pin (error: %d)", ret);
return ret;
}

return 0;
}

static int a4979_stepper_enable(const struct device *dev, bool enable)
{
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. */
if (has_enable_pin) {
ret = gpio_pin_set_dt(&config->en_pin, enable);
if (ret != 0) {
LOG_ERR("%s: Failed to set en_pin (error: %d)", dev->name, ret);
return ret;
}
}
else {

Check failure on line 68 in drivers/stepper/allegro/a4979.c

View workflow job for this annotation

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

ELSE_AFTER_BRACE

drivers/stepper/allegro/a4979.c:68 else should follow close brace '}'
LOG_ERR("%s: Failed to enable/disable device, enable pin is not available. "
"The device is always on.",
dev->name);
return -ENOTSUP;
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disabling of the timing source was fine in case of calling this with false what I meant with my comment is if there is a benefit to setting the step pin to 0 after disabling it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is taken from the drv824 driver. The drv8424 only takes a step on a rising edge. Thus it is there to ensure that the step pin has the correct value when a new movement command is called. Before step-dir was separated, the step pin was in state high for half of the step time and thus the disable command could otherwise leave the step pin in high, which would lead to a missing step for the new movement command.
I am not sure if this can still happen with the current step-dir implementation, as it toggles the pin twice in the same interrupt. That said, it still ensures the correct pin value, but one could argue, that it better fits during enabling.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized during testing that disabling of the timing source now was missing.
The a4979 also only takes a step on a rising edge. The implementation does work without ensuring the step pin having the correct value before a new movement command is called. Still I think it would make sense to set the step pin to zero during the init function. What are your opinions?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 on init would make sense, through 0 on enable (before setting enable pin to 1) might also make sense. At least on the drv8424 the step pin value is irrelevant while disabled but becomes important once enabled. I admit, I dont know what happens on the driver when you enable with step pin on 1. That said, this depends on how one views things happening on the step pin while disabled. If this is classified as undefined behavior, then 0 on init is enough.

data->enabled = enable;
if (!enable) {
config->common.timing_source->stop(dev);
}

return 0;
}

static int a4979_stepper_set_micro_step_res(const struct device *dev,
const enum stepper_micro_step_resolution micro_step_res)
{
const struct a4979_config *config = dev->config;
struct a4979_data *data = dev->data;
int ret;

uint8_t m0_value = 0;
uint8_t m1_value = 0;

switch (micro_step_res) {
case STEPPER_MICRO_STEP_1:
m0_value = 0;
m1_value = 0;
break;
case STEPPER_MICRO_STEP_2:
m0_value = 1;
m1_value = 0;
break;
case STEPPER_MICRO_STEP_4:
m0_value = 0;
m1_value = 1;
case STEPPER_MICRO_STEP_16:
m0_value = 1;
m1_value = 1;
break;
default:
LOG_ERR("Unsupported micro step resolution %d", micro_step_res);
return -EINVAL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should return -ENOTSUP. This is an error in the drv8424 driver and test suite that has not yet been fixed.

}

ret = a4979_set_microstep_pin(dev, &config->m0_pin, m0_value);
if (ret != 0) {
return ret;
}
ret = a4979_set_microstep_pin(dev, &config->m1_pin, m1_value);
if (ret != 0) {
return ret;
}

data->micro_step_res = micro_step_res;
return 0;
}

static int a4979_stepper_get_micro_step_res(const struct device *dev,
enum stepper_micro_step_resolution *micro_step_res)
{
struct a4979_data *data = dev->data;

*micro_step_res = data->micro_step_res;
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;
struct a4979_data *data = dev->data;
int ret;

bool has_enable_pin = config->en_pin.port != NULL;
bool has_reset_pin = config->reset_pin.port != NULL;

LOG_DBG("Initializing %s gpios", dev->name);

/* Configure reset pin if it is available */
if (has_reset_pin) {
if (!gpio_is_ready_dt(&config->reset_pin)) {
LOG_ERR("Enable Pin is not ready");
return -ENODEV;
}

ret = gpio_pin_configure_dt(&config->reset_pin, GPIO_OUTPUT_ACTIVE);
if (ret != 0) {
LOG_ERR("%s: Failed to configure reset_pin (error: %d)", dev->name, ret);
return ret;
}
}

/* Configure enable pin if it is available */
if (has_enable_pin) {
if (!gpio_is_ready_dt(&config->en_pin)) {
LOG_ERR("Enable Pin is not ready");
return -ENODEV;
}

ret = gpio_pin_configure_dt(&config->en_pin, GPIO_OUTPUT_INACTIVE);
if (ret != 0) {
LOG_ERR("%s: Failed to configure en_pin (error: %d)", dev->name, ret);
return ret;
}
}

/* Configure microstep pin 0 */
if (!gpio_is_ready_dt(&config->m0_pin)) {
LOG_ERR("m0 Pin is not ready");
return -ENODEV;
}
ret = gpio_pin_configure_dt(&config->m0_pin, GPIO_OUTPUT_INACTIVE);
if (ret != 0) {
LOG_ERR("%s: Failed to configure m0_pin (error: %d)", dev->name, ret);
return ret;
}

/* Configure microstep pin 1 */
if (!gpio_is_ready_dt(&config->m1_pin)) {
LOG_ERR("m1 Pin is not ready");
return -ENODEV;
}
ret = gpio_pin_configure_dt(&config->m1_pin, GPIO_OUTPUT_INACTIVE);
if (ret != 0) {
LOG_ERR("%s: Failed to configure m1_pin (error: %d)", dev->name, ret);
return ret;
}

ret = a4979_stepper_set_micro_step_res(dev, data->micro_step_res);
if (ret != 0) {
LOG_ERR("Failed to set micro step resolution: %d", ret);
return ret;
}

ret = step_dir_stepper_common_init(dev);
if (ret != 0) {
LOG_ERR("Failed to initialize common stepper data: %d", ret);
return ret;
}

return 0;
}

static DEVICE_API(stepper, a4979_stepper_api) = {
.enable = a4979_stepper_enable,
.move_by = a4979_stepper_move_by,
.move_to = a4979_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,
.set_micro_step_res = a4979_stepper_set_micro_step_res,
.get_micro_step_res = a4979_stepper_get_micro_step_res,
.set_event_callback = step_dir_stepper_common_set_event_callback
};

#define A4979_DEVICE(inst) \
\
static const struct a4979_config a4979_config_##inst = { \
.common = STEP_DIR_STEPPER_DT_INST_COMMON_CONFIG_INIT(inst), \
.en_pin = GPIO_DT_SPEC_INST_GET_OR(inst, en_gpios, {0}), \
.reset_pin = GPIO_DT_SPEC_INST_GET_OR(inst, reset_gpios, {0}), \
.m0_pin = GPIO_DT_SPEC_INST_GET(inst, m0_gpios), \
.m1_pin = GPIO_DT_SPEC_INST_GET(inst, m1_gpios), \
}; \
\
static struct a4979_data a4979_data_##inst = { \
.common = STEP_DIR_STEPPER_DT_INST_COMMON_DATA_INIT(inst), \
.micro_step_res = DT_INST_PROP(inst, micro_step_res), \
}; \
\
DEVICE_DT_INST_DEFINE(inst, a4979_init, NULL, &a4979_data_##inst, \
&a4979_config_##inst, POST_KERNEL, \
CONFIG_STEPPER_INIT_PRIORITY, &a4979_stepper_api);

DT_INST_FOREACH_STATUS_OKAY(A4979_DEVICE)
49 changes: 49 additions & 0 deletions dts/bindings/stepper/allegro/allegro,a4979.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# SPDX-FileCopyrightText: Copyright (c) 2025 Carl Zeiss Meditec AG
# SPDX-License-Identifier: Apache-2.0

description: |
Allegro A4979 microstepping stepper motor driver.
A4979 is a flexible microstepping motor driver with built-in translator for easy operation.
It is designed to operate bipolar stepper motors in full-, half-, quarter-, and sixteenth-step
modes.

Example:
a4979: a4979 {
status = "okay";
compatible = "allegro,a4979";
micro-step-res = <2>;
reset-gpios = <&gpiod 10 GPIO_ACTIVE_HIGH>;
dir-gpios = <&gpiod 14 GPIO_ACTIVE_HIGH>;
step-gpios = <&gpiod 15 GPIO_ACTIVE_HIGH>;
en-gpios = <&gpiod 11 GPIO_ACTIVE_HIGH>;
m0-gpios = <&gpiod 13 0>;
m1-gpios = <&gpiod 12 0>;
counter = <&counter5>;
};

compatible: "allegro,a4979"

include:
- name: stepper-controller.yaml
property-allowlist:
- micro-step-res
- step-gpios
- dir-gpios
- en-gpios
- counter

properties:
m0-gpios:
required: true
type: phandle-array
description: Microstep configuration pin 0.

m1-gpios:
required: true
type: phandle-array
description: Microstep configuration pin 1.

reset-gpios:
type: phandle-array
required: true
description: Reset pin
Loading
Loading