* [PATCH v3 0/7] tty/serial: Add helpers to use GPIOs to control modem lines and implement atmel_serial.c
@ 2014-02-17 16:57 ` Richard Genoud
0 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-17 16:57 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Uwe Kleine-König, Nicolas Ferre, Linus Walleij,
Alexander Shiyan, linux-serial, linux-arm-kernel, Richard Genoud
The USART controller on sam9x5 chips (and also all AT91/SAMA5 chips
but at91rm9200) are not capable of handling DTR/DSR/DCD/RI signal.
Moreover, even if the controller can handle CTS/RTS, the dedicated
CTS/RTS pins are already muxed for other peripherals (LCDC/EMAC/MMC).
So this patchset adds the possibility to control those lines via GPIO,
as it is done for RTS in the patch "switch atmel serial to use gpiolib"
As it was suggested by Alexander Shiyan, I made that available for
every board.
Patch 1 implements the generic helpers to control modem lines via GPIO
Patches 2 and 3 are just a little tidy up of atmel_serial.c.
Patch 4 implements modem control lines in atmel_serial atmel_serial.
Patches 5 and 6 implement the get_direction() gpio call for at91, as
it is needed by gpiod_get_direction().
Patch 7 implement the interrupts of CTS/DSR/DCD/RI.
This is based on 3.14-rc3 + Linus Walleij/Nicolas Ferre's patch:
354e57f3a0a2 ARM/serial: at91: switch atmel serial to use gpiolib
(in Uwe's tree git://git.pengutronix.de/git/ukl/linux.git dropmachtimexh )
and Philipp Zabel's patch:
8f984bc11e1cc gpiolib: make gpiod_direction_output take a logical value
(in gpio tree git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git )
(there won't be a merge conflict if this last patch is not present, the
gpios will "just" be on the wrong direction.)
Tested on at91sam9g35, with a null modem cable between 2 serial ports,
one with CTS/RTS controlled by the USART controller, the other via GPIO,
full duplex transfers.
Did some tests also with null modem cables on a PC.
Updates from v2:
- remove UART_GPIO_MIN/UART_GPIO_MAX_INPUT and use a direction
boolean instead.
- implement get_direction in at91 pinctrl and mach-at91/gpio.c.
- remove the get_mctrl_gpio_name() function that was used for
logs only.
- split atmel_serial.c patch in 2.
- use a gpio lookup table to declare modem gpios in platform
devices boards. So there's no more special case for platform
data gpios in atmel_serial.c.
Updates from v1:
- Instead of controlling modem signal only on atmel board, the
code is now available for every board.
- The active low flag from device tree is now used.
Richard Genoud (7):
tty/serial: Add GPIOLIB helpers for controlling modem lines
tty/serial: at91: use dev_err instead of printk
tty/serial: at91: remove unused open/close hooks
tty/serial: at91: use mctrl_gpio helpers
ARM: at91: gpio: implement get_direction
pinctrl: at91: implement get_direction
tty/serial: at91: add interrupts for modem control lines
Documentation/serial/driver | 21 +++
arch/arm/mach-at91/at91rm9200_devices.c | 16 +-
arch/arm/mach-at91/at91sam9260_devices.c | 7 -
arch/arm/mach-at91/at91sam9261_devices.c | 4 -
arch/arm/mach-at91/at91sam9263_devices.c | 4 -
arch/arm/mach-at91/at91sam9g45_devices.c | 5 -
arch/arm/mach-at91/at91sam9rl_devices.c | 5 -
arch/arm/mach-at91/gpio.c | 13 ++
drivers/pinctrl/pinctrl-at91.c | 12 ++
drivers/tty/serial/Kconfig | 4 +
drivers/tty/serial/Makefile | 3 +
drivers/tty/serial/atmel_serial.c | 249 +++++++++++++++++++++++--------
drivers/tty/serial/serial_mctrl_gpio.c | 112 ++++++++++++++
drivers/tty/serial/serial_mctrl_gpio.h | 92 ++++++++++++
include/linux/platform_data/atmel.h | 1 -
15 files changed, 456 insertions(+), 92 deletions(-)
create mode 100644 drivers/tty/serial/serial_mctrl_gpio.c
create mode 100644 drivers/tty/serial/serial_mctrl_gpio.h
--
1.8.5
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 0/7] tty/serial: Add helpers to use GPIOs to control modem lines and implement atmel_serial.c
@ 2014-02-17 16:57 ` Richard Genoud
0 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-17 16:57 UTC (permalink / raw)
To: linux-arm-kernel
The USART controller on sam9x5 chips (and also all AT91/SAMA5 chips
but at91rm9200) are not capable of handling DTR/DSR/DCD/RI signal.
Moreover, even if the controller can handle CTS/RTS, the dedicated
CTS/RTS pins are already muxed for other peripherals (LCDC/EMAC/MMC).
So this patchset adds the possibility to control those lines via GPIO,
as it is done for RTS in the patch "switch atmel serial to use gpiolib"
As it was suggested by Alexander Shiyan, I made that available for
every board.
Patch 1 implements the generic helpers to control modem lines via GPIO
Patches 2 and 3 are just a little tidy up of atmel_serial.c.
Patch 4 implements modem control lines in atmel_serial atmel_serial.
Patches 5 and 6 implement the get_direction() gpio call for at91, as
it is needed by gpiod_get_direction().
Patch 7 implement the interrupts of CTS/DSR/DCD/RI.
This is based on 3.14-rc3 + Linus Walleij/Nicolas Ferre's patch:
354e57f3a0a2 ARM/serial: at91: switch atmel serial to use gpiolib
(in Uwe's tree git://git.pengutronix.de/git/ukl/linux.git dropmachtimexh )
and Philipp Zabel's patch:
8f984bc11e1cc gpiolib: make gpiod_direction_output take a logical value
(in gpio tree git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git )
(there won't be a merge conflict if this last patch is not present, the
gpios will "just" be on the wrong direction.)
Tested on at91sam9g35, with a null modem cable between 2 serial ports,
one with CTS/RTS controlled by the USART controller, the other via GPIO,
full duplex transfers.
Did some tests also with null modem cables on a PC.
Updates from v2:
- remove UART_GPIO_MIN/UART_GPIO_MAX_INPUT and use a direction
boolean instead.
- implement get_direction in at91 pinctrl and mach-at91/gpio.c.
- remove the get_mctrl_gpio_name() function that was used for
logs only.
- split atmel_serial.c patch in 2.
- use a gpio lookup table to declare modem gpios in platform
devices boards. So there's no more special case for platform
data gpios in atmel_serial.c.
Updates from v1:
- Instead of controlling modem signal only on atmel board, the
code is now available for every board.
- The active low flag from device tree is now used.
Richard Genoud (7):
tty/serial: Add GPIOLIB helpers for controlling modem lines
tty/serial: at91: use dev_err instead of printk
tty/serial: at91: remove unused open/close hooks
tty/serial: at91: use mctrl_gpio helpers
ARM: at91: gpio: implement get_direction
pinctrl: at91: implement get_direction
tty/serial: at91: add interrupts for modem control lines
Documentation/serial/driver | 21 +++
arch/arm/mach-at91/at91rm9200_devices.c | 16 +-
arch/arm/mach-at91/at91sam9260_devices.c | 7 -
arch/arm/mach-at91/at91sam9261_devices.c | 4 -
arch/arm/mach-at91/at91sam9263_devices.c | 4 -
arch/arm/mach-at91/at91sam9g45_devices.c | 5 -
arch/arm/mach-at91/at91sam9rl_devices.c | 5 -
arch/arm/mach-at91/gpio.c | 13 ++
drivers/pinctrl/pinctrl-at91.c | 12 ++
drivers/tty/serial/Kconfig | 4 +
drivers/tty/serial/Makefile | 3 +
drivers/tty/serial/atmel_serial.c | 249 +++++++++++++++++++++++--------
drivers/tty/serial/serial_mctrl_gpio.c | 112 ++++++++++++++
drivers/tty/serial/serial_mctrl_gpio.h | 92 ++++++++++++
include/linux/platform_data/atmel.h | 1 -
15 files changed, 456 insertions(+), 92 deletions(-)
create mode 100644 drivers/tty/serial/serial_mctrl_gpio.c
create mode 100644 drivers/tty/serial/serial_mctrl_gpio.h
--
1.8.5
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 1/7] tty/serial: Add GPIOLIB helpers for controlling modem lines
2014-02-17 16:57 ` Richard Genoud
@ 2014-02-17 16:57 ` Richard Genoud
-1 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-17 16:57 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Uwe Kleine-König, Nicolas Ferre, Linus Walleij,
Alexander Shiyan, linux-serial, linux-arm-kernel, Richard Genoud
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 | 3 +
drivers/tty/serial/Makefile | 3 +
drivers/tty/serial/serial_mctrl_gpio.c | 112 +++++++++++++++++++++++++++++++++
drivers/tty/serial/serial_mctrl_gpio.h | 92 +++++++++++++++++++++++++++
5 files changed, 231 insertions(+)
create mode 100644 drivers/tty/serial/serial_mctrl_gpio.c
create mode 100644 drivers/tty/serial/serial_mctrl_gpio.h
diff --git a/Documentation/serial/driver b/Documentation/serial/driver
index c3a7689a90e6..23aa32667d68 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. devm_* functions are
+ used, so there's no need to call mctrl_gpio_free().
+
+mctrl_gpio_free(dev, gpios):
+ This will free the requested gpios in mctrl_gpio_init().
+ As devm_* function are used, there's generally no need to call
+ this function.
+
+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.
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index a3815eaed421..4fe8ca16f492 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1509,4 +1509,7 @@ config SERIAL_ST_ASC_CONSOLE
endmenu
+config SERIAL_MCTRL_GPIO
+ tristate
+
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..bd8e4a7c981c
--- /dev/null
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -0,0 +1,112 @@
+/*
+ * Helpers for controlling modem lines via GPIO
+ *
+ * Copyright (C) 2014 Paratronic S.A.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <uapi/asm-generic/termios.h>
+
+#include "serial_mctrl_gpio.h"
+
+static const struct {
+ const char *name;
+ unsigned int mctrl;
+ bool dir_out;
+} mctrl_gpios_desc[] = {
+ { "cts", TIOCM_CTS, false, },
+ { "dsr", TIOCM_DSR, false, },
+ { "dcd", TIOCM_CD, false, },
+ { "ri", TIOCM_RI, false, },
+ { "rts", TIOCM_RTS, true, },
+ { "dtr", TIOCM_DTR, true, },
+ { "out1", TIOCM_OUT1, true, },
+ { "out2", TIOCM_OUT2, true, },
+};
+
+void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
+{
+ enum mctrl_gpio_idx i;
+
+ for (i = 0; i < UART_GPIO_MAX; i++)
+ if (!IS_ERR_OR_NULL(gpios->gpio[i]) &&
+ mctrl_gpios_desc[i].dir_out)
+ gpiod_set_value(gpios->gpio[i],
+ !!(mctrl & mctrl_gpios_desc[i].mctrl));
+}
+EXPORT_SYMBOL_GPL(mctrl_gpio_set);
+
+unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
+{
+ enum mctrl_gpio_idx i;
+
+ for (i = 0; i < UART_GPIO_MAX; i++) {
+ if (!IS_ERR_OR_NULL(gpios->gpio[i]) &&
+ !mctrl_gpios_desc[i].dir_out) {
+ if (gpiod_get_value(gpios->gpio[i]))
+ *mctrl |= mctrl_gpios_desc[i].mctrl;
+ else
+ *mctrl &= ~mctrl_gpios_desc[i].mctrl;
+ }
+ }
+
+ 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 = 0; i < UART_GPIO_MAX; i++) {
+ gpios->gpio[i] = devm_gpiod_get(dev, mctrl_gpios_desc[i].name);
+
+ /*
+ * The GPIOs are maybe not all filled,
+ * this is not an error.
+ */
+ if (IS_ERR_OR_NULL(gpios->gpio[i]))
+ continue;
+
+ if (mctrl_gpios_desc[i].dir_out)
+ err = gpiod_direction_output(gpios->gpio[i], 0);
+ else
+ err = gpiod_direction_input(gpios->gpio[i]);
+ if (err) {
+ dev_err(dev, "Unable to set direction for %s GPIO",
+ mctrl_gpios_desc[i].name);
+ devm_gpiod_put(dev, gpios->gpio[i]);
+ gpios->gpio[i] = NULL;
+ ret--;
+ }
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(mctrl_gpio_init);
+
+void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
+{
+ enum mctrl_gpio_idx i;
+
+ for (i = 0; i < UART_GPIO_MAX; i++)
+ if (!IS_ERR_OR_NULL(gpios->gpio[i])) {
+ devm_gpiod_put(dev, 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..e9797323d531
--- /dev/null
+++ b/drivers/tty/serial/serial_mctrl_gpio.h
@@ -0,0 +1,92 @@
+/*
+ * Helpers for controlling modem lines via GPIO
+ *
+ * Copyright (C) 2014 Paratronic S.A.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef __SERIAL_MCTRL_GPIO__
+#define __SERIAL_MCTRL_GPIO__
+
+#include <linux/gpio/consumer.h>
+
+enum mctrl_gpio_idx {
+ UART_GPIO_CTS,
+ UART_GPIO_DSR,
+ UART_GPIO_DCD,
+ UART_GPIO_RI,
+ UART_GPIO_RTS,
+ UART_GPIO_DTR,
+ UART_GPIO_OUT1,
+ UART_GPIO_OUT2,
+ UART_GPIO_MAX,
+};
+
+struct mctrl_gpios {
+ struct gpio_desc *gpio[UART_GPIO_MAX];
+};
+
+#ifdef CONFIG_GPIOLIB
+
+/*
+ * 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.
+ * devm_* functions are used, so there's no need to call mctrl_gpio_free().
+ * 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.
+ * Normally, this function will not be called, as the GPIOs will
+ * be disposed of by the resource management code.
+ */
+void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios);
+
+#else /* GPIOLIB */
+
+static inline
+void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
+{
+}
+
+static inline
+unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
+{
+ return 0;
+}
+
+static inline
+int mctrl_gpio_init(struct device *dev, struct mctrl_gpios *gpios)
+{
+ return -UART_GPIO_MAX;
+}
+
+static inline
+void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
+{
+}
+
+#endif /* GPIOLIB */
+
+#endif
--
1.8.5
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v3 1/7] tty/serial: Add GPIOLIB helpers for controlling modem lines
@ 2014-02-17 16:57 ` Richard Genoud
0 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-17 16:57 UTC (permalink / raw)
To: linux-arm-kernel
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 | 3 +
drivers/tty/serial/Makefile | 3 +
drivers/tty/serial/serial_mctrl_gpio.c | 112 +++++++++++++++++++++++++++++++++
drivers/tty/serial/serial_mctrl_gpio.h | 92 +++++++++++++++++++++++++++
5 files changed, 231 insertions(+)
create mode 100644 drivers/tty/serial/serial_mctrl_gpio.c
create mode 100644 drivers/tty/serial/serial_mctrl_gpio.h
diff --git a/Documentation/serial/driver b/Documentation/serial/driver
index c3a7689a90e6..23aa32667d68 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. devm_* functions are
+ used, so there's no need to call mctrl_gpio_free().
+
+mctrl_gpio_free(dev, gpios):
+ This will free the requested gpios in mctrl_gpio_init().
+ As devm_* function are used, there's generally no need to call
+ this function.
+
+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.
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index a3815eaed421..4fe8ca16f492 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1509,4 +1509,7 @@ config SERIAL_ST_ASC_CONSOLE
endmenu
+config SERIAL_MCTRL_GPIO
+ tristate
+
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..bd8e4a7c981c
--- /dev/null
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -0,0 +1,112 @@
+/*
+ * Helpers for controlling modem lines via GPIO
+ *
+ * Copyright (C) 2014 Paratronic S.A.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <uapi/asm-generic/termios.h>
+
+#include "serial_mctrl_gpio.h"
+
+static const struct {
+ const char *name;
+ unsigned int mctrl;
+ bool dir_out;
+} mctrl_gpios_desc[] = {
+ { "cts", TIOCM_CTS, false, },
+ { "dsr", TIOCM_DSR, false, },
+ { "dcd", TIOCM_CD, false, },
+ { "ri", TIOCM_RI, false, },
+ { "rts", TIOCM_RTS, true, },
+ { "dtr", TIOCM_DTR, true, },
+ { "out1", TIOCM_OUT1, true, },
+ { "out2", TIOCM_OUT2, true, },
+};
+
+void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
+{
+ enum mctrl_gpio_idx i;
+
+ for (i = 0; i < UART_GPIO_MAX; i++)
+ if (!IS_ERR_OR_NULL(gpios->gpio[i]) &&
+ mctrl_gpios_desc[i].dir_out)
+ gpiod_set_value(gpios->gpio[i],
+ !!(mctrl & mctrl_gpios_desc[i].mctrl));
+}
+EXPORT_SYMBOL_GPL(mctrl_gpio_set);
+
+unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
+{
+ enum mctrl_gpio_idx i;
+
+ for (i = 0; i < UART_GPIO_MAX; i++) {
+ if (!IS_ERR_OR_NULL(gpios->gpio[i]) &&
+ !mctrl_gpios_desc[i].dir_out) {
+ if (gpiod_get_value(gpios->gpio[i]))
+ *mctrl |= mctrl_gpios_desc[i].mctrl;
+ else
+ *mctrl &= ~mctrl_gpios_desc[i].mctrl;
+ }
+ }
+
+ 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 = 0; i < UART_GPIO_MAX; i++) {
+ gpios->gpio[i] = devm_gpiod_get(dev, mctrl_gpios_desc[i].name);
+
+ /*
+ * The GPIOs are maybe not all filled,
+ * this is not an error.
+ */
+ if (IS_ERR_OR_NULL(gpios->gpio[i]))
+ continue;
+
+ if (mctrl_gpios_desc[i].dir_out)
+ err = gpiod_direction_output(gpios->gpio[i], 0);
+ else
+ err = gpiod_direction_input(gpios->gpio[i]);
+ if (err) {
+ dev_err(dev, "Unable to set direction for %s GPIO",
+ mctrl_gpios_desc[i].name);
+ devm_gpiod_put(dev, gpios->gpio[i]);
+ gpios->gpio[i] = NULL;
+ ret--;
+ }
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(mctrl_gpio_init);
+
+void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
+{
+ enum mctrl_gpio_idx i;
+
+ for (i = 0; i < UART_GPIO_MAX; i++)
+ if (!IS_ERR_OR_NULL(gpios->gpio[i])) {
+ devm_gpiod_put(dev, 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..e9797323d531
--- /dev/null
+++ b/drivers/tty/serial/serial_mctrl_gpio.h
@@ -0,0 +1,92 @@
+/*
+ * Helpers for controlling modem lines via GPIO
+ *
+ * Copyright (C) 2014 Paratronic S.A.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef __SERIAL_MCTRL_GPIO__
+#define __SERIAL_MCTRL_GPIO__
+
+#include <linux/gpio/consumer.h>
+
+enum mctrl_gpio_idx {
+ UART_GPIO_CTS,
+ UART_GPIO_DSR,
+ UART_GPIO_DCD,
+ UART_GPIO_RI,
+ UART_GPIO_RTS,
+ UART_GPIO_DTR,
+ UART_GPIO_OUT1,
+ UART_GPIO_OUT2,
+ UART_GPIO_MAX,
+};
+
+struct mctrl_gpios {
+ struct gpio_desc *gpio[UART_GPIO_MAX];
+};
+
+#ifdef CONFIG_GPIOLIB
+
+/*
+ * 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.
+ * devm_* functions are used, so there's no need to call mctrl_gpio_free().
+ * 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.
+ * Normally, this function will not be called, as the GPIOs will
+ * be disposed of by the resource management code.
+ */
+void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios);
+
+#else /* GPIOLIB */
+
+static inline
+void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
+{
+}
+
+static inline
+unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
+{
+ return 0;
+}
+
+static inline
+int mctrl_gpio_init(struct device *dev, struct mctrl_gpios *gpios)
+{
+ return -UART_GPIO_MAX;
+}
+
+static inline
+void mctrl_gpio_free(struct device *dev, struct mctrl_gpios *gpios)
+{
+}
+
+#endif /* GPIOLIB */
+
+#endif
--
1.8.5
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v3 2/7] tty/serial: at91: use dev_err instead of printk
2014-02-17 16:57 ` Richard Genoud
@ 2014-02-17 16:57 ` Richard Genoud
-1 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-17 16:57 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Uwe Kleine-König, Nicolas Ferre, Linus Walleij,
Alexander Shiyan, linux-serial, linux-arm-kernel, Richard Genoud
For better consistency.
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
drivers/tty/serial/atmel_serial.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 91c0d8839570..ed9621a21d67 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -1549,7 +1549,7 @@ static int atmel_startup(struct uart_port *port)
retval = request_irq(port->irq, atmel_interrupt, IRQF_SHARED,
tty ? tty->name : "atmel_serial", port);
if (retval) {
- printk("atmel_serial: atmel_startup - Can't get irq\n");
+ dev_err(port->dev, "atmel_startup - Can't get irq\n");
return retval;
}
@@ -1732,7 +1732,7 @@ static void atmel_serial_pm(struct uart_port *port, unsigned int state,
clk_disable_unprepare(atmel_port->clk);
break;
default:
- printk(KERN_ERR "atmel_serial: unknown pm %d\n", state);
+ dev_err(port->dev, "atmel_serial: unknown pm %d\n", state);
}
}
--
1.8.5
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v3 2/7] tty/serial: at91: use dev_err instead of printk
@ 2014-02-17 16:57 ` Richard Genoud
0 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-17 16:57 UTC (permalink / raw)
To: linux-arm-kernel
For better consistency.
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
drivers/tty/serial/atmel_serial.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 91c0d8839570..ed9621a21d67 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -1549,7 +1549,7 @@ static int atmel_startup(struct uart_port *port)
retval = request_irq(port->irq, atmel_interrupt, IRQF_SHARED,
tty ? tty->name : "atmel_serial", port);
if (retval) {
- printk("atmel_serial: atmel_startup - Can't get irq\n");
+ dev_err(port->dev, "atmel_startup - Can't get irq\n");
return retval;
}
@@ -1732,7 +1732,7 @@ static void atmel_serial_pm(struct uart_port *port, unsigned int state,
clk_disable_unprepare(atmel_port->clk);
break;
default:
- printk(KERN_ERR "atmel_serial: unknown pm %d\n", state);
+ dev_err(port->dev, "atmel_serial: unknown pm %d\n", state);
}
}
--
1.8.5
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v3 3/7] tty/serial: at91: remove unused open/close hooks
2014-02-17 16:57 ` Richard Genoud
@ 2014-02-17 16:57 ` Richard Genoud
-1 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-17 16:57 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Uwe Kleine-König, Nicolas Ferre, Linus Walleij,
Alexander Shiyan, linux-serial, linux-arm-kernel, Richard Genoud
commit 95e629b761ce36996d1befe2824d5346b5a220b9 removed the use of board
specific hooks in serial_at91.h, so now, the open/close hook are just
dead code.
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
drivers/tty/serial/atmel_serial.c | 21 ---------------------
1 file changed, 21 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index ed9621a21d67..a51b3a762948 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -112,9 +112,6 @@ static void atmel_stop_rx(struct uart_port *port);
#define UART_PUT_TCR(port,v) __raw_writel(v, (port)->membase + ATMEL_PDC_TCR)
#define UART_GET_TCR(port) __raw_readl((port)->membase + ATMEL_PDC_TCR)
-static int (*atmel_open_hook)(struct uart_port *);
-static void (*atmel_close_hook)(struct uart_port *);
-
struct atmel_dma_buffer {
unsigned char *buf;
dma_addr_t dma_addr;
@@ -1569,17 +1566,6 @@ static int atmel_startup(struct uart_port *port)
if (retval < 0)
atmel_set_ops(port);
}
- /*
- * If there is a specific "open" function (to register
- * control line interrupts)
- */
- if (atmel_open_hook) {
- retval = atmel_open_hook(port);
- if (retval) {
- free_irq(port->irq, port);
- return retval;
- }
- }
/* Save current CSR for comparison in atmel_tasklet_func() */
atmel_port->irq_status_prev = UART_GET_CSR(port);
@@ -1678,13 +1664,6 @@ static void atmel_shutdown(struct uart_port *port)
* Free the interrupt
*/
free_irq(port->irq, port);
-
- /*
- * If there is a specific "close" function (to unregister
- * control line interrupts)
- */
- if (atmel_close_hook)
- atmel_close_hook(port);
}
/*
--
1.8.5
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v3 3/7] tty/serial: at91: remove unused open/close hooks
@ 2014-02-17 16:57 ` Richard Genoud
0 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-17 16:57 UTC (permalink / raw)
To: linux-arm-kernel
commit 95e629b761ce36996d1befe2824d5346b5a220b9 removed the use of board
specific hooks in serial_at91.h, so now, the open/close hook are just
dead code.
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
drivers/tty/serial/atmel_serial.c | 21 ---------------------
1 file changed, 21 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index ed9621a21d67..a51b3a762948 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -112,9 +112,6 @@ static void atmel_stop_rx(struct uart_port *port);
#define UART_PUT_TCR(port,v) __raw_writel(v, (port)->membase + ATMEL_PDC_TCR)
#define UART_GET_TCR(port) __raw_readl((port)->membase + ATMEL_PDC_TCR)
-static int (*atmel_open_hook)(struct uart_port *);
-static void (*atmel_close_hook)(struct uart_port *);
-
struct atmel_dma_buffer {
unsigned char *buf;
dma_addr_t dma_addr;
@@ -1569,17 +1566,6 @@ static int atmel_startup(struct uart_port *port)
if (retval < 0)
atmel_set_ops(port);
}
- /*
- * If there is a specific "open" function (to register
- * control line interrupts)
- */
- if (atmel_open_hook) {
- retval = atmel_open_hook(port);
- if (retval) {
- free_irq(port->irq, port);
- return retval;
- }
- }
/* Save current CSR for comparison in atmel_tasklet_func() */
atmel_port->irq_status_prev = UART_GET_CSR(port);
@@ -1678,13 +1664,6 @@ static void atmel_shutdown(struct uart_port *port)
* Free the interrupt
*/
free_irq(port->irq, port);
-
- /*
- * If there is a specific "close" function (to unregister
- * control line interrupts)
- */
- if (atmel_close_hook)
- atmel_close_hook(port);
}
/*
--
1.8.5
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v3 4/7] tty/serial: at91: use mctrl_gpio helpers
2014-02-17 16:57 ` Richard Genoud
@ 2014-02-17 16:57 ` Richard Genoud
-1 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-17 16:57 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Uwe Kleine-König, Nicolas Ferre, Linus Walleij,
Alexander Shiyan, linux-serial, linux-arm-kernel, Richard Genoud
On sam9x5, dedicated CTS (and RTS) pins are unusable together with the
LCDC, the EMAC, or the MMC because they share the same line.
Moreover, the USART controller doesn't handle DTR/DSR/DCD/RI signals,
so we have to control them via GPIO.
This patch permits to use GPIOs to control the CTS/RTS/DTR/DSR/DCD/RI
signals.
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
arch/arm/mach-at91/at91rm9200_devices.c | 16 +++--
arch/arm/mach-at91/at91sam9260_devices.c | 7 ---
arch/arm/mach-at91/at91sam9261_devices.c | 4 --
arch/arm/mach-at91/at91sam9263_devices.c | 4 --
arch/arm/mach-at91/at91sam9g45_devices.c | 5 --
arch/arm/mach-at91/at91sam9rl_devices.c | 5 --
drivers/tty/serial/Kconfig | 1 +
drivers/tty/serial/atmel_serial.c | 100 ++++++++++++++++++++-----------
include/linux/platform_data/atmel.h | 1 -
9 files changed, 76 insertions(+), 67 deletions(-)
diff --git a/arch/arm/mach-at91/at91rm9200_devices.c b/arch/arm/mach-at91/at91rm9200_devices.c
index 605add05af7e..6dd386e3d9fe 100644
--- a/arch/arm/mach-at91/at91rm9200_devices.c
+++ b/arch/arm/mach-at91/at91rm9200_devices.c
@@ -15,6 +15,7 @@
#include <linux/dma-mapping.h>
#include <linux/gpio.h>
+#include <linux/gpio/driver.h>
#include <linux/platform_device.h>
#include <linux/i2c-gpio.h>
@@ -922,7 +923,6 @@ static struct resource dbgu_resources[] = {
static struct atmel_uart_data dbgu_data = {
.use_dma_tx = 0,
.use_dma_rx = 0, /* DBGU not capable of receive DMA */
- .rts_gpio = -EINVAL,
};
static u64 dbgu_dmamask = DMA_BIT_MASK(32);
@@ -961,7 +961,14 @@ static struct resource uart0_resources[] = {
static struct atmel_uart_data uart0_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
+};
+
+struct gpiod_lookup_table uart0_gpios_table = {
+ .dev_id = "atmel_usart",
+ .table = {
+ GPIO_LOOKUP("pioA", 21, "rts", GPIO_ACTIVE_LOW),
+ { },
+ },
};
static u64 uart0_dmamask = DMA_BIT_MASK(32);
@@ -992,7 +999,7 @@ static inline void configure_usart0_pins(unsigned pins)
* We need to drive the pin manually. The serial driver will driver
* this to high when initializing.
*/
- uart0_data.rts_gpio = AT91_PIN_PA21;
+ gpiod_add_lookup_table(&uart0_gpios_table);
}
}
@@ -1012,7 +1019,6 @@ static struct resource uart1_resources[] = {
static struct atmel_uart_data uart1_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart1_dmamask = DMA_BIT_MASK(32);
@@ -1064,7 +1070,6 @@ static struct resource uart2_resources[] = {
static struct atmel_uart_data uart2_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart2_dmamask = DMA_BIT_MASK(32);
@@ -1108,7 +1113,6 @@ static struct resource uart3_resources[] = {
static struct atmel_uart_data uart3_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart3_dmamask = DMA_BIT_MASK(32);
diff --git a/arch/arm/mach-at91/at91sam9260_devices.c b/arch/arm/mach-at91/at91sam9260_devices.c
index b52527c78b12..eda8d1679d40 100644
--- a/arch/arm/mach-at91/at91sam9260_devices.c
+++ b/arch/arm/mach-at91/at91sam9260_devices.c
@@ -819,7 +819,6 @@ static struct resource dbgu_resources[] = {
static struct atmel_uart_data dbgu_data = {
.use_dma_tx = 0,
.use_dma_rx = 0, /* DBGU not capable of receive DMA */
- .rts_gpio = -EINVAL,
};
static u64 dbgu_dmamask = DMA_BIT_MASK(32);
@@ -858,7 +857,6 @@ static struct resource uart0_resources[] = {
static struct atmel_uart_data uart0_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart0_dmamask = DMA_BIT_MASK(32);
@@ -910,7 +908,6 @@ static struct resource uart1_resources[] = {
static struct atmel_uart_data uart1_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart1_dmamask = DMA_BIT_MASK(32);
@@ -954,7 +951,6 @@ static struct resource uart2_resources[] = {
static struct atmel_uart_data uart2_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart2_dmamask = DMA_BIT_MASK(32);
@@ -998,7 +994,6 @@ static struct resource uart3_resources[] = {
static struct atmel_uart_data uart3_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart3_dmamask = DMA_BIT_MASK(32);
@@ -1042,7 +1037,6 @@ static struct resource uart4_resources[] = {
static struct atmel_uart_data uart4_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart4_dmamask = DMA_BIT_MASK(32);
@@ -1081,7 +1075,6 @@ static struct resource uart5_resources[] = {
static struct atmel_uart_data uart5_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart5_dmamask = DMA_BIT_MASK(32);
diff --git a/arch/arm/mach-at91/at91sam9261_devices.c b/arch/arm/mach-at91/at91sam9261_devices.c
index 6c1a2ecc306f..b2a34740146a 100644
--- a/arch/arm/mach-at91/at91sam9261_devices.c
+++ b/arch/arm/mach-at91/at91sam9261_devices.c
@@ -880,7 +880,6 @@ static struct resource dbgu_resources[] = {
static struct atmel_uart_data dbgu_data = {
.use_dma_tx = 0,
.use_dma_rx = 0, /* DBGU not capable of receive DMA */
- .rts_gpio = -EINVAL,
};
static u64 dbgu_dmamask = DMA_BIT_MASK(32);
@@ -919,7 +918,6 @@ static struct resource uart0_resources[] = {
static struct atmel_uart_data uart0_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart0_dmamask = DMA_BIT_MASK(32);
@@ -963,7 +961,6 @@ static struct resource uart1_resources[] = {
static struct atmel_uart_data uart1_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart1_dmamask = DMA_BIT_MASK(32);
@@ -1007,7 +1004,6 @@ static struct resource uart2_resources[] = {
static struct atmel_uart_data uart2_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart2_dmamask = DMA_BIT_MASK(32);
diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
index 97cc2a0d6f90..4aeadddbc181 100644
--- a/arch/arm/mach-at91/at91sam9263_devices.c
+++ b/arch/arm/mach-at91/at91sam9263_devices.c
@@ -1324,7 +1324,6 @@ static struct resource dbgu_resources[] = {
static struct atmel_uart_data dbgu_data = {
.use_dma_tx = 0,
.use_dma_rx = 0, /* DBGU not capable of receive DMA */
- .rts_gpio = -EINVAL,
};
static u64 dbgu_dmamask = DMA_BIT_MASK(32);
@@ -1363,7 +1362,6 @@ static struct resource uart0_resources[] = {
static struct atmel_uart_data uart0_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart0_dmamask = DMA_BIT_MASK(32);
@@ -1407,7 +1405,6 @@ static struct resource uart1_resources[] = {
static struct atmel_uart_data uart1_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart1_dmamask = DMA_BIT_MASK(32);
@@ -1451,7 +1448,6 @@ static struct resource uart2_resources[] = {
static struct atmel_uart_data uart2_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart2_dmamask = DMA_BIT_MASK(32);
diff --git a/arch/arm/mach-at91/at91sam9g45_devices.c b/arch/arm/mach-at91/at91sam9g45_devices.c
index c10149588e21..cb36fa872d30 100644
--- a/arch/arm/mach-at91/at91sam9g45_devices.c
+++ b/arch/arm/mach-at91/at91sam9g45_devices.c
@@ -1587,7 +1587,6 @@ static struct resource dbgu_resources[] = {
static struct atmel_uart_data dbgu_data = {
.use_dma_tx = 0,
.use_dma_rx = 0,
- .rts_gpio = -EINVAL,
};
static u64 dbgu_dmamask = DMA_BIT_MASK(32);
@@ -1626,7 +1625,6 @@ static struct resource uart0_resources[] = {
static struct atmel_uart_data uart0_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart0_dmamask = DMA_BIT_MASK(32);
@@ -1670,7 +1668,6 @@ static struct resource uart1_resources[] = {
static struct atmel_uart_data uart1_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart1_dmamask = DMA_BIT_MASK(32);
@@ -1714,7 +1711,6 @@ static struct resource uart2_resources[] = {
static struct atmel_uart_data uart2_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart2_dmamask = DMA_BIT_MASK(32);
@@ -1758,7 +1754,6 @@ static struct resource uart3_resources[] = {
static struct atmel_uart_data uart3_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart3_dmamask = DMA_BIT_MASK(32);
diff --git a/arch/arm/mach-at91/at91sam9rl_devices.c b/arch/arm/mach-at91/at91sam9rl_devices.c
index 4120af972b61..a698bdab2cce 100644
--- a/arch/arm/mach-at91/at91sam9rl_devices.c
+++ b/arch/arm/mach-at91/at91sam9rl_devices.c
@@ -956,7 +956,6 @@ static struct resource dbgu_resources[] = {
static struct atmel_uart_data dbgu_data = {
.use_dma_tx = 0,
.use_dma_rx = 0, /* DBGU not capable of receive DMA */
- .rts_gpio = -EINVAL,
};
static u64 dbgu_dmamask = DMA_BIT_MASK(32);
@@ -995,7 +994,6 @@ static struct resource uart0_resources[] = {
static struct atmel_uart_data uart0_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart0_dmamask = DMA_BIT_MASK(32);
@@ -1047,7 +1045,6 @@ static struct resource uart1_resources[] = {
static struct atmel_uart_data uart1_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart1_dmamask = DMA_BIT_MASK(32);
@@ -1091,7 +1088,6 @@ static struct resource uart2_resources[] = {
static struct atmel_uart_data uart2_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart2_dmamask = DMA_BIT_MASK(32);
@@ -1135,7 +1131,6 @@ static struct resource uart3_resources[] = {
static struct atmel_uart_data uart3_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart3_dmamask = DMA_BIT_MASK(32);
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 4fe8ca16f492..59219318d7c9 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -97,6 +97,7 @@ config SERIAL_ATMEL
bool "AT91 / AT32 on-chip serial port support"
depends on ARCH_AT91 || AVR32
select SERIAL_CORE
+ select SERIAL_MCTRL_GPIO
help
This enables the driver for the on-chip UARTs of the Atmel
AT91 and AT32 processors.
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index a51b3a762948..7ac2d1dfe78f 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -43,6 +43,7 @@
#include <linux/platform_data/atmel.h>
#include <linux/timer.h>
#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <asm/io.h>
#include <asm/ioctls.h>
@@ -57,6 +58,8 @@
#include <linux/serial_core.h>
+#include "serial_mctrl_gpio.h"
+
static void atmel_start_rx(struct uart_port *port);
static void atmel_stop_rx(struct uart_port *port);
@@ -162,7 +165,7 @@ struct atmel_uart_port {
struct circ_buf rx_ring;
struct serial_rs485 rs485; /* rs485 settings */
- int rts_gpio; /* optional RTS GPIO */
+ struct mctrl_gpios gpios;
unsigned int tx_done_mask;
bool is_usart; /* usart or uart */
struct timer_list uart_timer; /* uart timer */
@@ -237,6 +240,46 @@ static bool atmel_use_dma_rx(struct uart_port *port)
return atmel_port->use_dma_rx;
}
+static unsigned int atmel_get_lines_status(struct uart_port *port)
+{
+ struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+ unsigned int status, ret = 0;
+
+ status = UART_GET_CSR(port);
+
+ mctrl_gpio_get(&atmel_port->gpios, &ret);
+
+ if (!IS_ERR_OR_NULL(atmel_port->gpios.gpio[UART_GPIO_CTS])) {
+ if (ret & TIOCM_CTS)
+ status &= ~ATMEL_US_CTS;
+ else
+ status |= ATMEL_US_CTS;
+ }
+
+ if (!IS_ERR_OR_NULL(atmel_port->gpios.gpio[UART_GPIO_DSR])) {
+ if (ret & TIOCM_DSR)
+ status &= ~ATMEL_US_DSR;
+ else
+ status |= ATMEL_US_DSR;
+ }
+
+ if (!IS_ERR_OR_NULL(atmel_port->gpios.gpio[UART_GPIO_RI])) {
+ if (ret & TIOCM_RI)
+ status &= ~ATMEL_US_RI;
+ else
+ status |= ATMEL_US_RI;
+ }
+
+ if (!IS_ERR_OR_NULL(atmel_port->gpios.gpio[UART_GPIO_DCD])) {
+ if (ret & TIOCM_CD)
+ status &= ~ATMEL_US_DCD;
+ else
+ status |= ATMEL_US_DCD;
+ }
+
+ return status;
+}
+
/* Enable or disable the rs485 support */
void atmel_config_rs485(struct uart_port *port, struct serial_rs485 *rs485conf)
{
@@ -296,17 +339,6 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
unsigned int mode;
struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
- /*
- * AT91RM9200 Errata #39: RTS0 is not internally connected
- * to PA21. We need to drive the pin as a GPIO.
- */
- if (gpio_is_valid(atmel_port->rts_gpio)) {
- if (mctrl & TIOCM_RTS)
- gpio_set_value(atmel_port->rts_gpio, 0);
- else
- gpio_set_value(atmel_port->rts_gpio, 1);
- }
-
if (mctrl & TIOCM_RTS)
control |= ATMEL_US_RTSEN;
else
@@ -319,6 +351,8 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
UART_PUT_CR(port, control);
+ mctrl_gpio_set(&atmel_port->gpios, mctrl);
+
/* Local loopback mode? */
mode = UART_GET_MR(port) & ~ATMEL_US_CHMODE;
if (mctrl & TIOCM_LOOP)
@@ -346,7 +380,8 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
*/
static u_int atmel_get_mctrl(struct uart_port *port)
{
- unsigned int status, ret = 0;
+ struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+ unsigned int ret = 0, status;
status = UART_GET_CSR(port);
@@ -362,7 +397,7 @@ static u_int atmel_get_mctrl(struct uart_port *port)
if (!(status & ATMEL_US_RI))
ret |= TIOCM_RI;
- return ret;
+ return mctrl_gpio_get(&atmel_port->gpios, &ret);
}
/*
@@ -1042,7 +1077,7 @@ static irqreturn_t atmel_interrupt(int irq, void *dev_id)
unsigned int status, pending, pass_counter = 0;
do {
- status = UART_GET_CSR(port);
+ status = atmel_get_lines_status(port);
pending = status & UART_GET_IMR(port);
if (!pending)
break;
@@ -1568,7 +1603,7 @@ static int atmel_startup(struct uart_port *port)
}
/* Save current CSR for comparison in atmel_tasklet_func() */
- atmel_port->irq_status_prev = UART_GET_CSR(port);
+ atmel_port->irq_status_prev = atmel_get_lines_status(port);
atmel_port->irq_status = atmel_port->irq_status_prev;
/*
@@ -2327,6 +2362,15 @@ 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)
+{
+ enum mctrl_gpio_idx i;
+ int err;
+
+ err = mctrl_gpio_init(dev, &p->gpios);
+ return err;
+}
+
static int atmel_serial_probe(struct platform_device *pdev)
{
struct atmel_uart_port *port;
@@ -2362,25 +2406,11 @@ static int atmel_serial_probe(struct platform_device *pdev)
port = &atmel_ports[ret];
port->backup_imr = 0;
port->uart.line = ret;
- port->rts_gpio = -EINVAL; /* Invalid, zero could be valid */
- if (pdata)
- port->rts_gpio = pdata->rts_gpio;
- else if (np)
- port->rts_gpio = of_get_named_gpio(np, "rts-gpios", 0);
-
- if (gpio_is_valid(port->rts_gpio)) {
- ret = devm_gpio_request(&pdev->dev, port->rts_gpio, "RTS");
- if (ret) {
- dev_err(&pdev->dev, "error requesting RTS GPIO\n");
- goto err;
- }
- /* Default to 1 as RTS is active low */
- ret = gpio_direction_output(port->rts_gpio, 1);
- if (ret) {
- dev_err(&pdev->dev, "error setting up RTS GPIO\n");
- goto err;
- }
- }
+
+ ret = atmel_init_gpios(port, &pdev->dev);
+ if (ret < 0)
+ dev_err(&pdev->dev, "Failed to initialize %d GPIOs. The serial port may not work as expected",
+ ret * -1);
ret = atmel_init_port(port, pdev);
if (ret)
diff --git a/include/linux/platform_data/atmel.h b/include/linux/platform_data/atmel.h
index e26b0c14edea..cea9f70133c5 100644
--- a/include/linux/platform_data/atmel.h
+++ b/include/linux/platform_data/atmel.h
@@ -84,7 +84,6 @@ struct atmel_uart_data {
short use_dma_rx; /* use receive DMA? */
void __iomem *regs; /* virt. base address, if any */
struct serial_rs485 rs485; /* rs485 settings */
- int rts_gpio; /* optional RTS GPIO */
};
/* Touchscreen Controller */
--
1.8.5
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v3 4/7] tty/serial: at91: use mctrl_gpio helpers
@ 2014-02-17 16:57 ` Richard Genoud
0 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-17 16:57 UTC (permalink / raw)
To: linux-arm-kernel
On sam9x5, dedicated CTS (and RTS) pins are unusable together with the
LCDC, the EMAC, or the MMC because they share the same line.
Moreover, the USART controller doesn't handle DTR/DSR/DCD/RI signals,
so we have to control them via GPIO.
This patch permits to use GPIOs to control the CTS/RTS/DTR/DSR/DCD/RI
signals.
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
arch/arm/mach-at91/at91rm9200_devices.c | 16 +++--
arch/arm/mach-at91/at91sam9260_devices.c | 7 ---
arch/arm/mach-at91/at91sam9261_devices.c | 4 --
arch/arm/mach-at91/at91sam9263_devices.c | 4 --
arch/arm/mach-at91/at91sam9g45_devices.c | 5 --
arch/arm/mach-at91/at91sam9rl_devices.c | 5 --
drivers/tty/serial/Kconfig | 1 +
drivers/tty/serial/atmel_serial.c | 100 ++++++++++++++++++++-----------
include/linux/platform_data/atmel.h | 1 -
9 files changed, 76 insertions(+), 67 deletions(-)
diff --git a/arch/arm/mach-at91/at91rm9200_devices.c b/arch/arm/mach-at91/at91rm9200_devices.c
index 605add05af7e..6dd386e3d9fe 100644
--- a/arch/arm/mach-at91/at91rm9200_devices.c
+++ b/arch/arm/mach-at91/at91rm9200_devices.c
@@ -15,6 +15,7 @@
#include <linux/dma-mapping.h>
#include <linux/gpio.h>
+#include <linux/gpio/driver.h>
#include <linux/platform_device.h>
#include <linux/i2c-gpio.h>
@@ -922,7 +923,6 @@ static struct resource dbgu_resources[] = {
static struct atmel_uart_data dbgu_data = {
.use_dma_tx = 0,
.use_dma_rx = 0, /* DBGU not capable of receive DMA */
- .rts_gpio = -EINVAL,
};
static u64 dbgu_dmamask = DMA_BIT_MASK(32);
@@ -961,7 +961,14 @@ static struct resource uart0_resources[] = {
static struct atmel_uart_data uart0_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
+};
+
+struct gpiod_lookup_table uart0_gpios_table = {
+ .dev_id = "atmel_usart",
+ .table = {
+ GPIO_LOOKUP("pioA", 21, "rts", GPIO_ACTIVE_LOW),
+ { },
+ },
};
static u64 uart0_dmamask = DMA_BIT_MASK(32);
@@ -992,7 +999,7 @@ static inline void configure_usart0_pins(unsigned pins)
* We need to drive the pin manually. The serial driver will driver
* this to high when initializing.
*/
- uart0_data.rts_gpio = AT91_PIN_PA21;
+ gpiod_add_lookup_table(&uart0_gpios_table);
}
}
@@ -1012,7 +1019,6 @@ static struct resource uart1_resources[] = {
static struct atmel_uart_data uart1_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart1_dmamask = DMA_BIT_MASK(32);
@@ -1064,7 +1070,6 @@ static struct resource uart2_resources[] = {
static struct atmel_uart_data uart2_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart2_dmamask = DMA_BIT_MASK(32);
@@ -1108,7 +1113,6 @@ static struct resource uart3_resources[] = {
static struct atmel_uart_data uart3_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart3_dmamask = DMA_BIT_MASK(32);
diff --git a/arch/arm/mach-at91/at91sam9260_devices.c b/arch/arm/mach-at91/at91sam9260_devices.c
index b52527c78b12..eda8d1679d40 100644
--- a/arch/arm/mach-at91/at91sam9260_devices.c
+++ b/arch/arm/mach-at91/at91sam9260_devices.c
@@ -819,7 +819,6 @@ static struct resource dbgu_resources[] = {
static struct atmel_uart_data dbgu_data = {
.use_dma_tx = 0,
.use_dma_rx = 0, /* DBGU not capable of receive DMA */
- .rts_gpio = -EINVAL,
};
static u64 dbgu_dmamask = DMA_BIT_MASK(32);
@@ -858,7 +857,6 @@ static struct resource uart0_resources[] = {
static struct atmel_uart_data uart0_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart0_dmamask = DMA_BIT_MASK(32);
@@ -910,7 +908,6 @@ static struct resource uart1_resources[] = {
static struct atmel_uart_data uart1_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart1_dmamask = DMA_BIT_MASK(32);
@@ -954,7 +951,6 @@ static struct resource uart2_resources[] = {
static struct atmel_uart_data uart2_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart2_dmamask = DMA_BIT_MASK(32);
@@ -998,7 +994,6 @@ static struct resource uart3_resources[] = {
static struct atmel_uart_data uart3_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart3_dmamask = DMA_BIT_MASK(32);
@@ -1042,7 +1037,6 @@ static struct resource uart4_resources[] = {
static struct atmel_uart_data uart4_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart4_dmamask = DMA_BIT_MASK(32);
@@ -1081,7 +1075,6 @@ static struct resource uart5_resources[] = {
static struct atmel_uart_data uart5_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart5_dmamask = DMA_BIT_MASK(32);
diff --git a/arch/arm/mach-at91/at91sam9261_devices.c b/arch/arm/mach-at91/at91sam9261_devices.c
index 6c1a2ecc306f..b2a34740146a 100644
--- a/arch/arm/mach-at91/at91sam9261_devices.c
+++ b/arch/arm/mach-at91/at91sam9261_devices.c
@@ -880,7 +880,6 @@ static struct resource dbgu_resources[] = {
static struct atmel_uart_data dbgu_data = {
.use_dma_tx = 0,
.use_dma_rx = 0, /* DBGU not capable of receive DMA */
- .rts_gpio = -EINVAL,
};
static u64 dbgu_dmamask = DMA_BIT_MASK(32);
@@ -919,7 +918,6 @@ static struct resource uart0_resources[] = {
static struct atmel_uart_data uart0_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart0_dmamask = DMA_BIT_MASK(32);
@@ -963,7 +961,6 @@ static struct resource uart1_resources[] = {
static struct atmel_uart_data uart1_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart1_dmamask = DMA_BIT_MASK(32);
@@ -1007,7 +1004,6 @@ static struct resource uart2_resources[] = {
static struct atmel_uart_data uart2_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart2_dmamask = DMA_BIT_MASK(32);
diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
index 97cc2a0d6f90..4aeadddbc181 100644
--- a/arch/arm/mach-at91/at91sam9263_devices.c
+++ b/arch/arm/mach-at91/at91sam9263_devices.c
@@ -1324,7 +1324,6 @@ static struct resource dbgu_resources[] = {
static struct atmel_uart_data dbgu_data = {
.use_dma_tx = 0,
.use_dma_rx = 0, /* DBGU not capable of receive DMA */
- .rts_gpio = -EINVAL,
};
static u64 dbgu_dmamask = DMA_BIT_MASK(32);
@@ -1363,7 +1362,6 @@ static struct resource uart0_resources[] = {
static struct atmel_uart_data uart0_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart0_dmamask = DMA_BIT_MASK(32);
@@ -1407,7 +1405,6 @@ static struct resource uart1_resources[] = {
static struct atmel_uart_data uart1_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart1_dmamask = DMA_BIT_MASK(32);
@@ -1451,7 +1448,6 @@ static struct resource uart2_resources[] = {
static struct atmel_uart_data uart2_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart2_dmamask = DMA_BIT_MASK(32);
diff --git a/arch/arm/mach-at91/at91sam9g45_devices.c b/arch/arm/mach-at91/at91sam9g45_devices.c
index c10149588e21..cb36fa872d30 100644
--- a/arch/arm/mach-at91/at91sam9g45_devices.c
+++ b/arch/arm/mach-at91/at91sam9g45_devices.c
@@ -1587,7 +1587,6 @@ static struct resource dbgu_resources[] = {
static struct atmel_uart_data dbgu_data = {
.use_dma_tx = 0,
.use_dma_rx = 0,
- .rts_gpio = -EINVAL,
};
static u64 dbgu_dmamask = DMA_BIT_MASK(32);
@@ -1626,7 +1625,6 @@ static struct resource uart0_resources[] = {
static struct atmel_uart_data uart0_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart0_dmamask = DMA_BIT_MASK(32);
@@ -1670,7 +1668,6 @@ static struct resource uart1_resources[] = {
static struct atmel_uart_data uart1_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart1_dmamask = DMA_BIT_MASK(32);
@@ -1714,7 +1711,6 @@ static struct resource uart2_resources[] = {
static struct atmel_uart_data uart2_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart2_dmamask = DMA_BIT_MASK(32);
@@ -1758,7 +1754,6 @@ static struct resource uart3_resources[] = {
static struct atmel_uart_data uart3_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart3_dmamask = DMA_BIT_MASK(32);
diff --git a/arch/arm/mach-at91/at91sam9rl_devices.c b/arch/arm/mach-at91/at91sam9rl_devices.c
index 4120af972b61..a698bdab2cce 100644
--- a/arch/arm/mach-at91/at91sam9rl_devices.c
+++ b/arch/arm/mach-at91/at91sam9rl_devices.c
@@ -956,7 +956,6 @@ static struct resource dbgu_resources[] = {
static struct atmel_uart_data dbgu_data = {
.use_dma_tx = 0,
.use_dma_rx = 0, /* DBGU not capable of receive DMA */
- .rts_gpio = -EINVAL,
};
static u64 dbgu_dmamask = DMA_BIT_MASK(32);
@@ -995,7 +994,6 @@ static struct resource uart0_resources[] = {
static struct atmel_uart_data uart0_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart0_dmamask = DMA_BIT_MASK(32);
@@ -1047,7 +1045,6 @@ static struct resource uart1_resources[] = {
static struct atmel_uart_data uart1_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart1_dmamask = DMA_BIT_MASK(32);
@@ -1091,7 +1088,6 @@ static struct resource uart2_resources[] = {
static struct atmel_uart_data uart2_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart2_dmamask = DMA_BIT_MASK(32);
@@ -1135,7 +1131,6 @@ static struct resource uart3_resources[] = {
static struct atmel_uart_data uart3_data = {
.use_dma_tx = 1,
.use_dma_rx = 1,
- .rts_gpio = -EINVAL,
};
static u64 uart3_dmamask = DMA_BIT_MASK(32);
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 4fe8ca16f492..59219318d7c9 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -97,6 +97,7 @@ config SERIAL_ATMEL
bool "AT91 / AT32 on-chip serial port support"
depends on ARCH_AT91 || AVR32
select SERIAL_CORE
+ select SERIAL_MCTRL_GPIO
help
This enables the driver for the on-chip UARTs of the Atmel
AT91 and AT32 processors.
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index a51b3a762948..7ac2d1dfe78f 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -43,6 +43,7 @@
#include <linux/platform_data/atmel.h>
#include <linux/timer.h>
#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <asm/io.h>
#include <asm/ioctls.h>
@@ -57,6 +58,8 @@
#include <linux/serial_core.h>
+#include "serial_mctrl_gpio.h"
+
static void atmel_start_rx(struct uart_port *port);
static void atmel_stop_rx(struct uart_port *port);
@@ -162,7 +165,7 @@ struct atmel_uart_port {
struct circ_buf rx_ring;
struct serial_rs485 rs485; /* rs485 settings */
- int rts_gpio; /* optional RTS GPIO */
+ struct mctrl_gpios gpios;
unsigned int tx_done_mask;
bool is_usart; /* usart or uart */
struct timer_list uart_timer; /* uart timer */
@@ -237,6 +240,46 @@ static bool atmel_use_dma_rx(struct uart_port *port)
return atmel_port->use_dma_rx;
}
+static unsigned int atmel_get_lines_status(struct uart_port *port)
+{
+ struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+ unsigned int status, ret = 0;
+
+ status = UART_GET_CSR(port);
+
+ mctrl_gpio_get(&atmel_port->gpios, &ret);
+
+ if (!IS_ERR_OR_NULL(atmel_port->gpios.gpio[UART_GPIO_CTS])) {
+ if (ret & TIOCM_CTS)
+ status &= ~ATMEL_US_CTS;
+ else
+ status |= ATMEL_US_CTS;
+ }
+
+ if (!IS_ERR_OR_NULL(atmel_port->gpios.gpio[UART_GPIO_DSR])) {
+ if (ret & TIOCM_DSR)
+ status &= ~ATMEL_US_DSR;
+ else
+ status |= ATMEL_US_DSR;
+ }
+
+ if (!IS_ERR_OR_NULL(atmel_port->gpios.gpio[UART_GPIO_RI])) {
+ if (ret & TIOCM_RI)
+ status &= ~ATMEL_US_RI;
+ else
+ status |= ATMEL_US_RI;
+ }
+
+ if (!IS_ERR_OR_NULL(atmel_port->gpios.gpio[UART_GPIO_DCD])) {
+ if (ret & TIOCM_CD)
+ status &= ~ATMEL_US_DCD;
+ else
+ status |= ATMEL_US_DCD;
+ }
+
+ return status;
+}
+
/* Enable or disable the rs485 support */
void atmel_config_rs485(struct uart_port *port, struct serial_rs485 *rs485conf)
{
@@ -296,17 +339,6 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
unsigned int mode;
struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
- /*
- * AT91RM9200 Errata #39: RTS0 is not internally connected
- * to PA21. We need to drive the pin as a GPIO.
- */
- if (gpio_is_valid(atmel_port->rts_gpio)) {
- if (mctrl & TIOCM_RTS)
- gpio_set_value(atmel_port->rts_gpio, 0);
- else
- gpio_set_value(atmel_port->rts_gpio, 1);
- }
-
if (mctrl & TIOCM_RTS)
control |= ATMEL_US_RTSEN;
else
@@ -319,6 +351,8 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
UART_PUT_CR(port, control);
+ mctrl_gpio_set(&atmel_port->gpios, mctrl);
+
/* Local loopback mode? */
mode = UART_GET_MR(port) & ~ATMEL_US_CHMODE;
if (mctrl & TIOCM_LOOP)
@@ -346,7 +380,8 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
*/
static u_int atmel_get_mctrl(struct uart_port *port)
{
- unsigned int status, ret = 0;
+ struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+ unsigned int ret = 0, status;
status = UART_GET_CSR(port);
@@ -362,7 +397,7 @@ static u_int atmel_get_mctrl(struct uart_port *port)
if (!(status & ATMEL_US_RI))
ret |= TIOCM_RI;
- return ret;
+ return mctrl_gpio_get(&atmel_port->gpios, &ret);
}
/*
@@ -1042,7 +1077,7 @@ static irqreturn_t atmel_interrupt(int irq, void *dev_id)
unsigned int status, pending, pass_counter = 0;
do {
- status = UART_GET_CSR(port);
+ status = atmel_get_lines_status(port);
pending = status & UART_GET_IMR(port);
if (!pending)
break;
@@ -1568,7 +1603,7 @@ static int atmel_startup(struct uart_port *port)
}
/* Save current CSR for comparison in atmel_tasklet_func() */
- atmel_port->irq_status_prev = UART_GET_CSR(port);
+ atmel_port->irq_status_prev = atmel_get_lines_status(port);
atmel_port->irq_status = atmel_port->irq_status_prev;
/*
@@ -2327,6 +2362,15 @@ 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)
+{
+ enum mctrl_gpio_idx i;
+ int err;
+
+ err = mctrl_gpio_init(dev, &p->gpios);
+ return err;
+}
+
static int atmel_serial_probe(struct platform_device *pdev)
{
struct atmel_uart_port *port;
@@ -2362,25 +2406,11 @@ static int atmel_serial_probe(struct platform_device *pdev)
port = &atmel_ports[ret];
port->backup_imr = 0;
port->uart.line = ret;
- port->rts_gpio = -EINVAL; /* Invalid, zero could be valid */
- if (pdata)
- port->rts_gpio = pdata->rts_gpio;
- else if (np)
- port->rts_gpio = of_get_named_gpio(np, "rts-gpios", 0);
-
- if (gpio_is_valid(port->rts_gpio)) {
- ret = devm_gpio_request(&pdev->dev, port->rts_gpio, "RTS");
- if (ret) {
- dev_err(&pdev->dev, "error requesting RTS GPIO\n");
- goto err;
- }
- /* Default to 1 as RTS is active low */
- ret = gpio_direction_output(port->rts_gpio, 1);
- if (ret) {
- dev_err(&pdev->dev, "error setting up RTS GPIO\n");
- goto err;
- }
- }
+
+ ret = atmel_init_gpios(port, &pdev->dev);
+ if (ret < 0)
+ dev_err(&pdev->dev, "Failed to initialize %d GPIOs. The serial port may not work as expected",
+ ret * -1);
ret = atmel_init_port(port, pdev);
if (ret)
diff --git a/include/linux/platform_data/atmel.h b/include/linux/platform_data/atmel.h
index e26b0c14edea..cea9f70133c5 100644
--- a/include/linux/platform_data/atmel.h
+++ b/include/linux/platform_data/atmel.h
@@ -84,7 +84,6 @@ struct atmel_uart_data {
short use_dma_rx; /* use receive DMA? */
void __iomem *regs; /* virt. base address, if any */
struct serial_rs485 rs485; /* rs485 settings */
- int rts_gpio; /* optional RTS GPIO */
};
/* Touchscreen Controller */
--
1.8.5
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v3 5/7] ARM: at91: gpio: implement get_direction
2014-02-17 16:57 ` Richard Genoud
@ 2014-02-17 16:57 ` Richard Genoud
-1 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-17 16:57 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Uwe Kleine-König, Nicolas Ferre, Linus Walleij,
Alexander Shiyan, linux-serial, linux-arm-kernel, Richard Genoud,
Jean-Christophe Plagniol-Villard
This is needed for gpiod_get_direction().
Otherwise, it returns -EINVAL.
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
arch/arm/mach-at91/gpio.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index a5afcf76550e..afbe34027e62 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -49,6 +49,7 @@ static int at91_gpiolib_request(struct gpio_chip *chip, unsigned offset);
static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip);
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val);
static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
+static int at91_gpiolib_get_direction(struct gpio_chip *chip, unsigned offset);
static int at91_gpiolib_direction_output(struct gpio_chip *chip,
unsigned offset, int val);
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
@@ -60,6 +61,7 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset);
.chip = { \
.label = name, \
.request = at91_gpiolib_request, \
+ .get_direction = at91_gpiolib_get_direction, \
.direction_input = at91_gpiolib_direction_input, \
.direction_output = at91_gpiolib_direction_output, \
.get = at91_gpiolib_get, \
@@ -799,6 +801,17 @@ static int at91_gpiolib_request(struct gpio_chip *chip, unsigned offset)
return 0;
}
+static int at91_gpiolib_get_direction(struct gpio_chip *chip, unsigned offset)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+ unsigned mask = 1 << offset;
+ u32 osr;
+
+ osr = __raw_readl(pio + PIO_OSR);
+ return !(osr & mask);
+}
+
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
unsigned offset)
{
--
1.8.5
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v3 5/7] ARM: at91: gpio: implement get_direction
@ 2014-02-17 16:57 ` Richard Genoud
0 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-17 16:57 UTC (permalink / raw)
To: linux-arm-kernel
This is needed for gpiod_get_direction().
Otherwise, it returns -EINVAL.
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
arch/arm/mach-at91/gpio.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index a5afcf76550e..afbe34027e62 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -49,6 +49,7 @@ static int at91_gpiolib_request(struct gpio_chip *chip, unsigned offset);
static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip);
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val);
static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
+static int at91_gpiolib_get_direction(struct gpio_chip *chip, unsigned offset);
static int at91_gpiolib_direction_output(struct gpio_chip *chip,
unsigned offset, int val);
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
@@ -60,6 +61,7 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset);
.chip = { \
.label = name, \
.request = at91_gpiolib_request, \
+ .get_direction = at91_gpiolib_get_direction, \
.direction_input = at91_gpiolib_direction_input, \
.direction_output = at91_gpiolib_direction_output, \
.get = at91_gpiolib_get, \
@@ -799,6 +801,17 @@ static int at91_gpiolib_request(struct gpio_chip *chip, unsigned offset)
return 0;
}
+static int at91_gpiolib_get_direction(struct gpio_chip *chip, unsigned offset)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+ unsigned mask = 1 << offset;
+ u32 osr;
+
+ osr = __raw_readl(pio + PIO_OSR);
+ return !(osr & mask);
+}
+
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
unsigned offset)
{
--
1.8.5
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v3 6/7] pinctrl: at91: implement get_direction
2014-02-17 16:57 ` Richard Genoud
@ 2014-02-17 16:57 ` Richard Genoud
-1 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-17 16:57 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Uwe Kleine-König, Nicolas Ferre, Linus Walleij,
Alexander Shiyan, linux-serial, linux-arm-kernel, Richard Genoud,
Jean-Christophe Plagniol-Villard
This is needed for gpiod_get_direction().
Otherwise, it returns -EINVAL.
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
drivers/pinctrl/pinctrl-at91.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index d990e33d8aa7..9bd18fdc604a 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -1137,6 +1137,17 @@ static void at91_gpio_free(struct gpio_chip *chip, unsigned offset)
pinctrl_free_gpio(gpio);
}
+static int at91_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+ unsigned mask = 1 << offset;
+ u32 osr;
+
+ osr = readl_relaxed(pio + PIO_OSR);
+ return !(osr & mask);
+}
+
static int at91_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
{
struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
@@ -1543,6 +1554,7 @@ static int at91_gpio_of_irq_setup(struct device_node *node,
static struct gpio_chip at91_gpio_template = {
.request = at91_gpio_request,
.free = at91_gpio_free,
+ .get_direction = at91_gpio_get_direction,
.direction_input = at91_gpio_direction_input,
.get = at91_gpio_get,
.direction_output = at91_gpio_direction_output,
--
1.8.5
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v3 6/7] pinctrl: at91: implement get_direction
@ 2014-02-17 16:57 ` Richard Genoud
0 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-17 16:57 UTC (permalink / raw)
To: linux-arm-kernel
This is needed for gpiod_get_direction().
Otherwise, it returns -EINVAL.
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
drivers/pinctrl/pinctrl-at91.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index d990e33d8aa7..9bd18fdc604a 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -1137,6 +1137,17 @@ static void at91_gpio_free(struct gpio_chip *chip, unsigned offset)
pinctrl_free_gpio(gpio);
}
+static int at91_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+ unsigned mask = 1 << offset;
+ u32 osr;
+
+ osr = readl_relaxed(pio + PIO_OSR);
+ return !(osr & mask);
+}
+
static int at91_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
{
struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
@@ -1543,6 +1554,7 @@ static int at91_gpio_of_irq_setup(struct device_node *node,
static struct gpio_chip at91_gpio_template = {
.request = at91_gpio_request,
.free = at91_gpio_free,
+ .get_direction = at91_gpio_get_direction,
.direction_input = at91_gpio_direction_input,
.get = at91_gpio_get,
.direction_output = at91_gpio_direction_output,
--
1.8.5
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v3 7/7] tty/serial: at91: add interrupts for modem control lines
2014-02-17 16:57 ` Richard Genoud
@ 2014-02-17 16:57 ` Richard Genoud
-1 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-17 16:57 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Uwe Kleine-König, Nicolas Ferre, Linus Walleij,
Alexander Shiyan, linux-serial, linux-arm-kernel, Richard Genoud
Handle CTS/DSR/RI/DCD GPIO interrupts in atmel_serial.
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
drivers/tty/serial/atmel_serial.c | 126 +++++++++++++++++++++++++++++++++++++-
1 file changed, 123 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 7ac2d1dfe78f..1a0af62060f2 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -44,6 +44,7 @@
#include <linux/timer.h>
#include <linux/gpio.h>
#include <linux/gpio/consumer.h>
+#include <linux/irq.h>
#include <asm/io.h>
#include <asm/ioctls.h>
@@ -166,7 +167,9 @@ struct atmel_uart_port {
struct serial_rs485 rs485; /* rs485 settings */
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 */
struct timer_list uart_timer; /* uart timer */
int (*prepare_rx)(struct uart_port *port);
@@ -484,8 +487,38 @@ static void atmel_stop_rx(struct uart_port *port)
*/
static void atmel_enable_ms(struct uart_port *port)
{
- UART_PUT_IER(port, ATMEL_US_RIIC | ATMEL_US_DSRIC
- | ATMEL_US_DCDIC | ATMEL_US_CTSIC);
+ struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+ uint32_t ier = 0;
+
+ /*
+ * Interrupt should not be enabled twice
+ */
+ if (atmel_port->ms_irq_enabled)
+ return;
+
+ 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
+ ier |= ATMEL_US_CTSIC;
+
+ if (atmel_port->gpio_irq[UART_GPIO_DSR] >= 0)
+ enable_irq(atmel_port->gpio_irq[UART_GPIO_DSR]);
+ else
+ ier |= ATMEL_US_DSRIC;
+
+ if (atmel_port->gpio_irq[UART_GPIO_RI] >= 0)
+ enable_irq(atmel_port->gpio_irq[UART_GPIO_RI]);
+ else
+ ier |= ATMEL_US_RIIC;
+
+ if (atmel_port->gpio_irq[UART_GPIO_DCD] >= 0)
+ enable_irq(atmel_port->gpio_irq[UART_GPIO_DCD]);
+ else
+ ier |= ATMEL_US_DCDIC;
+
+ UART_PUT_IER(port, ier);
}
/*
@@ -1074,11 +1107,35 @@ 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 >= 0) &&
+ (irq == atmel_port->gpio_irq[UART_GPIO_CTS]))
+ pending |= ATMEL_US_CTSIC;
+
+ if ((irq >= 0) &&
+ (irq == atmel_port->gpio_irq[UART_GPIO_DSR]))
+ pending |= ATMEL_US_DSRIC;
+
+ if ((irq >= 0) &&
+ (irq == atmel_port->gpio_irq[UART_GPIO_RI]))
+ pending |= ATMEL_US_RIIC;
+
+ if ((irq >= 0) &&
+ (irq == atmel_port->gpio_irq[UART_GPIO_DCD]))
+ pending |= ATMEL_US_DCDIC;
+
+ gpio_handled = true;
+ }
if (!pending)
break;
@@ -1558,6 +1615,45 @@ 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)
+ if (irq[i] >= 0)
+ free_irq(irq[i], port);
+
+ return err;
+}
+
/*
* Perform initialization and enable port for reception
*/
@@ -1574,6 +1670,7 @@ static int atmel_startup(struct uart_port *port)
* handle an unexpected interrupt
*/
UART_PUT_IDR(port, -1);
+ atmel_port->ms_irq_enabled = false;
/*
* Allocate the IRQ
@@ -1586,6 +1683,13 @@ static int atmel_startup(struct uart_port *port)
}
/*
+ * Get the GPIO lines IRQ
+ */
+ retval = atmel_request_gpio_irq(port);
+ if (retval)
+ goto free_irq;
+
+ /*
* Initialize DMA (if necessary)
*/
atmel_init_property(atmel_port, pdev);
@@ -1649,6 +1753,11 @@ static int atmel_startup(struct uart_port *port)
}
return 0;
+
+free_irq:
+ free_irq(port->irq, port);
+
+ return retval;
}
/*
@@ -1696,9 +1805,12 @@ static void atmel_shutdown(struct uart_port *port)
atmel_port->rx_ring.tail = 0;
/*
- * Free the interrupt
+ * Free the interrupts
*/
free_irq(port->irq, port);
+ atmel_free_gpio_irq(port);
+
+ atmel_port->ms_irq_enabled = false;
}
/*
@@ -2368,6 +2480,14 @@ static int atmel_init_gpios(struct atmel_uart_port *p, struct device *dev)
int err;
err = mctrl_gpio_init(dev, &p->gpios);
+
+ for (i = 0; i < UART_GPIO_MAX; i++)
+ if (!IS_ERR_OR_NULL(p->gpios.gpio[i]) &&
+ (gpiod_get_direction(p->gpios.gpio[i]) == GPIOF_DIR_IN))
+ p->gpio_irq[i] = gpiod_to_irq(p->gpios.gpio[i]);
+ else
+ p->gpio_irq[i] = -EINVAL;
+
return err;
}
--
1.8.5
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH v3 7/7] tty/serial: at91: add interrupts for modem control lines
@ 2014-02-17 16:57 ` Richard Genoud
0 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-17 16:57 UTC (permalink / raw)
To: linux-arm-kernel
Handle CTS/DSR/RI/DCD GPIO interrupts in atmel_serial.
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
drivers/tty/serial/atmel_serial.c | 126 +++++++++++++++++++++++++++++++++++++-
1 file changed, 123 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 7ac2d1dfe78f..1a0af62060f2 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -44,6 +44,7 @@
#include <linux/timer.h>
#include <linux/gpio.h>
#include <linux/gpio/consumer.h>
+#include <linux/irq.h>
#include <asm/io.h>
#include <asm/ioctls.h>
@@ -166,7 +167,9 @@ struct atmel_uart_port {
struct serial_rs485 rs485; /* rs485 settings */
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 */
struct timer_list uart_timer; /* uart timer */
int (*prepare_rx)(struct uart_port *port);
@@ -484,8 +487,38 @@ static void atmel_stop_rx(struct uart_port *port)
*/
static void atmel_enable_ms(struct uart_port *port)
{
- UART_PUT_IER(port, ATMEL_US_RIIC | ATMEL_US_DSRIC
- | ATMEL_US_DCDIC | ATMEL_US_CTSIC);
+ struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+ uint32_t ier = 0;
+
+ /*
+ * Interrupt should not be enabled twice
+ */
+ if (atmel_port->ms_irq_enabled)
+ return;
+
+ 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
+ ier |= ATMEL_US_CTSIC;
+
+ if (atmel_port->gpio_irq[UART_GPIO_DSR] >= 0)
+ enable_irq(atmel_port->gpio_irq[UART_GPIO_DSR]);
+ else
+ ier |= ATMEL_US_DSRIC;
+
+ if (atmel_port->gpio_irq[UART_GPIO_RI] >= 0)
+ enable_irq(atmel_port->gpio_irq[UART_GPIO_RI]);
+ else
+ ier |= ATMEL_US_RIIC;
+
+ if (atmel_port->gpio_irq[UART_GPIO_DCD] >= 0)
+ enable_irq(atmel_port->gpio_irq[UART_GPIO_DCD]);
+ else
+ ier |= ATMEL_US_DCDIC;
+
+ UART_PUT_IER(port, ier);
}
/*
@@ -1074,11 +1107,35 @@ 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 >= 0) &&
+ (irq == atmel_port->gpio_irq[UART_GPIO_CTS]))
+ pending |= ATMEL_US_CTSIC;
+
+ if ((irq >= 0) &&
+ (irq == atmel_port->gpio_irq[UART_GPIO_DSR]))
+ pending |= ATMEL_US_DSRIC;
+
+ if ((irq >= 0) &&
+ (irq == atmel_port->gpio_irq[UART_GPIO_RI]))
+ pending |= ATMEL_US_RIIC;
+
+ if ((irq >= 0) &&
+ (irq == atmel_port->gpio_irq[UART_GPIO_DCD]))
+ pending |= ATMEL_US_DCDIC;
+
+ gpio_handled = true;
+ }
if (!pending)
break;
@@ -1558,6 +1615,45 @@ 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)
+ if (irq[i] >= 0)
+ free_irq(irq[i], port);
+
+ return err;
+}
+
/*
* Perform initialization and enable port for reception
*/
@@ -1574,6 +1670,7 @@ static int atmel_startup(struct uart_port *port)
* handle an unexpected interrupt
*/
UART_PUT_IDR(port, -1);
+ atmel_port->ms_irq_enabled = false;
/*
* Allocate the IRQ
@@ -1586,6 +1683,13 @@ static int atmel_startup(struct uart_port *port)
}
/*
+ * Get the GPIO lines IRQ
+ */
+ retval = atmel_request_gpio_irq(port);
+ if (retval)
+ goto free_irq;
+
+ /*
* Initialize DMA (if necessary)
*/
atmel_init_property(atmel_port, pdev);
@@ -1649,6 +1753,11 @@ static int atmel_startup(struct uart_port *port)
}
return 0;
+
+free_irq:
+ free_irq(port->irq, port);
+
+ return retval;
}
/*
@@ -1696,9 +1805,12 @@ static void atmel_shutdown(struct uart_port *port)
atmel_port->rx_ring.tail = 0;
/*
- * Free the interrupt
+ * Free the interrupts
*/
free_irq(port->irq, port);
+ atmel_free_gpio_irq(port);
+
+ atmel_port->ms_irq_enabled = false;
}
/*
@@ -2368,6 +2480,14 @@ static int atmel_init_gpios(struct atmel_uart_port *p, struct device *dev)
int err;
err = mctrl_gpio_init(dev, &p->gpios);
+
+ for (i = 0; i < UART_GPIO_MAX; i++)
+ if (!IS_ERR_OR_NULL(p->gpios.gpio[i]) &&
+ (gpiod_get_direction(p->gpios.gpio[i]) == GPIOF_DIR_IN))
+ p->gpio_irq[i] = gpiod_to_irq(p->gpios.gpio[i]);
+ else
+ p->gpio_irq[i] = -EINVAL;
+
return err;
}
--
1.8.5
^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH v3 0/7] tty/serial: Add helpers to use GPIOs to control modem lines and implement atmel_serial.c
2014-02-17 16:57 ` Richard Genoud
@ 2014-02-17 17:53 ` Alexander Shiyan
-1 siblings, 0 replies; 40+ messages in thread
From: Alexander Shiyan @ 2014-02-17 17:53 UTC (permalink / raw)
To: Richard Genoud
Cc: Greg Kroah-Hartman, Uwe Kleine-König, Nicolas Ferre,
Linus Walleij, linux-serial, linux-arm-kernel
Hello.
Понедельник, 17 февраля 2014, 17:57 +01:00 от Richard Genoud <richard.genoud@gmail.com>:
> The USART controller on sam9x5 chips (and also all AT91/SAMA5 chips
> but at91rm9200) are not capable of handling DTR/DSR/DCD/RI signal.
> Moreover, even if the controller can handle CTS/RTS, the dedicated
> CTS/RTS pins are already muxed for other peripherals (LCDC/EMAC/MMC).
>
> So this patchset adds the possibility to control those lines via GPIO,
> as it is done for RTS in the patch "switch atmel serial to use gpiolib"
>
> As it was suggested by Alexander Shiyan, I made that available for
> every board.
>
> Patch 1 implements the generic helpers to control modem lines via GPIO
> Patches 2 and 3 are just a little tidy up of atmel_serial.c.
> Patch 4 implements modem control lines in atmel_serial atmel_serial.
> Patches 5 and 6 implement the get_direction() gpio call for at91, as
> it is needed by gpiod_get_direction().
> Patch 7 implement the interrupts of CTS/DSR/DCD/RI.
I still recommend split this series. The first patch must be a separate,
2 and 3 - independent to this series, 5 and 6 - to the appropriate mailing lists,
and finally as soon as all of the previous will be applied - you can send 7.
Thanks.
---
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 0/7] tty/serial: Add helpers to use GPIOs to control modem lines and implement atmel_serial.c
@ 2014-02-17 17:53 ` Alexander Shiyan
0 siblings, 0 replies; 40+ messages in thread
From: Alexander Shiyan @ 2014-02-17 17:53 UTC (permalink / raw)
To: linux-arm-kernel
Hello.
???????????, 17 ??????? 2014, 17:57 +01:00 ?? Richard Genoud <richard.genoud@gmail.com>:
> The USART controller on sam9x5 chips (and also all AT91/SAMA5 chips
> but at91rm9200) are not capable of handling DTR/DSR/DCD/RI signal.
> Moreover, even if the controller can handle CTS/RTS, the dedicated
> CTS/RTS pins are already muxed for other peripherals (LCDC/EMAC/MMC).
>
> So this patchset adds the possibility to control those lines via GPIO,
> as it is done for RTS in the patch "switch atmel serial to use gpiolib"
>
> As it was suggested by Alexander Shiyan, I made that available for
> every board.
>
> Patch 1 implements the generic helpers to control modem lines via GPIO
> Patches 2 and 3 are just a little tidy up of atmel_serial.c.
> Patch 4 implements modem control lines in atmel_serial atmel_serial.
> Patches 5 and 6 implement the get_direction() gpio call for at91, as
> it is needed by gpiod_get_direction().
> Patch 7 implement the interrupts of CTS/DSR/DCD/RI.
I still recommend split this series. The first patch must be a separate,
2 and 3 - independent to this series, 5 and 6 - to the appropriate mailing lists,
and finally as soon as all of the previous will be applied - you can send 7.
Thanks.
---
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 1/7] tty/serial: Add GPIOLIB helpers for controlling modem lines
2014-02-17 16:57 ` Richard Genoud
@ 2014-02-17 18:37 ` Alexander Shiyan
-1 siblings, 0 replies; 40+ messages in thread
From: Alexander Shiyan @ 2014-02-17 18:37 UTC (permalink / raw)
To: Richard Genoud
Cc: Greg Kroah-Hartman, Linus Walleij, Nicolas Ferre, linux-serial,
Uwe Kleine-König, linux-arm-kernel
Hello.
A few comments below..
Понедельник, 17 февраля 2014, 17:57 +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>
> ---
...
> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
...
> +static const struct {
> + const char *name;
> + unsigned int mctrl;
> + bool dir_out;
> +} mctrl_gpios_desc[] = {
> + { "cts", TIOCM_CTS, false, },
> + { "dsr", TIOCM_DSR, false, },
> + { "dcd", TIOCM_CD, false, },
> + { "ri", TIOCM_RI, false, },
> + { "rts", TIOCM_RTS, true, },
> + { "dtr", TIOCM_DTR, true, },
> + { "out1", TIOCM_OUT1, true, },
> + { "out2", TIOCM_OUT2, true, },
> +};
> +
> +void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
> +{
> + enum mctrl_gpio_idx i;
> +
> + for (i = 0; i < UART_GPIO_MAX; i++)
Use ARRAY_SIZE(mctrl_gpios_desc) here and elsewhere below.
> + if (!IS_ERR_OR_NULL(gpios->gpio[i]) &&
> + mctrl_gpios_desc[i].dir_out)
> + gpiod_set_value(gpios->gpio[i],
> + !!(mctrl & mctrl_gpios_desc[i].mctrl));
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_set);
> +
> +unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
> +{
Why you want to put mctrl as parameter here?
Let's return mctrl from GPIOs, then handle this value as you want int the driver.
> + enum mctrl_gpio_idx i;
> +
> + for (i = 0; i < UART_GPIO_MAX; i++) {
> + if (!IS_ERR_OR_NULL(gpios->gpio[i]) &&
> + !mctrl_gpios_desc[i].dir_out) {
> + if (gpiod_get_value(gpios->gpio[i]))
> + *mctrl |= mctrl_gpios_desc[i].mctrl;
> + else
> + *mctrl &= ~mctrl_gpios_desc[i].mctrl;
> + }
> + }
> +
> + return *mctrl;
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_get);
> +
> +int mctrl_gpio_init(struct device *dev, struct mctrl_gpios *gpios)
> +{
I suggest to allocate "gpios" here and return pointer to this structure
or ERR_PTR(). Additionally, as I mentioned before, add "index" variable
to specify port number.
> + enum mctrl_gpio_idx i;
> + int err = 0;
> + int ret = 0;
> +
> + for (i = 0; i < UART_GPIO_MAX; i++) {
> + gpios->gpio[i] = devm_gpiod_get(dev, mctrl_gpios_desc[i].name);
> +
> + /*
> + * The GPIOs are maybe not all filled,
> + * this is not an error.
> + */
> + if (IS_ERR_OR_NULL(gpios->gpio[i]))
> + continue;
> +
> + if (mctrl_gpios_desc[i].dir_out)
> + err = gpiod_direction_output(gpios->gpio[i], 0);
> + else
> + err = gpiod_direction_input(gpios->gpio[i]);
> + if (err) {
> + dev_err(dev, "Unable to set direction for %s GPIO",
> + mctrl_gpios_desc[i].name);
> + devm_gpiod_put(dev, gpios->gpio[i]);
> + gpios->gpio[i] = NULL;
> + ret--;
> + }
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_init);
...
> diff --git a/drivers/tty/serial/serial_mctrl_gpio.h b/drivers/tty/serial/serial_mctrl_gpio.h
...
> +enum mctrl_gpio_idx {
> + UART_GPIO_CTS,
> + UART_GPIO_DSR,
> + UART_GPIO_DCD,
> + UART_GPIO_RI,
> + UART_GPIO_RTS,
> + UART_GPIO_DTR,
> + UART_GPIO_OUT1,
> + UART_GPIO_OUT2,
> + UART_GPIO_MAX,
> +};
> +
> +struct mctrl_gpios {
> + struct gpio_desc *gpio[UART_GPIO_MAX];
struct gpio_desc *gpio;
...
> +static inline
> +int mctrl_gpio_init(struct device *dev, struct mctrl_gpios *gpios)
> +{
> + return -UART_GPIO_MAX;
return -ENOSYS;
...
Thanks.
---
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 1/7] tty/serial: Add GPIOLIB helpers for controlling modem lines
@ 2014-02-17 18:37 ` Alexander Shiyan
0 siblings, 0 replies; 40+ messages in thread
From: Alexander Shiyan @ 2014-02-17 18:37 UTC (permalink / raw)
To: linux-arm-kernel
Hello.
A few comments below..
???????????, 17 ??????? 2014, 17:57 +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>
> ---
...
> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
...
> +static const struct {
> + const char *name;
> + unsigned int mctrl;
> + bool dir_out;
> +} mctrl_gpios_desc[] = {
> + { "cts", TIOCM_CTS, false, },
> + { "dsr", TIOCM_DSR, false, },
> + { "dcd", TIOCM_CD, false, },
> + { "ri", TIOCM_RI, false, },
> + { "rts", TIOCM_RTS, true, },
> + { "dtr", TIOCM_DTR, true, },
> + { "out1", TIOCM_OUT1, true, },
> + { "out2", TIOCM_OUT2, true, },
> +};
> +
> +void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
> +{
> + enum mctrl_gpio_idx i;
> +
> + for (i = 0; i < UART_GPIO_MAX; i++)
Use ARRAY_SIZE(mctrl_gpios_desc) here and elsewhere below.
> + if (!IS_ERR_OR_NULL(gpios->gpio[i]) &&
> + mctrl_gpios_desc[i].dir_out)
> + gpiod_set_value(gpios->gpio[i],
> + !!(mctrl & mctrl_gpios_desc[i].mctrl));
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_set);
> +
> +unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
> +{
Why you want to put mctrl as parameter here?
Let's return mctrl from GPIOs, then handle this value as you want int the driver.
> + enum mctrl_gpio_idx i;
> +
> + for (i = 0; i < UART_GPIO_MAX; i++) {
> + if (!IS_ERR_OR_NULL(gpios->gpio[i]) &&
> + !mctrl_gpios_desc[i].dir_out) {
> + if (gpiod_get_value(gpios->gpio[i]))
> + *mctrl |= mctrl_gpios_desc[i].mctrl;
> + else
> + *mctrl &= ~mctrl_gpios_desc[i].mctrl;
> + }
> + }
> +
> + return *mctrl;
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_get);
> +
> +int mctrl_gpio_init(struct device *dev, struct mctrl_gpios *gpios)
> +{
I suggest to allocate "gpios" here and return pointer to this structure
or ERR_PTR(). Additionally, as I mentioned before, add "index" variable
to specify port number.
> + enum mctrl_gpio_idx i;
> + int err = 0;
> + int ret = 0;
> +
> + for (i = 0; i < UART_GPIO_MAX; i++) {
> + gpios->gpio[i] = devm_gpiod_get(dev, mctrl_gpios_desc[i].name);
> +
> + /*
> + * The GPIOs are maybe not all filled,
> + * this is not an error.
> + */
> + if (IS_ERR_OR_NULL(gpios->gpio[i]))
> + continue;
> +
> + if (mctrl_gpios_desc[i].dir_out)
> + err = gpiod_direction_output(gpios->gpio[i], 0);
> + else
> + err = gpiod_direction_input(gpios->gpio[i]);
> + if (err) {
> + dev_err(dev, "Unable to set direction for %s GPIO",
> + mctrl_gpios_desc[i].name);
> + devm_gpiod_put(dev, gpios->gpio[i]);
> + gpios->gpio[i] = NULL;
> + ret--;
> + }
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(mctrl_gpio_init);
...
> diff --git a/drivers/tty/serial/serial_mctrl_gpio.h b/drivers/tty/serial/serial_mctrl_gpio.h
...
> +enum mctrl_gpio_idx {
> + UART_GPIO_CTS,
> + UART_GPIO_DSR,
> + UART_GPIO_DCD,
> + UART_GPIO_RI,
> + UART_GPIO_RTS,
> + UART_GPIO_DTR,
> + UART_GPIO_OUT1,
> + UART_GPIO_OUT2,
> + UART_GPIO_MAX,
> +};
> +
> +struct mctrl_gpios {
> + struct gpio_desc *gpio[UART_GPIO_MAX];
struct gpio_desc *gpio;
...
> +static inline
> +int mctrl_gpio_init(struct device *dev, struct mctrl_gpios *gpios)
> +{
> + return -UART_GPIO_MAX;
return -ENOSYS;
...
Thanks.
---
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 0/7] tty/serial: Add helpers to use GPIOs to control modem lines and implement atmel_serial.c
2014-02-17 17:53 ` Alexander Shiyan
@ 2014-02-18 9:59 ` Richard Genoud
-1 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-18 9:59 UTC (permalink / raw)
To: Alexander Shiyan
Cc: Greg Kroah-Hartman, Uwe Kleine-König, Nicolas Ferre,
Linus Walleij, linux-serial, linux-arm-kernel
On 17/02/2014 18:53, Alexander Shiyan wrote:
> Hello.
>
> Понедельник, 17 февраля 2014, 17:57 +01:00 от Richard Genoud <richard.genoud@gmail.com>:
>> The USART controller on sam9x5 chips (and also all AT91/SAMA5 chips
>> but at91rm9200) are not capable of handling DTR/DSR/DCD/RI signal.
>> Moreover, even if the controller can handle CTS/RTS, the dedicated
>> CTS/RTS pins are already muxed for other peripherals (LCDC/EMAC/MMC).
>>
>> So this patchset adds the possibility to control those lines via GPIO,
>> as it is done for RTS in the patch "switch atmel serial to use gpiolib"
>>
>> As it was suggested by Alexander Shiyan, I made that available for
>> every board.
>>
>> Patch 1 implements the generic helpers to control modem lines via GPIO
>> Patches 2 and 3 are just a little tidy up of atmel_serial.c.
>> Patch 4 implements modem control lines in atmel_serial atmel_serial.
>> Patches 5 and 6 implement the get_direction() gpio call for at91, as
>> it is needed by gpiod_get_direction().
>> Patch 7 implement the interrupts of CTS/DSR/DCD/RI.
>
> I still recommend split this series. The first patch must be a separate,
> 2 and 3 - independent to this series, 5 and 6 - to the appropriate mailing lists,
> and finally as soon as all of the previous will be applied - you can send 7.
Well, I did this because I think:
- it's easier to review a patch when you have an implementation, like
atmel_serial, that follows.
- it's also easier to test atmel_serial patches if the patches it needs
are in the same thread. Testers/reviewers won't have to dig the MLs for
needed patches.
- having all those patches in one thread shows their order, and prevents
from breaking kernel compilation between patches (it's really annoying
when you hunt a bug with git bisect and at some point the kernel doesn't
compile anymore). If the series is split and patches goes in different
trees, nothing will prevent patch 1 to be applied after patch 7 and
break kernel compilation in between (AFAIK).
- maintainers are Cced to this thread, so they can easily speak to
each-other to say if one should take the whole series in its tree or if
it should go to several tree or...
But, I'm not a maintainer, so I may not see all the pros and cons of this...
Richard.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 0/7] tty/serial: Add helpers to use GPIOs to control modem lines and implement atmel_serial.c
@ 2014-02-18 9:59 ` Richard Genoud
0 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-18 9:59 UTC (permalink / raw)
To: linux-arm-kernel
On 17/02/2014 18:53, Alexander Shiyan wrote:
> Hello.
>
> ???????????, 17 ??????? 2014, 17:57 +01:00 ?? Richard Genoud <richard.genoud@gmail.com>:
>> The USART controller on sam9x5 chips (and also all AT91/SAMA5 chips
>> but at91rm9200) are not capable of handling DTR/DSR/DCD/RI signal.
>> Moreover, even if the controller can handle CTS/RTS, the dedicated
>> CTS/RTS pins are already muxed for other peripherals (LCDC/EMAC/MMC).
>>
>> So this patchset adds the possibility to control those lines via GPIO,
>> as it is done for RTS in the patch "switch atmel serial to use gpiolib"
>>
>> As it was suggested by Alexander Shiyan, I made that available for
>> every board.
>>
>> Patch 1 implements the generic helpers to control modem lines via GPIO
>> Patches 2 and 3 are just a little tidy up of atmel_serial.c.
>> Patch 4 implements modem control lines in atmel_serial atmel_serial.
>> Patches 5 and 6 implement the get_direction() gpio call for at91, as
>> it is needed by gpiod_get_direction().
>> Patch 7 implement the interrupts of CTS/DSR/DCD/RI.
>
> I still recommend split this series. The first patch must be a separate,
> 2 and 3 - independent to this series, 5 and 6 - to the appropriate mailing lists,
> and finally as soon as all of the previous will be applied - you can send 7.
Well, I did this because I think:
- it's easier to review a patch when you have an implementation, like
atmel_serial, that follows.
- it's also easier to test atmel_serial patches if the patches it needs
are in the same thread. Testers/reviewers won't have to dig the MLs for
needed patches.
- having all those patches in one thread shows their order, and prevents
from breaking kernel compilation between patches (it's really annoying
when you hunt a bug with git bisect and at some point the kernel doesn't
compile anymore). If the series is split and patches goes in different
trees, nothing will prevent patch 1 to be applied after patch 7 and
break kernel compilation in between (AFAIK).
- maintainers are Cced to this thread, so they can easily speak to
each-other to say if one should take the whole series in its tree or if
it should go to several tree or...
But, I'm not a maintainer, so I may not see all the pros and cons of this...
Richard.
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 1/7] tty/serial: Add GPIOLIB helpers for controlling modem lines
2014-02-17 18:37 ` Alexander Shiyan
@ 2014-02-18 9:59 ` Richard Genoud
-1 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-18 9:59 UTC (permalink / raw)
To: Alexander Shiyan
Cc: Greg Kroah-Hartman, Uwe Kleine-König, Nicolas Ferre,
Linus Walleij, linux-serial, linux-arm-kernel
On 17/02/2014 19:37, Alexander Shiyan wrote:
> Hello.
Hi !
>
> A few comments below..
>
> Понедельник, 17 февраля 2014, 17:57 +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>
>> ---
> ...
>> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
> ...
>> +static const struct {
>> + const char *name;
>> + unsigned int mctrl;
>> + bool dir_out;
>> +} mctrl_gpios_desc[] = {
>> + { "cts", TIOCM_CTS, false, },
>> + { "dsr", TIOCM_DSR, false, },
>> + { "dcd", TIOCM_CD, false, },
>> + { "ri", TIOCM_RI, false, },
>> + { "rts", TIOCM_RTS, true, },
>> + { "dtr", TIOCM_DTR, true, },
>> + { "out1", TIOCM_OUT1, true, },
>> + { "out2", TIOCM_OUT2, true, },
>> +};
>> +
>> +void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
>> +{
>> + enum mctrl_gpio_idx i;
>> +
>> + for (i = 0; i < UART_GPIO_MAX; i++)
>
> Use ARRAY_SIZE(mctrl_gpios_desc) here and elsewhere below.
Could you explain why ARRAY_SIZE(mctrl_gpios_desc) seems better than
UART_GPIO_MAX ?
>
>> + if (!IS_ERR_OR_NULL(gpios->gpio[i]) &&
>> + mctrl_gpios_desc[i].dir_out)
>> + gpiod_set_value(gpios->gpio[i],
>> + !!(mctrl & mctrl_gpios_desc[i].mctrl));
>> +}
>> +EXPORT_SYMBOL_GPL(mctrl_gpio_set);
>> +
>> +unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
>> +{
>
> Why you want to put mctrl as parameter here?
> Let's return mctrl from GPIOs, then handle this value as you want int the driver.
It's because I like when it's simple :).
Use case:
Your USART controller handles CTS/RTS, and you add DTR/DSR as gpios.
In get_mctrl() callback, with current implementation, you'll have
something like this: (cf atmel_get_mctrl() for a real life example)
{
unsigned int mctrl;
mctrl = usart_controller_get_mctrl(); /* driver specific */
return mctrl_gpio_get(gpios, &mctrl);
}
If I use as you suggest mctrl_gpio_get(struct mctrl_gpios *gpios), we'll
have:
{
unsigned int usart_mctrl;
unsigned int gpio_mctrl;
int i;
usart_mctrl = usart_controller_get_mctrl();
gpio_mctrl = mctrl_gpio_get(gpios);
if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_CTS]) {
if (gpio_mctrl & TIOCM_CTS)
usart_mctrl |= TIOCM_CTS;
else
usart_mctrl &= ~TIOCM_CTS;
}
if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_DSR]) {
if (gpio_mctrl & TIOCM_DSR)
usart_mctrl |= TIOCM_DSR;
else
usart_mctrl &= ~TIOCM_DSR;
}
etc...
}
Because when gpio_mctrl is returned, I don't know if a flag is not set
because a gpio is at 1 or because the gpio has not been requested.
In the later case, the value should not override the value retrieved by
the controller.
another solution would be to use a mask filled at startup:
init_gpios(...)
{
unsigned int driver_port_gpio_input_mask = 0;
mctrl_gpio_init(dev, &p->gpios);
if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_CTS])
driver_port_gpio_mask |= TIOCM_CTS;
if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_DSR])
driver_port_gpio_mask |= TIOCM_DSR;
if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_DCD])
driver_port_gpio_mask |= TIOCM_DCD;
if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_RI])
driver_port_gpio_mask |= TIOCM_RI;
}
and then, in get_mctrl():
{
unsigned int mctrl;
int i;
mctrl = usart_controller_get_mctrl();
mctrl &= ~driver_port_gpio_input_mask;
mctrl |= mctrl_gpio_get(gpios);
return mctrl;
}
But both solutions seems to me more complicated than the first one.
>> + enum mctrl_gpio_idx i;
>> +
>> + for (i = 0; i < UART_GPIO_MAX; i++) {
>> + if (!IS_ERR_OR_NULL(gpios->gpio[i]) &&
>> + !mctrl_gpios_desc[i].dir_out) {
>> + if (gpiod_get_value(gpios->gpio[i]))
>> + *mctrl |= mctrl_gpios_desc[i].mctrl;
>> + else
>> + *mctrl &= ~mctrl_gpios_desc[i].mctrl;
>> + }
>> + }
>> +
>> + return *mctrl;
>> +}
>> +EXPORT_SYMBOL_GPL(mctrl_gpio_get);
>> +
>
>> +int mctrl_gpio_init(struct device *dev, struct mctrl_gpios *gpios)
>> +{
>
> I suggest to allocate "gpios" here and return pointer to this structure
> or ERR_PTR(). Additionally, as I mentioned before, add "index" variable
> to specify port number.
I don't understand the benefit of dynamically allocating something that
has a fixed size...
Or maybe in the case no GPIO are used, the array is not allocated, and
I'll have to add "if (!gpios)" test in other functions. That could save
some bytes.
Could you explain a little more your idea of ""index" variable to
specify port number" ?
I'm not sure to get it.
>> + enum mctrl_gpio_idx i;
>> + int err = 0;
>> + int ret = 0;
>> +
>> + for (i = 0; i < UART_GPIO_MAX; i++) {
>> + gpios->gpio[i] = devm_gpiod_get(dev, mctrl_gpios_desc[i].name);
>> +
>> + /*
>> + * The GPIOs are maybe not all filled,
>> + * this is not an error.
>> + */
>> + if (IS_ERR_OR_NULL(gpios->gpio[i]))
>> + continue;
>> +
>> + if (mctrl_gpios_desc[i].dir_out)
>> + err = gpiod_direction_output(gpios->gpio[i], 0);
>> + else
>> + err = gpiod_direction_input(gpios->gpio[i]);
>> + if (err) {
>> + dev_err(dev, "Unable to set direction for %s GPIO",
>> + mctrl_gpios_desc[i].name);
>> + devm_gpiod_put(dev, gpios->gpio[i]);
>> + gpios->gpio[i] = NULL;
>> + ret--;
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(mctrl_gpio_init);
> ...
>> diff --git a/drivers/tty/serial/serial_mctrl_gpio.h b/drivers/tty/serial/serial_mctrl_gpio.h
> ...
>> +enum mctrl_gpio_idx {
>> + UART_GPIO_CTS,
>> + UART_GPIO_DSR,
>> + UART_GPIO_DCD,
>> + UART_GPIO_RI,
>> + UART_GPIO_RTS,
>> + UART_GPIO_DTR,
>> + UART_GPIO_OUT1,
>> + UART_GPIO_OUT2,
>> + UART_GPIO_MAX,
>> +};
>> +
>> +struct mctrl_gpios {
>> + struct gpio_desc *gpio[UART_GPIO_MAX];
>
> struct gpio_desc *gpio;
>
> ...
>> +static inline
>> +int mctrl_gpio_init(struct device *dev, struct mctrl_gpios *gpios)
>> +{
>> + return -UART_GPIO_MAX;
>
> return -ENOSYS;
yes, that's right.
>
> ...
>
> Thanks.
Thanks for your review !
Richard.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 1/7] tty/serial: Add GPIOLIB helpers for controlling modem lines
@ 2014-02-18 9:59 ` Richard Genoud
0 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-18 9:59 UTC (permalink / raw)
To: linux-arm-kernel
On 17/02/2014 19:37, Alexander Shiyan wrote:
> Hello.
Hi !
>
> A few comments below..
>
> ???????????, 17 ??????? 2014, 17:57 +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>
>> ---
> ...
>> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
> ...
>> +static const struct {
>> + const char *name;
>> + unsigned int mctrl;
>> + bool dir_out;
>> +} mctrl_gpios_desc[] = {
>> + { "cts", TIOCM_CTS, false, },
>> + { "dsr", TIOCM_DSR, false, },
>> + { "dcd", TIOCM_CD, false, },
>> + { "ri", TIOCM_RI, false, },
>> + { "rts", TIOCM_RTS, true, },
>> + { "dtr", TIOCM_DTR, true, },
>> + { "out1", TIOCM_OUT1, true, },
>> + { "out2", TIOCM_OUT2, true, },
>> +};
>> +
>> +void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
>> +{
>> + enum mctrl_gpio_idx i;
>> +
>> + for (i = 0; i < UART_GPIO_MAX; i++)
>
> Use ARRAY_SIZE(mctrl_gpios_desc) here and elsewhere below.
Could you explain why ARRAY_SIZE(mctrl_gpios_desc) seems better than
UART_GPIO_MAX ?
>
>> + if (!IS_ERR_OR_NULL(gpios->gpio[i]) &&
>> + mctrl_gpios_desc[i].dir_out)
>> + gpiod_set_value(gpios->gpio[i],
>> + !!(mctrl & mctrl_gpios_desc[i].mctrl));
>> +}
>> +EXPORT_SYMBOL_GPL(mctrl_gpio_set);
>> +
>> +unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
>> +{
>
> Why you want to put mctrl as parameter here?
> Let's return mctrl from GPIOs, then handle this value as you want int the driver.
It's because I like when it's simple :).
Use case:
Your USART controller handles CTS/RTS, and you add DTR/DSR as gpios.
In get_mctrl() callback, with current implementation, you'll have
something like this: (cf atmel_get_mctrl() for a real life example)
{
unsigned int mctrl;
mctrl = usart_controller_get_mctrl(); /* driver specific */
return mctrl_gpio_get(gpios, &mctrl);
}
If I use as you suggest mctrl_gpio_get(struct mctrl_gpios *gpios), we'll
have:
{
unsigned int usart_mctrl;
unsigned int gpio_mctrl;
int i;
usart_mctrl = usart_controller_get_mctrl();
gpio_mctrl = mctrl_gpio_get(gpios);
if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_CTS]) {
if (gpio_mctrl & TIOCM_CTS)
usart_mctrl |= TIOCM_CTS;
else
usart_mctrl &= ~TIOCM_CTS;
}
if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_DSR]) {
if (gpio_mctrl & TIOCM_DSR)
usart_mctrl |= TIOCM_DSR;
else
usart_mctrl &= ~TIOCM_DSR;
}
etc...
}
Because when gpio_mctrl is returned, I don't know if a flag is not set
because a gpio is at 1 or because the gpio has not been requested.
In the later case, the value should not override the value retrieved by
the controller.
another solution would be to use a mask filled at startup:
init_gpios(...)
{
unsigned int driver_port_gpio_input_mask = 0;
mctrl_gpio_init(dev, &p->gpios);
if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_CTS])
driver_port_gpio_mask |= TIOCM_CTS;
if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_DSR])
driver_port_gpio_mask |= TIOCM_DSR;
if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_DCD])
driver_port_gpio_mask |= TIOCM_DCD;
if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_RI])
driver_port_gpio_mask |= TIOCM_RI;
}
and then, in get_mctrl():
{
unsigned int mctrl;
int i;
mctrl = usart_controller_get_mctrl();
mctrl &= ~driver_port_gpio_input_mask;
mctrl |= mctrl_gpio_get(gpios);
return mctrl;
}
But both solutions seems to me more complicated than the first one.
>> + enum mctrl_gpio_idx i;
>> +
>> + for (i = 0; i < UART_GPIO_MAX; i++) {
>> + if (!IS_ERR_OR_NULL(gpios->gpio[i]) &&
>> + !mctrl_gpios_desc[i].dir_out) {
>> + if (gpiod_get_value(gpios->gpio[i]))
>> + *mctrl |= mctrl_gpios_desc[i].mctrl;
>> + else
>> + *mctrl &= ~mctrl_gpios_desc[i].mctrl;
>> + }
>> + }
>> +
>> + return *mctrl;
>> +}
>> +EXPORT_SYMBOL_GPL(mctrl_gpio_get);
>> +
>
>> +int mctrl_gpio_init(struct device *dev, struct mctrl_gpios *gpios)
>> +{
>
> I suggest to allocate "gpios" here and return pointer to this structure
> or ERR_PTR(). Additionally, as I mentioned before, add "index" variable
> to specify port number.
I don't understand the benefit of dynamically allocating something that
has a fixed size...
Or maybe in the case no GPIO are used, the array is not allocated, and
I'll have to add "if (!gpios)" test in other functions. That could save
some bytes.
Could you explain a little more your idea of ""index" variable to
specify port number" ?
I'm not sure to get it.
>> + enum mctrl_gpio_idx i;
>> + int err = 0;
>> + int ret = 0;
>> +
>> + for (i = 0; i < UART_GPIO_MAX; i++) {
>> + gpios->gpio[i] = devm_gpiod_get(dev, mctrl_gpios_desc[i].name);
>> +
>> + /*
>> + * The GPIOs are maybe not all filled,
>> + * this is not an error.
>> + */
>> + if (IS_ERR_OR_NULL(gpios->gpio[i]))
>> + continue;
>> +
>> + if (mctrl_gpios_desc[i].dir_out)
>> + err = gpiod_direction_output(gpios->gpio[i], 0);
>> + else
>> + err = gpiod_direction_input(gpios->gpio[i]);
>> + if (err) {
>> + dev_err(dev, "Unable to set direction for %s GPIO",
>> + mctrl_gpios_desc[i].name);
>> + devm_gpiod_put(dev, gpios->gpio[i]);
>> + gpios->gpio[i] = NULL;
>> + ret--;
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(mctrl_gpio_init);
> ...
>> diff --git a/drivers/tty/serial/serial_mctrl_gpio.h b/drivers/tty/serial/serial_mctrl_gpio.h
> ...
>> +enum mctrl_gpio_idx {
>> + UART_GPIO_CTS,
>> + UART_GPIO_DSR,
>> + UART_GPIO_DCD,
>> + UART_GPIO_RI,
>> + UART_GPIO_RTS,
>> + UART_GPIO_DTR,
>> + UART_GPIO_OUT1,
>> + UART_GPIO_OUT2,
>> + UART_GPIO_MAX,
>> +};
>> +
>> +struct mctrl_gpios {
>> + struct gpio_desc *gpio[UART_GPIO_MAX];
>
> struct gpio_desc *gpio;
>
> ...
>> +static inline
>> +int mctrl_gpio_init(struct device *dev, struct mctrl_gpios *gpios)
>> +{
>> + return -UART_GPIO_MAX;
>
> return -ENOSYS;
yes, that's right.
>
> ...
>
> Thanks.
Thanks for your review !
Richard.
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 4/7] tty/serial: at91: use mctrl_gpio helpers
2014-02-17 16:57 ` Richard Genoud
@ 2014-02-18 15:04 ` Alexander Shiyan
-1 siblings, 0 replies; 40+ messages in thread
From: Alexander Shiyan @ 2014-02-18 15:04 UTC (permalink / raw)
To: Richard Genoud
Cc: Greg Kroah-Hartman, Uwe Kleine-König, Nicolas Ferre,
Linus Walleij, linux-serial, linux-arm-kernel
On Mon, 17 Feb 2014 17:57:24 +0100
Richard Genoud <richard.genoud@gmail.com> wrote:
> On sam9x5, dedicated CTS (and RTS) pins are unusable together with the
> LCDC, the EMAC, or the MMC because they share the same line.
>
> Moreover, the USART controller doesn't handle DTR/DSR/DCD/RI signals,
> so we have to control them via GPIO.
>
> This patch permits to use GPIOs to control the CTS/RTS/DTR/DSR/DCD/RI
> signals.
>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
...
> diff --git a/arch/arm/mach-at91/at91rm9200_devices.c b/arch/arm/mach-at91/at91rm9200_devices.c
> index 605add05af7e..6dd386e3d9fe 100644
> --- a/arch/arm/mach-at91/at91rm9200_devices.c
> +++ b/arch/arm/mach-at91/at91rm9200_devices.c
...
> @@ -961,7 +961,14 @@ static struct resource uart0_resources[] = {
> static struct atmel_uart_data uart0_data = {
> .use_dma_tx = 1,
> .use_dma_rx = 1,
> - .rts_gpio = -EINVAL,
> +};
> +
> +struct gpiod_lookup_table uart0_gpios_table = {
static
> + .dev_id = "atmel_usart",
> + .table = {
> + GPIO_LOOKUP("pioA", 21, "rts", GPIO_ACTIVE_LOW),
> + { },
> + },
> };
...
--
Alexander Shiyan <shc_work@mail.ru>
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 4/7] tty/serial: at91: use mctrl_gpio helpers
@ 2014-02-18 15:04 ` Alexander Shiyan
0 siblings, 0 replies; 40+ messages in thread
From: Alexander Shiyan @ 2014-02-18 15:04 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 17 Feb 2014 17:57:24 +0100
Richard Genoud <richard.genoud@gmail.com> wrote:
> On sam9x5, dedicated CTS (and RTS) pins are unusable together with the
> LCDC, the EMAC, or the MMC because they share the same line.
>
> Moreover, the USART controller doesn't handle DTR/DSR/DCD/RI signals,
> so we have to control them via GPIO.
>
> This patch permits to use GPIOs to control the CTS/RTS/DTR/DSR/DCD/RI
> signals.
>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
...
> diff --git a/arch/arm/mach-at91/at91rm9200_devices.c b/arch/arm/mach-at91/at91rm9200_devices.c
> index 605add05af7e..6dd386e3d9fe 100644
> --- a/arch/arm/mach-at91/at91rm9200_devices.c
> +++ b/arch/arm/mach-at91/at91rm9200_devices.c
...
> @@ -961,7 +961,14 @@ static struct resource uart0_resources[] = {
> static struct atmel_uart_data uart0_data = {
> .use_dma_tx = 1,
> .use_dma_rx = 1,
> - .rts_gpio = -EINVAL,
> +};
> +
> +struct gpiod_lookup_table uart0_gpios_table = {
static
> + .dev_id = "atmel_usart",
> + .table = {
> + GPIO_LOOKUP("pioA", 21, "rts", GPIO_ACTIVE_LOW),
> + { },
> + },
> };
...
--
Alexander Shiyan <shc_work@mail.ru>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 4/7] tty/serial: at91: use mctrl_gpio helpers
2014-02-18 15:04 ` Alexander Shiyan
@ 2014-02-18 15:09 ` Richard Genoud
-1 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-18 15:09 UTC (permalink / raw)
To: Alexander Shiyan
Cc: Greg Kroah-Hartman, Uwe Kleine-König, Nicolas Ferre,
Linus Walleij, linux-serial, linux-arm-kernel
2014-02-18 16:04 GMT+01:00 Alexander Shiyan <shc_work@mail.ru>:
> On Mon, 17 Feb 2014 17:57:24 +0100
> Richard Genoud <richard.genoud@gmail.com> wrote:
>
>> On sam9x5, dedicated CTS (and RTS) pins are unusable together with the
>> LCDC, the EMAC, or the MMC because they share the same line.
>>
>> Moreover, the USART controller doesn't handle DTR/DSR/DCD/RI signals,
>> so we have to control them via GPIO.
>>
>> This patch permits to use GPIOs to control the CTS/RTS/DTR/DSR/DCD/RI
>> signals.
>>
>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> ...
>> diff --git a/arch/arm/mach-at91/at91rm9200_devices.c b/arch/arm/mach-at91/at91rm9200_devices.c
>> index 605add05af7e..6dd386e3d9fe 100644
>> --- a/arch/arm/mach-at91/at91rm9200_devices.c
>> +++ b/arch/arm/mach-at91/at91rm9200_devices.c
> ...
>> @@ -961,7 +961,14 @@ static struct resource uart0_resources[] = {
>> static struct atmel_uart_data uart0_data = {
>> .use_dma_tx = 1,
>> .use_dma_rx = 1,
>> - .rts_gpio = -EINVAL,
>> +};
>> +
>> +struct gpiod_lookup_table uart0_gpios_table = {
>
> static
arg ! I missed this one !
good catch !
>> + .dev_id = "atmel_usart",
>> + .table = {
>> + GPIO_LOOKUP("pioA", 21, "rts", GPIO_ACTIVE_LOW),
>> + { },
>> + },
>> };
> ...
>
> --
> Alexander Shiyan <shc_work@mail.ru>
--
for me, ck means con kolivas and not calvin klein... does it mean I'm a geek ?
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 4/7] tty/serial: at91: use mctrl_gpio helpers
@ 2014-02-18 15:09 ` Richard Genoud
0 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-18 15:09 UTC (permalink / raw)
To: linux-arm-kernel
2014-02-18 16:04 GMT+01:00 Alexander Shiyan <shc_work@mail.ru>:
> On Mon, 17 Feb 2014 17:57:24 +0100
> Richard Genoud <richard.genoud@gmail.com> wrote:
>
>> On sam9x5, dedicated CTS (and RTS) pins are unusable together with the
>> LCDC, the EMAC, or the MMC because they share the same line.
>>
>> Moreover, the USART controller doesn't handle DTR/DSR/DCD/RI signals,
>> so we have to control them via GPIO.
>>
>> This patch permits to use GPIOs to control the CTS/RTS/DTR/DSR/DCD/RI
>> signals.
>>
>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> ...
>> diff --git a/arch/arm/mach-at91/at91rm9200_devices.c b/arch/arm/mach-at91/at91rm9200_devices.c
>> index 605add05af7e..6dd386e3d9fe 100644
>> --- a/arch/arm/mach-at91/at91rm9200_devices.c
>> +++ b/arch/arm/mach-at91/at91rm9200_devices.c
> ...
>> @@ -961,7 +961,14 @@ static struct resource uart0_resources[] = {
>> static struct atmel_uart_data uart0_data = {
>> .use_dma_tx = 1,
>> .use_dma_rx = 1,
>> - .rts_gpio = -EINVAL,
>> +};
>> +
>> +struct gpiod_lookup_table uart0_gpios_table = {
>
> static
arg ! I missed this one !
good catch !
>> + .dev_id = "atmel_usart",
>> + .table = {
>> + GPIO_LOOKUP("pioA", 21, "rts", GPIO_ACTIVE_LOW),
>> + { },
>> + },
>> };
> ...
>
> --
> Alexander Shiyan <shc_work@mail.ru>
--
for me, ck means con kolivas and not calvin klein... does it mean I'm a geek ?
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 1/7] tty/serial: Add GPIOLIB helpers for controlling modem lines
2014-02-18 9:59 ` Richard Genoud
@ 2014-02-18 15:26 ` Alexander Shiyan
-1 siblings, 0 replies; 40+ messages in thread
From: Alexander Shiyan @ 2014-02-18 15:26 UTC (permalink / raw)
To: Richard Genoud
Cc: Greg Kroah-Hartman, Uwe Kleine-König, Nicolas Ferre,
Linus Walleij, linux-serial, linux-arm-kernel
On Tue, 18 Feb 2014 10:59:56 +0100
Richard Genoud <richard.genoud@gmail.com> wrote:
> On 17/02/2014 19:37, Alexander Shiyan wrote:
> > Hello.
> Hi !
> >
> > A few comments below..
> >
> > Понедельник, 17 февраля 2014, 17:57 +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>
> >> ---
> > ...
> >> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
> > ...
> >> +static const struct {
> >> + const char *name;
> >> + unsigned int mctrl;
> >> + bool dir_out;
> >> +} mctrl_gpios_desc[] = {
> >> + { "cts", TIOCM_CTS, false, },
> >> + { "dsr", TIOCM_DSR, false, },
> >> + { "dcd", TIOCM_CD, false, },
> >> + { "ri", TIOCM_RI, false, },
> >> + { "rts", TIOCM_RTS, true, },
> >> + { "dtr", TIOCM_DTR, true, },
> >> + { "out1", TIOCM_OUT1, true, },
> >> + { "out2", TIOCM_OUT2, true, },
> >> +};
> >> +
> >> +void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
> >> +{
> >> + enum mctrl_gpio_idx i;
> >> +
> >> + for (i = 0; i < UART_GPIO_MAX; i++)
> >
> > Use ARRAY_SIZE(mctrl_gpios_desc) here and elsewhere below.
> Could you explain why ARRAY_SIZE(mctrl_gpios_desc) seems better than
> UART_GPIO_MAX ?
Because you iterate through the array.
I do not see the use of UART_GPIO_MAX in this module. You use UART_GPIO_MAX
in the at91 serial driver only, here we must be sure that we are within the
boundaries of the array.
...
> >> +unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
> >> +{
> >
> > Why you want to put mctrl as parameter here?
> > Let's return mctrl from GPIOs, then handle this value as you want int the driver.
> It's because I like when it's simple :).
> Use case:
> Your USART controller handles CTS/RTS, and you add DTR/DSR as gpios.
> In get_mctrl() callback, with current implementation, you'll have
> something like this: (cf atmel_get_mctrl() for a real life example)
> {
> unsigned int mctrl;
> mctrl = usart_controller_get_mctrl(); /* driver specific */
> return mctrl_gpio_get(gpios, &mctrl);
> }
> If I use as you suggest mctrl_gpio_get(struct mctrl_gpios *gpios), we'll
> have:
> {
> unsigned int usart_mctrl;
> unsigned int gpio_mctrl;
> int i;
>
> usart_mctrl = usart_controller_get_mctrl();
> gpio_mctrl = mctrl_gpio_get(gpios);
> if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_CTS]) {
> if (gpio_mctrl & TIOCM_CTS)
> usart_mctrl |= TIOCM_CTS;
> else
> usart_mctrl &= ~TIOCM_CTS;
> }
> if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_DSR]) {
> if (gpio_mctrl & TIOCM_DSR)
> usart_mctrl |= TIOCM_DSR;
> else
> usart_mctrl &= ~TIOCM_DSR;
> }
No, just like this:
{
unsigned int mctrl = usart_controller_get_mctrl(); /* driver specific */
mctrl |= mctrl_gpio_get(gpios, mctrl) & ~(TIOCM_CTS | TIOCM_DSR);
return mctrl;
}
or in reverse order:
{
unsigned int mctrl = mctrl_gpio_get(gpios, mctrl);
return mctrl | usart_controller_get_mctrl(); /* driver specific */
}
...
> >> +int mctrl_gpio_init(struct device *dev, struct mctrl_gpios *gpios)
> >> +{
> >
> > I suggest to allocate "gpios" here and return pointer to this structure
> > or ERR_PTR(). Additionally, as I mentioned before, add "index" variable
> > to specify port number.
>
> I don't understand the benefit of dynamically allocating something that
> has a fixed size...
> Or maybe in the case no GPIO are used, the array is not allocated, and
> I'll have to add "if (!gpios)" test in other functions. That could save
> some bytes.
Yes, but basically this able to use just a pointer in your driver data,
this will not depend on GPIOLIB, since without GPIOLIB we do not know storage
size of struct gpio_desc.
> Could you explain a little more your idea of ""index" variable to
> specify port number" ?
> I'm not sure to get it.
Index can be used for drivers which allocate more than one port for one device.
In your implementation you should just replace devm_gpiod_get() to
devm_gpiod_get_index() and add an additional parameter to mctrl_gpio_init().
For at91 serial this parameter should be 0.
>
> >> + enum mctrl_gpio_idx i;
> >> + int err = 0;
> >> + int ret = 0;
> >> +
> >> + for (i = 0; i < UART_GPIO_MAX; i++) {
> >> + gpios->gpio[i] = devm_gpiod_get(dev, mctrl_gpios_desc[i].name);
...
--
Alexander Shiyan <shc_work@mail.ru>
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 1/7] tty/serial: Add GPIOLIB helpers for controlling modem lines
@ 2014-02-18 15:26 ` Alexander Shiyan
0 siblings, 0 replies; 40+ messages in thread
From: Alexander Shiyan @ 2014-02-18 15:26 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 18 Feb 2014 10:59:56 +0100
Richard Genoud <richard.genoud@gmail.com> wrote:
> On 17/02/2014 19:37, Alexander Shiyan wrote:
> > Hello.
> Hi !
> >
> > A few comments below..
> >
> > ???????????, 17 ??????? 2014, 17:57 +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>
> >> ---
> > ...
> >> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
> > ...
> >> +static const struct {
> >> + const char *name;
> >> + unsigned int mctrl;
> >> + bool dir_out;
> >> +} mctrl_gpios_desc[] = {
> >> + { "cts", TIOCM_CTS, false, },
> >> + { "dsr", TIOCM_DSR, false, },
> >> + { "dcd", TIOCM_CD, false, },
> >> + { "ri", TIOCM_RI, false, },
> >> + { "rts", TIOCM_RTS, true, },
> >> + { "dtr", TIOCM_DTR, true, },
> >> + { "out1", TIOCM_OUT1, true, },
> >> + { "out2", TIOCM_OUT2, true, },
> >> +};
> >> +
> >> +void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
> >> +{
> >> + enum mctrl_gpio_idx i;
> >> +
> >> + for (i = 0; i < UART_GPIO_MAX; i++)
> >
> > Use ARRAY_SIZE(mctrl_gpios_desc) here and elsewhere below.
> Could you explain why ARRAY_SIZE(mctrl_gpios_desc) seems better than
> UART_GPIO_MAX ?
Because you iterate through the array.
I do not see the use of UART_GPIO_MAX in this module. You use UART_GPIO_MAX
in the at91 serial driver only, here we must be sure that we are within the
boundaries of the array.
...
> >> +unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
> >> +{
> >
> > Why you want to put mctrl as parameter here?
> > Let's return mctrl from GPIOs, then handle this value as you want int the driver.
> It's because I like when it's simple :).
> Use case:
> Your USART controller handles CTS/RTS, and you add DTR/DSR as gpios.
> In get_mctrl() callback, with current implementation, you'll have
> something like this: (cf atmel_get_mctrl() for a real life example)
> {
> unsigned int mctrl;
> mctrl = usart_controller_get_mctrl(); /* driver specific */
> return mctrl_gpio_get(gpios, &mctrl);
> }
> If I use as you suggest mctrl_gpio_get(struct mctrl_gpios *gpios), we'll
> have:
> {
> unsigned int usart_mctrl;
> unsigned int gpio_mctrl;
> int i;
>
> usart_mctrl = usart_controller_get_mctrl();
> gpio_mctrl = mctrl_gpio_get(gpios);
> if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_CTS]) {
> if (gpio_mctrl & TIOCM_CTS)
> usart_mctrl |= TIOCM_CTS;
> else
> usart_mctrl &= ~TIOCM_CTS;
> }
> if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_DSR]) {
> if (gpio_mctrl & TIOCM_DSR)
> usart_mctrl |= TIOCM_DSR;
> else
> usart_mctrl &= ~TIOCM_DSR;
> }
No, just like this:
{
unsigned int mctrl = usart_controller_get_mctrl(); /* driver specific */
mctrl |= mctrl_gpio_get(gpios, mctrl) & ~(TIOCM_CTS | TIOCM_DSR);
return mctrl;
}
or in reverse order:
{
unsigned int mctrl = mctrl_gpio_get(gpios, mctrl);
return mctrl | usart_controller_get_mctrl(); /* driver specific */
}
...
> >> +int mctrl_gpio_init(struct device *dev, struct mctrl_gpios *gpios)
> >> +{
> >
> > I suggest to allocate "gpios" here and return pointer to this structure
> > or ERR_PTR(). Additionally, as I mentioned before, add "index" variable
> > to specify port number.
>
> I don't understand the benefit of dynamically allocating something that
> has a fixed size...
> Or maybe in the case no GPIO are used, the array is not allocated, and
> I'll have to add "if (!gpios)" test in other functions. That could save
> some bytes.
Yes, but basically this able to use just a pointer in your driver data,
this will not depend on GPIOLIB, since without GPIOLIB we do not know storage
size of struct gpio_desc.
> Could you explain a little more your idea of ""index" variable to
> specify port number" ?
> I'm not sure to get it.
Index can be used for drivers which allocate more than one port for one device.
In your implementation you should just replace devm_gpiod_get() to
devm_gpiod_get_index() and add an additional parameter to mctrl_gpio_init().
For at91 serial this parameter should be 0.
>
> >> + enum mctrl_gpio_idx i;
> >> + int err = 0;
> >> + int ret = 0;
> >> +
> >> + for (i = 0; i < UART_GPIO_MAX; i++) {
> >> + gpios->gpio[i] = devm_gpiod_get(dev, mctrl_gpios_desc[i].name);
...
--
Alexander Shiyan <shc_work@mail.ru>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 1/7] tty/serial: Add GPIOLIB helpers for controlling modem lines
2014-02-18 15:26 ` Alexander Shiyan
@ 2014-02-20 11:20 ` Richard Genoud
-1 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-20 11:20 UTC (permalink / raw)
To: Alexander Shiyan
Cc: Greg Kroah-Hartman, Uwe Kleine-König, Nicolas Ferre,
Linus Walleij, linux-serial, linux-arm-kernel
On 18/02/2014 16:26, Alexander Shiyan wrote:
> On Tue, 18 Feb 2014 10:59:56 +0100
> Richard Genoud <richard.genoud@gmail.com> wrote:
>
>> On 17/02/2014 19:37, Alexander Shiyan wrote:
>>> Hello.
>> Hi !
>>>
>>> A few comments below..
>>>
>>> Понедельник, 17 февраля 2014, 17:57 +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>
>>>> ---
>>> ...
>>>> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
>>> ...
>>>> +static const struct {
>>>> + const char *name;
>>>> + unsigned int mctrl;
>>>> + bool dir_out;
>>>> +} mctrl_gpios_desc[] = {
>>>> + { "cts", TIOCM_CTS, false, },
>>>> + { "dsr", TIOCM_DSR, false, },
>>>> + { "dcd", TIOCM_CD, false, },
>>>> + { "ri", TIOCM_RI, false, },
>>>> + { "rts", TIOCM_RTS, true, },
>>>> + { "dtr", TIOCM_DTR, true, },
>>>> + { "out1", TIOCM_OUT1, true, },
>>>> + { "out2", TIOCM_OUT2, true, },
>>>> +};
>>>> +
>>>> +void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
>>>> +{
>>>> + enum mctrl_gpio_idx i;
>>>> +
>>>> + for (i = 0; i < UART_GPIO_MAX; i++)
>>>
>>> Use ARRAY_SIZE(mctrl_gpios_desc) here and elsewhere below.
>> Could you explain why ARRAY_SIZE(mctrl_gpios_desc) seems better than
>> UART_GPIO_MAX ?
>
> Because you iterate through the array.
> I do not see the use of UART_GPIO_MAX in this module. You use UART_GPIO_MAX
> in the at91 serial driver only, here we must be sure that we are within the
> boundaries of the array.
Well, the loop iterates through the gpios->gpio[] array which size is
UART_GPIO_MAX, and uses also the mctrl_gpios_desc[] which size is implicit.
Anyway, if you absolutely want this change, I'll do it.
(Or I could also force the length of mctrl_gpios_desc[] like that:
static const struct {
const char *name;
unsigned int mctrl;
bool dir_out;
} mctrl_gpios_desc[UART_GPIO_MAX] = {
{ "cts", TIOCM_CTS, false, },
{ "dsr", TIOCM_DSR, false, },
{ "dcd", TIOCM_CD, false, },
{ "ri", TIOCM_RI, false, },
{ "rts", TIOCM_RTS, true, },
{ "dtr", TIOCM_DTR, true, },
{ "out1", TIOCM_OUT1, true, },
{ "out2", TIOCM_OUT2, true, },
};
This way, all loops will be correct. )
> ...
>>>> +unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
>>>> +{
>>>
>>> Why you want to put mctrl as parameter here?
>>> Let's return mctrl from GPIOs, then handle this value as you want int the driver.
>> It's because I like when it's simple :).
>> Use case:
>> Your USART controller handles CTS/RTS, and you add DTR/DSR as gpios.
>> In get_mctrl() callback, with current implementation, you'll have
>> something like this: (cf atmel_get_mctrl() for a real life example)
>> {
>> unsigned int mctrl;
>> mctrl = usart_controller_get_mctrl(); /* driver specific */
>> return mctrl_gpio_get(gpios, &mctrl);
>> }
>> If I use as you suggest mctrl_gpio_get(struct mctrl_gpios *gpios), we'll
>> have:
>> {
>> unsigned int usart_mctrl;
>> unsigned int gpio_mctrl;
>> int i;
>>
>> usart_mctrl = usart_controller_get_mctrl();
>> gpio_mctrl = mctrl_gpio_get(gpios);
>> if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_CTS]) {
>> if (gpio_mctrl & TIOCM_CTS)
>> usart_mctrl |= TIOCM_CTS;
>> else
>> usart_mctrl &= ~TIOCM_CTS;
>> }
>> if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_DSR]) {
>> if (gpio_mctrl & TIOCM_DSR)
>> usart_mctrl |= TIOCM_DSR;
>> else
>> usart_mctrl &= ~TIOCM_DSR;
>> }
>
> No, just like this:
> {
> unsigned int mctrl = usart_controller_get_mctrl(); /* driver specific */
> mctrl |= mctrl_gpio_get(gpios, mctrl) & ~(TIOCM_CTS | TIOCM_DSR);
> return mctrl;
> }
Hum, I think you made a mistake here.
The "mctrl_gpio_get(gpios, mctrl) & ~(TIOCM_CTS | TIOCM_DSR)" doesn't make
any sense. Or I don't understand what you want to do...
>
> or in reverse order:
> {
> unsigned int mctrl = mctrl_gpio_get(gpios, mctrl);
> return mctrl | usart_controller_get_mctrl(); /* driver specific */
> }
Ok, I didn't explain everything on that:
In atmel_serial (at least), the function usart_controller_get_mctrl()
is in fact a read on a register (Channel Status Register) + conversion
from the register flags to TIOCM_* flags.
That's pretty classical.
BUT when, for example CTS, is muxed as a GPIO and not as the peripheral
function, the value read for the register is undefined (I've seen it
changed randomly). (and it's not very surprising)
So, the value(s) of declared GPIO have to supersede the value(s)
retrieved from the controller.
Thus, a simple OR won't do the trick.
If you really think that mctrl_gpio_get() as it is, is over-complicated,
I could do something like:
static inline unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios) {
int mctrl = 0;
return mctrl_gpio_update(gpios, &mctrl);
}
unsigned int mctrl_gpio_update(struct mctrl_gpios *gpios, unsigned int *mctrl)
{
/* like the old mctrl_gpio_get() */
}
But is it really worth it ?
>
> ...
>>>> +int mctrl_gpio_init(struct device *dev, struct mctrl_gpios *gpios)
>>>> +{
>>>
>>> I suggest to allocate "gpios" here and return pointer to this structure
>>> or ERR_PTR(). Additionally, as I mentioned before, add "index" variable
>>> to specify port number.
>>
>> I don't understand the benefit of dynamically allocating something that
>> has a fixed size...
>> Or maybe in the case no GPIO are used, the array is not allocated, and
>> I'll have to add "if (!gpios)" test in other functions. That could save
>> some bytes.
>
> Yes, but basically this able to use just a pointer in your driver data,
> this will not depend on GPIOLIB, since without GPIOLIB we do not know storage
> size of struct gpio_desc.
Ok, convinced.
>
>> Could you explain a little more your idea of ""index" variable to
>> specify port number" ?
>> I'm not sure to get it.
>
> Index can be used for drivers which allocate more than one port for one device.
> In your implementation you should just replace devm_gpiod_get() to
> devm_gpiod_get_index() and add an additional parameter to mctrl_gpio_init().
> For at91 serial this parameter should be 0.
Understood.
>>
>>>> + enum mctrl_gpio_idx i;
>>>> + int err = 0;
>>>> + int ret = 0;
>>>> +
>>>> + for (i = 0; i < UART_GPIO_MAX; i++) {
>>>> + gpios->gpio[i] = devm_gpiod_get(dev, mctrl_gpios_desc[i].name);
> ...
>
Thanks !
Richard.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 1/7] tty/serial: Add GPIOLIB helpers for controlling modem lines
@ 2014-02-20 11:20 ` Richard Genoud
0 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-20 11:20 UTC (permalink / raw)
To: linux-arm-kernel
On 18/02/2014 16:26, Alexander Shiyan wrote:
> On Tue, 18 Feb 2014 10:59:56 +0100
> Richard Genoud <richard.genoud@gmail.com> wrote:
>
>> On 17/02/2014 19:37, Alexander Shiyan wrote:
>>> Hello.
>> Hi !
>>>
>>> A few comments below..
>>>
>>> ???????????, 17 ??????? 2014, 17:57 +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>
>>>> ---
>>> ...
>>>> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
>>> ...
>>>> +static const struct {
>>>> + const char *name;
>>>> + unsigned int mctrl;
>>>> + bool dir_out;
>>>> +} mctrl_gpios_desc[] = {
>>>> + { "cts", TIOCM_CTS, false, },
>>>> + { "dsr", TIOCM_DSR, false, },
>>>> + { "dcd", TIOCM_CD, false, },
>>>> + { "ri", TIOCM_RI, false, },
>>>> + { "rts", TIOCM_RTS, true, },
>>>> + { "dtr", TIOCM_DTR, true, },
>>>> + { "out1", TIOCM_OUT1, true, },
>>>> + { "out2", TIOCM_OUT2, true, },
>>>> +};
>>>> +
>>>> +void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
>>>> +{
>>>> + enum mctrl_gpio_idx i;
>>>> +
>>>> + for (i = 0; i < UART_GPIO_MAX; i++)
>>>
>>> Use ARRAY_SIZE(mctrl_gpios_desc) here and elsewhere below.
>> Could you explain why ARRAY_SIZE(mctrl_gpios_desc) seems better than
>> UART_GPIO_MAX ?
>
> Because you iterate through the array.
> I do not see the use of UART_GPIO_MAX in this module. You use UART_GPIO_MAX
> in the at91 serial driver only, here we must be sure that we are within the
> boundaries of the array.
Well, the loop iterates through the gpios->gpio[] array which size is
UART_GPIO_MAX, and uses also the mctrl_gpios_desc[] which size is implicit.
Anyway, if you absolutely want this change, I'll do it.
(Or I could also force the length of mctrl_gpios_desc[] like that:
static const struct {
const char *name;
unsigned int mctrl;
bool dir_out;
} mctrl_gpios_desc[UART_GPIO_MAX] = {
{ "cts", TIOCM_CTS, false, },
{ "dsr", TIOCM_DSR, false, },
{ "dcd", TIOCM_CD, false, },
{ "ri", TIOCM_RI, false, },
{ "rts", TIOCM_RTS, true, },
{ "dtr", TIOCM_DTR, true, },
{ "out1", TIOCM_OUT1, true, },
{ "out2", TIOCM_OUT2, true, },
};
This way, all loops will be correct. )
> ...
>>>> +unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
>>>> +{
>>>
>>> Why you want to put mctrl as parameter here?
>>> Let's return mctrl from GPIOs, then handle this value as you want int the driver.
>> It's because I like when it's simple :).
>> Use case:
>> Your USART controller handles CTS/RTS, and you add DTR/DSR as gpios.
>> In get_mctrl() callback, with current implementation, you'll have
>> something like this: (cf atmel_get_mctrl() for a real life example)
>> {
>> unsigned int mctrl;
>> mctrl = usart_controller_get_mctrl(); /* driver specific */
>> return mctrl_gpio_get(gpios, &mctrl);
>> }
>> If I use as you suggest mctrl_gpio_get(struct mctrl_gpios *gpios), we'll
>> have:
>> {
>> unsigned int usart_mctrl;
>> unsigned int gpio_mctrl;
>> int i;
>>
>> usart_mctrl = usart_controller_get_mctrl();
>> gpio_mctrl = mctrl_gpio_get(gpios);
>> if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_CTS]) {
>> if (gpio_mctrl & TIOCM_CTS)
>> usart_mctrl |= TIOCM_CTS;
>> else
>> usart_mctrl &= ~TIOCM_CTS;
>> }
>> if (!IS_ERR_OR_NULL(gpios->gpio[UART_GPIO_DSR]) {
>> if (gpio_mctrl & TIOCM_DSR)
>> usart_mctrl |= TIOCM_DSR;
>> else
>> usart_mctrl &= ~TIOCM_DSR;
>> }
>
> No, just like this:
> {
> unsigned int mctrl = usart_controller_get_mctrl(); /* driver specific */
> mctrl |= mctrl_gpio_get(gpios, mctrl) & ~(TIOCM_CTS | TIOCM_DSR);
> return mctrl;
> }
Hum, I think you made a mistake here.
The "mctrl_gpio_get(gpios, mctrl) & ~(TIOCM_CTS | TIOCM_DSR)" doesn't make
any sense. Or I don't understand what you want to do...
>
> or in reverse order:
> {
> unsigned int mctrl = mctrl_gpio_get(gpios, mctrl);
> return mctrl | usart_controller_get_mctrl(); /* driver specific */
> }
Ok, I didn't explain everything on that:
In atmel_serial (at least), the function usart_controller_get_mctrl()
is in fact a read on a register (Channel Status Register) + conversion
from the register flags to TIOCM_* flags.
That's pretty classical.
BUT when, for example CTS, is muxed as a GPIO and not as the peripheral
function, the value read for the register is undefined (I've seen it
changed randomly). (and it's not very surprising)
So, the value(s) of declared GPIO have to supersede the value(s)
retrieved from the controller.
Thus, a simple OR won't do the trick.
If you really think that mctrl_gpio_get() as it is, is over-complicated,
I could do something like:
static inline unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios) {
int mctrl = 0;
return mctrl_gpio_update(gpios, &mctrl);
}
unsigned int mctrl_gpio_update(struct mctrl_gpios *gpios, unsigned int *mctrl)
{
/* like the old mctrl_gpio_get() */
}
But is it really worth it ?
>
> ...
>>>> +int mctrl_gpio_init(struct device *dev, struct mctrl_gpios *gpios)
>>>> +{
>>>
>>> I suggest to allocate "gpios" here and return pointer to this structure
>>> or ERR_PTR(). Additionally, as I mentioned before, add "index" variable
>>> to specify port number.
>>
>> I don't understand the benefit of dynamically allocating something that
>> has a fixed size...
>> Or maybe in the case no GPIO are used, the array is not allocated, and
>> I'll have to add "if (!gpios)" test in other functions. That could save
>> some bytes.
>
> Yes, but basically this able to use just a pointer in your driver data,
> this will not depend on GPIOLIB, since without GPIOLIB we do not know storage
> size of struct gpio_desc.
Ok, convinced.
>
>> Could you explain a little more your idea of ""index" variable to
>> specify port number" ?
>> I'm not sure to get it.
>
> Index can be used for drivers which allocate more than one port for one device.
> In your implementation you should just replace devm_gpiod_get() to
> devm_gpiod_get_index() and add an additional parameter to mctrl_gpio_init().
> For at91 serial this parameter should be 0.
Understood.
>>
>>>> + enum mctrl_gpio_idx i;
>>>> + int err = 0;
>>>> + int ret = 0;
>>>> +
>>>> + for (i = 0; i < UART_GPIO_MAX; i++) {
>>>> + gpios->gpio[i] = devm_gpiod_get(dev, mctrl_gpios_desc[i].name);
> ...
>
Thanks !
Richard.
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 5/7] ARM: at91: gpio: implement get_direction
2014-02-17 16:57 ` Richard Genoud
@ 2014-02-24 14:42 ` Linus Walleij
-1 siblings, 0 replies; 40+ messages in thread
From: Linus Walleij @ 2014-02-24 14:42 UTC (permalink / raw)
To: Richard Genoud
Cc: Greg Kroah-Hartman, Uwe Kleine-König, Nicolas Ferre,
Alexander Shiyan, linux-serial, linux-arm-kernel,
Jean-Christophe Plagniol-Villard
On Mon, Feb 17, 2014 at 5:57 PM, Richard Genoud
<richard.genoud@gmail.com> wrote:
> This is needed for gpiod_get_direction().
> Otherwise, it returns -EINVAL.
>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Nicolas: please apply this through the AT91 tree.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 5/7] ARM: at91: gpio: implement get_direction
@ 2014-02-24 14:42 ` Linus Walleij
0 siblings, 0 replies; 40+ messages in thread
From: Linus Walleij @ 2014-02-24 14:42 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Feb 17, 2014 at 5:57 PM, Richard Genoud
<richard.genoud@gmail.com> wrote:
> This is needed for gpiod_get_direction().
> Otherwise, it returns -EINVAL.
>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Nicolas: please apply this through the AT91 tree.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 6/7] pinctrl: at91: implement get_direction
2014-02-17 16:57 ` Richard Genoud
@ 2014-02-24 14:44 ` Linus Walleij
-1 siblings, 0 replies; 40+ messages in thread
From: Linus Walleij @ 2014-02-24 14:44 UTC (permalink / raw)
To: Richard Genoud, Nicolas Ferre
Cc: Greg Kroah-Hartman, Uwe Kleine-König, Alexander Shiyan,
linux-serial, linux-arm-kernel, Jean-Christophe Plagniol-Villard
On Mon, Feb 17, 2014 at 5:57 PM, Richard Genoud
<richard.genoud@gmail.com> wrote:
> This is needed for gpiod_get_direction().
> Otherwise, it returns -EINVAL.
>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Can I apply this directly to the pinctrl tree or do you want it to
go through AT91 ARM SoC with the rest of these patches
or something?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 6/7] pinctrl: at91: implement get_direction
@ 2014-02-24 14:44 ` Linus Walleij
0 siblings, 0 replies; 40+ messages in thread
From: Linus Walleij @ 2014-02-24 14:44 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Feb 17, 2014 at 5:57 PM, Richard Genoud
<richard.genoud@gmail.com> wrote:
> This is needed for gpiod_get_direction().
> Otherwise, it returns -EINVAL.
>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Can I apply this directly to the pinctrl tree or do you want it to
go through AT91 ARM SoC with the rest of these patches
or something?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 6/7] pinctrl: at91: implement get_direction
2014-02-24 14:44 ` Linus Walleij
@ 2014-02-24 14:56 ` Richard Genoud
-1 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-24 14:56 UTC (permalink / raw)
To: Linus Walleij
Cc: Nicolas Ferre, Greg Kroah-Hartman, Uwe Kleine-König,
Alexander Shiyan, linux-serial, linux-arm-kernel,
Jean-Christophe Plagniol-Villard
2014-02-24 15:44 GMT+01:00 Linus Walleij <linus.walleij@linaro.org>:
> On Mon, Feb 17, 2014 at 5:57 PM, Richard Genoud
> <richard.genoud@gmail.com> wrote:
>
>> This is needed for gpiod_get_direction().
>> Otherwise, it returns -EINVAL.
>>
>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>
> Can I apply this directly to the pinctrl tree or do you want it to
> go through AT91 ARM SoC with the rest of these patches
> or something?
Well, I guess you can apply it on pinctrl tree.
I'll comme with an updated version of patches 1,4,7 soon, and I'll
write clearly in the cover letter the needed patches, and the tree
they're in to prevent breaking the compilation.
thanks !
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 6/7] pinctrl: at91: implement get_direction
@ 2014-02-24 14:56 ` Richard Genoud
0 siblings, 0 replies; 40+ messages in thread
From: Richard Genoud @ 2014-02-24 14:56 UTC (permalink / raw)
To: linux-arm-kernel
2014-02-24 15:44 GMT+01:00 Linus Walleij <linus.walleij@linaro.org>:
> On Mon, Feb 17, 2014 at 5:57 PM, Richard Genoud
> <richard.genoud@gmail.com> wrote:
>
>> This is needed for gpiod_get_direction().
>> Otherwise, it returns -EINVAL.
>>
>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>
> Can I apply this directly to the pinctrl tree or do you want it to
> go through AT91 ARM SoC with the rest of these patches
> or something?
Well, I guess you can apply it on pinctrl tree.
I'll comme with an updated version of patches 1,4,7 soon, and I'll
write clearly in the cover letter the needed patches, and the tree
they're in to prevent breaking the compilation.
thanks !
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 6/7] pinctrl: at91: implement get_direction
2014-02-24 14:56 ` Richard Genoud
@ 2014-02-25 9:34 ` Linus Walleij
-1 siblings, 0 replies; 40+ messages in thread
From: Linus Walleij @ 2014-02-25 9:34 UTC (permalink / raw)
To: Richard Genoud
Cc: Nicolas Ferre, Greg Kroah-Hartman, Uwe Kleine-König,
Alexander Shiyan, linux-serial, linux-arm-kernel,
Jean-Christophe Plagniol-Villard
On Mon, Feb 24, 2014 at 3:56 PM, Richard Genoud
<richard.genoud@gmail.com> wrote:
> 2014-02-24 15:44 GMT+01:00 Linus Walleij <linus.walleij@linaro.org>:
>> On Mon, Feb 17, 2014 at 5:57 PM, Richard Genoud
>> <richard.genoud@gmail.com> wrote:
>>
>>> This is needed for gpiod_get_direction().
>>> Otherwise, it returns -EINVAL.
>>>
>>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>>
>> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> Can I apply this directly to the pinctrl tree or do you want it to
>> go through AT91 ARM SoC with the rest of these patches
>> or something?
>
> Well, I guess you can apply it on pinctrl tree.
OK patch applied tentatively unless Nicolas or Jean-Christophe
has objections.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 6/7] pinctrl: at91: implement get_direction
@ 2014-02-25 9:34 ` Linus Walleij
0 siblings, 0 replies; 40+ messages in thread
From: Linus Walleij @ 2014-02-25 9:34 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Feb 24, 2014 at 3:56 PM, Richard Genoud
<richard.genoud@gmail.com> wrote:
> 2014-02-24 15:44 GMT+01:00 Linus Walleij <linus.walleij@linaro.org>:
>> On Mon, Feb 17, 2014 at 5:57 PM, Richard Genoud
>> <richard.genoud@gmail.com> wrote:
>>
>>> This is needed for gpiod_get_direction().
>>> Otherwise, it returns -EINVAL.
>>>
>>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>>
>> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> Can I apply this directly to the pinctrl tree or do you want it to
>> go through AT91 ARM SoC with the rest of these patches
>> or something?
>
> Well, I guess you can apply it on pinctrl tree.
OK patch applied tentatively unless Nicolas or Jean-Christophe
has objections.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 40+ messages in thread
end of thread, other threads:[~2014-02-25 9:34 UTC | newest]
Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-17 16:57 [PATCH v3 0/7] tty/serial: Add helpers to use GPIOs to control modem lines and implement atmel_serial.c Richard Genoud
2014-02-17 16:57 ` Richard Genoud
2014-02-17 16:57 ` [PATCH v3 1/7] tty/serial: Add GPIOLIB helpers for controlling modem lines Richard Genoud
2014-02-17 16:57 ` Richard Genoud
2014-02-17 18:37 ` Alexander Shiyan
2014-02-17 18:37 ` Alexander Shiyan
2014-02-18 9:59 ` Richard Genoud
2014-02-18 9:59 ` Richard Genoud
2014-02-18 15:26 ` Alexander Shiyan
2014-02-18 15:26 ` Alexander Shiyan
2014-02-20 11:20 ` Richard Genoud
2014-02-20 11:20 ` Richard Genoud
2014-02-17 16:57 ` [PATCH v3 2/7] tty/serial: at91: use dev_err instead of printk Richard Genoud
2014-02-17 16:57 ` Richard Genoud
2014-02-17 16:57 ` [PATCH v3 3/7] tty/serial: at91: remove unused open/close hooks Richard Genoud
2014-02-17 16:57 ` Richard Genoud
2014-02-17 16:57 ` [PATCH v3 4/7] tty/serial: at91: use mctrl_gpio helpers Richard Genoud
2014-02-17 16:57 ` Richard Genoud
2014-02-18 15:04 ` Alexander Shiyan
2014-02-18 15:04 ` Alexander Shiyan
2014-02-18 15:09 ` Richard Genoud
2014-02-18 15:09 ` Richard Genoud
2014-02-17 16:57 ` [PATCH v3 5/7] ARM: at91: gpio: implement get_direction Richard Genoud
2014-02-17 16:57 ` Richard Genoud
2014-02-24 14:42 ` Linus Walleij
2014-02-24 14:42 ` Linus Walleij
2014-02-17 16:57 ` [PATCH v3 6/7] pinctrl: at91: " Richard Genoud
2014-02-17 16:57 ` Richard Genoud
2014-02-24 14:44 ` Linus Walleij
2014-02-24 14:44 ` Linus Walleij
2014-02-24 14:56 ` Richard Genoud
2014-02-24 14:56 ` Richard Genoud
2014-02-25 9:34 ` Linus Walleij
2014-02-25 9:34 ` Linus Walleij
2014-02-17 16:57 ` [PATCH v3 7/7] tty/serial: at91: add interrupts for modem control lines Richard Genoud
2014-02-17 16:57 ` Richard Genoud
2014-02-17 17:53 ` [PATCH v3 0/7] tty/serial: Add helpers to use GPIOs to control modem lines and implement atmel_serial.c Alexander Shiyan
2014-02-17 17:53 ` Alexander Shiyan
2014-02-18 9:59 ` Richard Genoud
2014-02-18 9:59 ` Richard Genoud
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.