[alsa-devel] [EXTERNAL] Re: [PATCH 2/2] ASoC: cs35l36: Add device tree documentation for CS35L36

[Schulman, James]

On Tue, 13 Nov 2018, Mark Brown wrote:

> On Tue, Nov 13, 2018 at 12:49:17PM -0600, James Schulman wrote:
>
>> +  - cirrus,amp-gain-zc : Boolean to determine if to use Amplifier gain-change
>> +  zero-cross
>
> Why is this a DT setting and not either enabled unconditionally or set
> from userspace (many devices do the latter)?  It seems like more of a
> use case dependent setting than a feature of the board.
>
>> +  - cirrus,ldm-mode-select : Amplifier level-dependent muting enable mode selection.
>> +  Configures whether the amplifier level-dependent muting is enabled for the various
>> +  blocks when the selected audio source has been below the AMP_LDM_THLD for a period
>> +  of time configured by AMP_LDM_DELAY. Any and all bits can be set.
>
> Might the user not want to set these at runtime?
>
>> +  - cirrus,pdm-ldm-exit : Boolean : Amplifier PDM mode digital volume ramp enable.
>> +  When AMP_PDM_MODE = 1 controls whether the digital volume soft ramp is enabled
>> +  or disabled when exiting a level-dependent muted condition. The AMP_RAMP_PDM_DN
>> +  control is used to control desired ramp rate.
>
> This also seems like a use case dependent thing.
>

Ok, I agree. These can be use case controls instead of DT settings.

>> +  - cirrus,imon-pol-inv : Boolean to determine Amplifier will invert the
>> +  polarity of outbound IMON feedback data
>> +
>> +  - cirrus,vmon-pol-inv : Boolean to determine Amplifier will invert the
>> +  polarity of outbound VMON feedback data
>
> Why might anyone ever want to set these?
>

They were useful for some algorithm debug at a customer site. They aren't 
used for production.

>> +  - cirrus,pdm-channel-select : PDM Channel select.
>> +  0x00 (Default) : Selects Channel A data when the PDM audio input is selected.
>> +  0x01 : Selects Channel B data when the PDM audio input is selected
>
> This looks like routing control, shouldn't it be in DAPM?
>

Agreed, we'll put this in DAPM.