| Message ID | 20241221094122.27325-3-ryan@testtoast.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ASoC: sun4i-codec: add headphone dectection for Anbernic RG35XX devices | expand |
On Sat, Dec 21, 2024 at 5:41 PM Ryan Walklin <ryan@testtoast.com> wrote: > > From: Chris Morgan <macromorgan@hotmail.com> > > Add support for GPIO headphone detection with the hp-det-gpios > property. In order for this to properly disable the path upon > removal of headphones, the output must be labelled Headphone which > is a common sink in the driver. Note this patch also adds the output > of Headphone for the H616 codec. > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com> > Signed-off-by: Ryan Walklin <ryan@testtoast.com> > --- > sound/soc/sunxi/sun4i-codec.c | 59 +++++++++++++++++++++++++++++++++-- > 1 file changed, 57 insertions(+), 2 deletions(-) > > diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c > index 933a0913237c8..5f718dd3fcf0a 100644 > --- a/sound/soc/sunxi/sun4i-codec.c > +++ b/sound/soc/sunxi/sun4i-codec.c > @@ -21,6 +21,7 @@ > #include <linux/gpio/consumer.h> > > #include <sound/core.h> > +#include <sound/jack.h> > #include <sound/pcm.h> > #include <sound/pcm_params.h> > #include <sound/soc.h> > @@ -272,6 +273,7 @@ struct sun4i_codec { > struct clk *clk_module; > struct reset_control *rst; > struct gpio_desc *gpio_pa; > + struct gpio_desc *gpio_hp; > > /* ADC_FIFOC register is at different offset on different SoCs */ > struct regmap_field *reg_adc_fifoc; > @@ -1302,6 +1304,49 @@ static struct snd_soc_dai_driver dummy_cpu_dai = { > .ops = &dummy_dai_ops, > }; > > +static struct snd_soc_jack sun4i_headphone_jack; > + > +static struct snd_soc_jack_pin sun4i_headphone_jack_pins[] = { > + { .pin = "Headphone", .mask = SND_JACK_HEADPHONE}, > +}; > + > +static struct snd_soc_jack_gpio sun4i_headphone_jack_gpio = { > + .name = "hp-det", > + .report = SND_JACK_HEADPHONE, > + .debounce_time = 150, > +}; > + > +static int sun4i_codec_machine_init(struct snd_soc_pcm_runtime *rtd) > +{ > + struct snd_soc_card *card = rtd->card; > + struct sun4i_codec *scodec = snd_soc_card_get_drvdata(card); > + int ret; > + > + if (scodec->gpio_hp) { > + ret = snd_soc_card_jack_new_pins(card, "Headphone Jack", > + SND_JACK_HEADPHONE, > + &sun4i_headphone_jack, > + sun4i_headphone_jack_pins, > + ARRAY_SIZE(sun4i_headphone_jack_pins)); > + if (ret) { > + dev_err(rtd->dev, > + "Headphone jack creation failed: %d\n", ret); > + return ret; > + } > + > + sun4i_headphone_jack_gpio.desc = scodec->gpio_hp; > + ret = snd_soc_jack_add_gpios(&sun4i_headphone_jack, 1, > + &sun4i_headphone_jack_gpio); > + > + if (ret) { > + dev_err(rtd->dev, "Headphone GPIO not added: %d\n", ret); > + return ret; > + } > + } > + > + return 0; > +} > + > static struct snd_soc_dai_link *sun4i_codec_create_link(struct device *dev, > int *num_links) > { > @@ -1327,6 +1372,7 @@ static struct snd_soc_dai_link *sun4i_codec_create_link(struct device *dev, > link->codecs->name = dev_name(dev); > link->platforms->name = dev_name(dev); > link->dai_fmt = SND_SOC_DAIFMT_I2S; > + link->init = sun4i_codec_machine_init; > > *num_links = 1; > > @@ -1635,10 +1681,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"), Why? > }; > > 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), > }; > @@ -1684,7 +1731,7 @@ static struct snd_soc_card *sun50i_h616_codec_create_card(struct device *dev) > > card->dev = dev; > card->owner = THIS_MODULE; > - card->name = "H616 Audio Codec"; > + card->name = "h616-audio-codec"; Why? > card->driver_name = "sun4i-codec"; > card->controls = sun50i_h616_card_controls; > card->num_controls = ARRAY_SIZE(sun50i_h616_card_controls); > @@ -1940,6 +1987,14 @@ static int sun4i_codec_probe(struct platform_device *pdev) > return ret; > } > > + scodec->gpio_hp = devm_gpiod_get_optional(&pdev->dev, "allwinner,hp-det", > + GPIOD_IN); Just put it on the same line. Lines can go up to 100 characters. > + if (IS_ERR(scodec->gpio_hp)) { > + ret = PTR_ERR(scodec->gpio_hp); > + dev_err_probe(&pdev->dev, ret, "Failed to get hp-det gpio\n"); > + return ret; > + } > + > /* reg_field setup */ > scodec->reg_adc_fifoc = devm_regmap_field_alloc(&pdev->dev, > scodec->regmap, > -- > 2.47.1 > >
Hi Chen-Yu, Thanks for the review! On Mon, 23 Dec 2024, at 6:15 AM, Chen-Yu Tsai wrote: > On Sat, Dec 21, 2024 at 5:41 PM Ryan Walklin <ryan@testtoast.com> wrote: >> @@ -1635,10 +1681,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"), > > Why? The speaker amp is controlled by a GPIO, this needs a specific card control to toggle on and off, discrete from the line-out volume. Muting the output entirely is not what is required, hence the separate control. I also understand (although it is IMO not well documented on the user-space side) that "Speaker" has a specific meaning to the user-space routing. Can add this to the merge message. >> @@ -1684,7 +1731,7 @@ static struct snd_soc_card *sun50i_h616_codec_create_card(struct device *dev) >> >> card->dev = dev; >> card->owner = THIS_MODULE; >> - card->name = "H616 Audio Codec"; >> + card->name = "h616-audio-codec"; > > Why? As mentioned in the cover letter, ALSA UCM in user space uses the card name to detect and apply the relevant configuration, by loading /usr/share/alsa/ucm2/conf.d/<driver_name>/<card_name>.conf. Spaces are therefore removed in the card name to make this easier to manage. Happy to add this to the commit message too, and this could be changed to card->long_name if it is desired to maintain a human-readable card->name. See https://github.com/alsa-project/alsa-ucm-conf/pull/491/files for the UCM patch. >> @@ -1940,6 +1987,14 @@ static int sun4i_codec_probe(struct platform_device *pdev) >> return ret; >> } >> >> + scodec->gpio_hp = devm_gpiod_get_optional(&pdev->dev, "allwinner,hp-det", >> + GPIOD_IN); > > Just put it on the same line. Lines can go up to 100 characters. Thanks, will do. Regards, Ryan
diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c index 933a0913237c8..5f718dd3fcf0a 100644 --- a/sound/soc/sunxi/sun4i-codec.c +++ b/sound/soc/sunxi/sun4i-codec.c @@ -21,6 +21,7 @@ #include <linux/gpio/consumer.h> #include <sound/core.h> +#include <sound/jack.h> #include <sound/pcm.h> #include <sound/pcm_params.h> #include <sound/soc.h> @@ -272,6 +273,7 @@ struct sun4i_codec { struct clk *clk_module; struct reset_control *rst; struct gpio_desc *gpio_pa; + struct gpio_desc *gpio_hp; /* ADC_FIFOC register is at different offset on different SoCs */ struct regmap_field *reg_adc_fifoc; @@ -1302,6 +1304,49 @@ static struct snd_soc_dai_driver dummy_cpu_dai = { .ops = &dummy_dai_ops, }; +static struct snd_soc_jack sun4i_headphone_jack; + +static struct snd_soc_jack_pin sun4i_headphone_jack_pins[] = { + { .pin = "Headphone", .mask = SND_JACK_HEADPHONE}, +}; + +static struct snd_soc_jack_gpio sun4i_headphone_jack_gpio = { + .name = "hp-det", + .report = SND_JACK_HEADPHONE, + .debounce_time = 150, +}; + +static int sun4i_codec_machine_init(struct snd_soc_pcm_runtime *rtd) +{ + struct snd_soc_card *card = rtd->card; + struct sun4i_codec *scodec = snd_soc_card_get_drvdata(card); + int ret; + + if (scodec->gpio_hp) { + ret = snd_soc_card_jack_new_pins(card, "Headphone Jack", + SND_JACK_HEADPHONE, + &sun4i_headphone_jack, + sun4i_headphone_jack_pins, + ARRAY_SIZE(sun4i_headphone_jack_pins)); + if (ret) { + dev_err(rtd->dev, + "Headphone jack creation failed: %d\n", ret); + return ret; + } + + sun4i_headphone_jack_gpio.desc = scodec->gpio_hp; + ret = snd_soc_jack_add_gpios(&sun4i_headphone_jack, 1, + &sun4i_headphone_jack_gpio); + + if (ret) { + dev_err(rtd->dev, "Headphone GPIO not added: %d\n", ret); + return ret; + } + } + + return 0; +} + static struct snd_soc_dai_link *sun4i_codec_create_link(struct device *dev, int *num_links) { @@ -1327,6 +1372,7 @@ static struct snd_soc_dai_link *sun4i_codec_create_link(struct device *dev, link->codecs->name = dev_name(dev); link->platforms->name = dev_name(dev); link->dai_fmt = SND_SOC_DAIFMT_I2S; + link->init = sun4i_codec_machine_init; *num_links = 1; @@ -1635,10 +1681,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), }; @@ -1684,7 +1731,7 @@ static struct snd_soc_card *sun50i_h616_codec_create_card(struct device *dev) card->dev = dev; card->owner = THIS_MODULE; - card->name = "H616 Audio Codec"; + card->name = "h616-audio-codec"; card->driver_name = "sun4i-codec"; card->controls = sun50i_h616_card_controls; card->num_controls = ARRAY_SIZE(sun50i_h616_card_controls); @@ -1940,6 +1987,14 @@ static int sun4i_codec_probe(struct platform_device *pdev) return ret; } + scodec->gpio_hp = devm_gpiod_get_optional(&pdev->dev, "allwinner,hp-det", + GPIOD_IN); + if (IS_ERR(scodec->gpio_hp)) { + ret = PTR_ERR(scodec->gpio_hp); + dev_err_probe(&pdev->dev, ret, "Failed to get hp-det gpio\n"); + return ret; + } + /* reg_field setup */ scodec->reg_adc_fifoc = devm_regmap_field_alloc(&pdev->dev, scodec->regmap,