linux-kernel - Re: [PATCH v3 2/7] ASoC: codecs: wcd937x: add wcd937x codec driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Tue, 7 May 2024 17:10:10 +0530
From: Mohammad Rafi Shaik <quic_mohs@...cinc.com>
To: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        "Srinivas
 Kandagatla" <srinivas.kandagatla@...aro.org>,
        Banajit Goswami
	<bgoswami@...cinc.com>,
        Liam Girdwood <lgirdwood@...il.com>, Mark Brown
	<broonie@...nel.org>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski
	<krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>, Jaroslav Kysela
	<perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>
CC: <linux-arm-msm@...r.kernel.org>, <alsa-devel@...a-project.org>,
        <linux-sound@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <quic_rohkumar@...cinc.com>,
        <quic_pkumpatl@...cinc.com>
Subject: Re: [PATCH v3 2/7] ASoC: codecs: wcd937x: add wcd937x codec driver

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/sound/soc/codecs/wcd938x.c?h=next-20240507), 
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/drivers/soundwire/qcom.c?h=next-20240507

Thanks & Regards,
Rafi.