Skip to content

soundwire: cadence_master: use full data slots in BRA frames #5482

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bardliao wants to merge 2 commits into from
Closed

soundwire: cadence_master: use full data slots in BRA frames #5482

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5f68ab3
soundwire: cadence_master: make frame index trace more readable
bardliao Jul 4, 2025
7c9cc51
soundwire: cadence_master: use full data slots in BRA frames
bardliao Jul 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 26 additions & 65 deletions drivers/soundwire/cadence_master.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2109,7 +2109,6 @@ int sdw_cdns_bpt_find_buffer_sizes(int command, /* 0: write, 1: read */
unsigned int actual_bpt_bytes;
unsigned int pdi0_tx_size;
unsigned int pdi1_rx_size;
unsigned int remainder;

if (!data_bytes)
return -EINVAL;
Expand All @@ -2118,9 +2117,6 @@ int sdw_cdns_bpt_find_buffer_sizes(int command, /* 0: write, 1: read */
if (!actual_bpt_bytes)
return -EINVAL;

if (data_bytes < actual_bpt_bytes)
actual_bpt_bytes = data_bytes;

/*
* the caller may want to set the number of bytes per frame,
* allow when possible
Expand All @@ -2130,43 +2126,25 @@ int sdw_cdns_bpt_find_buffer_sizes(int command, /* 0: write, 1: read */

*data_per_frame = actual_bpt_bytes;

/*
* For writes we need to send all the data_bytes per frame, even for the last frame
* 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.
Copy link

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

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

Typo in comment: 'date' should be 'data'.

Suggested change
* header and ignore the unrequested date.
* header and ignore the unrequested data.

Copilot uses AI. Check for mistakes.
*/
*num_frames = DIV_ROUND_UP(data_bytes, actual_bpt_bytes);
if (command == 0) {
/*
* for writes we need to send all the data_bytes per frame,
* even for the last frame which may only transport fewer bytes
*/

*num_frames = DIV_ROUND_UP(data_bytes, actual_bpt_bytes);

pdi0_tx_size = sdw_cdns_write_pdi0_buffer_size(actual_bpt_bytes);
pdi1_rx_size = SDW_CDNS_WRITE_PDI1_BUFFER_SIZE;

*pdi0_buffer_size = pdi0_tx_size * *num_frames;
*pdi1_buffer_size = pdi1_rx_size * *num_frames;
} else {
/*
* for reads we need to retrieve only what is requested in the BPT
* header, so the last frame needs to be special-cased
*/
*num_frames = data_bytes / actual_bpt_bytes;

pdi0_tx_size = SDW_CDNS_READ_PDI0_BUFFER_SIZE;
pdi1_rx_size = sdw_cdns_read_pdi1_buffer_size(actual_bpt_bytes);

*pdi0_buffer_size = pdi0_tx_size * *num_frames;
*pdi1_buffer_size = pdi1_rx_size * *num_frames;

remainder = data_bytes % actual_bpt_bytes;
if (remainder) {
pdi0_tx_size = SDW_CDNS_READ_PDI0_BUFFER_SIZE;
pdi1_rx_size = sdw_cdns_read_pdi1_buffer_size(remainder);

*num_frames = *num_frames + 1;
*pdi0_buffer_size += pdi0_tx_size;
*pdi1_buffer_size += pdi1_rx_size;
}
}

*pdi0_buffer_size = pdi0_tx_size * *num_frames;
*pdi1_buffer_size = pdi1_rx_size * *num_frames;

return 0;
}
EXPORT_SYMBOL(sdw_cdns_bpt_find_buffer_sizes);
Expand Down Expand Up @@ -2382,7 +2360,11 @@ int sdw_cdns_prepare_read_dma_buffer(u8 dev_num, u32 start_register, int data_si
header[0] |= GENMASK(7, 6); /* header is active */
header[0] |= (dev_num << 2);

while (data_size >= data_per_frame) {
/*
* 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) {
Copy link

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

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

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) {

Copilot uses AI. Check for mistakes.
header[1] = data_per_frame;
header[2] = start_register >> 24 & 0xFF;
header[3] = start_register >> 16 & 0xFF;
Expand All @@ -2405,23 +2387,6 @@ int sdw_cdns_prepare_read_dma_buffer(u8 dev_num, u32 start_register, int data_si

start_register += data_per_frame;
}

if (data_size) {
header[1] = data_size;
header[2] = start_register >> 24 & 0xFF;
header[3] = start_register >> 16 & 0xFF;
header[4] = start_register >> 8 & 0xFF;
header[5] = start_register >> 0 & 0xFF;

ret = sdw_cdns_prepare_read_pd0_buffer(header, SDW_CDNS_BRA_HDR, p_dma_buffer,
dma_buffer_size, &dma_data_written,
counter);
if (ret < 0)
return ret;

total_dma_data_written += dma_data_written;
}

*dma_buffer_total_bytes = total_dma_data_written;

return 0;
Expand Down Expand Up @@ -2499,14 +2464,14 @@ int sdw_cdns_check_write_response(struct device *dev, u8 *dma_buffer,
ret = check_frame_start(header, counter);
if (ret < 0) {
dev_err(dev, "%s: bad frame %d/%d start header %x\n",
__func__, i, num_frames, header);
__func__, i + 1, num_frames, header);
return ret;
}

ret = check_frame_end(footer);
if (ret < 0) {
dev_err(dev, "%s: bad frame %d/%d end footer %x\n",
__func__, i, num_frames, footer);
__func__, i + 1, num_frames, footer);
return ret;
}

Expand Down Expand Up @@ -2563,47 +2528,43 @@ int sdw_cdns_check_read_response(struct device *dev, u8 *dma_buffer, int dma_buf
u32 footer;
u8 expected_crc;
u8 crc;
int len;
int ret;
int i;

counter = CDNS_BPT_ROLLING_COUNTER_START;
p_data = (u32 *)dma_buffer;
p_buf = buffer;

/* All frames include the last frame have data_per_frame data */
for (i = 0; i < num_frames; i++) {
header = *p_data++;

ret = check_frame_start(header, counter);
if (ret < 0) {
dev_err(dev, "%s: bad frame %d/%d start header %x\n",
__func__, i, num_frames, header);
__func__, i + 1, num_frames, header);
return ret;
}

len = data_per_frame;
if (total_num_bytes + data_per_frame > buffer_size)
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);
Copy link

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

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

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).

Copilot uses AI. Check for mistakes.

p_data += (len + 1) / 2;
p_data += (data_per_frame + 1) / 2;
expected_crc = *p_data++ & 0xff;

if (crc != expected_crc) {
dev_err(dev, "%s: bad frame %d/%d crc %#x expected %#x\n",
__func__, i, num_frames, crc, expected_crc);
__func__, i + 1, num_frames, crc, expected_crc);
return -EIO;
}

p_buf += len;
total_num_bytes += len;
p_buf += data_per_frame;
total_num_bytes += data_per_frame;

footer = *p_data++;
ret = check_frame_end(footer);
if (ret < 0) {
dev_err(dev, "%s: bad frame %d/%d end footer %x\n",
__func__, i, num_frames, footer);
__func__, i + 1, num_frames, footer);
return ret;
}

Expand Down
Loading