[PATCH v2 0/6] serial: imx: handshaking fixes and improvments

[PATCH v2 0/6] serial: imx: handshaking fixes and improvments

[Uwe Kleine-König]

Hello,

this is the reworked series to address the handshaking problems I saw.

Among others it made my i.MX25 stuck when the other side toggles DCD.
The first 4 patches are fixes that I'd like to see in 4.6-rcX, the last two
patches are merge window material.

Best regards
Uwe

Uwe Kleine-König (6):
  serial: imx: fix polarity of RI
  serial: imx: let irq handler return IRQ_NONE if no event was handled
  serial: imx: make sure unhandled irqs are disabled
  serial: imx: only count 0->1 transitions for RNG
  serial: imx: reorder functions to simplify next patch
  serial: imx: implement DSR irq handling for DTE mode

 drivers/tty/serial/imx.c | 156 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 100 insertions(+), 56 deletions(-)

-- 
2.7.0


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

[PATCH v2 1/6] serial: imx: fix polarity of RI

[Uwe Kleine-König]

When in DTE mode, the bit USR2_RIIN is active low. So invert the logic
accordingly.

Fixes: 90ebc4838666 ("serial: imx: repair and complete handshaking")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/tty/serial/imx.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 231e7d5caf6c..bfc4555c63d1 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -797,9 +797,9 @@ static unsigned int imx_get_hwmctrl(struct imx_port *sport)
 	if (!(usr1 & USR2_DCDIN))
 		tmp |= TIOCM_CAR;
 
-	/* in DCE mode RIIN is always 0 */
-	if (readl(sport->port.membase + USR2) & USR2_RIIN)
-		tmp |= TIOCM_RI;
+	if (sport->dte_mode)
+		if (!(readl(sport->port.membase + USR2) & USR2_RIIN))
+			tmp |= TIOCM_RI;
 
 	return tmp;
 }
-- 
2.7.0


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

[PATCH v2 2/6] serial: imx: let irq handler return IRQ_NONE if no event was handled

[Uwe Kleine-König]

This gives the irq core a chance to disable the serial interrupt in case
an event isn't cleared in the handler.

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

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index bfc4555c63d1..5ced61e1de6a 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -734,6 +734,7 @@ static irqreturn_t imx_int(int irq, void *dev_id)
 	struct imx_port *sport = dev_id;
 	unsigned int sts;
 	unsigned int sts2;
+	irqreturn_t ret = IRQ_NONE;
 
 	sts = readl(sport->port.membase + USR1);
 	sts2 = readl(sport->port.membase + USR2);
@@ -743,26 +744,34 @@ static irqreturn_t imx_int(int irq, void *dev_id)
 			imx_dma_rxint(sport);
 		else
 			imx_rxint(irq, dev_id);
+		ret = IRQ_HANDLED;
 	}
 
 	if ((sts & USR1_TRDY &&
 	     readl(sport->port.membase + UCR1) & UCR1_TXMPTYEN) ||
 	    (sts2 & USR2_TXDC &&
-	     readl(sport->port.membase + UCR4) & UCR4_TCEN))
+	     readl(sport->port.membase + UCR4) & UCR4_TCEN)) {
 		imx_txint(irq, dev_id);
+		ret = IRQ_HANDLED;
+	}
 
-	if (sts & USR1_RTSD)
+	if (sts & USR1_RTSD) {
 		imx_rtsint(irq, dev_id);
+		ret = IRQ_HANDLED;
+	}
 
-	if (sts & USR1_AWAKE)
+	if (sts & USR1_AWAKE) {
 		writel(USR1_AWAKE, sport->port.membase + USR1);
+		ret = IRQ_HANDLED;
+	}
 
 	if (sts2 & USR2_ORE) {
 		sport->port.icount.overrun++;
 		writel(USR2_ORE, sport->port.membase + USR2);
+		ret = IRQ_HANDLED;
 	}
 
-	return IRQ_HANDLED;
+	return ret;
 }
 
 /*
-- 
2.7.0


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

[PATCH v2 3/6] serial: imx: make sure unhandled irqs are disabled

[Uwe Kleine-König]

Make sure that events that are not handled in the irq function don't
trigger an interrupt.

When the serial port is operated in DTE mode, the events for DCD and RI
events are enabled after a system reset by default.

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

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 5ced61e1de6a..fcd48fd78bf5 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1221,11 +1221,32 @@ static int imx_startup(struct uart_port *port)
 	temp |= (UCR2_RXEN | UCR2_TXEN);
 	if (!sport->have_rtscts)
 		temp |= UCR2_IRTS;
+	/*
+	 * make sure the edge sensitive RTS-irq is disabled,
+	 * we're using RTSD instead.
+	 */
+	if (!is_imx1_uart(sport))
+		temp &= ~UCR2_RTSEN;
 	writel(temp, sport->port.membase + UCR2);
 
 	if (!is_imx1_uart(sport)) {
 		temp = readl(sport->port.membase + UCR3);
-		temp |= IMX21_UCR3_RXDMUXSEL | UCR3_ADNIMP;
+
+		/*
+		 * The effect of RI and DCD differs depending on the UFCR_DCEDTE
+		 * bit. In DCE mode they control the outputs, in DTE mode they
+		 * enable the respective irqs. At least the DCD irq cannot be
+		 * cleared on i.MX25 at least, so it's not usable and must be
+		 * disabled. I don't have test hardware to check if RI has the
+		 * same problem but I consider this likely so it's disabled for
+		 * now, too.
+		 */
+		temp |= IMX21_UCR3_RXDMUXSEL | UCR3_ADNIMP |
+			UCR3_RI | UCR3_DCD;
+
+		if (sport->dte_mode)
+			temp &= ~(UCR3_RI | UCR3_DCD);
+
 		writel(temp, sport->port.membase + UCR3);
 	}
 
-- 
2.7.0


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

[PATCH v2 4/6] serial: imx: only count 0->1 transitions for RNG

[Uwe Kleine-König]

According to tty_ioctl(4) (from man-pages 4.04) the rng member only
counts 0->1 transitions. For the other signals (DSR, CD, CTS) both edges
are supposed to be counted.

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

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index fcd48fd78bf5..1bf7b86acf66 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -884,7 +884,7 @@ static void imx_mctrl_check(struct imx_port *sport)
 
 	sport->old_status = status;
 
-	if (changed & TIOCM_RI)
+	if (changed & TIOCM_RI && status & TIOCM_RI)
 		sport->port.icount.rng++;
 	if (changed & TIOCM_DSR)
 		sport->port.icount.dsr++;
-- 
2.7.0


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

[PATCH v2 5/6] serial: imx: reorder functions to simplify next patch

[Uwe Kleine-König]

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

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 1bf7b86acf66..c07e652f8f58 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -729,6 +729,55 @@ static void imx_dma_rxint(struct imx_port *sport)
 	spin_unlock_irqrestore(&sport->port.lock, flags);
 }
 
+/*
+ * We have a modem side uart, so the meanings of RTS and CTS are inverted.
+ */
+static unsigned int imx_get_hwmctrl(struct imx_port *sport)
+{
+	unsigned int tmp = TIOCM_DSR;
+	unsigned usr1 = readl(sport->port.membase + USR1);
+
+	if (usr1 & USR1_RTSS)
+		tmp |= TIOCM_CTS;
+
+	/* in DCE mode DCDIN is always 0 */
+	if (!(usr1 & USR2_DCDIN))
+		tmp |= TIOCM_CAR;
+
+	if (sport->dte_mode)
+		if (!(readl(sport->port.membase + USR2) & USR2_RIIN))
+			tmp |= TIOCM_RI;
+
+	return tmp;
+}
+
+/*
+ * Handle any change of modem status signal since we were last called.
+ */
+static void imx_mctrl_check(struct imx_port *sport)
+{
+	unsigned int status, changed;
+
+	status = imx_get_hwmctrl(sport);
+	changed = status ^ sport->old_status;
+
+	if (changed == 0)
+		return;
+
+	sport->old_status = status;
+
+	if (changed & TIOCM_RI && status & TIOCM_RI)
+		sport->port.icount.rng++;
+	if (changed & TIOCM_DSR)
+		sport->port.icount.dsr++;
+	if (changed & TIOCM_CAR)
+		uart_handle_dcd_change(&sport->port, status & TIOCM_CAR);
+	if (changed & TIOCM_CTS)
+		uart_handle_cts_change(&sport->port, status & TIOCM_CTS);
+
+	wake_up_interruptible(&sport->port.state->port.delta_msr_wait);
+}
+
 static irqreturn_t imx_int(int irq, void *dev_id)
 {
 	struct imx_port *sport = dev_id;
@@ -791,28 +840,6 @@ static unsigned int imx_tx_empty(struct uart_port *port)
 	return ret;
 }
 
-/*
- * We have a modem side uart, so the meanings of RTS and CTS are inverted.
- */
-static unsigned int imx_get_hwmctrl(struct imx_port *sport)
-{
-	unsigned int tmp = TIOCM_DSR;
-	unsigned usr1 = readl(sport->port.membase + USR1);
-
-	if (usr1 & USR1_RTSS)
-		tmp |= TIOCM_CTS;
-
-	/* in DCE mode DCDIN is always 0 */
-	if (!(usr1 & USR2_DCDIN))
-		tmp |= TIOCM_CAR;
-
-	if (sport->dte_mode)
-		if (!(readl(sport->port.membase + USR2) & USR2_RIIN))
-			tmp |= TIOCM_RI;
-
-	return tmp;
-}
-
 static unsigned int imx_get_mctrl(struct uart_port *port)
 {
 	struct imx_port *sport = (struct imx_port *)port;
@@ -870,33 +897,6 @@ static void imx_break_ctl(struct uart_port *port, int break_state)
 }
 
 /*
- * Handle any change of modem status signal since we were last called.
- */
-static void imx_mctrl_check(struct imx_port *sport)
-{
-	unsigned int status, changed;
-
-	status = imx_get_hwmctrl(sport);
-	changed = status ^ sport->old_status;
-
-	if (changed == 0)
-		return;
-
-	sport->old_status = status;
-
-	if (changed & TIOCM_RI && status & TIOCM_RI)
-		sport->port.icount.rng++;
-	if (changed & TIOCM_DSR)
-		sport->port.icount.dsr++;
-	if (changed & TIOCM_CAR)
-		uart_handle_dcd_change(&sport->port, status & TIOCM_CAR);
-	if (changed & TIOCM_CTS)
-		uart_handle_cts_change(&sport->port, status & TIOCM_CTS);
-
-	wake_up_interruptible(&sport->port.state->port.delta_msr_wait);
-}
-
-/*
  * This is our per-port timeout handler, for checking the
  * modem status signals.
  */
-- 
2.7.0


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

[PATCH v2 6/6] serial: imx: implement DSR irq handling for DTE mode

[Uwe Kleine-König]

Enable reporting of DSR events (which is named DTR in the registers
because Freescale uses the names as seem from a DCE).

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

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index c07e652f8f58..621e488cbb12 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -114,6 +114,7 @@
 #define UCR3_RXDSEN	(1<<6)	/* Receive status interrupt enable */
 #define UCR3_AIRINTEN	(1<<5)	/* Async IR wake interrupt enable */
 #define UCR3_AWAKEN	(1<<4)	/* Async wake interrupt enable */
+#define UCR3_DTRDEN	(1<<3)	/* Data Terminal Ready Delta Enable. */
 #define IMX21_UCR3_RXDMUXSEL	(1<<2)	/* RXD Muxed Input Select */
 #define UCR3_INVT	(1<<1)	/* Inverted Infrared transmission */
 #define UCR3_BPEN	(1<<0)	/* Preset registers enable */
@@ -142,7 +143,7 @@
 #define USR1_FRAMERR	(1<<10) /* Frame error interrupt flag */
 #define USR1_RRDY	(1<<9)	 /* Receiver ready interrupt/dma flag */
 #define USR1_AGTIM	(1<<8)	 /* Ageing timer interrupt flag */
-#define USR1_TIMEOUT	(1<<7)	 /* Receive timeout interrupt status */
+#define USR1_DTRD	(1<<7)	 /* DTR Delta */
 #define USR1_RXDS	 (1<<6)	 /* Receiver idle interrupt flag */
 #define USR1_AIRINT	 (1<<5)	 /* Async IR wake interrupt flag */
 #define USR1_AWAKE	 (1<<4)	 /* Aysnc wake interrupt flag */
@@ -804,6 +805,19 @@ static irqreturn_t imx_int(int irq, void *dev_id)
 		ret = IRQ_HANDLED;
 	}
 
+	if (sts & USR1_DTRD) {
+		unsigned long flags;
+
+		if (sts & USR1_DTRD)
+			writel(USR1_DTRD, sport->port.membase + USR1);
+
+		spin_lock_irqsave(&sport->port.lock, flags);
+		imx_mctrl_check(sport);
+		spin_unlock_irqrestore(&sport->port.lock, flags);
+
+		ret = IRQ_HANDLED;
+	}
+
 	if (sts & USR1_RTSD) {
 		imx_rtsint(irq, dev_id);
 		ret = IRQ_HANDLED;
@@ -1202,7 +1216,7 @@ static int imx_startup(struct uart_port *port)
 	/*
 	 * Finally, clear and enable interrupts
 	 */
-	writel(USR1_RTSD, sport->port.membase + USR1);
+	writel(USR1_RTSD | USR1_DTRD, sport->port.membase + USR1);
 	writel(USR2_ORE, sport->port.membase + USR2);
 
 	if (sport->dma_is_inited && !sport->dma_is_enabled)
@@ -1242,7 +1256,7 @@ static int imx_startup(struct uart_port *port)
 		 * now, too.
 		 */
 		temp |= IMX21_UCR3_RXDMUXSEL | UCR3_ADNIMP |
-			UCR3_RI | UCR3_DCD;
+			UCR3_DTRDEN | UCR3_RI | UCR3_DCD;
 
 		if (sport->dte_mode)
 			temp &= ~(UCR3_RI | UCR3_DCD);
-- 
2.7.0


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

Re: serial: imx: regression triggered by newly introduced DSR irq handling

[Uwe Kleine-König]

Hello Christoph,

Cc += Shawn, Fabio

On Fri, Aug 05, 2016 at 12:44:23AM +0200, Christoph Fritz wrote:
> here on a imx6sx board newly introduced DSR-irq-handling breaks serial
> console and network support.
> 
> Reverting commit 27e16501052e5341934d3 "serial: imx: implement DSR irq
> handling for DTE mode" fixes the issue.
> 
> The underlying cause is the RMII network configuration where register
> bit SION in IOMUXC_SW_MUX_CTL_PAD_ENET1_TX_CLK is necessary. But this
> also falsely feeds signal UART1_DTR_B and triggers DSR irq...
> 
> Can we revert this commit or add a quirk for RMII-SION configs or make
> this optional by a device tree option?

Hmm, MX6SX_PAD_ENET1_TX_CLK__ENET1_TX_CLK in
arch/arm/boot/dts/imx6sx-pinfunc.h doesn't have SION set.

I remember there were issues around eth and SION on i.MX6DL, but don't
know the details. A quick net research found
https://community.nxp.com/thread/359531 which has: "Set the SION bit.
Note that this is not required because the funtion setting controls the
signal path, but it is good practice as it reminds the user that the
clock needs to fed back into the Ethernet MAC.". Sounds like dangerous
smattering and a wrong expectation about "users".

https://community.nxp.com/thread/376821 is from someone who has the same
problem(?) on i.MX6SX, but no solution yet.

Does that mean MX6SX_PAD_ENET1_TX_CLK__ENET1_TX_CLK always needs the
SION bit set? Even if not, I'd like to have that documented there,
similar to da4fa6fa8016 ("ARM: imx25-pinfunc: document SION being
important for MX25_PAD_SD1_CMD__SD1_CMD").

Once this is done (and still an issue) I'd suggest to standardize a dt
property

	disable-dsr;

(and the same for the other handshaking lines) and support th{is,ese} in
the imx uart driver. Reverting 27e16501052e IMO isn't a sensible option.

Best regards
Uwe

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

Re: serial: imx: regression triggered by newly introduced DSR irq handling

[Christoph Fritz]

On Fri, 2016-08-05 at 08:58 +0200, Uwe Kleine-König wrote:
> On Fri, Aug 05, 2016 at 12:44:23AM +0200, Christoph Fritz wrote:
> > here on a imx6sx board newly introduced DSR-irq-handling breaks serial
> > console and network support.
> > 
> > Reverting commit 27e16501052e5341934d3 "serial: imx: implement DSR irq
> > handling for DTE mode" fixes the issue.
> > 
> > The underlying cause is the RMII network configuration where register
> > bit SION in IOMUXC_SW_MUX_CTL_PAD_ENET1_TX_CLK is necessary. But this
> > also falsely feeds signal UART1_DTR_B and triggers DSR irq...
> > 
> > Can we revert this commit or add a quirk for RMII-SION configs or make
> > this optional by a device tree option?
> 
> Hmm, MX6SX_PAD_ENET1_TX_CLK__ENET1_TX_CLK in
> arch/arm/boot/dts/imx6sx-pinfunc.h doesn't have SION set.
> 
> I remember there were issues around eth and SION on i.MX6DL, but don't
> know the details. A quick net research found
> https://community.nxp.com/thread/359531 which has: "Set the SION bit.
> Note that this is not required because the funtion setting controls the
> signal path, but it is good practice as it reminds the user that the
> clock needs to fed back into the Ethernet MAC.". Sounds like dangerous
> smattering and a wrong expectation about "users".
> 
> https://community.nxp.com/thread/376821 is from someone who has the same
> problem(?) on i.MX6SX, but no solution yet.
> 
> Does that mean MX6SX_PAD_ENET1_TX_CLK__ENET1_TX_CLK always needs the
> SION bit set?

SION is necessary for pinconfig MX6SX_PAD_ENET1_TX_CLK__ENET1_REF_CLK1
if bit 17 in IOMUXC_GPR_GPR1 is set and bit 13 not.

So that ref_enetpll1 provides a clock not only for the external PHY but
also for the internal controller.

Using SION is a quirk here, because the silicon doesn't feed the clock
back to the internal controller automatically.

On the other hand, NXP could argue: You need to add a wire on your PCB
between ENET1_REF_CLK1 and ENET1_TX_CLK -- but referring to the
datasheet ENET1_TX_CLK isn't used in RMII config...

So if Fabio or Shawn agrees with my assumption above, I'll add something
like 27e16501052e5341934d3 "serial: imx: implement DSR irq
handling for DTE mode".

Okay?


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

Re: serial: imx: regression triggered by newly introduced DSR irq handling

[Fabio Estevam]

Hi Christoph,

On Fri, Aug 5, 2016 at 9:03 AM, Christoph Fritz
<chf.fritz@googlemail.com> wrote:

> SION is necessary for pinconfig MX6SX_PAD_ENET1_TX_CLK__ENET1_REF_CLK1
> if bit 17 in IOMUXC_GPR_GPR1 is set and bit 13 not.
>
> So that ref_enetpll1 provides a clock not only for the external PHY but
> also for the internal controller.
>
> Using SION is a quirk here, because the silicon doesn't feed the clock
> back to the internal controller automatically.
>
> On the other hand, NXP could argue: You need to add a wire on your PCB
> between ENET1_REF_CLK1 and ENET1_TX_CLK -- but referring to the
> datasheet ENET1_TX_CLK isn't used in RMII config...
>
> So if Fabio or Shawn agrees with my assumption above, I'll add something
> like 27e16501052e5341934d3 "serial: imx: implement DSR irq
> handling for DTE mode".
>
> Okay?

Could you please post your suggestion as a patch or RFC so that we can
understand what your proposal is?

Thanks

Re: serial: imx: regression triggered by newly introduced DSR irq handling

[Christoph Fritz]

On Wed, 2016-08-10 at 17:54 -0300, Fabio Estevam wrote:
> On Fri, Aug 5, 2016 at 9:03 AM, Christoph Fritz
> <chf.fritz@googlemail.com> wrote:
> 
> > SION is necessary for pinconfig MX6SX_PAD_ENET1_TX_CLK__ENET1_REF_CLK1
> > if bit 17 in IOMUXC_GPR_GPR1 is set and bit 13 not.
> >
> > So that ref_enetpll1 provides a clock not only for the external PHY but
> > also for the internal controller.
> >
> > Using SION is a quirk here, because the silicon doesn't feed the clock
> > back to the internal controller automatically.
> >
> > On the other hand, NXP could argue: You need to add a wire on your PCB
> > between ENET1_REF_CLK1 and ENET1_TX_CLK -- but referring to the
> > datasheet ENET1_TX_CLK isn't used in RMII config...
> >
> > So if Fabio or Shawn agrees with my assumption above, I'll add something
> > like 27e16501052e5341934d3 "serial: imx: implement DSR irq
> > handling for DTE mode".

To correct myself, I'm referring to commit da4fa6fa8016dba54ae "ARM:
imx25-pinfunc: document SION being important for
MX25_PAD_SD1_CMD__SD1_CMD".

> >
> > Okay?
> 
> Could you please post your suggestion as a patch or RFC so that we can
> understand what your proposal is?

Sure, here is my proposal:

---
 arch/arm/boot/dts/imx6sx-pinfunc.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/imx6sx-pinfunc.h b/arch/arm/boot/dts/imx6sx-pinfunc.h
index bb9c6b7..67a3dd7 100644
--- a/arch/arm/boot/dts/imx6sx-pinfunc.h
+++ b/arch/arm/boot/dts/imx6sx-pinfunc.h
@@ -308,6 +308,19 @@
 #define MX6SX_PAD_ENET1_RX_CLK__VDEC_DEBUG_35                     0x008C 0x03D4 0x0000 0x8 0x0
 #define MX6SX_PAD_ENET1_RX_CLK__PCIE_CTRL_DEBUG_29                0x008C 0x03D4 0x0000 0x9 0x0
 #define MX6SX_PAD_ENET1_TX_CLK__ENET1_TX_CLK                      0x0090 0x03D8 0x0000 0x0 0x0
+/*
+ * SION bit is necessary for ENET1_REF_CLK1 (ENET2_REF_CLK2 untested) if it is
+ * used as clock output of IMX6SX_CLK_ENET_REF (ENET1_TX_CLK) to e.g. supply a
+ * PHY in RMII mode. This configuration is true if:
+ *  - bit 1 in field IMX6SX_GPR1_FEC_CLOCK_PAD_DIR_MASK is set
+ *  - bit 1 in field IMX6SX_GPR1_FEC_CLOCK_MUX_SEL_MASK unset
+ * It seems to be a silicon bug that in this configuration ENET1_TX reference
+ * clock isn't provided automatically.  According to i.mx6sx reference manual
+ * (IOMUXC_GPR_GPR1 field descriptions: ENET1_CLK_SEL, Rev. 0 from 2/2015) it
+ * should be the case.
+ * This might have side effects for other hardware units that are connected to
+ * that pin and use the respective function as input (e.g. DSR irq handling).
+ */
 #define MX6SX_PAD_ENET1_TX_CLK__ENET1_REF_CLK1                    0x0090 0x03D8 0x0760 0x1 0x1
 #define MX6SX_PAD_ENET1_TX_CLK__AUDMUX_AUD4_RXD                   0x0090 0x03D8 0x0644 0x2 0x1
 #define MX6SX_PAD_ENET1_TX_CLK__UART1_DTR_B                       0x0090 0x03D8 0x0000 0x3 0x0
--

Re: serial: imx: regression triggered by newly introduced DSR irq handling

[Uwe Kleine-König]

Hello Christoph,

On Sat, Aug 13, 2016 at 10:35:32PM +0200, Christoph Fritz wrote:
> On Wed, 2016-08-10 at 17:54 -0300, Fabio Estevam wrote:
> > Could you please post your suggestion as a patch or RFC so that we can
> > understand what your proposal is?
> 
> Sure, here is my proposal:

I like it, just a few minor nits below.

> +/*
> + * SION bit is necessary for ENET1_REF_CLK1 (ENET2_REF_CLK2 untested) if it is
> + * used as clock output of IMX6SX_CLK_ENET_REF (ENET1_TX_CLK) to e.g. supply a
> + * PHY in RMII mode. This configuration is true if:

Sounds a bit strage to me. Maybe s/true/necessary/? Or "valid"?

> + *  - bit 1 in field IMX6SX_GPR1_FEC_CLOCK_PAD_DIR_MASK is set
> + *  - bit 1 in field IMX6SX_GPR1_FEC_CLOCK_MUX_SEL_MASK unset
> + * It seems to be a silicon bug that in this configuration ENET1_TX reference
> + * clock isn't provided automatically.  According to i.mx6sx reference manual

s/i.mx6sx/i.MX6SX/

> + * (IOMUXC_GPR_GPR1 field descriptions: ENET1_CLK_SEL, Rev. 0 from 2/2015) it
> + * should be the case.
> + * This might have side effects for other hardware units that are connected to
> + * that pin and use the respective function as input (e.g. DSR irq handling).

s/DSR irq/UART1's DTR/. Then it's more obvious to relate to
MX6SX_PAD_ENET1_TX_CLK__UART1_DTR_B.

> + */
>  #define MX6SX_PAD_ENET1_TX_CLK__ENET1_REF_CLK1                    0x0090 0x03D8 0x0760 0x1 0x1
>  #define MX6SX_PAD_ENET1_TX_CLK__AUDMUX_AUD4_RXD                   0x0090 0x03D8 0x0644 0x2 0x1
>  #define MX6SX_PAD_ENET1_TX_CLK__UART1_DTR_B                       0x0090 0x03D8 0x0000 0x3 0x0

Thanks
Uwe

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

[PATCH] ARM: dts: imx6sx: document SION necessity of ENET1_REF_CLK1

[Christoph Fritz]

Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
---
 arch/arm/boot/dts/imx6sx-pinfunc.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/imx6sx-pinfunc.h b/arch/arm/boot/dts/imx6sx-pinfunc.h
index bb9c6b7..42c4c80 100644
--- a/arch/arm/boot/dts/imx6sx-pinfunc.h
+++ b/arch/arm/boot/dts/imx6sx-pinfunc.h
@@ -308,6 +308,20 @@
 #define MX6SX_PAD_ENET1_RX_CLK__VDEC_DEBUG_35                     0x008C 0x03D4 0x0000 0x8 0x0
 #define MX6SX_PAD_ENET1_RX_CLK__PCIE_CTRL_DEBUG_29                0x008C 0x03D4 0x0000 0x9 0x0
 #define MX6SX_PAD_ENET1_TX_CLK__ENET1_TX_CLK                      0x0090 0x03D8 0x0000 0x0 0x0
+/*
+ * SION bit is necessary for ENET1_REF_CLK1 (ENET2_REF_CLK2 untested) if it is
+ * used as clock output of IMX6SX_CLK_ENET_REF (ENET1_TX_CLK) to e.g. supply a
+ * PHY in RMII mode. This configuration is valid if:
+ *  - bit 1 in field IMX6SX_GPR1_FEC_CLOCK_PAD_DIR_MASK is set
+ *  - bit 1 in field IMX6SX_GPR1_FEC_CLOCK_MUX_SEL_MASK unset
+ * It seems to be a silicon bug that in this configuration ENET1_TX reference
+ * clock isn't provided automatically.  According to i.MX6SX reference manual
+ * (IOMUXC_GPR_GPR1 field descriptions: ENET1_CLK_SEL, Rev. 0 from 2/2015) it
+ * should be the case.
+ * So this might have unwanted side effects for other hardware units that are
+ * also connected to that pin and using respective function as input (e.g.
+ * UART1's DTR handling on MX6SX_PAD_ENET1_TX_CLK__UART1_DTR_B).
+ */
 #define MX6SX_PAD_ENET1_TX_CLK__ENET1_REF_CLK1                    0x0090 0x03D8 0x0760 0x1 0x1
 #define MX6SX_PAD_ENET1_TX_CLK__AUDMUX_AUD4_RXD                   0x0090 0x03D8 0x0644 0x2 0x1
 #define MX6SX_PAD_ENET1_TX_CLK__UART1_DTR_B                       0x0090 0x03D8 0x0000 0x3 0x0
-- 
2.1.4

Re: [PATCH] ARM: dts: imx6sx: document SION necessity of ENET1_REF_CLK1

[Fabio Estevam]

Hi Christoph,

On Wed, Aug 17, 2016 at 6:25 AM, Christoph Fritz
<chf.fritz@googlemail.com> wrote:

> +/*
> + * SION bit is necessary for ENET1_REF_CLK1 (ENET2_REF_CLK2 untested) if it is
> + * used as clock output of IMX6SX_CLK_ENET_REF (ENET1_TX_CLK) to e.g. supply a
> + * PHY in RMII mode. This configuration is valid if:
> + *  - bit 1 in field IMX6SX_GPR1_FEC_CLOCK_PAD_DIR_MASK is set
> + *  - bit 1 in field IMX6SX_GPR1_FEC_CLOCK_MUX_SEL_MASK unset

So in your case the imx6sx_enet_clk_sel() does not do what you need:

static void __init imx6sx_enet_clk_sel(void)
{
    struct regmap *gpr;

    gpr = syscon_regmap_lookup_by_compatible("fsl,imx6sx-iomuxc-gpr");
    if (!IS_ERR(gpr)) {
        regmap_update_bits(gpr, IOMUXC_GPR1,
                   IMX6SX_GPR1_FEC_CLOCK_MUX_SEL_MASK, 0);
        regmap_update_bits(gpr, IOMUXC_GPR1,
                   IMX6SX_GPR1_FEC_CLOCK_PAD_DIR_MASK, 0);
    } else {
        pr_err("failed to find fsl,imx6sx-iomux-gpr regmap\n");
    }
}

It seems that it is not a good idea to have imx6sx_enet_clk_sel() in
common code as the GPR1 setting can change from board to board.

I think we need the fec driver to be in charge of configuring the GPR1 register.

This is unrelated to your patch though :-)

Re: [PATCH] ARM: dts: imx6sx: document SION necessity of ENET1_REF_CLK1

[Christoph Fritz]

On Wed, 2016-08-17 at 11:26 -0300, Fabio Estevam wrote:
> Hi Christoph,
> 
> On Wed, Aug 17, 2016 at 6:25 AM, Christoph Fritz
> <chf.fritz@googlemail.com> wrote:
> 
> > +/*
> > + * SION bit is necessary for ENET1_REF_CLK1 (ENET2_REF_CLK2 untested) if it is
> > + * used as clock output of IMX6SX_CLK_ENET_REF (ENET1_TX_CLK) to e.g. supply a
> > + * PHY in RMII mode. This configuration is valid if:
> > + *  - bit 1 in field IMX6SX_GPR1_FEC_CLOCK_PAD_DIR_MASK is set
> > + *  - bit 1 in field IMX6SX_GPR1_FEC_CLOCK_MUX_SEL_MASK unset
> 
> So in your case the imx6sx_enet_clk_sel() does not do what you need:
> 
> static void __init imx6sx_enet_clk_sel(void)
> {
>     struct regmap *gpr;
> 
>     gpr = syscon_regmap_lookup_by_compatible("fsl,imx6sx-iomuxc-gpr");
>     if (!IS_ERR(gpr)) {
>         regmap_update_bits(gpr, IOMUXC_GPR1,
>                    IMX6SX_GPR1_FEC_CLOCK_MUX_SEL_MASK, 0);
>         regmap_update_bits(gpr, IOMUXC_GPR1,
>                    IMX6SX_GPR1_FEC_CLOCK_PAD_DIR_MASK, 0);
>     } else {
>         pr_err("failed to find fsl,imx6sx-iomux-gpr regmap\n");
>     }
> }
> 
> It seems that it is not a good idea to have imx6sx_enet_clk_sel() in
> common code as the GPR1 setting can change from board to board.

Yes, currently I'm adapting/quirking the fec config there and in
imx6sx_clocks_init() to set the Phy-Clock-Frequency to 50Mhz.

> I think we need the fec driver to be in charge of configuring the GPR1 register.

What about making the Phy-Clock-Frequency IMX6SX_CLK_ENET_REF
adjustable?

> This is unrelated to your patch though :-)

Thanks
 -- Christoph

Re: [PATCH] ARM: dts: imx6sx: document SION necessity of ENET1_REF_CLK1

[Christoph Fritz]

On Wed, 2016-08-17 at 11:25 +0200, Christoph Fritz wrote:
> Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
> ---
>  arch/arm/boot/dts/imx6sx-pinfunc.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx6sx-pinfunc.h b/arch/arm/boot/dts/imx6sx-pinfunc.h
> index bb9c6b7..42c4c80 100644
> --- a/arch/arm/boot/dts/imx6sx-pinfunc.h
> +++ b/arch/arm/boot/dts/imx6sx-pinfunc.h
> @@ -308,6 +308,20 @@
>  #define MX6SX_PAD_ENET1_RX_CLK__VDEC_DEBUG_35                     0x008C 0x03D4 0x0000 0x8 0x0
>  #define MX6SX_PAD_ENET1_RX_CLK__PCIE_CTRL_DEBUG_29                0x008C 0x03D4 0x0000 0x9 0x0
>  #define MX6SX_PAD_ENET1_TX_CLK__ENET1_TX_CLK                      0x0090 0x03D8 0x0000 0x0 0x0
> +/*
> + * SION bit is necessary for ENET1_REF_CLK1 (ENET2_REF_CLK2 untested) if it is
> + * used as clock output of IMX6SX_CLK_ENET_REF (ENET1_TX_CLK) to e.g. supply a
> + * PHY in RMII mode. This configuration is valid if:
> + *  - bit 1 in field IMX6SX_GPR1_FEC_CLOCK_PAD_DIR_MASK is set
> + *  - bit 1 in field IMX6SX_GPR1_FEC_CLOCK_MUX_SEL_MASK unset
> + * It seems to be a silicon bug that in this configuration ENET1_TX reference
> + * clock isn't provided automatically.  According to i.MX6SX reference manual
> + * (IOMUXC_GPR_GPR1 field descriptions: ENET1_CLK_SEL, Rev. 0 from 2/2015) it
> + * should be the case.
> + * So this might have unwanted side effects for other hardware units that are
> + * also connected to that pin and using respective function as input (e.g.
> + * UART1's DTR handling on MX6SX_PAD_ENET1_TX_CLK__UART1_DTR_B).
> + */
>  #define MX6SX_PAD_ENET1_TX_CLK__ENET1_REF_CLK1                    0x0090 0x03D8 0x0760 0x1 0x1
>  #define MX6SX_PAD_ENET1_TX_CLK__AUDMUX_AUD4_RXD                   0x0090 0x03D8 0x0644 0x2 0x1
>  #define MX6SX_PAD_ENET1_TX_CLK__UART1_DTR_B                       0x0090 0x03D8 0x0000 0x3 0x0

@Uwe and Fabio: Can I get your Ack on this?
@Shawn: Can you queue this patch for upstream?

Re: [PATCH] ARM: dts: imx6sx: document SION necessity of ENET1_REF_CLK1

[Shawn Guo]

On Wed, Aug 17, 2016 at 11:25:31AM +0200, Christoph Fritz wrote:
> 
> Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>

Applied, thanks.

> ---
>  arch/arm/boot/dts/imx6sx-pinfunc.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx6sx-pinfunc.h b/arch/arm/boot/dts/imx6sx-pinfunc.h
> index bb9c6b7..42c4c80 100644
> --- a/arch/arm/boot/dts/imx6sx-pinfunc.h
> +++ b/arch/arm/boot/dts/imx6sx-pinfunc.h
> @@ -308,6 +308,20 @@
>  #define MX6SX_PAD_ENET1_RX_CLK__VDEC_DEBUG_35                     0x008C 0x03D4 0x0000 0x8 0x0
>  #define MX6SX_PAD_ENET1_RX_CLK__PCIE_CTRL_DEBUG_29                0x008C 0x03D4 0x0000 0x9 0x0
>  #define MX6SX_PAD_ENET1_TX_CLK__ENET1_TX_CLK                      0x0090 0x03D8 0x0000 0x0 0x0
> +/*
> + * SION bit is necessary for ENET1_REF_CLK1 (ENET2_REF_CLK2 untested) if it is
> + * used as clock output of IMX6SX_CLK_ENET_REF (ENET1_TX_CLK) to e.g. supply a
> + * PHY in RMII mode. This configuration is valid if:
> + *  - bit 1 in field IMX6SX_GPR1_FEC_CLOCK_PAD_DIR_MASK is set
> + *  - bit 1 in field IMX6SX_GPR1_FEC_CLOCK_MUX_SEL_MASK unset
> + * It seems to be a silicon bug that in this configuration ENET1_TX reference
> + * clock isn't provided automatically.  According to i.MX6SX reference manual
> + * (IOMUXC_GPR_GPR1 field descriptions: ENET1_CLK_SEL, Rev. 0 from 2/2015) it
> + * should be the case.
> + * So this might have unwanted side effects for other hardware units that are
> + * also connected to that pin and using respective function as input (e.g.
> + * UART1's DTR handling on MX6SX_PAD_ENET1_TX_CLK__UART1_DTR_B).
> + */
>  #define MX6SX_PAD_ENET1_TX_CLK__ENET1_REF_CLK1                    0x0090 0x03D8 0x0760 0x1 0x1
>  #define MX6SX_PAD_ENET1_TX_CLK__AUDMUX_AUD4_RXD                   0x0090 0x03D8 0x0644 0x2 0x1
>  #define MX6SX_PAD_ENET1_TX_CLK__UART1_DTR_B                       0x0090 0x03D8 0x0000 0x3 0x0
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: serial: imx: regression triggered by newly introduced DSR irq handling

[Christoph Fritz]

Hey Uwe

On Fri, 2016-08-05 at 08:58 +0200, Uwe Kleine-König wrote:
> Once this is done (and still an issue)

Yes, patch b5ca028fe9ba2090a (ARM: dts: imx6sx: document SION necessity
of ENET1_REF_CLK1) is done and mainline and DSR irq handling is still an
issue.

>  I'd suggest to standardize a dt
> property
> 
> 	disable-dsr;

Would you go forward?

> (and the same for the other handshaking lines) and support th{is,ese} in
> the imx uart driver. Reverting 27e16501052e IMO isn't a sensible option.

Okay, I'm fine with a 'disable-dsr' solution.

Thanks
  -- Christoph



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

Re: serial: imx: regression triggered by newly introduced DSR irq handling

[Uwe Kleine-König]

Hello Christoph,

On Mon, Nov 21, 2016 at 12:00:11PM +0100, Christoph Fritz wrote:
> On Fri, 2016-08-05 at 08:58 +0200, Uwe Kleine-König wrote:
> > Once this is done (and still an issue)
> 
> Yes, patch b5ca028fe9ba2090a (ARM: dts: imx6sx: document SION necessity
> of ENET1_REF_CLK1) is done and mainline and DSR irq handling is still an
> issue.
> 
> >  I'd suggest to standardize a dt
> > property
> > 
> > 	disable-dsr;
> 
> Would you go forward?

Currently I don't have the need and time to do this myself. So if you
want to propose a patch and send if for review to the dt and serial MLs
that would be a good next step. If you add me to Cc (or To) I can take a
look.

Best regards
Uwe

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

Re: [PATCH v2 0/6] serial: imx: handshaking fixes and improvments

[Petr Štetiar]

Uwe Kleine-König <u.kleine-koenig@pengutronix.de> [2016-03-24 14:24:19]:

> Among others it made my i.MX25 stuck when the other side toggles DCD.
> The first 4 patches are fixes that I'd like to see in 4.6-rcX, the last two
> patches are merge window material.

Hi,

just FYI, my i.MX6 board has stopped booting over NFS on 4.6-rc1 and I've just
bisected it down to the commit "serial: imx: repair and complete handshaking"
90ebc48386, it's leading to the following CPU stall:

[   72.084538] INFO: rcu_sched self-detected stall on CPU
[   72.089719]  0-...: (1 GPs behind) idle=9bd/2/0 softirq=97/100 fqs=5996
[   72.096425]   (t=6000 jiffies g=-239 c=-240 q=47)
[   72.101154] Task dump for CPU 0:
[   72.104389] swapper/0       R running      0     0      0 0x00000002
[   72.110778] Backtrace:
[   72.113276] [<8010a580>] (dump_backtrace) from [<8010a778>] (show_stack+0x18/0x1c)
[   72.120850]  r7:80a09bc0 r6:80a02458 r5:80a05c00 r4:00000000
[   72.126589] [<8010a760>] (show_stack) from [<80137ee0>] (sched_show_task+0xb8/0xd8)
[   72.134258] [<80137e28>] (sched_show_task) from [<80139780>] (dump_cpu_task+0x34/0x44)
[   72.142177]  r5:00000000 r4:00000000
[   72.145806] [<8013974c>] (dump_cpu_task) from [<80157de8>] (rcu_dump_cpu_stacks+0x7c/0xa0)
[   72.154072]  r5:00000000 r4:80a09bc0
[   72.157697] [<80157d6c>] (rcu_dump_cpu_stacks) from [<8015ae74>] (rcu_check_callbacks+0x218/0x65c)
[   72.166661]  r9:80a024a0 r8:0000002f r7:80a09bc0 r6:80a02100 r5:ef1a61c0 r4:80a09bc0
[   72.174499] [<8015ac5c>] (rcu_check_callbacks) from [<8015d0d0>] (update_process_times+0x38/0x68)
[   72.183376]  r10:ffffffff r9:ef1a2b40 r8:80a2d567 r7:00000010 r6:c84d9643 r5:00000000
[   72.191283]  r4:80a05c00
[   72.193854] [<8015d098>] (update_process_times) from [<8016c6bc>] (tick_sched_handle+0x50/0x54)
[   72.202555]  r5:80a01e20 r4:ef1a2cb8
[   72.206177] [<8016c66c>] (tick_sched_handle) from [<8016c70c>] (tick_sched_timer+0x4c/0x8c)
[   72.214546] [<8016c6c0>] (tick_sched_timer) from [<8015da94>] (__hrtimer_run_queues+0xd0/0x17c)
[   72.223247]  r7:00000001 r6:ef1a2b4c r5:ef1a2cb8 r4:ef1a2b00
[   72.228979] [<8015d9c4>] (__hrtimer_run_queues) from [<8015e074>] (hrtimer_interrupt+0xb8/0x208)
[   72.237768]  r10:ffffffff r9:7fffffff r8:00000003 r7:6e858000 r6:80a01d48 r5:8094ab00
[   72.237768]  r10:ffffffff r9:7fffffff r8:00000003 r7:6e858000 r6:80a01d48 r5:8094ab00
[   72.245674]  r4:ef1a2b00
[   72.248240] [<8015dfbc>] (hrtimer_interrupt) from [<8010dadc>] (twd_handler+0x34/0x40)
[   72.256159]  r10:80a01e70 r9:8093da28 r8:00000010 r7:eec1cc40 r6:80a026dc r5:eec03c00
[   72.264067]  r4:00000001
[   72.266629] [<8010daa8>] (twd_handler) from [<80152ef4>] (handle_percpu_devid_irq+0x70/0x8c)
[   72.275069]  r5:eec03c00 r4:ef1a8840
[   72.278695] [<80152e84>] (handle_percpu_devid_irq) from [<8014ec68>] (generic_handle_irq+0x20/0x30)
[   72.287744]  r9:8093da28 r8:eec05000 r7:00000010 r6:8094b044 r5:80a01f28 r4:00000000
[   72.295579] [<8014ec48>] (generic_handle_irq) from [<8014ef80>] (__handle_domain_irq+0x94/0xbc)
[   72.304292] [<8014eeec>] (__handle_domain_irq) from [<80101408>] (gic_handle_irq+0x58/0x98)
[   72.312646]  r9:8093da28 r8:f4001100 r7:80a11c50 r6:80a01e20 r5:80a026dc r4:f4000100
[   72.320482] [<801013b0>] (gic_handle_irq) from [<8010b214>] (__irq_svc+0x54/0x70)
[   72.327971] Exception stack(0x80a01e20 to 0x80a01e68)
[   72.333036] 1e20: 00000000 00200000 00000000 00000000 80a00000 00000000 00000282 80a2e100
[   72.341227] 1e40: eec05000 8093da28 80a01e70 80a01ec4 80a01ec8 80a01e70 8011a2bc 80119ea0
[   72.349409] 1e60: 60000113 ffffffff
[   72.352902]  r9:8093da28 r8:eec05000 r7:80a01e54 r6:ffffffff r5:60000113 r4:80119ea0
[   72.360748] [<80119e2c>] (__do_softirq) from [<8011a2bc>] (irq_exit+0x8c/0xfc)
[   72.367974]  r10:00000000 r9:8093da28 r8:eec05000 r7:00000010 r6:8094b044 r5:00000000
[   72.375880]  r4:00000000
[   72.378446] [<8011a230>] (irq_exit) from [<8014ef84>] (__handle_domain_irq+0x98/0xbc)
[   72.386290] [<8014eeec>] (__handle_domain_irq) from [<80101408>] (gic_handle_irq+0x58/0x98)
[   72.394644]  r9:8093da28 r8:f4001100 r7:80a11c50 r6:80a01f28 r5:80a026dc r4:f4000100
[   72.402476] [<801013b0>] (gic_handle_irq) from [<8010b214>] (__irq_svc+0x54/0x70)
[   72.409964] Exception stack(0x80a01f28 to 0x80a01f70)
[   72.415027] 1f20:                   00000000 000012ce ef1a227c 80112b00 80a00000 80a02430
[   72.423215] 1f40: 80a02430 80a02380 efffc840 8093da28 00000000 80a01f84 80a01f88 80a01f78
[   72.431400] 1f60: 801079a4 801079a8 60000013 ffffffff
[   72.436455]  r9:8093da28 r8:efffc840 r7:80a01f5c r6:ffffffff r5:60000013 r4:801079a8
[   72.444289] [<80107974>] (arch_cpu_idle) from [<80148174>] (default_idle_call+0x30/0x34)
[   72.452395] [<80148144>] (default_idle_call) from [<8014826c>] (cpu_startup_entry+0xf4/0x15c)
[   72.460936] [<80148178>] (cpu_startup_entry) from [<806294a0>] (rest_init+0x68/0x80)
[   72.468699] [<80629438>] (rest_init) from [<80900c74>] (start_kernel+0x2fc/0x368)
[   72.476193] [<80900978>] (start_kernel) from [<1000807c>] (0x1000807c)
[   72.482724]  r9:412fc09a r8:1000406a r7:80a06c7c r6:8093da24 r5:80a023dc r4:80a2da14

To fix this problem, I've to revert the 90ebc48386 commit or apply this patch series.

-- ynezz

Re: [PATCH v2 0/6] serial: imx: handshaking fixes and improvments

[Uwe Kleine-König]

Hello Petr,

On Mon, Apr 11, 2016 at 06:01:28PM +0200, Petr Štetiar wrote:
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> [2016-03-24 14:24:19]:
> 
> > Among others it made my i.MX25 stuck when the other side toggles DCD.
> > The first 4 patches are fixes that I'd like to see in 4.6-rcX, the last two
> > patches are merge window material.
> 
> just FYI, my i.MX6 board has stopped booting over NFS on 4.6-rc1 and I've just
> bisected it down to the commit "serial: imx: repair and complete handshaking"
> 90ebc48386, it's leading to the following CPU stall:

hmm, I'm confused (because I don't understand it) and something between
happy and sad (because both 90ebc48386 and the series that fixes it are
mine). 

This is an i.MX6 Solo (i.e. only one cpu)?

> [   72.084538] INFO: rcu_sched self-detected stall on CPU
> [   72.089719]  0-...: (1 GPs behind) idle=9bd/2/0 softirq=97/100 fqs=5996
> [   72.096425]   (t=6000 jiffies g=-239 c=-240 q=47)
> [   72.101154] Task dump for CPU 0:
> [   72.104389] swapper/0       R running      0     0      0 0x00000002
> [   72.110778] Backtrace:
> [   72.113276] [<8010a580>] (dump_backtrace) from [<8010a778>] (show_stack+0x18/0x1c)
> [   72.120850]  r7:80a09bc0 r6:80a02458 r5:80a05c00 r4:00000000
> [   72.126589] [<8010a760>] (show_stack) from [<80137ee0>] (sched_show_task+0xb8/0xd8)
> [   72.134258] [<80137e28>] (sched_show_task) from [<80139780>] (dump_cpu_task+0x34/0x44)
> [   72.142177]  r5:00000000 r4:00000000
> [   72.145806] [<8013974c>] (dump_cpu_task) from [<80157de8>] (rcu_dump_cpu_stacks+0x7c/0xa0)
> [   72.154072]  r5:00000000 r4:80a09bc0
> [   72.157697] [<80157d6c>] (rcu_dump_cpu_stacks) from [<8015ae74>] (rcu_check_callbacks+0x218/0x65c)
> [   72.166661]  r9:80a024a0 r8:0000002f r7:80a09bc0 r6:80a02100 r5:ef1a61c0 r4:80a09bc0
> [   72.174499] [<8015ac5c>] (rcu_check_callbacks) from [<8015d0d0>] (update_process_times+0x38/0x68)
> [   72.183376]  r10:ffffffff r9:ef1a2b40 r8:80a2d567 r7:00000010 r6:c84d9643 r5:00000000
> [   72.191283]  r4:80a05c00
> [   72.193854] [<8015d098>] (update_process_times) from [<8016c6bc>] (tick_sched_handle+0x50/0x54)
> [   72.202555]  r5:80a01e20 r4:ef1a2cb8
> [   72.206177] [<8016c66c>] (tick_sched_handle) from [<8016c70c>] (tick_sched_timer+0x4c/0x8c)
> [   72.214546] [<8016c6c0>] (tick_sched_timer) from [<8015da94>] (__hrtimer_run_queues+0xd0/0x17c)
> [   72.223247]  r7:00000001 r6:ef1a2b4c r5:ef1a2cb8 r4:ef1a2b00
> [   72.228979] [<8015d9c4>] (__hrtimer_run_queues) from [<8015e074>] (hrtimer_interrupt+0xb8/0x208)
> [   72.237768]  r10:ffffffff r9:7fffffff r8:00000003 r7:6e858000 r6:80a01d48 r5:8094ab00
> [   72.237768]  r10:ffffffff r9:7fffffff r8:00000003 r7:6e858000 r6:80a01d48 r5:8094ab00
> [   72.245674]  r4:ef1a2b00
> [   72.248240] [<8015dfbc>] (hrtimer_interrupt) from [<8010dadc>] (twd_handler+0x34/0x40)
> [   72.256159]  r10:80a01e70 r9:8093da28 r8:00000010 r7:eec1cc40 r6:80a026dc r5:eec03c00
> [   72.264067]  r4:00000001
> [   72.266629] [<8010daa8>] (twd_handler) from [<80152ef4>] (handle_percpu_devid_irq+0x70/0x8c)
> [   72.275069]  r5:eec03c00 r4:ef1a8840
> [   72.278695] [<80152e84>] (handle_percpu_devid_irq) from [<8014ec68>] (generic_handle_irq+0x20/0x30)
> [   72.287744]  r9:8093da28 r8:eec05000 r7:00000010 r6:8094b044 r5:80a01f28 r4:00000000
> [   72.295579] [<8014ec48>] (generic_handle_irq) from [<8014ef80>] (__handle_domain_irq+0x94/0xbc)
> [   72.304292] [<8014eeec>] (__handle_domain_irq) from [<80101408>] (gic_handle_irq+0x58/0x98)
> [   72.312646]  r9:8093da28 r8:f4001100 r7:80a11c50 r6:80a01e20 r5:80a026dc r4:f4000100
> [   72.320482] [<801013b0>] (gic_handle_irq) from [<8010b214>] (__irq_svc+0x54/0x70)
> [   72.327971] Exception stack(0x80a01e20 to 0x80a01e68)
> [   72.333036] 1e20: 00000000 00200000 00000000 00000000 80a00000 00000000 00000282 80a2e100
> [   72.341227] 1e40: eec05000 8093da28 80a01e70 80a01ec4 80a01ec8 80a01e70 8011a2bc 80119ea0
> [   72.349409] 1e60: 60000113 ffffffff
> [   72.352902]  r9:8093da28 r8:eec05000 r7:80a01e54 r6:ffffffff r5:60000113 r4:80119ea0
> [   72.360748] [<80119e2c>] (__do_softirq) from [<8011a2bc>] (irq_exit+0x8c/0xfc)
> [   72.367974]  r10:00000000 r9:8093da28 r8:eec05000 r7:00000010 r6:8094b044 r5:00000000
> [   72.375880]  r4:00000000
> [   72.378446] [<8011a230>] (irq_exit) from [<8014ef84>] (__handle_domain_irq+0x98/0xbc)
> [   72.386290] [<8014eeec>] (__handle_domain_irq) from [<80101408>] (gic_handle_irq+0x58/0x98)
> [   72.394644]  r9:8093da28 r8:f4001100 r7:80a11c50 r6:80a01f28 r5:80a026dc r4:f4000100
> [   72.402476] [<801013b0>] (gic_handle_irq) from [<8010b214>] (__irq_svc+0x54/0x70)
> [   72.409964] Exception stack(0x80a01f28 to 0x80a01f70)
> [   72.415027] 1f20:                   00000000 000012ce ef1a227c 80112b00 80a00000 80a02430
> [   72.423215] 1f40: 80a02430 80a02380 efffc840 8093da28 00000000 80a01f84 80a01f88 80a01f78
> [   72.431400] 1f60: 801079a4 801079a8 60000013 ffffffff
> [   72.436455]  r9:8093da28 r8:efffc840 r7:80a01f5c r6:ffffffff r5:60000013 r4:801079a8
> [   72.444289] [<80107974>] (arch_cpu_idle) from [<80148174>] (default_idle_call+0x30/0x34)
> [   72.452395] [<80148144>] (default_idle_call) from [<8014826c>] (cpu_startup_entry+0xf4/0x15c)
> [   72.460936] [<80148178>] (cpu_startup_entry) from [<806294a0>] (rest_init+0x68/0x80)
> [   72.468699] [<80629438>] (rest_init) from [<80900c74>] (start_kernel+0x2fc/0x368)
> [   72.476193] [<80900978>] (start_kernel) from [<1000807c>] (0x1000807c)
> [   72.482724]  r9:412fc09a r8:1000406a r7:80a06c7c r6:8093da24 r5:80a023dc r4:80a2da14
> 
> To fix this problem, I've to revert the 90ebc48386 commit or apply this patch series.

Which patch fixes your problem? Is it enough to revert just the last
hunk of 90ebc48386? Do you use fsl,dte-mode for the uart in question?

IMHO patches 1-4 should get applied before 4.6.

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 v2 0/6] serial: imx: handshaking fixes and improvments

[Petr Štetiar]

Uwe Kleine-König <u.kleine-koenig@pengutronix.de> [2016-04-12 09:46:22]:

Hi,

> On Mon, Apr 11, 2016 at 06:01:28PM +0200, Petr Štetiar wrote:
> > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> [2016-03-24 14:24:19]:
> > 
> > > Among others it made my i.MX25 stuck when the other side toggles DCD.
> > > The first 4 patches are fixes that I'd like to see in 4.6-rcX, the last two
> > > patches are merge window material.
> > 
> > just FYI, my i.MX6 board has stopped booting over NFS on 4.6-rc1 and I've just
> > bisected it down to the commit "serial: imx: repair and complete handshaking"
> > 90ebc48386, it's leading to the following CPU stall:
> 
> hmm, I'm confused (because I don't understand it) and something between
> happy and sad (because both 90ebc48386 and the series that fixes it are
> mine).

I don't know what's going on, probably something related to the serial IRQs?

> This is an i.MX6 Solo (i.e. only one cpu)?

This is i.MX6 Quad, Toradex Apalis Ixora (imx6q-apalis-ixora.dts).

> Which patch fixes your problem?

First 3 patches:

  [v2,1/6] serial: imx: fix polarity of RI
  [v2,2/6] serial: imx: let irq handler return IRQ_NONE if no event was handled
  [v2,3/6] serial: imx: make sure unhandled irqs are disabled

> Is it enough to revert just the last hunk of 90ebc48386?

Yes, this fixes it also:

	diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
	index 231e7d5..dab081c 100644
	--- a/drivers/tty/serial/imx.c
	+++ b/drivers/tty/serial/imx.c
	@@ -827,11 +827,6 @@ static void imx_set_mctrl(struct uart_port *port, unsigned int mctrl)
			writel(temp, sport->port.membase + UCR2);
		}
	 
	-       temp = readl(sport->port.membase + UCR3) & ~UCR3_DSR;
	-       if (!(mctrl & TIOCM_DTR))
	-               temp |= UCR3_DSR;
	-       writel(temp, sport->port.membase + UCR3);
	-
		temp = readl(sport->port.membase + uts_reg(sport)) & ~UTS_LOOP;
		if (mctrl & TIOCM_LOOP)
			temp |= UTS_LOOP;

> Do you use fsl,dte-mode for the uart in question?

Yes, from imx6qdl-apalis.dtsi:

	&uart1 {
		pinctrl-names = "default";
		pinctrl-0 = <&pinctrl_uart1_dte &pinctrl_uart1_ctrl>;
		fsl,dte-mode;
		fsl,uart-has-rtscts;
		status = "disabled";
	};

> IMHO patches 1-4 should get applied before 4.6.

Ok, good, but this leaves 4.5 broken, right? Thanks.

-- ynezz

Re: [PATCH v2 0/6] serial: imx: handshaking fixes and improvments

[Uwe Kleine-König]

Hello Petr,

On Tue, Apr 12, 2016 at 11:48:46AM +0200, Petr Štetiar wrote:
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> [2016-04-12 09:46:22]:
> > On Mon, Apr 11, 2016 at 06:01:28PM +0200, Petr Štetiar wrote:
> > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> [2016-03-24 14:24:19]:
> > > 
> > > > Among others it made my i.MX25 stuck when the other side toggles DCD.
> > > > The first 4 patches are fixes that I'd like to see in 4.6-rcX, the last two
> > > > patches are merge window material.
> > > 
> > > just FYI, my i.MX6 board has stopped booting over NFS on 4.6-rc1 and I've just
> > > bisected it down to the commit "serial: imx: repair and complete handshaking"
> > > 90ebc48386, it's leading to the following CPU stall:
> > 
> > hmm, I'm confused (because I don't understand it) and something between
> > happy and sad (because both 90ebc48386 and the series that fixes it are
> > mine).
> 
> I don't know what's going on, probably something related to the serial IRQs?
> 
> > This is an i.MX6 Solo (i.e. only one cpu)?
> 
> This is i.MX6 Quad, Toradex Apalis Ixora (imx6q-apalis-ixora.dts).
> 
> > Which patch fixes your problem?
> 
> First 3 patches:
> 
>   [v2,1/6] serial: imx: fix polarity of RI
>   [v2,2/6] serial: imx: let irq handler return IRQ_NONE if no event was handled
>   [v2,3/6] serial: imx: make sure unhandled irqs are disabled
> 
> > Is it enough to revert just the last hunk of 90ebc48386?
> 
> Yes, this fixes it also:
> 
> 	diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> 	index 231e7d5..dab081c 100644
> 	--- a/drivers/tty/serial/imx.c
> 	+++ b/drivers/tty/serial/imx.c
> 	@@ -827,11 +827,6 @@ static void imx_set_mctrl(struct uart_port *port, unsigned int mctrl)
> 			writel(temp, sport->port.membase + UCR2);
> 		}
> 	 
> 	-       temp = readl(sport->port.membase + UCR3) & ~UCR3_DSR;
> 	-       if (!(mctrl & TIOCM_DTR))
> 	-               temp |= UCR3_DSR;
> 	-       writel(temp, sport->port.membase + UCR3);
> 	-
> 		temp = readl(sport->port.membase + uts_reg(sport)) & ~UTS_LOOP;
> 		if (mctrl & TIOCM_LOOP)
> 			temp |= UTS_LOOP;
> 
> > Do you use fsl,dte-mode for the uart in question?
> 
> Yes, from imx6qdl-apalis.dtsi:
> 
> 	&uart1 {
> 		pinctrl-names = "default";
> 		pinctrl-0 = <&pinctrl_uart1_dte &pinctrl_uart1_ctrl>;
> 		fsl,dte-mode;
> 		fsl,uart-has-rtscts;
> 		status = "disabled";
> 	};

ok, so UFCR_DCEDTE is set and UCR3_DSR controls the DTR output as
intended. What is connected to the respective pin?
MX6QDL_PAD_EIM_D25__UART1_DSR_B I think is the one.

> > IMHO patches 1-4 should get applied before 4.6.
> 
> Ok, good, but this leaves 4.5 broken, right? Thanks.

Once they landed in 4.6-rc they can and probably should be backported
for stable, yes.

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 v2 0/6] serial: imx: handshaking fixes and improvments

[Petr Štetiar]

Uwe Kleine-König <u.kleine-koenig@pengutronix.de> [2016-04-12 12:58:56]:

> ok, so UFCR_DCEDTE is set and UCR3_DSR controls the DTR output as
> intended. What is connected to the respective pin?
> MX6QDL_PAD_EIM_D25__UART1_DSR_B I think is the one.

It's routed out to Apalis SoM pin 120, which is then connected via 22R
resistor and through SN65C3243DBR RS-232 line driver to header which I use as
debug/serial console.

-- ynezz

Re: [PATCH v2 0/6] serial: imx: handshaking fixes and improvments

[Uwe Kleine-König]

On Tue, Apr 12, 2016 at 02:17:59PM +0200, Petr Štetiar wrote:
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> [2016-04-12 12:58:56]:
> 
> > ok, so UFCR_DCEDTE is set and UCR3_DSR controls the DTR output as
> > intended. What is connected to the respective pin?
> > MX6QDL_PAD_EIM_D25__UART1_DSR_B I think is the one.
> 
> It's routed out to Apalis SoM pin 120, which is then connected via 22R
> resistor and through SN65C3243DBR RS-232 line driver to header which I use as
> debug/serial console.

Then I guess the problem is that the rs232-partner asserts the imx6's
DSR in reply to the imx6 asserting DTR?

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 v2 0/6] serial: imx: handshaking fixes and improvments

[Petr Štetiar]

Uwe Kleine-König <u.kleine-koenig@pengutronix.de> [2016-04-12 19:30:32]:

> On Tue, Apr 12, 2016 at 02:17:59PM +0200, Petr Štetiar wrote:
> > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> [2016-04-12 12:58:56]:
> > 
> > > ok, so UFCR_DCEDTE is set and UCR3_DSR controls the DTR output as
> > > intended. What is connected to the respective pin?
> > > MX6QDL_PAD_EIM_D25__UART1_DSR_B I think is the one.
> > 
> > It's routed out to Apalis SoM pin 120, which is then connected via 22R
> > resistor and through SN65C3243DBR RS-232 line driver to header which I use as
> > debug/serial console.
> 
> Then I guess the problem is that the rs232-partner asserts the imx6's
> DSR in reply to the imx6 asserting DTR?

Yes, that's correct.

-- ynezz

Re: [PATCH v2 0/6] serial: imx: handshaking fixes and improvments

[Uwe Kleine-König]

On Wed, Apr 13, 2016 at 11:13:36AM +0200, Petr Štetiar wrote:
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> [2016-04-12 19:30:32]:
> 
> > On Tue, Apr 12, 2016 at 02:17:59PM +0200, Petr Štetiar wrote:
> > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> [2016-04-12 12:58:56]:
> > > 
> > > > ok, so UFCR_DCEDTE is set and UCR3_DSR controls the DTR output as
> > > > intended. What is connected to the respective pin?
> > > > MX6QDL_PAD_EIM_D25__UART1_DSR_B I think is the one.
> > > 
> > > It's routed out to Apalis SoM pin 120, which is then connected via 22R
> > > resistor and through SN65C3243DBR RS-232 line driver to header which I use as
> > > debug/serial console.
> > 
> > Then I guess the problem is that the rs232-partner asserts the imx6's
> > DSR in reply to the imx6 asserting DTR?
> 
> Yes, that's correct.

OK, that fits nicely in my picture. So we only have to wait until Greg
happily picks up the fixes (i.e. patches 1 up to 4).

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