Re: [PATCH] ASoC: sun4i-codec: Distinguish sun4i from sun7i

[Mark Brown]

On Mon, Sep 05, 2016 at 09:24:04PM +0200, Danny Milosavljevic wrote:

> -static const struct snd_kcontrol_new sun4i_codec_widgets[] = {
> - SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,
> -SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,
> -sun4i_codec_pa_volume_scale),
> +#define SUN4I_COMMON_CODEC_CONTROLS \
> + SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,\
> +SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,\
> +sun4i_codec_pa_volume_scale)
> +
> +static const struct snd_kcontrol_new sun4i_codec_controls[] = {
> + SUN4I_COMMON_CODEC_CONTROLS,
> +};

No, please don't do this - macros like this just make the code harder to
work with.  Do what everything else that handles multiple devices in one
driver does and have several different tables of controls, registering
the ones that are appropriate for the current device at runtime.


signature.asc
Description: PGP signature

Re: [PATCH] ASoC: sun4i-codec: Distinguish sun4i from sun7i

[Mark Brown]

On Mon, Sep 05, 2016 at 09:24:04PM +0200, Danny Milosavljevic wrote:

> -static const struct snd_kcontrol_new sun4i_codec_widgets[] = {
> - SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,
> -SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,
> -sun4i_codec_pa_volume_scale),
> +#define SUN4I_COMMON_CODEC_CONTROLS \
> + SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,\
> +SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,\
> +sun4i_codec_pa_volume_scale)
> +
> +static const struct snd_kcontrol_new sun4i_codec_controls[] = {
> + SUN4I_COMMON_CODEC_CONTROLS,
> +};

No, please don't do this - macros like this just make the code harder to
work with.  Do what everything else that handles multiple devices in one
driver does and have several different tables of controls, registering
the ones that are appropriate for the current device at runtime.


signature.asc
Description: PGP signature

Re: [PATCH] ASoC: sun4i-codec: Distinguish sun4i from sun7i

[Maxime Ripard]

Hi,

On Mon, Sep 05, 2016 at 09:24:04PM +0200, Danny Milosavljevic wrote:
> ASoC: sun4i-codec: Distinguish sun4i from sun7i
> 
> Introduce mechanism to detect sun7i and provide a different regmap, codec
> and different controls for it - different compared to sun4i Allwinner A10.
> 
> The controls will be extended in a forthcoming patch - it is necessary to
> distinguish between sun4i and sun7i controls because the mic gains are in
> different registers.
> 
> Renamed SUN4I_CODEC_AC_SYS_VERI to SUN7I_CODEC_AC_DAC_CAL and renamed
> SUN4I_CODEC_AC_MIC_PHONE_CAL to SUN7I_CODEC_AC_MIC_PHONE_CAL because these
> are actually not present on Allwinner A10.
> 
> Handle quirks by regmap config and codec and select the correct quirks
> automatically.
> 
> Signed-off-by: Danny Milosavljevic 
> ---
>  sound/soc/sunxi/sun4i-codec.c | 66 
> +++
>  1 file changed, 54 insertions(+), 12 deletions(-)
> 
> diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> index 0e19c50..8531c93 100644
> --- a/sound/soc/sunxi/sun4i-codec.c
> +++ b/sound/soc/sunxi/sun4i-codec.c
> @@ -96,8 +96,8 @@
>  /* Other various ADC registers */
>  #define SUN4I_CODEC_DAC_TXCNT(0x30)
>  #define SUN4I_CODEC_ADC_RXCNT(0x34)
> -#define SUN4I_CODEC_AC_SYS_VERI  (0x38)
> -#define SUN4I_CODEC_AC_MIC_PHONE_CAL (0x3c)
> +#define SUN7I_CODEC_AC_DAC_CAL   (0x38)
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL (0x3c)
>  
>  struct sun4i_codec {
>   struct device   *dev;
> @@ -509,10 +509,17 @@ static const struct snd_kcontrol_new 
> sun4i_codec_pa_mute =
>  
>  static DECLARE_TLV_DB_SCALE(sun4i_codec_pa_volume_scale, -6300, 100, 1);
>  
> -static const struct snd_kcontrol_new sun4i_codec_widgets[] = {
> - SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,
> -SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,
> -sun4i_codec_pa_volume_scale),
> +#define SUN4I_COMMON_CODEC_CONTROLS \
> + SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,\
> +SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,\
> +sun4i_codec_pa_volume_scale)
> +
> +static const struct snd_kcontrol_new sun4i_codec_controls[] = {
> + SUN4I_COMMON_CODEC_CONTROLS,
> +};
> +
> +static const struct snd_kcontrol_new sun7i_codec_controls[] = {
> + SUN4I_COMMON_CODEC_CONTROLS,
>  };
>  
>  static const struct snd_kcontrol_new sun4i_codec_left_mixer_controls[] = {
> @@ -629,8 +636,19 @@ static const struct snd_soc_dapm_route 
> sun4i_codec_codec_dapm_routes[] = {
>  
>  static struct snd_soc_codec_driver sun4i_codec_codec = {
>   .component_driver = {
> - .controls   = sun4i_codec_widgets,
> - .num_controls   = ARRAY_SIZE(sun4i_codec_widgets),
> + .controls   = sun4i_codec_controls,
> + .num_controls   = ARRAY_SIZE(sun4i_codec_controls),
> + .dapm_widgets   = sun4i_codec_codec_dapm_widgets,
> + .num_dapm_widgets   = 
> ARRAY_SIZE(sun4i_codec_codec_dapm_widgets),
> + .dapm_routes= sun4i_codec_codec_dapm_routes,
> + .num_dapm_routes= 
> ARRAY_SIZE(sun4i_codec_codec_dapm_routes),
> + },
> +};
> +
> +static struct snd_soc_codec_driver sun7i_codec_codec = {
> + .component_driver = {
> + .controls   = sun7i_codec_controls,
> + .num_controls   = ARRAY_SIZE(sun7i_codec_controls),
>   .dapm_widgets   = sun4i_codec_codec_dapm_widgets,
>   .num_dapm_widgets   = 
> ARRAY_SIZE(sun4i_codec_codec_dapm_widgets),
>   .dapm_routes= sun4i_codec_codec_dapm_routes,
> @@ -682,12 +700,34 @@ static const struct regmap_config 
> sun4i_codec_regmap_config = {
>   .reg_bits   = 32,
>   .reg_stride = 4,
>   .val_bits   = 32,
> - .max_register   = SUN4I_CODEC_AC_MIC_PHONE_CAL,
> + .max_register   = SUN4I_CODEC_ADC_RXCNT,
> +};
> +
> +static const struct regmap_config sun7i_codec_regmap_config = {
> + .reg_bits   = 32,
> + .reg_stride = 4,
> + .val_bits   = 32,
> + .max_register   = SUN7I_CODEC_AC_MIC_PHONE_CAL,
> +};
> +
> +struct sun4i_quirks {
> + const struct snd_soc_codec_driver *codec;
> + const struct regmap_config *regmap_config;
> +};
> +
> +static const struct sun4i_quirks sun4i_quirks = {
> + .codec = _codec_codec,
> + .regmap_config = _codec_regmap_config,
> +};
> +
> +static const struct sun4i_quirks sun7i_quirks = {
> + .codec = _codec_codec,
> + .regmap_config = _codec_regmap_config,
>  };
>  
>  static const struct of_device_id sun4i_codec_of_match[] = {
> - { .compatible = "allwinner,sun4i-a10-codec" },
> - { .compatible = "allwinner,sun7i-a20-codec" },
> + { .compatible = 

Re: [PATCH] ASoC: sun4i-codec: Distinguish sun4i from sun7i

[Maxime Ripard]

Hi,

On Mon, Sep 05, 2016 at 09:24:04PM +0200, Danny Milosavljevic wrote:
> ASoC: sun4i-codec: Distinguish sun4i from sun7i
> 
> Introduce mechanism to detect sun7i and provide a different regmap, codec
> and different controls for it - different compared to sun4i Allwinner A10.
> 
> The controls will be extended in a forthcoming patch - it is necessary to
> distinguish between sun4i and sun7i controls because the mic gains are in
> different registers.
> 
> Renamed SUN4I_CODEC_AC_SYS_VERI to SUN7I_CODEC_AC_DAC_CAL and renamed
> SUN4I_CODEC_AC_MIC_PHONE_CAL to SUN7I_CODEC_AC_MIC_PHONE_CAL because these
> are actually not present on Allwinner A10.
> 
> Handle quirks by regmap config and codec and select the correct quirks
> automatically.
> 
> Signed-off-by: Danny Milosavljevic 
> ---
>  sound/soc/sunxi/sun4i-codec.c | 66 
> +++
>  1 file changed, 54 insertions(+), 12 deletions(-)
> 
> diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> index 0e19c50..8531c93 100644
> --- a/sound/soc/sunxi/sun4i-codec.c
> +++ b/sound/soc/sunxi/sun4i-codec.c
> @@ -96,8 +96,8 @@
>  /* Other various ADC registers */
>  #define SUN4I_CODEC_DAC_TXCNT(0x30)
>  #define SUN4I_CODEC_ADC_RXCNT(0x34)
> -#define SUN4I_CODEC_AC_SYS_VERI  (0x38)
> -#define SUN4I_CODEC_AC_MIC_PHONE_CAL (0x3c)
> +#define SUN7I_CODEC_AC_DAC_CAL   (0x38)
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL (0x3c)
>  
>  struct sun4i_codec {
>   struct device   *dev;
> @@ -509,10 +509,17 @@ static const struct snd_kcontrol_new 
> sun4i_codec_pa_mute =
>  
>  static DECLARE_TLV_DB_SCALE(sun4i_codec_pa_volume_scale, -6300, 100, 1);
>  
> -static const struct snd_kcontrol_new sun4i_codec_widgets[] = {
> - SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,
> -SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,
> -sun4i_codec_pa_volume_scale),
> +#define SUN4I_COMMON_CODEC_CONTROLS \
> + SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,\
> +SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,\
> +sun4i_codec_pa_volume_scale)
> +
> +static const struct snd_kcontrol_new sun4i_codec_controls[] = {
> + SUN4I_COMMON_CODEC_CONTROLS,
> +};
> +
> +static const struct snd_kcontrol_new sun7i_codec_controls[] = {
> + SUN4I_COMMON_CODEC_CONTROLS,
>  };
>  
>  static const struct snd_kcontrol_new sun4i_codec_left_mixer_controls[] = {
> @@ -629,8 +636,19 @@ static const struct snd_soc_dapm_route 
> sun4i_codec_codec_dapm_routes[] = {
>  
>  static struct snd_soc_codec_driver sun4i_codec_codec = {
>   .component_driver = {
> - .controls   = sun4i_codec_widgets,
> - .num_controls   = ARRAY_SIZE(sun4i_codec_widgets),
> + .controls   = sun4i_codec_controls,
> + .num_controls   = ARRAY_SIZE(sun4i_codec_controls),
> + .dapm_widgets   = sun4i_codec_codec_dapm_widgets,
> + .num_dapm_widgets   = 
> ARRAY_SIZE(sun4i_codec_codec_dapm_widgets),
> + .dapm_routes= sun4i_codec_codec_dapm_routes,
> + .num_dapm_routes= 
> ARRAY_SIZE(sun4i_codec_codec_dapm_routes),
> + },
> +};
> +
> +static struct snd_soc_codec_driver sun7i_codec_codec = {
> + .component_driver = {
> + .controls   = sun7i_codec_controls,
> + .num_controls   = ARRAY_SIZE(sun7i_codec_controls),
>   .dapm_widgets   = sun4i_codec_codec_dapm_widgets,
>   .num_dapm_widgets   = 
> ARRAY_SIZE(sun4i_codec_codec_dapm_widgets),
>   .dapm_routes= sun4i_codec_codec_dapm_routes,
> @@ -682,12 +700,34 @@ static const struct regmap_config 
> sun4i_codec_regmap_config = {
>   .reg_bits   = 32,
>   .reg_stride = 4,
>   .val_bits   = 32,
> - .max_register   = SUN4I_CODEC_AC_MIC_PHONE_CAL,
> + .max_register   = SUN4I_CODEC_ADC_RXCNT,
> +};
> +
> +static const struct regmap_config sun7i_codec_regmap_config = {
> + .reg_bits   = 32,
> + .reg_stride = 4,
> + .val_bits   = 32,
> + .max_register   = SUN7I_CODEC_AC_MIC_PHONE_CAL,
> +};
> +
> +struct sun4i_quirks {
> + const struct snd_soc_codec_driver *codec;
> + const struct regmap_config *regmap_config;
> +};
> +
> +static const struct sun4i_quirks sun4i_quirks = {
> + .codec = _codec_codec,
> + .regmap_config = _codec_regmap_config,
> +};
> +
> +static const struct sun4i_quirks sun7i_quirks = {
> + .codec = _codec_codec,
> + .regmap_config = _codec_regmap_config,
>  };
>  
>  static const struct of_device_id sun4i_codec_of_match[] = {
> - { .compatible = "allwinner,sun4i-a10-codec" },
> - { .compatible = "allwinner,sun7i-a20-codec" },
> + { .compatible = 

Re: [PATCH] ASoC: sun4i-codec: Distinguish sun4i from sun7i

[Chen-Yu Tsai]

On Tue, Sep 6, 2016 at 3:24 AM, Danny Milosavljevic
 wrote:
> ASoC: sun4i-codec: Distinguish sun4i from sun7i
>
> Introduce mechanism to detect sun7i and provide a different regmap, codec
> and different controls for it - different compared to sun4i Allwinner A10.
>
> The controls will be extended in a forthcoming patch - it is necessary to
> distinguish between sun4i and sun7i controls because the mic gains are in
> different registers.
>
> Renamed SUN4I_CODEC_AC_SYS_VERI to SUN7I_CODEC_AC_DAC_CAL and renamed
> SUN4I_CODEC_AC_MIC_PHONE_CAL to SUN7I_CODEC_AC_MIC_PHONE_CAL because these
> are actually not present on Allwinner A10.
>
> Handle quirks by regmap config and codec and select the correct quirks
> automatically.
>
> Signed-off-by: Danny Milosavljevic 

Acked-by: Chen-Yu Tsai 

Re: [PATCH] ASoC: sun4i-codec: Distinguish sun4i from sun7i

[Chen-Yu Tsai]

On Tue, Sep 6, 2016 at 3:24 AM, Danny Milosavljevic
 wrote:
> ASoC: sun4i-codec: Distinguish sun4i from sun7i
>
> Introduce mechanism to detect sun7i and provide a different regmap, codec
> and different controls for it - different compared to sun4i Allwinner A10.
>
> The controls will be extended in a forthcoming patch - it is necessary to
> distinguish between sun4i and sun7i controls because the mic gains are in
> different registers.
>
> Renamed SUN4I_CODEC_AC_SYS_VERI to SUN7I_CODEC_AC_DAC_CAL and renamed
> SUN4I_CODEC_AC_MIC_PHONE_CAL to SUN7I_CODEC_AC_MIC_PHONE_CAL because these
> are actually not present on Allwinner A10.
>
> Handle quirks by regmap config and codec and select the correct quirks
> automatically.
>
> Signed-off-by: Danny Milosavljevic 

Acked-by: Chen-Yu Tsai 

Re: [PATCH] ASoC: sun4i-codec: Distinguish sun4i from sun7i

[Mark Brown]

On Mon, Sep 05, 2016 at 09:24:03PM +0200, Danny Milosavljevic wrote:
> Here's just the part that allows distinguishing sun4i from sun7i.
> 
> Since I'm waiting for Chen-Yu's DAPM_DOUBLE patch series I figured I'd post 
> the preliminaries in the mean time which will not change and can be applied 
> independently without problems.

Please don't send cover letters for single patches, if there is anything
that needs saying put it in the changelog of the patch or after the ---
if it's administrative stuff.  This reduces mail volume and ensures that 
any important information is recorded in the changelog rather than being
lost. 


signature.asc
Description: PGP signature

Re: [PATCH] ASoC: sun4i-codec: Distinguish sun4i from sun7i

[Mark Brown]

On Mon, Sep 05, 2016 at 09:24:03PM +0200, Danny Milosavljevic wrote:
> Here's just the part that allows distinguishing sun4i from sun7i.
> 
> Since I'm waiting for Chen-Yu's DAPM_DOUBLE patch series I figured I'd post 
> the preliminaries in the mean time which will not change and can be applied 
> independently without problems.

Please don't send cover letters for single patches, if there is anything
that needs saying put it in the changelog of the patch or after the ---
if it's administrative stuff.  This reduces mail volume and ensures that 
any important information is recorded in the changelog rather than being
lost. 


signature.asc
Description: PGP signature