On Sun, Apr 23, 2017 at 10:40 PM, Pali Rohár <pali.rohar@xxxxxxxxx> wrote:
> When changing keyboard backlight state on new Dell laptops, firmware
> expects a new timeout AC value filled in Set New State SMBIOS call.
>
> Without it any change of keyboard backlight state on new Dell laptops
> fails. And user can see following error message in dmesg:
>
> dell_laptop: Setting old previous keyboard state failed
> leds dell::kbd_backlight: Setting an LED's brightness failed (-6)
>
> This patch adds support for retrieving current timeout AC values and also
> updating them. Current timeout value in sysfs is displayed based on current
> AC status, like current display brightness value.
>
> Detection if Dell laptop supports or not new timeout AC settings is done by
> checking existence of Keyboard Backlight with AC SMBIOS token (0x0451).
>
> Signed-off-by: Pali Rohár <pali.rohar@xxxxxxxxx>
What base did you use for that?
This patch misses few others that were applied to our testing / for-next.
I have applied it to testing with corrected conflict (easy to fix),
though check if everything still as supposed to be.
> ---
> I have not tested this patch yet as I do not have affected machine. I would
> appreciate some testing of this patch on more new Dell laptops. Without
> this patch changing keyboard backlight is not possible on affected machines.
> ---
> drivers/platform/x86/dell-laptop.c | 59 ++++++++++++++++++++++++++++++++----
> 1 file changed, 53 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
> index f57dd28..cddf3c2 100644
> --- a/drivers/platform/x86/dell-laptop.c
> +++ b/drivers/platform/x86/dell-laptop.c
> @@ -42,6 +42,7 @@
> #define KBD_LED_AUTO_50_TOKEN 0x02EB
> #define KBD_LED_AUTO_75_TOKEN 0x02EC
> #define KBD_LED_AUTO_100_TOKEN 0x02F6
> +#define KBD_LED_AC_TOKEN 0x0451
>
> struct quirk_entry {
> u8 touchpad_led;
> @@ -1024,7 +1025,7 @@ static void touchpad_led_exit(void)
> * bit 2 Pointing stick
> * bit 3 Any mouse
> * bits 4-7 Reserved for future use
> - * cbRES2, byte3 Current Timeout
> + * cbRES2, byte3 Current Timeout on battery
> * bits 7:6 Timeout units indicator:
> * 00b Seconds
> * 01b Minutes
> @@ -1036,6 +1037,15 @@ static void touchpad_led_exit(void)
> * cbRES3, byte0 Current setting of ALS value that turns the light on or off.
> * cbRES3, byte1 Current ALS reading
> * cbRES3, byte2 Current keyboard light level.
> + * cbRES3, byte3 Current timeout on AC Power
> + * bits 7:6 Timeout units indicator:
> + * 00b Seconds
> + * 01b Minutes
> + * 10b Hours
> + * 11b Days
> + * Bits 5:0 Timeout value (0-63) in sec/min/hr/day
> + * NOTE: A value of 0 means always on (no timeout) if any bits of RES3 byte2
> + * are set upon return from the upon return from the [Get Feature information] call.
> *
> * cbArg1 0x2 = Set New State
> * cbRES1 Standard return codes (0, -1, -2)
> @@ -1058,7 +1068,7 @@ static void touchpad_led_exit(void)
> * bit 2 Pointing stick
> * bit 3 Any mouse
> * bits 4-7 Reserved for future use
> - * cbArg2, byte3 Desired Timeout
> + * cbArg2, byte3 Desired Timeout on battery
> * bits 7:6 Timeout units indicator:
> * 00b Seconds
> * 01b Minutes
> @@ -1067,6 +1077,13 @@ static void touchpad_led_exit(void)
> * bits 5:0 Timeout value (0-63) in sec/min/hr/day
> * cbArg3, byte0 Desired setting of ALS value that turns the light on or off.
> * cbArg3, byte2 Desired keyboard light level.
> + * cbArg3, byte3 Desired Timeout on AC power
> + * bits 7:6 Timeout units indicator:
> + * 00b Seconds
> + * 01b Minutes
> + * 10b Hours
> + * 11b Days
> + * bits 5:0 Timeout value (0-63) in sec/min/hr/day
> */
>
>
> @@ -1112,6 +1129,8 @@ struct kbd_state {
> u8 triggers;
> u8 timeout_value;
> u8 timeout_unit;
> + u8 timeout_value_ac;
> + u8 timeout_unit_ac;
> u8 als_setting;
> u8 als_value;
> u8 level;
> @@ -1131,6 +1150,7 @@ struct kbd_state {
> static struct kbd_info kbd_info;
> static bool kbd_als_supported;
> static bool kbd_triggers_supported;
> +static bool kbd_timeout_ac_supported;
>
> static u8 kbd_mode_levels[16];
> static int kbd_mode_levels_count;
> @@ -1269,6 +1289,8 @@ static int kbd_get_state(struct kbd_state *state)
> state->als_setting = buffer->output[2] & 0xFF;
> state->als_value = (buffer->output[2] >> 8) & 0xFF;
> state->level = (buffer->output[2] >> 16) & 0xFF;
> + state->timeout_value_ac = (buffer->output[2] >> 24) & 0x3F;
> + state->timeout_unit_ac = (buffer->output[2] >> 30) & 0x3;
>
> out:
> dell_smbios_release_buffer();
> @@ -1288,6 +1310,8 @@ static int kbd_set_state(struct kbd_state *state)
> buffer->input[1] |= (state->timeout_unit & 0x3) << 30;
> buffer->input[2] = state->als_setting & 0xFF;
> buffer->input[2] |= (state->level & 0xFF) << 16;
> + buffer->input[2] |= (state->timeout_value_ac & 0x3F) << 24;
> + buffer->input[2] |= (state->timeout_unit_ac & 0x3) << 30;
> dell_smbios_send_request(4, 11);
> ret = buffer->output[0];
> dell_smbios_release_buffer();
> @@ -1394,6 +1418,13 @@ static inline int kbd_init_info(void)
> if (ret)
> return ret;
>
> + /* NOTE: Old models without KBD_LED_AC_TOKEN token supports only one
> + * timeout value which is shared for both battery and AC power
> + * settings. So do not try to set AC values on old models.
> + */
> + if (dell_smbios_find_token(KBD_LED_AC_TOKEN))
> + kbd_timeout_ac_supported = true;
> +
> kbd_get_state(&state);
>
> /* NOTE: timeout value is stored in 6 bits so max value is 63 */
> @@ -1573,8 +1604,14 @@ static ssize_t kbd_led_timeout_store(struct device *dev,
> return ret;
>
> new_state = state;
> - new_state.timeout_value = value;
> - new_state.timeout_unit = unit;
> +
> + if (kbd_timeout_ac_supported && power_supply_is_system_supplied() > 0) {
> + new_state.timeout_value_ac = value;
> + new_state.timeout_unit_ac = unit;
> + } else {
> + new_state.timeout_value = value;
> + new_state.timeout_unit = unit;
> + }
>
> ret = kbd_set_state_safe(&new_state, &state);
> if (ret)
> @@ -1587,16 +1624,26 @@ static ssize_t kbd_led_timeout_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> struct kbd_state state;
> + int value;
> int ret;
> int len;
> + u8 unit;
>
> ret = kbd_get_state(&state);
> if (ret)
> return ret;
>
> - len = sprintf(buf, "%d", state.timeout_value);
> + if (kbd_timeout_ac_supported && power_supply_is_system_supplied() > 0) {
> + value = state.timeout_value_ac;
> + unit = state.timeout_unit_ac;
> + } else {
> + value = state.timeout_value;
> + unit = state.timeout_unit;
> + }
> +
> + len = sprintf(buf, "%d", value);
>
> - switch (state.timeout_unit) {
> + switch (unit) {
> case KBD_TIMEOUT_SECONDS:
> return len + sprintf(buf+len, "s\n");
> case KBD_TIMEOUT_MINUTES:
> --
> 1.7.9.5
>
--
With Best Regards,
Andy Shevchenko
