On Sat, Feb 28, 2015 at 10:04:30PM -0500, Yakir Yang wrote: > + ret = snd_soc_dai_set_fmt(cpu_dai, dai_fmt); > + if (ret < 0) { > + dev_err(cpu_dai->dev, "failed to set cpu_dai fmt.\n"); > + return ret; > + } You've already set this in the dai_link, no need to do it again. > + dev_info(&pdev->dev, "hdmi audio init success.\n"); Please remove noisy prints like this. > +free_cpu_of_node: > + hdmi_audio_dai.cpu_of_node = NULL; > + hdmi_audio_dai.platform_of_node = NULL; > +free_priv_data: > + snd_soc_card_set_drvdata(card, NULL); > + platform_set_drvdata(pdev, NULL); > + card->dev = NULL; If any of these assignments is doing anything there's a problem with the code. > +{ > + struct snd_soc_card *card = platform_get_drvdata(pdev); > + > + snd_soc_unregister_card(card); devm_snd_soc_register_card() and you can remove this function entirely. > +static const struct of_device_id rockchip_hdmi_audio_of_match[] = { > + { .compatible = "rockchip,rk3288-hdmi-audio", }, > + {}, > +}; There is no documentation for this binding, binding documentation is mandatory. Based on the compatible string this looks like it's specific to the SoC rather than a design for a board - is the whole card part of the SoC?