Re: [PATCH v2 4/4] ASoC: cdns: Add Cadence I2S-MC controller driver

[Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>]

On 9/28/2026 11:06 AM, joakim.zhang@cixtech.com wrote:
> From: Joakim Zhang <joakim.zhang@cixtech.com>
> 
> Add support for the Cadence I2S-MC multi-channel controller found in
> the CIX SKY1 audio subsystem.
> 
> The controller moves up to 16 channels through a configurable set of
> its 8 data pins, each pin carrying two slots. Wired-out pins may sit
> at any pin index and are described by cix,pin-out-num, cix,pin-rx-mask
> and cix,pin-tx-mask. The driver exposes a transmit DAI and a receive
> DAI which can act as clock provider or consumer independently, and
> streams with fewer channels than the wired pins narrow the enabled
> pin mask at runtime.
> 
> The driver registers the dmaengine PCM helper, selects the audio
> reference clock parent for the 8 kHz or 11.025 kHz sample rate family
> and keeps the minimum functional-clock to bit-clock ratio required
> for safe clock domain crossing, with an extra margin cycle when the
> bit clock is sourced externally. Runtime and system suspend/resume
> restore the registers through the regcache.
> 
> Signed-off-by: Joakim Zhang <joakim.zhang@cixtech.com>
> ---

> +static void cdns_i2s_mc_rxtx_config(struct cdns_i2s_mc_priv *i2s_mc_priv, bool on,
> +				    u8 pin_mask, u32 strobe, u32 sync_rst)
> +{
> +	u32 irq_mask = 0, clk_mask = 0, i2s_mask = 0;
> +
> +	irq_mask |= FIELD_PREP(I2S_CID_CTRL_I2S_MASK, pin_mask);
> +
> +	clk_mask |= FIELD_PREP(I2S_CID_CTRL_I2S_STROBE, pin_mask) | strobe;
> +
> +	i2s_mask |= FIELD_PREP(I2S_CTRL_I2S_EN, pin_mask);
> +

This patch has the same build issue as the SC driver patch: 
cdns-i2s-mc.c uses FIELD_PREP() but does not include <linux/bitfield.h>.

ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "Failed to register dmaengine component\n");
> +
> +	pm_runtime_get_noresume(&pdev->dev);
> +	pm_runtime_set_active(&pdev->dev);
> +	ret = devm_pm_runtime_enable(&pdev->dev);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "Failed to enable runtime PM\n");
> +
> +	ret = cdns_i2s_mc_clks_enable(i2s_mc_priv);
> +	if (ret) {
> +		dev_err_probe(&pdev->dev, ret, "Failed to enable clocks\n");
> +		pm_runtime_put_noidle(&pdev->dev);
> +		return ret;
> +	}
> +
> +	cdns_i2s_mc_rst(i2s_mc_priv);
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0)
> +		return irq;
> +	i2s_mc_priv->irq = irq;
> +
> +	ret = devm_request_irq(&pdev->dev, irq, cdns_i2s_mc_isr, 0,
> +			       dev_name(&pdev->dev), i2s_mc_priv);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to request irq: %d\n", ret);
> +		pm_runtime_put_noidle(&pdev->dev);
> +		return ret;
> +	}

After cdns_i2s_mc_clks_enable() succeeds, the platform_get_irq() failure 
path returns directly without disabling the clocks or dropping the 
runtime PM usage count. The devm_request_irq() failure path also calls 
pm_runtime_put_noidle(), but that will not run the runtime suspend 
callback, so the clocks enabled  manually above remain prepared/enabled.

Thanks
Ajay Kumar Nandam

> +
> +	/*
> +	 * Let pm_runtime_put_sync() disable the clocks, and take regcache effect.
> +	 * If CONFIG_PM is not enabled, the clock will stay powered.