Re: [PATCHv4 2/4] ASoC: codec: cpcap: new codec

[Sebastian Reichel <sebastian.reichel@collabora.co.uk>]

[-- Attachment #1: Type: text/plain, Size: 1498 bytes --]

Hi Mark,

On Fri, Feb 16, 2018 at 11:39:01AM +0000, Mark Brown wrote:
> On Wed, Feb 14, 2018 at 11:07:39PM +0100, Sebastian Reichel wrote:
> 
> > +config SND_SOC_CPCAP
> > +	tristate "Motorola CPCAP codec"
> > +	depends on MFD_CPCAP
> > +	default MFD_CPCAP
> > +
> 
> We don't have default lines like this for other MFDs...

Is there a reason not to add them? It looks very useful to me:

99% of people are not interested in the MFD. They don't enable
it and are not affected at all. They do not even see extra
configuration entry due to the dependency.

0.9% of people have the MFD in their system and want their
hardware to 'just work'. With this line it's enough to enable
the MFD and the audio subsystem, so less knowledge and configuration
is required.

0.1% of people have special requirements and want the MFD
and generic audio support, but no support for the MFD audio
codec. There is a high chance, that they know how to configure
it properly :)

> > +	SOC_ENUM("Hifi Left Phase Playback Switch",
> > +		cpcap_hifi_l_phase_inv_enum),
> > +	SOC_ENUM("Ext Left Phase Playback Switch",
> > +		cpcap_ext_l_phase_inv_enum),
> 
> If this is a Switch control it should be a simple on/off binary control.
> If it's an enumeration it shouldn't have Switch at the end of the name
> since that's a keyword interpreted by userspace.  Either option seems
> fine in this case, if it's a switch it should be "Foo Invert Switch" or
> similar.

Ok.

-- Sebastian

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