linux-kernel - Re: [PATCH 2/2] ASoC: spacemit: i2s: add support for K1 SoC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <7352191F0DCAFF7E+aJ3c3tGKdcM3G-Uc@troy-wujie14pro-arch>
Date: Thu, 14 Aug 2025 20:55:58 +0800
From: Troy Mitchell <troy.mitchell@...ux.spacemit.com>
To: Philipp Zabel <p.zabel@...gutronix.de>,
	Troy Mitchell <troy.mitchell@...ux.spacemit.com>,
	Liam Girdwood <lgirdwood@...il.com>,
	Mark Brown <broonie@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Yixun Lan <dlan@...too.org>,
	Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>
Cc: linux-sound@...r.kernel.org, devicetree@...r.kernel.org,
	linux-riscv@...ts.infradead.org, spacemit@...ts.linux.dev,
	linux-kernel@...r.kernel.org,
	Jinmei Wei <weijinmei@...ux.spacemit.com>
Subject: Re: [PATCH 2/2] ASoC: spacemit: i2s: add support for K1 SoC

On Thu, Aug 14, 2025 at 12:36:45PM +0200, Philipp Zabel wrote:
> On Do, 2025-08-14 at 16:54 +0800, Troy Mitchell wrote:
> > Add ASoC platform driver for the SpacemiT K1 SoC full-duplex I2S
> > controller.
> > 
> > Co-developer: Jinmei Wei <weijinmei@...ux.spacemit.com>
> > Signed-off-by: Jinmei Wei <weijinmei@...ux.spacemit.com>
> > Signed-off-by: Troy Mitchell <troy.mitchell@...ux.spacemit.com>
> > ---
> >  sound/soc/Kconfig           |   1 +
> >  sound/soc/Makefile          |   1 +
> >  sound/soc/spacemit/Kconfig  |  14 ++
> >  sound/soc/spacemit/Makefile |   5 +
> >  sound/soc/spacemit/k1_i2s.c | 444 ++++++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 465 insertions(+)
> > 
> [...]
> > diff --git a/sound/soc/spacemit/k1_i2s.c b/sound/soc/spacemit/k1_i2s.c
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..9e41525afbf3f08ab9a26b562772861d86f39cd7
> > --- /dev/null
> > +++ b/sound/soc/spacemit/k1_i2s.c
> > @@ -0,0 +1,444 @@
> [...]
> 
> > +static int spacemit_i2s_probe(struct platform_device *pdev)
> > +{
> > +	struct snd_soc_dai_driver *dai;
> > +	struct spacemit_i2s_dev *i2s;
> > +	struct resource *res;
> > +	struct clk *clk;
> > +	int ret;
> > +
> > +	i2s = devm_kzalloc(&pdev->dev, sizeof(*i2s), GFP_KERNEL);
> > +	if (!i2s)
> > +		return -ENOMEM;
> > +
> > +	i2s->dev = &pdev->dev;
> > +
> > +	i2s->sysclk = devm_clk_get_enabled(i2s->dev, "sysclk");
> > +	if (IS_ERR(i2s->sysclk))
> > +		return dev_err_probe(i2s->dev, PTR_ERR(i2s->sysclk),
> > +				     "failed to enable sysbase clock\n");
> > +
> > +	i2s->bclk = devm_clk_get_enabled(i2s->dev, "bclk");
> > +	if (IS_ERR(i2s->bclk))
> > +		return dev_err_probe(i2s->dev, PTR_ERR(i2s->bclk), "failed to enable bit clock\n");
> > +
> > +	clk = devm_clk_get_enabled(i2s->dev, "sspa_bus");
> > +	if (IS_ERR(clk))
> > +		return dev_err_probe(i2s->dev, PTR_ERR(clk), "failed to enable sspa_bus clock\n");
> > +
> > +	i2s->sspa_clk = devm_clk_get_enabled(i2s->dev, "sspa");
> > +	if (IS_ERR(clk))
> > +		return dev_err_probe(i2s->dev, PTR_ERR(clk), "failed to enable sspa clock\n");
> > +
> > +	i2s->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
> > +	if (IS_ERR(i2s->base))
> > +		return dev_err_probe(i2s->dev, PTR_ERR(i2s->base), "failed to map registers\n");
> > +
> > +	i2s->reset =  devm_reset_control_get(&pdev->dev, NULL);
> 
> Please use devm_reset_control_get_exclusive() directly.
Tnx. I will use in the next version.

> 
> > +	if (IS_ERR(i2s->reset))
> > +		return dev_err_probe(i2s->dev, PTR_ERR(i2s->reset),
> > +				     "failed to get reset control");
> > +
> > +	dev_set_drvdata(i2s->dev, i2s);
> > +
> > +	spacemit_i2s_init_dai(i2s, &dai, res->start + SSDATR);
> > +
> > +	ret = devm_snd_soc_register_component(i2s->dev,
> > +					      &spacemit_i2s_component,
> > +					      dai, 1);
> > +	if (ret)
> > +		return dev_err_probe(i2s->dev, ret, "failed to register component");
> > +
> > +	return devm_snd_dmaengine_pcm_register(&pdev->dev, &spacemit_dmaengine_pcm_config, 0);
> > +}
> > +
> > +static void spacemit_i2s_remove(struct platform_device *pdev)
> > +{
> > +	struct spacemit_i2s_dev *i2s = dev_get_drvdata(&pdev->dev);
> > +
> > +	reset_control_assert(i2s->reset);
> 
> I'd move the reset_control_assert() be moved to snd_soc_dai_ops::remove
> for symmetry.
>
It makes sense.

deassert in ops::probe and assert in ops::remove.

                - Troy
> 
> regards
> Philipp
>