lkml.org 
[lkml]   [2011]   [Oct]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [ibm-acpi-devel] [PATCH] thinkpad-acpi: disable alsa volume mixer for SL410/SL510
    On Thu, Mar 17, 2011 at 2:57 AM, Henrique de Moraes Holschuh
    <hmh@hmh.eng.br> wrote:
    > 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?
    >

    The firmware does not change its behaviour with the key enabled in the keymask.
    The mute key (and the LED on it) of the two models works good without
    thinkpad_acpi loaded. So in the patch I like to exclude the volume
    control part for the two models.

    >>  #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.
    >

    Thanks for the suggestion. I will be sending a new version of patches.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2011-10-06 05:53    [W:0.026 / U:34.052 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site