On 4/25/2024 7:13 PM, Pierre-Louis Bossart wrote:
+struct wcd937x_priv {
- struct sdw_slave *tx_sdw_dev;
- struct sdw_slave *rx_sdw_dev;
Does this mean that the codec has 2 SoundWire interfaces?
Yes, Similar to WCD938x (https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/sou...), it has two soundwire interfaces.
All read/write operations associated with WCD937x goes over tx_sdw.
rx_sdw_dev is unused in this driver. I will remove it.
If yes, aren't there merits in splitting the implementation in two separate drivers, one for each interface and probing on the relevant partID?
This is how the RT713 was handled. The mic function was exposed as the RT1713.
By representing the device as a single entity, things could be fun because the two interfaces are really independent. things like clock stop are handled at the interface level.
The code in this driver is difficult to review, for example in the probe you wait for the TX part to complete the enumeration/initialization, but there's nothing mentioned or stated on the RX part, and there's really nothing related to the detection of this device. I don't actually see a sdw_driver at all, it's a platform driver.
wcd937x waits for TX part to complete as it is needed for register read/write operations.
Please refer patch 3/7 for slave driver.
Would you mind adding a paragraph on how the SoundWire interfaces are handled and how the SoundWire bus is involved if there's no sdw_driver?
Thanks!
Sure, will add details in the commit message.
Soundwire master driver is already upstreamed and is available at https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/dri...
Thanks & Regards, Rafi.