Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752785AbbLMU6p (ORCPT ); Sun, 13 Dec 2015 15:58:45 -0500 Received: from down.free-electrons.com ([37.187.137.238]:60623 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752574AbbLMU6n (ORCPT ); Sun, 13 Dec 2015 15:58:43 -0500 Date: Sun, 13 Dec 2015 21:58:39 +0100 From: Maxime Ripard To: Danny Milosavljevic Cc: linux-sunxi@googlegroups.com, Liam Girdwood , Mark Brown , Jaroslav Kysela , Takashi Iwai , Chen-Yu Tsai , alsa-devel@alsa-project.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs Message-ID: <20151213205839.GA19456@lukather> References: <20151209235530.7240df0a@dayas> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="6c2NcOVqGQ03X4Wi" Content-Disposition: inline In-Reply-To: <20151209235530.7240df0a@dayas> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 14622 Lines: 402 --6c2NcOVqGQ03X4Wi Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi, On Wed, Dec 09, 2015 at 11:55:30PM +0100, Danny Milosavljevic wrote: > Add inputs to sun4i-codec: > - FM-in Left and Right > - Line-in Left and Right > - Mic1-in > - Mic2-in >=20 > Signed-off-by: Danny Milosavljevic > Tested-by: Danny Milosavljevic (ONLY ON A20!) We'd expect you to have tested it. You can drop this tag. However, it would indeed be nice to have someone with an A10 testing it. Chen-Yu? > --- > Hi, >=20 > this is the sixth version of the patch that adds inputs to sun4i-codec. >=20 > Changes compared to v5 are: > - Mic preamplifier special cases for A20 and A10 now are now not icky: > There are two different _widget arrays and the probe() function now se= lects=20 > the right one to pass to snd_soc_register_codec() unmodified. > - sun7i-specific things (A20-specific things) are now grouped together. >=20 > I successfully tested it again on an A20 board using alsamixer, headphone= s,=20 > a radio and my ears. > Note that because of missing capturing support I tested only the mixing, > for Mic, Line, and FM. >=20 > The patches are on top of > , > branch "sunxi/for-next". This is not the branch you should be basing your patch on. This is an ASoC patch, base it on the ASoC tree. > Regards, > Danny > --- > b/sound/soc/sunxi/sun4i-codec.c | 179 +++++++++++++++++++++++++++++++++= ++++--- > 1 file changed, 166 insertions(+), 13 deletions(-) >=20 > diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c > index bcbf4da..4241999 100644 > --- a/sound/soc/sunxi/sun4i-codec.c > +++ b/sound/soc/sunxi/sun4i-codec.c > @@ -57,9 +57,20 @@ > #define SUN4I_CODEC_DAC_ACTL_DACAENR (31) > #define SUN4I_CODEC_DAC_ACTL_DACAENL (30) > #define SUN4I_CODEC_DAC_ACTL_MIXEN (29) > +#define SUN4I_CODEC_DAC_ACTL_LNG (26) > +#define SUN4I_CODEC_DAC_ACTL_FMG (23) > +#define SUN4I_CODEC_DAC_ACTL_MICG (20) > +#define SUN4I_CODEC_DAC_ACTL_LLNS (19) > +#define SUN4I_CODEC_DAC_ACTL_RLNS (18) > +#define SUN4I_CODEC_DAC_ACTL_LFMS (17) > +#define SUN4I_CODEC_DAC_ACTL_RFMS (16) > #define SUN4I_CODEC_DAC_ACTL_LDACLMIXS (15) > #define SUN4I_CODEC_DAC_ACTL_RDACRMIXS (14) > #define SUN4I_CODEC_DAC_ACTL_LDACRMIXS (13) > +#define SUN4I_CODEC_DAC_ACTL_MIC1LS (12) > +#define SUN4I_CODEC_DAC_ACTL_MIC1RS (11) > +#define SUN4I_CODEC_DAC_ACTL_MIC2LS (10) > +#define SUN4I_CODEC_DAC_ACTL_MIC2RS (9) > #define SUN4I_CODEC_DAC_ACTL_DACPAS (8) > #define SUN4I_CODEC_DAC_ACTL_MIXPAS (7) > #define SUN4I_CODEC_DAC_ACTL_PA_MUTE (6) > @@ -84,8 +95,11 @@ > #define SUN4I_CODEC_ADC_ACTL_PREG1EN (29) > #define SUN4I_CODEC_ADC_ACTL_PREG2EN (28) > #define SUN4I_CODEC_ADC_ACTL_VMICEN (27) > +#define SUN4I_CODEC_ADC_ACTL_PREG1_A10 (25) > +#define SUN4I_CODEC_ADC_ACTL_PREG2_A10 (23) > #define SUN4I_CODEC_ADC_ACTL_VADCG (20) > #define SUN4I_CODEC_ADC_ACTL_ADCIS (17) > +#define SUN4I_CODEC_ADC_ACTL_LNRDF (16) > #define SUN4I_CODEC_ADC_ACTL_PA_EN (4) > #define SUN4I_CODEC_ADC_ACTL_DDE (3) > #define SUN4I_CODEC_ADC_DEBUG (0x2c) > @@ -94,7 +108,6 @@ > #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) > =20 > struct sun4i_codec { > struct device *dev; > @@ -402,16 +415,77 @@ static const struct snd_kcontrol_new sun4i_codec_pa= _mute =3D > SUN4I_CODEC_DAC_ACTL_PA_MUTE, 1, 0); > =20 > static DECLARE_TLV_DB_SCALE(sun4i_codec_pa_volume_scale, -6300, 100, 1); > +static DECLARE_TLV_DB_SCALE(sun4i_codec_linein_loopback_gain_scale, -150= , 150, 0); > +static DECLARE_TLV_DB_SCALE(sun4i_codec_fmin_loopback_gain_scale, -450, = 150, 0); > +static DECLARE_TLV_DB_SCALE(sun4i_codec_micin_loopback_gain_scale, -450,= 150, 0); > +static DECLARE_TLV_DB_RANGE(sun4i_codec_micin_preamp_gain_scale_a10, > + 0, 0, TLV_DB_SCALE_ITEM(0, 0, 0), > + 1, 7, TLV_DB_SCALE_ITEM(3500, 300, 0) > +); > + > +#define SUN4I_COMMON_CODEC_WIDGETS \ > + SOC_SINGLE_TLV("PA Volume", SUN4I_CODEC_DAC_ACTL, \ > + SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0, \ > + sun4i_codec_pa_volume_scale), \ > + /* Line-In, FM-In, Mic1-In, Mic2-In */ \ > + SOC_SINGLE_TLV("Line-In Playback Gain", \ > + SUN4I_CODEC_DAC_ACTL, \ > + SUN4I_CODEC_DAC_ACTL_LNG, \ > + 1, \ > + 0, \ > + sun4i_codec_linein_loopback_gain_scale), \ > + SOC_SINGLE_TLV("FM-In Playback Gain", \ > + SUN4I_CODEC_DAC_ACTL, \ > + SUN4I_CODEC_DAC_ACTL_FMG, \ > + 3, \ > + 0, \ > + sun4i_codec_fmin_loopback_gain_scale), \ > + SOC_SINGLE_TLV("Mic-In Playback Gain", \ > + SUN4I_CODEC_DAC_ACTL, \ > + SUN4I_CODEC_DAC_ACTL_MICG, \ > + 7, \ > + 0, \ > + sun4i_codec_micin_loopback_gain_scale), \ > + SOC_SINGLE_TLV("Mic1-In Boost Switch", \ > + SUN4I_CODEC_ADC_ACTL, \ > + SUN4I_CODEC_ADC_ACTL_PREG1EN, \ > + 1, \ > + 0, \ > + NULL), \ > + SOC_SINGLE_TLV("Mic2-In Boost Switch", \ > + SUN4I_CODEC_ADC_ACTL, \ > + SUN4I_CODEC_ADC_ACTL_PREG2EN, \ > + 1, \ > + 0, \ > + NULL) You have a bunch of checkpatch errors and warnings, make sure you fix them. > static const struct snd_kcontrol_new sun4i_codec_widgets[] =3D { > - SOC_SINGLE_TLV("PA Volume", SUN4I_CODEC_DAC_ACTL, > - SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0, > - sun4i_codec_pa_volume_scale), > + SUN4I_COMMON_CODEC_WIDGETS, > + SOC_SINGLE_TLV("Mic1-In Boost Gain", > + SUN4I_CODEC_ADC_ACTL, > + SUN4I_CODEC_ADC_ACTL_PREG1_A10, > + 3, > + 0, > + sun4i_codec_micin_preamp_gain_scale_a10), > + SOC_SINGLE_TLV("Mic2-In Boost Gain", > + SUN4I_CODEC_ADC_ACTL, > + SUN4I_CODEC_ADC_ACTL_PREG2_A10, > + 3, > + 0, > + sun4i_codec_micin_preamp_gain_scale_a10), > }; > =20 > static const struct snd_kcontrol_new sun4i_codec_left_mixer_controls[] = =3D { > SOC_DAPM_SINGLE("Left DAC Playback Switch", SUN4I_CODEC_DAC_ACTL, > SUN4I_CODEC_DAC_ACTL_LDACLMIXS, 1, 0), > + SOC_DAPM_SINGLE("Left Line-In Playback Switch", SUN4I_CODEC_DAC_ACTL, > + SUN4I_CODEC_DAC_ACTL_LLNS, 1, 0), > + SOC_DAPM_SINGLE("Left FM-In Playback Switch", SUN4I_CODEC_DAC_ACTL, > + SUN4I_CODEC_DAC_ACTL_LFMS, 1, 0), > + SOC_DAPM_SINGLE("Mic1-In Playback Switch", SUN4I_CODEC_DAC_ACTL, > + SUN4I_CODEC_DAC_ACTL_MIC1LS, 1, 0), > + SOC_DAPM_SINGLE("Mic2-In Playback Switch", SUN4I_CODEC_DAC_ACTL, > + SUN4I_CODEC_DAC_ACTL_MIC2LS, 1, 0), > }; > =20 > static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = =3D { > @@ -419,6 +493,14 @@ static const struct snd_kcontrol_new sun4i_codec_rig= ht_mixer_controls[] =3D { > SUN4I_CODEC_DAC_ACTL_RDACRMIXS, 1, 0), > SOC_DAPM_SINGLE("Left DAC Playback Switch", SUN4I_CODEC_DAC_ACTL, > SUN4I_CODEC_DAC_ACTL_LDACRMIXS, 1, 0), > + SOC_DAPM_SINGLE("Right Line-In Playback Switch", SUN4I_CODEC_DAC_ACTL, > + SUN4I_CODEC_DAC_ACTL_RLNS, 1, 0), > + SOC_DAPM_SINGLE("Right FM-In Playback Switch", SUN4I_CODEC_DAC_ACTL, > + SUN4I_CODEC_DAC_ACTL_RFMS, 1, 0), > + SOC_DAPM_SINGLE("Mic1-In Playback Switch", SUN4I_CODEC_DAC_ACTL, > + SUN4I_CODEC_DAC_ACTL_MIC1RS, 1, 0), > + SOC_DAPM_SINGLE("Mic2-In Playback Switch", SUN4I_CODEC_DAC_ACTL, > + SUN4I_CODEC_DAC_ACTL_MIC2RS, 1, 0), > }; > =20 > static const struct snd_kcontrol_new sun4i_codec_pa_mixer_controls[] =3D= { > @@ -462,8 +544,16 @@ static const struct snd_soc_dapm_widget sun4i_codec_= dapm_widgets[] =3D { > =20 > SND_SOC_DAPM_OUTPUT("HP Right"), > SND_SOC_DAPM_OUTPUT("HP Left"), > + > + SND_SOC_DAPM_INPUT("Line-In Right"), > + SND_SOC_DAPM_INPUT("Line-In Left"), > + SND_SOC_DAPM_INPUT("FM-In Right"), > + SND_SOC_DAPM_INPUT("FM-In Left"), > + SND_SOC_DAPM_INPUT("Mic1-In"), > + SND_SOC_DAPM_INPUT("Mic2-In"), > }; > =20 > +/* {sink, control, source} */ > static const struct snd_soc_dapm_route sun4i_codec_dapm_routes[] =3D { > /* Left DAC Routes */ > { "Left DAC", NULL, "DAC" }, > @@ -490,9 +580,23 @@ static const struct snd_soc_dapm_route sun4i_codec_d= apm_routes[] =3D { > { "Pre-Amplifier Mute", "Switch", "Pre-Amplifier" }, > { "HP Right", NULL, "Pre-Amplifier Mute" }, > { "HP Left", NULL, "Pre-Amplifier Mute" }, > + > + /* Line-In, FM-In */ > + { "Right Mixer", "Right Line-In Playback Switch", "Line-In Right" }, > + { "Left Mixer", "Left Line-In Playback Switch", "Line-In Left" }, > + { "Right Mixer", "Right FM-In Playback Switch", "FM-In Right" }, > + { "Left Mixer", "Left FM-In Playback Switch", "FM-In Left" }, > + > + /* Mic1-In */ > + { "Right Mixer", "Mic1-In Playback Switch", "Mic1-In" }, > + { "Left Mixer", "Mic1-In Playback Switch", "Mic1-In" }, > + > + /* Mic2-In */ > + { "Right Mixer", "Mic2-In Playback Switch", "Mic2-In" }, > + { "Left Mixer", "Mic2-In Playback Switch", "Mic2-In" }, > }; You're doing several things in this patch: - Reworking things to be able to have different widgets and routes between the compatibles - Add new controls. Please make two separate patches. > -static struct snd_soc_codec_driver sun4i_codec_codec =3D { > +static const struct snd_soc_codec_driver sun4i_codec_codec =3D { And this belongs in a separate patch too. > .controls =3D sun4i_codec_widgets, > .num_controls =3D ARRAY_SIZE(sun4i_codec_widgets), > .dapm_widgets =3D sun4i_codec_dapm_widgets, > @@ -533,13 +637,6 @@ static struct snd_soc_dai_driver dummy_cpu_dai =3D { > }, > }; > =20 > -static const struct regmap_config sun4i_codec_regmap_config =3D { > - .reg_bits =3D 32, > - .reg_stride =3D 4, > - .val_bits =3D 32, > - .max_register =3D SUN4I_CODEC_AC_MIC_PHONE_CAL, > -}; > - Why is this moved? > static const struct of_device_id sun4i_codec_of_match[] =3D { > { .compatible =3D "allwinner,sun4i-a10-codec" }, > { .compatible =3D "allwinner,sun7i-a20-codec" }, > @@ -586,6 +683,56 @@ static struct snd_soc_card *sun4i_codec_create_card(= struct device *dev) > return card; > }; > =20 > +/* sun7i-specific things: */ > +/* MIC_PHONE_CAL register offsets and bit fields (A20 only) */ > +#define SUN7I_CODEC_AC_MIC_PHONE_CAL (0x3c) > +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG1 (29) > +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG2 (26) > +/* note: no idea where the output pins for the following are. */ > +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTG (5) > +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTEN (4) > +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTS3 (3) > +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTS2 (2) > +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTS1 (1) > +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTS0 (0) > + > +static DECLARE_TLV_DB_RANGE(sun7i_codec_micin_preamp_gain_scale, > + 0, 0, TLV_DB_SCALE_ITEM(0, 0, 0), > + 1, 7, TLV_DB_SCALE_ITEM(2400, 300, 0) > +); > + > +static const struct snd_kcontrol_new sun7i_codec_widgets[] =3D { > + SUN4I_COMMON_CODEC_WIDGETS, > + SOC_SINGLE_TLV("Mic1-In Boost Gain", > + SUN7I_CODEC_AC_MIC_PHONE_CAL, > + SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG1, > + 7, > + 0, > + sun7i_codec_micin_preamp_gain_scale), > + SOC_SINGLE_TLV("Mic2-In Boost Gain", > + SUN7I_CODEC_AC_MIC_PHONE_CAL, > + SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG2, > + 7, > + 0, > + sun7i_codec_micin_preamp_gain_scale), > +}; > + > +static const struct snd_soc_codec_driver sun7i_codec_codec =3D { > + .controls =3D sun7i_codec_widgets, > + .num_controls =3D ARRAY_SIZE(sun7i_codec_widgets), > + .dapm_widgets =3D sun4i_codec_dapm_widgets, > + .num_dapm_widgets =3D ARRAY_SIZE(sun4i_codec_dapm_widgets), > + .dapm_routes =3D sun4i_codec_dapm_routes, > + .num_dapm_routes =3D ARRAY_SIZE(sun4i_codec_dapm_routes), > +}; > +static const struct regmap_config sun4i_codec_regmap_config =3D { > + .reg_bits =3D 32, > + .reg_stride =3D 4, > + .val_bits =3D 32, > + .max_register =3D SUN7I_CODEC_AC_MIC_PHONE_CAL, > +}; > +/* end sun7i-specific things */ > + > static int sun4i_codec_probe(struct platform_device *pdev) > { > struct snd_soc_card *card; > @@ -593,6 +740,7 @@ static int sun4i_codec_probe(struct platform_device *= pdev) > struct resource *res; > void __iomem *base; > int ret; > + const struct snd_soc_codec_driver* codec_codec; I guess a single codec is enough :) > =20 > scodec =3D devm_kzalloc(&pdev->dev, sizeof(*scodec), GFP_KERNEL); > if (!scodec) > @@ -638,7 +786,12 @@ static int sun4i_codec_probe(struct platform_device = *pdev) > scodec->playback_dma_data.maxburst =3D 4; > scodec->playback_dma_data.addr_width =3D DMA_SLAVE_BUSWIDTH_2_BYTES; > =20 > - ret =3D snd_soc_register_codec(&pdev->dev, &sun4i_codec_codec, > + if (of_device_is_compatible(pdev->dev.of_node,=20 > + "allwinner,sun7i-a20-codec")) > + codec_codec =3D &sun7i_codec_codec; > + else > + codec_codec =3D &sun4i_codec_codec; > + ret =3D snd_soc_register_codec(&pdev->dev, codec_codec, > &sun4i_codec_dai, 1); > if (ret) { > dev_err(&pdev->dev, "Failed to register our codec\n"); Thanks! Maxime --=20 Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com --6c2NcOVqGQ03X4Wi Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWbdv/AAoJEBx+YmzsjxAgG1MQAI3iPVOoBcU/neDol3uEvL52 0RLtJcPI68LVGCoyULMQVeB9Ub/OANtJMWhRqOFaBz1AOJv5qPNI2yfTUeLyX7iI nfAjP6clf/7NgtXXGOyx04ArSt7zXiuwxBPD//S9WsUk/Ala9apeHmc28l8W0lqE QEHze3M8oA129T3iTW+1dx+mOsadsK9p2QIOCHtgq/9dbTy/39tSaFc95ZbuKG0U 2FP3g+7FNjIO8izf46HJ4WoJ3qvwjc7BkMW5iPXHhcRKHK0fXqmETS181RalJ8+x YwD8EMwuOtdpyh9aJ9cqz1HqoDRiLsvtpQBYp5V/fGnJhJX8gkyXez787vkS1FtD DDfywcxvzHGEQD/wj35TkwICLG64Lp+d3l5DgGB6yETJrw1wk55nyRcOgpGdwqlA 17pyXxRrOoT2VNux+SCSmK0IxJNaiQq2ryWiEL/Qfbvs1WVZ9E1rQAafJn5lnufz 635b3mkIOgA9PuRITMcj/VTrjtvw1x4FMCbOIgbJY8MNWG8ebNfyqC3q0hsRSUDC Yd8fVaIBN3V6v1DXJ6tbJ1FqODd2uOnlDAvcd2RrWWOa25cYMlz/yiEfY6G6SSYU lJkC/oCryITQZKHVj1fGac54Mbo/xTMlH0KI7t10LNDcJIBY3SCzYhT2nzRfl6bw sU/tj6S3bQ3k68BY8GU6 =mMsZ -----END PGP SIGNATURE----- --6c2NcOVqGQ03X4Wi-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/