[v2] gpio: omap: implement get_direction

Commit Message

Yegor Yefremov April 22, 2014, 1:31 p.m. UTC

From: Yegor Yefremov <[e-mail]>

This patch implements gpio_chip's get_direction() routine, that
lets other drivers get particular GPIOs direction using
struct gpio_desc.

Signed-off-by: Yegor Yefremov <[e-mail]>
Acked-by: Javier Martinez Canillas <[e-mail]>
---
Changes:
	v2: rework return value calculation

 drivers/gpio/gpio-omap.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

Comments

Linus Walleij April 23, 2014, 1:45 p.m. UTC | #1

On Tue, Apr 22, 2014 at 3:31 PM,  <[e-mail]> wrote:

> From: Yegor Yefremov <[e-mail]>
>
> This patch implements gpio_chip's get_direction() routine, that
> lets other drivers get particular GPIOs direction using
> struct gpio_desc.
>
> Signed-off-by: Yegor Yefremov <[e-mail]>
> Acked-by: Javier Martinez Canillas <[e-mail]>
> ---
> Changes:
>         v2: rework return value calculation
(...)

I don't see why this need to be a broken-out function? Can it not
be factored into gpio_get_direction?

> +static int _get_gpio_direction(struct gpio_bank *bank, int gpio)
> +{
> +       void __iomem *reg = bank->base;
> +       u32 l;
> +
> +       reg += bank->regs->direction;
> +       l = (readl_relaxed(reg) >> gpio);
> +
> +       return (l & 0x00000001);

Rewrite the last two statements like this:

#include <linux/bitops.h>

return !!(readl_relaxed(reg) & BIT(gpio));

> +static int gpio_get_direction(struct gpio_chip *chip, unsigned offset)
> +{
> +       struct gpio_bank *bank;
> +       unsigned long flags;
> +       int dir;
> +
> +       bank = container_of(chip, struct gpio_bank, chip);
> +       spin_lock_irqsave(&bank->lock, flags);
> +       dir = _get_gpio_direction(bank, offset);
> +       spin_unlock_irqrestore(&bank->lock, flags);
> +       return dir;
> +}

So since _get_gpio_direction is never called from unlocked context,
can it not just be part of this function then?

(I make a mental note to prefix all functions in this driver
with omap_*....)

Yours,
Linus Walleij

Patch

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 19b886c..fadc45e 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -102,6 +102,17 @@  static int omap_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
 	return irq_find_mapping(bank->domain, offset);
 }
 
+static int _get_gpio_direction(struct gpio_bank *bank, int gpio)
+{
+	void __iomem *reg = bank->base;
+	u32 l;
+
+	reg += bank->regs->direction;
+	l = (readl_relaxed(reg) >> gpio);
+
+	return (l & 0x00000001);
+}
+
 static void _set_gpio_direction(struct gpio_bank *bank, int gpio, int is_input)
 {
 	void __iomem *reg = bank->base;
@@ -936,6 +947,19 @@  static inline void mpuio_init(struct gpio_bank *bank)
 
 /*---------------------------------------------------------------------*/
 
+static int gpio_get_direction(struct gpio_chip *chip, unsigned offset)
+{
+	struct gpio_bank *bank;
+	unsigned long flags;
+	int dir;
+
+	bank = container_of(chip, struct gpio_bank, chip);
+	spin_lock_irqsave(&bank->lock, flags);
+	dir = _get_gpio_direction(bank, offset);
+	spin_unlock_irqrestore(&bank->lock, flags);
+	return dir;
+}
+
 static int gpio_input(struct gpio_chip *chip, unsigned offset)
 {
 	struct gpio_bank *bank;
@@ -1092,6 +1116,7 @@  static void omap_gpio_chip_init(struct gpio_bank *bank)
 	 */
 	bank->chip.request = omap_gpio_request;
 	bank->chip.free = omap_gpio_free;
+	bank->chip.get_direction = gpio_get_direction;
 	bank->chip.direction_input = gpio_input;
 	bank->chip.get = gpio_get;
 	bank->chip.direction_output = gpio_output;