Re: [PATCH v2 6/8] ASoC: SOF: imx: merge imx8 and imx8m drivers

From: Frank Li
Date: Thu Feb 06 2025 - 11:27:16 EST


On Wed, Feb 05, 2025 at 03:30:20PM -0500, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <laurentiu.mihalcea@xxxxxxx>
>
> Now that the common interface for imx chip has been introduced,
> there's no longer a need to have a separate platform driver for
> imx8m. As such, merge the driver with the imx8 driver. Furthermore,
> delete the old driver as it's no longer useful.
>
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@xxxxxxx>
> ---
> sound/soc/sof/imx/Kconfig | 9 -
> sound/soc/sof/imx/Makefile | 2 -
> sound/soc/sof/imx/imx8.c | 136 ++++++++-
> sound/soc/sof/imx/imx8m.c | 567 -------------------------------------
> 4 files changed, 134 insertions(+), 580 deletions(-)
> delete mode 100644 sound/soc/sof/imx/imx8m.c
>
> diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig
> index 4751b04d5e6f..92fdf80d6e51 100644
> --- a/sound/soc/sof/imx/Kconfig
> +++ b/sound/soc/sof/imx/Kconfig
> @@ -32,15 +32,6 @@ config SND_SOC_SOF_IMX8
> Say Y if you have such a device.
> If unsure select "N".
>
> -config SND_SOC_SOF_IMX8M
> - tristate "SOF support for i.MX8M"
> - depends on IMX_DSP
> - select SND_SOC_SOF_IMX_COMMON
> - help
> - This adds support for Sound Open Firmware for NXP i.MX8M platforms.
> - Say Y if you have such a device.
> - If unsure select "N".
> -
> config SND_SOC_SOF_IMX8ULP
> tristate "SOF support for i.MX8ULP"
> depends on IMX_DSP
> diff --git a/sound/soc/sof/imx/Makefile b/sound/soc/sof/imx/Makefile
> index be0bf0736dfa..852140bb8104 100644
> --- a/sound/soc/sof/imx/Makefile
> +++ b/sound/soc/sof/imx/Makefile
> @@ -1,11 +1,9 @@
> # SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
> snd-sof-imx8-y := imx8.o
> -snd-sof-imx8m-y := imx8m.o
> snd-sof-imx8ulp-y := imx8ulp.o
>
> snd-sof-imx-common-y := imx-common.o
>
> obj-$(CONFIG_SND_SOC_SOF_IMX8) += snd-sof-imx8.o
> -obj-$(CONFIG_SND_SOC_SOF_IMX8M) += snd-sof-imx8m.o
> obj-$(CONFIG_SND_SOC_SOF_IMX8ULP) += snd-sof-imx8ulp.o
> obj-$(CONFIG_SND_SOC_SOF_IMX_COMMON) += imx-common.o
> diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c
...
> return 0;
> }
>
> +static int imx8m_probe(struct snd_sof_dev *sdev)
> +{
> + struct imx_common_data *common;
> + struct imx8m_chip_data *chip;
> +
> + common = sdev->pdata->hw_pdata;
> +
> + chip = devm_kzalloc(sdev->dev, sizeof(*chip), GFP_KERNEL);
> + if (!chip)
> + return dev_err_probe(sdev->dev, -ENOMEM,
> + "failed to allocate chip data\n");
> +
> + chip->dap = devm_ioremap(sdev->dev, IMX8M_DAP_DEBUG, IMX8M_DAP_DEBUG_SIZE);
> + if (!chip->dap)
> + return dev_err_probe(sdev->dev, -ENODEV,
> + "failed to ioremap DAP\n");

It is okay for cleanup code. But I think the IMX8M_DAP_DEBUG should come
from dts as one of reg.

> +
> + chip->regmap = syscon_regmap_lookup_by_phandle(sdev->dev->of_node, "fsl,dsp-ctrl");
> + if (IS_ERR(chip->regmap))
> + return dev_err_probe(sdev->dev, PTR_ERR(chip->regmap),
> + "failed to fetch dsp ctrl regmap\n");

The same here, it should use standard reset interface instead.

Please remember two points, which need be improved later

For this patch, it is fine because just copy old code to here.

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>

> +
> + common->chip_pdata = chip;
> +
> + return 0;
> +}
> +
...
> -module_platform_driver(snd_sof_of_imx8m_driver);
> -
> -MODULE_LICENSE("Dual BSD/GPL");
> -MODULE_DESCRIPTION("SOF support for IMX8M platforms");
> -MODULE_IMPORT_NS("SND_SOC_SOF_XTENSA");
> --
> 2.34.1
>