Looks mostly good, couple of comments below.
+static int wsa881x_prepare(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)+{
- struct wsa881x_priv *wsa881x = dev_get_drvdata(dai->dev);
- int ret;
- if (wsa881x->stream_prepared) {
sdw_disable_stream(wsa881x->sruntime);sdw_deprepare_stream(wsa881x->sruntime);wsa881x->stream_prepared = false;- }
in what scenario would you have a transition from a stream active to prepared?
- ret = sdw_prepare_stream(wsa881x->sruntime);
- if (ret)
return ret;- /**
* NOTE: there is a strict hw requirement about the ordering of port* enables and actual PA enable. PA enable should only happen after
PA == power amplifiers?
* soundwire ports are enabled if not DC on the line is accumlated
accumulated
* resulting in Click/Pop Noise*/- ret = sdw_enable_stream(wsa881x->sruntime);
I guess this answers to my question above, you are not using the 'usual' mapping between ALSA states and SoundWire stream states. Enabling the stream will cause a bank switch and (zero?) data to be transmitted, is this intentional?
If this is due to the order with the PA, then where is the PA handled?
- if (ret) {
sdw_deprepare_stream(wsa881x->sruntime);return ret;- }
- wsa881x->stream_prepared = true;
- return ret;
+}
+static int wsa881x_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params,struct snd_soc_dai *dai)+{
- struct wsa881x_priv *wsa881x = dev_get_drvdata(dai->dev);
- int i;
- wsa881x->active_ports = 0;
- for (i = 0; i < WSA881X_MAX_SWR_PORTS; i++) {
if (!wsa881x->port_enable[i])continue;wsa881x->port_config[wsa881x->active_ports] =wsa881x_pconfig[i];wsa881x->active_ports++;- }
- return sdw_stream_add_slave(wsa881x->slave, &wsa881x->sconfig,
wsa881x->port_config, wsa881x->active_ports,wsa881x->sruntime);+}
+static int wsa881x_hw_free(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)+{
- struct wsa881x_priv *wsa881x = dev_get_drvdata(dai->dev);
- sdw_disable_stream(wsa881x->sruntime);
- sdw_deprepare_stream(wsa881x->sruntime);
This works if you do a hw_params->prepare->hw_free transition, but isn't it possible to have hw_params->hw_free as well? In that case the stream would not enabled/prepared, so shouldn't you have the same test as in prepare?
if (wsa881x->stream_prepared) { sdw_disable_stream(wsa881x->sruntime); sdw_deprepare_stream(wsa881x->sruntime); wsa881x->stream_prepared = false; }
- sdw_stream_remove_slave(wsa881x->slave, wsa881x->sruntime);
- wsa881x->stream_prepared = false;
- return 0;
+}
+static struct snd_soc_dai_driver wsa881x_dais[] = {
- [0] = {
is that [0] needed?
.name = "SPKR",.id = 0,.playback = {.stream_name = "SPKR Playback",.rate_max = 48000,.rate_min = 48000,.channels_min = 1,.channels_max = 1,},.ops = &wsa881x_dai_ops,- },
+};