From cef305d4eb0733f25215793ed30b056a7db9bb62 Mon Sep 17 00:00:00 2001 From: Samuel Holland Date: Sat, 25 Jul 2020 20:25:51 -0500 Subject: [PATCH 1/4] ASoC: dt-bindings: Add a new compatible for the A64 codec The audio codecs in the A33 and A64 are both integrated variants of the X-Powers AC100 codec. However, there are some differences between them that merit having a separate compatible: - The A64 has a second DRC block, not present in the AC100 or A33. - The A33 has some extra muxing options for AIF1/2/3 in the AIF3_SGP_CTRL register, which are not present in the AC100 or A64. - The A33 is missing registers providing jack detection functionality. - The A33 is claimed to invert LRCK, but this is not seen on A64. Since the driver will continue to work on the A64 using the A33 compatible, albeit without jack detection functionality and with possibly inverted channels, as it does now, allow the A33 compatible to be used as a fallback. Signed-off-by: Samuel Holland Reviewed-by: Rob Herring Link: https://lore.kernel.org/r/20200726012557.38282-2-samuel@sholland.org Signed-off-by: Mark Brown --- .../bindings/sound/allwinner,sun8i-a33-codec.yaml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/sound/allwinner,sun8i-a33-codec.yaml b/Documentation/devicetree/bindings/sound/allwinner,sun8i-a33-codec.yaml index 55d28268d2f4..67405e6d8168 100644 --- a/Documentation/devicetree/bindings/sound/allwinner,sun8i-a33-codec.yaml +++ b/Documentation/devicetree/bindings/sound/allwinner,sun8i-a33-codec.yaml @@ -15,7 +15,11 @@ properties: const: 0 compatible: - const: allwinner,sun8i-a33-codec + oneOf: + - items: + - const: allwinner,sun50i-a64-codec + - const: allwinner,sun8i-a33-codec + - const: allwinner,sun8i-a33-codec reg: maxItems: 1 From 90cac932976e93b17203b4216ba83bdcd68e0ed0 Mon Sep 17 00:00:00 2001 From: Samuel Holland Date: Sat, 25 Jul 2020 20:25:52 -0500 Subject: [PATCH 2/4] ASoC: sun8i-codec: Fix DAPM to match the hardware topology The A33/A64 digital codec has 4 physical inputs and 4 physical outputs: 3 AIFs/DAIs and one ADC/DAC pair. Internal routing is accomplished by a 4-channel mixer connected to each output. The analog and digital sides of the ADC/DAC are in separate ASoC components, so card-level DAPM routes (provided in the device tree) are necessary to connect them together. Currently, these routes are wrong. For AIF1 Playback, the correct topology is: ||<<============ sun8i-codec ===========>>|| || || CPU DAI -> AIF1 DA0 -> DAC Mixer -> DAC (digital) -> DAC (analog) || || but the driver and device trees currently describe: || || CPU DAI -> AIF1 DA0 -------------------------------> DAC (analog) || \--> DAC Mixer -> ??? [dead end] || For AIF1 Capture, there is an additional problem, because the Mixer route is backward. The topology should be: || || ADC (analog) -> ADC (digital) -> AIF1 AD0 Mixer -> AIF1 AD0 -> CPU DAI || || but the driver and device trees currently describe: || || ADC (analog) -> AIF1 AD0 ------------------------------------> CPU DAI || \--> ADC Mixer -> ??? [dead end] || The ADC/DAC are only powered because AIF1 AD0 (capture) has supply routes from the ADC, and AIF1 DA0 (playback) has supply routes from the DAC. However, neither set of supply routes matches the hardware topology. Audio can be routed among AIF1/2/3 without using the ADC or DAC at all; and audio can be routed from the ADC to the DAC without using any AIFs (via the "ADC Digital DAC Playback Switch"). Because the DAPM routes are wrong, both of these use cases are currently broken. This commit adds the necessary widgets and routes to represent the real hardware topology, with functionality equivalent to the current driver. For the existing "allwinner,sun8i-a33-codec" compatible, widgets with the old names are kept as wrappers around the new widgets, so existing device trees will continue to work. For "allwinner,sun50i-a64-codec", the old widgets can be omitted, because no device trees yet use that compatible. Signed-off-by: Samuel Holland Link: https://lore.kernel.org/r/20200726012557.38282-3-samuel@sholland.org Signed-off-by: Mark Brown --- sound/soc/sunxi/sun8i-codec.c | 120 +++++++++++++++++++++++++++------- 1 file changed, 95 insertions(+), 25 deletions(-) diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c index ca51af114419..ffeac150c086 100644 --- a/sound/soc/sunxi/sun8i-codec.c +++ b/sound/soc/sunxi/sun8i-codec.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -85,10 +86,15 @@ #define SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV_MASK GENMASK(8, 6) #define SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV_MASK GENMASK(12, 9) +struct sun8i_codec_quirks { + bool legacy_widgets : 1; +}; + struct sun8i_codec { - struct regmap *regmap; - struct clk *clk_module; - struct clk *clk_bus; + struct regmap *regmap; + struct clk *clk_module; + struct clk *clk_bus; + const struct sun8i_codec_quirks *quirks; }; static int sun8i_codec_runtime_resume(struct device *dev) @@ -388,22 +394,30 @@ static const struct snd_soc_dapm_widget sun8i_codec_dapm_widgets[] = { SND_SOC_DAPM_SUPPLY("ADC", SUN8I_ADC_DIG_CTRL, SUN8I_ADC_DIG_CTRL_ENDA, 0, NULL, 0), - /* Analog DAC AIF */ - SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Left", "Playback", 0, + /* AIF "DAC" Inputs */ + SND_SOC_DAPM_AIF_IN("AIF1 DA0L", "Playback", 0, SUN8I_AIF1_DACDAT_CTRL, SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0L_ENA, 0), - SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Right", "Playback", 0, + SND_SOC_DAPM_AIF_IN("AIF1 DA0R", "Playback", 0, SUN8I_AIF1_DACDAT_CTRL, SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0R_ENA, 0), - /* Analog ADC AIF */ - SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Left ADC", "Capture", 0, + /* AIF "ADC" Outputs */ + SND_SOC_DAPM_AIF_IN("AIF1 AD0L", "Capture", 0, SUN8I_AIF1_ADCDAT_CTRL, SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0L_ENA, 0), - SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Right ADC", "Capture", 0, + SND_SOC_DAPM_AIF_IN("AIF1 AD0R", "Capture", 0, SUN8I_AIF1_ADCDAT_CTRL, SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0R_ENA, 0), + /* ADC Inputs (connected to analog codec DAPM context) */ + SND_SOC_DAPM_ADC("ADCL", NULL, SND_SOC_NOPM, 0, 0), + SND_SOC_DAPM_ADC("ADCR", NULL, SND_SOC_NOPM, 0, 0), + + /* DAC Outputs (connected to analog codec DAPM context) */ + SND_SOC_DAPM_DAC("DACL", NULL, SND_SOC_NOPM, 0, 0), + SND_SOC_DAPM_DAC("DACR", NULL, SND_SOC_NOPM, 0, 0), + /* DAC and ADC Mixers */ SOC_MIXER_ARRAY("Left Digital DAC Mixer", SND_SOC_NOPM, 0, 0, sun8i_dac_mixer_controls), @@ -449,40 +463,86 @@ static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = { /* Clock Routes */ { "AIF1", NULL, "SYSCLK AIF1" }, { "AIF1 PLL", NULL, "AIF1" }, - { "RST AIF1", NULL, "AIF1 PLL" }, + { "SYSCLK", NULL, "AIF1 PLL" }, + + { "RST AIF1", NULL, "SYSCLK" }, { "MODCLK AFI1", NULL, "RST AIF1" }, - { "DAC", NULL, "MODCLK AFI1" }, - { "ADC", NULL, "MODCLK AFI1" }, + { "AIF1 AD0L", NULL, "MODCLK AFI1" }, + { "AIF1 AD0R", NULL, "MODCLK AFI1" }, + { "AIF1 DA0L", NULL, "MODCLK AFI1" }, + { "AIF1 DA0R", NULL, "MODCLK AFI1" }, { "RST DAC", NULL, "SYSCLK" }, { "MODCLK DAC", NULL, "RST DAC" }, { "DAC", NULL, "MODCLK DAC" }, + { "DACL", NULL, "DAC" }, + { "DACR", NULL, "DAC" }, { "RST ADC", NULL, "SYSCLK" }, { "MODCLK ADC", NULL, "RST ADC" }, { "ADC", NULL, "MODCLK ADC" }, + { "ADCL", NULL, "ADC" }, + { "ADCR", NULL, "ADC" }, /* DAC Routes */ - { "AIF1 Slot 0 Right", NULL, "DAC" }, - { "AIF1 Slot 0 Left", NULL, "DAC" }, + { "DACL", NULL, "Left Digital DAC Mixer" }, + { "DACR", NULL, "Right Digital DAC Mixer" }, /* DAC Mixer Routes */ - { "Left Digital DAC Mixer", "AIF1 Slot 0 Digital DAC Playback Switch", - "AIF1 Slot 0 Left"}, - { "Right Digital DAC Mixer", "AIF1 Slot 0 Digital DAC Playback Switch", - "AIF1 Slot 0 Right"}, + { "Left Digital DAC Mixer", "AIF1 Slot 0 Digital DAC Playback Switch", "AIF1 DA0L" }, + { "Right Digital DAC Mixer", "AIF1 Slot 0 Digital DAC Playback Switch", "AIF1 DA0R" }, /* ADC Routes */ - { "AIF1 Slot 0 Right ADC", NULL, "ADC" }, - { "AIF1 Slot 0 Left ADC", NULL, "ADC" }, + { "AIF1 AD0L", NULL, "Left Digital ADC Mixer" }, + { "AIF1 AD0R", NULL, "Right Digital ADC Mixer" }, /* ADC Mixer Routes */ - { "Left Digital ADC Mixer", "AIF1 Data Digital ADC Capture Switch", - "AIF1 Slot 0 Left ADC" }, - { "Right Digital ADC Mixer", "AIF1 Data Digital ADC Capture Switch", - "AIF1 Slot 0 Right ADC" }, + { "Left Digital ADC Mixer", "AIF1 Data Digital ADC Capture Switch", "ADCL" }, + { "Right Digital ADC Mixer", "AIF1 Data Digital ADC Capture Switch", "ADCR" }, }; +static const struct snd_soc_dapm_widget sun8i_codec_legacy_widgets[] = { + /* Legacy ADC Inputs (connected to analog codec DAPM context) */ + SND_SOC_DAPM_ADC("AIF1 Slot 0 Left ADC", NULL, SND_SOC_NOPM, 0, 0), + SND_SOC_DAPM_ADC("AIF1 Slot 0 Right ADC", NULL, SND_SOC_NOPM, 0, 0), + + /* Legacy DAC Outputs (connected to analog codec DAPM context) */ + SND_SOC_DAPM_DAC("AIF1 Slot 0 Left", NULL, SND_SOC_NOPM, 0, 0), + SND_SOC_DAPM_DAC("AIF1 Slot 0 Right", NULL, SND_SOC_NOPM, 0, 0), +}; + +static const struct snd_soc_dapm_route sun8i_codec_legacy_routes[] = { + /* Legacy ADC Routes */ + { "ADCL", NULL, "AIF1 Slot 0 Left ADC" }, + { "ADCR", NULL, "AIF1 Slot 0 Right ADC" }, + + /* Legacy DAC Routes */ + { "AIF1 Slot 0 Left", NULL, "DACL" }, + { "AIF1 Slot 0 Right", NULL, "DACR" }, +}; + +static int sun8i_codec_component_probe(struct snd_soc_component *component) +{ + struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component); + struct sun8i_codec *scodec = snd_soc_component_get_drvdata(component); + int ret; + + /* Add widgets for backward compatibility with old device trees. */ + if (scodec->quirks->legacy_widgets) { + ret = snd_soc_dapm_new_controls(dapm, sun8i_codec_legacy_widgets, + ARRAY_SIZE(sun8i_codec_legacy_widgets)); + if (ret) + return ret; + + ret = snd_soc_dapm_add_routes(dapm, sun8i_codec_legacy_routes, + ARRAY_SIZE(sun8i_codec_legacy_routes)); + if (ret) + return ret; + } + + return 0; +} + static const struct snd_soc_dai_ops sun8i_codec_dai_ops = { .hw_params = sun8i_codec_hw_params, .set_fmt = sun8i_set_fmt, @@ -566,6 +626,8 @@ static int sun8i_codec_probe(struct platform_device *pdev) return PTR_ERR(scodec->regmap); } + scodec->quirks = of_device_get_match_data(&pdev->dev); + platform_set_drvdata(pdev, scodec); pm_runtime_enable(&pdev->dev); @@ -603,8 +665,16 @@ static int sun8i_codec_remove(struct platform_device *pdev) return 0; } +static const struct sun8i_codec_quirks sun8i_a33_quirks = { + .legacy_widgets = true, +}; + +static const struct sun8i_codec_quirks sun50i_a64_quirks = { +}; + static const struct of_device_id sun8i_codec_of_match[] = { - { .compatible = "allwinner,sun8i-a33-codec" }, + { .compatible = "allwinner,sun8i-a33-codec", .data = &sun8i_a33_quirks }, + { .compatible = "allwinner,sun50i-a64-codec", .data = &sun50i_a64_quirks }, {} }; MODULE_DEVICE_TABLE(of, sun8i_codec_of_match); From e47d2dcd88fc3e6837f8aa0060ce820ec9001e26 Mon Sep 17 00:00:00 2001 From: Samuel Holland Date: Sat, 25 Jul 2020 20:25:53 -0500 Subject: [PATCH 3/4] ASoC: sun8i-codec: Add missing mixer routes The sun8i-codec driver provides ALSA controls for enabling/disabling each of the inputs to the AIF1 Slot 0 and DAC mixers. For two of these inputs (ADC->DAC and AIF1 DA0->AIF1 AD0), the audio source is implemented, so the mixer inputs can be used. However, because the DAPM routes are missing, these mixer inputs only work when both the source and the mixer happen to be part of other active audio paths. Adding the appropriate routes makes these ALSA controls function all of the time. Signed-off-by: Samuel Holland Link: https://lore.kernel.org/r/20200726012557.38282-4-samuel@sholland.org Signed-off-by: Mark Brown --- sound/soc/sunxi/sun8i-codec.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c index ffeac150c086..a75be9e82d22 100644 --- a/sound/soc/sunxi/sun8i-codec.c +++ b/sound/soc/sunxi/sun8i-codec.c @@ -490,14 +490,20 @@ static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = { /* DAC Mixer Routes */ { "Left Digital DAC Mixer", "AIF1 Slot 0 Digital DAC Playback Switch", "AIF1 DA0L" }, + { "Left Digital DAC Mixer", "ADC Digital DAC Playback Switch", "ADCL" }, + { "Right Digital DAC Mixer", "AIF1 Slot 0 Digital DAC Playback Switch", "AIF1 DA0R" }, + { "Right Digital DAC Mixer", "ADC Digital DAC Playback Switch", "ADCR" }, /* ADC Routes */ { "AIF1 AD0L", NULL, "Left Digital ADC Mixer" }, { "AIF1 AD0R", NULL, "Right Digital ADC Mixer" }, /* ADC Mixer Routes */ + { "Left Digital ADC Mixer", "AIF1 Slot 0 Digital ADC Capture Switch", "AIF1 DA0L" }, { "Left Digital ADC Mixer", "AIF1 Data Digital ADC Capture Switch", "ADCL" }, + + { "Right Digital ADC Mixer", "AIF1 Slot 0 Digital ADC Capture Switch", "AIF1 DA0R" }, { "Right Digital ADC Mixer", "AIF1 Data Digital ADC Capture Switch", "ADCR" }, }; From 7518805fb636308909a6a7953e9fdb194abb15f8 Mon Sep 17 00:00:00 2001 From: Samuel Holland Date: Sat, 25 Jul 2020 20:25:54 -0500 Subject: [PATCH 4/4] ASoC: sun8i-codec: Add a quirk for LRCK inversion On the A64, as tested using the PinePhone, the current code causes the left/right channels to be swapped during I2S playback from the CPU on AIF1, and breaks DSP_A communication with the modem on AIF2. Both of these are fixed when LRCK is no longer inverted. Trusting that the comment in the code is correct, the existing behavior is kept for the A33. Signed-off-by: Samuel Holland Link: https://lore.kernel.org/r/20200726012557.38282-5-samuel@sholland.org Signed-off-by: Mark Brown --- sound/soc/sunxi/sun8i-codec.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c index a75be9e82d22..304683a71acd 100644 --- a/sound/soc/sunxi/sun8i-codec.c +++ b/sound/soc/sunxi/sun8i-codec.c @@ -88,6 +88,7 @@ struct sun8i_codec_quirks { bool legacy_widgets : 1; + bool lrck_inversion : 1; }; struct sun8i_codec { @@ -215,18 +216,19 @@ static int sun8i_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) value << SUN8I_AIF1CLK_CTRL_AIF1_BCLK_INV); /* - * It appears that the DAI and the codec don't share the same - * polarity for the LRCK signal when they mean 'normal' and - * 'inverted' in the datasheet. + * It appears that the DAI and the codec in the A33 SoC don't + * share the same polarity for the LRCK signal when they mean + * 'normal' and 'inverted' in the datasheet. * * Since the DAI here is our regular i2s driver that have been * tested with way more codecs than just this one, it means * that the codec probably gets it backward, and we have to * invert the value here. */ + value ^= scodec->quirks->lrck_inversion; regmap_update_bits(scodec->regmap, SUN8I_AIF1CLK_CTRL, BIT(SUN8I_AIF1CLK_CTRL_AIF1_LRCK_INV), - !value << SUN8I_AIF1CLK_CTRL_AIF1_LRCK_INV); + value << SUN8I_AIF1CLK_CTRL_AIF1_LRCK_INV); /* DAI format */ switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { @@ -673,6 +675,7 @@ static int sun8i_codec_remove(struct platform_device *pdev) static const struct sun8i_codec_quirks sun8i_a33_quirks = { .legacy_widgets = true, + .lrck_inversion = true, }; static const struct sun8i_codec_quirks sun50i_a64_quirks = {