Allows querying GPIO direction from the pad config register.
If the pad is not in GPIO mode, return an error.
Signed-off-by: Javier Arteaga <[email protected]>
---
This is needed by the drivers for the UP Squared board, an APL-based
platform. (For now, these drivers are out-of-tree.)
An earlier version of this patch was reviewed some time ago by Andy
Shevchenko outside of the mailing lists:
<https://github.com/emutex/ubilinux-kernel/commit/708a776f2ffd1b1c>
drivers/pinctrl/intel/pinctrl-intel.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 96e73e30204e..1e24a6b8a64e 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -788,6 +788,24 @@ static void intel_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
raw_spin_unlock_irqrestore(&pctrl->lock, flags);
}
+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;
+
+ reg = intel_get_padcfg(pctrl, offset, PADCFG0);
+ if (!reg)
+ return -EINVAL;
+
+ padcfg0 = readl(reg);
+
+ if (padcfg0 & PADCFG0_PMODE_MASK)
+ return -EINVAL;
+
+ return !!(padcfg0 & PADCFG0_GPIOTXDIS);
+}
+
static int intel_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
{
return pinctrl_gpio_direction_input(chip->base + offset);
@@ -804,6 +822,7 @@ static const struct gpio_chip intel_gpio_chip = {
.owner = THIS_MODULE,
.request = gpiochip_generic_request,
.free = gpiochip_generic_free,
+ .get_direction = intel_gpio_get_direction,
.direction_input = intel_gpio_direction_input,
.direction_output = intel_gpio_direction_output,
.get = intel_gpio_get,
--
2.16.2
(Fix Andy's Cc: - apologies)
On 06/03/2018 13:42, Javier Arteaga wrote:
> Allows querying GPIO direction from the pad config register.
> If the pad is not in GPIO mode, return an error.
>
> Signed-off-by: Javier Arteaga <[email protected]>
> ---
> This is needed by the drivers for the UP Squared board, an APL-based
> platform. (For now, these drivers are out-of-tree.)
>
> An earlier version of this patch was reviewed some time ago by Andy
> Shevchenko outside of the mailing lists:
> <https://github.com/emutex/ubilinux-kernel/commit/708a776f2ffd1b1c>
>
> drivers/pinctrl/intel/pinctrl-intel.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
> index 96e73e30204e..1e24a6b8a64e 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.c
> +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> @@ -788,6 +788,24 @@ static void intel_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
> raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> }
>
> +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;
> +
> + reg = intel_get_padcfg(pctrl, offset, PADCFG0);
> + if (!reg)
> + return -EINVAL;
> +
> + padcfg0 = readl(reg);
> +
> + if (padcfg0 & PADCFG0_PMODE_MASK)
> + return -EINVAL;
> +
> + return !!(padcfg0 & PADCFG0_GPIOTXDIS);
> +}
> +
> static int intel_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
> {
> return pinctrl_gpio_direction_input(chip->base + offset);
> @@ -804,6 +822,7 @@ static const struct gpio_chip intel_gpio_chip = {
> .owner = THIS_MODULE,
> .request = gpiochip_generic_request,
> .free = gpiochip_generic_free,
> + .get_direction = intel_gpio_get_direction,
> .direction_input = intel_gpio_direction_input,
> .direction_output = intel_gpio_direction_output,
> .get = intel_gpio_get,
>
On Tue, 2018-03-06 at 14:31 +0000, Javier Arteaga wrote:
> (Fix Andy's Cc: - apologies)
>
> On 06/03/2018 13:42, Javier Arteaga wrote:
> > Allows querying GPIO direction from the pad config register.
> > If the pad is not in GPIO mode, return an error.
> >
> > Signed-off-by: Javier Arteaga <[email protected]>
> > ---
> > This is needed by the drivers for the UP Squared board, an APL-based
> > platform. (For now, these drivers are out-of-tree.)
> >
> > An earlier version of this patch was reviewed some time ago by Andy
> > Shevchenko outside of the mailing lists:
> > <https://github.com/emutex/ubilinux-kernel/commit/708a776f2ffd1b1c>
> >
Yes, this one look good!
Reviewed-by: Andy Shevchenko <[email protected]>
> > drivers/pinctrl/intel/pinctrl-intel.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/pinctrl/intel/pinctrl-intel.c
> > b/drivers/pinctrl/intel/pinctrl-intel.c
> > index 96e73e30204e..1e24a6b8a64e 100644
> > --- a/drivers/pinctrl/intel/pinctrl-intel.c
> > +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> > @@ -788,6 +788,24 @@ static void intel_gpio_set(struct gpio_chip
> > *chip, unsigned offset, int value)
> > raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> > }
> >
> > +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;
> > +
> > + reg = intel_get_padcfg(pctrl, offset, PADCFG0);
> > + if (!reg)
> > + return -EINVAL;
> > +
> > + padcfg0 = readl(reg);
> > +
> > + if (padcfg0 & PADCFG0_PMODE_MASK)
> > + return -EINVAL;
Actually we might return direction of GPIO function while pin is in some
other mode, though it would probably make not much sense in practice.
> > +
> > + return !!(padcfg0 & PADCFG0_GPIOTXDIS);
> > +}
> > +
> > static int intel_gpio_direction_input(struct gpio_chip *chip,
> > unsigned offset)
> > {
> > return pinctrl_gpio_direction_input(chip->base + offset);
> > @@ -804,6 +822,7 @@ static const struct gpio_chip intel_gpio_chip =
> > {
> > .owner = THIS_MODULE,
> > .request = gpiochip_generic_request,
> > .free = gpiochip_generic_free,
> > + .get_direction = intel_gpio_get_direction,
> > .direction_input = intel_gpio_direction_input,
> > .direction_output = intel_gpio_direction_output,
> > .get = intel_gpio_get,
> >
--
Andy Shevchenko <[email protected]>
Intel Finland Oy
On Tue, 2018-03-06 at 16:56 +0200, Andy Shevchenko wrote:
> On Tue, 2018-03-06 at 14:31 +0000, Javier Arteaga wrote:
> > > +static int intel_gpio_get_direction(struct gpio_chip *chip,
> > > unsigned int offset)
> > > +{
> > > + if (padcfg0 & PADCFG0_PMODE_MASK)
> > > + return -EINVAL;
>
> Actually we might return direction of GPIO function while pin is in
> some
> other mode, though it would probably make not much sense in practice.
One more though, this is a call back for GPIO function anyway, so, above
condition should never happen. I think it's safe to remove it
completely.
> > > +
> > > + return !!(padcfg0 & PADCFG0_GPIOTXDIS);
> > > +}
--
Andy Shevchenko <[email protected]>
Intel Finland Oy
On 06/03/2018 14:59, Andy Shevchenko wrote:
> On Tue, 2018-03-06 at 16:56 +0200, Andy Shevchenko wrote:
>> On Tue, 2018-03-06 at 14:31 +0000, Javier Arteaga wrote:
>
>>>> +static int intel_gpio_get_direction(struct gpio_chip *chip,
>>>> unsigned int offset)
>>>> +{
>
>>>> + if (padcfg0 & PADCFG0_PMODE_MASK)
>>>> + return -EINVAL;
>>
>> Actually we might return direction of GPIO function while pin is in
>> some
>> other mode, though it would probably make not much sense in practice.
>
> One more though, this is a call back for GPIO function anyway, so, above
> condition should never happen. I think it's safe to remove it
> completely.
The story behind that check is likely *not* a valid usecase: the current
iteration of the UP board drivers use gpiod_get_direction() *while*
requesting GPIOs to mirror SoC GPIO config on the on-board FPGA. So the
direction doesn't make sense for pins set to function mode.
As per your other feedback that driver should be reworked anyway -
that's not a reason to keep the check. I just thought it's a bit more
defensive, and saw there's some precedent of doing this:
f002d07c56c7 ("gpio: tegra: Implement gpio_get_direction callback")
That being said I don't have a strong argument either way :)
I'll resend if you still feel it's unnecessary.
Thanks for reviewing!
On Tue, Mar 06, 2018 at 04:56:05PM +0200, Andy Shevchenko wrote:
> On Tue, 2018-03-06 at 14:31 +0000, Javier Arteaga wrote:
> > (Fix Andy's Cc: - apologies)
> >
> > On 06/03/2018 13:42, Javier Arteaga wrote:
> > > Allows querying GPIO direction from the pad config register.
> > > If the pad is not in GPIO mode, return an error.
> > >
> > > Signed-off-by: Javier Arteaga <[email protected]>
> > > ---
> > > This is needed by the drivers for the UP Squared board, an APL-based
> > > platform. (For now, these drivers are out-of-tree.)
> > >
> > > An earlier version of this patch was reviewed some time ago by Andy
> > > Shevchenko outside of the mailing lists:
> > > <https://github.com/emutex/ubilinux-kernel/commit/708a776f2ffd1b1c>
> > >
>
> Yes, this one look good!
>
> Reviewed-by: Andy Shevchenko <[email protected]>
Acked-by: Mika Westerberg <[email protected]>