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

[Philipp Zabel]

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@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Jinmei Wei <weijinmei@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Troy Mitchell <troy.mitchell@xxxxxxxxxxxxxxxxxx>
> ---
>  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.

> +	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.

regards
Philipp