fsl_lpuart/VF610: Division by zero.

fsl_lpuart/VF610: Division by zero.

[Bill Pringlemeir]

I have this 'dmesg' with ttyLP1,

Division by zero in kernel.
CPU: 0 PID: 187 Comm: getty Not tainted 3.15.0-rc8-04600-ge74a685-dirty #465
[<80013b18>] (unwind_backtrace) from [<80011e3c>] (show_stack+0x10/0x14)
[<80011e3c>] (show_stack) from [<8019a11c>] (Ldiv0+0x8/0x10)
[<8019a11c>] (Ldiv0) from [<801f0ad4>] (lpuart_startup+0x1e4/0x398)
[<801f0ad4>] (lpuart_startup) from [<801ef2e8>] (uart_startup.part.18+0x84/0x1e8)
[<801ef2e8>] (uart_startup.part.18) from [<801efd40>] (uart_open+0xe4/0x14c)
[<801efd40>] (uart_open) from [<801d586c>] (tty_open+0x1a0/0x5d0)
[<801d586c>] (tty_open) from [<800a1008>] (chrdev_open+0x9c/0x160)
[<800a1008>] (chrdev_open) from [<8009bd0c>] (do_dentry_open.isra.15+0x1b4/0x270)
[<8009bd0c>] (do_dentry_open.isra.15) from [<8009be98>] (finish_open+0x20/0x3c)
[<8009be98>] (finish_open) from [<800a8bfc>] (do_last.isra.66+0x404/0xc60)
[<800a8bfc>] (do_last.isra.66) from [<800a950c>] (path_openat+0xb4/0x5b0)
[<800a950c>] (path_openat) from [<800aa900>] (do_filp_open+0x2c/0x80)
[<800aa900>] (do_filp_open) from [<8009ce50>] (do_sys_open+0x120/0x1e0)
[<8009ce50>] (do_sys_open) from [<8000ed80>] (ret_fast_syscall+0x0/0x30)
random: nonblocking pool is initialized

I looked at the vmlinux objdump and it is here,

static int lpuart_dma_rx_request(struct uart_port *port)
{
...
	sport->dma_rx_timeout = (sport->port.timeout - HZ / 50) *
				FSL_UART_RX_DMA_BUFFER_SIZE * 3 /
				sport->rxfifo_size / 2;

The variable 'sport->rxfifo_size' is set in lpuart_setup_watermark() and
this is called after lpuart_dma_rx_request() in lpuart_startup().

It looks the same in the 'tty/serial' tree?

 https://git.kernel.org/cgit/linux/kernel/git/gregkh/tty.git/tree/drivers/tty/serial/fsl_lpuart.c#n768

I tried some simple changes but then I get no serial output, so I
thought it might be better to see if I am using the wrong git trees or
if the fix is more obvious to someone familiar with the code.

I think that normally everyone runs the console on this port and
'lpuart_console_setup()' is setting 'sport->rxfifo_size' before hand?
Or I don't understand why no one else is seeing this?

Thanks,
Bill Pringlemeir.

fsl_lpuart/VF610: Division by zero.

[Bill Pringlemeir]

On 25 Jul 2014, bpringlemeir at nbsps.com wrote:

> I have this 'dmesg' with ttyLP1,
>
> Division by zero in kernel.  CPU: 0 PID: 187 Comm: getty Not tainted
> 3.15.0-rc8-04600-ge74a685-dirty #465 [<80013b18>] (unwind_backtrace)
> from [<80011e3c>] (show_stack+0x10/0x14) [<80011e3c>] (show_stack)
> from [<8019a11c>] (Ldiv0+0x8/0x10) [<8019a11c>] (Ldiv0) from
> [<801f0ad4>] (lpuart_startup+0x1e4/0x398) [<801f0ad4>]
> (lpuart_startup) from [<801ef2e8>] (uart_startup.part.18+0x84/0x1e8)
> [<801ef2e8>] (uart_startup.part.18) from [<801efd40>]
> (uart_open+0xe4/0x14c) [<801efd40>] (uart_open) from [<801d586c>]
> (tty_open+0x1a0/0x5d0) [<801d586c>] (tty_open) from [<800a1008>]
> (chrdev_open+0x9c/0x160) [<800a1008>] (chrdev_open) from [<8009bd0c>]
> (do_dentry_open.isra.15+0x1b4/0x270) [<8009bd0c>]
> (do_dentry_open.isra.15) from [<8009be98>] (finish_open+0x20/0x3c)
> [<8009be98>] (finish_open) from [<800a8bfc>]
> (do_last.isra.66+0x404/0xc60) [<800a8bfc>] (do_last.isra.66) from
> [<800a950c>] (path_openat+0xb4/0x5b0) [<800a950c>] (path_openat) from
> [<800aa900>] (do_filp_open+0x2c/0x80) [<800aa900>] (do_filp_open) from
> [<8009ce50>] (do_sys_open+0x120/0x1e0) [<8009ce50>] (do_sys_open) from
> [<8000ed80>] (ret_fast_syscall+0x0/0x30) random: nonblocking pool is
> initialized
>
> I looked at the vmlinux objdump and it is here,
>
> static int lpuart_dma_rx_request(struct uart_port *port)
> {
> ...
> 	sport->dma_rx_timeout = (sport->port.timeout - HZ / 50) *
> 				FSL_UART_RX_DMA_BUFFER_SIZE * 3 /
> 				sport->rxfifo_size / 2;
>
> The variable 'sport->rxfifo_size' is set in lpuart_setup_watermark()
> and this is called after lpuart_dma_rx_request() in lpuart_startup().
>
> It looks the same in the 'tty/serial' tree?
>
> https://git.kernel.org/cgit/linux/kernel/git/gregkh/tty.git/tree/drivers/tty/serial/fsl_lpuart.c#n768
>
> I tried some simple changes but then I get no serial output, so I
> thought it might be better to see if I am using the wrong git trees or
> if the fix is more obvious to someone familiar with the code.
>
> I think that normally everyone runs the console on this port and
> 'lpuart_console_setup()' is setting 'sport->rxfifo_size' before hand?
> Or I don't understand why no one else is seeing this?

I failed to add,

   &uart1 {
           pinctrl-names = "default";
           pinctrl-0 = <&pinctrl_uart1>;
           status = "okay";
   };

to my device tree and then this condition triggered.  I can see the
'dmesg' by telneting to the device.  So maybe it is not so interesting.

If I put the proper initialization as above, then it is fine.

Thanks,
Bill Pringlemeir.

fsl_lpuart/VF610: Division by zero.

[Stefan Agner]

Hi Bill,

Am 2014-07-26 00:36, schrieb Bill Pringlemeir:
> On 25 Jul 2014, bpringlemeir at nbsps.com wrote:
> 
>> I have this 'dmesg' with ttyLP1,
>>
>> Division by zero in kernel.  CPU: 0 PID: 187 Comm: getty Not tainted
>> 3.15.0-rc8-04600-ge74a685-dirty #465 [<80013b18>] (unwind_backtrace)
>> from [<80011e3c>] (show_stack+0x10/0x14) [<80011e3c>] (show_stack)
>> from [<8019a11c>] (Ldiv0+0x8/0x10) [<8019a11c>] (Ldiv0) from
>> [<801f0ad4>] (lpuart_startup+0x1e4/0x398) [<801f0ad4>]
>> (lpuart_startup) from [<801ef2e8>] (uart_startup.part.18+0x84/0x1e8)
>> [<801ef2e8>] (uart_startup.part.18) from [<801efd40>]
>> (uart_open+0xe4/0x14c) [<801efd40>] (uart_open) from [<801d586c>]
>> (tty_open+0x1a0/0x5d0) [<801d586c>] (tty_open) from [<800a1008>]
>> (chrdev_open+0x9c/0x160) [<800a1008>] (chrdev_open) from [<8009bd0c>]
>> (do_dentry_open.isra.15+0x1b4/0x270) [<8009bd0c>]
>> (do_dentry_open.isra.15) from [<8009be98>] (finish_open+0x20/0x3c)
>> [<8009be98>] (finish_open) from [<800a8bfc>]
>> (do_last.isra.66+0x404/0xc60) [<800a8bfc>] (do_last.isra.66) from
>> [<800a950c>] (path_openat+0xb4/0x5b0) [<800a950c>] (path_openat) from
>> [<800aa900>] (do_filp_open+0x2c/0x80) [<800aa900>] (do_filp_open) from
>> [<8009ce50>] (do_sys_open+0x120/0x1e0) [<8009ce50>] (do_sys_open) from
>> [<8000ed80>] (ret_fast_syscall+0x0/0x30) random: nonblocking pool is
>> initialized
>>
>> I looked at the vmlinux objdump and it is here,
>>
>> static int lpuart_dma_rx_request(struct uart_port *port)
>> {
>> ...
>> 	sport->dma_rx_timeout = (sport->port.timeout - HZ / 50) *
>> 				FSL_UART_RX_DMA_BUFFER_SIZE * 3 /
>> 				sport->rxfifo_size / 2;
>>
>> The variable 'sport->rxfifo_size' is set in lpuart_setup_watermark()
>> and this is called after lpuart_dma_rx_request() in lpuart_startup().

I observed this division by zero too, I recently sent a patch for that.
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/268772.html

Greg already added that to his tty-next tree:
https://git.kernel.org/cgit/linux/kernel/git/gregkh/tty.git/tree/drivers/tty/serial/fsl_lpuart.c?h=tty-next#n1309

I also observed that DMA did not work yet, hence I a created a second
patch:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/268773.html

But eDMA in general was broken too,  this patch solved that, hence you
might want apply that patch too:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/268199.html

All patches are lined up for 3.17, so things should be smooth from that
release on.

In my last tests the driver worked more or less reliable, as is, however
I think the RX path still has some problems regarding DMA: The function
lpuart_dma_rx_complete never gets called, 
lpuart_timer_func receives the data later on. When setting
FSL_UART_RX_DMA_BUFFER_SIZE to 16 bytes (for UART0),
lpuart_dma_rx_complete was called, but then receive did not work
reliable anymore. I had not yet the time to investigate this further.

--
Stefan

fsl_lpuart/VF610: Division by zero.

[Bill Pringlemeir]

On 26 Jul 2014, stefan at agner.ch wrote:

> Hi Bill,
>
> Am 2014-07-26 00:36, schrieb Bill Pringlemeir:
>> On 25 Jul 2014, bpringlemeir at nbsps.com wrote:
>
>>> I looked at the vmlinux objdump and it is here,
>>>
>>> static int lpuart_dma_rx_request(struct uart_port *port)
>>> {
>>> ...
>>> 	sport->dma_rx_timeout = (sport->port.timeout - HZ / 50) *
>>> 				FSL_UART_RX_DMA_BUFFER_SIZE * 3 /
>>> 				sport->rxfifo_size / 2;
>>>
>>> The variable 'sport->rxfifo_size' is set in lpuart_setup_watermark()
>>> and this is called after lpuart_dma_rx_request() in
>>> lpuart_startup().

> I observed this division by zero too, I recently sent a patch for
> that.
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/268772.html

> Greg already added that to his tty-next tree:
> https://git.kernel.org/cgit/linux/kernel/git/gregkh/tty.git/tree/drivers/tty/serial/fsl_lpuart.c?h=tty-next#n1309

> I also observed that DMA did not work yet, hence I a created a second
> patch:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/268773.html
>
> But eDMA in general was broken too, this patch solved that, hence you
> might want apply that patch too:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/268199.html
>
> All patches are lined up for 3.17, so things should be smooth from
> that release on.

Thanks, I applied all three patches to the arm-soc 'for-next' and now
the serial ports seems to be working.  I am using the Tower board and
they have a 'TWR-SER2' card.

http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=TWR-SER2

Unfortunately, the IOMUX conflicts with the Audio daughter-board in
vf610-twr.dts.   The I/O conflicts are,

	VF610_PAD_PTB6__FTM0_CH6
	VF610_PAD_PTB7__FTM0_CH7
versus
        VF610_PAD_PTB6__UART2_TX
        VF610_PAD_PTB7__UART2_RX

Below are the DT changes to get two 'ttyLP' working with the Tower
System which probably will never apply cleanly, but are just meant for
reference.

--- a/arch/arm/boot/dts/vf610-twr.dts
+++ b/arch/arm/boot/dts/vf610-twr.dts
@@ -58,7 +59,7 @@
                        regulator-max-microvolt = <3300000>;
                };
        };
-
+/*
        sound {
                compatible = "simple-audio-card";
                simple-audio-card,format = "i2s";
@@ -87,6 +88,7 @@
                        bitclock-master;
                };
        };
+*/
 };
 
 &adc0 {
@@ -241,7 +243,7 @@
                };
-
+/*
                pinctrl_pwm0: pwm0grp {
                        fsl,pins = <
                                VF610_PAD_PTB0__FTM0_CH0                0x1582
@@ -252,7 +254,7 @@
                                VF610_PAD_PTB7__FTM0_CH7                0x1582
                        >;
                };
-
+*/
                pinctrl_sai2: sai2grp {
                        fsl,pins = <
                                VF610_PAD_PTA16__SAI2_TX_BCLK           0x02ed
@@ -271,6 +273,13 @@
                                VF610_PAD_PTB5__UART1_RX                0x21a1
                        >;
                };
+
+               pinctrl_uart2: uart2grp {
+                       fsl,pins = <
+                                VF610_PAD_PTB6__UART2_TX                0x21a2
+                                VF610_PAD_PTB7__UART2_RX                0x21a1
+                       >;
+               };
        };
 };
 
@@ -284,21 +293,31 @@
        status = "okay";
 };
 
+/*
 &pwm0 {
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_pwm0>;
        status = "okay";
 };
-
 &sai2 {
        #sound-dai-cells = <0>;
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_sai2>;
        status = "okay";
 };
-
+*/
+&uart2 {
+       pinctrl-names = "default";
+       pinctrl-0 = <&pinctrl_uart2>;
+       status = "okay";
+};

The standard jumpers on the Tower will either route ttyLP1 to the 'USB'
or to the 'serial board'; ttyLP2 will be the opposite with the 'serial
board' or the 'USB'.

Thanks again.  I should have read the linux-arm list more diligently.

Bill Pringlemeir.