drivers: stepper: Add timing source API #83342
Conversation
zephyrbot
requested review from
bjarki-andreasen,
decsny,
dipakgmx,
fabiobaltieri,
galak,
jilaypandya and
rruuaanng
faxe1008
force-pushed
the
stepper_counter
branch
3 times, most recently
from
fea6068 to
feab07a
Compare
| help | ||
| Enable library used for step direction stepper drivers. | ||
|
|
||
| config STEP_DIR_STEPPER_GENERATE_ISR_SAFE_EVENTS |
There was a problem hiding this comment.
I think this feature could be used by other drivers as well, hence could be probably refactored out to Kconfig.stepper_event_template
| The maximum number of stepper events that can be pending before new events | ||
| are dropped. | ||
|
|
||
| config STEP_DIR_STEPPER_COUNTER_TIMING |
There was a problem hiding this comment.
I think the idea was to go via device tree instead of Kconfig, in order to be able to select on per instance basis. #82843 (comment)
There was a problem hiding this comment.
Yeah I added the Kconfig to remove the source file from compilation if you are sure you only need the delayed work.
| int step_counter_timing_source_start(const struct device *dev); | ||
| bool step_counter_timing_source_needs_reschedule(const struct device *dev); | ||
| int step_counter_timing_source_stop(const struct device *dev); | ||
| bool step_counter_timing_source_is_running(const struct device *dev); |
There was a problem hiding this comment.
Do these functions need to be exposed?
There was a problem hiding this comment.
True, they do not need to be exposed. Removed the headers :^)
| stepper_timing_source_stop stop; | ||
| stepper_timing_source_is_running is_running; | ||
| }; | ||
|
|
There was a problem hiding this comment.
#if CONFIG_STEP_DIR_STEPPER_COUNTER_TIMING
extern const struct stepper_timing_source_api step_counter_timing_source_api;
#endif
extern const struct stepper_timing_source_api step_work_timing_source_api;
I think then drivers/stepper/step_dir/step_dir_stepper_counter_timing.h and drivers/stepper/step_dir/step_dir_stepper_work_timing.h could be dropped.
Adds a timing source api which is used by the step-dir stepper common code. This allows the reusable common code to configure different timing sources, since the initial delayable work implementation was inacurate for higher maximum velocities. Signed-off-by: Fabian Blatz <[email protected]>
Enables use of the counter dts property which allows to configure a counter device as the timing source for the stepping. Signed-off-by: Fabian Blatz <[email protected]>
Adds the counter property to the tmc2209 node to ensure counter dependant step-dir code is build by CI. Signed-off-by: Fabian Blatz <[email protected]>
faxe1008
force-pushed
the
stepper_counter
branch
from
feab07a to
5d4639a
Compare
| * | ||
| * @param node_id The devicetree node identifier. | ||
| */ | ||
| #define STEP_DIR_STEPPER_DT_COMMON_CONFIG_INIT(node_id) \ | ||
| { \ | ||
| .step_pin = GPIO_DT_SPEC_GET(node_id, step_gpios), \ | ||
| .dir_pin = GPIO_DT_SPEC_GET(node_id, dir_gpios), \ | ||
| .dir_pin = GPIO_DT_SPEC_GET(node_id, dir_gpios), \ | ||
| .dual_edge = DT_PROP_OR(node_id, dual_edge_step, false), \ |
There was a problem hiding this comment.
sorry just caught this: booleans are implicitly "false" if not set, this can just be DT_PROP(node_id, dual_edge_step), wanna change it while you are at it?
| .counter = DEVICE_DT_GET_OR_NULL(DT_PHANDLE(node_id, counter)), \ | ||
| .timing_source = COND_CODE_1(DT_NODE_HAS_PROP(node_id, counter), \ | ||
| (&step_counter_timing_source_api), \ | ||
| (&step_work_timing_source_api)), \ |
There was a problem hiding this comment.
nit: tailing tab alignment is off
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| zephyr_library() | ||
| zephyr_library_property(ALLOW_EMPTY TRUE) |
There was a problem hiding this comment.
this is probably not needed since there's some files enabled unconditionally
Adds a timing source API for the common step dir stepper code. This can also be used to simplify the ti drv8424 driver.