Skip to content

Adding I2S restart callback#256

Merged
mbanth merged 5 commits into
xmos:developfrom
LeslieXMOS:feature/i2s_restart_cb
Oct 28, 2025
Merged

Adding I2S restart callback#256
mbanth merged 5 commits into
xmos:developfrom
LeslieXMOS:feature/i2s_restart_cb

Conversation

@LeslieXMOS

Copy link
Copy Markdown
Contributor

This pull request added an API to allow user to add a I2S restart callback.

Comment thread modules/drivers/i2s/api/rtos_i2s.h Outdated
*/
typedef size_t (*rtos_i2s_receive_filter_cb_t)(rtos_i2s_t *ctx, void *app_data, int32_t *i2s_frame, size_t i2s_frame_size, int32_t *receive_buf, size_t sample_spaces_free);

typedef size_t (*rtos_i2s_restart_cb_t)(rtos_i2s_t *ctx, void *app_data);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please provide the function pointer documentation. The documentation for rtos_i2s_send_filter_cb_t and rtos_i2s_receive_filter_cb_t provide examples of what is expected.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Michael

I had updated the documentation and return type of rtos_i2s_restart_cb_t, it should be more clear now

Comment thread modules/drivers/i2s/src/rtos_i2s.c Outdated
static i2s_restart_t i2s_restart_check(rtos_i2s_t *ctx)
{
if (ctx->restart_cb) {
ctx->restart_cb(ctx, ctx->restart_app_data);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't clear to me whether or not the registered restart_cb function should return a value that indicates if a restart of I2S is needed. The function registered for this callback in PR 393 of sln_voice (i2s_restart_cb in file main.c in the ffva example) has a signature that returns a size_t, but the function itself doesn't include a return statement. However, the last function called in i2s_restart_cb returns an enum that could be used in deciding whther or not to restart I2S.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Michael

The i2s_restart_check had updated to return i2s_restart_t value from ctx->restart_cb if ctx->restart_cb exist.
PR 393 of sln_voice also updated with the new i2s restart callback return value, it will return I2S_NO_RESTART.

Comment thread modules/drivers/i2s/api/rtos_i2s.h Outdated
*
* These functions must not block.
*
* \param ctx A pointer to the associated I2C slave driver instance.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: I2C -> I2S

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Ross

Fixed both my typo and the typo existed before

Comment thread modules/drivers/i2s/src/rtos_i2s.c Outdated
{
uint32_t core_exclude_map;

printf("mclk bclk ratio: %d\n", mclk_bclk_ratio);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it intentional to leave this print in place?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Ross

Sorry for accidentally leaving it here, removed now.

@mbanth mbanth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes look okay.

@mbanth

mbanth commented Oct 27, 2025

Copy link
Copy Markdown
Collaborator

@LeslieXMOS, do you have any idea how to fix the problem shown by the USB Drivers test?

@LeslieXMOS LeslieXMOS closed this Oct 28, 2025
@LeslieXMOS LeslieXMOS reopened this Oct 28, 2025
@mbanth
mbanth merged commit 8c543a7 into xmos:develop Oct 28, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants