Re: [PATCH 3/3] ASoC: mediatek: mt8188-mt6359: add SOF support

[AngeloGioacchino Del Regno]

Il 25/07/23 05:53, Trevor Wu ha scritto:
> SOF is enabled when adsp phandle is assigned to "mediatek,adsp".
> The required callback will be assigned when SOF is enabled.
>
> Additionally, "mediatek,dai-link" is introduced to decide the supported
> dai links for a project, so user can reuse the machine driver regardless
> of dai link combination.
>
> Signed-off-by: Trevor Wu <trevor.wu@xxxxxxxxxxxx>
> ---
>   sound/soc/mediatek/mt8188/mt8188-mt6359.c | 217 ++++++++++++++++++++--
>   1 file changed, 205 insertions(+), 12 deletions(-)
>
> diff --git a/sound/soc/mediatek/mt8188/mt8188-mt6359.c b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> index 667d79f33bf2..e205de2899a9 100644
> --- a/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> +++ b/sound/soc/mediatek/mt8188/mt8188-mt6359.c

..snip..

> @@ -1074,21 +1215,64 @@ static int mt8188_mt6359_dev_probe(struct platform_device *pdev)
>   	if (!card->name)
>   		card->name = card_data->name;
>   
> -	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> -	if (!priv)
> -		return  -ENOMEM;
> -
>   	if (of_property_read_bool(pdev->dev.of_node, "audio-routing")) {
>   		ret = snd_soc_of_parse_audio_routing(card, "audio-routing");
>   		if (ret)
>   			return ret;
>   	}
>   
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	soc_card_data = devm_kzalloc(&pdev->dev, sizeof(*card_data), GFP_KERNEL);
> +	if (!soc_card_data)
> +		return -ENOMEM;
> +
> +	soc_card_data->mach_priv = priv;
> +
> +	adsp_node = of_parse_phandle(pdev->dev.of_node, "mediatek,adsp", 0);
> +	if (adsp_node) {
> +		struct mtk_sof_priv *sof_priv;
> +
> +		sof_priv = devm_kzalloc(&pdev->dev, sizeof(*sof_priv), GFP_KERNEL);
> +		if (!sof_priv) {
> +			ret = -ENOMEM;
> +			goto err_adsp_node;
> +		}
> +		sof_priv->conn_streams = g_sof_conn_streams;
> +		sof_priv->num_streams = ARRAY_SIZE(g_sof_conn_streams);
> +		soc_card_data->sof_priv = sof_priv;
> +		card->probe = mtk_sof_card_probe;
> +		card->late_probe = mtk_sof_card_late_probe;
> +		if (!card->topology_shortname_created) {
> +			snprintf(card->topology_shortname, 32, "sof-%s", card->name);
> +			card->topology_shortname_created = true;
> +		}
> +		card->name = card->topology_shortname;
> +	}
> +
> +	if (of_property_read_bool(pdev->dev.of_node, "mediatek,dai-link")) {
> +		ret = mtk_sof_dailink_parse_of(card, pdev->dev.of_node,
> +					       "mediatek,dai-link",
> +					       mt8188_mt6359_dai_links,
> +					       ARRAY_SIZE(mt8188_mt6359_dai_links));
> +		if (ret) {
> +			dev_err_probe(&pdev->dev, ret, "Parse dai-link fail\n");
> +			goto err_adsp_node;
> +		}
> +	} else {
> +		if (!adsp_node)
> +			card->num_links = DAI_LINK_REGULAR_NUM;
> +	}
> +
>   	platform_node = of_parse_phandle(pdev->dev.of_node,
>   					 "mediatek,platform", 0);
>   	if (!platform_node) {
>   		ret = -EINVAL;
> -		return dev_err_probe(&pdev->dev, ret, "Property 'platform' missing or invalid\n");
> +		dev_err_probe(&pdev->dev, ret, "Property 'platform' missing or invalid\n");

We could shorten this with

ret = dev_err_probe(&pdev->dev, -EINVAL, "Property ....");
goto err_adsp_node;

...but I don't have strong opinions about that, it's your choice.

> +		goto err_platform_node;
> +
>   	}
>   
>   	ret = parse_dai_link_info(card);
> @@ -1096,8 +1280,12 @@ static int mt8188_mt6359_dev_probe(struct platform_device *pdev)
>   		goto err;
>   
>   	for_each_card_prelinks(card, i, dai_link) {
> -		if (!dai_link->platforms->name)
> -			dai_link->platforms->of_node = platform_node;
> +		if (!dai_link->platforms->name) {
> +			if (!strncmp(dai_link->name, "AFE_SOF", strlen("AFE_SOF")) && adsp_node)
> +				dai_link->platforms->of_node = adsp_node;
> +			else
> +				dai_link->platforms->of_node = platform_node;
> +		}
>   
>   		if (strcmp(dai_link->name, "DPTX_BE") == 0) {
>   			if (strcmp(dai_link->codecs->dai_name, "snd-soc-dummy-dai"))
> @@ -1140,7 +1328,7 @@ static int mt8188_mt6359_dev_probe(struct platform_device *pdev)
>   	}
>   
>   	priv->private_data = card_data;
> -	snd_soc_card_set_drvdata(card, priv);
> +	snd_soc_card_set_drvdata(card, soc_card_data);
>   
>   	ret = devm_snd_soc_register_card(&pdev->dev, card);
>   	if (ret)
> @@ -1149,6 +1337,11 @@ static int mt8188_mt6359_dev_probe(struct platform_device *pdev)
>   err:
>   	of_node_put(platform_node);
>   	clean_card_reference(card);
> +
> +err_adsp_node:
> +err_platform_node:

Just one label is enough. Keep err_adsp_node, remove err_platform_node.

Regards,
Angelo