lkml.org 
[lkml]   [2011]   [Mar]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [ibm-acpi-devel] [PATCH] thinkpad-acpi: disable alsa volume mixer for SL410/SL510
On Tue, 15 Mar 2011, Keng-Yu Lin wrote:
> The mute key on SL410/SL510 only works when the alsa volume mixer
> is not enabled. This patch makes the alsa volume mixer disabled
> on the matched SL410/SL510 EC versions.

I'd like more data on this. Is this some sort of weird interaction with
userspace, or does the firmware actually changes behaviour when we
enable the MUTE HKEY event in the event mask?

> #define TPACPI_VOL_Q_MUTEONLY 0x0001 /* Mute-only control available */
> #define TPACPI_VOL_Q_LEVEL 0x0002 /* Volume control available */
> +#define TPACPI_VOL_Q_IGNORE 0x0003 /* No Volume control available */

Change the comment to /* Blacklist volume control */, please. And name
it TPACPI_VOL_Q_BLACKLIST.

> @@ -6921,20 +6924,21 @@ static int __init volume_init(struct ibm_init_struct *iibm)
> if (volume_capabilities >= TPACPI_VOL_CAP_MAX)
> return -EINVAL;
>
> + quirks = tpacpi_check_quirks(volume_quirk_table,
> + ARRAY_SIZE(volume_quirk_table));
> +
> /*
> * The ALSA mixer is our primary interface.
> * When disabled, don't install the subdriver at all
> */
> - if (!alsa_enable) {
> + if (!alsa_enable || quirks & TPACPI_VOL_Q_IGNORE) {
> vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_MIXER,
> "ALSA mixer disabled by parameter, "
> "not loading volume subdriver...\n");
> + alsa_enable = 0; /* reflect the quirk status in sysfs */
> return 1;
> }
>
> - quirks = tpacpi_check_quirks(volume_quirk_table,
> - ARRAY_SIZE(volume_quirk_table));
> -
> switch (volume_capabilities) {
> case TPACPI_VOL_CAP_AUTO:
> if (quirks & TPACPI_VOL_Q_MUTEONLY)

Do not overload error messages like that. It will now spill an
incorrect message when the quirk list blacklists the volume control.

Unfortunately, we're constrained by the unextensible alsa module command
line ABI, so something more sensible (implementing "auto") is not
available.

Instead, please do it like this:

1. Do not move the quirks test or error message up.

2. After the quirk check, do this:

if (quirks & TPACPI_VOL_Q_IGNORE) {
dbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_MIXER,
"ALSA mixer blacklisted for this model, not loading
volume subdriver...\n");
return 1;
}

Note that it is a "dbg_printk", and NOT a "vdbg_printk".

I'd much rather have something that allows the user to override the
blacklisting, though. But it cannot be done through alsa_enable. Maybe
you could be persuaded to add support for a "force_volume" parameter?

If you do add support for "force_volume", please make it an unsigned int
(not bool), use 0 for no, and 1 for yes, and leave the other values
undefined. And add it to the documentation in
Documentation/laptops/thinkpad-acpi.txt.

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh


\
 
 \ /
  Last update: 2011-03-16 20:01    [W:0.075 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site