[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <11f54c022cb21ad2ee416cf6ee4130e4351f0482.camel@mediatek.com>
Date: Wed, 26 Jul 2023 02:25:01 +0000
From: Trevor Wu (吳文良) <Trevor.Wu@...iatek.com>
To: "lgirdwood@...il.com" <lgirdwood@...il.com>,
"matthias.bgg@...il.com" <matthias.bgg@...il.com>,
"angelogioacchino.delregno@...labora.com"
<angelogioacchino.delregno@...labora.com>,
"broonie@...nel.org" <broonie@...nel.org>,
"tiwai@...e.com" <tiwai@...e.com>,
"perex@...ex.cz" <perex@...ex.cz>
CC: "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-mediatek@...ts.infradead.org"
<linux-mediatek@...ts.infradead.org>,
"alsa-devel@...a-project.org" <alsa-devel@...a-project.org>
Subject: Re: [PATCH 3/3] ASoC: mediatek: mt8188-mt6359: add SOF support
On Tue, 2023-07-25 at 09:11 +0200, AngeloGioacchino Del Regno wrote:
> 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@...iatek.com>
> > ---
> > 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.
>
It is concise. I will apply it in v2.
> > + 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.
>
OK. I will remove one label in v2.
Thanks,
Trevor
Powered by blists - more mailing lists