Re: [PATCH v3 6/8] ASoC: cs42l42: Add SoundWire support

From: Richard Fitzgerald
Date: Tue Jan 31 2023 - 05:59:33 EST


On 30/01/2023 16:39, Pierre-Louis Bossart wrote:


On 1/27/23 10:51, Stefan Binding wrote:
From: Richard Fitzgerald <rf@xxxxxxxxxxxxxxxxxxxxx>

This adds support for using CS42L42 as a SoundWire device.

SoundWire-specifics are kept separate from the I2S implementation as
much as possible, aiming to limit the risk of breaking the I2C+I2S
support.

There are some important differences in the silicon behaviour between
I2S and SoundWire mode that are reflected in the implementation:

- ASP (I2S) most not be used in SoundWire mode because the two interfaces
share pins.

- The SoundWire capture (record) port only supports 1 channel. It does
not have left-to-right duplication like the ASP.

- DP2 can only be prepared if the HP has powered-up. DP1 can only be
prepared if the ADC has powered-up. (This ordering restriction does
not exist for ASPs.) The SoundWire core port-prepare step is
triggered by the DAI-link prepare(). This happens before the
codec DAI prepare() or the DAPM sequence so these cannot be used
to enable HP/ADC. Instead the HP/ADC enable/disable are done during
the port_prep callback.

- The SRCs are an integral part of the audio chain but in silicon their
power control is linked to the ASP. There is no equivalent power link
to SoundWire DPs so the driver must take "manual" control of SRC power.

- The SoundWire control registers occupy the lower part of the SoundWire
address space so cs42l42 registers are offset by 0x8000 (non-paged) in
SoundWire mode.

- Register addresses are 8-bit paged in I2C mode but 16-bit unpaged in
SoundWire.

- Special procedures are needed on register read/writes to (a) ensure
that the previous internal bus transaction has completed, and
(b) handle delayed read results, when the read value could not be
returned within the SoundWire read command.

There are also some differences in driver implementation between I2S
and SoundWire operation:

- CS42L42 I2S does not runtime_suspend, but runtime_suspend/resume support
has been added into the driver in SoundWire mode as the most convenient
way to power-up the bus manager and to handle the unattach_request
condition, though the CS42L42 chip does not itself suspend or resume.

- Intel SoundWire host controllers have a low-power clock-stop mode that
requires resetting all peripherals when resuming. This means that the
interrupt registers will be reset in between the interrupt being
generated and the interrupt being handled, and since the interrupt
status is debounced, these values may not be accurate immediately,
and may cause spurious unplug events before settling.

- As in I2S mode, the PLL is only used while audio is active because
of clocking quirks in the silicon. For SoundWire the cs42l42_pll_config()
is deferred until the DAI prepare(), to allow the cs42l42_bus_config()
callback to set the SCLK.

Signed-off-by: Richard Fitzgerald <rf@xxxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Stefan Binding <sbinding@xxxxxxxxxxxxxxxxxxxxx>

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>

+static int cs42l42_sdw_dai_set_sdw_stream(struct snd_soc_dai *dai, void *sdw_stream,
+ int direction)
+{
+ if (!sdw_stream)
+ return 0;
+
+ if (direction == SNDRV_PCM_STREAM_PLAYBACK)
+ dai->playback_dma_data = sdw_stream;
+ else
+ dai->capture_dma_data = sdw_stream;

This may need to be updated to
snd_soc_dai_dma_data_set(dai, direction, stream);

which is being introduced by Morimoto-san

To avoid dependencies this could be updated later.

We'll do it later. As you say, to avoid dependencies.
I've thought a few times about making a function for this but
couldn't decide what to call it to distinguish from the existing
snd_soc_dai_set_dma_data().
My preference was to create a SoC function that does exactly the same as
cs42l42_sdw_dai_set_sdw_stream() so if that's all you need you can
point your snd_soc_dai_ops directly at it.