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

Pierre-Louis Bossart
2023-01-30T16:46:03Z

On 1/27/23 10:51, Stefan Binding wrote:
> From: Richard Fitzgerald <rf@opensource.cirrus.com>
>
> 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@opensource.cirrus.com>
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

> +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.