LINUX.ORG.RU
Форум — Development  

по следам «проблемы с es8336»

 , , kernel. sound,


0

2

Всем доброго рабочего времени! :)

безобразия с es8336 продолжаются - патч, который я написал для второй проблемы приводил к тому, что сперва звука не было вообще, но после подключения наушников звух появлялся на них, а после их отключения - на спикере.
я попытался исправить ситуацию патчем ниже, но он нормально работает крайне редко. Чаще приводит к кернел паникам (при загрузке) в самых неожиданных местах... :-\

кто может посмотреть незамыленным взглядом - где тут может происходить неприличное?

diff --git a/sound/soc/codecs/es8316.c b/sound/soc/codecs/es8316.c
index 09a3ca8d6..6b999a214 100644
--- a/sound/soc/codecs/es8316.c
+++ b/sound/soc/codecs/es8316.c
@@ -24,6 +24,11 @@
 #include <sound/jack.h>
 #include "es8316.h"
 
+struct jack_gpio {
+	struct snd_soc_jack *jack;
+	struct gpio_desc *gpio;
+};
+
 /* In slave mode at single speed, the codec is documented as accepting 5
  * MCLK/LRCK ratios, but we also add ratio 400, which is commonly used on
  * Intel Cherry Trail platforms (19.2MHz MCLK, 48kHz LRCK). */
@@ -590,11 +595,13 @@ static irqreturn_t es8316_irq(int irq, void *data)
 {
 	struct es8316_priv *es8316 = data;
 	struct snd_soc_component *comp = es8316->component;
-	static struct gpio_desc *gpio = NULL;
+//	static struct gpio_desc *gpio = NULL;
 	unsigned int flags;
+	struct jack_gpio *jack_gpio;
 
 	mutex_lock(&es8316->lock);
 
+	jack_gpio = snd_soc_card_get_drvdata(comp->card);
 	regmap_read(es8316->regmap, ES8316_GPIO_FLAG, &flags);
 	if (flags == 0x00)
 		goto out; /* Powered-down / reset */
@@ -617,15 +624,15 @@ static irqreturn_t es8316_irq(int irq, void *data)
 					    SND_JACK_HEADSET | SND_JACK_BTN_0);
 			dev_info(comp->dev, "jack unplugged\n");
 
-			if (gpio)
-				devm_gpiod_put(comp->dev, gpio);
-			gpio = devm_gpiod_get_optional(comp->dev, "speakers-enable", GPIOD_OUT_LOW);
-			gpiod_set_value_cansleep(gpio, 0);
+			if (jack_gpio->gpio)
+				devm_gpiod_put(comp->dev, jack_gpio->gpio);
+			jack_gpio->gpio = devm_gpiod_get_optional(comp->dev, "speakers-enable", GPIOD_OUT_LOW);
+			gpiod_set_value_cansleep(jack_gpio->gpio, 0);
 
-			if (IS_ERR(gpio))
-				return dev_err_probe(comp->dev, PTR_ERR(gpio), "Get gpiod failed: %ld\n",
-							 PTR_ERR(gpio));
-		} printk("spurious IRQ\n");
+			if (IS_ERR(jack_gpio->gpio))
+				return dev_err_probe(comp->dev, PTR_ERR(jack_gpio->gpio), "Get gpiod failed: %ld\n",
+							 PTR_ERR(jack_gpio->gpio));
+		}
 	} else if (!(es8316->jack->status & SND_JACK_HEADPHONE)) {
 		/* Jack inserted, determine type */
 		es8316_enable_micbias_for_mic_gnd_short_detect(comp);
@@ -646,11 +653,11 @@ static irqreturn_t es8316_irq(int irq, void *data)
 			/* Keep mic-gnd-short detection on for button press */
 		} else {
 			printk("HEADPHONES\n");
-			if (gpio)
-				devm_gpiod_put(comp->dev, gpio);
-			gpio = devm_gpiod_get_optional(comp->dev, "speakers-enable", GPIOD_OUT_HIGH);
+			if (jack_gpio->gpio)
+				devm_gpiod_put(comp->dev, jack_gpio->gpio);
+			jack_gpio->gpio = devm_gpiod_get_optional(comp->dev, "speakers-enable", GPIOD_OUT_HIGH);
 			/* Shorted, headphones */
-			gpiod_set_value_cansleep(gpio, 1);
+			gpiod_set_value_cansleep(jack_gpio->gpio, 1);
 			snd_soc_jack_report(es8316->jack,
 					    SND_JACK_HEADPHONE,
 					    SND_JACK_HEADSET);
diff --git a/sound/soc/intel/avs/boards/es8336.c b/sound/soc/intel/avs/boards/es8336.c
index 141d8b8b9..6cf96f112 100644
--- a/sound/soc/intel/avs/boards/es8336.c
+++ b/sound/soc/intel/avs/boards/es8336.c
@@ -20,7 +20,10 @@
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
 #include <sound/soc-acpi.h>
-
+struct jack_gpio {
+	struct snd_soc_jack *jack;
+	struct gpio_desc *gpio;
+};
 #define ES8336_CODEC_DAI	"ES8316 HiFi"
 
 static const struct acpi_gpio_params speakers_gpio_en_params = { 0, 0, true };
@@ -40,14 +43,14 @@ static int avs_es8336_speaker_power_event(struct snd_soc_dapm_widget *w,
 	bool speaker_en;
 
 	codec_dai = snd_soc_card_get_codec_dai(card, ES8336_CODEC_DAI);
-//	gpio = gpiod_get_optional(codec_dai->dev, "speakers-enable", GPIOD_OUT_LOW);
+	gpio = gpiod_get_optional(codec_dai->dev, "speakers-enable", GPIOD_OUT_LOW);
 	speaker_en = !SND_SOC_DAPM_EVENT_ON(event);
 
 	if (SND_SOC_DAPM_EVENT_ON(event))
 		msleep(70);
 
 //	printk("gpiod_set_value_cansleep(gpio, %d)\n", speaker_en);
-//	gpiod_set_value_cansleep(gpio, speaker_en);
+	gpiod_set_value_cansleep(gpio, speaker_en);
 	return 0;
 }
 
@@ -103,19 +106,19 @@ static int avs_es8336_codec_init(struct snd_soc_pcm_runtime *runtime)
 	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(runtime, 0);
 	struct snd_soc_component *component = codec_dai->component;
 	struct snd_soc_jack_pin *pins;
-	struct snd_soc_jack *jack;
+//	struct snd_soc_jack *jack;
 	struct snd_soc_card *card = runtime->card;
-	struct gpio_desc *gpio;
+	struct jack_gpio *jack_gpio;
 	int num_pins, ret;
 
-	jack = snd_soc_card_get_drvdata(card);
+	jack_gpio = snd_soc_card_get_drvdata(card);
 	num_pins = ARRAY_SIZE(card_headset_pins);
 
 	pins = devm_kmemdup(card->dev, card_headset_pins, sizeof(*pins) * num_pins, GFP_KERNEL);
 	if (!pins)
 		return -ENOMEM;
 
-	ret = snd_soc_card_jack_new_pins(card, "Headset", SND_JACK_HEADSET | SND_JACK_BTN_0, jack,
+	ret = snd_soc_card_jack_new_pins(card, "Headset", SND_JACK_HEADSET | SND_JACK_BTN_0, jack_gpio->jack,
 					 pins, num_pins);
 	if (ret)
 		return ret;
@@ -123,17 +126,17 @@ static int avs_es8336_codec_init(struct snd_soc_pcm_runtime *runtime)
 	if (ret)
 		dev_warn(codec_dai->dev, "Unable to add GPIO mapping table\n");
 
-	printk("%s: jack->status = %x\n", __FUNCTION__, jack->status);
+	printk("%s: jack->status = %x\n", __FUNCTION__, jack_gpio->jack->status);
 
-	if (jack->status) {
-		gpio = devm_gpiod_get_optional(codec_dai->dev, "speakers-enable", GPIOD_OUT_LOW);
-		if (IS_ERR(gpio))
-			return dev_err_probe(codec_dai->dev, PTR_ERR(gpio), "Get gpiod failed: %ld\n",
-						 PTR_ERR(gpio));
+	if (jack_gpio->jack->status) {
+		jack_gpio->gpio = devm_gpiod_get_optional(codec_dai->dev, "speakers-enable", GPIOD_OUT_LOW);
+		if (IS_ERR(jack_gpio->gpio))
+			return dev_err_probe(codec_dai->dev, PTR_ERR(jack_gpio->gpio), "Get gpiod failed: %ld\n",
+						 PTR_ERR(jack_gpio->gpio));
 	}
 
-	snd_jack_set_key(jack->jack, SND_JACK_BTN_0, KEY_PLAYPAUSE);
-	snd_soc_component_set_jack(component, jack, NULL);
+	snd_jack_set_key(jack_gpio->jack->jack, SND_JACK_BTN_0, KEY_PLAYPAUSE);
+	snd_soc_component_set_jack(component, jack_gpio->jack, NULL);
 
 	card->dapm.idle_bias_off = true;
 
@@ -246,10 +249,10 @@ static int avs_es8336_probe(struct platform_device *pdev)
 	struct snd_soc_dai_link *dai_link;
 	struct snd_soc_acpi_mach *mach;
 	struct snd_soc_card *card;
-	struct snd_soc_jack *jack;
 	struct device *dev = &pdev->dev;
 	const char *pname;
 	int num_routes, ssp_port, ret;
+	struct jack_gpio *jack_gpio;
 
 	printk("inside %s\n", __FUNCTION__);
 	mach = dev_get_platdata(dev);
@@ -268,9 +271,12 @@ static int avs_es8336_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	jack = devm_kzalloc(dev, sizeof(*jack), GFP_KERNEL);
+	jack_gpio = devm_kzalloc(dev, sizeof(struct jack_gpio), GFP_KERNEL);
+	if (!jack_gpio)
+		return -ENOMEM;
+	jack_gpio->jack = devm_kzalloc(dev, sizeof(struct jack_gpio), GFP_KERNEL);
 	card = devm_kzalloc(dev, sizeof(*card), GFP_KERNEL);
-	if (!jack || !card)
+	if (!jack_gpio->jack || !card)
 		return -ENOMEM;
 
 	card->name = "avs_es8336";
@@ -287,7 +293,7 @@ static int avs_es8336_probe(struct platform_device *pdev)
 	card->dapm_routes = routes;
 	card->num_dapm_routes = num_routes;
 	card->fully_routed = true;
-	snd_soc_card_set_drvdata(card, jack);
+	snd_soc_card_set_drvdata(card, jack_gpio);
 
 	ret = snd_soc_fixup_dai_links_platform_name(card, pname);
 	if (ret)
diff --git a/sound/soc/intel/boards/sof_es8336.c b/sound/soc/intel/boards/sof_es8336.c
index 894b6610b..eb09252c2 100644
--- a/sound/soc/intel/boards/sof_es8336.c
+++ b/sound/soc/intel/boards/sof_es8336.c
@@ -64,6 +64,7 @@ struct sof_es8336_private {
 	struct list_head hdmi_pcm_list;
 	bool speaker_en;
 	struct delayed_work pcm_pop_work;
+	struct gpio_desc *gpio;
 };
 
 struct sof_hdmi_pcm {
@@ -302,6 +303,7 @@ static int sof_es8316_init(struct snd_soc_pcm_runtime *runtime)
 	snd_jack_set_key(priv->jack.jack, SND_JACK_BTN_0, KEY_PLAYPAUSE);
 
 	snd_soc_component_set_jack(codec, &priv->jack, NULL);
+//	priv->gpio = gpiod_get_optional(codec_dai->dev, "speakers-enable", GPIOD_OUT_LOW);
 
 	return 0;
 }

★★★★★

Последнее исправление: metawishmaster (всего исправлений: 2)

Ответ на: комментарий от annulen

тут вопрос %)

девайс сейчас начал паниковать при загрузке, не поднявши сеть...

а как достать человеческие логи из бинагного системдшного лога?

upd: 4-я зашзузка получилась :)

metawishmaster ★★★★★
() автор топика
Последнее исправление: metawishmaster (всего исправлений: 1)
Ответ на: комментарий от annulen

а у меня после перезагрузки после панического бута в логах нету ничего...

но на этот раз загрузка прошла и получалась вот такая красота...

// определенно, плохо работаю с памятью...

metawishmaster ★★★★★
() автор топика
Последнее исправление: metawishmaster (всего исправлений: 3)
Ответ на: комментарий от metawishmaster

Интересная часть:

[ 160.300523] BUG: kernel NULL pointer dereference, address: 000000000000000a
...
[  160.300557] Call Trace:
[  160.300559]  <TASK>
[  160.300565]  snd_soc_jack_report.part.0+0xaa/0x190 [snd_soc_core]
[  160.300586]  ? disable_irq_nosync+0x20/0x20
[  160.300589]  snd_soc_jack_report+0x13/0x20 [snd_soc_core]
[ 160.300600] es8316_irq.cold+0x15b/0x213 [snd_soc_es8316]

Похоже, что ты в snd_soc_jack_report передаешь нулевой указатель и он дальше внутри разыменовывается

annulen ★★★★★
()
Ответ на: комментарий от ox55ff

А говорите, что rust не нужен.

Осталось найти добровольца для переписывания snd_soc_core и всех зависящих от него драйверов, а так же для допиливания rust-gcc до стабильного состояния

annulen ★★★★★
()
Ответ на: комментарий от annulen

Пару лет назад и включение поддержки rust в ядре считалось бы нонсенсом. А ты погляди ка. Вполне реально корпорации могут подсуетиться и выделить бюджет под это дело.

ox55ff ★★★★★
()
Ответ на: комментарий от annulen

вообще-то, чтобы хоть как-то работало я изменил две функции:

static int avs_es8336_speaker_power_event(struct snd_soc_dapm_widget *w,
					  struct snd_kcontrol *kcontrol, int event)
{
	struct snd_soc_card *card = w->dapm->card;
	struct snd_soc_dai *codec_dai;
	struct gpio_desc *gpio;
	bool speaker_en;

	codec_dai = snd_soc_card_get_codec_dai(card, ES8336_CODEC_DAI);
//	gpio = gpiod_get_optional(codec_dai->dev, "speakers-enable", GPIOD_OUT_LOW);
	speaker_en = !SND_SOC_DAPM_EVENT_ON(event);

	if (SND_SOC_DAPM_EVENT_ON(event))
		msleep(70);

//	printk("gpiod_set_value_cansleep(gpio, %d)\n", speaker_en);
//	gpiod_set_value_cansleep(gpio, speaker_en);
	return 0;
}

и
static irqreturn_t es8316_irq(int irq, void *data)
{
	struct es8316_priv *es8316 = data;
	struct snd_soc_component *comp = es8316->component;
	static struct gpio_desc *gpio = NULL;
	unsigned int flags;

	mutex_lock(&es8316->lock);

	regmap_read(es8316->regmap, ES8316_GPIO_FLAG, &flags);
	if (flags == 0x00)
		goto out; /* Powered-down / reset */

	/* Catch spurious IRQ before set_jack is called */
	if (!es8316->jack)
		goto out;

	if (es8316->jd_inverted)
		flags ^= ES8316_GPIO_FLAG_HP_NOT_INSERTED;

	dev_info(comp->dev, "gpio flags %#04x\n", flags);
	if (flags & ES8316_GPIO_FLAG_HP_NOT_INSERTED) {
		/* Jack removed, or spurious IRQ? */
		if (es8316->jack->status & SND_JACK_MICROPHONE)
			es8316_disable_micbias_for_mic_gnd_short_detect(comp);

		if (es8316->jack->status & SND_JACK_HEADPHONE) {
			snd_soc_jack_report(es8316->jack, 0,
					    SND_JACK_HEADSET | SND_JACK_BTN_0);
			dev_info(comp->dev, "jack unplugged\n");

			if (gpio)
				devm_gpiod_put(comp->dev, gpio);
			gpio = devm_gpiod_get_optional(comp->dev, "speakers-enable", GPIOD_OUT_LOW);
			gpiod_set_value_cansleep(gpio, 0);

			if (IS_ERR(gpio))
				return dev_err_probe(comp->dev, PTR_ERR(gpio), "Get gpiod failed: %ld\n",
							 PTR_ERR(gpio));
		} printk("spurious IRQ\n");
	} else if (!(es8316->jack->status & SND_JACK_HEADPHONE)) {
		/* Jack inserted, determine type */
		es8316_enable_micbias_for_mic_gnd_short_detect(comp);
		regmap_read(es8316->regmap, ES8316_GPIO_FLAG, &flags);
		if (es8316->jd_inverted)
			flags ^= ES8316_GPIO_FLAG_HP_NOT_INSERTED;
		dev_info(comp->dev, "gpio flags %#04x\n", flags);
		if (flags & ES8316_GPIO_FLAG_HP_NOT_INSERTED) {
			/* Jack unplugged underneath us */
			printk("Jack unplugged underneath u\n");
			es8316_disable_micbias_for_mic_gnd_short_detect(comp);
		} else if (flags & ES8316_GPIO_FLAG_GM_NOT_SHORTED) {
			printk("Open, headset\n");
			/* Open, headset */
			snd_soc_jack_report(es8316->jack,
					    SND_JACK_HEADSET,
					    SND_JACK_HEADSET);
			/* Keep mic-gnd-short detection on for button press */
		} else {
			printk("HEADPHONES\n");
			if (gpio)
				devm_gpiod_put(comp->dev, gpio);
			gpio = devm_gpiod_get_optional(comp->dev, "speakers-enable", GPIOD_OUT_HIGH);
			/* Shorted, headphones */
			gpiod_set_value_cansleep(gpio, 1);
			snd_soc_jack_report(es8316->jack,
					    SND_JACK_HEADPHONE,
					    SND_JACK_HEADSET);
			/* No longer need mic-gnd-short detection */
			es8316_disable_micbias_for_mic_gnd_short_detect(comp);
		}
	} else if (es8316->jack->status & SND_JACK_MICROPHONE) {
		/* Interrupt while jack inserted, report button state */
		if (flags & ES8316_GPIO_FLAG_GM_NOT_SHORTED) {
			/* Open, button release */
			printk("Jack button released\n");
			snd_soc_jack_report(es8316->jack, 0, SND_JACK_BTN_0);
		} else {
			/* Short, button press */
			printk("Jack button pressed\n");
			snd_soc_jack_report(es8316->jack,
					    SND_JACK_BTN_0,
					    SND_JACK_BTN_0);
		}
	}

out:
	mutex_unlock(&es8316->lock);
	return IRQ_HANDLED;
}


но, конечно «struct gpio_desc *gpio» хотелось бы заныкать в какое-нить private поле (что-то похожее было в том патче, который выводил ядро из душевного равновесия)

но в какое - я пока не разобрался :-\
там же у одной карточки есть (или может быть?) несколько поддевайсов, и на какое приходит прерывание тоже пока не понятно :-\

Вы не можете подсказать, в каком направлении копать?

metawishmaster ★★★★★
() автор топика
Последнее исправление: metawishmaster (всего исправлений: 1)
Ответ на: комментарий от metawishmaster

Все равно патчи не полностью ложатся, а я не настолько хорошо (*) знаю этот код, чтобы комментировать патчи без контекста

(*) скорее, совсем никак

annulen ★★★★★
()
Последнее исправление: annulen (всего исправлений: 1)
Ответ на: комментарий от metawishmaster

там же у одной карточки есть (или может быть?) несколько поддевайсов, и на какое приходит прерывание тоже пока не понятно

Если я правильно понимаю, то тут возможны только два варинта: либо у каждого «поддевайса» свой номер прерывания, либо он для всех общий и тогда различить нельзя.

annulen ★★★★★
()
Ответ на: комментарий от monk

Предполагается, что ты не будешь писать код в unsafe-блоках без крайней необходимости (например, для чтения из аппаратных регистров, или взаимодействия с другими ЯП). См. https://doc.rust-lang.org/std/keyword.unsafe.html

annulen ★★★★★
()
31 января 2024 г.

А у меня вопрос такой. Вы где взяли документацию полную на чип ES8316? На everest-semi документация какая-то обрезанная Там только описание входов-выходов и электрические параметры, а описания регистров внутренних нет. Просто у меня тоже проблема с этим чипом. На ROCK4PI A не могу включить микрофон и, чувствую, также придётся в драйвера лезть.

leonopulos
()
Development