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

[Joakim Zhang (张强庆)]

Hello,


> -----Original Message-----
> From: Ajay Kumar Nandam <ajay.nandam@xxxxxxxxxxxxxxxx>
> Sent: Wednesday, October 7, 2026 5:54 PM
> To: Joakim Zhang (张强庆) <joakim.zhang@xxxxxxxxxxx>; lgirdwood@xxxxxxxxx;
> broonie@xxxxxxxxxx; robh@xxxxxxxxxx; krzk+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx;
> perex@xxxxxxxx; tiwai@xxxxxxxx; p.zabel@xxxxxxxxxxxxxx
> Cc: cix-kernel-upstream <cix-kernel-upstream@xxxxxxxxxxx>; linux-
> sound@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; ajay.nandam@xxxxxxxxxxxxxxxx
> Subject: Re: [PATCH v2 4/4] ASoC: cdns: Add Cadence I2S-MC controller driver
> 
> 
> EXTERNAL EMAIL
> 
> On 9/28/2026 11:06 AM, joakim.zhang@xxxxxxxxxxx wrote:
> > From: Joakim Zhang <joakim.zhang@xxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxx>
> > ---
> 
> > +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>.

Thanks, will update in v3.

> 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, will update in v3.

Joakim