Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1713531 > unrolled thread

[PATCH v3 2/9] ASoC: rockchip: Remove obsolete dmic-delay

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v3 2/9] ASoC: rockchip: Remove obsolete dmic-delay Jeffy Chen <jeffy.chen@rock-chips.com> - 2017-08-17 06:50 +0200
    Re: [PATCH v3 2/9] ASoC: rockchip: Remove obsolete dmic-delay Mark Brown <broonie@kernel.org> - 2017-08-17 19:20 +0200
      Re: [PATCH v3 2/9] ASoC: rockchip: Remove obsolete dmic-delay jeffy <jeffy.chen@rock-chips.com> - 2017-08-18 03:50 +0200
        Re: [PATCH v3 2/9] ASoC: rockchip: Remove obsolete dmic-delay Mark Brown <broonie@kernel.org> - 2017-08-18 14:00 +0200
          Re: [PATCH v3 2/9] ASoC: rockchip: Remove obsolete dmic-delay jeffy <jeffy.chen@rock-chips.com> - 2017-08-18 16:50 +0200

#1713531 — [PATCH v3 2/9] ASoC: rockchip: Remove obsolete dmic-delay

This property is no longer used.

Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---

Changes in v3: None
Changes in v2: None

 .../bindings/sound/rockchip,rk3399-gru-sound.txt           |  7 -------
 sound/soc/rockchip/rk3399_gru_sound.c                      | 14 --------------
 2 files changed, 21 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/rockchip,rk3399-gru-sound.txt b/Documentation/devicetree/bindings/sound/rockchip,rk3399-gru-sound.txt
index eac91db07178..f19b6c830a34 100644
--- a/Documentation/devicetree/bindings/sound/rockchip,rk3399-gru-sound.txt
+++ b/Documentation/devicetree/bindings/sound/rockchip,rk3399-gru-sound.txt
@@ -6,17 +6,10 @@ Required properties:
   connected to the codecs
 - rockchip,codec: The phandle of the MAX98357A/RT5514/DA7219 codecs
 
-Optional properties:
-- dmic-wakeup-delay-ms : specify delay time (ms) for DMIC ready.
-  If this option is specified, which means it's required dmic need
-  delay for DMIC to ready so that rt5514 can avoid recording before
-  DMIC send valid data
-
 Example:
 
 sound {
 	compatible = "rockchip,rk3399-gru-sound";
 	rockchip,cpu = <&i2s0>;
 	rockchip,codec = <&max98357a &rt5514 &da7219>;
-	dmic-wakeup-delay-ms = <20>;
 };
diff --git a/sound/soc/rockchip/rk3399_gru_sound.c b/sound/soc/rockchip/rk3399_gru_sound.c
index 3475c61a5fa0..70876d3c34e6 100644
--- a/sound/soc/rockchip/rk3399_gru_sound.c
+++ b/sound/soc/rockchip/rk3399_gru_sound.c
@@ -38,8 +38,6 @@
 
 #define SOUND_FS	256
 
-static unsigned int rt5514_dmic_delay;
-
 static struct snd_soc_jack rockchip_sound_jack;
 
 static const struct snd_soc_dapm_widget rockchip_dapm_widgets[] = {
@@ -125,9 +123,6 @@ static int rockchip_sound_rt5514_hw_params(struct snd_pcm_substream *substream,
 		return ret;
 	}
 
-	/* Wait for DMIC stable */
-	msleep(rt5514_dmic_delay);
-
 	return 0;
 }
 
@@ -348,15 +343,6 @@ static int rockchip_sound_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	/* Set DMIC delay */
-	ret = device_property_read_u32(&pdev->dev, "dmic-delay",
-					&rt5514_dmic_delay);
-	if (ret) {
-		rt5514_dmic_delay = 0;
-		dev_dbg(&pdev->dev,
-			"no optional property 'dmic-delay' found, default: no delay\n");
-	}
-
 	rockchip_dailinks[DAILINK_RT5514_DSP].cpu_name = kstrdup_const(dev_name(dev), GFP_KERNEL);
 	rockchip_dailinks[DAILINK_RT5514_DSP].cpu_dai_name = kstrdup_const(dev_name(dev), GFP_KERNEL);
 	rockchip_dailinks[DAILINK_RT5514_DSP].platform_name = kstrdup_const(dev_name(dev), GFP_KERNEL);
-- 
2.11.0

[toc] | [next] | [standalone]

#1714227

[Multipart message — attachments visible in raw view] — view raw

On Thu, Aug 17, 2017 at 12:44:10PM +0800, Jeffy Chen wrote:

> This property is no longer used.

> -Optional properties:
> -- dmic-wakeup-delay-ms : specify delay time (ms) for DMIC ready.
> -  If this option is specified, which means it's required dmic need
> -  delay for DMIC to ready so that rt5514 can avoid recording before
> -  DMIC send valid data

Why would nobody ever want to use this in future?

[toc] | [prev] | [next] | [standalone]

#1714520

Hi Mark,

thanks for your reply

On 08/18/2017 01:11 AM, Mark Brown wrote:
> On Thu, Aug 17, 2017 at 12:44:10PM +0800, Jeffy Chen wrote:
>
>> This property is no longer used.
>
>> -Optional properties:
>> -- dmic-wakeup-delay-ms : specify delay time (ms) for DMIC ready.
>> -  If this option is specified, which means it's required dmic need
>> -  delay for DMIC to ready so that rt5514 can avoid recording before
>> -  DMIC send valid data
>
> Why would nobody ever want to use this in future?
>

hmmm, check again, rt5514 fixed this issue in there driver instead in:
a5461fd6c8f7 ASoC: rt5514: Add the DMIC initial delay to wait it ready.

so we don't need this...

[toc] | [prev] | [next] | [standalone]

#1714839

[Multipart message — attachments visible in raw view] — view raw

On Fri, Aug 18, 2017 at 09:41:54AM +0800, jeffy wrote:
> On 08/18/2017 01:11 AM, Mark Brown wrote:
> > On Thu, Aug 17, 2017 at 12:44:10PM +0800, Jeffy Chen wrote:

> > > -Optional properties:
> > > -- dmic-wakeup-delay-ms : specify delay time (ms) for DMIC ready.
> > > -  If this option is specified, which means it's required dmic need
> > > -  delay for DMIC to ready so that rt5514 can avoid recording before
> > > -  DMIC send valid data

> > Why would nobody ever want to use this in future?

> hmmm, check again, rt5514 fixed this issue in there driver instead in:
> a5461fd6c8f7 ASoC: rt5514: Add the DMIC initial delay to wait it ready.

> so we don't need this...

You don't need it on your board for new DTs but why is it not possible
that another board might need it?

[toc] | [prev] | [next] | [standalone]

#1715208

Hi Mark,

On 08/18/2017 07:52 PM, Mark Brown wrote:
> On Fri, Aug 18, 2017 at 09:41:54AM +0800, jeffy wrote:
>> On 08/18/2017 01:11 AM, Mark Brown wrote:
>>> On Thu, Aug 17, 2017 at 12:44:10PM +0800, Jeffy Chen wrote:
>
>>>> -Optional properties:
>>>> -- dmic-wakeup-delay-ms : specify delay time (ms) for DMIC ready.
>>>> -  If this option is specified, which means it's required dmic need
>>>> -  delay for DMIC to ready so that rt5514 can avoid recording before
>>>> -  DMIC send valid data
>
>>> Why would nobody ever want to use this in future?
>
>> hmmm, check again, rt5514 fixed this issue in there driver instead in:
>> a5461fd6c8f7 ASoC: rt5514: Add the DMIC initial delay to wait it ready.
>
>> so we don't need this...
>
> You don't need it on your board for new DTs but why is it not possible
> that another board might need it?
>

this property is added in:
3a6f9dce6116 ASoC: rk3399_gru_sound: fix recording pop at first attempt

to fix pop noise by performing a delay after set dmic clk.

the binding said "dmic-wakeup-delay-ms", but actually driver parses 
"dmic-delay":

+       /* Set DMIC delay */
+       ret = device_property_read_u32(&pdev->dev, "dmic-delay",
+                                       &rt5514_dmic_delay);

@@ -123,6 +125,9 @@ static int rockchip_sound_rt5514_hw_params(struct 
snd_pcm_subst
ream *substream,
                 return ret;
         }

+       /* Wait for DMIC stable */
+       msleep(rt5514_dmic_delay); <-- perform a delay after set dmic clk
+
         return 0;
  }

then realtek guys fixed this issue in the rt5514 codec driver:
a5461fd6c8f7 ASoC: rt5514: Add the DMIC initial delay to wait it ready.

+       device_property_read_u32(dev, "realtek,dmic-init-delay-ms",
+               &rt5514->pdata.dmic_init_delay);

@@ -452,6 +452,9 @@ static int rt5514_set_dmic_clk(struct 
snd_soc_dapm_widget *w,
                         RT5514_CLK_DMIC_OUT_SEL_MASK,
                         idx << RT5514_CLK_DMIC_OUT_SEL_SFT);

+       if (rt5514->pdata.dmic_init_delay)
+               msleep(rt5514->pdata.dmic_init_delay); <-- perform a 
delay after set dmic clk


so we don't need to deal with it in every boards' drivers which using 
rt5514 codec, and we are using this new property now:
ag dmic arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
620:            realtek,dmic-init-delay = <20>;   <---- oops, seems 
missing "-ms" at the end, guess due to realtek patch ends up differently 
on upstream. will send a patch to fix it later.


anyway, i think it's ok to remove that out-dated property in our driver :)

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web