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

[Cezary Rojewski]

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 at realtek.com>
> Reported-by: Sameer Pujar <spujar at 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)