drivers: tmc51xx: move DT_DRV_COMPAT to tmc51xx.c #100196
Open
+136
−189
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jilaypandya
force-pushed
the
refactor/tmc51xx_dt_drv_compat
branch
2 times, most recently
from
8f2b1e9 to
d809af8
Compare
jilaypandya
force-pushed
the
refactor/tmc51xx_dt_drv_compat
branch
from
d809af8 to
cf3054b
Compare
zephyrbot
requested review from
MaureenHelm,
bjarki-andreasen,
fabiobaltieri,
faxe1008,
galak,
jbehrensnx,
microbuilder,
ozersa,
ttmut and
yasinustunerg
fabiobaltieri
previously approved these changes
Nov 28, 2025
jbehrensnx
previously approved these changes
Nov 28, 2025
ttmut
reviewed
Nov 28, 2025
Comment on lines
26
to
34
| #if TMC51XX_BUS_SPI | ||
| /* SPI bus I/O operations for TMC51xx devices */ | ||
| const struct tmc_bus_io tmc51xx_spi_bus_io; | ||
| #endif | ||
|
|
||
| #if TMC51XX_BUS_UART | ||
| /* UART bus I/O operations for TMC51xx devices */ | ||
| const struct tmc_bus_io tmc51xx_uart_bus_io; | ||
| #endif |
Contributor
There was a problem hiding this comment.
Suggested change
| #if TMC51XX_BUS_SPI | |
| /* SPI bus I/O operations for TMC51xx devices */ | |
| const struct tmc_bus_io tmc51xx_spi_bus_io; | |
| #endif | |
| #if TMC51XX_BUS_UART | |
| /* UART bus I/O operations for TMC51xx devices */ | |
| const struct tmc_bus_io tmc51xx_uart_bus_io; | |
| #endif |
tmc51xx_spi_bus_io and tmc51xx_uart_bus_io are already being declared in lines 107 and 158.
ttmut
reviewed
Nov 28, 2025
| } | ||
|
|
||
| /* Wait for the read to complete */ | ||
| k_sleep(K_MSEC(1)); |
Contributor
There was a problem hiding this comment.
I am not familiar with stepper driver ICs but why do you need a delay after a read operation?
Member
Author
There was a problem hiding this comment.
Not really sure about this as well. Ping @dipakgmx.
Member
There was a problem hiding this comment.
Not necessary. Can be eliminated.
ttmut
reviewed
Nov 28, 2025
| } | ||
|
|
||
| /* Wait for the write to complete */ | ||
| k_sleep(K_MSEC(1)); |
Contributor
There was a problem hiding this comment.
Is the delay really needed here?
TMC51XX would have to be remodeled as a mfd, this means that each device tmc51xx, tmc51xx_motion_controller and tmc51xx_stepper_driver will have their own DT_DRV_COMPATs. Whenever the common header is included as of now, the adi_tmc51xx DT_DRV_COMPAT would have to be undef-ed each time in order to define a new DT_DRV_COMPAT i.e. required by the devices of the tmc51xx mfd. Signed-off-by: Jilay Pandya <[email protected]>
jilaypandya
dismissed stale reviews from jbehrensnx and fabiobaltieri
via
71e1bcc
jilaypandya
force-pushed
the
refactor/tmc51xx_dt_drv_compat
branch
from
cf3054b to
71e1bcc
Compare
|
dipakgmx
approved these changes
Nov 28, 2025
fabiobaltieri
approved these changes
Nov 28, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TMC51XX would have to be remodeled as a mfd, this means that each device tmc51xx, tmc51xx_motion_controller and tmc51xx_stepper_driver will have their own DT_DRV_COMPATs.
Whenever the common header is included as of now, the adi_tmc51xx DT_DRV_COMPAT would have to be undef-ed each time in order to define a new DT_DRV_COMPAT i.e. required by the devices of the tmc51xx mfd.