* [PATCH] serial: sh-sci: Fix setting SCSCR_TIE while transferring data
From: Nguyen An Hoan @ 2019-03-18 9:26 UTC (permalink / raw)
To: linux-renesas-soc, geert+renesas
Cc: gregkh, jslaby, ulrich.hecht+renesas, linux-serial, linux-kernel,
kuninori.morimoto.gx, magnus.damm, yoshihiro.shimoda.uh,
h-inayoshi, nv-dung, na-hoan, cv-dong
From: Hoan Nguyen An <na-hoan@jinso.co.jp>
We disable transmission interrupt (clear SCSCR_TIE) after all data has been transmitted
(if uart_circ_empty(xmit)). While transmitting, if the data is still in the tty buffer,
re-enable the SCSCR_TIE bit, which was done at sci_start_tx().
This is unnecessary processing, wasting CPU operation if the data transmission length is large.
And further, transmit end, FIFO empty bits disabling have also been performed in the step above.
Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
---
drivers/tty/serial/sh-sci.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 64bbeb7..93bd90f 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -838,19 +838,9 @@ static void sci_transmit_chars(struct uart_port *port)
if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
uart_write_wakeup(port);
- if (uart_circ_empty(xmit)) {
+ if (uart_circ_empty(xmit))
sci_stop_tx(port);
- } else {
- ctrl = serial_port_in(port, SCSCR);
-
- if (port->type != PORT_SCI) {
- serial_port_in(port, SCxSR); /* Dummy read */
- sci_clear_SCxSR(port, SCxSR_TDxE_CLEAR(port));
- }
- ctrl |= SCSCR_TIE;
- serial_port_out(port, SCSCR, ctrl);
- }
}
/* On SH3, SCIF may read end-of-break as a space->mark char */
--
2.7.4
^ permalink raw reply related
* [PATCH v9] arm64: dts: Add Mediatek SoC MT8183 and evaluation board dts and Makefile
From: Erin Lo @ 2019-03-18 8:42 UTC (permalink / raw)
To: Matthias Brugger, Rob Herring, Mark Rutland
Cc: devicetree, srv_heupstream, linux-kernel, linux-serial,
linux-mediatek, linux-arm-kernel, erin.lo, mars.cheng,
eddie.huang, Ben Ho, Seiya Wang, Weiyi Lu, Hsin-Hsiung Wang
In-Reply-To: <1552898552-13045-1-git-send-email-erin.lo@mediatek.com>
From: Ben Ho <Ben.Ho@mediatek.com>
Add basic chip support for Mediatek 8183, include
uart node with correct uart clocks, pwrap device
Add clock controller nodes, include topckgen, infracfg,
apmixedsys and subsystem.
Signed-off-by: Ben Ho <Ben.Ho@mediatek.com>
Signed-off-by: Erin Lo <erin.lo@mediatek.com>
Signed-off-by: Seiya Wang <seiya.wang@mediatek.com>
Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
Signed-off-by: Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>
Signed-off-by: Eddie Huang <eddie.huang@mediatek.com>
---
arch/arm64/boot/dts/mediatek/Makefile | 1 +
arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 31 +++
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 311 ++++++++++++++++++++++++++++
3 files changed, 343 insertions(+)
create mode 100644 arch/arm64/boot/dts/mediatek/mt8183-evb.dts
create mode 100644 arch/arm64/boot/dts/mediatek/mt8183.dtsi
diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
index e8f952f..458bbc4 100644
--- a/arch/arm64/boot/dts/mediatek/Makefile
+++ b/arch/arm64/boot/dts/mediatek/Makefile
@@ -7,3 +7,4 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb
dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-evb.dtb
+dtb-$(CONFIG_ARCH_MEDIATEK) += mt8183-evb.dtb
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
new file mode 100644
index 0000000..9b52559
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * Copyright (c) 2018 MediaTek Inc.
+ * Author: Ben Ho <ben.ho@mediatek.com>
+ * Erin Lo <erin.lo@mediatek.com>
+ */
+
+/dts-v1/;
+#include "mt8183.dtsi"
+
+/ {
+ model = "MediaTek MT8183 evaluation board";
+ compatible = "mediatek,mt8183-evb", "mediatek,mt8183";
+
+ aliases {
+ serial0 = &uart0;
+ };
+
+ memory@40000000 {
+ device_type = "memory";
+ reg = <0 0x40000000 0 0x80000000>;
+ };
+
+ chosen {
+ stdout-path = "serial0:921600n8";
+ };
+};
+
+&uart0 {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
new file mode 100644
index 0000000..08274bf
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -0,0 +1,311 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * Copyright (c) 2018 MediaTek Inc.
+ * Author: Ben Ho <ben.ho@mediatek.com>
+ * Erin Lo <erin.lo@mediatek.com>
+ */
+
+#include <dt-bindings/clock/mt8183-clk.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+
+/ {
+ compatible = "mediatek,mt8183";
+ interrupt-parent = <&sysirq>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpu-map {
+ cluster0 {
+ core0 {
+ cpu = <&cpu0>;
+ };
+ core1 {
+ cpu = <&cpu1>;
+ };
+ core2 {
+ cpu = <&cpu2>;
+ };
+ core3 {
+ cpu = <&cpu3>;
+ };
+ };
+
+ cluster1 {
+ core0 {
+ cpu = <&cpu4>;
+ };
+ core1 {
+ cpu = <&cpu5>;
+ };
+ core2 {
+ cpu = <&cpu6>;
+ };
+ core3 {
+ cpu = <&cpu7>;
+ };
+ };
+ };
+
+ cpu0: cpu@0 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x000>;
+ enable-method = "psci";
+ };
+
+ cpu1: cpu@1 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x001>;
+ enable-method = "psci";
+ };
+
+ cpu2: cpu@2 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x002>;
+ enable-method = "psci";
+ };
+
+ cpu3: cpu@3 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x003>;
+ enable-method = "psci";
+ };
+
+ cpu4: cpu@100 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a73";
+ reg = <0x100>;
+ enable-method = "psci";
+ };
+
+ cpu5: cpu@101 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a73";
+ reg = <0x101>;
+ enable-method = "psci";
+ };
+
+ cpu6: cpu@102 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a73";
+ reg = <0x102>;
+ enable-method = "psci";
+ };
+
+ cpu7: cpu@103 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a73";
+ reg = <0x103>;
+ enable-method = "psci";
+ };
+ };
+
+ pmu-a53 {
+ compatible = "arm,cortex-a53-pmu";
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW &ppi_cluster0>;
+ };
+
+ pmu-a73 {
+ compatible = "arm,cortex-a73-pmu";
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW &ppi_cluster1>;
+ };
+
+ psci {
+ compatible = "arm,psci-1.0";
+ method = "smc";
+ };
+
+ clk26m: oscillator {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <26000000>;
+ clock-output-names = "clk26m";
+ };
+
+ timer {
+ compatible = "arm,armv8-timer";
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW 0>,
+ <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW 0>,
+ <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW 0>,
+ <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW 0>;
+ };
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ compatible = "simple-bus";
+ ranges;
+
+ gic: interrupt-controller@c000000 {
+ compatible = "arm,gic-v3";
+ #interrupt-cells = <4>;
+ interrupt-parent = <&gic>;
+ interrupt-controller;
+ reg = <0 0x0c000000 0 0x40000>, /* GICD */
+ <0 0x0c100000 0 0x200000>, /* GICR */
+ <0 0x0c400000 0 0x2000>, /* GICC */
+ <0 0x0c410000 0 0x1000>, /* GICH */
+ <0 0x0c420000 0 0x2000>; /* GICV */
+
+ interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH 0>;
+ ppi-partitions {
+ ppi_cluster0: interrupt-partition-0 {
+ affinity = <&cpu0 &cpu1 &cpu2 &cpu3>;
+ };
+ ppi_cluster1: interrupt-partition-1 {
+ affinity = <&cpu4 &cpu5 &cpu6 &cpu7>;
+ };
+ };
+ };
+
+ mcucfg: syscon@c530000 {
+ compatible = "mediatek,mt8183-mcucfg", "syscon";
+ reg = <0 0x0c530000 0 0x1000>;
+ #clock-cells = <1>;
+ };
+
+ sysirq: interrupt-controller@c530a80 {
+ compatible = "mediatek,mt8183-sysirq",
+ "mediatek,mt6577-sysirq";
+ interrupt-controller;
+ #interrupt-cells = <3>;
+ interrupt-parent = <&gic>;
+ reg = <0 0x0c530a80 0 0x50>;
+ };
+
+ topckgen: syscon@10000000 {
+ compatible = "mediatek,mt8183-topckgen", "syscon";
+ reg = <0 0x10000000 0 0x1000>;
+ #clock-cells = <1>;
+ };
+
+ infracfg: syscon@10001000 {
+ compatible = "mediatek,mt8183-infracfg", "syscon";
+ reg = <0 0x10001000 0 0x1000>;
+ #clock-cells = <1>;
+ };
+
+ apmixedsys: syscon@1000c000 {
+ compatible = "mediatek,mt8183-apmixedsys", "syscon";
+ reg = <0 0x1000c000 0 0x1000>;
+ #clock-cells = <1>;
+ };
+
+ pwrap: pwrap@1000d000 {
+ compatible = "mediatek,mt8183-pwrap";
+ reg = <0 0x1000d000 0 0x1000>;
+ reg-names = "pwrap";
+ interrupts = <GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&topckgen CLK_TOP_MUX_PMICSPI>,
+ <&infracfg CLK_INFRA_PMIC_AP>;
+ clock-names = "spi", "wrap";
+ };
+
+ uart0: serial@11002000 {
+ compatible = "mediatek,mt8183-uart",
+ "mediatek,mt6577-uart";
+ reg = <0 0x11002000 0 0x1000>;
+ interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&clk26m>, <&infracfg CLK_INFRA_UART0>;
+ clock-names = "baud", "bus";
+ status = "disabled";
+ };
+
+ uart1: serial@11003000 {
+ compatible = "mediatek,mt8183-uart",
+ "mediatek,mt6577-uart";
+ reg = <0 0x11003000 0 0x1000>;
+ interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&clk26m>, <&infracfg CLK_INFRA_UART1>;
+ clock-names = "baud", "bus";
+ status = "disabled";
+ };
+
+ uart2: serial@11004000 {
+ compatible = "mediatek,mt8183-uart",
+ "mediatek,mt6577-uart";
+ reg = <0 0x11004000 0 0x1000>;
+ interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&clk26m>, <&infracfg CLK_INFRA_UART2>;
+ clock-names = "baud", "bus";
+ status = "disabled";
+ };
+
+ audiosys: syscon@11220000 {
+ compatible = "mediatek,mt8183-audiosys", "syscon";
+ reg = <0 0x11220000 0 0x1000>;
+ #clock-cells = <1>;
+ };
+
+ mfgcfg: syscon@13000000 {
+ compatible = "mediatek,mt8183-mfgcfg", "syscon";
+ reg = <0 0x13000000 0 0x1000>;
+ #clock-cells = <1>;
+ };
+
+ mmsys: syscon@14000000 {
+ compatible = "mediatek,mt8183-mmsys", "syscon";
+ reg = <0 0x14000000 0 0x1000>;
+ #clock-cells = <1>;
+ };
+
+ imgsys: syscon@15020000 {
+ compatible = "mediatek,mt8183-imgsys", "syscon";
+ reg = <0 0x15020000 0 0x1000>;
+ #clock-cells = <1>;
+ };
+
+ vdecsys: syscon@16000000 {
+ compatible = "mediatek,mt8183-vdecsys", "syscon";
+ reg = <0 0x16000000 0 0x1000>;
+ #clock-cells = <1>;
+ };
+
+ vencsys: syscon@17000000 {
+ compatible = "mediatek,mt8183-vencsys", "syscon";
+ reg = <0 0x17000000 0 0x1000>;
+ #clock-cells = <1>;
+ };
+
+ ipu_conn: syscon@19000000 {
+ compatible = "mediatek,mt8183-ipu_conn", "syscon";
+ reg = <0 0x19000000 0 0x1000>;
+ #clock-cells = <1>;
+ };
+
+ ipu_adl: syscon@19010000 {
+ compatible = "mediatek,mt8183-ipu_adl", "syscon";
+ reg = <0 0x19010000 0 0x1000>;
+ #clock-cells = <1>;
+ };
+
+ ipu_core0: syscon@19180000 {
+ compatible = "mediatek,mt8183-ipu_core0", "syscon";
+ reg = <0 0x19180000 0 0x1000>;
+ #clock-cells = <1>;
+ };
+
+ ipu_core1: syscon@19280000 {
+ compatible = "mediatek,mt8183-ipu_core1", "syscon";
+ reg = <0 0x19280000 0 0x1000>;
+ #clock-cells = <1>;
+ };
+
+ camsys: syscon@1a000000 {
+ compatible = "mediatek,mt8183-camsys", "syscon";
+ reg = <0 0x1a000000 0 0x1000>;
+ #clock-cells = <1>;
+ };
+ };
+};
--
1.9.1
^ permalink raw reply related
* [PATCH v9] Add basic and clock support for Mediatek MT8183 SoC
From: Erin Lo @ 2019-03-18 8:42 UTC (permalink / raw)
To: Matthias Brugger, Rob Herring, Mark Rutland
Cc: devicetree, srv_heupstream, erin.lo, linux-kernel, linux-mediatek,
linux-serial, mars.cheng, eddie.huang, linux-arm-kernel
MT8183 is a SoC based on 64bit ARMv8 architecture.
It contains 4 CA53 and 4 CA73 cores.
MT8183 share many HW IP with MT65xx series.
This patchset was tested on MT8183 evaluation board and use correct clock to shell.
Based on v5.1-rc1 and
http://lists.infradead.org/pipermail/linux-mediatek/2019-March/017963.html
Change in v9:
Remove pio node since binding is not documented yet
Change in v8:
1. Fix interrupt-parent of pio node
2. Remove pinfunc.h and spi node patches
Change in v7:
1. Place all the MMIO peripherals under one or more simple-bus nodes
2. Make the pinfunc.h and spi node into seperate patch
3. Modify SPIs pamerater from 4 back to 3
and remove patch "support 4 interrupt parameters for sysirq"
4. Rename intpol-controller to interrupt-controller
5. Rename pinctrl@1000b000 to pinctrl@10005000
Change in v6:
1. Remove power and iommu nodes
2. Fix dtb build warning
3. Fix pinctrl binding doc
4. Fix '_' in node names
Change in v5:
1. Collect all device tree nodes to the last patch
2. Add PMU
3. Add Signed-off-by
4. Remove clock driver code and binding doc
5. Add pinctrl, iommu, spi, and pwrap nodes
Change in v4:
1. Correct syntax error in dtsi
2. Add MT8183 clock support
Change in v3:
1. Fill out GICC, GICH, GICV regions
2. Update Copyright to 2018
Change in v2:
1. Split dt-bindings into different patches
2. Correct bindings for supported SoCs (mtk-uart.txt)
Ben Ho (1):
arm64: dts: Add Mediatek SoC MT8183 and evaluation board dts and
Makefile
arch/arm64/boot/dts/mediatek/Makefile | 1 +
arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 31 +++
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 311 ++++++++++++++++++++++++++++
3 files changed, 343 insertions(+)
create mode 100644 arch/arm64/boot/dts/mediatek/mt8183-evb.dts
create mode 100644 arch/arm64/boot/dts/mediatek/mt8183.dtsi
--
1.9.1
^ permalink raw reply
* Re: [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions
From: Maciej W. Rozycki @ 2019-03-18 8:03 UTC (permalink / raw)
To: Enrico Weigelt, metux IT consult
Cc: Greg KH, Enrico Weigelt, metux IT consult, linux-kernel, eric,
stefan.wahren, f.fainelli, rjui, sbranden,
bcm-kernel-feedback-list, andriy.shevchenko, vz, matthias.bgg,
yamada.masahiro, tklauser, richard.genoud, u.kleine-koenig,
kernel, slemieux.tyco, andy.gross, david.brown, shawnguo, s.hauer,
festevam, linux-imx, baohua, jacmet, linux-serial, linux-arm-msm,
linuxppc-dev
In-Reply-To: <fd8d6a9b-3e85-9904-2195-1f1bbc42bd2d@metux.net>
On Sat, 16 Mar 2019, Enrico Weigelt, metux IT consult wrote:
> > No, it's just that those systems do not allow those devices to be
> > removed because they are probably not on a removable bus.
>
> Ok, devices (hw) might not be removable - that also the case for uarts
> builtin some SoCs, or the good old PC w/ 8250. But does that also mean
> that the driver should not be removable ?
>
> IMHO, even if that's the case, it's still inconsistent. The driver then
> shouldn't support a remove at all (or even builtin only), not just
> incomplete remove.
This device (as well as `dz') is typically used for the serial console as
well, so being built-in is the usual configuration. Nevertheless modular
operation is supposed to be supported, however it may not have been
verified for ages.
A further complication is in the virtual console configuration one of the
serial lines is dedicated for the keyboard, so again you want the driver
built-in (although hooking up the virtual console keyboard this way has
been broken with the conversion to the serial core in the 2.6 timeframe
and I have never figured it out how it is supposed to be done correctly
with the new serial infrastructure and SERIO_SERPORT; I believe some
platforms do it with the use of horrible hacks rather than SERIO_SERPORT).
Maciej
^ permalink raw reply
* Re: [PATCH v2 07/45] drivers: tty: serial: 8250_uniphier: use devm_ioremap_resource()
From: Masahiro Yamada @ 2019-03-18 7:44 UTC (permalink / raw)
To: Enrico Weigelt, metux IT consult
Cc: Linux Kernel Mailing List, Greg Kroah-Hartman, Eric Anholt,
Stefan Wahren, Florian Fainelli, Ray Jui, Scott Branden,
Broadcom Kernel Feedback List, Andy Shevchenko,
Vladimir Zapolskiy, Matthias Brugger, Tobias Klauser,
Richard Genoud, Maciej W. Rozycki, Uwe Kleine-König,
Sascha Hauer
In-Reply-To: <1552602855-26086-8-git-send-email-info@metux.net>
On Fri, Mar 15, 2019 at 7:35 AM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:
>
> Instead of fetching out data from a struct resource for passing
> it to devm_ioremap(), directly use devm_ioremap_resource()
>
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
> ---
NACK.
This patch would break my driver.
Probably, all the 8250* drivers would be broken in the same way.
For 8250 drivers, request_mem_region() is called
from 8250_port.c
Let's not touch around the code you do not
understand how it works.
Thanks.
Masahiro Yamada
> drivers/tty/serial/8250/8250_uniphier.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/8250/8250_uniphier.c b/drivers/tty/serial/8250/8250_uniphier.c
> index 164ba13..9c1244e 100644
> --- a/drivers/tty/serial/8250/8250_uniphier.c
> +++ b/drivers/tty/serial/8250/8250_uniphier.c
> @@ -171,7 +171,7 @@ static int uniphier_uart_probe(struct platform_device *pdev)
> return -EINVAL;
> }
>
> - membase = devm_ioremap(dev, regs->start, resource_size(regs));
> + membase = devm_ioremap_resource(dev, regs);
> if (!membase)
> return -ENOMEM;
>
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH] tty: atmel_serial: fix a NULL pointer dereference
From: Richard Genoud @ 2019-03-18 7:28 UTC (permalink / raw)
To: Kangjie Lu
Cc: pakki001, Greg Kroah-Hartman, Jiri Slaby, Nicolas Ferre,
Alexandre Belloni, Ludovic Desroches, linux-serial,
linux-arm-kernel, linux-kernel
In-Reply-To: <20190315171606.6282-1-kjlu@umn.edu>
Le 15/03/2019 à 18:16, Kangjie Lu a écrit :
> In case dmaengine_prep_dma_cyclic fails, the fix returns a proper
> error code to avoid NULL pointer dereference.
>
> Signed-off-by: Kangjie Lu <kjlu@umn.edu>
> Fixes: 34df42f59a60 ("serial: at91: add rx dma support")
Acked-by: Richard Genoud <richard.genoud@gmail.com>
>
> ---
> V2: simplified the patch as suggested by
> Richard Genoud <richard.genoud@gmail.com>
> ---
> drivers/tty/serial/atmel_serial.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 05147fe24343..41b728d223d1 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -1288,6 +1288,10 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
> sg_dma_len(&atmel_port->sg_rx)/2,
> DMA_DEV_TO_MEM,
> DMA_PREP_INTERRUPT);
> + if (!desc) {
> + dev_err(port->dev, "Preparing DMA cyclic failed\n");
> + goto chan_err;
> + }
> desc->callback = atmel_complete_rx_dma;
> desc->callback_param = port;
> atmel_port->desc_rx = desc;
>
Thanks !
Richard
^ permalink raw reply
* Re: [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions
From: Greg KH @ 2019-03-17 9:54 UTC (permalink / raw)
To: Enrico Weigelt, metux IT consult
Cc: Enrico Weigelt, metux IT consult, linux-kernel, eric,
stefan.wahren, f.fainelli, rjui, sbranden,
bcm-kernel-feedback-list, andriy.shevchenko, vz, matthias.bgg,
yamada.masahiro, tklauser, richard.genoud, macro, u.kleine-koenig,
kernel, slemieux.tyco, andy.gross, david.brown, shawnguo, s.hauer,
festevam, linux-imx, baohua, jacmet, linux-serial, linux-arm-msm,
linuxppc-dev
In-Reply-To: <fd8d6a9b-3e85-9904-2195-1f1bbc42bd2d@metux.net>
On Sat, Mar 16, 2019 at 10:17:11AM +0100, Enrico Weigelt, metux IT consult wrote:
> On 16.03.19 04:26, Greg KH wrote:
>
> > No, it's just that those systems do not allow those devices to be
> > removed because they are probably not on a removable bus.
>
> Ok, devices (hw) might not be removable - that also the case for uarts
> builtin some SoCs, or the good old PC w/ 8250. But does that also mean
> that the driver should not be removable ?
No, but 'rmmod' is not a normal operation that anyone ever does in a
working system. It is only for developer's ease-of-use.
> IMHO, even if that's the case, it's still inconsistent. The driver then
> shouldn't support a remove at all (or even builtin only), not just
> incomplete remove.
Cleaning up properly when the module is unloaded is a good idea, but so
far the patches you submitted did not change anything from a logic point
of view. They all just cleaned up memory the same way it was cleaned up
before, so I really do not understand what you are trying to do here.
> >> Okay, I was on a wrong track here - I had the silly idea that it would
> >> make things easier if we'd do it the same way everywhere.
> >
> > "Consistent" is good, and valid, but touching old drivers that have few
> > users is always risky, and you need a solid reason to do so.
>
> Understood.
>
> By the way: do we have some people who have those old hw and could test?
> Should we (try to) create some ? Perhaps some "tester" entry in
> MAINTAINERS file ? (I could ask around several people who might have
> lots of old / rare hardware.)
Let's not clutter up MAINTAINERS with anything else please.
> >> Understood. Assuming I've found some of these cases, shall I use devm
> >> oder just add the missing release ?
> >
> > If it actually makes the code "simpler" or "more obvious", sure, that's
> > fine. But churn for churns sake is not ok.
>
> Ok.
>
> > I put the review of new patch submissions on hold, yes. Almost all
> > maintainers do that as we can not add new patches to our trees at that
> > point in time.
>
> hmm, looks like a pipeline stall ;-)
> why not collecting in a separate branch, which later gets rebased to
> mainline when rc is out ?
I do do that for subsystems that actually have a high patch rate. The
tty/serial subsystem is not such a thing, and it can handle 2 weeks of
delay just fine.
> > And I do have other things I do during that period so it's not like I'm
> > just sitting around doing nothing :)
>
> So it's also a fixed schedule for your other work. Understood.
>
> It seems that this workflow can confuse people. Few days ago, somebody
> became nervous about missing reactions on patches. Your autoresponder
> worked for me, but maybe not for everybody.
Why would it not work for everybody? Kernel development has been done
in this manner for over a decade. Having a 2 week window like this is
good for the maintainers, remember they are the most limited resource we
have, not developers.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions
From: Enrico Weigelt, metux IT consult @ 2019-03-16 9:17 UTC (permalink / raw)
To: Greg KH
Cc: Enrico Weigelt, metux IT consult, linux-kernel, eric,
stefan.wahren, f.fainelli, rjui, sbranden,
bcm-kernel-feedback-list, andriy.shevchenko, vz, matthias.bgg,
yamada.masahiro, tklauser, richard.genoud, macro, u.kleine-koenig,
kernel, slemieux.tyco, andy.gross, david.brown, shawnguo, s.hauer,
festevam, linux-imx, baohua, jacmet, linux-serial, linux-arm-msm,
linuxppc-dev
In-Reply-To: <20190316032630.GB2499@kroah.com>
On 16.03.19 04:26, Greg KH wrote:
> No, it's just that those systems do not allow those devices to be
> removed because they are probably not on a removable bus.
Ok, devices (hw) might not be removable - that also the case for uarts
builtin some SoCs, or the good old PC w/ 8250. But does that also mean
that the driver should not be removable ?
IMHO, even if that's the case, it's still inconsistent. The driver then
shouldn't support a remove at all (or even builtin only), not just
incomplete remove.
>> Okay, I was on a wrong track here - I had the silly idea that it would
>> make things easier if we'd do it the same way everywhere.
>
> "Consistent" is good, and valid, but touching old drivers that have few
> users is always risky, and you need a solid reason to do so.
Understood.
By the way: do we have some people who have those old hw and could test?
Should we (try to) create some ? Perhaps some "tester" entry in
MAINTAINERS file ? (I could ask around several people who might have
lots of old / rare hardware.)
>> Understood. Assuming I've found some of these cases, shall I use devm
>> oder just add the missing release ?
>
> If it actually makes the code "simpler" or "more obvious", sure, that's
> fine. But churn for churns sake is not ok.
Ok.
> I put the review of new patch submissions on hold, yes. Almost all
> maintainers do that as we can not add new patches to our trees at that
> point in time.
hmm, looks like a pipeline stall ;-)
why not collecting in a separate branch, which later gets rebased to
mainline when rc is out ?
> And I do have other things I do during that period so it's not like I'm
> just sitting around doing nothing :)
So it's also a fixed schedule for your other work. Understood.
It seems that this workflow can confuse people. Few days ago, somebody
became nervous about missing reactions on patches. Your autoresponder
worked for me, but maybe not for everybody.
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287
^ permalink raw reply
* Re: [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions
From: Greg KH @ 2019-03-16 3:26 UTC (permalink / raw)
To: Enrico Weigelt, metux IT consult
Cc: Enrico Weigelt, metux IT consult, linux-kernel, eric,
stefan.wahren, f.fainelli, rjui, sbranden,
bcm-kernel-feedback-list, andriy.shevchenko, vz, matthias.bgg,
yamada.masahiro, tklauser, richard.genoud, macro, u.kleine-koenig,
kernel, slemieux.tyco, andy.gross, david.brown, shawnguo, s.hauer,
festevam, linux-imx, baohua, jacmet, linux-serial, linux-arm-msm,
linuxppc-dev
In-Reply-To: <d81364fc-c5da-c1b2-f38a-3a23c701c50d@metux.net>
On Fri, Mar 15, 2019 at 08:12:47PM +0100, Enrico Weigelt, metux IT consult wrote:
> On 15.03.19 15:26, Greg KH wrote:
> > Yes, there are lots of drivers for devices that are never unloaded or
> > removed from the system. The fact that no one has reported any problems
> > with them means that they are never used in situations like this.
>
> So, never touch a running system ?
No, it's just that those systems do not allow those devices to be
removed because they are probably not on a removable bus.
> > No, you need to have a good reason why it needs to be changed, when you
> > can not verify that this actually resolves a problem. As this patch
> > shows, you just replaced one api call with another, so nothing changed
> > at all, except you actually took up more memory and logic to do the same
> > thing :(
>
> Okay, I was on a wrong track here - I had the silly idea that it would
> make things easier if we'd do it the same way everywhere.
"Consistent" is good, and valid, but touching old drivers that have few
users is always risky, and you need a solid reason to do so.
> >> IMHO, we should have a closer look at those and check whether that's
> >> really okay (just adding request/release blindly could cause trouble)
> >
> > I agree, please feel free to audit these to verify they are all correct.
> > But that's not what you did here, so that's not a valid justification
> > for these patches to be accepted.
>
> Understood. Assuming I've found some of these cases, shall I use devm
> oder just add the missing release ?
If it actually makes the code "simpler" or "more obvious", sure, that's
fine. But churn for churns sake is not ok.
> >> By the way: do you have some public branch where you're collecting
> >> accepted patches, which I could base mine on ? (tty.git/tty-next ?)
> >
> > Yes, that is the tree and branch, but remember that none of my trees can
> > open up until after -rc1 is out.
>
> So, within a merge window, you put everything else on hold ?
I put the review of new patch submissions on hold, yes. Almost all
maintainers do that as we can not add new patches to our trees at that
point in time.
And I do have other things I do during that period so it's not like I'm
just sitting around doing nothing :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions
From: Enrico Weigelt, metux IT consult @ 2019-03-15 19:12 UTC (permalink / raw)
To: Greg KH
Cc: Enrico Weigelt, metux IT consult, linux-kernel, eric,
stefan.wahren, f.fainelli, rjui, sbranden,
bcm-kernel-feedback-list, andriy.shevchenko, vz, matthias.bgg,
yamada.masahiro, tklauser, richard.genoud, macro, u.kleine-koenig,
kernel, slemieux.tyco, andy.gross, david.brown, shawnguo, s.hauer,
festevam, linux-imx, baohua, jacmet, linux-serial, linux-arm-msm,
linuxppc-dev
In-Reply-To: <20190315142628.GA30650@kroah.com>
On 15.03.19 15:26, Greg KH wrote:
> On Fri, Mar 15, 2019 at 10:06:16AM +0100, Enrico Weigelt, metux IT consult wrote:
>> On 14.03.19 23:52, Greg KH wrote:
>>> On Thu, Mar 14, 2019 at 11:33:40PM +0100, Enrico Weigelt, metux IT consult wrote:
>>>> Use the safer devm versions of memory mapping functions.
>>>
>>> What is "safer" about them?
>>
>> Garbage collection :)
>
> At times, but not when you do not use the api correctly :)
Okay, my fault that I didn't read the code careful enough :o
But still, I think the name is a bit misleading as it *sounds* as just
a wrapper around devm_ioremap() that just picks the params from a
struct resource. I guess we can't change the name easily ?
> Yes, there are lots of drivers for devices that are never unloaded or
> removed from the system. The fact that no one has reported any problems
> with them means that they are never used in situations like this.
So, never touch a running system ?
> No, you need to have a good reason why it needs to be changed, when you
> can not verify that this actually resolves a problem. As this patch
> shows, you just replaced one api call with another, so nothing changed
> at all, except you actually took up more memory and logic to do the same
> thing :(
Okay, I was on a wrong track here - I had the silly idea that it would
make things easier if we'd do it the same way everywhere.
>> IMHO, we should have a closer look at those and check whether that's
>> really okay (just adding request/release blindly could cause trouble)
>
> I agree, please feel free to audit these to verify they are all correct.
> But that's not what you did here, so that's not a valid justification
> for these patches to be accepted.
Understood. Assuming I've found some of these cases, shall I use devm
oder just add the missing release ?
>> In this consolidation process, I'm trying to move everything to
>> devm_*, to have it more generic (for now, I still need two versions
>> of the request/release/ioremap/iounmap helpers - one w/ and one
>> w/o devm).
>
> Move everything in what part of the kernel? The whole kernel or just
> one subsystem?
Just talking about the serial drivers.
>> My idea was moving to devm first, so it can be reviewed/tested
>> independently, before moving forward. Smaller, easily digestable
>> pieces should minimize the risk of breaking anything. But if you
>> prefer having this things squashed together, just let me know.
>
> Small pieces are required, that's fine, but those pieces need to have a
> justification for why they should be accepted at all points along the
> way.
Hmm, okay, in these cases, I agree there's no real justification if
we're not seeing it as an intermediate step to the upcoming stuff.
Having thought a bit more about this, my underlying dumbness was
setting everything on the devm horse when converting introducing the
helpers, and then splitted out the change to devm in even more patches
... Silly me, I better should have catched some sleep instead :o
>> In the queue are also other minor cleanups like using dev_err()
>> instead of printk(), etc. Should I send these separately ?
>
> Of course.
Ok. I'll collect those things in a separate branch and send out the
queue from time to time:
https://github.com/metux/linux/tree/wip/serial/charlady
>> By the way: do you have some public branch where you're collecting
>> accepted patches, which I could base mine on ? (tty.git/tty-next ?)
>
> Yes, that is the tree and branch, but remember that none of my trees can
> open up until after -rc1 is out.
So, within a merge window, you put everything else on hold ?
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287
^ permalink raw reply
* Re: serial driver cleanups v2
From: Andy Shevchenko @ 2019-03-15 18:45 UTC (permalink / raw)
To: Enrico Weigelt, metux IT consult
Cc: Enrico Weigelt, metux IT consult, Linux Kernel Mailing List,
Greg Kroah-Hartman, Eric Anholt, Stefan Wahren, Florian Fainelli,
Ray Jui, Scott Branden, bcm-kernel-feedback-list,
Vladimir Zapolskiy, Matthias Brugger, Masahiro Yamada,
Tobias Klauser, Richard Genoud, macro, Uwe Kleine-König,
Sascha Hauer, slemieux.tyco
In-Reply-To: <d9ac8446-c7ff-6e57-5eaa-f1a09d549081@metux.net>
On Fri, Mar 15, 2019 at 8:44 PM Enrico Weigelt, metux IT consult
<lkml@metux.net> wrote:
> On 15.03.19 19:11, Andy Shevchenko wrote:
> OTOH, the name, IMHO, is a bit misleading. Any chance of ever changing
> it to a more clear name (eg. devm_request_and_ioremap_resource()) ?
Compare iomap vs. ioREmap.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: serial driver cleanups v2
From: Enrico Weigelt, metux IT consult @ 2019-03-15 18:41 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Enrico Weigelt, metux IT consult, Linux Kernel Mailing List,
Greg Kroah-Hartman, Eric Anholt, Stefan Wahren, Florian Fainelli,
Ray Jui, Scott Branden, bcm-kernel-feedback-list,
Vladimir Zapolskiy, Matthias Brugger, Masahiro Yamada,
Tobias Klauser, Richard Genoud, macro, Uwe Kleine-König,
Sascha Hauer, slemieux.tyco
In-Reply-To: <20190315181118.GF9224@smile.fi.intel.com>
On 15.03.19 19:11, Andy Shevchenko wrote:
>> The actual goal is generalizing the whole iomem handling, so individual
>> usually just need to call some helpers that do most of the things.
>> Finally, I also wanted to have all io region information consolidated
>> in struct resource.
>
> That's should be a selling point, not just conversion per se.
hmm, I never was good at selling :o
but I'll try anyways: it shall make the code smaller and easier to
read/understand.
does that count ?
> You have to explain that in each commit message, that the change does bring a
> possible new error message printed.
Ok, I wasn't aware of that. Yes, I missed to read the code of that
function carefully and didn't expect that it does more than just a dumb
wrapper around dev_ioremap() that picks out the params from struct
resource.
OTOH, the name, IMHO, is a bit misleading. Any chance of ever changing
it to a more clear name (eg. devm_request_and_ioremap_resource()) ?
> The performance side of the deal, you are lucky here, is not significant
> because it's slow path.
okay :)
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287
^ permalink raw reply
* Re: serial driver cleanups v2
From: Andy Shevchenko @ 2019-03-15 18:11 UTC (permalink / raw)
To: Enrico Weigelt, metux IT consult
Cc: Enrico Weigelt, metux IT consult, Linux Kernel Mailing List,
Greg Kroah-Hartman, Eric Anholt, Stefan Wahren, Florian Fainelli,
Ray Jui, Scott Branden, bcm-kernel-feedback-list,
Vladimir Zapolskiy, Matthias Brugger, Masahiro Yamada,
Tobias Klauser, Richard Genoud, macro, Uwe Kleine-König,
Sascha Hauer, slemieux.tyco
In-Reply-To: <afc51d27-3312-cdf4-68a5-4486db1d006b@metux.net>
On Fri, Mar 15, 2019 at 11:36:04AM +0100, Enrico Weigelt, metux IT consult wrote:
> On 15.03.19 10:12, Andy Shevchenko wrote:
>
> >> Part II will be about moving the mmio range from mapbase and
> >> mapsize (which are used quite inconsistently) to a struct resource
> >> and using helpers for that. But this one isn't finished yet.
> >> (if somebody likes to have a look at it, I can send it, too)
> >
> > Let's do that way you are preparing a branch somewhere and anounce
> > here as an RFC, since this was neither tested nor correct.
>
> Okay, here it is:
>
> I. https://github.com/metux/linux/tree/submit/serial-clean-v3
> --> general cleanups, as basis for II
>
> II. https://github.com/metux/linux/tree/wip/serial-res
> --> moving towards using struct resource consistently
>
> III. https://github.com/metux/linux/tree/hack/serial
> --> the final steps, which are yet completely broken
> (more a notepad for things still to do :o)
>
> The actual goal is generalizing the whole iomem handling, so individual
> usually just need to call some helpers that do most of the things.
> Finally, I also wanted to have all io region information consolidated
> in struct resource.
That's should be a selling point, not just conversion per se.
> > And selling point for many of them is not true: it doesn't make any
> > difference in the size in code, but increases a time to run
> > (devm_ioremap_resource() does more than plain devm_iomap() call).
>
> Okay, just seen it. Does the the runtime overhead cause any problems ?
You have to explain that in each commit message, that the change does bring a
possible new error message printed.
The performance side of the deal, you are lucky here, is not significant
because it's slow path.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH] tty: atmel_serial: fix a NULL pointer dereference
From: Kangjie Lu @ 2019-03-15 17:16 UTC (permalink / raw)
To: kjlu
Cc: pakki001, Richard Genoud, Greg Kroah-Hartman, Jiri Slaby,
Nicolas Ferre, Alexandre Belloni, Ludovic Desroches, linux-serial,
linux-arm-kernel, linux-kernel
In-Reply-To: <d8978a94-23fc-3610-e3da-5e1da2d86914@sorico.fr>
In case dmaengine_prep_dma_cyclic fails, the fix returns a proper
error code to avoid NULL pointer dereference.
Signed-off-by: Kangjie Lu <kjlu@umn.edu>
Fixes: 34df42f59a60 ("serial: at91: add rx dma support")
---
V2: simplified the patch as suggested by
Richard Genoud <richard.genoud@gmail.com>
---
drivers/tty/serial/atmel_serial.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 05147fe24343..41b728d223d1 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -1288,6 +1288,10 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
sg_dma_len(&atmel_port->sg_rx)/2,
DMA_DEV_TO_MEM,
DMA_PREP_INTERRUPT);
+ if (!desc) {
+ dev_err(port->dev, "Preparing DMA cyclic failed\n");
+ goto chan_err;
+ }
desc->callback = atmel_complete_rx_dma;
desc->callback_param = port;
atmel_port->desc_rx = desc;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions
From: Greg KH @ 2019-03-15 14:26 UTC (permalink / raw)
To: Enrico Weigelt, metux IT consult
Cc: Enrico Weigelt, metux IT consult, linux-kernel, eric,
stefan.wahren, f.fainelli, rjui, sbranden,
bcm-kernel-feedback-list, andriy.shevchenko, vz, matthias.bgg,
yamada.masahiro, tklauser, richard.genoud, macro, u.kleine-koenig,
kernel, slemieux.tyco, andy.gross, david.brown, shawnguo, s.hauer,
festevam, linux-imx, baohua, jacmet, linux-serial, linux-arm-msm,
linuxppc-dev
In-Reply-To: <3734d588-6b9c-29e2-45b6-82e778f47602@metux.net>
On Fri, Mar 15, 2019 at 10:06:16AM +0100, Enrico Weigelt, metux IT consult wrote:
> On 14.03.19 23:52, Greg KH wrote:
> > On Thu, Mar 14, 2019 at 11:33:40PM +0100, Enrico Weigelt, metux IT consult wrote:
> >> Use the safer devm versions of memory mapping functions.
> >
> > What is "safer" about them?
>
> Garbage collection :)
At times, but not when you do not use the api correctly :)
> Several drivers didn't seem to clean up properly (maybe these're just
> used compiled-in, so nobody noticed yet).
Yes, there are lots of drivers for devices that are never unloaded or
removed from the system. The fact that no one has reported any problems
with them means that they are never used in situations like this.
> In general, I think devm_* should be the standard case, unless there's
> a really good reason to do otherwise.
No, you need to have a good reason why it needs to be changed, when you
can not verify that this actually resolves a problem. As this patch
shows, you just replaced one api call with another, so nothing changed
at all, except you actually took up more memory and logic to do the same
thing :(
> > Isn't the whole goal of the devm* functions such that you are not
> > required to call "release" on them?
>
> Looks that one slipped through, when I was doing that big bulk change
> in the middle of the night and not enough coffe ;-)
>
> One problem here is that many drivers do this stuff in request/release
> port, instead of probe/remove. I'm not sure yet, whether we should
> rewrite that. There're also cases which do request/release, but no
> ioremap (doing hardcoded register accesses), others do ioremap w/o
> request/release.
>
> IMHO, we should have a closer look at those and check whether that's
> really okay (just adding request/release blindly could cause trouble)
I agree, please feel free to audit these to verify they are all correct.
But that's not what you did here, so that's not a valid justification
for these patches to be accepted.
> > And also, why make the change, you aren't changing any functionality for
> > these old drivers at all from what I can tell (for the devm calls).
> > What am I missing here?
>
> Okay, there's a bigger story behind, you can't know yet. Finally, I'd
> like to move everything to using struct resource and corresponding
> helpers consistently, so most of the drivers would be pretty simple
> at that point. (there're of course special cases, like devices w/
> multiple register spaces, etc)
>
> Here's my wip branch:
>
> https://github.com/metux/linux/commits/wip/serial-res
>
> In this consolidation process, I'm trying to move everything to
> devm_*, to have it more generic (for now, I still need two versions
> of the request/release/ioremap/iounmap helpers - one w/ and one
> w/o devm).
Move everything in what part of the kernel? The whole kernel or just
one subsystem?
> My idea was moving to devm first, so it can be reviewed/tested
> independently, before moving forward. Smaller, easily digestable
> pieces should minimize the risk of breaking anything. But if you
> prefer having this things squashed together, just let me know.
Small pieces are required, that's fine, but those pieces need to have a
justification for why they should be accepted at all points along the
way.
> In the queue are also other minor cleanups like using dev_err()
> instead of printk(), etc. Should I send these separately ?
Of course.
> By the way: do you have some public branch where you're collecting
> accepted patches, which I could base mine on ? (tty.git/tty-next ?)
Yes, that is the tree and branch, but remember that none of my trees can
open up until after -rc1 is out.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH serial v3] sc16is7xx: missing unregister/delete driver on error in sc16is7xx_init()
From: Greg KH @ 2019-03-15 14:19 UTC (permalink / raw)
To: maowenan
Cc: Dan Carpenter, jslaby, linux-serial, linux-kernel,
kernel-janitors, vz
In-Reply-To: <566a03ba-aae9-3169-5db3-3417d5eb60f9@huawei.com>
On Fri, Mar 15, 2019 at 09:56:07PM +0800, maowenan wrote:
> ping...
For what? It's the middle of the merge window at the moment, you should
have gotten an email from my scripts saying what happens at this point
in time, right?
greg k-h
^ permalink raw reply
* Re: [PATCH serial v3] sc16is7xx: missing unregister/delete driver on error in sc16is7xx_init()
From: maowenan @ 2019-03-15 13:56 UTC (permalink / raw)
To: Dan Carpenter
Cc: gregkh, jslaby, linux-serial, linux-kernel, kernel-janitors, vz
In-Reply-To: <20190311121918.GD2412@kadam>
ping...
On 2019/3/11 20:19, Dan Carpenter wrote:
> On Mon, Mar 11, 2019 at 05:39:59PM +0800, Mao Wenan wrote:
>> Add the missing uart_unregister_driver() and i2c_del_driver() before
>> return from sc16is7xx_init() in the error handling case.
>>
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>> Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
>
> This looks nice. Thanks!
>
> Reviewed-by: Dan Carpeter <dan.carpenter@oracle.com>
>
> regards,
> dan carpenter
>
>
>
^ permalink raw reply
* Re: serial driver cleanups v2
From: Enrico Weigelt, metux IT consult @ 2019-03-15 10:36 UTC (permalink / raw)
To: Andy Shevchenko, Enrico Weigelt, metux IT consult
Cc: Linux Kernel Mailing List, Greg Kroah-Hartman, Eric Anholt,
Stefan Wahren, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Andy Shevchenko, Vladimir Zapolskiy,
Matthias Brugger, Masahiro Yamada, Tobias Klauser, Richard Genoud,
macro, Uwe Kleine-König, Sascha Hauer, slemieux.tyco
In-Reply-To: <CAHp75VeH9isvFkp+AECgsN8du__8tBBp_4zZEVXJ083BdFweeg@mail.gmail.com>
On 15.03.19 10:12, Andy Shevchenko wrote:
>> Part II will be about moving the mmio range from mapbase and
>> mapsize (which are used quite inconsistently) to a struct resource
>> and using helpers for that. But this one isn't finished yet.
>> (if somebody likes to have a look at it, I can send it, too)
>
> Let's do that way you are preparing a branch somewhere and anounce
> here as an RFC, since this was neither tested nor correct.
Okay, here it is:
I. https://github.com/metux/linux/tree/submit/serial-clean-v3
--> general cleanups, as basis for II
II. https://github.com/metux/linux/tree/wip/serial-res
--> moving towards using struct resource consistently
III. https://github.com/metux/linux/tree/hack/serial
--> the final steps, which are yet completely broken
(more a notepad for things still to do :o)
The actual goal is generalizing the whole iomem handling, so individual
usually just need to call some helpers that do most of the things.
Finally, I also wanted to have all io region information consolidated
in struct resource.
Meanwhile I've learned that I probably was a bit too eager w/ that.
Guess I'll have to rethink my strategy.
> And selling point for many of them is not true: it doesn't make any
> difference in the size in code, but increases a time to run
> (devm_ioremap_resource() does more than plain devm_iomap() call).
Okay, just seen it. Does the the runtime overhead cause any problems ?
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287
^ permalink raw reply
* Re: [PATCH v2 02/45] drivers: tty: serial: 8250_dw: use devm_ioremap_resource()
From: Enrico Weigelt, metux IT consult @ 2019-03-15 9:37 UTC (permalink / raw)
To: Andy Shevchenko, Enrico Weigelt, metux IT consult
Cc: Linux Kernel Mailing List, Greg Kroah-Hartman, Eric Anholt,
Stefan Wahren, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Andy Shevchenko, Vladimir Zapolskiy,
Matthias Brugger, Masahiro Yamada, Tobias Klauser, Richard Genoud,
macro, Uwe Kleine-König, Sascha Hauer, slemieux.tyco
In-Reply-To: <CAHp75VegzKNhH1Uza9gwQMOAEOdh57WTzsHUx7vPdVs7XvJROQ@mail.gmail.com>
On 15.03.19 10:04, Andy Shevchenko wrote:
> On Fri, Mar 15, 2019 at 12:41 AM Enrico Weigelt, metux IT consult
> <info@metux.net> wrote:
>>
>> Instead of fetching out data from a struct resource for passing
>> it to devm_ioremap(), directly use devm_ioremap_resource()
>
> I don't see any advantage of this change.
> See also below.
I see that the whole story wasn't clear. Please see my reply to Greg,
hope that clears it up a little bit.
>> --- a/drivers/tty/serial/8250/8250_dw.c
>> +++ b/drivers/tty/serial/8250/8250_dw.c
>> @@ -526,7 +526,7 @@ static int dw8250_probe(struct platform_device *pdev)
>> p->set_ldisc = dw8250_set_ldisc;
>> p->set_termios = dw8250_set_termios;
>>
>> - p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
>> + p->membase = devm_ioremap_resource(dev, regs);
>> if (!p->membase)
>
> And how did you test this? devm_ioremap_resource() returns error
> pointer in case of error.
hmm, devm_ioremap_resource() does so, but devm_ioremap() does not ?
I really didn't expect that. Thanks for pointing that out.
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287
^ permalink raw reply
* [PATCH 2/2] tty/serial: atmel: RS485 HD w/DMA: enable RX after TX is stopped
From: Razvan Stefanescu @ 2019-03-15 9:23 UTC (permalink / raw)
To: Richard Genoud, Greg Kroah-Hartman, Jiri Slaby
Cc: Nicolas Ferre, Alexandre Belloni, Ludovic Desroches, linux-serial,
linux-arm-kernel, linux-kernel
In-Reply-To: <20190315092334.13246-1-razvan.stefanescu@microchip.com>
In half-duplex operation, RX should be started after TX completes.
If DMA is used, there is a case when the DMA transfer completes but the
TX FIFO is not emptied, so the RX cannot be restarted just yet.
Use a boolean variable to store this state and rearm TX interrupt mask
to be signaled again that the transfer finished. In interrupt transmit
handler this variable is used to start RX.
Signed-off-by: Razvan Stefanescu <razvan.stefanescu@microchip.com>
---
drivers/tty/serial/atmel_serial.c | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index a6577b1c4461..b0141dcbbb61 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -166,6 +166,8 @@ struct atmel_uart_port {
unsigned int pending_status;
spinlock_t lock_suspended;
+ bool hd_start_rx; /* can start RX during half-duplex operation */
+
/* ISO7816 */
unsigned int fidi_min;
unsigned int fidi_max;
@@ -934,8 +936,13 @@ static void atmel_complete_tx_dma(void *arg)
if (!uart_circ_empty(xmit))
atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
else if (atmel_uart_is_half_duplex(port)) {
- /* DMA done, stop TX, start RX for RS485 */
- atmel_start_rx(port);
+ /*
+ * DMA done, re-enable TXEMPTY and signal that we can stop
+ * TX and start RX for RS485
+ */
+ atmel_port->hd_start_rx = true;
+ atmel_uart_writel(port, ATMEL_US_IER,
+ atmel_port->tx_done_mask);
}
spin_unlock_irqrestore(&port->lock, flags);
@@ -1379,9 +1386,21 @@ atmel_handle_transmit(struct uart_port *port, unsigned int pending)
struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
if (pending & atmel_port->tx_done_mask) {
- /* Either PDC or interrupt transmission */
atmel_uart_writel(port, ATMEL_US_IDR,
atmel_port->tx_done_mask);
+
+ /* Start RX if flag was set and FIFO is empty */
+ if (atmel_port->hd_start_rx) {
+ if (atmel_uart_readl(port, ATMEL_US_CSR)
+ & ATMEL_US_TXEMPTY) {
+ atmel_port->hd_start_rx = false;
+ atmel_start_rx(port);
+ } else {
+ dev_warn(port->dev, "Should start RX, but TX fifo is not empty\n");
+ }
+ return;
+ }
+
atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
}
}
--
2.19.1
^ permalink raw reply related
* [PATCH 1/2] tty/serial: atmel: Add is_half_duplex helper
From: Razvan Stefanescu @ 2019-03-15 9:23 UTC (permalink / raw)
To: Richard Genoud, Greg Kroah-Hartman, Jiri Slaby
Cc: Nicolas Ferre, Alexandre Belloni, Ludovic Desroches, linux-serial,
linux-arm-kernel, linux-kernel
In-Reply-To: <20190315092334.13246-1-razvan.stefanescu@microchip.com>
Use a helper function to check that a port needs to use half duplex
communication, replacing several occurrences of multi-line bit checking.
Signed-off-by: Razvan Stefanescu <razvan.stefanescu@microchip.com>
---
drivers/tty/serial/atmel_serial.c | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 05147fe24343..a6577b1c4461 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -231,6 +231,13 @@ static inline void atmel_uart_write_char(struct uart_port *port, u8 value)
__raw_writeb(value, port->membase + ATMEL_US_THR);
}
+static inline int atmel_uart_is_half_duplex(struct uart_port *port)
+{
+ return ((port->rs485.flags & SER_RS485_ENABLED) &&
+ !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
+ (port->iso7816.flags & SER_ISO7816_ENABLED);
+}
+
#ifdef CONFIG_SERIAL_ATMEL_PDC
static bool atmel_use_pdc_rx(struct uart_port *port)
{
@@ -608,10 +615,10 @@ static void atmel_stop_tx(struct uart_port *port)
/* Disable interrupts */
atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
- if (((port->rs485.flags & SER_RS485_ENABLED) &&
- !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
- port->iso7816.flags & SER_ISO7816_ENABLED)
- atmel_start_rx(port);
+ if (atmel_uart_is_half_duplex(port))
+ if (!atomic_read(&atmel_port->tasklet_shutdown))
+ atmel_start_rx(port);
+
}
/*
@@ -628,9 +635,7 @@ static void atmel_start_tx(struct uart_port *port)
return;
if (atmel_use_pdc_tx(port) || atmel_use_dma_tx(port))
- if (((port->rs485.flags & SER_RS485_ENABLED) &&
- !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
- port->iso7816.flags & SER_ISO7816_ENABLED)
+ if (atmel_uart_is_half_duplex(port))
atmel_stop_rx(port);
if (atmel_use_pdc_tx(port))
@@ -928,9 +933,7 @@ static void atmel_complete_tx_dma(void *arg)
*/
if (!uart_circ_empty(xmit))
atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
- else if (((port->rs485.flags & SER_RS485_ENABLED) &&
- !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
- port->iso7816.flags & SER_ISO7816_ENABLED) {
+ else if (atmel_uart_is_half_duplex(port)) {
/* DMA done, stop TX, start RX for RS485 */
atmel_start_rx(port);
}
@@ -1508,9 +1511,7 @@ static void atmel_tx_pdc(struct uart_port *port)
atmel_uart_writel(port, ATMEL_US_IER,
atmel_port->tx_done_mask);
} else {
- if (((port->rs485.flags & SER_RS485_ENABLED) &&
- !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
- port->iso7816.flags & SER_ISO7816_ENABLED) {
+ if (atmel_uart_is_half_duplex(port)) {
/* DMA done, stop TX, start RX for RS485 */
atmel_start_rx(port);
}
--
2.19.1
^ permalink raw reply related
* [PATCH 0/2] tty/serial: atmel: Fix RS485 half duplex operation
From: Razvan Stefanescu @ 2019-03-15 9:23 UTC (permalink / raw)
To: Richard Genoud, Greg Kroah-Hartman, Jiri Slaby
Cc: Nicolas Ferre, Alexandre Belloni, Ludovic Desroches, linux-serial,
linux-arm-kernel, linux-kernel
Using a loopback serial cable with RS485 protocol shows that data is
received:
$ stty -F /dev/ttyS3 raw -echo speed 4800
$ cat /dev/ttyS3 &
$ echo "Hello, world" > /dev/ttyS3
Hello, world
Last line should not be displayed, as it indicates that RX was started
before TX finished.
This happens because driver activates RX when the DMA transfer
completes, but that does not necessarily mean the TX FIFO was emptied.
First patch will add a helper that checks if the transmission is
half-duplex and uses it throughout the driver, replacing multiple lines
of code.
Second patch implements the fix by adding a variable to the port struct.
This is used to indicate that RX needs to be started. When the DMA
transfer completes, the variable is set and the ATMEL_US_TXEMPTY is
reactivated. In the interrupt handler, if the variable is set, RX is
started.
Razvan Stefanescu (2):
tty/serial: atmel: Add is_half_duplex helper
tty/serial: atmel: RS485 HD w/DMA: enable RX after TX is stopped
drivers/tty/serial/atmel_serial.c | 52 +++++++++++++++++++++----------
1 file changed, 36 insertions(+), 16 deletions(-)
--
2.19.1
^ permalink raw reply
* Re: serial driver cleanups v2
From: Andy Shevchenko @ 2019-03-15 9:20 UTC (permalink / raw)
To: Enrico Weigelt, metux IT consult
Cc: Linux Kernel Mailing List, Greg Kroah-Hartman, Eric Anholt,
Stefan Wahren, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Andy Shevchenko, Vladimir Zapolskiy,
Matthias Brugger, Masahiro Yamada, Tobias Klauser, Richard Genoud,
macro, Uwe Kleine-König, Sascha Hauer, slemieux.tyco
In-Reply-To: <CAHp75VeH9isvFkp+AECgsN8du__8tBBp_4zZEVXJ083BdFweeg@mail.gmail.com>
On Fri, Mar 15, 2019 at 11:12 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, Mar 15, 2019 at 12:40 AM Enrico Weigelt, metux IT consult
> <info@metux.net> wrote:
>
> > here's v2 of my serial cleanups queue - part I:
> >
> > essentially using helpers to code more compact and switching to
> > devm_*() functions for mmio management.
> >
> > Part II will be about moving the mmio range from mapbase and
> > mapsize (which are used quite inconsistently) to a struct resource
> > and using helpers for that. But this one isn't finished yet.
> > (if somebody likes to have a look at it, I can send it, too)
>
> Let's do that way you are preparing a branch somewhere and anounce
> here as an RFC, since this was neither tested nor correct.
> And selling point for many of them is not true: it doesn't make any
> difference in the size in code, but increases a time to run
> (devm_ioremap_resource() does more than plain devm_iomap() call).
And one more thing, perhaps you can run existing and / or contribute
to coccinelle since this all scriptable and maintainers can decide if
this or that coccinelle script is useful.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: serial driver cleanups v2
From: Andy Shevchenko @ 2019-03-15 9:12 UTC (permalink / raw)
To: Enrico Weigelt, metux IT consult
Cc: Linux Kernel Mailing List, Greg Kroah-Hartman, Eric Anholt,
Stefan Wahren, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Andy Shevchenko, Vladimir Zapolskiy,
Matthias Brugger, Masahiro Yamada, Tobias Klauser, Richard Genoud,
macro, Uwe Kleine-König, Sascha Hauer, slemieux.tyco
In-Reply-To: <1552602855-26086-1-git-send-email-info@metux.net>
On Fri, Mar 15, 2019 at 12:40 AM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:
> here's v2 of my serial cleanups queue - part I:
>
> essentially using helpers to code more compact and switching to
> devm_*() functions for mmio management.
>
> Part II will be about moving the mmio range from mapbase and
> mapsize (which are used quite inconsistently) to a struct resource
> and using helpers for that. But this one isn't finished yet.
> (if somebody likes to have a look at it, I can send it, too)
Let's do that way you are preparing a branch somewhere and anounce
here as an RFC, since this was neither tested nor correct.
And selling point for many of them is not true: it doesn't make any
difference in the size in code, but increases a time to run
(devm_ioremap_resource() does more than plain devm_iomap() call).
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 45/45] drivers: tty: serial: mux: use devm_* functions
From: Andy Shevchenko @ 2019-03-15 9:08 UTC (permalink / raw)
To: Enrico Weigelt, metux IT consult
Cc: Linux Kernel Mailing List, Greg Kroah-Hartman, Eric Anholt,
Stefan Wahren, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Andy Shevchenko, Vladimir Zapolskiy,
Matthias Brugger, Masahiro Yamada, Tobias Klauser, Richard Genoud,
macro, Uwe Kleine-König, Sascha Hauer, slemieux.tyco
In-Reply-To: <1552602855-26086-46-git-send-email-info@metux.net>
On Fri, Mar 15, 2019 at 12:37 AM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:
>
> Use the safer devm versions of memory mapping functions.
If you are going to use devm_*_free(), what's the point to have this
change from the beginning?
P.S. Disregard that this is untested series...
> --- a/drivers/tty/serial/mux.c
> +++ b/drivers/tty/serial/mux.c
> @@ -456,8 +456,9 @@ static int __init mux_probe(struct parisc_device *dev)
> printk(KERN_INFO "Serial mux driver (%d ports) Revision: 0.6\n", port_count);
>
> dev_set_drvdata(&dev->dev, (void *)(long)port_count);
> - request_mem_region(dev->hpa.start + MUX_OFFSET,
> - port_count * MUX_LINE_OFFSET, "Mux");
> + devm_request_mem_region(&dev->dev,
> + dev->hpa.start + MUX_OFFSET,
> + port_count * MUX_LINE_OFFSET, "Mux");
...and on top of this where is error checking?
>
> if(!port_cnt) {
> mux_driver.cons = MUX_CONSOLE;
> @@ -474,7 +475,9 @@ static int __init mux_probe(struct parisc_device *dev)
> port->iobase = 0;
> port->mapbase = dev->hpa.start + MUX_OFFSET +
> (i * MUX_LINE_OFFSET);
> - port->membase = ioremap_nocache(port->mapbase, MUX_LINE_OFFSET);
> + port->membase = devm_ioremap_nocache(port->dev,
> + port->mapbase,
> + MUX_LINE_OFFSET);
> port->iotype = UPIO_MEM;
> port->type = PORT_MUX;
> port->irq = 0;
> @@ -517,10 +520,12 @@ static int __exit mux_remove(struct parisc_device *dev)
>
> uart_remove_one_port(&mux_driver, port);
> if(port->membase)
> - iounmap(port->membase);
> + devm_iounmap(port->dev, port->membase);
> }
>
> - release_mem_region(dev->hpa.start + MUX_OFFSET, port_count * MUX_LINE_OFFSET);
> + devm_release_mem_region(&dev->dev,
> + dev->hpa.start + MUX_OFFSET,
> + port_count * MUX_LINE_OFFSET);
> return 0;
> }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox