linux-kernel - Re: [PATCH 3/4] ASoC: codecs: tas5720: add TAS5722 specific volume control

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <ada57df8-66c0-0692-9250-422ef37cce95@ti.com>
Date:   Mon, 15 Jan 2018 08:50:09 -0600
From:   "Andrew F. Davis" <afd@...com>
To:     Mark Brown <broonie@...nel.org>
CC:     Liam Girdwood <lgirdwood@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        <alsa-devel@...a-project.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/4] ASoC: codecs: tas5720: add TAS5722 specific volume
 control

On 12/12/2017 06:01 AM, Mark Brown wrote:
> On Mon, Dec 11, 2017 at 01:01:56PM -0600, Andrew F. Davis wrote:
> 
>> The TAS5722 supports modifying volume in 0.25dB steps (as opposed to 0.5dB
>> steps on the TAS5720). Introduce a custom mixer control that allows taking
>> advantage of this finer output volume granularity.
> 
> Don't do this, it's just making things more complicated.  Instead do
> what other drivers do and register different sets of controls depending
> on which part you're working with.  The normal thing is to have a big
> table for all the shared controls that are the same on all variants then
> register additional tables during probe with those that vary for the
> individul devices.
> 

That is what we are doing here, the reason for the custom mixer control
is that the controlled bits span two registers in a odd way that is not
supported by the standard handlers.

>>  static const struct snd_kcontrol_new tas5720_snd_controls[] = {
>>  	SOC_SINGLE_TLV("Speaker Driver Playback Volume",
>> -		       TAS5720_VOLUME_CTRL_REG, 0, 0xff, 0, dac_tlv),
>> +		       TAS5720_VOLUME_CTRL_REG, 0, 0xff, 0, tas5720_dac_tlv),
>> +	SOC_SINGLE_TLV("Speaker Driver Analog Gain", TAS5720_ANALOG_CTRL_REG,
>> +		       TAS5720_ANALOG_GAIN_SHIFT, 3, 0, dac_analog_tlv),
> 
> As ever all volume controls should end in Volume (like the immediately
> adjacent control does).
> 

This was done so this table exactly matches the existing table. If you
would like me to change this then I can, and can do it for the other
table as well, up to you.