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

Hi Mark:
	Thanks for the suggestion.
1.  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.
	Agreed that holding a runtime PM reference for the lifetime of jack
detection is clearer than repeatedly calling pm_runtime_mark_last_busy()
from the poll scheduler.
I will switch to that approach in v3: take a PM reference when jack
detection is enabled via set_jack(), and drop it when jack detection is
disabled / on remove, instead of refreshing autosuspend from
sn624x_jack_schedule_poll().

2.  There's a bunch of error paths that could leave us with remembering the
jack is set.
	Fixed in v3: take the runtime PM reference before assigning hs_jack,
so a failed get cannot leave jack detection marked as enabled.
3.  These errors...
	Thanks. Those Function Status / defaults failures were being logged
at debug and then ignored while still completing io_init. They are now
returned from io_init so the device is not marked initialized, and
update_status can retry on the next attach.
4. ...all end up reporting success after skipping some init?  That doesn't
seem right.
	Agreed - that was wrong. Those failures no longer skip ahead to a
successful io_init; they are returned so hw_init is not set and attach can
retry.
5. Are all the fields in these initialized?
	Good catch - stream_config.type was left uninitialized. Both configs
are now zero-initialized (= {}), then filled via snd_sdw_params_to_config(),
with only port_config.num set afterwards (as the helper documents).
6. That seems to require symmetric rates?
	Yes - writing both CS SampleFreqIndex controls on every jack
hw_params effectively forced symmetric rates. Only the direction being
configured is programmed now (OUT for playback, CAP for capture).
7. I'm not seeing anything that marks the cache as dirty in the event of a
power loss or reset (eg, over suspend).
	Good point - without marking the cache dirty after a power
loss/reset, regcache_sync() would skip restoring clean entries. We now mark
the cache dirty on suspend (clock-stop / power loss) and again on re-io_init
after a bus re-attach.
Best regards
Qianghua Wang
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
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).