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

[Charles Keepax <ckeepax@opensource.cirrus.com>]

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.
> 
> Signed-off-by: Qianghua Wang <qianghua.wang@senarytech.com>
> ---
> +static int sn624x_sdca_mbq_size(struct device *dev, unsigned int reg)
> +{
> +	if (!SDW_SDCA_VALID_CTL(reg))
> +		return 1;
> +
> +	/*
> +	 * FU_VOLUME and GE35 DETECTED_MODE share control selector 0x02.
> +	 * Only list known FU volume addresses as 16-bit MBQ — matching
> +	 * DETECTED_MODE by csel alone makes GE35 use SDW_SDCA_MBQ_CTL and
> +	 * fails with -ENODATA (-61), which breaks jack plug/unplug.
> +	 */
> +	switch (reg) {
> +	case SN624X_REG_VOL:
> +	case SN624X_REG_CHR_VOL:
> +	case SN624X_REG_JACK_OUT_VOL:
> +	case SN624X_REG_JACK_OUT_CHR_VOL:
> +	case SN624X_REG_JACK_CAP_VOL:
> +	case SN624X_REG_JACK_CAP_CHR_VOL:
> +	case SN624X_REG_DMIC_CAP_VOL:
> +	case SN624X_REG_DMIC_CAP_CHR_VOL:
> +		return 2;
> +	default:
> +		return 1;
> +	}
> +}
> +
> +static bool sn624x_sdca_readable_register(struct device *dev, unsigned int reg)
> +{
> +	return sn624x_sdca_mbq_size(dev, reg) > 0;
> +}

This is just the same as return true;, and as 00268f9452d2
("regmap: sdw-mbq: don't call an unset readable_reg callback")
is now merged you can probably just drop the readable callback
completely, assuming you are happy with everything being marked
as readable.

> +static const struct reg_default sn624x_sdca_reg_defaults[] = {
> +	{}
> +};

Do you really want a completely empty defaults struct?  Wouldn't
it make more sense to just not have one.

> +static int sn624x_parse_sdca_functions(struct sn624x_sdca_priv *sn624x)
> +{
> +	struct sdw_slave *slave = sn624x->slave;
> +	struct device *dev = &slave->dev;
> +	int i, ret;
> +
> +	if (!slave->sdca_data.num_functions) {
> +		dev_dbg(dev, "sn624x: no SDCA function descriptors from DisCo\n");
> +		return 0;
> +	}
> +
> +	for (i = 0; i < slave->sdca_data.num_functions; i++) {
> +		struct sdca_function_desc *desc = &slave->sdca_data.function[i];
> +		struct sdca_function_data *fn;
> +		bool is_jack = false, is_mic = false, is_amp = false;
> +
> +		switch (desc->type) {
> +		case SDCA_FUNCTION_TYPE_UAJ:
> +		case SDCA_FUNCTION_TYPE_SIMPLE_JACK:
> +		case SDCA_FUNCTION_TYPE_RJ:
> +			is_jack = true;
> +			break;
> +		case SDCA_FUNCTION_TYPE_SMART_MIC:
> +		case SDCA_FUNCTION_TYPE_SIMPLE_MIC:
> +			is_mic = true;
> +			break;
> +		case SDCA_FUNCTION_TYPE_SMART_AMP:
> +		case SDCA_FUNCTION_TYPE_SIMPLE_AMP:
> +		case SDCA_FUNCTION_TYPE_SPEAKER_MIC:
> +			is_amp = true;
> +			break;
> +		default:
> +			break;
> +		}
> +
> +		if (desc->adr == SN624X_FUNC_NUM_JACK_CODEC)
> +			is_jack = true;
> +		else if (desc->adr == SN624X_FUNC_NUM_MIC_ARRAY)
> +			is_mic = true;
> +		else if (desc->adr == SN624X_FUNC_NUM_SPEAKER_AMP)
> +			is_amp = true;
> +
> +		if (!is_jack && !is_mic && !is_amp)
> +			continue;
> +
> +		fn = devm_kzalloc(dev, sizeof(*fn), GFP_KERNEL);
> +		if (!fn)
> +			return -ENOMEM;
> +
> +		fn->desc = desc;
> +		ret = sdca_parse_function(dev, slave, fn);
> +		if (ret) {
> +			/*
> +			 * Init table is parsed before entities. Keep a partial
> +			 * function if the ACPI init table was already loaded.
> +			 */
> +			dev_warn(dev,
> +				 "sn624x: sdca_parse_function(%s adr=%u) failed (%d)%s\n",
> +				 desc->name ? desc->name : "?", desc->adr, ret,
> +				 fn->num_init_table ?
> +				 ", keeping ACPI init table" : "");
> +			if (!fn->num_init_table)
> +				continue;

Not sure I follow this, what is happening here?

> +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;
> +	enum sdw_data_direction direction;
> +	struct sdw_stream_runtime *sdw_stream;
> +	unsigned int ch = params_channels(params);
> +	unsigned int sampling_rate;
> +	int port;
> +	int ret;
> +
> +	dev_dbg(dai->dev,
> +		 "sn624x: hw_params: entered dai=%s id=%d stream=%s\n",
> +		 dai->name, dai->id, snd_pcm_stream_str(substream));
> +
> +	sdw_stream = snd_soc_dai_get_dma_data(dai, substream);
> +	if (!sdw_stream) {
> +		dev_warn(dai->dev,
> +			 "sn624x: hw_params: no SDW stream (set_stream not run yet?)\n");
> +		return -EINVAL;
> +	}
> +	if (!sn624x->slave) {
> +		dev_warn(dai->dev, "sn624x: hw_params: slave NULL\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = pm_runtime_resume(component->dev);
> +	if (ret < 0 && ret != -EACCES) {
> +		dev_err(dai->dev,
> +			"sn624x: hw_params: pm_runtime_resume failed (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +		direction = SDW_DATA_DIR_RX;
> +		if (dai->id == SN624X_DAI_JACK)
> +			port = SN624X_PORT_JACK_PLAYBACK;
> +		else if (dai->id == SN624X_DAI_SPEAKER)
> +			port = SN624X_PORT_SPEAKER_PLAYBACK;
> +		else
> +			return -EINVAL;
> +	} else {
> +		direction = SDW_DATA_DIR_TX;
> +		if (dai->id == SN624X_DAI_JACK) {
> +			port = SN624X_PORT_JACK_CAPTURE;
> +		} else if (dai->id == SN624X_DAI_DMIC) {
> +			port = SN624X_PORT_DMIC_CAPTURE;
> +		} else {
> +			return -EINVAL;
> +		}
> +	}
> +	stream_config.frame_rate = params_rate(params);
> +	stream_config.ch_count = ch;
> +	stream_config.bps = snd_pcm_format_width(params_format(params));
> +	stream_config.direction = direction;
> +	port_config.ch_mask = GENMASK(ch - 1, 0);

snd_sdw_params_to_config()

> +static int sn624x_sdca_regmap_resume(struct device *dev)
> +{
> +	struct sdw_slave *slave = dev_to_sdw_dev(dev);
> +	struct sn624x_sdca_priv *sn624x = dev_get_drvdata(dev);
> +	unsigned long time;
> +
> +	if (!sn624x->first_hw_init)
> +		return 0;
> +
> +	SN624X_DBG(dev, "resume: unattach_request=%u\n", slave->unattach_request);
> +	if (!slave->unattach_request)
> +		goto regmap_sync;
> +
> +	time = wait_for_completion_timeout(&slave->initialization_complete,
> +					   msecs_to_jiffies(SN624X_PROBE_TIMEOUT_MS));
> +	if (!time) {
> +		dev_err(dev, "%s: initialization timed out\n", __func__);
> +		sdw_show_ping_status(slave->bus, true);
> +		return -ETIMEDOUT;
> +	}

sdw_slave_wait_for_init()


Thanks,
Charles