tty: serial_mctrl_gpio: Add irqs helpers for input lines

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 <[email protected]>

---

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(-)

drivers/tty/serial/serial_mctrl_gpio.c +107 -2

index a38596c..215b15c 100644
@@ -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);

drivers/tty/serial/serial_mctrl_gpio.h +56 -3

index 400ba04..13ba3f4 100644
@@ -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

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.

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 <[email protected]>

---

There is no changes since v1 (rebased only).

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

drivers/tty/serial/mxs-auart.c +0 -5

index ec553f8..2ddba69 100644
[...]

@@ -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)

Hello,

Quoted 11 lines · 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 <[email protected]>

---

There is no changes since v1 (rebased only).

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

Very nice!

Quoted 18 lines

drivers/tty/serial/mxs-auart.c +0 -9

index ec553f8..2ddba69 100644
[...]

@@ -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]);

drivers/tty/serial/mxs-auart.c static void mxs_auart_enable_ms(struct uart_port *port) · line 463

Quoted 6 lines

drivers/tty/serial/mxs-auart.c +3 -3

-
-	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

W dniu 2015-01-13 o 09:08, Uwe Kleine-K?nig pisze:

Quoted 32 lines

Hello,

Quoted 11 lines · 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 <[email protected]>

---

There is no changes since v1 (rebased only).

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

Very nice!

Quoted 16 lines

drivers/tty/serial/mxs-auart.c +0 -9

index ec553f8..2ddba69 100644
[...]

@@ -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]);

Quoted 8 lines

drivers/tty/serial/mxs-auart.c static void mxs_auart_enable_ms(struct uart_port *port) · line 463

Quoted 6 lines

drivers/tty/serial/mxs-auart.c +3 -3

-
-	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.

Hello,

Quoted 41 lines · Janusz Użycki

W dniu 2015-01-13 o 09:08, Uwe Kleine-König pisze:

Quoted 32 lines

Hello,

Quoted 11 lines · 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 <[email protected]>

---

There is no changes since v1 (rebased only).

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

Very nice!

Quoted 16 lines

drivers/tty/serial/mxs-auart.c +0 -9

index ec553f8..2ddba69 100644
[...]

@@ -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]);

Quoted 8 lines

drivers/tty/serial/mxs-auart.c static void mxs_auart_enable_ms(struct uart_port *port) · line 463

Quoted 6 lines

drivers/tty/serial/mxs-auart.c +3 -3

-
-	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.

Quoted 3 lines

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.
	 */

Quoted 6 lines

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

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 <[email protected]>

---

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(-)

drivers/tty/serial/atmel_serial.c +20 -104

index 4d848a2..a59a892 100644
@@ -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

2015-01-10 15:32 GMT+01:00 Janusz Uzycki <[email protected]>:

Quoted 224 lines

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 <[email protected]>

---

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(-)

drivers/tty/serial/atmel_serial.c +20 -104

index 4d848a2..a59a892 100644
@@ -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");
 
--

Quoted 2 lines

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 <[email protected]>

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 <[email protected]>

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.

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 <[email protected]>

---

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(-)

drivers/tty/serial/serial_mctrl_gpio.c +107 -2

index a38596c..215b15c 100644
@@ -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);

drivers/tty/serial/serial_mctrl_gpio.h +56 -3

index 400ba04..13ba3f4 100644
@@ -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

Signature · 1 line

1.7.11.3

Quoted 6 lines

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

Quoted 8 lines

- 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.

Quoted 5 lines

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.

Quoted 7 lines

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.

Quoted 3 lines

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.
*/

Quoted 2 lines

Signed-off-by Janusz Uzycki <[email protected]>

There is no binding documentation for mctrl. This should be fixed, too.

Quoted 182 lines

---

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(-)

drivers/tty/serial/serial_mctrl_gpio.c +99 -2

index a38596c..215b15c 100644
@@ -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)

drivers/tty/serial/serial_mctrl_gpio.c void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios) · line 254

Quoted 8 lines

drivers/tty/serial/serial_mctrl_gpio.c +8 -0

+{
+	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);

Quoted 108 lines

drivers/tty/serial/serial_mctrl_gpio.h +56 -3

index 400ba04..13ba3f4 100644
@@ -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 */
 

Quoted 5 lines

drivers/tty/serial/serial_mctrl_gpio.h +0 -0

  #endif

Signature · 1 line

1.7.11.3

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 <[email protected]>

---

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(-)

drivers/tty/serial/serial_mctrl_gpio.c +107 -2

index a38596c..215b15c 100644
@@ -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);

drivers/tty/serial/serial_mctrl_gpio.h +1 -0

index 400ba04..13ba3f4 100644
@@ -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,

drivers/tty/serial/serial_mctrl_gpio.h +48 -4

 #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

2015-01-10 15:32 GMT+01:00 Janusz Uzycki <[email protected]>:

Quoted 233 lines

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 <[email protected]>

---

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(-)

drivers/tty/serial/serial_mctrl_gpio.c +107 -2

index a38596c..215b15c 100644
@@ -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);

drivers/tty/serial/serial_mctrl_gpio.h +1 -0

index 400ba04..13ba3f4 100644
@@ -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,

drivers/tty/serial/serial_mctrl_gpio.h enum mctrl_gpio_idx { · line 50

Quoted 10 lines

drivers/tty/serial/serial_mctrl_gpio.h +7 -0

@@ -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

Quoted 87 lines

drivers/tty/serial/serial_mctrl_gpio.h +48 -3

  #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 */
 

Quoted 4 lines

drivers/tty/serial/serial_mctrl_gpio.h +0 -1

  #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.

W dniu 2015-01-13 o 14:04, Richard Genoud pisze:

Quoted 68 lines

2015-01-10 15:32 GMT+01:00 Janusz Uzycki <[email protected]>:

Quoted 15 lines

drivers/tty/serial/serial_mctrl_gpio.h +1 -0

index 400ba04..13ba3f4 100644
@@ -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 {
   */

drivers/tty/serial/serial_mctrl_gpio.h enum mctrl_gpio_idx { · line 51

Quoted 10 lines

drivers/tty/serial/serial_mctrl_gpio.h +7 -0

  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:

Quoted 8 lines

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.

2015-01-13 14:52 GMT+01:00 Janusz Użycki <[email protected]>:

Quoted 77 lines

W dniu 2015-01-13 o 14:04, Richard Genoud pisze:

Quoted 74 lines

2015-01-10 15:32 GMT+01:00 Janusz Uzycki <[email protected]>:

Quoted 15 lines

drivers/tty/serial/serial_mctrl_gpio.h +1 -0

index 400ba04..13ba3f4 100644
@@ -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 {
    */

drivers/tty/serial/serial_mctrl_gpio.h enum mctrl_gpio_idx { · line 51

Quoted 10 lines

drivers/tty/serial/serial_mctrl_gpio.h +7 -0

   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

Quoted 7 lines

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

W dniu 2015-01-13 o 15:30, Richard Genoud pisze:

Quoted 91 lines

2015-01-13 14:52 GMT+01:00 Janusz U?ycki <[email protected]>:

Quoted 90 lines

W dniu 2015-01-13 o 14:04, Richard Genoud pisze:

Quoted 69 lines

2015-01-10 15:32 GMT+01:00 Janusz Uzycki <[email protected]>:

Quoted 16 lines

drivers/tty/serial/serial_mctrl_gpio.h +1 -0

index 400ba04..13ba3f4 100644
@@ -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 {
    */

drivers/tty/serial/serial_mctrl_gpio.h enum mctrl_gpio_idx { · line 51

Quoted 10 lines

drivers/tty/serial/serial_mctrl_gpio.h +7 -0

   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:

Quoted 8 lines

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.

Quoted 1 line

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

2015-01-13 15:33 GMT+01:00 Janusz Użycki <[email protected]>:

Quoted 89 lines

W dniu 2015-01-13 o 15:30, Richard Genoud pisze:

Quoted 87 lines

2015-01-13 14:52 GMT+01:00 Janusz Użycki <[email protected]>:

Quoted 86 lines

W dniu 2015-01-13 o 14:04, Richard Genoud pisze:

Quoted 67 lines

2015-01-10 15:32 GMT+01:00 Janusz Uzycki <[email protected]>:

Quoted 15 lines

drivers/tty/serial/serial_mctrl_gpio.h +1 -0

index 400ba04..13ba3f4 100644
@@ -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 {
     */

drivers/tty/serial/serial_mctrl_gpio.h enum mctrl_gpio_idx { · line 51

Quoted 10 lines

drivers/tty/serial/serial_mctrl_gpio.h +7 -0

    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:

Quoted 8 lines

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.

Quoted 5 lines

Quoted 1 line

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: