[PATCH 0/6] Convert sa1100 serial to use mctrl gpios

[PATCH 0/6] Convert sa1100 serial to use mctrl gpios

[Russell King - ARM Linux admin]

This series takes the next step along the path of cleaning up and
modernising the sa11x0 code, concentrating on the sa1100 serial
driver.  At the end of this conversion, we are left with collie
being the only platform that uses the old platform data get_mctrl
and set_mctrl methods.  As the locomo code is also used with PXA,
this is left for a future cleanup.

The first patch was previously acked quite some time ago.

I'm intending to send this to arm-soc.

 arch/arm/mach-sa1100/assabet.c  |  91 +++++++++++----------------------
 arch/arm/mach-sa1100/badge4.c   |   2 -
 arch/arm/mach-sa1100/h3xxx.c    |  64 ++++-------------------
 arch/arm/mach-sa1100/hackkit.c  |  48 ------------------
 arch/arm/mach-sa1100/neponset.c | 109 +++++++++++-----------------------------
 drivers/tty/serial/Kconfig      |   1 +
 drivers/tty/serial/sa1100.c     |  42 ++++++++++++++--
 7 files changed, 106 insertions(+), 251 deletions(-)


-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

[PATCH 0/6] Convert sa1100 serial to use mctrl gpios

[Russell King - ARM Linux admin]

This series takes the next step along the path of cleaning up and
modernising the sa11x0 code, concentrating on the sa1100 serial
driver.  At the end of this conversion, we are left with collie
being the only platform that uses the old platform data get_mctrl
and set_mctrl methods.  As the locomo code is also used with PXA,
this is left for a future cleanup.

The first patch was previously acked quite some time ago.

I'm intending to send this to arm-soc.

 arch/arm/mach-sa1100/assabet.c  |  91 +++++++++++----------------------
 arch/arm/mach-sa1100/badge4.c   |   2 -
 arch/arm/mach-sa1100/h3xxx.c    |  64 ++++-------------------
 arch/arm/mach-sa1100/hackkit.c  |  48 ------------------
 arch/arm/mach-sa1100/neponset.c | 109 +++++++++++-----------------------------
 drivers/tty/serial/Kconfig      |   1 +
 drivers/tty/serial/sa1100.c     |  42 ++++++++++++++--
 7 files changed, 106 insertions(+), 251 deletions(-)


-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/6] serial: sa1100: add support for mctrl gpios

[Russell King]

Add support for the generic mctrl gpio helper.  This will allow us to
convert several board files to use the gpiod tables to assign GPIOs to
serial ports, rather than needing to have private function callbacks.

If the generic mctrl gpio helper fails, ignore the mctrl gpios rather
than preventing the (possibly console) serial port from being created.

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/tty/serial/Kconfig  |  1 +
 drivers/tty/serial/sa1100.c | 42 ++++++++++++++++++++++++++++++++++++++----
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 72966bc0ac76..f4372ac1a774 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -511,6 +511,7 @@ config SERIAL_SA1100
 	bool "SA1100 serial port support"
 	depends on ARCH_SA1100
 	select SERIAL_CORE
+	select SERIAL_MCTRL_GPIO if GPIOLIB
 	help
 	  If you have a machine based on a SA1100/SA1110 StrongARM(R) CPU you
 	  can enable its onboard serial port by enabling this option.
diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
index a399772be3fc..97bdfeccbea9 100644
--- a/drivers/tty/serial/sa1100.c
+++ b/drivers/tty/serial/sa1100.c
@@ -28,6 +28,8 @@
 #include <mach/hardware.h>
 #include <mach/irqs.h>
 
+#include "serial_mctrl_gpio.h"
+
 /* We've been assigned a range on the "Low-density serial ports" major */
 #define SERIAL_SA1100_MAJOR	204
 #define MINOR_START		5
@@ -77,6 +79,7 @@ struct sa1100_port {
 	struct uart_port	port;
 	struct timer_list	timer;
 	unsigned int		old_status;
+	struct mctrl_gpios	*gpios;
 };
 
 /*
@@ -174,6 +177,8 @@ static void sa1100_enable_ms(struct uart_port *port)
 		container_of(port, struct sa1100_port, port);
 
 	mod_timer(&sport->timer, jiffies);
+
+	mctrl_gpio_enable_ms(sport->gpios);
 }
 
 static void
@@ -322,11 +327,21 @@ static unsigned int sa1100_tx_empty(struct uart_port *port)
 
 static unsigned int sa1100_get_mctrl(struct uart_port *port)
 {
-	return TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
+	struct sa1100_port *sport =
+		container_of(port, struct sa1100_port, port);
+	int ret = TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
+
+	mctrl_gpio_get(sport->gpios, &ret);
+
+	return ret;
 }
 
 static void sa1100_set_mctrl(struct uart_port *port, unsigned int mctrl)
 {
+	struct sa1100_port *sport =
+		container_of(port, struct sa1100_port, port);
+
+	mctrl_gpio_set(sport->gpios, mctrl);
 }
 
 /*
@@ -842,6 +857,27 @@ static int sa1100_serial_resume(struct platform_device *dev)
 	return 0;
 }
 
+static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
+{
+	sport->port.dev = &dev->dev;
+	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);
+	if (IS_ERR(sport->gpios)) {
+		int err = PTR_ERR(sport->gpios);
+
+		dev_err(sport->port.dev, "failed to get mctrl gpios: %d\n",
+			err);
+
+		if (err == -EPROBE_DEFER)
+			return err;
+
+		sport->gpios = NULL;
+	}
+
+	platform_set_drvdata(dev, sport);
+
+	return uart_add_one_port(&sa1100_reg, &sport->port);
+}
+
 static int sa1100_serial_probe(struct platform_device *dev)
 {
 	struct resource *res = dev->resource;
@@ -856,9 +892,7 @@ static int sa1100_serial_probe(struct platform_device *dev)
 			if (sa1100_ports[i].port.mapbase != res->start)
 				continue;
 
-			sa1100_ports[i].port.dev = &dev->dev;
-			uart_add_one_port(&sa1100_reg, &sa1100_ports[i].port);
-			platform_set_drvdata(dev, &sa1100_ports[i]);
+			sa1100_serial_add_one_port(&sa1100_ports[i], dev);
 			break;
 		}
 	}
-- 
2.7.4

[PATCH 1/6] serial: sa1100: add support for mctrl gpios

[Russell King]

Add support for the generic mctrl gpio helper.  This will allow us to
convert several board files to use the gpiod tables to assign GPIOs to
serial ports, rather than needing to have private function callbacks.

If the generic mctrl gpio helper fails, ignore the mctrl gpios rather
than preventing the (possibly console) serial port from being created.

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/tty/serial/Kconfig  |  1 +
 drivers/tty/serial/sa1100.c | 42 ++++++++++++++++++++++++++++++++++++++----
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 72966bc0ac76..f4372ac1a774 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -511,6 +511,7 @@ config SERIAL_SA1100
 	bool "SA1100 serial port support"
 	depends on ARCH_SA1100
 	select SERIAL_CORE
+	select SERIAL_MCTRL_GPIO if GPIOLIB
 	help
 	  If you have a machine based on a SA1100/SA1110 StrongARM(R) CPU you
 	  can enable its onboard serial port by enabling this option.
diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
index a399772be3fc..97bdfeccbea9 100644
--- a/drivers/tty/serial/sa1100.c
+++ b/drivers/tty/serial/sa1100.c
@@ -28,6 +28,8 @@
 #include <mach/hardware.h>
 #include <mach/irqs.h>
 
+#include "serial_mctrl_gpio.h"
+
 /* We've been assigned a range on the "Low-density serial ports" major */
 #define SERIAL_SA1100_MAJOR	204
 #define MINOR_START		5
@@ -77,6 +79,7 @@ struct sa1100_port {
 	struct uart_port	port;
 	struct timer_list	timer;
 	unsigned int		old_status;
+	struct mctrl_gpios	*gpios;
 };
 
 /*
@@ -174,6 +177,8 @@ static void sa1100_enable_ms(struct uart_port *port)
 		container_of(port, struct sa1100_port, port);
 
 	mod_timer(&sport->timer, jiffies);
+
+	mctrl_gpio_enable_ms(sport->gpios);
 }
 
 static void
@@ -322,11 +327,21 @@ static unsigned int sa1100_tx_empty(struct uart_port *port)
 
 static unsigned int sa1100_get_mctrl(struct uart_port *port)
 {
-	return TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
+	struct sa1100_port *sport =
+		container_of(port, struct sa1100_port, port);
+	int ret = TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
+
+	mctrl_gpio_get(sport->gpios, &ret);
+
+	return ret;
 }
 
 static void sa1100_set_mctrl(struct uart_port *port, unsigned int mctrl)
 {
+	struct sa1100_port *sport =
+		container_of(port, struct sa1100_port, port);
+
+	mctrl_gpio_set(sport->gpios, mctrl);
 }
 
 /*
@@ -842,6 +857,27 @@ static int sa1100_serial_resume(struct platform_device *dev)
 	return 0;
 }
 
+static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
+{
+	sport->port.dev = &dev->dev;
+	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);
+	if (IS_ERR(sport->gpios)) {
+		int err = PTR_ERR(sport->gpios);
+
+		dev_err(sport->port.dev, "failed to get mctrl gpios: %d\n",
+			err);
+
+		if (err == -EPROBE_DEFER)
+			return err;
+
+		sport->gpios = NULL;
+	}
+
+	platform_set_drvdata(dev, sport);
+
+	return uart_add_one_port(&sa1100_reg, &sport->port);
+}
+
 static int sa1100_serial_probe(struct platform_device *dev)
 {
 	struct resource *res = dev->resource;
@@ -856,9 +892,7 @@ static int sa1100_serial_probe(struct platform_device *dev)
 			if (sa1100_ports[i].port.mapbase != res->start)
 				continue;
 
-			sa1100_ports[i].port.dev = &dev->dev;
-			uart_add_one_port(&sa1100_reg, &sa1100_ports[i].port);
-			platform_set_drvdata(dev, &sa1100_ports[i]);
+			sa1100_serial_add_one_port(&sa1100_ports[i], dev);
 			break;
 		}
 	}
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/6] ARM: sa1100/assabet: convert serial to gpiod APIs

[Russell King]

Convert the Assabet serial modem control signals to use the gpiod APIs
rather than custom callbacks into platform code.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mach-sa1100/assabet.c | 91 +++++++++++++-----------------------------
 1 file changed, 28 insertions(+), 63 deletions(-)

diff --git a/arch/arm/mach-sa1100/assabet.c b/arch/arm/mach-sa1100/assabet.c
index d09c3f236186..bc035821a42b 100644
--- a/arch/arm/mach-sa1100/assabet.c
+++ b/arch/arm/mach-sa1100/assabet.c
@@ -522,6 +522,29 @@ static const struct gpio_keys_platform_data assabet_keys_pdata = {
 	.rep = 0,
 };
 
+static struct gpiod_lookup_table assabet_uart1_gpio_table = {
+	.dev_id = "sa11x0-uart.1",
+	.table = {
+		GPIO_LOOKUP("assabet", 16, "dtr", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("assabet", 17, "rts", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("assabet", 25, "dcd", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("assabet", 26, "cts", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("assabet", 27, "dsr", GPIO_ACTIVE_LOW),
+		{ },
+	},
+};
+
+static struct gpiod_lookup_table assabet_uart3_gpio_table = {
+	.dev_id = "sa11x0-uart.3",
+	.table = {
+		GPIO_LOOKUP("assabet", 28, "cts", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("assabet", 29, "dsr", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("assabet", 30, "dcd", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("assabet", 31, "rng", GPIO_ACTIVE_LOW),
+		{ },
+	},
+};
+
 static void __init assabet_init(void)
 {
 	/*
@@ -568,7 +591,10 @@ static void __init assabet_init(void)
 			neponset_resources, ARRAY_SIZE(neponset_resources));
 #endif
 	} else {
+		gpiod_add_lookup_table(&assabet_uart1_gpio_table);
+		gpiod_add_lookup_table(&assabet_uart3_gpio_table);
 		gpiod_add_lookup_table(&assabet_cf_vcc_gpio_table);
+
 		sa11x0_register_fixed_regulator(0, &assabet_cf_vcc_pdata,
 					assabet_cf_vcc_consumers,
 					ARRAY_SIZE(assabet_cf_vcc_consumers),
@@ -658,74 +684,13 @@ static void assabet_uart_pm(struct uart_port *port, u_int state, u_int oldstate)
 {
 	if (port->mapbase == _Ser1UTCR0) {
 		if (state)
-			ASSABET_BCR_clear(ASSABET_BCR_RS232EN |
-					  ASSABET_BCR_COM_RTS |
-					  ASSABET_BCR_COM_DTR);
-		else
-			ASSABET_BCR_set(ASSABET_BCR_RS232EN |
-					ASSABET_BCR_COM_RTS |
-					ASSABET_BCR_COM_DTR);
-	}
-}
-
-/*
- * Assabet uses COM_RTS and COM_DTR for both UART1 (com port)
- * and UART3 (radio module).  We only handle them for UART1 here.
- */
-static void assabet_set_mctrl(struct uart_port *port, u_int mctrl)
-{
-	if (port->mapbase == _Ser1UTCR0) {
-		u_int set = 0, clear = 0;
-
-		if (mctrl & TIOCM_RTS)
-			clear |= ASSABET_BCR_COM_RTS;
+			ASSABET_BCR_clear(ASSABET_BCR_RS232EN);
 		else
-			set |= ASSABET_BCR_COM_RTS;
-
-		if (mctrl & TIOCM_DTR)
-			clear |= ASSABET_BCR_COM_DTR;
-		else
-			set |= ASSABET_BCR_COM_DTR;
-
-		ASSABET_BCR_clear(clear);
-		ASSABET_BCR_set(set);
-	}
-}
-
-static u_int assabet_get_mctrl(struct uart_port *port)
-{
-	u_int ret = 0;
-	u_int bsr = ASSABET_BSR;
-
-	/* need 2 reads to read current value */
-	bsr = ASSABET_BSR;
-
-	if (port->mapbase == _Ser1UTCR0) {
-		if (bsr & ASSABET_BSR_COM_DCD)
-			ret |= TIOCM_CD;
-		if (bsr & ASSABET_BSR_COM_CTS)
-			ret |= TIOCM_CTS;
-		if (bsr & ASSABET_BSR_COM_DSR)
-			ret |= TIOCM_DSR;
-	} else if (port->mapbase == _Ser3UTCR0) {
-		if (bsr & ASSABET_BSR_RAD_DCD)
-			ret |= TIOCM_CD;
-		if (bsr & ASSABET_BSR_RAD_CTS)
-			ret |= TIOCM_CTS;
-		if (bsr & ASSABET_BSR_RAD_DSR)
-			ret |= TIOCM_DSR;
-		if (bsr & ASSABET_BSR_RAD_RI)
-			ret |= TIOCM_RI;
-	} else {
-		ret = TIOCM_CD | TIOCM_CTS | TIOCM_DSR;
+			ASSABET_BCR_set(ASSABET_BCR_RS232EN);
 	}
-
-	return ret;
 }
 
 static struct sa1100_port_fns assabet_port_fns __initdata = {
-	.set_mctrl	= assabet_set_mctrl,
-	.get_mctrl	= assabet_get_mctrl,
 	.pm		= assabet_uart_pm,
 };
 
-- 
2.7.4

[PATCH 2/6] ARM: sa1100/assabet: convert serial to gpiod APIs

[Russell King]

Convert the Assabet serial modem control signals to use the gpiod APIs
rather than custom callbacks into platform code.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mach-sa1100/assabet.c | 91 +++++++++++++-----------------------------
 1 file changed, 28 insertions(+), 63 deletions(-)

diff --git a/arch/arm/mach-sa1100/assabet.c b/arch/arm/mach-sa1100/assabet.c
index d09c3f236186..bc035821a42b 100644
--- a/arch/arm/mach-sa1100/assabet.c
+++ b/arch/arm/mach-sa1100/assabet.c
@@ -522,6 +522,29 @@ static const struct gpio_keys_platform_data assabet_keys_pdata = {
 	.rep = 0,
 };
 
+static struct gpiod_lookup_table assabet_uart1_gpio_table = {
+	.dev_id = "sa11x0-uart.1",
+	.table = {
+		GPIO_LOOKUP("assabet", 16, "dtr", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("assabet", 17, "rts", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("assabet", 25, "dcd", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("assabet", 26, "cts", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("assabet", 27, "dsr", GPIO_ACTIVE_LOW),
+		{ },
+	},
+};
+
+static struct gpiod_lookup_table assabet_uart3_gpio_table = {
+	.dev_id = "sa11x0-uart.3",
+	.table = {
+		GPIO_LOOKUP("assabet", 28, "cts", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("assabet", 29, "dsr", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("assabet", 30, "dcd", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("assabet", 31, "rng", GPIO_ACTIVE_LOW),
+		{ },
+	},
+};
+
 static void __init assabet_init(void)
 {
 	/*
@@ -568,7 +591,10 @@ static void __init assabet_init(void)
 			neponset_resources, ARRAY_SIZE(neponset_resources));
 #endif
 	} else {
+		gpiod_add_lookup_table(&assabet_uart1_gpio_table);
+		gpiod_add_lookup_table(&assabet_uart3_gpio_table);
 		gpiod_add_lookup_table(&assabet_cf_vcc_gpio_table);
+
 		sa11x0_register_fixed_regulator(0, &assabet_cf_vcc_pdata,
 					assabet_cf_vcc_consumers,
 					ARRAY_SIZE(assabet_cf_vcc_consumers),
@@ -658,74 +684,13 @@ static void assabet_uart_pm(struct uart_port *port, u_int state, u_int oldstate)
 {
 	if (port->mapbase == _Ser1UTCR0) {
 		if (state)
-			ASSABET_BCR_clear(ASSABET_BCR_RS232EN |
-					  ASSABET_BCR_COM_RTS |
-					  ASSABET_BCR_COM_DTR);
-		else
-			ASSABET_BCR_set(ASSABET_BCR_RS232EN |
-					ASSABET_BCR_COM_RTS |
-					ASSABET_BCR_COM_DTR);
-	}
-}
-
-/*
- * Assabet uses COM_RTS and COM_DTR for both UART1 (com port)
- * and UART3 (radio module).  We only handle them for UART1 here.
- */
-static void assabet_set_mctrl(struct uart_port *port, u_int mctrl)
-{
-	if (port->mapbase == _Ser1UTCR0) {
-		u_int set = 0, clear = 0;
-
-		if (mctrl & TIOCM_RTS)
-			clear |= ASSABET_BCR_COM_RTS;
+			ASSABET_BCR_clear(ASSABET_BCR_RS232EN);
 		else
-			set |= ASSABET_BCR_COM_RTS;
-
-		if (mctrl & TIOCM_DTR)
-			clear |= ASSABET_BCR_COM_DTR;
-		else
-			set |= ASSABET_BCR_COM_DTR;
-
-		ASSABET_BCR_clear(clear);
-		ASSABET_BCR_set(set);
-	}
-}
-
-static u_int assabet_get_mctrl(struct uart_port *port)
-{
-	u_int ret = 0;
-	u_int bsr = ASSABET_BSR;
-
-	/* need 2 reads to read current value */
-	bsr = ASSABET_BSR;
-
-	if (port->mapbase == _Ser1UTCR0) {
-		if (bsr & ASSABET_BSR_COM_DCD)
-			ret |= TIOCM_CD;
-		if (bsr & ASSABET_BSR_COM_CTS)
-			ret |= TIOCM_CTS;
-		if (bsr & ASSABET_BSR_COM_DSR)
-			ret |= TIOCM_DSR;
-	} else if (port->mapbase == _Ser3UTCR0) {
-		if (bsr & ASSABET_BSR_RAD_DCD)
-			ret |= TIOCM_CD;
-		if (bsr & ASSABET_BSR_RAD_CTS)
-			ret |= TIOCM_CTS;
-		if (bsr & ASSABET_BSR_RAD_DSR)
-			ret |= TIOCM_DSR;
-		if (bsr & ASSABET_BSR_RAD_RI)
-			ret |= TIOCM_RI;
-	} else {
-		ret = TIOCM_CD | TIOCM_CTS | TIOCM_DSR;
+			ASSABET_BCR_set(ASSABET_BCR_RS232EN);
 	}
-
-	return ret;
 }
 
 static struct sa1100_port_fns assabet_port_fns __initdata = {
-	.set_mctrl	= assabet_set_mctrl,
-	.get_mctrl	= assabet_get_mctrl,
 	.pm		= assabet_uart_pm,
 };
 
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/6] ARM: sa1100/h3xxx: convert serial to gpiod APIs

[Russell King]

Convert the iPAQ H3xxx serial modem control signals to use the gpiod
APIs rather than custom callbacks into platform code.

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mach-sa1100/h3xxx.c | 64 ++++++++------------------------------------
 1 file changed, 11 insertions(+), 53 deletions(-)

diff --git a/arch/arm/mach-sa1100/h3xxx.c b/arch/arm/mach-sa1100/h3xxx.c
index 36a78b0c106f..e7362292612a 100644
--- a/arch/arm/mach-sa1100/h3xxx.c
+++ b/arch/arm/mach-sa1100/h3xxx.c
@@ -87,57 +87,6 @@ static struct resource h3xxx_flash_resource =
 /*
  * H3xxx uart support
  */
-static struct gpio h3xxx_uart_gpio[] = {
-	{ H3XXX_GPIO_COM_DCD,	GPIOF_IN,		"COM DCD" },
-	{ H3XXX_GPIO_COM_CTS,	GPIOF_IN,		"COM CTS" },
-	{ H3XXX_GPIO_COM_RTS,	GPIOF_OUT_INIT_LOW,	"COM RTS" },
-};
-
-static bool h3xxx_uart_request_gpios(void)
-{
-	static bool h3xxx_uart_gpio_ok;
-	int rc;
-
-	if (h3xxx_uart_gpio_ok)
-		return true;
-
-	rc = gpio_request_array(h3xxx_uart_gpio, ARRAY_SIZE(h3xxx_uart_gpio));
-	if (rc)
-		pr_err("h3xxx_uart_request_gpios: error %d\n", rc);
-	else
-		h3xxx_uart_gpio_ok = true;
-
-	return h3xxx_uart_gpio_ok;
-}
-
-static void h3xxx_uart_set_mctrl(struct uart_port *port, u_int mctrl)
-{
-	if (port->mapbase == _Ser3UTCR0) {
-		if (!h3xxx_uart_request_gpios())
-			return;
-		gpio_set_value(H3XXX_GPIO_COM_RTS, !(mctrl & TIOCM_RTS));
-	}
-}
-
-static u_int h3xxx_uart_get_mctrl(struct uart_port *port)
-{
-	u_int ret = TIOCM_CD | TIOCM_CTS | TIOCM_DSR;
-
-	if (port->mapbase == _Ser3UTCR0) {
-		if (!h3xxx_uart_request_gpios())
-			return ret;
-		/*
-		 * DCD and CTS bits are inverted in GPLR by RS232 transceiver
-		 */
-		if (gpio_get_value(H3XXX_GPIO_COM_DCD))
-			ret &= ~TIOCM_CD;
-		if (gpio_get_value(H3XXX_GPIO_COM_CTS))
-			ret &= ~TIOCM_CTS;
-	}
-
-	return ret;
-}
-
 static void h3xxx_uart_pm(struct uart_port *port, u_int state, u_int oldstate)
 {
 	if (port->mapbase == _Ser3UTCR0) {
@@ -170,12 +119,20 @@ static int h3xxx_uart_set_wake(struct uart_port *port, u_int enable)
 }
 
 static struct sa1100_port_fns h3xxx_port_fns __initdata = {
-	.set_mctrl	= h3xxx_uart_set_mctrl,
-	.get_mctrl	= h3xxx_uart_get_mctrl,
 	.pm		= h3xxx_uart_pm,
 	.set_wake	= h3xxx_uart_set_wake,
 };
 
+static struct gpiod_lookup_table h3xxx_uart3_gpio_table = {
+	.dev_id = "sa11x0-uart.3",
+	.table = {
+		GPIO_LOOKUP("gpio", H3XXX_GPIO_COM_DCD, "dcd", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("gpio", H3XXX_GPIO_COM_CTS, "cts", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("gpio", H3XXX_GPIO_COM_RTS, "rts", GPIO_ACTIVE_LOW),
+		{ },
+	},
+};
+
 /*
  * EGPIO
  */
@@ -283,6 +240,7 @@ static struct gpiod_lookup_table h3xxx_pcmcia_gpio_table = {
 void __init h3xxx_mach_init(void)
 {
 	gpiod_add_lookup_table(&h3xxx_pcmcia_gpio_table);
+	gpiod_add_lookup_table(&h3xxx_uart3_gpio_table);
 	sa1100_register_uart_fns(&h3xxx_port_fns);
 	sa11x0_register_mtd(&h3xxx_flash_data, &h3xxx_flash_resource, 1);
 	platform_add_devices(h3xxx_devices, ARRAY_SIZE(h3xxx_devices));
-- 
2.7.4

[PATCH 3/6] ARM: sa1100/h3xxx: convert serial to gpiod APIs

[Russell King]

Convert the iPAQ H3xxx serial modem control signals to use the gpiod
APIs rather than custom callbacks into platform code.

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mach-sa1100/h3xxx.c | 64 ++++++++------------------------------------
 1 file changed, 11 insertions(+), 53 deletions(-)

diff --git a/arch/arm/mach-sa1100/h3xxx.c b/arch/arm/mach-sa1100/h3xxx.c
index 36a78b0c106f..e7362292612a 100644
--- a/arch/arm/mach-sa1100/h3xxx.c
+++ b/arch/arm/mach-sa1100/h3xxx.c
@@ -87,57 +87,6 @@ static struct resource h3xxx_flash_resource =
 /*
  * H3xxx uart support
  */
-static struct gpio h3xxx_uart_gpio[] = {
-	{ H3XXX_GPIO_COM_DCD,	GPIOF_IN,		"COM DCD" },
-	{ H3XXX_GPIO_COM_CTS,	GPIOF_IN,		"COM CTS" },
-	{ H3XXX_GPIO_COM_RTS,	GPIOF_OUT_INIT_LOW,	"COM RTS" },
-};
-
-static bool h3xxx_uart_request_gpios(void)
-{
-	static bool h3xxx_uart_gpio_ok;
-	int rc;
-
-	if (h3xxx_uart_gpio_ok)
-		return true;
-
-	rc = gpio_request_array(h3xxx_uart_gpio, ARRAY_SIZE(h3xxx_uart_gpio));
-	if (rc)
-		pr_err("h3xxx_uart_request_gpios: error %d\n", rc);
-	else
-		h3xxx_uart_gpio_ok = true;
-
-	return h3xxx_uart_gpio_ok;
-}
-
-static void h3xxx_uart_set_mctrl(struct uart_port *port, u_int mctrl)
-{
-	if (port->mapbase == _Ser3UTCR0) {
-		if (!h3xxx_uart_request_gpios())
-			return;
-		gpio_set_value(H3XXX_GPIO_COM_RTS, !(mctrl & TIOCM_RTS));
-	}
-}
-
-static u_int h3xxx_uart_get_mctrl(struct uart_port *port)
-{
-	u_int ret = TIOCM_CD | TIOCM_CTS | TIOCM_DSR;
-
-	if (port->mapbase == _Ser3UTCR0) {
-		if (!h3xxx_uart_request_gpios())
-			return ret;
-		/*
-		 * DCD and CTS bits are inverted in GPLR by RS232 transceiver
-		 */
-		if (gpio_get_value(H3XXX_GPIO_COM_DCD))
-			ret &= ~TIOCM_CD;
-		if (gpio_get_value(H3XXX_GPIO_COM_CTS))
-			ret &= ~TIOCM_CTS;
-	}
-
-	return ret;
-}
-
 static void h3xxx_uart_pm(struct uart_port *port, u_int state, u_int oldstate)
 {
 	if (port->mapbase == _Ser3UTCR0) {
@@ -170,12 +119,20 @@ static int h3xxx_uart_set_wake(struct uart_port *port, u_int enable)
 }
 
 static struct sa1100_port_fns h3xxx_port_fns __initdata = {
-	.set_mctrl	= h3xxx_uart_set_mctrl,
-	.get_mctrl	= h3xxx_uart_get_mctrl,
 	.pm		= h3xxx_uart_pm,
 	.set_wake	= h3xxx_uart_set_wake,
 };
 
+static struct gpiod_lookup_table h3xxx_uart3_gpio_table = {
+	.dev_id = "sa11x0-uart.3",
+	.table = {
+		GPIO_LOOKUP("gpio", H3XXX_GPIO_COM_DCD, "dcd", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("gpio", H3XXX_GPIO_COM_CTS, "cts", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("gpio", H3XXX_GPIO_COM_RTS, "rts", GPIO_ACTIVE_LOW),
+		{ },
+	},
+};
+
 /*
  * EGPIO
  */
@@ -283,6 +240,7 @@ static struct gpiod_lookup_table h3xxx_pcmcia_gpio_table = {
 void __init h3xxx_mach_init(void)
 {
 	gpiod_add_lookup_table(&h3xxx_pcmcia_gpio_table);
+	gpiod_add_lookup_table(&h3xxx_uart3_gpio_table);
 	sa1100_register_uart_fns(&h3xxx_port_fns);
 	sa11x0_register_mtd(&h3xxx_flash_data, &h3xxx_flash_resource, 1);
 	platform_add_devices(h3xxx_devices, ARRAY_SIZE(h3xxx_devices));
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/6] ARM: sa1100/badge4: remove commented out modem control initialisers

[Russell King]

Remove the commented out modem control initialisers.  These are doing
nothing useful.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mach-sa1100/badge4.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/mach-sa1100/badge4.c b/arch/arm/mach-sa1100/badge4.c
index 63361b6d04e9..17d28b4dab5e 100644
--- a/arch/arm/mach-sa1100/badge4.c
+++ b/arch/arm/mach-sa1100/badge4.c
@@ -315,8 +315,6 @@ badge4_uart_pm(struct uart_port *port, u_int state, u_int oldstate)
 }
 
 static struct sa1100_port_fns badge4_port_fns __initdata = {
-	//.get_mctrl	= badge4_get_mctrl,
-	//.set_mctrl	= badge4_set_mctrl,
 	.pm		= badge4_uart_pm,
 };
 
-- 
2.7.4

[PATCH 4/6] ARM: sa1100/badge4: remove commented out modem control initialisers

[Russell King]

Remove the commented out modem control initialisers.  These are doing
nothing useful.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mach-sa1100/badge4.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/mach-sa1100/badge4.c b/arch/arm/mach-sa1100/badge4.c
index 63361b6d04e9..17d28b4dab5e 100644
--- a/arch/arm/mach-sa1100/badge4.c
+++ b/arch/arm/mach-sa1100/badge4.c
@@ -315,8 +315,6 @@ badge4_uart_pm(struct uart_port *port, u_int state, u_int oldstate)
 }
 
 static struct sa1100_port_fns badge4_port_fns __initdata = {
-	//.get_mctrl	= badge4_get_mctrl,
-	//.set_mctrl	= badge4_set_mctrl,
 	.pm		= badge4_uart_pm,
 };
 
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/6] ARM: sa1100/hackkit: remove empty serial mctrl functions

[Russell King]

Remove the empty serial modem control signal functions from hackkit
as these are unnecessary - the core code can copes fine without
these.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mach-sa1100/hackkit.c | 48 ------------------------------------------
 1 file changed, 48 deletions(-)

diff --git a/arch/arm/mach-sa1100/hackkit.c b/arch/arm/mach-sa1100/hackkit.c
index 643d5f2d9af9..9faf602666cf 100644
--- a/arch/arm/mach-sa1100/hackkit.c
+++ b/arch/arm/mach-sa1100/hackkit.c
@@ -49,8 +49,6 @@
 /* init funcs */
 static void __init hackkit_map_io(void);
 
-static u_int hackkit_get_mctrl(struct uart_port *port);
-static void hackkit_set_mctrl(struct uart_port *port, u_int mctrl);
 static void hackkit_uart_pm(struct uart_port *port, u_int state, u_int oldstate);
 
 /**********************************************************************
@@ -71,8 +69,6 @@ static struct map_desc hackkit_io_desc[] __initdata = {
 };
 
 static struct sa1100_port_fns hackkit_port_fns __initdata = {
-	.set_mctrl	= hackkit_set_mctrl,
-	.get_mctrl	= hackkit_get_mctrl,
 	.pm		= hackkit_uart_pm,
 };
 
@@ -105,50 +101,6 @@ static void hackkit_uart_pm(struct uart_port *port, u_int state, u_int oldstate)
 	/* TODO: switch on/off uart in powersave mode */
 }
 
-/*
- * Note! this can be called from IRQ context.
- * FIXME: No modem ctrl lines yet.
- */
-static void hackkit_set_mctrl(struct uart_port *port, u_int mctrl)
-{
-#if 0
-	if (port->mapbase == _Ser1UTCR0) {
-		u_int set = 0, clear = 0;
-
-		if (mctrl & TIOCM_RTS)
-			set |= PT_CTRL2_RS1_RTS;
-		else
-			clear |= PT_CTRL2_RS1_RTS;
-
-		if (mctrl & TIOCM_DTR)
-			set |= PT_CTRL2_RS1_DTR;
-		else
-			clear |= PT_CTRL2_RS1_DTR;
-
-		PTCTRL2_clear(clear);
-		PTCTRL2_set(set);
-	}
-#endif
-}
-
-static u_int hackkit_get_mctrl(struct uart_port *port)
-{
-	u_int ret = 0;
-#if 0
-	u_int irqsr = PT_IRQSR;
-
-	/* need 2 reads to read current value */
-	irqsr = PT_IRQSR;
-
-	/* TODO: check IRQ source register for modem/com
-	 status lines and set them correctly. */
-#endif
-
-	ret = TIOCM_CD | TIOCM_CTS | TIOCM_DSR;
-
-	return ret;
-}
-
 static struct mtd_partition hackkit_partitions[] = {
 	{
 		.name		= "BLOB",
-- 
2.7.4

[PATCH 5/6] ARM: sa1100/hackkit: remove empty serial mctrl functions

[Russell King]

Remove the empty serial modem control signal functions from hackkit
as these are unnecessary - the core code can copes fine without
these.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mach-sa1100/hackkit.c | 48 ------------------------------------------
 1 file changed, 48 deletions(-)

diff --git a/arch/arm/mach-sa1100/hackkit.c b/arch/arm/mach-sa1100/hackkit.c
index 643d5f2d9af9..9faf602666cf 100644
--- a/arch/arm/mach-sa1100/hackkit.c
+++ b/arch/arm/mach-sa1100/hackkit.c
@@ -49,8 +49,6 @@
 /* init funcs */
 static void __init hackkit_map_io(void);
 
-static u_int hackkit_get_mctrl(struct uart_port *port);
-static void hackkit_set_mctrl(struct uart_port *port, u_int mctrl);
 static void hackkit_uart_pm(struct uart_port *port, u_int state, u_int oldstate);
 
 /**********************************************************************
@@ -71,8 +69,6 @@ static struct map_desc hackkit_io_desc[] __initdata = {
 };
 
 static struct sa1100_port_fns hackkit_port_fns __initdata = {
-	.set_mctrl	= hackkit_set_mctrl,
-	.get_mctrl	= hackkit_get_mctrl,
 	.pm		= hackkit_uart_pm,
 };
 
@@ -105,50 +101,6 @@ static void hackkit_uart_pm(struct uart_port *port, u_int state, u_int oldstate)
 	/* TODO: switch on/off uart in powersave mode */
 }
 
-/*
- * Note! this can be called from IRQ context.
- * FIXME: No modem ctrl lines yet.
- */
-static void hackkit_set_mctrl(struct uart_port *port, u_int mctrl)
-{
-#if 0
-	if (port->mapbase == _Ser1UTCR0) {
-		u_int set = 0, clear = 0;
-
-		if (mctrl & TIOCM_RTS)
-			set |= PT_CTRL2_RS1_RTS;
-		else
-			clear |= PT_CTRL2_RS1_RTS;
-
-		if (mctrl & TIOCM_DTR)
-			set |= PT_CTRL2_RS1_DTR;
-		else
-			clear |= PT_CTRL2_RS1_DTR;
-
-		PTCTRL2_clear(clear);
-		PTCTRL2_set(set);
-	}
-#endif
-}
-
-static u_int hackkit_get_mctrl(struct uart_port *port)
-{
-	u_int ret = 0;
-#if 0
-	u_int irqsr = PT_IRQSR;
-
-	/* need 2 reads to read current value */
-	irqsr = PT_IRQSR;
-
-	/* TODO: check IRQ source register for modem/com
-	 status lines and set them correctly. */
-#endif
-
-	ret = TIOCM_CD | TIOCM_CTS | TIOCM_DSR;
-
-	return ret;
-}
-
 static struct mtd_partition hackkit_partitions[] = {
 	{
 		.name		= "BLOB",
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 6/6] ARM: sa1100/neponset: convert serial to use gpiod APIs

[Russell King]

Convert the serial modem control signals to use the gpiod APIs rather
than the private platform callbacks.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mach-sa1100/neponset.c | 109 +++++++++++-----------------------------
 1 file changed, 28 insertions(+), 81 deletions(-)

diff --git a/arch/arm/mach-sa1100/neponset.c b/arch/arm/mach-sa1100/neponset.c
index eb60a71cf125..f1f70fc98fb7 100644
--- a/arch/arm/mach-sa1100/neponset.c
+++ b/arch/arm/mach-sa1100/neponset.c
@@ -11,7 +11,6 @@
 #include <linux/irq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/platform_data/sa11x0-serial.h>
 #include <linux/platform_device.h>
 #include <linux/pm.h>
 #include <linux/serial_core.h>
@@ -49,23 +48,8 @@
 #define IRR_SA1111	(1 << 2)
 
 #define NCR_NGPIO	7
-
-#define MDM_CTL0_RTS1	(1 << 0)
-#define MDM_CTL0_DTR1	(1 << 1)
-#define MDM_CTL0_RTS2	(1 << 2)
-#define MDM_CTL0_DTR2	(1 << 3)
 #define MDM_CTL0_NGPIO	4
-
-#define MDM_CTL1_CTS1	(1 << 0)
-#define MDM_CTL1_DSR1	(1 << 1)
-#define MDM_CTL1_DCD1	(1 << 2)
-#define MDM_CTL1_CTS2	(1 << 3)
-#define MDM_CTL1_DSR2	(1 << 4)
-#define MDM_CTL1_DCD2	(1 << 5)
 #define MDM_CTL1_NGPIO	6
-
-#define AUD_SEL_1341	(1 << 0)
-#define AUD_MUTE_1341	(1 << 1)
 #define AUD_NGPIO	2
 
 extern void sa1110_mb_disable(void);
@@ -97,6 +81,30 @@ struct neponset_drvdata {
 	struct gpio_chip *gpio[4];
 };
 
+static struct gpiod_lookup_table neponset_uart1_gpio_table = {
+	.dev_id = "sa11x0-uart.1",
+	.table = {
+		GPIO_LOOKUP("neponset-mdm-ctl0", 2, "rts", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("neponset-mdm-ctl0", 3, "dtr", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("neponset-mdm-ctl1", 3, "cts", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("neponset-mdm-ctl1", 4, "dsr", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("neponset-mdm-ctl1", 5, "dcd", GPIO_ACTIVE_LOW),
+		{ },
+	},
+};
+
+static struct gpiod_lookup_table neponset_uart3_gpio_table = {
+	.dev_id = "sa11x0-uart.3",
+	.table = {
+		GPIO_LOOKUP("neponset-mdm-ctl0", 0, "rts", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("neponset-mdm-ctl0", 1, "dtr", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("neponset-mdm-ctl1", 0, "cts", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("neponset-mdm-ctl1", 1, "dsr", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("neponset-mdm-ctl1", 2, "dcd", GPIO_ACTIVE_LOW),
+		{ },
+	},
+};
+
 static struct gpiod_lookup_table neponset_pcmcia_table = {
 	.dev_id = "1800",
 	.table = {
@@ -124,69 +132,6 @@ void neponset_ncr_frob(unsigned int mask, unsigned int val)
 }
 EXPORT_SYMBOL(neponset_ncr_frob);
 
-static void neponset_set_mctrl(struct uart_port *port, u_int mctrl)
-{
-	struct neponset_drvdata *n = nep;
-	unsigned long mask, val = 0;
-
-	if (!n)
-		return;
-
-	if (port->mapbase == _Ser1UTCR0) {
-		mask = MDM_CTL0_RTS2 | MDM_CTL0_DTR2;
-
-		if (!(mctrl & TIOCM_RTS))
-			val |= MDM_CTL0_RTS2;
-
-		if (!(mctrl & TIOCM_DTR))
-			val |= MDM_CTL0_DTR2;
-	} else if (port->mapbase == _Ser3UTCR0) {
-		mask = MDM_CTL0_RTS1 | MDM_CTL0_DTR1;
-
-		if (!(mctrl & TIOCM_RTS))
-			val |= MDM_CTL0_RTS1;
-
-		if (!(mctrl & TIOCM_DTR))
-			val |= MDM_CTL0_DTR1;
-	}
-
-	n->gpio[1]->set_multiple(n->gpio[1], &mask, &val);
-}
-
-static u_int neponset_get_mctrl(struct uart_port *port)
-{
-	void __iomem *base = nep->base;
-	u_int ret = TIOCM_CD | TIOCM_CTS | TIOCM_DSR;
-	u_int mdm_ctl1;
-
-	if (!base)
-		return ret;
-
-	mdm_ctl1 = readb_relaxed(base + MDM_CTL_1);
-	if (port->mapbase == _Ser1UTCR0) {
-		if (mdm_ctl1 & MDM_CTL1_DCD2)
-			ret &= ~TIOCM_CD;
-		if (mdm_ctl1 & MDM_CTL1_CTS2)
-			ret &= ~TIOCM_CTS;
-		if (mdm_ctl1 & MDM_CTL1_DSR2)
-			ret &= ~TIOCM_DSR;
-	} else if (port->mapbase == _Ser3UTCR0) {
-		if (mdm_ctl1 & MDM_CTL1_DCD1)
-			ret &= ~TIOCM_CD;
-		if (mdm_ctl1 & MDM_CTL1_CTS1)
-			ret &= ~TIOCM_CTS;
-		if (mdm_ctl1 & MDM_CTL1_DSR1)
-			ret &= ~TIOCM_DSR;
-	}
-
-	return ret;
-}
-
-static struct sa1100_port_fns neponset_port_fns = {
-	.set_mctrl	= neponset_set_mctrl,
-	.get_mctrl	= neponset_get_mctrl,
-};
-
 /*
  * Install handler for Neponset IRQ.  Note that we have to loop here
  * since the ETHERNET and USAR IRQs are level based, and we need to
@@ -388,6 +333,8 @@ static int neponset_probe(struct platform_device *dev)
 			   d->base + AUD_CTL, AUD_NGPIO, false,
 			   neponset_aud_names);
 
+	gpiod_add_lookup_table(&neponset_uart1_gpio_table);
+	gpiod_add_lookup_table(&neponset_uart3_gpio_table);
 	gpiod_add_lookup_table(&neponset_pcmcia_table);
 
 	/*
@@ -402,8 +349,6 @@ static int neponset_probe(struct platform_device *dev)
 		 d->irq_base, d->irq_base + NEP_IRQ_NR - 1);
 	nep = d;
 
-	sa1100_register_uart_fns(&neponset_port_fns);
-
 	/* Ensure that the memory bus request/grant signals are setup */
 	sa1110_mb_disable();
 
@@ -442,6 +387,8 @@ static int neponset_remove(struct platform_device *dev)
 		platform_device_unregister(d->smc91x);
 
 	gpiod_remove_lookup_table(&neponset_pcmcia_table);
+	gpiod_remove_lookup_table(&neponset_uart3_gpio_table);
+	gpiod_remove_lookup_table(&neponset_uart1_gpio_table);
 
 	irq_set_chained_handler(irq, NULL);
 	irq_free_descs(d->irq_base, NEP_IRQ_NR);
-- 
2.7.4

[PATCH 6/6] ARM: sa1100/neponset: convert serial to use gpiod APIs

[Russell King]

Convert the serial modem control signals to use the gpiod APIs rather
than the private platform callbacks.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mach-sa1100/neponset.c | 109 +++++++++++-----------------------------
 1 file changed, 28 insertions(+), 81 deletions(-)

diff --git a/arch/arm/mach-sa1100/neponset.c b/arch/arm/mach-sa1100/neponset.c
index eb60a71cf125..f1f70fc98fb7 100644
--- a/arch/arm/mach-sa1100/neponset.c
+++ b/arch/arm/mach-sa1100/neponset.c
@@ -11,7 +11,6 @@
 #include <linux/irq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/platform_data/sa11x0-serial.h>
 #include <linux/platform_device.h>
 #include <linux/pm.h>
 #include <linux/serial_core.h>
@@ -49,23 +48,8 @@
 #define IRR_SA1111	(1 << 2)
 
 #define NCR_NGPIO	7
-
-#define MDM_CTL0_RTS1	(1 << 0)
-#define MDM_CTL0_DTR1	(1 << 1)
-#define MDM_CTL0_RTS2	(1 << 2)
-#define MDM_CTL0_DTR2	(1 << 3)
 #define MDM_CTL0_NGPIO	4
-
-#define MDM_CTL1_CTS1	(1 << 0)
-#define MDM_CTL1_DSR1	(1 << 1)
-#define MDM_CTL1_DCD1	(1 << 2)
-#define MDM_CTL1_CTS2	(1 << 3)
-#define MDM_CTL1_DSR2	(1 << 4)
-#define MDM_CTL1_DCD2	(1 << 5)
 #define MDM_CTL1_NGPIO	6
-
-#define AUD_SEL_1341	(1 << 0)
-#define AUD_MUTE_1341	(1 << 1)
 #define AUD_NGPIO	2
 
 extern void sa1110_mb_disable(void);
@@ -97,6 +81,30 @@ struct neponset_drvdata {
 	struct gpio_chip *gpio[4];
 };
 
+static struct gpiod_lookup_table neponset_uart1_gpio_table = {
+	.dev_id = "sa11x0-uart.1",
+	.table = {
+		GPIO_LOOKUP("neponset-mdm-ctl0", 2, "rts", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("neponset-mdm-ctl0", 3, "dtr", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("neponset-mdm-ctl1", 3, "cts", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("neponset-mdm-ctl1", 4, "dsr", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("neponset-mdm-ctl1", 5, "dcd", GPIO_ACTIVE_LOW),
+		{ },
+	},
+};
+
+static struct gpiod_lookup_table neponset_uart3_gpio_table = {
+	.dev_id = "sa11x0-uart.3",
+	.table = {
+		GPIO_LOOKUP("neponset-mdm-ctl0", 0, "rts", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("neponset-mdm-ctl0", 1, "dtr", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("neponset-mdm-ctl1", 0, "cts", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("neponset-mdm-ctl1", 1, "dsr", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("neponset-mdm-ctl1", 2, "dcd", GPIO_ACTIVE_LOW),
+		{ },
+	},
+};
+
 static struct gpiod_lookup_table neponset_pcmcia_table = {
 	.dev_id = "1800",
 	.table = {
@@ -124,69 +132,6 @@ void neponset_ncr_frob(unsigned int mask, unsigned int val)
 }
 EXPORT_SYMBOL(neponset_ncr_frob);
 
-static void neponset_set_mctrl(struct uart_port *port, u_int mctrl)
-{
-	struct neponset_drvdata *n = nep;
-	unsigned long mask, val = 0;
-
-	if (!n)
-		return;
-
-	if (port->mapbase == _Ser1UTCR0) {
-		mask = MDM_CTL0_RTS2 | MDM_CTL0_DTR2;
-
-		if (!(mctrl & TIOCM_RTS))
-			val |= MDM_CTL0_RTS2;
-
-		if (!(mctrl & TIOCM_DTR))
-			val |= MDM_CTL0_DTR2;
-	} else if (port->mapbase == _Ser3UTCR0) {
-		mask = MDM_CTL0_RTS1 | MDM_CTL0_DTR1;
-
-		if (!(mctrl & TIOCM_RTS))
-			val |= MDM_CTL0_RTS1;
-
-		if (!(mctrl & TIOCM_DTR))
-			val |= MDM_CTL0_DTR1;
-	}
-
-	n->gpio[1]->set_multiple(n->gpio[1], &mask, &val);
-}
-
-static u_int neponset_get_mctrl(struct uart_port *port)
-{
-	void __iomem *base = nep->base;
-	u_int ret = TIOCM_CD | TIOCM_CTS | TIOCM_DSR;
-	u_int mdm_ctl1;
-
-	if (!base)
-		return ret;
-
-	mdm_ctl1 = readb_relaxed(base + MDM_CTL_1);
-	if (port->mapbase == _Ser1UTCR0) {
-		if (mdm_ctl1 & MDM_CTL1_DCD2)
-			ret &= ~TIOCM_CD;
-		if (mdm_ctl1 & MDM_CTL1_CTS2)
-			ret &= ~TIOCM_CTS;
-		if (mdm_ctl1 & MDM_CTL1_DSR2)
-			ret &= ~TIOCM_DSR;
-	} else if (port->mapbase == _Ser3UTCR0) {
-		if (mdm_ctl1 & MDM_CTL1_DCD1)
-			ret &= ~TIOCM_CD;
-		if (mdm_ctl1 & MDM_CTL1_CTS1)
-			ret &= ~TIOCM_CTS;
-		if (mdm_ctl1 & MDM_CTL1_DSR1)
-			ret &= ~TIOCM_DSR;
-	}
-
-	return ret;
-}
-
-static struct sa1100_port_fns neponset_port_fns = {
-	.set_mctrl	= neponset_set_mctrl,
-	.get_mctrl	= neponset_get_mctrl,
-};
-
 /*
  * Install handler for Neponset IRQ.  Note that we have to loop here
  * since the ETHERNET and USAR IRQs are level based, and we need to
@@ -388,6 +333,8 @@ static int neponset_probe(struct platform_device *dev)
 			   d->base + AUD_CTL, AUD_NGPIO, false,
 			   neponset_aud_names);
 
+	gpiod_add_lookup_table(&neponset_uart1_gpio_table);
+	gpiod_add_lookup_table(&neponset_uart3_gpio_table);
 	gpiod_add_lookup_table(&neponset_pcmcia_table);
 
 	/*
@@ -402,8 +349,6 @@ static int neponset_probe(struct platform_device *dev)
 		 d->irq_base, d->irq_base + NEP_IRQ_NR - 1);
 	nep = d;
 
-	sa1100_register_uart_fns(&neponset_port_fns);
-
 	/* Ensure that the memory bus request/grant signals are setup */
 	sa1110_mb_disable();
 
@@ -442,6 +387,8 @@ static int neponset_remove(struct platform_device *dev)
 		platform_device_unregister(d->smc91x);
 
 	gpiod_remove_lookup_table(&neponset_pcmcia_table);
+	gpiod_remove_lookup_table(&neponset_uart3_gpio_table);
+	gpiod_remove_lookup_table(&neponset_uart1_gpio_table);
 
 	irq_set_chained_handler(irq, NULL);
 	irq_free_descs(d->irq_base, NEP_IRQ_NR);
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] serial: sa1100: add support for mctrl gpios

[Uwe Kleine-König]

On Fri, May 31, 2019 at 12:13:47PM +0100, Russell King wrote:
> Add support for the generic mctrl gpio helper.  This will allow us to
> convert several board files to use the gpiod tables to assign GPIOs to
> serial ports, rather than needing to have private function callbacks.
> 
> If the generic mctrl gpio helper fails, ignore the mctrl gpios rather
> than preventing the (possibly console) serial port from being created.
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/tty/serial/Kconfig  |  1 +
>  drivers/tty/serial/sa1100.c | 42 ++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 39 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 72966bc0ac76..f4372ac1a774 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -511,6 +511,7 @@ config SERIAL_SA1100
>  	bool "SA1100 serial port support"
>  	depends on ARCH_SA1100
>  	select SERIAL_CORE
> +	select SERIAL_MCTRL_GPIO if GPIOLIB
>  	help
>  	  If you have a machine based on a SA1100/SA1110 StrongARM(R) CPU you
>  	  can enable its onboard serial port by enabling this option.
> diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
> index a399772be3fc..97bdfeccbea9 100644
> --- a/drivers/tty/serial/sa1100.c
> +++ b/drivers/tty/serial/sa1100.c
> @@ -28,6 +28,8 @@
>  #include <mach/hardware.h>
>  #include <mach/irqs.h>
>  
> +#include "serial_mctrl_gpio.h"
> +
>  /* We've been assigned a range on the "Low-density serial ports" major */
>  #define SERIAL_SA1100_MAJOR	204
>  #define MINOR_START		5
> @@ -77,6 +79,7 @@ struct sa1100_port {
>  	struct uart_port	port;
>  	struct timer_list	timer;
>  	unsigned int		old_status;
> +	struct mctrl_gpios	*gpios;
>  };
>  
>  /*
> @@ -174,6 +177,8 @@ static void sa1100_enable_ms(struct uart_port *port)
>  		container_of(port, struct sa1100_port, port);
>  
>  	mod_timer(&sport->timer, jiffies);
> +
> +	mctrl_gpio_enable_ms(sport->gpios);
>  }
>  
>  static void
> @@ -322,11 +327,21 @@ static unsigned int sa1100_tx_empty(struct uart_port *port)
>  
>  static unsigned int sa1100_get_mctrl(struct uart_port *port)
>  {
> -	return TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
> +	struct sa1100_port *sport =
> +		container_of(port, struct sa1100_port, port);
> +	int ret = TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
> +
> +	mctrl_gpio_get(sport->gpios, &ret);
> +
> +	return ret;
>  }
>  
>  static void sa1100_set_mctrl(struct uart_port *port, unsigned int mctrl)
>  {
> +	struct sa1100_port *sport =
> +		container_of(port, struct sa1100_port, port);
> +
> +	mctrl_gpio_set(sport->gpios, mctrl);
>  }
>  
>  /*
> @@ -842,6 +857,27 @@ static int sa1100_serial_resume(struct platform_device *dev)
>  	return 0;
>  }
>  
> +static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
> +{
> +	sport->port.dev = &dev->dev;
> +	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);

the _noauto function was only introduced to ease a transition. I think
the driver would benefit to use mctrl_gpio_init() instead.

Getting rid of mctrl_gpio_init_noauto() was on my todo list for some
time, but it was pushed down too far :-|

Best regards
Uwe

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

Re: [PATCH 1/6] serial: sa1100: add support for mctrl gpios

[Uwe Kleine-König]

On Fri, May 31, 2019 at 12:13:47PM +0100, Russell King wrote:
> Add support for the generic mctrl gpio helper.  This will allow us to
> convert several board files to use the gpiod tables to assign GPIOs to
> serial ports, rather than needing to have private function callbacks.
> 
> If the generic mctrl gpio helper fails, ignore the mctrl gpios rather
> than preventing the (possibly console) serial port from being created.
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/tty/serial/Kconfig  |  1 +
>  drivers/tty/serial/sa1100.c | 42 ++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 39 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 72966bc0ac76..f4372ac1a774 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -511,6 +511,7 @@ config SERIAL_SA1100
>  	bool "SA1100 serial port support"
>  	depends on ARCH_SA1100
>  	select SERIAL_CORE
> +	select SERIAL_MCTRL_GPIO if GPIOLIB
>  	help
>  	  If you have a machine based on a SA1100/SA1110 StrongARM(R) CPU you
>  	  can enable its onboard serial port by enabling this option.
> diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
> index a399772be3fc..97bdfeccbea9 100644
> --- a/drivers/tty/serial/sa1100.c
> +++ b/drivers/tty/serial/sa1100.c
> @@ -28,6 +28,8 @@
>  #include <mach/hardware.h>
>  #include <mach/irqs.h>
>  
> +#include "serial_mctrl_gpio.h"
> +
>  /* We've been assigned a range on the "Low-density serial ports" major */
>  #define SERIAL_SA1100_MAJOR	204
>  #define MINOR_START		5
> @@ -77,6 +79,7 @@ struct sa1100_port {
>  	struct uart_port	port;
>  	struct timer_list	timer;
>  	unsigned int		old_status;
> +	struct mctrl_gpios	*gpios;
>  };
>  
>  /*
> @@ -174,6 +177,8 @@ static void sa1100_enable_ms(struct uart_port *port)
>  		container_of(port, struct sa1100_port, port);
>  
>  	mod_timer(&sport->timer, jiffies);
> +
> +	mctrl_gpio_enable_ms(sport->gpios);
>  }
>  
>  static void
> @@ -322,11 +327,21 @@ static unsigned int sa1100_tx_empty(struct uart_port *port)
>  
>  static unsigned int sa1100_get_mctrl(struct uart_port *port)
>  {
> -	return TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
> +	struct sa1100_port *sport =
> +		container_of(port, struct sa1100_port, port);
> +	int ret = TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
> +
> +	mctrl_gpio_get(sport->gpios, &ret);
> +
> +	return ret;
>  }
>  
>  static void sa1100_set_mctrl(struct uart_port *port, unsigned int mctrl)
>  {
> +	struct sa1100_port *sport =
> +		container_of(port, struct sa1100_port, port);
> +
> +	mctrl_gpio_set(sport->gpios, mctrl);
>  }
>  
>  /*
> @@ -842,6 +857,27 @@ static int sa1100_serial_resume(struct platform_device *dev)
>  	return 0;
>  }
>  
> +static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
> +{
> +	sport->port.dev = &dev->dev;
> +	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);

the _noauto function was only introduced to ease a transition. I think
the driver would benefit to use mctrl_gpio_init() instead.

Getting rid of mctrl_gpio_init_noauto() was on my todo list for some
time, but it was pushed down too far :-|

Best regards
Uwe

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] serial: sa1100: add support for mctrl gpios

[Russell King - ARM Linux admin]

On Fri, May 31, 2019 at 02:50:13PM +0200, Uwe Kleine-König wrote:
> On Fri, May 31, 2019 at 12:13:47PM +0100, Russell King wrote:
> > Add support for the generic mctrl gpio helper.  This will allow us to
> > convert several board files to use the gpiod tables to assign GPIOs to
> > serial ports, rather than needing to have private function callbacks.
> > 
> > If the generic mctrl gpio helper fails, ignore the mctrl gpios rather
> > than preventing the (possibly console) serial port from being created.
> > 
> > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> >  drivers/tty/serial/Kconfig  |  1 +
> >  drivers/tty/serial/sa1100.c | 42 ++++++++++++++++++++++++++++++++++++++----
> >  2 files changed, 39 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> > index 72966bc0ac76..f4372ac1a774 100644
> > --- a/drivers/tty/serial/Kconfig
> > +++ b/drivers/tty/serial/Kconfig
> > @@ -511,6 +511,7 @@ config SERIAL_SA1100
> >  	bool "SA1100 serial port support"
> >  	depends on ARCH_SA1100
> >  	select SERIAL_CORE
> > +	select SERIAL_MCTRL_GPIO if GPIOLIB
> >  	help
> >  	  If you have a machine based on a SA1100/SA1110 StrongARM(R) CPU you
> >  	  can enable its onboard serial port by enabling this option.
> > diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
> > index a399772be3fc..97bdfeccbea9 100644
> > --- a/drivers/tty/serial/sa1100.c
> > +++ b/drivers/tty/serial/sa1100.c
> > @@ -28,6 +28,8 @@
> >  #include <mach/hardware.h>
> >  #include <mach/irqs.h>
> >  
> > +#include "serial_mctrl_gpio.h"
> > +
> >  /* We've been assigned a range on the "Low-density serial ports" major */
> >  #define SERIAL_SA1100_MAJOR	204
> >  #define MINOR_START		5
> > @@ -77,6 +79,7 @@ struct sa1100_port {
> >  	struct uart_port	port;
> >  	struct timer_list	timer;
> >  	unsigned int		old_status;
> > +	struct mctrl_gpios	*gpios;
> >  };
> >  
> >  /*
> > @@ -174,6 +177,8 @@ static void sa1100_enable_ms(struct uart_port *port)
> >  		container_of(port, struct sa1100_port, port);
> >  
> >  	mod_timer(&sport->timer, jiffies);
> > +
> > +	mctrl_gpio_enable_ms(sport->gpios);
> >  }
> >  
> >  static void
> > @@ -322,11 +327,21 @@ static unsigned int sa1100_tx_empty(struct uart_port *port)
> >  
> >  static unsigned int sa1100_get_mctrl(struct uart_port *port)
> >  {
> > -	return TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
> > +	struct sa1100_port *sport =
> > +		container_of(port, struct sa1100_port, port);
> > +	int ret = TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
> > +
> > +	mctrl_gpio_get(sport->gpios, &ret);
> > +
> > +	return ret;
> >  }
> >  
> >  static void sa1100_set_mctrl(struct uart_port *port, unsigned int mctrl)
> >  {
> > +	struct sa1100_port *sport =
> > +		container_of(port, struct sa1100_port, port);
> > +
> > +	mctrl_gpio_set(sport->gpios, mctrl);
> >  }
> >  
> >  /*
> > @@ -842,6 +857,27 @@ static int sa1100_serial_resume(struct platform_device *dev)
> >  	return 0;
> >  }
> >  
> > +static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
> > +{
> > +	sport->port.dev = &dev->dev;
> > +	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);
> 
> the _noauto function was only introduced to ease a transition. I think
> the driver would benefit to use mctrl_gpio_init() instead.

In what way would the driver benefit?  mctrl_gpio_init() requires that
there are IRQs for each input GPIO.  This is not the case with most
SA11x0 platforms, where the GPIO controls are implemented using simple
latches, hence that interface is entirely unsuitable.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH 1/6] serial: sa1100: add support for mctrl gpios

[Russell King - ARM Linux admin]

On Fri, May 31, 2019 at 02:50:13PM +0200, Uwe Kleine-König wrote:
> On Fri, May 31, 2019 at 12:13:47PM +0100, Russell King wrote:
> > Add support for the generic mctrl gpio helper.  This will allow us to
> > convert several board files to use the gpiod tables to assign GPIOs to
> > serial ports, rather than needing to have private function callbacks.
> > 
> > If the generic mctrl gpio helper fails, ignore the mctrl gpios rather
> > than preventing the (possibly console) serial port from being created.
> > 
> > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> >  drivers/tty/serial/Kconfig  |  1 +
> >  drivers/tty/serial/sa1100.c | 42 ++++++++++++++++++++++++++++++++++++++----
> >  2 files changed, 39 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> > index 72966bc0ac76..f4372ac1a774 100644
> > --- a/drivers/tty/serial/Kconfig
> > +++ b/drivers/tty/serial/Kconfig
> > @@ -511,6 +511,7 @@ config SERIAL_SA1100
> >  	bool "SA1100 serial port support"
> >  	depends on ARCH_SA1100
> >  	select SERIAL_CORE
> > +	select SERIAL_MCTRL_GPIO if GPIOLIB
> >  	help
> >  	  If you have a machine based on a SA1100/SA1110 StrongARM(R) CPU you
> >  	  can enable its onboard serial port by enabling this option.
> > diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
> > index a399772be3fc..97bdfeccbea9 100644
> > --- a/drivers/tty/serial/sa1100.c
> > +++ b/drivers/tty/serial/sa1100.c
> > @@ -28,6 +28,8 @@
> >  #include <mach/hardware.h>
> >  #include <mach/irqs.h>
> >  
> > +#include "serial_mctrl_gpio.h"
> > +
> >  /* We've been assigned a range on the "Low-density serial ports" major */
> >  #define SERIAL_SA1100_MAJOR	204
> >  #define MINOR_START		5
> > @@ -77,6 +79,7 @@ struct sa1100_port {
> >  	struct uart_port	port;
> >  	struct timer_list	timer;
> >  	unsigned int		old_status;
> > +	struct mctrl_gpios	*gpios;
> >  };
> >  
> >  /*
> > @@ -174,6 +177,8 @@ static void sa1100_enable_ms(struct uart_port *port)
> >  		container_of(port, struct sa1100_port, port);
> >  
> >  	mod_timer(&sport->timer, jiffies);
> > +
> > +	mctrl_gpio_enable_ms(sport->gpios);
> >  }
> >  
> >  static void
> > @@ -322,11 +327,21 @@ static unsigned int sa1100_tx_empty(struct uart_port *port)
> >  
> >  static unsigned int sa1100_get_mctrl(struct uart_port *port)
> >  {
> > -	return TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
> > +	struct sa1100_port *sport =
> > +		container_of(port, struct sa1100_port, port);
> > +	int ret = TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
> > +
> > +	mctrl_gpio_get(sport->gpios, &ret);
> > +
> > +	return ret;
> >  }
> >  
> >  static void sa1100_set_mctrl(struct uart_port *port, unsigned int mctrl)
> >  {
> > +	struct sa1100_port *sport =
> > +		container_of(port, struct sa1100_port, port);
> > +
> > +	mctrl_gpio_set(sport->gpios, mctrl);
> >  }
> >  
> >  /*
> > @@ -842,6 +857,27 @@ static int sa1100_serial_resume(struct platform_device *dev)
> >  	return 0;
> >  }
> >  
> > +static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
> > +{
> > +	sport->port.dev = &dev->dev;
> > +	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);
> 
> the _noauto function was only introduced to ease a transition. I think
> the driver would benefit to use mctrl_gpio_init() instead.

In what way would the driver benefit?  mctrl_gpio_init() requires that
there are IRQs for each input GPIO.  This is not the case with most
SA11x0 platforms, where the GPIO controls are implemented using simple
latches, hence that interface is entirely unsuitable.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] serial: sa1100: add support for mctrl gpios

[Uwe Kleine-König]

On Fri, May 31, 2019 at 02:23:40PM +0100, Russell King - ARM Linux admin wrote:
> On Fri, May 31, 2019 at 02:50:13PM +0200, Uwe Kleine-König wrote:
> > On Fri, May 31, 2019 at 12:13:47PM +0100, Russell King wrote:
> > > +static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
> > > +{
> > > +	sport->port.dev = &dev->dev;
> > > +	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);
> > 
> > the _noauto function was only introduced to ease a transition. I think
> > the driver would benefit to use mctrl_gpio_init() instead.
> 
> In what way would the driver benefit?  mctrl_gpio_init() requires that
> there are IRQs for each input GPIO.  This is not the case with most
> SA11x0 platforms, where the GPIO controls are implemented using simple
> latches, hence that interface is entirely unsuitable.

Ah, but then you can only use the outputs reliably here as an edge on
(say) CTS stays unnoticed with both mctrl_gpio_init() and
mctrl_gpio_init_noauto().

Unless I miss something (which is quite possible given that it's quite
some time ago I looked into mctrl_gpio) with mctrl_gpio_init_noauto()
having a CTS-gpio is just ignored unless the modem ctrl lines are
explicitely requestet while with mctrl_gpio_init() it results in an
error. Isn't the error the better alternative?

Best regards
Uwe

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

Re: [PATCH 1/6] serial: sa1100: add support for mctrl gpios

[Uwe Kleine-König]

On Fri, May 31, 2019 at 02:23:40PM +0100, Russell King - ARM Linux admin wrote:
> On Fri, May 31, 2019 at 02:50:13PM +0200, Uwe Kleine-König wrote:
> > On Fri, May 31, 2019 at 12:13:47PM +0100, Russell King wrote:
> > > +static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
> > > +{
> > > +	sport->port.dev = &dev->dev;
> > > +	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);
> > 
> > the _noauto function was only introduced to ease a transition. I think
> > the driver would benefit to use mctrl_gpio_init() instead.
> 
> In what way would the driver benefit?  mctrl_gpio_init() requires that
> there are IRQs for each input GPIO.  This is not the case with most
> SA11x0 platforms, where the GPIO controls are implemented using simple
> latches, hence that interface is entirely unsuitable.

Ah, but then you can only use the outputs reliably here as an edge on
(say) CTS stays unnoticed with both mctrl_gpio_init() and
mctrl_gpio_init_noauto().

Unless I miss something (which is quite possible given that it's quite
some time ago I looked into mctrl_gpio) with mctrl_gpio_init_noauto()
having a CTS-gpio is just ignored unless the modem ctrl lines are
explicitely requestet while with mctrl_gpio_init() it results in an
error. Isn't the error the better alternative?

Best regards
Uwe

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] serial: sa1100: add support for mctrl gpios

[Russell King - ARM Linux admin]

On Fri, May 31, 2019 at 03:56:58PM +0200, Uwe Kleine-König wrote:
> On Fri, May 31, 2019 at 02:23:40PM +0100, Russell King - ARM Linux admin wrote:
> > On Fri, May 31, 2019 at 02:50:13PM +0200, Uwe Kleine-König wrote:
> > > On Fri, May 31, 2019 at 12:13:47PM +0100, Russell King wrote:
> > > > +static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
> > > > +{
> > > > +	sport->port.dev = &dev->dev;
> > > > +	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);
> > > 
> > > the _noauto function was only introduced to ease a transition. I think
> > > the driver would benefit to use mctrl_gpio_init() instead.
> > 
> > In what way would the driver benefit?  mctrl_gpio_init() requires that
> > there are IRQs for each input GPIO.  This is not the case with most
> > SA11x0 platforms, where the GPIO controls are implemented using simple
> > latches, hence that interface is entirely unsuitable.
> 
> Ah, but then you can only use the outputs reliably here as an edge on
> (say) CTS stays unnoticed with both mctrl_gpio_init() and
> mctrl_gpio_init_noauto().

Right that is a risk with a polled approach, but that is the approach
that the SA1100 serial driver has taken ever since it was written
almost twenty years ago, and no one has raised any concerns about
that until now.

> Unless I miss something (which is quite possible given that it's quite
> some time ago I looked into mctrl_gpio) with mctrl_gpio_init_noauto()
> having a CTS-gpio is just ignored unless the modem ctrl lines are
> explicitely requestet while with mctrl_gpio_init() it results in an
> error. Isn't the error the better alternative?

Unless the serial driver polls the modem control line status, which
the SA1100 driver continues to do in exactly the same way after this
conversion.

Do you suggest that we just regress the driver by ripping out this
support that no one has had any problems with, and that is known to
work sufficiently in its day, just because we now don't like it?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH 1/6] serial: sa1100: add support for mctrl gpios

[Russell King - ARM Linux admin]

On Fri, May 31, 2019 at 03:56:58PM +0200, Uwe Kleine-König wrote:
> On Fri, May 31, 2019 at 02:23:40PM +0100, Russell King - ARM Linux admin wrote:
> > On Fri, May 31, 2019 at 02:50:13PM +0200, Uwe Kleine-König wrote:
> > > On Fri, May 31, 2019 at 12:13:47PM +0100, Russell King wrote:
> > > > +static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
> > > > +{
> > > > +	sport->port.dev = &dev->dev;
> > > > +	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);
> > > 
> > > the _noauto function was only introduced to ease a transition. I think
> > > the driver would benefit to use mctrl_gpio_init() instead.
> > 
> > In what way would the driver benefit?  mctrl_gpio_init() requires that
> > there are IRQs for each input GPIO.  This is not the case with most
> > SA11x0 platforms, where the GPIO controls are implemented using simple
> > latches, hence that interface is entirely unsuitable.
> 
> Ah, but then you can only use the outputs reliably here as an edge on
> (say) CTS stays unnoticed with both mctrl_gpio_init() and
> mctrl_gpio_init_noauto().

Right that is a risk with a polled approach, but that is the approach
that the SA1100 serial driver has taken ever since it was written
almost twenty years ago, and no one has raised any concerns about
that until now.

> Unless I miss something (which is quite possible given that it's quite
> some time ago I looked into mctrl_gpio) with mctrl_gpio_init_noauto()
> having a CTS-gpio is just ignored unless the modem ctrl lines are
> explicitely requestet while with mctrl_gpio_init() it results in an
> error. Isn't the error the better alternative?

Unless the serial driver polls the modem control line status, which
the SA1100 driver continues to do in exactly the same way after this
conversion.

Do you suggest that we just regress the driver by ripping out this
support that no one has had any problems with, and that is known to
work sufficiently in its day, just because we now don't like it?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] serial: sa1100: add support for mctrl gpios

[Uwe Kleine-König]

On Fri, May 31, 2019 at 03:01:28PM +0100, Russell King - ARM Linux admin wrote:
> On Fri, May 31, 2019 at 03:56:58PM +0200, Uwe Kleine-König wrote:
> > Unless I miss something (which is quite possible given that it's quite
> > some time ago I looked into mctrl_gpio) with mctrl_gpio_init_noauto()
> > having a CTS-gpio is just ignored unless the modem ctrl lines are
> > explicitely requestet while with mctrl_gpio_init() it results in an
> > error. Isn't the error the better alternative?
> 
> Unless the serial driver polls the modem control line status, which
> the SA1100 driver continues to do in exactly the same way after this
> conversion.
> 
> Do you suggest that we just regress the driver by ripping out this
> support that no one has had any problems with, and that is known to
> work sufficiently in its day, just because we now don't like it?

No, of course not. A nice improvement would be to teach gpio_mctrl (or
serial core?) about polling. But this is of course out of scope for this
patch, so I suggest to stay with mctrl_gpio_init_noauto and document the
lack of irq-capability somewhere prominently such that someone who picks
up converting mctrl_gpio_init_noauto to mctrl_gpio_init notices this
problem before actually hitting it.

Best regards
Uwe

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

Re: [PATCH 1/6] serial: sa1100: add support for mctrl gpios

[Uwe Kleine-König]

On Fri, May 31, 2019 at 03:01:28PM +0100, Russell King - ARM Linux admin wrote:
> On Fri, May 31, 2019 at 03:56:58PM +0200, Uwe Kleine-König wrote:
> > Unless I miss something (which is quite possible given that it's quite
> > some time ago I looked into mctrl_gpio) with mctrl_gpio_init_noauto()
> > having a CTS-gpio is just ignored unless the modem ctrl lines are
> > explicitely requestet while with mctrl_gpio_init() it results in an
> > error. Isn't the error the better alternative?
> 
> Unless the serial driver polls the modem control line status, which
> the SA1100 driver continues to do in exactly the same way after this
> conversion.
> 
> Do you suggest that we just regress the driver by ripping out this
> support that no one has had any problems with, and that is known to
> work sufficiently in its day, just because we now don't like it?

No, of course not. A nice improvement would be to teach gpio_mctrl (or
serial core?) about polling. But this is of course out of scope for this
patch, so I suggest to stay with mctrl_gpio_init_noauto and document the
lack of irq-capability somewhere prominently such that someone who picks
up converting mctrl_gpio_init_noauto to mctrl_gpio_init notices this
problem before actually hitting it.

Best regards
Uwe

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] serial: sa1100: add note about modem control signals

[Russell King]

As suggested by Uwe, add a note indicating that the modem control
signals do not support interrupts, which precludes the driver from
using mctrl_gpio_init().

Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---

Uwe, something like this?

 drivers/tty/serial/sa1100.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
index a6b4309b62fc..d7dbf0576be8 100644
--- a/drivers/tty/serial/sa1100.c
+++ b/drivers/tty/serial/sa1100.c
@@ -949,6 +949,9 @@ static int sa1100_serial_resume(struct platform_device *dev)
 static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
 {
 	sport->port.dev = &dev->dev;
+
+	// GPIO driver does not support interrupts for these modem
+	// control signals, so the serial driver polls them.
 	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);
 	if (IS_ERR(sport->gpios)) {
 		int err = PTR_ERR(sport->gpios);
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] serial: sa1100: add note about modem control signals

[Russell King]

As suggested by Uwe, add a note indicating that the modem control
signals do not support interrupts, which precludes the driver from
using mctrl_gpio_init().

Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---

Uwe, something like this?

 drivers/tty/serial/sa1100.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
index a6b4309b62fc..d7dbf0576be8 100644
--- a/drivers/tty/serial/sa1100.c
+++ b/drivers/tty/serial/sa1100.c
@@ -949,6 +949,9 @@ static int sa1100_serial_resume(struct platform_device *dev)
 static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
 {
 	sport->port.dev = &dev->dev;
+
+	// GPIO driver does not support interrupts for these modem
+	// control signals, so the serial driver polls them.
 	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);
 	if (IS_ERR(sport->gpios)) {
 		int err = PTR_ERR(sport->gpios);
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] serial: sa1100: add note about modem control signals

[Uwe Kleine-König]

Hello Russell,

On Fri, May 31, 2019 at 04:24:04PM +0100, Russell King wrote:
> As suggested by Uwe, add a note indicating that the modem control
> signals do not support interrupts, which precludes the driver from
> using mctrl_gpio_init().
> 
> Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> 
> Uwe, something like this?
> 
>  drivers/tty/serial/sa1100.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
> index a6b4309b62fc..d7dbf0576be8 100644
> --- a/drivers/tty/serial/sa1100.c
> +++ b/drivers/tty/serial/sa1100.c
> @@ -949,6 +949,9 @@ static int sa1100_serial_resume(struct platform_device *dev)
>  static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
>  {
>  	sport->port.dev = &dev->dev;
> +
> +	// GPIO driver does not support interrupts for these modem
> +	// control signals, so the serial driver polls them.
>  	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);
>  	if (IS_ERR(sport->gpios)) {
>  		int err = PTR_ERR(sport->gpios);

I would mention that because of this mctrl_gpio_init cannot be used.
(And I would have expected C-like comments).

Best regards
Uwe

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

Re: [PATCH] serial: sa1100: add note about modem control signals

[Uwe Kleine-König]

Hello Russell,

On Fri, May 31, 2019 at 04:24:04PM +0100, Russell King wrote:
> As suggested by Uwe, add a note indicating that the modem control
> signals do not support interrupts, which precludes the driver from
> using mctrl_gpio_init().
> 
> Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> 
> Uwe, something like this?
> 
>  drivers/tty/serial/sa1100.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
> index a6b4309b62fc..d7dbf0576be8 100644
> --- a/drivers/tty/serial/sa1100.c
> +++ b/drivers/tty/serial/sa1100.c
> @@ -949,6 +949,9 @@ static int sa1100_serial_resume(struct platform_device *dev)
>  static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
>  {
>  	sport->port.dev = &dev->dev;
> +
> +	// GPIO driver does not support interrupts for these modem
> +	// control signals, so the serial driver polls them.
>  	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);
>  	if (IS_ERR(sport->gpios)) {
>  		int err = PTR_ERR(sport->gpios);

I would mention that because of this mctrl_gpio_init cannot be used.
(And I would have expected C-like comments).

Best regards
Uwe

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] serial: sa1100: add note about modem control signals

[Russell King - ARM Linux admin]

On Fri, May 31, 2019 at 05:31:35PM +0200, Uwe Kleine-König wrote:
> Hello Russell,
> 
> On Fri, May 31, 2019 at 04:24:04PM +0100, Russell King wrote:
> > As suggested by Uwe, add a note indicating that the modem control
> > signals do not support interrupts, which precludes the driver from
> > using mctrl_gpio_init().
> > 
> > Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> > 
> > Uwe, something like this?
> > 
> >  drivers/tty/serial/sa1100.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
> > index a6b4309b62fc..d7dbf0576be8 100644
> > --- a/drivers/tty/serial/sa1100.c
> > +++ b/drivers/tty/serial/sa1100.c
> > @@ -949,6 +949,9 @@ static int sa1100_serial_resume(struct platform_device *dev)
> >  static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
> >  {
> >  	sport->port.dev = &dev->dev;
> > +
> > +	// GPIO driver does not support interrupts for these modem
> > +	// control signals, so the serial driver polls them.
> >  	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);
> >  	if (IS_ERR(sport->gpios)) {
> >  		int err = PTR_ERR(sport->gpios);
> 
> I would mention that because of this mctrl_gpio_init cannot be used.

Ok.

> (And I would have expected C-like comments).

It seems that Linus has changed his opinion on C99 commenting style,
from "it's not real C" to "I prefer it":

https://lore.kernel.org/patchwork/patch/852060/

Yes, this is mostly about the SPDX commenting style, but Linus seems to
be expressing a general preference towards "//" style comments over
"/* */" comments, especially for new comments.

Specifically see replies #14 and #18, especially #18 where the
discussion moves towards trailing-line comments for structure members.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH] serial: sa1100: add note about modem control signals

[Russell King - ARM Linux admin]

On Fri, May 31, 2019 at 05:31:35PM +0200, Uwe Kleine-König wrote:
> Hello Russell,
> 
> On Fri, May 31, 2019 at 04:24:04PM +0100, Russell King wrote:
> > As suggested by Uwe, add a note indicating that the modem control
> > signals do not support interrupts, which precludes the driver from
> > using mctrl_gpio_init().
> > 
> > Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> > 
> > Uwe, something like this?
> > 
> >  drivers/tty/serial/sa1100.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
> > index a6b4309b62fc..d7dbf0576be8 100644
> > --- a/drivers/tty/serial/sa1100.c
> > +++ b/drivers/tty/serial/sa1100.c
> > @@ -949,6 +949,9 @@ static int sa1100_serial_resume(struct platform_device *dev)
> >  static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
> >  {
> >  	sport->port.dev = &dev->dev;
> > +
> > +	// GPIO driver does not support interrupts for these modem
> > +	// control signals, so the serial driver polls them.
> >  	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);
> >  	if (IS_ERR(sport->gpios)) {
> >  		int err = PTR_ERR(sport->gpios);
> 
> I would mention that because of this mctrl_gpio_init cannot be used.

Ok.

> (And I would have expected C-like comments).

It seems that Linus has changed his opinion on C99 commenting style,
from "it's not real C" to "I prefer it":

https://lore.kernel.org/patchwork/patch/852060/

Yes, this is mostly about the SPDX commenting style, but Linus seems to
be expressing a general preference towards "//" style comments over
"/* */" comments, especially for new comments.

Specifically see replies #14 and #18, especially #18 where the
discussion moves towards trailing-line comments for structure members.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2] serial: sa1100: add note about modem control signals

[Russell King]

As suggested by Uwe, add a note indicating that the modem control
signals do not support interrupts, which precludes the driver from
using mctrl_gpio_init().

Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/tty/serial/sa1100.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
index 97bdfeccbea9..8e618129e65c 100644
--- a/drivers/tty/serial/sa1100.c
+++ b/drivers/tty/serial/sa1100.c
@@ -860,6 +860,10 @@ static int sa1100_serial_resume(struct platform_device *dev)
 static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
 {
 	sport->port.dev = &dev->dev;
+
+	// mctrl_gpio_init() requires that the GPIO driver supports interrupts,
+	// but we need to support GPIO drivers for hardware that has no such
+	// interrupts.  Use mctrl_gpio_init_noauto() instead.
 	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);
 	if (IS_ERR(sport->gpios)) {
 		int err = PTR_ERR(sport->gpios);
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] serial: sa1100: add note about modem control signals

[Uwe Kleine-König]

Hello Russell,

On Fri, May 31, 2019 at 05:01:42PM +0100, Russell King wrote:
> As suggested by Uwe, add a note indicating that the modem control
> signals do not support interrupts, which precludes the driver from
> using mctrl_gpio_init().
> 
> Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/tty/serial/sa1100.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
> index 97bdfeccbea9..8e618129e65c 100644
> --- a/drivers/tty/serial/sa1100.c
> +++ b/drivers/tty/serial/sa1100.c
> @@ -860,6 +860,10 @@ static int sa1100_serial_resume(struct platform_device *dev)
>  static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
>  {
>  	sport->port.dev = &dev->dev;
> +
> +	// mctrl_gpio_init() requires that the GPIO driver supports interrupts,
> +	// but we need to support GPIO drivers for hardware that has no such
> +	// interrupts.  Use mctrl_gpio_init_noauto() instead.

I hope it's not an impostor who claimed to be Linus to spread deviance
from K&R :-)

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

If you want to, squash this in the commit that introduced
mctrl_gpio_init_noauto while keeping my Ack on the resulting patch.

Best regards
Uwe

>  	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);
>  	if (IS_ERR(sport->gpios)) {
>  		int err = PTR_ERR(sport->gpios);
> -- 
> 2.7.4
> 
> 

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

Re: [PATCH v2] serial: sa1100: add note about modem control signals

[Uwe Kleine-König]

Hello Russell,

On Fri, May 31, 2019 at 05:01:42PM +0100, Russell King wrote:
> As suggested by Uwe, add a note indicating that the modem control
> signals do not support interrupts, which precludes the driver from
> using mctrl_gpio_init().
> 
> Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/tty/serial/sa1100.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
> index 97bdfeccbea9..8e618129e65c 100644
> --- a/drivers/tty/serial/sa1100.c
> +++ b/drivers/tty/serial/sa1100.c
> @@ -860,6 +860,10 @@ static int sa1100_serial_resume(struct platform_device *dev)
>  static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
>  {
>  	sport->port.dev = &dev->dev;
> +
> +	// mctrl_gpio_init() requires that the GPIO driver supports interrupts,
> +	// but we need to support GPIO drivers for hardware that has no such
> +	// interrupts.  Use mctrl_gpio_init_noauto() instead.

I hope it's not an impostor who claimed to be Linus to spread deviance
from K&R :-)

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

If you want to, squash this in the commit that introduced
mctrl_gpio_init_noauto while keeping my Ack on the resulting patch.

Best regards
Uwe

>  	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);
>  	if (IS_ERR(sport->gpios)) {
>  		int err = PTR_ERR(sport->gpios);
> -- 
> 2.7.4
> 
> 

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] serial: sa1100: add note about modem control signals

[Russell King - ARM Linux admin]

On Fri, May 31, 2019 at 11:27:02PM +0200, Uwe Kleine-König wrote:
> Hello Russell,
> 
> On Fri, May 31, 2019 at 05:01:42PM +0100, Russell King wrote:
> > As suggested by Uwe, add a note indicating that the modem control
> > signals do not support interrupts, which precludes the driver from
> > using mctrl_gpio_init().
> > 
> > Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> >  drivers/tty/serial/sa1100.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
> > index 97bdfeccbea9..8e618129e65c 100644
> > --- a/drivers/tty/serial/sa1100.c
> > +++ b/drivers/tty/serial/sa1100.c
> > @@ -860,6 +860,10 @@ static int sa1100_serial_resume(struct platform_device *dev)
> >  static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
> >  {
> >  	sport->port.dev = &dev->dev;
> > +
> > +	// mctrl_gpio_init() requires that the GPIO driver supports interrupts,
> > +	// but we need to support GPIO drivers for hardware that has no such
> > +	// interrupts.  Use mctrl_gpio_init_noauto() instead.
> 
> I hope it's not an impostor who claimed to be Linus to spread deviance
> from K&R :-)
> 
> Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> If you want to, squash this in the commit that introduced
> mctrl_gpio_init_noauto while keeping my Ack on the resulting patch.

I'd prefer to keep it separate to avoid invalidating the acks that I
already have on the first patch.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH v2] serial: sa1100: add note about modem control signals

[Russell King - ARM Linux admin]

On Fri, May 31, 2019 at 11:27:02PM +0200, Uwe Kleine-König wrote:
> Hello Russell,
> 
> On Fri, May 31, 2019 at 05:01:42PM +0100, Russell King wrote:
> > As suggested by Uwe, add a note indicating that the modem control
> > signals do not support interrupts, which precludes the driver from
> > using mctrl_gpio_init().
> > 
> > Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> >  drivers/tty/serial/sa1100.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
> > index 97bdfeccbea9..8e618129e65c 100644
> > --- a/drivers/tty/serial/sa1100.c
> > +++ b/drivers/tty/serial/sa1100.c
> > @@ -860,6 +860,10 @@ static int sa1100_serial_resume(struct platform_device *dev)
> >  static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
> >  {
> >  	sport->port.dev = &dev->dev;
> > +
> > +	// mctrl_gpio_init() requires that the GPIO driver supports interrupts,
> > +	// but we need to support GPIO drivers for hardware that has no such
> > +	// interrupts.  Use mctrl_gpio_init_noauto() instead.
> 
> I hope it's not an impostor who claimed to be Linus to spread deviance
> from K&R :-)
> 
> Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> If you want to, squash this in the commit that introduced
> mctrl_gpio_init_noauto while keeping my Ack on the resulting patch.

I'd prefer to keep it separate to avoid invalidating the acks that I
already have on the first patch.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] serial: sa1100: add support for mctrl gpios

[Greg Kroah-Hartman]

On Mon, Aug 29, 2016 at 01:05:51PM +0100, Russell King wrote:
> Add support for the generic mctrl gpio helper.  This will allow us to
> convert several board files to use the gpiod tables to assign GPIOs to
> serial ports, rather than needing to have private function callbacks.
> 
> If the generic mctrl gpio helper fails, ignore the mctrl gpios rather
> than preventing the (possibly console) serial port from being created.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/tty/serial/Kconfig  |  1 +
>  drivers/tty/serial/sa1100.c | 42 ++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 39 insertions(+), 4 deletions(-)

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[PATCH 1/6] serial: sa1100: add support for mctrl gpios

[Greg Kroah-Hartman]

On Mon, Aug 29, 2016 at 01:05:51PM +0100, Russell King wrote:
> Add support for the generic mctrl gpio helper.  This will allow us to
> convert several board files to use the gpiod tables to assign GPIOs to
> serial ports, rather than needing to have private function callbacks.
> 
> If the generic mctrl gpio helper fails, ignore the mctrl gpios rather
> than preventing the (possibly console) serial port from being created.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/tty/serial/Kconfig  |  1 +
>  drivers/tty/serial/sa1100.c | 42 ++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 39 insertions(+), 4 deletions(-)

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[PATCH 1/6] serial: sa1100: add support for mctrl gpios

[Russell King]

Add support for the generic mctrl gpio helper.  This will allow us to
convert several board files to use the gpiod tables to assign GPIOs to
serial ports, rather than needing to have private function callbacks.

If the generic mctrl gpio helper fails, ignore the mctrl gpios rather
than preventing the (possibly console) serial port from being created.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/tty/serial/Kconfig  |  1 +
 drivers/tty/serial/sa1100.c | 42 ++++++++++++++++++++++++++++++++++++++----
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 518db24a5b36..528d2ea3c7e7 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -464,6 +464,7 @@ config SERIAL_SA1100
 	bool "SA1100 serial port support"
 	depends on ARCH_SA1100
 	select SERIAL_CORE
+	select SERIAL_MCTRL_GPIO if GPIOLIB
 	help
 	  If you have a machine based on a SA1100/SA1110 StrongARM(R) CPU you
 	  can enable its onboard serial port by enabling this option.
diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
index fd3d1329d48c..ce1f9a3fb36d 100644
--- a/drivers/tty/serial/sa1100.c
+++ b/drivers/tty/serial/sa1100.c
@@ -41,6 +41,8 @@
 #include <mach/hardware.h>
 #include <mach/irqs.h>
 
+#include "serial_mctrl_gpio.h"
+
 /* We've been assigned a range on the "Low-density serial ports" major */
 #define SERIAL_SA1100_MAJOR	204
 #define MINOR_START		5
@@ -90,6 +92,7 @@ struct sa1100_port {
 	struct uart_port	port;
 	struct timer_list	timer;
 	unsigned int		old_status;
+	struct mctrl_gpios	*gpios;
 };
 
 /*
@@ -187,6 +190,8 @@ static void sa1100_enable_ms(struct uart_port *port)
 		container_of(port, struct sa1100_port, port);
 
 	mod_timer(&sport->timer, jiffies);
+
+	mctrl_gpio_enable_ms(sport->gpios);
 }
 
 static void
@@ -335,11 +340,21 @@ static unsigned int sa1100_tx_empty(struct uart_port *port)
 
 static unsigned int sa1100_get_mctrl(struct uart_port *port)
 {
-	return TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
+	struct sa1100_port *sport =
+		container_of(port, struct sa1100_port, port);
+	int ret = TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
+
+	mctrl_gpio_get(sport->gpios, &ret);
+
+	return ret;
 }
 
 static void sa1100_set_mctrl(struct uart_port *port, unsigned int mctrl)
 {
+	struct sa1100_port *sport =
+		container_of(port, struct sa1100_port, port);
+
+	mctrl_gpio_set(sport->gpios, mctrl);
 }
 
 /*
@@ -857,6 +872,27 @@ static int sa1100_serial_resume(struct platform_device *dev)
 	return 0;
 }
 
+static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
+{
+	sport->port.dev = &dev->dev;
+	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);
+	if (IS_ERR(sport->gpios)) {
+		int err = PTR_ERR(sport->gpios);
+
+		dev_err(sport->port.dev, "failed to get mctrl gpios: %d\n",
+			err);
+
+		if (err == -EPROBE_DEFER)
+			return err;
+
+		sport->gpios = NULL;
+	}
+
+	platform_set_drvdata(dev, sport);
+
+	return uart_add_one_port(&sa1100_reg, &sport->port);
+}
+
 static int sa1100_serial_probe(struct platform_device *dev)
 {
 	struct resource *res = dev->resource;
@@ -871,9 +907,7 @@ static int sa1100_serial_probe(struct platform_device *dev)
 			if (sa1100_ports[i].port.mapbase != res->start)
 				continue;
 
-			sa1100_ports[i].port.dev = &dev->dev;
-			uart_add_one_port(&sa1100_reg, &sa1100_ports[i].port);
-			platform_set_drvdata(dev, &sa1100_ports[i]);
+			sa1100_serial_add_one_port(&sa1100_ports[i], dev);
 			break;
 		}
 	}
-- 
2.1.0

[PATCH 1/6] serial: sa1100: add support for mctrl gpios

[Russell King]

Add support for the generic mctrl gpio helper.  This will allow us to
convert several board files to use the gpiod tables to assign GPIOs to
serial ports, rather than needing to have private function callbacks.

If the generic mctrl gpio helper fails, ignore the mctrl gpios rather
than preventing the (possibly console) serial port from being created.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/tty/serial/Kconfig  |  1 +
 drivers/tty/serial/sa1100.c | 42 ++++++++++++++++++++++++++++++++++++++----
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 518db24a5b36..528d2ea3c7e7 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -464,6 +464,7 @@ config SERIAL_SA1100
 	bool "SA1100 serial port support"
 	depends on ARCH_SA1100
 	select SERIAL_CORE
+	select SERIAL_MCTRL_GPIO if GPIOLIB
 	help
 	  If you have a machine based on a SA1100/SA1110 StrongARM(R) CPU you
 	  can enable its onboard serial port by enabling this option.
diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
index fd3d1329d48c..ce1f9a3fb36d 100644
--- a/drivers/tty/serial/sa1100.c
+++ b/drivers/tty/serial/sa1100.c
@@ -41,6 +41,8 @@
 #include <mach/hardware.h>
 #include <mach/irqs.h>
 
+#include "serial_mctrl_gpio.h"
+
 /* We've been assigned a range on the "Low-density serial ports" major */
 #define SERIAL_SA1100_MAJOR	204
 #define MINOR_START		5
@@ -90,6 +92,7 @@ struct sa1100_port {
 	struct uart_port	port;
 	struct timer_list	timer;
 	unsigned int		old_status;
+	struct mctrl_gpios	*gpios;
 };
 
 /*
@@ -187,6 +190,8 @@ static void sa1100_enable_ms(struct uart_port *port)
 		container_of(port, struct sa1100_port, port);
 
 	mod_timer(&sport->timer, jiffies);
+
+	mctrl_gpio_enable_ms(sport->gpios);
 }
 
 static void
@@ -335,11 +340,21 @@ static unsigned int sa1100_tx_empty(struct uart_port *port)
 
 static unsigned int sa1100_get_mctrl(struct uart_port *port)
 {
-	return TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
+	struct sa1100_port *sport =
+		container_of(port, struct sa1100_port, port);
+	int ret = TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
+
+	mctrl_gpio_get(sport->gpios, &ret);
+
+	return ret;
 }
 
 static void sa1100_set_mctrl(struct uart_port *port, unsigned int mctrl)
 {
+	struct sa1100_port *sport =
+		container_of(port, struct sa1100_port, port);
+
+	mctrl_gpio_set(sport->gpios, mctrl);
 }
 
 /*
@@ -857,6 +872,27 @@ static int sa1100_serial_resume(struct platform_device *dev)
 	return 0;
 }
 
+static int sa1100_serial_add_one_port(struct sa1100_port *sport, struct platform_device *dev)
+{
+	sport->port.dev = &dev->dev;
+	sport->gpios = mctrl_gpio_init_noauto(sport->port.dev, 0);
+	if (IS_ERR(sport->gpios)) {
+		int err = PTR_ERR(sport->gpios);
+
+		dev_err(sport->port.dev, "failed to get mctrl gpios: %d\n",
+			err);
+
+		if (err == -EPROBE_DEFER)
+			return err;
+
+		sport->gpios = NULL;
+	}
+
+	platform_set_drvdata(dev, sport);
+
+	return uart_add_one_port(&sa1100_reg, &sport->port);
+}
+
 static int sa1100_serial_probe(struct platform_device *dev)
 {
 	struct resource *res = dev->resource;
@@ -871,9 +907,7 @@ static int sa1100_serial_probe(struct platform_device *dev)
 			if (sa1100_ports[i].port.mapbase != res->start)
 				continue;
 
-			sa1100_ports[i].port.dev = &dev->dev;
-			uart_add_one_port(&sa1100_reg, &sa1100_ports[i].port);
-			platform_set_drvdata(dev, &sa1100_ports[i]);
+			sa1100_serial_add_one_port(&sa1100_ports[i], dev);
 			break;
 		}
 	}
-- 
2.1.0