diff mbox

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

Message ID 20170817044417.22069-3-jeffy.chen@rock-chips.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jeffy Chen Aug. 17, 2017, 4:44 a.m. UTC 
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(-)

Comments

Mark Brown Aug. 17, 2017, 5:11 p.m. UTC | #1 
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?
Jeffy Chen Aug. 18, 2017, 1:41 a.m. UTC | #2 
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...
Mark Brown Aug. 18, 2017, 11:52 a.m. UTC | #3 
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?
Jeffy Chen Aug. 18, 2017, 2:41 p.m. UTC | #4 
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 :)
diff mbox

Patch

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);