Skip to content

Conversation

@abonislawski
Copy link
Member

@abonislawski abonislawski commented Nov 18, 2025

Update dai_set_config() to accept and propagate the size parameter for bespoke DAI configuration data, matching the updated Zephyr DAI driver API.

Changes:

  • Add size_t size parameter to dai_set_config() function
  • Update dai_set_config() signature in dai-zephyr.h header
  • Pass copier_cfg->gtw_cfg.config_length as size in IPC4 dai_config()
  • Forward size parameter to dai_config_set() Zephyr driver API call

This change completes the integration with the new Zephyr DAI API that requires explicit size validation for bespoke configuration blobs, enabling proper bounds checking in DAI drivers.

Zephyr PR:

Copilot AI review requested due to automatic review settings November 18, 2025 11:32
@abonislawski abonislawski added the DNM Do Not Merge tag label Nov 18, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the dai_set_config() function signature to accept and propagate a size parameter for bespoke DAI configuration data, aligning with an updated Zephyr DAI driver API that requires explicit size validation for configuration blobs.

Key Changes:

  • Added size_t size parameter to dai_set_config() function signature across the codebase
  • Updated the function call in IPC4 dai_config() to pass copier_cfg->gtw_cfg.config_length as the size argument
  • Forwarded the size parameter to the underlying dai_config_set() Zephyr driver API call

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/ipc/ipc4/dai.c Updated dai_set_config() call to include config_length parameter
src/include/sof/lib/dai-zephyr.h Added size parameter to dai_set_config() function declaration
src/audio/dai-zephyr.c Updated dai_set_config() implementation to accept and forward size parameter to dai_config_set()

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Collaborator

@lyakh lyakh left a comment

Choose a reason for hiding this comment

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

you could update west.yml to point to that Zephyr PR and to run tests with it

Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Change is good, but I'll mark with DNM until the Zephyr dependency is met.

Copy link
Collaborator

@softwarecki softwarecki left a comment

Choose a reason for hiding this comment

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

Very good change! (●'◡'●)

@lgirdwood
Copy link
Member

@abonislawski can you check CI thanks !

@abonislawski
Copy link
Member Author

@lgirdwood Yes, the Zephyr PR is merged, so I need to add a west update here. However, another Zephyr PR also got merged at the same time, so first we need to merge #10173 and then move to this one.

@kv2019i
Copy link
Collaborator

kv2019i commented Nov 27, 2025

@abonislawski I think this needs to now go next. The Zephyr PR was merged and SOF cannot be updated without the changes in this PR. And to not break bisect, you need to update the Zephyr commit-id in a single git commit with the changes you have here.

Copy link

@majunkier majunkier left a comment

Choose a reason for hiding this comment

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

tested locally, fixes issue with error: too few arguments to function 'dai_config_set'

@abonislawski
Copy link
Member Author

@kv2019i firstly we need to merge #10173 with one zephyr update and then we can merge this one (with zephyr update to the newest version). This PR will fail without 10173

@kv2019i
Copy link
Collaborator

kv2019i commented Nov 27, 2025

Ack @abonislawski , #10173 now merged.

@lgirdwood
Copy link
Member

Ack @abonislawski , #10173 now merged.

rerun CI.

@lgirdwood
Copy link
Member

SOFCI TEST

b33fc9a7b2d4 drivers: dai: intel: ssp: Fix SSP blob v3.0 TLV parsing
b038ee72fae2 drivers: dai: Add size parameter to dai_config_set API
552c1514a6c5 drivers: dai: intel: ssp: Add support for
                                     SSP_GTW_DMA_CONFIG_ID TLV type

Signed-off-by: Adrian Bonislawski <adrian.bonislawski@intel.com>
Update dai_set_config() to accept and propagate the size parameter
for bespoke DAI configuration data, matching the updated Zephyr
DAI driver API.

Changes:
- Add size_t size parameter to dai_set_config() function
- Update dai_set_config() signature in dai-zephyr.h header
- Pass copier_cfg->gtw_cfg.config_length as size in IPC4 dai_config()
- Forward size parameter to dai_config_set() Zephyr driver API call

This change completes the integration with the new Zephyr DAI API
that requires explicit size validation for bespoke configuration
blobs, enabling proper bounds checking in DAI drivers.

Signed-off-by: Adrian Bonislawski <adrian.bonislawski@intel.com>
@abonislawski abonislawski force-pushed the ssp_dai_api_size branch 2 times, most recently from a5ff6f2 to e16862b Compare November 28, 2025 07:52
@abonislawski
Copy link
Member Author

Final zephyr update added, CI should pass now

@abonislawski abonislawski removed the DNM Do Not Merge tag label Nov 28, 2025
@lgirdwood lgirdwood merged commit b9fb0b4 into thesofproject:main Nov 28, 2025
38 of 42 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.

7 participants