Messages in this thread | | | Date | Tue, 2 Jun 2015 20:55:06 +0100 | From | Mark Brown <> | Subject | Re: [PATCH v4 2/3] ASoC: qcom: add apq8016 sound card support |
| |
On Fri, May 22, 2015 at 04:54:07PM +0100, Srinivas Kandagatla wrote:
> + if (cpu_dai->id == MI2S_QUATERNARY) { > + /* Configure the Quat MI2S to TLMM */ > + writel(readl(pdata->mic_iomux) | > + MIC_CTRL_QUA_WS_SLAVE_SEL_10 | > + MIC_CTRL_TLMM_SCLK_EN, > + pdata->mic_iomux); > + > + return 0; > + } else if (cpu_dai->id == MI2S_PRIMARY) {
This looks like you're trying to write a switch statement. It's also somewhat unclear to me that this should be in a machine driver and not in a CODEC/aux driver that gets pulled in by a machine driver, I can't be entirely sure what this is controlling.
> + if (of_property_read_bool(np, "external")) > + name = "HDMI"; > + > + else > + name = "Headset";
Coding style. I'm also a bit concerned about the binding here - headsets sound external too?
> + card->dev = dev; > + data = apq8016_sbc_parse_of(card);
We parse the DT here and then...
> + ret = snd_soc_of_parse_card_name(card, "qcom,model"); > + if (ret) { > + dev_err(&pdev->dev, "Error parsing card name: %d\n", ret); > + return ret; > + }
...this other bit of DT here.
> + ret = devm_snd_soc_register_card(&pdev->dev, card); > + if (ret == -EPROBE_DEFER) { > + card->dev = NULL; > + return ret; > + } else if (ret) { > + dev_err(&pdev->dev, "Error registering soundcard: %d\n", ret); > + return ret; > + }
If setting card->dev does anything there something is broken, and in general it's just better form to not special case probe deferral. [unhandled content-type:application/pgp-signature] | |