Hi,
I'm trying to fix up incorrect usage of of_find_node_by_name() to lookup child nodes, and found the following code in sound/soc/mediatek/mt8173/mt8173-rt5650.c:
static int mt8173_rt5650_dev_probe(struct platform_device *pdev) { ...
platform_node = of_parse_phandle(pdev->dev.of_node, "mediatek,platform", 0); ...
if (of_find_node_by_name(platform_node, "codec-capture")) { np = of_get_child_by_name(pdev->dev.of_node, "codec-capture"); if (!np) { dev_err(&pdev->dev, "%s: Can't find codec-capture DT node\n", __func__); return -EINVAL; } ret = snd_soc_of_get_dai_name(np, &codec_capture_dai); if (ret < 0) { dev_err(&pdev->dev, "%s codec_capture_dai name fail %d\n", __func__, ret); return ret; } mt8173_rt5650_codecs[1].dai_name = codec_capture_dai; }
added by commit d349caeb0510 ("ASoC: mediatek: Add second I2S on mt8173-rt5650 machine driver").
First of all the "codec-capture" node is indeed documented as a child node of the sound node, so the tree-wide depth-first search from the platform_node looks entirely bogus.
Note that of_find_node_by_name() also drops a reference to its first argument, in this case the sound node, which could end up being prematurely freed.
And then the reference to any returned codec-capture node (from either lookup) is never dropped.
And since support for this second codec was added retrospectively and is documented as optional, that -EINVAL in case the node is missing looks broken too.
I figured I better just report this one to the author of the patch and the maintainers to be straightened out.
Thanks, Johan