diff mbox series

[v3,2/5] ASoC: sun4i-codec: correct dapm widgets and controls for h616

Message ID 20250214220247.10810-3-ryan@testtoast.com (mailing list archive)
State New
Headers show
Series ASoC: sun4i-codec: add headphone dectection for Anbernic RG35XX devices | expand

Commit Message

Ryan Walklin Feb. 14, 2025, 10:02 p.m. UTC
The previous H616 support patch added a single LINEOUT DAPM pin switch
to the card controls. As the codec in this SoC only has a single route,
this seemed reasonable at the time, however is redundant given the
existing DAPM codec widget definitions controlling the digital and
analog sides of the codec.

It is also insufficient to describe the scenario where separate
components (muxes, jack detection etc) are used to modify the audio
route external to the SoC. For example the Anbernic RG(##)XX series of
devices uses a headphone jack detection switch, GPIO-controlled speaker
amplifier and a passive external mux chip to route audio.

Remove the redundant LINEOUT card control, and add a Speaker pin switch
control and Headphone DAPM widget to allow control of the above
hardware.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Ryan Walklin <ryan@testtoast.com>
---
 sound/soc/sunxi/sun4i-codec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Jernej Škrabec Feb. 20, 2025, 7:05 p.m. UTC | #1
Dne petek, 14. februar 2025 ob 23:02:24 Srednjeevropski standardni čas je Ryan Walklin napisal(a):
> The previous H616 support patch added a single LINEOUT DAPM pin switch
> to the card controls. As the codec in this SoC only has a single route,
> this seemed reasonable at the time, however is redundant given the
> existing DAPM codec widget definitions controlling the digital and
> analog sides of the codec.
> 
> It is also insufficient to describe the scenario where separate
> components (muxes, jack detection etc) are used to modify the audio
> route external to the SoC. For example the Anbernic RG(##)XX series of
> devices uses a headphone jack detection switch, GPIO-controlled speaker
> amplifier and a passive external mux chip to route audio.
> 
> Remove the redundant LINEOUT card control, and add a Speaker pin switch
> control and Headphone DAPM widget to allow control of the above
> hardware.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> Signed-off-by: Ryan Walklin <ryan@testtoast.com>
> ---
>  sound/soc/sunxi/sun4i-codec.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> index 886b3fa537d26..f24bbefeb3923 100644
> --- a/sound/soc/sunxi/sun4i-codec.c
> +++ b/sound/soc/sunxi/sun4i-codec.c
> @@ -1916,10 +1916,11 @@ static const struct snd_soc_component_driver sun50i_h616_codec_codec = {
>  };
>  
>  static const struct snd_kcontrol_new sun50i_h616_card_controls[] = {
> -	SOC_DAPM_PIN_SWITCH("LINEOUT"),
> +	SOC_DAPM_PIN_SWITCH("Speaker"),

Will this break sun50i-h616-x96-mate and sun50i-h616-orangepi-zero based boards?
If so, other solution must be found.

Best regards,
Jernej

>  };
>  
>  static const struct snd_soc_dapm_widget sun50i_h616_codec_card_dapm_widgets[] = {
> +	SND_SOC_DAPM_HP("Headphone", NULL),
>  	SND_SOC_DAPM_LINE("Line Out", NULL),
>  	SND_SOC_DAPM_SPK("Speaker", sun4i_codec_spk_event),
>  };
>
Ryan Walklin Feb. 22, 2025, 5:10 a.m. UTC | #2
On Fri, 21 Feb 2025, at 8:05 AM, Jernej Škrabec wrote:
> Dne petek, 14. februar 2025 ob 23:02:24 Srednjeevropski standardni čas 
> je Ryan Walklin napisal(a):

>>  static const struct snd_kcontrol_new sun50i_h616_card_controls[] = {
>> -	SOC_DAPM_PIN_SWITCH("LINEOUT"),
>> +	SOC_DAPM_PIN_SWITCH("Speaker"),

> Will this break sun50i-h616-x96-mate and sun50i-h616-orangepi-zero based boards?
> If so, other solution must be found.

My understanding is that this a separate concept from the codec power control itself, which is already covered by the analog and digital power supply DAPM widgets. For the H616's single lineout output, it doesn't make sense to have a separate control for this, as the codec should power off when idle using the existing DAPM widgets, and the lineout route is always active when the codec is on. The LINEOUT control here is redundant, and not connected to any userspace mechanism in the current state. 

However for the RG35XX, which has a speaker amp controlled externally, with audio routed via a passive mux, it make sense to specify this as a separate route in userspace with a corresponding control (via GPIO pin in this case).

The other H616 devices (Orange Pi etc) only have a single route defined in their DTS, and no internal speaker to require a speaker switch. This control does not do anything for them, and removing it should not have an impact. I do not have this hardware to test, but on the RG35XX devices audio is produced without it being present, and with the speaker switch in either state (headphones plugged in and speaker amp off, or speaker amp on and headphones connected).

I did try to convey in the commit message but happy to correct the comment if you feel it is not clear.

Regards,

Ryan
diff mbox series

Patch

diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index 886b3fa537d26..f24bbefeb3923 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -1916,10 +1916,11 @@  static const struct snd_soc_component_driver sun50i_h616_codec_codec = {
 };
 
 static const struct snd_kcontrol_new sun50i_h616_card_controls[] = {
-	SOC_DAPM_PIN_SWITCH("LINEOUT"),
+	SOC_DAPM_PIN_SWITCH("Speaker"),
 };
 
 static const struct snd_soc_dapm_widget sun50i_h616_codec_card_dapm_widgets[] = {
+	SND_SOC_DAPM_HP("Headphone", NULL),
 	SND_SOC_DAPM_LINE("Line Out", NULL),
 	SND_SOC_DAPM_SPK("Speaker", sun4i_codec_spk_event),
 };