* [PATCH] [v2] platform/x86: thinkpad_acpi: set keyboard language @ 2021-01-25 2:59 Nitin Joshi 2021-01-25 20:33 ` Hans de Goede 2021-01-25 21:30 ` Andy Shevchenko 0 siblings, 2 replies; 8+ messages in thread From: Nitin Joshi @ 2021-01-25 2:59 UTC (permalink / raw) To: hdegoede Cc: bberg, peter.hutterer, maruichit, mpearson, platform-driver-x86, Nitin Joshi From: Nitin Joshi <njoshi1@lenovo.com> This patch is to create sysfs entry for setting keyboard language using ASL method. Some thinkpads models like T580 , T590 , T15 Gen 1 etc. has "=", "(',")" numeric keys, which are not displaying correctly, when keyboard language is other than "english". This patch fixes this issue by setting keyboard language to ECFW. Signed-off-by: Nitin Joshi <njoshi1@lenovo.com> --- Changes in v2: - used sysfs_streq() API instead of strcmp - used ARRAY_SIZE() API instead of strlen - addressed typo --- .../admin-guide/laptops/thinkpad-acpi.rst | 24 +++ drivers/platform/x86/thinkpad_acpi.c | 182 ++++++++++++++++++ 2 files changed, 206 insertions(+) diff --git a/Documentation/admin-guide/laptops/thinkpad-acpi.rst b/Documentation/admin-guide/laptops/thinkpad-acpi.rst index 5fe1ade88c17..b1188f05a99a 100644 --- a/Documentation/admin-guide/laptops/thinkpad-acpi.rst +++ b/Documentation/admin-guide/laptops/thinkpad-acpi.rst @@ -51,6 +51,7 @@ detailed description): - UWB enable and disable - LCD Shadow (PrivacyGuard) enable and disable - Lap mode sensor + - Setting keyboard language A compatibility table by model and feature is maintained on the web site, http://ibm-acpi.sf.net/. I appreciate any success or failure @@ -1466,6 +1467,29 @@ Sysfs notes rfkill controller switch "tpacpi_uwb_sw": refer to Documentation/driver-api/rfkill.rst for details. + +Setting keyboard language +------------------- + +sysfs: keyboard_lang + +This feature is used to set keyboard language to ECFW using ASL interface. +Fewer thinkpads models like T580 , T590 , T15 Gen 1 etc.. has "=", "(', +")" numeric keys, which are not displaying correctly, when keyboard language +is other than "english". This is because of default keyboard language in ECFW +is set as "english". Hence using this sysfs, user can set correct keyboard +language to ECFW and then these key's will work correctly . + +Example of command to set keyboard language is mentioned below:: + + echo jp > /sys/devices/platform/thinkpad_acpi/keyboard_lang + +Text corresponding to keyboard layout to be set in sysfs are : jp (Japan), be(Belgian), +cz(Czech), en(English), da(Danish), de(German), es(Spain) , et(Estonian), +fr(French) , fr-ch (French(Switzerland)), pl(Polish), sl(Slovenian), hu +(Hungarian), nl(Dutch), tr(Turkey), it(Italy), sv(Sweden), pt(portugese) + + Adaptive keyboard ----------------- diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c index e03df2881dc6..3cfc4a872c2d 100644 --- a/drivers/platform/x86/thinkpad_acpi.c +++ b/drivers/platform/x86/thinkpad_acpi.c @@ -9982,6 +9982,183 @@ static struct ibm_struct proxsensor_driver_data = { .exit = proxsensor_exit, }; +/************************************************************************* + * Keyboard language interface + */ + +struct keyboard_lang_data { + const char *lang_str; + int lang_code; +}; + +/* + * When adding new entries to keyboard_lang_data, please check that + * the select_lang[] buffer in keyboard_lang_show() is still large enough. + */ +struct keyboard_lang_data keyboard_lang_data[] = { + {"en", 0}, + {"be", 0x080c}, + {"cz", 0x0405}, + {"da", 0x0406}, + {"de", 0x0c07}, + {"es", 0x2c0a}, + {"et", 0x0425}, + {"fr", 0x040c}, + {"fr-ch", 0x100c}, + {"hu", 0x040e}, + {"it", 0x0410}, + {"jp", 0x0411}, + {"nl", 0x0413}, + {"nn", 0x0414}, + {"pl", 0x0415}, + {"pt", 0x0816}, + {"sl", 0x041b}, + {"sv", 0x081d}, + {"tr", 0x041f}, +}; + +static int set_keyboard_lang_command(int command) +{ + acpi_handle sskl_handle; + int output; + + if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "SSKL", &sskl_handle))) { + /* Platform doesn't support SSKL */ + return -ENODEV; + } + + if (!acpi_evalf(sskl_handle, &output, NULL, "dd", command)) + return -EIO; + + return 0; +} + +static int get_keyboard_lang(int *output) +{ + acpi_handle gskl_handle; + int kbd_lang; + + if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "GSKL", &gskl_handle))) { + /* Platform doesn't support GSKL */ + return -ENODEV; + } + + if (!acpi_evalf(gskl_handle, &kbd_lang, NULL, "dd", 0x02000000)) + return -EIO; + + *output = kbd_lang; + + return 0; +} + +/* sysfs keyboard language entry */ +static ssize_t keyboard_lang_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + int output, err, i; + char select_lang[80] = ""; + char lang[8] = ""; + + err = get_keyboard_lang(&output); + if (err) + return err; + + for (i = 0; i < ARRAY_SIZE(keyboard_lang_data); i++) { + if (i) + strcat(select_lang, " "); + + if (output == keyboard_lang_data[i].lang_code) { + strcat(lang, "["); + strcat(lang, keyboard_lang_data[i].lang_str); + strcat(lang, "]"); + strcat(select_lang, lang); + } else { + strcat(select_lang, keyboard_lang_data[i].lang_str); + } + } + + return sysfs_emit(buf, "%s\n", select_lang); +} + +static ssize_t keyboard_lang_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + int err, i; + bool lang_found = false; + int lang_code = 0; + + for (i = 0; i < ARRAY_SIZE(keyboard_lang_data); i++) { + if (sysfs_streq(buf, keyboard_lang_data[i].lang_str)) { + lang_code = keyboard_lang_data[i].lang_code; + lang_found = true; + break; + } + } + + if (lang_found) { + lang_code = lang_code | 1 << 24; + + /* Set language code */ + err = set_keyboard_lang_command(lang_code); + if (err) + return err; + } else { + pr_err("Unknown Keyboard language. Ignoring\n"); + return -EINVAL; + } + + tpacpi_disclose_usertask(attr->attr.name, + "keyboard language is set to %s\n", buf); + + sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "keyboard_lang"); + + return count; +} + +static DEVICE_ATTR_RW(keyboard_lang); + +static struct attribute *kbdlang_attributes[] = { + &dev_attr_keyboard_lang.attr, + NULL +}; + +static const struct attribute_group kbdlang_attr_group = { + .attrs = kbdlang_attributes, +}; + +static int tpacpi_kbdlang_init(struct ibm_init_struct *iibm) +{ + int err, output; + + err = get_keyboard_lang(&output); + /* + * If support isn't available (ENODEV) then don't return an error + * just don't create the sysfs group + */ + if (err == -ENODEV) + return 0; + + if (err) + return err; + + /* Platform supports this feature - create the sysfs file */ + err = sysfs_create_group(&tpacpi_pdev->dev.kobj, &kbdlang_attr_group); + + return err; +} + +static void kbdlang_exit(void) +{ + sysfs_remove_group(&tpacpi_pdev->dev.kobj, &kbdlang_attr_group); +} + +static struct ibm_struct kbdlang_driver_data = { + .name = "kbdlang", + .exit = kbdlang_exit, +}; + /**************************************************************************** **************************************************************************** * @@ -10474,6 +10651,11 @@ static struct ibm_init_struct ibms_init[] __initdata = { .init = tpacpi_proxsensor_init, .data = &proxsensor_driver_data, }, + { + .init = tpacpi_kbdlang_init, + .data = &kbdlang_driver_data, + }, + }; static int __init set_ibm_param(const char *val, const struct kernel_param *kp) -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] [v2] platform/x86: thinkpad_acpi: set keyboard language 2021-01-25 2:59 [PATCH] [v2] platform/x86: thinkpad_acpi: set keyboard language Nitin Joshi @ 2021-01-25 20:33 ` Hans de Goede 2021-01-26 0:21 ` [External] " Nitin Joshi1 2021-01-25 21:30 ` Andy Shevchenko 1 sibling, 1 reply; 8+ messages in thread From: Hans de Goede @ 2021-01-25 20:33 UTC (permalink / raw) To: Nitin Joshi Cc: bberg, peter.hutterer, maruichit, mpearson, platform-driver-x86, Nitin Joshi Hi, On 1/25/21 3:59 AM, Nitin Joshi wrote: > From: Nitin Joshi <njoshi1@lenovo.com> > > This patch is to create sysfs entry for setting keyboard language > using ASL method. Some thinkpads models like T580 , T590 , T15 Gen 1 > etc. has "=", "(',")" numeric keys, which are not displaying correctly, > when keyboard language is other than "english". > This patch fixes this issue by setting keyboard language to ECFW. > > Signed-off-by: Nitin Joshi <njoshi1@lenovo.com> > --- > Changes in v2: > - used sysfs_streq() API instead of strcmp > - used ARRAY_SIZE() API instead of strlen > - addressed typo Thank you for your patch, I've applied this patch to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Nitn, I did not notice one small issue while testing this on a X1C8 I will send out a follow-up patch addressing that. If you can test the follow-up patch on one of the affected models (T580 , T590 , T15 Gen 1, etc.) that would be great. Note this patch will show up in my review-hans branch once I've pushed my local branch there, which might take a while. Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window. Regards, Hans > --- > .../admin-guide/laptops/thinkpad-acpi.rst | 24 +++ > drivers/platform/x86/thinkpad_acpi.c | 182 ++++++++++++++++++ > 2 files changed, 206 insertions(+) > > diff --git a/Documentation/admin-guide/laptops/thinkpad-acpi.rst b/Documentation/admin-guide/laptops/thinkpad-acpi.rst > index 5fe1ade88c17..b1188f05a99a 100644 > --- a/Documentation/admin-guide/laptops/thinkpad-acpi.rst > +++ b/Documentation/admin-guide/laptops/thinkpad-acpi.rst > @@ -51,6 +51,7 @@ detailed description): > - UWB enable and disable > - LCD Shadow (PrivacyGuard) enable and disable > - Lap mode sensor > + - Setting keyboard language > > A compatibility table by model and feature is maintained on the web > site, http://ibm-acpi.sf.net/. I appreciate any success or failure > @@ -1466,6 +1467,29 @@ Sysfs notes > rfkill controller switch "tpacpi_uwb_sw": refer to > Documentation/driver-api/rfkill.rst for details. > > + > +Setting keyboard language > +------------------- > + > +sysfs: keyboard_lang > + > +This feature is used to set keyboard language to ECFW using ASL interface. > +Fewer thinkpads models like T580 , T590 , T15 Gen 1 etc.. has "=", "(', > +")" numeric keys, which are not displaying correctly, when keyboard language > +is other than "english". This is because of default keyboard language in ECFW > +is set as "english". Hence using this sysfs, user can set correct keyboard > +language to ECFW and then these key's will work correctly . > + > +Example of command to set keyboard language is mentioned below:: > + > + echo jp > /sys/devices/platform/thinkpad_acpi/keyboard_lang > + > +Text corresponding to keyboard layout to be set in sysfs are : jp (Japan), be(Belgian), > +cz(Czech), en(English), da(Danish), de(German), es(Spain) , et(Estonian), > +fr(French) , fr-ch (French(Switzerland)), pl(Polish), sl(Slovenian), hu > +(Hungarian), nl(Dutch), tr(Turkey), it(Italy), sv(Sweden), pt(portugese) > + > + > Adaptive keyboard > ----------------- > > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c > index e03df2881dc6..3cfc4a872c2d 100644 > --- a/drivers/platform/x86/thinkpad_acpi.c > +++ b/drivers/platform/x86/thinkpad_acpi.c > @@ -9982,6 +9982,183 @@ static struct ibm_struct proxsensor_driver_data = { > .exit = proxsensor_exit, > }; > > +/************************************************************************* > + * Keyboard language interface > + */ > + > +struct keyboard_lang_data { > + const char *lang_str; > + int lang_code; > +}; > + > +/* > + * When adding new entries to keyboard_lang_data, please check that > + * the select_lang[] buffer in keyboard_lang_show() is still large enough. > + */ > +struct keyboard_lang_data keyboard_lang_data[] = { > + {"en", 0}, > + {"be", 0x080c}, > + {"cz", 0x0405}, > + {"da", 0x0406}, > + {"de", 0x0c07}, > + {"es", 0x2c0a}, > + {"et", 0x0425}, > + {"fr", 0x040c}, > + {"fr-ch", 0x100c}, > + {"hu", 0x040e}, > + {"it", 0x0410}, > + {"jp", 0x0411}, > + {"nl", 0x0413}, > + {"nn", 0x0414}, > + {"pl", 0x0415}, > + {"pt", 0x0816}, > + {"sl", 0x041b}, > + {"sv", 0x081d}, > + {"tr", 0x041f}, > +}; > + > +static int set_keyboard_lang_command(int command) > +{ > + acpi_handle sskl_handle; > + int output; > + > + if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "SSKL", &sskl_handle))) { > + /* Platform doesn't support SSKL */ > + return -ENODEV; > + } > + > + if (!acpi_evalf(sskl_handle, &output, NULL, "dd", command)) > + return -EIO; > + > + return 0; > +} > + > +static int get_keyboard_lang(int *output) > +{ > + acpi_handle gskl_handle; > + int kbd_lang; > + > + if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "GSKL", &gskl_handle))) { > + /* Platform doesn't support GSKL */ > + return -ENODEV; > + } > + > + if (!acpi_evalf(gskl_handle, &kbd_lang, NULL, "dd", 0x02000000)) > + return -EIO; > + > + *output = kbd_lang; > + > + return 0; > +} > + > +/* sysfs keyboard language entry */ > +static ssize_t keyboard_lang_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + int output, err, i; > + char select_lang[80] = ""; > + char lang[8] = ""; > + > + err = get_keyboard_lang(&output); > + if (err) > + return err; > + > + for (i = 0; i < ARRAY_SIZE(keyboard_lang_data); i++) { > + if (i) > + strcat(select_lang, " "); > + > + if (output == keyboard_lang_data[i].lang_code) { > + strcat(lang, "["); > + strcat(lang, keyboard_lang_data[i].lang_str); > + strcat(lang, "]"); > + strcat(select_lang, lang); > + } else { > + strcat(select_lang, keyboard_lang_data[i].lang_str); > + } > + } > + > + return sysfs_emit(buf, "%s\n", select_lang); > +} > + > +static ssize_t keyboard_lang_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + int err, i; > + bool lang_found = false; > + int lang_code = 0; > + > + for (i = 0; i < ARRAY_SIZE(keyboard_lang_data); i++) { > + if (sysfs_streq(buf, keyboard_lang_data[i].lang_str)) { > + lang_code = keyboard_lang_data[i].lang_code; > + lang_found = true; > + break; > + } > + } > + > + if (lang_found) { > + lang_code = lang_code | 1 << 24; > + > + /* Set language code */ > + err = set_keyboard_lang_command(lang_code); > + if (err) > + return err; > + } else { > + pr_err("Unknown Keyboard language. Ignoring\n"); > + return -EINVAL; > + } > + > + tpacpi_disclose_usertask(attr->attr.name, > + "keyboard language is set to %s\n", buf); > + > + sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "keyboard_lang"); > + > + return count; > +} > + > +static DEVICE_ATTR_RW(keyboard_lang); > + > +static struct attribute *kbdlang_attributes[] = { > + &dev_attr_keyboard_lang.attr, > + NULL > +}; > + > +static const struct attribute_group kbdlang_attr_group = { > + .attrs = kbdlang_attributes, > +}; > + > +static int tpacpi_kbdlang_init(struct ibm_init_struct *iibm) > +{ > + int err, output; > + > + err = get_keyboard_lang(&output); > + /* > + * If support isn't available (ENODEV) then don't return an error > + * just don't create the sysfs group > + */ > + if (err == -ENODEV) > + return 0; > + > + if (err) > + return err; > + > + /* Platform supports this feature - create the sysfs file */ > + err = sysfs_create_group(&tpacpi_pdev->dev.kobj, &kbdlang_attr_group); > + > + return err; > +} > + > +static void kbdlang_exit(void) > +{ > + sysfs_remove_group(&tpacpi_pdev->dev.kobj, &kbdlang_attr_group); > +} > + > +static struct ibm_struct kbdlang_driver_data = { > + .name = "kbdlang", > + .exit = kbdlang_exit, > +}; > + > /**************************************************************************** > **************************************************************************** > * > @@ -10474,6 +10651,11 @@ static struct ibm_init_struct ibms_init[] __initdata = { > .init = tpacpi_proxsensor_init, > .data = &proxsensor_driver_data, > }, > + { > + .init = tpacpi_kbdlang_init, > + .data = &kbdlang_driver_data, > + }, > + > }; > > static int __init set_ibm_param(const char *val, const struct kernel_param *kp) > ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [External] Re: [PATCH] [v2] platform/x86: thinkpad_acpi: set keyboard language 2021-01-25 20:33 ` Hans de Goede @ 2021-01-26 0:21 ` Nitin Joshi1 0 siblings, 0 replies; 8+ messages in thread From: Nitin Joshi1 @ 2021-01-26 0:21 UTC (permalink / raw) To: Hans de Goede, Nitin Joshi Cc: bberg, peter.hutterer, Tomoki Maruichi, Mark Pearson, platform-driver-x86 Hello Hans , >-----Original Message----- >From: Hans de Goede <hdegoede@redhat.com> >Sent: Tuesday, January 26, 2021 5:34 AM >To: Nitin Joshi <nitjoshi@gmail.com> >Cc: bberg@redhat.com; peter.hutterer@redhat.com; Tomoki Maruichi ><maruichit@lenovo.com>; Mark Pearson <mpearson@lenovo.com>; >platform-driver-x86@vger.kernel.org; Nitin Joshi1 <njoshi1@lenovo.com> >Subject: [External] Re: [PATCH] [v2] platform/x86: thinkpad_acpi: set keyboard >language > >Hi, > >On 1/25/21 3:59 AM, Nitin Joshi wrote: >> From: Nitin Joshi <njoshi1@lenovo.com> >> >> This patch is to create sysfs entry for setting keyboard language >> using ASL method. Some thinkpads models like T580 , T590 , T15 Gen 1 >> etc. has "=", "(',")" numeric keys, which are not displaying >> correctly, when keyboard language is other than "english". >> This patch fixes this issue by setting keyboard language to ECFW. >> >> Signed-off-by: Nitin Joshi <njoshi1@lenovo.com> >> --- >> Changes in v2: >> - used sysfs_streq() API instead of strcmp >> - used ARRAY_SIZE() API instead of strlen >> - addressed typo > >Thank you for your patch, I've applied this patch to my review-hans >branch: >https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers- >x86.git/log/?h=review-hans > >Nitn, I did not notice one small issue while testing this on a X1C8 I will send out >a follow-up patch addressing that. If you can test the follow-up patch on one >of the affected models >(T580 , T590 , T15 Gen 1, etc.) that would be great. Yes , sure . I will test it . Thank you > >Note this patch will show up in my review-hans branch once I've pushed my >local branch there, which might take a while. > >Once I've run some tests on this branch the patches there will be added to the >platform-drivers-x86/for-next branch and eventually will be included in the >pdx86 pull-request to Linus for the next merge-window. > >Regards, > >Hans > Thanks & Regards, Nitin Joshi > >> --- >> .../admin-guide/laptops/thinkpad-acpi.rst | 24 +++ >> drivers/platform/x86/thinkpad_acpi.c | 182 ++++++++++++++++++ >> 2 files changed, 206 insertions(+) >> >> diff --git a/Documentation/admin-guide/laptops/thinkpad-acpi.rst >> b/Documentation/admin-guide/laptops/thinkpad-acpi.rst >> index 5fe1ade88c17..b1188f05a99a 100644 >> --- a/Documentation/admin-guide/laptops/thinkpad-acpi.rst >> +++ b/Documentation/admin-guide/laptops/thinkpad-acpi.rst >> @@ -51,6 +51,7 @@ detailed description): >> - UWB enable and disable >> - LCD Shadow (PrivacyGuard) enable and disable >> - Lap mode sensor >> + - Setting keyboard language >> >> A compatibility table by model and feature is maintained on the web >> site, http://ibm-acpi.sf.net/. I appreciate any success or failure @@ >> -1466,6 +1467,29 @@ Sysfs notes >> rfkill controller switch "tpacpi_uwb_sw": refer to >> Documentation/driver-api/rfkill.rst for details. >> >> + >> +Setting keyboard language >> +------------------- >> + >> +sysfs: keyboard_lang >> + >> +This feature is used to set keyboard language to ECFW using ASL interface. >> +Fewer thinkpads models like T580 , T590 , T15 Gen 1 etc.. has "=", >> +"(', ")" numeric keys, which are not displaying correctly, when >> +keyboard language is other than "english". This is because of default >> +keyboard language in ECFW is set as "english". Hence using this >> +sysfs, user can set correct keyboard language to ECFW and then these key's >will work correctly . >> + >> +Example of command to set keyboard language is mentioned below:: >> + >> + echo jp > /sys/devices/platform/thinkpad_acpi/keyboard_lang >> + >> +Text corresponding to keyboard layout to be set in sysfs are : jp >> +(Japan), be(Belgian), cz(Czech), en(English), da(Danish), de(German), >> +es(Spain) , et(Estonian), >> +fr(French) , fr-ch (French(Switzerland)), pl(Polish), sl(Slovenian), >> +hu (Hungarian), nl(Dutch), tr(Turkey), it(Italy), sv(Sweden), >> +pt(portugese) >> + >> + >> Adaptive keyboard >> ----------------- >> >> diff --git a/drivers/platform/x86/thinkpad_acpi.c >> b/drivers/platform/x86/thinkpad_acpi.c >> index e03df2881dc6..3cfc4a872c2d 100644 >> --- a/drivers/platform/x86/thinkpad_acpi.c >> +++ b/drivers/platform/x86/thinkpad_acpi.c >> @@ -9982,6 +9982,183 @@ static struct ibm_struct proxsensor_driver_data >= { >> .exit = proxsensor_exit, >> }; >> >> >+/*************************************************************** >***** >> +***** >> + * Keyboard language interface >> + */ >> + >> +struct keyboard_lang_data { >> + const char *lang_str; >> + int lang_code; >> +}; >> + >> +/* >> + * When adding new entries to keyboard_lang_data, please check that >> + * the select_lang[] buffer in keyboard_lang_show() is still large enough. >> + */ >> +struct keyboard_lang_data keyboard_lang_data[] = { >> + {"en", 0}, >> + {"be", 0x080c}, >> + {"cz", 0x0405}, >> + {"da", 0x0406}, >> + {"de", 0x0c07}, >> + {"es", 0x2c0a}, >> + {"et", 0x0425}, >> + {"fr", 0x040c}, >> + {"fr-ch", 0x100c}, >> + {"hu", 0x040e}, >> + {"it", 0x0410}, >> + {"jp", 0x0411}, >> + {"nl", 0x0413}, >> + {"nn", 0x0414}, >> + {"pl", 0x0415}, >> + {"pt", 0x0816}, >> + {"sl", 0x041b}, >> + {"sv", 0x081d}, >> + {"tr", 0x041f}, >> +}; >> + >> +static int set_keyboard_lang_command(int command) { >> + acpi_handle sskl_handle; >> + int output; >> + >> + if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "SSKL", >&sskl_handle))) { >> + /* Platform doesn't support SSKL */ >> + return -ENODEV; >> + } >> + >> + if (!acpi_evalf(sskl_handle, &output, NULL, "dd", command)) >> + return -EIO; >> + >> + return 0; >> +} >> + >> +static int get_keyboard_lang(int *output) { >> + acpi_handle gskl_handle; >> + int kbd_lang; >> + >> + if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "GSKL", >&gskl_handle))) { >> + /* Platform doesn't support GSKL */ >> + return -ENODEV; >> + } >> + >> + if (!acpi_evalf(gskl_handle, &kbd_lang, NULL, "dd", 0x02000000)) >> + return -EIO; >> + >> + *output = kbd_lang; >> + >> + return 0; >> +} >> + >> +/* sysfs keyboard language entry */ >> +static ssize_t keyboard_lang_show(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + int output, err, i; >> + char select_lang[80] = ""; >> + char lang[8] = ""; >> + >> + err = get_keyboard_lang(&output); >> + if (err) >> + return err; >> + >> + for (i = 0; i < ARRAY_SIZE(keyboard_lang_data); i++) { >> + if (i) >> + strcat(select_lang, " "); >> + >> + if (output == keyboard_lang_data[i].lang_code) { >> + strcat(lang, "["); >> + strcat(lang, keyboard_lang_data[i].lang_str); >> + strcat(lang, "]"); >> + strcat(select_lang, lang); >> + } else { >> + strcat(select_lang, keyboard_lang_data[i].lang_str); >> + } >> + } >> + >> + return sysfs_emit(buf, "%s\n", select_lang); } >> + >> +static ssize_t keyboard_lang_store(struct device *dev, >> + struct device_attribute *attr, >> + const char *buf, size_t count) >> +{ >> + int err, i; >> + bool lang_found = false; >> + int lang_code = 0; >> + >> + for (i = 0; i < ARRAY_SIZE(keyboard_lang_data); i++) { >> + if (sysfs_streq(buf, keyboard_lang_data[i].lang_str)) { >> + lang_code = keyboard_lang_data[i].lang_code; >> + lang_found = true; >> + break; >> + } >> + } >> + >> + if (lang_found) { >> + lang_code = lang_code | 1 << 24; >> + >> + /* Set language code */ >> + err = set_keyboard_lang_command(lang_code); >> + if (err) >> + return err; >> + } else { >> + pr_err("Unknown Keyboard language. Ignoring\n"); >> + return -EINVAL; >> + } >> + >> + tpacpi_disclose_usertask(attr->attr.name, >> + "keyboard language is set to %s\n", buf); >> + >> + sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "keyboard_lang"); >> + >> + return count; >> +} >> + >> +static DEVICE_ATTR_RW(keyboard_lang); >> + >> +static struct attribute *kbdlang_attributes[] = { >> + &dev_attr_keyboard_lang.attr, >> + NULL >> +}; >> + >> +static const struct attribute_group kbdlang_attr_group = { >> + .attrs = kbdlang_attributes, >> +}; >> + >> +static int tpacpi_kbdlang_init(struct ibm_init_struct *iibm) { >> + int err, output; >> + >> + err = get_keyboard_lang(&output); >> + /* >> + * If support isn't available (ENODEV) then don't return an error >> + * just don't create the sysfs group >> + */ >> + if (err == -ENODEV) >> + return 0; >> + >> + if (err) >> + return err; >> + >> + /* Platform supports this feature - create the sysfs file */ >> + err = sysfs_create_group(&tpacpi_pdev->dev.kobj, >> +&kbdlang_attr_group); >> + >> + return err; >> +} >> + >> +static void kbdlang_exit(void) >> +{ >> + sysfs_remove_group(&tpacpi_pdev->dev.kobj, >&kbdlang_attr_group); } >> + >> +static struct ibm_struct kbdlang_driver_data = { >> + .name = "kbdlang", >> + .exit = kbdlang_exit, >> +}; >> + >> >/**************************************************************** >************ >> >***************************************************************** >*********** >> * >> @@ -10474,6 +10651,11 @@ static struct ibm_init_struct ibms_init[] >__initdata = { >> .init = tpacpi_proxsensor_init, >> .data = &proxsensor_driver_data, >> }, >> + { >> + .init = tpacpi_kbdlang_init, >> + .data = &kbdlang_driver_data, >> + }, >> + >> }; >> >> static int __init set_ibm_param(const char *val, const struct >> kernel_param *kp) >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [v2] platform/x86: thinkpad_acpi: set keyboard language 2021-01-25 2:59 [PATCH] [v2] platform/x86: thinkpad_acpi: set keyboard language Nitin Joshi 2021-01-25 20:33 ` Hans de Goede @ 2021-01-25 21:30 ` Andy Shevchenko 2021-01-26 14:22 ` Andy Shevchenko [not found] ` <SG2PR03MB27186C2289B137A914B9F0D28CBC0@SG2PR03MB2718.apcprd03.prod.outlook.com> 1 sibling, 2 replies; 8+ messages in thread From: Andy Shevchenko @ 2021-01-25 21:30 UTC (permalink / raw) To: Nitin Joshi Cc: Hans de Goede, Benjamin Berg, peter.hutterer, maruichit, Mark Pearson, Platform Driver, Nitin Joshi On Mon, Jan 25, 2021 at 5:03 AM Nitin Joshi <nitjoshi@gmail.com> wrote: > > From: Nitin Joshi <njoshi1@lenovo.com> > Maybe it's a bit late, but... nevertheless my comments below. > This patch is to create sysfs entry for setting keyboard language create a sysfs > using ASL method. Some thinkpads models like T580 , T590 , T15 Gen 1 > etc. has "=", "(',")" numeric keys, which are not displaying correctly, > when keyboard language is other than "english". > This patch fixes this issue by setting keyboard language to ECFW. > +This feature is used to set keyboard language to ECFW using ASL interface. > +Fewer thinkpads models like T580 , T590 , T15 Gen 1 etc.. has "=", "(', > +")" numeric keys, which are not displaying correctly, when keyboard language > +is other than "english". This is because of default keyboard language in ECFW because the default > +is set as "english". Hence using this sysfs, user can set correct keyboard the user can set the correct > +language to ECFW and then these key's will work correctly . > + > +Example of command to set keyboard language is mentioned below:: > + > + echo jp > /sys/devices/platform/thinkpad_acpi/keyboard_lang > + > +Text corresponding to keyboard layout to be set in sysfs are : jp (Japan), be(Belgian), > +cz(Czech), en(English), da(Danish), de(German), es(Spain) , et(Estonian), > +fr(French) , fr-ch (French(Switzerland)), pl(Polish), sl(Slovenian), hu > +(Hungarian), nl(Dutch), tr(Turkey), it(Italy), sv(Sweden), pt(portugese) Can we keep this sorted? Also see below. ... > +struct keyboard_lang_data keyboard_lang_data[] = { > + {"en", 0}, 0x0000 ? > + {"be", 0x080c}, > + {"cz", 0x0405}, > + {"da", 0x0406}, > + {"de", 0x0c07}, > + {"es", 0x2c0a}, > + {"et", 0x0425}, > + {"fr", 0x040c}, > + {"fr-ch", 0x100c}, > + {"hu", 0x040e}, > + {"it", 0x0410}, > + {"jp", 0x0411}, > + {"nl", 0x0413}, > + {"nn", 0x0414}, > + {"pl", 0x0415}, > + {"pt", 0x0816}, > + {"sl", 0x041b}, > + {"sv", 0x081d}, > + {"tr", 0x041f}, > +}; So, the above definitely has a meaning of the second byte as bit field. I believe that be is something like be-be and so on. We have 0x04,0x08, 0x0c, 0x2c, and 0x10. 0x04 seems like a basic variant, what about the rest? ... > + if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "GSKL", &gskl_handle))) { > + /* Platform doesn't support GSKL */ > + return -ENODEV; Perhaps EOPNOTSUPP is better? > + } ... > + char select_lang[80] = ""; > + char lang[8] = ""; I don't think we need assignments and moreover, we need these variables. See below for the details. > + > + err = get_keyboard_lang(&output); > + if (err) > + return err; > + > + for (i = 0; i < ARRAY_SIZE(keyboard_lang_data); i++) { > + if (i) > + strcat(select_lang, " "); > + > + if (output == keyboard_lang_data[i].lang_code) { > + strcat(lang, "["); > + strcat(lang, keyboard_lang_data[i].lang_str); > + strcat(lang, "]"); > + strcat(select_lang, lang); > + } else { > + strcat(select_lang, keyboard_lang_data[i].lang_str); > + } > + } > + > + return sysfs_emit(buf, "%s\n", select_lang); We have sysfs_emit_at(), please use it instead of these ugly str*() calls. > +} > + > +static ssize_t keyboard_lang_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + int err, i; > + bool lang_found = false; Redundant variable. > + int lang_code = 0; > + > + for (i = 0; i < ARRAY_SIZE(keyboard_lang_data); i++) { > + if (sysfs_streq(buf, keyboard_lang_data[i].lang_str)) { > + lang_code = keyboard_lang_data[i].lang_code; > + lang_found = true; > + break; > + } > + } Don't we have sysfs_match_string() ? > + if (lang_found) { Use traditional pattern, like ret = sysfs_match_string(...); if (ret < 0) return ret; > + lang_code = lang_code | 1 << 24; > + > + /* Set language code */ > + err = set_keyboard_lang_command(lang_code); > + if (err) > + return err; > + } else { > + pr_err("Unknown Keyboard language. Ignoring\n"); Why not dev_err() ? > + return -EINVAL; > + } > + > + tpacpi_disclose_usertask(attr->attr.name, > + "keyboard language is set to %s\n", buf); > + > + sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "keyboard_lang"); > + > + return count; > +} > + Redundant blank line. > +static DEVICE_ATTR_RW(keyboard_lang); ... > + /* > + * If support isn't available (ENODEV) then don't return an error > + * just don't create the sysfs group Missed period. > + */ ... > + /* Platform supports this feature - create the sysfs file */ > + err = sysfs_create_group(&tpacpi_pdev->dev.kobj, &kbdlang_attr_group); > + > + return err; return sysfs_create_group(); > +} -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [v2] platform/x86: thinkpad_acpi: set keyboard language 2021-01-25 21:30 ` Andy Shevchenko @ 2021-01-26 14:22 ` Andy Shevchenko 2021-01-26 16:02 ` [External] " Nitin Joshi1 [not found] ` <SG2PR03MB27186C2289B137A914B9F0D28CBC0@SG2PR03MB2718.apcprd03.prod.outlook.com> 1 sibling, 1 reply; 8+ messages in thread From: Andy Shevchenko @ 2021-01-26 14:22 UTC (permalink / raw) To: Nitin Joshi Cc: Hans de Goede, Benjamin Berg, peter.hutterer, maruichit, Mark Pearson, Platform Driver, Nitin Joshi On Mon, Jan 25, 2021 at 11:30 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Mon, Jan 25, 2021 at 5:03 AM Nitin Joshi <nitjoshi@gmail.com> wrote: > > > > From: Nitin Joshi <njoshi1@lenovo.com> > Maybe it's a bit late, but... nevertheless my comments below. Nitin, if you are not going to submit a series of fix ups as per my comments I would probably do it myself later on. Just tell what you want on this matter, -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [External] Re: [PATCH] [v2] platform/x86: thinkpad_acpi: set keyboard language 2021-01-26 14:22 ` Andy Shevchenko @ 2021-01-26 16:02 ` Nitin Joshi1 0 siblings, 0 replies; 8+ messages in thread From: Nitin Joshi1 @ 2021-01-26 16:02 UTC (permalink / raw) To: Andy Shevchenko, Nitin Joshi Cc: Hans de Goede, Benjamin Berg, peter.hutterer, Tomoki Maruichi, Mark Pearson, Platform Driver Hello Andy , >-----Original Message----- >From: Andy Shevchenko <andy.shevchenko@gmail.com> >Sent: Tuesday, January 26, 2021 11:22 PM >To: Nitin Joshi <nitjoshi@gmail.com> >Cc: Hans de Goede <hdegoede@redhat.com>; Benjamin Berg ><bberg@redhat.com>; peter.hutterer@redhat.com; Tomoki Maruichi ><maruichit@lenovo.com>; Mark Pearson <mpearson@lenovo.com>; Platform >Driver <platform-driver-x86@vger.kernel.org>; Nitin Joshi1 ><njoshi1@lenovo.com> >Subject: [External] Re: [PATCH] [v2] platform/x86: thinkpad_acpi: set keyboard >language > >On Mon, Jan 25, 2021 at 11:30 PM Andy Shevchenko ><andy.shevchenko@gmail.com> wrote: >> >> On Mon, Jan 25, 2021 at 5:03 AM Nitin Joshi <nitjoshi@gmail.com> wrote: >> > >> > From: Nitin Joshi <njoshi1@lenovo.com> > >> Maybe it's a bit late, but... nevertheless my comments below. > >Nitin, if you are not going to submit a series of fix ups as per my comments I >would probably do it myself later on. Just tell what you want on this matter, Thank you for your comments and its helpful for me as I have just started pushing patch for upstream. Apologize for not replying today morning as I wanted to check it in detail. I have just replied on your review comment e-mail and will follow-up on that e-mail chain.. > >-- >With Best Regards, >Andy Shevchenko Thanks & Regards, Nitin Joshi ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <SG2PR03MB27186C2289B137A914B9F0D28CBC0@SG2PR03MB2718.apcprd03.prod.outlook.com>]
* Re: [External] Re: [PATCH] [v2] platform/x86: thinkpad_acpi: set keyboard language [not found] ` <SG2PR03MB27186C2289B137A914B9F0D28CBC0@SG2PR03MB2718.apcprd03.prod.outlook.com> @ 2021-01-26 16:03 ` Hans de Goede 2021-01-26 16:58 ` Nitin Joshi1 0 siblings, 1 reply; 8+ messages in thread From: Hans de Goede @ 2021-01-26 16:03 UTC (permalink / raw) To: Nitin Joshi1, Andy Shevchenko, Nitin Joshi Cc: Benjamin Berg, peter.hutterer, Tomoki Maruichi, Mark Pearson, Platform Driver Hi, On 1/26/21 4:58 PM, Nitin Joshi1 wrote: > Hello Andy , > >> -----Original Message----- >> From: Andy Shevchenko <andy.shevchenko@gmail.com> >> Sent: Tuesday, January 26, 2021 6:31 AM >> To: Nitin Joshi <nitjoshi@gmail.com> >> Cc: Hans de Goede <hdegoede@redhat.com>; Benjamin Berg >> <bberg@redhat.com>; peter.hutterer@redhat.com; Tomoki Maruichi >> <maruichit@lenovo.com>; Mark Pearson <mpearson@lenovo.com>; Platform >> Driver <platform-driver-x86@vger.kernel.org>; Nitin Joshi1 >> <njoshi1@lenovo.com> >> Subject: [External] Re: [PATCH] [v2] platform/x86: thinkpad_acpi: set keyboard >> language >> >> On Mon, Jan 25, 2021 at 5:03 AM Nitin Joshi <nitjoshi@gmail.com> wrote: >>> >>> From: Nitin Joshi <njoshi1@lenovo.com> >>> >> >> Maybe it's a bit late, but... nevertheless my comments below. >> > Thank you for comments . It was my mistake , I should have put mailing list before . > >>> This patch is to create sysfs entry for setting keyboard language >> >> create a sysfs >> >>> using ASL method. Some thinkpads models like T580 , T590 , T15 Gen 1 >>> etc. has "=", "(',")" numeric keys, which are not displaying >>> correctly, when keyboard language is other than "english". >>> This patch fixes this issue by setting keyboard language to ECFW. >> >>> +This feature is used to set keyboard language to ECFW using ASL interface. >>> +Fewer thinkpads models like T580 , T590 , T15 Gen 1 etc.. has "=", >>> +"(', ")" numeric keys, which are not displaying correctly, when >>> +keyboard language is other than "english". This is because of default >>> +keyboard language in ECFW >> >> because the default > > Ack . I will correct it. > >> >>> +is set as "english". Hence using this sysfs, user can set correct >>> +keyboard >> >> the user can set the correct > > Ack . I will correct it . > >> >>> +language to ECFW and then these key's will work correctly . >>> + >>> +Example of command to set keyboard language is mentioned below:: >>> + >>> + echo jp > /sys/devices/platform/thinkpad_acpi/keyboard_lang >>> + >>> +Text corresponding to keyboard layout to be set in sysfs are : jp >>> +(Japan), be(Belgian), cz(Czech), en(English), da(Danish), de(German), >>> +es(Spain) , et(Estonian), >>> +fr(French) , fr-ch (French(Switzerland)), pl(Polish), sl(Slovenian), >>> +hu (Hungarian), nl(Dutch), tr(Turkey), it(Italy), sv(Sweden), >>> +pt(portugese) >> >> Can we keep this sorted? > Ack . I will make this sorted. > >> Also see below. >> >> ... >> >>> +struct keyboard_lang_data keyboard_lang_data[] = { >>> + {"en", 0}, >> >> 0x0000 ? > Ack . yes , I will update it. While at it can you also please mark the table static const ? > >> >>> + {"be", 0x080c}, >>> + {"cz", 0x0405}, >>> + {"da", 0x0406}, >>> + {"de", 0x0c07}, >>> + {"es", 0x2c0a}, >>> + {"et", 0x0425}, >>> + {"fr", 0x040c}, >>> + {"fr-ch", 0x100c}, >>> + {"hu", 0x040e}, >>> + {"it", 0x0410}, >>> + {"jp", 0x0411}, >>> + {"nl", 0x0413}, >>> + {"nn", 0x0414}, >>> + {"pl", 0x0415}, >>> + {"pt", 0x0816}, >>> + {"sl", 0x041b}, >>> + {"sv", 0x081d}, >>> + {"tr", 0x041f}, >>> +}; >> >> So, the above definitely has a meaning of the second byte as bit field. I believe >> that be is something like be-be and so on. > > Language Id is based on Language and Location . > You can refer attached pdf file for example 0x0411 is for Japanese layout but 0x04 is not basic > Currently , Lenovo ECFW can change keyboard layout for only few language id by "SSKL" ASL method to fix > "=", "(', ")" numeric keys issue . hence , I have added only few language id (Actually I have received language id supported list from ECFW team). > >> We have 0x04,0x08, 0x0c, 0x2c, and 0x10. 0x04 seems like a basic variant, > By seeing the attached pdf , it seems not a basic variant. > >> what about the rest? > As per my understanding , ECFW is either not supporting rest or keyboard layout of others is similar to "English". > If in future , ECFW supports any other keyboard layout then we can add it . > >> >> ... >> >>> + if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "GSKL", >> &gskl_handle))) { >>> + /* Platform doesn't support GSKL */ >>> + return -ENODEV; >> >> Perhaps EOPNOTSUPP is better? > Ack . > How about keeping ENODEV and changing only text as below : > /* Platform doesn't have GSKL method */ > As per my understanding , in this case we will get ACPI failue only when GSKL method doesn’t exists in BIOS . > So , I think its better to keep ENODEV. > > Please correct me , if I am wrong. I think we should keep -ENODEV here, since this path is hit on devices which do not have the kbd-lang feature and returning -ENODEV from the getter in this case is how this is handled in most other getters in thinkpad_acpi. >> >>> + } >> >> ... >> >>> + char select_lang[80] = ""; >>> + char lang[8] = ""; >> >> I don't think we need assignments and moreover, we need these variables. >> See below for the details. >> > Ack . > >>> + >>> + err = get_keyboard_lang(&output); >>> + if (err) >>> + return err; >>> + >>> + for (i = 0; i < ARRAY_SIZE(keyboard_lang_data); i++) { >>> + if (i) >>> + strcat(select_lang, " "); >>> + >>> + if (output == keyboard_lang_data[i].lang_code) { >>> + strcat(lang, "["); >>> + strcat(lang, keyboard_lang_data[i].lang_str); >>> + strcat(lang, "]"); >>> + strcat(select_lang, lang); >>> + } else { >>> + strcat(select_lang, keyboard_lang_data[i].lang_str); >>> + } >>> + } >>> + >>> + return sysfs_emit(buf, "%s\n", select_lang); >> >> We have sysfs_emit_at(), please use it instead of these ugly str*() calls. > Ack . I will check about sysfs_emit_at() and get back on this . > Thanks for suggestion. > >> >>> +} >>> + >>> +static ssize_t keyboard_lang_store(struct device *dev, >>> + struct device_attribute *attr, >>> + const char *buf, size_t count) { >>> + int err, i; >> >>> + bool lang_found = false; >> >> Redundant variable. > > Ack . I think if we use sysfs_match_string() then it will be redundant . > But , I need to check it . sysfs_match_string() only works on arrays of strings, in this case we have an array of struct keyboard_lang_data, so using it here will not work. > >> >>> + int lang_code = 0; >>> + >>> + for (i = 0; i < ARRAY_SIZE(keyboard_lang_data); i++) { >>> + if (sysfs_streq(buf, keyboard_lang_data[i].lang_str)) { >>> + lang_code = keyboard_lang_data[i].lang_code; >>> + lang_found = true; >>> + break; >>> + } >>> + } >> >> Don't we have sysfs_match_string() ? > Ack . I will check it and get back . > >> >>> + if (lang_found) { >> >> Use traditional pattern, like >> >> ret = sysfs_match_string(...); >> if (ret < 0) >> return ret; >> > Noted > >>> + lang_code = lang_code | 1 << 24; >>> + >>> + /* Set language code */ >>> + err = set_keyboard_lang_command(lang_code); >>> + if (err) >>> + return err; >>> + } else { >> >>> + pr_err("Unknown Keyboard language. Ignoring\n"); >> >> Why not dev_err() ? > > Ack . I will change it . The reason for using dev_err will only to provide better message to userspace . is it correct ? >> >>> + return -EINVAL; >>> + } >>> + >>> + tpacpi_disclose_usertask(attr->attr.name, >>> + "keyboard language is set to %s\n", buf); >>> + >>> + sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "keyboard_lang"); >>> + >>> + return count; >>> +} >> >>> + >> >> Redundant blank line. >> > Ack. I will correct it. > >>> +static DEVICE_ATTR_RW(keyboard_lang); >> >> ... >> >>> + /* >>> + * If support isn't available (ENODEV) then don't return an error >>> + * just don't create the sysfs group >> >> Missed period. >> > Ack . I will correct it. > >>> + */ >> >> ... >> >>> + /* Platform supports this feature - create the sysfs file */ >>> + err = sysfs_create_group(&tpacpi_pdev->dev.kobj, >>> + &kbdlang_attr_group); >>> + >>> + return err; >> >> return sysfs_create_group(); >> > > Ack . I will correct it . > >>> +} >> >> -- >> With Best Regards, >> Andy Shevchenko > > I will update patch on top of my last patch and send it for review by next week . > Please correct me , if I am missing something as I have just started pushing patch for upstream. Sounds good, thank you. Regards, Hans ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [External] Re: [PATCH] [v2] platform/x86: thinkpad_acpi: set keyboard language 2021-01-26 16:03 ` Hans de Goede @ 2021-01-26 16:58 ` Nitin Joshi1 0 siblings, 0 replies; 8+ messages in thread From: Nitin Joshi1 @ 2021-01-26 16:58 UTC (permalink / raw) To: Hans de Goede, Andy Shevchenko, Nitin Joshi Cc: Benjamin Berg, peter.hutterer, Tomoki Maruichi, Mark Pearson, Platform Driver Hello Hans , >-----Original Message----- >From: Hans de Goede <hdegoede@redhat.com> >Subject: Re: [External] Re: [PATCH] [v2] platform/x86: thinkpad_acpi: set >keyboard language > >Hi, > >On 1/26/21 4:58 PM, Nitin Joshi1 wrote: >> Hello Andy , >> >>> -----Original Message----- >>> From: Andy Shevchenko <andy.shevchenko@gmail.com> >>> Sent: Tuesday, January 26, 2021 6:31 AM >>> To: Nitin Joshi <nitjoshi@gmail.com> >>> Cc: Hans de Goede <hdegoede@redhat.com>; Benjamin Berg >>> <bberg@redhat.com>; peter.hutterer@redhat.com; Tomoki Maruichi >>> <maruichit@lenovo.com>; Mark Pearson <mpearson@lenovo.com>; >Platform >>> Driver <platform-driver-x86@vger.kernel.org>; Nitin Joshi1 >>> <njoshi1@lenovo.com> >>> Subject: [External] Re: [PATCH] [v2] platform/x86: thinkpad_acpi: set >>> keyboard language >>> >>> On Mon, Jan 25, 2021 at 5:03 AM Nitin Joshi <nitjoshi@gmail.com> wrote: >>>> >>>> From: Nitin Joshi <njoshi1@lenovo.com> >>>> >>> >>> Maybe it's a bit late, but... nevertheless my comments below. >>> >> Thank you for comments . It was my mistake , I should have put mailing list >before . >> >>>> This patch is to create sysfs entry for setting keyboard language >>> >>> create a sysfs >>> >>>> using ASL method. Some thinkpads models like T580 , T590 , T15 Gen 1 >>>> etc. has "=", "(',")" numeric keys, which are not displaying >>>> correctly, when keyboard language is other than "english". >>>> This patch fixes this issue by setting keyboard language to ECFW. >>> >>>> +This feature is used to set keyboard language to ECFW using ASL >interface. >>>> +Fewer thinkpads models like T580 , T590 , T15 Gen 1 etc.. has "=", >>>> +"(', ")" numeric keys, which are not displaying correctly, when >>>> +keyboard language is other than "english". This is because of >>>> +default keyboard language in ECFW >>> >>> because the default >> >> Ack . I will correct it. >> >>> >>>> +is set as "english". Hence using this sysfs, user can set correct >>>> +keyboard >>> >>> the user can set the correct >> >> Ack . I will correct it . >> >>> >>>> +language to ECFW and then these key's will work correctly . >>>> + >>>> +Example of command to set keyboard language is mentioned below:: >>>> + >>>> + echo jp > /sys/devices/platform/thinkpad_acpi/keyboard_lang >>>> + >>>> +Text corresponding to keyboard layout to be set in sysfs are : jp >>>> +(Japan), be(Belgian), cz(Czech), en(English), da(Danish), >>>> +de(German), >>>> +es(Spain) , et(Estonian), >>>> +fr(French) , fr-ch (French(Switzerland)), pl(Polish), >>>> +sl(Slovenian), hu (Hungarian), nl(Dutch), tr(Turkey), it(Italy), >>>> +sv(Sweden), >>>> +pt(portugese) >>> >>> Can we keep this sorted? >> Ack . I will make this sorted. >> >>> Also see below. >>> >>> ... >>> >>>> +struct keyboard_lang_data keyboard_lang_data[] = { >>>> + {"en", 0}, >>> >>> 0x0000 ? >> Ack . yes , I will update it. > >While at it can you also please mark the table static const ? Yes , I will modify it . > >> >>> >>>> + {"be", 0x080c}, >>>> + {"cz", 0x0405}, >>>> + {"da", 0x0406}, >>>> + {"de", 0x0c07}, >>>> + {"es", 0x2c0a}, >>>> + {"et", 0x0425}, >>>> + {"fr", 0x040c}, >>>> + {"fr-ch", 0x100c}, >>>> + {"hu", 0x040e}, >>>> + {"it", 0x0410}, >>>> + {"jp", 0x0411}, >>>> + {"nl", 0x0413}, >>>> + {"nn", 0x0414}, >>>> + {"pl", 0x0415}, >>>> + {"pt", 0x0816}, >>>> + {"sl", 0x041b}, >>>> + {"sv", 0x081d}, >>>> + {"tr", 0x041f}, >>>> +}; >>> >>> So, the above definitely has a meaning of the second byte as bit >>> field. I believe that be is something like be-be and so on. >> >> Language Id is based on Language and Location . >> You can refer attached pdf file for example 0x0411 is for Japanese >> layout but 0x04 is not basic Currently , Lenovo ECFW can change >> keyboard layout for only few language id by "SSKL" ASL method to fix "=", "(', >")" numeric keys issue . hence , I have added only few language id (Actually I >have received language id supported list from ECFW team). >> >>> We have 0x04,0x08, 0x0c, 0x2c, and 0x10. 0x04 seems like a basic >>> variant, >> By seeing the attached pdf , it seems not a basic variant. >> >>> what about the rest? >> As per my understanding , ECFW is either not supporting rest or keyboard >layout of others is similar to "English". >> If in future , ECFW supports any other keyboard layout then we can add it . >> >>> >>> ... >>> >>>> + if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "GSKL", >>> &gskl_handle))) { >>>> + /* Platform doesn't support GSKL */ >>>> + return -ENODEV; >>> >>> Perhaps EOPNOTSUPP is better? >> Ack . >> How about keeping ENODEV and changing only text as below : >> /* Platform doesn't have GSKL method */ As per my understanding , in >> this case we will get ACPI failue only when GSKL method doesn’t exists in >BIOS . >> So , I think its better to keep ENODEV. >> >> Please correct me , if I am wrong. > >I think we should keep -ENODEV here, since this path is hit on devices which >do not have the kbd-lang feature and returning -ENODEV from the getter in >this case is how this is handled in most other getters in thinkpad_acpi. > Noted . >>> >>>> + } >>> >>> ... >>> >>>> + char select_lang[80] = ""; >>>> + char lang[8] = ""; >>> >>> I don't think we need assignments and moreover, we need these variables. >>> See below for the details. >>> >> Ack . >> >>>> + >>>> + err = get_keyboard_lang(&output); >>>> + if (err) >>>> + return err; >>>> + >>>> + for (i = 0; i < ARRAY_SIZE(keyboard_lang_data); i++) { >>>> + if (i) >>>> + strcat(select_lang, " "); >>>> + >>>> + if (output == keyboard_lang_data[i].lang_code) { >>>> + strcat(lang, "["); >>>> + strcat(lang, keyboard_lang_data[i].lang_str); >>>> + strcat(lang, "]"); >>>> + strcat(select_lang, lang); >>>> + } else { >>>> + strcat(select_lang, keyboard_lang_data[i].lang_str); >>>> + } >>>> + } >>>> + >>>> + return sysfs_emit(buf, "%s\n", select_lang); >>> >>> We have sysfs_emit_at(), please use it instead of these ugly str*() calls. >> Ack . I will check about sysfs_emit_at() and get back on this . >> Thanks for suggestion. >> >>> >>>> +} >>>> + >>>> +static ssize_t keyboard_lang_store(struct device *dev, >>>> + struct device_attribute *attr, >>>> + const char *buf, size_t count) { >>>> + int err, i; >>> >>>> + bool lang_found = false; >>> >>> Redundant variable. >> >> Ack . I think if we use sysfs_match_string() then it will be redundant . >> But , I need to check it . > >sysfs_match_string() only works on arrays of strings, in this case we have an >array of struct keyboard_lang_data, so using it here will not work. Noted with thanks . > >> >>> >>>> + int lang_code = 0; >>>> + >>>> + for (i = 0; i < ARRAY_SIZE(keyboard_lang_data); i++) { >>>> + if (sysfs_streq(buf, keyboard_lang_data[i].lang_str)) { >>>> + lang_code = keyboard_lang_data[i].lang_code; >>>> + lang_found = true; >>>> + break; >>>> + } >>>> + } >>> >>> Don't we have sysfs_match_string() ? >> Ack . I will check it and get back . >> >>> >>>> + if (lang_found) { >>> >>> Use traditional pattern, like >>> >>> ret = sysfs_match_string(...); >>> if (ret < 0) >>> return ret; >>> >> Noted >> >>>> + lang_code = lang_code | 1 << 24; >>>> + >>>> + /* Set language code */ >>>> + err = set_keyboard_lang_command(lang_code); >>>> + if (err) >>>> + return err; >>>> + } else { >>> >>>> + pr_err("Unknown Keyboard language. Ignoring\n"); >>> >>> Why not dev_err() ? >> >> Ack . I will change it . The reason for using dev_err will only to provide better >message to userspace . is it correct ? >>> >>>> + return -EINVAL; >>>> + } >>>> + >>>> + tpacpi_disclose_usertask(attr->attr.name, >>>> + "keyboard language is set to %s\n", buf); >>>> + >>>> + sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "keyboard_lang"); >>>> + >>>> + return count; >>>> +} >>> >>>> + >>> >>> Redundant blank line. >>> >> Ack. I will correct it. >> >>>> +static DEVICE_ATTR_RW(keyboard_lang); >>> >>> ... >>> >>>> + /* >>>> + * If support isn't available (ENODEV) then don't return an error >>>> + * just don't create the sysfs group >>> >>> Missed period. >>> >> Ack . I will correct it. >> >>>> + */ >>> >>> ... >>> >>>> + /* Platform supports this feature - create the sysfs file */ >>>> + err = sysfs_create_group(&tpacpi_pdev->dev.kobj, >>>> + &kbdlang_attr_group); >>>> + >>>> + return err; >>> >>> return sysfs_create_group(); >>> >> >> Ack . I will correct it . >> >>>> +} >>> >>> -- >>> With Best Regards, >>> Andy Shevchenko >> >> I will update patch on top of my last patch and send it for review by next >week . >> Please correct me , if I am missing something as I have just started pushing >patch for upstream. > >Sounds good, thank you. > >Regards, > >Hans Thanks & Regards, Nitin Joshi ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-01-27 0:59 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-25 2:59 [PATCH] [v2] platform/x86: thinkpad_acpi: set keyboard language Nitin Joshi 2021-01-25 20:33 ` Hans de Goede 2021-01-26 0:21 ` [External] " Nitin Joshi1 2021-01-25 21:30 ` Andy Shevchenko 2021-01-26 14:22 ` Andy Shevchenko 2021-01-26 16:02 ` [External] " Nitin Joshi1 [not found] ` <SG2PR03MB27186C2289B137A914B9F0D28CBC0@SG2PR03MB2718.apcprd03.prod.outlook.com> 2021-01-26 16:03 ` Hans de Goede 2021-01-26 16:58 ` Nitin Joshi1
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.