Re: [PATCH v2 1/8] ASoC: SOF: imx: introduce more common structures and functions

[Frank Li]

On Wed, Feb 05, 2025 at 03:30:15PM -0500, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <laurentiu.mihalcea@xxxxxxx>
>
> The SOF drivers for imx chips have a lot of duplicate code and
> routines/code snippets that could certainly be reused among drivers.
>
> As such, introduce a new set of structures and functions that will help
> eliminate the redundancy and code size of the drivers.
>
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@xxxxxxx>
> ---

some nits

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

>  sound/soc/sof/imx/imx-common.c | 432 ++++++++++++++++++++++++++++++++-
>  sound/soc/sof/imx/imx-common.h | 151 ++++++++++++
>  2 files changed, 582 insertions(+), 1 deletion(-)
>
...
> +static int imx_probe(struct snd_sof_dev *sdev)
> +{
> +	struct dev_pm_domain_attach_data domain_data = {
> +		.pd_names = NULL, /* no filtering */
> +		.pd_flags = PD_FLAG_DEV_LINK_ON,
> +	};
> +	struct imx_common_data *common;
> +	struct platform_device *pdev;
> +	int ret;
> +
> +	pdev = to_platform_device(sdev->dev);
> +
...
> +
> +	ret = devm_clk_bulk_get_all(sdev->dev, &common->clks);
> +	if (ret < 0)
> +		return dev_err_probe(sdev->dev, common->clk_num,
> +				     "failed to fetch clocks\n");
> +	common->clk_num = ret;
> +
> +	/* no effect if number of clocks is 0 */

this comments should be common sense, can be removed.

Frank

> +	ret = clk_bulk_prepare_enable(common->clk_num, common->clks);
> +	if (ret < 0)
> +		return dev_err_probe(sdev->dev, ret, "failed to enable clocks\n");
> +
> +	common->ipc_handle->ops = &imx_ipc_ops;
> +	imx_dsp_set_data(common->ipc_handle, sdev);
> +
> +	sdev->num_cores = 1;
> +	sdev->pdata->hw_pdata = common;
> +	sdev->mailbox_bar = SOF_FW_BLK_TYPE_SRAM;
> +	sdev->dsp_box.offset = get_chip_info(sdev)->ipc_info.boot_mbox_offset;
> +
> +	return imx_chip_probe(sdev);
> +}
> +
...
> +
>  #endif
> --
> 2.34.1
>