| 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
| ||
|
Message-ID: <20220329223002.uo7kiemopkh7ak4x@notapiano>
Date: Tue, 29 Mar 2022 18:30:02 -0400
From: Nícolas F. R. A. Prado
<nfraprado@...labora.com>
To: Jiaxin Yu <jiaxin.yu@...iatek.com>
Cc: broonie@...nel.org, robh+dt@...nel.org, tzungbi@...gle.com,
angelogioacchino.delregno@...labora.com, aaronyu@...gle.com,
matthias.bgg@...il.com, trevor.wu@...iatek.com, linmq006@...il.com,
alsa-devel@...a-project.org, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, linux-kernel@...r.kernel.org,
Project_Global_Chrome_Upstream_Group@...iatek.com,
Tzung-Bi Shih <tzungbi@...nel.org>
Subject: Re: [v7 2/4] ASoC: mediatek: mt8192: refactor for I2S3 DAI link of
speaker
Hi Jiaxin,
On Thu, Mar 24, 2022 at 02:45:09PM +0800, Jiaxin Yu wrote:
> MT8192 platform will use rt1015 or rt105p codec, so through the
> snd_soc_of_get_dai_link_codecs() to complete the configuration
> of dai_link's codecs.
>
> Signed-off-by: Jiaxin Yu <jiaxin.yu@...iatek.com>
> Reviewed-by: Tzung-Bi Shih <tzungbi@...nel.org>
> ---
> .../mt8192/mt8192-mt6359-rt1015-rt5682.c | 108 ++++++++++--------
> 1 file changed, 59 insertions(+), 49 deletions(-)
>
> diff --git a/sound/soc/mediatek/mt8192/mt8192-mt6359-rt1015-rt5682.c b/sound/soc/mediatek/mt8192/mt8192-mt6359-rt1015-rt5682.c
> index ee91569c0911..837c2ccd5b3d 100644
> --- a/sound/soc/mediatek/mt8192/mt8192-mt6359-rt1015-rt5682.c
> +++ b/sound/soc/mediatek/mt8192/mt8192-mt6359-rt1015-rt5682.c
> @@ -604,17 +604,9 @@ SND_SOC_DAILINK_DEFS(i2s2,
> DAILINK_COMP_ARRAY(COMP_DUMMY()),
> DAILINK_COMP_ARRAY(COMP_EMPTY()));
>
> -SND_SOC_DAILINK_DEFS(i2s3_rt1015,
> +SND_SOC_DAILINK_DEFS(i2s3,
> DAILINK_COMP_ARRAY(COMP_CPU("I2S3")),
> - DAILINK_COMP_ARRAY(COMP_CODEC(RT1015_DEV0_NAME,
> - RT1015_CODEC_DAI),
> - COMP_CODEC(RT1015_DEV1_NAME,
> - RT1015_CODEC_DAI)),
> - DAILINK_COMP_ARRAY(COMP_EMPTY()));
> -
> -SND_SOC_DAILINK_DEFS(i2s3_rt1015p,
> - DAILINK_COMP_ARRAY(COMP_CPU("I2S3")),
> - DAILINK_COMP_ARRAY(COMP_CODEC("rt1015p", "HiFi")),
> + DAILINK_COMP_ARRAY(COMP_EMPTY()),
> DAILINK_COMP_ARRAY(COMP_EMPTY()));
>
> SND_SOC_DAILINK_DEFS(i2s5,
> @@ -929,6 +921,7 @@ static struct snd_soc_dai_link mt8192_mt6359_dai_links[] = {
> .dpcm_playback = 1,
> .ignore_suspend = 1,
> .be_hw_params_fixup = mt8192_i2s_hw_params_fixup,
> + SND_SOC_DAILINK_REG(i2s3),
> },
> {
> .name = "I2S5",
> @@ -1100,55 +1093,64 @@ static struct snd_soc_card mt8192_mt6359_rt1015p_rt5682_card = {
> .num_dapm_routes = ARRAY_SIZE(mt8192_mt6359_rt1015p_rt5682_routes),
> };
>
> +static int mt8192_mt6359_card_set_be_link(struct snd_soc_card *card,
> + struct snd_soc_dai_link *link,
> + struct device_node *node,
> + char *link_name)
> +{
> + int ret;
> +
> + if (node && strcmp(link->name, link_name) == 0) {
> + ret = snd_soc_of_get_dai_link_codecs(card->dev, node, link);
> + if (ret < 0) {
> + dev_err_probe(card->dev, ret, "get dai link codecs fail\n");
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> static int mt8192_mt6359_dev_probe(struct platform_device *pdev)
> {
> struct snd_soc_card *card;
> - struct device_node *platform_node, *hdmi_codec;
> + struct device_node *platform_node, *hdmi_codec, *speaker_codec;
> int ret, i;
> struct snd_soc_dai_link *dai_link;
> struct mt8192_mt6359_priv *priv;
>
> - platform_node = of_parse_phandle(pdev->dev.of_node,
> - "mediatek,platform", 0);
> - if (!platform_node) {
> - dev_err(&pdev->dev, "Property 'platform' missing or invalid\n");
> + card = (struct snd_soc_card *)of_device_get_match_data(&pdev->dev);
> + if (!card)
> return -EINVAL;
> + card->dev = &pdev->dev;
> +
> + platform_node = of_parse_phandle(pdev->dev.of_node, "mediatek,platform", 0);
> + if (!platform_node) {
> + ret = -EINVAL;
> + dev_err_probe(&pdev->dev, ret, "Property 'platform' missing or invalid\n");
> + goto err_platform_node;
> }
>
> - card = (struct snd_soc_card *)of_device_get_match_data(&pdev->dev);
> - if (!card) {
> + hdmi_codec = of_parse_phandle(pdev->dev.of_node, "mediatek,hdmi-codec", 0);
> + if (!hdmi_codec) {
> ret = -EINVAL;
> - goto put_platform_node;
> + dev_err_probe(&pdev->dev, ret, "Property 'hdmi-codec' missing or invalid\n");
> + goto err_hdmi_codec;
You're making hdmi-codec a required property, since now the driver fails to
probe without it. Is it really required though? The driver code still checks for
the presence of hdmi_codec before using it, so shouldn't it be fine to let it be
optional?
If it is really required now though, then I guess at least the dt-binding should
be updated accordingly. (Although I think this would technically break the ABI?)
Thanks,
Nícolas
> }
> - card->dev = &pdev->dev;
>
> - hdmi_codec = of_parse_phandle(pdev->dev.of_node,
> - "mediatek,hdmi-codec", 0);
> + speaker_codec = of_get_child_by_name(pdev->dev.of_node, "speaker-codecs");
> + if (!speaker_codec) {
> + ret = -EINVAL;
> + dev_err_probe(&pdev->dev, ret, "Property 'speaker-codecs' missing or invalid\n");
> + goto err_speaker_codec;
> + }
>
> for_each_card_prelinks(card, i, dai_link) {
> - if (strcmp(dai_link->name, "I2S3") == 0) {
> - if (card == &mt8192_mt6359_rt1015_rt5682_card) {
> - dai_link->ops = &mt8192_rt1015_i2s_ops;
> - dai_link->cpus = i2s3_rt1015_cpus;
> - dai_link->num_cpus =
> - ARRAY_SIZE(i2s3_rt1015_cpus);
> - dai_link->codecs = i2s3_rt1015_codecs;
> - dai_link->num_codecs =
> - ARRAY_SIZE(i2s3_rt1015_codecs);
> - dai_link->platforms = i2s3_rt1015_platforms;
> - dai_link->num_platforms =
> - ARRAY_SIZE(i2s3_rt1015_platforms);
> - } else if (card == &mt8192_mt6359_rt1015p_rt5682_card) {
> - dai_link->cpus = i2s3_rt1015p_cpus;
> - dai_link->num_cpus =
> - ARRAY_SIZE(i2s3_rt1015p_cpus);
> - dai_link->codecs = i2s3_rt1015p_codecs;
> - dai_link->num_codecs =
> - ARRAY_SIZE(i2s3_rt1015p_codecs);
> - dai_link->platforms = i2s3_rt1015p_platforms;
> - dai_link->num_platforms =
> - ARRAY_SIZE(i2s3_rt1015p_platforms);
> - }
> + ret = mt8192_mt6359_card_set_be_link(card, dai_link, speaker_codec, "I2S3");
> + if (ret) {
> + dev_err_probe(&pdev->dev, ret, "%s set speaker_codec fail\n",
> + dai_link->name);
> + goto err_probe;
> }
>
> if (hdmi_codec && strcmp(dai_link->name, "TDM") == 0) {
> @@ -1156,6 +1158,9 @@ static int mt8192_mt6359_dev_probe(struct platform_device *pdev)
> dai_link->ignore = 0;
> }
>
> + if (strcmp(dai_link->codecs[0].dai_name, RT1015_CODEC_DAI) == 0)
> + dai_link->ops = &mt8192_rt1015_i2s_ops;
> +
> if (!dai_link->platforms->name)
> dai_link->platforms->of_node = platform_node;
> }
> @@ -1163,22 +1168,27 @@ static int mt8192_mt6359_dev_probe(struct platform_device *pdev)
> priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> if (!priv) {
> ret = -ENOMEM;
> - goto put_hdmi_codec;
> + goto err_probe;
> }
> snd_soc_card_set_drvdata(card, priv);
>
> ret = mt8192_afe_gpio_init(&pdev->dev);
> if (ret) {
> - dev_err(&pdev->dev, "init gpio error %d\n", ret);
> - goto put_hdmi_codec;
> + dev_err_probe(&pdev->dev, ret, "%s init gpio error\n", __func__);
> + goto err_probe;
> }
>
> ret = devm_snd_soc_register_card(&pdev->dev, card);
> + if (ret)
> + dev_err_probe(&pdev->dev, ret, "%s snd_soc_register_card fail\n", __func__);
>
> -put_hdmi_codec:
> +err_probe:
> + of_node_put(speaker_codec);
> +err_speaker_codec:
> of_node_put(hdmi_codec);
> -put_platform_node:
> +err_hdmi_codec:
> of_node_put(platform_node);
> +err_platform_node:
> return ret;
> }
>
> --
> 2.18.0
>
>
Powered by blists - more mailing lists