| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CACK8Z6F4+N1paQTSWcomnwLpLEHoospSOT3M9bqFQZHHfWmahA@mail.gmail.com>
Date: Tue, 18 Sep 2018 15:04:23 -0700
From: Rajat Jain <rajatja@...gle.com>
To: Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Linus Walleij <linus.walleij@...aro.org>,
casey.g.bowman@...el.com,
"Atwood, Matthew S" <matthew.s.atwood@...el.com>,
linux-gpio@...r.kernel.org,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] pinctrl: intel: Do pin translation in other GPIO
operations as well
On Tue, Sep 18, 2018 at 8:36 AM Mika Westerberg
<mika.westerberg@...ux.intel.com> wrote:
>
> For some reason I thought GPIOLIB handles translation from GPIO ranges
> to pinctrl pins but it turns out not to be the case. This means that
> when GPIOs operations are performed for a pin controller having a custom
> GPIO base such as Cannon Lake and Ice Lake incorrect pin number gets
> used internally.
>
> Fix this in the same way we did for lock/unlock IRQ operations and
> translate the GPIO number to pin before using it.
>
> Fixes: a60eac3239f0 ("pinctrl: intel: Allow custom GPIO base for pad groups")
> Reported-by: Rajat Jain <rajatja@...gle.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
Tested-by: Rajat Jain <rajatja@...gle.com>
This has fixed the issue for me.
One question, may not be related: I see this line in my logs everytime
I export a pin (GPIO40 = pin 16 in this case). Is that an indication
of a problem?
"gpio gpiochip0: Persistence not supported for GPIO 40"
Thanks,
Rajat
On Tue, Sep 18, 2018 at 8:36 AM Mika Westerberg
<mika.westerberg@...ux.intel.com> wrote:
>
> For some reason I thought GPIOLIB handles translation from GPIO ranges
> to pinctrl pins but it turns out not to be the case. This means that
> when GPIOs operations are performed for a pin controller having a custom
> GPIO base such as Cannon Lake and Ice Lake incorrect pin number gets
> used internally.
>
> Fix this in the same way we did for lock/unlock IRQ operations and
> translate the GPIO number to pin before using it.
>
> Fixes: a60eac3239f0 ("pinctrl: intel: Allow custom GPIO base for pad groups")
> Reported-by: Rajat Jain <rajatja@...gle.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
> ---
> drivers/pinctrl/intel/pinctrl-intel.c | 111 +++++++++++++++-----------
> 1 file changed, 63 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
> index 62b009b27eda..ec8dafc94694 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.c
> +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> @@ -747,13 +747,63 @@ static const struct pinctrl_desc intel_pinctrl_desc = {
> .owner = THIS_MODULE,
> };
>
> +/**
> + * intel_gpio_to_pin() - Translate from GPIO offset to pin number
> + * @pctrl: Pinctrl structure
> + * @offset: GPIO offset from gpiolib
> + * @commmunity: Community is filled here if not %NULL
> + * @padgrp: Pad group is filled here if not %NULL
> + *
> + * When coming through gpiolib irqchip, the GPIO offset is not
> + * automatically translated to pinctrl pin number. This function can be
> + * used to find out the corresponding pinctrl pin.
> + */
> +static int intel_gpio_to_pin(struct intel_pinctrl *pctrl, unsigned offset,
> + const struct intel_community **community,
> + const struct intel_padgroup **padgrp)
> +{
> + int i;
> +
> + for (i = 0; i < pctrl->ncommunities; i++) {
> + const struct intel_community *comm = &pctrl->communities[i];
> + int j;
> +
> + for (j = 0; j < comm->ngpps; j++) {
> + const struct intel_padgroup *pgrp = &comm->gpps[j];
> +
> + if (pgrp->gpio_base < 0)
> + continue;
> +
> + if (offset >= pgrp->gpio_base &&
> + offset < pgrp->gpio_base + pgrp->size) {
> + int pin;
> +
> + pin = pgrp->base + offset - pgrp->gpio_base;
> + if (community)
> + *community = comm;
> + if (padgrp)
> + *padgrp = pgrp;
> +
> + return pin;
> + }
> + }
> + }
> +
> + return -EINVAL;
> +}
> +
> static int intel_gpio_get(struct gpio_chip *chip, unsigned offset)
> {
> struct intel_pinctrl *pctrl = gpiochip_get_data(chip);
> void __iomem *reg;
> u32 padcfg0;
> + int pin;
> +
> + pin = intel_gpio_to_pin(pctrl, offset, NULL, NULL);
> + if (pin < 0)
> + return -EINVAL;
>
> - reg = intel_get_padcfg(pctrl, offset, PADCFG0);
> + reg = intel_get_padcfg(pctrl, pin, PADCFG0);
> if (!reg)
> return -EINVAL;
>
> @@ -770,8 +820,13 @@ static void intel_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
> unsigned long flags;
> void __iomem *reg;
> u32 padcfg0;
> + int pin;
> +
> + pin = intel_gpio_to_pin(pctrl, offset, NULL, NULL);
> + if (pin < 0)
> + return;
>
> - reg = intel_get_padcfg(pctrl, offset, PADCFG0);
> + reg = intel_get_padcfg(pctrl, pin, PADCFG0);
> if (!reg)
> return;
>
> @@ -790,8 +845,13 @@ static int intel_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
> struct intel_pinctrl *pctrl = gpiochip_get_data(chip);
> void __iomem *reg;
> u32 padcfg0;
> + int pin;
>
> - reg = intel_get_padcfg(pctrl, offset, PADCFG0);
> + pin = intel_gpio_to_pin(pctrl, offset, NULL, NULL);
> + if (pin < 0)
> + return -EINVAL;
> +
> + reg = intel_get_padcfg(pctrl, pin, PADCFG0);
> if (!reg)
> return -EINVAL;
>
> @@ -827,51 +887,6 @@ static const struct gpio_chip intel_gpio_chip = {
> .set_config = gpiochip_generic_config,
> };
>
> -/**
> - * intel_gpio_to_pin() - Translate from GPIO offset to pin number
> - * @pctrl: Pinctrl structure
> - * @offset: GPIO offset from gpiolib
> - * @commmunity: Community is filled here if not %NULL
> - * @padgrp: Pad group is filled here if not %NULL
> - *
> - * When coming through gpiolib irqchip, the GPIO offset is not
> - * automatically translated to pinctrl pin number. This function can be
> - * used to find out the corresponding pinctrl pin.
> - */
> -static int intel_gpio_to_pin(struct intel_pinctrl *pctrl, unsigned offset,
> - const struct intel_community **community,
> - const struct intel_padgroup **padgrp)
> -{
> - int i;
> -
> - for (i = 0; i < pctrl->ncommunities; i++) {
> - const struct intel_community *comm = &pctrl->communities[i];
> - int j;
> -
> - for (j = 0; j < comm->ngpps; j++) {
> - const struct intel_padgroup *pgrp = &comm->gpps[j];
> -
> - if (pgrp->gpio_base < 0)
> - continue;
> -
> - if (offset >= pgrp->gpio_base &&
> - offset < pgrp->gpio_base + pgrp->size) {
> - int pin;
> -
> - pin = pgrp->base + offset - pgrp->gpio_base;
> - if (community)
> - *community = comm;
> - if (padgrp)
> - *padgrp = pgrp;
> -
> - return pin;
> - }
> - }
> - }
> -
> - return -EINVAL;
> -}
> -
> static int intel_gpio_irq_reqres(struct irq_data *d)
> {
> struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> --
> 2.18.0
>
Powered by blists - more mailing lists