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

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z6Ti0hn+iANzm6wd@lizhi-Precision-Tower-5810>
Date: Thu, 6 Feb 2025 11:26:58 -0500
From: Frank Li <Frank.li@....com>
To: Laurentiu Mihalcea <laurentiumihalcea111@...il.com>
Cc: Bard Liao <yung-chuan.liao@...ux.intel.com>,
	Daniel Baluta <daniel.baluta@....com>,
	Iuliana Prodan <iuliana.prodan@....com>,
	Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
	Mark Brown <broonie@...nel.org>, linux-kernel@...r.kernel.org,
	linux-sound@...r.kernel.org, imx@...ts.linux.dev
Subject: Re: [PATCH v2 6/8] ASoC: SOF: imx: merge imx8 and imx8m drivers

On Wed, Feb 05, 2025 at 03:30:20PM -0500, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <laurentiu.mihalcea@....com>
>
> 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@....com>
> ---
>  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@....com>

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