Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH v5 2/2] arm64: dts: Add Mediatek SoC MT2712 and evaluation board dts and Makefile
From: YT Shen @ 2017-07-28 11:37 UTC (permalink / raw)
  To: Rob Herring, Matthias Brugger
  Cc: Mark Rutland, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Greg Kroah-Hartman, Catalin Marinas, Will Deacon, Mars Cheng,
	YT Shen, devicetree, linux-kernel, linux-serial, linux-arm-kernel,
	linux-mediatek, srv_heupstream
In-Reply-To: <1501241835-47849-1-git-send-email-yt.shen@mediatek.com>

This adds basic chip support for Mediatek 2712

Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/Makefile       |   1 +
 arch/arm64/boot/dts/mediatek/mt2712-evb.dts |  32 +++++
 arch/arm64/boot/dts/mediatek/mt2712e.dtsi   | 178 ++++++++++++++++++++++++++++
 3 files changed, 211 insertions(+)
 create mode 100644 arch/arm64/boot/dts/mediatek/mt2712-evb.dts
 create mode 100644 arch/arm64/boot/dts/mediatek/mt2712e.dtsi

diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
index 015eb07..f323936 100644
--- a/arch/arm64/boot/dts/mediatek/Makefile
+++ b/arch/arm64/boot/dts/mediatek/Makefile
@@ -1,3 +1,4 @@
+dtb-$(CONFIG_ARCH_MEDIATEK) += mt2712-evb.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt6755-evb.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-evb.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
diff --git a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
new file mode 100644
index 0000000..8c804df
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
@@ -0,0 +1,32 @@
+/*
+ * Copyright (c) 2017 MediaTek Inc.
+ * Author: YT Shen <yt.shen@mediatek.com>
+ *
+ * SPDX-License-Identifier: (GPL-2.0 OR MIT)
+ */
+
+/dts-v1/;
+#include "mt2712e.dtsi"
+
+/ {
+	model = "MediaTek MT2712 evaluation board";
+	compatible = "mediatek,mt2712-evb", "mediatek,mt2712";
+
+	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/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
new file mode 100644
index 0000000..1e135af
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
@@ -0,0 +1,178 @@
+/*
+ * Copyright (c) 2017 MediaTek Inc.
+ * Author: YT Shen <yt.shen@mediatek.com>
+ *
+ * SPDX-License-Identifier: (GPL-2.0 OR MIT)
+ */
+
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/ {
+	compatible = "mediatek,mt2712";
+	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>;
+				};
+			};
+
+			cluster1 {
+				core0 {
+					cpu = <&cpu2>;
+				};
+			};
+		};
+
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a35";
+			reg = <0x000>;
+		};
+
+		cpu1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a35";
+			reg = <0x001>;
+			enable-method = "psci";
+		};
+
+		cpu2: cpu@200 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0x200>;
+			enable-method = "psci";
+		};
+	};
+
+	psci {
+		compatible = "arm,psci-0.2";
+		method = "smc";
+	};
+
+	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_RAW(0x13) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 14
+			      (GIC_CPU_MASK_RAW(0x13) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 11
+			      (GIC_CPU_MASK_RAW(0x13) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 10
+			      (GIC_CPU_MASK_RAW(0x13) | IRQ_TYPE_LEVEL_LOW)>;
+	};
+
+	soc {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		compatible = "simple-bus";
+		ranges;
+
+		uart5: serial@1000f000 {
+			compatible = "mediatek,mt2712-uart",
+				     "mediatek,mt6577-uart";
+			reg = <0 0x1000f000 0 0x400>;
+			interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&baud_clk>, <&sys_clk>;
+			clock-names = "baud", "bus";
+			status = "disabled";
+		};
+
+		sysirq: interrupt-controller@10220a80 {
+			compatible = "mediatek,mt2712-sysirq",
+				     "mediatek,mt6577-sysirq";
+			interrupt-controller;
+			#interrupt-cells = <3>;
+			interrupt-parent = <&gic>;
+			reg = <0 0x10220a80 0 0x40>;
+		};
+
+		gic: interrupt-controller@10510000 {
+			compatible = "arm,gic-400";
+			#interrupt-cells = <3>;
+			interrupt-parent = <&gic>;
+			interrupt-controller;
+			reg = <0 0x10510000 0 0x10000>,
+			      <0 0x10520000 0 0x20000>,
+			      <0 0x10540000 0 0x20000>,
+			      <0 0x10560000 0 0x20000>;
+			interrupts = <GIC_PPI 9
+				(GIC_CPU_MASK_RAW(0x13) | IRQ_TYPE_LEVEL_HIGH)>;
+		};
+
+		uart0: serial@11002000 {
+			compatible = "mediatek,mt2712-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,mt2712-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";
+		};
+
+		uart2: serial@11004000 {
+			compatible = "mediatek,mt2712-uart",
+				     "mediatek,mt6577-uart";
+			reg = <0 0x11004000 0 0x400>;
+			interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&baud_clk>, <&sys_clk>;
+			clock-names = "baud", "bus";
+			status = "disabled";
+		};
+
+		uart3: serial@11005000 {
+			compatible = "mediatek,mt2712-uart",
+				     "mediatek,mt6577-uart";
+			reg = <0 0x11005000 0 0x400>;
+			interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&baud_clk>, <&sys_clk>;
+			clock-names = "baud", "bus";
+			status = "disabled";
+		};
+
+		uart4: serial@11019000 {
+			compatible = "mediatek,mt2712-uart",
+				     "mediatek,mt6577-uart";
+			reg = <0 0x11019000 0 0x400>;
+			interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&baud_clk>, <&sys_clk>;
+			clock-names = "baud", "bus";
+			status = "disabled";
+		};
+	};
+};
+
-- 
1.9.1

^ permalink raw reply related

* [PATCH v5 1/2] dt-bindings: arm: Add bindings for Mediatek MT2712 SoC Platform
From: YT Shen @ 2017-07-28 11:37 UTC (permalink / raw)
  To: Rob Herring, Matthias Brugger
  Cc: Mark Rutland, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Greg Kroah-Hartman, Catalin Marinas, Will Deacon, Mars Cheng,
	YT Shen, devicetree, linux-kernel, linux-serial, linux-arm-kernel,
	linux-mediatek, srv_heupstream
In-Reply-To: <1501241835-47849-1-git-send-email-yt.shen@mediatek.com>

This adds dt-binding documentation for Mediatek MT2712.
Only include very basic items: cpu, gic and uart.

Signed-off-by: YT Shen <yt.shen@mediatek.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/arm/mediatek.txt                    | 4 ++++
 .../devicetree/bindings/interrupt-controller/mediatek,sysirq.txt      | 1 +
 Documentation/devicetree/bindings/serial/mtk-uart.txt                 | 1 +
 3 files changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/mediatek.txt b/Documentation/devicetree/bindings/arm/mediatek.txt
index da7bd13..7aa0e98 100644
--- a/Documentation/devicetree/bindings/arm/mediatek.txt
+++ b/Documentation/devicetree/bindings/arm/mediatek.txt
@@ -7,6 +7,7 @@ Required root node property:
 
 compatible: Must contain one of
    "mediatek,mt2701"
+   "mediatek,mt2712"
    "mediatek,mt6580"
    "mediatek,mt6589"
    "mediatek,mt6592"
@@ -25,6 +26,9 @@ Supported boards:
 - Evaluation board for MT2701:
     Required root node properties:
       - compatible = "mediatek,mt2701-evb", "mediatek,mt2701";
+- Evaluation board for MT2712:
+    Required root node properties:
+      - compatible = "mediatek,mt2712-evb", "mediatek,mt2712";
 - Evaluation board for MT6580:
     Required root node properties:
       - compatible = "mediatek,mt6580-evbp1", "mediatek,mt6580";
diff --git a/Documentation/devicetree/bindings/interrupt-controller/mediatek,sysirq.txt b/Documentation/devicetree/bindings/interrupt-controller/mediatek,sysirq.txt
index 11cc87a..07bf0b9 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/mediatek,sysirq.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/mediatek,sysirq.txt
@@ -17,6 +17,7 @@ Required properties:
 	"mediatek,mt6582-sysirq", "mediatek,mt6577-sysirq": for MT6582
 	"mediatek,mt6580-sysirq", "mediatek,mt6577-sysirq": for MT6580
 	"mediatek,mt6577-sysirq": for MT6577
+	"mediatek,mt2712-sysirq", "mediatek,mt6577-sysirq": for MT2712
 	"mediatek,mt2701-sysirq", "mediatek,mt6577-sysirq": for MT2701
 - interrupt-controller : Identifies the node as an interrupt controller
 - #interrupt-cells : Use the same format as specified by GIC in arm,gic.txt.
diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
index b6cf384..f73abff 100644
--- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
+++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
@@ -3,6 +3,7 @@
 Required properties:
 - compatible should contain:
   * "mediatek,mt2701-uart" for MT2701 compatible UARTS
+  * "mediatek,mt2712-uart" for MT2712 compatible UARTS
   * "mediatek,mt6580-uart" for MT6580 compatible UARTS
   * "mediatek,mt6582-uart" for MT6582 compatible UARTS
   * "mediatek,mt6589-uart" for MT6589 compatible UARTS
-- 
1.9.1

^ permalink raw reply related

* [PATCH v5 0/2] Add basic support for Mediatek MT2712 SoC
From: YT Shen @ 2017-07-28 11:37 UTC (permalink / raw)
  To: Rob Herring, Matthias Brugger
  Cc: Mark Rutland, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Greg Kroah-Hartman, Catalin Marinas, Will Deacon, Mars Cheng,
	YT Shen, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	srv_heupstream-NuS5LvNUpcJWk0Htik3J/w

MT2712 is a SoC based on 64bit ARMv8 architecture.
MT2712 share many HW IP with MT8173.  This patchset was tested on MT2712 evaluation board, and boot to shell ok.

This series contains document bindings, device tree including interrupt and uart.

Changes compared to v4:
- rebase to 4.13-rc1
- use two clocks (baud_clk & sys_clk) instead of one uart_clk to the correct bindings

Changes compared to v3:
- use two uart clocks refer to the bindings

Changes compared to v2:
- remove alias from serial1 to serial5
- remove initrd-start and initrd-end
- change GIC_CPU_MASK_SIMPLE(6) to GIC_CPU_MASK_RAW(0x13)
- change gic-400 reg range

Changes compared to v1:
- change subject prefix for bindings
- change device tree license to SPDX tag.
- change bootargs parameter to DT usage.
- change intpol-controller to interrupt-controller

YT Shen (2):
  dt-bindings: arm: Add bindings for Mediatek MT2712 SoC Platform
  arm64: dts: Add Mediatek SoC MT2712 and evaluation board dts and
    Makefile

 Documentation/devicetree/bindings/arm/mediatek.txt |   4 +
 .../interrupt-controller/mediatek,sysirq.txt       |   1 +
 .../devicetree/bindings/serial/mtk-uart.txt        |   1 +
 arch/arm64/boot/dts/mediatek/Makefile              |   1 +
 arch/arm64/boot/dts/mediatek/mt2712-evb.dts        |  32 ++++
 arch/arm64/boot/dts/mediatek/mt2712e.dtsi          | 178 +++++++++++++++++++++
 6 files changed, 217 insertions(+)
 create mode 100644 arch/arm64/boot/dts/mediatek/mt2712-evb.dts
 create mode 100644 arch/arm64/boot/dts/mediatek/mt2712e.dtsi

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 0/3] tty: serial: 8250 introduce mctrl_gpio helpers
From: Tony Lindgren @ 2017-07-28 11:29 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, jslaby-IBi9RG/b67k,
	robh-DgEjT+Ai2ygdnm+yROfE0A,
	mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
	uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1501238206.29303.277.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

* Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> [170728 03:37]:
> On Thu, 2017-07-27 at 23:40 -0700, Tony Lindgren wrote:
> > * yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> [170727
> > 06:18]:
> > > From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> > > 
> > > This patch series aims to reintroduce the  mctrl_gpio helpers for
> > > 8250
> > > UARTs.
> > > 
> > > There are some UARTs that use GPIO signals as a wakeup-sourse.
> > > The first patch addresses this issue and tries to destinguish GPIO
> > > usage
> > > via searching for "wakeup-sourse" property. Though it must be
> > > decided whether
> > > this property is secure to use for this purpose.
> > 
> > The wakeup-source part you should be able to handle pretty much
> > out of box with Linux generic wakeirqs if configured. See for example
> > drivers/i2c/i2c-core.c for the dev_pm_set_dedicated_wake_irq() part.
> 
> Here are 3 cases:
> 1) out-of-band interrupt (usually GPIO) which can be wake source;
> 2) in-band interrupt (have not seen this for UART, though it's supported
> by tty framework for a long time AFAIU);

And case 2) is somewhat useless as it requires the uart to be
clocked.

> 3) special case when Out-of-band interrupt uses the pin in UART pool of
> pins.
> 
> We are talking here about case 3).

OK at least for am335x/am437x, changing it's mode between gpio and
uart use can be done with pinctrl framework with named pinctrl
named modes.

> > As long as the 8250 driver has runtime PM implemented
> 
> As you remember it has not (the ugly hack which is used right now is not
> correct implementation of RPM).
> 
> >  it will wake
> > up the 8250 device.
> 
> See above, in case 3) we would be able to achieve that when pin mode is
> switched to GPIO and back. I don't remember if OMAP uses this approach.

Yes that's needed for am335x and am437x to get a wake-up event from
rx pin for example. Not needed for omap3/4/5.

> > This should work just fine also with am335x gpios, just configure the
> > secondary wakeup gpio interrupt using interrupts-extended in device
> > tree. Typically the interrupts are named "irq" and "wakeup". And if
> > the
> > pin is used as gpio, you can just dev_pm_clear_wake_irq() during
> > runtime.
> 
> I think this one is not related to case 3).

Hmm but it seems we already have that working for 8250_omap for
omap3/4/5 where rx pin provides wakeup events when 8250_omap is
in runtime suspend state. See the dev_pm_set_dedicated_wake_irq()
parts in 8250_omap.c. For am335x/am437x I think the only thing
missing is toggling between uart and gpio mode for the rx pin
using pinctrl named modes. There's an example for am335x in
omap_hsmmc.c with pinctrl_pm_select_idle_state() if that helps.

But maybe you have something in case 3) that I don't follow?

> > If having issues, we're still missing the wakeirq level configuration,
> > the patch below should do the trick there.
> 
> >  	err = request_threaded_irq(irq, NULL,
> > handle_threaded_wake_irq,
> > -				   IRQF_ONESHOT, dev_name(dev),
> > wirq);
> > +				   irq_get_trigger_type(irq) |
> > IRQF_ONESHOT,
> 
> This is not needed if you use DT or ACPI and framework (serial in this
> case) checks for interrupt correctly.

I think it's needed in the DT gpio interrupt case as some
gpio controllers need to configure edge vs level.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 0/3] tty: serial: 8250 introduce mctrl_gpio helpers
From: Andy Shevchenko @ 2017-07-28 10:36 UTC (permalink / raw)
  To: Tony Lindgren, yegorslists-gM/Ye1E23mwN+BqQ9rBEUg
  Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, jslaby-IBi9RG/b67k,
	robh-DgEjT+Ai2ygdnm+yROfE0A,
	mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
	uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170728064004.GI10026-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>

On Thu, 2017-07-27 at 23:40 -0700, Tony Lindgren wrote:
> * yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> [170727
> 06:18]:
> > From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> > 
> > This patch series aims to reintroduce the  mctrl_gpio helpers for
> > 8250
> > UARTs.
> > 
> > There are some UARTs that use GPIO signals as a wakeup-sourse.
> > The first patch addresses this issue and tries to destinguish GPIO
> > usage
> > via searching for "wakeup-sourse" property. Though it must be
> > decided whether
> > this property is secure to use for this purpose.
> 
> The wakeup-source part you should be able to handle pretty much
> out of box with Linux generic wakeirqs if configured. See for example
> drivers/i2c/i2c-core.c for the dev_pm_set_dedicated_wake_irq() part.

Here are 3 cases:
1) out-of-band interrupt (usually GPIO) which can be wake source;
2) in-band interrupt (have not seen this for UART, though it's supported
by tty framework for a long time AFAIU);
3) special case when Out-of-band interrupt uses the pin in UART pool of
pins.

We are talking here about case 3).

> As long as the 8250 driver has runtime PM implemented

As you remember it has not (the ugly hack which is used right now is not
correct implementation of RPM).

>  it will wake
> up the 8250 device.

See above, in case 3) we would be able to achieve that when pin mode is
switched to GPIO and back. I don't remember if OMAP uses this approach.

> This should work just fine also with am335x gpios, just configure the
> secondary wakeup gpio interrupt using interrupts-extended in device
> tree. Typically the interrupts are named "irq" and "wakeup". And if
> the
> pin is used as gpio, you can just dev_pm_clear_wake_irq() during
> runtime.

I think this one is not related to case 3).

> If having issues, we're still missing the wakeirq level configuration,
> the patch below should do the trick there.

>  	err = request_threaded_irq(irq, NULL,
> handle_threaded_wake_irq,
> -				   IRQF_ONESHOT, dev_name(dev),
> wirq);
> +				   irq_get_trigger_type(irq) |
> IRQF_ONESHOT,

This is not needed if you use DT or ACPI and framework (serial in this
case) checks for interrupt correctly.

-- 
Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/3] serial: mctrl_gpio: restrict MCTRL initialization
From: Yegor Yefremov @ 2017-07-28  8:42 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Greg KH,
	Jiri Slaby, Tony Lindgren, Rob Herring, Mika Westerberg,
	Uwe Kleine-König,
	linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1501164092.29303.270.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

On Thu, Jul 27, 2017 at 4:01 PM, Andy Shevchenko
<andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> On Thu, 2017-07-27 at 15:17 +0200, yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org wrote:
>> From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>>
>> GPIOs specified for serial port can be used either as GPIO driven
>> MCTRL
>> or as a wakeup-source. Use the latter property to abort the MCTRL
>> initialization.
>
> Please, elaborate on a choice of error code for this (-ENOSYS).

Something like this:

Return -ENOSYS as this would just skip mctrl initialization in the
case of wakeup source usage and not disable the port completely due to
failed serial8250_register_8250_port().

Yegor

>>
>> Cc: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>
> Suggested-by: I suppose.
>
> I dunno about documentation part for this property, I leave it for DT
> people to decide. Below part looks good to me.
>
>> Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>> ---
>>  drivers/tty/serial/serial_mctrl_gpio.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c
>> b/drivers/tty/serial/serial_mctrl_gpio.c
>> index d2da6aa..dce661c 100644
>> --- a/drivers/tty/serial/serial_mctrl_gpio.c
>> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
>> @@ -19,6 +19,7 @@
>>  #include <linux/irq.h>
>>  #include <linux/gpio/consumer.h>
>>  #include <linux/termios.h>
>> +#include <linux/property.h>
>>  #include <linux/serial_core.h>
>>  #include <linux/module.h>
>>
>> @@ -118,6 +119,9 @@ struct mctrl_gpios *mctrl_gpio_init_noauto(struct
>> device *dev, unsigned int idx)
>>       struct mctrl_gpios *gpios;
>>       enum mctrl_gpio_idx i;
>>
>> +     if (device_property_present(dev, "wakeup-source"))
>> +             return ERR_PTR(-ENOSYS);
>> +
>>       gpios = devm_kzalloc(dev, sizeof(*gpios), GFP_KERNEL);
>>       if (!gpios)
>>               return ERR_PTR(-ENOMEM);
>
> --
> Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 0/3] tty: serial: 8250 introduce mctrl_gpio helpers
From: Tony Lindgren @ 2017-07-28  6:40 UTC (permalink / raw)
  To: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg
  Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA, jslaby-IBi9RG/b67k,
	robh-DgEjT+Ai2ygdnm+yROfE0A,
	mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
	uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1501161456-13367-1-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

* yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> [170727 06:18]:
> From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> 
> This patch series aims to reintroduce the  mctrl_gpio helpers for 8250
> UARTs.
> 
> There are some UARTs that use GPIO signals as a wakeup-sourse.
> The first patch addresses this issue and tries to destinguish GPIO usage
> via searching for "wakeup-sourse" property. Though it must be decided whether
> this property is secure to use for this purpose.

The wakeup-source part you should be able to handle pretty much
out of box with Linux generic wakeirqs if configured. See for example
drivers/i2c/i2c-core.c for the dev_pm_set_dedicated_wake_irq() part.
As long as the 8250 driver has runtime PM implemented it will wake
up the 8250 device.

This should work just fine also with am335x gpios, just configure the
secondary wakeup gpio interrupt using interrupts-extended in device
tree. Typically the interrupts are named "irq" and "wakeup". And if the
pin is used as gpio, you can just dev_pm_clear_wake_irq() during
runtime.

If having issues, we're still missing the wakeirq level configuration,
the patch below should do the trick there.

Regards,

Tony

8< -----------
diff --git a/drivers/base/power/wakeirq.c b/drivers/base/power/wakeirq.c
--- a/drivers/base/power/wakeirq.c
+++ b/drivers/base/power/wakeirq.c
@@ -198,7 +198,8 @@ int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq)
 	 * so we use a threaded irq.
 	 */
 	err = request_threaded_irq(irq, NULL, handle_threaded_wake_irq,
-				   IRQF_ONESHOT, dev_name(dev), wirq);
+				   irq_get_trigger_type(irq) | IRQF_ONESHOT,
+				   dev_name(dev), wirq);
 	if (err)
 		goto err_free;
 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] [for 4.13] tty: pl011: fix initialization order of QDF2400 E44
From: Timur Tabi @ 2017-07-27 21:15 UTC (permalink / raw)
  To: Russell King, Greg Kroah-Hartman, Jiri Slaby, linux-arm-kernel,
	linux-serial
  Cc: timur

The work-around for Qualcomm Technologies QDF2400 Erratum 44 hinges on a
global variable defined in the pl011 driver.  The ACPI SPCR parsing code
determines whether the work-around is needed, and if so, it changes the
console name from "pl011" to "qdf2400_e44".  The expectation is that
the pl011 driver will implement the work-around when it sees the console
name.  The global variable qdf2400_e44_present is set when that happens.

The problem is that work-around needs to be enabled when the pl011
driver probes, not when the console name is queried.  However, sbsa_probe()
is called before pl011_console_match().  The work-around appeared to work
previously because the default console on QDF2400 platforms was always
ttyAMA1.  The first time sbsa_probe() is called (for ttyAMA0),
qdf2400_e44_present is still false.  Then pl011_console_match() is called,
and it sets qdf2400_e44_present to true.  All subsequent calls to
sbsa_probe() enable the work-around.

The solution is to move the global variable into spcr.c and let the
pl011 driver query it during probe time.  This works because all QDF2400
platforms require SPCR, so parse_spcr() will always be called.
pl011_console_match still checks for the "qdf2400_e44" console name,
but it doesn't do anything else special.

Fixes: 5a0722b898f8 ("tty: pl011: use "qdf2400_e44" as the earlycon name for QDF2400 E44")
Tested-by: Jeffrey Hugo <jhugo@codeaurora.org>
Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
 drivers/acpi/spcr.c             | 36 ++++++++++++++++++++++++++++++++++--
 drivers/tty/serial/amba-pl011.c | 37 +++++++++++++++++++------------------
 include/linux/acpi.h            |  1 +
 3 files changed, 54 insertions(+), 20 deletions(-)

diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
index 4ac3e06b41d8..98aa8c808a33 100644
--- a/drivers/acpi/spcr.c
+++ b/drivers/acpi/spcr.c
@@ -17,6 +17,16 @@
 #include <linux/serial_core.h>
 
 /*
+ * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
+ * occasionally getting stuck as 1. To avoid the potential for a hang, check
+ * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
+ * implementations, so only do so if an affected platform is detected in
+ * parse_spcr().
+ */
+bool qdf2400_e44_present;
+EXPORT_SYMBOL(qdf2400_e44_present);
+
+/*
  * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
  * Detect them by examining the OEM fields in the SPCR header, similiar to PCI
  * quirk detection in pci_mcfg.c.
@@ -147,8 +157,30 @@ int __init parse_spcr(bool earlycon)
 		goto done;
 	}
 
-	if (qdf2400_erratum_44_present(&table->header))
-		uart = "qdf2400_e44";
+	/*
+	 * If the E44 erratum is required, then we need to tell the pl011
+	 * driver to implement the work-around.
+	 *
+	 * The global variable is used by the probe function when it
+	 * creates the UARTs, whether or not they're used as a console.
+	 *
+	 * If the user specifies "traditional" earlycon, the qdf2400_e44
+	 * console name matches the EARLYCON_DECLARE() statement, and
+	 * SPCR is not used.  Parameter "earlycon" is false.
+	 *
+	 * If the user specifies "SPCR" earlycon, then we need to update
+	 * the console name so that it also says "qdf2400_e44".  Parameter
+	 * "earlycon" is true.
+	 *
+	 * For consistency, if we change the console name, then we do it
+	 * for everyone, not just earlycon.
+	 */
+	if (qdf2400_erratum_44_present(&table->header)) {
+		qdf2400_e44_present = true;
+		if (earlycon)
+			uart = "qdf2400_e44";
+	}
+
 	if (xgene_8250_erratum_present(table))
 		iotype = "mmio32";
 
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 8a857bb34fbb..1888d168a41c 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -142,15 +142,7 @@ static unsigned int get_fifosize_arm(struct amba_device *dev)
 	.fixed_options		= true,
 };
 
-/*
- * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
- * occasionally getting stuck as 1. To avoid the potential for a hang, check
- * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
- * implementations, so only do so if an affected platform is detected in
- * parse_spcr().
- */
-static bool qdf2400_e44_present = false;
-
+#ifdef CONFIG_ACPI_SPCR_TABLE
 static struct vendor_data vendor_qdt_qdf2400_e44 = {
 	.reg_offset		= pl011_std_offsets,
 	.fr_busy		= UART011_FR_TXFE,
@@ -165,6 +157,7 @@ static unsigned int get_fifosize_arm(struct amba_device *dev)
 	.always_enabled		= true,
 	.fixed_options		= true,
 };
+#endif
 
 static u16 pl011_st_offsets[REG_ARRAY_SIZE] = {
 	[REG_DR] = UART01x_DR,
@@ -2375,12 +2368,14 @@ static int __init pl011_console_match(struct console *co, char *name, int idx,
 	resource_size_t addr;
 	int i;
 
-	if (strcmp(name, "qdf2400_e44") == 0) {
-		pr_info_once("UART: Working around QDF2400 SoC erratum 44");
-		qdf2400_e44_present = true;
-	} else if (strcmp(name, "pl011") != 0) {
+	/*
+	 * Systems affected by the Qualcomm Technologies QDF2400 E44 erratum
+	 * have a distinct console name, so make sure we check for that.
+	 * The actual implementation of the erratum occurs in the probe
+	 * function.
+	 */
+	if ((strcmp(name, "qdf2400_e44") != 0) && (strcmp(name, "pl011") != 0))
 		return -ENODEV;
-	}
 
 	if (uart_parse_earlycon(options, &iotype, &addr, &options))
 		return -ENODEV;
@@ -2734,11 +2729,17 @@ static int sbsa_uart_probe(struct platform_device *pdev)
 	}
 	uap->port.irq	= ret;
 
-	uap->reg_offset	= vendor_sbsa.reg_offset;
-	uap->vendor	= qdf2400_e44_present ?
-					&vendor_qdt_qdf2400_e44 : &vendor_sbsa;
+#ifdef CONFIG_ACPI_SPCR_TABLE
+	if (qdf2400_e44_present) {
+		dev_info(&pdev->dev, "working around QDF2400 SoC erratum 44\n");
+		uap->vendor = &vendor_qdt_qdf2400_e44;
+	} else
+#endif
+		uap->vendor = &vendor_sbsa;
+
+	uap->reg_offset	= uap->vendor->reg_offset;
 	uap->fifosize	= 32;
-	uap->port.iotype = vendor_sbsa.access_32b ? UPIO_MEM32 : UPIO_MEM;
+	uap->port.iotype = uap->vendor->access_32b ? UPIO_MEM32 : UPIO_MEM;
 	uap->port.ops	= &sbsa_uart_pops;
 	uap->fixed_baud = baudrate;
 
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index c749eef1daa1..27b4b6615263 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1209,6 +1209,7 @@ static inline void acpi_table_upgrade(void) { }
 #endif
 
 #ifdef CONFIG_ACPI_SPCR_TABLE
+extern bool qdf2400_e44_present;
 int parse_spcr(bool earlycon);
 #else
 static inline int parse_spcr(bool earlycon) { return 0; }
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply related

* Re: [PATCH 1/3] serial: mctrl_gpio: restrict MCTRL initialization
From: Andy Shevchenko @ 2017-07-27 14:55 UTC (permalink / raw)
  To: Uwe Kleine-König, yegorslists-gM/Ye1E23mwN+BqQ9rBEUg
  Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, jslaby-IBi9RG/b67k,
	tony-4v6yS6AI5VpBDgjK7y7TUQ, robh-DgEjT+Ai2ygdnm+yROfE0A,
	mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170727140008.myhk2w6mwxxinhyb-T6qyLwKrzP+Pq0V0m3QNwQq/OYV65a7L4Y2cMoPwMik@public.gmane.org>

On Thu, 2017-07-27 at 16:00 +0200, Uwe Kleine-König wrote:
> Hello,
> 
> On Thu, Jul 27, 2017 at 03:17:34PM +0200, yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org
> wrote:
> > GPIOs specified for serial port can be used either as GPIO driven
> > MCTRL
> > or as a wakeup-source. Use the latter property to abort the MCTRL
> > initialization.
> 
> Why is that an either-or and not an and?

That is how mctrl is designed right now.

Note marking the pin as a wakeup source inside mctrl driver will not
help since there is much bigger issue, i.e. pinctrl is not ready for a
such.

-- 
Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/3] serial: mctrl_gpio: restrict MCTRL initialization
From: Andy Shevchenko @ 2017-07-27 14:01 UTC (permalink / raw)
  To: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg,
	linux-serial-u79uwXL29TY76Z2rM5mHXA
  Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, jslaby-IBi9RG/b67k,
	tony-4v6yS6AI5VpBDgjK7y7TUQ, robh-DgEjT+Ai2ygdnm+yROfE0A,
	mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
	uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1501161456-13367-2-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

On Thu, 2017-07-27 at 15:17 +0200, yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org wrote:
> From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> 
> GPIOs specified for serial port can be used either as GPIO driven
> MCTRL
> or as a wakeup-source. Use the latter property to abort the MCTRL
> initialization.

Please, elaborate on a choice of error code for this (-ENOSYS).

> 
> Cc: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Suggested-by: I suppose.

I dunno about documentation part for this property, I leave it for DT
people to decide. Below part looks good to me.

> Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  drivers/tty/serial/serial_mctrl_gpio.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/tty/serial/serial_mctrl_gpio.c
> b/drivers/tty/serial/serial_mctrl_gpio.c
> index d2da6aa..dce661c 100644
> --- a/drivers/tty/serial/serial_mctrl_gpio.c
> +++ b/drivers/tty/serial/serial_mctrl_gpio.c
> @@ -19,6 +19,7 @@
>  #include <linux/irq.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/termios.h>
> +#include <linux/property.h>
>  #include <linux/serial_core.h>
>  #include <linux/module.h>
>  
> @@ -118,6 +119,9 @@ struct mctrl_gpios *mctrl_gpio_init_noauto(struct
> device *dev, unsigned int idx)
>  	struct mctrl_gpios *gpios;
>  	enum mctrl_gpio_idx i;
>  
> +	if (device_property_present(dev, "wakeup-source"))
> +		return ERR_PTR(-ENOSYS);
> +
>  	gpios = devm_kzalloc(dev, sizeof(*gpios), GFP_KERNEL);
>  	if (!gpios)
>  		return ERR_PTR(-ENOMEM);

-- 
Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/3] serial: mctrl_gpio: restrict MCTRL initialization
From: Uwe Kleine-König @ 2017-07-27 14:00 UTC (permalink / raw)
  To: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg
  Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA, jslaby-IBi9RG/b67k,
	tony-4v6yS6AI5VpBDgjK7y7TUQ, robh-DgEjT+Ai2ygdnm+yROfE0A,
	mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1501161456-13367-2-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

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

Hello,

On Thu, Jul 27, 2017 at 03:17:34PM +0200, yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org wrote:
> GPIOs specified for serial port can be used either as GPIO driven MCTRL
> or as a wakeup-source. Use the latter property to abort the MCTRL
> initialization.

Why is that an either-or and not an and?

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* [PATCH 3/3] tty/serial/8250: add DT bindings description for mctrl_gpio helpers
From: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg @ 2017-07-27 13:17 UTC (permalink / raw)
  To: linux-serial-u79uwXL29TY76Z2rM5mHXA
  Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA, jslaby-IBi9RG/b67k,
	tony-4v6yS6AI5VpBDgjK7y7TUQ, robh-DgEjT+Ai2ygdnm+yROfE0A,
	mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
	uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Yegor Yefremov
In-Reply-To: <1501161456-13367-1-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

Describe {rts,cts,dtr,dsr,rng,dcd}-gpios usage.

Cc: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 Documentation/devicetree/bindings/serial/8250.txt | 26 +++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
index 419ff6c..1c09db2 100644
--- a/Documentation/devicetree/bindings/serial/8250.txt
+++ b/Documentation/devicetree/bindings/serial/8250.txt
@@ -48,6 +48,16 @@ Optional properties:
 - tx-threshold: Specify the TX FIFO low water indication for parts with
   programmable TX FIFO thresholds.
 - resets : phandle + reset specifier pairs
+- rts-gpios:
+- cts-gpios:
+- dtr-gpios:
+- dsr-gpios:
+- rng-gpios:
+- dcd-gpios: specify a GPIO for RTS/CTS/DTR/DSR/RI/DCD
+  line respectively. It will use specified GPIO instead of the peripheral
+  function pin for the UART feature. If unsure, don't specify this property.
+  If "wakeup-source" property is specified for the device this GPIO
+  initialization will be aborted.
 
 Note:
 * fsl,ns16550:
@@ -69,3 +79,19 @@ Example:
 		interrupts = <10>;
 		reg-shift = <2>;
 	};
+
+Example for OMAP UART using GPIO-based modem control signals:
+
+	uart4: serial@49042000 {
+		compatible = "ti,omap3-uart";
+		reg = <0x49042000 0x400>;
+		interrupts = <80>;
+		ti,hwmods = "uart4";
+		clock-frequency = <48000000>;
+		cts-gpios = <&gpio3 5 GPIO_ACTIVE_LOW>;
+		rts-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
+		dtr-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
+		dsr-gpios = <&gpio1 13 GPIO_ACTIVE_LOW>;
+		dcd-gpios = <&gpio1 14 GPIO_ACTIVE_LOW>;
+		rng-gpios = <&gpio1 15 GPIO_ACTIVE_LOW>;
+	};
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 2/3] tty/serial/8250: use mctrl_gpio helpers
From: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg @ 2017-07-27 13:17 UTC (permalink / raw)
  To: linux-serial-u79uwXL29TY76Z2rM5mHXA
  Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA, jslaby-IBi9RG/b67k,
	tony-4v6yS6AI5VpBDgjK7y7TUQ, robh-DgEjT+Ai2ygdnm+yROfE0A,
	mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
	uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Yegor Yefremov
In-Reply-To: <1501161456-13367-1-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

This patch permits the usage for GPIOs to control
the CTS/RTS/DTR/DSR/DCD/RI signals.

Cc: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 drivers/tty/serial/8250/8250.h      | 35 ++++++++++++++++++++++++++++++++++-
 drivers/tty/serial/8250/8250_core.c |  9 +++++++++
 drivers/tty/serial/8250/8250_omap.c | 31 ++++++++++++++++++-------------
 drivers/tty/serial/8250/8250_port.c |  7 ++++++-
 drivers/tty/serial/8250/Kconfig     |  1 +
 include/linux/serial_8250.h         |  1 +
 6 files changed, 69 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index b2bdc35..c47ef69 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -15,6 +15,8 @@
 #include <linux/serial_reg.h>
 #include <linux/dmaengine.h>
 
+#include "../serial_mctrl_gpio.h"
+
 struct uart_8250_dma {
 	int (*tx_dma)(struct uart_8250_port *p);
 	int (*rx_dma)(struct uart_8250_port *p);
@@ -145,12 +147,43 @@ void serial8250_em485_destroy(struct uart_8250_port *p);
 
 static inline void serial8250_out_MCR(struct uart_8250_port *up, int value)
 {
+	int mctrl_gpio = 0;
+
 	serial_out(up, UART_MCR, value);
+
+	if (value & UART_MCR_RTS)
+		mctrl_gpio |= TIOCM_RTS;
+	if (value & UART_MCR_DTR)
+		mctrl_gpio |= TIOCM_DTR;
+
+	mctrl_gpio_set(up->gpios, mctrl_gpio);
 }
 
 static inline int serial8250_in_MCR(struct uart_8250_port *up)
 {
-	return serial_in(up, UART_MCR);
+	int mctrl, mctrl_gpio = 0;
+
+	mctrl = serial_in(up, UART_MCR);
+
+	/* save current MCR values */
+	if (mctrl & UART_MCR_RTS)
+		mctrl_gpio |= TIOCM_RTS;
+	if (mctrl & UART_MCR_DTR)
+		mctrl_gpio |= TIOCM_DTR;
+
+	mctrl_gpio = mctrl_gpio_get_outputs(up->gpios, &mctrl_gpio);
+
+	if (mctrl_gpio & TIOCM_RTS)
+		mctrl |= UART_MCR_RTS;
+	else
+		mctrl &= ~UART_MCR_RTS;
+
+	if (mctrl_gpio & TIOCM_DTR)
+		mctrl |= UART_MCR_DTR;
+	else
+		mctrl &= ~UART_MCR_DTR;
+
+	return mctrl;
 }
 
 #if defined(__alpha__) && !defined(CONFIG_PCI)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index b5def35..7a3772e 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -975,6 +975,8 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
 
 	uart = serial8250_find_match_or_unused(&up->port);
 	if (uart && uart->port.type != PORT_8250_CIR) {
+		struct mctrl_gpios *gpios;
+
 		if (uart->port.dev)
 			uart_remove_one_port(&serial8250_reg, &uart->port);
 
@@ -1012,6 +1014,13 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
 		if (up->port.flags & UPF_FIXED_TYPE)
 			uart->port.type = up->port.type;
 
+		gpios = mctrl_gpio_init(&uart->port, 0);
+		if (IS_ERR(gpios)) {
+			if (PTR_ERR(gpios) != -ENOSYS)
+				return PTR_ERR(gpios);
+		} else
+			uart->gpios = gpios;
+
 		serial8250_set_defaults(uart);
 
 		/* Possibly override default I/O functions.  */
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 833771b..93fe5b9 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -134,18 +134,21 @@ static void omap8250_set_mctrl(struct uart_port *port, unsigned int mctrl)
 
 	serial8250_do_set_mctrl(port, mctrl);
 
-	/*
-	 * Turn off autoRTS if RTS is lowered and restore autoRTS setting
-	 * if RTS is raised
-	 */
-	lcr = serial_in(up, UART_LCR);
-	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
-	if ((mctrl & TIOCM_RTS) && (port->status & UPSTAT_AUTORTS))
-		priv->efr |= UART_EFR_RTS;
-	else
-		priv->efr &= ~UART_EFR_RTS;
-	serial_out(up, UART_EFR, priv->efr);
-	serial_out(up, UART_LCR, lcr);
+	if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(up->gpios,
+						UART_GPIO_RTS))) {
+		/*
+		 * Turn off autoRTS if RTS is lowered and restore autoRTS
+		 * setting if RTS is raised
+		 */
+		lcr = serial_in(up, UART_LCR);
+		serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+		if ((mctrl & TIOCM_RTS) && (port->status & UPSTAT_AUTORTS))
+			priv->efr |= UART_EFR_RTS;
+		else
+			priv->efr &= ~UART_EFR_RTS;
+		serial_out(up, UART_EFR, priv->efr);
+		serial_out(up, UART_LCR, lcr);
+	}
 }
 
 /*
@@ -446,7 +449,9 @@ static void omap_8250_set_termios(struct uart_port *port,
 	priv->efr = 0;
 	up->port.status &= ~(UPSTAT_AUTOCTS | UPSTAT_AUTORTS | UPSTAT_AUTOXOFF);
 
-	if (termios->c_cflag & CRTSCTS && up->port.flags & UPF_HARD_FLOW) {
+	if (termios->c_cflag & CRTSCTS && up->port.flags & UPF_HARD_FLOW
+		&& IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(up->gpios,
+							UART_GPIO_RTS))) {
 		/* Enable AUTOCTS (autoRTS is enabled when RTS is raised) */
 		up->port.status |= UPSTAT_AUTOCTS | UPSTAT_AUTORTS;
 		priv->efr |= UART_EFR_CTS;
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index a5fe0e6..f173cda 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1631,6 +1631,8 @@ static void serial8250_disable_ms(struct uart_port *port)
 	if (up->bugs & UART_BUG_NOMSR)
 		return;
 
+	mctrl_gpio_disable_ms(up->gpios);
+
 	up->ier &= ~UART_IER_MSI;
 	serial_port_out(port, UART_IER, up->ier);
 }
@@ -1643,6 +1645,8 @@ static void serial8250_enable_ms(struct uart_port *port)
 	if (up->bugs & UART_BUG_NOMSR)
 		return;
 
+	mctrl_gpio_enable_ms(up->gpios);
+
 	up->ier |= UART_IER_MSI;
 
 	serial8250_rpm_get(up);
@@ -1946,7 +1950,8 @@ unsigned int serial8250_do_get_mctrl(struct uart_port *port)
 		ret |= TIOCM_DSR;
 	if (status & UART_MSR_CTS)
 		ret |= TIOCM_CTS;
-	return ret;
+
+	return mctrl_gpio_get(up->gpios, &ret);
 }
 EXPORT_SYMBOL_GPL(serial8250_do_get_mctrl);
 
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index a1161ec..c0e51b0 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -6,6 +6,7 @@
 config SERIAL_8250
 	tristate "8250/16550 and compatible serial support"
 	select SERIAL_CORE
+	select SERIAL_MCTRL_GPIO if GPIOLIB
 	---help---
 	  This selects whether you want to include the driver for the standard
 	  serial ports.  The standard answer is Y.  People who might say N
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index 61fbb44..23b6ab1 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -113,6 +113,7 @@ struct uart_8250_port {
 						 *   if no_console_suspend
 						 */
 	unsigned char		probe;
+	struct mctrl_gpios	*gpios;
 #define UART_PROBE_RSA	(1 << 0)
 
 	/*
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 1/3] serial: mctrl_gpio: restrict MCTRL initialization
From: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg @ 2017-07-27 13:17 UTC (permalink / raw)
  To: linux-serial-u79uwXL29TY76Z2rM5mHXA
  Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA, jslaby-IBi9RG/b67k,
	tony-4v6yS6AI5VpBDgjK7y7TUQ, robh-DgEjT+Ai2ygdnm+yROfE0A,
	mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
	uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Yegor Yefremov
In-Reply-To: <1501161456-13367-1-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

GPIOs specified for serial port can be used either as GPIO driven MCTRL
or as a wakeup-source. Use the latter property to abort the MCTRL
initialization.

Cc: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 drivers/tty/serial/serial_mctrl_gpio.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index d2da6aa..dce661c 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -19,6 +19,7 @@
 #include <linux/irq.h>
 #include <linux/gpio/consumer.h>
 #include <linux/termios.h>
+#include <linux/property.h>
 #include <linux/serial_core.h>
 #include <linux/module.h>
 
@@ -118,6 +119,9 @@ struct mctrl_gpios *mctrl_gpio_init_noauto(struct device *dev, unsigned int idx)
 	struct mctrl_gpios *gpios;
 	enum mctrl_gpio_idx i;
 
+	if (device_property_present(dev, "wakeup-source"))
+		return ERR_PTR(-ENOSYS);
+
 	gpios = devm_kzalloc(dev, sizeof(*gpios), GFP_KERNEL);
 	if (!gpios)
 		return ERR_PTR(-ENOMEM);
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 0/3] tty: serial: 8250 introduce mctrl_gpio helpers
From: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg @ 2017-07-27 13:17 UTC (permalink / raw)
  To: linux-serial-u79uwXL29TY76Z2rM5mHXA
  Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA, jslaby-IBi9RG/b67k,
	tony-4v6yS6AI5VpBDgjK7y7TUQ, robh-DgEjT+Ai2ygdnm+yROfE0A,
	mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
	uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Yegor Yefremov

From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

This patch series aims to reintroduce the  mctrl_gpio helpers for 8250
UARTs.

There are some UARTs that use GPIO signals as a wakeup-sourse.
The first patch addresses this issue and tries to destinguish GPIO usage
via searching for "wakeup-sourse" property. Though it must be decided whether
this property is secure to use for this purpose.

Yegor Yefremov (3):
  serial: mctrl_gpio: restrict MCTRL initialization
  tty/serial/8250: use mctrl_gpio helpers
  tty/serial/8250: add DT bindings description for mctrl_gpio helpers

 Documentation/devicetree/bindings/serial/8250.txt | 26 +++++++++++++++++
 drivers/tty/serial/8250/8250.h                    | 35 ++++++++++++++++++++++-
 drivers/tty/serial/8250/8250_core.c               |  9 ++++++
 drivers/tty/serial/8250/8250_omap.c               | 31 +++++++++++---------
 drivers/tty/serial/8250/8250_port.c               |  7 ++++-
 drivers/tty/serial/8250/Kconfig                   |  1 +
 drivers/tty/serial/serial_mctrl_gpio.c            |  4 +++
 include/linux/serial_8250.h                       |  1 +
 8 files changed, 99 insertions(+), 15 deletions(-)

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/2] printk/console: Enhance the check for consoles using init memory
From: Petr Mladek @ 2017-07-27 10:08 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Sergey Senozhatsky, Steven Rostedt, Andrew Morton, Peter Zijlstra,
	Matt Redfearn, Greg Kroah-Hartman, Jiri Slaby, David S. Miller,
	Alan Cox, Fabio M. Di Nitto, linux-serial, linux-kernel
In-Reply-To: <20170727095100.GA653@jagdpanzerIV.localdomain>

On Thu 2017-07-27 18:51:01, Sergey Senozhatsky wrote:
> On (07/27/17 11:29), Petr Mladek wrote:
> [..]
> > > > Signed-off-by: Petr Mladek <pmladek@suse.com>
> > > 
> > > Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > 
> > Thanks for the review. I am going to push the two patches into
> > for-4.14 branch so that we could get some testing via linux-next.
> 
> good. agree.
> 
> how do you think,
> 
> would pr_warn() be enough for people to notice the wrongdoing or
> shall we put WARN_ON() there for a while at least? (assuming that
> people pay more attention to backtraces).

I would keep pr_warn(). The warning is interesting only when people
want to debug and the real console does not appear in time.
Then this will be the last but one message on the early console.

Best Regards,
Petr

^ permalink raw reply

* Re: [PATCH 2/2] printk/console: Enhance the check for consoles using init memory
From: Sergey Senozhatsky @ 2017-07-27  9:52 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Sergey Senozhatsky, Sergey Senozhatsky, Steven Rostedt,
	Andrew Morton, Peter Zijlstra, Matt Redfearn, Greg Kroah-Hartman,
	Jiri Slaby, David S. Miller, Alan Cox, Fabio M. Di Nitto,
	linux-serial, linux-kernel
In-Reply-To: <20170727092825.GC3799@pathway.suse.cz>

On (07/27/17 11:28), Petr Mladek wrote:
> > well, the console output is far from perfect, so I can imagine future
> > changes ;)
> 
> Sure and we will need to deal with it. Anyway, I still thing that this
> check is better than nothing. Even if we "fix" all consoles and move
> them out of init section then this check will be useful to catch at least
> some future mistakes.

yep, let's keep it.

[..]
> > let's move. to normal section, or to special section. depending on how
> > much space we can saved unloading the consoles.
> 
> I agree. We will do or suggest this when anyone see the warning
> and ask for help.

works for me.

	-ss

^ permalink raw reply

* Re: [PATCH 2/2] printk/console: Enhance the check for consoles using init memory
From: Sergey Senozhatsky @ 2017-07-27  9:51 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Sergey Senozhatsky, Sergey Senozhatsky, Steven Rostedt,
	Andrew Morton, Peter Zijlstra, Matt Redfearn, Greg Kroah-Hartman,
	Jiri Slaby, David S. Miller, Alan Cox, Fabio M. Di Nitto,
	linux-serial, linux-kernel
In-Reply-To: <20170727092957.GD3799@pathway.suse.cz>

On (07/27/17 11:29), Petr Mladek wrote:
[..]
> > > Signed-off-by: Petr Mladek <pmladek@suse.com>
> > 
> > Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> 
> Thanks for the review. I am going to push the two patches into
> for-4.14 branch so that we could get some testing via linux-next.

good. agree.

how do you think,

would pr_warn() be enough for people to notice the wrongdoing or
shall we put WARN_ON() there for a while at least? (assuming that
people pay more attention to backtraces).

	-ss

^ permalink raw reply

* Re: [PATCH 2/2] printk/console: Enhance the check for consoles using init memory
From: Petr Mladek @ 2017-07-27  9:29 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Sergey Senozhatsky, Steven Rostedt, Andrew Morton, Peter Zijlstra,
	Matt Redfearn, Greg Kroah-Hartman, Jiri Slaby, David S. Miller,
	Alan Cox, Fabio M. Di Nitto, linux-serial, linux-kernel
In-Reply-To: <20170726130804.GB2173@jagdpanzerIV.localdomain>

On Wed 2017-07-26 22:08:04, Sergey Senozhatsky wrote:
> On (07/14/17 14:51), Petr Mladek wrote:
> > printk_late_init() is responsible for disabling boot consoles that
> > use init memory. It checks the address of struct console for this.
> > 
> > But this is not enough. For example, there are several early
> > consoles that have write() method in the init section and
> > struct console in the normal section. They are not disabled
> > and could cause fancy and hard to debug system states.
> > 
> > It is even more complicated by the macros EARLYCON_DECLARE() and
> > OF_EARLYCON_DECLARE() where various struct members are set at
> > runtime by the provided setup() function.
> > 
> > I have tried to reproduce this problem and forced the classic uart
> > early console to stay using keep_bootcon parameter. In particular
> > I used earlycon=uart,io,0x3f8 keep_bootcon console=ttyS0,115200.
> > The system did not boot:
> > 
> > [    1.570496] PM: Image not found (code -22)
> > [    1.570496] PM: Image not found (code -22)
> > [    1.571886] PM: Hibernation image not present or could not be loaded.
> > [    1.571886] PM: Hibernation image not present or could not be loaded.
> > [    1.576407] Freeing unused kernel memory: 2528K
> > [    1.577244] kernel tried to execute NX-protected page - exploit attempt? (uid: 0)
> > 
> > The double lines are caused by having both early uart console and
> > ttyS0 console enabled at the same time. The early console stopped
> > working when the init memory was freed. Fortunately, the invalid
> > call was caught by the NX-protexted page check and did not cause
> > any silent fancy problems.
> > 
> > This patch adds a check for many other addresses stored in
> > struct console. It omits setup() and match() that are used
> > only when the console is registered. Therefore they have
> > already been used at this point and there is no reason
> > to use them again.
> > 
> > Signed-off-by: Petr Mladek <pmladek@suse.com>
> 
> Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

Thanks for the review. I am going to push the two patches into
for-4.14 branch so that we could get some testing via linux-next.

Best Regards,
Petr

^ permalink raw reply

* Re: [PATCH 2/2] printk/console: Enhance the check for consoles using init memory
From: Petr Mladek @ 2017-07-27  9:28 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Sergey Senozhatsky, Steven Rostedt, Andrew Morton, Peter Zijlstra,
	Matt Redfearn, Greg Kroah-Hartman, Jiri Slaby, David S. Miller,
	Alan Cox, Fabio M. Di Nitto, linux-serial, linux-kernel
In-Reply-To: <20170724020356.GA515@jagdpanzerIV.localdomain>

On Mon 2017-07-24 11:03:56, Sergey Senozhatsky wrote:
> Hello,
> 
> On (07/21/17 16:32), Petr Mladek wrote:
> [..]
> > > sort of a problem here is that the next time anyone adds a new ->foo()
> > > callback to struct console, that person also needs to remember to update
> > > printk_late_init().
> > 
> > I am not super happy with this as well. Any hint how to do it better
> > or more secure is welcome. But I do not see a beter solution at the moment.
> > 
> > Note that there are only 3 commits in the git history that change this
> > structure. Neither of them invalidates this check!
> 
> well, the console output is far from perfect, so I can imagine future
> changes ;)

Sure and we will need to deal with it. Anyway, I still thing that this
check is better than nothing. Even if we "fix" all consoles and move
them out of init section then this check will be useful to catch at least
some future mistakes.


> to fix the rootcause of the problem, and not its aftershock, we still
> need to either:
> 	a) move consoles to normal section
> or
> 	b) move consoles to a special section
> 
> I don't mind that warning, but I think we also need to tweak the
> affected consoles. otherwise, upon maintainer's request to keep
> bootcon, a user can just report back "uses init memory and must
> be disabled even before the real one is ready" warning, yet the
> kernel still would crash (a theoretical case, but for some reason
> someone wanted to keep bootcon after all).

I would leave this to the maintainers and developers of the respective
architectures. It would be nice to fix it now but I am not sure if
I would be able to do and test the changes properly and effectively.

Also it might be hard to sell. Note that it makes sense to keep early
con in the init section when the related real console is registered
during kernel initialization (no deferred probe) before late init
calls.


> > Instead it would make sense to move all these consoles into the normal
> > section. But it is not strictly needed if the normal console is
> > registered using an init call (always in time). In this case, it is "enough"
> > to mention the real console as the last one on the command line.
> 
> let's move. to normal section, or to special section. depending on how
> much space we can saved unloading the consoles.

I agree. We will do or suggest this when anyone see the warning
and ask for help.

Best Regards,
Petr

^ permalink raw reply

* Re: [PATCH 2/2] printk/console: Enhance the check for consoles using init memory
From: Sergey Senozhatsky @ 2017-07-26 13:08 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Sergey Senozhatsky, Steven Rostedt, Andrew Morton, Peter Zijlstra,
	Matt Redfearn, Greg Kroah-Hartman, Jiri Slaby, David S. Miller,
	Alan Cox, Fabio M. Di Nitto, linux-serial, linux-kernel,
	Sergey Senozhatsky
In-Reply-To: <1500036673-7122-3-git-send-email-pmladek@suse.com>

On (07/14/17 14:51), Petr Mladek wrote:
> printk_late_init() is responsible for disabling boot consoles that
> use init memory. It checks the address of struct console for this.
> 
> But this is not enough. For example, there are several early
> consoles that have write() method in the init section and
> struct console in the normal section. They are not disabled
> and could cause fancy and hard to debug system states.
> 
> It is even more complicated by the macros EARLYCON_DECLARE() and
> OF_EARLYCON_DECLARE() where various struct members are set at
> runtime by the provided setup() function.
> 
> I have tried to reproduce this problem and forced the classic uart
> early console to stay using keep_bootcon parameter. In particular
> I used earlycon=uart,io,0x3f8 keep_bootcon console=ttyS0,115200.
> The system did not boot:
> 
> [    1.570496] PM: Image not found (code -22)
> [    1.570496] PM: Image not found (code -22)
> [    1.571886] PM: Hibernation image not present or could not be loaded.
> [    1.571886] PM: Hibernation image not present or could not be loaded.
> [    1.576407] Freeing unused kernel memory: 2528K
> [    1.577244] kernel tried to execute NX-protected page - exploit attempt? (uid: 0)
> 
> The double lines are caused by having both early uart console and
> ttyS0 console enabled at the same time. The early console stopped
> working when the init memory was freed. Fortunately, the invalid
> call was caught by the NX-protexted page check and did not cause
> any silent fancy problems.
> 
> This patch adds a check for many other addresses stored in
> struct console. It omits setup() and match() that are used
> only when the console is registered. Therefore they have
> already been used at this point and there is no reason
> to use them again.
> 
> Signed-off-by: Petr Mladek <pmladek@suse.com>

Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

	-ss

^ permalink raw reply

* Re: [PATCH 1/2] printk/console: Always disable boot consoles that use init memory before it is freed
From: Sergey Senozhatsky @ 2017-07-26 13:07 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Sergey Senozhatsky, Steven Rostedt, Andrew Morton, Peter Zijlstra,
	Matt Redfearn, Greg Kroah-Hartman, Jiri Slaby, David S. Miller,
	Alan Cox, Fabio M. Di Nitto, linux-serial, linux-kernel,
	Sergey Senozhatsky
In-Reply-To: <1500036673-7122-2-git-send-email-pmladek@suse.com>

On (07/14/17 14:51), Petr Mladek wrote:
> From: Matt Redfearn <matt.redfearn@imgtec.com>
> 
> Commit 4c30c6f566c0 ("kernel/printk: do not turn off bootconsole in
> printk_late_init() if keep_bootcon") added a check on keep_bootcon to
> ensure that boot consoles were kept around until the real console is
> registered.
> 
> This can lead to problems if the boot console data and code are in the
> init section, since it can be freed before the boot console is
> unregistered.
> 
> Commit 81cc26f2bd11 ("printk: only unregister boot consoles when
> necessary") fixed this a better way. It allowed to keep boot consoles
> that did not use init data. Unfortunately it did not remove the check
> of keep_bootcon.
> 
> This can lead to crashes and weird panics when the bootconsole is
> accessed after free, especially if page poisoning is in use and the
> code / data have been overwritten with a poison value.
> 
> To prevent this, always free the boot console if it is within the init
> section. In addition, print a warning about that the console is removed
> prematurely.
> 
> Finally there is a new comment how to avoid the warning. It replaced
> an explanation that duplicated a more comprehensive function
> description few lines above.
> 
> Fixes: 4c30c6f566c0 ("kernel/printk: do not turn off bootconsole in printk_late_init() if keep_bootcon")
> Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
> [pmladek@suse.com: print the warning, code and comments clean up]
> Signed-off-by: Petr Mladek <pmladek@suse.com>

Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

	-ss

^ permalink raw reply

* Re: [PATCH 0/5] earlycon hang under some conditions
From: Doug Anderson @ 2017-07-24 23:50 UTC (permalink / raw)
  To: Jeffy Chen
  Cc: linux-kernel@vger.kernel.org, Greg Kroah-Hartman, Brian Norris,
	Peter Hurley, Sören Brinkmann, Michal Simek, Marc Gonzalez,
	Jiri Slaby, Vineet Gupta, linux-serial, linux-snps-arc,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <1500357778-23169-1-git-send-email-jeffy.chen@rock-chips.com>

Hi,

On Mon, Jul 17, 2017 at 11:02 PM, Jeffy Chen <jeffy.chen@rock-chips.com> wrote:
> I was testing earlycon with 8250 dw serial console. And it hangs in
> these cases:
> 1/ kernel hang when calling early write function after free_initmem:
> a) the earlycon not disabled after the init code(due to keep_bootcon or
>    not specify a real console to switch to)
> b) the early write func is marked as __init, for example 8250_early.
>
> 2/ kernel hang when calling early write function after disable unused
> clks/pm domain:
> a) the earlycon not disabled after the init code
> b) the disable unused clks/pm domain kill the requiered clks/pm
>    domain, since they are not referenced by the earlycon.
>
> 3/ kernel hang when calling early write function after the serial
>    console driver runtime suspended:
> a) the earlycon not disabled after the init code
> b) the serial console driver's runtime suspend kills the requiered
>    clks/pm domain, since they are not referenced by the earlycon.
>
> This serial fix 1/ case only.
>
>
>
> Jeffy Chen (5):
>   serial: arc: Remove __init marking from early write
>   serial: omap: Remove __init marking from early write
>   serial: xuartps: Remove __init marking from early write
>   serial: 8250_ingenic: Remove __init marking from early write
>   serial: 8250_early: Remove __init marking from early write
>
>  drivers/tty/serial/8250/8250_early.c   |  8 ++++----
>  drivers/tty/serial/8250/8250_ingenic.c |  8 ++++----
>  drivers/tty/serial/arc_uart.c          |  4 ++--
>  drivers/tty/serial/omap-serial.c       | 13 ++++++-------
>  drivers/tty/serial/xilinx_uartps.c     |  2 +-
>  5 files changed, 17 insertions(+), 18 deletions(-)

I looked through all 5 patches and they seem sane to me.  I didn't go
through and test them since Brian already tested the patches for the
one UART driver I'd have access to anyway.

As per my previous email, I don't see any need to solve all the
world'd problems with this patch series, plus the keep_bootcon seems
to be an experts / debugging type option and it seems sane if you need
to take care in using it.

So officially for the series (FWIW since I'm just an interested 3rd party):

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


-Doug

^ permalink raw reply

* AW: [PATCH] serial: imx: disable DMA for RS-485 on i.MX6 SMP
From: Schenk, Gavin @ 2017-07-24 11:49 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Clemens Gruber, linux-serial@vger.kernel.org, Greg Kroah-Hartman,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	u.kleine-koenig@pengutronix.de, Fabio Estevam, Ian Jamison

Hi,

sorry for the delay!

> > Uwe Kleine-König had the idea to disable DMA on the i.MX6dl based
> platform via dts and this is our current workaround.
> 
> Please test Ian's patch:
> http://marc.info/?l=linux-serial&m=150005865213531&w=2
> 
> It should fix this DMA problem with RS485.

With this patch applied my test works fine on i.MX6dl in RS485-mode and DMA enabled.
 
Regards
Gavin Schenk



Eckelmann AG
Vorstand: Dipl.-Ing. Peter Frankenbach (Sprecher) Dipl.-Wi.-Ing. Philipp Eckelmann
Dr.-Ing. Marco Münchhof Dr.-Ing. Frank Uhlemann
Vorsitzender des Aufsichtsrats: Hubertus G. Krossa
Stv. Vorsitzender des Aufsichtsrats: Dr.-Ing. Gerd Eckelmann
Sitz der Gesellschaft: Berliner Str. 161, 65205 Wiesbaden, Amtsgericht Wiesbaden HRB 12636
http://www.eckelmann.de 


^ permalink raw reply

* Re: [PATCH 000/102] Convert drivers to explicit reset API
From: Philipp Zabel @ 2017-07-24  8:33 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Dmitry Torokhov, Thomas Petazzoni, lkml, Andrew Lunn,
	Prashant Gaikwad, Heiko Stuebner, Peter Chen, DRI, Marc Dietrich,
	Rakesh Iyer, Peter Meerwald-Stadler, linux-clk, Wim Van Sebroeck,
	Wolfram Sang, Xinliang Liu, Chanwoo Choi, Alan Stern, Jiri Slaby,
	Michael Turquette
In-Reply-To: <CACRpkdaXOv7mX+b-q1K34CB5w0SWPXCKa21wHaxL7qjf91PJXQ@mail.gmail.com>

On Sun, 2017-07-23 at 20:41 +0200, Linus Walleij wrote:
> On Thu, Jul 20, 2017 at 10:46 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Thu, Jul 20, 2017 at 5:55 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> 
> >>> What about reset_control_get(struct device *, const char *, int flags)
> >>> to replace all those variants ?
> >>
> >> While I like how this looks, unfortunately (devm_)reset_control_get
> >> already exists without the flags, so we can't change to that with a
> >> gentle transition.
> >
> > This was done for gpiod_get() and its flags argument with horrifying
> > #define-ry, which thankfully was completely hidden from users.
> 
> For your reference:
> 
> commit bae48da237fcedd7ad09569025483b988635efb7
> "gpiolib: add gpiod_get() and gpiod_put() functions"
> 
> commit 39b2bbe3d715cf5013b5c48695ccdd25bd3bf120
> "gpio: add flags argument to gpiod_get*() functions"
> 
> commit 0dbc8b7afef6e4fddcfebcbacbeb269a0a3b06d5
> "gpio: move varargs hack outside #ifdef GPIOLIB"
> 
> commit b17d1bf16cc72a374a48d748940f700009d40ff4
> "gpio: make flags mandatory for gpiod_get functions"
> 
> Retrospectively ... was that really a good idea... it was a LOT
> of trouble to add a flag, maybe it had been better to try and
> just slam all users in a single go.
> 
> But it worked.

Thanks for the hint and the references. It seems this turned out okay,
but I wouldn't dare to introduce such macro horror^Wmagic.
I'd rather have all users converted to the _exclusive/_shared function
calls and maybe then replace the internal __reset_control_get with
Thomas' suggestion.

regards
Philipp

^ 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