Date:   Mon, 1 Nov 2021 10:34:45 +0000
From:   Jonathan Clarke <jonathan.a.clarke@gmail.com>
To:     Takashi Iwai <tiwai@suse.de>
Cc:     linux-kernel@vger.kernel.org, Takashi Iwai <tiwai@suse.com>,
        alsa-devel@alsa-project.org
Subject: Re: [PATCH] Make top/rear speaker, mute and micmute leds work on HP
 x360 14-ea000 laptops that use Realtek 245 codec
Message-ID: <20211101103445.GA1064130@rampage-hp2>
References: <20211029154313.1005394-1-jonathan.a.clarke@gmail.com>
 <s5h35oisug0.wl-tiwai@suse.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <s5h35oisug0.wl-tiwai@suse.de>
Precedence: bulk

Thanks for taking a look at this patch so quickly, Takashi.

On Sat, Oct 30, 2021 at 12:01:03PM +0200, Takashi Iwai wrote:
> Could you give more descriptions?  The patch isn't trivial at all, and
> it needs more explanations.

Yes, will do.

> > +           /* output mute is set via SET_COEF_INDEX,SET_PROC_COEF */
> > +           spec->mute_led_polarity = 0;
> > +           spec->mute_led_coef.idx = 0x0b;
> > +           spec->mute_led_coef.mask = 0xffff;
> > +           spec->mute_led_coef.on = 0xa02f;
> > +           spec->mute_led_coef.off = 0x7774;
> > +           snd_hda_gen_add_mute_led_cdev(codec, coef_mute_led_set);
>
> I guess this COEF isn't only about mute-LED but actually does mute the
> output?  IIRC, the bit 0x08 corresponds to the LED.  If so, it's
> better to split.  Basically this snd_hda_gen_add_mute_led_cdev() and
> mute_led_coef stuff are only for the mute LED control.  e.g. you can
> change the mute LED independently via sysfs.

Thanks for suggesting this.

Having tested, I can confirm that setting this coef only affects
the output mute LED, and does not affect output.

I will therefore assume that current implementation in my patch is OK,
but let me know if it still needs changing (maybe I've misunderstood).

For reference to other users, the commands to test are:
# output LED on
hda-verb /dev/snd/hwC0D0 0x20 SET_COEF_INDEX 0x0b
hda-verb /dev/snd/hwC0D0 0x20 SET_PROC_COEF 0xa02f

# output LED off
hda-verb /dev/snd/hwC0D0 0x20 SET_COEF_INDEX 0x0b
hda-verb /dev/snd/hwC0D0 0x20 SET_PROC_COEF 0x7774

Many thanks, Jonathan