Re: [PATCH v2 1/3] ASoC: codecs: add SN624x SDCA SoundWire driver

On Thu, Aug 13, 2026 at 10:45:25AM +0800, Qianghua Wang wrote:
> Add a SoundWire SDCA driver for Senary SN624x multi-function codecs
> (jack, speaker amp, and DMIC). Program SDCA SampleFreqIndex in
> hw_params, keep SDCA jack IRQs masked with poll-based detection, and
> extend the Senary MAINTAINERS entry for the new codec files only.

> +static void sn624x_jack_schedule_poll(struct sn624x_sdca_priv *sn624x)
> +{
> +	if (!sn624x || !sn624x->hs_jack || !sn624x->slave)
> +		return;
> +
> +	mod_delayed_work(system_power_efficient_wq, &sn624x->jack_detect_work,
> +			 msecs_to_jiffies(SN624X_JACK_POLL_MS));
> +
> +	/* Keep poll alive across autosuspend (3s default) when idle */
> +	if (sn624x->component)
> +		pm_runtime_mark_last_busy(sn624x->component->dev);
> +}

This works but it feels like it'd be easier to just hold a runtime PM
reference so long as jack detection is active, the net effect would be
the same.

> +static int sn624x_sdca_set_jack_detect(struct snd_soc_component *component,
> +				       struct snd_soc_jack *hs_jack, void *data)
> +{
> +	struct sn624x_sdca_priv *sn624x = snd_soc_component_get_drvdata(component);
> +	int ret;
> +
> +	sn624x->hs_jack = hs_jack;

There's a bunch of error paths that could leave us with remembering the
jack is set.

> +static int sn624x_sdca_io_init(struct device *dev, struct sdw_slave *slave)
> +{
> +	struct sn624x_sdca_priv *sn624x = dev_get_drvdata(dev);
> +	int ret;
> +	unsigned int jack_func_status, dmic_func_status;
> +
> +	if (sn624x->hw_init) {
> +		SN624X_DBG(dev, "io_init: skip (hw_init already true)\n");
> +		return 0;
> +	}

> +io_init_next:
> +	ret = regmap_read_bypassed(sn624x->regmap,
> +					SN624X_REG_DMIC_FUN_STATUS_CTL,
> +					&dmic_func_status);
> +	if (ret < 0) {
> +		dev_dbg(dev,
> +			"sn624x: io_init: DMIC FUN STATUS read failed (%d)\n",
> +			ret);
> +		goto io_init_end;
> +	}

These errors...

> +io_init_end:
> +	pm_runtime_set_active(dev);
> +
> +	sn624x->hw_init = true;
> +	sn624x->first_hw_init = true;
> +
> +	pm_runtime_put_autosuspend(dev);
> +
> +	SN624X_DBG(dev, "io_init: complete (hw_init set), jack %s\n",
> +		   sn624x->hs_jack ? "registered" : "not registered yet");
> +	return 0;
> +}

...all end up reporting success after skipping some init?  That doesn't
seem right.

> +static int sn624x_sdca_pcm_hw_params(struct snd_pcm_substream *substream,
> +				     struct snd_pcm_hw_params *params,
> +				     struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_component *component = dai->component;
> +	struct sn624x_sdca_priv *sn624x = snd_soc_component_get_drvdata(component);
> +	struct sdw_stream_config stream_config;
> +	struct sdw_port_config port_config;

Are all the fields in these initialized?

> +	if (dai->id == SN624X_DAI_JACK) {
> +		regmap_write(sn624x->regmap, SN624X_REG_JACK_OUT_RATE_SEL,
> +			     sampling_rate);
> +		regmap_write(sn624x->regmap, SN624X_REG_JACK_CAP_RATE_SEL,
> +			     sampling_rate);

That seems to require symmetric rates?

> +regmap_sync:
> +	slave->unattach_request = 0;
> +	regcache_cache_only(sn624x->regmap, false);
> +	regcache_sync(sn624x->regmap);
> +
> +	return 0;
> +}

I'm not seeing anything that marks the cache as dirty in the event of a
power loss or reset (eg, over suspend).

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmp92B8ACgkQJNaLcl1U
h9Ct4wf9FonXGxQXjHDiyQqxPgdRgUqEnZTolRbmo1YABMKsp5247pAQ0AB0znJs
48XkG2CxPXyvob4RCo8sHELOMAwWopMB8OOaVjFsTCTc6aLV5dME5cGWnC5Fy2u6
xjvXFbnykqlT0qTjr5kpmSAVb+3pGI4Wpnd5EU/PD4GqZEewqbYJmkI1q2bz7K9F
OsawX+1WH1C9SCnELBNNKoXYNQamPVMJBAm4SNDfAG6cV3lqaG+QO35aYEkj2MQg
699J7VCmldmXEUJ5hi0OPknyr6ev2WDIvA1JkOCX0f8nzlSIMMsCQITU80r1Vg2c
s7Dgfx+J4b9LCCGE/VY5+RUOw7Gtdg==
=5+dU
-----END PGP SIGNATURE-----