diff mbox series

pinctrl: intel: Do pin translation in other GPIO operations as well

Message ID 20180918153621.71984-1-mika.westerberg@linux.intel.com
State New
Headers show
Series pinctrl: intel: Do pin translation in other GPIO operations as well | expand

Commit Message

Mika Westerberg Sept. 18, 2018, 3:36 p.m. UTC
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@google.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/pinctrl/intel/pinctrl-intel.c | 111 +++++++++++++++-----------
 1 file changed, 63 insertions(+), 48 deletions(-)

Comments

Rajat Jain Sept. 18, 2018, 10:04 p.m. UTC | #1
On Tue, Sep 18, 2018 at 8:36 AM Mika Westerberg
<mika.westerberg@linux.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@google.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Tested-by: Rajat Jain <rajatja@google.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@linux.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@google.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.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
>
Rajat Jain Sept. 18, 2018, 10:14 p.m. UTC | #2
On Tue, Sep 18, 2018 at 3:04 PM Rajat Jain <rajatja@google.com> wrote:
>
> On Tue, Sep 18, 2018 at 8:36 AM Mika Westerberg
> <mika.westerberg@linux.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@google.com>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>
> Tested-by: Rajat Jain <rajatja@google.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"
>


Also consider fixing the checkpatch warning:

        Errors:
            * checkpatch.pl errors/warnings

            WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
            #48: FILE: drivers/pinctrl/intel/pinctrl-intel.c:764:
            +static int intel_gpio_to_pin(struct intel_pinctrl *pctrl,
unsigned offset,



> Thanks,
>
> Rajat
>
> On Tue, Sep 18, 2018 at 8:36 AM Mika Westerberg
> <mika.westerberg@linux.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@google.com>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.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
> >
Mika Westerberg Sept. 19, 2018, 8:36 a.m. UTC | #3
On Tue, Sep 18, 2018 at 03:04:23PM -0700, Rajat Jain wrote:
> On Tue, Sep 18, 2018 at 8:36 AM Mika Westerberg
> <mika.westerberg@linux.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@google.com>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> Tested-by: Rajat Jain <rajatja@google.com>
> 
> This has fixed the issue for me.

Thanks for testing!

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

It seems to be debug print if the underlying GPIO chip does not support
PIN_CONFIG_PERSIST_STATE (pinctrl-intel.c does not). I would not worry
about it.
Mika Westerberg Sept. 19, 2018, 8:40 a.m. UTC | #4
On Tue, Sep 18, 2018 at 03:14:44PM -0700, Rajat Jain wrote:
> Also consider fixing the checkpatch warning:
> 
>         Errors:
>             * checkpatch.pl errors/warnings
> 
>             WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
>             #48: FILE: drivers/pinctrl/intel/pinctrl-intel.c:764:
>             +static int intel_gpio_to_pin(struct intel_pinctrl *pctrl,
> unsigned offset,

Right, it is currently the "convention" used in Intel pinctrl drivers so
I did not want to change that single entry.

We should eventually convert the whole set of Intel pinctrl drivers to
use unsigned int instead of unsigned.
Rajat Jain Sept. 19, 2018, 5:35 p.m. UTC | #5
On Wed, Sep 19, 2018 at 1:40 AM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
>
> On Tue, Sep 18, 2018 at 03:14:44PM -0700, Rajat Jain wrote:
> > Also consider fixing the checkpatch warning:
> >
> >         Errors:
> >             * checkpatch.pl errors/warnings
> >
> >             WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> >             #48: FILE: drivers/pinctrl/intel/pinctrl-intel.c:764:
> >             +static int intel_gpio_to_pin(struct intel_pinctrl *pctrl,
> > unsigned offset,
>
> Right, it is currently the "convention" used in Intel pinctrl drivers so
> I did not want to change that single entry.
>
> We should eventually convert the whole set of Intel pinctrl drivers to
> use unsigned int instead of unsigned.

OK, Thanks for the clarification.
Linus Walleij Sept. 20, 2018, 3:26 p.m. UTC | #6
On Tue, Sep 18, 2018 at 8:36 AM Mika Westerberg
<mika.westerberg@linux.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@google.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

I applied this for fixes.

However when merging with devel I get some a merge conflict,
probably due to some cleanups from Andy.

I tried to fix it up (just use your code) but please check the
result.

Yours,
Linus Walleij
Mika Westerberg Sept. 21, 2018, 7:56 a.m. UTC | #7
On Thu, Sep 20, 2018 at 08:26:25AM -0700, Linus Walleij wrote:
> On Tue, Sep 18, 2018 at 8:36 AM Mika Westerberg
> <mika.westerberg@linux.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@google.com>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> I applied this for fixes.
> 
> However when merging with devel I get some a merge conflict,
> probably due to some cleanups from Andy.
> 
> I tried to fix it up (just use your code) but please check the
> result.

Looks good to me. Thanks!
diff mbox series

Patch

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);