Hi Syed, I'll look to try this out this week! The code has changed a bit since I last looked at it properly, I had a query in amd_sdw_bpt_wait() and I think this may be missing bra_block_alignment support that recently entered the tree in https://lore.kernel.org/all/[email protected]/
commit c0840f8be5d59071096b8e6b42693a0d912b7cba Author: Richard Fitzgerald <[email protected]> Date: Tue Jul 28 20:46:36 2026 +0800 soundwire: Add bra_block_alignment property support Add a property to struct sdw_slave_prop equivalent to the Disco property "mipi-sdw-bra-mode-block-alignment". The SoundWire Disco specification defines this as: "The data payload size for this BRA Mode shall be an integer multiple of the value of this Property." Signed-off-by: Richard Fitzgerald <[email protected]> Co-developed-by: Bard Liao <[email protected]> Signed-off-by: Bard Liao <[email protected]> Reviewed-by: Péter Ujfalusi <[email protected]> Reviewed-by: Pierre-Louis Bossart <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Vinod Koul <[email protected]> Thanks, -Simon On 9/9/26 13:56, Syed Saba Kareem wrote: > ... > - Non-contiguous firmware sections are handled by iterating > per-section: large sections use BRA DMA, small sections > (< one BRA frame) fall back to sdw_nwrite/sdw_nread. Query about handling of contiguous small sections > ... > --- a/drivers/soundwire/amd_manager.c > +++ b/drivers/soundwire/amd_manager.c > ..... > +static int amd_sdw_bpt_wait(struct sdw_bus *bus, > + struct sdw_slave *slave, > + struct sdw_bpt_msg *msg) > +{ > ... > + * Prepare DP0 via SoundWire framework so the core programs the > + * peripheral DP0 transport/port registers and issues PREPARECTRL. > + * This is invoked from the BPT transfer context (firmware callback) > + * and not from update_status(), so it is safe w.r.t. sdw_dev_lock. > + */ > + ret = sdw_prepare_stream(bus->bpt_stream); > + if (ret < 0) { > + dev_err(amd_manager->dev, > + "BPT: sdw_prepare_stream failed: %d\n", ret); > + goto deconfigure_pte; > + } > + dev_dbg(amd_manager->dev, > + "BPT: stream prepared, curr_bank=%u next_bank=%u > state=%d\n", > + bus->params.curr_bank, bus->params.next_bank, > + bus->bpt_stream->state); > + > + if (amd_sdw_sections_are_contiguous(msg)) { Have we dropped handling of when a contiguous transfer is less than bytes_per_frame like in the non-contiguous case? > + /* > + * All sections are contiguous in peripheral address space. > + * A single BRA call covers the entire firmware image. > + */ > + ret = amd_sdw_bra_transfer(amd_manager, slave, > + msg->sec[0].addr, > + acp_sys_addr, > + total_len, is_write, > + &dma_unsafe); > + if (ret < 0) { > + dev_err(amd_manager->dev, > + "BPT contiguous transfer failed: addr=0x%x > len=%zu ret=%d\n", > + msg->sec[0].addr, total_len, ret); > + /* > + * Skip the read-back copy below so a failed read > + * cannot return stale DMA buffer contents to the > + * caller as if the transfer had succeeded. > + */ > + goto deconfigure_pte; > + } > + } else { > + /* > + * Non-contiguous sections: each section targets a different > + * peripheral address range. The ACP BRA DMA engine is > + * triggered by sdw_enable_stream() (bank switch + > CHANNELEN), so > + * each section needs its own full config -> activate -> > + * run_dma -> deactivate -> deconfig cycle. > + * > + * Sections smaller than one BRA frame (bytes_per_frame) > + * cannot be transferred via DMA because the engine never > + * starts for sub-frame payloads. Use regular SDW register > + * read/write commands for those tiny sections instead. > + */ > + offset = 0; > + for (i = 0; i < msg->sections; i++) { > + if (i < 3 || i == msg->sections - 1) > + dev_dbg(amd_manager->dev, > + "BPT nc sec[%d/%d]: periph=0x%08x > len=%u acp=0x%08x\n", > + i, msg->sections, msg->sec[i].addr, > + msg->sec[i].len, > + acp_sys_addr + (u32)offset); > + if (msg->sec[i].len < prep_params.bytes_per_frame) { > + /* > + * Section too small for BRA DMA -- use > + * regular SDW byte-level commands instead. > + */ > + if (is_write) > + ret = sdw_nwrite_no_pm(slave, > + msg->sec[i].addr, > + msg->sec[i].len, > + dma_buf + > offset); > + else > + ret = sdw_nread_no_pm(slave, > + msg->sec[i].addr, > + msg->sec[i].len, > + dma_buf + offset); > + if (ret < 0) > ...
