On Wed, 2022-03-30 at 11:20 -0400, Nícolas F. R. A. Prado wrote:
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
Hi Nícolas,
Thanks for your comment. Indeed I made hdmi-codec a required property, because it is a must in this machine driver. I prefer to report errors during the registration rather than during the use.
But what do you mean that it is required in this machine driver? The code checks for presence of hdmi_codec and ignores it if it's not set, so it does really seem optional to me. Also, I have tested this driver on mt8192- asurada-spherion without hdmi-codec set in the DT and the speaker and headphone sound works just fine.
Besides, there might be machines using this driver that don't support HDMI, and requiring an hdmi-codec in the DT for them would not make any sense. So keeping hdmi-codec as optional seems like the most sensible solution to me, really.
Thanks, Nícolas
Yes, I agree with you. In the past, if there was a new board without HDMI audio, we would choose to add a new machine driver and a new dt- bindings. But now, in order to simplify the code, we tend to share one machine driver for boards that use similar codecs. And we are doing this now.
Thanks, Jiaxin.Yu