Re: [PATCHv2 3/6] ASoC: codec: cpcap: new codec

[Mark Brown]

On Mon, Jul 17, 2017 at 04:18:08PM +0200, Sebastian Reichel wrote:
> On Mon, Jul 17, 2017 at 02:29:29AM -0700, Tony Lindgren wrote:

> > Should it also have depends on SND_AUDIO_GRAPH_CARD?

> The codec does not depend on audio graph card. You can
> also use it with (deprecated) simple card or a custom
> written one. In case of a custom written one it would
> make sense to add a dependency from the custom written
> one to SND_SOC_CPCAP. So I think we should not add such
> a dependency.

Right, the dependencies should go in the opposite direction.


signature.asc
Description: PGP signature

Re: [PATCHv2 3/6] ASoC: codec: cpcap: new codec

[Mark Brown]

On Mon, Jul 17, 2017 at 04:18:08PM +0200, Sebastian Reichel wrote:
> On Mon, Jul 17, 2017 at 02:29:29AM -0700, Tony Lindgren wrote:

> > Should it also have depends on SND_AUDIO_GRAPH_CARD?

> The codec does not depend on audio graph card. You can
> also use it with (deprecated) simple card or a custom
> written one. In case of a custom written one it would
> make sense to add a dependency from the custom written
> one to SND_SOC_CPCAP. So I think we should not add such
> a dependency.

Right, the dependencies should go in the opposite direction.


signature.asc
Description: PGP signature

Re: [PATCHv2 3/6] ASoC: codec: cpcap: new codec

[Sebastian Reichel]

Hi,

On Mon, Jul 17, 2017 at 02:29:29AM -0700, Tony Lindgren wrote:
> * Sebastian Reichel  [170712 08:19]:
> > diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> > index 6c78b0b49b81..1615e5acc8fd 100644
> > --- a/sound/soc/codecs/Kconfig
> > +++ b/sound/soc/codecs/Kconfig
> > @@ -401,6 +401,11 @@ config SND_SOC_ALC5632
> >  config SND_SOC_BT_SCO
> > tristate "Dummy BT SCO codec driver"
> >  
> > +config SND_SOC_CPCAP
> > +   tristate "Motorola CPCAP codec"
> > +   depends on MFD_CPCAP
> > +   default MFD_CPCAP
> > +
> 
> Should it also have depends on SND_AUDIO_GRAPH_CARD?

The codec does not depend on audio graph card. You can
also use it with (deprecated) simple card or a custom
written one. In case of a custom written one it would
make sense to add a dependency from the custom written
one to SND_SOC_CPCAP. So I think we should not add such
a dependency.

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCHv2 3/6] ASoC: codec: cpcap: new codec

[Sebastian Reichel]

Hi,

On Mon, Jul 17, 2017 at 02:29:29AM -0700, Tony Lindgren wrote:
> * Sebastian Reichel  [170712 08:19]:
> > diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> > index 6c78b0b49b81..1615e5acc8fd 100644
> > --- a/sound/soc/codecs/Kconfig
> > +++ b/sound/soc/codecs/Kconfig
> > @@ -401,6 +401,11 @@ config SND_SOC_ALC5632
> >  config SND_SOC_BT_SCO
> > tristate "Dummy BT SCO codec driver"
> >  
> > +config SND_SOC_CPCAP
> > +   tristate "Motorola CPCAP codec"
> > +   depends on MFD_CPCAP
> > +   default MFD_CPCAP
> > +
> 
> Should it also have depends on SND_AUDIO_GRAPH_CARD?

The codec does not depend on audio graph card. You can
also use it with (deprecated) simple card or a custom
written one. In case of a custom written one it would
make sense to add a dependency from the custom written
one to SND_SOC_CPCAP. So I think we should not add such
a dependency.

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCHv2 3/6] ASoC: codec: cpcap: new codec

[Tony Lindgren]

* Sebastian Reichel  [170712 08:19]:
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index 6c78b0b49b81..1615e5acc8fd 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -401,6 +401,11 @@ config SND_SOC_ALC5632
>  config SND_SOC_BT_SCO
>   tristate "Dummy BT SCO codec driver"
>  
> +config SND_SOC_CPCAP
> + tristate "Motorola CPCAP codec"
> + depends on MFD_CPCAP
> + default MFD_CPCAP
> +

Should it also have depends on SND_AUDIO_GRAPH_CARD?

Tony

Re: [PATCHv2 3/6] ASoC: codec: cpcap: new codec

[Tony Lindgren]

* Sebastian Reichel  [170712 08:19]:
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index 6c78b0b49b81..1615e5acc8fd 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -401,6 +401,11 @@ config SND_SOC_ALC5632
>  config SND_SOC_BT_SCO
>   tristate "Dummy BT SCO codec driver"
>  
> +config SND_SOC_CPCAP
> + tristate "Motorola CPCAP codec"
> + depends on MFD_CPCAP
> + default MFD_CPCAP
> +

Should it also have depends on SND_AUDIO_GRAPH_CARD?

Tony

Re: [PATCHv2 3/6] ASoC: codec: cpcap: new codec

[Takashi Iwai]

On Wed, 12 Jul 2017 17:18:43 +0200,
Sebastian Reichel wrote:
> 
> +/* Capture Gain Control: 0dB to 31dB in 1dB steps */
> +static DECLARE_TLV_DB_SCALE(mic_gain_tlv, 0, 100, 0);
> +
> +/* Playback Gain Control: -33dB to 12dB in 3dB steps */
> +static DECLARE_TLV_DB_SCALE(vol_tlv, -3300, 300, 0);

Better to set const for TLV declarations here...

> +static const char * const cpcap_phase_invert_texts[] = {
> + "Non Inverted", "Inverted"
> +};
> +static const SOC_ENUM_SINGLE_DECL(cpcap_hifi_l_phase_inv_enum,
> + CPCAP_REG_RXSDOA, CPCAP_BIT_MONO_DAC0, cpcap_phase_invert_texts);
> +static const SOC_ENUM_SINGLE_DECL(cpcap_ext_l_phase_inv_enum,
> + CPCAP_REG_RXEPOA, CPCAP_BIT_MONO_EXT0, cpcap_phase_invert_texts);

... while SOC_ENUM_*_DECL() already contain const, so you should omit
const here.


thanks,

Takashi

Re: [PATCHv2 3/6] ASoC: codec: cpcap: new codec

[Takashi Iwai]

On Wed, 12 Jul 2017 17:18:43 +0200,
Sebastian Reichel wrote:
> 
> +/* Capture Gain Control: 0dB to 31dB in 1dB steps */
> +static DECLARE_TLV_DB_SCALE(mic_gain_tlv, 0, 100, 0);
> +
> +/* Playback Gain Control: -33dB to 12dB in 3dB steps */
> +static DECLARE_TLV_DB_SCALE(vol_tlv, -3300, 300, 0);

Better to set const for TLV declarations here...

> +static const char * const cpcap_phase_invert_texts[] = {
> + "Non Inverted", "Inverted"
> +};
> +static const SOC_ENUM_SINGLE_DECL(cpcap_hifi_l_phase_inv_enum,
> + CPCAP_REG_RXSDOA, CPCAP_BIT_MONO_DAC0, cpcap_phase_invert_texts);
> +static const SOC_ENUM_SINGLE_DECL(cpcap_ext_l_phase_inv_enum,
> + CPCAP_REG_RXEPOA, CPCAP_BIT_MONO_EXT0, cpcap_phase_invert_texts);

... while SOC_ENUM_*_DECL() already contain const, so you should omit
const here.


thanks,

Takashi