Re: [PATCH v4 2/6] ASoC: codecs: Add msm8916-wcd digital codec
From: Mark Brown <hidden>
Date: 2016-09-06 11:24:56
Also in:
alsa-devel, linux-arm-msm
On Tue, Sep 06, 2016 at 10:57:41AM +0100, Srinivas Kandagatla wrote:
+static const struct soc_enum rx_mix2_inp1_chain_enum = +SOC_ENUM_SINGLE(LPASS_CDC_CONN_RX1_B3_CTL, 0, 3, rx_mix2_text);
Indentation again.
+ case SND_SOC_DAPM_PRE_PMU:
+ if (++msm8916_wcd->dmic_clk_cnt == 1) {
+ snd_soc_update_bits(codec, dmic_clk_reg,This looks like you're open coding a supply widget.
+ if (dmic == 1) + snd_soc_update_bits(codec, LPASS_CDC_TX1_DMIC_CTL, + TXN_DMIC_CTL_CLK_SEL_MASK, + TXN_DMIC_CTL_CLK_SEL_DIV3); + if (dmic == 2) + snd_soc_update_bits(codec, LPASS_CDC_TX2_DMIC_CTL, + TXN_DMIC_CTL_CLK_SEL_MASK, + TXN_DMIC_CTL_CLK_SEL_DIV3);
This looks like you want a switch statement.
+static int msm8916_wcd_digital_enable_clock_block(struct snd_soc_codec
+ *codec, int enable)
+{+static int msm8916_wcd_digital_codec_remove(struct snd_soc_codec *codec)
+{
+ return 0;
+}Remove empty functions.
+static int msm8916_wcd_digital_startup(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ msm8916_wcd_digital_enable_clock_block(dai->codec, 1);
+ return 0;
+}
+
+static void msm8916_wcd_digital_shutdown(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ msm8916_wcd_digital_enable_clock_block(dai->codec, 0);
+}Two problems here: one is that the power management should be in DAPM, the other is that the _enable_clock_block() function is totally pointless - it has only these two call locations and there is absolutely no shared code between the enable and disable paths so they should just be inlined.
+static int msm8916_wcd_digital_remove(struct platform_device *pdev)
+{
+ struct msm8916_wcd_digital_priv *priv = dev_get_drvdata(&pdev->dev);
+
+ clk_disable_unprepare(priv->mclk);
+ clk_disable_unprepare(priv->ahbclk);
+ snd_soc_unregister_codec(&pdev->dev);This is disabling the clocks while the CODEC is still registered and might be in use, you should unregister first.
+static const struct of_device_id msm8916_wcd_digital_match_table[] = {
+ {.compatible = "qcom,msm8916-wcd-digital-codec"},
+ {}Spaces please.
+static struct platform_driver msm8916_wcd_digital_driver = {
+ .driver = {
+ .name = "msm8916-wcd-digital-codec",
+ .of_match_table = msm8916_wcd_digital_match_table,
+ },Please line up the }.
Attachments
- signature.asc [application/pgp-signature] 455 bytes