Re: [PATCH v4 14/15] ASoC: rockchip/rockchip-hdmi-audio: add sound driver for hdmi audio

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Mark,

On 2015年03月27日 02:16, Mark Brown wrote:
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.

Okay, correct it in next v5.


+	dev_info(&pdev->dev, "hdmi audio init success.\n");
Please remove noisy prints like this.

Okay, turn it to dev_debug(...)

+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.

Yes, when probe failed, program will goto this 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.
do you mean that when I take devm_snd_soc_register_card() to register card,
then I do not need unregister card any more(destroy with device) ?

+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?
It's my fault, cause the dts patch have not CC you, I will correct it in next v5

Thanks :)
Yakir

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux