diff mbox

[v2,1/4] tty/serial: Add GPIOLIB helpers for controlling modem lines

Message ID 1392140715-15295-2-git-send-email-richard.genoud@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Richard Genoud Feb. 11, 2014, 5:45 p.m. UTC
This patch add some helpers to control modem lines (CTS/RTS/DSR...) via
GPIO.
This will be useful for many boards which have a serial controller that
only handle CTS/RTS pins (or even just RX/TX).

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 Documentation/serial/driver            |  21 ++++++
 drivers/tty/serial/Kconfig             |   4 ++
 drivers/tty/serial/Makefile            |   3 +
 drivers/tty/serial/serial_mctrl_gpio.c | 113 +++++++++++++++++++++++++++++++++
 drivers/tty/serial/serial_mctrl_gpio.h |  56 ++++++++++++++++
 5 files changed, 197 insertions(+)
 create mode 100644 drivers/tty/serial/serial_mctrl_gpio.c
 create mode 100644 drivers/tty/serial/serial_mctrl_gpio.h

Comments

Alexander Shiyan Feb. 11, 2014, 6:23 p.m. UTC | #1
Hello.

???????, 11 ??????? 2014, 18:45 +01:00 ?? Richard Genoud <richard.genoud@gmail.com>:
> This patch add some helpers to control modem lines (CTS/RTS/DSR...) via
> GPIO.
> This will be useful for many boards which have a serial controller that
> only handle CTS/RTS pins (or even just RX/TX).
> 
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> ---

OK, a few comments below.

...
> +config SERIAL_MCTRL_GPIO
> +	def_bool y
> +	depends on GPIOLIB

I suggest to move GPIOLIB dependency to serial_mctrl_gpio.h header, so
unit can be used with or without GPIOLIB, so Kconfig will look something like this:

config SERIAL_MCTRL_GPIO
  tristate

Then, you can select this option for particular UART:
config SERIAL_ATMEL
  ...
  select SERIAL_MCTRL_GPIO

...
> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
...
> +static const char *mctrl_gpio_of_names[UART_GPIO_MAX] = {
> +	"cts", "dsr", "dcd", "ri", "rts", "dtr"
> +};

Make a combined array. This will use cycles for set/get operations.

static struct {
  const char *name;
  unsigned int mctrl;
} mctrl_gpios[] = {
  { "cts", TIOCM_CTS, },
  ...
};

...
> +int mctrl_gpio_init(struct device *dev, struct mctrl_gpios *gpios)
> +{

I'm not sure whether to make a non-DT support at all ...

> +	enum mctrl_gpio_idx i;
> +	int err = 0;
> +	int ret = 0;
> +
> +	for (i = UART_GPIO_MIN; i < UART_GPIO_MAX; i++) {
> +		gpios->gpio[i] = gpiod_get(dev, mctrl_gpio_of_names[i]);

What a reason to using gpiod_xxx() ?
Why we cannot use standart devm_gpio_request/get/set etc. ?

In addition, I recommend create this patch as separate,
because it will most likely still be comments later.
Thanks.

---
Richard Genoud Feb. 12, 2014, 8:33 a.m. UTC | #2
2014-02-11 19:23 GMT+01:00 Alexander Shiyan <shc_work@mail.ru>:
> Hello.
Hi !
>
> ???????, 11 ??????? 2014, 18:45 +01:00 ?? Richard Genoud <richard.genoud@gmail.com>:
>> This patch add some helpers to control modem lines (CTS/RTS/DSR...) via
>> GPIO.
>> This will be useful for many boards which have a serial controller that
>> only handle CTS/RTS pins (or even just RX/TX).
>>
>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>> ---
>
> OK, a few comments below.
>
> ...
>> +config SERIAL_MCTRL_GPIO
>> +     def_bool y
>> +     depends on GPIOLIB
>
> I suggest to move GPIOLIB dependency to serial_mctrl_gpio.h header, so
> unit can be used with or without GPIOLIB, so Kconfig will look something like this:
>
> config SERIAL_MCTRL_GPIO
>   tristate
>
> Then, you can select this option for particular UART:
> config SERIAL_ATMEL
>   ...
>   select SERIAL_MCTRL_GPIO
Yes, you're right, it seems better like that.

> ...
>> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
> ...
>> +static const char *mctrl_gpio_of_names[UART_GPIO_MAX] = {
>> +     "cts", "dsr", "dcd", "ri", "rts", "dtr"
>> +};
>
> Make a combined array. This will use cycles for set/get operations.
>
> static struct {
>   const char *name;
>   unsigned int mctrl;
> } mctrl_gpios[] = {
>   { "cts", TIOCM_CTS, },
>   ...
> };
yes, good idea !

> ...
>> +int mctrl_gpio_init(struct device *dev, struct mctrl_gpios *gpios)
>> +{
>
> I'm not sure whether to make a non-DT support at all ...
I don't understand what you mean.
You would like another _init() function for non-DT board ?
Well, I thought that non DT board could handle their GPIO init (like I
did in atmel_serial.c for platform_data based boards), but still use
the mctrl_gpio_{get,set} functions.


>> +     enum mctrl_gpio_idx i;
>> +     int err = 0;
>> +     int ret = 0;
>> +
>> +     for (i = UART_GPIO_MIN; i < UART_GPIO_MAX; i++) {
>> +             gpios->gpio[i] = gpiod_get(dev, mctrl_gpio_of_names[i]);
>
> What a reason to using gpiod_xxx() ?
> Why we cannot use standart devm_gpio_request/get/set etc. ?
Ah ! I missed devm_gpiod_get()
I'll use it.

>
> In addition, I recommend create this patch as separate,
> because it will most likely still be comments later.
Well, I'd rather not separate this patch from at least "patch 4/4
tty/serial: at91: use mctrl_gpio helpers"
because if the patch 4/4 is merged before this one, the compilation
will fail, and git bisecting will be a pain.
Or there's another solution ?

> Thanks.

Thanks !
diff mbox

Patch

diff --git a/Documentation/serial/driver b/Documentation/serial/driver
index c3a7689a90e6..b33f0f9e1b8e 100644
--- a/Documentation/serial/driver
+++ b/Documentation/serial/driver
@@ -429,3 +429,24 @@  thus:
 		struct uart_port	port;
 		int			my_stuff;
 	};
+
+Modem control lines via GPIO
+----------------------------
+
+Some helpers are provided in order to set/get modem control lines via GPIO.
+
+mctrl_gpio_init(dev, gpios):
+	This will get the {cts,rts,...}-gpios from device tree if they are
+	present and request them, set direction etc.
+
+mctrl_gpio_free(gpios):
+	This will free the requested gpios in mctrl_gpio_init().
+
+mctrl_gpio_set(gpios, mctrl):
+	This will sets the gpios according to the mctrl state.
+
+mctrl_gpio_get(gpios, mctrl):
+	This will update mctrl with the gpios values.
+
+get_mctrl_gpio_name(mctrl_gpio_idx):
+	Returns a name associated to the GPIO modem line.
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index a3815eaed421..58c54d7eae53 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1509,4 +1509,8 @@  config SERIAL_ST_ASC_CONSOLE
 
 endmenu
 
+config SERIAL_MCTRL_GPIO
+	def_bool y
+	depends on GPIOLIB
+
 endif # TTY
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index 3680854fef41..bcf31da267dd 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -87,3 +87,6 @@  obj-$(CONFIG_SERIAL_EFM32_UART) += efm32-uart.o
 obj-$(CONFIG_SERIAL_ARC)	+= arc_uart.o
 obj-$(CONFIG_SERIAL_RP2)	+= rp2.o
 obj-$(CONFIG_SERIAL_FSL_LPUART)	+= fsl_lpuart.o
+
+# GPIOLIB helpers for modem control lines
+obj-$(CONFIG_SERIAL_MCTRL_GPIO) += serial_mctrl_gpio.o
diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
new file mode 100644
index 000000000000..ddcc7538f487
--- /dev/null
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -0,0 +1,113 @@ 
+/*
+ *  Helpers for controlling modem lines via GPIO
+ *
+ *  Copyright (C) 2014 Paratronic S.A.
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <uapi/asm-generic/termios.h>
+
+#include "serial_mctrl_gpio.h"
+
+static const char *mctrl_gpio_of_names[UART_GPIO_MAX] = {
+	"cts", "dsr", "dcd", "ri", "rts", "dtr"
+};
+
+const char *get_mctrl_gpio_name(enum mctrl_gpio_idx idx)
+{
+	return mctrl_gpio_of_names[idx];
+}
+EXPORT_SYMBOL_GPL(get_mctrl_gpio_name);
+
+void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
+{
+	if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_RTS]))
+		gpiod_set_value(gpios->gpio[UART_GPIO_RTS],
+				!!(mctrl & TIOCM_RTS));
+
+	if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_DTR]))
+		gpiod_set_value(gpios->gpio[UART_GPIO_DTR],
+				!!(mctrl & TIOCM_DTR));
+}
+EXPORT_SYMBOL_GPL(mctrl_gpio_set);
+
+unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
+{
+	if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_CTS])) {
+		if (gpiod_get_value(gpios->gpio[UART_GPIO_CTS]))
+			*mctrl |= TIOCM_CTS;
+		else
+			*mctrl &= ~TIOCM_CTS;
+	}
+
+	if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_DSR])) {
+		if (gpiod_get_value(gpios->gpio[UART_GPIO_DSR]))
+			*mctrl |= TIOCM_DSR;
+		else
+			*mctrl &= ~TIOCM_DSR;
+	}
+
+	if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_RI])) {
+		if (gpiod_get_value(gpios->gpio[UART_GPIO_RI]))
+			*mctrl |= TIOCM_RI;
+		else
+			*mctrl &= ~TIOCM_RI;
+	}
+
+	if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_DCD])) {
+		if (gpiod_get_value(gpios->gpio[UART_GPIO_DCD]))
+			*mctrl |= TIOCM_CD;
+		else
+			*mctrl &= ~TIOCM_CD;
+	}
+
+	return *mctrl;
+}
+EXPORT_SYMBOL_GPL(mctrl_gpio_get);
+
+int mctrl_gpio_init(struct device *dev, struct mctrl_gpios *gpios)
+{
+	enum mctrl_gpio_idx i;
+	int err = 0;
+	int ret = 0;
+
+	for (i = UART_GPIO_MIN; i < UART_GPIO_MAX; i++) {
+		gpios->gpio[i] = gpiod_get(dev, mctrl_gpio_of_names[i]);
+
+		/*
+		 * The GPIOs are maybe not all filled,
+		 * this is not an error.
+		 */
+		if (IS_ERR_OR_NULL(gpios->gpio[i]))
+			continue;
+
+		if (i < UART_GPIO_MAX_INPUT)
+			err = gpiod_direction_input(gpios->gpio[i]);
+		else
+			err = gpiod_direction_output(gpios->gpio[i], 0);
+		if (err) {
+			dev_err(dev, "Unable to set direction for %s GPIO",
+				mctrl_gpio_of_names[i]);
+			gpiod_put(gpios->gpio[i]);
+			gpios->gpio[i] = NULL;
+			ret--;
+		}
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(mctrl_gpio_init);
+
+void mctrl_gpio_free(struct mctrl_gpios *gpios)
+{
+	enum mctrl_gpio_idx i;
+
+	for (i = UART_GPIO_MIN; i < UART_GPIO_MAX; i++)
+		if (!IS_ERR_OR_NULL(gpios->gpio[i])) {
+			gpiod_put(gpios->gpio[i]);
+			gpios->gpio[i] = NULL;
+		}
+}
+EXPORT_SYMBOL_GPL(mctrl_gpio_free);
diff --git a/drivers/tty/serial/serial_mctrl_gpio.h b/drivers/tty/serial/serial_mctrl_gpio.h
new file mode 100644
index 000000000000..5b8be48f6442
--- /dev/null
+++ b/drivers/tty/serial/serial_mctrl_gpio.h
@@ -0,0 +1,56 @@ 
+/*
+ *  Helpers for controlling modem lines via GPIO
+ *
+ *  Copyright (C) 2014 Paratronic S.A.
+ *
+ */
+
+#ifndef __SERIAL_MCTRL_GPIO__
+#define __SERIAL_MCTRL_GPIO__
+
+#include <linux/gpio/consumer.h>
+
+enum mctrl_gpio_idx {
+	UART_GPIO_MIN = 0,
+	UART_GPIO_CTS = 0,
+	UART_GPIO_DSR = 1,
+	UART_GPIO_DCD = 2,
+	UART_GPIO_RI = 3,
+	UART_GPIO_MAX_INPUT = 4,
+	UART_GPIO_RTS = 4,
+	UART_GPIO_DTR = 5,
+	UART_GPIO_MAX,
+};
+
+struct mctrl_gpios {
+	struct gpio_desc *gpio[UART_GPIO_MAX];
+};
+
+/*
+ * Names of the modem lines (cts, dsr, dcd...)
+ */
+const char *get_mctrl_gpio_name(enum mctrl_gpio_idx idx);
+
+/*
+ * Set state of the modem control output lines via GPIOs.
+ */
+void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl);
+
+/*
+ * Get state of the modem control output lines from GPIOs.
+ * The mctrl flags are updated and returned.
+ */
+unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl);
+
+/*
+ * Request and set direction of modem control lines GPIOs.
+ * Returns 0 if ok, -nb_err if setting direction failed.
+ */
+int mctrl_gpio_init(struct device *dev, struct mctrl_gpios *gpios);
+
+/*
+ * Free all modem control lines GPIOs
+ */
+void mctrl_gpio_free(struct mctrl_gpios *gpios);
+
+#endif