gpio: mxc: Remove reliance on <linux/gpio.h>

Message ID 20180419082933.5953-1-linus.walleij@linaro.org
State Accepted
Commit 8d0bd9a5c2ce31942f71d5a831ee8cecda5201a8
Headers show
Series
  • gpio: mxc: Remove reliance on <linux/gpio.h>
Related show

Commit Message

Linus Walleij April 19, 2018, 8:29 a.m.
This is a driver so we should only include <linux/gpio.h>.
However this driver was using gpio_get_value() to fetch the
current value of a GPIO used as IRQ line to determine trigger
direction, so we need a better way than looping over the
global GPIO numberspace.

Fix this by just calling the .get() function in the GPIO chip,
as we don't want to end up creating a consumer dependency
on ourselves.

Cc: Vladimir Zapolskiy <vz@mleia.com>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
 drivers/gpio/gpio-mxc.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

-- 
2.14.3

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Lars-Peter Clausen April 19, 2018, 10:49 a.m. | #1
On 04/19/2018 10:29 AM, Linus Walleij wrote:
> This is a driver so we should only include <linux/gpio.h>.


Should be linux/gpio/driver.h I guess.
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shawn Guo May 2, 2018, 8:44 a.m. | #2
On Thu, Apr 19, 2018 at 10:29:33AM +0200, Linus Walleij wrote:
> This is a driver so we should only include <linux/gpio.h>.

> However this driver was using gpio_get_value() to fetch the

> current value of a GPIO used as IRQ line to determine trigger

> direction, so we need a better way than looping over the

> global GPIO numberspace.

> 

> Fix this by just calling the .get() function in the GPIO chip,

> as we don't want to end up creating a consumer dependency

> on ourselves.

> 

> Cc: Vladimir Zapolskiy <vz@mleia.com>

> Cc: Fabio Estevam <fabio.estevam@nxp.com>

> Cc: Shawn Guo <shawnguo@kernel.org>

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>


Other than Lars-Peter's comment on commit log,

Acked-by: Shawn Guo <shawnguo@kernel.org>

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
index 5245a2fe62ae..11ec7228ab08 100644
--- a/drivers/gpio/gpio-mxc.c
+++ b/drivers/gpio/gpio-mxc.c
@@ -30,8 +30,6 @@ 
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/gpio/driver.h>
-/* FIXME: for gpio_get_value() replace this with direct register read */
-#include <linux/gpio.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/bug.h>
@@ -174,7 +172,6 @@  static int gpio_set_irq_type(struct irq_data *d, u32 type)
 	struct mxc_gpio_port *port = gc->private;
 	u32 bit, val;
 	u32 gpio_idx = d->hwirq;
-	u32 gpio = port->gc.base + gpio_idx;
 	int edge;
 	void __iomem *reg = port->base;
 
@@ -190,13 +187,13 @@  static int gpio_set_irq_type(struct irq_data *d, u32 type)
 		if (GPIO_EDGE_SEL >= 0) {
 			edge = GPIO_INT_BOTH_EDGES;
 		} else {
-			val = gpio_get_value(gpio);
+			val = port->gc.get(&port->gc, gpio_idx);
 			if (val) {
 				edge = GPIO_INT_LOW_LEV;
-				pr_debug("mxc: set GPIO %d to low trigger\n", gpio);
+				pr_debug("mxc: set GPIO %d to low trigger\n", gpio_idx);
 			} else {
 				edge = GPIO_INT_HIGH_LEV;
-				pr_debug("mxc: set GPIO %d to high trigger\n", gpio);
+				pr_debug("mxc: set GPIO %d to high trigger\n", gpio_idx);
 			}
 			port->both_edges |= 1 << gpio_idx;
 		}