All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pavel Machek <pavel@ucw.cz>
To: Takashi Iwai <tiwai@suse.de>
Cc: ALSA Development Mailing List <alsa-devel@alsa-project.org>,
	Ayman Bagabas <ayman.bagabas@gmail.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Hui Wang <hui.wang@canonical.com>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Kailang Yang <kailang@realtek.com>,
	Darren Hart <dvhart@infradead.org>,
	Platform Driver <platform-driver-x86@vger.kernel.org>,
	Andy Shevchenko <andy@infradead.org>
Subject: Re: [PATCH v3 3/3] ALSA: hda: add support for Huawei WMI micmute LED
Date: Fri, 23 Nov 2018 23:05:23 +0100	[thread overview]
Message-ID: <20181123220523.GA22118@amd> (raw)
In-Reply-To: <s5h8t1l5ish.wl-tiwai@suse.de>


[-- Attachment #1.1: Type: text/plain, Size: 3067 bytes --]

HI!

> > > > > > > You have general-purpose LED, yet you are treating it as "something
> > > > > > > special". That means ugly code (quoted above) and lack of flexibility.
> > > > > > >
> > > > > > > For example, if my notebook lacks HDD LED, I can use scrollock LED for
> > > > > > > that instead. Or, in reverse way, maybe "mic mute" LED is not useful
> > > > > > > for me, and I'd like to use it for notifications instead.
> > > > > >
> > > > > > I'm not against adding the LEDs device implementation for any exotic
> > > > > > usage.
> > > > > >
> > > > > > But for the audio mute LED features, you'll need really lots of other
> > > > > > works if it were implemented via leds device.  That's the hardest
> > > > > > part, and a few lines of hooks solves it easily in the kernel side.
> > > > > > That's all about it.
> > > > > >
> > > > > > If you are ready for submitting the real solutions in user-space side
> > > > > > (patching PulseAudio and whatever all existing sound daemons, and
> > > > > > creating yet another daemon for non-PA systems (another footprint,
> > > > > > lovely), and so on), we can happily delete such in-kernel hooks :)
> > > > >
> > > > > I'm not saying we should move it to the userspace.
> > > > >
> > > > > I'm saying this should be "normal" led. drivers/leds/led-huawei-acpi.c,
> > > > > or something. Perhaps this acpi stuff is so similar you don't really
> > > > > need .c code.
> > > > >
> > > > > And then there should be a "mic muted" trigger. Similar to
> > > > > drivers/leds/trigger/ledtrig-disk.c.
> > > >
> > > > And who will trigger this, e.g. when the mixer is muted?
> > > 
> > > Is this settled? I'm encouraged to promote this series to our for-next branch.
> > 
> > I'd prefer this to be normal LED and "mic muted" to become normal
> > trigger.
> 
> But how would you solve the existing problem?
> 
> As already mentioned, you'll need to hook the LED trigger and the
> actual mixer value change.  This is the biggest missing piece, and
> it's the very reason we have the exported symbol from the platform
> driver side.
> 
> So, if you prefer in that way, please implement that for the existing
> driver (thinkpad_acpi and dell-laptop) at first.  I'll be really happy
> to get rid of the present ugly solution!  But it's been there just
> because it's not so trivial at all.  FWIW, this must be all done
> inside the kernel; otherwise you'll hit a regression.

I am really have enough work to do at the moment, but let me take a look.

> If the path via leds class can't be achieved quickly, I'd prefer
> taking the current approach at first.  Once after it's done for those
> above, we can apply the same for huawei-wmi later, too.

I'd prefer not adding more mess. Don't expect full solution from me,
but ... I should have something better than what is in the tree rather
soon.

Best regards,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



WARNING: multiple messages have this Message-ID (diff)
From: Pavel Machek <pavel@ucw.cz>
To: Takashi Iwai <tiwai@suse.de>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>,
	Ayman Bagabas <ayman.bagabas@gmail.com>,
	ALSA Development Mailing List <alsa-devel@alsa-project.org>,
	Hui Wang <hui.wang@canonical.com>,
	Andy Shevchenko <andy@infradead.org>,
	Darren Hart <dvhart@infradead.org>,
	Jaroslav Kysela <perex@perex.cz>,
	Kailang Yang <kailang@realtek.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Platform Driver <platform-driver-x86@vger.kernel.org>
Subject: Re: [PATCH v3 3/3] ALSA: hda: add support for Huawei WMI micmute LED
Date: Fri, 23 Nov 2018 23:05:23 +0100	[thread overview]
Message-ID: <20181123220523.GA22118@amd> (raw)
In-Reply-To: <s5h8t1l5ish.wl-tiwai@suse.de>

[-- Attachment #1: Type: text/plain, Size: 3067 bytes --]

HI!

> > > > > > > You have general-purpose LED, yet you are treating it as "something
> > > > > > > special". That means ugly code (quoted above) and lack of flexibility.
> > > > > > >
> > > > > > > For example, if my notebook lacks HDD LED, I can use scrollock LED for
> > > > > > > that instead. Or, in reverse way, maybe "mic mute" LED is not useful
> > > > > > > for me, and I'd like to use it for notifications instead.
> > > > > >
> > > > > > I'm not against adding the LEDs device implementation for any exotic
> > > > > > usage.
> > > > > >
> > > > > > But for the audio mute LED features, you'll need really lots of other
> > > > > > works if it were implemented via leds device.  That's the hardest
> > > > > > part, and a few lines of hooks solves it easily in the kernel side.
> > > > > > That's all about it.
> > > > > >
> > > > > > If you are ready for submitting the real solutions in user-space side
> > > > > > (patching PulseAudio and whatever all existing sound daemons, and
> > > > > > creating yet another daemon for non-PA systems (another footprint,
> > > > > > lovely), and so on), we can happily delete such in-kernel hooks :)
> > > > >
> > > > > I'm not saying we should move it to the userspace.
> > > > >
> > > > > I'm saying this should be "normal" led. drivers/leds/led-huawei-acpi.c,
> > > > > or something. Perhaps this acpi stuff is so similar you don't really
> > > > > need .c code.
> > > > >
> > > > > And then there should be a "mic muted" trigger. Similar to
> > > > > drivers/leds/trigger/ledtrig-disk.c.
> > > >
> > > > And who will trigger this, e.g. when the mixer is muted?
> > > 
> > > Is this settled? I'm encouraged to promote this series to our for-next branch.
> > 
> > I'd prefer this to be normal LED and "mic muted" to become normal
> > trigger.
> 
> But how would you solve the existing problem?
> 
> As already mentioned, you'll need to hook the LED trigger and the
> actual mixer value change.  This is the biggest missing piece, and
> it's the very reason we have the exported symbol from the platform
> driver side.
> 
> So, if you prefer in that way, please implement that for the existing
> driver (thinkpad_acpi and dell-laptop) at first.  I'll be really happy
> to get rid of the present ugly solution!  But it's been there just
> because it's not so trivial at all.  FWIW, this must be all done
> inside the kernel; otherwise you'll hit a regression.

I am really have enough work to do at the moment, but let me take a look.

> If the path via leds class can't be achieved quickly, I'd prefer
> taking the current approach at first.  Once after it's done for those
> above, we can apply the same for huawei-wmi later, too.

I'd prefer not adding more mess. Don't expect full solution from me,
but ... I should have something better than what is in the tree rather
soon.

Best regards,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

  reply	other threads:[~2018-11-23 22:05 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-08 17:16 [PATCH v3 0/3] Huawei laptops Ayman Bagabas
2018-11-08 17:16 ` [PATCH v3 1/3] x86: add support for Huawei WMI hotkeys Ayman Bagabas
2018-11-08 19:58   ` Andy Shevchenko
2018-11-09  3:52     ` ayman.bagabas
2018-11-09 10:40       ` Andy Shevchenko
2018-11-08 17:16 ` [PATCH v3 2/3] ALSA: hda: fix front speakers on Huawei MBXP Ayman Bagabas
2018-11-09  8:55   ` Takashi Iwai
2018-11-09  8:55     ` Takashi Iwai
2018-11-08 17:16 ` [PATCH v3 3/3] ALSA: hda: add support for Huawei WMI micmute LED Ayman Bagabas
2018-11-09  9:01   ` Takashi Iwai
2018-11-09  9:01     ` Takashi Iwai
2018-11-09 13:20     ` Ayman Bagabas
2018-11-09 13:28       ` Takashi Iwai
2018-11-19 23:57   ` Pavel Machek
2018-11-20  7:07     ` Takashi Iwai
2018-11-20  7:07       ` Takashi Iwai
2018-11-20  9:10       ` Pavel Machek
2018-11-20  9:23         ` Takashi Iwai
2018-11-20  9:36           ` Pavel Machek
2018-11-20  9:36             ` Pavel Machek
2018-11-20  9:49             ` Takashi Iwai
2018-11-20 11:51               ` Pavel Machek
2018-11-20 12:19                 ` Takashi Iwai
2018-11-22 11:36                   ` Andy Shevchenko
2018-11-22 13:18                     ` Pavel Machek
2018-11-22 13:43                       ` Takashi Iwai
2018-11-23 22:05                         ` Pavel Machek [this message]
2018-11-23 22:05                           ` Pavel Machek
2018-11-23 23:33                         ` Pavel Machek
2018-11-23 23:33                           ` Pavel Machek
2018-11-24  8:10                           ` Takashi Iwai
2018-11-24  8:10                             ` Takashi Iwai
2018-11-24 10:41                             ` Pavel Machek
2018-11-22 13:12                   ` Pavel Machek
2018-11-22 13:14                     ` Takashi Iwai
2018-11-08 19:59 ` [PATCH v3 0/3] Huawei laptops Andy Shevchenko
2018-11-09  3:38   ` ayman.bagabas
2018-11-09  7:41   ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181123220523.GA22118@amd \
    --to=pavel@ucw.cz \
    --cc=alsa-devel@alsa-project.org \
    --cc=andy.shevchenko@gmail.com \
    --cc=andy@infradead.org \
    --cc=ayman.bagabas@gmail.com \
    --cc=dvhart@infradead.org \
    --cc=hui.wang@canonical.com \
    --cc=kailang@realtek.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.