Re: [PATCH v2] pinctrl: cherryview: Add a quirk to make Acer Chromebook keyboard work again

[Adam Levy <theadamlevy@xxxxxxxxx>]

Mika,

Your patch v2 still works on my Edgar CYAN Acer Chromebook 14 (CB3-431).

Tested on latest upstream kernel 4.11rc6.r4.gc08e611b7d01.

Let me know if you need anything else.

Thank you,
Adam Levy

On Mon, Apr 10, 2017 at 2:16 AM, Mika Westerberg
<mika.westerberg@xxxxxxxxxxxxxxx> wrote:
> After commit 47c950d10202 ("pinctrl: cherryview: Do not add all
> southwest and north GPIOs to IRQ domain") the driver does not add all
> GPIOs to the irqdomain. The reason for that is that those GPIOs cannot
> generate IRQs at all, only GPEs (General Purpose Events). This causes
> Linux virtual IRQ numbering to change.
>
> However, it seems some CYAN Chromebooks, including Acer Chromebook
> hardcodes these Linux IRQ numbers in the ACPI tables of the machine.
> Since the numbering is different now, the IRQ meant for keyboard does
> not match the Linux virtual IRQ number anymore making the keyboard
> non-functional.
>
> Work this around by adding special quirk just for these machines where
> we add back all GPIOs to the irqdomain. Rest of the Cherryview/Braswell
> based machines will not be affected by the change.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=194945
> Fixes: 47c950d10202 ("pinctrl: cherryview: Do not add all southwest and north GPIOs to IRQ domain")
> Reported-by: Adam S Levy <theadamlevy@xxxxxxxxx>
> Signed-off-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> ---
> Changes from v1:
>  - Add BIOS date to DMI match string to keep this quirk from affecting
>    future BIOS versions where the issue might be fixed already.
>
> Marc, Linus,
>
> I went for adding the quirk here instead of quirking it in i8042 keyboard
> driver because there might be other devices hard-coding the Linux IRQ
> number. This way we only need to touch one driver.
>
> Adam,
>
> Can you please try if this patch still fixes the problem for you? Thanks.
>
>  drivers/pinctrl/intel/pinctrl-cherryview.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
> index f80134e3e0b6..9ff790174906 100644
> --- a/drivers/pinctrl/intel/pinctrl-cherryview.c
> +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
> @@ -13,6 +13,7 @@
>   * published by the Free Software Foundation.
>   */
>
> +#include <linux/dmi.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/init.h>
> @@ -1524,10 +1525,31 @@ static void chv_gpio_irq_handler(struct irq_desc *desc)
>         chained_irq_exit(chip, desc);
>  }
>
> +/*
> + * Certain machines seem to hardcode Linux IRQ numbers in their ACPI
> + * tables. Since we leave GPIOs that are not capable of generating
> + * interrupts out of the irqdomain the numbering will be different and
> + * cause devices using the hardcoded IRQ numbers fail. In order not to
> + * break such machines we will only mask pins from irqdomain if the machine
> + * is not listed below.
> + */
> +static const struct dmi_system_id chv_no_valid_mask[] = {
> +       {
> +               /* See https://bugzilla.kernel.org/show_bug.cgi?id=194945 */
> +               .ident = "Acer Chromebook (CYAN)",
> +               .matches = {
> +                       DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
> +                       DMI_MATCH(DMI_PRODUCT_NAME, "Edgar"),
> +                       DMI_MATCH(DMI_BIOS_DATE, "05/21/2016"),
> +               },
> +       }
> +};
> +
>  static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
>  {
>         const struct chv_gpio_pinrange *range;
>         struct gpio_chip *chip = &pctrl->chip;
> +       bool need_valid_mask = !dmi_check_system(chv_no_valid_mask);
>         int ret, i, offset;
>
>         *chip = chv_gpio_chip;
> @@ -1536,7 +1558,7 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
>         chip->label = dev_name(pctrl->dev);
>         chip->parent = pctrl->dev;
>         chip->base = -1;
> -       chip->irq_need_valid_mask = true;
> +       chip->irq_need_valid_mask = need_valid_mask;
>
>         ret = devm_gpiochip_add_data(pctrl->dev, chip, pctrl);
>         if (ret) {
> @@ -1567,7 +1589,7 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
>                 intsel &= CHV_PADCTRL0_INTSEL_MASK;
>                 intsel >>= CHV_PADCTRL0_INTSEL_SHIFT;
>
> -               if (intsel >= pctrl->community->nirqs)
> +               if (need_valid_mask && intsel >= pctrl->community->nirqs)
>                         clear_bit(i, chip->irq_valid_mask);
>         }
>
> --
> 2.11.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html