linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] pinctrl: intel: Implement intel_gpio_get_direction callback
@ 2018-03-06 13:42 Javier Arteaga
  2018-03-06 14:31 ` Javier Arteaga
  0 siblings, 1 reply; 6+ messages in thread
From: Javier Arteaga @ 2018-03-06 13:42 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Heikki Krogerus, Linus Walleij, Andy Shevchenko, linux-gpio,
	linux-kernel, Javier Arteaga

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 <javier@emutex.com>
---
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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] pinctrl: intel: Implement intel_gpio_get_direction callback
  2018-03-06 13:42 [PATCH] pinctrl: intel: Implement intel_gpio_get_direction callback Javier Arteaga
@ 2018-03-06 14:31 ` Javier Arteaga
  2018-03-06 14:56   ` Andy Shevchenko
  0 siblings, 1 reply; 6+ messages in thread
From: Javier Arteaga @ 2018-03-06 14:31 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Heikki Krogerus, Linus Walleij, Andy Shevchenko, linux-gpio,
	linux-kernel

(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 <javier@emutex.com>
> ---
> 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,
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] pinctrl: intel: Implement intel_gpio_get_direction callback
  2018-03-06 14:31 ` Javier Arteaga
@ 2018-03-06 14:56   ` Andy Shevchenko
  2018-03-06 14:59     ` Andy Shevchenko
  2018-03-06 18:06     ` Mika Westerberg
  0 siblings, 2 replies; 6+ messages in thread
From: Andy Shevchenko @ 2018-03-06 14:56 UTC (permalink / raw)
  To: Javier Arteaga, Mika Westerberg
  Cc: Heikki Krogerus, Linus Walleij, linux-gpio, linux-kernel

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 <javier@emutex.com>
> > ---
> > 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 <andriy.shevchenko@linux.intel.com>

> >  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 <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] pinctrl: intel: Implement intel_gpio_get_direction callback
  2018-03-06 14:56   ` Andy Shevchenko
@ 2018-03-06 14:59     ` Andy Shevchenko
  2018-03-06 15:29       ` Javier Arteaga
  2018-03-06 18:06     ` Mika Westerberg
  1 sibling, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2018-03-06 14:59 UTC (permalink / raw)
  To: Javier Arteaga, Mika Westerberg
  Cc: Heikki Krogerus, Linus Walleij, linux-gpio, linux-kernel

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 <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] pinctrl: intel: Implement intel_gpio_get_direction callback
  2018-03-06 14:59     ` Andy Shevchenko
@ 2018-03-06 15:29       ` Javier Arteaga
  0 siblings, 0 replies; 6+ messages in thread
From: Javier Arteaga @ 2018-03-06 15:29 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Mika Westerberg, Heikki Krogerus, Linus Walleij, linux-gpio,
	linux-kernel

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!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] pinctrl: intel: Implement intel_gpio_get_direction callback
  2018-03-06 14:56   ` Andy Shevchenko
  2018-03-06 14:59     ` Andy Shevchenko
@ 2018-03-06 18:06     ` Mika Westerberg
  1 sibling, 0 replies; 6+ messages in thread
From: Mika Westerberg @ 2018-03-06 18:06 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Javier Arteaga, Heikki Krogerus, Linus Walleij, linux-gpio, linux-kernel

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 <javier@emutex.com>
> > > ---
> > > 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 <andriy.shevchenko@linux.intel.com>

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-03-06 18:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-06 13:42 [PATCH] pinctrl: intel: Implement intel_gpio_get_direction callback Javier Arteaga
2018-03-06 14:31 ` Javier Arteaga
2018-03-06 14:56   ` Andy Shevchenko
2018-03-06 14:59     ` Andy Shevchenko
2018-03-06 15:29       ` Javier Arteaga
2018-03-06 18:06     ` Mika Westerberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).