drivers: spi: atmel spi_sam0: Reset peripheral on init #90921
Conversation
github-actions
bot
added
platform: Microchip SAM
github-actions
bot
requested review from
attie-argentum,
mnkp,
nandojve,
pdgendt,
stephanosio,
tbursztyka and
teburd
There was a problem hiding this comment.
Overall LGTM, just small things and CI must be green : )
BTW, you need to follow the commit message guidelines.
I'm looking forward to accept you PR.
|
Sweet. Could you let me know what should be improved in the commit message? |
pdietl
force-pushed
the
pdietl/reset_sercom_periph_during_init
branch
2 times, most recently
from
6aa7608 to
c95b27e
Compare
|
@nandojve I rebased, split the changes into two commits, added conditional compilation for the CTRLC register, and reworded the commit messages. Please let me know if you find this satisfactory or if you desire additional changes :) |
The sam0 SPI driver does not ensure that it clears the 32-bit extension option during init. The 32-bit extension option, which comprises of a field in the CTRLC register and the LENGTH register enables better bus utilization by allowing 32-bit writes to the SPI DATA register (as opposed to the usual 8-bit writes). The driver breaks down if this option is enabled by causing each intended byte of output to become four bytes. We fix this by explicitly disabling the 32-bit extension option in init. Signed-off-by: Pete Dietl <[email protected]>
Reset the SPI peripheral to its default state and register values on init by setting its SWRST bit. This is important since the driver assumes that certain registers are at their default values. Signed-off-by: Pete Dietl <[email protected]>
pdietl
force-pushed
the
pdietl/reset_sercom_periph_during_init
branch
from
c95b27e to
1720bcb
Compare
|
|
@pdgendt I think you need to resolve some requested changes. |
How so? All mine are resolved, also note that unresolved comments do not block PRs. |
|
@pdgendt ah ok. I just see this I guess we need someone with write access to approve? |
|
@pdgendt Thanks again for your time with this review! |
I ran into a problem where a vendor-provided bootloader also configured the same SPI peripheral before booting my Zephyr application. The bootloader configured the ENABLE32B option to make writes to the DATA register be 32 bits and not 8 bits -- this broke the Zephyr driver. Since the configuration of the SPI peripheral may be arbitrarily configured before the Zephyr driver attempts to initialize it and because the Zephyr SPI driver assumes that the peripheral is in its default state, it makes sense to use the SWRST bit to ensure that the peripheral is indeed in its default state.