drivers: i2s: nrf_tdm: Add guards for buffers size #100288
Conversation
adamkondraciuk
requested review from
masz-nordic,
nika-nordic,
nordic-krch and
nordic-segl
zephyrbot
requested review from
TomasBarakNXP,
anangl,
bjarki-andreasen,
jaz1-nordic,
kl-cruz,
lstnl and
magp-nordic
adamkondraciuk
force-pushed
the
NRFX-8224-TDM-size-guards
branch
from
f31fdd2 to
22e1dc0
Compare
adamkondraciuk
force-pushed
the
NRFX-8224-TDM-size-guards
branch
2 times, most recently
from
b555fef to
9c34500
Compare
drivers/i2s/i2s_nrf_tdm.c
Outdated
| tdm_cfg->block_size <= NRFX_TDM_MIN_TRANSFER_SIZE_ALLOWED) { | ||
| LOG_ERR("This device can only transmit full 32-bit words greater than %u bytes.", | ||
| NRFX_TDM_MIN_TRANSFER_SIZE_ALLOWED); | ||
| return -EIO; |
There was a problem hiding this comment.
i2s_configure() can not return -EIO. It must be -EINVAL.
See API documentation https://docs.zephyrproject.org/latest/doxygen/html/group__i2s__interface.html#ga299003d72146c127f88d7c12c08889cc
Same in line 692 and ?686?.
There was a problem hiding this comment.
Argh.
I didn't noticed that lines 692 and 686 are in i2s_write() function which instead shall return -EIO while it can't return -EINVAL.
https://docs.zephyrproject.org/latest/doxygen/html/group__i2s__interface.html#ga01edf23acc6c16bbaf718dab8061a7a0
Thus, line 487 -> shall be -EINVAL
lines 692 and 686 -> shall be -EIO
There was a problem hiding this comment.
Please align test if needed:
https://github.com/zephyrproject-rtos/zephyr/blob/main/tests/drivers/i2s/i2s_additional/src/main.c#L763
If I understand, once you align with Sebastian suggestion, there is not need to change test. |
adamkondraciuk
dismissed stale reviews from kl-cruz and nordic-krch
via
15cac1c
adamkondraciuk
force-pushed
the
NRFX-8224-TDM-size-guards
branch
from
9c34500 to
15cac1c
Compare
TDM peripheral requires TXD.MAXCNT and RXD.MAXCNT registers to be: - multiple of 4 bytes - larger than 8 bytes Signed-off-by: Adam Kondraciuk <[email protected]>
adamkondraciuk
force-pushed
the
NRFX-8224-TDM-size-guards
branch
from
15cac1c to
5be6e2e
Compare
|
Works in CI |
|
10 participants
TDM peripheral requires TXD.MAXCNT and RXD.MAXCNT registers to be: