lkml.org 
[lkml]   [2015]   [Mar]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 14/15] ASoC: rockchip/rockchip-hdmi-audio: add sound driver for hdmi audio
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



\
 
 \ /
  Last update: 2015-03-27 02:21    [W:0.077 / U:1.000 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site