-
Notifications
You must be signed in to change notification settings - Fork 132
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.
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
ASoC/SoundWire: add BTP/BRA prerequisites #4735
ASoC/SoundWire: add BTP/BRA prerequisites #4735
Conversation
Likely a copy-paste error, wrong CONFIG used. Signed-off-by: Pierre-Louis Bossart <[email protected]>
Export this helper so that we can report the DPIB position if the BPT DMA do not complete - this is very useful to see if the DMA started or gets stuck somehow with invalid bandwidth configurations. Signed-off-by: Pierre-Louis Bossart <[email protected]>
For some reason, we add an offset to the PDI, presumably to skip the PDI0 and PDI1 which are reserved for BPT. This code is however completely wrong and leads to an out-of-bounds access. We were just lucky so far since we used only a couple of PDIs and remained within the PDI array bounds. A Fixes: tag is not provided since there are no known platforms where the out-of-bounds would be accessed, and the initial code had problems as well. A follow-up patch completely removes this useless offset. Signed-off-by: Pierre-Louis Bossart <[email protected]>
This offset is set to exactly zero and serves no purpose. Remove. Signed-off-by: Pierre-Louis Bossart <[email protected]>
This is redundant with sdw_bus_params, and was never used. Signed-off-by: Pierre-Louis Bossart <[email protected]>
The definitions for DP0 are missing a set of fields that are required to reuse the same configuration code as DPn. Signed-off-by: Pierre-Louis Bossart <[email protected]>
The existing code sets the maximum address at 0x80000000, which is not completely accurate. The last 2 Gbytes are indeed reserved, but so are the 896 Mbytes just before. The maximum address which can be used with paging or BRA is 0x47FFFFFF per Table 131 of the SoundWire 1.2.1 specification. Signed-off-by: Pierre-Louis Bossart <[email protected]>
We have an existing debugfs files to read standard registers (DP0/SCP/DPn). This patch provides a more generic interface to ANY set of read/write contiguous registers in a peripheral device. In follow-up patches, this interface will be extended to use BRA transfers. The sequence is to use the following files added under the existing debugsfs directory for each peripheral device: command (write 0, read 1) num_bytes start_address firmware_file (only for writes) read_buffer (only for reads) Example for a read command - this checks the 6 bytes used for enumeration. cd /sys/kernel/debug/soundwire/master-0-0/sdw\:0\:025d\:0711\:01/ echo 1 > command echo 6 > num_bytes echo 0x50 > start_address echo 1 > go cat read_buffer address 0x50 val 0x30 address 0x51 val 0x02 address 0x52 val 0x5d address 0x53 val 0x07 address 0x54 val 0x11 address 0x55 val 0x01 Example with a 2-byte firmware file written in DP0 address 0x22 od -x /lib/firmware/test_firmware 0000000 0a37 0000002 cd /sys/kernel/debug/soundwire/master-0-0/sdw\:0\:025d\:0711\:01/ echo 0 > command echo 2 > num_bytes echo 0x22 > start_address echo "test_firmware" > firmware_file echo 1 > go cd /sys/kernel/debug/soundwire/master-0-0/sdw\:0\:025d\:0711\:01/ echo 1 > command echo 2 > num_bytes echo 0x22 > start_address echo 1 > go cat read_buffer address 0x22 val 0x37 address 0x23 val 0x0a Signed-off-by: Pierre-Louis Bossart <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -931,6 +932,7 @@ struct sdw_bus { | |||
u32 bank_switch_timeout; | |||
bool multi_link; | |||
int hw_sync_min_links; | |||
int stream_refcount; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refcount is native supported by Linux kernel. How about to use it ? It can help to decrease multi-thread risk
refcount_inc, refcount_set, refcount_dec_and_test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a good point @RanderWang, I initially planned to use the refcount helpers, but there's a protection:
sdw_master_rt_alloc() and sdw_master_rt_free() need to be called with bus_lock held, it's clearly written in the documentation and it's used this way.
I didn't see the need for additional protection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will add a comment in the commit message to make this clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, get it
The notion of stream is by construction based on a multi-bus capability, to allow for aggregation of Peripheral devices or functions located on different segments. We currently count how many master_rt contexts are used by a stream, but we don't have the dual refcount of how many streams are allocated on a given bus. This refcount will be useful to check if BTP/BRA streams can be allocated. Note that the stream_refcount is modified in sdw_master_rt_alloc() and sdw_master_rt_free() which are both called with the bus_lock mutex held, so there's no need for refcount_ primitives for additional protection. Signed-off-by: Pierre-Louis Bossart <[email protected]>
0b2fd12
to
54b0b97
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
9aa94e7
into
thesofproject:topic/sof-dev
This PR includes non-controversial patches that should be merged and sent upstream in this kernel cycle. The full set of BTP/BRA patches is still in PR #4679 and will have to wait for an RFC and further comments.
@bardliao @ujfalusi @ranj063 @RanderWang thanks for your comments.