Linux Serial subsystem development
 help / color / mirror / Atom feed
* Re: [PATCH] tty: serial: qcom_geni_serial: Add suspend support to non console UART
From: Doug Anderson @ 2018-10-16 20:14 UTC (permalink / raw)
  To: msavaliy; +Cc: alokc, Greg Kroah-Hartman, Jiri Slaby, linux-serial, LKML
In-Reply-To: <1539696848-21988-1-git-send-email-msavaliy@codeaurora.org>

Hi,

On Tue, Oct 16, 2018 at 6:34 AM Mukesh, Savaliya
<msavaliy@codeaurora.org> wrote:
>
> From: Mukesh Kumar Savaliya <msavaliy@codeaurora.org>
>
> UART driver checks for the PM state and denies suspend if state is ACTIVE.
> This makes UART to deny suspend when client keeps port open which is not
> correct. Instead follow framework and obey suspend-resume callbacks.
>
> Signed-off-by: Mukesh Kumar Savaliya <msavaliy@codeaurora.org>
> ---
>  drivers/tty/serial/qcom_geni_serial.c | 36 ++++++-----------------------------
>  1 file changed, 6 insertions(+), 30 deletions(-)

Looks like this patch was re-sent about an hour later with a different
subject of ("tty: serial: qcom_geni_serial: Rectify UART suspend
mechanism") at <https://lkml.kernel.org/r/1539699060-3055-1-git-send-email-msavaliy@codeaurora.org>.
The subject of the later patch seems better.  I'll do my review there.

-Doug

^ permalink raw reply

* Re: [PATCH] tty: serial: qcom_geni_serial: Rectify UART suspend mechanism
From: Doug Anderson @ 2018-10-16 20:24 UTC (permalink / raw)
  To: msavaliy; +Cc: alokc, Greg Kroah-Hartman, Jiri Slaby, linux-serial, LKML
In-Reply-To: <1539699060-3055-1-git-send-email-msavaliy@codeaurora.org>

Hi,

On Tue, Oct 16, 2018 at 7:11 AM Mukesh, Savaliya
<msavaliy@codeaurora.org> wrote:
>
> From: Mukesh Kumar Savaliya <msavaliy@codeaurora.org>
>
> UART driver checks for the PM state and denies suspend if state is ACTIVE.
> This makes UART to deny suspend when client keeps port open which is not
> correct. Instead follow framework and obey suspend-resume callbacks.
>
> Signed-off-by: Mukesh Kumar Savaliya <msavaliy@codeaurora.org>
> ---
>  drivers/tty/serial/qcom_geni_serial.c | 36 ++++++-----------------------------
>  1 file changed, 6 insertions(+), 30 deletions(-)

Looks peachy.  Getting rid of all that unnecessary code and moving
away from the "no_irq" suspend op makes lots of sense to me.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

^ permalink raw reply

* Re: Crash in msm serial on dragonboard with ftrace bootargs
From: Stephen Boyd @ 2018-10-16 20:51 UTC (permalink / raw)
  To: Sai Prakash Ranjan, Steven Rostedt
  Cc: 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: <0b014d85-9988-0bf5-7b72-911dbbcdadfc@codeaurora.org>

Quoting Sai Prakash Ranjan (2018-10-16 12:35:57)
> On 10/17/2018 12:45 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.
> >>
> > 
> > Which compiler are you using and can you send me your config.
> > 
> Config attached.
> 
> Compiler: aarch64-linux-gnu-gcc (Linaro GCC 6.3-2017.02) 6.3.1 20170109
> 
> > I wonder if there's something screwing up with the way ftrace nops are
> > working on this board.
> > 
> > A couple things of note, 1) it works fine after boot up. 2) it crashes
> > in the initcall code, so it's not due to ftrace being enabled too early.
> > 
> > I'd like to see the full command line as well. I bet if you remove the
> > qcom,msm-uartdm from the command line, and had just ftrace=function, it
> > may also boot fine too. Can you try that?
> > 
> 
> Kernel command line: root=/dev/disk/by-partlabel/rootfs rw rootwait 
> ftrace=function ftrace_notrace=m* earlycon console=ttyMSM0,115200n8
> 
> qcom,msm-uartdm is not in command line, it is the earlycon. So without 
> earlycon(bootconsole), board boots fine as we discussed earlier.
> 

Have you tried with earlycon and no ftrace things on the commandline?

 root=/dev/disk/by-partlabel/rootfs rw rootwait earlycon console=ttyMSM0,115200n8

If earlycon is causing problems, it sounds like this may be another case
of earlycon uart handing off to the uart driver and that failing because
something gets printed while the uart is transitioning from the earlycon
console to the kernel boot console. I recall the uart would trample on
itself in interesting ways.

^ permalink raw reply

* Re: [PATCH] tty: serial: qcom_geni_serial: Don't slow all ports just for kgdb
From: Stephen Boyd @ 2018-10-16 20:53 UTC (permalink / raw)
  To: gregkh
  Cc: msavaliy, linux-arm-msm, alokc, mka, evgreen, Douglas Anderson,
	linux-kernel, linux-serial, Jiri Slaby
In-Reply-To: <20181015204425.84537-1-dianders@chromium.org>

Quoting Douglas Anderson (2018-10-15 13:44:24)
> If you turn on CONFIG_KGDB then you'll get CONFIG_CONSOLE_POLL
> selected.
> 
> If you have CONFIG_CONSOLE_POLL selected then the GENI serial driver
> was setting RX_BYTES_PW to 1 for _all_ UART ports.
> 
> This doesn't seem like such a good idea.  Let's only set RX_BYTES_PW
> to 1 for the console port.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

^ permalink raw reply

* Re: Crash in msm serial on dragonboard with ftrace bootargs
From: Joel Fernandes @ 2018-10-16 23:09 UTC (permalink / raw)
  To: Sai Prakash Ranjan
  Cc: Stephen Boyd, Bjorn Andersson, Andy Gross, David Brown,
	Jiri Slaby, Ivan T. Ivanov, Steven Rostedt (VMware), 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: <1cae8f10-55f5-20ce-9105-30af6f88bd6e@codeaurora.org>

On Tue, Oct 16, 2018 at 05:08:25PM +0530, Sai Prakash Ranjan wrote:
> Hi,
> 
> On dragonboard 410c, with "ftrace=function" boot args, the console output
> slows down and board resets without any backtrace as below. This is tested
> on latest kernel and seems to exist even in older kernels as well.
> 
> [    2.949164] EINJ: ACPI disabled.
> [    3.133001] Serial: 8250/16550 dri
> Format: Log Type - Time(microsec) - Message - Optional Info
> Log Type: B - Since Boot(Power On Reset),  D - Delta,  S - Statistic
> 
> But with pstore enabled, able to get the below backtrace:
> <4>[    2.949164] EINJ: ACPI disabled.
> <6>[    3.133001] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
> <6>[    3.164097] SuperH (H)SCI(F) driver initialized
> <0>[    3.164471] Internal error: synchronous external abort: 96000010 [#1]
> PREEMPT SMP
> <4>[    3.164479] Modules linked in:
> <4>[    3.164495] CPU: 2 PID: 1 Comm: swapper/0 Not tainted
> 4.19.0-rc8-00008-ge033b9909fff-dirty #175
> <4>[    3.164501] Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC
> (DT)
> <4>[    3.164508] pstate: 40000085 (nZcv daIf -PAN -UAO)
> <4>[    3.164514] pc : msm_read.isra.2+0x20/0x50
> <4>[    3.164520] lr : msm_read.isra.2+0x1c/0x50
> <4>[    3.164526] sp : ffff000008033a50
> <4>[    3.164531] x29: ffff000008033a50 x28: ffff000009486018
> <4>[    3.164548] x27: 0000000000000001 x26: ffff7dfffe7ff070
> <4>[    3.164565] x25: 0000000000000034 x24: ffff000009486000
> <4>[    3.164582] x23: 0000000000000000 x22: ffff00000978e190
> <4>[    3.164599] x21: ffff0000095e8228 x20: 0000000000000034
> <4>[    3.164616] x19: ffff7dfffe7ff008 x18: ffffffffffffffff
> <4>[    3.164632] x17: 0000000000000000 x16: 0000000000000000
> <4>[    3.164649] x15: ffff0000094a96c8 x14: ffff00008978e6bf
> <4>[    3.164666] x13: ffff00000978e6cd x12: 0000000000000038
> <4>[    3.164683] x11: ffff0000094c6000 x10: 0000000000000c24
> <4>[    3.164699] x9 : ffff80003c89b400 x8 : ffff000008033970
> <4>[    3.164716] x7 : ffff80000eb04100 x6 : 00000000000af304
> <4>[    3.164732] x5 : 0000000000000c40 x4 : ffff80003c06f000
> <4>[    3.164750] x3 : ffff80003c89b498 x2 : 0000000000000000
> <4>[    3.164766] x1 : ffff80003ca68000 x0 : 0000000000000800
> <0>[    3.164785] Process swapper/0 (pid: 1, stack limit =
> 0x(____ptrval____))
> <4>[    3.164791] Call trace:
> <4>[    3.164797]  msm_read.isra.2+0x20/0x50
> <4>[    3.164804]  msm_reset_dm_count+0x44/0x80
> <4>[    3.164810]  __msm_console_write+0x1c8/0x1d0
> <4>[    3.164816]  msm_serial_early_write_dm+0x3c/0x50
> <4>[    3.164823]  console_unlock.part.6+0x468/0x528
> <4>[    3.164829]  vprintk_emit+0x210/0x218
> <4>[    3.164835]  vprintk_default+0x48/0x58
> <4>[    3.164841]  vprintk_func+0xf0/0x1c0
> <4>[    3.164847]  printk+0x74/0x94
> <4>[    3.164853]  sci_init+0x24/0x3c
> <4>[    3.164859]  do_one_initcall+0x54/0x248
> <4>[    3.164866]  kernel_init_freeable+0x210/0x378
> <4>[    3.164872]  kernel_init+0x18/0x118
> <4>[    3.164878]  ret_from_fork+0x10/0x1c
> <0>[    3.164884] Code: aa1e03e0 8b214273 97e616f7 d503201f (b9400260)
> 
> Seems to be issue with the msm serial driver and not ftrace.
> Could someone look into it.
> 
> One more thing is for pstore dmesg-ramoops, I had to change late_initcall to
> postcore_initcall which brings the question as to why we changed to
> late_initcall?
> Simple git blame shows to support crypto compress api, but is it really
> helpful? A lot of boottime issues can be caught with pstore enabled at
> postcore_initcall rather than late_initcall, this backtrace
> is just one example. Is there any way we could change this?

Any chance you are able to dig deeper into the stack trace? I would
disassemble vmlinux and see which part of msm_read is generating the
synchronous external abort and look into that.

Also similar to what Steve suggested, I wonder if it boots for you if you
annotate all the functions in the serial driver with 'notrace'.

thanks,

 - Joel

^ permalink raw reply

* [[PATCH] 0/9] *** DMA support for UART in ASPEED's AST2500  ***
From: sudheer.v @ 2018-10-17  4:10 UTC (permalink / raw)
  To: Vinod Koul, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
	Joel Stanley, Andrew Jeffery, Russell King, Dan Williams,
	Jiri Slaby, Thomas Gleixner, Marc Zyngier, Benjamin Herrenschmidt,
	Christian Borntraeger, Michael Moese, Hendrik Brueckner,
	Kate Stewart, Philippe Ombredanne, dmaengine, devicetree,
	linux-kernel
  Cc: sudheer.v, Sudheer V, ShivahShankar Shakarnarayan rao

DMA controller driver  and UART dma client driver for aspeed's AST2500

sudheer.v (9):
  DT-changes-for-DMA-UART-of-AST2500
  Defconfig-changes-for-DMA-UART-of-AST2500
  configuration-for-DMA-of-AST2500
  Documentation-DTbindings-DMA-controller-of-AST2500
  DMA-driver-for-AST2500
  configuration-for-DMA-UART-of-AST2500
  Documentation-DTbindings-DMA-UART-of-AST2500
  DMA-UART-Driver-for-AST2500
  updating MAINTAINERS for DMA and DMA-UART drivers of AST2500

 .../devicetree/bindings/dma/ast-uart-sdma.txt      |   23 +
 .../devicetree/bindings/serial/ast-sdma-uart.txt   |   34 +
 MAINTAINERS                                        |   26 +
 arch/arm/boot/dts/aspeed-ast2500-evb.dts           |   20 +
 arch/arm/boot/dts/aspeed-g5.dtsi                   |   85 ++
 arch/arm/configs/aspeed_g5_defconfig               |    6 +-
 drivers/dma/Kconfig                                |    9 +
 drivers/dma/Makefile                               |    1 +
 drivers/dma/aspeed_uart_sdma.c                     |  810 ++++++++++
 drivers/tty/serial/8250/8250_aspeed_uart_dma.c     | 1594 ++++++++++++++++++++
 drivers/tty/serial/8250/Kconfig                    |   34 +
 drivers/tty/serial/8250/Makefile                   |    1 +
 12 files changed, 2641 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/dma/ast-uart-sdma.txt
 create mode 100644 Documentation/devicetree/bindings/serial/ast-sdma-uart.txt
 create mode 100644 drivers/dma/aspeed_uart_sdma.c
 create mode 100644 drivers/tty/serial/8250/8250_aspeed_uart_dma.c

-- 
1.9.1

^ permalink raw reply

* [[PATCH] 1/9] DT-changes-for-DMA-UART-of-AST2500
From: sudheer.v @ 2018-10-17  4:10 UTC (permalink / raw)
  To: Vinod Koul, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
	Joel Stanley, Andrew Jeffery, Russell King, Dan Williams,
	Jiri Slaby, Thomas Gleixner, Marc Zyngier, Benjamin Herrenschmidt,
	Christian Borntraeger, Michael Moese, Hendrik Brueckner,
	Kate Stewart, Philippe Ombredanne, dmaengine, devicetree,
	linux-kernel
  Cc: sudheer.v, Sudheer V, ShivahShankar Shakarnarayan rao
In-Reply-To: <1539749466-3912-1-git-send-email-open.sudheer@gmail.com>

Signed-off-by: sudheer.v <open.sudheer@gmail.com>
---
 arch/arm/boot/dts/aspeed-ast2500-evb.dts | 20 ++++++++
 arch/arm/boot/dts/aspeed-g5.dtsi         | 85 ++++++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-ast2500-evb.dts b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
index 5dbb33c..f98d55b 100644
--- a/arch/arm/boot/dts/aspeed-ast2500-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
@@ -64,6 +64,26 @@
 	status = "okay";
 };
 
+&ast_uart_sdma{
+	status = "okay";
+};
+
+&dma_uart0 {
+	status = "disabled";
+};
+
+&dma_uart1 {
+	status = "disabled";
+};
+
+&dma_uart2 {
+	status = "okay";
+};
+
+&dma_uart3 {
+	status = "okay";
+};
+
 &mac0 {
 	status = "okay";
 
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index d92f047..ba8edd1 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -436,6 +436,91 @@
 				status = "disabled";
 			};
 
+			ast_uart_sdma: uart_sdma@1e79e000 {
+				compatible = "aspeed,ast-uart-sdma";
+				reg = <0x1e79e000 0x400>;
+				interrupts = <50>;
+				#dma-cells = <1>;
+				dma-channels = <8>;
+				status = "disabled";
+			};
+
+			dma_uart0: dma_uart0@1e783000{
+				compatible = "aspeed,ast-sdma-uart";
+				reg = <0x1e783000 0x1000>;
+				interrupts = <9>;
+				clocks = <&syscon ASPEED_CLK_GATE_UART1CLK>;
+				reg-shift = <2>;
+				dma-channel = <0>;
+				no-loopback-test;
+				pinctrl-names = "default";
+				pinctrl-0 = <&pinctrl_txd1_default
+				&pinctrl_rxd1_default &pinctrl_ncts1_default
+				&pinctrl_ndcd1_default &pinctrl_ndsr1_default
+				&pinctrl_ndtr1_default &pinctrl_nri1_default
+				&pinctrl_nrts1_default>;
+				dma-names = "rx", "tx";
+				dmas = <&ast_uart_sdma 1>, <&ast_uart_sdma 0>;
+				status = "disabled";
+			};
+
+			dma_uart1: dma_uart1@1e78d000{
+				compatible = "aspeed,ast-sdma-uart";
+				reg = <0x1e78d000 0x1000>;
+				interrupts = <32>;
+				clocks = <&syscon ASPEED_CLK_GATE_UART2CLK>;
+				reg-shift = <2>;
+				dma-channel = <1>;
+				no-loopback-test;
+				pinctrl-names = "default";
+				pinctrl-0 = <&pinctrl_txd2_default
+				&pinctrl_rxd2_default &pinctrl_ncts2_default
+				&pinctrl_ndcd2_default &pinctrl_ndsr2_default
+				&pinctrl_ndtr2_default &pinctrl_nri2_default
+				&pinctrl_nrts2_default>;
+				dma-names = "rx", "tx";
+				dmas = <&ast_uart_sdma 3>, <&ast_uart_sdma 2>;
+				status = "disabled";
+};
+
+			dma_uart2: dma_uart2@1e78e000{
+				compatible = "aspeed,ast-sdma-uart";
+				reg = <0x1e78e000 0x1000>;
+				interrupts = <33>;
+				clocks = <&syscon ASPEED_CLK_GATE_UART3CLK>;
+				reg-shift = <2>;
+				dma-channel = <2>;
+				no-loopback-test;
+				pinctrl-names = "default";
+				pinctrl-0 = <&pinctrl_txd3_default
+				&pinctrl_rxd3_default &pinctrl_ncts3_default
+				&pinctrl_ndcd3_default &pinctrl_ndsr3_default
+				&pinctrl_ndtr3_default &pinctrl_nri3_default
+				&pinctrl_nrts3_default>;
+				dma-names = "rx", "tx";
+				dmas = <&ast_uart_sdma 5>, <&ast_uart_sdma 4>;
+				status = "disabled";
+};
+
+			dma_uart3: dma_uart3@1e78f000{
+				compatible = "aspeed,ast-sdma-uart";
+				reg = <0x1e78f000 0x1000>;
+				interrupts = <34>;
+				clocks = <&syscon ASPEED_CLK_GATE_UART4CLK>;
+				reg-shift = <2>;
+				dma-channel = <3>;
+				no-loopback-test;
+				pinctrl-names = "default";
+				pinctrl-0 = <&pinctrl_txd4_default
+				&pinctrl_rxd4_default &pinctrl_ncts4_default
+				&pinctrl_ndcd4_default &pinctrl_ndsr4_default
+				&pinctrl_ndtr4_default &pinctrl_nri4_default
+				&pinctrl_nrts4_default>;
+				dma-names = "rx", "tx";
+				dmas = <&ast_uart_sdma 7>, <&ast_uart_sdma 6>;
+				status = "disabled";
+};
+
 			i2c: i2c@1e78a000 {
 				compatible = "simple-bus";
 				#address-cells = <1>;
-- 
1.9.1

^ permalink raw reply related

* [[PATCH] 2/9] Defconfig-changes-for-DMA-UART-of-AST2500
From: sudheer.v @ 2018-10-17  4:10 UTC (permalink / raw)
  To: Vinod Koul, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
	Joel Stanley, Andrew Jeffery, Russell King, Dan Williams,
	Jiri Slaby, Thomas Gleixner, Marc Zyngier, Benjamin Herrenschmidt,
	Christian Borntraeger, Michael Moese, Hendrik Brueckner,
	Kate Stewart, Philippe Ombredanne, dmaengine, devicetree,
	linux-kernel
  Cc: sudheer.v, Sudheer V, ShivahShankar Shakarnarayan rao
In-Reply-To: <1539749466-3912-1-git-send-email-open.sudheer@gmail.com>

Signed-off-by: sudheer.v <open.sudheer@gmail.com>
---
 arch/arm/configs/aspeed_g5_defconfig | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
index b7f8fa1..25813b5 100644
--- a/arch/arm/configs/aspeed_g5_defconfig
+++ b/arch/arm/configs/aspeed_g5_defconfig
@@ -132,8 +132,8 @@ CONFIG_KEYBOARD_GPIO_POLLED=y
 CONFIG_SERIAL_8250=y
 # CONFIG_SERIAL_8250_DEPRECATED_OPTIONS is not set
 CONFIG_SERIAL_8250_CONSOLE=y
-CONFIG_SERIAL_8250_NR_UARTS=6
-CONFIG_SERIAL_8250_RUNTIME_UARTS=6
+CONFIG_SERIAL_8250_NR_UARTS=1
+CONFIG_SERIAL_8250_RUNTIME_UARTS=1
 CONFIG_SERIAL_8250_EXTENDED=y
 CONFIG_SERIAL_8250_ASPEED_VUART=y
 CONFIG_SERIAL_8250_SHARE_IRQ=y
@@ -211,6 +211,8 @@ CONFIG_RTC_CLASS=y
 CONFIG_RTC_DRV_DS1307=y
 CONFIG_RTC_DRV_PCF8523=y
 CONFIG_RTC_DRV_RV8803=y
+CONFIG_DMADEVICES=y
+CONFIG_AST_UART_SDMA=y
 # CONFIG_VIRTIO_MENU is not set
 # CONFIG_IOMMU_SUPPORT is not set
 CONFIG_IIO=y
-- 
1.9.1

^ permalink raw reply related

* [[PATCH] 4/9] Documentation-DTbindings-DMA-controller-of-AST2500
From: sudheer.v @ 2018-10-17  4:11 UTC (permalink / raw)
  To: Vinod Koul, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
	Joel Stanley, Andrew Jeffery, Russell King, Dan Williams,
	Jiri Slaby, Thomas Gleixner, Marc Zyngier, Benjamin Herrenschmidt,
	Christian Borntraeger, Michael Moese, Hendrik Brueckner,
	Kate Stewart, Philippe Ombredanne, dmaengine, devicetree,
	linux-kernel
  Cc: sudheer.v, Sudheer V, ShivahShankar Shakarnarayan rao
In-Reply-To: <1539749466-3912-1-git-send-email-open.sudheer@gmail.com>

Signed-off-by: sudheer.v <open.sudheer@gmail.com>
---
 .../devicetree/bindings/dma/ast-uart-sdma.txt      | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/ast-uart-sdma.txt

diff --git a/Documentation/devicetree/bindings/dma/ast-uart-sdma.txt b/Documentation/devicetree/bindings/dma/ast-uart-sdma.txt
new file mode 100644
index 0000000..e770df2
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/ast-uart-sdma.txt
@@ -0,0 +1,23 @@
+* Aspeed Direct Memory Access (DMA) Controller for AST25XX
+
+
+* DMA controller
+
+Required properties:
+- compatible : Should be "aspeed,ast-uart-sdma"
+- reg : Should contain SDMA registers location and length
+- interrupts : Should contain SDMA interrupt
+- dma-channels: number of DMA channels in DMA controller
+
+Optional properties:
+
+Example
+                        ast_uart_sdma: uart_sdma@1e79e000 {
+                                compatible = "aspeed,ast-uart-sdma";
+                                reg = <0x1e79e000 0x400>;
+                                interrupts = <50>;
+                                #dma-cells = <1>;
+                                dma-channels = <8>;
+                                status = "disabled";
+                        };
+
-- 
1.9.1

^ permalink raw reply related

* [[PATCH] 8/9] DMA-UART-Driver-for-AST2500
From: sudheer.v @ 2018-10-17  4:11 UTC (permalink / raw)
  To: Vinod Koul, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
	Joel Stanley, Andrew Jeffery, Russell King, Dan Williams,
	Jiri Slaby, Thomas Gleixner, Marc Zyngier, Benjamin Herrenschmidt,
	Christian Borntraeger, Michael Moese, Hendrik Brueckner,
	Kate Stewart, Philippe Ombredanne, dmaengine, devicetree,
	linux-kernel
  Cc: sudheer.v, Sudheer V, ShivahShankar Shakarnarayan rao
In-Reply-To: <1539749466-3912-1-git-send-email-open.sudheer@gmail.com>

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
@@ -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
+ *
+ */
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/ioport.h>
+#include <linux/init.h>
+#include <linux/console.h>
+#include<linux/slab.h>
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
+#include <linux/serial_reg.h>
+#include <linux/serial_core.h>
+#include <linux/serial.h>
+#include <linux/serial_8250.h>
+#include <linux/nmi.h>
+#include <linux/mutex.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <asm/irq.h>
+
+#include "8250.h"
+#include <linux/dma-mapping.h>
+#define SDMA_RX_BUFF_SIZE                               0x10000 //65536
+#define DMA_BUFF_SIZE                           0x1000  //4096
+
+
+
+
+#undef UART_XMIT_SIZE
+#define UART_XMIT_SIZE	0x1000
+#define UART_RX_SIZE	0x10000
+
+#ifdef UART_DMA_DEBUG
+	#define UART_DBG(fmt, args...) pr_debug("%s() " fmt, __func__, ## args)
+#else
+	#define UART_DBG(fmt, args...)
+#endif
+
+#ifdef CONFIG_UART_TX_DMA_DEBUG
+	#define UART_TX_DBG(fmt, args...) pr_debug("%s()"fmt, __func__, ## args)
+#else
+	#define UART_TX_DBG(fmt, args...)
+#endif
+
+/*
+ * Configuration:
+ *   share_irqs - whether we pass IRQF_SHARED to request_irq().  This option
+ *                is unsafe when used on edge-triggered interrupts.
+ */
+static unsigned int share_irqs = SERIAL8250_SHARE_IRQS;
+
+static unsigned int nr_uarts = CONFIG_AST_RUNTIME_DMA_UARTS;
+
+/*
+ * Debugging.
+ */
+#if 0
+#define DEBUG_AUTOCONF(fmt...)	UART_DBG(fmt)
+#else
+#define DEBUG_AUTOCONF(fmt...)	do { } while (0)
+#endif
+
+#if 0
+#define DEBUG_INTR(fmt...)	UART_DBG(fmt)
+#else
+#define DEBUG_INTR(fmt...)	do { } while (0)
+#endif
+
+#define PASS_LIMIT	256
+
+#include <asm/serial.h>
+
+
+#define UART_DMA_NR		CONFIG_AST_NR_DMA_UARTS
+
+struct ast_uart_port {
+	struct uart_port	port;
+	struct platform_device	*pdev;
+	unsigned short		capabilities;	/* port capabilities */
+	unsigned short		bugs;		/* port bugs */
+	unsigned int		tx_loadsz;	/* transmit fifo load size */
+	unsigned char		acr;
+	unsigned char		ier;
+	unsigned char		lcr;
+	unsigned char		mcr;
+	unsigned char		mcr_mask;	/* mask of user bits */
+	unsigned char		mcr_force;	/* mask of forced bits */
+	unsigned int		channel_no;
+	struct scatterlist	rx_sgl;
+	struct dma_chan		*rx_dma_chan;
+	struct circ_buf		rx_dma_buf;
+	dma_addr_t		dma_rx_addr;
+	u8			rx_in_progress;
+	struct dma_async_tx_descriptor		*rx_dma_desc;
+	dma_cookie_t		rx_cookie;
+	unsigned int		rx_bytes_requested;
+	unsigned int		rx_bytes_transferred;
+	struct tasklet_struct	rx_tasklet;
+	struct scatterlist	tx_sgl;
+	struct dma_chan		*tx_dma_chan;
+	struct circ_buf		tx_dma_buf;
+	dma_addr_t		dma_tx_addr;
+	u8			tx_in_progress;
+	struct dma_async_tx_descriptor		*tx_dma_desc;
+	dma_cookie_t		tx_cookie;
+	unsigned int		tx_bytes_requested;
+	unsigned int		tx_bytes_transferred;
+	struct tasklet_struct	tx_tasklet;
+	spinlock_t lock;
+	int			tx_done;
+	int			tx_count;
+	/*
+	 * Some bits in registers are cleared on a read, so they must
+	 * be saved whenever the register is read but the bits will not
+	 * be immediately processed.
+	 */
+#define LSR_SAVE_FLAGS UART_LSR_BRK_ERROR_BITS
+	unsigned char		lsr_saved_flags;
+#define MSR_SAVE_FLAGS UART_MSR_ANY_DELTA
+	unsigned char		msr_saved_flags;
+
+	/*
+	 * We provide a per-port pm hook.
+	 */
+	void			(*pm)(struct uart_port *port,
+				      unsigned int state, unsigned int old);
+};
+
+static struct ast_uart_port ast_uart_ports[UART_DMA_NR];
+
+static int ast_dma_channel_setup(struct ast_uart_port *up);
+static inline struct ast_uart_port *
+to_ast_dma_uart_port(struct uart_port *uart)
+{
+	return container_of(uart, struct ast_uart_port, port);
+}
+
+struct irq_info {
+	spinlock_t		lock;
+	struct ast_uart_port *up;
+};
+
+static void ast_dma_channel_teardown(struct ast_uart_port *s);
+static struct irq_info ast_uart_irq[1];
+static DEFINE_MUTEX(ast_uart_mutex);
+
+/*
+ * Here we define the default xmit fifo size used for each type of UART.
+ */
+static const struct serial8250_config uart_config[] = {
+	[PORT_UNKNOWN] = {
+		.name		= "unknown",
+		.fifo_size	= 1,
+		.tx_loadsz	= 1,
+	},
+	[PORT_8250] = {
+		.name		= "8250",
+		.fifo_size	= 1,
+		.tx_loadsz	= 1,
+	},
+	[PORT_16450] = {
+		.name		= "16450",
+		.fifo_size	= 1,
+		.tx_loadsz	= 1,
+	},
+	[PORT_16550] = {
+		.name		= "16550",
+		.fifo_size	= 1,
+		.tx_loadsz	= 1,
+	},
+	[PORT_16550A] = {
+		.name		= "16550A",
+		.fifo_size	= 16,
+		.tx_loadsz	= 16,
+		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10
+							| UART_FCR_DMA_SELECT,
+		.flags		= UART_CAP_FIFO,
+	},
+};
+
+/* sane hardware needs no mapping */
+#define map_8250_in_reg(up, offset) (offset)
+#define map_8250_out_reg(up, offset) (offset)
+
+static void ast_uart_unregister_port(int line);
+static int ast_uart_register_port(struct uart_port *port,
+					unsigned int channel_no);
+
+static unsigned int ast_serial_in(struct ast_uart_port *up, int offset)
+{
+	offset = map_8250_in_reg(up, offset) << up->port.regshift;
+
+		return readb(up->port.membase + offset);
+}
+
+static void
+ast_serial_out(struct ast_uart_port *up, int offset, int value)
+{
+	/* Save the offset before it's remapped */
+	offset = map_8250_out_reg(up, offset) << up->port.regshift;
+
+		writeb(value, up->port.membase + offset);
+}
+
+
+/*
+ * We used to support using pause I/O for certain machines.  We
+ * haven't supported this for a while, but just in case it's badly
+ * needed for certain old 386 machines, I've left these #define's
+ * in....
+ */
+#define serial_inp(up, offset)		ast_serial_in(up, offset)
+#define serial_outp(up, offset, value)	ast_serial_out(up, offset, value)
+
+/* Uart divisor latch read */
+static inline int _serial_dl_read(struct ast_uart_port *up)
+{
+	return serial_inp(up, UART_DLL) | serial_inp(up, UART_DLM) << 8;
+}
+
+/* Uart divisor latch write */
+static inline void _serial_dl_write(struct ast_uart_port *up, int value)
+{
+	serial_outp(up, UART_DLL, value & 0xff);
+	serial_outp(up, UART_DLM, value >> 8 & 0xff);
+}
+
+#define serial_dl_read(up) _serial_dl_read(up)
+#define serial_dl_write(up, value) _serial_dl_write(up, value)
+
+static void ast_uart_tx_dma_complete(void *args);
+
+static int ast_uart_start_tx_dma(struct ast_uart_port *up,
+		unsigned long count)
+{
+	struct circ_buf *xmit = &up->port.state->xmit;
+	dma_addr_t tx_phys_addr;
+
+	UART_DBG("Entered %s count is %d\n", __func__, count);
+	UART_DBG("up->tx_cookie = %d\n", up->tx_cookie);
+	if (up->tx_cookie == 0) {
+		dma_sync_single_for_device(up->port.dev, up->dma_tx_addr,
+						UART_XMIT_SIZE, DMA_TO_DEVICE);
+	tx_phys_addr = up->dma_tx_addr + xmit->tail;
+	UART_DBG("Transmit address is %x actual is %x\n", tx_phys_addr,
+							up->dma_tx_addr);
+	up->tx_dma_desc = dmaengine_prep_slave_single(up->tx_dma_chan,
+		tx_phys_addr, count, DMA_MEM_TO_DEV, DMA_PREP_INTERRUPT);
+	if (!up->tx_dma_desc) {
+		dev_err(up->port.dev, "Not able to get desc for Tx\n");
+		return -EIO;
+	}
+
+	up->tx_dma_desc->callback = ast_uart_tx_dma_complete;
+	up->tx_dma_desc->callback_param = up;
+	up->tx_in_progress = 1;
+	up->tx_bytes_requested = count;
+	up->tx_bytes_transferred = 0;
+	up->tx_cookie = dmaengine_submit(up->tx_dma_desc);
+	}
+	dma_async_issue_pending(up->tx_dma_chan);
+	return 0;
+}
+static void ast_uart_start_next_tx(struct ast_uart_port *up)
+{
+	unsigned long count;
+	struct circ_buf *xmit = &up->port.state->xmit;
+
+	count = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
+	if (count)
+		ast_uart_start_tx_dma(up, count);
+}
+static void ast_uart_tx_sdma_tasklet_func(unsigned long data)
+{
+	struct ast_uart_port *up = ((struct ast_uart_port *)data);
+	struct circ_buf *xmit = &up->port.state->xmit;
+	unsigned long flags = 0;
+
+	UART_DBG("In %s bytes to send is %d\n", __func__, CIRC_CNT(xmit->head,
+	spin_lock_irqsave(&up->port.lock, flags);
+					    xmit->tail, UART_XMIT_SIZE));
+	if (!uart_circ_empty(xmit) && !up->tx_in_progress) {
+		UART_DBG("Calling ast_uart_start_next_tx\n");
+		ast_uart_start_next_tx(up);
+	}
+	spin_unlock_irqrestore(&up->port.lock, flags);
+}
+
+static void ast_uart_tx_dma_complete(void *args)
+{
+	struct ast_uart_port *up = args;
+	struct circ_buf *xmit = &up->port.state->xmit;
+	struct dma_tx_state state;
+	unsigned long flags;
+	unsigned int count;
+	enum dma_status status;
+
+	status = dmaengine_tx_status(up->tx_dma_chan, up->tx_cookie, &state);
+	UART_DBG("%s:state.residue=%d\n", __func__, state.residue);
+	UART_DBG("up->tx_bytes_requested=%d up->tx_bytes_transferred=%d\n",
+			    up->tx_bytes_requested, up->tx_bytes_transferred);
+	if (status == DMA_COMPLETE) {
+		up->tx_cookie = 0;
+		count = up->tx_bytes_requested - up->tx_bytes_transferred;
+		UART_DBG("DMA_COMPLETE:bytes till end of Buffer=%d\n", count);
+	} else{
+		count = up->tx_bytes_requested - state.residue;
+		up->tx_bytes_transferred += count;
+		UART_DBG("DMA_not_COMPLETE: count=%d\n", count);
+	}
+	UART_DBG("up->tx_bytes_requested=%d up->tx_bytes_transferred=%d\n",
+			    up->tx_bytes_requested, up->tx_bytes_transferred);
+	UART_DBG("xmit->head=%d and xmit->tail=%d\n", xmit->head, xmit->tail);
+	async_tx_ack(up->tx_dma_desc);
+	spin_lock_irqsave(&up->port.lock, flags);
+	xmit->tail = (xmit->tail + count) & (UART_XMIT_SIZE - 1);
+	up->tx_in_progress = 0;
+	UART_DBG("updated xmit->head=%d and xmit->tail=%d\n",
+					    xmit->head, xmit->tail);
+	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+		uart_write_wakeup(&up->port);
+	ast_uart_start_next_tx(up);
+	spin_unlock_irqrestore(&up->port.lock, flags);
+}
+
+static void ast_uart_rx_sdma_tasklet_func(unsigned long data);
+static void ast_uart_rx_dma_complete(void *args)
+{
+	struct ast_uart_port *up = args;
+	struct dma_tx_state state;
+	struct circ_buf *rx_ring = &up->rx_dma_buf;
+	unsigned int count;
+	unsigned int temp = 0;
+	enum dma_status status;
+
+	UART_DBG("line [%d],head = %d, len : %d\n",
+				up->port.line, up->rx_dma_buf.head, count);
+	status = dmaengine_tx_status(up->rx_dma_chan, up->rx_cookie, &state);
+	UART_DBG("Freespace in buffer=%d\n", state.residue);
+	UART_DBG("up->rx_bytes_requested=%d up->rx_bytes_transferred=%d\n",
+			    up->rx_bytes_requested, up->rx_bytes_transferred);
+	if (status == DMA_COMPLETE) {
+		up->rx_cookie = 0;
+		count = up->rx_bytes_requested - up->rx_bytes_transferred;
+		up->rx_in_progress = 0;
+		UART_DBG("DMA_COMPLETE:bytes till end of Buffer=%d\n", count);
+	} else{
+		temp = up->rx_bytes_requested - state.residue;
+		count = temp - up->rx_bytes_transferred;
+		up->rx_bytes_transferred = temp;
+		UART_DBG("DMA_not_COMPLETE:fill index =%d\n", temp, count);
+		UART_DBG("bytes to be rxed in current lap=%d\n", count);
+		UART_DBG("rx_bytes_transfred=%d\n", up->rx_bytes_transferred);
+	}
+
+	UART_DBG("rx_ring->head=%d rx_ring->tail=%d\n",
+				    rx_ring->head, rx_ring->tail);
+	rx_ring->head = (rx_ring->head + count) & (UART_RX_SIZE - 1);
+	UART_DBG("updated rx_ring->head=%d rx_ring->tail=%d\n",
+					rx_ring->head, rx_ring->tail);
+	ast_uart_rx_sdma_tasklet_func((unsigned long)up);
+}
+
+static int ast_uart_start_rx_dma(struct ast_uart_port *up,
+					unsigned long count)
+{
+	struct circ_buf *rx_ring = &up->rx_dma_buf;
+	dma_addr_t rx_phys_addr;
+
+	UART_DBG("%s:up->rx_dma_chan= %d\n", __func__, up->rx_dma_chan);
+	UART_DBG("up->rx_cookie = %d\n", up->rx_cookie);
+	if (up->rx_cookie == 0) {
+		dma_sync_single_for_device(up->port.dev, up->dma_rx_addr,
+					UART_RX_SIZE, DMA_FROM_DEVICE);
+		rx_phys_addr = up->dma_rx_addr + rx_ring->tail;
+		up->rx_dma_desc = dmaengine_prep_slave_single(up->rx_dma_chan,
+		rx_phys_addr, UART_RX_SIZE, DMA_DEV_TO_MEM, DMA_PREP_INTERRUPT);
+		if (!up->rx_dma_desc) {
+			dev_err(up->port.dev, "Not able to get desc for Rx\n");
+			return -EIO;
+		}
+		up->rx_dma_desc->callback = ast_uart_rx_dma_complete;
+		up->rx_dma_desc->callback_param = up;
+		up->rx_in_progress = 1;
+		up->rx_bytes_requested = UART_RX_SIZE;
+		up->rx_bytes_transferred = 0;
+		up->rx_cookie = dmaengine_submit(up->rx_dma_desc);
+	}
+	dma_async_issue_pending(up->rx_dma_chan);
+	return 0;
+}
+
+static void ast_uart_rx_sdma_tasklet_func(unsigned long data)
+{
+	struct ast_uart_port *up = ((struct ast_uart_port *)data);
+	struct tty_port *port = &up->port.state->port;
+	struct circ_buf *rx_ring = &up->rx_dma_buf;
+	unsigned long flags;
+	int count;
+	int copy = 0;
+
+	UART_DBG("line [%d], rx_ring->head = %d, rx_ring->tail = %d\n",
+			    up->port.line, rx_ring->head, rx_ring->tail);
+	spin_lock_irqsave(&up->port.lock, flags);
+	if (rx_ring->head > rx_ring->tail) {
+		count = rx_ring->head - rx_ring->tail;
+		UART_DBG("^^^^ count=%d rx_ring->head=%d rx_ring->tail=%d\n",
+					count, rx_ring->head, rx_ring->tail);
+		copy = tty_insert_flip_string(port,
+					rx_ring->buf + rx_ring->tail, count);
+	} else if (rx_ring->head < rx_ring->tail) {
+		count = SDMA_RX_BUFF_SIZE - rx_ring->tail;
+		UART_DBG("rollovr:count=%d rx_ring->head=%d rx_ring->tail=%d\n",
+				    count, rx_ring->head, rx_ring->tail);
+		copy = tty_insert_flip_string(port,
+					rx_ring->buf + rx_ring->tail, count);
+	} else {
+		count = 0;
+	}
+	if (copy != count)
+		UART_DBG("!!!!!!!! ERROR 111\n");
+	if (count) {
+		UART_DBG("count = %d\n", count);
+		rx_ring->tail += count;
+		rx_ring->tail &= (SDMA_RX_BUFF_SIZE - 1);
+		up->port.icount.rx += count;
+		tty_flip_buffer_push(port);
+		spin_unlock_irqrestore(&up->port.lock, flags);
+		ast_uart_start_rx_dma(up, count);
+		spin_lock_irqsave(&up->port.lock, flags);
+	}
+	spin_unlock_irqrestore(&up->port.lock, flags);
+}
+
+/*
+ * FIFO support.
+ */
+static inline void serial8250_clear_fifos(struct ast_uart_port *p)
+{
+	serial_outp(p, UART_FCR, UART_FCR_ENABLE_FIFO);
+	serial_outp(p, UART_FCR, UART_FCR_ENABLE_FIFO |
+		       UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT);
+	serial_outp(p, UART_FCR, 0);
+}
+
+/*
+ * This routine is called by rs_init() to initialize a specific serial
+ * port.
+ */
+static void autoconfig(struct ast_uart_port *up)
+{
+	unsigned long flags;
+
+	UART_DBG("line [%d]\n", up->port.line);
+	if (!up->port.iobase && !up->port.mapbase && !up->port.membase)
+		return;
+
+	DEBUG_AUTOCONF("ttyDMA%d: autoconf (0x%04x, 0x%p): ",
+			up->port.line, up->port.iobase, up->port.membase);
+
+	spin_lock_irqsave(&up->port.lock, flags);
+
+	up->capabilities = 0;
+	up->bugs = 0;
+
+	up->port.type = PORT_16550A;
+	up->capabilities |= UART_CAP_FIFO;
+
+	up->port.fifosize = uart_config[up->port.type].fifo_size;
+	up->capabilities = uart_config[up->port.type].flags;
+	up->tx_loadsz = uart_config[up->port.type].tx_loadsz;
+
+	if (up->port.type == PORT_UNKNOWN)
+		goto out;
+
+	/*
+	 * Reset the UART.
+	 */
+	serial8250_clear_fifos(up);
+	ast_serial_in(up, UART_RX);
+	serial_outp(up, UART_IER, 0);
+
+ out:
+	spin_unlock_irqrestore(&up->port.lock, flags);
+	DEBUG_AUTOCONF("type=%s\n", uart_config[up->port.type].name);
+}
+
+
+static inline void __stop_tx(struct ast_uart_port *p)
+{
+	if (p->ier & UART_IER_THRI) {
+		p->ier &= ~UART_IER_THRI;
+		ast_serial_out(p, UART_IER, p->ier);
+	}
+}
+
+static void serial8250_stop_tx(struct uart_port *port)
+{
+	struct ast_uart_port *up = to_ast_dma_uart_port(port);
+	struct circ_buf *xmit = &up->port.state->xmit;
+	struct dma_tx_state state;
+	unsigned int count;
+
+	__stop_tx(up);
+	if (!up->tx_in_progress)
+		return;
+	dmaengine_terminate_all(up->tx_dma_chan);
+	dmaengine_tx_status(up->tx_dma_chan, up->tx_cookie, &state);
+	count = up->tx_bytes_requested - state.residue;
+	async_tx_ack(up->tx_dma_desc);
+	xmit->tail = (xmit->tail + count) & (UART_XMIT_SIZE - 1);
+	up->tx_in_progress = 0;
+}
+
+static void transmit_chars(struct ast_uart_port *up);
+
+static void serial8250_start_tx(struct uart_port *port)
+{
+	struct ast_uart_port *up = to_ast_dma_uart_port(port);
+	struct circ_buf *xmit = &up->port.state->xmit;
+
+	UART_DBG("\n%s:line %d", __func__, port->line);
+	UART_TX_DBG("line [%d]\n", port->line);
+	if (!uart_circ_empty(xmit) && !up->tx_in_progress) {
+		UART_DBG("Calling ast_uart_start_next_tx\n");
+		ast_uart_start_next_tx(up);
+	}
+}
+
+static void serial8250_stop_rx(struct uart_port *port)
+{
+	struct ast_uart_port *up = to_ast_dma_uart_port(port);
+	struct dma_tx_state state;
+
+	up->ier &= ~UART_IER_RLSI;
+	up->port.read_status_mask &= ~UART_LSR_DR;
+	ast_serial_out(up, UART_IER, up->ier);
+	if (!up->rx_in_progress)
+		return;
+	dmaengine_terminate_all(up->rx_dma_chan);
+	dmaengine_tx_status(up->rx_dma_chan, up->rx_cookie, &state);
+	up->rx_in_progress = 0;
+	up->rx_bytes_transferred = 0;
+}
+
+static void serial8250_enable_ms(struct uart_port *port)
+{
+	struct ast_uart_port *up = to_ast_dma_uart_port(port);
+
+	UART_DBG("line [%d]\n", port->line);
+	up->ier |= UART_IER_MSI;
+	ast_serial_out(up, UART_IER, up->ier);
+}
+
+static void transmit_chars(struct ast_uart_port *up)
+{
+	struct circ_buf *xmit = &up->port.state->xmit;
+	int count;
+
+	if (up->port.x_char) {
+		serial_outp(up, UART_TX, up->port.x_char);
+		up->port.icount.tx++;
+		up->port.x_char = 0;
+		return;
+	}
+	if (uart_tx_stopped(&up->port)) {
+		serial8250_stop_tx(&up->port);
+		return;
+	}
+	if (uart_circ_empty(xmit)) {
+		__stop_tx(up);
+		return;
+	}
+
+	count = up->tx_loadsz;
+	do {
+		ast_serial_out(up, UART_TX, xmit->buf[xmit->tail]);
+		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
+		up->port.icount.tx++;
+		if (uart_circ_empty(xmit))
+			break;
+	} while (--count > 0);
+
+	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+		uart_write_wakeup(&up->port);
+
+	if (uart_circ_empty(xmit))
+		__stop_tx(up);
+}
+
+static unsigned int check_modem_status(struct ast_uart_port *up)
+{
+	unsigned int status = ast_serial_in(up, UART_MSR);
+
+	UART_DBG("line [%d]\n", up->port.line);
+	status |= up->msr_saved_flags;
+	up->msr_saved_flags = 0;
+	if (status & UART_MSR_ANY_DELTA && up->ier & UART_IER_MSI &&
+					    up->port.state != NULL) {
+		if (status & UART_MSR_TERI)
+			up->port.icount.rng++;
+		if (status & UART_MSR_DDSR)
+			up->port.icount.dsr++;
+		if (status & UART_MSR_DDCD)
+			uart_handle_dcd_change(&up->port,
+							status & UART_MSR_DCD);
+		if (status & UART_MSR_DCTS)
+			uart_handle_cts_change(&up->port,
+							status & UART_MSR_CTS);
+		wake_up_interruptible(&up->port.state->port.delta_msr_wait);
+	}
+	return status;
+}
+
+/*
+ * This handles the interrupt from one port.
+ */
+static inline void
+serial8250_handle_port(struct ast_uart_port *up)
+{
+	unsigned int status;
+	unsigned long flags;
+
+	spin_lock_irqsave(&up->port.lock, flags);
+
+	status = serial_inp(up, UART_LSR);
+
+	DEBUG_INTR("status = %x...", status);
+
+	check_modem_status(up);
+	if (status & UART_LSR_THRE)
+		transmit_chars(up);
+
+	spin_unlock_irqrestore(&up->port.lock, flags);
+}
+
+/*
+ * This is the serial driver's interrupt routine.
+ */
+static irqreturn_t ast_uart_interrupt(int irq, void *dev_id)
+{
+	struct irq_info *i = dev_id;
+	int pass_counter = 0, handled = 0, end = 0;
+
+	DEBUG_INTR("(%d) ", irq);
+	spin_lock(&i->lock);
+	do {
+		struct ast_uart_port *up;
+		unsigned int iir;
+
+		up = (struct ast_uart_port *)(i->up);
+		iir = ast_serial_in(up, UART_IIR);
+		if (!(iir & UART_IIR_NO_INT)) {
+			serial8250_handle_port(up);
+			handled = 1;
+		} else
+			end = 1;
+
+		if (pass_counter++ > PASS_LIMIT) {
+			/* If we hit this, we're dead. */
+			UART_DBG(KERN_ERR
+			  "ast-uart-dma:too much work for irqi%d", irq);
+			break;
+		}
+	} while (end);
+
+	spin_unlock(&i->lock);
+
+	DEBUG_INTR("end.\n");
+
+	return IRQ_RETVAL(handled);
+}
+
+static unsigned int serial8250_tx_empty(struct uart_port *port)
+{
+	struct ast_uart_port *up = to_ast_dma_uart_port(port);
+	unsigned long flags;
+	unsigned int lsr;
+
+	UART_TX_DBG("line [%d]\n", up->port.line);
+
+	spin_lock_irqsave(&up->port.lock, flags);
+	lsr = ast_serial_in(up, UART_LSR);
+	up->lsr_saved_flags |= lsr & LSR_SAVE_FLAGS;
+	spin_unlock_irqrestore(&up->port.lock, flags);
+
+	return lsr & UART_LSR_TEMT ? TIOCSER_TEMT : 0;
+}
+
+static unsigned int serial8250_get_mctrl(struct uart_port *port)
+{
+	struct ast_uart_port *up = to_ast_dma_uart_port(port);
+	unsigned int status;
+	unsigned int ret;
+
+	status = check_modem_status(up);
+
+	ret = 0;
+	if (status & UART_MSR_DCD)
+		ret |= TIOCM_CAR;
+	if (status & UART_MSR_RI)
+		ret |= TIOCM_RNG;
+	if (status & UART_MSR_DSR)
+		ret |= TIOCM_DSR;
+	if (status & UART_MSR_CTS)
+		ret |= TIOCM_CTS;
+	return ret;
+}
+
+static void serial8250_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+	struct ast_uart_port *up = to_ast_dma_uart_port(port);
+	unsigned char mcr = 0;
+	//UART_DBG("serial8250_set_mctrl %x\n",mctrl);
+	//TODO .... Issue for fix ......
+	mctrl = 0;
+
+	if (mctrl & TIOCM_RTS)
+		mcr |= UART_MCR_RTS;
+	if (mctrl & TIOCM_DTR)
+		mcr |= UART_MCR_DTR;
+	if (mctrl & TIOCM_OUT1)
+		mcr |= UART_MCR_OUT1;
+	if (mctrl & TIOCM_OUT2)
+		mcr |= UART_MCR_OUT2;
+	if (mctrl & TIOCM_LOOP)
+		mcr |= UART_MCR_LOOP;
+
+	mcr = (mcr & up->mcr_mask) | up->mcr_force | up->mcr;
+
+	ast_serial_out(up, UART_MCR, mcr);
+}
+
+static void serial8250_break_ctl(struct uart_port *port, int break_state)
+{
+	struct ast_uart_port *up = to_ast_dma_uart_port(port);
+	unsigned long flags;
+
+	spin_lock_irqsave(&up->port.lock, flags);
+	if (break_state == -1)
+		up->lcr |= UART_LCR_SBC;
+	else
+		up->lcr &= ~UART_LCR_SBC;
+	ast_serial_out(up, UART_LCR, up->lcr);
+	spin_unlock_irqrestore(&up->port.lock, flags);
+}
+
+static int serial8250_startup(struct uart_port *port)
+{
+	struct ast_uart_port *up = to_ast_dma_uart_port(port);
+	//TX DMA
+	struct circ_buf *xmit = &up->port.state->xmit;
+	unsigned long flags;
+	unsigned char lsr, iir;
+	int retval;
+	struct dma_slave_config dma_sconfig;
+	int irq_flags = up->port.flags & UPF_SHARE_IRQ ? IRQF_SHARED : 0;
+
+	up->capabilities = uart_config[up->port.type].flags;
+	up->mcr = 0;
+	/*
+	 * Clear the FIFO buffers and disable them.
+	 * (they will be reenabled in set_termios())
+	 */
+	serial8250_clear_fifos(up);
+	UART_DBG("1: line [%d]\n", port->line);
+
+	/*
+	 * Clear the interrupt registers.
+	 */
+	(void) serial_inp(up, UART_LSR);
+	(void) serial_inp(up, UART_RX);
+	(void) serial_inp(up, UART_IIR);
+	(void) serial_inp(up, UART_MSR);
+
+	ast_uart_irq[0].up = up;
+	retval = request_irq(up->port.irq, ast_uart_interrupt,
+				 irq_flags, "ast-uart-dma", ast_uart_irq);
+	if (retval)
+		return retval;
+
+	/*
+	 * Now, initialize the UART
+	 */
+	serial_outp(up, UART_LCR, UART_LCR_WLEN8);
+
+	spin_lock_irqsave(&up->port.lock, flags);
+	up->port.mctrl |= TIOCM_OUT2;
+
+	serial8250_set_mctrl(&up->port, up->port.mctrl);
+
+	/*
+	 * Do a quick test to see if we receive an
+	 * interrupt when we enable the TX irq.
+	 */
+	serial_outp(up, UART_IER, UART_IER_THRI);
+	lsr = ast_serial_in(up, UART_LSR);
+	iir = ast_serial_in(up, UART_IIR);
+	serial_outp(up, UART_IER, 0);
+
+	if (lsr & UART_LSR_TEMT && iir & UART_IIR_NO_INT) {
+		if (!(up->bugs & UART_BUG_TXEN)) {
+			up->bugs |= UART_BUG_TXEN;
+			UART_DBG("ttyDMA%d - enabling bad tx status\n",
+				 port->line);
+		}
+	} else {
+		up->bugs &= ~UART_BUG_TXEN;
+	}
+
+	spin_unlock_irqrestore(&up->port.lock, flags);
+
+	/*
+	 * Clear the interrupt registers again for luck, and clear the
+	 * saved flags to avoid getting false values from polling
+	 * routines or the previous session.
+	 */
+	serial_inp(up, UART_LSR);
+	serial_inp(up, UART_RX);
+	serial_inp(up, UART_IIR);
+	serial_inp(up, UART_MSR);
+	up->lsr_saved_flags = 0;
+	up->msr_saved_flags = 0;
+
+	//RX DMA
+	up->rx_dma_buf.head = 0;
+	up->rx_dma_buf.tail = 0;
+	up->port.icount.rx = 0;
+	ast_dma_channel_setup(up);
+	up->rx_dma_buf.buf = dma_alloc_coherent(port->dev, UART_RX_SIZE,
+					&up->dma_rx_addr, GFP_KERNEL);
+	if (!up->rx_dma_buf.buf)
+		ast_dma_channel_teardown(up);
+#if 1
+	memset(&dma_sconfig, 0, sizeof(struct dma_slave_config));
+	dma_sconfig.dst_addr = up->dma_rx_addr;
+	dma_sconfig.dst_port_window_size = UART_RX_SIZE;
+	dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	dma_sconfig.dst_maxburst = 4;
+	dma_sconfig.slave_id = up->channel_no;
+
+	dmaengine_slave_config(up->rx_dma_chan, &dma_sconfig);
+
+	//ast_uart_start_rx_dma(up, UART_RX_SIZE);
+	dma_sync_single_for_device(up->port.dev, up->dma_rx_addr,
+			UART_RX_SIZE, DMA_FROM_DEVICE);
+
+	up->rx_dma_desc = dmaengine_prep_slave_single(up->rx_dma_chan,
+			up->dma_rx_addr, UART_RX_SIZE, DMA_DEV_TO_MEM,
+			DMA_PREP_INTERRUPT);
+	if (!up->rx_dma_desc) {
+		dev_err(up->port.dev, "Not able to get desc for Rx\n");
+		return -EIO;
+	}
+	up->rx_dma_desc->callback = ast_uart_rx_dma_complete;
+	up->rx_dma_desc->callback_param = up;
+	up->rx_in_progress = 1;
+	up->rx_bytes_requested = UART_RX_SIZE;
+	up->rx_cookie = dmaengine_submit(up->rx_dma_desc);
+#endif
+
+	memset(&dma_sconfig, 0, sizeof(struct dma_slave_config));
+
+	up->tx_done = 1;
+	up->tx_count = 0;
+	up->tx_dma_buf.head = 0;
+	up->tx_dma_buf.tail = 0;
+	up->tx_dma_buf.buf = xmit->buf;
+	UART_DBG("head:0x%x tail:0x%x\n", xmit->head, xmit->tail);
+	xmit->head = 0;
+	xmit->tail = 0;
+
+	up->dma_tx_addr = dma_map_single(port->dev,
+				       up->tx_dma_buf.buf,
+				       UART_XMIT_SIZE,
+				       DMA_TO_DEVICE);
+#if 1
+	dma_sconfig.src_addr = up->dma_tx_addr;
+	dma_sconfig.src_port_window_size = UART_XMIT_SIZE;
+	dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	dma_sconfig.src_maxburst = 4;
+	dma_sconfig.slave_id = up->channel_no;
+	dmaengine_slave_config(up->tx_dma_chan, &dma_sconfig);
+#endif
+	//STOP and TRIGGER is done in SDMA driver
+	return 0;
+}
+
+static void serial8250_shutdown(struct uart_port *port)
+{
+	struct ast_uart_port *up = to_ast_dma_uart_port(port);
+	unsigned long flags;
+
+	up->ier = 0;
+	serial_outp(up, UART_IER, 0);
+
+	spin_lock_irqsave(&up->port.lock, flags);
+	up->port.mctrl &= ~TIOCM_OUT2;
+
+	serial8250_set_mctrl(&up->port, up->port.mctrl);
+	spin_unlock_irqrestore(&up->port.lock, flags);
+
+	/*
+	 * Disable break condition and FIFOs
+	 */
+	ast_serial_out(up, UART_LCR, serial_inp(up, UART_LCR) & ~UART_LCR_SBC);
+	serial8250_clear_fifos(up);
+
+	(void) ast_serial_in(up, UART_RX);
+
+	up->rx_in_progress = 0;
+	up->tx_in_progress = 0;
+	dma_release_channel(up->rx_dma_chan);
+	dma_release_channel(up->tx_dma_chan);
+	dma_free_coherent(port->dev, UART_RX_SIZE,
+					up->rx_dma_buf.buf, up->dma_rx_addr);
+	dma_unmap_single(port->dev, up->dma_tx_addr,
+					UART_XMIT_SIZE, DMA_TO_DEVICE);
+	up->rx_dma_chan = NULL;
+	up->tx_dma_chan = NULL;
+	up->dma_rx_addr = 0;
+	up->dma_rx_addr = 0;
+	//Tx buffer will free by serial_core.c
+	free_irq(up->port.irq, ast_uart_irq);
+}
+
+static unsigned int serial8250_get_divisor(struct uart_port *port,
+							unsigned int baud)
+{
+	unsigned int quot;
+
+	quot = uart_get_divisor(port, baud);
+	return quot;
+}
+
+static void
+serial8250_set_termios(struct uart_port *port, struct ktermios *termios,
+		       struct ktermios *old)
+{
+	struct ast_uart_port *up = to_ast_dma_uart_port(port);
+	unsigned char cval, fcr = 0;
+	unsigned long flags;
+	unsigned int baud, quot;
+
+	switch (termios->c_cflag & CSIZE) {
+	case CS5:
+		cval = UART_LCR_WLEN5;
+		break;
+	case CS6:
+		cval = UART_LCR_WLEN6;
+		break;
+	case CS7:
+		cval = UART_LCR_WLEN7;
+		break;
+	default:
+	case CS8:
+		cval = UART_LCR_WLEN8;
+		break;
+	}
+
+	if (termios->c_cflag & CSTOPB)
+		cval |= UART_LCR_STOP;
+	if (termios->c_cflag & PARENB)
+		cval |= UART_LCR_PARITY;
+	if (!(termios->c_cflag & PARODD))
+		cval |= UART_LCR_EPAR;
+#ifdef CMSPAR
+	if (termios->c_cflag & CMSPAR)
+		cval |= UART_LCR_SPAR;
+#endif
+
+	/*
+	 * Ask the core to calculate the divisor for us.
+	 */
+	baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk/16);
+	quot = serial8250_get_divisor(port, baud);
+
+	if (up->capabilities & UART_CAP_FIFO && up->port.fifosize > 1) {
+		if (baud < 2400)
+			fcr = UART_FCR_ENABLE_FIFO | UART_FCR_TRIGGER_1;
+		else
+			fcr = uart_config[up->port.type].fcr;
+	}
+
+	/*
+	 * Ok, we're now changing the port state.  Do it with
+	 * interrupts disabled.
+	 */
+	spin_lock_irqsave(&up->port.lock, flags);
+
+	/*
+	 * Update the per-port timeout.
+	 */
+	uart_update_timeout(port, termios->c_cflag, baud);
+
+	up->port.read_status_mask = UART_LSR_OE | UART_LSR_THRE | UART_LSR_DR;
+	if (termios->c_iflag & INPCK)
+		up->port.read_status_mask |= UART_LSR_FE | UART_LSR_PE;
+	if (termios->c_iflag & (BRKINT | PARMRK))
+		up->port.read_status_mask |= UART_LSR_BI;
+
+	/*
+	 * Characteres to ignore
+	 */
+	up->port.ignore_status_mask = 0;
+	if (termios->c_iflag & IGNPAR)
+		up->port.ignore_status_mask |= UART_LSR_PE | UART_LSR_FE;
+	if (termios->c_iflag & IGNBRK) {
+		up->port.ignore_status_mask |= UART_LSR_BI;
+		/*
+		 * If we're ignoring parity and break indicators,
+		 * ignore overruns too (for real raw support).
+		 */
+		if (termios->c_iflag & IGNPAR)
+			up->port.ignore_status_mask |= UART_LSR_OE;
+	}
+
+	/*
+	 * ignore all characters if CREAD is not set
+	 */
+	if ((termios->c_cflag & CREAD) == 0)
+		up->port.ignore_status_mask |= UART_LSR_DR;
+
+	/*
+	 * CTS flow control flag and modem status interrupts
+	 */
+	up->ier &= ~UART_IER_MSI;
+	if (UART_ENABLE_MS(&up->port, termios->c_cflag))
+		up->ier |= UART_IER_MSI;
+
+	ast_serial_out(up, UART_IER, up->ier);
+
+
+	serial_outp(up, UART_LCR, cval | UART_LCR_DLAB);/* set DLAB */
+
+	serial_dl_write(up, quot);
+
+	/*
+	 * LCR DLAB must be set to enable 64-byte FIFO mode. If the FCR
+	 * is written without DLAB set, this mode will be disabled.
+	 */
+
+	serial_outp(up, UART_LCR, cval);		/* reset DLAB */
+	up->lcr = cval;					/* Save LCR */
+		if (fcr & UART_FCR_ENABLE_FIFO) {
+			/* emulated UARTs (Lucent Venus 167x) need two steps */
+			serial_outp(up, UART_FCR, UART_FCR_ENABLE_FIFO);
+		}
+		serial_outp(up, UART_FCR, fcr);		/* set fcr */
+	serial8250_set_mctrl(&up->port, up->port.mctrl);
+	spin_unlock_irqrestore(&up->port.lock, flags);
+	/* Don't rewrite B0 */
+	if (tty_termios_baud_rate(termios))
+		tty_termios_encode_baud_rate(termios, baud, baud);
+}
+
+static void
+serial8250_pm(struct uart_port *port, unsigned int state,
+	      unsigned int oldstate)
+{
+	struct ast_uart_port *p = (struct ast_uart_port *)port;
+
+	if (p->pm)
+		p->pm(port, state, oldstate);
+}
+
+/*
+ * Resource handling.
+ */
+static int serial8250_request_std_resource(struct ast_uart_port *up)
+{
+	unsigned int size = 8 << up->port.regshift;
+	int ret = 0;
+
+	if (!up->port.mapbase)
+		return ret;
+
+	if (!request_mem_region(up->port.mapbase, size, "ast-uart-dma")) {
+		ret = -EBUSY;
+		return ret;
+	}
+
+	if (up->port.flags & UPF_IOREMAP) {
+		up->port.membase = ioremap_nocache(up->port.mapbase, size);
+		if (!up->port.membase) {
+			release_mem_region(up->port.mapbase, size);
+			ret = -ENOMEM;
+			return ret;
+		}
+	}
+	return ret;
+}
+
+static void serial8250_release_std_resource(struct ast_uart_port *up)
+{
+	unsigned int size = 8 << up->port.regshift;
+
+		if (!up->port.mapbase)
+			return;
+
+		if (up->port.flags & UPF_IOREMAP) {
+			iounmap(up->port.membase);
+			up->port.membase = NULL;
+		}
+
+		release_mem_region(up->port.mapbase, size);
+}
+
+
+static void serial8250_release_port(struct uart_port *port)
+{
+	struct ast_uart_port *up = (struct ast_uart_port *)port;
+
+	serial8250_release_std_resource(up);
+}
+
+static int serial8250_request_port(struct uart_port *port)
+{
+	struct ast_uart_port *up = (struct ast_uart_port *)port;
+	int ret = 0;
+
+	ret = serial8250_request_std_resource(up);
+	if (ret == 0)
+		serial8250_release_std_resource(up);
+
+	return ret;
+}
+
+static void serial8250_config_port(struct uart_port *port, int flags)
+{
+	struct ast_uart_port *up = (struct ast_uart_port *)port;
+	int ret;
+
+	/*
+	 * Find the region that we can probe for.  This in turn
+	 * tells us whether we can probe for the type of port.
+	 */
+	ret = serial8250_request_std_resource(up);
+	if (ret < 0)
+		return;
+
+	if (flags & UART_CONFIG_TYPE)
+		autoconfig(up);
+
+	if (up->port.type == PORT_UNKNOWN)
+		serial8250_release_std_resource(up);
+}
+
+static int
+serial8250_verify_port(struct uart_port *port, struct serial_struct *ser)
+{
+	return 0;
+}
+
+static const char *
+serial8250_type(struct uart_port *port)
+{
+	int type = port->type;
+
+	if (type >= ARRAY_SIZE(uart_config))
+		type = 0;
+	return uart_config[type].name;
+}
+
+static const struct uart_ops serial8250_pops = {
+	.tx_empty	= serial8250_tx_empty,
+	.set_mctrl	= serial8250_set_mctrl,
+	.get_mctrl	= serial8250_get_mctrl,
+	.stop_tx	= serial8250_stop_tx,
+	.start_tx	= serial8250_start_tx,
+	.stop_rx	= serial8250_stop_rx,
+	.enable_ms	= serial8250_enable_ms,
+	.break_ctl	= serial8250_break_ctl,
+	.startup	= serial8250_startup,
+	.shutdown	= serial8250_shutdown,
+	.set_termios	= serial8250_set_termios,
+	.pm		= serial8250_pm,
+	.type		= serial8250_type,
+	.release_port	= serial8250_release_port,
+	.request_port	= serial8250_request_port,
+	.config_port	= serial8250_config_port,
+	.verify_port	= serial8250_verify_port,
+};
+
+static void __init serial8250_isa_init_ports(void)
+{
+	static int first = 1;
+	int i;
+
+	if (!first)
+		return;
+	first = 0;
+
+	for (i = 0; i < nr_uarts; i++) {
+		struct ast_uart_port *up = &ast_uart_ports[i];
+
+		up->port.line = i;
+		spin_lock_init(&up->port.lock);
+
+		/*
+		 * ALPHA_KLUDGE_MCR needs to be killed.
+		 */
+		up->mcr_mask = ~ALPHA_KLUDGE_MCR;
+		up->mcr_force = ALPHA_KLUDGE_MCR;
+
+		up->port.ops = &serial8250_pops;
+	}
+
+}
+
+static void __init
+serial8250_register_ports(struct uart_driver *drv, struct device *dev)
+{
+	int i;
+	struct ast_uart_port *up = NULL;
+
+	serial8250_isa_init_ports();
+	for (i = 0; i < nr_uarts; i++) {
+		up = &ast_uart_ports[i];
+		up->port.dev = dev;
+		uart_add_one_port(drv, &up->port);
+	}
+}
+
+#define SERIAL8250_CONSOLE	NULL
+
+static struct uart_driver serial8250_reg = {
+	.owner			= THIS_MODULE,
+	.driver_name		= "ast-uart-dma",
+	.dev_name		= "ttyDMA",
+#if 0
+	.major			= TTY_MAJOR,
+	.minor			= 64,
+#else
+	.major			= 204, // like atmel_serial
+	.minor			= 155,
+#endif
+	.nr			= UART_DMA_NR,
+	.cons			= SERIAL8250_CONSOLE,
+};
+
+static void ast_dma_channel_teardown(struct ast_uart_port *s)
+{
+	UART_DBG("Teardown called\n");
+	if (s->tx_dma_chan) {
+		dma_release_channel(s->tx_dma_chan);
+		s->tx_dma_chan = NULL;
+	}
+	if (s->rx_dma_chan) {
+		dma_release_channel(s->rx_dma_chan);
+		s->rx_dma_chan = NULL;
+	}
+
+}
+static int ast_dma_channel_setup(struct ast_uart_port *up)
+{
+	up->rx_dma_chan = dma_request_slave_channel(up->port.dev, "rx");
+	UART_DBG("Entered %s ptr is %p\n", __func__, up->rx_dma_chan);
+	if (!up->rx_dma_chan)
+		goto err_out;
+
+	up->tx_dma_chan = dma_request_slave_channel(up->port.dev, "tx");
+	UART_DBG("Entered %s ptr is %p\n", __func__, up->tx_dma_chan);
+	if (!up->tx_dma_chan)
+		goto err_out;
+
+	return 0;
+err_out:
+	ast_dma_channel_teardown(up);
+	return -EINVAL;
+}
+
+/*
+ * Register a set of serial devices attached to a platform device.  The
+ * list is terminated with a zero flags entry, which means we expect
+ * all entries to have at least UPF_BOOT_AUTOCONF set.
+ */
+struct clk *clk;
+static int serial8250_probe(struct platform_device *dev)
+{
+	struct device_node *np = dev->dev.of_node;
+	struct uart_port port;
+	int ret;
+	u32 read = 0;
+	struct resource *res = 0;
+
+	if (UART_XMIT_SIZE > DMA_BUFF_SIZE)
+		UART_DBG("UART_XMIT_SIZE > DMA_BUFF_SIZE : Please Check\n");
+	memset(&port, 0, sizeof(struct uart_port));
+
+	port.irq = platform_get_irq(dev, 0);
+	if (port.irq < 0) {
+		dev_err(&dev->dev, "cannot get irq\n");
+		return port.irq;
+	}
+	res = platform_get_resource(dev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&dev->dev, "Register base not found");
+		return -ENODEV;
+	}
+	port.mapbase = res->start;
+	clk = devm_clk_get(&dev->dev, NULL);
+	if (IS_ERR(clk))
+		dev_err(&dev->dev, "missing controller clock");
+
+	ret = clk_prepare_enable(clk);
+	if (ret) {
+		dev_err(&dev->dev, "failed to enable DMA UART Clk\n");
+		return ret;
+	}
+	port.uartclk = clk_get_rate(clk);
+
+	if (of_property_read_u32(np, "reg-shift", &read) == 0)
+		port.regshift = read;
+	port.iotype = UPIO_MEM;
+	port.flags = UPF_IOREMAP | UPF_BOOT_AUTOCONF | UPF_SKIP_TEST;
+	port.dev = &dev->dev;
+	if (share_irqs)
+		port.flags |= UPF_SHARE_IRQ;
+	ret = ast_uart_register_port(&port, read);
+	if (ret < 0) {
+		dev_err(&dev->dev,
+		"Fail:register_port at index %d(IO%lx MEM%llx IRQ%d): %d\n",
+		read, port.iobase, (unsigned long long)port.mapbase,
+								port.irq, ret);
+	}
+	return ret;
+}
+
+/*
+ * Remove serial ports registered against a platform device.
+ */
+static int serial8250_remove(struct platform_device *dev)
+{
+	int i;
+
+	for (i = 0; i < nr_uarts; i++) {
+		struct ast_uart_port *up = &ast_uart_ports[i];
+
+		if (up->port.dev == &dev->dev)
+			ast_uart_unregister_port(i);
+		ast_dma_channel_teardown(up);
+	}
+	return 0;
+}
+
+static int serial8250_suspend(struct platform_device *dev, pm_message_t state)
+{
+	int i;
+
+	for (i = 0; i < UART_DMA_NR; i++) {
+		struct ast_uart_port *up = &ast_uart_ports[i];
+
+		if (up->port.type != PORT_UNKNOWN && up->port.dev == &dev->dev)
+			uart_suspend_port(&serial8250_reg, &up->port);
+	}
+
+	return 0;
+}
+
+static int serial8250_resume(struct platform_device *dev)
+{
+	int i;
+
+	for (i = 0; i < UART_DMA_NR; i++) {
+		struct ast_uart_port *up = &ast_uart_ports[i];
+
+		if (up->port.type != PORT_UNKNOWN && up->port.dev == &dev->dev)
+			serial8250_resume_port(i);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id ast_serial_dt_ids[] = {
+	 { .compatible = "aspeed,ast-sdma-uart", },
+	 { /* sentinel */ }
+};
+
+static struct platform_driver serial8250_ast_dma_driver = {
+	.probe		= serial8250_probe,
+	.remove		= serial8250_remove,
+	.suspend	= serial8250_suspend,
+	.resume		= serial8250_resume,
+	.driver		= {
+		.name	= "ast-uart-dma",
+		.of_match_table     = of_match_ptr(ast_serial_dt_ids),
+	},
+};
+
+/*
+ * This "device" covers _all_ ISA 8250-compatible serial devices listed
+ * in the table in include/asm/serial.h
+ */
+static struct platform_device *serial8250_isa_devs;
+
+/*
+ * serial8250_register_port and serial8250_unregister_port allows for
+ * 16x50 serial ports to be configured at run-time, to support PCMCIA
+ * modems and PCI multiport cards.
+ */
+
+static struct ast_uart_port *
+		serial8250_find_match_or_unused(struct uart_port *port)
+{
+	int i;
+
+	/*
+	 * First, find a port entry which matches.
+	 */
+	for (i = 0; i < nr_uarts; i++) {
+		if (uart_match_port(&ast_uart_ports[i].port, port))
+			return &ast_uart_ports[i];
+	}
+	/*
+	 * We didn't find a matching entry, so look for the first
+	 * free entry.  We look for one which hasn't been previously
+	 * used (indicated by zero iobase).
+	 */
+	for (i = 0; i < nr_uarts; i++)
+		if (ast_uart_ports[i].port.type == PORT_UNKNOWN &&
+		    ast_uart_ports[i].port.iobase == 0)
+			return &ast_uart_ports[i];
+	/*
+	 * That also failed.  Last resort is to find any entry which
+	 * doesn't have a real port associated with it.
+	 */
+	for (i = 0; i < nr_uarts; i++)
+		if (ast_uart_ports[i].port.type == PORT_UNKNOWN)
+			return &ast_uart_ports[i];
+
+	return NULL;
+}
+
+/**
+ *	serial8250_register_port - register a serial port
+ *	@port: serial port template
+ *
+ *	Configure the serial port specified by the request. If the
+ *	port exists and is in use, it is hung up and unregistered
+ *	first.
+ *
+ *	The port is then probed and if necessary the IRQ is autodetected
+ *	If this fails an error is returned.
+ *
+ *	On success the port is ready to use and the line number is returned.
+ */
+static int ast_uart_register_port(struct uart_port *port,
+					unsigned int channel_no)
+{
+	struct ast_uart_port *uart;
+	int ret = -ENOSPC;
+
+	if (port->uartclk == 0)
+		return -EINVAL;
+
+	mutex_lock(&ast_uart_mutex);
+
+	uart = serial8250_find_match_or_unused(port);
+	if (uart) {
+		uart_remove_one_port(&serial8250_reg, &uart->port);
+		uart->port.iobase       = port->iobase;
+		uart->port.membase      = port->membase;
+		uart->port.irq          = port->irq;
+		uart->port.uartclk      = port->uartclk;
+		uart->port.fifosize     = port->fifosize;
+		uart->port.regshift     = port->regshift;
+		uart->port.iotype       = port->iotype;
+		uart->port.flags        = port->flags | UPF_BOOT_AUTOCONF;
+		uart->port.mapbase      = port->mapbase;
+		uart->port.private_data = uart;
+		if (port->dev) {
+			UART_DBG("Writing dev\n");
+			uart->port.dev = port->dev;
+		}
+		ret = uart_add_one_port(&serial8250_reg, &uart->port);
+		if (ret != 0) {
+			UART_DBG("uart_add_one_port: Failed for port=%p",
+								&uart->port);
+			return ret;
+		}
+		uart->channel_no = channel_no;
+		spin_lock_init(&uart->lock);
+
+		tasklet_init(&uart->tx_tasklet, ast_uart_tx_sdma_tasklet_func,
+				(unsigned long)uart);
+		tasklet_init(&uart->rx_tasklet, ast_uart_rx_sdma_tasklet_func,
+				(unsigned long)uart);
+	}
+
+	mutex_unlock(&ast_uart_mutex);
+	return ret;
+}
+EXPORT_SYMBOL(ast_uart_register_port);
+
+/**
+ *	serial8250_unregister_port - remove a 16x50 serial port at runtime
+ *	@line: serial line number
+ *
+ *	Remove one serial port.  This may not be called from interrupt
+ *	context.  We hand the port back to the our control.
+ */
+static void ast_uart_unregister_port(int line)
+{
+	struct ast_uart_port *uart = &ast_uart_ports[line];
+
+	mutex_lock(&ast_uart_mutex);
+	uart_remove_one_port(&serial8250_reg, &uart->port);
+	if (serial8250_isa_devs) {
+		uart->port.flags &= ~UPF_BOOT_AUTOCONF;
+		uart->port.type = PORT_UNKNOWN;
+		uart->port.dev = &serial8250_isa_devs->dev;
+		uart_add_one_port(&serial8250_reg, &uart->port);
+	} else {
+		uart->port.dev = NULL;
+	}
+	mutex_unlock(&ast_uart_mutex);
+}
+EXPORT_SYMBOL(ast_uart_unregister_port);
+
+static int __init ast_uart_init(void)
+{
+	int ret;
+
+	if (nr_uarts > UART_DMA_NR)
+		nr_uarts = UART_DMA_NR;
+
+	UART_DBG(KERN_INFO
+	"ast-uart-dma: UART driver with DMA %d ports, IRQ sharing %sabled\n",
+					nr_uarts, share_irqs ? "en" : "dis");
+	spin_lock_init(&ast_uart_irq[0].lock);
+
+	ret = uart_register_driver(&serial8250_reg);
+	if (ret)
+		goto out;
+
+	serial8250_isa_devs = platform_device_alloc("ast-uart-dma",
+						    PLAT8250_DEV_LEGACY);
+	if (!serial8250_isa_devs) {
+		ret = -ENOMEM;
+		goto unreg_uart_drv;
+	}
+
+	ret = platform_device_add(serial8250_isa_devs);
+	if (ret)
+		goto put_dev;
+
+	serial8250_register_ports(&serial8250_reg, &serial8250_isa_devs->dev);
+
+	ret = platform_driver_register(&serial8250_ast_dma_driver);
+	if (ret == 0)
+		goto out;
+
+	platform_device_del(serial8250_isa_devs);
+ put_dev:
+	platform_device_put(serial8250_isa_devs);
+ unreg_uart_drv:
+	uart_unregister_driver(&serial8250_reg);
+ out:
+	return ret;
+}
+
+static void __exit ast_uart_exit(void)
+{
+	struct platform_device *isa_dev = serial8250_isa_devs;
+
+	/*
+	 * This tells serial8250_unregister_port() not to re-register
+	 * the ports (thereby making serial8250_ast_dma_driver permanently
+	 * in use.)
+	 */
+	serial8250_isa_devs = NULL;
+
+	platform_driver_unregister(&serial8250_ast_dma_driver);
+	platform_device_unregister(isa_dev);
+
+	uart_unregister_driver(&serial8250_reg);
+}
+
+late_initcall(ast_uart_init);
+module_exit(ast_uart_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("AST DMA serial driver");
+MODULE_ALIAS_CHARDEV_MAJOR(TTY_MAJOR);
-- 
1.9.1

^ permalink raw reply related

* Re: [RFC][PATCHv2 2/4] printk: move printk_safe macros to printk header
From: Sergey Senozhatsky @ 2018-10-17  4:32 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Sergey Senozhatsky, linux-kernel, Petr Mladek, Steven Rostedt,
	Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
	Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
	Sergey Senozhatsky
In-Reply-To: <20181016125415.GA3121@hirez.programming.kicks-ass.net>

On (10/16/18 14:54), Peter Zijlstra wrote:
> On Tue, Oct 16, 2018 at 09:27:34PM +0900, Sergey Senozhatsky wrote:
> > per-CPU printk_safe _semi-magic_ makes some things simple to handle.
> > We can't just remove per-CPU buffers and add a wake_up_process() at
> > the bottom of vprintk_emit(). Because this will deadlock:
> > 
> >   printk()
> >    wake_up_process()
> >     try_to_wake_up()
> >      raw_spin_lock_irqsave()
> >  <NMI>
> >       printk()
> >        wake_up_process()
> >         try_to_wake_up()
> >          raw_spin_lock_irqsave()
> > 
> > So we still need some amount of per-CPU printk() semi-magic anyway.
> 
> All we need is 4 max-line-length buffers per-CPU. Nothing more.

OK, similar to what Steven did with cpu_buffer->current_context.

> The above trainwreck is the direct result of forcing synchronous
> printk'ing (which I'm otherwise a big fan of, but regular console
> drivers stink and are unfixable).

Yep.

> > And printk-kthread offloding will not eliminate the need of
> > printk_deferred().
> 
> Why not? printk() will reduce to a lockless buffer insert. IOW _all_
> printk is deferred.

Aha! Interesting. I didn't realize you were talking about
"all printk()-s are deferred".
OK, jump to the last part of this mail.

> All you need are 4 max-line-length buffers per CPU and a global/shared
> lockless buffer.
> 
> printk will determine the current context:
> 
>   task, softirq, hardirq or NMI
> 
> and pick the corresponding per-cpu line buffer and do the vsnprintf()
> thing. Then we have the actual line length and content. With the length
> we reserve the bytes from the global buffer, we memcpy into the buffer
> and commit.
> 
> Done.
> 
> The printk thread will observe it lags behind the buffer head and will
> start printk-ing crud from task context.

[you can skip this part]

This probably will be a bit more hairy. logbuf is written to by many
sources and is read from by many sides, including user-space [both read()
and write()]. So we will need more flags/magic around memcpy(). A simple,
"grab the logbuf entry, set the proper offset to point to the next available
logbuf record and then do memcpy()" won't suffice. We need a flag for
"memcpy() complete, we can read this entry". Otherwise:

CPU0		CPU1		CPU2			CPU3
printk		printk					printk_kthread
logbuf_entry A	logbuf_entry B  syslog(read all)	call_console_drivers
memcpy		memcpy		read unfinished		print unfinished
					A and B			A and B

[..]

> > We do, however, have loads of problems with all those dependencies which
> > come from serial drivers and friends: timekeeping, scheduler (scheduler
> > is brilliant and cool, but we do have some deadlocks in printk because of
> > it ;), tty, net, MM, wq, etc. So I generally like the idea of "detached
> > serial consoles" (that's what I call it). IOW, elimination of the direct
> > printk -> serial console path. 
> 
> Right; we need to get rid of that in the generic case. Only allow
> lockless consoles (earlycon) to be used synchonously. With maybe a
> special case for the BUG/PANIC case to push things out ASAP.
[..]
> > So, unless I'm missing something, things are not entirely that simple:
> > - throw away printk_safe semi-magic
> > - add a lockless logbuf
> > - add wake_up_process() to vprintk_emit().
> 
> No, no wakups. irq_work to wake the printk-thread, at most.

All right. OK. So we are on the same page here:

printk has internal locks - logbuf spin_lock; and external locks - all
the scheduler locks, console_sem, net, tty, wq, you name it. printk() is
not aware of those external locks; the only way to fix it is to remove
them from printk(). And that's why

	"turn printk() into printk_deferred() and fix printk() deadlocks
	 in general case"

was my final proposal at the 2016 KS, NM, USA [1] (grep for printk_deferred).
I mentioned this idea several times since then, and even sent a patch, doing
this "printk is now printk_deferred unless we are in panic" thing. As far
as I remember, back then the idea/patch were rejected [2], and one of
reviewers even hinted that I was crazy :-) I have absolutely no issues
with that, but, considering past experiences, I'd really like to:

- Have more opinions on this. People please speak out.
- Have clear "let's do it" from Cc-ed people.


If we are really doing this, then let's split it and have
incremental changes. Namely, what I suggest is:

- keep internal printk lock - logbuf lock for now; we know how to
  handle it. I promise.

- keep printk_safe for now, we need it to deal with logbuf lock

- keep printk_safe completely internal to printk

- add printk_kthread

- do printk()->irq_work()->wake_up_process(printk_kthread)
  change and remove external locks dependency

- use direct printk() for panic() case

- do something about early_printk


That's big enough already.
>From there, once we land this thing, we can start building new logbuf,
stealing code from Steven, improving per-CPU buffers and so on.


Are we doing this?


[1] https://lwn.net/Articles/705938/
[2] https://lore.kernel.org/lkml/20170202090722.GW6515@twins.programming.kicks-ass.net/

	-ss

^ permalink raw reply

* Re: [RFC][PATCHv2 1/4] panic: avoid deadlocks in re-entrant console drivers
From: Sergey Senozhatsky @ 2018-10-17  4:48 UTC (permalink / raw)
  To: linux-kernel
  Cc: Petr Mladek, Steven Rostedt, Daniel Wang, Peter Zijlstra,
	Andrew Morton, Linus Torvalds, Greg Kroah-Hartman, Alan Cox,
	Jiri Slaby, Peter Feiner, linux-serial, Sergey Senozhatsky,
	Sergey Senozhatsky
In-Reply-To: <20181016050428.17966-2-sergey.senozhatsky@gmail.com>

On (10/16/18 14:04), Sergey Senozhatsky wrote:
> 
> Fix this by setting oops_in_progress before console_flush_on_panic(),
> so re-entrant console drivers will trylock the port->lock instead of
> spinning on it forever.
> 

Just a small note:
    Regardless of what's going to happen to the series, this patch
    deserves attention.

	-ss

^ permalink raw reply

* Re: [[PATCH] 8/9] DMA-UART-Driver-for-AST2500
From: Vinod @ 2018-10-17  6:05 UTC (permalink / raw)
  To: sudheer.v
  Cc: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Joel Stanley,
	Andrew Jeffery, Russell King, Dan Williams, Jiri Slaby,
	Thomas Gleixner, Marc Zyngier, Benjamin Herrenschmidt,
	Christian Borntraeger, Michael Moese, Hendrik Brueckner,
	Kate Stewart, Philippe Ombredanne, dmaengine, devicetree,
	linux-kernel, linux-serial
In-Reply-To: <1539749466-3912-9-git-send-email-open.sudheer@gmail.com>

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.

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: [PATCH] tty: serial: qcom_geni_serial: Don't slow all ports just for kgdb
From: msavaliy @ 2018-10-17  7:34 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: gregkh, linux-arm-msm, alokc, mka, evgreen, linux-kernel,
	linux-serial, Jiri Slaby, linux-arm-msm-owner
In-Reply-To: <20181015204425.84537-1-dianders@chromium.org>

On 2018-10-16 02:14, Douglas Anderson wrote:
> If you turn on CONFIG_KGDB then you'll get CONFIG_CONSOLE_POLL
> selected.
> 
> If you have CONFIG_CONSOLE_POLL selected then the GENI serial driver
> was setting RX_BYTES_PW to 1 for _all_ UART ports.
> 
> This doesn't seem like such a good idea.  Let's only set RX_BYTES_PW
> to 1 for the console port.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> 
>  drivers/tty/serial/qcom_geni_serial.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
Good finding, Makes sense to keep changes this way for console only 
port.

Reviewed-by: Mukesh Kumar Savaliya <msavaliy@codeaurora.org>

^ permalink raw reply

* Re: [RFC][PATCHv2 2/4] printk: move printk_safe macros to printk header
From: Peter Zijlstra @ 2018-10-17  7:57 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: linux-kernel, Petr Mladek, Steven Rostedt, Daniel Wang,
	Andrew Morton, Linus Torvalds, Greg Kroah-Hartman, Alan Cox,
	Jiri Slaby, Peter Feiner, linux-serial, Sergey Senozhatsky
In-Reply-To: <20181017043251.GC1068@jagdpanzerIV>

On Wed, Oct 17, 2018 at 01:32:51PM +0900, Sergey Senozhatsky wrote:
> This probably will be a bit more hairy. logbuf is written to by many
> sources and is read from by many sides, including user-space [both read()
> and write()]. So we will need more flags/magic around memcpy(). A simple,
> "grab the logbuf entry, set the proper offset to point to the next available
> logbuf record and then do memcpy()" won't suffice. We need a flag for
> "memcpy() complete, we can read this entry". Otherwise:

Sure, but lockless buffers mostly have reserve and commit stages anyway.
Exactly to avoid that problem.

> All right. OK. So we are on the same page here:

> - Have more opinions on this. People please speak out.
> - Have clear "let's do it" from Cc-ed people.
> 
> 
> If we are really doing this, then let's split it and have
> incremental changes. Namely, what I suggest is:

I'd start by replacing logbuf with the lockless buffer and ripping out
the current nmi/safe/etc.. bollocks.

There is absolutely no point what so ever in doing anything until that
is sorted.

After that, move the console output to a kthread, but keep earlycon (if
set) synchronous.

Also avoid the whole panic/warn special case when earlycon is set, no
point in doing dodgy crap when you know you have a good option already.

^ permalink raw reply

* Re: [PATCH v3 1/1] serial: imx - Add dma buffer confugration via sysfs
From: Fabien Lahoudere @ 2018-10-17  8:20 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, linux-serial
In-Reply-To: <20181015140118.GA16234@kroah.com>

Hi Greg,

On Mon, 2018-10-15 at 16:01 +0200, Greg KH wrote:
> On Thu, Oct 11, 2018 at 11:25:03AM +0200, Fabien Lahoudere wrote:
> > In order to optimize serial communication on imx53 and imx6, we may
> > need to tweak DMA period and buffer length per period.
> 
> Why can you not just automatically determine this information?  What is
> userspace going to know that the kernel can not just learn now?
> 
> Having tunables is nice, but it is even better to not need them at all.

I agree that it is better to let the kernel do the configuration.
However in our case we use several serial communication for different device
and one of them need to tweak that configuration to get small data with a better
period. However having this parameter for all serial port implies a bigger number
of interrupts for others too.

> 
> thanks,
> 
> greg k-h
-- 
Fabien

^ permalink raw reply

* Re: [[PATCH] 8/9] DMA-UART-Driver-for-AST2500
From: Benjamin Herrenschmidt @ 2018-10-17  8:56 UTC (permalink / raw)
  To: Vinod, sudheer.v
  Cc: 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, linux-aspee
In-Reply-To: <20181017060531.GU2400@vkoul-mobl>

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.

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: Crash in msm serial on dragonboard with ftrace bootargs
From: Joel Fernandes @ 2018-10-17 10:13 UTC (permalink / raw)
  To: Kees Cook
  Cc: Steven Rostedt, Sai Prakash Ranjan, 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: <CAGXu5jLBtBU1eX7tGX1gOVKSFKeLMeoke3bGHUuqxk86dLSnng@mail.gmail.com>

Hi Kees,

On Tue, Oct 16, 2018 at 10:02:53AM -0700, Kees Cook wrote:
> On Tue, Oct 16, 2018 at 8:29 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> > On Tue, 16 Oct 2018 17:08:25 +0530
> > Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> wrote:
> >> One more thing is for pstore dmesg-ramoops, I had to change
> >> late_initcall to postcore_initcall which brings the question as to why
> >> we changed to late_initcall?
> >> Simple git blame shows to support crypto compress api, but is it really
> >> helpful? A lot of boottime issues can be caught with pstore enabled at
> >> postcore_initcall rather than late_initcall, this backtrace
> >> is just one example. Is there any way we could change this?
> >
> > Does it break if the crypto is not initialized? Perhaps add a command
> > line flag to have it happen earlier:
> >
> >  ramoops=earlyinit
> >
> > and add a postcore_initcall that checks if that flag is set, and if so,
> > it does the work then, and the late_initcall() will do nothing.
> >
> > That way, you can still have unmodified kernels use pstore when it
> > crashes at boot up.
> 
> Even better, if we could find a way to make it work with a late
> initialization of compression (i.e. postcore_initcall() by default
> means anything caught would be uncompressed, but anything after
> late_initcall() would be compressed). I'd be very happy to review
> patches for that!

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)
 {
-- 
2.19.1.331.ge82ca0e54c-goog

^ permalink raw reply related

* Re: [RFC][PATCHv2 2/4] printk: move printk_safe macros to printk header
From: Petr Mladek @ 2018-10-17 10:50 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Sergey Senozhatsky, linux-kernel, Steven Rostedt, Daniel Wang,
	Andrew Morton, Linus Torvalds, Greg Kroah-Hartman, Alan Cox,
	Jiri Slaby, Peter Feiner, linux-serial, Sergey Senozhatsky
In-Reply-To: <20181016121752.GA2537@hirez.programming.kicks-ass.net>

On Tue 2018-10-16 14:17:52, Peter Zijlstra wrote:
> On Tue, Oct 16, 2018 at 01:40:06PM +0200, Petr Mladek wrote:
> > On Tue 2018-10-16 09:27:19, Peter Zijlstra wrote:
> 
> > > Instead of this tinkering around the edges, why don't you make the main
> > > logbuf a lockless ringbuffer and then delegate the actual printing of
> > > that buffer to a kthread, except for earlycon, which can do synchronous
> > > output.
> > 
> > In fact, there is no problem with printk log buffer. This patchset
> > tries to avoid deadlock caused by console-specific locks
> > used by console->write() methods.
> > 
> > By other words, we neither need printk_safe or lockless log buffer
> > to fix this prolem. Instead, we need either printk_deferred context
> > or lockless consoles.
> 
> If you have a lockless buffer and a trailing printk thread, that's
> deferred.
> 
> And earlycon _should_ be lockless (yes, I know, some suck)
> 
> But if you do this deferred nonsense that's currently in printk, then
> you risk never seeing the message -- the same problem I have with the
> whole printk_safe thing too.

What do you mean with printing the message, please? Storing it
into a buffer or showing on console?

I guess that it is storing because you suggest handling
the console by a printk kthread.

Now, please, what is your vision of the lock less buffer?
Would it be similar with the one used by ftrace?

The current implementation of the lockless buffer is very tricky
and supports only one reader. It will be even bigger maze
of code when we add such a support.

Also the messages are spread in many page-size buffers that
are linked in a per-cpu lists. The page used by the reader
is put outside the list. This all makes it very hard to
reconstruct the log. AFAIK, people use ftrace_dump_on_oops
even with a crashdump because nobody improved crash
to extract the messages from the complicated lockless
buffer for years.

IMHO, the current printk buffer is much more easy from
this point of view. It is one global buffer plus
two per-cpu buffers that are mostly empty.


I still think that the buffer is the minor problem these
days. The big problem are consoles and I do not see
how any lockless buffer could help with it.

Also note that by deferred printk I mean deferring the console
handling! IMHO, there are _no more problems_ with storing
the messages into the buffer if we accept that the current
very limited use of printk_safe per-cpu buffers is easier
than any complicated generic lockless buffer.

Best Regards,
Petr

^ permalink raw reply

* Re: Crash in msm serial on dragonboard with ftrace bootargs
From: Sai Prakash Ranjan @ 2018-10-17 11:27 UTC (permalink / raw)
  To: Stephen Boyd, Steven Rostedt
  Cc: 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: <153972306834.5275.7510298070540576604@swboyd.mtv.corp.google.com>

On 10/17/2018 2:21 AM, Stephen Boyd wrote:
> Quoting Sai Prakash Ranjan (2018-10-16 12:35:57)
>> On 10/17/2018 12:45 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.
>>>>
>>>
>>> Which compiler are you using and can you send me your config.
>>>
>> Config attached.
>>
>> Compiler: aarch64-linux-gnu-gcc (Linaro GCC 6.3-2017.02) 6.3.1 20170109
>>
>>> I wonder if there's something screwing up with the way ftrace nops are
>>> working on this board.
>>>
>>> A couple things of note, 1) it works fine after boot up. 2) it crashes
>>> in the initcall code, so it's not due to ftrace being enabled too early.
>>>
>>> I'd like to see the full command line as well. I bet if you remove the
>>> qcom,msm-uartdm from the command line, and had just ftrace=function, it
>>> may also boot fine too. Can you try that?
>>>
>>
>> Kernel command line: root=/dev/disk/by-partlabel/rootfs rw rootwait
>> ftrace=function ftrace_notrace=m* earlycon console=ttyMSM0,115200n8
>>
>> qcom,msm-uartdm is not in command line, it is the earlycon. So without
>> earlycon(bootconsole), board boots fine as we discussed earlier.
>>
> 
> Have you tried with earlycon and no ftrace things on the commandline?
> 
>   root=/dev/disk/by-partlabel/rootfs rw rootwait earlycon console=ttyMSM0,115200n8
> 
> If earlycon is causing problems, it sounds like this may be another case
> of earlycon uart handing off to the uart driver and that failing because
> something gets printed while the uart is transitioning from the earlycon
> console to the kernel boot console. I recall the uart would trample on
> itself in interesting ways.
> 

Yes I have tried with only earlycon enabled and everything is fine. 
Issue is reproduced only with ftrace=function cmdline.

-- 
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: Sai Prakash Ranjan @ 2018-10-17 11:38 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: <20181017101355.GA230639@joelaf.mtv.corp.google.com>

On 10/17/2018 3:43 PM, Joel Fernandes wrote:
> Hi Kees,
> 
> On Tue, Oct 16, 2018 at 10:02:53AM -0700, Kees Cook wrote:
>> On Tue, Oct 16, 2018 at 8:29 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>>> On Tue, 16 Oct 2018 17:08:25 +0530
>>> Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> wrote:
>>>> One more thing is for pstore dmesg-ramoops, I had to change
>>>> late_initcall to postcore_initcall which brings the question as to why
>>>> we changed to late_initcall?
>>>> Simple git blame shows to support crypto compress api, but is it really
>>>> helpful? A lot of boottime issues can be caught with pstore enabled at
>>>> postcore_initcall rather than late_initcall, this backtrace
>>>> is just one example. Is there any way we could change this?
>>>
>>> Does it break if the crypto is not initialized? Perhaps add a command
>>> line flag to have it happen earlier:
>>>
>>>   ramoops=earlyinit
>>>
>>> and add a postcore_initcall that checks if that flag is set, and if so,
>>> it does the work then, and the late_initcall() will do nothing.
>>>
>>> That way, you can still have unmodified kernels use pstore when it
>>> crashes at boot up.
>>
>> Even better, if we could find a way to make it work with a late
>> initialization of compression (i.e. postcore_initcall() by default
>> means anything caught would be uncompressed, but anything after
>> late_initcall() would be compressed). I'd be very happy to review
>> patches for that!
> 
> 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>

-- 
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: Sai Prakash Ranjan @ 2018-10-17 11:53 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Stephen Boyd, Bjorn Andersson, Andy Gross, David Brown,
	Jiri Slaby, Ivan T. Ivanov, Steven Rostedt (VMware), 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: <20181016230938.GA140578@joelaf.mtv.corp.google.com>

On 10/17/2018 4:39 AM, Joel Fernandes wrote:
> On Tue, Oct 16, 2018 at 05:08:25PM +0530, Sai Prakash Ranjan wrote:
>> Hi,
>>
>> On dragonboard 410c, with "ftrace=function" boot args, the console output
>> slows down and board resets without any backtrace as below. This is tested
>> on latest kernel and seems to exist even in older kernels as well.
>>
>> [    2.949164] EINJ: ACPI disabled.
>> [    3.133001] Serial: 8250/16550 dri
>> Format: Log Type - Time(microsec) - Message - Optional Info
>> Log Type: B - Since Boot(Power On Reset),  D - Delta,  S - Statistic
>>
>> But with pstore enabled, able to get the below backtrace:
>> <4>[    2.949164] EINJ: ACPI disabled.
>> <6>[    3.133001] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
>> <6>[    3.164097] SuperH (H)SCI(F) driver initialized
>> <0>[    3.164471] Internal error: synchronous external abort: 96000010 [#1]
>> PREEMPT SMP
>> <4>[    3.164479] Modules linked in:
>> <4>[    3.164495] CPU: 2 PID: 1 Comm: swapper/0 Not tainted
>> 4.19.0-rc8-00008-ge033b9909fff-dirty #175
>> <4>[    3.164501] Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC
>> (DT)
>> <4>[    3.164508] pstate: 40000085 (nZcv daIf -PAN -UAO)
>> <4>[    3.164514] pc : msm_read.isra.2+0x20/0x50
>> <4>[    3.164520] lr : msm_read.isra.2+0x1c/0x50
>> <4>[    3.164526] sp : ffff000008033a50
>> <4>[    3.164531] x29: ffff000008033a50 x28: ffff000009486018
>> <4>[    3.164548] x27: 0000000000000001 x26: ffff7dfffe7ff070
>> <4>[    3.164565] x25: 0000000000000034 x24: ffff000009486000
>> <4>[    3.164582] x23: 0000000000000000 x22: ffff00000978e190
>> <4>[    3.164599] x21: ffff0000095e8228 x20: 0000000000000034
>> <4>[    3.164616] x19: ffff7dfffe7ff008 x18: ffffffffffffffff
>> <4>[    3.164632] x17: 0000000000000000 x16: 0000000000000000
>> <4>[    3.164649] x15: ffff0000094a96c8 x14: ffff00008978e6bf
>> <4>[    3.164666] x13: ffff00000978e6cd x12: 0000000000000038
>> <4>[    3.164683] x11: ffff0000094c6000 x10: 0000000000000c24
>> <4>[    3.164699] x9 : ffff80003c89b400 x8 : ffff000008033970
>> <4>[    3.164716] x7 : ffff80000eb04100 x6 : 00000000000af304
>> <4>[    3.164732] x5 : 0000000000000c40 x4 : ffff80003c06f000
>> <4>[    3.164750] x3 : ffff80003c89b498 x2 : 0000000000000000
>> <4>[    3.164766] x1 : ffff80003ca68000 x0 : 0000000000000800
>> <0>[    3.164785] Process swapper/0 (pid: 1, stack limit =
>> 0x(____ptrval____))
>> <4>[    3.164791] Call trace:
>> <4>[    3.164797]  msm_read.isra.2+0x20/0x50
>> <4>[    3.164804]  msm_reset_dm_count+0x44/0x80
>> <4>[    3.164810]  __msm_console_write+0x1c8/0x1d0
>> <4>[    3.164816]  msm_serial_early_write_dm+0x3c/0x50
>> <4>[    3.164823]  console_unlock.part.6+0x468/0x528
>> <4>[    3.164829]  vprintk_emit+0x210/0x218
>> <4>[    3.164835]  vprintk_default+0x48/0x58
>> <4>[    3.164841]  vprintk_func+0xf0/0x1c0
>> <4>[    3.164847]  printk+0x74/0x94
>> <4>[    3.164853]  sci_init+0x24/0x3c
>> <4>[    3.164859]  do_one_initcall+0x54/0x248
>> <4>[    3.164866]  kernel_init_freeable+0x210/0x378
>> <4>[    3.164872]  kernel_init+0x18/0x118
>> <4>[    3.164878]  ret_from_fork+0x10/0x1c
>> <0>[    3.164884] Code: aa1e03e0 8b214273 97e616f7 d503201f (b9400260)
>>
>> Seems to be issue with the msm serial driver and not ftrace.
>> Could someone look into it.
>>
>> One more thing is for pstore dmesg-ramoops, I had to change late_initcall to
>> postcore_initcall which brings the question as to why we changed to
>> late_initcall?
>> Simple git blame shows to support crypto compress api, but is it really
>> helpful? A lot of boottime issues can be caught with pstore enabled at
>> postcore_initcall rather than late_initcall, this backtrace
>> is just one example. Is there any way we could change this?
> 
> Any chance you are able to dig deeper into the stack trace? I would
> disassemble vmlinux and see which part of msm_read is generating the
> synchronous external abort and look into that.
> 

Yes I had checked the part of msm_read which was generating the abort 
and it always seems to be in "msm_wait_for_xmitr" at below pointed location.

static inline void msm_wait_for_xmitr(struct uart_port *port)
{
         while (!(msm_read(port, UART_SR) & UART_SR_TX_EMPTY)) {     <---
                 if (msm_read(port, UART_ISR) & UART_ISR_TX_READY)
                         break;
                 udelay(1);
         }
         msm_write(port, UART_CR_CMD_RESET_TX_READY, UART_CR);
}

Also I could confirm that this path is entered repeatedly(with tracing 
register reads/writes from my patch series in 
https://lore.kernel.org/patchwork/cover/983795/ and tp_printk) and crash 
is seen at some random time, so could not get much from this.

> Also similar to what Steve suggested, I wonder if it boots for you if you
> annotate all the functions in the serial driver with 'notrace'.
> 

I have tried this too, but still the target crashes. So I am doubtful if 
this is ftrace issue? Maybe earlycon uart issue as Stephen is suspecting.

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: [RFC][PATCHv2 2/4] printk: move printk_safe macros to printk header
From: Sergey Senozhatsky @ 2018-10-17 13:36 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Sergey Senozhatsky, linux-kernel, Petr Mladek, Steven Rostedt,
	Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
	Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
	Sergey Senozhatsky
In-Reply-To: <20181017075728.GF3121@hirez.programming.kicks-ass.net>

On (10/17/18 09:57), Peter Zijlstra wrote:
> On Wed, Oct 17, 2018 at 01:32:51PM +0900, Sergey Senozhatsky wrote:
> > This probably will be a bit more hairy. logbuf is written to by many
> > sources and is read from by many sides, including user-space [both read()
> > and write()]. So we will need more flags/magic around memcpy(). A simple,
> > "grab the logbuf entry, set the proper offset to point to the next available
> > logbuf record and then do memcpy()" won't suffice. We need a flag for
> > "memcpy() complete, we can read this entry". Otherwise:
> 
> Sure, but lockless buffers mostly have reserve and commit stages anyway.
> Exactly to avoid that problem.

Right. So it should be a lockless ringbuffer supporting multiple
parallel readers and multiple parallel writers from multiple contexts
(task, sortirq, irq, NMI); with sane wrap around, etc. And we probably
need to somehow keep the existing format of logbuf entries to avoid
breakages in crash tool, and so on. It will take a lot of time, I'm
afraid.

> > All right. OK. So we are on the same page here:
> 
> > - Have more opinions on this. People please speak out.
> > - Have clear "let's do it" from Cc-ed people.
> > 
> > 
> > If we are really doing this, then let's split it and have
> > incremental changes. Namely, what I suggest is:
> 
> I'd start by replacing logbuf with the lockless buffer and ripping out
> the current nmi/safe/etc.. bollocks.
> 
> There is absolutely no point what so ever in doing anything until that
> is sorted.

Peter, you have your point, however, I think I have a slightly different
priority list. On the top of my list - deadlocks in printk(). That's the
whole reason I sent this series. Deadlocks are real. We have real reports;
in the best case in form of lockdep splats; in the worst case - nothing,
simply because the system deadlocks in printk(), and even more - printk()
can deadlock in panic(). So addressing deadlocks in printk() will fix a
rather big, real problem. Then we can have fun with the rest of the things
you have mentioned. What do you think?

	-ss

^ permalink raw reply

* Re: [RFC][PATCHv2 2/4] printk: move printk_safe macros to printk header
From: Peter Zijlstra @ 2018-10-17 14:00 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Sergey Senozhatsky, linux-kernel, Steven Rostedt, Daniel Wang,
	Andrew Morton, Linus Torvalds, Greg Kroah-Hartman, Alan Cox,
	Jiri Slaby, Peter Feiner, linux-serial, Sergey Senozhatsky,
	john.ogness
In-Reply-To: <20181017105015.udzegzfh7cxgatso@pathway.suse.cz>

On Wed, Oct 17, 2018 at 12:50:15PM +0200, Petr Mladek wrote:

> > If you have a lockless buffer and a trailing printk thread, that's
> > deferred.
> > 
> > And earlycon _should_ be lockless (yes, I know, some suck)
> > 
> > But if you do this deferred nonsense that's currently in printk, then
> > you risk never seeing the message -- the same problem I have with the
> > whole printk_safe thing too.
> 
> What do you mean with printing the message, please? Storing it
> into a buffer or showing on console?
> 
> I guess that it is storing because you suggest handling
> the console by a printk kthread.

I'm not sure from the context you quoted.

> Now, please, what is your vision of the lock less buffer?
> Would it be similar with the one used by ftrace?

No, exactly not like ftrace in fact.

> The current implementation of the lockless buffer is very tricky
> and supports only one reader. It will be even bigger maze
> of code when we add such a support.

The ftrace thing is way complicated and unsuitable. It does what it
does, but I would've never build it that way around.

> IMHO, the current printk buffer is much more easy from
> this point of view. It is one global buffer plus
> two per-cpu buffers that are mostly empty.

I'm arguing for a single (global) lockless buffer with per-cpu line
buffers.

> I still think that the buffer is the minor problem these
> days. The big problem are consoles and I do not see
> how any lockless buffer could help with it.

Yes, the consoles are horrible unfixable crap (some far worse than
others).

But the logbuf is also horrible, and that we can in fact fix.

> Also note that by deferred printk I mean deferring the console
> handling! IMHO, there are _no more problems_ with storing
> the messages into the buffer if we accept that the current
> very limited use of printk_safe per-cpu buffers is easier
> than any complicated generic lockless buffer.

They hide messages. The whole two radically different printk paths is
also quite horrible.

And lockless buffers aren't all _that_ complicated, esp. not when
performance isn't the top priority.

And earlycon is mostly usable, esp. the serial ones. Those, when
configured, should synchronously print along. The current design
also makes that difficult.

A wee little like so; typed in a hurry, never been near a compiler.

struct ll_buffer {
	void *buffer;
	unsigned int size;

	unsigned int tail, reserve, head;

	unsigned int cpu;
	unsigned int ctx;
};

struct ll_entry {
	unsigned int size;
	char data[0];
};

struct ll_handle {
	unsigned int cpu;
	struct ll_entry *lhe;
};

static struct ll_buffer logbuf = {
	.buffer = /* static storage */,
	.cpu = -1;
};

static void __ll_get(struct ll_buffer *llb, struct ll_handle *llh)
{
	unsigned int cpu, old;

	cpu = smp_processor_id();
	for (;;) {
		old = READ_ONCE(llb->cpu);
		if (old == -1) {
			if (try_cmpxchg_acquire(&llb->cpu, &old, cpu))
				break;
		}
		if (old == cpu)
			break;

		cpu_relax();
	}

	llh->cpu = old;
}

static void __ll_put(struct ll_buffer *llb, struct ll_handle *llh)
{
	smp_store_release(llb->cpu, llh->cpu);
}

static char *__ll_reserve(struct ll_buffer *llb, struct ll_hande *llh, unsigned int size)
{
	unsigned int tail, res;
	struct ll_entry *lle;

	__ll_get(llb, llh);

	llb->ctx++;

	size += sizeof(struct ll_entry);
	size += 3;
	size &= ~3;

	do {
		for (;;) {
			tail = READ_ONCE(llb->tail);
			res = READ_ONCE(llb->resserve);

			if (CIRC_SPACE(tail, res, llb->size) > size)
				break;

			lle = llb->buffer + (tail % llb->size);
			cmpxchg(&llb->tail, tail, tail + lhe->size);
		}

	} while (cmpxchg(&llb->reserve, res, res + size) != res);

	llh->lhe = llb->buffer + (res % llb->size);
	llh->lhe->size = size;
}

static void __ll_commit(struct ll_buffer *llb, struct ll_handle *llh)
{
	for (;;) {
		unsigned int ctx, res;

		res = READ_ONCE(llb->res);
		barrier();
		ctx = --llb->ctx;
		if (ctx)
			break;

		smp_store_release(llb->head, res);

		if (likely(READ_ONCE(llb->res) == res))
			break;

		llb->ctx++;
	}

	__ll_put(llb, llh);
}

#define MAX_LINE	1020
#define MAX_CTX		4

struct line_buffers {
	int ctx;
	char buffers[MAX_CTX][MAX_LINE];
};

static DEFINE_PER_CPU(struct line_buffers, linbufs);

int vprintk(const char *fmt, va_list args)
{
	struct ll_handle handle;
	struct line_buffers *lb;
	struct ll_entry *lhe;
	char linbuf;
	int sz;

	preempt_disable();
	lb = this_cpu_ptr(&linbufs);
	linbuf = lb->buffers[lb->ctx++];

	sz = scnprintf(linbuf, MAX_LINE, fmt, args);

	__ll_reserve(&logbuf, &handle, sz);
	memcpy(llh->lhe->data, linbuf, sz); // XXX doesn't wrap on llb->size
	__ll_commit(&logbuf, &handle);

	ln->ctx--;
	preempt_enable();
}


struct ll_reader {
	unsigned int tail;
	char line[MAX_LINE];
};

static bool __ll_read(struct ll_buffer *llb, struct ll_reader *llr)
{
	unsigned int tail, reader, head, sz;
	struct ll_entry *lhe;
	bool lost = false;

	for (;;) {
		head = READ_ONCE(llb->head);
		smp_rmb();
		tail = READ_ONCE(llb->tail);
		reader = READ_ONCE(llr->tail);
		if (unlikely((signed)(tail - reader) < 0)) {
			lost = true;
			reader = tail;
		}
		smp_rmb();
		lhe = llb->buffer + reader;
		sz = lhe->size;
		llr->tail = reader + sz;
		sz -= sizeof(struct ll_entry);

		if ((signed)(head - reader) > 0)
			memcpy(llr->line, lhe->data, sz); // XXX wrap

		smp_rmb();
		tail = READ_ONCE(llb->tail);
		if ((signed)(tail - reader) >= 0)
			break;
	}

	return lost;
}

^ permalink raw reply

* [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