soundwire: cadence_master: use full data slots in BRA frames #5482
Closed
bardliao wants to merge 2 commits intothesofproject:topic/sof-devfrom
Closed
soundwire: cadence_master: use full data slots in BRA frames #5482bardliao wants to merge 2 commits intothesofproject:topic/sof-devfrom
bardliao wants to merge 2 commits intothesofproject:topic/sof-devfrom
Conversation
The existing code uses i as the frame index which is 0 based. And "bad frame %d/%d", i, num_frames where num_frames is 1 based. Change the frame index to i + 1 to align with num_frames and more readable. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
For some reason, the transport may not complete if the last frame is not fully used. Always fully use the date slots in the frames and ignore the unrequired data can avoid the transport issue. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
There was a problem hiding this comment.
Pull Request Overview
The PR ensures that all BRA frames use the full data_per_frame length—even the last frame—to avoid transport hangs.
- Always calculate num_frames with DIV_ROUND_UP and apply full-slot sizing for both reads and writes in
sdw_cdns_bpt_find_buffer_sizes. - Simplify
sdw_cdns_prepare_read_dma_bufferby removing partial‐frame handling and usingwhile (data_size >= 0)to emit every frame at full length. - Convert error‐message frame indices from 0‐based to 1‐based in both write and read response checks.
Reviewed Changes
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| drivers/soundwire/cadence_master.c | Unify buffer‐size calculation to always use full slots; remove remainder logic in sdw_cdns_bpt_find_buffer_sizes. |
| drivers/soundwire/cadence_master.c | Change loop condition and remove special‐case for partial frames in sdw_cdns_prepare_read_dma_buffer. |
| drivers/soundwire/cadence_master.c | Adjust dev_err format to report frames as i+1 in sdw_cdns_check_write_response and sdw_cdns_check_read_response. |
| * Set message length = data_per_frame even if the required data is less then | ||
| * data_per_frame in the last frame. | ||
| */ | ||
| while (data_size >= 0) { |
There was a problem hiding this comment.
Looping on data_size >= 0 introduces an extra iteration when data_size hits zero, causing an unintended frame. Consider using while (data_size > 0) or a do { ... } while (data_size > 0) to accurately emit exactly DIV_ROUND_UP frames.
Suggested change
| while (data_size >= 0) { | |
| while (data_size > 0) { |
| * which may only transport fewer bytes. | ||
| * For reads for some reason the transport may not complete if the last frame is not | ||
| * fully used. We will set the data length of all frames are fully used to the BPT | ||
| * header and ignore the unrequested date. |
There was a problem hiding this comment.
Typo in comment: 'date' should be 'data'.
Suggested change
| * header and ignore the unrequested date. | |
| * header and ignore the unrequested data. |
| len = buffer_size - total_num_bytes; | ||
|
|
||
| crc = extract_read_data(p_data, len, p_buf); | ||
| crc = extract_read_data(p_data, data_per_frame, p_buf); |
There was a problem hiding this comment.
Removed boundary check means extract_read_data may write beyond the target buffer when the last frame is smaller than data_per_frame. Reintroduce logic to cap the extracted length to the remaining buffer size (buffer_size - total_num_bytes).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For some reason, the transport may not complete if the last frame is not
fully used. Always fully use the date slots in the frames and ignore the
unrequired data can avoid the transport issue.