[RFC PATCH v2 1/4] tty: serial_mctrl_gpio: Add irqs helpers for input lines

[RFC PATCH v2 1/4] tty: serial_mctrl_gpio: Add irqs helpers for input lines

[Janusz Uzycki]

A driver using mctrl_gpio called gpiod_get_direction() to check if
gpio is input line. However .get_direction callback is not available
for all platforms. The patch allows to avoid the function.
The patch introduces the following helpers:
- mctrl_gpio_request_irqs
- mctrl_gpio_free_irqs
- mctrl_gpio_enable_ms
- mctrl_gpio_disable_ms
They allow to:
- simplify drivers which use mctrl_gpio
- hide irq table in mctrl_gpio
- use default irq handler for gpios
- better separate code for gpio modem lines from uart's code
In addition mctrl_gpio_init() has been renamed to mctrl_gpio_init_dt()
to focus DT usage. Also mctrl_gpio_init_dt() initializes irq table for
gpios now and passes struct uart_port into struct mctrl_gpios.
This resulted in changed mctrl_gpio_init_dt() parameter.
It also requires port->dev is set before the function is called.

There were also fixed:
- irq = 0 means invalid/unused (-EINVAL no more used)
- mctrl_gpio_request_irqs() doesn't use negative enum value
  if request_irq() failed. It just calls mctrl_gpio_free_irqs().

The mctrl_gpio_is_gpio() inline function is under discussion
and likely it can replace exported mctrl_gpio_to_gpiod() function.

IRQ_NOAUTOEN setting and request_irq() order was not commented
but it looks right according to other drivers.

Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
---

The patch requires to update the drivers which use mctrl_gpio:
- atmel_serial
- mxs-auart
- clps711x

Changes since RFC v1:
 - patch renamed from:
   ("serial: mctrl-gpio: Add irqs helpers for input lines")
   to:
   ("tty: serial_mctrl_gpio: Add irqs helpers for input lines")
 - mctrl_gpio_request_irqs: changed mctrl_gpio_free_irqs() and
   dev_err() order to make debug easier
 - added patches for atmel_serial and clps711x serial drivers

The patch applies to next (3.19.0-rc2) and was tested with mxs-auart
using kernel 3.14 and 3.18. It wasn't tested on the next (only compiled).

The patchset delivers patches for mxs-auart, atmel_serial and clps711x.
atmel_serial and clps711x were not tested - only compile tests were done.

---
 drivers/tty/serial/serial_mctrl_gpio.c | 109 ++++++++++++++++++++++++++++++++-
 drivers/tty/serial/serial_mctrl_gpio.h |  59 +++++++++++++++++-
 2 files changed, 163 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index a38596c..215b15c 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -19,11 +19,15 @@
 #include <linux/device.h>
 #include <linux/gpio/consumer.h>
 #include <linux/termios.h>
+#include <linux/irq.h>
 
 #include "serial_mctrl_gpio.h"
 
 struct mctrl_gpios {
+	struct uart_port *port;
 	struct gpio_desc *gpio[UART_GPIO_MAX];
+	int irq[UART_GPIO_MAX];
+	unsigned int mctrl_prev;
 };
 
 static const struct {
@@ -72,6 +76,12 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
 }
 EXPORT_SYMBOL_GPL(mctrl_gpio_to_gpiod);
 
+inline
+bool mctrl_gpio_is_gpio(struct mctrl_gpios *gpios, enum mctrl_gpio_idx gidx)
+{
+	return !IS_ERR_OR_NULL(gpios->gpio[gidx]);
+}
+
 unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
 {
 	enum mctrl_gpio_idx i;
@@ -96,8 +106,9 @@ unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
 }
 EXPORT_SYMBOL_GPL(mctrl_gpio_get);
 
-struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
+struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx)
 {
+	struct device *dev = port->dev;
 	struct mctrl_gpios *gpios;
 	enum mctrl_gpio_idx i;
 	int err;
@@ -106,6 +117,7 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
 	if (!gpios)
 		return ERR_PTR(-ENOMEM);
 
+	gpios->port = port;
 	for (i = 0; i < UART_GPIO_MAX; i++) {
 		gpios->gpio[i] = devm_gpiod_get_index(dev,
 						      mctrl_gpios_desc[i].name,
@@ -128,11 +140,13 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
 			devm_gpiod_put(dev, gpios->gpio[i]);
 			gpios->gpio[i] = NULL;
 		}
+		if (gpios->gpio[i] && !mctrl_gpios_desc[i].dir_out)
+			gpios->irq[i] = gpiod_to_irq(gpios->gpio[i]);
 	}
 
 	return gpios;
 }
-EXPORT_SYMBOL_GPL(mctrl_gpio_init);
+EXPORT_SYMBOL_GPL(mctrl_gpio_init_dt);
 
 void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
 {
@@ -147,3 +161,94 @@ void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
 	devm_kfree(dev, gpios);
 }
 EXPORT_SYMBOL_GPL(mctrl_gpio_free);
+
+/*
+ * Dealing with GPIO interrupt
+ */
+#define MCTRL_ANY_DELTA        (TIOCM_RI | TIOCM_DSR | TIOCM_CD | TIOCM_CTS)
+static irqreturn_t mctrl_gpio_irq_handle(int irq, void *context)
+{
+	struct mctrl_gpios *gpios = context;
+	struct uart_port *port = gpios->port;
+	u32 mctrl = gpios->mctrl_prev;
+	u32 mctrl_diff;
+
+	mctrl_gpio_get(gpios, &mctrl);
+
+	mctrl_diff = mctrl ^ gpios->mctrl_prev;
+	gpios->mctrl_prev = mctrl;
+	if (mctrl_diff & MCTRL_ANY_DELTA && port->state != NULL) {
+		if (mctrl_diff & TIOCM_RI)
+			port->icount.rng++;
+		if (mctrl_diff & TIOCM_DSR)
+			port->icount.dsr++;
+		if (mctrl_diff & TIOCM_CD)
+			uart_handle_dcd_change(port, mctrl & TIOCM_CD);
+		if (mctrl_diff & TIOCM_CTS)
+			uart_handle_cts_change(port, mctrl & TIOCM_CTS);
+
+		wake_up_interruptible(&port->state->port.delta_msr_wait);
+	}
+
+	return IRQ_HANDLED;
+}
+
+int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios)
+{
+	struct uart_port *port = gpios->port;
+	int *irq = gpios->irq;
+	enum mctrl_gpio_idx i;
+	int err = 0;
+
+	for (i = 0; i < UART_GPIO_MAX; i++) {
+		if (irq[i] <= 0)
+			continue;
+
+		irq_set_status_flags(irq[i], IRQ_NOAUTOEN);
+		err = request_irq(irq[i], mctrl_gpio_irq_handle,
+				  IRQ_TYPE_EDGE_BOTH,
+				  dev_name(port->dev), gpios);
+		if (err) {
+			dev_err(port->dev, "%s: Can't get %d irq\n",
+				__func__, irq[i]);
+			mctrl_gpio_free_irqs(gpios);
+			break;
+		}
+	}
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(mctrl_gpio_request_irqs);
+
+void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios)
+{
+	enum mctrl_gpio_idx i;
+
+	for (i = 0; i < UART_GPIO_MAX; i++)
+		if (gpios->irq[i] > 0)
+			free_irq(gpios->irq[i], gpios);
+}
+EXPORT_SYMBOL_GPL(mctrl_gpio_free_irqs);
+
+void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios)
+{
+	enum mctrl_gpio_idx i;
+
+	/* get initial status of modem lines GPIOs */
+	mctrl_gpio_get(gpios, &gpios->mctrl_prev);
+
+	for (i = 0; i < UART_GPIO_MAX; i++)
+		if (gpios->irq[i] > 0)
+			enable_irq(gpios->irq[i]);
+}
+EXPORT_SYMBOL_GPL(mctrl_gpio_enable_ms);
+
+void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios)
+{
+	enum mctrl_gpio_idx i;
+
+	for (i = 0; i < UART_GPIO_MAX; i++)
+		if (gpios->irq[i] > 0)
+			disable_irq(gpios->irq[i]);
+}
+EXPORT_SYMBOL_GPL(mctrl_gpio_disable_ms);
diff --git a/drivers/tty/serial/serial_mctrl_gpio.h b/drivers/tty/serial/serial_mctrl_gpio.h
index 400ba04..13ba3f4 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.h
+++ b/drivers/tty/serial/serial_mctrl_gpio.h
@@ -21,6 +21,7 @@
 #include <linux/err.h>
 #include <linux/device.h>
 #include <linux/gpio/consumer.h>
+#include <linux/serial_core.h>
 
 enum mctrl_gpio_idx {
 	UART_GPIO_CTS,
@@ -40,6 +41,13 @@ enum mctrl_gpio_idx {
  */
 struct mctrl_gpios;
 
+/*
+ * Return true if gidx is GPIO line, otherwise false.
+ * It assumes that gpios is allocated and not NULL.
+ */
+inline
+bool mctrl_gpio_is_gpio(struct mctrl_gpios *gpios, enum mctrl_gpio_idx gidx);
+
 #ifdef CONFIG_GPIOLIB
 
 /*
@@ -60,12 +68,13 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
 				      enum mctrl_gpio_idx gidx);
 
 /*
- * Request and set direction of modem control lines GPIOs.
+ * Request and set direction of modem control lines GPIOs. DT is used.
+ * Initialize irq table for GPIOs.
  * devm_* functions are used, so there's no need to call mctrl_gpio_free().
  * Returns a pointer to the allocated mctrl structure if ok, -ENOMEM on
  * allocation error.
  */
-struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx);
+struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx);
 
 /*
  * Free the mctrl_gpios structure.
@@ -74,6 +83,28 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx);
  */
 void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios);
 
+/*
+ * Request irqs for input lines GPIOs. The irqs are set disabled
+ * and triggered on both edges.
+ * Returns zero if OK, otherwise an error code.
+ */
+int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios);
+
+/*
+ * Free irqs for input lines GPIOs.
+ */
+void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios);
+
+/*
+ * Disable modem status interrupts assigned to GPIOs
+ */
+void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios);
+
+/*
+ * Enable modem status interrupts assigned to GPIOs
+ */
+void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios);
+
 #else /* GPIOLIB */
 
 static inline
@@ -95,7 +126,7 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
 }
 
 static inline
-struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
+struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx)
 {
 	return ERR_PTR(-ENOSYS);
 }
@@ -105,6 +136,28 @@ void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
 {
 }
 
+static inline
+int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios)
+{
+	/*return -ENOTSUP;*/
+	return 0;
+}
+
+static inline
+void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios)
+{
+}
+
+static inline
+void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios)
+{
+}
+
+static inline
+void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios)
+{
+}
+
 #endif /* GPIOLIB */
 
 #endif
-- 
1.7.11.3

[RFC PATCH v2 2/4] serial: mxs-auart: Use helpers for gpio irqs

[Janusz Uzycki]

The patch updates mxs-auart driver to use new mctrl_gpio helpers for
gpio irqs. The code is much simpler now.

Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
---

There is no changes since v1 (rebased only).

---
 drivers/tty/serial/mxs-auart.c | 133 ++++-------------------------------------
 1 file changed, 13 insertions(+), 120 deletions(-)

diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index ec553f8..2ddba69 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -149,7 +149,6 @@ struct mxs_auart_port {
 #define MXS_AUART_DMA_RX_READY	3  /* bit 3 */
 #define MXS_AUART_RTSCTS	4  /* bit 4 */
 	unsigned long flags;
-	unsigned int mctrl_prev;
 	enum mxs_auart_type devtype;
 
 	unsigned int irq;
@@ -167,7 +166,6 @@ struct mxs_auart_port {
 	void *rx_dma_buf;
 
 	struct mctrl_gpios	*gpios;
-	int			gpio_irq[UART_GPIO_MAX];
 	bool			ms_irq_enabled;
 };
 
@@ -433,29 +431,6 @@ static void mxs_auart_set_mctrl(struct uart_port *u, unsigned mctrl)
 	mctrl_gpio_set(s->gpios, mctrl);
 }
 
-#define MCTRL_ANY_DELTA        (TIOCM_RI | TIOCM_DSR | TIOCM_CD | TIOCM_CTS)
-static u32 mxs_auart_modem_status(struct mxs_auart_port *s, u32 mctrl)
-{
-	u32 mctrl_diff;
-
-	mctrl_diff = mctrl ^ s->mctrl_prev;
-	s->mctrl_prev = mctrl;
-	if (mctrl_diff & MCTRL_ANY_DELTA && s->ms_irq_enabled &&
-						s->port.state != NULL) {
-		if (mctrl_diff & TIOCM_RI)
-			s->port.icount.rng++;
-		if (mctrl_diff & TIOCM_DSR)
-			s->port.icount.dsr++;
-		if (mctrl_diff & TIOCM_CD)
-			uart_handle_dcd_change(&s->port, mctrl & TIOCM_CD);
-		if (mctrl_diff & TIOCM_CTS)
-			uart_handle_cts_change(&s->port, mctrl & TIOCM_CTS);
-
-		wake_up_interruptible(&s->port.state->port.delta_msr_wait);
-	}
-	return mctrl;
-}
-
 static u32 mxs_auart_get_mctrl(struct uart_port *u)
 {
 	struct mxs_auart_port *s = to_auart_port(u);
@@ -483,18 +458,9 @@ static void mxs_auart_enable_ms(struct uart_port *port)
 
 	s->ms_irq_enabled = true;
 
-	if (s->gpio_irq[UART_GPIO_CTS] >= 0)
-		enable_irq(s->gpio_irq[UART_GPIO_CTS]);
-	/* TODO: enable AUART_INTR_CTSMIEN otherwise */
-
-	if (s->gpio_irq[UART_GPIO_DSR] >= 0)
-		enable_irq(s->gpio_irq[UART_GPIO_DSR]);
-
-	if (s->gpio_irq[UART_GPIO_RI] >= 0)
-		enable_irq(s->gpio_irq[UART_GPIO_RI]);
-
-	if (s->gpio_irq[UART_GPIO_DCD] >= 0)
-		enable_irq(s->gpio_irq[UART_GPIO_DCD]);
+	mctrl_gpio_enable_ms(s->gpios);
+	/* TODO: enable AUART_INTR_CTSMIEN
+	 * if s->gpios->irq[UART_GPIO_CTS] == 0 */
 }
 
 /*
@@ -512,18 +478,9 @@ static void mxs_auart_disable_ms(struct uart_port *port)
 
 	s->ms_irq_enabled = false;
 
-	if (s->gpio_irq[UART_GPIO_CTS] >= 0)
-		disable_irq(s->gpio_irq[UART_GPIO_CTS]);
-	/* TODO: disable AUART_INTR_CTSMIEN otherwise */
-
-	if (s->gpio_irq[UART_GPIO_DSR] >= 0)
-		disable_irq(s->gpio_irq[UART_GPIO_DSR]);
-
-	if (s->gpio_irq[UART_GPIO_RI] >= 0)
-		disable_irq(s->gpio_irq[UART_GPIO_RI]);
-
-	if (s->gpio_irq[UART_GPIO_DCD] >= 0)
-		disable_irq(s->gpio_irq[UART_GPIO_DCD]);
+	mctrl_gpio_disable_ms(s->gpios);
+	/* TODO: disable AUART_INTR_CTSMIEN
+	 * if s->gpios->irq[UART_GPIO_CTS] == 0 */
 }
 
 static int mxs_auart_dma_prep_rx(struct mxs_auart_port *s);
@@ -799,7 +756,6 @@ static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
 {
 	u32 istat;
 	struct mxs_auart_port *s = context;
-	u32 mctrl_temp = s->mctrl_prev;
 	u32 stat = readl(s->port.membase + AUART_STAT);
 
 	istat = readl(s->port.membase + AUART_INTR);
@@ -811,16 +767,6 @@ static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
 		| AUART_INTR_CTSMIS),
 			s->port.membase + AUART_INTR_CLR);
 
-	/*
-	 * Dealing with GPIO interrupt
-	 */
-	if (irq == s->gpio_irq[UART_GPIO_CTS] ||
-	    irq == s->gpio_irq[UART_GPIO_DCD] ||
-	    irq == s->gpio_irq[UART_GPIO_DSR] ||
-	    irq == s->gpio_irq[UART_GPIO_RI])
-		mxs_auart_modem_status(s,
-				mctrl_gpio_get(s->gpios, &mctrl_temp));
-
 	if (istat & AUART_INTR_CTSMIS) {
 		if (CTS_AT_AUART() && s->ms_irq_enabled)
 			uart_handle_cts_change(&s->port,
@@ -885,9 +831,6 @@ static int mxs_auart_startup(struct uart_port *u)
 	 */
 	writel(AUART_LINECTRL_FEN, u->membase + AUART_LINECTRL_SET);
 
-	/* get initial status of modem lines */
-	mctrl_gpio_get(s->gpios, &s->mctrl_prev);
-
 	s->ms_irq_enabled = false;
 	return 0;
 }
@@ -1157,71 +1100,23 @@ static int serial_mxs_probe_dt(struct mxs_auart_port *s,
 	return 0;
 }
 
-static bool mxs_auart_init_gpios(struct mxs_auart_port *s, struct device *dev)
+static bool mxs_auart_init_gpios(struct mxs_auart_port *s)
 {
-	enum mctrl_gpio_idx i;
-	struct gpio_desc *gpiod;
-
-	s->gpios = mctrl_gpio_init(dev, 0);
+	s->gpios = mctrl_gpio_init_dt(&s->port, 0);
 	if (IS_ERR_OR_NULL(s->gpios))
 		return false;
 
 	/* Block (enabled before) DMA option if RTS or CTS is GPIO line */
 	if (!RTS_AT_AUART() || !CTS_AT_AUART()) {
 		if (test_bit(MXS_AUART_RTSCTS, &s->flags))
-			dev_warn(dev,
+			dev_warn(s->dev,
 				 "DMA and flow control via gpio may cause some problems. DMA disabled!\n");
 		clear_bit(MXS_AUART_RTSCTS, &s->flags);
 	}
 
-	for (i = 0; i < UART_GPIO_MAX; i++) {
-		gpiod = mctrl_gpio_to_gpiod(s->gpios, i);
-		if (gpiod && (gpiod_get_direction(gpiod) == GPIOF_DIR_IN))
-			s->gpio_irq[i] = gpiod_to_irq(gpiod);
-		else
-			s->gpio_irq[i] = -EINVAL;
-	}
-
 	return true;
 }
 
-static void mxs_auart_free_gpio_irq(struct mxs_auart_port *s)
-{
-	enum mctrl_gpio_idx i;
-
-	for (i = 0; i < UART_GPIO_MAX; i++)
-		if (s->gpio_irq[i] >= 0)
-			free_irq(s->gpio_irq[i], s);
-}
-
-static int mxs_auart_request_gpio_irq(struct mxs_auart_port *s)
-{
-	int *irq = s->gpio_irq;
-	enum mctrl_gpio_idx i;
-	int err = 0;
-
-	for (i = 0; (i < UART_GPIO_MAX) && !err; i++) {
-		if (irq[i] < 0)
-			continue;
-
-		irq_set_status_flags(irq[i], IRQ_NOAUTOEN);
-		err = request_irq(irq[i], mxs_auart_irq_handle,
-				IRQ_TYPE_EDGE_BOTH, dev_name(s->dev), s);
-		if (err)
-			dev_err(s->dev, "%s - Can't get %d irq\n",
-				__func__, irq[i]);
-	}
-
-	/*
-	 * If something went wrong, rollback.
-	 */
-	while (err && (--i >= 0))
-		if (irq[i] >= 0)
-			free_irq(irq[i], s);
-
-	return err;
-}
-
 static int mxs_auart_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *of_id =
@@ -1269,8 +1164,6 @@ static int mxs_auart_probe(struct platform_device *pdev)
 	s->port.type = PORT_IMX;
 	s->port.dev = s->dev = &pdev->dev;
 
-	s->mctrl_prev = 0;
-
 	s->irq = platform_get_irq(pdev, 0);
 	s->port.irq = s->irq;
 	ret = request_irq(s->irq, mxs_auart_irq_handle, 0, dev_name(&pdev->dev), s);
@@ -1279,14 +1172,14 @@ static int mxs_auart_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, s);
 
-	if (!mxs_auart_init_gpios(s, &pdev->dev))
+	if (!mxs_auart_init_gpios(s))
 		dev_err(&pdev->dev,
 			"Failed to initialize GPIOs. The serial port may not work as expected\n");
 
 	/*
 	 * Get the GPIO lines IRQ
 	 */
-	ret = mxs_auart_request_gpio_irq(s);
+	ret = mctrl_gpio_request_irqs(s->gpios);
 	if (ret)
 		goto out_free_irq;
 
@@ -1306,7 +1199,7 @@ static int mxs_auart_probe(struct platform_device *pdev)
 	return 0;
 
 out_free_gpio_irq:
-	mxs_auart_free_gpio_irq(s);
+	mctrl_gpio_free_irqs(s->gpios);
 out_free_irq:
 	auart_port[pdev->id] = NULL;
 	free_irq(s->irq, s);
@@ -1326,7 +1219,7 @@ static int mxs_auart_remove(struct platform_device *pdev)
 
 	auart_port[pdev->id] = NULL;
 
-	mxs_auart_free_gpio_irq(s);
+	mctrl_gpio_free_irqs(s->gpios);
 	clk_put(s->clk);
 	free_irq(s->irq, s);
 	kfree(s);
-- 
1.7.11.3

[RFC PATCH v2 3/4] serial: at91: Use helpers for gpio irqs

[Janusz Uzycki]

The patch updates atmel_serial driver to use new mctrl_gpio helpers for
gpio irqs. The code is simpler now.

Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
---

The patch exists since v2.
Compile-test only - please test it on real hardware.

---
 drivers/tty/serial/atmel_serial.c | 123 +++++++-------------------------------
 1 file changed, 20 insertions(+), 103 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 4d848a2..a59a892 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -168,7 +168,6 @@ struct atmel_uart_port {
 	struct circ_buf		rx_ring;
 
 	struct mctrl_gpios	*gpios;
-	int			gpio_irq[UART_GPIO_MAX];
 	unsigned int		tx_done_mask;
 	bool			ms_irq_enabled;
 	bool			is_usart;	/* usart or uart */
@@ -499,24 +498,18 @@ static void atmel_enable_ms(struct uart_port *port)
 
 	atmel_port->ms_irq_enabled = true;
 
-	if (atmel_port->gpio_irq[UART_GPIO_CTS] >= 0)
-		enable_irq(atmel_port->gpio_irq[UART_GPIO_CTS]);
-	else
+	mctrl_gpio_enable_ms(atmel_port->gpios);
+
+	if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_CTS))
 		ier |= ATMEL_US_CTSIC;
 
-	if (atmel_port->gpio_irq[UART_GPIO_DSR] >= 0)
-		enable_irq(atmel_port->gpio_irq[UART_GPIO_DSR]);
-	else
+	if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_DSR))
 		ier |= ATMEL_US_DSRIC;
 
-	if (atmel_port->gpio_irq[UART_GPIO_RI] >= 0)
-		enable_irq(atmel_port->gpio_irq[UART_GPIO_RI]);
-	else
+	if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_RI))
 		ier |= ATMEL_US_RIIC;
 
-	if (atmel_port->gpio_irq[UART_GPIO_DCD] >= 0)
-		enable_irq(atmel_port->gpio_irq[UART_GPIO_DCD]);
-	else
+	if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_DCD))
 		ier |= ATMEL_US_DCDIC;
 
 	UART_PUT_IER(port, ier);
@@ -538,24 +531,18 @@ static void atmel_disable_ms(struct uart_port *port)
 
 	atmel_port->ms_irq_enabled = false;
 
-	if (atmel_port->gpio_irq[UART_GPIO_CTS] >= 0)
-		disable_irq(atmel_port->gpio_irq[UART_GPIO_CTS]);
-	else
+	mctrl_gpio_disable_ms(atmel_port->gpios);
+
+	if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_CTS))
 		idr |= ATMEL_US_CTSIC;
 
-	if (atmel_port->gpio_irq[UART_GPIO_DSR] >= 0)
-		disable_irq(atmel_port->gpio_irq[UART_GPIO_DSR]);
-	else
+	if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_DSR))
 		idr |= ATMEL_US_DSRIC;
 
-	if (atmel_port->gpio_irq[UART_GPIO_RI] >= 0)
-		disable_irq(atmel_port->gpio_irq[UART_GPIO_RI]);
-	else
+	if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_RI))
 		idr |= ATMEL_US_RIIC;
 
-	if (atmel_port->gpio_irq[UART_GPIO_DCD] >= 0)
-		disable_irq(atmel_port->gpio_irq[UART_GPIO_DCD]);
-	else
+	if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_DCD))
 		idr |= ATMEL_US_DCDIC;
 
 	UART_PUT_IDR(port, idr);
@@ -1161,31 +1148,11 @@ atmel_handle_status(struct uart_port *port, unsigned int pending,
 static irqreturn_t atmel_interrupt(int irq, void *dev_id)
 {
 	struct uart_port *port = dev_id;
-	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
 	unsigned int status, pending, pass_counter = 0;
-	bool gpio_handled = false;
 
 	do {
 		status = atmel_get_lines_status(port);
 		pending = status & UART_GET_IMR(port);
-		if (!gpio_handled) {
-			/*
-			 * Dealing with GPIO interrupt
-			 */
-			if (irq == atmel_port->gpio_irq[UART_GPIO_CTS])
-				pending |= ATMEL_US_CTSIC;
-
-			if (irq == atmel_port->gpio_irq[UART_GPIO_DSR])
-				pending |= ATMEL_US_DSRIC;
-
-			if (irq == atmel_port->gpio_irq[UART_GPIO_RI])
-				pending |= ATMEL_US_RIIC;
-
-			if (irq == atmel_port->gpio_irq[UART_GPIO_DCD])
-				pending |= ATMEL_US_DCDIC;
-
-			gpio_handled = true;
-		}
 		if (!pending)
 			break;
 
@@ -1665,45 +1632,6 @@ static void atmel_get_ip_name(struct uart_port *port)
 	}
 }
 
-static void atmel_free_gpio_irq(struct uart_port *port)
-{
-	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
-	enum mctrl_gpio_idx i;
-
-	for (i = 0; i < UART_GPIO_MAX; i++)
-		if (atmel_port->gpio_irq[i] >= 0)
-			free_irq(atmel_port->gpio_irq[i], port);
-}
-
-static int atmel_request_gpio_irq(struct uart_port *port)
-{
-	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
-	int *irq = atmel_port->gpio_irq;
-	enum mctrl_gpio_idx i;
-	int err = 0;
-
-	for (i = 0; (i < UART_GPIO_MAX) && !err; i++) {
-		if (irq[i] < 0)
-			continue;
-
-		irq_set_status_flags(irq[i], IRQ_NOAUTOEN);
-		err = request_irq(irq[i], atmel_interrupt, IRQ_TYPE_EDGE_BOTH,
-				  "atmel_serial", port);
-		if (err)
-			dev_err(port->dev, "atmel_startup - Can't get %d irq\n",
-				irq[i]);
-	}
-
-	/*
-	 * If something went wrong, rollback.
-	 */
-	while (err && (--i >= 0))
-		if (irq[i] >= 0)
-			free_irq(irq[i], port);
-
-	return err;
-}
-
 /*
  * Perform initialization and enable port for reception
  */
@@ -1735,7 +1663,7 @@ static int atmel_startup(struct uart_port *port)
 	/*
 	 * Get the GPIO lines IRQ
 	 */
-	retval = atmel_request_gpio_irq(port);
+	retval = mctrl_gpio_request_irqs(atmel_port->gpios);
 	if (retval)
 		goto free_irq;
 
@@ -1872,7 +1800,7 @@ static void atmel_shutdown(struct uart_port *port)
 	 * Free the interrupts
 	 */
 	free_irq(port->irq, port);
-	atmel_free_gpio_irq(port);
+	mctrl_gpio_free_irqs(atmel_port->gpios);
 
 	atmel_port->ms_irq_enabled = false;
 
@@ -2501,24 +2429,13 @@ static int atmel_serial_resume(struct platform_device *pdev)
 #define atmel_serial_resume NULL
 #endif
 
-static int atmel_init_gpios(struct atmel_uart_port *p, struct device *dev)
+static bool atmel_init_gpios(struct atmel_uart_port *p)
 {
-	enum mctrl_gpio_idx i;
-	struct gpio_desc *gpiod;
-
-	p->gpios = mctrl_gpio_init(dev, 0);
+	p->gpios = mctrl_gpio_init_dt(&p->uart, 0);
 	if (IS_ERR_OR_NULL(p->gpios))
-		return -1;
+		return false;
 
-	for (i = 0; i < UART_GPIO_MAX; i++) {
-		gpiod = mctrl_gpio_to_gpiod(p->gpios, i);
-		if (gpiod && (gpiod_get_direction(gpiod) == GPIOF_DIR_IN))
-			p->gpio_irq[i] = gpiod_to_irq(gpiod);
-		else
-			p->gpio_irq[i] = -EINVAL;
-	}
-
-	return 0;
+	return true;
 }
 
 static int atmel_serial_probe(struct platform_device *pdev)
@@ -2558,8 +2475,8 @@ static int atmel_serial_probe(struct platform_device *pdev)
 	port->backup_imr = 0;
 	port->uart.line = ret;
 
-	ret = atmel_init_gpios(port, &pdev->dev);
-	if (ret < 0)
+	port->uart.dev = &pdev->dev;
+	if (!atmel_init_gpios(port))
 		dev_err(&pdev->dev, "%s",
 			"Failed to initialize GPIOs. The serial port may not work as expected");
 
-- 
1.7.11.3

[RFC PATCH v2 4/4] serial: clps711x: Update to new mctrl_gpio_init_dt

[Janusz Uzycki]

The patch updates clps711x serial driver to new mctrl_gpio_init_dt
which came with new helpers for gpio irqs.

Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
---

The patch exists since v2.
Compile-test only - please test it on real hardware.

---
 drivers/tty/serial/clps711x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/clps711x.c b/drivers/tty/serial/clps711x.c
index 6e11c27..4fa0210 100644
--- a/drivers/tty/serial/clps711x.c
+++ b/drivers/tty/serial/clps711x.c
@@ -500,7 +500,7 @@ static int uart_clps711x_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, s);
 
-	s->gpios = mctrl_gpio_init(&pdev->dev, 0);
+	s->gpios = mctrl_gpio_init_dt(&s->port, 0);
 
 	ret = uart_add_one_port(&clps711x_uart, &s->port);
 	if (ret)
-- 
1.7.11.3

[RFC PATCH v2 1/4] tty: serial_mctrl_gpio: Add irqs helpers for input lines

[Alexandre Courbot]

On Sat, Jan 10, 2015 at 11:32 PM, Janusz Uzycki <j.uzycki@elproma.com.pl> wrote:
> A driver using mctrl_gpio called gpiod_get_direction() to check if
> gpio is input line. However .get_direction callback is not available
> for all platforms. The patch allows to avoid the function.
> The patch introduces the following helpers:
> - mctrl_gpio_request_irqs
> - mctrl_gpio_free_irqs
> - mctrl_gpio_enable_ms
> - mctrl_gpio_disable_ms
> They allow to:
> - simplify drivers which use mctrl_gpio
> - hide irq table in mctrl_gpio
> - use default irq handler for gpios
> - better separate code for gpio modem lines from uart's code
> In addition mctrl_gpio_init() has been renamed to mctrl_gpio_init_dt()
> to focus DT usage. Also mctrl_gpio_init_dt() initializes irq table for
> gpios now and passes struct uart_port into struct mctrl_gpios.
> This resulted in changed mctrl_gpio_init_dt() parameter.
> It also requires port->dev is set before the function is called.
>
> There were also fixed:
> - irq = 0 means invalid/unused (-EINVAL no more used)
> - mctrl_gpio_request_irqs() doesn't use negative enum value
>   if request_irq() failed. It just calls mctrl_gpio_free_irqs().
>
> The mctrl_gpio_is_gpio() inline function is under discussion
> and likely it can replace exported mctrl_gpio_to_gpiod() function.
>
> IRQ_NOAUTOEN setting and request_irq() order was not commented
> but it looks right according to other drivers.

I cannot provide a full review on this part of the kernel, but this
series is also useful in that it will allow us to make
gpiod_get_direction() private. This function should never have been
public in the first place.

The series,

Acked-by: Alexandre Courbot <acourbot@nvidia.com>

[RFC PATCH v2 1/4] tty: serial_mctrl_gpio: Add irqs helpers for input lines

[Uwe Kleine-König]

On Sat, Jan 10, 2015 at 03:32:43PM +0100, Janusz Uzycki wrote:
> A driver using mctrl_gpio called gpiod_get_direction() to check if
> gpio is input line. However .get_direction callback is not available
> for all platforms. The patch allows to avoid the function.
> The patch introduces the following helpers:
> - mctrl_gpio_request_irqs
> - mctrl_gpio_free_irqs
> - mctrl_gpio_enable_ms
> - mctrl_gpio_disable_ms
> They allow to:
> - simplify drivers which use mctrl_gpio
> - hide irq table in mctrl_gpio
> - use default irq handler for gpios
> - better separate code for gpio modem lines from uart's code
> In addition mctrl_gpio_init() has been renamed to mctrl_gpio_init_dt()
Note that this breaks the drivers that are already using
mctrl_gpio_init. This is fixed up in the follow up patches, but still
you break bisection.

> to focus DT usage. Also mctrl_gpio_init_dt() initializes irq table for
> gpios now and passes struct uart_port into struct mctrl_gpios.
> This resulted in changed mctrl_gpio_init_dt() parameter.
> It also requires port->dev is set before the function is called.
> 
> There were also fixed:
> - irq = 0 means invalid/unused (-EINVAL no more used)
> - mctrl_gpio_request_irqs() doesn't use negative enum value
>   if request_irq() failed. It just calls mctrl_gpio_free_irqs().
> 
> The mctrl_gpio_is_gpio() inline function is under discussion
> and likely it can replace exported mctrl_gpio_to_gpiod() function.
> 
> IRQ_NOAUTOEN setting and request_irq() order was not commented
> but it looks right according to other drivers.
I suggest to add a comment for that. Something like:

	/*
	 * XXX: This is not undone in the error path. To be fixed
	 * properly this needs to be set atomically together with
	 * request_irq.
	 */
 
> Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
There is no binding documentation for mctrl. This should be fixed, too.

> ---
> 
> The patch requires to update the drivers which use mctrl_gpio:
> - atmel_serial
> - mxs-auart
> - clps711x
> 
> Changes since RFC v1:
>  - patch renamed from:
>    ("serial: mctrl-gpio: Add irqs helpers for input lines")
>    to:
>    ("tty: serial_mctrl_gpio: Add irqs helpers for input lines")
>  - mctrl_gpio_request_irqs: changed mctrl_gpio_free_irqs() and
>    dev_err() order to make debug easier
>  - added patches for atmel_serial and clps711x serial drivers
> 
> The patch applies to next (3.19.0-rc2) and was tested with mxs-auart
> using kernel 3.14 and 3.18. It wasn't tested on the next (only compiled).
> 
> The patchset delivers patches for mxs-auart, atmel_serial and clps711x.
> atmel_serial and clps711x were not tested - only compile tests were done.
> 
> ---
>  drivers/tty/serial/serial_mctrl_gpio.c | 109 ++++++++++++++++++++++++++++++++-
>  drivers/tty/serial/serial_mctrl_gpio.h |  59 +++++++++++++++++-
>  2 files changed, 163 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
> index a38596c..215b15c 100644
> --- a/drivers/tty/serial/serial_mctrl_gpio.c
> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
> @@ -19,11 +19,15 @@
>  #include <linux/device.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/termios.h>
> +#include <linux/irq.h>
>  
>  #include "serial_mctrl_gpio.h"
>  
>  struct mctrl_gpios {
> +	struct uart_port *port;
>  	struct gpio_desc *gpio[UART_GPIO_MAX];
> +	int irq[UART_GPIO_MAX];
> +	unsigned int mctrl_prev;
>  };
>  
>  static const struct {
> @@ -72,6 +76,12 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
>  }
>  EXPORT_SYMBOL_GPL(mctrl_gpio_to_gpiod);
>  
> +inline
> +bool mctrl_gpio_is_gpio(struct mctrl_gpios *gpios, enum mctrl_gpio_idx gidx)
> +{
> +	return !IS_ERR_OR_NULL(gpios->gpio[gidx]);
> +}
> +
>  unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
>  {
>  	enum mctrl_gpio_idx i;
> @@ -96,8 +106,9 @@ unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
>  }
>  EXPORT_SYMBOL_GPL(mctrl_gpio_get);
>  
> -struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
> +struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx)
>  {
> +	struct device *dev = port->dev;
>  	struct mctrl_gpios *gpios;
>  	enum mctrl_gpio_idx i;
>  	int err;
> @@ -106,6 +117,7 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
>  	if (!gpios)
>  		return ERR_PTR(-ENOMEM);
>  
> +	gpios->port = port;
>  	for (i = 0; i < UART_GPIO_MAX; i++) {
>  		gpios->gpio[i] = devm_gpiod_get_index(dev,
>  						      mctrl_gpios_desc[i].name,
> @@ -128,11 +140,13 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
>  			devm_gpiod_put(dev, gpios->gpio[i]);
>  			gpios->gpio[i] = NULL;
>  		}
> +		if (gpios->gpio[i] && !mctrl_gpios_desc[i].dir_out)
> +			gpios->irq[i] = gpiod_to_irq(gpios->gpio[i]);
>  	}
>  
>  	return gpios;
>  }
> -EXPORT_SYMBOL_GPL(mctrl_gpio_init);
> +EXPORT_SYMBOL_GPL(mctrl_gpio_init_dt);
>  
>  void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
>  {
> @@ -147,3 +161,94 @@ void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
>  	devm_kfree(dev, gpios);
>  }
>  EXPORT_SYMBOL_GPL(mctrl_gpio_free);
> +
> +/*
> + * Dealing with GPIO interrupt
> + */
> +#define MCTRL_ANY_DELTA        (TIOCM_RI | TIOCM_DSR | TIOCM_CD | TIOCM_CTS)
> +static irqreturn_t mctrl_gpio_irq_handle(int irq, void *context)
> +{
> +	struct mctrl_gpios *gpios = context;
> +	struct uart_port *port = gpios->port;
> +	u32 mctrl = gpios->mctrl_prev;
> +	u32 mctrl_diff;
> +
> +	mctrl_gpio_get(gpios, &mctrl);
> +
> +	mctrl_diff = mctrl ^ gpios->mctrl_prev;
> +	gpios->mctrl_prev = mctrl;
> +	if (mctrl_diff & MCTRL_ANY_DELTA && port->state != NULL) {
> +		if (mctrl_diff & TIOCM_RI)
> +			port->icount.rng++;
> +		if (mctrl_diff & TIOCM_DSR)
> +			port->icount.dsr++;
> +		if (mctrl_diff & TIOCM_CD)
> +			uart_handle_dcd_change(port, mctrl & TIOCM_CD);
> +		if (mctrl_diff & TIOCM_CTS)
> +			uart_handle_cts_change(port, mctrl & TIOCM_CTS);
> +
> +		wake_up_interruptible(&port->state->port.delta_msr_wait);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios)
> +{
> +	struct uart_port *port = gpios->port;
> +	int *irq = gpios->irq;
> +	enum mctrl_gpio_idx i;
> +	int err = 0;
> +
> +	for (i = 0; i < UART_GPIO_MAX; i++) {
> +		if (irq[i] <= 0)
> +			continue;
> +
> +		irq_set_status_flags(irq[i], IRQ_NOAUTOEN);
> +		err = request_irq(irq[i], mctrl_gpio_irq_handle,
> +				  IRQ_TYPE_EDGE_BOTH,
> +				  dev_name(port->dev), gpios);
> +		if (err) {
> +			dev_err(port->dev, "%s: Can't get %d irq\n",
> +				__func__, irq[i]);
> +			mctrl_gpio_free_irqs(gpios);
> +			break;
> +		}
> +	}
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_request_irqs);
> +
> +void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios)
> +{
> +	enum mctrl_gpio_idx i;
> +
> +	for (i = 0; i < UART_GPIO_MAX; i++)
> +		if (gpios->irq[i] > 0)
> +			free_irq(gpios->irq[i], gpios);
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_free_irqs);
> +
> +void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios)
> +{
> +	enum mctrl_gpio_idx i;
> +
> +	/* get initial status of modem lines GPIOs */
> +	mctrl_gpio_get(gpios, &gpios->mctrl_prev);
> +
> +	for (i = 0; i < UART_GPIO_MAX; i++)
> +		if (gpios->irq[i] > 0)
> +			enable_irq(gpios->irq[i]);
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_enable_ms);
> +
> +void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios)
> +{
> +	enum mctrl_gpio_idx i;
> +
> +	for (i = 0; i < UART_GPIO_MAX; i++)
> +		if (gpios->irq[i] > 0)
> +			disable_irq(gpios->irq[i]);
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_disable_ms);
> diff --git a/drivers/tty/serial/serial_mctrl_gpio.h b/drivers/tty/serial/serial_mctrl_gpio.h
> index 400ba04..13ba3f4 100644
> --- a/drivers/tty/serial/serial_mctrl_gpio.h
> +++ b/drivers/tty/serial/serial_mctrl_gpio.h
> @@ -21,6 +21,7 @@
>  #include <linux/err.h>
>  #include <linux/device.h>
>  #include <linux/gpio/consumer.h>
> +#include <linux/serial_core.h>
>  
>  enum mctrl_gpio_idx {
>  	UART_GPIO_CTS,
> @@ -40,6 +41,13 @@ enum mctrl_gpio_idx {
>   */
>  struct mctrl_gpios;
>  
> +/*
> + * Return true if gidx is GPIO line, otherwise false.
> + * It assumes that gpios is allocated and not NULL.
> + */
> +inline
> +bool mctrl_gpio_is_gpio(struct mctrl_gpios *gpios, enum mctrl_gpio_idx gidx);
> +
>  #ifdef CONFIG_GPIOLIB
>  
>  /*
> @@ -60,12 +68,13 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
>  				      enum mctrl_gpio_idx gidx);
>  
>  /*
> - * Request and set direction of modem control lines GPIOs.
> + * Request and set direction of modem control lines GPIOs. DT is used.
> + * Initialize irq table for GPIOs.
>   * devm_* functions are used, so there's no need to call mctrl_gpio_free().
>   * Returns a pointer to the allocated mctrl structure if ok, -ENOMEM on
>   * allocation error.
>   */
> -struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx);
> +struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx);
>  
>  /*
>   * Free the mctrl_gpios structure.
> @@ -74,6 +83,28 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx);
>   */
>  void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios);
>  
> +/*
> + * Request irqs for input lines GPIOs. The irqs are set disabled
> + * and triggered on both edges.
> + * Returns zero if OK, otherwise an error code.
> + */
> +int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios);
> +
> +/*
> + * Free irqs for input lines GPIOs.
> + */
> +void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios);
> +
> +/*
> + * Disable modem status interrupts assigned to GPIOs
> + */
> +void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios);
> +
> +/*
> + * Enable modem status interrupts assigned to GPIOs
> + */
> +void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios);
> +
>  #else /* GPIOLIB */
>  
>  static inline
> @@ -95,7 +126,7 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
>  }
>  
>  static inline
> -struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
> +struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx)
>  {
>  	return ERR_PTR(-ENOSYS);
>  }
> @@ -105,6 +136,28 @@ void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
>  {
>  }
>  
> +static inline
> +int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios)
> +{
> +	/*return -ENOTSUP;*/
> +	return 0;
> +}
> +
> +static inline
> +void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios)
> +{
> +}
> +
> +static inline
> +void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios)
> +{
> +}
> +
> +static inline
> +void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios)
> +{
> +}
> +
>  #endif /* GPIOLIB */
>  
>  #endif
> -- 
> 1.7.11.3
> 
> 

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[RFC PATCH v2 2/4] serial: mxs-auart: Use helpers for gpio irqs

[Uwe Kleine-König]

Hello,

On Sat, Jan 10, 2015 at 03:32:44PM +0100, Janusz Uzycki wrote:
> The patch updates mxs-auart driver to use new mctrl_gpio helpers for
> gpio irqs. The code is much simpler now.
> 
> Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
> ---
> 
> There is no changes since v1 (rebased only).
> 
> ---
>  drivers/tty/serial/mxs-auart.c | 133 ++++-------------------------------------
>  1 file changed, 13 insertions(+), 120 deletions(-)
Very nice!

> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index ec553f8..2ddba69 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
> [...]
> @@ -483,18 +458,9 @@ static void mxs_auart_enable_ms(struct uart_port *port)
>  
>  	s->ms_irq_enabled = true;
>  
> -	if (s->gpio_irq[UART_GPIO_CTS] >= 0)
> -		enable_irq(s->gpio_irq[UART_GPIO_CTS]);
> -	/* TODO: enable AUART_INTR_CTSMIEN otherwise */
> -
> -	if (s->gpio_irq[UART_GPIO_DSR] >= 0)
> -		enable_irq(s->gpio_irq[UART_GPIO_DSR]);
> -
> -	if (s->gpio_irq[UART_GPIO_RI] >= 0)
> -		enable_irq(s->gpio_irq[UART_GPIO_RI]);
> -
> -	if (s->gpio_irq[UART_GPIO_DCD] >= 0)
> -		enable_irq(s->gpio_irq[UART_GPIO_DCD]);
> +	mctrl_gpio_enable_ms(s->gpios);
> +	/* TODO: enable AUART_INTR_CTSMIEN
> +	 * if s->gpios->irq[UART_GPIO_CTS] == 0 */
What is the problem here? For the other lines nothing needs to be done?
This comment doesn't match the coding style.

Other than that, this patch looks good.

Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[RFC PATCH v2 1/4] tty: serial_mctrl_gpio: Add irqs helpers for input lines

[Janusz Użycki]

Hello Uwe.

W dniu 2015-01-13 o 09:03, Uwe Kleine-K?nig pisze:
> On Sat, Jan 10, 2015 at 03:32:43PM +0100, Janusz Uzycki wrote:
>> A driver using mctrl_gpio called gpiod_get_direction() to check if
>> gpio is input line. However .get_direction callback is not available
>> for all platforms. The patch allows to avoid the function.
>> The patch introduces the following helpers:
>> - mctrl_gpio_request_irqs
>> - mctrl_gpio_free_irqs
>> - mctrl_gpio_enable_ms
>> - mctrl_gpio_disable_ms
>> They allow to:
>> - simplify drivers which use mctrl_gpio
>> - hide irq table in mctrl_gpio
>> - use default irq handler for gpios
>> - better separate code for gpio modem lines from uart's code
>> In addition mctrl_gpio_init() has been renamed to mctrl_gpio_init_dt()
> Note that this breaks the drivers that are already using
> mctrl_gpio_init. This is fixed up in the follow up patches, but still
> you break bisection.

Could you give me more details about "break bisection" problem and how 
to correct it?
I've decided to change the name in the same patch as parameter has been 
also changed.


>> to focus DT usage. Also mctrl_gpio_init_dt() initializes irq table for
>> gpios now and passes struct uart_port into struct mctrl_gpios.
>> This resulted in changed mctrl_gpio_init_dt() parameter.
>> It also requires port->dev is set before the function is called.
>>
>> There were also fixed:
>> - irq = 0 means invalid/unused (-EINVAL no more used)
>> - mctrl_gpio_request_irqs() doesn't use negative enum value
>>    if request_irq() failed. It just calls mctrl_gpio_free_irqs().
>>
>> The mctrl_gpio_is_gpio() inline function is under discussion
>> and likely it can replace exported mctrl_gpio_to_gpiod() function.

What do you think about? mctrl_gpio_to_gpiod() could be removed.

>>
>> IRQ_NOAUTOEN setting and request_irq() order was not commented
>> but it looks right according to other drivers.
> I suggest to add a comment for that. Something like:
>
> 	/*
> 	 * XXX: This is not undone in the error path. To be fixed
> 	 * properly this needs to be set atomically together with
> 	 * request_irq.
> 	 */

OK. What does XXX mean?
Since v1 I've seen more drivers set the flag but I didn't find any 
comment about.

>   
>> Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
> There is no binding documentation for mctrl. This should be fixed, too.

The bindings are added to atmel_serial and mxs-auart drivers because 
they use mctrl.
Is it possible to improve bindings and how?

>
>> ---
>>
>> The patch requires to update the drivers which use mctrl_gpio:
>> - atmel_serial
>> - mxs-auart
>> - clps711x
>>
>> Changes since RFC v1:
>>   - patch renamed from:
>>     ("serial: mctrl-gpio: Add irqs helpers for input lines")
>>     to:
>>     ("tty: serial_mctrl_gpio: Add irqs helpers for input lines")
>>   - mctrl_gpio_request_irqs: changed mctrl_gpio_free_irqs() and
>>     dev_err() order to make debug easier
>>   - added patches for atmel_serial and clps711x serial drivers
>>
>> The patch applies to next (3.19.0-rc2) and was tested with mxs-auart
>> using kernel 3.14 and 3.18. It wasn't tested on the next (only compiled).
>>
>> The patchset delivers patches for mxs-auart, atmel_serial and clps711x.
>> atmel_serial and clps711x were not tested - only compile tests were done.
>>
>> ---
>>   drivers/tty/serial/serial_mctrl_gpio.c | 109 ++++++++++++++++++++++++++++++++-
>>   drivers/tty/serial/serial_mctrl_gpio.h |  59 +++++++++++++++++-
>>   2 files changed, 163 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
>> index a38596c..215b15c 100644
>> --- a/drivers/tty/serial/serial_mctrl_gpio.c
>> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
>> @@ -19,11 +19,15 @@
>>   #include <linux/device.h>
>>   #include <linux/gpio/consumer.h>
>>   #include <linux/termios.h>
>> +#include <linux/irq.h>
>>   
>>   #include "serial_mctrl_gpio.h"
>>   
>>   struct mctrl_gpios {
>> +	struct uart_port *port;
>>   	struct gpio_desc *gpio[UART_GPIO_MAX];
>> +	int irq[UART_GPIO_MAX];
>> +	unsigned int mctrl_prev;
>>   };
>>   
>>   static const struct {
>> @@ -72,6 +76,12 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
>>   }
>>   EXPORT_SYMBOL_GPL(mctrl_gpio_to_gpiod);
>>   
>> +inline
>> +bool mctrl_gpio_is_gpio(struct mctrl_gpios *gpios, enum mctrl_gpio_idx gidx)
>> +{
>> +	return !IS_ERR_OR_NULL(gpios->gpio[gidx]);
>> +}
>> +
>>   unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
>>   {
>>   	enum mctrl_gpio_idx i;
>> @@ -96,8 +106,9 @@ unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
>>   }
>>   EXPORT_SYMBOL_GPL(mctrl_gpio_get);
>>   
>> -struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
>> +struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx)
>>   {
>> +	struct device *dev = port->dev;
>>   	struct mctrl_gpios *gpios;
>>   	enum mctrl_gpio_idx i;
>>   	int err;
>> @@ -106,6 +117,7 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
>>   	if (!gpios)
>>   		return ERR_PTR(-ENOMEM);
>>   
>> +	gpios->port = port;
>>   	for (i = 0; i < UART_GPIO_MAX; i++) {
>>   		gpios->gpio[i] = devm_gpiod_get_index(dev,
>>   						      mctrl_gpios_desc[i].name,
>> @@ -128,11 +140,13 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
>>   			devm_gpiod_put(dev, gpios->gpio[i]);
>>   			gpios->gpio[i] = NULL;
>>   		}
>> +		if (gpios->gpio[i] && !mctrl_gpios_desc[i].dir_out)
>> +			gpios->irq[i] = gpiod_to_irq(gpios->gpio[i]);
>>   	}
>>   
>>   	return gpios;
>>   }
>> -EXPORT_SYMBOL_GPL(mctrl_gpio_init);
>> +EXPORT_SYMBOL_GPL(mctrl_gpio_init_dt);
>>   
>>   void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
>>   {
>> @@ -147,3 +161,94 @@ void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
>>   	devm_kfree(dev, gpios);
>>   }
>>   EXPORT_SYMBOL_GPL(mctrl_gpio_free);
>> +
>> +/*
>> + * Dealing with GPIO interrupt
>> + */
>> +#define MCTRL_ANY_DELTA        (TIOCM_RI | TIOCM_DSR | TIOCM_CD | TIOCM_CTS)
>> +static irqreturn_t mctrl_gpio_irq_handle(int irq, void *context)
>> +{
>> +	struct mctrl_gpios *gpios = context;
>> +	struct uart_port *port = gpios->port;
>> +	u32 mctrl = gpios->mctrl_prev;
>> +	u32 mctrl_diff;
>> +
>> +	mctrl_gpio_get(gpios, &mctrl);
>> +
>> +	mctrl_diff = mctrl ^ gpios->mctrl_prev;
>> +	gpios->mctrl_prev = mctrl;
>> +	if (mctrl_diff & MCTRL_ANY_DELTA && port->state != NULL) {
>> +		if (mctrl_diff & TIOCM_RI)
>> +			port->icount.rng++;
>> +		if (mctrl_diff & TIOCM_DSR)
>> +			port->icount.dsr++;
>> +		if (mctrl_diff & TIOCM_CD)
>> +			uart_handle_dcd_change(port, mctrl & TIOCM_CD);
>> +		if (mctrl_diff & TIOCM_CTS)
>> +			uart_handle_cts_change(port, mctrl & TIOCM_CTS);
>> +
>> +		wake_up_interruptible(&port->state->port.delta_msr_wait);
>> +	}
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios)
>> +{
>> +	struct uart_port *port = gpios->port;
>> +	int *irq = gpios->irq;
>> +	enum mctrl_gpio_idx i;
>> +	int err = 0;
>> +
>> +	for (i = 0; i < UART_GPIO_MAX; i++) {
>> +		if (irq[i] <= 0)
>> +			continue;
>> +
>> +		irq_set_status_flags(irq[i], IRQ_NOAUTOEN);
>> +		err = request_irq(irq[i], mctrl_gpio_irq_handle,
>> +				  IRQ_TYPE_EDGE_BOTH,
>> +				  dev_name(port->dev), gpios);
>> +		if (err) {
>> +			dev_err(port->dev, "%s: Can't get %d irq\n",
>> +				__func__, irq[i]);
>> +			mctrl_gpio_free_irqs(gpios);
>> +			break;
>> +		}
>> +	}
>> +
>> +	return err;
>> +}
>> +EXPORT_SYMBOL_GPL(mctrl_gpio_request_irqs);
>> +
>> +void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios)
>> +{
>> +	enum mctrl_gpio_idx i;
>> +
>> +	for (i = 0; i < UART_GPIO_MAX; i++)
>> +		if (gpios->irq[i] > 0)
>> +			free_irq(gpios->irq[i], gpios);
>> +}
>> +EXPORT_SYMBOL_GPL(mctrl_gpio_free_irqs);
>> +
>> +void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios)
>> +{
>> +	enum mctrl_gpio_idx i;
>> +
>> +	/* get initial status of modem lines GPIOs */
>> +	mctrl_gpio_get(gpios, &gpios->mctrl_prev);
>> +
>> +	for (i = 0; i < UART_GPIO_MAX; i++)
>> +		if (gpios->irq[i] > 0)
>> +			enable_irq(gpios->irq[i]);
>> +}
>> +EXPORT_SYMBOL_GPL(mctrl_gpio_enable_ms);
>> +
>> +void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios)
>> +{
>> +	enum mctrl_gpio_idx i;
>> +
>> +	for (i = 0; i < UART_GPIO_MAX; i++)
>> +		if (gpios->irq[i] > 0)
>> +			disable_irq(gpios->irq[i]);
>> +}
>> +EXPORT_SYMBOL_GPL(mctrl_gpio_disable_ms);

It looks the functions should check if gpios is not NULL or drivers 
should fail probe on it.
Which is the right way?

>> diff --git a/drivers/tty/serial/serial_mctrl_gpio.h b/drivers/tty/serial/serial_mctrl_gpio.h
>> index 400ba04..13ba3f4 100644
>> --- a/drivers/tty/serial/serial_mctrl_gpio.h
>> +++ b/drivers/tty/serial/serial_mctrl_gpio.h
>> @@ -21,6 +21,7 @@
>>   #include <linux/err.h>
>>   #include <linux/device.h>
>>   #include <linux/gpio/consumer.h>
>> +#include <linux/serial_core.h>
>>   
>>   enum mctrl_gpio_idx {
>>   	UART_GPIO_CTS,
>> @@ -40,6 +41,13 @@ enum mctrl_gpio_idx {
>>    */
>>   struct mctrl_gpios;
>>   
>> +/*
>> + * Return true if gidx is GPIO line, otherwise false.
>> + * It assumes that gpios is allocated and not NULL.
>> + */
>> +inline
>> +bool mctrl_gpio_is_gpio(struct mctrl_gpios *gpios, enum mctrl_gpio_idx gidx);
>> +
>>   #ifdef CONFIG_GPIOLIB
>>   
>>   /*
>> @@ -60,12 +68,13 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
>>   				      enum mctrl_gpio_idx gidx);
>>   
>>   /*
>> - * Request and set direction of modem control lines GPIOs.
>> + * Request and set direction of modem control lines GPIOs. DT is used.
>> + * Initialize irq table for GPIOs.
>>    * devm_* functions are used, so there's no need to call mctrl_gpio_free().
>>    * Returns a pointer to the allocated mctrl structure if ok, -ENOMEM on
>>    * allocation error.
>>    */
>> -struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx);
>> +struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx);
>>   
>>   /*
>>    * Free the mctrl_gpios structure.
>> @@ -74,6 +83,28 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx);
>>    */
>>   void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios);
>>   
>> +/*
>> + * Request irqs for input lines GPIOs. The irqs are set disabled
>> + * and triggered on both edges.
>> + * Returns zero if OK, otherwise an error code.
>> + */
>> +int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios);
>> +
>> +/*
>> + * Free irqs for input lines GPIOs.
>> + */
>> +void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios);
>> +
>> +/*
>> + * Disable modem status interrupts assigned to GPIOs
>> + */
>> +void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios);
>> +
>> +/*
>> + * Enable modem status interrupts assigned to GPIOs
>> + */
>> +void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios);
>> +
>>   #else /* GPIOLIB */
>>   
>>   static inline
>> @@ -95,7 +126,7 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
>>   }
>>   
>>   static inline
>> -struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
>> +struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx)
>>   {
>>   	return ERR_PTR(-ENOSYS);
>>   }
>> @@ -105,6 +136,28 @@ void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
>>   {
>>   }
>>   
>> +static inline
>> +int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios)
>> +{
>> +	/*return -ENOTSUP;*/
>> +	return 0;
>> +}
>> +
>> +static inline
>> +void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios)
>> +{
>> +}
>> +
>> +static inline
>> +void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios)
>> +{
>> +}
>> +
>> +static inline
>> +void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios)
>> +{
>> +}
>> +
>>   #endif /* GPIOLIB */
>>   
>>   #endif
>> -- 
>> 1.7.11.3
>>
>>

[RFC PATCH v2 2/4] serial: mxs-auart: Use helpers for gpio irqs

[Janusz Użycki]

W dniu 2015-01-13 o 09:08, Uwe Kleine-K?nig pisze:
> Hello,
>
> On Sat, Jan 10, 2015 at 03:32:44PM +0100, Janusz Uzycki wrote:
>> The patch updates mxs-auart driver to use new mctrl_gpio helpers for
>> gpio irqs. The code is much simpler now.
>>
>> Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
>> ---
>>
>> There is no changes since v1 (rebased only).
>>
>> ---
>>   drivers/tty/serial/mxs-auart.c | 133 ++++-------------------------------------
>>   1 file changed, 13 insertions(+), 120 deletions(-)
> Very nice!
>
>> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
>> index ec553f8..2ddba69 100644
>> --- a/drivers/tty/serial/mxs-auart.c
>> +++ b/drivers/tty/serial/mxs-auart.c
>> [...]
>> @@ -483,18 +458,9 @@ static void mxs_auart_enable_ms(struct uart_port *port)
>>   
>>   	s->ms_irq_enabled = true;
>>   
>> -	if (s->gpio_irq[UART_GPIO_CTS] >= 0)
>> -		enable_irq(s->gpio_irq[UART_GPIO_CTS]);
>> -	/* TODO: enable AUART_INTR_CTSMIEN otherwise */
>> -
>> -	if (s->gpio_irq[UART_GPIO_DSR] >= 0)
>> -		enable_irq(s->gpio_irq[UART_GPIO_DSR]);
>> -
>> -	if (s->gpio_irq[UART_GPIO_RI] >= 0)
>> -		enable_irq(s->gpio_irq[UART_GPIO_RI]);
>> -
>> -	if (s->gpio_irq[UART_GPIO_DCD] >= 0)
>> -		enable_irq(s->gpio_irq[UART_GPIO_DCD]);
>> +	mctrl_gpio_enable_ms(s->gpios);
>> +	/* TODO: enable AUART_INTR_CTSMIEN
>> +	 * if s->gpios->irq[UART_GPIO_CTS] == 0 */
> What is the problem here? For the other lines nothing needs to be done?
> This comment doesn't match the coding style.

Right, the comment should be rather:
/* TODO: enable AUART_INTR_CTSMIEN
  * if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_CTS)) */

In this place I marked that CTSMIEN should be switched on 
enable/disable_ms if CTS
is not a gpio. The driver enables CTSMIEN forever what is wrong but I 
can't test it
and I don't need it so I've just marked the fact in the comment.

>
> Other than that, this patch looks good.
>
> Uwe
>

Thanks
Janusz

[RFC PATCH v2 2/4] serial: mxs-auart: Use helpers for gpio irqs

[Uwe Kleine-König]

Hello,

On Tue, Jan 13, 2015 at 10:29:44AM +0100, Janusz U?ycki wrote:
> 
> W dniu 2015-01-13 o 09:08, Uwe Kleine-K?nig pisze:
> >Hello,
> >
> >On Sat, Jan 10, 2015 at 03:32:44PM +0100, Janusz Uzycki wrote:
> >>The patch updates mxs-auart driver to use new mctrl_gpio helpers for
> >>gpio irqs. The code is much simpler now.
> >>
> >>Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
> >>---
> >>
> >>There is no changes since v1 (rebased only).
> >>
> >>---
> >>  drivers/tty/serial/mxs-auart.c | 133 ++++-------------------------------------
> >>  1 file changed, 13 insertions(+), 120 deletions(-)
> >Very nice!
> >
> >>diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> >>index ec553f8..2ddba69 100644
> >>--- a/drivers/tty/serial/mxs-auart.c
> >>+++ b/drivers/tty/serial/mxs-auart.c
> >>[...]
> >>@@ -483,18 +458,9 @@ static void mxs_auart_enable_ms(struct uart_port *port)
> >>  	s->ms_irq_enabled = true;
> >>-	if (s->gpio_irq[UART_GPIO_CTS] >= 0)
> >>-		enable_irq(s->gpio_irq[UART_GPIO_CTS]);
> >>-	/* TODO: enable AUART_INTR_CTSMIEN otherwise */
> >>-
> >>-	if (s->gpio_irq[UART_GPIO_DSR] >= 0)
> >>-		enable_irq(s->gpio_irq[UART_GPIO_DSR]);
> >>-
> >>-	if (s->gpio_irq[UART_GPIO_RI] >= 0)
> >>-		enable_irq(s->gpio_irq[UART_GPIO_RI]);
> >>-
> >>-	if (s->gpio_irq[UART_GPIO_DCD] >= 0)
> >>-		enable_irq(s->gpio_irq[UART_GPIO_DCD]);
> >>+	mctrl_gpio_enable_ms(s->gpios);
> >>+	/* TODO: enable AUART_INTR_CTSMIEN
> >>+	 * if s->gpios->irq[UART_GPIO_CTS] == 0 */
> >What is the problem here? For the other lines nothing needs to be done?
> >This comment doesn't match the coding style.
> 
> Right, the comment should be rather:
> /* TODO: enable AUART_INTR_CTSMIEN
>  * if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_CTS)) */
I'd say:

	/*
	 * TODO: enable AUART_INTR_CTSMIEN if CTS isn't handled by
	 * mctrl_gpio.
	 */

> In this place I marked that CTSMIEN should be switched on
> enable/disable_ms if CTS
> is not a gpio. The driver enables CTSMIEN forever what is wrong but
> I can't test it
> and I don't need it so I've just marked the fact in the comment.
That's what I thought. You're not affected because CTS is a gpio for
you (or not?)?  What would be the effect otherwise?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[RFC PATCH v2 2/4] serial: mxs-auart: Use helpers for gpio irqs

[Janusz Użycki]

W dniu 2015-01-13 o 10:35, Uwe Kleine-K?nig pisze:
> Hello,
>
> On Tue, Jan 13, 2015 at 10:29:44AM +0100, Janusz U?ycki wrote:
>> W dniu 2015-01-13 o 09:08, Uwe Kleine-K?nig pisze:
>>> Hello,
>>>
>>> On Sat, Jan 10, 2015 at 03:32:44PM +0100, Janusz Uzycki wrote:
>>>> The patch updates mxs-auart driver to use new mctrl_gpio helpers for
>>>> gpio irqs. The code is much simpler now.
>>>>
>>>> Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
>>>> ---
>>>>
>>>> There is no changes since v1 (rebased only).
>>>>
>>>> ---
>>>>   drivers/tty/serial/mxs-auart.c | 133 ++++-------------------------------------
>>>>   1 file changed, 13 insertions(+), 120 deletions(-)
>>> Very nice!
>>>
>>>> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
>>>> index ec553f8..2ddba69 100644
>>>> --- a/drivers/tty/serial/mxs-auart.c
>>>> +++ b/drivers/tty/serial/mxs-auart.c
>>>> [...]
>>>> @@ -483,18 +458,9 @@ static void mxs_auart_enable_ms(struct uart_port *port)
>>>>   	s->ms_irq_enabled = true;
>>>> -	if (s->gpio_irq[UART_GPIO_CTS] >= 0)
>>>> -		enable_irq(s->gpio_irq[UART_GPIO_CTS]);
>>>> -	/* TODO: enable AUART_INTR_CTSMIEN otherwise */
>>>> -
>>>> -	if (s->gpio_irq[UART_GPIO_DSR] >= 0)
>>>> -		enable_irq(s->gpio_irq[UART_GPIO_DSR]);
>>>> -
>>>> -	if (s->gpio_irq[UART_GPIO_RI] >= 0)
>>>> -		enable_irq(s->gpio_irq[UART_GPIO_RI]);
>>>> -
>>>> -	if (s->gpio_irq[UART_GPIO_DCD] >= 0)
>>>> -		enable_irq(s->gpio_irq[UART_GPIO_DCD]);
>>>> +	mctrl_gpio_enable_ms(s->gpios);
>>>> +	/* TODO: enable AUART_INTR_CTSMIEN
>>>> +	 * if s->gpios->irq[UART_GPIO_CTS] == 0 */
>>> What is the problem here? For the other lines nothing needs to be done?
>>> This comment doesn't match the coding style.
>> Right, the comment should be rather:
>> /* TODO: enable AUART_INTR_CTSMIEN
>>   * if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_CTS)) */
> I'd say:
>
> 	/*
> 	 * TODO: enable AUART_INTR_CTSMIEN if CTS isn't handled by
> 	 * mctrl_gpio.
> 	 */

exactly, thanks

>
>> In this place I marked that CTSMIEN should be switched on
>> enable/disable_ms if CTS
>> is not a gpio. The driver enables CTSMIEN forever what is wrong but
>> I can't test it
>> and I don't need it so I've just marked the fact in the comment.
> That's what I thought. You're not affected because CTS is a gpio for
> you (or not?)?  What would be the effect otherwise?

Yes, my CTS is a gpio.
CTSMIEN control CTS signal of auart block. There is a choice in DT:
- use auart block's CTS: hardware flow control works for all baud rates, 
DMA can be used
- use gpio as CTS: hardware flow control is limited, DMA disabled but 
CTS line is
   not limited by hardware pinmux
Both options can't be set at once. I workarounded auart block's CTS irq 
handler in condition:
"if (CTS_AT_AUART() && s->ms_irq_enabled)". Support by _ms would be more 
elegance
but as I wrote I couldn't test all cases. Therefore the code for auart 
block's CTS is preserved.

best regards
Janusz

>
> Best regards
> Uwe
>

[RFC PATCH v2 1/4] tty: serial_mctrl_gpio: Add irqs helpers for input lines

[Richard Genoud]

2015-01-10 15:32 GMT+01:00 Janusz Uzycki <j.uzycki@elproma.com.pl>:
> A driver using mctrl_gpio called gpiod_get_direction() to check if
> gpio is input line. However .get_direction callback is not available
> for all platforms. The patch allows to avoid the function.
> The patch introduces the following helpers:
> - mctrl_gpio_request_irqs
> - mctrl_gpio_free_irqs
> - mctrl_gpio_enable_ms
> - mctrl_gpio_disable_ms
> They allow to:
> - simplify drivers which use mctrl_gpio
> - hide irq table in mctrl_gpio
> - use default irq handler for gpios
> - better separate code for gpio modem lines from uart's code
> In addition mctrl_gpio_init() has been renamed to mctrl_gpio_init_dt()
> to focus DT usage. Also mctrl_gpio_init_dt() initializes irq table for
> gpios now and passes struct uart_port into struct mctrl_gpios.
> This resulted in changed mctrl_gpio_init_dt() parameter.
> It also requires port->dev is set before the function is called.
>
> There were also fixed:
> - irq = 0 means invalid/unused (-EINVAL no more used)
> - mctrl_gpio_request_irqs() doesn't use negative enum value
>   if request_irq() failed. It just calls mctrl_gpio_free_irqs().
>
> The mctrl_gpio_is_gpio() inline function is under discussion
> and likely it can replace exported mctrl_gpio_to_gpiod() function.
>
> IRQ_NOAUTOEN setting and request_irq() order was not commented
> but it looks right according to other drivers.
>
> Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
> ---
>
> The patch requires to update the drivers which use mctrl_gpio:
> - atmel_serial
> - mxs-auart
> - clps711x
>
> Changes since RFC v1:
>  - patch renamed from:
>    ("serial: mctrl-gpio: Add irqs helpers for input lines")
>    to:
>    ("tty: serial_mctrl_gpio: Add irqs helpers for input lines")
>  - mctrl_gpio_request_irqs: changed mctrl_gpio_free_irqs() and
>    dev_err() order to make debug easier
>  - added patches for atmel_serial and clps711x serial drivers
>
> The patch applies to next (3.19.0-rc2) and was tested with mxs-auart
> using kernel 3.14 and 3.18. It wasn't tested on the next (only compiled).
>
> The patchset delivers patches for mxs-auart, atmel_serial and clps711x.
> atmel_serial and clps711x were not tested - only compile tests were done.
>
> ---
>  drivers/tty/serial/serial_mctrl_gpio.c | 109 ++++++++++++++++++++++++++++++++-
>  drivers/tty/serial/serial_mctrl_gpio.h |  59 +++++++++++++++++-
>  2 files changed, 163 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
> index a38596c..215b15c 100644
> --- a/drivers/tty/serial/serial_mctrl_gpio.c
> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
> @@ -19,11 +19,15 @@
>  #include <linux/device.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/termios.h>
> +#include <linux/irq.h>
>
>  #include "serial_mctrl_gpio.h"
>
>  struct mctrl_gpios {
> +       struct uart_port *port;
>         struct gpio_desc *gpio[UART_GPIO_MAX];
> +       int irq[UART_GPIO_MAX];
> +       unsigned int mctrl_prev;
>  };
>
>  static const struct {
> @@ -72,6 +76,12 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
>  }
>  EXPORT_SYMBOL_GPL(mctrl_gpio_to_gpiod);
>
> +inline
> +bool mctrl_gpio_is_gpio(struct mctrl_gpios *gpios, enum mctrl_gpio_idx gidx)
> +{
> +       return !IS_ERR_OR_NULL(gpios->gpio[gidx]);
> +}
> +
>  unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
>  {
>         enum mctrl_gpio_idx i;
> @@ -96,8 +106,9 @@ unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
>  }
>  EXPORT_SYMBOL_GPL(mctrl_gpio_get);
>
> -struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
> +struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx)
>  {
> +       struct device *dev = port->dev;
>         struct mctrl_gpios *gpios;
>         enum mctrl_gpio_idx i;
>         int err;
> @@ -106,6 +117,7 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
>         if (!gpios)
>                 return ERR_PTR(-ENOMEM);
>
> +       gpios->port = port;
>         for (i = 0; i < UART_GPIO_MAX; i++) {
>                 gpios->gpio[i] = devm_gpiod_get_index(dev,
>                                                       mctrl_gpios_desc[i].name,
> @@ -128,11 +140,13 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
>                         devm_gpiod_put(dev, gpios->gpio[i]);
>                         gpios->gpio[i] = NULL;
>                 }
> +               if (gpios->gpio[i] && !mctrl_gpios_desc[i].dir_out)
> +                       gpios->irq[i] = gpiod_to_irq(gpios->gpio[i]);
>         }
>
>         return gpios;
>  }
> -EXPORT_SYMBOL_GPL(mctrl_gpio_init);
> +EXPORT_SYMBOL_GPL(mctrl_gpio_init_dt);
>
>  void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
>  {
> @@ -147,3 +161,94 @@ void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
>         devm_kfree(dev, gpios);
>  }
>  EXPORT_SYMBOL_GPL(mctrl_gpio_free);
> +
> +/*
> + * Dealing with GPIO interrupt
> + */
> +#define MCTRL_ANY_DELTA        (TIOCM_RI | TIOCM_DSR | TIOCM_CD | TIOCM_CTS)
> +static irqreturn_t mctrl_gpio_irq_handle(int irq, void *context)
> +{
> +       struct mctrl_gpios *gpios = context;
> +       struct uart_port *port = gpios->port;
> +       u32 mctrl = gpios->mctrl_prev;
> +       u32 mctrl_diff;
> +
> +       mctrl_gpio_get(gpios, &mctrl);
> +
> +       mctrl_diff = mctrl ^ gpios->mctrl_prev;
> +       gpios->mctrl_prev = mctrl;
> +       if (mctrl_diff & MCTRL_ANY_DELTA && port->state != NULL) {
> +               if (mctrl_diff & TIOCM_RI)
> +                       port->icount.rng++;
> +               if (mctrl_diff & TIOCM_DSR)
> +                       port->icount.dsr++;
> +               if (mctrl_diff & TIOCM_CD)
> +                       uart_handle_dcd_change(port, mctrl & TIOCM_CD);
> +               if (mctrl_diff & TIOCM_CTS)
> +                       uart_handle_cts_change(port, mctrl & TIOCM_CTS);
> +
> +               wake_up_interruptible(&port->state->port.delta_msr_wait);
> +       }
> +
> +       return IRQ_HANDLED;
> +}
> +
> +int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios)
> +{
> +       struct uart_port *port = gpios->port;
> +       int *irq = gpios->irq;
> +       enum mctrl_gpio_idx i;
> +       int err = 0;
> +
> +       for (i = 0; i < UART_GPIO_MAX; i++) {
> +               if (irq[i] <= 0)
> +                       continue;
> +
> +               irq_set_status_flags(irq[i], IRQ_NOAUTOEN);
> +               err = request_irq(irq[i], mctrl_gpio_irq_handle,
> +                                 IRQ_TYPE_EDGE_BOTH,
> +                                 dev_name(port->dev), gpios);
> +               if (err) {
> +                       dev_err(port->dev, "%s: Can't get %d irq\n",
> +                               __func__, irq[i]);
> +                       mctrl_gpio_free_irqs(gpios);
> +                       break;
> +               }
> +       }
> +
> +       return err;
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_request_irqs);
> +
> +void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios)
> +{
> +       enum mctrl_gpio_idx i;
> +
> +       for (i = 0; i < UART_GPIO_MAX; i++)
> +               if (gpios->irq[i] > 0)
> +                       free_irq(gpios->irq[i], gpios);
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_free_irqs);
> +
> +void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios)
> +{
> +       enum mctrl_gpio_idx i;
> +
> +       /* get initial status of modem lines GPIOs */
> +       mctrl_gpio_get(gpios, &gpios->mctrl_prev);
> +
> +       for (i = 0; i < UART_GPIO_MAX; i++)
> +               if (gpios->irq[i] > 0)
> +                       enable_irq(gpios->irq[i]);
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_enable_ms);
> +
> +void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios)
> +{
> +       enum mctrl_gpio_idx i;
> +
> +       for (i = 0; i < UART_GPIO_MAX; i++)
> +               if (gpios->irq[i] > 0)
> +                       disable_irq(gpios->irq[i]);
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_disable_ms);
> diff --git a/drivers/tty/serial/serial_mctrl_gpio.h b/drivers/tty/serial/serial_mctrl_gpio.h
> index 400ba04..13ba3f4 100644
> --- a/drivers/tty/serial/serial_mctrl_gpio.h
> +++ b/drivers/tty/serial/serial_mctrl_gpio.h
> @@ -21,6 +21,7 @@
>  #include <linux/err.h>
>  #include <linux/device.h>
>  #include <linux/gpio/consumer.h>
> +#include <linux/serial_core.h>
>
>  enum mctrl_gpio_idx {
>         UART_GPIO_CTS,
> @@ -40,6 +41,13 @@ enum mctrl_gpio_idx {
>   */
>  struct mctrl_gpios;
>
> +/*
> + * Return true if gidx is GPIO line, otherwise false.
> + * It assumes that gpios is allocated and not NULL.
> + */
> +inline
> +bool mctrl_gpio_is_gpio(struct mctrl_gpios *gpios, enum mctrl_gpio_idx gidx);
> +
This leads to a compile error :
  CC      drivers/tty/serial/atmel_serial.o
In file included from drivers/tty/serial/atmel_serial.c:64:0:
drivers/tty/serial/atmel_serial.c: In function 'atmel_disable_ms.part.22':
drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
call to always_inline 'mctrl_gpio_is_gpio': function body not
available
drivers/tty/serial/atmel_serial.c:536:25: error: called from here
In file included from drivers/tty/serial/atmel_serial.c:64:0:
drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
call to always_inline 'mctrl_gpio_is_gpio': function body not
available
drivers/tty/serial/atmel_serial.c:539:25: error: called from here
In file included from drivers/tty/serial/atmel_serial.c:64:0:
drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
call to always_inline 'mctrl_gpio_is_gpio': function body not
available
drivers/tty/serial/atmel_serial.c:542:25: error: called from here
In file included from drivers/tty/serial/atmel_serial.c:64:0:
drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
call to always_inline 'mctrl_gpio_is_gpio': function body not
available
drivers/tty/serial/atmel_serial.c:545:25: error: called from here
In file included from drivers/tty/serial/atmel_serial.c:64:0:
drivers/tty/serial/atmel_serial.c: In function 'atmel_enable_ms':
drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
call to always_inline 'mctrl_gpio_is_gpio': function body not
available
drivers/tty/serial/atmel_serial.c:503:25: error: called from here
In file included from drivers/tty/serial/atmel_serial.c:64:0:
drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
call to always_inline 'mctrl_gpio_is_gpio': function body not
available
drivers/tty/serial/atmel_serial.c:506:25: error: called from here
In file included from drivers/tty/serial/atmel_serial.c:64:0:
drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
call to always_inline 'mctrl_gpio_is_gpio': function body not
available
drivers/tty/serial/atmel_serial.c:509:25: error: called from here
In file included from drivers/tty/serial/atmel_serial.c:64:0:
drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
call to always_inline 'mctrl_gpio_is_gpio': function body not
available
drivers/tty/serial/atmel_serial.c:512:25: error: called from here
make[3]: *** [drivers/tty/serial/atmel_serial.o] Error 1
make[2]: *** [drivers/tty/serial] Error 2
make[1]: *** [drivers/tty] Error 2
make: *** [drivers] Error 2



>  #ifdef CONFIG_GPIOLIB
>
>  /*
> @@ -60,12 +68,13 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
>                                       enum mctrl_gpio_idx gidx);
>
>  /*
> - * Request and set direction of modem control lines GPIOs.
> + * Request and set direction of modem control lines GPIOs. DT is used.
> + * Initialize irq table for GPIOs.
>   * devm_* functions are used, so there's no need to call mctrl_gpio_free().
>   * Returns a pointer to the allocated mctrl structure if ok, -ENOMEM on
>   * allocation error.
>   */
> -struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx);
> +struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx);
>
>  /*
>   * Free the mctrl_gpios structure.
> @@ -74,6 +83,28 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx);
>   */
>  void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios);
>
> +/*
> + * Request irqs for input lines GPIOs. The irqs are set disabled
> + * and triggered on both edges.
> + * Returns zero if OK, otherwise an error code.
> + */
> +int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios);
> +
> +/*
> + * Free irqs for input lines GPIOs.
> + */
> +void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios);
> +
> +/*
> + * Disable modem status interrupts assigned to GPIOs
> + */
> +void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios);
> +
> +/*
> + * Enable modem status interrupts assigned to GPIOs
> + */
> +void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios);
> +
>  #else /* GPIOLIB */
>
>  static inline
> @@ -95,7 +126,7 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
>  }
>
>  static inline
> -struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
> +struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx)
>  {
>         return ERR_PTR(-ENOSYS);
>  }
> @@ -105,6 +136,28 @@ void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
>  {
>  }
>
> +static inline
> +int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios)
> +{
> +       /*return -ENOTSUP;*/
> +       return 0;
> +}
> +
> +static inline
> +void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios)
> +{
> +}
> +
> +static inline
> +void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios)
> +{
> +}
> +
> +static inline
> +void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios)
> +{
> +}
> +
>  #endif /* GPIOLIB */
>
>  #endif
> --
> 1.7.11.3
>

I think you should also update the documentation on mctrl_ helpers :
Documentation/serial/driver

I'm going to do some tests on atmel_serial.c

regards,
Richard.

[RFC PATCH v2 1/4] tty: serial_mctrl_gpio: Add irqs helpers for input lines

[Janusz Użycki]

W dniu 2015-01-13 o 14:04, Richard Genoud pisze:
> 2015-01-10 15:32 GMT+01:00 Janusz Uzycki <j.uzycki@elproma.com.pl>:
>> A driver using mctrl_gpio called gpiod_get_direction() to check if
>> gpio is input line. However .get_direction callback is not available
>> for all platforms. The patch allows to avoid the function.
>> The patch introduces the following helpers:
>> - mctrl_gpio_request_irqs
>> - mctrl_gpio_free_irqs
>> - mctrl_gpio_enable_ms
>> - mctrl_gpio_disable_ms
>> They allow to:
>> - simplify drivers which use mctrl_gpio
>> - hide irq table in mctrl_gpio
>> - use default irq handler for gpios
>> - better separate code for gpio modem lines from uart's code
>> In addition mctrl_gpio_init() has been renamed to mctrl_gpio_init_dt()
>> to focus DT usage. Also mctrl_gpio_init_dt() initializes irq table for
>> gpios now and passes struct uart_port into struct mctrl_gpios.
>> This resulted in changed mctrl_gpio_init_dt() parameter.
>> It also requires port->dev is set before the function is called.
>>
>> There were also fixed:
>> - irq = 0 means invalid/unused (-EINVAL no more used)
>> - mctrl_gpio_request_irqs() doesn't use negative enum value
>>    if request_irq() failed. It just calls mctrl_gpio_free_irqs().
>>
>> The mctrl_gpio_is_gpio() inline function is under discussion
>> and likely it can replace exported mctrl_gpio_to_gpiod() function.
>>
>> IRQ_NOAUTOEN setting and request_irq() order was not commented
>> but it looks right according to other drivers.
>>
>> Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
>> ---
>>
>> The patch requires to update the drivers which use mctrl_gpio:
>> - atmel_serial
>> - mxs-auart
>> - clps711x
>>
>> Changes since RFC v1:
>>   - patch renamed from:
>>     ("serial: mctrl-gpio: Add irqs helpers for input lines")
>>     to:
>>     ("tty: serial_mctrl_gpio: Add irqs helpers for input lines")
>>   - mctrl_gpio_request_irqs: changed mctrl_gpio_free_irqs() and
>>     dev_err() order to make debug easier
>>   - added patches for atmel_serial and clps711x serial drivers
>>
>> The patch applies to next (3.19.0-rc2) and was tested with mxs-auart
>> using kernel 3.14 and 3.18. It wasn't tested on the next (only compiled).
>>
>> The patchset delivers patches for mxs-auart, atmel_serial and clps711x.
>> atmel_serial and clps711x were not tested - only compile tests were done.
>>
>> ---
>>   drivers/tty/serial/serial_mctrl_gpio.c | 109 ++++++++++++++++++++++++++++++++-
>>   drivers/tty/serial/serial_mctrl_gpio.h |  59 +++++++++++++++++-
>>   2 files changed, 163 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
>> index a38596c..215b15c 100644
>> --- a/drivers/tty/serial/serial_mctrl_gpio.c
>> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
>> @@ -19,11 +19,15 @@
>>   #include <linux/device.h>
>>   #include <linux/gpio/consumer.h>
>>   #include <linux/termios.h>
>> +#include <linux/irq.h>
>>
>>   #include "serial_mctrl_gpio.h"
>>
>>   struct mctrl_gpios {
>> +       struct uart_port *port;
>>          struct gpio_desc *gpio[UART_GPIO_MAX];
>> +       int irq[UART_GPIO_MAX];
>> +       unsigned int mctrl_prev;
>>   };
>>
>>   static const struct {
>> @@ -72,6 +76,12 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
>>   }
>>   EXPORT_SYMBOL_GPL(mctrl_gpio_to_gpiod);
>>
>> +inline
>> +bool mctrl_gpio_is_gpio(struct mctrl_gpios *gpios, enum mctrl_gpio_idx gidx)
>> +{
>> +       return !IS_ERR_OR_NULL(gpios->gpio[gidx]);
>> +}
>> +
>>   unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
>>   {
>>          enum mctrl_gpio_idx i;
>> @@ -96,8 +106,9 @@ unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
>>   }
>>   EXPORT_SYMBOL_GPL(mctrl_gpio_get);
>>
>> -struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
>> +struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx)
>>   {
>> +       struct device *dev = port->dev;
>>          struct mctrl_gpios *gpios;
>>          enum mctrl_gpio_idx i;
>>          int err;
>> @@ -106,6 +117,7 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
>>          if (!gpios)
>>                  return ERR_PTR(-ENOMEM);
>>
>> +       gpios->port = port;
>>          for (i = 0; i < UART_GPIO_MAX; i++) {
>>                  gpios->gpio[i] = devm_gpiod_get_index(dev,
>>                                                        mctrl_gpios_desc[i].name,
>> @@ -128,11 +140,13 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
>>                          devm_gpiod_put(dev, gpios->gpio[i]);
>>                          gpios->gpio[i] = NULL;
>>                  }
>> +               if (gpios->gpio[i] && !mctrl_gpios_desc[i].dir_out)
>> +                       gpios->irq[i] = gpiod_to_irq(gpios->gpio[i]);
>>          }
>>
>>          return gpios;
>>   }
>> -EXPORT_SYMBOL_GPL(mctrl_gpio_init);
>> +EXPORT_SYMBOL_GPL(mctrl_gpio_init_dt);
>>
>>   void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
>>   {
>> @@ -147,3 +161,94 @@ void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
>>          devm_kfree(dev, gpios);
>>   }
>>   EXPORT_SYMBOL_GPL(mctrl_gpio_free);
>> +
>> +/*
>> + * Dealing with GPIO interrupt
>> + */
>> +#define MCTRL_ANY_DELTA        (TIOCM_RI | TIOCM_DSR | TIOCM_CD | TIOCM_CTS)
>> +static irqreturn_t mctrl_gpio_irq_handle(int irq, void *context)
>> +{
>> +       struct mctrl_gpios *gpios = context;
>> +       struct uart_port *port = gpios->port;
>> +       u32 mctrl = gpios->mctrl_prev;
>> +       u32 mctrl_diff;
>> +
>> +       mctrl_gpio_get(gpios, &mctrl);
>> +
>> +       mctrl_diff = mctrl ^ gpios->mctrl_prev;
>> +       gpios->mctrl_prev = mctrl;
>> +       if (mctrl_diff & MCTRL_ANY_DELTA && port->state != NULL) {
>> +               if (mctrl_diff & TIOCM_RI)
>> +                       port->icount.rng++;
>> +               if (mctrl_diff & TIOCM_DSR)
>> +                       port->icount.dsr++;
>> +               if (mctrl_diff & TIOCM_CD)
>> +                       uart_handle_dcd_change(port, mctrl & TIOCM_CD);
>> +               if (mctrl_diff & TIOCM_CTS)
>> +                       uart_handle_cts_change(port, mctrl & TIOCM_CTS);
>> +
>> +               wake_up_interruptible(&port->state->port.delta_msr_wait);
>> +       }
>> +
>> +       return IRQ_HANDLED;
>> +}
>> +
>> +int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios)
>> +{
>> +       struct uart_port *port = gpios->port;
>> +       int *irq = gpios->irq;
>> +       enum mctrl_gpio_idx i;
>> +       int err = 0;
>> +
>> +       for (i = 0; i < UART_GPIO_MAX; i++) {
>> +               if (irq[i] <= 0)
>> +                       continue;
>> +
>> +               irq_set_status_flags(irq[i], IRQ_NOAUTOEN);
>> +               err = request_irq(irq[i], mctrl_gpio_irq_handle,
>> +                                 IRQ_TYPE_EDGE_BOTH,
>> +                                 dev_name(port->dev), gpios);
>> +               if (err) {
>> +                       dev_err(port->dev, "%s: Can't get %d irq\n",
>> +                               __func__, irq[i]);
>> +                       mctrl_gpio_free_irqs(gpios);
>> +                       break;
>> +               }
>> +       }
>> +
>> +       return err;
>> +}
>> +EXPORT_SYMBOL_GPL(mctrl_gpio_request_irqs);
>> +
>> +void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios)
>> +{
>> +       enum mctrl_gpio_idx i;
>> +
>> +       for (i = 0; i < UART_GPIO_MAX; i++)
>> +               if (gpios->irq[i] > 0)
>> +                       free_irq(gpios->irq[i], gpios);
>> +}
>> +EXPORT_SYMBOL_GPL(mctrl_gpio_free_irqs);
>> +
>> +void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios)
>> +{
>> +       enum mctrl_gpio_idx i;
>> +
>> +       /* get initial status of modem lines GPIOs */
>> +       mctrl_gpio_get(gpios, &gpios->mctrl_prev);
>> +
>> +       for (i = 0; i < UART_GPIO_MAX; i++)
>> +               if (gpios->irq[i] > 0)
>> +                       enable_irq(gpios->irq[i]);
>> +}
>> +EXPORT_SYMBOL_GPL(mctrl_gpio_enable_ms);
>> +
>> +void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios)
>> +{
>> +       enum mctrl_gpio_idx i;
>> +
>> +       for (i = 0; i < UART_GPIO_MAX; i++)
>> +               if (gpios->irq[i] > 0)
>> +                       disable_irq(gpios->irq[i]);
>> +}
>> +EXPORT_SYMBOL_GPL(mctrl_gpio_disable_ms);
>> diff --git a/drivers/tty/serial/serial_mctrl_gpio.h b/drivers/tty/serial/serial_mctrl_gpio.h
>> index 400ba04..13ba3f4 100644
>> --- a/drivers/tty/serial/serial_mctrl_gpio.h
>> +++ b/drivers/tty/serial/serial_mctrl_gpio.h
>> @@ -21,6 +21,7 @@
>>   #include <linux/err.h>
>>   #include <linux/device.h>
>>   #include <linux/gpio/consumer.h>
>> +#include <linux/serial_core.h>
>>
>>   enum mctrl_gpio_idx {
>>          UART_GPIO_CTS,
>> @@ -40,6 +41,13 @@ enum mctrl_gpio_idx {
>>    */
>>   struct mctrl_gpios;
>>
>> +/*
>> + * Return true if gidx is GPIO line, otherwise false.
>> + * It assumes that gpios is allocated and not NULL.
>> + */
>> +inline
>> +bool mctrl_gpio_is_gpio(struct mctrl_gpios *gpios, enum mctrl_gpio_idx gidx);
>> +
> This leads to a compile error :
>    CC      drivers/tty/serial/atmel_serial.o
> In file included from drivers/tty/serial/atmel_serial.c:64:0:
> drivers/tty/serial/atmel_serial.c: In function 'atmel_disable_ms.part.22':
> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
> call to always_inline 'mctrl_gpio_is_gpio': function body not
> available
> drivers/tty/serial/atmel_serial.c:536:25: error: called from here
> In file included from drivers/tty/serial/atmel_serial.c:64:0:
> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
> call to always_inline 'mctrl_gpio_is_gpio': function body not
> available
> drivers/tty/serial/atmel_serial.c:539:25: error: called from here
> In file included from drivers/tty/serial/atmel_serial.c:64:0:
> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
> call to always_inline 'mctrl_gpio_is_gpio': function body not
> available
> drivers/tty/serial/atmel_serial.c:542:25: error: called from here
> In file included from drivers/tty/serial/atmel_serial.c:64:0:
> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
> call to always_inline 'mctrl_gpio_is_gpio': function body not
> available
> drivers/tty/serial/atmel_serial.c:545:25: error: called from here
> In file included from drivers/tty/serial/atmel_serial.c:64:0:
> drivers/tty/serial/atmel_serial.c: In function 'atmel_enable_ms':
> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
> call to always_inline 'mctrl_gpio_is_gpio': function body not
> available
> drivers/tty/serial/atmel_serial.c:503:25: error: called from here
> In file included from drivers/tty/serial/atmel_serial.c:64:0:
> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
> call to always_inline 'mctrl_gpio_is_gpio': function body not
> available
> drivers/tty/serial/atmel_serial.c:506:25: error: called from here
> In file included from drivers/tty/serial/atmel_serial.c:64:0:
> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
> call to always_inline 'mctrl_gpio_is_gpio': function body not
> available
> drivers/tty/serial/atmel_serial.c:509:25: error: called from here
> In file included from drivers/tty/serial/atmel_serial.c:64:0:
> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
> call to always_inline 'mctrl_gpio_is_gpio': function body not
> available
> drivers/tty/serial/atmel_serial.c:512:25: error: called from here
> make[3]: *** [drivers/tty/serial/atmel_serial.o] Error 1
> make[2]: *** [drivers/tty/serial] Error 2
> make[1]: *** [drivers/tty] Error 2
> make: *** [drivers] Error 2

Do you compile atmel_serial as module? I compiled built-in and it was 
fine for me.
So the function should be exported like mctrl_gpio_to_gpiod() I guess.
An other reason can be you have CONFIG_GPIOLIB=n ?
In fact, mctrl_gpio_is_gpio() should depend on CONFIG_GPIOLIB for empty 
body.

>
>
>>   #ifdef CONFIG_GPIOLIB
>>
>>   /*
>> @@ -60,12 +68,13 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
>>                                        enum mctrl_gpio_idx gidx);
>>
>>   /*
>> - * Request and set direction of modem control lines GPIOs.
>> + * Request and set direction of modem control lines GPIOs. DT is used.
>> + * Initialize irq table for GPIOs.
>>    * devm_* functions are used, so there's no need to call mctrl_gpio_free().
>>    * Returns a pointer to the allocated mctrl structure if ok, -ENOMEM on
>>    * allocation error.
>>    */
>> -struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx);
>> +struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx);
>>
>>   /*
>>    * Free the mctrl_gpios structure.
>> @@ -74,6 +83,28 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx);
>>    */
>>   void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios);
>>
>> +/*
>> + * Request irqs for input lines GPIOs. The irqs are set disabled
>> + * and triggered on both edges.
>> + * Returns zero if OK, otherwise an error code.
>> + */
>> +int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios);
>> +
>> +/*
>> + * Free irqs for input lines GPIOs.
>> + */
>> +void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios);
>> +
>> +/*
>> + * Disable modem status interrupts assigned to GPIOs
>> + */
>> +void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios);
>> +
>> +/*
>> + * Enable modem status interrupts assigned to GPIOs
>> + */
>> +void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios);
>> +
>>   #else /* GPIOLIB */
>>
>>   static inline
>> @@ -95,7 +126,7 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
>>   }
>>
>>   static inline
>> -struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
>> +struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx)
>>   {
>>          return ERR_PTR(-ENOSYS);
>>   }
>> @@ -105,6 +136,28 @@ void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
>>   {
>>   }
>>
>> +static inline
>> +int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios)
>> +{
>> +       /*return -ENOTSUP;*/
>> +       return 0;
>> +}
>> +
>> +static inline
>> +void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios)
>> +{
>> +}
>> +
>> +static inline
>> +void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios)
>> +{
>> +}
>> +
>> +static inline
>> +void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios)
>> +{
>> +}
>> +
>>   #endif /* GPIOLIB */
>>
>>   #endif
>> --
>> 1.7.11.3
>>
> I think you should also update the documentation on mctrl_ helpers :
> Documentation/serial/driver

Right!

>
> I'm going to do some tests on atmel_serial.c

Thanks,
Janusz

>
> regards,
> Richard.

[RFC PATCH v2 1/4] tty: serial_mctrl_gpio: Add irqs helpers for input lines

[Richard Genoud]

2015-01-13 14:52 GMT+01:00 Janusz U?ycki <j.uzycki@elproma.com.pl>:
>
> W dniu 2015-01-13 o 14:04, Richard Genoud pisze:
>
>> 2015-01-10 15:32 GMT+01:00 Janusz Uzycki <j.uzycki@elproma.com.pl>:

>>> diff --git a/drivers/tty/serial/serial_mctrl_gpio.h
>>> b/drivers/tty/serial/serial_mctrl_gpio.h
>>> index 400ba04..13ba3f4 100644
>>> --- a/drivers/tty/serial/serial_mctrl_gpio.h
>>> +++ b/drivers/tty/serial/serial_mctrl_gpio.h
>>> @@ -21,6 +21,7 @@
>>>   #include <linux/err.h>
>>>   #include <linux/device.h>
>>>   #include <linux/gpio/consumer.h>
>>> +#include <linux/serial_core.h>
>>>
>>>   enum mctrl_gpio_idx {
>>>          UART_GPIO_CTS,
>>> @@ -40,6 +41,13 @@ enum mctrl_gpio_idx {
>>>    */
>>>   struct mctrl_gpios;
>>>
>>> +/*
>>> + * Return true if gidx is GPIO line, otherwise false.
>>> + * It assumes that gpios is allocated and not NULL.
>>> + */
>>> +inline
>>> +bool mctrl_gpio_is_gpio(struct mctrl_gpios *gpios, enum mctrl_gpio_idx
>>> gidx);
>>> +
>>
>> This leads to a compile error :
>>    CC      drivers/tty/serial/atmel_serial.o
>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>> drivers/tty/serial/atmel_serial.c: In function 'atmel_disable_ms.part.22':
>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>> available
>> drivers/tty/serial/atmel_serial.c:536:25: error: called from here
>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>> available
>> drivers/tty/serial/atmel_serial.c:539:25: error: called from here
>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>> available
>> drivers/tty/serial/atmel_serial.c:542:25: error: called from here
>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>> available
>> drivers/tty/serial/atmel_serial.c:545:25: error: called from here
>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>> drivers/tty/serial/atmel_serial.c: In function 'atmel_enable_ms':
>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>> available
>> drivers/tty/serial/atmel_serial.c:503:25: error: called from here
>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>> available
>> drivers/tty/serial/atmel_serial.c:506:25: error: called from here
>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>> available
>> drivers/tty/serial/atmel_serial.c:509:25: error: called from here
>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>> available
>> drivers/tty/serial/atmel_serial.c:512:25: error: called from here
>> make[3]: *** [drivers/tty/serial/atmel_serial.o] Error 1
>> make[2]: *** [drivers/tty/serial] Error 2
>> make[1]: *** [drivers/tty] Error 2
>> make: *** [drivers] Error 2
>
>
> Do you compile atmel_serial as module? I compiled built-in and it was fine
> for me.
> So the function should be exported like mctrl_gpio_to_gpiod() I guess.
> An other reason can be you have CONFIG_GPIOLIB=n ?
> In fact, mctrl_gpio_is_gpio() should depend on CONFIG_GPIOLIB for empty
> body.
>
well, I'm also compiling it built-in (I even have config_module=N)
kernel is 3.19-rc3 with your patches ontop.
I attached my .config (saved as a defconfig)

arm-linux-gcc (GCC) 4.7.3
-------------- next part --------------
A non-text attachment was scrubbed...
Name: defconfig
Type: application/octet-stream
Size: 4486 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150113/32e3eca8/attachment-0001.obj>

[RFC PATCH v2 1/4] tty: serial_mctrl_gpio: Add irqs helpers for input lines

[Janusz Użycki]

W dniu 2015-01-13 o 15:30, Richard Genoud pisze:
> 2015-01-13 14:52 GMT+01:00 Janusz U?ycki <j.uzycki@elproma.com.pl>:
>> W dniu 2015-01-13 o 14:04, Richard Genoud pisze:
>>
>>> 2015-01-10 15:32 GMT+01:00 Janusz Uzycki <j.uzycki@elproma.com.pl>:
>>>> diff --git a/drivers/tty/serial/serial_mctrl_gpio.h
>>>> b/drivers/tty/serial/serial_mctrl_gpio.h
>>>> index 400ba04..13ba3f4 100644
>>>> --- a/drivers/tty/serial/serial_mctrl_gpio.h
>>>> +++ b/drivers/tty/serial/serial_mctrl_gpio.h
>>>> @@ -21,6 +21,7 @@
>>>>    #include <linux/err.h>
>>>>    #include <linux/device.h>
>>>>    #include <linux/gpio/consumer.h>
>>>> +#include <linux/serial_core.h>
>>>>
>>>>    enum mctrl_gpio_idx {
>>>>           UART_GPIO_CTS,
>>>> @@ -40,6 +41,13 @@ enum mctrl_gpio_idx {
>>>>     */
>>>>    struct mctrl_gpios;
>>>>
>>>> +/*
>>>> + * Return true if gidx is GPIO line, otherwise false.
>>>> + * It assumes that gpios is allocated and not NULL.
>>>> + */
>>>> +inline
>>>> +bool mctrl_gpio_is_gpio(struct mctrl_gpios *gpios, enum mctrl_gpio_idx
>>>> gidx);
>>>> +
>>> This leads to a compile error :
>>>     CC      drivers/tty/serial/atmel_serial.o
>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>> drivers/tty/serial/atmel_serial.c: In function 'atmel_disable_ms.part.22':
>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>> available
>>> drivers/tty/serial/atmel_serial.c:536:25: error: called from here
>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>> available
>>> drivers/tty/serial/atmel_serial.c:539:25: error: called from here
>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>> available
>>> drivers/tty/serial/atmel_serial.c:542:25: error: called from here
>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>> available
>>> drivers/tty/serial/atmel_serial.c:545:25: error: called from here
>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>> drivers/tty/serial/atmel_serial.c: In function 'atmel_enable_ms':
>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>> available
>>> drivers/tty/serial/atmel_serial.c:503:25: error: called from here
>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>> available
>>> drivers/tty/serial/atmel_serial.c:506:25: error: called from here
>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>> available
>>> drivers/tty/serial/atmel_serial.c:509:25: error: called from here
>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>> available
>>> drivers/tty/serial/atmel_serial.c:512:25: error: called from here
>>> make[3]: *** [drivers/tty/serial/atmel_serial.o] Error 1
>>> make[2]: *** [drivers/tty/serial] Error 2
>>> make[1]: *** [drivers/tty] Error 2
>>> make: *** [drivers] Error 2
>>
>> Do you compile atmel_serial as module? I compiled built-in and it was fine
>> for me.
>> So the function should be exported like mctrl_gpio_to_gpiod() I guess.
>> An other reason can be you have CONFIG_GPIOLIB=n ?
>> In fact, mctrl_gpio_is_gpio() should depend on CONFIG_GPIOLIB for empty
>> body.
>>
> well, I'm also compiling it built-in (I even have config_module=N)

You have no CONFIG_GPIOLIB enabled and it is the bug in the patchset.
Thanks.
Janusz

> kernel is 3.19-rc3 with your patches ontop.
> I attached my .config (saved as a defconfig)
>
> arm-linux-gcc (GCC) 4.7.3

[RFC PATCH v2 1/4] tty: serial_mctrl_gpio: Add irqs helpers for input lines

[Richard Genoud]

2015-01-13 15:33 GMT+01:00 Janusz U?ycki <j.uzycki@elproma.com.pl>:
>
> W dniu 2015-01-13 o 15:30, Richard Genoud pisze:
>
>> 2015-01-13 14:52 GMT+01:00 Janusz U?ycki <j.uzycki@elproma.com.pl>:
>>>
>>> W dniu 2015-01-13 o 14:04, Richard Genoud pisze:
>>>
>>>> 2015-01-10 15:32 GMT+01:00 Janusz Uzycki <j.uzycki@elproma.com.pl>:
>>>>>
>>>>> diff --git a/drivers/tty/serial/serial_mctrl_gpio.h
>>>>> b/drivers/tty/serial/serial_mctrl_gpio.h
>>>>> index 400ba04..13ba3f4 100644
>>>>> --- a/drivers/tty/serial/serial_mctrl_gpio.h
>>>>> +++ b/drivers/tty/serial/serial_mctrl_gpio.h
>>>>> @@ -21,6 +21,7 @@
>>>>>    #include <linux/err.h>
>>>>>    #include <linux/device.h>
>>>>>    #include <linux/gpio/consumer.h>
>>>>> +#include <linux/serial_core.h>
>>>>>
>>>>>    enum mctrl_gpio_idx {
>>>>>           UART_GPIO_CTS,
>>>>> @@ -40,6 +41,13 @@ enum mctrl_gpio_idx {
>>>>>     */
>>>>>    struct mctrl_gpios;
>>>>>
>>>>> +/*
>>>>> + * Return true if gidx is GPIO line, otherwise false.
>>>>> + * It assumes that gpios is allocated and not NULL.
>>>>> + */
>>>>> +inline
>>>>> +bool mctrl_gpio_is_gpio(struct mctrl_gpios *gpios, enum mctrl_gpio_idx
>>>>> gidx);
>>>>> +
>>>>
>>>> This leads to a compile error :
>>>>     CC      drivers/tty/serial/atmel_serial.o
>>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>>> drivers/tty/serial/atmel_serial.c: In function
>>>> 'atmel_disable_ms.part.22':
>>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>>> available
>>>> drivers/tty/serial/atmel_serial.c:536:25: error: called from here
>>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>>> available
>>>> drivers/tty/serial/atmel_serial.c:539:25: error: called from here
>>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>>> available
>>>> drivers/tty/serial/atmel_serial.c:542:25: error: called from here
>>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>>> available
>>>> drivers/tty/serial/atmel_serial.c:545:25: error: called from here
>>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>>> drivers/tty/serial/atmel_serial.c: In function 'atmel_enable_ms':
>>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>>> available
>>>> drivers/tty/serial/atmel_serial.c:503:25: error: called from here
>>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>>> available
>>>> drivers/tty/serial/atmel_serial.c:506:25: error: called from here
>>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>>> available
>>>> drivers/tty/serial/atmel_serial.c:509:25: error: called from here
>>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>>> available
>>>> drivers/tty/serial/atmel_serial.c:512:25: error: called from here
>>>> make[3]: *** [drivers/tty/serial/atmel_serial.o] Error 1
>>>> make[2]: *** [drivers/tty/serial] Error 2
>>>> make[1]: *** [drivers/tty] Error 2
>>>> make: *** [drivers] Error 2
>>>
>>>
>>> Do you compile atmel_serial as module? I compiled built-in and it was
>>> fine
>>> for me.
>>> So the function should be exported like mctrl_gpio_to_gpiod() I guess.
>>> An other reason can be you have CONFIG_GPIOLIB=n ?
>>> In fact, mctrl_gpio_is_gpio() should depend on CONFIG_GPIOLIB for empty
>>> body.
>>>
>> well, I'm also compiling it built-in (I even have config_module=N)
>
>
> You have no CONFIG_GPIOLIB enabled and it is the bug in the patchset.
> Thanks.
> Janusz

Actually, I have CONFIG_GPIOLIB=y, only it doesn't appears in the defconfig.
I did:
# cp defconfig arch/arm/configs/toto_defconfig
# ARCH=arm make toto_defconfig

# grep CONFIG_GPIOLIB .config
CONFIG_GPIOLIB=y
CONFIG_GPIOLIB_IRQCHIP=y

# LANG=C ARCH=arm
CROSS_COMPILE=~/dev/LNS/buildroot-VIP/output/host/usr/bin/arm-linux-
make -j12 uImage
[...]
drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
call to always_inline 'mctrl_gpio_is_gpio': function body not
available
drivers/tty/serial/atmel_serial.c:536:25: error: called from here
In file included from drivers/tty/serial/atmel_serial.c:64:0:

[RFC PATCH v2 1/4] tty: serial_mctrl_gpio: Add irqs helpers for input lines

[Janusz Użycki]

W dniu 2015-01-13 o 15:41, Richard Genoud pisze:
> 2015-01-13 15:33 GMT+01:00 Janusz U?ycki <j.uzycki@elproma.com.pl>:
>> W dniu 2015-01-13 o 15:30, Richard Genoud pisze:
>>
>>> 2015-01-13 14:52 GMT+01:00 Janusz U?ycki <j.uzycki@elproma.com.pl>:
>>>> W dniu 2015-01-13 o 14:04, Richard Genoud pisze:
>>>>
>>>>> 2015-01-10 15:32 GMT+01:00 Janusz Uzycki <j.uzycki@elproma.com.pl>:
>>>>>> diff --git a/drivers/tty/serial/serial_mctrl_gpio.h
>>>>>> b/drivers/tty/serial/serial_mctrl_gpio.h
>>>>>> index 400ba04..13ba3f4 100644
>>>>>> --- a/drivers/tty/serial/serial_mctrl_gpio.h
>>>>>> +++ b/drivers/tty/serial/serial_mctrl_gpio.h
>>>>>> @@ -21,6 +21,7 @@
>>>>>>     #include <linux/err.h>
>>>>>>     #include <linux/device.h>
>>>>>>     #include <linux/gpio/consumer.h>
>>>>>> +#include <linux/serial_core.h>
>>>>>>
>>>>>>     enum mctrl_gpio_idx {
>>>>>>            UART_GPIO_CTS,
>>>>>> @@ -40,6 +41,13 @@ enum mctrl_gpio_idx {
>>>>>>      */
>>>>>>     struct mctrl_gpios;
>>>>>>
>>>>>> +/*
>>>>>> + * Return true if gidx is GPIO line, otherwise false.
>>>>>> + * It assumes that gpios is allocated and not NULL.
>>>>>> + */
>>>>>> +inline
>>>>>> +bool mctrl_gpio_is_gpio(struct mctrl_gpios *gpios, enum mctrl_gpio_idx
>>>>>> gidx);
>>>>>> +
>>>>> This leads to a compile error :
>>>>>      CC      drivers/tty/serial/atmel_serial.o
>>>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>>>> drivers/tty/serial/atmel_serial.c: In function
>>>>> 'atmel_disable_ms.part.22':
>>>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>>>> available
>>>>> drivers/tty/serial/atmel_serial.c:536:25: error: called from here
>>>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>>>> available
>>>>> drivers/tty/serial/atmel_serial.c:539:25: error: called from here
>>>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>>>> available
>>>>> drivers/tty/serial/atmel_serial.c:542:25: error: called from here
>>>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>>>> available
>>>>> drivers/tty/serial/atmel_serial.c:545:25: error: called from here
>>>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>>>> drivers/tty/serial/atmel_serial.c: In function 'atmel_enable_ms':
>>>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>>>> available
>>>>> drivers/tty/serial/atmel_serial.c:503:25: error: called from here
>>>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>>>> available
>>>>> drivers/tty/serial/atmel_serial.c:506:25: error: called from here
>>>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>>>> available
>>>>> drivers/tty/serial/atmel_serial.c:509:25: error: called from here
>>>>> In file included from drivers/tty/serial/atmel_serial.c:64:0:
>>>>> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
>>>>> call to always_inline 'mctrl_gpio_is_gpio': function body not
>>>>> available
>>>>> drivers/tty/serial/atmel_serial.c:512:25: error: called from here
>>>>> make[3]: *** [drivers/tty/serial/atmel_serial.o] Error 1
>>>>> make[2]: *** [drivers/tty/serial] Error 2
>>>>> make[1]: *** [drivers/tty] Error 2
>>>>> make: *** [drivers] Error 2
>>>>
>>>> Do you compile atmel_serial as module? I compiled built-in and it was
>>>> fine
>>>> for me.
>>>> So the function should be exported like mctrl_gpio_to_gpiod() I guess.
>>>> An other reason can be you have CONFIG_GPIOLIB=n ?
>>>> In fact, mctrl_gpio_is_gpio() should depend on CONFIG_GPIOLIB for empty
>>>> body.
>>>>
>>> well, I'm also compiling it built-in (I even have config_module=N)
>>
>> You have no CONFIG_GPIOLIB enabled and it is the bug in the patchset.
>> Thanks.
>> Janusz
> Actually, I have CONFIG_GPIOLIB=y, only it doesn't appears in the defconfig.
> I did:
> # cp defconfig arch/arm/configs/toto_defconfig
> # ARCH=arm make toto_defconfig
>
> # grep CONFIG_GPIOLIB .config
> CONFIG_GPIOLIB=y
> CONFIG_GPIOLIB_IRQCHIP=y
>
> # LANG=C ARCH=arm
> CROSS_COMPILE=~/dev/LNS/buildroot-VIP/output/host/usr/bin/arm-linux-
> make -j12 uImage
> [...]
> drivers/tty/serial/serial_mctrl_gpio.h:49:6: error: inlining failed in
> call to always_inline 'mctrl_gpio_is_gpio': function body not
> available
> drivers/tty/serial/atmel_serial.c:536:25: error: called from here
> In file included from drivers/tty/serial/atmel_serial.c:64:0:

OK, I will fix the bug. It's interesting I compiled it using gcc version 
4.2.4.

thanks,
Janusz

[RFC PATCH v2 3/4] serial: at91: Use helpers for gpio irqs

[Richard Genoud]

2015-01-10 15:32 GMT+01:00 Janusz Uzycki <j.uzycki@elproma.com.pl>:
> The patch updates atmel_serial driver to use new mctrl_gpio helpers for
> gpio irqs. The code is simpler now.
>
> Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
> ---
>
> The patch exists since v2.
> Compile-test only - please test it on real hardware.
>
> ---
>  drivers/tty/serial/atmel_serial.c | 123 +++++++-------------------------------
>  1 file changed, 20 insertions(+), 103 deletions(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 4d848a2..a59a892 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -168,7 +168,6 @@ struct atmel_uart_port {
>         struct circ_buf         rx_ring;
>
>         struct mctrl_gpios      *gpios;
> -       int                     gpio_irq[UART_GPIO_MAX];
>         unsigned int            tx_done_mask;
>         bool                    ms_irq_enabled;
>         bool                    is_usart;       /* usart or uart */
> @@ -499,24 +498,18 @@ static void atmel_enable_ms(struct uart_port *port)
>
>         atmel_port->ms_irq_enabled = true;
>
> -       if (atmel_port->gpio_irq[UART_GPIO_CTS] >= 0)
> -               enable_irq(atmel_port->gpio_irq[UART_GPIO_CTS]);
> -       else
> +       mctrl_gpio_enable_ms(atmel_port->gpios);
> +
> +       if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_CTS))
>                 ier |= ATMEL_US_CTSIC;
>
> -       if (atmel_port->gpio_irq[UART_GPIO_DSR] >= 0)
> -               enable_irq(atmel_port->gpio_irq[UART_GPIO_DSR]);
> -       else
> +       if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_DSR))
>                 ier |= ATMEL_US_DSRIC;
>
> -       if (atmel_port->gpio_irq[UART_GPIO_RI] >= 0)
> -               enable_irq(atmel_port->gpio_irq[UART_GPIO_RI]);
> -       else
> +       if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_RI))
>                 ier |= ATMEL_US_RIIC;
>
> -       if (atmel_port->gpio_irq[UART_GPIO_DCD] >= 0)
> -               enable_irq(atmel_port->gpio_irq[UART_GPIO_DCD]);
> -       else
> +       if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_DCD))
>                 ier |= ATMEL_US_DCDIC;
>
>         UART_PUT_IER(port, ier);
> @@ -538,24 +531,18 @@ static void atmel_disable_ms(struct uart_port *port)
>
>         atmel_port->ms_irq_enabled = false;
>
> -       if (atmel_port->gpio_irq[UART_GPIO_CTS] >= 0)
> -               disable_irq(atmel_port->gpio_irq[UART_GPIO_CTS]);
> -       else
> +       mctrl_gpio_disable_ms(atmel_port->gpios);
> +
> +       if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_CTS))
>                 idr |= ATMEL_US_CTSIC;
>
> -       if (atmel_port->gpio_irq[UART_GPIO_DSR] >= 0)
> -               disable_irq(atmel_port->gpio_irq[UART_GPIO_DSR]);
> -       else
> +       if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_DSR))
>                 idr |= ATMEL_US_DSRIC;
>
> -       if (atmel_port->gpio_irq[UART_GPIO_RI] >= 0)
> -               disable_irq(atmel_port->gpio_irq[UART_GPIO_RI]);
> -       else
> +       if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_RI))
>                 idr |= ATMEL_US_RIIC;
>
> -       if (atmel_port->gpio_irq[UART_GPIO_DCD] >= 0)
> -               disable_irq(atmel_port->gpio_irq[UART_GPIO_DCD]);
> -       else
> +       if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_DCD))
>                 idr |= ATMEL_US_DCDIC;
>
>         UART_PUT_IDR(port, idr);
> @@ -1161,31 +1148,11 @@ atmel_handle_status(struct uart_port *port, unsigned int pending,
>  static irqreturn_t atmel_interrupt(int irq, void *dev_id)
>  {
>         struct uart_port *port = dev_id;
> -       struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>         unsigned int status, pending, pass_counter = 0;
> -       bool gpio_handled = false;
>
>         do {
>                 status = atmel_get_lines_status(port);
>                 pending = status & UART_GET_IMR(port);
> -               if (!gpio_handled) {
> -                       /*
> -                        * Dealing with GPIO interrupt
> -                        */
> -                       if (irq == atmel_port->gpio_irq[UART_GPIO_CTS])
> -                               pending |= ATMEL_US_CTSIC;
> -
> -                       if (irq == atmel_port->gpio_irq[UART_GPIO_DSR])
> -                               pending |= ATMEL_US_DSRIC;
> -
> -                       if (irq == atmel_port->gpio_irq[UART_GPIO_RI])
> -                               pending |= ATMEL_US_RIIC;
> -
> -                       if (irq == atmel_port->gpio_irq[UART_GPIO_DCD])
> -                               pending |= ATMEL_US_DCDIC;
> -
> -                       gpio_handled = true;
> -               }
>                 if (!pending)
>                         break;
>
> @@ -1665,45 +1632,6 @@ static void atmel_get_ip_name(struct uart_port *port)
>         }
>  }
>
> -static void atmel_free_gpio_irq(struct uart_port *port)
> -{
> -       struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> -       enum mctrl_gpio_idx i;
> -
> -       for (i = 0; i < UART_GPIO_MAX; i++)
> -               if (atmel_port->gpio_irq[i] >= 0)
> -                       free_irq(atmel_port->gpio_irq[i], port);
> -}
> -
> -static int atmel_request_gpio_irq(struct uart_port *port)
> -{
> -       struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> -       int *irq = atmel_port->gpio_irq;
> -       enum mctrl_gpio_idx i;
> -       int err = 0;
> -
> -       for (i = 0; (i < UART_GPIO_MAX) && !err; i++) {
> -               if (irq[i] < 0)
> -                       continue;
> -
> -               irq_set_status_flags(irq[i], IRQ_NOAUTOEN);
> -               err = request_irq(irq[i], atmel_interrupt, IRQ_TYPE_EDGE_BOTH,
> -                                 "atmel_serial", port);
> -               if (err)
> -                       dev_err(port->dev, "atmel_startup - Can't get %d irq\n",
> -                               irq[i]);
> -       }
> -
> -       /*
> -        * If something went wrong, rollback.
> -        */
> -       while (err && (--i >= 0))
> -               if (irq[i] >= 0)
> -                       free_irq(irq[i], port);
> -
> -       return err;
> -}
> -
>  /*
>   * Perform initialization and enable port for reception
>   */
> @@ -1735,7 +1663,7 @@ static int atmel_startup(struct uart_port *port)
>         /*
>          * Get the GPIO lines IRQ
>          */
> -       retval = atmel_request_gpio_irq(port);
> +       retval = mctrl_gpio_request_irqs(atmel_port->gpios);
>         if (retval)
>                 goto free_irq;
>
> @@ -1872,7 +1800,7 @@ static void atmel_shutdown(struct uart_port *port)
>          * Free the interrupts
>          */
>         free_irq(port->irq, port);
> -       atmel_free_gpio_irq(port);
> +       mctrl_gpio_free_irqs(atmel_port->gpios);
>
>         atmel_port->ms_irq_enabled = false;
>
> @@ -2501,24 +2429,13 @@ static int atmel_serial_resume(struct platform_device *pdev)
>  #define atmel_serial_resume NULL
>  #endif
>
> -static int atmel_init_gpios(struct atmel_uart_port *p, struct device *dev)
> +static bool atmel_init_gpios(struct atmel_uart_port *p)
>  {
> -       enum mctrl_gpio_idx i;
> -       struct gpio_desc *gpiod;
> -
> -       p->gpios = mctrl_gpio_init(dev, 0);
> +       p->gpios = mctrl_gpio_init_dt(&p->uart, 0);
>         if (IS_ERR_OR_NULL(p->gpios))
> -               return -1;
> +               return false;
>
> -       for (i = 0; i < UART_GPIO_MAX; i++) {
> -               gpiod = mctrl_gpio_to_gpiod(p->gpios, i);
> -               if (gpiod && (gpiod_get_direction(gpiod) == GPIOF_DIR_IN))
> -                       p->gpio_irq[i] = gpiod_to_irq(gpiod);
> -               else
> -                       p->gpio_irq[i] = -EINVAL;
> -       }
> -
> -       return 0;
> +       return true;
>  }
>
>  static int atmel_serial_probe(struct platform_device *pdev)
> @@ -2558,8 +2475,8 @@ static int atmel_serial_probe(struct platform_device *pdev)
>         port->backup_imr = 0;
>         port->uart.line = ret;
>
> -       ret = atmel_init_gpios(port, &pdev->dev);
> -       if (ret < 0)
> +       port->uart.dev = &pdev->dev;
> +       if (!atmel_init_gpios(port))
>                 dev_err(&pdev->dev, "%s",
>                         "Failed to initialize GPIOs. The serial port may not work as expected");
>
> --
> 1.7.11.3
>

It's ok, no regression found in atmel-serial.
Tested on an at91sam9g35, full duplex between 2 serial ports at 57600
(one with CTS/RTS handled by the controller, the other with CTS/RTS
handled via GPIO).

Tested-by: Richard Genoud <richard.genoud@gmail.com>

[RFC PATCH v2 3/4] serial: at91: Use helpers for gpio irqs

[Nicolas Ferre]

Le 13/01/2015 17:08, Richard Genoud a ?crit :
> 2015-01-10 15:32 GMT+01:00 Janusz Uzycki <j.uzycki@elproma.com.pl>:
>> The patch updates atmel_serial driver to use new mctrl_gpio helpers for
>> gpio irqs. The code is simpler now.
>>
>> Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
>> ---
>>
>> The patch exists since v2.
>> Compile-test only - please test it on real hardware.
>>
>> ---
>>  drivers/tty/serial/atmel_serial.c | 123 +++++++-------------------------------
>>  1 file changed, 20 insertions(+), 103 deletions(-)
>>
>> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
>> index 4d848a2..a59a892 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -168,7 +168,6 @@ struct atmel_uart_port {
>>         struct circ_buf         rx_ring;
>>
>>         struct mctrl_gpios      *gpios;
>> -       int                     gpio_irq[UART_GPIO_MAX];
>>         unsigned int            tx_done_mask;
>>         bool                    ms_irq_enabled;
>>         bool                    is_usart;       /* usart or uart */
>> @@ -499,24 +498,18 @@ static void atmel_enable_ms(struct uart_port *port)
>>
>>         atmel_port->ms_irq_enabled = true;
>>
>> -       if (atmel_port->gpio_irq[UART_GPIO_CTS] >= 0)
>> -               enable_irq(atmel_port->gpio_irq[UART_GPIO_CTS]);
>> -       else
>> +       mctrl_gpio_enable_ms(atmel_port->gpios);
>> +
>> +       if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_CTS))
>>                 ier |= ATMEL_US_CTSIC;
>>
>> -       if (atmel_port->gpio_irq[UART_GPIO_DSR] >= 0)
>> -               enable_irq(atmel_port->gpio_irq[UART_GPIO_DSR]);
>> -       else
>> +       if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_DSR))
>>                 ier |= ATMEL_US_DSRIC;
>>
>> -       if (atmel_port->gpio_irq[UART_GPIO_RI] >= 0)
>> -               enable_irq(atmel_port->gpio_irq[UART_GPIO_RI]);
>> -       else
>> +       if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_RI))
>>                 ier |= ATMEL_US_RIIC;
>>
>> -       if (atmel_port->gpio_irq[UART_GPIO_DCD] >= 0)
>> -               enable_irq(atmel_port->gpio_irq[UART_GPIO_DCD]);
>> -       else
>> +       if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_DCD))
>>                 ier |= ATMEL_US_DCDIC;
>>
>>         UART_PUT_IER(port, ier);
>> @@ -538,24 +531,18 @@ static void atmel_disable_ms(struct uart_port *port)
>>
>>         atmel_port->ms_irq_enabled = false;
>>
>> -       if (atmel_port->gpio_irq[UART_GPIO_CTS] >= 0)
>> -               disable_irq(atmel_port->gpio_irq[UART_GPIO_CTS]);
>> -       else
>> +       mctrl_gpio_disable_ms(atmel_port->gpios);
>> +
>> +       if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_CTS))
>>                 idr |= ATMEL_US_CTSIC;
>>
>> -       if (atmel_port->gpio_irq[UART_GPIO_DSR] >= 0)
>> -               disable_irq(atmel_port->gpio_irq[UART_GPIO_DSR]);
>> -       else
>> +       if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_DSR))
>>                 idr |= ATMEL_US_DSRIC;
>>
>> -       if (atmel_port->gpio_irq[UART_GPIO_RI] >= 0)
>> -               disable_irq(atmel_port->gpio_irq[UART_GPIO_RI]);
>> -       else
>> +       if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_RI))
>>                 idr |= ATMEL_US_RIIC;
>>
>> -       if (atmel_port->gpio_irq[UART_GPIO_DCD] >= 0)
>> -               disable_irq(atmel_port->gpio_irq[UART_GPIO_DCD]);
>> -       else
>> +       if (!mctrl_gpio_is_gpio(atmel_port->gpios, UART_GPIO_DCD))
>>                 idr |= ATMEL_US_DCDIC;
>>
>>         UART_PUT_IDR(port, idr);
>> @@ -1161,31 +1148,11 @@ atmel_handle_status(struct uart_port *port, unsigned int pending,
>>  static irqreturn_t atmel_interrupt(int irq, void *dev_id)
>>  {
>>         struct uart_port *port = dev_id;
>> -       struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>>         unsigned int status, pending, pass_counter = 0;
>> -       bool gpio_handled = false;
>>
>>         do {
>>                 status = atmel_get_lines_status(port);
>>                 pending = status & UART_GET_IMR(port);
>> -               if (!gpio_handled) {
>> -                       /*
>> -                        * Dealing with GPIO interrupt
>> -                        */
>> -                       if (irq == atmel_port->gpio_irq[UART_GPIO_CTS])
>> -                               pending |= ATMEL_US_CTSIC;
>> -
>> -                       if (irq == atmel_port->gpio_irq[UART_GPIO_DSR])
>> -                               pending |= ATMEL_US_DSRIC;
>> -
>> -                       if (irq == atmel_port->gpio_irq[UART_GPIO_RI])
>> -                               pending |= ATMEL_US_RIIC;
>> -
>> -                       if (irq == atmel_port->gpio_irq[UART_GPIO_DCD])
>> -                               pending |= ATMEL_US_DCDIC;
>> -
>> -                       gpio_handled = true;
>> -               }
>>                 if (!pending)
>>                         break;
>>
>> @@ -1665,45 +1632,6 @@ static void atmel_get_ip_name(struct uart_port *port)
>>         }
>>  }
>>
>> -static void atmel_free_gpio_irq(struct uart_port *port)
>> -{
>> -       struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>> -       enum mctrl_gpio_idx i;
>> -
>> -       for (i = 0; i < UART_GPIO_MAX; i++)
>> -               if (atmel_port->gpio_irq[i] >= 0)
>> -                       free_irq(atmel_port->gpio_irq[i], port);
>> -}
>> -
>> -static int atmel_request_gpio_irq(struct uart_port *port)
>> -{
>> -       struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>> -       int *irq = atmel_port->gpio_irq;
>> -       enum mctrl_gpio_idx i;
>> -       int err = 0;
>> -
>> -       for (i = 0; (i < UART_GPIO_MAX) && !err; i++) {
>> -               if (irq[i] < 0)
>> -                       continue;
>> -
>> -               irq_set_status_flags(irq[i], IRQ_NOAUTOEN);
>> -               err = request_irq(irq[i], atmel_interrupt, IRQ_TYPE_EDGE_BOTH,
>> -                                 "atmel_serial", port);
>> -               if (err)
>> -                       dev_err(port->dev, "atmel_startup - Can't get %d irq\n",
>> -                               irq[i]);
>> -       }
>> -
>> -       /*
>> -        * If something went wrong, rollback.
>> -        */
>> -       while (err && (--i >= 0))
>> -               if (irq[i] >= 0)
>> -                       free_irq(irq[i], port);
>> -
>> -       return err;
>> -}
>> -
>>  /*
>>   * Perform initialization and enable port for reception
>>   */
>> @@ -1735,7 +1663,7 @@ static int atmel_startup(struct uart_port *port)
>>         /*
>>          * Get the GPIO lines IRQ
>>          */
>> -       retval = atmel_request_gpio_irq(port);
>> +       retval = mctrl_gpio_request_irqs(atmel_port->gpios);
>>         if (retval)
>>                 goto free_irq;
>>
>> @@ -1872,7 +1800,7 @@ static void atmel_shutdown(struct uart_port *port)
>>          * Free the interrupts
>>          */
>>         free_irq(port->irq, port);
>> -       atmel_free_gpio_irq(port);
>> +       mctrl_gpio_free_irqs(atmel_port->gpios);
>>
>>         atmel_port->ms_irq_enabled = false;
>>
>> @@ -2501,24 +2429,13 @@ static int atmel_serial_resume(struct platform_device *pdev)
>>  #define atmel_serial_resume NULL
>>  #endif
>>
>> -static int atmel_init_gpios(struct atmel_uart_port *p, struct device *dev)
>> +static bool atmel_init_gpios(struct atmel_uart_port *p)
>>  {
>> -       enum mctrl_gpio_idx i;
>> -       struct gpio_desc *gpiod;
>> -
>> -       p->gpios = mctrl_gpio_init(dev, 0);
>> +       p->gpios = mctrl_gpio_init_dt(&p->uart, 0);
>>         if (IS_ERR_OR_NULL(p->gpios))
>> -               return -1;
>> +               return false;
>>
>> -       for (i = 0; i < UART_GPIO_MAX; i++) {
>> -               gpiod = mctrl_gpio_to_gpiod(p->gpios, i);
>> -               if (gpiod && (gpiod_get_direction(gpiod) == GPIOF_DIR_IN))
>> -                       p->gpio_irq[i] = gpiod_to_irq(gpiod);
>> -               else
>> -                       p->gpio_irq[i] = -EINVAL;
>> -       }
>> -
>> -       return 0;
>> +       return true;
>>  }
>>
>>  static int atmel_serial_probe(struct platform_device *pdev)
>> @@ -2558,8 +2475,8 @@ static int atmel_serial_probe(struct platform_device *pdev)
>>         port->backup_imr = 0;
>>         port->uart.line = ret;
>>
>> -       ret = atmel_init_gpios(port, &pdev->dev);
>> -       if (ret < 0)
>> +       port->uart.dev = &pdev->dev;
>> +       if (!atmel_init_gpios(port))
>>                 dev_err(&pdev->dev, "%s",
>>                         "Failed to initialize GPIOs. The serial port may not work as expected");
>>
>> --
>> 1.7.11.3
>>
> 
> It's ok, no regression found in atmel-serial.
> Tested on an at91sam9g35, full duplex between 2 serial ports at 57600
> (one with CTS/RTS handled by the controller, the other with CTS/RTS
> handled via GPIO).
> 
> Tested-by: Richard Genoud <richard.genoud@gmail.com>

Richard,

Thanks a lot for having tested this series.
On my side, it seems good so:
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Bye,
-- 
Nicolas Ferre

[RFC PATCH v2 3/4] serial: at91: Use helpers for gpio irqs

[Linus Walleij]

On Sat, Jan 10, 2015 at 3:32 PM, Janusz Uzycki <j.uzycki@elproma.com.pl> wrote:

> The patch updates atmel_serial driver to use new mctrl_gpio helpers for
> gpio irqs. The code is simpler now.
>
> Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>

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

Yours,
Linus Walleij

[RFC PATCH v2 1/4] tty: serial_mctrl_gpio: Add irqs helpers for input lines

[Janusz Użycki]

Hi,

v3 is almost ready but on the moment I have no time to finish. I think 
I'll do it for 1-2 weeks.

best regards
Janusz

W dniu 2015-01-10 o 15:32, Janusz Uzycki pisze:
> A driver using mctrl_gpio called gpiod_get_direction() to check if
> gpio is input line. However .get_direction callback is not available
> for all platforms. The patch allows to avoid the function.
> The patch introduces the following helpers:
> - mctrl_gpio_request_irqs
> - mctrl_gpio_free_irqs
> - mctrl_gpio_enable_ms
> - mctrl_gpio_disable_ms
> They allow to:
> - simplify drivers which use mctrl_gpio
> - hide irq table in mctrl_gpio
> - use default irq handler for gpios
> - better separate code for gpio modem lines from uart's code
> In addition mctrl_gpio_init() has been renamed to mctrl_gpio_init_dt()
> to focus DT usage. Also mctrl_gpio_init_dt() initializes irq table for
> gpios now and passes struct uart_port into struct mctrl_gpios.
> This resulted in changed mctrl_gpio_init_dt() parameter.
> It also requires port->dev is set before the function is called.
>
> There were also fixed:
> - irq = 0 means invalid/unused (-EINVAL no more used)
> - mctrl_gpio_request_irqs() doesn't use negative enum value
>    if request_irq() failed. It just calls mctrl_gpio_free_irqs().
>
> The mctrl_gpio_is_gpio() inline function is under discussion
> and likely it can replace exported mctrl_gpio_to_gpiod() function.
>
> IRQ_NOAUTOEN setting and request_irq() order was not commented
> but it looks right according to other drivers.
>
> Signed-off-by: Janusz Uzycki <j.uzycki@elproma.com.pl>
> ---
>
> The patch requires to update the drivers which use mctrl_gpio:
> - atmel_serial
> - mxs-auart
> - clps711x
>
> Changes since RFC v1:
>   - patch renamed from:
>     ("serial: mctrl-gpio: Add irqs helpers for input lines")
>     to:
>     ("tty: serial_mctrl_gpio: Add irqs helpers for input lines")
>   - mctrl_gpio_request_irqs: changed mctrl_gpio_free_irqs() and
>     dev_err() order to make debug easier
>   - added patches for atmel_serial and clps711x serial drivers
>
> The patch applies to next (3.19.0-rc2) and was tested with mxs-auart
> using kernel 3.14 and 3.18. It wasn't tested on the next (only compiled).
>
> The patchset delivers patches for mxs-auart, atmel_serial and clps711x.
> atmel_serial and clps711x were not tested - only compile tests were done.
>
> ---
>   drivers/tty/serial/serial_mctrl_gpio.c | 109 ++++++++++++++++++++++++++++++++-
>   drivers/tty/serial/serial_mctrl_gpio.h |  59 +++++++++++++++++-
>   2 files changed, 163 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
> index a38596c..215b15c 100644
> --- a/drivers/tty/serial/serial_mctrl_gpio.c
> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
> @@ -19,11 +19,15 @@
>   #include <linux/device.h>
>   #include <linux/gpio/consumer.h>
>   #include <linux/termios.h>
> +#include <linux/irq.h>
>   
>   #include "serial_mctrl_gpio.h"
>   
>   struct mctrl_gpios {
> +	struct uart_port *port;
>   	struct gpio_desc *gpio[UART_GPIO_MAX];
> +	int irq[UART_GPIO_MAX];
> +	unsigned int mctrl_prev;
>   };
>   
>   static const struct {
> @@ -72,6 +76,12 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
>   }
>   EXPORT_SYMBOL_GPL(mctrl_gpio_to_gpiod);
>   
> +inline
> +bool mctrl_gpio_is_gpio(struct mctrl_gpios *gpios, enum mctrl_gpio_idx gidx)
> +{
> +	return !IS_ERR_OR_NULL(gpios->gpio[gidx]);
> +}
> +
>   unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
>   {
>   	enum mctrl_gpio_idx i;
> @@ -96,8 +106,9 @@ unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
>   }
>   EXPORT_SYMBOL_GPL(mctrl_gpio_get);
>   
> -struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
> +struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx)
>   {
> +	struct device *dev = port->dev;
>   	struct mctrl_gpios *gpios;
>   	enum mctrl_gpio_idx i;
>   	int err;
> @@ -106,6 +117,7 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
>   	if (!gpios)
>   		return ERR_PTR(-ENOMEM);
>   
> +	gpios->port = port;
>   	for (i = 0; i < UART_GPIO_MAX; i++) {
>   		gpios->gpio[i] = devm_gpiod_get_index(dev,
>   						      mctrl_gpios_desc[i].name,
> @@ -128,11 +140,13 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
>   			devm_gpiod_put(dev, gpios->gpio[i]);
>   			gpios->gpio[i] = NULL;
>   		}
> +		if (gpios->gpio[i] && !mctrl_gpios_desc[i].dir_out)
> +			gpios->irq[i] = gpiod_to_irq(gpios->gpio[i]);
>   	}
>   
>   	return gpios;
>   }
> -EXPORT_SYMBOL_GPL(mctrl_gpio_init);
> +EXPORT_SYMBOL_GPL(mctrl_gpio_init_dt);
>   
>   void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
>   {
> @@ -147,3 +161,94 @@ void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
>   	devm_kfree(dev, gpios);
>   }
>   EXPORT_SYMBOL_GPL(mctrl_gpio_free);
> +
> +/*
> + * Dealing with GPIO interrupt
> + */
> +#define MCTRL_ANY_DELTA        (TIOCM_RI | TIOCM_DSR | TIOCM_CD | TIOCM_CTS)
> +static irqreturn_t mctrl_gpio_irq_handle(int irq, void *context)
> +{
> +	struct mctrl_gpios *gpios = context;
> +	struct uart_port *port = gpios->port;
> +	u32 mctrl = gpios->mctrl_prev;
> +	u32 mctrl_diff;
> +
> +	mctrl_gpio_get(gpios, &mctrl);
> +
> +	mctrl_diff = mctrl ^ gpios->mctrl_prev;
> +	gpios->mctrl_prev = mctrl;
> +	if (mctrl_diff & MCTRL_ANY_DELTA && port->state != NULL) {
> +		if (mctrl_diff & TIOCM_RI)
> +			port->icount.rng++;
> +		if (mctrl_diff & TIOCM_DSR)
> +			port->icount.dsr++;
> +		if (mctrl_diff & TIOCM_CD)
> +			uart_handle_dcd_change(port, mctrl & TIOCM_CD);
> +		if (mctrl_diff & TIOCM_CTS)
> +			uart_handle_cts_change(port, mctrl & TIOCM_CTS);
> +
> +		wake_up_interruptible(&port->state->port.delta_msr_wait);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios)
> +{
> +	struct uart_port *port = gpios->port;
> +	int *irq = gpios->irq;
> +	enum mctrl_gpio_idx i;
> +	int err = 0;
> +
> +	for (i = 0; i < UART_GPIO_MAX; i++) {
> +		if (irq[i] <= 0)
> +			continue;
> +
> +		irq_set_status_flags(irq[i], IRQ_NOAUTOEN);
> +		err = request_irq(irq[i], mctrl_gpio_irq_handle,
> +				  IRQ_TYPE_EDGE_BOTH,
> +				  dev_name(port->dev), gpios);
> +		if (err) {
> +			dev_err(port->dev, "%s: Can't get %d irq\n",
> +				__func__, irq[i]);
> +			mctrl_gpio_free_irqs(gpios);
> +			break;
> +		}
> +	}
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_request_irqs);
> +
> +void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios)
> +{
> +	enum mctrl_gpio_idx i;
> +
> +	for (i = 0; i < UART_GPIO_MAX; i++)
> +		if (gpios->irq[i] > 0)
> +			free_irq(gpios->irq[i], gpios);
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_free_irqs);
> +
> +void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios)
> +{
> +	enum mctrl_gpio_idx i;
> +
> +	/* get initial status of modem lines GPIOs */
> +	mctrl_gpio_get(gpios, &gpios->mctrl_prev);
> +
> +	for (i = 0; i < UART_GPIO_MAX; i++)
> +		if (gpios->irq[i] > 0)
> +			enable_irq(gpios->irq[i]);
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_enable_ms);
> +
> +void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios)
> +{
> +	enum mctrl_gpio_idx i;
> +
> +	for (i = 0; i < UART_GPIO_MAX; i++)
> +		if (gpios->irq[i] > 0)
> +			disable_irq(gpios->irq[i]);
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_disable_ms);
> diff --git a/drivers/tty/serial/serial_mctrl_gpio.h b/drivers/tty/serial/serial_mctrl_gpio.h
> index 400ba04..13ba3f4 100644
> --- a/drivers/tty/serial/serial_mctrl_gpio.h
> +++ b/drivers/tty/serial/serial_mctrl_gpio.h
> @@ -21,6 +21,7 @@
>   #include <linux/err.h>
>   #include <linux/device.h>
>   #include <linux/gpio/consumer.h>
> +#include <linux/serial_core.h>
>   
>   enum mctrl_gpio_idx {
>   	UART_GPIO_CTS,
> @@ -40,6 +41,13 @@ enum mctrl_gpio_idx {
>    */
>   struct mctrl_gpios;
>   
> +/*
> + * Return true if gidx is GPIO line, otherwise false.
> + * It assumes that gpios is allocated and not NULL.
> + */
> +inline
> +bool mctrl_gpio_is_gpio(struct mctrl_gpios *gpios, enum mctrl_gpio_idx gidx);
> +
>   #ifdef CONFIG_GPIOLIB
>   
>   /*
> @@ -60,12 +68,13 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
>   				      enum mctrl_gpio_idx gidx);
>   
>   /*
> - * Request and set direction of modem control lines GPIOs.
> + * Request and set direction of modem control lines GPIOs. DT is used.
> + * Initialize irq table for GPIOs.
>    * devm_* functions are used, so there's no need to call mctrl_gpio_free().
>    * Returns a pointer to the allocated mctrl structure if ok, -ENOMEM on
>    * allocation error.
>    */
> -struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx);
> +struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx);
>   
>   /*
>    * Free the mctrl_gpios structure.
> @@ -74,6 +83,28 @@ struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx);
>    */
>   void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios);
>   
> +/*
> + * Request irqs for input lines GPIOs. The irqs are set disabled
> + * and triggered on both edges.
> + * Returns zero if OK, otherwise an error code.
> + */
> +int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios);
> +
> +/*
> + * Free irqs for input lines GPIOs.
> + */
> +void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios);
> +
> +/*
> + * Disable modem status interrupts assigned to GPIOs
> + */
> +void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios);
> +
> +/*
> + * Enable modem status interrupts assigned to GPIOs
> + */
> +void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios);
> +
>   #else /* GPIOLIB */
>   
>   static inline
> @@ -95,7 +126,7 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
>   }
>   
>   static inline
> -struct mctrl_gpios *mctrl_gpio_init(struct device *dev, unsigned int idx)
> +struct mctrl_gpios *mctrl_gpio_init_dt(struct uart_port *port, unsigned int idx)
>   {
>   	return ERR_PTR(-ENOSYS);
>   }
> @@ -105,6 +136,28 @@ void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
>   {
>   }
>   
> +static inline
> +int mctrl_gpio_request_irqs(struct mctrl_gpios *gpios)
> +{
> +	/*return -ENOTSUP;*/
> +	return 0;
> +}
> +
> +static inline
> +void mctrl_gpio_free_irqs(struct mctrl_gpios *gpios)
> +{
> +}
> +
> +static inline
> +void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios)
> +{
> +}
> +
> +static inline
> +void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios)
> +{
> +}
> +
>   #endif /* GPIOLIB */
>   
>   #endif

[RFC PATCH v2 1/4] tty: serial_mctrl_gpio: Add irqs helpers for input lines

[Fabio Estevam]

Hi Janusz,

On Thu, Jan 22, 2015 at 8:33 AM, Janusz U?ycki <j.uzycki@elproma.com.pl> wrote:
> Hi,
>
> v3 is almost ready but on the moment I have no time to finish. I think I'll
> do it for 1-2 weeks.

When you send v3, please remove RFC from the subject. Thanks