Skip to content

drivers: stepper: STM Timer Based Step-Dir Implementation #87800

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jbehrensnx
Copy link
Contributor

This PR includes a step-dir implementation using stm timers - making it vendor and board specific, but requiring neither the workqueue nor interrupts for the step signal generation.
Its goal is to showcase an example of how to implement something like this and to support an RFC about changes on how multiple step-dir implementations should be handled.

@jbehrensnx
Copy link
Contributor Author

@jilaypandya @faxe1008
While it is some time until I will remove the draft status, I am still interested in some feedback for the new step-dir implementation.

.sleep_pin = GPIO_DT_SPEC_INST_GET_OR(inst, sleep_gpios, {0}), \
.en_pin = GPIO_DT_SPEC_INST_GET_OR(inst, en_gpios, {0}), \
.m0_pin = GPIO_DT_SPEC_INST_GET(inst, m0_gpios), \
.m1_pin = GPIO_DT_SPEC_INST_GET(inst, m1_gpios), \
}; \
\
static struct drv8424_data drv8424_data_##inst = { \
.common = STEP_DIR_STEPPER_DT_INST_COMMON_DATA_INIT(inst), \
.common = STEP_DIR_STEPPER_DT_INST_STM_TIMER_DATA_INIT(inst), \
Copy link
Member

Choose a reason for hiding this comment

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

The driver then supports only STM timer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct. That's its limitation. For different timers (nxp, nordic, ...) different drivers need to be developed. One of the goals of this step-dir implementation is to raise awareness that you can use stepper drivers this way and to create a blueprint for this class of step-dir implementations to show how they can be implemented.

Added an additional step-dir implementation using stm-timers.

Signed-off-by: Jan Behrens <[email protected]>
Added, for demonstration purposes, the stm-timer based step-dir implementation
to the drv8424 stepper driver.
This is done only for the Nucleo F767zi boards. Consequently all other drv8424
test are currently broken. This is not an issue, as this commit is intended to
be droped before merge.

Signed-off-by: Jan Behrens <[email protected]>
@jbehrensnx jbehrensnx marked this pull request as ready for review May 23, 2025 08:48
@jbehrensnx jbehrensnx force-pushed the stm-step-dir-experiments branch from b2c43e1 to deb09ab Compare May 23, 2025 08:49
@jbehrensnx
Copy link
Contributor Author

Made a lot of small (and some larger changes). Primarily focused on bugfixes, but some new features like automatic timer prescaler calculation were introduced as well.

I am at this point primarily interested in feedback on the algorithm itself.
Currently the stm-timer based step-dir is hardcoded into the drv8424 driver, as the current setup does not allow for switching the step-dir implementation. I do have plans for a solution for this, involving storing the step-dir function pointers in a struct - similar to how it is currently done for the timing-sources. I plan to open a separate PR for this approach.

The advantage of this step-dir approach, vendor locked as it may be, is speed. On a Nucleo WB55RG the maximum step frequency with the counter timing-source is about 20 kHz, higher and the step signal becomes unstable. At this point the processor is basically servicing ISRs the whole time, which makes this very inefficient. With the st-timer approach, I was easily able to surpass the drv8424s maximum Frequency of 500 kHz, through that had its own issue, as described below.

There is currently an issue at very high step frequencies. The stepper will take additional steps before stopping, taking 101 instead of 100 steps for example. The position is still tracked correctly however, returning a position of 101 in that example. The effect gets worse the higher the frequency and is caused by hardware and software delays between the step generator generating the last step and the timer being stopped. The starting threshold is board specific and on the Nucleo WB55RG lies somewhere between 60-80 kHz step frequency. The real world effect of this issue should however be fairly low, as the use of these high frequencies also usually also includes acceleration and deceleration (at this point in time at application level), at which point the frequency before stopping is low enough that this issue would not occur. And in addition, the position is still tracked correctly, allowing move_to to prevent cumulative errors.
Another issue is that this step-dir implementation does not allow for single steps, always requiring at least 2 steps, a limitation of the timers.

I also used the drv8424 api tests to test the step-dir implementation in addition to manual tests with the shell.
The WB55RG and F767ZI overlays also show a difference. On the WB55RG the internal timer connection (trigger-input) is defined through a board specific constant, while on the F767ZI the value is entered directly, as these constants are not defined for this board. This idea comes from #26221, the first proposal for a stepper api, which features a stepper driver with the same approach, through I only discovered the PR late in development.

I am removing the draft status in the hopes of increasing engagement, as I have so far gotten little feedback.

Copy link

@bjarki-andreasen
Copy link
Contributor

bjarki-andreasen commented May 23, 2025

The step/dir would be common for all stepper drivers which use the step/dir paradigm, this is completely common is it not? The step generation is vendor specific sure, but a stepper driver like the drv8424 surely does not care how the steps are signals are generated, just like a sensor would not care exactly how an I2C transaction is performed. I thought we had aligned on this with the implementation of the common step_dir and timing sources? I would way prefer we update the common step_dir "API" to support both gpio "bit banging" step dir generation and vendor specific step/dir generation, and then update the drivers to use this more flexible abstration :)

Simple example: instead of having both

step_dir_stepper_stm_timer_move_by(const struct device *dev, const int32_t micro_steps);

and

step_dir_stepper_common_move_by(const struct device *dev, const int32_t micro_steps);

have

int (*step_dir_move_by_api)(const struct step_dir_ctx *ctx, const int32_t micro_steps);

struct step_dir_ctx {
        step_dir_move_by_api *move_by;
};

static inline int step_dir_move_by(const struct step_dir_ctx *ctx, const int32_t micro_steps)
{
        step_dir_ctx->move_by(ctx, micro_steps);
}

like we do for other abstract implementations :)

(&step_work_timing_source_api)), \
}
(&step_work_timing_source_api)), \
}
Copy link
Member

Choose a reason for hiding this comment

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

unrelated changes :)

struct counter_top_cfg cfg_count;
uint32_t counter_gen_base_freq;

#ifdef CONFIG_STEPPER_STEP_DIR_GENERATE_ISR_SAFE_EVENTS
Copy link
Member

Choose a reason for hiding this comment

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

I have been thinking about this since quite sometime, should we transfer this concern to the application code and reduce a bit of overhead on the driver side?

Ping @andre-stefanov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which concern exactly? If you mean that on the api side we always assume that the event callbacks are ISR safe, I am fine with that.

@jbehrensnx
Copy link
Contributor Author

The step/dir would be common for all stepper drivers which use the step/dir paradigm, this is completely common is it not? The step generation is vendor specific sure, but a stepper driver like the drv8424 surely does not care how the steps are signals are generated, just like a sensor would not care exactly how an I2C transaction is performed. I thought we had aligned on this with the implementation of the common step_dir and timing sources? I would way prefer we update the common step_dir "API" to support both gpio "bit banging" step dir generation and vendor specific step/dir generation, and then update the drivers to use this more flexible abstration :)

If I understood you correctly, that is my plan as well. It is just that this part is not ready yet. I was waiting on how the stepper-api rework went, as that would have solved that issue a different way. With that not coming for a while a different solution is needed and my idea goes along the same way as yours. But I simply hadn't had time to work on it yet. And in the meantime, I can can collect and implement feedback on the already existing stm-timer code, as the proposed step-dir changes would not lead to massive changes.
With other words, I am interested in feedback on the stm-timer process themself, while acknowledging that changes to the step-dir system are still needed to allow switching between them, which I plan to make in the form you proposed.

@bjarki-andreasen
Copy link
Contributor

The step/dir would be common for all stepper drivers which use the step/dir paradigm, this is completely common is it not? The step generation is vendor specific sure, but a stepper driver like the drv8424 surely does not care how the steps are signals are generated, just like a sensor would not care exactly how an I2C transaction is performed. I thought we had aligned on this with the implementation of the common step_dir and timing sources? I would way prefer we update the common step_dir "API" to support both gpio "bit banging" step dir generation and vendor specific step/dir generation, and then update the drivers to use this more flexible abstration :)

If I understood you correctly, that is my plan as well. It is just that this part is not ready yet. I was waiting on how the stepper-api rework went, as that would have solved that issue a different way. With that not coming for a while a different solution is needed and my idea goes along the same way as yours. But I simply hadn't had time to work on it yet. And in the meantime, I can can collect and implement feedback on the already existing stm-timer code, as the proposed step-dir changes would not lead to massive changes. With other words, I am interested in feedback on the stm-timer process themself, while acknowledging that changes to the step-dir system are still needed to allow switching between them, which I plan to make in the form you proposed.

ok, makes sense :)

Comment on lines +6 to +8
include:
- name: pinctrl-device.yaml

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
include:
- name: pinctrl-device.yaml
include: pinctrl-device.yaml

@@ -9,7 +9,8 @@
#include <zephyr/drivers/stepper.h>
#include <zephyr/drivers/gpio.h>
#include <zephyr/drivers/stepper/stepper_drv8424.h>
#include "../step_dir/step_dir_stepper_common.h"
// #include "../step_dir/step_dir_stepper_common.h"
#include "../step_dir/step_dir_stepper_stm_timer.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think no relative paths shall be used. Can't find any rules from zephyr side - but I've not seen any relative include paths so far.

@jilaypandya
Copy link
Member

Ping @adrienbruant, i think you also tried something similar in the same direction sometime back. #35778 (comment)

Using an extended API seems to be the correct approach.

@adrienbruant
Copy link
Contributor

adrienbruant commented Jul 7, 2025

Ping @adrienbruant, i think you also tried something similar in the same direction sometime back. #35778 (comment)

Sorry I don't work on the project that got me involved with steppers anymore, so I haven't followed the latest developments, and my work was from before the stepper API so I can only give general feedback.

As I understand it, this PR uses two chained timers in primary secondary mode. I had considered it but instead decided to use the repeat counter (TIMx_RCR register) to:

  • Generate the microstep pulses at every counter overflow,
  • Generate an ISR every n overflow events, with n being the microstep resolution,
  • Update the pulse rate (to manage acceleration profile) during the ISR,

which saved one counter. The difference was that my stepper API only moved by full steps.

An idea I had but never got around to implement was to precalculate acceleration profiles and use DMA to load the new value in the counter. I imagine this could be necessary at higher pulse rate. This would be in the scope of a motion planner but maybe it needs provisioning at the driver level to connect the various events.

@jbehrensnx
Copy link
Contributor Author

jbehrensnx commented Jul 8, 2025

As I understand it, this PR uses two chained timers in primary secondary mode. I had considered it but instead decided to use the repeat counter (TIMx_RCR register)

I considered the repeat counter as well, but I found 3 issues that prevented me from using it (through they might not have been relevant to your api):

  • On many timers the repetition counter (if they have one at all) is only 8 bit, so a max value of 255. This would still require a large number of interrupts per second at high speeds, something that I planned to avert with this driver.
  • There is to my knowledge no way to get the current state of the repetition counter. This makes position checks during movement not possible and precludes getting the correct position after stopping from the run command
  • You can not, to my knowledge automatically stop the timer when the repetition counter triggers. As a result the additional steps that occur at extremely high speeds would still happen, just with the difference that their effect on the position would not be detected by the driver. That said, thinking about it more, a combination of repetition counter and one-shot mode might work, this will need more investigating. Still, the other points remain, but it might make a second driver with only one counter viable).

I know that these things were not relevant for you, but I want others to know the reasoning for using 2 timers instead of one.

An idea I had but never got around to implement was to precalculate acceleration profiles and use DMA to load the new value in the counter. I imagine this could be necessary at higher pulse rate. This would be in the scope of a motion planner but maybe it needs provisioning at the driver level to connect the various events.

This is something I thought about as well, but I wanted to star without it, as DMA under Zephyr has its share of pitfalls and I wanted to do something simpler to start. That said, depending on how the project I am developing this for goes, DMA based acceleration might come later.

EDIT: I did some further investigation into combining repetition counter and and one-pulse mode. It is possible to create a stepper driver from this, but it has its own set of limitations and pitfalls. It might be worthwhile to create such a driver, but in terms of capabilities it would have some serious differences from this driver. Still, I think there would be several use cases for it, but I don't plan to develop it myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants