diff mbox series

ASoC: rt5640: Do not manipulate pin "Platform Clock" if the "Platform Clock" is not in the DAPM

Message ID 20220516103055.20003-1-oder_chiou@realtek.com (mailing list archive)
State Accepted
Commit 832296804bc7171730884e78c761c29f6d258e13
Headers show
Series ASoC: rt5640: Do not manipulate pin "Platform Clock" if the "Platform Clock" is not in the DAPM | expand

Commit Message

Oder Chiou May 16, 2022, 10:30 a.m. UTC
The pin "Platform Clock" was only used by the Intel Byt CR platform. In the
others, the error log will be informed. The patch will set the flag to
avoid the pin "Platform Clock" manipulated by the other platforms.

Signed-off-by: Oder Chiou <oder_chiou@realtek.com>
Reported-by: Sameer Pujar <spujar@nvidia.com>
---
 sound/soc/codecs/rt5640.c             | 11 +++++++++--
 sound/soc/codecs/rt5640.h             |  2 ++
 sound/soc/intel/boards/bytcr_rt5640.c |  2 ++
 3 files changed, 13 insertions(+), 2 deletions(-)

Comments

Cezary Rojewski May 16, 2022, 10:48 a.m. UTC | #1
On 2022-05-16 12:30 PM, Oder Chiou wrote:
> The pin "Platform Clock" was only used by the Intel Byt CR platform. In the
> others, the error log will be informed. The patch will set the flag to
> avoid the pin "Platform Clock" manipulated by the other platforms.
> 
> Signed-off-by: Oder Chiou <oder_chiou@realtek.com>
> Reported-by: Sameer Pujar <spujar@nvidia.com>

Hello,

Please shorten your commit title a bit. It exceeds the recommendations 
of cannonical format.

Hope I find some spare time within the next two days to test this change 
on HSW/BDW machines as you mention only BYT here whereas there are other 
users of rt5640 codec. My testing is not a blocker - if I'll be late, 
will send appropriate fix - hopefully won't come to that.


Below some nitpicks.

> ---
>   sound/soc/codecs/rt5640.c             | 11 +++++++++--
>   sound/soc/codecs/rt5640.h             |  2 ++
>   sound/soc/intel/boards/bytcr_rt5640.c |  2 ++
>   3 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c
> index 12da2bea1a7b..69c80d80ed9d 100644
> --- a/sound/soc/codecs/rt5640.c
> +++ b/sound/soc/codecs/rt5640.c
> @@ -2094,12 +2094,14 @@ EXPORT_SYMBOL_GPL(rt5640_sel_asrc_clk_src);
>   void rt5640_enable_micbias1_for_ovcd(struct snd_soc_component *component)
>   {
>   	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
> +	struct rt5640_priv *rt5640 = snd_soc_component_get_drvdata(component);


Naming a local variable 'rt5640' is not a good idea. Conflicts with all 
the filtering. Use of 'priv' or 'data' (alone or as a suffix) is 
recommended.

>   
>   	snd_soc_dapm_mutex_lock(dapm);
>   	snd_soc_dapm_force_enable_pin_unlocked(dapm, "LDO2");
>   	snd_soc_dapm_force_enable_pin_unlocked(dapm, "MICBIAS1");
>   	/* OVCD is unreliable when used with RCCLK as sysclk-source */
> -	snd_soc_dapm_force_enable_pin_unlocked(dapm, "Platform Clock");
> +	if (rt5640->use_platform_clock)
> +		snd_soc_dapm_force_enable_pin_unlocked(dapm, "Platform Clock");
>   	snd_soc_dapm_sync_unlocked(dapm);
>   	snd_soc_dapm_mutex_unlock(dapm);
>   }
> @@ -2108,9 +2110,11 @@ EXPORT_SYMBOL_GPL(rt5640_enable_micbias1_for_ovcd);
>   void rt5640_disable_micbias1_for_ovcd(struct snd_soc_component *component)
>   {
>   	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
> +	struct rt5640_priv *rt5640 = snd_soc_component_get_drvdata(component);
>   
>   	snd_soc_dapm_mutex_lock(dapm);
> -	snd_soc_dapm_disable_pin_unlocked(dapm, "Platform Clock");
> +	if (rt5640->use_platform_clock)
> +		snd_soc_dapm_disable_pin_unlocked(dapm, "Platform Clock");
>   	snd_soc_dapm_disable_pin_unlocked(dapm, "MICBIAS1");
>   	snd_soc_dapm_disable_pin_unlocked(dapm, "LDO2");
>   	snd_soc_dapm_sync_unlocked(dapm);
> @@ -2535,6 +2539,9 @@ static void rt5640_enable_jack_detect(struct snd_soc_component *component,
>   		rt5640->jd_gpio_irq_requested = true;
>   	}
>   
> +	if (jack_data && jack_data->use_platform_clock)
> +		rt5640->use_platform_clock = jack_data->use_platform_clock;
> +
>   	ret = request_irq(rt5640->irq, rt5640_irq,
>   			  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
>   			  "rt5640", rt5640);
> diff --git a/sound/soc/codecs/rt5640.h b/sound/soc/codecs/rt5640.h
> index 9e49b9a0ccaa..505c93514051 100644
> --- a/sound/soc/codecs/rt5640.h
> +++ b/sound/soc/codecs/rt5640.h
> @@ -2155,11 +2155,13 @@ struct rt5640_priv {
>   	bool jd_inverted;
>   	unsigned int ovcd_th;
>   	unsigned int ovcd_sf;
> +	bool use_platform_clock;
>   };
>   
>   struct rt5640_set_jack_data {
>   	int codec_irq_override;
>   	struct gpio_desc *jd_gpio;
> +	bool use_platform_clock;
>   };


Why do we need 'use_platform_clock' twice?

>   
>   int rt5640_dmic_enable(struct snd_soc_component *component,
> diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
> index 7b948a219177..ed9fa1728722 100644
> --- a/sound/soc/intel/boards/bytcr_rt5640.c
> +++ b/sound/soc/intel/boards/bytcr_rt5640.c
> @@ -1191,12 +1191,14 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime)
>   {
>   	struct snd_soc_card *card = runtime->card;
>   	struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(card);
> +	struct rt5640_set_jack_data *jack_data = &priv->jack_data;
>   	struct snd_soc_component *component = asoc_rtd_to_codec(runtime, 0)->component;
>   	const struct snd_soc_dapm_route *custom_map = NULL;
>   	int num_routes = 0;
>   	int ret;
>   
>   	card->dapm.idle_bias_off = true;
> +	jack_data->use_platform_clock = true;
>   
>   	/* Start with RC clk for jack-detect (we disable MCLK below) */
>   	if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN)
Mark Brown May 26, 2022, 3:55 p.m. UTC | #2
On Mon, 16 May 2022 18:30:55 +0800, Oder Chiou wrote:
> The pin "Platform Clock" was only used by the Intel Byt CR platform. In the
> others, the error log will be informed. The patch will set the flag to
> avoid the pin "Platform Clock" manipulated by the other platforms.
> 
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: rt5640: Do not manipulate pin "Platform Clock" if the "Platform Clock" is not in the DAPM
      commit: 832296804bc7171730884e78c761c29f6d258e13

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
diff mbox series

Patch

diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c
index 12da2bea1a7b..69c80d80ed9d 100644
--- a/sound/soc/codecs/rt5640.c
+++ b/sound/soc/codecs/rt5640.c
@@ -2094,12 +2094,14 @@  EXPORT_SYMBOL_GPL(rt5640_sel_asrc_clk_src);
 void rt5640_enable_micbias1_for_ovcd(struct snd_soc_component *component)
 {
 	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
+	struct rt5640_priv *rt5640 = snd_soc_component_get_drvdata(component);
 
 	snd_soc_dapm_mutex_lock(dapm);
 	snd_soc_dapm_force_enable_pin_unlocked(dapm, "LDO2");
 	snd_soc_dapm_force_enable_pin_unlocked(dapm, "MICBIAS1");
 	/* OVCD is unreliable when used with RCCLK as sysclk-source */
-	snd_soc_dapm_force_enable_pin_unlocked(dapm, "Platform Clock");
+	if (rt5640->use_platform_clock)
+		snd_soc_dapm_force_enable_pin_unlocked(dapm, "Platform Clock");
 	snd_soc_dapm_sync_unlocked(dapm);
 	snd_soc_dapm_mutex_unlock(dapm);
 }
@@ -2108,9 +2110,11 @@  EXPORT_SYMBOL_GPL(rt5640_enable_micbias1_for_ovcd);
 void rt5640_disable_micbias1_for_ovcd(struct snd_soc_component *component)
 {
 	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
+	struct rt5640_priv *rt5640 = snd_soc_component_get_drvdata(component);
 
 	snd_soc_dapm_mutex_lock(dapm);
-	snd_soc_dapm_disable_pin_unlocked(dapm, "Platform Clock");
+	if (rt5640->use_platform_clock)
+		snd_soc_dapm_disable_pin_unlocked(dapm, "Platform Clock");
 	snd_soc_dapm_disable_pin_unlocked(dapm, "MICBIAS1");
 	snd_soc_dapm_disable_pin_unlocked(dapm, "LDO2");
 	snd_soc_dapm_sync_unlocked(dapm);
@@ -2535,6 +2539,9 @@  static void rt5640_enable_jack_detect(struct snd_soc_component *component,
 		rt5640->jd_gpio_irq_requested = true;
 	}
 
+	if (jack_data && jack_data->use_platform_clock)
+		rt5640->use_platform_clock = jack_data->use_platform_clock;
+
 	ret = request_irq(rt5640->irq, rt5640_irq,
 			  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
 			  "rt5640", rt5640);
diff --git a/sound/soc/codecs/rt5640.h b/sound/soc/codecs/rt5640.h
index 9e49b9a0ccaa..505c93514051 100644
--- a/sound/soc/codecs/rt5640.h
+++ b/sound/soc/codecs/rt5640.h
@@ -2155,11 +2155,13 @@  struct rt5640_priv {
 	bool jd_inverted;
 	unsigned int ovcd_th;
 	unsigned int ovcd_sf;
+	bool use_platform_clock;
 };
 
 struct rt5640_set_jack_data {
 	int codec_irq_override;
 	struct gpio_desc *jd_gpio;
+	bool use_platform_clock;
 };
 
 int rt5640_dmic_enable(struct snd_soc_component *component,
diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index 7b948a219177..ed9fa1728722 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -1191,12 +1191,14 @@  static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime)
 {
 	struct snd_soc_card *card = runtime->card;
 	struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(card);
+	struct rt5640_set_jack_data *jack_data = &priv->jack_data;
 	struct snd_soc_component *component = asoc_rtd_to_codec(runtime, 0)->component;
 	const struct snd_soc_dapm_route *custom_map = NULL;
 	int num_routes = 0;
 	int ret;
 
 	card->dapm.idle_bias_off = true;
+	jack_data->use_platform_clock = true;
 
 	/* Start with RC clk for jack-detect (we disable MCLK below) */
 	if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN)