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

[Joakim Zhang (张强庆)]

Hello Uwe,


> -----Original Message-----
> From: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxx>
> Sent: Tuesday, September 22, 2026 11:02 PM
> To: Joakim Zhang (张强庆) <joakim.zhang@xxxxxxxxxxx>
> Cc: lgirdwood@xxxxxxxxx; broonie@xxxxxxxxxx; robh@xxxxxxxxxx;
> krzk+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; perex@xxxxxxxx; tiwai@xxxxxxxx;
> p.zabel@xxxxxxxxxxxxxx; cix-kernel-upstream <cix-kernel-upstream@xxxxxxxxxxx>;
> linux-sound@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH 4/4] ASoC: cdns: Add Cadence I2S-MC controller driver
> 
> Hello Joakim,
> 
> On Tue, Sep 22, 2026 at 07:21:34PM +0800, joakim.zhang@xxxxxxxxxxx wrote:
> > [...]
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/module.h>
> > +#include <linux/mod_devicetable.h>
> 
> Please don't include <linux/mod_devicetable.h>. You can rely on
> <linux/platform_device.h> to provide of_device_id instead. (Or if you prefer it, use
> <linux/device-id/of.h>
> 
> Same for patch #2.
Will update.

> > +#include <linux/pinctrl/consumer.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/regmap.h>
> > +#include <linux/reset.h>
> > +#include <sound/dmaengine_pcm.h>
> > +#include <sound/pcm_params.h>
> > +#include <sound/soc.h>
> 
> But please don't rely on <sound/soc.h> pulling in <linux/platform_device.h>, so
> please include the latter explicitly.
OK.

> > [...]
> > +static const struct of_device_id cdns_i2s_mc_of_match[] = {
> > +	{ .compatible = "cix,sky1-i2s-mc", .data = &sky1_devtype_data},
> 
> Missing space before closing }.
OK.

> > +	{ /* sentinel */ },
> 
> Please no comma after the list terminator.
OK.

Thanks,
Joakim