[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4F855A2B.6070402@canonical.com>
Date: Wed, 11 Apr 2012 18:17:15 +0800
From: Alex Hung <alex.hung@...onical.com>
To: Alex Hung <alex.hung@...onical.com>
CC: ibm-acpi@....eng.br, mjg@...hat.com,
ibm-acpi-devel@...ts.sourceforge.net,
platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] thinkpad_acpi: added BIOS mute interfaces for volume
Hi,
I am modifying thinkpad_acpi so it can support the mute led, and I am
looking for suggestions and feedbacks.
For newer BIOS, it includes three aml methods that can be used to get,
set and enable the mute led.
In *volume_read* function, I modify it to call BIOS GSMS method when it
is presented in AML. This is more straight-forward.
However, I had debated with myself whether it should be included inside
the if-else statement. The reason is that the new function is not
necessarily dependent on volume_get_status, but including it in if-else
statement works perfectly and it has minimal impacts on the original
behaviors and therefore I decide to put my patch there.
I implemented as it is now to demonstrate what I am intending to do;
however, deciding where to add changes to *volume_write* is much more
difficult to decide for the following reasons:
1. The new mute functions (calling to AML's SSMS and SHDA) are
dependents on BIOS and they can be called when SSMS and SHDA are supported.
2. If the new functions are included in the original cmd handler (the
while-loop), they will be blocked by the first if-statement *if
(!volume_control_allowed && tpacpi_lifecycle != TPACPI_LIFE_INIT)*.
3. However, the changes now will changes the original behaviors (i.e.
"up", "down" and so on) if BIOS supports new SSMS and SHDA methods.
Any feedbacks and suggestions are most appreciated.
Best Regards,
Alex Hung
On 04/11/2012 06:15 PM, Alex Hung wrote:
> Signed-off-by: Alex Hung<alex.hung@...onical.com>
> ---
> drivers/platform/x86/thinkpad_acpi.c | 115 +++++++++++++++++++++++++++++++---
> 1 files changed, 106 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 7b82868..dc22a4c 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -6486,12 +6486,86 @@ enum tpacpi_volume_capabilities {
> TPACPI_VOL_CAP_MAX
> };
>
> +enum {
> + TPACPI_AML_MUTE_GET_FUNC = 0x01,
> + TPACPI_AML_MUTE_SET_FUNC = 0x02,
> + TPACPI_AML_MUTE_SUPPORT_FUNC = 0x04,
> + TPACPI_AML_MUTE_READ_MASK = 0x01,
> + TPACPI_AML_MUTE_ERROR_STATE_MASK = 0x80000000,
> +};
> +
> static enum tpacpi_volume_access_mode volume_mode =
> TPACPI_VOL_MODE_MAX;
>
> static enum tpacpi_volume_capabilities volume_capabilities;
> static int volume_control_allowed;
>
> +
> +static bool volume_bios_support(int func)
> +{
> + acpi_handle temp;
> +
> + if ((func | TPACPI_AML_MUTE_GET_FUNC)&&
> + !ACPI_SUCCESS(acpi_get_handle(hkey_handle, "GSMS",&temp)))
> + return false;
> +
> + if ((func | TPACPI_AML_MUTE_SET_FUNC)&&
> + !ACPI_SUCCESS(acpi_get_handle(hkey_handle, "SSMS",&temp)))
> + return false;
> +
> + if ((func | TPACPI_AML_MUTE_SUPPORT_FUNC)&&
> + !ACPI_SUCCESS(acpi_get_handle(hkey_handle, "SHDA",&temp)))
> + return false;
> +
> + return true;
> +}
> +
> +static int hotkey_get_mute_state(int *state)
> +{
> + if (!acpi_evalf(hkey_handle, state, "GSMS", "dd"))
> + return -EIO;
> +
> + if (*state& TPACPI_AML_MUTE_ERROR_STATE_MASK)
> + pr_warning("getting mute state failed.\n");
> +
> + *state&= TPACPI_AML_MUTE_READ_MASK;
> + pr_info("get mute state = %s.\n", *state ? "muted" : "unmuted");
> +
> + return 0;
> +}
> +
> +static int hotkey_set_mute_state(int state)
> +{
> + int output;
> +
> + if (!acpi_evalf(hkey_handle,&output, "SSMS", "dd", state))
> + return -EIO;
> +
> + if (output& TPACPI_AML_MUTE_ERROR_STATE_MASK) {
> + pr_warning("setting mute state failed.\n");
> + return -EIO;
> + }
> + pr_info("set to mute led state to %s.\n", state ? "on" : "off");
> +
> + return 0;
> +}
> +
> +static int hotkey_set_mute_support(int support)
> +{
> + int output;
> +
> + if (!acpi_evalf(hkey_handle,&output, "SHDA", "dd", support))
> + return -EIO;
> +
> + if (output& TPACPI_AML_MUTE_ERROR_STATE_MASK) {
> + pr_warning("setting mute support failed.\n");
> + return -EIO;
> + }
> + pr_info("%s mute led support.\n", support ? "disable" : "enable");
> +
> + return 0;
> +}
> +
> /*
> * Used to syncronize writers to TP_EC_AUDIO and
> * TP_NVRAM_ADDR_MIXER, as we need to do read-modify-write
> @@ -6982,6 +7056,7 @@ static int __init volume_init(struct ibm_init_struct *iibm)
> static int volume_read(struct seq_file *m)
> {
> u8 status;
> + int mute;
>
> if (volume_get_status(&status)< 0) {
> seq_printf(m, "level:\t\tunreadable\n");
> @@ -6991,8 +7066,12 @@ static int volume_read(struct seq_file *m)
> else
> seq_printf(m, "level:\t\t%d\n",
> status& TP_EC_AUDIO_LVL_MSK);
> -
> - seq_printf(m, "mute:\t\t%s\n",
> + if (volume_bios_support(TPACPI_AML_MUTE_GET_FUNC)&&
> + !hotkey_get_mute_state(&mute))
> + seq_printf(m, "mute:\t\t%s\n",
> + mute ? "muted" : "unmuted");
> + else
> + seq_printf(m, "mute:\t\t%s\n",
> onoff(status, TP_EC_AUDIO_MUTESW));
>
> if (volume_control_allowed) {
> @@ -7005,7 +7084,8 @@ static int volume_read(struct seq_file *m)
> " (<level> is 0-%d)\n",
> TP_EC_VOLUME_MAX);
> }
> - }
> + } else if (volume_bios_support(TPACPI_AML_MUTE_SET_FUNC))
> + seq_printf(m, "commands:\tunmute, mute\n");
> }
>
> return 0;
> @@ -7019,6 +7099,21 @@ static int volume_write(char *buf)
> char *cmd;
> int rc;
>
> + if (volume_bios_support(
> + TPACPI_AML_MUTE_SET_FUNC | TPACPI_AML_MUTE_SUPPORT_FUNC)) {
> + while ((cmd = next_cmd(&buf))) {
> + if (strlencmp(cmd, "mute") == 0)
> + hotkey_set_mute_state(1);
> + else if (strlencmp(cmd, "unmute") == 0)
> + hotkey_set_mute_state(0);
> + else if (strlencmp(cmd, "disable") == 0)
> + hotkey_set_mute_support(1);
> + else if (strlencmp(cmd, "enable") == 0)
> + hotkey_set_mute_support(0);
> + }
> + return -EINVAL;
> + }
> +
> /*
> * We do allow volume control at driver startup, so that the
> * user can set initial state through the volume=... parameter hack.
> @@ -7061,12 +7156,14 @@ static int volume_write(char *buf)
> continue;
> }
> }
> - if (strlencmp(cmd, "mute") == 0)
> - new_mute = TP_EC_AUDIO_MUTESW_MSK;
> - else if (strlencmp(cmd, "unmute") == 0)
> - new_mute = 0;
> - else
> - return -EINVAL;
> + if (!volume_bios_support(TPACPI_AML_MUTE_SET_FUNC)) {
> + if (strlencmp(cmd, "mute") == 0)
> + new_mute = TP_EC_AUDIO_MUTESW_MSK;
> + else if (strlencmp(cmd, "unmute") == 0)
> + new_mute = 0;
> + else
> + return -EINVAL;
> + }
> }
>
> if (tp_features.mixer_no_level_control) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists