Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH v2 2/2] arm64: dts: mediatek: add mt6765 support
From: Mars Cheng @ 2018-06-26  2:04 UTC (permalink / raw)
  To: Matthias Brugger, Rob Herring, Marc Zyngier
  Cc: CC Hwang, Loda Chou, Miles Chen, Jades Shih, Yingjoe Chen,
	My Chuang, linux-kernel, linux-mediatek, devicetree, wsd_upstream,
	linux-serial, linux-arm-kernel, Mars Cheng
In-Reply-To: <1529978646-28976-1-git-send-email-mars.cheng@mediatek.com>

This adds basic chip support for MT6765 SoC.

Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/Makefile       |    1 +
 arch/arm64/boot/dts/mediatek/mt6765-evb.dts |   33 ++++++
 arch/arm64/boot/dts/mediatek/mt6765.dtsi    |  158 +++++++++++++++++++++++++++
 3 files changed, 192 insertions(+)
 create mode 100644 arch/arm64/boot/dts/mediatek/mt6765-evb.dts
 create mode 100644 arch/arm64/boot/dts/mediatek/mt6765.dtsi

diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
index ac17f60..7506b0d 100644
--- a/arch/arm64/boot/dts/mediatek/Makefile
+++ b/arch/arm64/boot/dts/mediatek/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt2712-evb.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt6755-evb.dtb
+dtb-$(CONFIG_ARCH_MEDIATEK) += mt6765-evb.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-evb.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
diff --git a/arch/arm64/boot/dts/mediatek/mt6765-evb.dts b/arch/arm64/boot/dts/mediatek/mt6765-evb.dts
new file mode 100644
index 0000000..36dddff2
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt6765-evb.dts
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * dts file for Mediatek MT6765
+ *
+ * (C) Copyright 2018. Mediatek, Inc.
+ *
+ * Mars Cheng <mars.cheng@mediatek.com>
+ */
+
+/dts-v1/;
+#include "mt6765.dtsi"
+
+/ {
+	model = "MediaTek MT6765 EVB";
+	compatible = "mediatek,mt6765-evb", "mediatek,mt6765";
+
+	aliases {
+		serial0 = &uart0;
+	};
+
+	memory@40000000 {
+		device_type = "memory";
+		reg = <0 0x40000000 0 0x1e800000>;
+	};
+
+	chosen {
+		stdout-path = "serial0:921600n8";
+	};
+};
+
+&uart0 {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/mediatek/mt6765.dtsi b/arch/arm64/boot/dts/mediatek/mt6765.dtsi
new file mode 100644
index 0000000..ab34c0f
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt6765.dtsi
@@ -0,0 +1,158 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * dts file for Mediatek MT6765
+ *
+ * (C) Copyright 2018. Mediatek, Inc.
+ *
+ * Mars Cheng <mars.cheng@mediatek.com>
+ */
+
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/ {
+	compatible = "mediatek,mt6765";
+	interrupt-parent = <&sysirq>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	psci {
+		compatible = "arm,psci-0.2";
+		method = "smc";
+	};
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			enable-method = "psci";
+			reg = <0x000>;
+		};
+
+		cpu1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			enable-method = "psci";
+			reg = <0x001>;
+		};
+
+		cpu2: cpu@2 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			enable-method = "psci";
+			reg = <0x002>;
+		};
+
+		cpu3: cpu@3 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			enable-method = "psci";
+			reg = <0x003>;
+		};
+
+		cpu4: cpu@100 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			enable-method = "psci";
+			reg = <0x100>;
+		};
+
+		cpu5: cpu@101 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			enable-method = "psci";
+			reg = <0x101>;
+		};
+
+		cpu6: cpu@102 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			enable-method = "psci";
+			reg = <0x102>;
+		};
+
+		cpu7: cpu@103 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			enable-method = "psci";
+			reg = <0x103>;
+		};
+	};
+
+	baud_clk: dummy26m {
+		compatible = "fixed-clock";
+		clock-frequency = <26000000>;
+		#clock-cells = <0>;
+	};
+
+	sys_clk: dummyclk {
+		compatible = "fixed-clock";
+		clock-frequency = <26000000>;
+		#clock-cells = <0>;
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupt-parent = <&gic>;
+		interrupts = <GIC_PPI 13
+			     (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 14
+			     (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 11
+			     (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 10
+			     (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>;
+	};
+
+	soc {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		compatible = "simple-bus";
+		ranges;
+
+		sysirq: intpol-controller@10200a80 {
+			compatible = "mediatek,mt6765-sysirq",
+				     "mediatek,mt6577-sysirq";
+			interrupt-controller;
+			#interrupt-cells = <3>;
+			interrupt-parent = <&gic>;
+			reg = <0 0x10200a80 0 0x50>;
+		};
+
+		gic: interrupt-controller@0c000000 {
+			compatible = "arm,gic-v3";
+			#interrupt-cells = <3>;
+			#address-cells = <2>;
+			#size-cells = <2>;
+			#redistributor-regions = <1>;
+			interrupt-parent = <&gic>;
+			interrupt-controller;
+			reg = <0 0x0c000000 0 0x40000>, // distributor
+			      <0 0x0c100000 0 0x200000>; // redistributor
+			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
+		};
+
+		uart0: serial@11002000 {
+			compatible = "mediatek,mt6765-uart",
+				     "mediatek,mt6577-uart";
+			reg = <0 0x11002000 0 0x400>;
+			interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&baud_clk>, <&sys_clk>;
+			clock-names = "baud", "bus";
+			status = "disabled";
+		};
+
+		uart1: serial@11003000 {
+			compatible = "mediatek,mt6765-uart",
+				     "mediatek,mt6577-uart";
+			reg = <0 0x11003000 0 0x400>;
+			interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&baud_clk>, <&sys_clk>;
+			clock-names = "baud", "bus";
+			status = "disabled";
+		};
+	}; /* end of soc */
+};
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH v2 2/2] arm64: dts: mediatek: add mt6765 support
From: Marc Zyngier @ 2018-06-26  7:53 UTC (permalink / raw)
  To: Mars Cheng
  Cc: Matthias Brugger, Rob Herring, CC Hwang, Loda Chou, Miles Chen,
	Jades Shih, Yingjoe Chen, My Chuang, linux-kernel, linux-mediatek,
	devicetree, wsd_upstream, linux-serial, linux-arm-kernel
In-Reply-To: <1529978646-28976-3-git-send-email-mars.cheng@mediatek.com>

On Tue, 26 Jun 2018 03:04:06 +0100,
Mars Cheng <mars.cheng@mediatek.com> wrote:
> 
> This adds basic chip support for MT6765 SoC.
> 
> Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/Makefile       |    1 +
>  arch/arm64/boot/dts/mediatek/mt6765-evb.dts |   33 ++++++
>  arch/arm64/boot/dts/mediatek/mt6765.dtsi    |  158 +++++++++++++++++++++++++++
>  3 files changed, 192 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/mediatek/mt6765-evb.dts
>  create mode 100644 arch/arm64/boot/dts/mediatek/mt6765.dtsi
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt6765.dtsi b/arch/arm64/boot/dts/mediatek/mt6765.dtsi
> new file mode 100644
> index 0000000..ab34c0f
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt6765.dtsi

[...]

> +	timer {
> +		compatible = "arm,armv8-timer";
> +		interrupt-parent = <&gic>;
> +		interrupts = <GIC_PPI 13
> +			     (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 14
> +			     (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 11
> +			     (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 10
> +			     (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>;

GICv3 doesn't encode the PPI affinity in its interrupt specifiers (or
at least not this way). Please drop it.

> +	};
> +
> +	soc {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		compatible = "simple-bus";
> +		ranges;
> +
> +		sysirq: intpol-controller@10200a80 {
> +			compatible = "mediatek,mt6765-sysirq",
> +				     "mediatek,mt6577-sysirq";
> +			interrupt-controller;
> +			#interrupt-cells = <3>;
> +			interrupt-parent = <&gic>;
> +			reg = <0 0x10200a80 0 0x50>;
> +		};
> +
> +		gic: interrupt-controller@0c000000 {
> +			compatible = "arm,gic-v3";
> +			#interrupt-cells = <3>;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			#redistributor-regions = <1>;

A single redistributor is the default, and you don't need to specify
it in the DT.

> +			interrupt-parent = <&gic>;
> +			interrupt-controller;
> +			reg = <0 0x0c000000 0 0x40000>, // distributor
> +			      <0 0x0c100000 0 0x200000>; // redistributor

How about the GICv2 compatibility regions, which are provided by the
CPUs at a fixed offset from PERIPHBASE? See the Cortex-A53 TRM for
detail, and please add the missing regions.

> +			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> +		};

Thanks,

	M.

-- 
Jazz is not dead, it just smell funny.

^ permalink raw reply

* Re: [PATCH v3 23/27] devicetree: fix name of pinctrl-bindings.txt
From: Linus Walleij @ 2018-06-26  9:33 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linux-doc, Mauro Carvalho Chehab, linux-kernel@vger.kernel.org,
	Jonathan Corbet, Rob Herring, Mark Rutland, Lee Jones,
	Ulf Hansson, Greg KH, Mark Brown,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-mmc, open list:GPIO SUBSYSTEM, linux-serial, linux-spi
In-Reply-To: <d04d47f9fba035cf9de8589a288964c636324d58.1528990947.git.mchehab+samsung@kernel.org>

On Thu, Jun 14, 2018 at 6:09 PM Mauro Carvalho Chehab
<mchehab+samsung@kernel.org> wrote:

> Rename:
>         pinctrl-binding.txt -> pinctrl-bindings.txt
>
> In order to match the current name of this file.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Is this applied in some other tree or something I should
apply to the pinctrl tree?

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH v2 2/2] arm64: dts: mediatek: add mt6765 support
From: Mars Cheng @ 2018-06-26 11:23 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Matthias Brugger, Rob Herring, CC Hwang, Loda Chou, Miles Chen,
	Jades Shih, Yingjoe Chen, My Chuang, linux-kernel, linux-mediatek,
	devicetree, wsd_upstream, linux-serial, linux-arm-kernel
In-Reply-To: <86h8lq6l1r.wl-marc.zyngier@arm.com>

Hi Marc

On Tue, 2018-06-26 at 08:53 +0100, Marc Zyngier wrote:
> On Tue, 26 Jun 2018 03:04:06 +0100,
> Mars Cheng <mars.cheng@mediatek.com> wrote:
> > 
> > This adds basic chip support for MT6765 SoC.
> > 
> > Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> > ---
> >  arch/arm64/boot/dts/mediatek/Makefile       |    1 +
> >  arch/arm64/boot/dts/mediatek/mt6765-evb.dts |   33 ++++++
> >  arch/arm64/boot/dts/mediatek/mt6765.dtsi    |  158 +++++++++++++++++++++++++++
> >  3 files changed, 192 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/mediatek/mt6765-evb.dts
> >  create mode 100644 arch/arm64/boot/dts/mediatek/mt6765.dtsi
> > 
> > diff --git a/arch/arm64/boot/dts/mediatek/mt6765.dtsi b/arch/arm64/boot/dts/mediatek/mt6765.dtsi
> > new file mode 100644
> > index 0000000..ab34c0f
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/mediatek/mt6765.dtsi
> 
> [...]
> 
> > +	timer {
> > +		compatible = "arm,armv8-timer";
> > +		interrupt-parent = <&gic>;
> > +		interrupts = <GIC_PPI 13
> > +			     (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> > +			     <GIC_PPI 14
> > +			     (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> > +			     <GIC_PPI 11
> > +			     (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> > +			     <GIC_PPI 10
> > +			     (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>;
> 
> GICv3 doesn't encode the PPI affinity in its interrupt specifiers (or
> at least not this way). Please drop it.

Got it, will fix it.

> 
> > +	};
> > +
> > +	soc {
> > +		#address-cells = <2>;
> > +		#size-cells = <2>;
> > +		compatible = "simple-bus";
> > +		ranges;
> > +
> > +		sysirq: intpol-controller@10200a80 {
> > +			compatible = "mediatek,mt6765-sysirq",
> > +				     "mediatek,mt6577-sysirq";
> > +			interrupt-controller;
> > +			#interrupt-cells = <3>;
> > +			interrupt-parent = <&gic>;
> > +			reg = <0 0x10200a80 0 0x50>;
> > +		};
> > +
> > +		gic: interrupt-controller@0c000000 {
> > +			compatible = "arm,gic-v3";
> > +			#interrupt-cells = <3>;
> > +			#address-cells = <2>;
> > +			#size-cells = <2>;
> > +			#redistributor-regions = <1>;
> 
> A single redistributor is the default, and you don't need to specify
> it in the DT.
> 

sure, it's really unnecessary. will remove it.

> > +			interrupt-parent = <&gic>;
> > +			interrupt-controller;
> > +			reg = <0 0x0c000000 0 0x40000>, // distributor
> > +			      <0 0x0c100000 0 0x200000>; // redistributor
> 
> How about the GICv2 compatibility regions, which are provided by the
> CPUs at a fixed offset from PERIPHBASE? See the Cortex-A53 TRM for
> detail, and please add the missing regions.
> 

Thanks. will add it soon.

> > +			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> > +		};
> 
> Thanks,
> 
> 	M.
> 

^ permalink raw reply

* Re: [PATCH v5 0/7] add virt-dma support for imx-sdma
From: Lucas Stach @ 2018-06-26 15:04 UTC (permalink / raw)
  To: Robin Gong, vkoul, s.hauer, dan.j.williams, gregkh, jslaby
  Cc: linux-serial, dmaengine, linux-kernel, linux-arm-kernel,
	linux-imx
In-Reply-To: <1529427424-12321-1-git-send-email-yibin.gong@nxp.com>

Hi Robin,

I've tested this whole series with the SDMA being used for SPI, UART
and SSI with no regressions spotted. As this should cover most common
use-cases, I think this series is good to go in.

Tested-by: Lucas Stach <l.stach@pengutronix.de>

Regards,
Lucas

Am Mittwoch, den 20.06.2018, 00:56 +0800 schrieb Robin Gong:
> The legacy sdma driver has below limitations or drawbacks:
>   1. Hardcode the max BDs number as "PAGE_SIZE / sizeof(*)", and alloc
>      one page size for one channel regardless of only few BDs needed
>      most time. But in few cases, the max PAGE_SIZE maybe not enough.
>   2. One SDMA channel can't stop immediatley once channel disabled which
>      means SDMA interrupt may come in after this channel terminated.There
>      are some patches for this corner case such as commit "2746e2c389f9",
>      but not cover non-cyclic.
> 
> The common virt-dma overcomes the above limitations. It can alloc bd
> dynamically and free bd once this tx transfer done. No memory wasted or
> maximum limititation here, only depends on how many memory can be requested
> from kernel. For No.2, such issue can be workaround by checking if there
> is available descript("sdmac->desc") now once the unwanted interrupt
> coming. At last the common virt-dma is easier for sdma driver maintain.
> 
> Change from v4:
>   1. identify lockdep issue which caused by allocate memory with
>      'GFP_KERNEL', change to 'GFP_NOWAIT' instead so that lockdep
>      ignore check. That also make sense since Audio/uart driver may
>      call dma function after spin_lock_irqsave()...
>   2. use dma pool instead for bd description allocated,since audio
>      driver may call dma_terminate_all in irq. Please refer to 7/7.
>   3. remove 7/7 serial patch in v4, since lockdep issued fixed by No.1 
> 
> Change from v3:
>   1. add two uart patches which impacted by this patchset.
>   2. unlock 'vc.lock' before cyclic dma callback and lock again after
>      it because some driver such as uart will call dmaengine_tx_status
>      which will acquire 'vc.lock' again and dead lock comes out.
>   3. remove 'Revert commit' stuff since that patch is not wrong and
>      combine two patch into one patch as Sascha's comment.
> 
> Change from v2:
>   1. include Sascha's patch to make the main patch easier to review.
>      Thanks Sacha.
>   2. remove useless 'desc'/'chan' in struct sdma_channe.
> 
> Change from v1:
>   1. split v1 patch into 5 patches.
>   2. remove some unnecessary condition check.
>   3. remove unnecessary 'pending' list.
> 
> Robin Gong (6):
>   tty: serial: imx: correct dma cookie status
>   dmaengine: imx-sdma: add virt-dma support
>   dmaengine: imx-sdma: remove useless 'lock' and 'enabled' in 'struct
>     sdma_channel'
>   dmaengine: imx-sdma: remove the maximum limitation for bd numbers
>   dmaengine: imx-sdma: add sdma_transfer_init to decrease code overlap
>   dmaengine: imx-sdma: alloclate bd memory from dma pool
> 
> Sascha Hauer (1):
>   dmaengine: imx-sdma: factor out a struct sdma_desc from struct
>     sdma_channel
> 
>  drivers/dma/Kconfig      |   1 +
>  drivers/dma/imx-sdma.c   | 400 +++++++++++++++++++++++++++--------------------
>  drivers/tty/serial/imx.c |   2 +-
>  3 files changed, 235 insertions(+), 168 deletions(-)
> 

^ permalink raw reply

* Re: [PATCH v5 1/7] tty: serial: imx: correct dma cookie status
From: Uwe Kleine-König @ 2018-06-26 19:22 UTC (permalink / raw)
  To: Robin Gong
  Cc: vkoul, s.hauer, l.stach, dan.j.williams, gregkh, jslaby,
	dmaengine, linux-imx, linux-kernel, linux-serial,
	linux-arm-kernel
In-Reply-To: <1529427424-12321-2-git-send-email-yibin.gong@nxp.com>

On Wed, Jun 20, 2018 at 12:56:58AM +0800, Robin Gong wrote:
> Correct to check the right rx dma cookie status in spit of it
> works because only one cookie is running in the current sdma.
> But it will not once sdma driver support multi cookies
> running based on virt-dma.
> 
> Signed-off-by: Robin Gong <yibin.gong@nxp.com>
Looks wrong (because of tx_status vs rx_cookie), but is right
nevertheless I think:

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks
Uwe

> ---
>  drivers/tty/serial/imx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 4e85357..2879407 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -1051,7 +1051,7 @@ static void imx_uart_dma_rx_callback(void *data)
>  	unsigned int r_bytes;
>  	unsigned int bd_size;
>  
> -	status = dmaengine_tx_status(chan, (dma_cookie_t)0, &state);
> +	status = dmaengine_tx_status(chan, sport->rx_cookie, &state);
>  
>  	if (status == DMA_ERROR) {
>  		imx_uart_clear_rx_errors(sport);
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

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

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: serial: 8250_omap: Add compatible for AM654 UART controller
From: Rob Herring @ 2018-06-26 21:12 UTC (permalink / raw)
  To: Nishanth Menon
  Cc: Mark Rutland, Greg Kroah-Hartman, linux-kernel, devicetree,
	linux-serial, Tony Lindgren, Tero Kristo, Vignesh R, Jiri Slaby,
	Sekhar Nori
In-Reply-To: <20180619194450.6353-2-nm@ti.com>

On Tue, Jun 19, 2018 at 02:44:49PM -0500, Nishanth Menon wrote:
> AM654 uses a UART controller that is only partially compatible with
> existing 8250 UART. UART DMA integration is substantially different
> and even a match against standard 8250 or omap4 would result in
> non-working UART once DMA is enabled by default.
> 
> Introduce a specific compatible to help build up the differences in
> follow on patches.
> 
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Vignesh R <vigneshr@ti.com>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---

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

^ permalink raw reply

* RE: [PATCH v5 0/7] add virt-dma support for imx-sdma
From: Robin Gong @ 2018-06-27  1:18 UTC (permalink / raw)
  To: Lucas Stach, vkoul@kernel.org, s.hauer@pengutronix.de,
	dan.j.williams@intel.com, gregkh@linuxfoundation.org,
	jslaby@suse.com
  Cc: linux-serial@vger.kernel.org, dmaengine@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, dl-linux-imx
In-Reply-To: <1530025442.9910.44.camel@pengutronix.de>

Thanks Lucas. Let's wait for comments from Vinod.

-----Original Message-----
From: Lucas Stach [mailto:l.stach@pengutronix.de] 
Sent: 2018年6月26日 23:04
To: Robin Gong <yibin.gong@nxp.com>; vkoul@kernel.org; s.hauer@pengutronix.de; dan.j.williams@intel.com; gregkh@linuxfoundation.org; jslaby@suse.com
Cc: linux-serial@vger.kernel.org; dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>
Subject: Re: [PATCH v5 0/7] add virt-dma support for imx-sdma

Hi Robin,

I've tested this whole series with the SDMA being used for SPI, UART and SSI with no regressions spotted. As this should cover most common use-cases, I think this series is good to go in.

Tested-by: Lucas Stach <l.stach@pengutronix.de>

Regards,
Lucas

Am Mittwoch, den 20.06.2018, 00:56 +0800 schrieb Robin Gong:
> The legacy sdma driver has below limitations or drawbacks:
>   1. Hardcode the max BDs number as "PAGE_SIZE / sizeof(*)", and alloc
>      one page size for one channel regardless of only few BDs needed
>      most time. But in few cases, the max PAGE_SIZE maybe not enough.
>   2. One SDMA channel can't stop immediatley once channel disabled 
> which
>      means SDMA interrupt may come in after this channel 
> terminated.There
>      are some patches for this corner case such as commit 
> "2746e2c389f9",
>      but not cover non-cyclic.
> 
> The common virt-dma overcomes the above limitations. It can alloc bd 
> dynamically and free bd once this tx transfer done. No memory wasted 
> or maximum limititation here, only depends on how many memory can be 
> requested from kernel. For No.2, such issue can be workaround by 
> checking if there is available descript("sdmac->desc") now once the 
> unwanted interrupt coming. At last the common virt-dma is easier for sdma driver maintain.
> 
> Change from v4:
>   1. identify lockdep issue which caused by allocate memory with
>      'GFP_KERNEL', change to 'GFP_NOWAIT' instead so that lockdep
>      ignore check. That also make sense since Audio/uart driver may
>      call dma function after spin_lock_irqsave()...
>   2. use dma pool instead for bd description allocated,since audio
>      driver may call dma_terminate_all in irq. Please refer to 7/7.
>   3. remove 7/7 serial patch in v4, since lockdep issued fixed by No.1
> 
> Change from v3:
>   1. add two uart patches which impacted by this patchset.
>   2. unlock 'vc.lock' before cyclic dma callback and lock again after
>      it because some driver such as uart will call dmaengine_tx_status
>      which will acquire 'vc.lock' again and dead lock comes out.
>   3. remove 'Revert commit' stuff since that patch is not wrong and
>      combine two patch into one patch as Sascha's comment.
> 
> Change from v2:
>   1. include Sascha's patch to make the main patch easier to review.
>      Thanks Sacha.
>   2. remove useless 'desc'/'chan' in struct sdma_channe.
> 
> Change from v1:
>   1. split v1 patch into 5 patches.
>   2. remove some unnecessary condition check.
>   3. remove unnecessary 'pending' list.
> 
> Robin Gong (6):
>   tty: serial: imx: correct dma cookie status
>   dmaengine: imx-sdma: add virt-dma support
>   dmaengine: imx-sdma: remove useless 'lock' and 'enabled' in 'struct
>     sdma_channel'
>   dmaengine: imx-sdma: remove the maximum limitation for bd numbers
>   dmaengine: imx-sdma: add sdma_transfer_init to decrease code overlap
>   dmaengine: imx-sdma: alloclate bd memory from dma pool
> 
> Sascha Hauer (1):
>   dmaengine: imx-sdma: factor out a struct sdma_desc from struct
>     sdma_channel
> 
>  drivers/dma/Kconfig      |   1 +
>  drivers/dma/imx-sdma.c   | 400 
> +++++++++++++++++++++++++++--------------------
>  drivers/tty/serial/imx.c |   2 +-
>  3 files changed, 235 insertions(+), 168 deletions(-)
> 

^ permalink raw reply

* [PATCH v4 2/7] serdev: add dev_pm_domain_attach|detach()
From: sean.wang @ 2018-06-27  5:43 UTC (permalink / raw)
  To: robh+dt, mark.rutland, marcel, johan.hedberg
  Cc: devicetree, linux-bluetooth, linux-arm-kernel, linux-mediatek,
	linux-kernel, Sean Wang, Rob Herring, Ulf Hansson,
	Greg Kroah-Hartman, Jiri Slaby, linux-serial
In-Reply-To: <cover.1530004712.git.sean.wang@mediatek.com>

From: Sean Wang <sean.wang@mediatek.com>

In order to open up the required power gate before any operation can be
effectively performed over the serial bus between CPU and serdev, it's
clearly essential to add common attach functions for PM domains to serdev
at the probe phase.

Similarly, the relevant dettach function for the PM domains should be
properly and reversely added at the remove phase.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: linux-serial@vger.kernel.org
---
 drivers/tty/serdev/core.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index df93b72..8096138 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/pm_domain.h>
 #include <linux/serdev.h>
 #include <linux/slab.h>
 
@@ -330,8 +331,17 @@ EXPORT_SYMBOL_GPL(serdev_device_set_tiocm);
 static int serdev_drv_probe(struct device *dev)
 {
 	const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
+	int ret;
 
-	return sdrv->probe(to_serdev_device(dev));
+	ret = dev_pm_domain_attach(dev, true);
+	if (ret)
+		return ret;
+
+	ret = sdrv->probe(to_serdev_device(dev));
+	if (ret)
+		dev_pm_domain_detach(dev, true);
+
+	return ret;
 }
 
 static int serdev_drv_remove(struct device *dev)
@@ -339,6 +349,9 @@ static int serdev_drv_remove(struct device *dev)
 	const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
 	if (sdrv->remove)
 		sdrv->remove(to_serdev_device(dev));
+
+	dev_pm_domain_detach(dev, true);
+
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH v4 2/7] serdev: add dev_pm_domain_attach|detach()
From: Ulf Hansson @ 2018-06-27  8:00 UTC (permalink / raw)
  To: sean.wang
  Cc: Rob Herring, Mark Rutland, Marcel Holtmann, Johan Hedberg,
	devicetree, linux-bluetooth, Linux ARM, linux-mediatek,
	Linux Kernel Mailing List, Rob Herring, Greg Kroah-Hartman,
	Jiri Slaby, linux-serial
In-Reply-To: <ed497b2a9751df3b2f84cc9db6d6834816a6027f.1530004712.git.sean.wang@mediatek.com>

On 27 June 2018 at 07:43,  <sean.wang@mediatek.com> wrote:
> From: Sean Wang <sean.wang@mediatek.com>
>
> In order to open up the required power gate before any operation can be
> effectively performed over the serial bus between CPU and serdev, it's
> clearly essential to add common attach functions for PM domains to serdev
> at the probe phase.
>
> Similarly, the relevant dettach function for the PM domains should be
> properly and reversely added at the remove phase.
>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jslaby@suse.com>
> Cc: linux-serial@vger.kernel.org

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

> ---
>  drivers/tty/serdev/core.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index df93b72..8096138 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -13,6 +13,7 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/pm_domain.h>
>  #include <linux/serdev.h>
>  #include <linux/slab.h>
>
> @@ -330,8 +331,17 @@ EXPORT_SYMBOL_GPL(serdev_device_set_tiocm);
>  static int serdev_drv_probe(struct device *dev)
>  {
>         const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
> +       int ret;
>
> -       return sdrv->probe(to_serdev_device(dev));
> +       ret = dev_pm_domain_attach(dev, true);
> +       if (ret)
> +               return ret;
> +
> +       ret = sdrv->probe(to_serdev_device(dev));
> +       if (ret)
> +               dev_pm_domain_detach(dev, true);
> +
> +       return ret;
>  }
>
>  static int serdev_drv_remove(struct device *dev)
> @@ -339,6 +349,9 @@ static int serdev_drv_remove(struct device *dev)
>         const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
>         if (sdrv->remove)
>                 sdrv->remove(to_serdev_device(dev));
> +
> +       dev_pm_domain_detach(dev, true);
> +
>         return 0;
>  }
>
> --
> 2.7.4
>

^ permalink raw reply

* Re: [RFC PATCH v2 1/6] serial: uartps: Do not initialize field to zero again
From: Greg Kroah-Hartman @ 2018-06-27 10:09 UTC (permalink / raw)
  To: Michal Simek
  Cc: Michal Simek, linux-kernel, gnomes, Alexander Graf, shubhraj,
	robh, Jiri Slaby, linux-serial, linux-arm-kernel
In-Reply-To: <e7b01bd9-5763-75c0-1384-2536313f5854@monstr.eu>

On Tue, Jun 19, 2018 at 10:09:05AM +0200, Michal Simek wrote:
> On 6.6.2018 14:41, Michal Simek wrote:
> > Writing zero and NULLs to already initialized fields is not needed.
> > Remove this additional writes.
> > 
> > Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> > ---
> > 
> > Changes in v2:
> > - new patch - it can be sent separately too
> > 
> >  drivers/tty/serial/xilinx_uartps.c | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
> > index 8a3e34234e98..5f116f3ecd4a 100644
> > --- a/drivers/tty/serial/xilinx_uartps.c
> > +++ b/drivers/tty/serial/xilinx_uartps.c
> > @@ -1510,15 +1510,12 @@ static int cdns_uart_probe(struct platform_device *pdev)
> >  
> >  	/* At this point, we've got an empty uart_port struct, initialize it */
> >  	spin_lock_init(&port->lock);
> > -	port->membase	= NULL;
> > -	port->irq	= 0;
> >  	port->type	= PORT_UNKNOWN;
> >  	port->iotype	= UPIO_MEM32;
> >  	port->flags	= UPF_BOOT_AUTOCONF;
> >  	port->ops	= &cdns_uart_ops;
> >  	port->fifosize	= CDNS_UART_FIFO_SIZE;
> >  	port->line	= id;
> > -	port->dev	= NULL;
> >  
> >  	/*
> >  	 * Register the port.
> > 
> 
> Alan, Rob, Greg: Any comment about this RFC?

I rarely review RFC patchesets as obviously you don't think it is good
enough to be submitted "for real" :)

If you think this is all good, great, please resend it without the RFC
and it will end up in my queue.

thanks,

greg k-h

^ permalink raw reply

* Re: [RFC PATCH v2 1/6] serial: uartps: Do not initialize field to zero again
From: Michal Simek @ 2018-06-27 14:19 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Michal Simek
  Cc: Michal Simek, linux-kernel, gnomes, Alexander Graf, shubhraj,
	robh, Jiri Slaby, linux-serial, linux-arm-kernel
In-Reply-To: <20180627100917.GA20940@kroah.com>

On 27.6.2018 12:09, Greg Kroah-Hartman wrote:
> On Tue, Jun 19, 2018 at 10:09:05AM +0200, Michal Simek wrote:
>> On 6.6.2018 14:41, Michal Simek wrote:
>>> Writing zero and NULLs to already initialized fields is not needed.
>>> Remove this additional writes.
>>>
>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>> ---
>>>
>>> Changes in v2:
>>> - new patch - it can be sent separately too
>>>
>>>  drivers/tty/serial/xilinx_uartps.c | 3 ---
>>>  1 file changed, 3 deletions(-)
>>>
>>> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
>>> index 8a3e34234e98..5f116f3ecd4a 100644
>>> --- a/drivers/tty/serial/xilinx_uartps.c
>>> +++ b/drivers/tty/serial/xilinx_uartps.c
>>> @@ -1510,15 +1510,12 @@ static int cdns_uart_probe(struct platform_device *pdev)
>>>  
>>>  	/* At this point, we've got an empty uart_port struct, initialize it */
>>>  	spin_lock_init(&port->lock);
>>> -	port->membase	= NULL;
>>> -	port->irq	= 0;
>>>  	port->type	= PORT_UNKNOWN;
>>>  	port->iotype	= UPIO_MEM32;
>>>  	port->flags	= UPF_BOOT_AUTOCONF;
>>>  	port->ops	= &cdns_uart_ops;
>>>  	port->fifosize	= CDNS_UART_FIFO_SIZE;
>>>  	port->line	= id;
>>> -	port->dev	= NULL;
>>>  
>>>  	/*
>>>  	 * Register the port.
>>>
>>
>> Alan, Rob, Greg: Any comment about this RFC?
> 
> I rarely review RFC patchesets as obviously you don't think it is good
> enough to be submitted "for real" :)

There is one missing minor part but I want to review concept first
because I didn't find any driver which is using this style.

> If you think this is all good, great, please resend it without the RFC
> and it will end up in my queue.

I will definitely do it but please look at the concept itself because I
would like to use this with at least 3 other drivers.

Thanks,
Michal

^ permalink raw reply

* Re: [RFC PATCH v2 1/6] serial: uartps: Do not initialize field to zero again
From: Greg Kroah-Hartman @ 2018-06-27 23:48 UTC (permalink / raw)
  To: Michal Simek
  Cc: Michal Simek, linux-kernel, gnomes, Alexander Graf, shubhraj,
	robh, Jiri Slaby, linux-serial, linux-arm-kernel
In-Reply-To: <05e553c2-202b-0b4b-8b88-60e330c80fa3@xilinx.com>

On Wed, Jun 27, 2018 at 04:19:46PM +0200, Michal Simek wrote:
> On 27.6.2018 12:09, Greg Kroah-Hartman wrote:
> > On Tue, Jun 19, 2018 at 10:09:05AM +0200, Michal Simek wrote:
> >> On 6.6.2018 14:41, Michal Simek wrote:
> >>> Writing zero and NULLs to already initialized fields is not needed.
> >>> Remove this additional writes.
> >>>
> >>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> >>> ---
> >>>
> >>> Changes in v2:
> >>> - new patch - it can be sent separately too
> >>>
> >>>  drivers/tty/serial/xilinx_uartps.c | 3 ---
> >>>  1 file changed, 3 deletions(-)
> >>>
> >>> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
> >>> index 8a3e34234e98..5f116f3ecd4a 100644
> >>> --- a/drivers/tty/serial/xilinx_uartps.c
> >>> +++ b/drivers/tty/serial/xilinx_uartps.c
> >>> @@ -1510,15 +1510,12 @@ static int cdns_uart_probe(struct platform_device *pdev)
> >>>  
> >>>  	/* At this point, we've got an empty uart_port struct, initialize it */
> >>>  	spin_lock_init(&port->lock);
> >>> -	port->membase	= NULL;
> >>> -	port->irq	= 0;
> >>>  	port->type	= PORT_UNKNOWN;
> >>>  	port->iotype	= UPIO_MEM32;
> >>>  	port->flags	= UPF_BOOT_AUTOCONF;
> >>>  	port->ops	= &cdns_uart_ops;
> >>>  	port->fifosize	= CDNS_UART_FIFO_SIZE;
> >>>  	port->line	= id;
> >>> -	port->dev	= NULL;
> >>>  
> >>>  	/*
> >>>  	 * Register the port.
> >>>
> >>
> >> Alan, Rob, Greg: Any comment about this RFC?
> > 
> > I rarely review RFC patchesets as obviously you don't think it is good
> > enough to be submitted "for real" :)
> 
> There is one missing minor part but I want to review concept first
> because I didn't find any driver which is using this style.
> 
> > If you think this is all good, great, please resend it without the RFC
> > and it will end up in my queue.
> 
> I will definitely do it but please look at the concept itself because I
> would like to use this with at least 3 other drivers.

I don't have the time right now to review "concepts", sorry.

greg k-h

^ permalink raw reply

* Re: [RFC][PATCH 0/6] Use printk_safe context for TTY and UART port locks
From: Sergey Senozhatsky @ 2018-06-28  2:55 UTC (permalink / raw)
  To: Linus Torvalds, One Thousand Gnomes, Steven Rostedt
  Cc: Sergey Senozhatsky, Petr Mladek, Greg Kroah-Hartman, Jiri Slaby,
	Peter Zijlstra, Andrew Morton, Dmitry Vyukov,
	Linux Kernel Mailing List, linux-serial, SergeySenozhatsky
In-Reply-To: <CA+55aFzC5=awDhzOy9u_2FNaQmRprUTxj_KV2xO+GnpNAFg8MQ@mail.gmail.com>

On (06/20/18 12:38), Linus Torvalds wrote:
> On Wed, Jun 20, 2018 at 11:50 AM Sergey Senozhatsky
> <sergey.senozhatsky.work@gmail.com> wrote:
> >
> > It's not UART on its own that immediately calls into printk(), that would
> > be trivial to fix, it's all those subsystems that serial console driver
> > can call into.
> 
> We already have the whole PRINTK_SAFE_CONTEXT_MASK model that only
> adds it to a secondary buffer if you get recursion.  Why isn't that
> triggering? That's the whole point of it.

Linus, Alan, Steven,
are you on board with the patch set?
What shall I do to improve it?

PRINTK_SAFE_CONTEXT_MASK is what we answer nowadays when someone says
"printk causes deadlocks". We really can't remove all printk-s that can
cause uart->...->printk->uart recursion, and the only other option is to
use spin_trylock on uart_port->lock in every driver and discard con->write()
if we see that we have re-entered uart. I'd rather use per-CPU printk_safe
buffer in this case.

	-ss

^ permalink raw reply

* Re: [RFC PATCH v2 1/6] serial: uartps: Do not initialize field to zero again
From: Michal Simek @ 2018-06-28  7:18 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Michal Simek
  Cc: Michal Simek, linux-kernel, gnomes, Alexander Graf, shubhraj,
	robh, Jiri Slaby, linux-serial, linux-arm-kernel
In-Reply-To: <20180627234836.GA1691@kroah.com>

On 28.6.2018 01:48, Greg Kroah-Hartman wrote:
> On Wed, Jun 27, 2018 at 04:19:46PM +0200, Michal Simek wrote:
>> On 27.6.2018 12:09, Greg Kroah-Hartman wrote:
>>> On Tue, Jun 19, 2018 at 10:09:05AM +0200, Michal Simek wrote:
>>>> On 6.6.2018 14:41, Michal Simek wrote:
>>>>> Writing zero and NULLs to already initialized fields is not needed.
>>>>> Remove this additional writes.
>>>>>
>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>>> ---
>>>>>
>>>>> Changes in v2:
>>>>> - new patch - it can be sent separately too
>>>>>
>>>>>  drivers/tty/serial/xilinx_uartps.c | 3 ---
>>>>>  1 file changed, 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
>>>>> index 8a3e34234e98..5f116f3ecd4a 100644
>>>>> --- a/drivers/tty/serial/xilinx_uartps.c
>>>>> +++ b/drivers/tty/serial/xilinx_uartps.c
>>>>> @@ -1510,15 +1510,12 @@ static int cdns_uart_probe(struct platform_device *pdev)
>>>>>  
>>>>>  	/* At this point, we've got an empty uart_port struct, initialize it */
>>>>>  	spin_lock_init(&port->lock);
>>>>> -	port->membase	= NULL;
>>>>> -	port->irq	= 0;
>>>>>  	port->type	= PORT_UNKNOWN;
>>>>>  	port->iotype	= UPIO_MEM32;
>>>>>  	port->flags	= UPF_BOOT_AUTOCONF;
>>>>>  	port->ops	= &cdns_uart_ops;
>>>>>  	port->fifosize	= CDNS_UART_FIFO_SIZE;
>>>>>  	port->line	= id;
>>>>> -	port->dev	= NULL;
>>>>>  
>>>>>  	/*
>>>>>  	 * Register the port.
>>>>>
>>>>
>>>> Alan, Rob, Greg: Any comment about this RFC?
>>>
>>> I rarely review RFC patchesets as obviously you don't think it is good
>>> enough to be submitted "for real" :)
>>
>> There is one missing minor part but I want to review concept first
>> because I didn't find any driver which is using this style.
>>
>>> If you think this is all good, great, please resend it without the RFC
>>> and it will end up in my queue.
>>
>> I will definitely do it but please look at the concept itself because I
>> would like to use this with at least 3 other drivers.
> 
> I don't have the time right now to review "concepts", sorry.

Ok. I will revup that missing part to get it review to be able to use it.

Thanks,
Michal

^ permalink raw reply

* [PATCH v2] dt-bindings: serial: imx: clarify rs485 support usage
From: Baruch Siach @ 2018-06-28  7:25 UTC (permalink / raw)
  To: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, Greg Kroah-Hartman
  Cc: devicetree, Baruch Siach, linux-serial, Uwe Kleine-König,
	linux-arm-kernel, Lothar Waßmann

The i.MX UART peripheral uses the RST_B signal as input, and CTS_B as
output. This is just like the DCE role in RS-232. This is true
regardless of the "DTE mode" setting of this peripheral.

As a result, rs485 support hardware must use the CTS_B signal to control
the RS-485 transceiver. This is in contrast to generic rs485 kernel
code, documentation, and DT property names that consistently refer to
the RTS as transceiver control signal.

Add a note in the DT binding document about that, to reduce the
confusion somewhat.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
v2: Fix commit log typos (Lothar Waßmann)
---
 Documentation/devicetree/bindings/serial/fsl-imx-uart.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/serial/fsl-imx-uart.txt b/Documentation/devicetree/bindings/serial/fsl-imx-uart.txt
index afcfbc34e243..35957cbf1571 100644
--- a/Documentation/devicetree/bindings/serial/fsl-imx-uart.txt
+++ b/Documentation/devicetree/bindings/serial/fsl-imx-uart.txt
@@ -9,7 +9,11 @@ Optional properties:
 - fsl,dte-mode : Indicate the uart works in DTE mode. The uart works
                   in DCE mode by default.
 - rs485-rts-delay, rs485-rts-active-low, rs485-rx-during-tx,
-  linux,rs485-enabled-at-boot-time: see rs485.txt
+  linux,rs485-enabled-at-boot-time: see rs485.txt. Note that for RS485
+  you must enable either the "uart-has-rtscts" or the "rts-gpios"
+  properties. In case you use "uart-has-rtscts" the signal that controls
+  the transceiver is actually CTS_B, not RTS_B. CTS_B is always output,
+  and RTS_B is input, regardless of dte-mode.
 
 Please check Documentation/devicetree/bindings/serial/serial.txt
 for the complete list of generic properties.
-- 
2.18.0


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

^ permalink raw reply related

* Re: [PATCH v2] dt-bindings: serial: imx: clarify rs485 support usage
From: Uwe Kleine-König @ 2018-06-28  8:14 UTC (permalink / raw)
  To: Baruch Siach
  Cc: devicetree, linux-serial, Greg Kroah-Hartman, Sascha Hauer,
	NXP Linux Team, Pengutronix Kernel Team, Fabio Estevam, Shawn Guo,
	linux-arm-kernel, Lothar Waßmann
In-Reply-To: <e9beff38e9035c2f33d691f362e37d8e62aaa660.1530170732.git.baruch@tkos.co.il>

On Thu, Jun 28, 2018 at 10:25:32AM +0300, Baruch Siach wrote:
> The i.MX UART peripheral uses the RST_B signal as input, and CTS_B as
> output. This is just like the DCE role in RS-232. This is true
> regardless of the "DTE mode" setting of this peripheral.
> 
> As a result, rs485 support hardware must use the CTS_B signal to control
> the RS-485 transceiver. This is in contrast to generic rs485 kernel
> code, documentation, and DT property names that consistently refer to
> the RTS as transceiver control signal.

Well, the reason is that the pin function is called CTS because
Freescale chose to name it from the DCE's POV which is unusual.
So this isn't exactly specific to rs485 but also affect rs232 operation.

> Add a note in the DT binding document about that, to reduce the
> confusion somewhat.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> v2: Fix commit log typos (Lothar Waßmann)
> ---
>  Documentation/devicetree/bindings/serial/fsl-imx-uart.txt | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/serial/fsl-imx-uart.txt b/Documentation/devicetree/bindings/serial/fsl-imx-uart.txt
> index afcfbc34e243..35957cbf1571 100644
> --- a/Documentation/devicetree/bindings/serial/fsl-imx-uart.txt
> +++ b/Documentation/devicetree/bindings/serial/fsl-imx-uart.txt
> @@ -9,7 +9,11 @@ Optional properties:
>  - fsl,dte-mode : Indicate the uart works in DTE mode. The uart works
>                    in DCE mode by default.
>  - rs485-rts-delay, rs485-rts-active-low, rs485-rx-during-tx,
> -  linux,rs485-enabled-at-boot-time: see rs485.txt
> +  linux,rs485-enabled-at-boot-time: see rs485.txt. Note that for RS485
> +  you must enable either the "uart-has-rtscts" or the "rts-gpios"
> +  properties. In case you use "uart-has-rtscts" the signal that controls
> +  the transceiver is actually CTS_B, not RTS_B. CTS_B is always output,
> +  and RTS_B is input, regardless of dte-mode.

Still this section is correct and better than what we had (not) before.
So:
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Best regards
Uwe

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

^ permalink raw reply

* Re: [PATCH] uart: fix race between uart_put_char() and uart_shutdown()
From: Greg Kroah-Hartman @ 2018-06-28 12:05 UTC (permalink / raw)
  To: Tycho Andersen; +Cc: Jiri Slaby, linux-serial, linux-kernel, Serge E . Hallyn
In-Reply-To: <20180605000127.5495-1-tycho@tycho.ws>

On Mon, Jun 04, 2018 at 06:01:27PM -0600, Tycho Andersen wrote:
> We have reports of the following crash:
> 
>     PID: 7 TASK: ffff88085c6d61c0 CPU: 1 COMMAND: "kworker/u25:0"
>     #0 [ffff88085c6db710] machine_kexec at ffffffff81046239
>     #1 [ffff88085c6db760] crash_kexec at ffffffff810fc248
>     #2 [ffff88085c6db830] oops_end at ffffffff81008ae7
>     #3 [ffff88085c6db860] no_context at ffffffff81050b8f
>     #4 [ffff88085c6db8b0] __bad_area_nosemaphore at ffffffff81050d75
>     #5 [ffff88085c6db900] bad_area_nosemaphore at ffffffff81050e83
>     #6 [ffff88085c6db910] __do_page_fault at ffffffff8105132e
>     #7 [ffff88085c6db9b0] do_page_fault at ffffffff8105152c
>     #8 [ffff88085c6db9c0] page_fault at ffffffff81a3f122
>     [exception RIP: uart_put_char+149]
>     RIP: ffffffff814b67b5 RSP: ffff88085c6dba78 RFLAGS: 00010006
>     RAX: 0000000000000292 RBX: ffffffff827c5120 RCX: 0000000000000081
>     RDX: 0000000000000000 RSI: 000000000000005f RDI: ffffffff827c5120
>     RBP: ffff88085c6dba98 R8: 000000000000012c R9: ffffffff822ea320
>     R10: ffff88085fe4db04 R11: 0000000000000001 R12: ffff881059f9c000
>     R13: 0000000000000001 R14: 000000000000005f R15: 0000000000000fba
>     ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018
>     #9 [ffff88085c6dbaa0] tty_put_char at ffffffff81497544
>     #10 [ffff88085c6dbac0] do_output_char at ffffffff8149c91c
>     #11 [ffff88085c6dbae0] __process_echoes at ffffffff8149cb8b
>     #12 [ffff88085c6dbb30] commit_echoes at ffffffff8149cdc2
>     #13 [ffff88085c6dbb60] n_tty_receive_buf_fast at ffffffff8149e49b
>     #14 [ffff88085c6dbbc0] __receive_buf at ffffffff8149ef5a
>     #15 [ffff88085c6dbc20] n_tty_receive_buf_common at ffffffff8149f016
>     #16 [ffff88085c6dbca0] n_tty_receive_buf2 at ffffffff8149f194
>     #17 [ffff88085c6dbcb0] flush_to_ldisc at ffffffff814a238a
>     #18 [ffff88085c6dbd50] process_one_work at ffffffff81090be2
>     #19 [ffff88085c6dbe20] worker_thread at ffffffff81091b4d
>     #20 [ffff88085c6dbeb0] kthread at ffffffff81096384
>     #21 [ffff88085c6dbf50] ret_from_fork at ffffffff81a3d69f​
> 
> after slogging through some dissasembly:
> 
> ffffffff814b6720 <uart_put_char>:
> ffffffff814b6720:	55                   	push   %rbp
> ffffffff814b6721:	48 89 e5             	mov    %rsp,%rbp
> ffffffff814b6724:	48 83 ec 20          	sub    $0x20,%rsp
> ffffffff814b6728:	48 89 1c 24          	mov    %rbx,(%rsp)
> ffffffff814b672c:	4c 89 64 24 08       	mov    %r12,0x8(%rsp)
> ffffffff814b6731:	4c 89 6c 24 10       	mov    %r13,0x10(%rsp)
> ffffffff814b6736:	4c 89 74 24 18       	mov    %r14,0x18(%rsp)
> ffffffff814b673b:	e8 b0 8e 58 00       	callq  ffffffff81a3f5f0 <mcount>
> ffffffff814b6740:	4c 8b a7 88 02 00 00 	mov    0x288(%rdi),%r12
> ffffffff814b6747:	45 31 ed             	xor    %r13d,%r13d
> ffffffff814b674a:	41 89 f6             	mov    %esi,%r14d
> ffffffff814b674d:	49 83 bc 24 70 01 00 	cmpq   $0x0,0x170(%r12)
> ffffffff814b6754:	00 00
> ffffffff814b6756:	49 8b 9c 24 80 01 00 	mov    0x180(%r12),%rbx
> ffffffff814b675d:	00
> ffffffff814b675e:	74 2f                	je     ffffffff814b678f <uart_put_char+0x6f>
> ffffffff814b6760:	48 89 df             	mov    %rbx,%rdi
> ffffffff814b6763:	e8 a8 67 58 00       	callq  ffffffff81a3cf10 <_raw_spin_lock_irqsave>
> ffffffff814b6768:	41 8b 8c 24 78 01 00 	mov    0x178(%r12),%ecx
> ffffffff814b676f:	00
> ffffffff814b6770:	89 ca                	mov    %ecx,%edx
> ffffffff814b6772:	f7 d2                	not    %edx
> ffffffff814b6774:	41 03 94 24 7c 01 00 	add    0x17c(%r12),%edx
> ffffffff814b677b:	00
> ffffffff814b677c:	81 e2 ff 0f 00 00    	and    $0xfff,%edx
> ffffffff814b6782:	75 23                	jne    ffffffff814b67a7 <uart_put_char+0x87>
> ffffffff814b6784:	48 89 c6             	mov    %rax,%rsi
> ffffffff814b6787:	48 89 df             	mov    %rbx,%rdi
> ffffffff814b678a:	e8 e1 64 58 00       	callq  ffffffff81a3cc70 <_raw_spin_unlock_irqrestore>
> ffffffff814b678f:	44 89 e8             	mov    %r13d,%eax
> ffffffff814b6792:	48 8b 1c 24          	mov    (%rsp),%rbx
> ffffffff814b6796:	4c 8b 64 24 08       	mov    0x8(%rsp),%r12
> ffffffff814b679b:	4c 8b 6c 24 10       	mov    0x10(%rsp),%r13
> ffffffff814b67a0:	4c 8b 74 24 18       	mov    0x18(%rsp),%r14
> ffffffff814b67a5:	c9                   	leaveq
> ffffffff814b67a6:	c3                   	retq
> ffffffff814b67a7:	49 8b 94 24 70 01 00 	mov    0x170(%r12),%rdx
> ffffffff814b67ae:	00
> ffffffff814b67af:	48 63 c9             	movslq %ecx,%rcx
> ffffffff814b67b2:	41 b5 01             	mov    $0x1,%r13b
> ffffffff814b67b5:	44 88 34 0a          	mov    %r14b,(%rdx,%rcx,1)
> ffffffff814b67b9:	41 8b 94 24 78 01 00 	mov    0x178(%r12),%edx
> ffffffff814b67c0:	00
> ffffffff814b67c1:	83 c2 01             	add    $0x1,%edx
> ffffffff814b67c4:	81 e2 ff 0f 00 00    	and    $0xfff,%edx
> ffffffff814b67ca:	41 89 94 24 78 01 00 	mov    %edx,0x178(%r12)
> ffffffff814b67d1:	00
> ffffffff814b67d2:	eb b0                	jmp    ffffffff814b6784 <uart_put_char+0x64>
> ffffffff814b67d4:	66 66 66 2e 0f 1f 84 	data32 data32 nopw %cs:0x0(%rax,%rax,1)
> ffffffff814b67db:	00 00 00 00 00
> 
> for our build, this is crashing at:
> 
>     circ->buf[circ->head] = c;
> 
> Looking in uart_port_startup(), it seems that circ->buf (state->xmit.buf)
> protected by the "per-port mutex", which based on uart_port_check() is
> state->port.mutex. Indeed, the lock acquired in uart_put_char() is
> uport->lock, i.e. not the same lock.
> 
> Anyway, since the lock is not acquired, if uart_shutdown() is called, the
> last chunk of that function may release state->xmit.buf before its assigned
> to null, and cause the race above.
> 
> To fix it, we simply also acquire state->port.mutex.

Ick.  Can't we just grab the same lock in the other place?  Grabbing 2
locks for every individual character seems _really_ heavy, don't you
think?

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH v2] dt-bindings: serial: imx: clarify rs485 support usage
From: Fabio Estevam @ 2018-06-28 16:30 UTC (permalink / raw)
  To: Baruch Siach
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-serial, Greg Kroah-Hartman, Sascha Hauer, NXP Linux Team,
	Pengutronix Kernel Team, Uwe Kleine-König, Fabio Estevam,
	Shawn Guo,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Lothar Waßmann
In-Reply-To: <e9beff38e9035c2f33d691f362e37d8e62aaa660.1530170732.git.baruch@tkos.co.il>

Hi Baruch,

On Thu, Jun 28, 2018 at 4:25 AM, Baruch Siach <baruch@tkos.co.il> wrote:
> The i.MX UART peripheral uses the RST_B signal as input, and CTS_B as
> output. This is just like the DCE role in RS-232. This is true
> regardless of the "DTE mode" setting of this peripheral.
>
> As a result, rs485 support hardware must use the CTS_B signal to control
> the RS-485 transceiver. This is in contrast to generic rs485 kernel
> code, documentation, and DT property names that consistently refer to
> the RTS as transceiver control signal.
>
> Add a note in the DT binding document about that, to reduce the
> confusion somewhat.
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>

Yes, this is helpful:

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

^ permalink raw reply

* [PATCH v2] uart: fix race between uart_put_char() and uart_shutdown()
From: Tycho Andersen @ 2018-06-29 10:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-serial, linux-kernel, Serge E . Hallyn, Tycho Andersen
In-Reply-To: <20180628120542.GA4065@kroah.com>

We have reports of the following crash:

    PID: 7 TASK: ffff88085c6d61c0 CPU: 1 COMMAND: "kworker/u25:0"
    #0 [ffff88085c6db710] machine_kexec at ffffffff81046239
    #1 [ffff88085c6db760] crash_kexec at ffffffff810fc248
    #2 [ffff88085c6db830] oops_end at ffffffff81008ae7
    #3 [ffff88085c6db860] no_context at ffffffff81050b8f
    #4 [ffff88085c6db8b0] __bad_area_nosemaphore at ffffffff81050d75
    #5 [ffff88085c6db900] bad_area_nosemaphore at ffffffff81050e83
    #6 [ffff88085c6db910] __do_page_fault at ffffffff8105132e
    #7 [ffff88085c6db9b0] do_page_fault at ffffffff8105152c
    #8 [ffff88085c6db9c0] page_fault at ffffffff81a3f122
    [exception RIP: uart_put_char+149]
    RIP: ffffffff814b67b5 RSP: ffff88085c6dba78 RFLAGS: 00010006
    RAX: 0000000000000292 RBX: ffffffff827c5120 RCX: 0000000000000081
    RDX: 0000000000000000 RSI: 000000000000005f RDI: ffffffff827c5120
    RBP: ffff88085c6dba98 R8: 000000000000012c R9: ffffffff822ea320
    R10: ffff88085fe4db04 R11: 0000000000000001 R12: ffff881059f9c000
    R13: 0000000000000001 R14: 000000000000005f R15: 0000000000000fba
    ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018
    #9 [ffff88085c6dbaa0] tty_put_char at ffffffff81497544
    #10 [ffff88085c6dbac0] do_output_char at ffffffff8149c91c
    #11 [ffff88085c6dbae0] __process_echoes at ffffffff8149cb8b
    #12 [ffff88085c6dbb30] commit_echoes at ffffffff8149cdc2
    #13 [ffff88085c6dbb60] n_tty_receive_buf_fast at ffffffff8149e49b
    #14 [ffff88085c6dbbc0] __receive_buf at ffffffff8149ef5a
    #15 [ffff88085c6dbc20] n_tty_receive_buf_common at ffffffff8149f016
    #16 [ffff88085c6dbca0] n_tty_receive_buf2 at ffffffff8149f194
    #17 [ffff88085c6dbcb0] flush_to_ldisc at ffffffff814a238a
    #18 [ffff88085c6dbd50] process_one_work at ffffffff81090be2
    #19 [ffff88085c6dbe20] worker_thread at ffffffff81091b4d
    #20 [ffff88085c6dbeb0] kthread at ffffffff81096384
    #21 [ffff88085c6dbf50] ret_from_fork at ffffffff81a3d69f​

after slogging through some dissasembly:

ffffffff814b6720 <uart_put_char>:
ffffffff814b6720:	55                   	push   %rbp
ffffffff814b6721:	48 89 e5             	mov    %rsp,%rbp
ffffffff814b6724:	48 83 ec 20          	sub    $0x20,%rsp
ffffffff814b6728:	48 89 1c 24          	mov    %rbx,(%rsp)
ffffffff814b672c:	4c 89 64 24 08       	mov    %r12,0x8(%rsp)
ffffffff814b6731:	4c 89 6c 24 10       	mov    %r13,0x10(%rsp)
ffffffff814b6736:	4c 89 74 24 18       	mov    %r14,0x18(%rsp)
ffffffff814b673b:	e8 b0 8e 58 00       	callq  ffffffff81a3f5f0 <mcount>
ffffffff814b6740:	4c 8b a7 88 02 00 00 	mov    0x288(%rdi),%r12
ffffffff814b6747:	45 31 ed             	xor    %r13d,%r13d
ffffffff814b674a:	41 89 f6             	mov    %esi,%r14d
ffffffff814b674d:	49 83 bc 24 70 01 00 	cmpq   $0x0,0x170(%r12)
ffffffff814b6754:	00 00
ffffffff814b6756:	49 8b 9c 24 80 01 00 	mov    0x180(%r12),%rbx
ffffffff814b675d:	00
ffffffff814b675e:	74 2f                	je     ffffffff814b678f <uart_put_char+0x6f>
ffffffff814b6760:	48 89 df             	mov    %rbx,%rdi
ffffffff814b6763:	e8 a8 67 58 00       	callq  ffffffff81a3cf10 <_raw_spin_lock_irqsave>
ffffffff814b6768:	41 8b 8c 24 78 01 00 	mov    0x178(%r12),%ecx
ffffffff814b676f:	00
ffffffff814b6770:	89 ca                	mov    %ecx,%edx
ffffffff814b6772:	f7 d2                	not    %edx
ffffffff814b6774:	41 03 94 24 7c 01 00 	add    0x17c(%r12),%edx
ffffffff814b677b:	00
ffffffff814b677c:	81 e2 ff 0f 00 00    	and    $0xfff,%edx
ffffffff814b6782:	75 23                	jne    ffffffff814b67a7 <uart_put_char+0x87>
ffffffff814b6784:	48 89 c6             	mov    %rax,%rsi
ffffffff814b6787:	48 89 df             	mov    %rbx,%rdi
ffffffff814b678a:	e8 e1 64 58 00       	callq  ffffffff81a3cc70 <_raw_spin_unlock_irqrestore>
ffffffff814b678f:	44 89 e8             	mov    %r13d,%eax
ffffffff814b6792:	48 8b 1c 24          	mov    (%rsp),%rbx
ffffffff814b6796:	4c 8b 64 24 08       	mov    0x8(%rsp),%r12
ffffffff814b679b:	4c 8b 6c 24 10       	mov    0x10(%rsp),%r13
ffffffff814b67a0:	4c 8b 74 24 18       	mov    0x18(%rsp),%r14
ffffffff814b67a5:	c9                   	leaveq
ffffffff814b67a6:	c3                   	retq
ffffffff814b67a7:	49 8b 94 24 70 01 00 	mov    0x170(%r12),%rdx
ffffffff814b67ae:	00
ffffffff814b67af:	48 63 c9             	movslq %ecx,%rcx
ffffffff814b67b2:	41 b5 01             	mov    $0x1,%r13b
ffffffff814b67b5:	44 88 34 0a          	mov    %r14b,(%rdx,%rcx,1)
ffffffff814b67b9:	41 8b 94 24 78 01 00 	mov    0x178(%r12),%edx
ffffffff814b67c0:	00
ffffffff814b67c1:	83 c2 01             	add    $0x1,%edx
ffffffff814b67c4:	81 e2 ff 0f 00 00    	and    $0xfff,%edx
ffffffff814b67ca:	41 89 94 24 78 01 00 	mov    %edx,0x178(%r12)
ffffffff814b67d1:	00
ffffffff814b67d2:	eb b0                	jmp    ffffffff814b6784 <uart_put_char+0x64>
ffffffff814b67d4:	66 66 66 2e 0f 1f 84 	data32 data32 nopw %cs:0x0(%rax,%rax,1)
ffffffff814b67db:	00 00 00 00 00

for our build, this is crashing at:

    circ->buf[circ->head] = c;

Looking in uart_port_startup(), it seems that circ->buf (state->xmit.buf)
protected by the "per-port mutex", which based on uart_port_check() is
state->port.mutex. Indeed, the lock acquired in uart_put_char() is
uport->lock, i.e. not the same lock.

Anyway, since the lock is not acquired, if uart_shutdown() is called, the
last chunk of that function may release state->xmit.buf before its assigned
to null, and cause the race above.

To fix it, let's lock uport->lock when allocating/deallocating
state->xmit.buf in addition to the per-port mutex.

v2: switch to locking uport->lock on allocation/deallocation instead of
    locking the per-port mutex in uart_put_char. Note that since
    uport->lock is a spin lock, we have to switch the allocation to
    GFP_ATOMIC.

Signed-off-by: Tycho Andersen <tycho@tycho.ws>
---
 drivers/tty/serial/serial_core.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 9c14a453f73c..790f3ea7ffca 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -181,7 +181,7 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
 		int init_hw)
 {
 	struct uart_port *uport = uart_port_check(state);
-	unsigned long page;
+	unsigned long page, flags = 0;
 	int retval = 0;
 
 	if (uport->type == PORT_UNKNOWN)
@@ -196,15 +196,19 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
 	 * Initialise and allocate the transmit and temporary
 	 * buffer.
 	 */
+	uart_port_lock(state, flags);
 	if (!state->xmit.buf) {
-		/* This is protected by the per port mutex */
-		page = get_zeroed_page(GFP_KERNEL);
-		if (!page)
+		page = get_zeroed_page(GFP_ATOMIC);
+		if (!page) {
+			uart_port_unlock(uport, flags);
 			return -ENOMEM;
+		}
 
 		state->xmit.buf = (unsigned char *) page;
 		uart_circ_clear(&state->xmit);
 	}
+	uart_port_unlock(uport, flags);
+
 
 	retval = uport->ops->startup(uport);
 	if (retval == 0) {
@@ -263,6 +267,7 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
 {
 	struct uart_port *uport = uart_port_check(state);
 	struct tty_port *port = &state->port;
+	unsigned long flags = 0;
 
 	/*
 	 * Set the TTY IO error marker
@@ -295,10 +300,12 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
 	/*
 	 * Free the transmit buffer page.
 	 */
+	uart_port_lock(state, flags);
 	if (state->xmit.buf) {
 		free_page((unsigned long)state->xmit.buf);
 		state->xmit.buf = NULL;
 	}
+	uart_port_unlock(uport, flags);
 }
 
 /**
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH v5 1/7] tty: serial: imx: correct dma cookie status
From: Vinod @ 2018-06-29 11:03 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Robin Gong, s.hauer, l.stach, dan.j.williams, gregkh, jslaby,
	dmaengine, linux-imx, linux-kernel, linux-serial,
	linux-arm-kernel
In-Reply-To: <20180626192252.6qdpwzpbpn6s5sd3@pengutronix.de>

On 26-06-18, 21:22, Uwe Kleine-König wrote:
> On Wed, Jun 20, 2018 at 12:56:58AM +0800, Robin Gong wrote:
> > Correct to check the right rx dma cookie status in spit of it
> > works because only one cookie is running in the current sdma.
> > But it will not once sdma driver support multi cookies
> > running based on virt-dma.
> > 
> > Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> Looks wrong (because of tx_status vs rx_cookie), but is right
> nevertheless I think:

hehe, tx refers to transfer status for rx (receive) cookie and not
transmit .. yeah notations can be better!

> 
> Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> Thanks
> Uwe
> 
> > ---
> >  drivers/tty/serial/imx.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> > index 4e85357..2879407 100644
> > --- a/drivers/tty/serial/imx.c
> > +++ b/drivers/tty/serial/imx.c
> > @@ -1051,7 +1051,7 @@ static void imx_uart_dma_rx_callback(void *data)
> >  	unsigned int r_bytes;
> >  	unsigned int bd_size;
> >  
> > -	status = dmaengine_tx_status(chan, (dma_cookie_t)0, &state);
> > +	status = dmaengine_tx_status(chan, sport->rx_cookie, &state);
> >  
> >  	if (status == DMA_ERROR) {
> >  		imx_uart_clear_rx_errors(sport);
> > -- 
> > 2.7.4
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > 
> 
> -- 
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |

-- 
~Vinod

^ permalink raw reply

* Re: [PATCH v2] uart: fix race between uart_put_char() and uart_shutdown()
From: Tycho Andersen @ 2018-06-29 16:43 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-serial, linux-kernel, Serge E . Hallyn
In-Reply-To: <20180629102446.11189-1-tycho@tycho.ws>

On Fri, Jun 29, 2018 at 04:24:46AM -0600, Tycho Andersen wrote:
> v2: switch to locking uport->lock on allocation/deallocation instead of
>     locking the per-port mutex in uart_put_char. Note that since
>     uport->lock is a spin lock, we have to switch the allocation to
>     GFP_ATOMIC.

Serge pointed out off-list that we may want to do the allocation
before the lock so that it's more likely to be successful. I'm happy
to send that change to this if it's what we want to do, I don't have a
strong preference.

Tycho

^ permalink raw reply

* [GIT PULL] TTY/Serial fixes for 4.18-rc3
From: Greg KH @ 2018-07-01  8:40 UTC (permalink / raw)
  To: Linus Torvalds, Jiri Slaby
  Cc: Stephen Rothwell, Andrew Morton, linux-kernel, linux-serial

The following changes since commit 7daf201d7fe8334e2d2364d4e8ed3394ec9af819:

  Linux 4.18-rc2 (2018-06-24 20:54:29 +0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/ tags/tty-4.18-rc3

for you to fetch changes up to 21eff69aaaa0e766ca0ce445b477698dc6a9f55a:

  vt: prevent leaking uninitialized data to userspace via /dev/vcs* (2018-06-28 21:34:39 +0900)

----------------------------------------------------------------
TTY/Serial fixes for 4.18-rc3

Here are 5 fixes for the tty core and some serial drivers.

The tty core one fix some security and other issues reported by the
syzbot that I have taken too long in responding to (sorry Tetsuo!).  The
8350 serial driver fix resolves an issue of devices that used to work
properly stopping working as they shouldn't have been added to a
blacklist.

All of these have been in linux-next for a few days with no reported
issues.

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

----------------------------------------------------------------
Alexander Potapenko (1):
      vt: prevent leaking uninitialized data to userspace via /dev/vcs*

Andy Shevchenko (1):
      serial: 8250_pci: Remove stalled entries in blacklist

Johan Hovold (1):
      serdev: fix memleak on module unload

Tetsuo Handa (2):
      n_tty: Fix stall at n_tty_receive_char_special().
      n_tty: Access echo_* variables carefully.

 drivers/tty/n_tty.c                | 55 ++++++++++++++++++++++----------------
 drivers/tty/serdev/core.c          |  1 +
 drivers/tty/serial/8250/8250_pci.c |  2 --
 drivers/tty/vt/vt.c                |  4 +--
 4 files changed, 35 insertions(+), 27 deletions(-)

^ permalink raw reply

* [PATCH 4.4 035/105] lib/vsprintf: Remove atomic-unsafe support for %pCr
From: Greg Kroah-Hartman @ 2018-07-01 16:01 UTC (permalink / raw)
  To: linux-kernel, Jia-Ju Bai, Jonathan Corbet, Michael Turquette,
	Stephen Boyd, Zhang Rui, Eduardo Valentin, Eric Anholt,
	Stefan Wahren
  Cc: Petr Mladek, Sergey Senozhatsky, Geert Uytterhoeven, linux-doc,
	Greg Kroah-Hartman, linux-pm, Steven Rostedt, linux-renesas-soc,
	stable, linux-serial, Linus Torvalds, linux-clk, linux-arm-kernel
In-Reply-To: <20180701153149.382300170@linuxfoundation.org>

4.4-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Geert Uytterhoeven <geert+renesas@glider.be>

commit 666902e42fd8344b923c02dc5b0f37948ff4f225 upstream.

"%pCr" formats the current rate of a clock, and calls clk_get_rate().
The latter obtains a mutex, hence it must not be called from atomic
context.

Remove support for this rarely-used format, as vsprintf() (and e.g.
printk()) must be callable from any context.

Any remaining out-of-tree users will start seeing the clock's name
printed instead of its rate.

Reported-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Fixes: 900cca2944254edd ("lib/vsprintf: add %pC{,n,r} format specifiers for clocks")
Link: http://lkml.kernel.org/r/1527845302-12159-5-git-send-email-geert+renesas@glider.be
To: Jia-Ju Bai <baijiaju1990@gmail.com>
To: Jonathan Corbet <corbet@lwn.net>
To: Michael Turquette <mturquette@baylibre.com>
To: Stephen Boyd <sboyd@kernel.org>
To: Zhang Rui <rui.zhang@intel.com>
To: Eduardo Valentin <edubezval@gmail.com>
To: Eric Anholt <eric@anholt.net>
To: Stefan Wahren <stefan.wahren@i2se.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-doc@vger.kernel.org
Cc: linux-clk@vger.kernel.org
Cc: linux-pm@vger.kernel.org
Cc: linux-serial@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-renesas-soc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: stable@vger.kernel.org # 4.1+
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 Documentation/printk-formats.txt |    3 +--
 lib/vsprintf.c                   |    3 ---
 2 files changed, 1 insertion(+), 5 deletions(-)

--- a/Documentation/printk-formats.txt
+++ b/Documentation/printk-formats.txt
@@ -273,11 +273,10 @@ struct clk:
 
 	%pC	pll1
 	%pCn	pll1
-	%pCr	1560000000
 
 	For printing struct clk structures. '%pC' and '%pCn' print the name
 	(Common Clock Framework) or address (legacy clock framework) of the
-	structure; '%pCr' prints the current clock rate.
+	structure.
 
 	Passed by reference.
 
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1345,9 +1345,6 @@ char *clock(char *buf, char *end, struct
 		return string(buf, end, NULL, spec);
 
 	switch (fmt[1]) {
-	case 'r':
-		return number(buf, end, clk_get_rate(clk), spec);
-
 	case 'n':
 	default:
 #ifdef CONFIG_COMMON_CLK

^ permalink raw reply

* [PATCH 4.17 031/220] thermal: bcm2835: Stop using printk format %pCr
From: Greg Kroah-Hartman @ 2018-07-01 16:20 UTC (permalink / raw)
  To: linux-kernel, Jia-Ju Bai, Jonathan Corbet, Michael Turquette,
	Stephen Boyd, Zhang Rui, Eduardo Valentin, Eric Anholt,
	Stefan Wahren
  Cc: Petr Mladek, Sergey Senozhatsky, Geert Uytterhoeven, linux-doc,
	Greg Kroah-Hartman, linux-pm, Steven Rostedt, linux-renesas-soc,
	stable, linux-serial, Linus Torvalds, linux-clk, linux-arm-kernel
In-Reply-To: <20180701160908.272447118@linuxfoundation.org>

4.17-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Geert Uytterhoeven <geert+renesas@glider.be>

commit bd2a07f71a1e2e198f8a30cb551d9defe422d83d upstream.

Printk format "%pCr" will be removed soon, as clk_get_rate() must not be
called in atomic context.

Replace it by printing the variable that already holds the clock rate.
Note that calling clk_get_rate() is safe here, as the code runs in task
context.

Link: http://lkml.kernel.org/r/1527845302-12159-3-git-send-email-geert+renesas@glider.be
To: Jia-Ju Bai <baijiaju1990@gmail.com>
To: Jonathan Corbet <corbet@lwn.net>
To: Michael Turquette <mturquette@baylibre.com>
To: Stephen Boyd <sboyd@kernel.org>
To: Zhang Rui <rui.zhang@intel.com>
To: Eduardo Valentin <edubezval@gmail.com>
To: Eric Anholt <eric@anholt.net>
To: Stefan Wahren <stefan.wahren@i2se.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-doc@vger.kernel.org
Cc: linux-clk@vger.kernel.org
Cc: linux-pm@vger.kernel.org
Cc: linux-serial@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-renesas-soc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: stable@vger.kernel.org # 4.12+
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/thermal/broadcom/bcm2835_thermal.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/drivers/thermal/broadcom/bcm2835_thermal.c
+++ b/drivers/thermal/broadcom/bcm2835_thermal.c
@@ -213,8 +213,8 @@ static int bcm2835_thermal_probe(struct
 	rate = clk_get_rate(data->clk);
 	if ((rate < 1920000) || (rate > 5000000))
 		dev_warn(&pdev->dev,
-			 "Clock %pCn running at %pCr Hz is outside of the recommended range: 1.92 to 5MHz\n",
-			 data->clk, data->clk);
+			 "Clock %pCn running at %lu Hz is outside of the recommended range: 1.92 to 5MHz\n",
+			 data->clk, rate);
 
 	/* register of thermal sensor and get info from DT */
 	tz = thermal_zone_of_sensor_register(&pdev->dev, 0, data,

^ permalink raw reply


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