Linux Serial subsystem development
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] tty: serial: add driver for the SiFive UART
From: kbuild test robot @ 2018-10-19  9:55 UTC (permalink / raw)
  Cc: kbuild-all, linux-serial, Paul Walmsley, Greg Kroah-Hartman,
	Jiri Slaby, Palmer Dabbelt, Wesley Terpstra, linux-riscv,
	linux-kernel, Paul Walmsley
In-Reply-To: <20181018234352.26788-3-paul.walmsley@sifive.com>

[-- Attachment #1: Type: text/plain, Size: 3509 bytes --]

Hi Paul,

I love your patch! Yet something to improve:

[auto build test ERROR on tty/tty-testing]
[also build test ERROR on v4.19-rc8 next-20181019]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Paul-Walmsley/dt-bindings-serial-add-documentation-for-the-SiFive-UART-driver/20181019-165529
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> drivers/tty/serial/sifive.c:891:20: error: 'sifive_serial_poll_put_char' undeclared here (not in a function); did you mean 'sifive_serial_clk_notifier'?
     .poll_put_char  = sifive_serial_poll_put_char,
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
                       sifive_serial_clk_notifier
>> drivers/tty/serial/sifive.c:892:20: error: 'sifive_serial_poll_get_char' undeclared here (not in a function); did you mean 'sifive_serial_poll_put_char'?
     .poll_get_char  = sifive_serial_poll_get_char,
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
                       sifive_serial_poll_put_char
   In file included from drivers/tty/serial/sifive.c:47:0:
   drivers/tty/serial/sifive.c:1028:25: error: 'sifive_serial_match' undeclared here (not in a function); did you mean 'sifive_serial_of_match'?
    MODULE_DEVICE_TABLE(of, sifive_serial_match);
                            ^
   include/linux/module.h:213:15: note: in definition of macro 'MODULE_DEVICE_TABLE'
    extern typeof(name) __mod_##type##__##name##_device_table  \
                  ^~~~
   include/linux/module.h:213:21: error: '__mod_of__sifive_serial_match_device_table' aliased to undefined symbol 'sifive_serial_match'
    extern typeof(name) __mod_##type##__##name##_device_table  \
                        ^
   drivers/tty/serial/sifive.c:1028:1: note: in expansion of macro 'MODULE_DEVICE_TABLE'
    MODULE_DEVICE_TABLE(of, sifive_serial_match);
    ^~~~~~~~~~~~~~~~~~~
   drivers/tty/serial/sifive.c:522:13: warning: '__ssp_wait_for_xmitr' defined but not used [-Wunused-function]
    static void __ssp_wait_for_xmitr(struct sifive_serial_port *ssp)
                ^~~~~~~~~~~~~~~~~~~~

vim +891 drivers/tty/serial/sifive.c

   873	
   874	static const struct uart_ops sifive_serial_uops = {
   875		.tx_empty	= sifive_serial_tx_empty,
   876		.set_mctrl	= sifive_serial_set_mctrl,
   877		.get_mctrl	= sifive_serial_get_mctrl,
   878		.stop_tx	= sifive_serial_stop_tx,
   879		.start_tx	= sifive_serial_start_tx,
   880		.stop_rx	= sifive_serial_stop_rx,
   881		.break_ctl	= sifive_serial_break_ctl,
   882		.startup	= sifive_serial_startup,
   883		.shutdown	= sifive_serial_shutdown,
   884		.set_termios	= sifive_serial_set_termios,
   885		.type		= sifive_serial_type,
   886		.release_port	= sifive_serial_release_port,
   887		.request_port	= sifive_serial_request_port,
   888		.config_port	= sifive_serial_config_port,
   889		.verify_port	= sifive_serial_verify_port,
   890	#ifdef CONFIG_CONSOLE_POLL
 > 891		.poll_put_char  = sifive_serial_poll_put_char,
 > 892		.poll_get_char  = sifive_serial_poll_get_char,
   893	#endif
   894	};
   895	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 65228 bytes --]

^ permalink raw reply

* Re: [[PATCH] 8/9] DMA-UART-Driver-for-AST2500
From: sudheer.v @ 2018-10-19  7:11 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Vinod, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
	Joel Stanley, Andrew Jeffery, Russell King, Dan Williams,
	Jiri Slaby, Thomas Gleixner, Marc Zyngier, Christian Borntraeger,
	Michael Moese, Hendrik Brueckner, Kate Stewart,
	Philippe Ombredanne, dmaengine, devicetree, linux-kernel,
	linux-serial, linux-arm-kernel
In-Reply-To: <a3fecf7913d6a85a6294afbc5a1a18b7714d6756.camel@kernel.crashing.org>

On Fri, Oct 19, 2018 at 10:32:24AM +1100, Benjamin Herrenschmidt wrote:
> On Thu, 2018-10-18 at 15:25 +0530, Vinod wrote:
> > 
> > > It's not a dmaengine driver. It's a serial UART driver that happens to
> > > use a dedicated DMA engine.
> > 
> > Then I see no reason for it to use dmaengine APIs. The framework allows
> > people to share a controller for many clients, but if you have dedicated
> > one then you may use it directly
> 
> Well... the engine is shared by a few UARTs, they have dedicated rings
> but there's a common set of regs for interrupt handling etc.
> 
> That said, I still think it could be contained within a UART driver,
> there's little benefit in adding the framework overhead, esp since
> these are really weak cores, any overhead will be felt.
> 
> Ben.
> 
> > > It's unclear whether it should be split into two drivers, or just have
> > > the serial driver directly use the dma engine since that engine is
> > > dedicated in HW to only work on those UARTs and nothing else...
> > > 
> > > Cheers,
> > > Ben.

Initially we wanted to have  a single driver,
however we had an informal discussion with one of the maintainer 
and based on the feedback, followed the Linux DMA and UART architecture.

If this seperate DMA-engine driver adds more overhead than benifit,
we will merge them into a single UART driver and resubmitt the patches.
Vinod,
      can this dma-controller driver sit under dma subsystem?.
      or better to move it under UART framework.

Thank you.
-- Sudheer

^ permalink raw reply

* Re: Crash in msm serial on dragonboard with ftrace bootargs
From: Sai Prakash Ranjan @ 2018-10-19  6:54 UTC (permalink / raw)
  To: Joel Fernandes, Steven Rostedt
  Cc: Stephen Boyd, Bjorn Andersson, Andy Gross, David Brown,
	Jiri Slaby, Kees Cook, Geliang Tang, Greg Kroah-Hartman,
	Pramod Gurav, linux-arm-msm, linux-soc, linux-serial,
	linux-kernel, Rajendra Nayak, Vivek Gautam, Sibi Sankar
In-Reply-To: <20181019041740.GB141835@joelaf.mtv.corp.google.com>

On 10/19/2018 9:47 AM, Joel Fernandes wrote:
> On Thu, Oct 18, 2018 at 09:17:06AM -0400, Steven Rostedt wrote:
>> On Thu, 18 Oct 2018 10:51:18 +0530
>> Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> wrote:
>>
>>>> So something else is causing an issue besides just msm_read.
>>>>
>>>> Can you do an objdump -dr of the entire vmlinux binary and gzip it and
>>>> post it somewhere. Not sure if it would be too big to email. You could
>>>> try sending it to me privately. I'd like to see the binary that you are
>>>> using.
>>>>    
>>>
>>> I have sent the objdump and dot config to you privately.
>>
>> Thanks. I don't see anything that pops out, but then again, my arm asm
>> foo is very rusty (it has been literally decades since I did any arm
>> asm). I wonder if it could simply be a timing issue?
>>
>> ffff0000086eb538 <msm_read.isra.1>:
>> ffff0000086eb538:       a9be7bfd        stp     x29, x30, [sp,#-32]!
>> ffff0000086eb53c:       910003fd        mov     x29, sp
>> ffff0000086eb540:       a90153f3        stp     x19, x20, [sp,#16]
>> ffff0000086eb544:       aa0003f4        mov     x20, x0
>> ffff0000086eb548:       2a0103f3        mov     w19, w1
>> ffff0000086eb54c:       aa1e03e0        mov     x0, x30
>> ffff0000086eb550:       97e6bae4        bl      ffff00000809a0e0 <_mcount>
>>
>> The above is changed to nop on boot, but then to:
>>
>> 	bl ftrace_caller
>>
>> When ftrace is enabled.
>>
>> ffff0000086eb554:       8b334280        add     x0, x20, w19, uxtw
>> ffff0000086eb558:       b9400000        ldr     w0, [x0]
>> ffff0000086eb55c:       a94153f3        ldp     x19, x20, [sp,#16]
>> ffff0000086eb560:       a8c27bfd        ldp     x29, x30, [sp],#32
>> ffff0000086eb564:       d65f03c0        ret
>>
>>
>>
>> ffff00000809a0e4 <ftrace_caller>:
>> ffff00000809a0e4:       a9bf7bfd        stp     x29, x30, [sp,#-16]!
>> ffff00000809a0e8:       910003fd        mov     x29, sp
>> ffff00000809a0ec:       d10013c0        sub     x0, x30, #0x4
>> ffff00000809a0f0:       f94003a1        ldr     x1, [x29]
>> ffff00000809a0f4:       f9400421        ldr     x1, [x1,#8]
>> ffff00000809a0f8:       d1001021        sub     x1, x1, #0x4
>>
>> ffff00000809a0fc <ftrace_call>:
>> ffff00000809a0fc:       d503201f        nop
>>
>> The above nop gets patched to:
>>
>> 	bl ftrace_ops_no_ops
>>
>> Which will iterate through all the registered functions.
>>
>>
>> ffff00000809a100 <ftrace_graph_call>:
>> ffff00000809a100:       d503201f        nop
>>
>> The above only gets set when function graph tracer is enabled, which it
>> is not in this case.
>>
>> ffff00000809a104:       a8c17bfd        ldp     x29, x30, [sp],#16
>> ffff00000809a108:       d65f03c0        ret
>>
>>
>> Anyone see any problems here?
> 
> This seems sane to me, he says in the other thread that he put 'notrace' to
> the msm serial functions (which AIUI should prevent ftrace instrumentation)
> and he still sees the issue.
> 

Yes I did add notrace to all functions in msm serial and checked the 
objdump to make sure that those were not instrumented, and yet the 
target crashed. This doesnt seem like an issue with ftrace but rather 
with msm early con.

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply

* Re: Crash in msm serial on dragonboard with ftrace bootargs
From: Joel Fernandes @ 2018-10-19  4:17 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Sai Prakash Ranjan, Stephen Boyd, Bjorn Andersson, Andy Gross,
	David Brown, Jiri Slaby, Kees Cook, Geliang Tang,
	Greg Kroah-Hartman, Pramod Gurav, linux-arm-msm, linux-soc,
	linux-serial, linux-kernel, Rajendra Nayak, Vivek Gautam,
	Sibi Sankar
In-Reply-To: <20181018091706.62310b38@gandalf.local.home>

On Thu, Oct 18, 2018 at 09:17:06AM -0400, Steven Rostedt wrote:
> On Thu, 18 Oct 2018 10:51:18 +0530
> Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> wrote:
> 
> > > So something else is causing an issue besides just msm_read.
> > > 
> > > Can you do an objdump -dr of the entire vmlinux binary and gzip it and
> > > post it somewhere. Not sure if it would be too big to email. You could
> > > try sending it to me privately. I'd like to see the binary that you are
> > > using.
> > >   
> > 
> > I have sent the objdump and dot config to you privately.
> 
> Thanks. I don't see anything that pops out, but then again, my arm asm
> foo is very rusty (it has been literally decades since I did any arm
> asm). I wonder if it could simply be a timing issue?
> 
> ffff0000086eb538 <msm_read.isra.1>:
> ffff0000086eb538:       a9be7bfd        stp     x29, x30, [sp,#-32]!
> ffff0000086eb53c:       910003fd        mov     x29, sp
> ffff0000086eb540:       a90153f3        stp     x19, x20, [sp,#16]
> ffff0000086eb544:       aa0003f4        mov     x20, x0
> ffff0000086eb548:       2a0103f3        mov     w19, w1
> ffff0000086eb54c:       aa1e03e0        mov     x0, x30
> ffff0000086eb550:       97e6bae4        bl      ffff00000809a0e0 <_mcount>
> 
> The above is changed to nop on boot, but then to:
> 
> 	bl ftrace_caller
> 
> When ftrace is enabled.
> 
> ffff0000086eb554:       8b334280        add     x0, x20, w19, uxtw
> ffff0000086eb558:       b9400000        ldr     w0, [x0]
> ffff0000086eb55c:       a94153f3        ldp     x19, x20, [sp,#16]
> ffff0000086eb560:       a8c27bfd        ldp     x29, x30, [sp],#32
> ffff0000086eb564:       d65f03c0        ret
> 
> 
> 
> ffff00000809a0e4 <ftrace_caller>:
> ffff00000809a0e4:       a9bf7bfd        stp     x29, x30, [sp,#-16]!
> ffff00000809a0e8:       910003fd        mov     x29, sp
> ffff00000809a0ec:       d10013c0        sub     x0, x30, #0x4
> ffff00000809a0f0:       f94003a1        ldr     x1, [x29]
> ffff00000809a0f4:       f9400421        ldr     x1, [x1,#8]
> ffff00000809a0f8:       d1001021        sub     x1, x1, #0x4
> 
> ffff00000809a0fc <ftrace_call>:
> ffff00000809a0fc:       d503201f        nop
> 
> The above nop gets patched to:
> 
> 	bl ftrace_ops_no_ops
> 
> Which will iterate through all the registered functions.
> 
> 
> ffff00000809a100 <ftrace_graph_call>:
> ffff00000809a100:       d503201f        nop
> 
> The above only gets set when function graph tracer is enabled, which it
> is not in this case.
> 
> ffff00000809a104:       a8c17bfd        ldp     x29, x30, [sp],#16
> ffff00000809a108:       d65f03c0        ret
> 
> 
> Anyone see any problems here?

This seems sane to me, he says in the other thread that he put 'notrace' to
the msm serial functions (which AIUI should prevent ftrace instrumentation)
and he still sees the issue.

thanks,

 - Joel

^ permalink raw reply

* [PATCH 2/2] tty: serial: add driver for the SiFive UART
From: Paul Walmsley @ 2018-10-18 23:43 UTC (permalink / raw)
  To: linux-serial
  Cc: Paul Walmsley, Greg Kroah-Hartman, Jiri Slaby, Palmer Dabbelt,
	Wesley Terpstra, linux-riscv, linux-kernel, Paul Walmsley
In-Reply-To: <20181018234352.26788-1-paul.walmsley@sifive.com>

Add a serial driver for the SiFive UART, found on SiFive FU540 devices
(among others).

The underlying serial IP block is relatively basic, and currently does
not support serial break detection.  Further information on the IP
block can be found in the documentation and Chisel sources:

https://static.dev.sifive.com/FU540-C000-v1.0.pdf

https://github.com/sifive/sifive-blocks/tree/master/src/main/scala/devices/uart

This driver was written in collaboration with Wesley Terpstra
<wesley@sifive.com>.

Boot-tested on a SiFive HiFive Unleashed A00 board.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Palmer Dabbelt <palmer@sifive.com>
Cc: Wesley Terpstra <wesley@sifive.com>
Cc: linux-serial@vger.kernel.org
Cc: linux-riscv@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 drivers/tty/serial/Kconfig       |   24 +
 drivers/tty/serial/Makefile      |    1 +
 drivers/tty/serial/sifive.c      | 1070 ++++++++++++++++++++++++++++++
 include/uapi/linux/serial_core.h |    3 +
 4 files changed, 1098 insertions(+)
 create mode 100644 drivers/tty/serial/sifive.c

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index df8bd0c7b97d..fdd624df75c8 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1060,6 +1060,30 @@ config SERIAL_OMAP_CONSOLE
 	  your boot loader about how to pass options to the kernel at
 	  boot time.)
 
+config SERIAL_SIFIVE
+	tristate "SiFive UART support"
+	depends on OF
+	select SERIAL_CORE
+	help
+	  Select this option if you are building a kernel for a device that
+	  contains a SiFive UART IP block.  This type of UART is present on
+	  SiFive FU540 SoCs, among others.
+
+config SERIAL_SIFIVE_CONSOLE
+	bool "Console on SiFive UART"
+	depends on SERIAL_SIFIVE=y
+	select SERIAL_CORE_CONSOLE
+	help
+	  Select this option if you would like to use a SiFive UART as the
+	  system console.
+
+	  Even if you say Y here, the currently visible virtual console
+	  (/dev/tty0) will still be used as the system console by default, but
+	  you can alter that using a kernel command line option such as
+	  "console=ttySIFx". (Try "man bootparam" or see the documentation of
+	  your boot loader about how to pass options to the kernel at
+	  boot time.)
+
 config SERIAL_LANTIQ
 	bool "Lantiq serial driver"
 	depends on LANTIQ
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index daac675612df..7e906d3c0455 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -89,6 +89,7 @@ obj-$(CONFIG_SERIAL_MVEBU_UART)	+= mvebu-uart.o
 obj-$(CONFIG_SERIAL_PIC32)	+= pic32_uart.o
 obj-$(CONFIG_SERIAL_MPS2_UART)	+= mps2-uart.o
 obj-$(CONFIG_SERIAL_OWL)	+= owl-uart.o
+obj-$(CONFIG_SERIAL_SIFIVE)	+= sifive.o
 
 # GPIOLIB helpers for modem control lines
 obj-$(CONFIG_SERIAL_MCTRL_GPIO)	+= serial_mctrl_gpio.o
diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c
new file mode 100644
index 000000000000..12232e74c464
--- /dev/null
+++ b/drivers/tty/serial/sifive.c
@@ -0,0 +1,1070 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * SiFive UART driver
+ * Copyright (C) 2018 Paul Walmsley <paul@pwsan.com>
+ * Copyright (C) 2018 SiFive
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Based partially on:
+ * - drivers/tty/serial/pxa.c
+ * - drivers/tty/serial/amba-pl011.c
+ * - drivers/tty/serial/uartlite.c
+ * - drivers/tty/serial/omap-serial.c
+ * - drivers/pwm/pwm-sifive.c
+ *
+ * See the following sources for further documentation:
+ * - Chapter 19 "Universal Asynchronous Receiver/Transmitter (UART)" of
+ *   SiFive FE310-G000 v2p3
+ * - The tree/master/src/main/scala/devices/uart directory of
+ *   https://github.com/sifive/sifive-blocks/
+ *
+ * The SiFive UART design is not 8250-compatible.  The following common
+ * features are not supported:
+ * - Word lengths other than 8 bits
+ * - Break handling
+ * - Parity
+ * - Flow control
+ * - Modem signals (DSR, RI, etc.)
+ * On the other hand, the design is free from the baggage of the 8250
+ * programming model.
+ */
+
+#include <linux/clk.h>
+#include <linux/console.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/serial_core.h>
+#include <linux/serial_reg.h>
+#include <linux/slab.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
+
+/*
+ * Register offsets
+ */
+
+/* TXDATA */
+#define SIFIVE_SERIAL_TXDATA_OFFS		0x0
+#define SIFIVE_SERIAL_TXDATA_FULL_SHIFT		31
+#define SIFIVE_SERIAL_TXDATA_FULL_MASK		(1 << SIFIVE_SERIAL_TXDATA_FULL_SHIFT)
+#define SIFIVE_SERIAL_TXDATA_DATA_SHIFT		0
+#define SIFIVE_SERIAL_TXDATA_DATA_MASK		(0xff << SIFIVE_SERIAL_TXDATA_DATA_SHIFT)
+
+/* RXDATA */
+#define SIFIVE_SERIAL_RXDATA_OFFS		0x4
+#define SIFIVE_SERIAL_RXDATA_EMPTY_SHIFT	31
+#define SIFIVE_SERIAL_RXDATA_EMPTY_MASK		(1 << SIFIVE_SERIAL_RXDATA_EMPTY_SHIFT)
+#define SIFIVE_SERIAL_RXDATA_DATA_SHIFT		0
+#define SIFIVE_SERIAL_RXDATA_DATA_MASK		(0xff << SIFIVE_SERIAL_RXDATA_DATA_SHIFT)
+
+/* TXCTRL */
+#define SIFIVE_SERIAL_TXCTRL_OFFS		0x8
+#define SIFIVE_SERIAL_TXCTRL_TXCNT_SHIFT	16
+#define SIFIVE_SERIAL_TXCTRL_TXCNT_MASK		(0x7 << SIFIVE_SERIAL_TXCTRL_TXCNT_SHIFT)
+#define SIFIVE_SERIAL_TXCTRL_NSTOP_SHIFT	1
+#define SIFIVE_SERIAL_TXCTRL_NSTOP_MASK		(1 << SIFIVE_SERIAL_TXCTRL_NSTOP_SHIFT)
+#define SIFIVE_SERIAL_TXCTRL_TXEN_SHIFT		0
+#define SIFIVE_SERIAL_TXCTRL_TXEN_MASK		(1 << SIFIVE_SERIAL_TXCTRL_TXEN_SHIFT)
+
+/* RXCTRL */
+#define SIFIVE_SERIAL_RXCTRL_OFFS		0xC
+#define SIFIVE_SERIAL_RXCTRL_RXCNT_SHIFT	16
+#define SIFIVE_SERIAL_RXCTRL_RXCNT_MASK		(0x7 << SIFIVE_SERIAL_TXCTRL_TXCNT_SHIFT)
+#define SIFIVE_SERIAL_RXCTRL_RXEN_SHIFT		0
+#define SIFIVE_SERIAL_RXCTRL_RXEN_MASK		(1 << SIFIVE_SERIAL_RXCTRL_RXEN_SHIFT)
+
+/* IE */
+#define SIFIVE_SERIAL_IE_OFFS			0x10
+#define SIFIVE_SERIAL_IE_RXWM_SHIFT		1
+#define SIFIVE_SERIAL_IE_RXWM_MASK		(1 << SIFIVE_SERIAL_IE_RXWM_SHIFT)
+#define SIFIVE_SERIAL_IE_TXWM_SHIFT		0
+#define SIFIVE_SERIAL_IE_TXWM_MASK		(1 << SIFIVE_SERIAL_IE_TXWM_SHIFT)
+
+/* IP */
+#define SIFIVE_SERIAL_IP_OFFS			0x14
+#define SIFIVE_SERIAL_IP_RXWM_SHIFT		1
+#define SIFIVE_SERIAL_IP_RXWM_MASK		(1 << SIFIVE_SERIAL_IP_RXWM_SHIFT)
+#define SIFIVE_SERIAL_IP_TXWM_SHIFT		0
+#define SIFIVE_SERIAL_IP_TXWM_MASK		(1 << SIFIVE_SERIAL_IP_TXWM_SHIFT)
+
+/* DIV */
+#define SIFIVE_SERIAL_DIV_OFFS			0x18
+#define SIFIVE_SERIAL_DIV_DIV_SHIFT		0
+#define SIFIVE_SERIAL_DIV_DIV_MASK		(0xffff << SIFIVE_SERIAL_IP_DIV_SHIFT)
+
+/*
+ * Config macros
+ */
+
+/*
+ * SIFIVE_SERIAL_MAX_PORTS: maximum number of UARTs on a device that can
+ *                          host a serial console
+ */
+#define SIFIVE_SERIAL_MAX_PORTS			8
+
+/*
+ * SIFIVE_DEFAULT_BAUD_RATE: default baud rate that the driver should
+ *                           configure itself to use
+ */
+#define SIFIVE_DEFAULT_BAUD_RATE		115200
+
+/* SIFIVE_SERIAL_NAME: our driver's name that we pass to the operating system */
+#define SIFIVE_SERIAL_NAME			"sifive-serial"
+
+/* SIFIVE_TTY_PREFIX: tty name prefix for SiFive serial ports */
+#define SIFIVE_TTY_PREFIX			"ttySIF"
+
+/* SIFIVE_TX_FIFO_DEPTH: depth of the TX FIFO (in bytes) */
+#define SIFIVE_TX_FIFO_DEPTH			8
+
+/* SIFIVE_RX_FIFO_DEPTH: depth of the TX FIFO (in bytes) */
+#define SIFIVE_RX_FIFO_DEPTH			8
+
+#if (SIFIVE_TX_FIFO_DEPTH != SIFIVE_RX_FIFO_DEPTH)
+#error Driver does not support configurations with different TX, RX FIFO sizes
+#endif
+
+/*
+ *
+ */
+
+/**
+ * sifive_serial_port - driver-specific data extension to struct uart_port
+ * @port: struct uart_port embedded in this struct
+ * @dev: struct device *
+ * @ier: shadowed copy of the interrupt enable register
+ * @clkin_rate: input clock to the UART IP block.
+ * @baud_rate: UART serial line rate (e.g., 115200 baud)
+ * @clk_notifier: clock rate change notifier for upstream clock changes
+ *
+ * Configuration data specific to this SiFive UART.
+ */
+struct sifive_serial_port {
+	struct uart_port	port;
+	struct device		*dev;
+	unsigned char		ier;
+	unsigned long		clkin_rate;
+	unsigned long		baud_rate;
+	struct clk		*clk;
+	struct notifier_block	clk_notifier;
+};
+
+/*
+ * Structure container-of macros
+ */
+
+#define port_to_sifive_serial_port(p) (container_of((p), \
+						    struct sifive_serial_port, \
+						    port))
+
+#define notifier_to_sifive_serial_port(nb) (container_of((nb), \
+							 struct sifive_serial_port, \
+							 clk_notifier))
+
+/*
+ * Forward declarations
+ */
+static void sifive_serial_stop_tx(struct uart_port *port);
+
+/*
+ * Internal functions
+ */
+
+/**
+ * __ssp_early_writel() - write to a SiFive serial port register (early)
+ * @port: pointer to a struct uart_port record
+ * @offs: register address offset from the IP block base address
+ * @v: value to write to the register
+ *
+ * Given a pointer @port to a struct uart_port record, write the value
+ * @v to the IP block register address offset @offs.  This function is
+ * intended for early console use.
+ *
+ * Context: Intended to be used only by the earlyconsole code.
+ */
+static void __ssp_early_writel(u32 v, u16 offs, struct uart_port *port)
+{
+	writel_relaxed(v, port->membase + offs);
+}
+
+/**
+ * __ssp_early_readl() - read from a SiFive serial port register (early)
+ * @port: pointer to a struct uart_port record
+ * @offs: register address offset from the IP block base address
+ *
+ * Given a pointer @port to a struct uart_port record, read the
+ * contents of the IP block register located at offset @offs from the
+ * IP block base and return it.  This function is intended for early
+ * console use.
+ *
+ * Context: Intended to be called only by the earlyconsole code or by
+ *          __ssp_readl() or __ssp_writel() (in this driver)
+ *
+ * Returns: the register value read from the UART.
+ */
+static u32 __ssp_early_readl(struct uart_port *port, u16 offs)
+{
+	return readl_relaxed(port->membase + offs);
+}
+
+/**
+ * __ssp_writel() - write to a SiFive serial port register
+ * @v: value to write to the register
+ * @offs: register address offset from the IP block base address
+ * @ssp: pointer to a struct sifive_serial_port record
+ *
+ * Write the value @v to the IP block register located at offset @offs from the
+ * IP block base, given a pointer @ssp to a struct sifive_serial_port record.
+ *
+ * Context: Any context.
+ */
+static void __ssp_writel(u32 v, u16 offs, struct sifive_serial_port *ssp)
+{
+	__ssp_early_writel(v, offs, &ssp->port);
+}
+
+/**
+ * __ssp_readl() - read from a SiFive serial port register
+ * @ssp: pointer to a struct sifive_serial_port record
+ * @offs: register address offset from the IP block base address
+ *
+ * Read the contents of the IP block register located at offset @offs from the
+ * IP block base, given a pointer @ssp to a struct sifive_serial_port record.
+ *
+ * Context: Any context.
+ *
+ * Returns: the value of the UART register
+ */
+static u32 __ssp_readl(struct sifive_serial_port *ssp, u16 offs)
+{
+	return __ssp_early_readl(&ssp->port, offs);
+}
+
+/**
+ * sifive_serial_is_txfifo_full() - is the TXFIFO full?
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Read the transmit FIFO "full" bit, returning a non-zero value if the
+ * TX FIFO is full, or zero if space remains.  Intended to be used to prevent
+ * writes to the TX FIFO when it's full.
+ *
+ * Returns: SIFIVE_SERIAL_TXDATA_FULL_MASK (non-zero) if the transmit FIFO
+ * is full, or 0 if space remains.
+ */
+static int sifive_serial_is_txfifo_full(struct sifive_serial_port *ssp)
+{
+	return __ssp_readl(ssp, SIFIVE_SERIAL_TXDATA_OFFS) &
+		SIFIVE_SERIAL_TXDATA_FULL_MASK;
+}
+
+/**
+ * __ssp_transmit_char() - enqueue a byte to transmit onto the TX FIFO
+ * @ssp: pointer to a struct sifive_serial_port
+ * @ch: character to transmit
+ *
+ * Enqueue a byte @ch onto the transmit FIFO, given a pointer @ssp to the
+ * struct sifive_serial_port * to transmit on.  Caller should first check to
+ * ensure that the TXFIFO has space; see sifive_serial_is_txfifo_full().
+ *
+ * Context: Any context.
+ */
+static void __ssp_transmit_char(struct sifive_serial_port *ssp, int ch)
+{
+	__ssp_writel(ch, SIFIVE_SERIAL_TXDATA_OFFS, ssp);
+}
+
+/**
+ * __ssp_transmit_chars() - enqueue multiple bytes onto the TX FIFO
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Transfer up to a TX FIFO size's worth of characters from the Linux serial
+ * transmit buffer to the SiFive UART TX FIFO.
+ *
+ * Context: Any context.  Expects @ssp->port.lock to be held by caller.
+ */
+static void __ssp_transmit_chars(struct sifive_serial_port *ssp)
+{
+	struct circ_buf *xmit = &ssp->port.state->xmit;
+	int count;
+
+	if (ssp->port.x_char) {
+		__ssp_transmit_char(ssp, ssp->port.x_char);
+		ssp->port.icount.tx++;
+		ssp->port.x_char = 0;
+		return;
+	}
+	if (uart_circ_empty(xmit) || uart_tx_stopped(&ssp->port)) {
+		sifive_serial_stop_tx(&ssp->port);
+		return;
+	}
+	count = SIFIVE_TX_FIFO_DEPTH;
+	do {
+		__ssp_transmit_char(ssp, xmit->buf[xmit->tail]);
+		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
+		ssp->port.icount.tx++;
+		if (uart_circ_empty(xmit))
+			break;
+	} while (--count > 0);
+
+	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+		uart_write_wakeup(&ssp->port);
+
+	if (uart_circ_empty(xmit))
+		sifive_serial_stop_tx(&ssp->port);
+}
+
+/**
+ * __ssp_enable_txwm() - enable transmit watermark interrupts
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Enable interrupt generation when the transmit FIFO watermark is reached
+ * on the SiFive UART referred to by @ssp.
+ */
+static void __ssp_enable_txwm(struct sifive_serial_port *ssp)
+{
+	if (ssp->ier & SIFIVE_SERIAL_IE_TXWM_MASK)
+		return;
+
+	ssp->ier |= SIFIVE_SERIAL_IE_TXWM_MASK;
+	__ssp_writel(ssp->ier, SIFIVE_SERIAL_IE_OFFS, ssp);
+}
+
+/**
+ * __ssp_enable_rxwm() - enable receive watermark interrupts
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Enable interrupt generation when the receive FIFO watermark is reached
+ * on the SiFive UART referred to by @ssp.
+ */
+static void __ssp_enable_rxwm(struct sifive_serial_port *ssp)
+{
+	if (ssp->ier & SIFIVE_SERIAL_IE_RXWM_MASK)
+		return;
+
+	ssp->ier |= SIFIVE_SERIAL_IE_RXWM_MASK;
+	__ssp_writel(ssp->ier, SIFIVE_SERIAL_IE_OFFS, ssp);
+}
+
+/**
+ * __ssp_disable_txwm() - disable transmit watermark interrupts
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Disable interrupt generation when the transmit FIFO watermark is reached
+ * on the UART referred to by @ssp.
+ */
+static void __ssp_disable_txwm(struct sifive_serial_port *ssp)
+{
+	if (!(ssp->ier & SIFIVE_SERIAL_IE_TXWM_MASK))
+		return;
+
+	ssp->ier &= ~SIFIVE_SERIAL_IE_TXWM_MASK;
+	__ssp_writel(ssp->ier, SIFIVE_SERIAL_IE_OFFS, ssp);
+}
+
+/**
+ * __ssp_disable_rxwm() - disable receive watermark interrupts
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Disable interrupt generation when the receive FIFO watermark is reached
+ * on the UART referred to by @ssp.
+ */
+static void __ssp_disable_rxwm(struct sifive_serial_port *ssp)
+{
+	if (!(ssp->ier & SIFIVE_SERIAL_IE_RXWM_MASK))
+		return;
+
+	ssp->ier &= ~SIFIVE_SERIAL_IE_RXWM_MASK;
+	__ssp_writel(ssp->ier, SIFIVE_SERIAL_IE_OFFS, ssp);
+}
+
+/**
+ * __ssp_receive_char() - receive a byte from the UART
+ * @ssp: pointer to a struct sifive_serial_port
+ * @is_empty: char pointer to return whether the RX FIFO is empty
+ *
+ * Try to read a byte from the SiFive UART RX FIFO, referenced by
+ * @ssp, and to return it.  Also returns the RX FIFO empty bit in
+ * the char pointed to by @ch.  The caller must pass the byte back to the
+ * Linux serial layer if needed.
+ *
+ * Returns: the byte read from the UART RX FIFO.
+ */
+static char __ssp_receive_char(struct sifive_serial_port *ssp, char *is_empty)
+{
+	u32 v;
+	u8 ch;
+
+	v = __ssp_readl(ssp, SIFIVE_SERIAL_RXDATA_OFFS);
+
+	if (!is_empty)
+		WARN_ON(1);
+	else
+		*is_empty = (v & SIFIVE_SERIAL_RXDATA_EMPTY_MASK) >>
+			SIFIVE_SERIAL_RXDATA_EMPTY_SHIFT;
+
+	ch = (v & SIFIVE_SERIAL_RXDATA_DATA_MASK) >>
+		SIFIVE_SERIAL_RXDATA_DATA_SHIFT;
+
+	return ch;
+}
+
+/**
+ * __ssp_receive_chars() - receive multiple bytes from the UART
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Receive up to an RX FIFO's worth of bytes from the SiFive UART referred
+ * to by @ssp and pass them up to the Linux serial layer.
+ *
+ * Context: Expects ssp->port.lock to be held by caller.
+ */
+static void __ssp_receive_chars(struct sifive_serial_port *ssp)
+{
+	unsigned char ch;
+	char is_empty;
+	int c;
+
+	for (c = SIFIVE_RX_FIFO_DEPTH; c > 0; --c) {
+		ch = __ssp_receive_char(ssp, &is_empty);
+		if (is_empty)
+			break;
+
+		ssp->port.icount.rx++;
+		uart_insert_char(&ssp->port, 0, 0, ch, TTY_NORMAL);
+	}
+
+	spin_unlock(&ssp->port.lock);
+	tty_flip_buffer_push(&ssp->port.state->port);
+	spin_lock(&ssp->port.lock);
+}
+
+/**
+ * __ssp_update_div() - calculate the divisor setting by the line rate
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Calculate the appropriate value of the clock divisor for the UART
+ * and target line rate referred to by @ssp and write it into the
+ * hardware.
+ */
+static void __ssp_update_div(struct sifive_serial_port *ssp)
+{
+	u16 div;
+
+	div = DIV_ROUND_UP(ssp->clkin_rate, ssp->baud_rate) - 1;
+
+	__ssp_writel(div, SIFIVE_SERIAL_DIV_OFFS, ssp);
+}
+
+/**
+ * __ssp_update_baud_rate() - set the UART "baud rate"
+ * @ssp: pointer to a struct sifive_serial_port
+ * @rate: new target bit rate
+ *
+ * Calculate the UART divisor value for the target bit rate @rate for the
+ * SiFive UART described by @ssp and program it into the UART.  There may
+ * be some error between the target bit rate and the actual bit rate implemented
+ * by the UART due to clock ratio granularity.
+ */
+static void __ssp_update_baud_rate(struct sifive_serial_port *ssp,
+				   unsigned int rate)
+{
+	if (ssp->baud_rate == rate)
+		return;
+
+	ssp->baud_rate = rate;
+	__ssp_update_div(ssp);
+}
+
+/**
+ * __ssp_set_stop_bits() - set the number of stop bits
+ * @ssp: pointer to a struct sifive_serial_port
+ * @nstop: 1 or 2 (stop bits)
+ *
+ * Program the SiFive UART referred to by @ssp to use @nstop stop bits.
+ */
+static void __ssp_set_stop_bits(struct sifive_serial_port *ssp, char nstop)
+{
+	u32 v;
+
+	if (nstop < 1 || nstop > 2) {
+		WARN_ON(1);
+		return;
+	}
+
+	v = __ssp_readl(ssp, SIFIVE_SERIAL_TXCTRL_OFFS);
+	v &= ~SIFIVE_SERIAL_TXCTRL_NSTOP_MASK;
+	v |= (nstop - 1) << SIFIVE_SERIAL_TXCTRL_NSTOP_SHIFT;
+	__ssp_writel(v, SIFIVE_SERIAL_TXCTRL_OFFS, ssp);
+}
+
+/**
+ * __ssp_wait_for_xmitr() - wait for an empty slot on the TX FIFO
+ * @ssp: pointer to a struct sifive_serial_port
+ *
+ * Delay while the UART TX FIFO referred to by @ssp is marked as full.
+ *
+ * Context: Any context.
+ */
+static void __ssp_wait_for_xmitr(struct sifive_serial_port *ssp)
+{
+	while (sifive_serial_is_txfifo_full(ssp))
+		udelay(1); /* XXX Could probably be more intelligent here */
+}
+
+/*
+ * Linux serial API functions
+ */
+
+static void sifive_serial_stop_tx(struct uart_port *port)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	__ssp_disable_txwm(ssp);
+}
+
+static void sifive_serial_stop_rx(struct uart_port *port)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	__ssp_disable_rxwm(ssp);
+}
+
+static void sifive_serial_start_tx(struct uart_port *port)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	__ssp_enable_txwm(ssp);
+}
+
+static irqreturn_t sifive_serial_irq(int irq, void *dev_id)
+{
+	struct sifive_serial_port *ssp = dev_id;
+	u32 ip;
+
+	spin_lock(&ssp->port.lock);
+
+	ip = __ssp_readl(ssp, SIFIVE_SERIAL_IP_OFFS);
+	if (!ip)
+		return IRQ_NONE;
+
+	if (ip & SIFIVE_SERIAL_IP_RXWM_MASK)
+		__ssp_receive_chars(ssp);
+	if (ip & SIFIVE_SERIAL_IP_TXWM_MASK)
+		__ssp_transmit_chars(ssp);
+
+	spin_unlock(&ssp->port.lock);
+
+	return IRQ_HANDLED;
+}
+
+static unsigned int sifive_serial_tx_empty(struct uart_port *port)
+{
+	return TIOCSER_TEMT;
+}
+
+static unsigned int sifive_serial_get_mctrl(struct uart_port *port)
+{
+	return TIOCM_CAR | TIOCM_CTS | TIOCM_DSR;
+}
+
+static void sifive_serial_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+	/* IP block does not support these signals */
+}
+
+static void sifive_serial_break_ctl(struct uart_port *port, int break_state)
+{
+	/* IP block does not support sending a break */
+}
+
+static int sifive_serial_startup(struct uart_port *port)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	__ssp_enable_rxwm(ssp);
+
+	return 0;
+}
+
+static void sifive_serial_shutdown(struct uart_port *port)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	__ssp_disable_rxwm(ssp);
+	__ssp_disable_txwm(ssp);
+}
+
+/**
+ * sifive_serial_clk_notifier() - clock post-rate-change notifier
+ * @nb: pointer to the struct notifier_block, from the notifier code
+ * @event: event mask from the notifier code
+ * @data: pointer to the struct clk_notifier_data from the notifier code
+ *
+ * On the V0 SoC, the UART IP block is derived from the CPU clock source
+ * after a synchronous divide-by-two divider, so any CPU clock rate change
+ * requires the UART baud rate to be updated.  This presumably could corrupt any
+ * serial word currently being transmitted or received.  It would probably
+ * be better to stop receives and transmits, then complete the baud rate
+ * change, then re-enable them.
+ */
+static int sifive_serial_clk_notifier(struct notifier_block *nb,
+				      unsigned long event, void *data)
+{
+	struct clk_notifier_data *cnd = data;
+	struct sifive_serial_port *ssp = notifier_to_sifive_serial_port(nb);
+
+	if (event == POST_RATE_CHANGE && ssp->clkin_rate != cnd->new_rate) {
+		ssp->clkin_rate = cnd->new_rate;
+		__ssp_update_div(ssp);
+	}
+
+	return NOTIFY_OK;
+}
+
+static void sifive_serial_set_termios(struct uart_port *port,
+				      struct ktermios *termios,
+				      struct ktermios *old)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+	unsigned long flags;
+	u32 v, old_v;
+	int rate;
+	char nstop;
+
+	if ((termios->c_cflag & CSIZE) != CS8) {
+		dev_err(ssp->port.dev, "only 8-bit words supported\n");
+		return;
+	}
+
+	/* Set number of stop bits */
+	nstop = (termios->c_cflag & CSTOPB) ? 2 : 1;
+	__ssp_set_stop_bits(ssp, nstop);
+
+	/* Set line rate */
+	rate = uart_get_baud_rate(port, termios, old, 0, ssp->clkin_rate / 16);
+	__ssp_update_baud_rate(ssp, rate);
+
+	spin_lock_irqsave(&ssp->port.lock, flags);
+
+	/*
+	 * Update the per-port timeout.
+	 */
+	uart_update_timeout(port, termios->c_cflag, rate);
+
+	ssp->port.read_status_mask = 0;
+	if (termios->c_iflag & INPCK) {
+		dev_err(ssp->port.dev, "INPCK flag not supported\n");
+		goto ssst_out;
+	}
+	if (termios->c_iflag & (BRKINT | PARMRK)) {
+		dev_err(ssp->port.dev, "BRKINT/PARMRK flag not supported\n");
+		goto ssst_out;
+	}
+
+	/*
+	 * ignore all characters if CREAD is not set
+	 */
+	v = __ssp_readl(ssp, SIFIVE_SERIAL_RXCTRL_OFFS);
+	old_v = v;
+	if ((termios->c_cflag & CREAD) == 0)
+		v &= SIFIVE_SERIAL_RXCTRL_RXEN_MASK;
+	else
+		v |= SIFIVE_SERIAL_RXCTRL_RXEN_MASK;
+	if (v != old_v)
+		__ssp_writel(v, SIFIVE_SERIAL_RXCTRL_OFFS, ssp);
+
+ssst_out:
+	spin_unlock_irqrestore(&ssp->port.lock, flags);
+}
+
+static void sifive_serial_release_port(struct uart_port *port)
+{
+}
+
+static int sifive_serial_request_port(struct uart_port *port)
+{
+	return 0;
+}
+
+static void sifive_serial_config_port(struct uart_port *port, int flags)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	ssp->port.type = PORT_SIFIVE_V0;
+}
+
+static int sifive_serial_verify_port(struct uart_port *port,
+				     struct serial_struct *ser)
+{
+	return -EINVAL;
+}
+
+static const char *sifive_serial_type(struct uart_port *port)
+{
+	return port->type == PORT_SIFIVE_V0 ? "SiFive UART v0" : NULL;
+}
+
+/*
+ * Early console support
+ */
+
+#ifdef CONFIG_SERIAL_EARLYCON
+static void early_sifive_serial_putc(struct uart_port *port, int c)
+{
+	while (__ssp_early_readl(port, SIFIVE_SERIAL_TXDATA_OFFS) &
+	       SIFIVE_SERIAL_TXDATA_FULL_MASK)
+		cpu_relax();
+
+	__ssp_early_writel(c, SIFIVE_SERIAL_TXDATA_OFFS, port);
+}
+
+static void early_sifive_serial_write(struct console *con, const char *s,
+				      unsigned int n)
+{
+	struct earlycon_device *dev = con->data;
+	struct uart_port *port = &dev->port;
+
+	uart_console_write(port, s, n, early_sifive_serial_putc);
+}
+
+static int __init early_sifive_serial_setup(struct earlycon_device *dev,
+					    const char *options)
+{
+	struct uart_port *port = &dev->port;
+
+	if (!port->membase)
+		return -ENODEV;
+
+	dev->con->write = early_sifive_serial_write;
+
+	return 0;
+}
+
+OF_EARLYCON_DECLARE(sifive, "sifive,uart0", early_sifive_serial_setup);
+OF_EARLYCON_DECLARE(sifive, "sifive,fu540-c000-uart0",
+		    early_sifive_serial_setup);
+#endif /* CONFIG_SERIAL_EARLYCON */
+
+/*
+ * Linux console interface
+ */
+
+#ifdef CONFIG_SERIAL_SIFIVE_CONSOLE
+
+static struct sifive_serial_port *sifive_serial_console_ports[SIFIVE_SERIAL_MAX_PORTS];
+
+static void sifive_serial_console_putchar(struct uart_port *port, int ch)
+{
+	struct sifive_serial_port *ssp = port_to_sifive_serial_port(port);
+
+	__ssp_wait_for_xmitr(ssp);
+	__ssp_transmit_char(ssp, ch);
+}
+
+static void sifive_serial_console_write(struct console *co, const char *s,
+					unsigned int count)
+{
+	struct sifive_serial_port *ssp = sifive_serial_console_ports[co->index];
+	unsigned long flags;
+	unsigned int ier;
+	int locked = 1;
+
+	if (!ssp)
+		return;
+
+	local_irq_save(flags);
+	if (ssp->port.sysrq)
+		locked = 0;
+	else if (oops_in_progress)
+		locked = spin_trylock(&ssp->port.lock);
+	else
+		spin_lock(&ssp->port.lock);
+
+	ier = __ssp_readl(ssp, SIFIVE_SERIAL_IE_OFFS);
+	__ssp_writel(0, SIFIVE_SERIAL_IE_OFFS, ssp);
+
+	uart_console_write(&ssp->port, s, count, sifive_serial_console_putchar);
+
+	__ssp_writel(ier, SIFIVE_SERIAL_IE_OFFS, ssp);
+
+	if (locked)
+		spin_unlock(&ssp->port.lock);
+	local_irq_restore(flags);
+}
+
+static int __init sifive_serial_console_setup(struct console *co, char *options)
+{
+	struct sifive_serial_port *ssp;
+	int baud = SIFIVE_DEFAULT_BAUD_RATE;
+	int bits = 8;
+	int parity = 'n';
+	int flow = 'n';
+
+	if (co->index < 0 || co->index >= SIFIVE_SERIAL_MAX_PORTS)
+		return -ENODEV;
+
+	ssp = sifive_serial_console_ports[co->index];
+	if (!ssp)
+		return -ENODEV;
+
+	if (options)
+		uart_parse_options(options, &baud, &parity, &bits, &flow);
+
+	pr_err("baud rate is %d\n", baud);
+	return uart_set_options(&ssp->port, co, baud, parity, bits, flow);
+}
+
+static struct uart_driver sifive_serial_uart_driver;
+
+static struct console sifive_serial_console = {
+	.name		= SIFIVE_TTY_PREFIX,
+	.write		= sifive_serial_console_write,
+	.device		= uart_console_device,
+	.setup		= sifive_serial_console_setup,
+	.flags		= CON_PRINTBUFFER,
+	.index		= -1,
+	.data		= &sifive_serial_uart_driver,
+};
+
+static int __init sifive_console_init(void)
+{
+	register_console(&sifive_serial_console);
+	return 0;
+}
+
+console_initcall(sifive_console_init);
+
+static void __ssp_add_console_port(struct sifive_serial_port *ssp)
+{
+	sifive_serial_console_ports[ssp->port.line] = ssp;
+}
+
+static void __ssp_remove_console_port(struct sifive_serial_port *ssp)
+{
+	sifive_serial_console_ports[ssp->port.line] = 0;
+}
+
+#define SIFIVE_SERIAL_CONSOLE	(&sifive_serial_console)
+
+#else
+
+#define SIFIVE_SERIAL_CONSOLE	NULL
+
+static inline void __ssp_add_console_port(struct sifive_serial_port *ssp)
+{}
+static void __ssp_remove_console_port(struct sifive_serial_port *ssp)
+{}
+
+#endif
+
+static const struct uart_ops sifive_serial_uops = {
+	.tx_empty	= sifive_serial_tx_empty,
+	.set_mctrl	= sifive_serial_set_mctrl,
+	.get_mctrl	= sifive_serial_get_mctrl,
+	.stop_tx	= sifive_serial_stop_tx,
+	.start_tx	= sifive_serial_start_tx,
+	.stop_rx	= sifive_serial_stop_rx,
+	.break_ctl	= sifive_serial_break_ctl,
+	.startup	= sifive_serial_startup,
+	.shutdown	= sifive_serial_shutdown,
+	.set_termios	= sifive_serial_set_termios,
+	.type		= sifive_serial_type,
+	.release_port	= sifive_serial_release_port,
+	.request_port	= sifive_serial_request_port,
+	.config_port	= sifive_serial_config_port,
+	.verify_port	= sifive_serial_verify_port,
+#ifdef CONFIG_CONSOLE_POLL
+	.poll_put_char  = sifive_serial_poll_put_char,
+	.poll_get_char  = sifive_serial_poll_get_char,
+#endif
+};
+
+static struct uart_driver sifive_serial_uart_driver = {
+	.owner		= THIS_MODULE,
+	.driver_name	= SIFIVE_SERIAL_NAME,
+	.dev_name	= SIFIVE_TTY_PREFIX,
+	.nr		= SIFIVE_SERIAL_MAX_PORTS,
+	.cons		= SIFIVE_SERIAL_CONSOLE,
+};
+
+static int sifive_serial_probe(struct platform_device *pdev)
+{
+	struct sifive_serial_port *ssp;
+	struct resource *mem;
+	struct clk *clk;
+	void __iomem *base;
+	int irq, id, r;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "could not acquire interrupt\n");
+		return -EPROBE_DEFER;
+	}
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(&pdev->dev, mem);
+	if (IS_ERR(base)) {
+		dev_err(&pdev->dev, "could not acquire device memory\n");
+		return PTR_ERR(base);
+	}
+
+	clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(clk)) {
+		dev_err(&pdev->dev, "unable to find controller clock\n");
+		return PTR_ERR(clk);
+	}
+
+	id = of_alias_get_id(pdev->dev.of_node, "serial");
+	if (id < 0) {
+		dev_err(&pdev->dev, "missing aliases entry\n");
+		return id;
+	}
+
+#ifdef CONFIG_SERIAL_SIFIVE_CONSOLE
+	if (id > SIFIVE_SERIAL_MAX_PORTS) {
+		dev_err(&pdev->dev, "too many UARTs (%d)\n", id);
+		return -EINVAL;
+	}
+#endif
+
+	ssp = devm_kzalloc(&pdev->dev, sizeof(*ssp), GFP_KERNEL);
+	if (!ssp)
+		return -ENOMEM;
+
+	ssp->port.dev = &pdev->dev;
+	ssp->port.type = PORT_SIFIVE_V0;
+	ssp->port.iotype = UPIO_MEM;
+	ssp->port.irq = irq;
+	ssp->port.fifosize = SIFIVE_TX_FIFO_DEPTH;
+	ssp->port.ops = &sifive_serial_uops;
+	ssp->port.line = id;
+	ssp->port.mapbase = mem->start;
+	ssp->port.membase = base;
+	ssp->dev = &pdev->dev;
+	ssp->clk = clk;
+	ssp->clk_notifier.notifier_call = sifive_serial_clk_notifier;
+
+	r = clk_notifier_register(ssp->clk, &ssp->clk_notifier);
+	if (r) {
+		dev_err(&pdev->dev, "could not register clock notifier: %d\n",
+			r);
+		goto probe_out1;
+	}
+
+	/* Set up clock divider */
+	ssp->clkin_rate = clk_get_rate(ssp->clk);
+	ssp->baud_rate = SIFIVE_DEFAULT_BAUD_RATE;
+	__ssp_update_div(ssp);
+
+	platform_set_drvdata(pdev, ssp);
+
+	/* Enable transmits and set the watermark level to 1 */
+	__ssp_writel((1 << SIFIVE_SERIAL_TXCTRL_TXCNT_SHIFT) |
+		     SIFIVE_SERIAL_TXCTRL_TXEN_MASK,
+		     SIFIVE_SERIAL_TXCTRL_OFFS, ssp);
+
+	/* Enable receives and set the watermark level to 0 */
+	__ssp_writel((0 << SIFIVE_SERIAL_RXCTRL_RXCNT_SHIFT) |
+		     SIFIVE_SERIAL_RXCTRL_RXEN_MASK,
+		     SIFIVE_SERIAL_RXCTRL_OFFS, ssp);
+
+	r = request_irq(ssp->port.irq, sifive_serial_irq, ssp->port.irqflags,
+			dev_name(&pdev->dev), ssp);
+	if (r) {
+		dev_err(&pdev->dev, "could not attach interrupt: %d\n", r);
+		goto probe_out2;
+	}
+
+	__ssp_add_console_port(ssp);
+
+	r = uart_add_one_port(&sifive_serial_uart_driver, &ssp->port);
+	if (r != 0) {
+		dev_err(&pdev->dev, "could not add uart: %d\n", r);
+		goto probe_out3;
+	}
+
+	return 0;
+
+probe_out3:
+	__ssp_remove_console_port(ssp);
+	free_irq(ssp->port.irq, ssp);
+probe_out2:
+	clk_notifier_unregister(ssp->clk, &ssp->clk_notifier);
+probe_out1:
+	return r;
+}
+
+static int sifive_serial_remove(struct platform_device *dev)
+{
+	struct sifive_serial_port *ssp = platform_get_drvdata(dev);
+
+	__ssp_remove_console_port(ssp);
+	uart_remove_one_port(&sifive_serial_uart_driver, &ssp->port);
+	free_irq(ssp->port.irq, ssp);
+	clk_notifier_unregister(ssp->clk, &ssp->clk_notifier);
+
+	return 0;
+}
+
+static const struct of_device_id sifive_serial_of_match[] = {
+	{ .compatible = "sifive,fu540-c000-uart0" },
+	{ .compatible = "sifive,uart0" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sifive_serial_match);
+
+static struct platform_driver sifive_serial_platform_driver = {
+	.probe		= sifive_serial_probe,
+	.remove		= sifive_serial_remove,
+	.driver		= {
+		.name	= SIFIVE_SERIAL_NAME,
+		.of_match_table = of_match_ptr(sifive_serial_of_match),
+	},
+};
+
+static int __init sifive_serial_init(void)
+{
+	int r;
+
+	r = uart_register_driver(&sifive_serial_uart_driver);
+	if (r)
+		goto init_out1;
+
+	r = platform_driver_register(&sifive_serial_platform_driver);
+	if (r)
+		goto init_out2;
+
+	return 0;
+
+init_out2:
+	uart_unregister_driver(&sifive_serial_uart_driver);
+init_out1:
+	return r;
+}
+
+static void __exit sifive_serial_exit(void)
+{
+	platform_driver_unregister(&sifive_serial_platform_driver);
+	uart_unregister_driver(&sifive_serial_uart_driver);
+}
+
+module_init(sifive_serial_init);
+module_exit(sifive_serial_exit);
+
+MODULE_DESCRIPTION("SiFive UART serial driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Paul Walmsley <paul@pwsan.com>");
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index dce5f9dae121..569d5172d23c 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -281,4 +281,7 @@
 /* MediaTek BTIF */
 #define PORT_MTK_BTIF	117
 
+/* SiFive UART */
+#define PORT_SIFIVE_V0	118
+
 #endif /* _UAPILINUX_SERIAL_CORE_H */
-- 
2.19.1

^ permalink raw reply related

* [PATCH 1/2] dt-bindings: serial: add documentation for the SiFive UART driver
From: Paul Walmsley @ 2018-10-18 23:43 UTC (permalink / raw)
  To: linux-serial
  Cc: Paul Walmsley, devicetree, linux-riscv, linux-kernel,
	Greg Kroah-Hartman, Rob Herring, Mark Rutland, Palmer Dabbelt,
	Paul Walmsley
In-Reply-To: <20181018234352.26788-1-paul.walmsley@sifive.com>

Add DT binding documentation for the Linux driver for the SiFive
asynchronous serial IP block.  Nothing too exotic.

Cc: linux-serial@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-riscv@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Palmer Dabbelt <palmer@sifive.com>
Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 .../bindings/serial/sifive-serial.txt         | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/sifive-serial.txt

diff --git a/Documentation/devicetree/bindings/serial/sifive-serial.txt b/Documentation/devicetree/bindings/serial/sifive-serial.txt
new file mode 100644
index 000000000000..8982338512f5
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/sifive-serial.txt
@@ -0,0 +1,21 @@
+SiFive asynchronous serial interface (UART)
+
+Required properties:
+
+- compatible: should be "sifive,fu540-c000-uart0" or "sifive,uart0"
+- reg: address and length of the register space
+- interrupt-parent: should contain a phandle pointing to the SoC interrupt
+    controller device node that the UART interrupts are connected to
+- interrupts: Should contain the UART interrupt identifier
+- clocks: Should contain a clock identifier for the UART's parent clock
+
+
+Example:
+
+uart0: serial@10010000 {
+	compatible = "sifive,uart0";
+	interrupt-parent = <&plic0>;
+	interrupts = <80>;
+	reg = <0x0 0x10010000 0x0 0x1000>;
+	clocks = <&prci PRCI_CLK_TLCLK>;
+};
-- 
2.19.1

^ permalink raw reply related

* Re: [[PATCH] 8/9] DMA-UART-Driver-for-AST2500
From: Benjamin Herrenschmidt @ 2018-10-18 23:32 UTC (permalink / raw)
  To: Vinod
  Cc: Mark Rutland, Kate Stewart, linux-aspeed, Hendrik Brueckner,
	Thomas Gleixner, Michael Moese, Russell King,
	Christian Borntraeger, Joel Stanley, linux-serial, Jiri Slaby,
	devicetree, Marc Zyngier, ShivahShankar Shakarnarayan rao,
	Rob Herring, Dan Williams, linux-arm-kernel, Sudheer V,
	Andrew Jeffery, Greg Kroah-Hartman, linux-kernel, sudheer.v,
	Philippe
In-Reply-To: <20181018095507.GC2400@vkoul-mobl>

On Thu, 2018-10-18 at 15:25 +0530, Vinod wrote:
> 
> > It's not a dmaengine driver. It's a serial UART driver that happens to
> > use a dedicated DMA engine.
> 
> Then I see no reason for it to use dmaengine APIs. The framework allows
> people to share a controller for many clients, but if you have dedicated
> one then you may use it directly

Well... the engine is shared by a few UARTs, they have dedicated rings
but there's a common set of regs for interrupt handling etc.

That said, I still think it could be contained within a UART driver,
there's little benefit in adding the framework overhead, esp since
these are really weak cores, any overhead will be felt.

Ben.

> > It's unclear whether it should be split into two drivers, or just have
> > the serial driver directly use the dma engine since that engine is
> > dedicated in HW to only work on those UARTs and nothing else...
> > 
> > Cheers,
> > Ben.
> > 
> > 
> > > While doing resubmission please take some time to understand subsystem
> > > tags to use. (hint git log <subsystem> will tell you)
> > > 
> > > Also series has [[PATCH] 8/9] whereas it should be [PATCH 8/9] please
> > > let git generate that for you (hint git format-patch start..end does a
> > > good job)
> > > 
> > > > @@ -0,0 +1,1594 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + *   drivers/tty/serial/8250/8250_aspeed_uart_dma.c
> > > > + *    1. 2018/07/01 Shivah Shankar created
> > > > + *    2. 2018/08/25 sudheer.veliseti<open.sudheer@gmail.com> modified
> > > 
> > > we dont use this log in kernel. I do not see s-o-b by Shivah, that
> > > should be added. I think he should be author and you need to list
> > > changes you did..
> > > 
> 
> 

^ permalink raw reply

* Re: [PATCH V3 3/8] dt-bindings: mmc: fsl-imx-esdhc: add imx8qxp compatible string
From: Rob Herring @ 2018-10-18 19:48 UTC (permalink / raw)
  Cc: A.s. Dong, Mark Rutland, Ulf Hansson, dongas86@gmail.com,
	devicetree@vger.kernel.org, catalin.marinas@arm.com,
	will.deacon@arm.com, Bough Chen, robh+dt@kernel.org, dl-linux-imx,
	kernel@pengutronix.de, linux-serial@vger.kernel.org,
	Fabio Estevam, linux-mmc@vger.kernel.org, shawnguo@kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <1539886483-15775-4-git-send-email-aisheng.dong@nxp.com>

On Thu, 18 Oct 2018 18:19:24 +0000, "A.s. Dong" wrote:
> Add imx8qxp compatible string
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: linux-serial@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-mmc@vger.kernel.org
> Cc: Haibo Chen <haibo.chen@nxp.com>
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
>  Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt | 1 +
>  1 file changed, 1 insertion(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH V3 2/8] dt-bindings: serial: lpuart: add imx8qxp compatible  string
From: Rob Herring @ 2018-10-18 19:48 UTC (permalink / raw)
  Cc: A.s. Dong, Mark Rutland, dongas86@gmail.com,
	devicetree@vger.kernel.org, catalin.marinas@arm.com,
	will.deacon@arm.com, robh+dt@kernel.org, dl-linux-imx,
	kernel@pengutronix.de, linux-serial@vger.kernel.org,
	Fabio Estevam, shawnguo@kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <1539886483-15775-3-git-send-email-aisheng.dong@nxp.com>

On Thu, 18 Oct 2018 18:19:18 +0000, "A.s. Dong" wrote:
> Add imx8qxp compatible string
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: linux-serial@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
>  Documentation/devicetree/bindings/serial/fsl-lpuart.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* [PATCH V3 3/8] dt-bindings: mmc: fsl-imx-esdhc: add imx8qxp compatible string
From: A.s. Dong @ 2018-10-18 18:19 UTC (permalink / raw)
  To: linux-arm-kernel@lists.infradead.org
  Cc: A.s. Dong, Mark Rutland, Ulf Hansson, dongas86@gmail.com,
	devicetree@vger.kernel.org, catalin.marinas@arm.com,
	will.deacon@arm.com, Bough Chen, robh+dt@kernel.org, dl-linux-imx,
	kernel@pengutronix.de, linux-serial@vger.kernel.org,
	Fabio Estevam, linux-mmc@vger.kernel.org, shawnguo@kernel.org
In-Reply-To: <1539886483-15775-1-git-send-email-aisheng.dong@nxp.com>

Add imx8qxp compatible string

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: linux-serial@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-mmc@vger.kernel.org
Cc: Haibo Chen <haibo.chen@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
index 3e29050..9201a7d 100644
--- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
+++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.txt
@@ -16,6 +16,7 @@ Required properties:
 	       "fsl,imx6sl-usdhc"
 	       "fsl,imx6sx-usdhc"
 	       "fsl,imx7d-usdhc"
+	       "fsl,imx8qxp-usdhc"
 
 Optional properties:
 - fsl,wp-controller : Indicate to use controller internal write protection
-- 
2.7.4

^ permalink raw reply related

* [PATCH V3 2/8] dt-bindings: serial: lpuart: add imx8qxp compatible string
From: A.s. Dong @ 2018-10-18 18:19 UTC (permalink / raw)
  To: linux-arm-kernel@lists.infradead.org
  Cc: A.s. Dong, Mark Rutland, dongas86@gmail.com,
	devicetree@vger.kernel.org, catalin.marinas@arm.com,
	will.deacon@arm.com, robh+dt@kernel.org, dl-linux-imx,
	kernel@pengutronix.de, linux-serial@vger.kernel.org,
	Fabio Estevam, shawnguo@kernel.org
In-Reply-To: <1539886483-15775-1-git-send-email-aisheng.dong@nxp.com>

Add imx8qxp compatible string

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-serial@vger.kernel.org
Cc: devicetree@vger.kernel.org
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 Documentation/devicetree/bindings/serial/fsl-lpuart.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/fsl-lpuart.txt b/Documentation/devicetree/bindings/serial/fsl-lpuart.txt
index 6bd3f2e..21483ba 100644
--- a/Documentation/devicetree/bindings/serial/fsl-lpuart.txt
+++ b/Documentation/devicetree/bindings/serial/fsl-lpuart.txt
@@ -8,6 +8,8 @@ Required properties:
     on LS1021A SoC with 32-bit big-endian register organization
   - "fsl,imx7ulp-lpuart" for lpuart compatible with the one integrated
     on i.MX7ULP SoC with 32-bit little-endian register organization
+  - "fsl,imx8qxp-lpuart" for lpuart compatible with the one integrated
+    on i.MX8QXP SoC with 32-bit little-endian register organization
 - reg : Address and length of the register set for the device
 - interrupts : Should contain uart interrupt
 - clocks : phandle + clock specifier pairs, one for each entry in clock-names
-- 
2.7.4

^ permalink raw reply related

* Re: Crash in msm serial on dragonboard with ftrace bootargs
From: Steven Rostedt @ 2018-10-18 13:17 UTC (permalink / raw)
  To: Sai Prakash Ranjan
  Cc: Stephen Boyd, Bjorn Andersson, Andy Gross, David Brown,
	Jiri Slaby, Kees Cook, Joel Fernandes (Google), Geliang Tang,
	Greg Kroah-Hartman, Pramod Gurav, linux-arm-msm, linux-soc,
	linux-serial, linux-kernel, Rajendra Nayak, Vivek Gautam,
	Sibi Sankar
In-Reply-To: <58d2474c-53cd-e6cb-2d25-db38d1a88da6@codeaurora.org>

On Thu, 18 Oct 2018 10:51:18 +0530
Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> wrote:

> > So something else is causing an issue besides just msm_read.
> > 
> > Can you do an objdump -dr of the entire vmlinux binary and gzip it and
> > post it somewhere. Not sure if it would be too big to email. You could
> > try sending it to me privately. I'd like to see the binary that you are
> > using.
> >   
> 
> I have sent the objdump and dot config to you privately.

Thanks. I don't see anything that pops out, but then again, my arm asm
foo is very rusty (it has been literally decades since I did any arm
asm). I wonder if it could simply be a timing issue?

ffff0000086eb538 <msm_read.isra.1>:
ffff0000086eb538:       a9be7bfd        stp     x29, x30, [sp,#-32]!
ffff0000086eb53c:       910003fd        mov     x29, sp
ffff0000086eb540:       a90153f3        stp     x19, x20, [sp,#16]
ffff0000086eb544:       aa0003f4        mov     x20, x0
ffff0000086eb548:       2a0103f3        mov     w19, w1
ffff0000086eb54c:       aa1e03e0        mov     x0, x30
ffff0000086eb550:       97e6bae4        bl      ffff00000809a0e0 <_mcount>

The above is changed to nop on boot, but then to:

	bl ftrace_caller

When ftrace is enabled.

ffff0000086eb554:       8b334280        add     x0, x20, w19, uxtw
ffff0000086eb558:       b9400000        ldr     w0, [x0]
ffff0000086eb55c:       a94153f3        ldp     x19, x20, [sp,#16]
ffff0000086eb560:       a8c27bfd        ldp     x29, x30, [sp],#32
ffff0000086eb564:       d65f03c0        ret



ffff00000809a0e4 <ftrace_caller>:
ffff00000809a0e4:       a9bf7bfd        stp     x29, x30, [sp,#-16]!
ffff00000809a0e8:       910003fd        mov     x29, sp
ffff00000809a0ec:       d10013c0        sub     x0, x30, #0x4
ffff00000809a0f0:       f94003a1        ldr     x1, [x29]
ffff00000809a0f4:       f9400421        ldr     x1, [x1,#8]
ffff00000809a0f8:       d1001021        sub     x1, x1, #0x4

ffff00000809a0fc <ftrace_call>:
ffff00000809a0fc:       d503201f        nop

The above nop gets patched to:

	bl ftrace_ops_no_ops

Which will iterate through all the registered functions.


ffff00000809a100 <ftrace_graph_call>:
ffff00000809a100:       d503201f        nop

The above only gets set when function graph tracer is enabled, which it
is not in this case.

ffff00000809a104:       a8c17bfd        ldp     x29, x30, [sp],#16
ffff00000809a108:       d65f03c0        ret


Anyone see any problems here?

-- Steve

^ permalink raw reply

* Re: [[PATCH] 8/9] DMA-UART-Driver-for-AST2500
From: Vinod @ 2018-10-18  9:55 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: sudheer.v, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
	Joel Stanley, Andrew Jeffery, Russell King, Dan Williams,
	Jiri Slaby, Thomas Gleixner, Marc Zyngier, Christian Borntraeger,
	Michael Moese, Hendrik Brueckner, Kate Stewart,
	Philippe Ombredanne, dmaengine, devicetree, linux-kernel,
	linux-serial, linux-arm-k
In-Reply-To: <351eecd5b1b21893e94f76b34c058c6257b7f837.camel@kernel.crashing.org>

On 17-10-18, 19:56, Benjamin Herrenschmidt wrote:
> On Wed, 2018-10-17 at 11:35 +0530, Vinod wrote:
> > On 17-10-18, 09:41, sudheer.v wrote:
> > 
> > Please add the change log describing the driver and its features
> > 
> > > Signed-off-by: sudheer.v <open.sudheer@gmail.com>
> > > ---
> > >  drivers/tty/serial/8250/8250_aspeed_uart_dma.c | 1594 ++++++++++++++++++++++++
> > >  1 file changed, 1594 insertions(+)
> > >  create mode 100644 drivers/tty/serial/8250/8250_aspeed_uart_dma.c
> > > 
> > > diff --git a/drivers/tty/serial/8250/8250_aspeed_uart_dma.c b/drivers/tty/serial/8250/8250_aspeed_uart_dma.c
> > > new file mode 100644
> > > index 0000000..e1019a8
> > > --- /dev/null
> > > +++ b/drivers/tty/serial/8250/8250_aspeed_uart_dma.c
> > 
> > why is this in serial. It is dmaengine driver so belongs to drivers/dma/
> > like other controllers. Please move it out and resubmit.
>  
> It's not a dmaengine driver. It's a serial UART driver that happens to
> use a dedicated DMA engine.

Then I see no reason for it to use dmaengine APIs. The framework allows
people to share a controller for many clients, but if you have dedicated
one then you may use it directly

> It's unclear whether it should be split into two drivers, or just have
> the serial driver directly use the dma engine since that engine is
> dedicated in HW to only work on those UARTs and nothing else...
> 
> Cheers,
> Ben.
> 
> 
> > While doing resubmission please take some time to understand subsystem
> > tags to use. (hint git log <subsystem> will tell you)
> > 
> > Also series has [[PATCH] 8/9] whereas it should be [PATCH 8/9] please
> > let git generate that for you (hint git format-patch start..end does a
> > good job)
> > 
> > > @@ -0,0 +1,1594 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + *   drivers/tty/serial/8250/8250_aspeed_uart_dma.c
> > > + *    1. 2018/07/01 Shivah Shankar created
> > > + *    2. 2018/08/25 sudheer.veliseti<open.sudheer@gmail.com> modified
> > 
> > we dont use this log in kernel. I do not see s-o-b by Shivah, that
> > should be added. I think he should be author and you need to list
> > changes you did..
> > 

-- 
~Vinod

^ permalink raw reply

* Re: Crash in msm serial on dragonboard with ftrace bootargs
From: Sai Prakash Ranjan @ 2018-10-18  5:21 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Stephen Boyd, Bjorn Andersson, Andy Gross, David Brown,
	Jiri Slaby, Kees Cook, Joel Fernandes (Google), Geliang Tang,
	Greg Kroah-Hartman, Pramod Gurav, linux-arm-msm, linux-soc,
	linux-serial, linux-kernel, Rajendra Nayak, Vivek Gautam,
	Sibi Sankar
In-Reply-To: <20181017223334.29ca2837@vmware.local.home>

On 10/18/2018 8:03 AM, Steven Rostedt wrote:
> On Wed, 17 Oct 2018 00:36:05 +0530
> Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> wrote:
> 
>> On 10/17/2018 12:33 AM, Steven Rostedt wrote:
>>> On Wed, 17 Oct 2018 00:31:03 +0530
>>> Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> wrote:
>>>    
>>>> Haa seems like you are right! With "ftrace=function
>>>> ftrace_filter=msm_read" , I can trigger the crash, but
>>>> sadly "ftrace_notrace=msm_read" also crashes.
>>>
>>> So there's more than one problem area.
>>>
>>> What about ftrace_notrace=m*
>>>
>>> ?
>>>    
>>
>> That too crashes.
>>
> 
> So something else is causing an issue besides just msm_read.
> 
> Can you do an objdump -dr of the entire vmlinux binary and gzip it and
> post it somewhere. Not sure if it would be too big to email. You could
> try sending it to me privately. I'd like to see the binary that you are
> using.
> 

I have sent the objdump and dot config to you privately.

Thanks,
Sai
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply

* Re: Crash in msm serial on dragonboard with ftrace bootargs
From: Steven Rostedt @ 2018-10-18  2:33 UTC (permalink / raw)
  To: Sai Prakash Ranjan
  Cc: Stephen Boyd, Bjorn Andersson, Andy Gross, David Brown,
	Jiri Slaby, Kees Cook, Joel Fernandes (Google), Geliang Tang,
	Greg Kroah-Hartman, Pramod Gurav, linux-arm-msm, linux-soc,
	linux-serial, linux-kernel, Rajendra Nayak, Vivek Gautam,
	Sibi Sankar
In-Reply-To: <c94103d2-ed5d-bf43-fc71-c60e108f303e@codeaurora.org>

On Wed, 17 Oct 2018 00:36:05 +0530
Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> wrote:

> On 10/17/2018 12:33 AM, Steven Rostedt wrote:
> > On Wed, 17 Oct 2018 00:31:03 +0530
> > Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> wrote:
> >   
> >> Haa seems like you are right! With "ftrace=function
> >> ftrace_filter=msm_read" , I can trigger the crash, but
> >> sadly "ftrace_notrace=msm_read" also crashes.  
> > 
> > So there's more than one problem area.
> > 
> > What about ftrace_notrace=m*
> > 
> > ?
> >   
> 
> That too crashes.
> 

So something else is causing an issue besides just msm_read.

Can you do an objdump -dr of the entire vmlinux binary and gzip it and
post it somewhere. Not sure if it would be too big to email. You could
try sending it to me privately. I'd like to see the binary that you are
using.

-- Steve

^ permalink raw reply

* Re: [PATCH 1/2] serial: set suppress_bind_attrs flag only if builtin
From: Jeffrey Hugo @ 2018-10-17 22:41 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Anders Roxell, Arnd Bergmann, gregkh, linux, linux-kernel,
	linux-mm, linux-serial, tj, linux-arm-kernel
In-Reply-To: <20181017153223.6c4e5156895dad7973f7d059@linux-foundation.org>

On 10/17/2018 4:32 PM, Andrew Morton wrote:
> On Wed, 17 Oct 2018 16:21:08 -0600 Jeffrey Hugo <jhugo@codeaurora.org> wrote:
> 
>> On 10/17/2018 4:05 PM, Andrew Morton wrote:
>>> On Wed, 17 Oct 2018 16:03:10 +0200 Anders Roxell <anders.roxell@linaro.org> wrote:
>>>
>>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>>> Co-developed-by: Arnd Bergmann <arnd@arndb.de>
>>>> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
>>>
>>> This should have Arnd's Signed-off-by: as well.
>>
>> I'm just interested to know, why?
> 
> So that Arnd certifies that
> 
>          (a) The contribution was created in whole or in part by me and I
>              have the right to submit it under the open source license
>              indicated in the file; or
> 
> and all the other stuff in Documentation/process/submitting-patches.rst
> section 11!
> 
> Also, because section 12 says so :)  And that final sentence is, I
> believe, appropriate.

Ah, interesting.  I see that the documentation was updated since I was 
told last year in the same situation by a different maintainer that 
co-authors should not have SOB lines on the patch.

Good to know.  Thanks for illuminating me.

-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply

* Re: [PATCH 1/2] serial: set suppress_bind_attrs flag only if builtin
From: Andrew Morton @ 2018-10-17 22:32 UTC (permalink / raw)
  To: Jeffrey Hugo
  Cc: Anders Roxell, Arnd Bergmann, gregkh, linux, linux-kernel,
	linux-mm, linux-serial, tj, linux-arm-kernel
In-Reply-To: <e0763032-3ae6-b352-e586-ad131ce689ca@codeaurora.org>

On Wed, 17 Oct 2018 16:21:08 -0600 Jeffrey Hugo <jhugo@codeaurora.org> wrote:

> On 10/17/2018 4:05 PM, Andrew Morton wrote:
> > On Wed, 17 Oct 2018 16:03:10 +0200 Anders Roxell <anders.roxell@linaro.org> wrote:
> > 
> >> Cc: Arnd Bergmann <arnd@arndb.de>
> >> Co-developed-by: Arnd Bergmann <arnd@arndb.de>
> >> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> > 
> > This should have Arnd's Signed-off-by: as well.
> 
> I'm just interested to know, why?

So that Arnd certifies that

        (a) The contribution was created in whole or in part by me and I
            have the right to submit it under the open source license
            indicated in the file; or

and all the other stuff in Documentation/process/submitting-patches.rst
section 11!

Also, because section 12 says so :)  And that final sentence is, I
believe, appropriate.

^ permalink raw reply

* Re: [PATCH 1/2] serial: set suppress_bind_attrs flag only if builtin
From: Jeffrey Hugo @ 2018-10-17 22:21 UTC (permalink / raw)
  To: Andrew Morton, Anders Roxell
  Cc: Arnd Bergmann, gregkh, linux, linux-kernel, linux-mm,
	linux-serial, tj, linux-arm-kernel
In-Reply-To: <20181017150546.0d451252950214bec74a6fc8@linux-foundation.org>

On 10/17/2018 4:05 PM, Andrew Morton wrote:
> On Wed, 17 Oct 2018 16:03:10 +0200 Anders Roxell <anders.roxell@linaro.org> wrote:
> 
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Co-developed-by: Arnd Bergmann <arnd@arndb.de>
>> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> 
> This should have Arnd's Signed-off-by: as well.

I'm just interested to know, why?

-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply

* Re: [PATCH 2/2] writeback: don't decrement wb->refcnt if !wb->bdi
From: Andrew Morton @ 2018-10-17 22:13 UTC (permalink / raw)
  To: Anders Roxell
  Cc: linux, gregkh, linux-serial, linux-kernel, linux-arm-kernel,
	linux-mm, tj, Arnd Bergmann
In-Reply-To: <20181017140311.28679-2-anders.roxell@linaro.org>

On Wed, 17 Oct 2018 16:03:11 +0200 Anders Roxell <anders.roxell@linaro.org> wrote:

> When enabling CONFIG_DEBUG_TEST_DRIVER_REMOVE devtmpfs gets killed
> because we try to remove a file and decrement the wb reference count
> before the noop_backing_device_info gets initialized.
> 
> Since arch_initcall(pl011_init) came before
> subsys_initcall(default_bdi_init), devtmpfs' handle_remove() crashes
> because the reference count is a NULL pointer only because bdi->wb
> hasn't been initialized yet.

Is this changelog correct?  What does drivers/tty/serial/amba-pl011.c
have to do with page writeback?  Confused.

> [    0.332075] Serial: AMBA PL011 UART driver
> [    0.485276] 9000000.pl011: ttyAMA0 at MMIO 0x9000000 (irq = 39, base_baud = 0) is a PL011 rev1
> [    0.502382] console [ttyAMA0] enabled
> [    0.515710] Unable to handle kernel paging request at virtual address 0000800074c12000
> [    0.516053] Mem abort info:
> [    0.516222]   ESR = 0x96000004
> [    0.516417]   Exception class = DABT (current EL), IL = 32 bits
> [    0.516641]   SET = 0, FnV = 0
> [    0.516826]   EA = 0, S1PTW = 0
> [    0.516984] Data abort info:
> [    0.517149]   ISV = 0, ISS = 0x00000004
> [    0.517339]   CM = 0, WnR = 0
> [    0.517553] [0000800074c12000] user address but active_mm is swapper
> [    0.517928] Internal error: Oops: 96000004 [#1] PREEMPT SMP
> [    0.518305] Modules linked in:
> [    0.518839] CPU: 0 PID: 13 Comm: kdevtmpfs Not tainted 4.19.0-rc5-next-20180928-00002-g2ba39ab0cd01-dirty #82
> [    0.519307] Hardware name: linux,dummy-virt (DT)
> [    0.519681] pstate: 80000005 (Nzcv daif -PAN -UAO)
> [    0.519959] pc : __destroy_inode+0x94/0x2a8
> [    0.520212] lr : __destroy_inode+0x78/0x2a8
> [    0.520401] sp : ffff0000098c3b20
> [    0.520590] x29: ffff0000098c3b20 x28: 00000000087a3714
> [    0.520904] x27: 0000000000002000 x26: 0000000000002000
> [    0.521179] x25: ffff000009583000 x24: 0000000000000000
> [    0.521467] x23: ffff80007bb52000 x22: ffff80007bbaa7c0
> [    0.521737] x21: ffff0000093f9338 x20: 0000000000000000
> [    0.522033] x19: ffff80007bbb05d8 x18: 0000000000000400
> [    0.522376] x17: 0000000000000000 x16: 0000000000000000
> [    0.522727] x15: 0000000000000400 x14: 0000000000000400
> [    0.523068] x13: 0000000000000001 x12: 0000000000000001
> [    0.523421] x11: 0000000000000000 x10: 0000000000000970
> [    0.523749] x9 : ffff0000098c3a60 x8 : ffff80007bbab190
> [    0.524017] x7 : ffff80007bbaa880 x6 : 0000000000000c88
> [    0.524305] x5 : ffff0000093d96c8 x4 : 61c8864680b583eb
> [    0.524567] x3 : ffff0000093d6180 x2 : ffffffffffffffff
> [    0.524872] x1 : 0000800074c12000 x0 : 0000800074c12000
> [    0.525207] Process kdevtmpfs (pid: 13, stack limit = 0x(____ptrval____))
> [    0.525529] Call trace:
> [    0.525806]  __destroy_inode+0x94/0x2a8
> [    0.526108]  destroy_inode+0x34/0x88
> [    0.526370]  evict+0x144/0x1c8
> [    0.526636]  iput+0x184/0x230
> [    0.526871]  dentry_unlink_inode+0x118/0x130
> [    0.527152]  d_delete+0xd8/0xe0
> [    0.527420]  vfs_unlink+0x240/0x270
> [    0.527665]  handle_remove+0x1d8/0x330
> [    0.527875]  devtmpfsd+0x138/0x1c8
> [    0.528085]  kthread+0x14c/0x158
> [    0.528291]  ret_from_fork+0x10/0x18
> [    0.528720] Code: 92800002 aa1403e0 d538d081 8b010000 (c85f7c04)

Seems that there is indeed some form of linkage.  Can this be spelled
out more in the changelog please?

> 
> Rework so that wb_put have an extra check if wb->bdi before decrement
> wb->refcnt and also add a WARN_ON to get a warning if it happens again
> in other drivers.
> 
> Fixes: 52ebea749aae ("writeback: make backing_dev_info host cgroup-specific bdi_writebacks")
> Cc: Arnd Bergmann <arnd@arndb.de>
> Co-developed-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>

Signed-off-by: Arnd, please.

> --- a/include/linux/backing-dev-defs.h
> +++ b/include/linux/backing-dev-defs.h
> @@ -258,7 +258,7 @@ static inline void wb_get(struct bdi_writeback *wb)
>   */
>  static inline void wb_put(struct bdi_writeback *wb)
>  {
> -	if (wb != &wb->bdi->wb)
> +	if (!WARN_ON(!wb->bdi) && wb != &wb->bdi->wb)
>  		percpu_ref_put(&wb->refcnt);
>  }

The !WARN_ON(!expr) isn't very easy to follow.   This:

{
	if (WARN_ON_ONCE(!wb->bdi)) {
		/*
		 * Nice comment explaining how this situation comes about
		 */
		return;
	}

	if (wb != &wb->bdi->wb)
		percpu_ref_put(&wb->refcnt);
}

is better, no?

Also, please note the s/WARN_ON/WARN_ON_ONCE/.  I don't think we gain
anything from reporting the same thing many times?

^ permalink raw reply

* Re: [PATCH 1/2] serial: set suppress_bind_attrs flag only if builtin
From: Andrew Morton @ 2018-10-17 22:05 UTC (permalink / raw)
  To: Anders Roxell
  Cc: linux, gregkh, linux-serial, linux-kernel, linux-arm-kernel,
	linux-mm, tj, Arnd Bergmann
In-Reply-To: <20181017140311.28679-1-anders.roxell@linaro.org>

On Wed, 17 Oct 2018 16:03:10 +0200 Anders Roxell <anders.roxell@linaro.org> wrote:

> Cc: Arnd Bergmann <arnd@arndb.de>
> Co-developed-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>

This should have Arnd's Signed-off-by: as well.

^ permalink raw reply

* Re: Crash in msm serial on dragonboard with ftrace bootargs
From: Kees Cook @ 2018-10-17 18:25 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Sai Prakash Ranjan, Steven Rostedt, Stephen Boyd, Bjorn Andersson,
	Andy Gross, David Brown, Jiri Slaby, Ivan T. Ivanov, Geliang Tang,
	Greg Kroah-Hartman, Pramod Gurav, linux-arm-msm, linux-soc,
	open list:SERIAL DRIVERS, LKML, Rajendra Nayak, Vivek Gautam,
	Sibi Sankar
In-Reply-To: <20181017175611.GB107185@joelaf.mtv.corp.google.com>

On Wed, Oct 17, 2018 at 10:56 AM, Joel Fernandes <joel@joelfernandes.org> wrote:
> On Wed, Oct 17, 2018 at 08:19:41PM +0530, Sai Prakash Ranjan wrote:
>> On 10/17/2018 5:08 PM, Sai Prakash Ranjan wrote:
>> > >
>> > > What do you think about the (untested) patch below? It seems to me
>> > > that it
>> > > should solve the issue of missing early crash dumps, but I have not
>> > > tested it
>> > > yet. Sai, would you mind trying it out and let me know if you can see the
>> > > early crash dumps properly now?
>> > >
>> > > ----8<---
>> > > From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
>> > > Subject: [RFC] pstore: allocate compression during late_initcall
>> > >
>> > > ramoop's pstore registration (using pstore_register) has to run during
>> > > late_initcall because crypto backend may not be ready during
>> > > postcore_initcall. This causes missing of dmesg crash dumps which could
>> > > have been caught by pstore.
>> > >
>> > > Instead, lets allow ramoops pstore registration earlier, and once crypto
>> > > is ready we can initialize the compression.
>> > >
>> > > Reported-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
>> > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
>> > > ---
>> > >   fs/pstore/platform.c | 13 +++++++++++++
>> > >   fs/pstore/ram.c      |  2 +-
>> > >   2 files changed, 14 insertions(+), 1 deletion(-)
>> > >
>> > > diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
>> > > index 15e99d5a681d..f09066db2d4d 100644
>> > > --- a/fs/pstore/platform.c
>> > > +++ b/fs/pstore/platform.c
>> > > @@ -780,6 +780,19 @@ void __init pstore_choose_compression(void)
>> > >       }
>> > >   }
>> > > +static int __init pstore_compression_late_init(void)
>> > > +{
>> > > +    /*
>> > > +     * Check if any pstore backends registered earlier but did not
>> > > allocate
>> > > +     * for compression because crypto was not ready, if so then
>> > > initialize
>> > > +     * compression.
>> > > +     */
>> > > +    if (psinfo && !tfm)
>> > > +        allocate_buf_for_compression();
>> > > +    return 0;
>> > > +}
>> > > +late_initcall(pstore_compression_late_init);
>> > > +
>> > >   module_param(compress, charp, 0444);
>> > >   MODULE_PARM_DESC(compress, "Pstore compression to use");
>> > > diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
>> > > index bbd1e357c23d..98e48d1a9776 100644
>> > > --- a/fs/pstore/ram.c
>> > > +++ b/fs/pstore/ram.c
>> > > @@ -940,7 +940,7 @@ static int __init ramoops_init(void)
>> > >       ramoops_register_dummy();
>> > >       return platform_driver_register(&ramoops_driver);
>> > >   }
>> > > -late_initcall(ramoops_init);
>> > > +postcore_initcall(ramoops_init);
>> > >   static void __exit ramoops_exit(void)
>> > >   {
>> > >
>> >
>> > Yes I could see the early crash dump. Also I tested with different
>> > compression (LZO) instead of deflate just to be sure and it works fine,
>> > thanks :)
>> >
>> > Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
>> >
>
> Thanks.
>
>> I just noticed that allocate_buf_for_compression() is also called from
>> pstore_register(). Shouldn't that call be removed now that ramoops_init is
>> moved to postcore_initcall and allocate_buf_for_compression() will just
>> return doing nothing when called from pstore_register()?
>
> Yes, that is the point. If crypto is not ready then my thought is
> allocate_buf_for_compression() called from pstore_register() should do
> nothing and pstore will work uncompressed and the kdump infrastructure will
> only cause uncompressed writes. But say if the kdump triggered *after* crypto
> was ready, then the dump write out would be compressed since pstore is ready
> for compression.
>
> The idea is if a future pstore backend calls pstore_register late, then it
> may as well do the allocate_buf_for_compression as well at that time when it
> runs. In that cause pstore_compression_late_init would do nothing.
>
> So this approach is both dynamic and future proof.

Yeah, thanks! I think this looks correct, but I'll spend some more
time testing it too.

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply

* Re: Crash in msm serial on dragonboard with ftrace bootargs
From: Joel Fernandes @ 2018-10-17 17:56 UTC (permalink / raw)
  To: Sai Prakash Ranjan
  Cc: Kees Cook, Steven Rostedt, Stephen Boyd, Bjorn Andersson,
	Andy Gross, David Brown, Jiri Slaby, Ivan T. Ivanov, Geliang Tang,
	Greg Kroah-Hartman, Pramod Gurav, linux-arm-msm, linux-soc,
	open list:SERIAL DRIVERS, LKML, Rajendra Nayak, Vivek Gautam,
	Sibi Sankar
In-Reply-To: <69d2f43d-dc96-9348-7f70-5db88e8f5c39@codeaurora.org>

On Wed, Oct 17, 2018 at 08:19:41PM +0530, Sai Prakash Ranjan wrote:
> On 10/17/2018 5:08 PM, Sai Prakash Ranjan wrote:
> > > 
> > > What do you think about the (untested) patch below? It seems to me
> > > that it
> > > should solve the issue of missing early crash dumps, but I have not
> > > tested it
> > > yet. Sai, would you mind trying it out and let me know if you can see the
> > > early crash dumps properly now?
> > > 
> > > ----8<---
> > > From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
> > > Subject: [RFC] pstore: allocate compression during late_initcall
> > > 
> > > ramoop's pstore registration (using pstore_register) has to run during
> > > late_initcall because crypto backend may not be ready during
> > > postcore_initcall. This causes missing of dmesg crash dumps which could
> > > have been caught by pstore.
> > > 
> > > Instead, lets allow ramoops pstore registration earlier, and once crypto
> > > is ready we can initialize the compression.
> > > 
> > > Reported-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > > ---
> > >   fs/pstore/platform.c | 13 +++++++++++++
> > >   fs/pstore/ram.c      |  2 +-
> > >   2 files changed, 14 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
> > > index 15e99d5a681d..f09066db2d4d 100644
> > > --- a/fs/pstore/platform.c
> > > +++ b/fs/pstore/platform.c
> > > @@ -780,6 +780,19 @@ void __init pstore_choose_compression(void)
> > >       }
> > >   }
> > > +static int __init pstore_compression_late_init(void)
> > > +{
> > > +    /*
> > > +     * Check if any pstore backends registered earlier but did not
> > > allocate
> > > +     * for compression because crypto was not ready, if so then
> > > initialize
> > > +     * compression.
> > > +     */
> > > +    if (psinfo && !tfm)
> > > +        allocate_buf_for_compression();
> > > +    return 0;
> > > +}
> > > +late_initcall(pstore_compression_late_init);
> > > +
> > >   module_param(compress, charp, 0444);
> > >   MODULE_PARM_DESC(compress, "Pstore compression to use");
> > > diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> > > index bbd1e357c23d..98e48d1a9776 100644
> > > --- a/fs/pstore/ram.c
> > > +++ b/fs/pstore/ram.c
> > > @@ -940,7 +940,7 @@ static int __init ramoops_init(void)
> > >       ramoops_register_dummy();
> > >       return platform_driver_register(&ramoops_driver);
> > >   }
> > > -late_initcall(ramoops_init);
> > > +postcore_initcall(ramoops_init);
> > >   static void __exit ramoops_exit(void)
> > >   {
> > > 
> > 
> > Yes I could see the early crash dump. Also I tested with different
> > compression (LZO) instead of deflate just to be sure and it works fine,
> > thanks :)
> > 
> > Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> > 

Thanks.

> I just noticed that allocate_buf_for_compression() is also called from
> pstore_register(). Shouldn't that call be removed now that ramoops_init is
> moved to postcore_initcall and allocate_buf_for_compression() will just
> return doing nothing when called from pstore_register()?

Yes, that is the point. If crypto is not ready then my thought is
allocate_buf_for_compression() called from pstore_register() should do
nothing and pstore will work uncompressed and the kdump infrastructure will
only cause uncompressed writes. But say if the kdump triggered *after* crypto
was ready, then the dump write out would be compressed since pstore is ready
for compression.

The idea is if a future pstore backend calls pstore_register late, then it
may as well do the allocate_buf_for_compression as well at that time when it
runs. In that cause pstore_compression_late_init would do nothing.

So this approach is both dynamic and future proof.

- Joel

^ permalink raw reply

* Re: Crash in msm serial on dragonboard with ftrace bootargs
From: Sai Prakash Ranjan @ 2018-10-17 14:49 UTC (permalink / raw)
  To: Joel Fernandes, Kees Cook
  Cc: Steven Rostedt, Stephen Boyd, Bjorn Andersson, Andy Gross,
	David Brown, Jiri Slaby, Ivan T. Ivanov, Geliang Tang,
	Greg Kroah-Hartman, Pramod Gurav, linux-arm-msm, linux-soc,
	open list:SERIAL DRIVERS, LKML, Rajendra Nayak, Vivek Gautam,
	Sibi Sankar
In-Reply-To: <2a23cd74-7364-0fb7-3c7b-7be79a881073@codeaurora.org>

On 10/17/2018 5:08 PM, Sai Prakash Ranjan wrote:
>>
>> What do you think about the (untested) patch below? It seems to me 
>> that it
>> should solve the issue of missing early crash dumps, but I have not 
>> tested it
>> yet. Sai, would you mind trying it out and let me know if you can see the
>> early crash dumps properly now?
>>
>> ----8<---
>> From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
>> Subject: [RFC] pstore: allocate compression during late_initcall
>>
>> ramoop's pstore registration (using pstore_register) has to run during
>> late_initcall because crypto backend may not be ready during
>> postcore_initcall. This causes missing of dmesg crash dumps which could
>> have been caught by pstore.
>>
>> Instead, lets allow ramoops pstore registration earlier, and once crypto
>> is ready we can initialize the compression.
>>
>> Reported-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
>> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
>> ---
>>   fs/pstore/platform.c | 13 +++++++++++++
>>   fs/pstore/ram.c      |  2 +-
>>   2 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
>> index 15e99d5a681d..f09066db2d4d 100644
>> --- a/fs/pstore/platform.c
>> +++ b/fs/pstore/platform.c
>> @@ -780,6 +780,19 @@ void __init pstore_choose_compression(void)
>>       }
>>   }
>> +static int __init pstore_compression_late_init(void)
>> +{
>> +    /*
>> +     * Check if any pstore backends registered earlier but did not 
>> allocate
>> +     * for compression because crypto was not ready, if so then 
>> initialize
>> +     * compression.
>> +     */
>> +    if (psinfo && !tfm)
>> +        allocate_buf_for_compression();
>> +    return 0;
>> +}
>> +late_initcall(pstore_compression_late_init);
>> +
>>   module_param(compress, charp, 0444);
>>   MODULE_PARM_DESC(compress, "Pstore compression to use");
>> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
>> index bbd1e357c23d..98e48d1a9776 100644
>> --- a/fs/pstore/ram.c
>> +++ b/fs/pstore/ram.c
>> @@ -940,7 +940,7 @@ static int __init ramoops_init(void)
>>       ramoops_register_dummy();
>>       return platform_driver_register(&ramoops_driver);
>>   }
>> -late_initcall(ramoops_init);
>> +postcore_initcall(ramoops_init);
>>   static void __exit ramoops_exit(void)
>>   {
>>
> 
> Yes I could see the early crash dump. Also I tested with different 
> compression (LZO) instead of deflate just to be sure and it works fine, 
> thanks :)
> 
> Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> 

I just noticed that allocate_buf_for_compression() is also called from 
pstore_register(). Shouldn't that call be removed now that ramoops_init 
is moved to postcore_initcall and allocate_buf_for_compression() will 
just return doing nothing when called from pstore_register()?

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply

* [PATCH 2/2] writeback: don't decrement wb->refcnt if !wb->bdi
From: Anders Roxell @ 2018-10-17 14:03 UTC (permalink / raw)
  To: linux, gregkh, akpm
  Cc: linux-serial, linux-kernel, linux-arm-kernel, linux-mm, tj,
	Anders Roxell, Arnd Bergmann
In-Reply-To: <20181017140311.28679-1-anders.roxell@linaro.org>

When enabling CONFIG_DEBUG_TEST_DRIVER_REMOVE devtmpfs gets killed
because we try to remove a file and decrement the wb reference count
before the noop_backing_device_info gets initialized.

Since arch_initcall(pl011_init) came before
subsys_initcall(default_bdi_init), devtmpfs' handle_remove() crashes
because the reference count is a NULL pointer only because bdi->wb
hasn't been initialized yet.

[    0.332075] Serial: AMBA PL011 UART driver
[    0.485276] 9000000.pl011: ttyAMA0 at MMIO 0x9000000 (irq = 39, base_baud = 0) is a PL011 rev1
[    0.502382] console [ttyAMA0] enabled
[    0.515710] Unable to handle kernel paging request at virtual address 0000800074c12000
[    0.516053] Mem abort info:
[    0.516222]   ESR = 0x96000004
[    0.516417]   Exception class = DABT (current EL), IL = 32 bits
[    0.516641]   SET = 0, FnV = 0
[    0.516826]   EA = 0, S1PTW = 0
[    0.516984] Data abort info:
[    0.517149]   ISV = 0, ISS = 0x00000004
[    0.517339]   CM = 0, WnR = 0
[    0.517553] [0000800074c12000] user address but active_mm is swapper
[    0.517928] Internal error: Oops: 96000004 [#1] PREEMPT SMP
[    0.518305] Modules linked in:
[    0.518839] CPU: 0 PID: 13 Comm: kdevtmpfs Not tainted 4.19.0-rc5-next-20180928-00002-g2ba39ab0cd01-dirty #82
[    0.519307] Hardware name: linux,dummy-virt (DT)
[    0.519681] pstate: 80000005 (Nzcv daif -PAN -UAO)
[    0.519959] pc : __destroy_inode+0x94/0x2a8
[    0.520212] lr : __destroy_inode+0x78/0x2a8
[    0.520401] sp : ffff0000098c3b20
[    0.520590] x29: ffff0000098c3b20 x28: 00000000087a3714
[    0.520904] x27: 0000000000002000 x26: 0000000000002000
[    0.521179] x25: ffff000009583000 x24: 0000000000000000
[    0.521467] x23: ffff80007bb52000 x22: ffff80007bbaa7c0
[    0.521737] x21: ffff0000093f9338 x20: 0000000000000000
[    0.522033] x19: ffff80007bbb05d8 x18: 0000000000000400
[    0.522376] x17: 0000000000000000 x16: 0000000000000000
[    0.522727] x15: 0000000000000400 x14: 0000000000000400
[    0.523068] x13: 0000000000000001 x12: 0000000000000001
[    0.523421] x11: 0000000000000000 x10: 0000000000000970
[    0.523749] x9 : ffff0000098c3a60 x8 : ffff80007bbab190
[    0.524017] x7 : ffff80007bbaa880 x6 : 0000000000000c88
[    0.524305] x5 : ffff0000093d96c8 x4 : 61c8864680b583eb
[    0.524567] x3 : ffff0000093d6180 x2 : ffffffffffffffff
[    0.524872] x1 : 0000800074c12000 x0 : 0000800074c12000
[    0.525207] Process kdevtmpfs (pid: 13, stack limit = 0x(____ptrval____))
[    0.525529] Call trace:
[    0.525806]  __destroy_inode+0x94/0x2a8
[    0.526108]  destroy_inode+0x34/0x88
[    0.526370]  evict+0x144/0x1c8
[    0.526636]  iput+0x184/0x230
[    0.526871]  dentry_unlink_inode+0x118/0x130
[    0.527152]  d_delete+0xd8/0xe0
[    0.527420]  vfs_unlink+0x240/0x270
[    0.527665]  handle_remove+0x1d8/0x330
[    0.527875]  devtmpfsd+0x138/0x1c8
[    0.528085]  kthread+0x14c/0x158
[    0.528291]  ret_from_fork+0x10/0x18
[    0.528720] Code: 92800002 aa1403e0 d538d081 8b010000 (c85f7c04)
[    0.529367] ---[ end trace 5a3dee47727f877c ]---

Rework so that wb_put have an extra check if wb->bdi before decrement
wb->refcnt and also add a WARN_ON to get a warning if it happens again
in other drivers.

Fixes: 52ebea749aae ("writeback: make backing_dev_info host cgroup-specific bdi_writebacks")
Cc: Arnd Bergmann <arnd@arndb.de>
Co-developed-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
 include/linux/backing-dev-defs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h
index 9a6bc0951cfa..20721550d32a 100644
--- a/include/linux/backing-dev-defs.h
+++ b/include/linux/backing-dev-defs.h
@@ -258,7 +258,7 @@ static inline void wb_get(struct bdi_writeback *wb)
  */
 static inline void wb_put(struct bdi_writeback *wb)
 {
-	if (wb != &wb->bdi->wb)
+	if (!WARN_ON(!wb->bdi) && wb != &wb->bdi->wb)
 		percpu_ref_put(&wb->refcnt);
 }
 
-- 
2.19.1

^ permalink raw reply related

* [PATCH 1/2] serial: set suppress_bind_attrs flag only if builtin
From: Anders Roxell @ 2018-10-17 14:03 UTC (permalink / raw)
  To: linux, gregkh, akpm
  Cc: linux-serial, linux-kernel, linux-arm-kernel, linux-mm, tj,
	Anders Roxell, Arnd Bergmann

When the test 'CONFIG_DEBUG_TEST_DRIVER_REMOVE=y' is enabled,
arch_initcall(pl011_init) came before subsys_initcall(default_bdi_init).
devtmpfs gets killed because we try to remove a file and decrement the
wb reference count before the noop_backing_device_info gets initialized.

[    0.332075] Serial: AMBA PL011 UART driver
[    0.485276] 9000000.pl011: ttyAMA0 at MMIO 0x9000000 (irq = 39, base_baud = 0) is a PL011 rev1
[    0.502382] console [ttyAMA0] enabled
[    0.515710] Unable to handle kernel paging request at virtual address 0000800074c12000
[    0.516053] Mem abort info:
[    0.516222]   ESR = 0x96000004
[    0.516417]   Exception class = DABT (current EL), IL = 32 bits
[    0.516641]   SET = 0, FnV = 0
[    0.516826]   EA = 0, S1PTW = 0
[    0.516984] Data abort info:
[    0.517149]   ISV = 0, ISS = 0x00000004
[    0.517339]   CM = 0, WnR = 0
[    0.517553] [0000800074c12000] user address but active_mm is swapper
[    0.517928] Internal error: Oops: 96000004 [#1] PREEMPT SMP
[    0.518305] Modules linked in:
[    0.518839] CPU: 0 PID: 13 Comm: kdevtmpfs Not tainted 4.19.0-rc5-next-20180928-00002-g2ba39ab0cd01-dirty #82
[    0.519307] Hardware name: linux,dummy-virt (DT)
[    0.519681] pstate: 80000005 (Nzcv daif -PAN -UAO)
[    0.519959] pc : __destroy_inode+0x94/0x2a8
[    0.520212] lr : __destroy_inode+0x78/0x2a8
[    0.520401] sp : ffff0000098c3b20
[    0.520590] x29: ffff0000098c3b20 x28: 00000000087a3714
[    0.520904] x27: 0000000000002000 x26: 0000000000002000
[    0.521179] x25: ffff000009583000 x24: 0000000000000000
[    0.521467] x23: ffff80007bb52000 x22: ffff80007bbaa7c0
[    0.521737] x21: ffff0000093f9338 x20: 0000000000000000
[    0.522033] x19: ffff80007bbb05d8 x18: 0000000000000400
[    0.522376] x17: 0000000000000000 x16: 0000000000000000
[    0.522727] x15: 0000000000000400 x14: 0000000000000400
[    0.523068] x13: 0000000000000001 x12: 0000000000000001
[    0.523421] x11: 0000000000000000 x10: 0000000000000970
[    0.523749] x9 : ffff0000098c3a60 x8 : ffff80007bbab190
[    0.524017] x7 : ffff80007bbaa880 x6 : 0000000000000c88
[    0.524305] x5 : ffff0000093d96c8 x4 : 61c8864680b583eb
[    0.524567] x3 : ffff0000093d6180 x2 : ffffffffffffffff
[    0.524872] x1 : 0000800074c12000 x0 : 0000800074c12000
[    0.525207] Process kdevtmpfs (pid: 13, stack limit = 0x(____ptrval____))
[    0.525529] Call trace:
[    0.525806]  __destroy_inode+0x94/0x2a8
[    0.526108]  destroy_inode+0x34/0x88
[    0.526370]  evict+0x144/0x1c8
[    0.526636]  iput+0x184/0x230
[    0.526871]  dentry_unlink_inode+0x118/0x130
[    0.527152]  d_delete+0xd8/0xe0
[    0.527420]  vfs_unlink+0x240/0x270
[    0.527665]  handle_remove+0x1d8/0x330
[    0.527875]  devtmpfsd+0x138/0x1c8
[    0.528085]  kthread+0x14c/0x158
[    0.528291]  ret_from_fork+0x10/0x18
[    0.528720] Code: 92800002 aa1403e0 d538d081 8b010000 (c85f7c04)
[    0.529367] ---[ end trace 5a3dee47727f877c ]---

Rework to set suppress_bind_attrs flag to avoid removing the device when
CONFIG_DEBUG_TEST_DRIVER_REMOVE=y. This applies for pic32_uart and
xilinx_uartps as well.

Cc: Arnd Bergmann <arnd@arndb.de>
Co-developed-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
 drivers/tty/serial/amba-pl011.c    | 2 ++
 drivers/tty/serial/pic32_uart.c    | 1 +
 drivers/tty/serial/xilinx_uartps.c | 1 +
 3 files changed, 4 insertions(+)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index ebd33c0232e6..89ade213a1a9 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2780,6 +2780,7 @@ static struct platform_driver arm_sbsa_uart_platform_driver = {
 		.name	= "sbsa-uart",
 		.of_match_table = of_match_ptr(sbsa_uart_of_match),
 		.acpi_match_table = ACPI_PTR(sbsa_uart_acpi_match),
+		.suppress_bind_attrs = IS_BUILTIN(CONFIG_SERIAL_AMBA_PL011),
 	},
 };
 
@@ -2808,6 +2809,7 @@ static struct amba_driver pl011_driver = {
 	.drv = {
 		.name	= "uart-pl011",
 		.pm	= &pl011_dev_pm_ops,
+		.suppress_bind_attrs = IS_BUILTIN(CONFIG_SERIAL_AMBA_PL011),
 	},
 	.id_table	= pl011_ids,
 	.probe		= pl011_probe,
diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
index fd80d999308d..0bdf1687983f 100644
--- a/drivers/tty/serial/pic32_uart.c
+++ b/drivers/tty/serial/pic32_uart.c
@@ -919,6 +919,7 @@ static struct platform_driver pic32_uart_platform_driver = {
 	.driver		= {
 		.name	= PIC32_DEV_NAME,
 		.of_match_table	= of_match_ptr(pic32_serial_dt_ids),
+		.suppress_bind_attrs = IS_BUILTIN(CONFIG_SERIAL_PIC32),
 	},
 };
 
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 0e3dae461f71..806a953ac8d4 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1717,6 +1717,7 @@ static struct platform_driver cdns_uart_platform_driver = {
 		.name = CDNS_UART_NAME,
 		.of_match_table = cdns_uart_of_match,
 		.pm = &cdns_uart_dev_pm_ops,
+		.suppress_bind_attrs = IS_BUILTIN(CONFIG_SERIAL_XILINX_PS_UART),
 		},
 };
 
-- 
2.19.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox