Re: [PATCH v2 1/5] ASoC: add mt6351 codec driver

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1544 bytes --]

On Sat, Apr 21, 2018 at 07:49:24AM +0800, KaiChieh Chuang wrote:

> > These gains should be put in TLVs rather than an enum so userspace can
> > handle them (also it should be dB not Db).  You can handle irregular
> > step sizes like these with DECLARE_TLV_DB_SCALE(), there's several
> > examples in the code already.

> We declare this enum for user space to operate on enum directly,
> unlike alsa-lib, we use tinyalsa in Android, if declared in tlv,
> the mixer control just become a integer control, which is not
> informative for user space.

Your custom userspace isn't really a factor for upstream.

> > > +	if (ucontrol->value.integer.value[0] == 0x1f)	/* reg idx for -40dB*/
> > > +		ucontrol->value.integer.value[0] = ARRAY_SIZE(dl_pga_gain) - 1;

> > Why do this rewriting?

> A little awkward gain register here,
> the range is  for 8dB ~ -10dB and -40dB, corresponding to reg index 0 ~
> 18 and 31.

> 00000: +8dB.
> 00001: +7dB.
> 10010: -10dB
> 11111: -40dB(Mute).

> and where 19 ~ 30 is not defined, and shouldn't be set.

> i think tlv not yet support this kind of gain control?

You can say there's a mute value, but gaps like that aren't easily
handled AFAIR.  You could assign vales to them (I'm guessing there
probably are some that just aren't doucmented for some reason) or...

> Maybe i'll just declare 8dB ~ -10dB for now.

...this is also common, the mixed mute and gain controls aren't always
super useful.  Depending on what goes on with pops and clicks a DAPM
autodisable control might be a good application too.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]