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

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Wed, Mar 29, 2017 at 11:59 AM, Mika Westerberg
<mika.westerberg@xxxxxxxxxxxxxxx> wrote:

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

Well ain't that great. They made the totally unstable Linux IRQ number
space into an ABI.

I wonder what the irqchip people think about that. I think Grant warned us
that this could happen.

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

Quirking seems right. But:

> +/*
> + * 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"),
> +               },
> +       }
> +};

We match but...

>         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 +1557,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;

Isn't the right solution to translate this back to the offset from the "Linux
IRQ" and use that offset? This quirk seems pretty violent.

Yours,
Linus Walleij
--
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