Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH v4 6/7] ARM: dts: meson8: switch to new bindings for UART nodes
From: Neil Armstrong @ 2017-06-09  9:49 UTC (permalink / raw)
  To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	khilman-rdvid1DuHRBWk0Htik3J/w
  Cc: Neil Armstrong, hgkr.klein-Re5JQEeQqe8AvxtiuMwx3w,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1497001756-942-1-git-send-email-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

Switch to the stable UART bindings by adding a XTAL node and using the
proper compatible strings.

Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 arch/arm/boot/dts/meson8.dtsi | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/meson8.dtsi b/arch/arm/boot/dts/meson8.dtsi
index 6993077..a2ea112 100644
--- a/arch/arm/boot/dts/meson8.dtsi
+++ b/arch/arm/boot/dts/meson8.dtsi
@@ -83,6 +83,13 @@
 		};
 	};
 
+	xtal: xtal-clk {
+		compatible = "fixed-clock";
+		clock-frequency = <24000000>;
+		clock-output-names = "xtal";
+		#clock-cells = <0>;
+	};
+
 	clk81: clk@0 {
 		#clock-cells = <0>;
 		compatible = "fixed-clock";
@@ -199,17 +206,25 @@
 };
 
 &uart_AO {
-	clocks = <&clk81>;
+	compatible = "amlogic,meson8-uart", "amlogic,meson-ao-uart";
+	clocks = <&xtal>, <&clk81>, <&clk81>;
+	clock-names = "xtal", "pclk", "baud";
 };
 
 &uart_A {
-	clocks = <&clk81>;
+	compatible = "amlogic,meson8-uart";
+	clocks = <&xtal>, <&clk81>, <&clk81>;
+	clock-names = "xtal", "pclk", "baud";
 };
 
 &uart_B {
-	clocks = <&clk81>;
+	compatible = "amlogic,meson8-uart";
+	clocks = <&xtal>, <&clk81>, <&clk81>;
+	clock-names = "xtal", "pclk", "baud";
 };
 
 &uart_C {
-	clocks = <&clk81>;
+	compatible = "amlogic,meson8-uart";
+	clocks = <&xtal>, <&clk81>, <&clk81>;
+	clock-names = "xtal", "pclk", "baud";
 };
-- 
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 related

* [PATCH v4 5/7] ARM: dts: meson6: switch to new bindings for UART nodes
From: Neil Armstrong @ 2017-06-09  9:49 UTC (permalink / raw)
  To: gregkh, khilman
  Cc: Neil Armstrong, hgkr.klein, linux-serial, linux-amlogic,
	linux-arm-kernel, linux-kernel, devicetree
In-Reply-To: <1497001756-942-1-git-send-email-narmstrong@baylibre.com>

Switch to the stable UART bindings by adding a XTAL node and using the
proper compatible strings.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 arch/arm/boot/dts/meson6.dtsi | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm/boot/dts/meson6.dtsi b/arch/arm/boot/dts/meson6.dtsi
index b0fc91f..a334fbe 100644
--- a/arch/arm/boot/dts/meson6.dtsi
+++ b/arch/arm/boot/dts/meson6.dtsi
@@ -70,9 +70,37 @@
 		};
 	};
 
+	xtal: xtal-clk {
+		compatible = "fixed-clock";
+		clock-frequency = <24000000>;
+		clock-output-names = "xtal";
+		#clock-cells = <0>;
+	};
+
 	clk81: clk@0 {
 		#clock-cells = <0>;
 		compatible = "fixed-clock";
 		clock-frequency = <200000000>;
 	};
 }; /* end of / */
+
+
+&uart_AO {
+	clocks = <&xtal>, <&clk81>, <&clk81>;
+	clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_A {
+	clocks = <&xtal>, <&clk81>, <&clk81>;
+	clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_B {
+	clocks = <&xtal>, <&clk81>, <&clk81>;
+	clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_C {
+	clocks = <&xtal>, <&clk81>, <&clk81>;
+	clock-names = "xtal", "pclk", "baud";
+};
-- 
1.9.1

^ permalink raw reply related

* [PATCH v4 4/7] ARM: dts: meson: use meson6 UART compatible like other nodes
From: Neil Armstrong @ 2017-06-09  9:49 UTC (permalink / raw)
  To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	khilman-rdvid1DuHRBWk0Htik3J/w
  Cc: Neil Armstrong, hgkr.klein-Re5JQEeQqe8AvxtiuMwx3w,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1497001756-942-1-git-send-email-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

The UART bindings needs specifying a SoC family, use the meson6 family
for the UART nodes like the other nodes.

Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 arch/arm/boot/dts/meson.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/meson.dtsi b/arch/arm/boot/dts/meson.dtsi
index 8d9c369..ecc9330 100644
--- a/arch/arm/boot/dts/meson.dtsi
+++ b/arch/arm/boot/dts/meson.dtsi
@@ -79,14 +79,14 @@
 			ranges = <0x0 0xc1100000 0x200000>;
 
 			uart_A: serial@84c0 {
-				compatible = "amlogic,meson-uart";
+				compatible = "amlogic,meson6-uart";
 				reg = <0x84c0 0x18>;
 				interrupts = <0 26 1>;
 				status = "disabled";
 			};
 
 			uart_B: serial@84dc {
-				compatible = "amlogic,meson-uart";
+				compatible = "amlogic,meson6-uart";
 				reg = <0x84dc 0x18>;
 				interrupts = <0 75 1>;
 				status = "disabled";
@@ -102,7 +102,7 @@
 			};
 
 			uart_C: serial@8700 {
-				compatible = "amlogic,meson-uart";
+				compatible = "amlogic,meson6-uart";
 				reg = <0x8700 0x18>;
 				interrupts = <0 93 1>;
 				status = "disabled";
@@ -153,7 +153,7 @@
 			};
 
 			uart_AO: serial@4c0 {
-				compatible = "amlogic,meson-uart";
+				compatible = "amlogic,meson6-uart", "amlogic,meson-ao-uart";
 				reg = <0x4c0 0x18>;
 				interrupts = <0 90 1>;
 				status = "disabled";
-- 
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 related

* [PATCH v4 3/7] ARM64: dts: meson-gx: use stable UART bindings with correct gate clock
From: Neil Armstrong @ 2017-06-09  9:49 UTC (permalink / raw)
  To: gregkh, khilman
  Cc: Helmut Klein, linux-serial, linux-amlogic, linux-arm-kernel,
	linux-kernel, devicetree, Neil Armstrong
In-Reply-To: <1497001756-942-1-git-send-email-narmstrong@baylibre.com>

From: Helmut Klein <hgkr.klein@gmail.com>

This patch switches to the stable UART bindings but also add the correct
gate clock to the non-AO UART nodes for GXBB and GXL SoCs.

Signed-off-by: Helmut Klein <hgkr.klein@gmail.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 arch/arm64/boot/dts/amlogic/meson-gx.dtsi   | 12 +++++-------
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 25 +++++++++++++++++++++++++
 arch/arm64/boot/dts/amlogic/meson-gxl.dtsi  | 25 +++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index 436b875..d2b8d52 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -225,7 +225,7 @@
 			};
 
 			uart_A: serial@84c0 {
-				compatible = "amlogic,meson-uart";
+				compatible = "amlogic,meson-gx-uart";
 				reg = <0x0 0x84c0 0x0 0x14>;
 				interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
 				clocks = <&xtal>;
@@ -233,7 +233,7 @@
 			};
 
 			uart_B: serial@84dc {
-				compatible = "amlogic,meson-uart";
+				compatible = "amlogic,meson-gx-uart";
 				reg = <0x0 0x84dc 0x0 0x14>;
 				interrupts = <GIC_SPI 75 IRQ_TYPE_EDGE_RISING>;
 				clocks = <&xtal>;
@@ -279,7 +279,7 @@
 			};
 
 			uart_C: serial@8700 {
-				compatible = "amlogic,meson-uart";
+				compatible = "amlogic,meson-gx-uart";
 				reg = <0x0 0x8700 0x0 0x14>;
 				interrupts = <GIC_SPI 93 IRQ_TYPE_EDGE_RISING>;
 				clocks = <&xtal>;
@@ -366,18 +366,16 @@
 			};
 
 			uart_AO: serial@4c0 {
-				compatible = "amlogic,meson-uart";
+				compatible = "amlogic,meson-gx-uart", "amlogic,meson-ao-uart";
 				reg = <0x0 0x004c0 0x0 0x14>;
 				interrupts = <GIC_SPI 193 IRQ_TYPE_EDGE_RISING>;
-				clocks = <&xtal>;
 				status = "disabled";
 			};
 
 			uart_AO_B: serial@4e0 {
-				compatible = "amlogic,meson-uart";
+				compatible = "amlogic,meson-gx-uart", "amlogic,meson-ao-uart";
 				reg = <0x0 0x004e0 0x0 0x14>;
 				interrupts = <GIC_SPI 197 IRQ_TYPE_EDGE_RISING>;
-				clocks = <&xtal>;
 				status = "disabled";
 			};
 
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index dbd300f..1ae8da7 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -675,6 +675,31 @@
 	clocks = <&clkc CLKID_SPI>;
 };
 
+&uart_A {
+	clocks = <&xtal>, <&clkc CLKID_UART0>, <&xtal>;
+	clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_AO {
+	clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
+	clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_AO_B {
+	clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
+	clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_B {
+	clocks = <&xtal>, <&clkc CLKID_UART1>, <&xtal>;
+	clock-names = "xtal", "core", "baud";
+};
+
+&uart_C {
+	clocks = <&xtal>, <&clkc CLKID_UART2>, <&xtal>;
+	clock-names = "xtal", "core", "baud";
+};
+
 &vpu {
 	compatible = "amlogic,meson-gxbb-vpu", "amlogic,meson-gx-vpu";
 };
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
index 4dfc22b..0c601f8 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
@@ -616,6 +616,31 @@
 	clocks = <&clkc CLKID_SPI>;
 };
 
+&uart_A {
+	clocks = <&xtal>, <&clkc CLKID_UART0>, <&xtal>;
+	clock-names = "xtal", "core", "baud";
+};
+
+&uart_AO {
+	clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
+	clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_AO_B {
+	clocks = <&xtal>, <&clkc CLKID_CLK81>, <&xtal>;
+	clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_B {
+	clocks = <&xtal>, <&clkc CLKID_UART1>, <&xtal>;
+	clock-names = "xtal", "core", "baud";
+};
+
+&uart_C {
+	clocks = <&xtal>, <&clkc CLKID_UART2>, <&xtal>;
+	clock-names = "xtal", "core", "baud";
+};
+
 &vpu {
 	compatible = "amlogic,meson-gxl-vpu", "amlogic,meson-gx-vpu";
 };
-- 
1.9.1

^ permalink raw reply related

* [PATCH v4 2/7] tty/serial: meson_uart: update to stable bindings
From: Neil Armstrong @ 2017-06-09  9:49 UTC (permalink / raw)
  To: gregkh, khilman
  Cc: Neil Armstrong, linux-kernel, Helmut Klein, linux-serial,
	linux-amlogic, linux-arm-kernel
In-Reply-To: <1497001756-942-1-git-send-email-narmstrong@baylibre.com>

From: Helmut Klein <hgkr.klein@gmail.com>

This patch handle the stable UART bindings but also keeps compatibility
with the legacy non-stable bindings until all boards uses them.

Signed-off-by: Helmut Klein <hgkr.klein@gmail.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/tty/serial/meson_uart.c | 109 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 103 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 60f1679..d2c8136 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -579,8 +579,12 @@ static void meson_serial_early_console_write(struct console *co,
 	device->con->write = meson_serial_early_console_write;
 	return 0;
 }
+/* Legacy bindings, should be removed when no more used */
 OF_EARLYCON_DECLARE(meson, "amlogic,meson-uart",
 		    meson_serial_early_console_setup);
+/* Stable bindings */
+OF_EARLYCON_DECLARE(meson, "amlogic,meson-ao-uart",
+		    meson_serial_early_console_setup);
 
 #define MESON_SERIAL_CONSOLE	(&meson_serial_console)
 #else
@@ -595,11 +599,95 @@ static void meson_serial_early_console_write(struct console *co,
 	.cons		= MESON_SERIAL_CONSOLE,
 };
 
+/*
+ * This function gets clocks in the legacy non-stable DT bindings.
+ * This code will be remove once all the platforms switch to the
+ * new DT bindings.
+ */
+static int meson_uart_probe_clocks_legacy(struct platform_device *pdev,
+					  struct uart_port *port)
+{
+	struct clk *clk = NULL;
+	int ret;
+
+	clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	ret = clk_prepare_enable(clk);
+	if (ret) {
+		dev_err(&pdev->dev, "couldn't enable clk\n");
+		return ret;
+	}
+
+	devm_add_action_or_reset(&pdev->dev,
+			(void(*)(void *))clk_disable_unprepare,
+			clk);
+
+	port->uartclk = clk_get_rate(clk);
+
+	return 0;
+}
+
+static int meson_uart_probe_clocks(struct platform_device *pdev,
+				   struct uart_port *port)
+{
+	struct clk *clk_xtal = NULL;
+	struct clk *clk_pclk = NULL;
+	struct clk *clk_baud = NULL;
+	int ret;
+
+	clk_pclk = devm_clk_get(&pdev->dev, "pclk");
+	if (IS_ERR(clk_pclk))
+		return PTR_ERR(clk_pclk);
+
+	clk_xtal = devm_clk_get(&pdev->dev, "xtal");
+	if (IS_ERR(clk_xtal))
+		return PTR_ERR(clk_xtal);
+
+	clk_baud = devm_clk_get(&pdev->dev, "baud");
+	if (IS_ERR(clk_xtal))
+		return PTR_ERR(clk_baud);
+
+	ret = clk_prepare_enable(clk_pclk);
+	if (ret) {
+		dev_err(&pdev->dev, "couldn't enable pclk\n");
+		return ret;
+	}
+
+	devm_add_action_or_reset(&pdev->dev,
+			(void(*)(void *))clk_disable_unprepare,
+			clk_pclk);
+
+	ret = clk_prepare_enable(clk_xtal);
+	if (ret) {
+		dev_err(&pdev->dev, "couldn't enable xtal\n");
+		return ret;
+	}
+
+	devm_add_action_or_reset(&pdev->dev,
+			(void(*)(void *))clk_disable_unprepare,
+			clk_xtal);
+
+	ret = clk_prepare_enable(clk_baud);
+	if (ret) {
+		dev_err(&pdev->dev, "couldn't enable baud clk\n");
+		return ret;
+	}
+
+	devm_add_action_or_reset(&pdev->dev,
+			(void(*)(void *))clk_disable_unprepare,
+			clk_baud);
+
+	port->uartclk = clk_get_rate(clk_baud);
+
+	return 0;
+}
+
 static int meson_uart_probe(struct platform_device *pdev)
 {
 	struct resource *res_mem, *res_irq;
 	struct uart_port *port;
-	struct clk *clk;
 	int ret = 0;
 
 	if (pdev->dev.of_node)
@@ -625,11 +713,15 @@ static int meson_uart_probe(struct platform_device *pdev)
 	if (!port)
 		return -ENOMEM;
 
-	clk = clk_get(&pdev->dev, NULL);
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
+	/* Use legacy way until all platforms switch to new bindings */
+	if (of_device_is_compatible(pdev->dev.of_node, "amlogic,meson-uart"))
+		ret = meson_uart_probe_clocks_legacy(pdev, port);
+	else
+		ret = meson_uart_probe_clocks(pdev, port);
+
+	if (ret)
+		return ret;
 
-	port->uartclk = clk_get_rate(clk);
 	port->iotype = UPIO_MEM;
 	port->mapbase = res_mem->start;
 	port->irq = res_irq->start;
@@ -668,9 +760,14 @@ static int meson_uart_remove(struct platform_device *pdev)
 	return 0;
 }
 
-
 static const struct of_device_id meson_uart_dt_match[] = {
+	/* Legacy bindings, should be removed when no more used */
 	{ .compatible = "amlogic,meson-uart" },
+	/* Stable bindings */
+	{ .compatible = "amlogic,meson6-uart" },
+	{ .compatible = "amlogic,meson8-uart" },
+	{ .compatible = "amlogic,meson8b-uart" },
+	{ .compatible = "amlogic,meson-gx-uart" },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, meson_uart_dt_match);
-- 
1.9.1

^ permalink raw reply related

* [PATCH v4 1/7] dt-bindings: serial: Add bindings for the Amlogic Meson UARTs
From: Neil Armstrong @ 2017-06-09  9:49 UTC (permalink / raw)
  To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	khilman-rdvid1DuHRBWk0Htik3J/w
  Cc: Helmut Klein, linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Neil Armstrong
In-Reply-To: <1497001756-942-1-git-send-email-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

From: Helmut Klein <hgkr.klein-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Add the documentation for the device tree binding of Amlogic Meson Serial UART.

Signed-off-by: Helmut Klein <hgkr.klein-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 .../bindings/serial/amlogic,meson-uart.txt         | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/amlogic,meson-uart.txt

diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.txt b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.txt
new file mode 100644
index 0000000..8ff65fa
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.txt
@@ -0,0 +1,38 @@
+Amlogic Meson SoC UART Serial Interface
+=======================================
+
+The Amlogic Meson SoC UART Serial Interface is present on a large range
+of SoCs, and can be present either in the "Always-On" power domain or the
+"Everything-Else" power domain.
+
+The particularity of the "Always-On" Serial Interface is that the hardware
+is active since power-on and does not need any clock gating and is usable
+as very early serial console.
+
+Required properties:
+- compatible : compatible: value should be different for each SoC family as :
+	- Meson6 : "amlogic,meson6-uart"
+	- Meson8 : "amlogic,meson8-uart"
+	- Meson8b : "amlogic,meson8b-uart"
+	- GX (GXBB, GXL, GXM) : "amlogic,meson-gx-uart"
+	eventually followed by : "amlogic,meson-ao-uart" if this UART interface
+	is in the "Always-On" power domain.
+- reg : offset and length of the register set for the device.
+- interrupts : identifier to the device interrupt
+- clocks : a list of phandle + clock-specifier pairs, one for each
+	   entry in clock names.
+- clocks-names :
+   * "xtal" for external xtal clock identifier
+   * "pclk" for the bus core clock, either the clk81 clock or the gate clock
+   * "baud" for the source of the baudrate generator, can be either the xtal
+	or the pclk.
+
+e.g.
+uart_A: serial@84c0 {
+	compatible = "amlogic,meson-gx-uart";
+	reg = <0x0 0x84c0 0x0 0x14>;
+	interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
+	/* Use xtal as baud rate clock source */
+	clocks = <&xtal>, <&clkc CLKID_UART0>, <&xtal>;
+	clock-names = "xtal", "pclk", "baud";
+};
-- 
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 related

* [PATCH v4 0/7] tty/serial: meson_uart: add support for core clock handling
From: Neil Armstrong @ 2017-06-09  9:49 UTC (permalink / raw)
  To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	khilman-rdvid1DuHRBWk0Htik3J/w
  Cc: Neil Armstrong, hgkr.klein-Re5JQEeQqe8AvxtiuMwx3w,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

This patchset is a re-spin of Helmut Klein's v3 patchset at [0].

Initially, the original patchset was made to enable usage on the non-AO UARTS
not enabled by the Bootloader (uart_B and uart_C), but the patchset needed
an overall change to have clean and stable DT bindings.

The Amlogic Meson UART Driver did not have stable DT bindings and mismatched
clock handling on non-AO UARTs since these "EE" UARTs needs a clock gate to
be ungated to works correctly.
In the same way, the AO UARTs does not need gating and can be used as
Early Consoles.

In the same time, the UART Interfaces can take clock input for the baudrate
generate from either the external Xtal or the internal Bus Clock (clk81).

So new bindings was necessary to meet these requirements and the DT
maintainers requirements.

The "legacy" binding actually used in the driver is left until all the DT
files are switched to the new bindings.

The GX DT has been tested, but the last 4 Meson6/Meson8/b are only
compile-tested, and testing is welcome.
Thus only the first 3 patches can be merged until the Meson6/Meson8/b are
formally tested.

It must be noted that the meson6 cannot work today except using an early
console since the UART driver could not probe without a clocks property.

[0] 20170331165437.26227-1-hgkr.klein-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

Helmut Klein (3):
  dt-bindings: serial: Add bindings for the Amlogic Meson UARTs
  tty/serial: meson_uart: update to stable bindings
  ARM64: dts: meson-gx: use stable UART bindings with correct gate clock

Neil Armstrong (4):
  ARM: dts: meson: use meson6 UART compatible like other nodes
  ARM: dts: meson6: switch to new bindings for UART nodes
  ARM: dts: meson8: switch to new bindings for UART nodes
  ARM: dts: meson8b: switch to new bindings for UART nodes

 .../bindings/serial/amlogic,meson-uart.txt         |  38 +++++++
 arch/arm/boot/dts/meson.dtsi                       |   8 +-
 arch/arm/boot/dts/meson6.dtsi                      |  28 ++++++
 arch/arm/boot/dts/meson8.dtsi                      |  23 ++++-
 arch/arm/boot/dts/meson8b.dtsi                     |  23 ++++-
 arch/arm64/boot/dts/amlogic/meson-gx.dtsi          |  12 +--
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi        |  25 +++++
 arch/arm64/boot/dts/amlogic/meson-gxl.dtsi         |  25 +++++
 drivers/tty/serial/meson_uart.c                    | 109 +++++++++++++++++++--
 9 files changed, 266 insertions(+), 25 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/serial/amlogic,meson-uart.txt

-- 
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 6/6] tty: serial: lpuart: add a more accurate baud rate calculation method
From: Andy Shevchenko @ 2017-06-09  9:26 UTC (permalink / raw)
  To: A.S. Dong
  Cc: linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm Mailing List, Greg Kroah-Hartman, Jiri Slaby, Andy Duan,
	Stefan Agner, Mingkai Hu, Y.B. Lu, Dong Aisheng
In-Reply-To: <AM3PR04MB306214514B820D66535E28B80CE0@AM3PR04MB306.eurprd04.prod.outlook.com>

On Fri, Jun 9, 2017 at 11:01 AM, A.S. Dong <aisheng.dong@nxp.com> wrote:

>> >> > +       u32 sbr, osr, baud_diff, tmp_osr, tmp_sbr, tmp_diff, tmp;
>> >> > +       u32 clk = sport->port.uartclk;
>> >> > +
>> >> > +       /*
>> >> > +        * The idea is to use the best OSR (over-sampling rate)
>> possible.
>> >> > +        * Note, OSR is typically hard-set to 16 in other LPUART
>> >> instantiations.
>> >> > +        * Loop to find the best OSR value possible, one that
>> >> > + generates
>> >> minimum
>> >> > +        * baud_diff iterate through the rest of the supported
>> >> > + values of
>> >> OSR.
>> >> > +        *
>> >> > +        * Calculation Formula:
>> >> > +        *  Baud Rate = baud clock / ((OSR+1) × SBR)
>> >> > +        */
>> >> > +       baud_diff = baudrate;
>> >> > +       osr = 0;
>> >> > +       sbr = 0;
>> >> > +
>> >>
>> >> > +       for (tmp_osr = 4; tmp_osr <= 32; tmp_osr++) {
>>
>> I missed one thing, what happened by default to OSR? What is the value in
>> use?
>>
>
> No valid default value. (osc/sbr are 0 by default)
> If no proper osc and sbr calculated, a WARNING will show.

Okay, so, it means the maximum supported speed is UART clock / 4. Correct?

>> So, the algo is the following:
>>
>> Assume the ranges like this:
>> OSR = [4 ... 32]
>> SBR = [2 ... 8192]
>>
>
> Baud Rate = baud clock / ((OSR+1) × SBR)
>
> In HW:
> OSR range : 3 – 31
> SBR range: 1 – 8191

I've read that, but think outside the box.

>> Then:
>>
>> 1. Get ratio factor as
>>       ratio = CLK / desired baud rate
>> 2. If ratio < 8192 * 9 / 2, just use (ratio / 4, 4) as (OSR, SBR) setting.
>> (Needs clarification on OSR < 4)
>
> Sorry that I'm a bit mess here.
> What is 8192 * 9 /2 meaning?

I forgot the details...

> And for (ratio / 4, 4) as (OSR,SBR), take 115200 as an example:
> Assuming baud clock 24Mhz.
>
> Ratio = 24000000 / 115200 = 208
> OSR = Ratio / 4 = 52
> Then OSR is out of range which seems wrong.

...yes...

>> 3. if ratio >= 8192 * 31, just use those
>> two numbers (8192, 31). You can't do anything better there.
>
> This actually may not happen.
> Even take a 9600 as example, the clk becomes:
> 8191 * 31 * 9600 = 2.4GHz
> Which is theoretically not exist.
>
>> 4. Otherwise, get a minimum required factor of OSR
>>       osr_min = ratio / 8192
>> 5. Start your loop from osr_min + 1 to 31.
>>
>> 6 (optional). Of course you may not consider baud_diff > osr_min, it's I
>> suppose obvious
>>
>> P.S. Note, all divisions by 2^n are just simple right shifts. Diffs are
>> calculated as multiplication of OSR and SBR in comparison to ratio. One
>> division so far.

> I'm not quite understand the approach.

...lemme prepare a python script demonstrating it.

> How about you send a separate baud algorithm improvement patch later?

Why not to do it right a way?

Just describe it in a comment if you afraid of reader can't understand
from the code.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v5 01/11] serial: exar: Leave MPIOs as output for Commtech adapters
From: Greg Kroah-Hartman @ 2017-06-09  8:29 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Jan Kiszka, Alexandre Courbot, Linux Kernel Mailing List,
	linux-serial@vger.kernel.org, linux-gpio@vger.kernel.org,
	Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <CACRpkdYeTGkp1e1gGAzhwXjS8dBCJiWror68Wov-j4g4t4zt2Q@mail.gmail.com>

On Fri, Jun 09, 2017 at 09:20:43AM +0200, Linus Walleij wrote:
> On Fri, Jun 2, 2017 at 9:28 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> 
> > Commtech adapters apparently need the original setting as outputs, see
> > https://marc.info/?l=linux-gpio&m=149557425201323&w=2. Account for that.
> >
> > Fixes: 7dea8165f1d6 ("serial: exar: Preconfigure xr17v35x MPIOs as output")
> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Greg can you pick this up for fixes?

Ok, will do, thanks.

greg k-h

^ permalink raw reply

* Re: [PATCH v5 03/11] gpio-exar/8250-exar: Fix passing in of parent PCI device
From: Greg Kroah-Hartman @ 2017-06-09  8:29 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Linus Walleij, Alexandre Courbot, Linux Kernel Mailing List,
	linux-serial, linux-gpio, Sudip Mukherjee, Andy Shevchenko,
	Sascha Weisenberger
In-Reply-To: <63100617e55d78ab881baaa6baf44e8d74d0e816.1496388533.git.jan.kiszka@siemens.com>

On Fri, Jun 02, 2017 at 09:28:46AM +0200, Jan Kiszka wrote:
> This fixes reloading of the GPIO driver for the same platform device
> instance as created by the exar UART driver: First of all, the driver
> sets drvdata to its own value during probing and does not restore the
> original value on exit. But this won't help anyway as the core clears
> drvdata after the driver left.
> 
> Set the platform device parent instead.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

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

^ permalink raw reply

* RE: [PATCH 6/6] tty: serial: lpuart: add a more accurate baud rate calculation method
From: A.S. Dong @ 2017-06-09  8:01 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm Mailing List, Greg Kroah-Hartman, Jiri Slaby, Andy Duan,
	Stefan Agner, Mingkai Hu, Y.B. Lu, Dong Aisheng
In-Reply-To: <CAHp75VcH0j3ddaZ3iStNO4QcAwJKZ7BAMwUTthum2no1ucs2ww@mail.gmail.com>

Hi Andy,

> -----Original Message-----
> From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com]
> Sent: Saturday, June 03, 2017 1:11 AM
> To: A.S. Dong
> Cc: linux-serial@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm
> Mailing List; Greg Kroah-Hartman; Jiri Slaby; Andy Duan; Stefan Agner;
> Mingkai Hu; Y.B. Lu
> Subject: Re: [PATCH 6/6] tty: serial: lpuart: add a more accurate baud
> rate calculation method
> 
> On Wed, May 31, 2017 at 5:18 PM, A.S. Dong <aisheng.dong@nxp.com> wrote:
> >> On Tue, May 9, 2017 at 10:50 AM, Dong Aisheng <aisheng.dong@nxp.com>
> wrote:
> 
> By some reason my previous message went privately.
> It didn't have anything major anyway and here I'm suggesting optimization
> of finding factors of the formula in use. See below.
> 
> >> > +       u32 sbr, osr, baud_diff, tmp_osr, tmp_sbr, tmp_diff, tmp;
> >> > +       u32 clk = sport->port.uartclk;
> >> > +
> >> > +       /*
> >> > +        * The idea is to use the best OSR (over-sampling rate)
> possible.
> >> > +        * Note, OSR is typically hard-set to 16 in other LPUART
> >> instantiations.
> >> > +        * Loop to find the best OSR value possible, one that
> >> > + generates
> >> minimum
> >> > +        * baud_diff iterate through the rest of the supported
> >> > + values of
> >> OSR.
> >> > +        *
> >> > +        * Calculation Formula:
> >> > +        *  Baud Rate = baud clock / ((OSR+1) × SBR)
> >> > +        */
> >> > +       baud_diff = baudrate;
> >> > +       osr = 0;
> >> > +       sbr = 0;
> >> > +
> >>
> >> > +       for (tmp_osr = 4; tmp_osr <= 32; tmp_osr++) {
> 
> I missed one thing, what happened by default to OSR? What is the value in
> use?
> 

No valid default value. (osc/sbr are 0 by default)
If no proper osc and sbr calculated, a WARNING will show.

> >> I _think_ you may simplify this and avoid for-loop if you reconsider
> >> approach.
> 
> > But there is indeed a optimization way, see below.
> 
> > To optimize the looping, we probably could do:
> > If (!baud_diff)
> >         Break;
> 
> It's a small one, we may have more interesting approach.
> 
> So, the algo is the following:
> 
> Assume the ranges like this:
> OSR = [4 ... 32]
> SBR = [2 ... 8192]
> 

Baud Rate = baud clock / ((OSR+1) × SBR)

In HW:
OSR range : 3 – 31
SBR range: 1 – 8191

> Then:
> 
> 1. Get ratio factor as
>       ratio = CLK / desired baud rate
> 2. If ratio < 8192 * 9 / 2, just use (ratio / 4, 4) as (OSR, SBR) setting.
> (Needs clarification on OSR < 4) 

Sorry that I'm a bit mess here.
What is 8192 * 9 /2 meaning?

And for (ratio / 4, 4) as (OSR,SBR), take 115200 as an example:
Assuming baud clock 24Mhz.

Ratio = 24000000 / 115200 = 208
OSR = Ratio / 4 = 52 
Then OSR is out of range which seems wrong.

> 3. if ratio >= 8192 * 31, just use those
> two numbers (8192, 31). You can't do anything better there.

This actually may not happen.
Even take a 9600 as example, the clk becomes:
8191 * 31 * 9600 = 2.4GHz
Which is theoretically not exist.

> 4. Otherwise, get a minimum required factor of OSR
>       osr_min = ratio / 8192
> 5. Start your loop from osr_min + 1 to 31.
> 
> 6 (optional). Of course you may not consider baud_diff > osr_min, it's I
> suppose obvious
> 
> P.S. Note, all divisions by 2^n are just simple right shifts. Diffs are
> calculated as multiplication of OSR and SBR in comparison to ratio. One
> division so far.
> 

I'm not quite understand the approach.

How about you send a separate baud algorithm improvement patch later?
Then it first can provide us a good patch history and also better to
understand for review.

Last, very appreciate for your kind and professional review.

Regards
Dong Aisheng

^ permalink raw reply

* Re: [PATCH v5 03/11] gpio-exar/8250-exar: Fix passing in of parent PCI device
From: Jan Kiszka @ 2017-06-09  7:44 UTC (permalink / raw)
  To: Linus Walleij, Greg Kroah-Hartman
  Cc: Alexandre Courbot, Linux Kernel Mailing List,
	linux-serial@vger.kernel.org, linux-gpio@vger.kernel.org,
	Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <CACRpkdah8Bgo2Ga5RK8+9=dqr0pizSYd50wqQk++TyycW6=xnQ@mail.gmail.com>

On 2017-06-09 09:21, Linus Walleij wrote:
> On Fri, Jun 2, 2017 at 9:28 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> 
>> This fixes reloading of the GPIO driver for the same platform device
>> instance as created by the exar UART driver: First of all, the driver
>> sets drvdata to its own value during probing and does not restore the
>> original value on exit. But this won't help anyway as the core clears
>> drvdata after the driver left.
>>
>> Set the platform device parent instead.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> If I get Gregs ACK on this I can merge patches 2-11 through the
> GPIO tree.
> 

Patch 10 needs an update, Andy had remarks. I need to resync and send it
- there will be v6 soon.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply

* Re: [PATCH v5 03/11] gpio-exar/8250-exar: Fix passing in of parent PCI device
From: Linus Walleij @ 2017-06-09  7:21 UTC (permalink / raw)
  To: Jan Kiszka, Greg Kroah-Hartman
  Cc: Alexandre Courbot, Linux Kernel Mailing List,
	linux-serial@vger.kernel.org, linux-gpio@vger.kernel.org,
	Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <63100617e55d78ab881baaa6baf44e8d74d0e816.1496388533.git.jan.kiszka@siemens.com>

On Fri, Jun 2, 2017 at 9:28 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:

> This fixes reloading of the GPIO driver for the same platform device
> instance as created by the exar UART driver: First of all, the driver
> sets drvdata to its own value during probing and does not restore the
> original value on exit. But this won't help anyway as the core clears
> drvdata after the driver left.
>
> Set the platform device parent instead.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

If I get Gregs ACK on this I can merge patches 2-11 through the
GPIO tree.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH v5 01/11] serial: exar: Leave MPIOs as output for Commtech adapters
From: Linus Walleij @ 2017-06-09  7:20 UTC (permalink / raw)
  To: Jan Kiszka, Greg Kroah-Hartman
  Cc: Alexandre Courbot, Linux Kernel Mailing List,
	linux-serial@vger.kernel.org, linux-gpio@vger.kernel.org,
	Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <0c7b59cdc5a4cb7035656b081e36457eb675fc29.1496388533.git.jan.kiszka@siemens.com>

On Fri, Jun 2, 2017 at 9:28 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:

> Commtech adapters apparently need the original setting as outputs, see
> https://marc.info/?l=linux-gpio&m=149557425201323&w=2. Account for that.
>
> Fixes: 7dea8165f1d6 ("serial: exar: Preconfigure xr17v35x MPIOs as output")
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Greg can you pick this up for fixes?

Yours,
Linus Walleij

^ permalink raw reply

* Re: CPU_BIG_ENDIAN in generic code
From: Arnd Bergmann @ 2017-06-08 14:36 UTC (permalink / raw)
  To: David Miller
  Cc: Babu Moger, Michael Ellerman, Geert Uytterhoeven, Peter Zijlstra,
	Ingo Molnar, sparclinux, Linux Kernel Mailing List, linux-arch,
	devicetree, linux-serial
In-Reply-To: <20170608.100222.1573468093040905016.davem@davemloft.net>

On Thu, Jun 8, 2017 at 4:02 PM, David Miller <davem@davemloft.net> wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Thu, 8 Jun 2017 10:01:59 +0200
>
>> I would also suggest adding a sanity check like
>
> Hmm, but this will kill the build for non-fixed endian architectures
> won't it?

I think only xtensa, all others already define CONFIG_CPU_BIG_ENDIAN
conditionally, and include the right header depending on that.

For xtensa, the decision is apparently made by the toolchain, and the
kernel just detects the macros set by the compiler, but that is slightly
fragile because it prevents us from making Kconfig decisions based on
endianess.

       Arnd

^ permalink raw reply

* Re: CPU_BIG_ENDIAN in generic code
From: David Miller @ 2017-06-08 14:02 UTC (permalink / raw)
  To: arnd
  Cc: babu.moger, mpe, geert, peterz, mingo, sparclinux, linux-kernel,
	linux-arch, devicetree, linux-serial
In-Reply-To: <CAK8P3a1WfU9V1k2r=TfJSVRTymuD8n2hkJDr6u1KcmC_scAZMA@mail.gmail.com>

From: Arnd Bergmann <arnd@arndb.de>
Date: Thu, 8 Jun 2017 10:01:59 +0200

> I would also suggest adding a sanity check like

Hmm, but this will kill the build for non-fixed endian architectures
won't it?

^ permalink raw reply

* Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
From: Arnd Bergmann @ 2017-06-08  8:01 UTC (permalink / raw)
  To: Babu Moger
  Cc: Michael Ellerman, Geert Uytterhoeven, David S. Miller,
	Peter Zijlstra, Ingo Molnar, sparclinux,
	linux-kernel@vger.kernel.org, Linux-Arch,
	devicetree@vger.kernel.org, linux-serial@vger.kernel.org
In-Reply-To: <28d6c874-4653-b15d-39b7-57a032abc9c6@oracle.com>

On Thu, Jun 8, 2017 at 1:07 AM, Babu Moger <babu.moger@oracle.com> wrote:
>
> On 5/29/2017 9:56 PM, Michael Ellerman wrote:
>>
>> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>>
>>> Hi Michael,
>>>
>>> On Mon, May 29, 2017 at 2:07 PM, Michael Ellerman <mpe@ellerman.id.au>
>>> wrote:
>>>>
>>>> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>>>>>
>>>>> On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger@oracle.com>
>>>>> wrote:

>>>
>>> So far we tried always doing that, but a few checks for the semi-existing
>>> Kconfig symbol crept in in generic code. Those could be replaced by the
>>> __*__
>>> variants, but consistently having the Kconfig symbols would be useful
>>> anyway
>>> (e.g. to avoid building the broken-on-big-endian ISDN drivers).
>>
>> Ah OK, the original mail was citing C code, but yeah I guess it would be
>> handy in Makefiles etc.
>
>
> Thanks for all the responses.  I see couple of options here.
>
> 1. Fix the c code in include/asm-generic/qrwlock.h using ifdef
> __BIG_ENDIAN__
>     This will fix the issue for us.
>
> 2. Define CPU_BIG_ENDIAN for all the missing fixed endian architectures.
> Because the problem is only for fixed big endian archs.
>
> I prefer the option 1.  What do you guys think ?

I would prefer option 2. If we leave out CONFIG_CPU_LITTLE_ENDIAN,
then the patch becomes much easier than what we had discussed earlier,
and we just need to patch a few Kconfig files to add

config CPU_BIG_ENDIAN
       def_bool y

I would also suggest adding a sanity check like

diff --git a/include/linux/byteorder/big_endian.h
b/include/linux/byteorder/big_endian.h
index 392041475c72..18a1ab5b0260 100644
--- a/include/linux/byteorder/big_endian.h
+++ b/include/linux/byteorder/big_endian.h
@@ -3,5 +3,9 @@

 #include <uapi/linux/byteorder/big_endian.h>

+#ifndef CONFIG_CPU_BIG_ENDIAN
+#warning inconsistent configuration, need CONFIG_CPU_BIG_ENDIAN
+#endif
+
 #include <linux/byteorder/generic.h>
 #endif /* _LINUX_BYTEORDER_BIG_ENDIAN_H */
diff --git a/include/linux/byteorder/little_endian.h
b/include/linux/byteorder/little_endian.h
index 08057377aa23..ba910bb9aad0 100644
--- a/include/linux/byteorder/little_endian.h
+++ b/include/linux/byteorder/little_endian.h
@@ -3,5 +3,9 @@

 #include <uapi/linux/byteorder/little_endian.h>

+#ifdef CONFIG_CPU_BIG_ENDIAN
+#warning inconsistent configuration, CONFIG_CPU_BIG_ENDIAN is set
+#endif
+
 #include <linux/byteorder/generic.h>
 #endif /* _LINUX_BYTEORDER_LITTLE_ENDIAN_H */

Fixing xtensa properly might still be tricky, but with that change, at least
we detect when things go wrong in this area.

        Arnd

^ permalink raw reply related

* Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)
From: Babu Moger @ 2017-06-07 23:07 UTC (permalink / raw)
  To: Michael Ellerman, Geert Uytterhoeven
  Cc: David S. Miller, Peter Zijlstra, Ingo Molnar, Arnd Bergmann,
	sparclinux, linux-kernel@vger.kernel.org, Linux-Arch,
	devicetree@vger.kernel.org, linux-serial@vger.kernel.org
In-Reply-To: <877f0z6oig.fsf@concordia.ellerman.id.au>


On 5/29/2017 9:56 PM, Michael Ellerman wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>
>> Hi Michael,
>>
>> On Mon, May 29, 2017 at 2:07 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>>> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>>>> On Tue, May 23, 2017 at 11:45 PM, Babu Moger <babu.moger@oracle.com> wrote:
>>>>> Found this problem while enabling queued rwlock on SPARC.
>>>>> The parameter CONFIG_CPU_BIG_ENDIAN is used to clear the
>>>>> specific byte in qrwlock structure. Without this parameter,
>>>>> we clear the wrong byte. Here is the code.
>>>>>
>>>>> static inline u8 *__qrwlock_write_byte(struct qrwlock *lock)
>>>>>   {
>>>>>          return (u8 *)lock + 3 * IS_BUILTIN(CONFIG_CPU_BIG_ENDIAN);
>>>>>   }
>>>>>
>>>>> Define CPU_BIG_ENDIAN for SPARC to fix it.
>>>>> --- a/arch/sparc/Kconfig
>>>>> +++ b/arch/sparc/Kconfig
>>>>> @@ -92,6 +92,10 @@ config ARCH_DEFCONFIG
>>>>>   config ARCH_PROC_KCORE_TEXT
>>>>>          def_bool y
>>>>>
>>>>> +config CPU_BIG_ENDIAN
>>>>> +       bool
>>>>> +       default y if SPARC
>>>> Nice catch!
>>>>
>>>> Traditionally, CPU_BIG_ENDIAN and CPU_LITTLE_ENDIAN were defined only on
>>>> architectures that may support both.  And it was checked in platform code
>>>> and drivers only.
>>>> Hence the symbol is lacking from most architectures. Heck, even
>>>> architectures that support both may default to one endiannes, and declare
>>>> only the symbol for the other endianness:
>>> I guess there's a reason we can't use __BIG_ENDIAN__ / __LITTLE_ENDIAN__ ?
>> I (C/asm) code we can, in Kconfig we cannot.
>>
>> So far we tried always doing that, but a few checks for the semi-existing
>> Kconfig symbol crept in in generic code. Those could be replaced by the __*__
>> variants, but consistently having the Kconfig symbols would be useful anyway
>> (e.g. to avoid building the broken-on-big-endian ISDN drivers).
> Ah OK, the original mail was citing C code, but yeah I guess it would be
> handy in Makefiles etc.

Thanks for all the responses.  I see couple of options here.

1. Fix the c code in include/asm-generic/qrwlock.h using ifdef 
__BIG_ENDIAN__
     This will fix the issue for us.

2. Define CPU_BIG_ENDIAN for all the missing fixed endian architectures. 
Because the problem is only for fixed big endian archs.

I prefer the option 1.  What do you guys think ?

> cheers

^ permalink raw reply

* Re: [PATCH v3] serial: 8250_of: Add reset support
From: Rob Herring @ 2017-06-07 20:10 UTC (permalink / raw)
  To: Joel Stanley
  Cc: Greg Kroah-Hartman, Mark Rutland, Philipp Zabel,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170529095753.1210-1-joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>

On Mon, May 29, 2017 at 07:27:52PM +0930, Joel Stanley wrote:
> This adds the hooks for an optional reset controller in the 8250 device
> tree node.
> 
> Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
> Reviewed-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> v3:
>  - deassert reset after disabling clock in _remove to to match _setup
>  order
> 
> v2:
>  Address Philipp's comments. Thanks for the review!
>  - use _shared variant
>  - remove unnecessary error handling
> 
>  Documentation/devicetree/bindings/serial/8250.txt |  1 +
>  drivers/tty/serial/8250/8250_of.c                 | 10 ++++++++++
>  2 files changed, 11 insertions(+)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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 v4 16/28] tty: serial: owl: Implement console driver
From: Andy Shevchenko @ 2017-06-07 14:37 UTC (permalink / raw)
  To: Andreas Färber
  Cc: support, 张天益, Greg Kroah-Hartman, 96boards,
	linux-kernel@vger.kernel.org, Thomas Liau, mp-cs,
	linux-serial@vger.kernel.org, 刘炜, Jiri Slaby,
	linux-arm Mailing List, 张东风
In-Reply-To: <20170606005426.26446-17-afaerber@suse.de>

On Tue, Jun 6, 2017 at 3:54 AM, Andreas Färber <afaerber@suse.de> wrote:
> Implement serial console driver to complement earlycon.
>
> Based on LeMaker linux-actions tree.

> +#define OWL_UART_CTL_DWLS_MASK         (0x3 << 0)

GENMASK() ?

> +#define OWL_UART_CTL_PRS_MASK          (0x7 << 4)

Ditto.

>  #define OWL_UART_STAT_TRFL_MASK                (0x1f << 11)

Ditto.

> +static struct owl_uart_port *owl_uart_ports[OWL_UART_PORT_NUM];

And this is needed for...?

> +static void owl_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
> +{
> +}

Do we need an empty stub?

> +static void owl_uart_send_chars(struct uart_port *port)
> +{

> +               xmit->tail = (xmit->tail + 1) & (SERIAL_XMIT_SIZE - 1);

% SERIAL_XMIT_SIZE shorter (I hope it's a power of 2), but it's up to you.

> +}

> +static irqreturn_t owl_uart_irq(int irq, void *dev_id)
> +{
> +       struct uart_port *port = (struct uart_port *)dev_id;

Useless casting.

> +       spin_lock(&port->lock);

spin_lock_irqsave() ?

Consider the kernel command option that switches all IRQ handlers to
be threaded.

> +}

> +static void owl_uart_change_baudrate(struct owl_uart_port *owl_port,
> +                                    unsigned long baud)
> +{
> +       clk_set_rate(owl_port->clk, baud * 8);
> +}

> +static void owl_uart_release_port(struct uart_port *port)
> +{
> +       struct platform_device *pdev = to_platform_device(port->dev);
> +       struct resource *res;
> +

> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res)
> +               return;
> +
> +       if (port->flags & UPF_IOREMAP) {
> +               devm_release_mem_region(port->dev, port->mapbase,
> +                       resource_size(res));
> +               devm_iounmap(port->dev, port->membase);
> +               port->membase = NULL;
> +       }

Above is entirely redundant.

> +}
> +
> +static int owl_uart_request_port(struct uart_port *port)
> +{
> +       struct platform_device *pdev = to_platform_device(port->dev);
> +       struct resource *res;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res)
> +               return -ENXIO;
> +
> +       if (!devm_request_mem_region(port->dev, port->mapbase,
> +                       resource_size(res), dev_name(port->dev)))
> +               return -EBUSY;
> +
> +       if (port->flags & UPF_IOREMAP) {
> +               port->membase = devm_ioremap_nocache(port->dev, port->mapbase,
> +                               resource_size(res));
> +               if (!port->membase)
> +                       return -EBUSY;
> +       }
> +
> +       return 0;
> +}

> +static void owl_uart_config_port(struct uart_port *port, int flags)
> +{

> +       if (flags & UART_CONFIG_TYPE) {

if (!(...))
 return;

?

> +               port->type = PORT_OWL;
> +               owl_uart_request_port(port);
> +       }
> +}


> +static int owl_uart_probe(struct platform_device *pdev)
> +{
> +       struct resource *res_mem, *res_irq;
> +       struct owl_uart_port *owl_port;
> +       int ret;
> +
> +       if (pdev->dev.of_node)
> +               pdev->id = of_alias_get_id(pdev->dev.of_node, "serial");
> +
> +       if (pdev->id < 0 || pdev->id >= OWL_UART_PORT_NUM) {
> +               dev_err(&pdev->dev, "id %d out of range\n", pdev->id);
> +               return -EINVAL;
> +       }
> +


> +       res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res_mem) {
> +               dev_err(&pdev->dev, "could not get mem\n");
> +               return -ENODEV;
> +       }

You can use

struct resource *mem;

mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
x = devm_ioremap_resource();
if (IS_ERR(x))
 return PTR_ERR(x);

and remote IOREMAP flag below.

> +       res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +       if (!res_irq) {
> +               dev_err(&pdev->dev, "could not get irq\n");
> +               return -ENODEV;
> +       }

platform_get_irq()

> +       if (owl_uart_ports[pdev->id]) {
> +               dev_err(&pdev->dev, "port %d already allocated\n", pdev->id);
> +               return -EBUSY;
> +       }

I guess it's redundant. It should be conflicting by resource (for example, IRQ).

> +       owl_port->clk = clk_get(&pdev->dev, NULL);

devm_ ?

> +       owl_port->port.iotype = UPIO_MEM;
> +       owl_port->port.mapbase = res_mem->start;
> +       owl_port->port.irq = res_irq->start;


> +       owl_port->port.uartclk = clk_get_rate(owl_port->clk);

You need to check if it's 0 and use device property instead or bail out.

> +       owl_port->port.flags = UPF_BOOT_AUTOCONF | UPF_IOREMAP | UPF_LOW_LATENCY;
> +       owl_port->port.x_char = 0;
> +       owl_port->port.fifosize = 16;
> +       owl_port->port.ops = &owl_uart_ops;
> +
> +       owl_uart_ports[pdev->id] = owl_port;

> +       platform_set_drvdata(pdev, owl_port);

It should be just before return 0, right?..

> +
> +       ret = uart_add_one_port(&owl_uart_driver, &owl_port->port);
> +       if (ret)
> +               owl_uart_ports[pdev->id] = NULL;

...and thus, taking into consideration redundancy of that global var:

      ret = uart_add_one_port(&owl_uart_driver, &owl_port->port);
      if (ret)
          retrun ret;

      platform_set_drvdata(pdev, owl_port);
      return 0;

> +       return ret;
> +}

> +
> +static int owl_uart_remove(struct platform_device *pdev)
> +{

> +       struct owl_uart_port *owl_port;
> +
> +       owl_port = platform_get_drvdata(pdev);

Do above in one line.

> +       uart_remove_one_port(&owl_uart_driver, &owl_port->port);

> +       owl_uart_ports[pdev->id] = NULL;

Redundant.

> +
> +       return 0;
> +}

> +/* Actions Semi Owl UART */
> +#define PORT_OWL       117

We can use holes for now IIUC.

See 36 or alike

-- 
With Best Regards,
Andy Shevchenko

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

^ permalink raw reply

* Re: [PATCH] Fix serial console on SNI RM400 machines
From: Andy Shevchenko @ 2017-06-07 13:47 UTC (permalink / raw)
  To: Thomas Bogendoerfer
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial@vger.kernel.org,
	linux-kernel@vger.kernel.org, Russell King - ARM Linux
In-Reply-To: <20170531203518.76311508766@solo.franken.de>

On Wed, May 31, 2017 at 11:21 PM, Thomas Bogendoerfer
<tsbogend@alpha.franken.de> wrote:
> sccnxp driver doesn't get the correct uart clock rate, if CONFIG_HAVE_CLOCK
> is disabled. Correct usage of clk API to make it work with/without it.

uart -> UART
clk -> CLK

>         clk = devm_clk_get(&pdev->dev, NULL);
>         if (IS_ERR(clk)) {
> -               if (PTR_ERR(clk) == -EPROBE_DEFER) {
> -                       ret = -EPROBE_DEFER;
> +               ret = PTR_ERR(clk);
> +               if (ret == -EPROBE_DEFER)
>                         goto err_out;
> -               }
> +               uartclk = 0;
> +       } else {

> +               clk_prepare_enable(clk);

This might fail.

> +               uartclk = clk_get_rate(clk);
> +       }

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH 0/9] tty: drop broken alt-speed handling
From: Andy Shevchenko @ 2017-06-07  9:53 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Greg Kroah-Hartman, Jiri Slaby, Tony Luck, Fenghua Yu,
	Samuel Ortiz, David S. Miller, linux-kernel@vger.kernel.org, USB,
	linux-serial@vger.kernel.org
In-Reply-To: <20170606105441.689-1-johan@kernel.org>

On Tue, Jun 6, 2017 at 1:54 PM, Johan Hovold <johan@kernel.org> wrote:
> Setting an alt-speed using TIOCSSERIAL and SPD flags has been deprecated
> since v2.1.69 and has been broken for all tty drivers but serial-core
> since v3.10 and commit 6865ff222cca ("TTY: do not warn about setting
> speed via SPD_*") without anyone noticing (for four years).
>
> Instead of reverting the offending commit, lets get rid of this legacy
> code while adding back a warning about the SPD flags being deprecated to
> the drivers that once implemented it. Note that drivers implementing
> SPD_CUST will continue to support using a custom divisor.
>
> Also note that serial-core did not rely on the TTY layer for SPD
> handling and continues to support it while warning about deprecation
> (since 2003 at least).
>
> Greg, I suggest you take it all trough the TTY tree even if merging
> through multiple trees and applying the final patch once everything is
> upstream would be an option. Also the irda clean up does not depend on
> the rest of series as the code implementing SPD handling was ifdefed
> out.

What a nice clean up!

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

>
> Johan
>
>
> Johan Hovold (9):
>   serial: rate limit custom-speed deprecation notice
>   USB: serial: ftdi_sio: simplify TIOCSSERIAL flag logic
>   USB: serial: ftdi_sio: remove broken alt-speed handling
>   tty: simserial: drop unused alt_speed handling
>   tty: amiserial: drop broken alt-speed support
>   tty: cyclades: drop broken alt-speed support
>   tty: rocket: drop broken alt-speed support
>   tty: ircomm: remove dead and broken ioctl code
>   tty: drop unused alt_speed from tty_struct
>
>  arch/ia64/hp/sim/simserial.c       |  13 -----
>  drivers/tty/amiserial.c            |  23 ++------
>  drivers/tty/cyclades.c             |  21 ++++----
>  drivers/tty/rocket.c               |  27 +++-------
>  drivers/tty/serial/serial_core.c   |   5 +-
>  drivers/usb/serial/ftdi_sio.c      |  67 +++++------------------
>  include/linux/tty.h                |   1 -
>  net/irda/ircomm/ircomm_tty_ioctl.c | 107 +------------------------------------
>  8 files changed, 34 insertions(+), 230 deletions(-)
>
> --
> 2.13.0
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v9 3/3] printk: fix double printing with earlycon
From: Sergey Senozhatsky @ 2017-06-07  9:13 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Aleksey Makarov, Sergey Senozhatsky, Sabrina Dubroca,
	linux-serial, linux-kernel, Sudeep Holla, Greg Kroah-Hartman,
	Peter Hurley, Jiri Slaby, Robin Murphy, Steven Rostedt,
	Nair, Jayachandran, Sergey Senozhatsky
In-Reply-To: <20170606160339.GC7604@pathway.suse.cz>

On (06/06/17 18:03), Petr Mladek wrote:
[..]
> From 142d75792d0df874e80493654311f69f6a1ca798 Mon Sep 17 00:00:00 2001
> From: Petr Mladek <pmladek@suse.com>
> Date: Tue, 6 Jun 2017 17:20:38 +0200
> Subject: [PATCH] Revert "printk: fix double printing with earlycon"
> 
> The commit cf39bf58afdaabc ("printk: fix double printing with earlycon")
> caused regression to users that define both console=ttyS1 and console=ttyS0
> on the command line, see
> https://lkml.kernel.org/r/20170509082915.GA13236@bistromath.localdomain
> 
> The kernel log messages always appeared only on one serial port. It is
> even documented in Documentation/admin-guide/serial-console.rst:
> 
> "Note that you can only define one console per device type (serial,
> video)."
> 
> The above mentioned commit changed the order in which the command line
> parameters are searched. As a result, the kernel log messages go to
> the last mentioned ttyS* instead of the first one.
> 
> We long thought that using two console=ttyS* on the command line
> did not make sense. But then we realized that console= parameters
> were handled also by systemd, see
> http://0pointer.de/blog/projects/serial-console.html
> 
> "By default systemd will instantiate one serial-getty@.service on
> the main kernel console, if it is not a virtual terminal."
> 
> where
> 
> "[4] If multiple kernel consoles are used simultaneously, the main
> console is the one listed first in /sys/class/tty/console/active,
> which is the last one listed on the kernel command line."
> 
> This puts the original report into another light. The system is running
> in qemu. The first serial port is used to store the messages into a file.
> The second one is used to login to the system via a socket. It depends
> on systemd and the historic kernel behavior.
> 
> By other words, systemd causes that it makes sense to define both
> console=ttyS1 console=ttyS0 on the command line. The kernel fix
> caused regression related to userspace (systemd) and need to be
> reverted.
> 
> In addition, it went out that the fix helped only partially.
> The messages still were duplicated when the boot console was
> removed early by late_initcall(printk_late_init). Then the entire
> log was replayed when the same console was registered as a normal one.
> 
> This reverts commit cf39bf58afdaabc0b86f141630fb3fd18190294e.

FWIW,

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


the whole thing is too fragile and, may be, it deserves a rewrite.

I agree with the decision to revert the patch. a user space visible
regression is a pretty annoying thing; even a rare one. it takes ~1-2
years until kernel release hits major vendors/users so the biggest part
of regressions might be years away, but in the meantime we can grow a
number of new setups that will depend on a new console= behavior. so
I'd prefer to revert early.

Petr, Aleksey, many thanks, good work.

	-ss

^ permalink raw reply

* Re: [PATCH v9 3/3] printk: fix double printing with earlycon
From: Petr Mladek @ 2017-06-06 16:03 UTC (permalink / raw)
  To: Aleksey Makarov
  Cc: Sergey Senozhatsky, Sabrina Dubroca, linux-serial, linux-kernel,
	Sudeep Holla, Greg Kroah-Hartman, Peter Hurley, Jiri Slaby,
	Robin Murphy, Steven Rostedt, Nair, Jayachandran,
	Sergey Senozhatsky
In-Reply-To: <20170606143149.GB7604@pathway.suse.cz>

On Tue 2017-06-06 16:31:49, Petr Mladek wrote:
> On Thu 2017-06-01 14:03:25, Petr Mladek wrote:
> Now, systemd seems to handle the console= parameters as well.
> According to http://0pointer.de/blog/projects/serial-console.html,
> systemd instantiates one serial-getty@.service on the preferred
> (last mentioned) console.
> 
> This might be a reason to define both console=ttyS0 console=ttyS1
> on the command line. People might actually want to get the logs
> and the login prompt on different console.

I have consulted this with a colleague. I am sorry to say but we
believe that this fits into the category of breaking userspace
(systemd, qemu). I would like to revert the patch ASAP,
before rc5. Here is a proposal of the commit message:


>From 142d75792d0df874e80493654311f69f6a1ca798 Mon Sep 17 00:00:00 2001
From: Petr Mladek <pmladek@suse.com>
Date: Tue, 6 Jun 2017 17:20:38 +0200
Subject: [PATCH] Revert "printk: fix double printing with earlycon"

The commit cf39bf58afdaabc ("printk: fix double printing with earlycon")
caused regression to users that define both console=ttyS1 and console=ttyS0
on the command line, see
https://lkml.kernel.org/r/20170509082915.GA13236@bistromath.localdomain

The kernel log messages always appeared only on one serial port. It is
even documented in Documentation/admin-guide/serial-console.rst:

"Note that you can only define one console per device type (serial,
video)."

The above mentioned commit changed the order in which the command line
parameters are searched. As a result, the kernel log messages go to
the last mentioned ttyS* instead of the first one.

We long thought that using two console=ttyS* on the command line
did not make sense. But then we realized that console= parameters
were handled also by systemd, see
http://0pointer.de/blog/projects/serial-console.html

"By default systemd will instantiate one serial-getty@.service on
the main kernel console, if it is not a virtual terminal."

where

"[4] If multiple kernel consoles are used simultaneously, the main
console is the one listed first in /sys/class/tty/console/active,
which is the last one listed on the kernel command line."

This puts the original report into another light. The system is running
in qemu. The first serial port is used to store the messages into a file.
The second one is used to login to the system via a socket. It depends
on systemd and the historic kernel behavior.

By other words, systemd causes that it makes sense to define both
console=ttyS1 console=ttyS0 on the command line. The kernel fix
caused regression related to userspace (systemd) and need to be
reverted.

In addition, it went out that the fix helped only partially.
The messages still were duplicated when the boot console was
removed early by late_initcall(printk_late_init). Then the entire
log was replayed when the same console was registered as a normal one.

This reverts commit cf39bf58afdaabc0b86f141630fb3fd18190294e.
---
 kernel/printk/printk.c | 46 ++++++++++------------------------------------
 1 file changed, 10 insertions(+), 36 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index a1aecf44ab07..a1db38abac5b 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -269,7 +269,6 @@ static void __up_console_sem(unsigned long ip)
 #define MAX_CMDLINECONSOLES 8
 
 static struct console_cmdline console_cmdline[MAX_CMDLINECONSOLES];
-static int console_cmdline_cnt;
 
 static int preferred_console = -1;
 int console_set_on_cmdline;
@@ -1906,25 +1905,12 @@ static int __add_preferred_console(char *name, int idx, char *options,
 	 *	See if this tty is not yet registered, and
 	 *	if we have a slot free.
 	 */
-	for (i = 0, c = console_cmdline; i < console_cmdline_cnt; i++, c++) {
+	for (i = 0, c = console_cmdline;
+	     i < MAX_CMDLINECONSOLES && c->name[0];
+	     i++, c++) {
 		if (strcmp(c->name, name) == 0 && c->index == idx) {
-			if (brl_options)
-				return 0;
-
-			/*
-			 * Maintain an invariant that will help to find if
-			 * the matching console is preferred, see
-			 * register_console():
-			 *
-			 * The last non-braille console is always
-			 * the preferred one.
-			 */
-			if (i != console_cmdline_cnt - 1)
-				swap(console_cmdline[i],
-				     console_cmdline[console_cmdline_cnt - 1]);
-
-			preferred_console = console_cmdline_cnt - 1;
-
+			if (!brl_options)
+				preferred_console = i;
 			return 0;
 		}
 	}
@@ -1937,7 +1923,6 @@ static int __add_preferred_console(char *name, int idx, char *options,
 	braille_set_options(c, brl_options);
 
 	c->index = idx;
-	console_cmdline_cnt++;
 	return 0;
 }
 /*
@@ -2477,23 +2462,12 @@ void register_console(struct console *newcon)
 	}
 
 	/*
-	 * See if this console matches one we selected on the command line.
-	 *
-	 * There may be several entries in the console_cmdline array matching
-	 * with the same console, one with newcon->match(), another by
-	 * name/index:
-	 *
-	 *	pl011,mmio,0x87e024000000,115200 -- added from SPCR
-	 *	ttyAMA0 -- added from command line
-	 *
-	 * Traverse the console_cmdline array in reverse order to be
-	 * sure that if this console is preferred then it will be the first
-	 * matching entry.  We use the invariant that is maintained in
-	 * __add_preferred_console().
+	 *	See if this console matches one we selected on
+	 *	the command line.
 	 */
-	for (i = console_cmdline_cnt - 1; i >= 0; i--) {
-		c = console_cmdline + i;
-
+	for (i = 0, c = console_cmdline;
+	     i < MAX_CMDLINECONSOLES && c->name[0];
+	     i++, c++) {
 		if (!newcon->match ||
 		    newcon->match(newcon, c->name, c->index, c->options) != 0) {
 			/* default matching */
-- 
1.8.5.6

^ permalink raw reply related

* Re: [PATCH v9 3/3] printk: fix double printing with earlycon
From: Petr Mladek @ 2017-06-06 14:31 UTC (permalink / raw)
  To: Aleksey Makarov
  Cc: Sergey Senozhatsky, Sabrina Dubroca, linux-serial, linux-kernel,
	Sudeep Holla, Greg Kroah-Hartman, Peter Hurley, Jiri Slaby,
	Robin Murphy, Steven Rostedt, Nair, Jayachandran,
	Sergey Senozhatsky
In-Reply-To: <20170601120325.GA25497@pathway.suse.cz>

On Thu 2017-06-01 14:03:25, Petr Mladek wrote:
> On Fri 2017-05-26 12:37:12, Aleksey Makarov wrote:
> > The console_cmdline array is the place where we add descriptions
> > of consoles that should receive log output. ACPI SPCR specifies exactly
> > that information, and I believe it should go to that array.
> 
> Are consoles defined by earlycon= command line parameter
> added to console_cmdline? I do not see this anywhere in
> the following functions:
> 
>   + param_setup_earlycon()
>   + setup_earlycon()
>   + register_earlycon()
>   + early_init_dt_scan_chosen_stdout()
>   + of_setup_earlycon()
> 
> IMHO, the main question is whether the console defined by
> ACPI SPCR is early or normal one.

The answer is that it depends. The console defined by ACPI
SPCR is early when just 'earlycon' is defined on the command
line. See param_setup_earlycon() and the use of
earlycon_init_is_deferred. Otherwise, it is supposed to
be a normal console.

We need to call add_preferred_console() in parse_spcr()
to allow using the console as a normal one.

We must also work correctly when the user explicitly defines
the console on the command line, using another alias.

As a result, we could not easily prevent having two matching consoles
in console_cmdline list. And as long as the last mentioned console
has the special status (preferred_console, CON_CONSDEV), it would
make sense to try matching the preferred console first.

Sadly, it causes the change of the behavior of ttyS0 and ttyS1.


Now, systemd seems to handle the console= parameters as well.
According to http://0pointer.de/blog/projects/serial-console.html,
systemd instantiates one serial-getty@.service on the preferred
(last mentioned) console.

This might be a reason to define both console=ttyS0 console=ttyS1
on the command line. People might actually want to get the logs
and the login prompt on different console.

Well, it is a bug that kernel shows the messages only on one
console.


There is one more thing that I found. Early consoles are forcefully
removed in a late_initcall(printk_late_init). It happens when they
use a code that is discarded after all initcalls finishes.
This has two effects.

First, boot consoles are removed even when the preferred one
is not registered. This explains why we need to call both
setup_earlycon() and add_preferred_console() in parse_spcr().

Second, the boot console might be removed before the normal/preferred
one is registered. For example, the normal one might be registered later
by a deferred probe call. As a result, the normal console might miss
that there was an early one, replays the entire log, and you
might see the beginning of the log twice.


All in all, I think that I have finally understood what
Aleksey wanted to say in his great summary, see
https://lkml.kernel.org/r/4e679bec-4804-9791-fc2b-0e96ad67d511@linaro.org
My proposed solution in parse_spcr() do not fly.

And I am still undecided about the Alexey's patch:

  + the patch makes some sense

  + but people might see the changed order as regressions;
    there might be more users, especially because of systemd

  + IMHO, the patch fixed the duplicit output only partly;
    it would not help when the normal console is registered
    later


Best Regards,
Petr

^ 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