Re: [PATCH] thinkpad-acpi: disable alsa volume mixer for SL410/SL510

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

------------------------------------------------------------------------------
Colocation vs. Managed Hosting
A question and answer guide to determining the best fit
for your organization - today and in the future.
http://p.sf.net/sfu/internap-sfd2d
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


[Index of Archives]     [Linux ACPI]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Photo]     [Yosemite Photos]     [Yosemite Advice]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux