Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 14 Jan 2022 17:37:42 +0100
Message-ID: <s5ha6fy46jt.wl-tiwai@suse.de>
From:   Takashi Iwai <tiwai@suse.de>
To:     Alexander Sergeyev <sergeev917@gmail.com>
Cc:     Jeremy Szu <jeremy.szu@canonical.com>, tiwai@suse.com,
        "moderated list:SOUND" <alsa-devel@alsa-project.org>,
        Kailang Yang <kailang@realtek.com>,
        open list <linux-kernel@vger.kernel.org>,
        Huacai Chen <chenhuacai@kernel.org>,
        Jian-Hong Pan <jhp@endlessos.org>,
        Hui Wang <hui.wang@canonical.com>,
        PeiSen Hou <pshou@realtek.com>
Subject: Re: [PATCH 1/4] ALSA: hda/realtek: fix mute/micmute LEDs for HP 855 G8
In-Reply-To: <20220113183141.kla37mbqmo4x6wxp@localhost.localdomain>
References: <20210519170357.58410-1-jeremy.szu@canonical.com>
        <20220111195229.a77wrpjclqwrx4bx@localhost.localdomain>
        <s5ho84h9tit.wl-tiwai@suse.de>
        <20220112101249.ya73jvpmqmeh4ggg@localhost.localdomain>
        <s5hilup9s87.wl-tiwai@suse.de>
        <20220112104827.4aymoth7ua65nwge@localhost.localdomain>
        <20220112201824.qmphnz2hx4frda6e@localhost.localdomain>
        <s5h8rvk85uy.wl-tiwai@suse.de>
        <20220113183141.kla37mbqmo4x6wxp@localhost.localdomain>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka)
 FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3
 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Thu, 13 Jan 2022 19:31:41 +0100,
Alexander Sergeyev wrote:
> 
> On Thu, Jan 13, 2022 at 08:14:29AM +0100, Takashi Iwai wrote:
> >> First, about unbind and bind via sysfs -- attempts to unbind the
> >> HD-audio controller immediately trigger BUGs:
> >> Is it normal/expected?
> >
> >A sort of. The sysfs unbind is little tested and may be still buggy
> >if done during the stream operation.
> >
> >To be sure, could you check with my latest sound.git tree for-linus
> >branch?  There are a few fixes that harden the dynamic unbind.
> 
> I assume that the referred repository is the one at [1]. I've tried
> 081c73701ef0 "ALSA: hda: intel-dsp-config: reorder the config
> table". It crashed with nearly identical logs.

OK, then it's still something to do with the led cdev
unregisteration.

Could you try the patch below?

> >> 1) Coeff 0x0b is flapping between 0x8003 and 0x7770 and does not seem
> >> to have any effect in both non-working and working versions. Not sure
> >> about this, maybe microphone is not operational since I haven't
> >> checked it yet.
> 
> I got some time to poke the internal microphone. It works, but
> oddities are there as well. Initially I get "Mic Boost", "Capture" and
> "Internal Mic Boost" controls in alsamixer. When I run arecord,
> "Digital" control appears, but it cannot be changed until arecord is
> stopped. Subsequent arecord calls do not lock "Digital" control. This
> control affects sensitivity of the microphone and seems useful.

That's presumably an alsa-lib softvol stuff.  It's created
dynamically.  So not really a kernel problem.


Takashi

---
diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c
index 3bf5e3410703..503cd979c908 100644
--- a/sound/pci/hda/hda_generic.c
+++ b/sound/pci/hda/hda_generic.c
@@ -84,13 +84,21 @@ static void free_kctls(struct hda_gen_spec *spec)
 	snd_array_free(&spec->kctls);
 }
 
-static void snd_hda_gen_spec_free(struct hda_gen_spec *spec)
+static void snd_hda_gen_spec_free(struct hda_codec *codec)
 {
+	struct hda_gen_spec *spec = codec->spec;
+
 	if (!spec)
 		return;
 	free_kctls(spec);
 	snd_array_free(&spec->paths);
 	snd_array_free(&spec->loopback_list);
+#ifdef CONFIG_SND_HDA_GENERIC_LEDS
+	if (spec->led_cdevs[LED_AUDIO_MUTE])
+		led_classdev_unregister(spec->led_cdevs[LED_AUDIO_MUTE]);
+	if (spec->led_cdevs[LED_AUDIO_MICMUTE])
+		led_classdev_unregister(spec->led_cdevs[LED_AUDIO_MICMUTE]);
+#endif
 }
 
 /*
@@ -3922,7 +3930,9 @@ static int create_mute_led_cdev(struct hda_codec *codec,
 						enum led_brightness),
 				bool micmute)
 {
+	struct hda_gen_spec *spec = codec->spec;
 	struct led_classdev *cdev;
+	int err;
 
 	cdev = devm_kzalloc(&codec->core.dev, sizeof(*cdev), GFP_KERNEL);
 	if (!cdev)
@@ -3935,7 +3945,11 @@ static int create_mute_led_cdev(struct hda_codec *codec,
 	cdev->brightness = ledtrig_audio_get(micmute ? LED_AUDIO_MICMUTE : LED_AUDIO_MUTE);
 	cdev->flags = LED_CORE_SUSPENDRESUME;
 
-	return devm_led_classdev_register(&codec->core.dev, cdev);
+	err = led_classdev_register(&codec->core.dev, cdev);
+	if (err < 0)
+		return err;
+	spec->led_cdevs[micmute ? LED_AUDIO_MICMUTE : LED_AUDIO_MUTE] = cdev;
+	return 0;
 }
 
 /**
@@ -5998,7 +6012,7 @@ EXPORT_SYMBOL_GPL(snd_hda_gen_init);
 void snd_hda_gen_free(struct hda_codec *codec)
 {
 	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_FREE);
-	snd_hda_gen_spec_free(codec->spec);
+	snd_hda_gen_spec_free(codec);
 	kfree(codec->spec);
 	codec->spec = NULL;
 }
diff --git a/sound/pci/hda/hda_generic.h b/sound/pci/hda/hda_generic.h
index 8e1bc8ea74fc..34eba40cc6e6 100644
--- a/sound/pci/hda/hda_generic.h
+++ b/sound/pci/hda/hda_generic.h
@@ -294,6 +294,9 @@ struct hda_gen_spec {
 				   struct hda_jack_callback *cb);
 	void (*mic_autoswitch_hook)(struct hda_codec *codec,
 				    struct hda_jack_callback *cb);
+
+	/* leds */
+	struct led_classdev *led_cdevs[NUM_AUDIO_LEDS];
 };
 
 /* values for add_stereo_mix_input flag */