+static int cs42l42_sdw_dai_startup(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct cs42l42_private *cs42l42 = snd_soc_component_get_drvdata(dai->component);
+ if (!cs42l42->init_done) + return -ENODEV;
Can this happen? IIRC the ASoC framework would use pm_runtime_resume_and_get() before .startup, which would guarantee that the device is initialized, no?
Yes, this can happen. Because of the way that the SoundWire enumeration was implemented in the core code, it isn't a probe event so we cannot call snd_soc_register_component() on enumeration because -EPROBE_DEFER wouldn't be handled. So the snd_soc_register_component() must be called from probe(). This leaves a limbo situation where we've registered the driver but in fact don't yet have any hardware. ALSA/ASoC doesn't know that we've registered before we are functional so they are happy to go ahead and try to use the soundcard. If for some reason the hardware failed to enumerate we can get here without having enumerated.
Humm, yes, but you've also made the regmap cache-only, so is there really a problem?
FWIW I don't see a startup callback in any other codec driver. It may be wrong but it's also a sign that this isn't a problem we've seen so far on existing Intel-based platforms.
+ return 0; +}
+static int cs42l42_sdw_dai_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai) +{ + struct cs42l42_private *cs42l42 = snd_soc_component_get_drvdata(dai->component); + struct sdw_stream_runtime *sdw_stream = snd_soc_dai_get_dma_data(dai, substream); + struct sdw_stream_config stream_config = {0}; + struct sdw_port_config port_config = {0}; + int ret;
+ if (!sdw_stream) + return -EINVAL;
+ /* Needed for PLL configuration when we are notified of new bus config */ + cs42l42->sample_rate = params_rate(params);
wouldn't it be better to check if the sample_rate is supported by the PLL here, instead of in the .prepare step ...
It depends on the soundwire bus clock. We need to know both to determine whether they are valid. IFF we can assume that the call to sdw_stream_add_slave() will always invoke the bus_config() callback we can call cs42l42_pll_config() from cs42l42_sdw_bus_config() and return an error from cs42l42_sdw_bus_config() if the {swire_clk, sample_rate} pair isn't valid.
You lost me here. Are you saying the soundwire bus clock is only known in the prepare stage?
+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;
+ return 0;
Humm, this is interesting, you are not using the sdw_stream_data that all other codecs use, but in hindsight I have no idea why we allocate something to only store a pointer.
Indeed. I can see no reason to wrap this pointer in another struct when we can store the pointer direct so I dropped the wrapper struct.
I'll see if we can simplify the other codec drivers.
+static int cs42l42_sdw_update_status(struct sdw_slave *peripheral, + enum sdw_slave_status status);s +{ + struct cs42l42_private *cs42l42 = dev_get_drvdata(&peripheral->dev);
+ switch (status) { + case SDW_SLAVE_ATTACHED: + dev_dbg(cs42l42->dev, "ATTACHED\n"); + if (!cs42l42->init_done) + cs42l42_sdw_init(peripheral);
unclear to me what happens is the bus suspends, how would you redo the init?
We don't need to re-run the init(). A regcache_sync() will restore settings.
ah, interesting. Other codec drivers play with specific registers that aren't in regmap. There's also headset calibration that's done differently in the first init or later.