[PATCH] serial: imx: add rx and tx led trigger

[PATCH] serial: imx: add rx and tx led trigger

[Uwe Kleine-König]

Add support for two led triggers per UART instance that blink on
transmission and reception of data respectively.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/tty/serial/imx.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 621e488cbb12..2b6ba3b8bdd5 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -39,6 +39,7 @@
 #include <linux/of_device.h>
 #include <linux/io.h>
 #include <linux/dma-mapping.h>
+#include <linux/leds.h>
 
 #include <asm/irq.h>
 #include <linux/platform_data/serial-imx.h>
@@ -227,6 +228,8 @@ struct imx_port {
 	wait_queue_head_t	dma_wait;
 	unsigned int            saved_reg[10];
 	bool			context_saved;
+	struct led_trigger	led_trigger_rx;
+	struct led_trigger	led_trigger_tx;
 };
 
 struct imx_port_ucrs {
@@ -293,6 +296,10 @@ static inline int is_imx6q_uart(struct imx_port *sport)
 {
 	return sport->devdata->devtype == IMX6Q_UART;
 }
+
+static unsigned long led_delay = 50;
+module_param(led_delay, ulong, 0644);
+
 /*
  * Save and restore functions for UCR1, UCR2 and UCR3 registers
  */
@@ -426,6 +433,9 @@ static inline void imx_transmit_buffer(struct imx_port *sport)
 		return;
 	}
 
+	led_trigger_blink_oneshot(&sport->led_trigger_tx,
+				  &led_delay, &led_delay, 1);
+
 	if (sport->dma_is_enabled) {
 		/*
 		 * We've just sent a X-char Ensure the TX DMA is enabled
@@ -635,6 +645,9 @@ static irqreturn_t imx_rxint(int irq, void *dev_id)
 	struct tty_port *port = &sport->port.state->port;
 	unsigned long flags, temp;
 
+	led_trigger_blink_oneshot(&sport->led_trigger_rx,
+				  &led_delay, &led_delay, 1);
+
 	spin_lock_irqsave(&sport->port.lock, flags);
 
 	while (readl(sport->port.membase + USR2) & USR2_RDR) {
@@ -708,6 +721,9 @@ static void imx_dma_rxint(struct imx_port *sport)
 	unsigned long temp;
 	unsigned long flags;
 
+	led_trigger_blink_oneshot(&sport->led_trigger_rx,
+				  &led_delay, &led_delay, 1);
+
 	spin_lock_irqsave(&sport->port.lock, flags);
 
 	temp = readl(sport->port.membase + USR2);
@@ -2002,6 +2018,42 @@ static void serial_imx_probe_pdata(struct imx_port *sport,
 		sport->have_rtscts = 1;
 }
 
+static void imx_register_led_trigger(struct device *dev, struct imx_port *sport)
+{
+#ifdef CONFIG_LEDS_TRIGGERS
+	int ret;
+	char *name;
+
+	name = devm_kasprintf(dev, GFP_KERNEL, "ttymxc%d-rx|ttymxc%d-tx",
+			      sport->port.line, sport->port.line);
+	if (!name) {
+		dev_warn(dev, "failed to allocate led trigger name\n");
+		return;
+	}
+
+	sport->led_trigger_rx.name = name;
+
+	name = strchr(name, '|');
+	*name = '\0';
+
+	sport->led_trigger_tx.name = name + 1;
+
+	ret = led_trigger_register(&sport->led_trigger_rx);
+	if (ret) {
+		dev_warn(dev, "failed to register rx led trigger\n");
+		goto err_register_rx;
+	}
+
+	ret = led_trigger_register(&sport->led_trigger_tx);
+	if (ret) {
+		dev_warn(dev, "failed to register tx led trigger\n");
+		led_trigger_unregister(&sport->led_trigger_rx);
+err_register_rx:
+		devm_kfree(dev, name);
+	}
+#endif
+}
+
 static int serial_imx_probe(struct platform_device *pdev)
 {
 	struct imx_port *sport;
@@ -2065,6 +2117,8 @@ static int serial_imx_probe(struct platform_device *pdev)
 
 	sport->port.uartclk = clk_get_rate(sport->clk_per);
 
+	imx_register_led_trigger(&pdev->dev, sport);
+
 	/* For register access, we only need to enable the ipg clock. */
 	ret = clk_prepare_enable(sport->clk_ipg);
 	if (ret)
@@ -2110,6 +2164,9 @@ static int serial_imx_remove(struct platform_device *pdev)
 {
 	struct imx_port *sport = platform_get_drvdata(pdev);
 
+#ifdef CONFIG_LEDS_TRIGGERS
+	led_trigger_unregister(&sport->led_trigger_rx);
+#endif
 	return uart_remove_one_port(&imx_reg, &sport->port);
 }
 
-- 
2.8.1


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

[PATCH] serial: imx: add rx and tx led trigger

[Uwe Kleine-König]

Add support for two led triggers per UART instance that blink on
transmission and reception of data respectively.

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
 drivers/tty/serial/imx.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 621e488cbb12..2b6ba3b8bdd5 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -39,6 +39,7 @@
 #include <linux/of_device.h>
 #include <linux/io.h>
 #include <linux/dma-mapping.h>
+#include <linux/leds.h>
 
 #include <asm/irq.h>
 #include <linux/platform_data/serial-imx.h>
@@ -227,6 +228,8 @@ struct imx_port {
 	wait_queue_head_t	dma_wait;
 	unsigned int            saved_reg[10];
 	bool			context_saved;
+	struct led_trigger	led_trigger_rx;
+	struct led_trigger	led_trigger_tx;
 };
 
 struct imx_port_ucrs {
@@ -293,6 +296,10 @@ static inline int is_imx6q_uart(struct imx_port *sport)
 {
 	return sport->devdata->devtype == IMX6Q_UART;
 }
+
+static unsigned long led_delay = 50;
+module_param(led_delay, ulong, 0644);
+
 /*
  * Save and restore functions for UCR1, UCR2 and UCR3 registers
  */
@@ -426,6 +433,9 @@ static inline void imx_transmit_buffer(struct imx_port *sport)
 		return;
 	}
 
+	led_trigger_blink_oneshot(&sport->led_trigger_tx,
+				  &led_delay, &led_delay, 1);
+
 	if (sport->dma_is_enabled) {
 		/*
 		 * We've just sent a X-char Ensure the TX DMA is enabled
@@ -635,6 +645,9 @@ static irqreturn_t imx_rxint(int irq, void *dev_id)
 	struct tty_port *port = &sport->port.state->port;
 	unsigned long flags, temp;
 
+	led_trigger_blink_oneshot(&sport->led_trigger_rx,
+				  &led_delay, &led_delay, 1);
+
 	spin_lock_irqsave(&sport->port.lock, flags);
 
 	while (readl(sport->port.membase + USR2) & USR2_RDR) {
@@ -708,6 +721,9 @@ static void imx_dma_rxint(struct imx_port *sport)
 	unsigned long temp;
 	unsigned long flags;
 
+	led_trigger_blink_oneshot(&sport->led_trigger_rx,
+				  &led_delay, &led_delay, 1);
+
 	spin_lock_irqsave(&sport->port.lock, flags);
 
 	temp = readl(sport->port.membase + USR2);
@@ -2002,6 +2018,42 @@ static void serial_imx_probe_pdata(struct imx_port *sport,
 		sport->have_rtscts = 1;
 }
 
+static void imx_register_led_trigger(struct device *dev, struct imx_port *sport)
+{
+#ifdef CONFIG_LEDS_TRIGGERS
+	int ret;
+	char *name;
+
+	name = devm_kasprintf(dev, GFP_KERNEL, "ttymxc%d-rx|ttymxc%d-tx",
+			      sport->port.line, sport->port.line);
+	if (!name) {
+		dev_warn(dev, "failed to allocate led trigger name\n");
+		return;
+	}
+
+	sport->led_trigger_rx.name = name;
+
+	name = strchr(name, '|');
+	*name = '\0';
+
+	sport->led_trigger_tx.name = name + 1;
+
+	ret = led_trigger_register(&sport->led_trigger_rx);
+	if (ret) {
+		dev_warn(dev, "failed to register rx led trigger\n");
+		goto err_register_rx;
+	}
+
+	ret = led_trigger_register(&sport->led_trigger_tx);
+	if (ret) {
+		dev_warn(dev, "failed to register tx led trigger\n");
+		led_trigger_unregister(&sport->led_trigger_rx);
+err_register_rx:
+		devm_kfree(dev, name);
+	}
+#endif
+}
+
 static int serial_imx_probe(struct platform_device *pdev)
 {
 	struct imx_port *sport;
@@ -2065,6 +2117,8 @@ static int serial_imx_probe(struct platform_device *pdev)
 
 	sport->port.uartclk = clk_get_rate(sport->clk_per);
 
+	imx_register_led_trigger(&pdev->dev, sport);
+
 	/* For register access, we only need to enable the ipg clock. */
 	ret = clk_prepare_enable(sport->clk_ipg);
 	if (ret)
@@ -2110,6 +2164,9 @@ static int serial_imx_remove(struct platform_device *pdev)
 {
 	struct imx_port *sport = platform_get_drvdata(pdev);
 
+#ifdef CONFIG_LEDS_TRIGGERS
+	led_trigger_unregister(&sport->led_trigger_rx);
+#endif
 	return uart_remove_one_port(&imx_reg, &sport->port);
 }
 
-- 
2.8.1

Re: [PATCH] serial: imx: add rx and tx led trigger

[Arnd Bergmann]

On Monday, July 4, 2016 5:34:12 PM CEST Uwe Kleine-König wrote:
> Add support for two led triggers per UART instance that blink on
> transmission and reception of data respectively.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/tty/serial/imx.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)

What is specific to this driver here? Could this be done in the
core tty code instead?

	Arnd

[PATCH] serial: imx: add rx and tx led trigger

[Arnd Bergmann]

On Monday, July 4, 2016 5:34:12 PM CEST Uwe Kleine-K?nig wrote:
> Add support for two led triggers per UART instance that blink on
> transmission and reception of data respectively.
> 
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/tty/serial/imx.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)

What is specific to this driver here? Could this be done in the
core tty code instead?

	Arnd

Re: [PATCH] serial: imx: add rx and tx led trigger

[Uwe Kleine-König]

On Mon, Jul 04, 2016 at 05:43:03PM +0200, Arnd Bergmann wrote:
> On Monday, July 4, 2016 5:34:12 PM CEST Uwe Kleine-König wrote:
> > Add support for two led triggers per UART instance that blink on
> > transmission and reception of data respectively.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  drivers/tty/serial/imx.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 57 insertions(+)
> 
> What is specific to this driver here? Could this be done in the
> core tty code instead?

The core doesn't notice when the driver starts to push out characters.

I also have a patch in my queue that adds support for rts delaying in
rs485 mode. While the delay between start_tx being called and the first
char leaving the hardware might not be big enough to care in rs232 mode,
with a big delay_rts_before_send it might matter.

Best regards
Uwe

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

[PATCH] serial: imx: add rx and tx led trigger

[Uwe Kleine-König]

On Mon, Jul 04, 2016 at 05:43:03PM +0200, Arnd Bergmann wrote:
> On Monday, July 4, 2016 5:34:12 PM CEST Uwe Kleine-K?nig wrote:
> > Add support for two led triggers per UART instance that blink on
> > transmission and reception of data respectively.
> > 
> > Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > ---
> >  drivers/tty/serial/imx.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 57 insertions(+)
> 
> What is specific to this driver here? Could this be done in the
> core tty code instead?

The core doesn't notice when the driver starts to push out characters.

I also have a patch in my queue that adds support for rts delaying in
rs485 mode. While the delay between start_tx being called and the first
char leaving the hardware might not be big enough to care in rs232 mode,
with a big delay_rts_before_send it might matter.

Best regards
Uwe

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

Re: [PATCH] serial: imx: add rx and tx led trigger

[Arnd Bergmann]

On Monday, July 4, 2016 5:50:09 PM CEST Uwe Kleine-König wrote:
> On Mon, Jul 04, 2016 at 05:43:03PM +0200, Arnd Bergmann wrote:
> > On Monday, July 4, 2016 5:34:12 PM CEST Uwe Kleine-König wrote:
> > > Add support for two led triggers per UART instance that blink on
> > > transmission and reception of data respectively.
> > > 
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > ---
> > >  drivers/tty/serial/imx.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 57 insertions(+)
> > 
> > What is specific to this driver here? Could this be done in the
> > core tty code instead?
> 
> The core doesn't notice when the driver starts to push out characters.
> 
> I also have a patch in my queue that adds support for rts delaying in
> rs485 mode. While the delay between start_tx being called and the first
> char leaving the hardware might not be big enough to care in rs232 mode,
> with a big delay_rts_before_send it might matter.

Right, that makes sense. It still seems odd to have this just in one
driver, and your changelog above doesn't really explain what the
blinkenlights are actually good for.

I'm sure you had something useful in mind, can you elaborate?

If this is something we may want to do on other platforms as well,
we should perhaps not hardwire the name of the imx tty device in
the led trigger name.

	Arnd

[PATCH] serial: imx: add rx and tx led trigger

[Arnd Bergmann]

On Monday, July 4, 2016 5:50:09 PM CEST Uwe Kleine-K?nig wrote:
> On Mon, Jul 04, 2016 at 05:43:03PM +0200, Arnd Bergmann wrote:
> > On Monday, July 4, 2016 5:34:12 PM CEST Uwe Kleine-K?nig wrote:
> > > Add support for two led triggers per UART instance that blink on
> > > transmission and reception of data respectively.
> > > 
> > > Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > > ---
> > >  drivers/tty/serial/imx.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 57 insertions(+)
> > 
> > What is specific to this driver here? Could this be done in the
> > core tty code instead?
> 
> The core doesn't notice when the driver starts to push out characters.
> 
> I also have a patch in my queue that adds support for rts delaying in
> rs485 mode. While the delay between start_tx being called and the first
> char leaving the hardware might not be big enough to care in rs232 mode,
> with a big delay_rts_before_send it might matter.

Right, that makes sense. It still seems odd to have this just in one
driver, and your changelog above doesn't really explain what the
blinkenlights are actually good for.

I'm sure you had something useful in mind, can you elaborate?

If this is something we may want to do on other platforms as well,
we should perhaps not hardwire the name of the imx tty device in
the led trigger name.

	Arnd

Re: [PATCH] serial: imx: add rx and tx led trigger

[Uwe Kleine-König]

On Wed, Jul 06, 2016 at 05:22:40PM +0200, Arnd Bergmann wrote:
> On Monday, July 4, 2016 5:50:09 PM CEST Uwe Kleine-König wrote:
> > On Mon, Jul 04, 2016 at 05:43:03PM +0200, Arnd Bergmann wrote:
> > > On Monday, July 4, 2016 5:34:12 PM CEST Uwe Kleine-König wrote:
> > > > Add support for two led triggers per UART instance that blink on
> > > > transmission and reception of data respectively.
> > > > 
> > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > > ---
> > > >  drivers/tty/serial/imx.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 57 insertions(+)
> > > 
> > > What is specific to this driver here? Could this be done in the
> > > core tty code instead?
> > 
> > The core doesn't notice when the driver starts to push out characters.
> > 
> > I also have a patch in my queue that adds support for rts delaying in
> > rs485 mode. While the delay between start_tx being called and the first
> > char leaving the hardware might not be big enough to care in rs232 mode,
> > with a big delay_rts_before_send it might matter.
> 
> Right, that makes sense. It still seems odd to have this just in one
> driver, and your changelog above doesn't really explain what the
> blinkenlights are actually good for.
> 
> I'm sure you had something useful in mind, can you elaborate?

I don't know the exact motivation. $customer wants to see when there is
traffic on the serial line. Is there a better motivation needed? If so,
what was the motivation for the mmc led trigger (which btw also used the
host's device name as trigger name).

> If this is something we may want to do on other platforms as well,
> we should perhaps not hardwire the name of the imx tty device in
> the led trigger name.

I cannot follow. If we have several serial lines and a trigger for each
of them, they must get different names. Using the device's name to
distinguish them seems like a good and obvious idea.

Best regards
Uwe

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

[PATCH] serial: imx: add rx and tx led trigger

[Uwe Kleine-König]

On Wed, Jul 06, 2016 at 05:22:40PM +0200, Arnd Bergmann wrote:
> On Monday, July 4, 2016 5:50:09 PM CEST Uwe Kleine-K?nig wrote:
> > On Mon, Jul 04, 2016 at 05:43:03PM +0200, Arnd Bergmann wrote:
> > > On Monday, July 4, 2016 5:34:12 PM CEST Uwe Kleine-K?nig wrote:
> > > > Add support for two led triggers per UART instance that blink on
> > > > transmission and reception of data respectively.
> > > > 
> > > > Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > > > ---
> > > >  drivers/tty/serial/imx.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 57 insertions(+)
> > > 
> > > What is specific to this driver here? Could this be done in the
> > > core tty code instead?
> > 
> > The core doesn't notice when the driver starts to push out characters.
> > 
> > I also have a patch in my queue that adds support for rts delaying in
> > rs485 mode. While the delay between start_tx being called and the first
> > char leaving the hardware might not be big enough to care in rs232 mode,
> > with a big delay_rts_before_send it might matter.
> 
> Right, that makes sense. It still seems odd to have this just in one
> driver, and your changelog above doesn't really explain what the
> blinkenlights are actually good for.
> 
> I'm sure you had something useful in mind, can you elaborate?

I don't know the exact motivation. $customer wants to see when there is
traffic on the serial line. Is there a better motivation needed? If so,
what was the motivation for the mmc led trigger (which btw also used the
host's device name as trigger name).

> If this is something we may want to do on other platforms as well,
> we should perhaps not hardwire the name of the imx tty device in
> the led trigger name.

I cannot follow. If we have several serial lines and a trigger for each
of them, they must get different names. Using the device's name to
distinguish them seems like a good and obvious idea.

Best regards
Uwe

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

Re: [PATCH] serial: imx: add rx and tx led trigger

[Arnd Bergmann]

On Wednesday, July 6, 2016 7:30:57 PM CEST Uwe Kleine-König wrote:
> On Wed, Jul 06, 2016 at 05:22:40PM +0200, Arnd Bergmann wrote:
> > On Monday, July 4, 2016 5:50:09 PM CEST Uwe Kleine-König wrote:
> > > On Mon, Jul 04, 2016 at 05:43:03PM +0200, Arnd Bergmann wrote:
> > > > On Monday, July 4, 2016 5:34:12 PM CEST Uwe Kleine-König wrote:
> > > > > Add support for two led triggers per UART instance that blink on
> > > > > transmission and reception of data respectively.
> > > > > 
> > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > > > ---
> > > > >  drivers/tty/serial/imx.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 57 insertions(+)
> > > > 
> > > > What is specific to this driver here? Could this be done in the
> > > > core tty code instead?
> > > 
> > > The core doesn't notice when the driver starts to push out characters.
> > > 
> > > I also have a patch in my queue that adds support for rts delaying in
> > > rs485 mode. While the delay between start_tx being called and the first
> > > char leaving the hardware might not be big enough to care in rs232 mode,
> > > with a big delay_rts_before_send it might matter.
> > 
> > Right, that makes sense. It still seems odd to have this just in one
> > driver, and your changelog above doesn't really explain what the
> > blinkenlights are actually good for.
> > 
> > I'm sure you had something useful in mind, can you elaborate?
> 
> I don't know the exact motivation. $customer wants to see when there is
> traffic on the serial line. Is there a better motivation needed?

I don't know, it depends a bit on how smart you think $customer is ;-)

Some customers know exactly what they need and why they ask for it,
some others are a bit confused about their own requirements.

I can certainly think of cases where this makes sense, e.g. a modem
appliance or a terminal server.

> If so, what was the motivation for the mmc led trigger (which btw also
> used the host's device name as trigger name).

I didn't see that patch.

> > If this is something we may want to do on other platforms as well,
> > we should perhaps not hardwire the name of the imx tty device in
> > the led trigger name.
> 
> I cannot follow. If we have several serial lines and a trigger for each
> of them, they must get different names. Using the device's name to
> distinguish them seems like a good and obvious idea.

The main problem I see is if someone puts the name of the trigger into
a dtb file, as this hardcodes the connection between the Linux driver
name and numbering system with the device tree binding, which are normally
separate.

If we could derive the trigger name from the "/aliases/serial%d" property
in DT instead, it would get a little more portable.

	Arnd

[PATCH] serial: imx: add rx and tx led trigger

[Arnd Bergmann]

On Wednesday, July 6, 2016 7:30:57 PM CEST Uwe Kleine-K?nig wrote:
> On Wed, Jul 06, 2016 at 05:22:40PM +0200, Arnd Bergmann wrote:
> > On Monday, July 4, 2016 5:50:09 PM CEST Uwe Kleine-K?nig wrote:
> > > On Mon, Jul 04, 2016 at 05:43:03PM +0200, Arnd Bergmann wrote:
> > > > On Monday, July 4, 2016 5:34:12 PM CEST Uwe Kleine-K?nig wrote:
> > > > > Add support for two led triggers per UART instance that blink on
> > > > > transmission and reception of data respectively.
> > > > > 
> > > > > Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > > > > ---
> > > > >  drivers/tty/serial/imx.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 57 insertions(+)
> > > > 
> > > > What is specific to this driver here? Could this be done in the
> > > > core tty code instead?
> > > 
> > > The core doesn't notice when the driver starts to push out characters.
> > > 
> > > I also have a patch in my queue that adds support for rts delaying in
> > > rs485 mode. While the delay between start_tx being called and the first
> > > char leaving the hardware might not be big enough to care in rs232 mode,
> > > with a big delay_rts_before_send it might matter.
> > 
> > Right, that makes sense. It still seems odd to have this just in one
> > driver, and your changelog above doesn't really explain what the
> > blinkenlights are actually good for.
> > 
> > I'm sure you had something useful in mind, can you elaborate?
> 
> I don't know the exact motivation. $customer wants to see when there is
> traffic on the serial line. Is there a better motivation needed?

I don't know, it depends a bit on how smart you think $customer is ;-)

Some customers know exactly what they need and why they ask for it,
some others are a bit confused about their own requirements.

I can certainly think of cases where this makes sense, e.g. a modem
appliance or a terminal server.

> If so, what was the motivation for the mmc led trigger (which btw also
> used the host's device name as trigger name).

I didn't see that patch.

> > If this is something we may want to do on other platforms as well,
> > we should perhaps not hardwire the name of the imx tty device in
> > the led trigger name.
> 
> I cannot follow. If we have several serial lines and a trigger for each
> of them, they must get different names. Using the device's name to
> distinguish them seems like a good and obvious idea.

The main problem I see is if someone puts the name of the trigger into
a dtb file, as this hardcodes the connection between the Linux driver
name and numbering system with the device tree binding, which are normally
separate.

If we could derive the trigger name from the "/aliases/serial%d" property
in DT instead, it would get a little more portable.

	Arnd

Re: [PATCH] serial: imx: add rx and tx led trigger

[Uwe Kleine-König]

Hello Arnd,

[adding Rob Herring to Cc]

On Wed, Jul 06, 2016 at 10:09:39PM +0200, Arnd Bergmann wrote:
> On Wednesday, July 6, 2016 7:30:57 PM CEST Uwe Kleine-König wrote:
> > On Wed, Jul 06, 2016 at 05:22:40PM +0200, Arnd Bergmann wrote:
> > > On Monday, July 4, 2016 5:50:09 PM CEST Uwe Kleine-König wrote:
> > > > On Mon, Jul 04, 2016 at 05:43:03PM +0200, Arnd Bergmann wrote:
> > > > > On Monday, July 4, 2016 5:34:12 PM CEST Uwe Kleine-König wrote:
> > > > > > Add support for two led triggers per UART instance that blink on
> > > > > > transmission and reception of data respectively.
> > > > > > 
> > > If this is something we may want to do on other platforms as well,
> > > we should perhaps not hardwire the name of the imx tty device in
> > > the led trigger name.
> > 
> > I cannot follow. If we have several serial lines and a trigger for each
> > of them, they must get different names. Using the device's name to
> > distinguish them seems like a good and obvious idea.
> 
> The main problem I see is if someone puts the name of the trigger into
> a dtb file, as this hardcodes the connection between the Linux driver
> name and numbering system with the device tree binding, which are normally
> separate.
> 
> If we could derive the trigger name from the "/aliases/serial%d" property
> in DT instead, it would get a little more portable.

Alternatively we could invent a more dtish way as aliases seem to be
frowned upon [1], something like:

	led#0 {
		label = "userled";
		linux,default-trigger = &uart1, "tx";
	};

	uart1: serial@43f90000 {
		...
		#trigger-cells = <1>;
	};

Having said that, I don't think it's a big problem if the value of
"linux,default-trigger" is Linux-specific. Moreover, is a default
trigger considered a hardware description?

Best regards
Uwe

[1] http://mid.gmane.org/20160705140546.GA10601@rob-hp-laptop
    Not sure this is a general objection to aliases, though.

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

[PATCH] serial: imx: add rx and tx led trigger

[Uwe Kleine-König]

Hello Arnd,

[adding Rob Herring to Cc]

On Wed, Jul 06, 2016 at 10:09:39PM +0200, Arnd Bergmann wrote:
> On Wednesday, July 6, 2016 7:30:57 PM CEST Uwe Kleine-K?nig wrote:
> > On Wed, Jul 06, 2016 at 05:22:40PM +0200, Arnd Bergmann wrote:
> > > On Monday, July 4, 2016 5:50:09 PM CEST Uwe Kleine-K?nig wrote:
> > > > On Mon, Jul 04, 2016 at 05:43:03PM +0200, Arnd Bergmann wrote:
> > > > > On Monday, July 4, 2016 5:34:12 PM CEST Uwe Kleine-K?nig wrote:
> > > > > > Add support for two led triggers per UART instance that blink on
> > > > > > transmission and reception of data respectively.
> > > > > > 
> > > If this is something we may want to do on other platforms as well,
> > > we should perhaps not hardwire the name of the imx tty device in
> > > the led trigger name.
> > 
> > I cannot follow. If we have several serial lines and a trigger for each
> > of them, they must get different names. Using the device's name to
> > distinguish them seems like a good and obvious idea.
> 
> The main problem I see is if someone puts the name of the trigger into
> a dtb file, as this hardcodes the connection between the Linux driver
> name and numbering system with the device tree binding, which are normally
> separate.
> 
> If we could derive the trigger name from the "/aliases/serial%d" property
> in DT instead, it would get a little more portable.

Alternatively we could invent a more dtish way as aliases seem to be
frowned upon [1], something like:

	led#0 {
		label = "userled";
		linux,default-trigger = &uart1, "tx";
	};

	uart1: serial at 43f90000 {
		...
		#trigger-cells = <1>;
	};

Having said that, I don't think it's a big problem if the value of
"linux,default-trigger" is Linux-specific. Moreover, is a default
trigger considered a hardware description?

Best regards
Uwe

[1] http://mid.gmane.org/20160705140546.GA10601 at rob-hp-laptop
    Not sure this is a general objection to aliases, though.

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

Re: [PATCH] serial: imx: add rx and tx led trigger

[Arnd Bergmann]

On Thursday, July 7, 2016 8:28:00 AM CEST Uwe Kleine-König wrote:
> Hello Arnd,
> 
> [adding Rob Herring to Cc]
> 
> On Wed, Jul 06, 2016 at 10:09:39PM +0200, Arnd Bergmann wrote:
> > On Wednesday, July 6, 2016 7:30:57 PM CEST Uwe Kleine-König wrote:
> > > On Wed, Jul 06, 2016 at 05:22:40PM +0200, Arnd Bergmann wrote:
> > > > On Monday, July 4, 2016 5:50:09 PM CEST Uwe Kleine-König wrote:
> > > > > On Mon, Jul 04, 2016 at 05:43:03PM +0200, Arnd Bergmann wrote:
> > > > > > On Monday, July 4, 2016 5:34:12 PM CEST Uwe Kleine-König wrote:
> > > > > > > Add support for two led triggers per UART instance that blink on
> > > > > > > transmission and reception of data respectively.
> > > > > > > 
> > > > If this is something we may want to do on other platforms as well,
> > > > we should perhaps not hardwire the name of the imx tty device in
> > > > the led trigger name.
> > > 
> > > I cannot follow. If we have several serial lines and a trigger for each
> > > of them, they must get different names. Using the device's name to
> > > distinguish them seems like a good and obvious idea.
> > 
> > The main problem I see is if someone puts the name of the trigger into
> > a dtb file, as this hardcodes the connection between the Linux driver
> > name and numbering system with the device tree binding, which are normally
> > separate.
> > 
> > If we could derive the trigger name from the "/aliases/serial%d" property
> > in DT instead, it would get a little more portable.
> 
> Alternatively we could invent a more dtish way as aliases seem to be
> frowned upon [1], something like:
> 
> 	led#0 {
> 		label = "userled";
> 		linux,default-trigger = &uart1, "tx";
> 	};
> 
> 	uart1: serial@43f90000 {
> 		...
> 		#trigger-cells = <1>;
> 	};

That looks nice, but I don't see how we could implement this in a
backwards compatible way, as we don't know whether the first cell
of the property is a phandle or a string.

> Having said that, I don't think it's a big problem if the value of
> "linux,default-trigger" is Linux-specific. Moreover, is a default
> trigger considered a hardware description?

What's more important here I think is that even in Linux, the name
of the uart is not always stable across versions or configurations,
e.g. in on OMAP we can have either ttyO%d or ttyS%d.

	Arnd

[PATCH] serial: imx: add rx and tx led trigger

[Arnd Bergmann]

On Thursday, July 7, 2016 8:28:00 AM CEST Uwe Kleine-K?nig wrote:
> Hello Arnd,
> 
> [adding Rob Herring to Cc]
> 
> On Wed, Jul 06, 2016 at 10:09:39PM +0200, Arnd Bergmann wrote:
> > On Wednesday, July 6, 2016 7:30:57 PM CEST Uwe Kleine-K?nig wrote:
> > > On Wed, Jul 06, 2016 at 05:22:40PM +0200, Arnd Bergmann wrote:
> > > > On Monday, July 4, 2016 5:50:09 PM CEST Uwe Kleine-K?nig wrote:
> > > > > On Mon, Jul 04, 2016 at 05:43:03PM +0200, Arnd Bergmann wrote:
> > > > > > On Monday, July 4, 2016 5:34:12 PM CEST Uwe Kleine-K?nig wrote:
> > > > > > > Add support for two led triggers per UART instance that blink on
> > > > > > > transmission and reception of data respectively.
> > > > > > > 
> > > > If this is something we may want to do on other platforms as well,
> > > > we should perhaps not hardwire the name of the imx tty device in
> > > > the led trigger name.
> > > 
> > > I cannot follow. If we have several serial lines and a trigger for each
> > > of them, they must get different names. Using the device's name to
> > > distinguish them seems like a good and obvious idea.
> > 
> > The main problem I see is if someone puts the name of the trigger into
> > a dtb file, as this hardcodes the connection between the Linux driver
> > name and numbering system with the device tree binding, which are normally
> > separate.
> > 
> > If we could derive the trigger name from the "/aliases/serial%d" property
> > in DT instead, it would get a little more portable.
> 
> Alternatively we could invent a more dtish way as aliases seem to be
> frowned upon [1], something like:
> 
> 	led#0 {
> 		label = "userled";
> 		linux,default-trigger = &uart1, "tx";
> 	};
> 
> 	uart1: serial at 43f90000 {
> 		...
> 		#trigger-cells = <1>;
> 	};

That looks nice, but I don't see how we could implement this in a
backwards compatible way, as we don't know whether the first cell
of the property is a phandle or a string.

> Having said that, I don't think it's a big problem if the value of
> "linux,default-trigger" is Linux-specific. Moreover, is a default
> trigger considered a hardware description?

What's more important here I think is that even in Linux, the name
of the uart is not always stable across versions or configurations,
e.g. in on OMAP we can have either ttyO%d or ttyS%d.

	Arnd

Re: [PATCH] serial: imx: add rx and tx led trigger

[Uwe Kleine-König]

Hello,

On Thu, Jul 07, 2016 at 10:27:51AM +0200, Arnd Bergmann wrote:
> On Thursday, July 7, 2016 8:28:00 AM CEST Uwe Kleine-König wrote:
> > On Wed, Jul 06, 2016 at 10:09:39PM +0200, Arnd Bergmann wrote:
> > > On Wednesday, July 6, 2016 7:30:57 PM CEST Uwe Kleine-König wrote:
> > > > On Wed, Jul 06, 2016 at 05:22:40PM +0200, Arnd Bergmann wrote:
> > > > > On Monday, July 4, 2016 5:50:09 PM CEST Uwe Kleine-König wrote:
> > > > > > On Mon, Jul 04, 2016 at 05:43:03PM +0200, Arnd Bergmann wrote:
> > > > > > > On Monday, July 4, 2016 5:34:12 PM CEST Uwe Kleine-König wrote:
> > > > > > > > Add support for two led triggers per UART instance that blink on
> > > > > > > > transmission and reception of data respectively.
> > > > > > > > 
> > > > > If this is something we may want to do on other platforms as well,
> > > > > we should perhaps not hardwire the name of the imx tty device in
> > > > > the led trigger name.
> > > > 
> > > > I cannot follow. If we have several serial lines and a trigger for each
> > > > of them, they must get different names. Using the device's name to
> > > > distinguish them seems like a good and obvious idea.
> > > 
> > > The main problem I see is if someone puts the name of the trigger into
> > > a dtb file, as this hardcodes the connection between the Linux driver
> > > name and numbering system with the device tree binding, which are normally
> > > separate.
> > > 
> > > If we could derive the trigger name from the "/aliases/serial%d" property
> > > in DT instead, it would get a little more portable.
> > 
> > Alternatively we could invent a more dtish way as aliases seem to be
> > frowned upon [1], something like:
> > 
> > 	led#0 {
> > 		label = "userled";
> > 		linux,default-trigger = &uart1, "tx";
> > 	};
> > 
> > 	uart1: serial@43f90000 {
> > 		...
> > 		#trigger-cells = <1>;
> > 	};
> 
> That looks nice, but I don't see how we could implement this in a
> backwards compatible way, as we don't know whether the first cell
> of the property is a phandle or a string.

If we agree, that this is OS-agnostic, we could use "default-trigger" as
property name instead of "linux,default-trigger".
 
Best regards
Uwe

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

[PATCH] serial: imx: add rx and tx led trigger

[Uwe Kleine-König]

Hello,

On Thu, Jul 07, 2016 at 10:27:51AM +0200, Arnd Bergmann wrote:
> On Thursday, July 7, 2016 8:28:00 AM CEST Uwe Kleine-K?nig wrote:
> > On Wed, Jul 06, 2016 at 10:09:39PM +0200, Arnd Bergmann wrote:
> > > On Wednesday, July 6, 2016 7:30:57 PM CEST Uwe Kleine-K?nig wrote:
> > > > On Wed, Jul 06, 2016 at 05:22:40PM +0200, Arnd Bergmann wrote:
> > > > > On Monday, July 4, 2016 5:50:09 PM CEST Uwe Kleine-K?nig wrote:
> > > > > > On Mon, Jul 04, 2016 at 05:43:03PM +0200, Arnd Bergmann wrote:
> > > > > > > On Monday, July 4, 2016 5:34:12 PM CEST Uwe Kleine-K?nig wrote:
> > > > > > > > Add support for two led triggers per UART instance that blink on
> > > > > > > > transmission and reception of data respectively.
> > > > > > > > 
> > > > > If this is something we may want to do on other platforms as well,
> > > > > we should perhaps not hardwire the name of the imx tty device in
> > > > > the led trigger name.
> > > > 
> > > > I cannot follow. If we have several serial lines and a trigger for each
> > > > of them, they must get different names. Using the device's name to
> > > > distinguish them seems like a good and obvious idea.
> > > 
> > > The main problem I see is if someone puts the name of the trigger into
> > > a dtb file, as this hardcodes the connection between the Linux driver
> > > name and numbering system with the device tree binding, which are normally
> > > separate.
> > > 
> > > If we could derive the trigger name from the "/aliases/serial%d" property
> > > in DT instead, it would get a little more portable.
> > 
> > Alternatively we could invent a more dtish way as aliases seem to be
> > frowned upon [1], something like:
> > 
> > 	led#0 {
> > 		label = "userled";
> > 		linux,default-trigger = &uart1, "tx";
> > 	};
> > 
> > 	uart1: serial at 43f90000 {
> > 		...
> > 		#trigger-cells = <1>;
> > 	};
> 
> That looks nice, but I don't see how we could implement this in a
> backwards compatible way, as we don't know whether the first cell
> of the property is a phandle or a string.

If we agree, that this is OS-agnostic, we could use "default-trigger" as
property name instead of "linux,default-trigger".
 
Best regards
Uwe

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

Re: [PATCH] serial: imx: add rx and tx led trigger

[Arnd Bergmann]

On Thursday, July 7, 2016 10:33:22 AM CEST Uwe Kleine-König wrote:
> On Thu, Jul 07, 2016 at 10:27:51AM +0200, Arnd Bergmann wrote:
> > On Thursday, July 7, 2016 8:28:00 AM CEST Uwe Kleine-König wrote:
> > > On Wed, Jul 06, 2016 at 10:09:39PM +0200, Arnd Bergmann wrote:
> > > > On Wednesday, July 6, 2016 7:30:57 PM CEST Uwe Kleine-König wrote:
> > > > > On Wed, Jul 06, 2016 at 05:22:40PM +0200, Arnd Bergmann wrote:
> > > > > > On Monday, July 4, 2016 5:50:09 PM CEST Uwe Kleine-König wrote:
> > > > > > > On Mon, Jul 04, 2016 at 05:43:03PM +0200, Arnd Bergmann wrote:
> > > > > > > > On Monday, July 4, 2016 5:34:12 PM CEST Uwe Kleine-König wrote:
> > > > > > > > > Add support for two led triggers per UART instance that blink on
> > > > > > > > > transmission and reception of data respectively.
> > > > > > > > > 
> > > > > > If this is something we may want to do on other platforms as well,
> > > > > > we should perhaps not hardwire the name of the imx tty device in
> > > > > > the led trigger name.
> > > > > 
> > > > > I cannot follow. If we have several serial lines and a trigger for each
> > > > > of them, they must get different names. Using the device's name to
> > > > > distinguish them seems like a good and obvious idea.
> > > > 
> > > > The main problem I see is if someone puts the name of the trigger into
> > > > a dtb file, as this hardcodes the connection between the Linux driver
> > > > name and numbering system with the device tree binding, which are normally
> > > > separate.
> > > > 
> > > > If we could derive the trigger name from the "/aliases/serial%d" property
> > > > in DT instead, it would get a little more portable.
> > > 
> > > Alternatively we could invent a more dtish way as aliases seem to be
> > > frowned upon [1], something like:
> > > 
> > >     led#0 {
> > >             label = "userled";
> > >             linux,default-trigger = &uart1, "tx";
> > >     };
> > > 
> > >     uart1: serial@43f90000 {
> > >             ...
> > >             #trigger-cells = <1>;
> > >     };
> > 
> > That looks nice, but I don't see how we could implement this in a
> > backwards compatible way, as we don't know whether the first cell
> > of the property is a phandle or a string.
> 
> If we agree, that this is OS-agnostic, we could use "default-trigger" as
> property name instead of "linux,default-trigger".

Yes, I think that can work, but why not just use linux,default-trigger="serial%d-tx"
where serial%d is the alias we already have?

	Arnd

[PATCH] serial: imx: add rx and tx led trigger

[Arnd Bergmann]

On Thursday, July 7, 2016 10:33:22 AM CEST Uwe Kleine-K?nig wrote:
> On Thu, Jul 07, 2016 at 10:27:51AM +0200, Arnd Bergmann wrote:
> > On Thursday, July 7, 2016 8:28:00 AM CEST Uwe Kleine-K?nig wrote:
> > > On Wed, Jul 06, 2016 at 10:09:39PM +0200, Arnd Bergmann wrote:
> > > > On Wednesday, July 6, 2016 7:30:57 PM CEST Uwe Kleine-K?nig wrote:
> > > > > On Wed, Jul 06, 2016 at 05:22:40PM +0200, Arnd Bergmann wrote:
> > > > > > On Monday, July 4, 2016 5:50:09 PM CEST Uwe Kleine-K?nig wrote:
> > > > > > > On Mon, Jul 04, 2016 at 05:43:03PM +0200, Arnd Bergmann wrote:
> > > > > > > > On Monday, July 4, 2016 5:34:12 PM CEST Uwe Kleine-K?nig wrote:
> > > > > > > > > Add support for two led triggers per UART instance that blink on
> > > > > > > > > transmission and reception of data respectively.
> > > > > > > > > 
> > > > > > If this is something we may want to do on other platforms as well,
> > > > > > we should perhaps not hardwire the name of the imx tty device in
> > > > > > the led trigger name.
> > > > > 
> > > > > I cannot follow. If we have several serial lines and a trigger for each
> > > > > of them, they must get different names. Using the device's name to
> > > > > distinguish them seems like a good and obvious idea.
> > > > 
> > > > The main problem I see is if someone puts the name of the trigger into
> > > > a dtb file, as this hardcodes the connection between the Linux driver
> > > > name and numbering system with the device tree binding, which are normally
> > > > separate.
> > > > 
> > > > If we could derive the trigger name from the "/aliases/serial%d" property
> > > > in DT instead, it would get a little more portable.
> > > 
> > > Alternatively we could invent a more dtish way as aliases seem to be
> > > frowned upon [1], something like:
> > > 
> > >     led#0 {
> > >             label = "userled";
> > >             linux,default-trigger = &uart1, "tx";
> > >     };
> > > 
> > >     uart1: serial at 43f90000 {
> > >             ...
> > >             #trigger-cells = <1>;
> > >     };
> > 
> > That looks nice, but I don't see how we could implement this in a
> > backwards compatible way, as we don't know whether the first cell
> > of the property is a phandle or a string.
> 
> If we agree, that this is OS-agnostic, we could use "default-trigger" as
> property name instead of "linux,default-trigger".

Yes, I think that can work, but why not just use linux,default-trigger="serial%d-tx"
where serial%d is the alias we already have?

	Arnd

Re: [PATCH] serial: imx: add rx and tx led trigger

[Greg KH]

On Mon, Jul 04, 2016 at 05:34:12PM +0200, Uwe Kleine-König wrote:
> Add support for two led triggers per UART instance that blink on
> transmission and reception of data respectively.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/tty/serial/imx.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
> 
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 621e488cbb12..2b6ba3b8bdd5 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -39,6 +39,7 @@
>  #include <linux/of_device.h>
>  #include <linux/io.h>
>  #include <linux/dma-mapping.h>
> +#include <linux/leds.h>
>  
>  #include <asm/irq.h>
>  #include <linux/platform_data/serial-imx.h>
> @@ -227,6 +228,8 @@ struct imx_port {
>  	wait_queue_head_t	dma_wait;
>  	unsigned int            saved_reg[10];
>  	bool			context_saved;
> +	struct led_trigger	led_trigger_rx;
> +	struct led_trigger	led_trigger_tx;
>  };
>  
>  struct imx_port_ucrs {
> @@ -293,6 +296,10 @@ static inline int is_imx6q_uart(struct imx_port *sport)
>  {
>  	return sport->devdata->devtype == IMX6Q_UART;
>  }
> +
> +static unsigned long led_delay = 50;
> +module_param(led_delay, ulong, 0644);

I hate module parameters, and so should you, this isn't the 1990's
anymore :(

And I'm with Arnd, let's make this work for all tty drivers that want to
use it please.

thanks,

greg k-h

[PATCH] serial: imx: add rx and tx led trigger

[Greg KH]

On Mon, Jul 04, 2016 at 05:34:12PM +0200, Uwe Kleine-K?nig wrote:
> Add support for two led triggers per UART instance that blink on
> transmission and reception of data respectively.
> 
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/tty/serial/imx.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
> 
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 621e488cbb12..2b6ba3b8bdd5 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -39,6 +39,7 @@
>  #include <linux/of_device.h>
>  #include <linux/io.h>
>  #include <linux/dma-mapping.h>
> +#include <linux/leds.h>
>  
>  #include <asm/irq.h>
>  #include <linux/platform_data/serial-imx.h>
> @@ -227,6 +228,8 @@ struct imx_port {
>  	wait_queue_head_t	dma_wait;
>  	unsigned int            saved_reg[10];
>  	bool			context_saved;
> +	struct led_trigger	led_trigger_rx;
> +	struct led_trigger	led_trigger_tx;
>  };
>  
>  struct imx_port_ucrs {
> @@ -293,6 +296,10 @@ static inline int is_imx6q_uart(struct imx_port *sport)
>  {
>  	return sport->devdata->devtype == IMX6Q_UART;
>  }
> +
> +static unsigned long led_delay = 50;
> +module_param(led_delay, ulong, 0644);

I hate module parameters, and so should you, this isn't the 1990's
anymore :(

And I'm with Arnd, let's make this work for all tty drivers that want to
use it please.

thanks,

greg k-h