xrun: storage: fix xrun_file_read_debounce() to return read bytes #97
Conversation
| @@ -43,10 +43,14 @@ static int xrun_file_read_debounce(struct fs_file_t *file, uint8_t *buf, size_t | |||
| LOG_DBG("file count %zd read %zd", count, read); | |||
| count -= read; | |||
| buf += read; | |||
| if (count && read < sizeof(debounce_buf)) { | |||
There was a problem hiding this comment.
You can just check for read==0 as this is the indication that read_fs reached end of file. At least this stands true for POSIX read() function ...
There was a problem hiding this comment.
Not sure what you mean - Zephyr doc says https://docs.zephyrproject.org/apidoc/latest/group__file__system__api.html#gaba7e07127777187eedcd6976d352ab76
A returned value may be lower than size if there were fewer bytes available than requested.
And it sounds logically correct - file size is 5K. buf size is 4K. Read request is 8K (for example)
- first read gets 4K.
- second read 1K. But not 0.
- total 5K
There was a problem hiding this comment.
Yeah, I was looking at POSIX docs, which add more context:
On success, the number of bytes read is returned (zero indicates end of file), and the file position is advanced by this number. It is not an error if this number is smaller than the
number of bytes requested; this may happen for example because fewer bytes are actually available right now (maybe because we were close to end-of-file, or because we are reading from
a pipe, or from a terminal), or because read() was interrupted by a signal.
and
If the file offset is at or past the
end of file, no bytes are read, and read() returns zero.
Honestly, I don't know how much this is applicable to Zephyr, but I expect that they plan basically the behaviour: ret = 0 indicates EOF and ret > 0 indicates that there still be might more data.
There was a problem hiding this comment.
Actually it's working as described above:
- file_pos is close to EOF. ret = EOF - file_pos
- file_pos is at EOF. ret = 0
xrun/Kconfig
Outdated
| @@ -47,7 +47,8 @@ config XRUN_IRQS_MAX | |||
|
|
|||
| config XRUN_STORAGE_DMA_DEBOUNCE | |||
| int "Set debounce buffer for FS storage access in KB" | |||
| default 4 | |||
| default 4 if BOARD_RPI_5 | |||
There was a problem hiding this comment.
We don't want zephyr-xenlib to know about specific board(s). This is dependency in wrong direction. Please remove this change.
There was a problem hiding this comment.
Ok. But here to options possible:
- Set it to 0 by default. and fix dom0-xt for RPI5
- Keep as is - but rcar don't need it
?
There was a problem hiding this comment.
Option 1, as it will not break existing setups.
Also, this feature is only required on boards with not IO-MMU, which are rare. So it should be disabled in most cases.
GrygiriiS
force-pushed
the
xenlib-fs_read_fix
branch
from
e9a37f7 to
3b4d59a
Compare
|
|
The xrun_file_read_debounce() should work the same way as fs_read() and so return number of read bytes on success. A returned value may be lower than size if there were fewer bytes available than requested. Hence fix xrun_file_read_debounce() to follow fs_read() approach. Signed-off-by: Grygorii Strashko <[email protected]> Acked-by: Dmytro Firsov <[email protected]> Tested-by: Oleksandr Grytsov <[email protected]>
The XRUN_STORAGE_DMA_DEBOUNCE is needed only for RPI5 for now, so set default value to 0. Signed-off-by: Grygorii Strashko <[email protected]> Acked-by: Dmytro Firsov <[email protected]> Tested-by: Oleksandr Grytsov <[email protected]>
GrygiriiS
force-pushed
the
xenlib-fs_read_fix
branch
from
3b4d59a to
0454386
Compare
This PR fixes: