Re: [PATCH v3 2/6] ASoC: mediatek: mt8195: merge machine driver - Tzung-Bi Shih

From: Tzung-Bi Shih <tzungbi@kernel.org>
To: Trevor Wu <trevor.wu@mediatek.com>
Cc: broonie@kernel.org, tiwai@suse.com, robh+dt@kernel.org,
	matthias.bgg@gmail.com, alsa-devel@alsa-project.org,
	linux-mediatek@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	aaronyu@google.com, angelogioacchino.delregno@collabora.com,
	yc.hung@mediatek.com
Subject: Re: [PATCH v3 2/6] ASoC: mediatek: mt8195: merge machine driver
Date: Mon, 21 Mar 2022 23:12:09 +0800	[thread overview]
Message-ID: <YjiVySElUQZnJG9C@google.com> (raw)
In-Reply-To: <20220321072312.14972-3-trevor.wu@mediatek.com>

On Mon, Mar 21, 2022 at 03:23:08PM +0800, Trevor Wu wrote:
> -config SND_SOC_MT8195_MT6359_RT1019_RT5682
> -	tristate "ASoC Audio driver for MT8195 with MT6359 RT1019 RT5682 codec"
> -	depends on I2C && GPIOLIB
> -	depends on SND_SOC_MT8195 && MTK_PMIC_WRAP
> -	select SND_SOC_MT6359
> -	select SND_SOC_RT1015P
> -	select SND_SOC_RT5682_I2C
> -	select SND_SOC_RT5682S
> -	select SND_SOC_DMIC
> -	select SND_SOC_HDMI_CODEC
> -	help
> -	  This adds ASoC driver for Mediatek MT8195 boards
> -	  with the MT6359 RT1019 RT5682 audio codec.
> -	  Select Y if you have such device.
> -	  If unsure select "N".
> -
> -config SND_SOC_MT8195_MT6359_RT1011_RT5682
> -	tristate "ASoC Audio driver for MT8195 with MT6359 RT1011 RT5682 codec"
> +config SND_SOC_MT8195_MT6359
> +	tristate "ASoC Audio driver for MT8195 with MT6359 and I2S codec"

s/codec/codecs/.

>  	help
> -	  This adds ASoC driver for Mediatek MT8195 boards
> -	  with the MT6359 RT1011 RT5682 audio codec.
> +	  This adds support for ASoC machine driver for Mediatek MT8195 
> +	  boards with the MT6359 and other I2S audio codec.

s/codec/codecs/.

> diff --git a/sound/soc/mediatek/mt8195/mt8195-mt6359-rt1019-rt5682.c b/sound/soc/mediatek/mt8195/mt8195-mt6359.c
[...]
> +static int mt8195_rt1011_etdm_hw_params(struct snd_pcm_substream *substream,
> +					struct snd_pcm_hw_params *params)
> +{
> +	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
> +	struct snd_soc_dai *codec_dai;
> +	struct snd_soc_card *card = rtd->card;
> +	int srate, i, ret = 0;
> +
> +	srate = params_rate(params);
> +
> +	for_each_rtd_codec_dais(rtd, i, codec_dai) {
> +		ret = snd_soc_dai_set_pll(codec_dai, 0, RT1011_PLL1_S_BCLK,
> +					  64 * srate, 256 * srate);
> +		if (ret < 0) {
> +			dev_err(card->dev, "codec_dai clock not set\n");
> +			return ret;
> +		}
> +
> +		ret = snd_soc_dai_set_sysclk(codec_dai,
> +					     RT1011_FS_SYS_PRE_S_PLL1,
> +					     256 * srate, SND_SOC_CLOCK_IN);
> +		if (ret < 0) {
> +			dev_err(card->dev, "codec_dai clock not set\n");
> +			return ret;
> +		}
> +	}
> +	return ret;

I guess it may be just copied from mt8195-mt6359-rt1011-rt5682.c.  However, it
is good to turn the code into a better shape.  To be clear, just return 0 here
so that the initialization can be removed.

> +static int mt8195_rt1011_init(struct snd_soc_pcm_runtime *rtd)
[...]
> +	ret = snd_soc_dapm_add_routes(&card->dapm, mt8195_rt1011_routes,
> +				      ARRAY_SIZE(mt8195_rt1011_routes));
> +
> +	if (ret)

Remove the extra blank line.

> +static int mt8195_rt1019_init(struct snd_soc_pcm_runtime *rtd)
[...]
> +	ret = snd_soc_dapm_add_routes(&card->dapm, mt8195_rt1019_routes,
> +				      ARRAY_SIZE(mt8195_rt1019_routes));
> +
> +	if (ret)

Remove the extra blank line.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel