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: drv8424 use step_dir common code #83430

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

Conversation

faxe1008
Copy link
Collaborator

@faxe1008 faxe1008 commented Dec 28, 2024

drivers: stepper: drv8424 use step_dir common code

Now that the common code also has the timing source API available adjust the driver to leverage it.
CC @Irockasingranite

Copy link
Contributor

@jilaypandya jilaypandya left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -45,7 +45,7 @@ struct step_dir_stepper_common_config {
{ \
.step_pin = GPIO_DT_SPEC_GET(node_id, step_gpios), \
.dir_pin = GPIO_DT_SPEC_GET(node_id, dir_gpios), \
.dual_edge = DT_PROP(node_id, dual_edge_step), \
.dual_edge = DT_PROP_OR(node_id, dual_edge_step, false), \
Copy link
Contributor

Choose a reason for hiding this comment

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

This commit is not required, i suppose :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's needed otherwise the dual-edge-step property needs to be part of either the driver dts or the stepper-controller dts binding. You might not want to do this if the IC does not support this setting. But if the property is not present in dts compilation with just DT_PROP will fail.

This way only drivers that have that setting can declare it in their own dts, otherwise its just false.

Adds a fallback for the dual-edge-step property in the step dir common
code. Without this drivers using the common code would have to declare the
dual-edge-step property so it can default to false when not set, even if
the IC does not support adjusting it.

Signed-off-by: Fabian Blatz <[email protected]>
Address issue where the timing source is not stopped upon step completion
and not adjusting the actual position after performing a step.

Signed-off-by: Fabian Blatz <[email protected]>
Adapt the drv8424 driver to use the common step dir interface.

Signed-off-by: Fabian Blatz <[email protected]>
@faxe1008
Copy link
Collaborator Author

Found some issue in the common step_dir code that I missed, revealed by the drv8424 test suite.
Very good thing to have @Irockasingranite :^)

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.

4 participants