Re: [PATCH v2 2/2] ASoC: codecs: Add driver for Iron Device MA5311

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
- [High] ASoC: ma5311: ma5311_irq_thread() does not clear fault or return IRQ_NONE
- [Medium] ASoC: ma5311: unchecked regmap calls in ADC PGA control get/put callbacks
- [Medium] ASoC: ma5311: bitfield overflow and uninhibited TDM regs in capture_mode_put()
- [Medium] ASoC: ma5311: missing I2S case in ma5311_playback_mode_put()
- [Medium] ASoC: ma5311: broken RSTN Switch control and state loss in ma5311_reset()
- [Medium] ASoC: ma5311: integer truncation before bounds check in ADC put callbacks
- [Medium] ASoC: ma5311: broken speaker mode update and PMU order in ma5311_aif_in_event()
- [Medium] ASoC: ma5311: missing register writes for TDM slots, sysclk, and ADC rate
- [Medium] ASoC: ma5311: broken I2S mono BCLK calculation and 96kHz 8ch PLL lookup
- [Low] ASoC: ma5311: invalid reg_defaults with REGCACHE_NONE and unsorted defaults
--

commit b66c1b240eead610149979ef5c3fc3dd398f1c55
Author: Hyunuk Son <hyunuk.son@irondevice.com>

ASoC: codecs: Add driver for Iron Device MA5311

This commit adds an ASoC codec driver for the Iron Device MA5311 audio codec,
supporting playback and capture over I2S and TDM interfaces with regmap-based
register access and ALSA mixer controls.

> diff --git a/sound/soc/codecs/ma5311.c b/sound/soc/codecs/ma5311.c
> new file mode 100644
> index 0000000000000..31b79f1154cf2
> --- /dev/null
> +++ b/sound/soc/codecs/ma5311.c

[ ... ]

> +static int ma5311_adc_volume_control_put(struct snd_kcontrol *kcontrol,
> +					 struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> +	struct ma5311_priv *ma5311 = snd_soc_component_get_drvdata(component);
> +	const struct soc_mixer_control *mc =
> +		(const struct soc_mixer_control *)kcontrol->private_value;
> +	unsigned int val = ucontrol->value.integer.value[0];
> +	unsigned int *cache;
> +	int ret;
> +
> +	if (val > MA5311_ADC_VOL_CTRL_MASK)
> +		return -EINVAL;

[Severity: Medium]
In both ma5311_adc_volume_control_put() and ma5311_adc_pga_control_put(),
ucontrol->value.integer.value[0] (which is a signed long) is assigned to an
unsigned int before checking whether it is within the valid range.

On 64-bit platforms, if userspace passes a negative or 64-bit out-of-range
value whose lower 32 bits fall within 0..MA5311_ADC_VOL_CTRL_MASK (for
example, LONG_MIN, which truncates to 0), won't this bypass the bounds check
and write the truncated value to the hardware register instead of returning
-EINVAL?

[ ... ]

> +static int ma5311_adc_pga_control_get(struct snd_kcontrol *kcontrol,
> +				      struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> +	struct ma5311_priv *ma5311 = snd_soc_component_get_drvdata(component);
> +	const struct soc_mixer_control *mc =
> +		(const struct soc_mixer_control *)kcontrol->private_value;
> +	unsigned int *cache = ma5311_adc_pga_control_cache(ma5311, mc->reg);
> +	unsigned int val;
> +
> +	if (!cache)
> +		return -EINVAL;
> +
> +	regmap_read(ma5311->adc_regmap, mc->reg, &val);
> +
> +	ucontrol->value.integer.value[0] = val & MA5311_ADC_PGA_CTRL_MASK;
> +	*cache = ucontrol->value.integer.value[0];
> +
> +	return 0;
> +}
> +
> +static int ma5311_adc_pga_control_put(struct snd_kcontrol *kcontrol,
> +				      struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> +	struct ma5311_priv *ma5311 = snd_soc_component_get_drvdata(component);
> +	const struct soc_mixer_control *mc =
> +		(const struct soc_mixer_control *)kcontrol->private_value;
> +	unsigned int *cache;
> +	unsigned int val = ucontrol->value.integer.value[0];
> +
> +	if (val > MA5311_ADC_PGA_CTRL_MASK)
> +		return -EINVAL;
> +
> +	cache = ma5311_adc_pga_control_cache(ma5311, mc->reg);
> +	if (!cache) {
> +		dev_err(ma5311->dev, "%s: Invalid Control ID - %s\n", __func__,
> +			kcontrol->id.name);
> +		return -EINVAL;
> +	}
> +
> +	if (*cache == val)
> +		return 0;
> +
> +	regmap_update_bits(ma5311->adc_regmap, mc->reg,
> +			   MA5311_ADC_PGA_CTRL_MASK, val);
> +
> +	*cache = val;
> +
> +	return 1;
> +}

[Severity: Medium]
Should the return values of regmap_read() in ma5311_adc_pga_control_get() and
regmap_update_bits() in ma5311_adc_pga_control_put() be checked like they are
in ma5311_adc_volume_control_get() and ma5311_adc_volume_control_put()?

Because ma5311->adc_regmap uses REGCACHE_NONE, an I2C read failure in
ma5311_adc_pga_control_get() leaves val uninitialized on the stack, copying
uninitialized stack bits to ucontrol->value.integer.value[0] and *cache.

Similarly, if regmap_update_bits() fails in ma5311_adc_pga_control_put(),
updating *cache = val causes the cached state to diverge from the hardware
register and prevents subsequent retries of the same value due to the
*cache == val check.

[ ... ]

> +static int ma5311_capture_mode_put(struct snd_kcontrol *kcontrol,
> +				   struct snd_ctl_elem_value *ucontrol)
> +{

[ ... ]

> +	switch (val) {
> +	case I2S_ADC1:
> +		regmap_write(ma5311->ctrl_regmap,
> +			     MA5311_DAC_39_DATA_PATH,
> +			     MA5311_DATA_PATH_I2S_ADC1);
> +		break;
> +
> +	case I2S_ADC2:
> +		regmap_write(ma5311->ctrl_regmap,
> +			     MA5311_DAC_39_DATA_PATH,
> +			     MA5311_DATA_PATH_I2S_ADC2);
> +		break;
> +
> +	case I2S_ADC3:
> +		regmap_write(ma5311->ctrl_regmap,
> +			     MA5311_DAC_39_DATA_PATH,
> +			     MA5311_DATA_PATH_I2S_ADC3);
> +		break;
> +
> +	case I2S_ADC4:
> +		regmap_write(ma5311->ctrl_regmap,
> +			     MA5311_DAC_39_DATA_PATH,
> +			     MA5311_DATA_PATH_I2S_ADC4);
> +		break;
> +
> +	case TDM_8CH:
> +		regmap_update_bits(ma5311->ctrl_regmap,
> +				   MA5311_DAC_38_TDM2,
> +				   MA5311_ADC_TDM2_SETTING_MASK,
> +				   MA5311_DAC_38_CAPTURE_8CH_CFG);
> +		regmap_write(ma5311->ctrl_regmap,
> +			     MA5311_DAC_39_DATA_PATH, MA5311_DATA_PATH_TDM_8CH);
> +		regmap_write(ma5311->ctrl_regmap,
> +			     MA5311_DAC_3A_TDM_SLOT_ENABLE,
> +			     MA5311_TDM_SLOT_ENABLE_ALL);
> +		break;
> +	}
> +
> +	regmap_update_bits(ma5311->ctrl_regmap, MA5311_DAC_39_DATA_PATH,
> +			   MA5311_DAC_SDO_DATA_PATH_MASK, val << 2);
> +
> +	regmap_update_bits(ma5311->ctrl_regmap, MA5311_DAC_91_ADC_DAC_MIX,
> +			   MA5311_DAC_ADC_DAC_MIX_CH_MASK, val);

[Severity: Medium]
When val is TDM_8CH (4), doesn't passing val << 2 (0x10) with
MA5311_DAC_SDO_DATA_PATH_MASK (GENMASK(3, 2)) and val (0x04) with
MA5311_DAC_ADC_DAC_MIX_CH_MASK (GENMASK(1, 0)) overflow both 2-bit masks and
clear those fields to 0?

Also, in the TDM_8CH case, MA5311_DAC_38_CAPTURE_8CH_CFG (0xc1) has bit 7 set,
which is outside MA5311_ADC_TDM2_SETTING_MASK (GENMASK(6, 0)). And if the
control is switched from TDM_8CH back to one of I2S_ADC1..I2S_ADC4, shouldn't
MA5311_DAC_38_TDM2 and MA5311_DAC_3A_TDM_SLOT_ENABLE be restored as well?

[ ... ]

> +static int ma5311_playback_mode_put(struct snd_kcontrol *kcontrol,
> +				    struct snd_ctl_elem_value *ucontrol)
> +{

[ ... ]

> +	switch (val) {
> +	case TDM01:
> +		regmap_update_bits(ma5311->ctrl_regmap,
> +				   MA5311_DAC_37_TDM1,
> +				   MA5311_TDM_SLOT0_RX_POS_MASK,
> +				   MA5311_TDM_SLOT0_RX_POS_0);
> +		regmap_update_bits(ma5311->ctrl_regmap,
> +				   MA5311_DAC_37_TDM1,
> +				   MA5311_TDM_SLOT1_RX_POS_MASK,
> +				   MA5311_TDM_SLOT1_RX_POS_1);
> +		break;

[ ... ]

> +	case TDM67:
> +		regmap_update_bits(ma5311->ctrl_regmap,
> +				   MA5311_DAC_37_TDM1,
> +				   MA5311_TDM_SLOT0_RX_POS_MASK,
> +				   MA5311_TDM_SLOT0_RX_POS_6);
> +		regmap_update_bits(ma5311->ctrl_regmap,
> +				   MA5311_DAC_37_TDM1,
> +				   MA5311_TDM_SLOT1_RX_POS_MASK,
> +				   MA5311_TDM_SLOT1_RX_POS_7);
> +		break;
> +	}
> +
> +	ma5311->playback_mode = val;
> +
> +	return 1;
> +}

[Severity: Medium]
What happens in ma5311_playback_mode_put() when switching from one of the TDM
modes back to I2S (val == I2S)? Since switch (val) has no case I2S,
ma5311->playback_mode is updated to I2S, but MA5311_DAC_37_TDM1 is left with
the previous TDM slot configuration.

> +
> +static int ma5311_reset_control_get(struct snd_kcontrol *kcontrol,
> +				    struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> +	struct ma5311_priv *ma5311 = snd_soc_component_get_drvdata(component);
> +
> +	if (!ma5311->reset_gpio)
> +		return -ENODEV;
> +
> +	/* Do not persist a reset request when ALSA saves the mixer state. */
> +	ucontrol->value.enumerated.item[0] = 0;
> +
> +	return 0;
> +}
> +
> +static int ma5311_reset_control_put(struct snd_kcontrol *kcontrol,
> +				    struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> +	struct ma5311_priv *ma5311 = snd_soc_component_get_drvdata(component);
> +	struct snd_soc_dapm_context *dapm =
> +		snd_soc_component_to_dapm(component);
> +	unsigned int val = ucontrol->value.enumerated.item[0];
> +	int ret;
> +
> +	if (!val)
> +		return 0;
> +
> +	if (!ma5311->reset_gpio)
> +		return -ENODEV;

[ ... ]

> +	ret = ma5311_reset(component);
> +
> +unlock:
> +	snd_soc_dapm_mutex_unlock(dapm);
> +	snd_soc_dpcm_mutex_unlock(component->card);
> +
> +	if (ret)
> +		return ret;
> +
> +	return 1;
> +}

[Severity: Medium]
There are a few issues around the "RSTN Switch" control and ma5311_reset():

1. Since reset_gpio is optional (acquired via devm_gpiod_get_optional()),
returning -ENODEV in ma5311_reset_control_get() when reset_gpio is NULL causes
userspace tools that read all mixer controls (such as alsactl, amixer, or
kselftest mixer-test) to fail on boards without a reset GPIO. Also, in ALSA
control naming conventions, the " Switch" suffix is reserved for boolean
controls, whereas this is an enumerated control.

2. ma5311_reset_control_put() does not check whether val >=
ARRAY_SIZE(ma5311_reset_text), so any out-of-range enum value triggers a
hardware reset instead of returning -EINVAL.

3. When ma5311_reset() runs after a runtime reset, it overwrites all registers
with ma5311_ctrl_reg_def, including MA5311_DAC_01_INPUT1_CTRL1 (wiping the DAI
clock provider/consumer role and clock polarity configured by
ma5311_dai_set_fmt()) and MA5311_DAC_37_TDM1, without restoring the DAI format
or resetting ma5311->playback_mode, ma5311->dapm_aif_in, and
ma5311->amp_mode. Doesn't this leave the driver state out of sync with the
hardware after a reset?

[ ... ]

> +static int ma5311_dapm_aif_in_put(struct snd_kcontrol *kcontrol,
> +				  struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_dapm_context *dapm =
> +		snd_soc_dapm_kcontrol_to_dapm(kcontrol);
> +	struct snd_soc_component *component = snd_soc_dapm_to_component(dapm);
> +	struct ma5311_priv *ma5311 = snd_soc_component_get_drvdata(component);
> +	unsigned int val = ucontrol->value.enumerated.item[0];
> +	unsigned int old_val = ma5311->dapm_aif_in;
> +	int ret;
> +
> +	if (val >= ARRAY_SIZE(ma5311_aif_in_source_text)) {
> +		dev_err(ma5311->dev, "%s: Out of range\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	if (ma5311->dapm_aif_in == val)
> +		return 0;
> +
> +	ma5311->dapm_aif_in = val;
> +
> +	ret = snd_soc_dapm_put_enum_double(kcontrol, ucontrol);
> +	if (ret < 0) {
> +		ma5311->dapm_aif_in = old_val;
> +		return ret;
> +	}
> +
> +	return 1;
> +}

[ ... ]

> +static int ma5311_aif_in_event(struct snd_soc_dapm_widget *w,
> +			       struct snd_kcontrol *kcontrol, int event)
> +{
> +	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
> +	struct ma5311_priv *ma5311 = snd_soc_component_get_drvdata(component);
> +	unsigned int mux = ma5311->dapm_aif_in;
> +
> +	switch (event) {
> +	case SND_SOC_DAPM_PRE_PMU:
> +		switch (mux) {
> +		case MA5311_MONO_BOTH_MODE:
> +			regmap_update_bits(ma5311->ctrl_regmap,
> +					   MA5311_DAC_10_SYSTEM_CTRL3,
> +					   MA5311_SPK_MODE_MASK,
> +					   MA5311_SPK_MONO_BOTH);
> +			break;
> +		case MA5311_STEREO_MODE:
> +			regmap_update_bits(ma5311->ctrl_regmap,
> +					   MA5311_DAC_10_SYSTEM_CTRL3,
> +					   MA5311_SPK_MODE_MASK,
> +					   MA5311_SPK_STEREO);
> +			break;
> +		default:
> +			dev_err(ma5311->dev, "%s: Invalid value (%u)\n",
> +				__func__, mux);
> +			return -EINVAL;
> +		}
> +
> +		ma5311->amp_mode = mux;
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ma5311_power_event(struct snd_soc_dapm_widget *w,
> +			      struct snd_kcontrol *kcontrol, int event)
> +{
> +	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
> +
> +	switch (event) {
> +	case SND_SOC_DAPM_POST_PMU:
> +		return ma5311_startup(component);
> +	case SND_SOC_DAPM_PRE_PMD:
> +		return ma5311_shutdown(component);
> +	}
> +
> +	return 0;
> +}

[Severity: Medium]
Both inputs of the "AIF IN Source" mux ("Mono Both" and "Stereo") come from
the same "AIF IN" widget. If a user changes "AIF IN Source" via
ma5311_dapm_aif_in_put() while playback is already active, the mux widget
remains powered and SND_SOC_DAPM_PRE_PMU will not fire, so
MA5311_DAC_10_SYSTEM_CTRL3 and ma5311->amp_mode are never updated.

In addition, when playback starts from power-down, DAPM runs all PRE_PMU
callbacks before POST_PMU callbacks. Doesn't that mean ma5311_aif_in_event()
enables the speaker mode in MA5311_DAC_10_SYSTEM_CTRL3 at PRE_PMU before
ma5311_power_event() calls ma5311_startup() at POST_PMU to turn on the PLL and
system power?

[ ... ]

> +static const struct regmap_config ma5311_ctrl_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +
> +	.max_register = MA5311_DAC_FF_VERSION,
> +	.readable_reg = ma5311_ctrl_readable_register,
> +	.writeable_reg = ma5311_ctrl_writeable_register,
> +	.volatile_reg = ma5311_ctrl_volatile_register,
> +
> +	.cache_type = REGCACHE_NONE,
> +	.reg_defaults = ma5311_ctrl_reg_def,
> +	.num_reg_defaults = ARRAY_SIZE(ma5311_ctrl_reg_def),
> +};
> +
> +static const struct regmap_config ma5311_adc_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +
> +	.max_register = MA5311_ADC_CH4_3F_OUTPUT_FORMAT,
> +	.readable_reg = ma5311_adc_readable_register,
> +	.writeable_reg = ma5311_adc_writeable_register,
> +	.volatile_reg = ma5311_adc_volatile_register,
> +
> +	.cache_type = REGCACHE_NONE,
> +	.reg_defaults = ma5311_adc_reg_def,
> +	.num_reg_defaults = ARRAY_SIZE(ma5311_adc_reg_def),
> +};

[Severity: Low]
Both ma5311_ctrl_regmap_config and ma5311_adc_regmap_config set .cache_type =
REGCACHE_NONE while also providing .reg_defaults and .num_reg_defaults, which
causes regcache_init() to emit a "No cache used with register defaults set!"
warning on every probe.

Also, in ma5311_ctrl_reg_def, register 0x91 is placed before 0x8A..0x8F:

	{ 0x89, 0x00 },
	{ 0x91, 0x00 },
	{ 0x8A, 0x00 },

Should .reg_defaults be dropped when using REGCACHE_NONE (or a cache type
enabled with 0x91 sorted in ascending order)?

[ ... ]

> +static int ma5311_dai_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 ma5311_priv *ma5311 = snd_soc_component_get_drvdata(component);
> +	unsigned int bclk = 0;
> +	int ret;
> +
> +	switch (params_rate(params)) {
> +	case 48000:
> +	case 96000:
> +		break;
> +
> +	default:
> +		dev_err(component->dev, "%s: unsupported sample rate: %u\n",
> +			__func__, params_rate(params));
> +
> +		return -EINVAL;
> +	}
> +
> +	if (params_channels(params) > 8) {
> +		dev_err(component->dev, "%s: unsupported channel count: %u\n",
> +			__func__, params_channels(params));
> +
> +		return -EINVAL;
> +	}
> +
> +	if (ma5311->format == SND_SOC_DAIFMT_DSP_A) {
> +		if (!ma5311->frame_size) {
> +			dev_err(component->dev,
> +				"TDM frame size is not configured\n");
> +			return -EINVAL;
> +		}
> +
> +		bclk = params_rate(params) * ma5311->frame_size;
> +		dev_dbg(component->dev, "Estimated TDM BCLK: %u Hz\n", bclk);
> +	} else if (ma5311->format == SND_SOC_DAIFMT_I2S) {
> +		bclk = params_rate(params) * params_physical_width(params) *
> +		       params_channels(params);
> +		dev_dbg(component->dev, "Estimated I2S BCLK: %u Hz\n", bclk);
> +	}

[Severity: Medium]
ma5311_dai advertises channels_min = 1, channels_max = 8, rates 48000 and
96000, and formats S16_LE and S32_LE.

In I2S mode, the bus frame always has 2 slots (matching the 32fs and 64fs SCK
rates configured below), so when params_channels(params) == 1, doesn't
multiplying by params_channels(params) compute half the actual BCLK frequency
(for example, 768 kHz at 48 kHz 16-bit, which fails in ma5311_setup_pll(), or
1.536 MHz instead of 3.072 MHz at 48 kHz 32-bit)?

Also, for a 96 kHz, 8-channel/8-slot, 32-bit stream, bclk is 24576000 Hz,
which has no entry in ma5311_pll_matches (max 12288000 Hz) and will always
fail in ma5311_setup_pll() with -EINVAL.

[ ... ]

> +static int ma5311_dai_set_sysclk(struct snd_soc_dai *dai, int clk_id,
> +				 unsigned int freq, int dir)
> +{
> +	struct snd_soc_component *component = dai->component;
> +	struct ma5311_priv *ma5311 = snd_soc_component_get_drvdata(component);
> +
> +	switch (clk_id) {
> +	case MA5311_EXTERNAL_CLOCK_19_2:
> +	case MA5311_EXTERNAL_CLOCK_24_576:
> +	case MA5311_PLL_CLKIN_MCLK:
> +	case MA5311_PLL_CLKIN_BCLK:
> +		if (clk_id == MA5311_PLL_CLKIN_BCLK)
> +			dev_dbg(component->dev, "PLL input: BCLK\n");
> +		break;
> +	default:
> +		dev_err(component->dev, "%s: Invalid clk id: %d\n", __func__,
> +			clk_id);
> +		return -EINVAL;
> +	}
> +
> +	ma5311->sys_clk_id = clk_id;
> +
> +	return 0;
> +}
> +
> +static int ma5311_dai_set_tdm_slot(struct snd_soc_dai *dai,
> +				   unsigned int tx_mask, unsigned int rx_mask,
> +				   int slots, int slot_width)
> +{
> +	struct snd_soc_component *component = dai->component;
> +	struct ma5311_priv *ma5311 = snd_soc_component_get_drvdata(component);
> +
> +	/* Reject invalid frame geometry before changing the hardware. */
> +	switch (slot_width) {
> +	case 16:
> +	case 32:
> +		break;
> +
> +	default:
> +		dev_err(component->dev, "%s: unsupported TDM slot width: %d\n",
> +			__func__, slot_width);
> +		return -EINVAL;
> +	}
> +
> +	switch (slots) {
> +	case 4:
> +	case 8:
> +		break;
> +
> +	default:
> +		dev_err(component->dev, "%s: unsupported TDM slot count: %d\n",
> +			__func__, slots);
> +		return -EINVAL;
> +	}
> +
> +	ma5311->frame_size = slot_width * slots;
> +	dev_dbg(component->dev, "TDM slots=%d width=%d\n", slots, slot_width);
> +
> +	return 0;
> +}

[Severity: Medium]
Several hardware configuration fields defined in ma5311.h are never written to
the device registers:

1. ma5311_dai_set_tdm_slot() validates slots (4 or 8), but never updates
MA5311_TDM_N_SLOT_MASK (MA5311_TDM_N_SLOT_4 vs MA5311_TDM_N_SLOT_8) in
MA5311_DAC_38_TDM2.

2. ma5311_dai_set_sysclk() accepts MA5311_EXTERNAL_CLOCK_19_2,
MA5311_EXTERNAL_CLOCK_24_576, and MA5311_PLL_CLKIN_MCLK, but ignores freq and
never writes any clock selection registers, while ma5311_setup_pll() only
configures the PLL when sys_clk_id == MA5311_PLL_CLKIN_BCLK.

3. ma5311_dai_hw_params() accepts both 48000 and 96000 Hz, but never updates
MA5311_ADC_SAMPLE_RATE_MASK (MA5311_ADC_SAMPLE_RATE_48K vs
MA5311_ADC_SAMPLE_RATE_96K) in the ADC general setting registers, leaving all
four ADCs hardcoded to the 96 kHz default (0xA5).

Should these register fields be programmed when configuring the DAI?

[ ... ]

> +static irqreturn_t ma5311_irq_thread(int irq, void *data)
> +{
> +	struct ma5311_priv *ma5311 = data;
> +	unsigned int dac_status;
> +	int ret = 0;
> +
> +	ret |= regmap_read(ma5311->ctrl_regmap,
> +			   MA5311_DAC_EA_DAC_STATUS, &dac_status);
> +	if (ret) {
> +		dev_err_ratelimited(ma5311->dev,
> +				    "I2C Read Failed During Interrupt Handling\n");
> +		return IRQ_NONE;
> +	}
> +
> +	if (dac_status & MA5311_DAC_EA_FAULT_MASK)
> +		dev_err_ratelimited(ma5311->dev, "FAULT Asserted\n");
> +
> +	return IRQ_HANDLED;
> +}

[Severity: High]
ma5311_irq_thread() is registered as a threaded oneshot handler for the
active-low level-triggered INTN interrupt, but it only reads
MA5311_DAC_EA_DAC_STATUS, logs a message if MA5311_DAC_EA_FAULT_MASK is set,
and unconditionally returns IRQ_HANDLED without clearing or recovering from
the fault condition.

When INTN is asserted low, won't unmasking the level-triggered interrupt line
after ma5311_irq_thread() returns IRQ_HANDLED cause an immediate re-interrupt
and an unbounded interrupt and I2C read storm (and bypass spurious IRQ
detection when no fault bit is set)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027093232.859755-1-hyunuk.son@irondevice.com?part=2