linux-kernel - Re: [PATCH v3 2/6] ASoC: mediatek: mt8195: merge machine driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1d3f9ddc742c8af2e4613e6e1008b4a63b33e244.camel@mediatek.com>
Date:   Wed, 23 Mar 2022 16:15:30 +0800
From:   Trevor Wu <trevor.wu@...iatek.com>
To:     Tzung-Bi Shih <tzungbi@...nel.org>
CC:     <broonie@...nel.org>, <tiwai@...e.com>, <robh+dt@...nel.org>,
        <matthias.bgg@...il.com>, <alsa-devel@...a-project.org>,
        <linux-mediatek@...ts.infradead.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <aaronyu@...gle.com>, <angelogioacchino.delregno@...labora.com>,
        <yc.hung@...iatek.com>
Subject: Re: [PATCH v3 2/6] ASoC: mediatek: mt8195: merge machine driver

On Mon, 2022-03-21 at 23:12 +0800, Tzung-Bi Shih wrote:
> 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/.

OK.
> 
> >  	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/.
OK.

> 
> > 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.
> 
Yes, I copied it directly.
I will update this in v4.

> > +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.

OK. I didn't notice the 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.

OK.

Thanks,
Trevor