* [PATCH v3 1/4] dt-bindings: mediatek: add support for mt6765 reference board
From: Mars Cheng @ 2018-07-04 1:52 UTC (permalink / raw)
To: Matthias Brugger, Rob Herring, Marc Zyngier, Greg Kroah-Hartman
Cc: CC Hwang, Loda Chou, linux-kernel, linux-mediatek, devicetree,
wsd_upstream, linux-serial, linux-arm-kernel, Mars Cheng
In-Reply-To: <1530669174-17623-1-git-send-email-mars.cheng@mediatek.com>
Update binding document for mt6765 reference board
Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
---
Documentation/devicetree/bindings/arm/mediatek.txt | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/mediatek.txt b/Documentation/devicetree/bindings/arm/mediatek.txt
index 7d21ab3..48fac4e 100644
--- a/Documentation/devicetree/bindings/arm/mediatek.txt
+++ b/Documentation/devicetree/bindings/arm/mediatek.txt
@@ -11,6 +11,7 @@ compatible: Must contain one of
"mediatek,mt6589"
"mediatek,mt6592"
"mediatek,mt6755"
+ "mediatek,mt6765"
"mediatek,mt6795"
"mediatek,mt6797"
"mediatek,mt7622"
@@ -41,6 +42,9 @@ Supported boards:
- Evaluation phone for MT6755(Helio P10):
Required root node properties:
- compatible = "mediatek,mt6755-evb", "mediatek,mt6755";
+- Evaluation board for MT6765(Helio P22):
+ Required root node properties:
+ - compatible = "mediatek,mt6765-evb", "mediatek,mt6765";
- Evaluation board for MT6795(Helio X10):
Required root node properties:
- compatible = "mediatek,mt6795-evb", "mediatek,mt6795";
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3 2/4] dt-bindings: mtk-uart: add mt6765 uart bindings
From: Mars Cheng @ 2018-07-04 1:52 UTC (permalink / raw)
To: Matthias Brugger, Rob Herring, Marc Zyngier, Greg Kroah-Hartman
Cc: CC Hwang, Loda Chou, linux-kernel, linux-mediatek, devicetree,
wsd_upstream, linux-serial, linux-arm-kernel, Mars Cheng
In-Reply-To: <1530669174-17623-1-git-send-email-mars.cheng@mediatek.com>
Add documentation for mt6765 uart dt-bindings
Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
---
.../devicetree/bindings/serial/mtk-uart.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
index f73abff..742cb47 100644
--- a/Documentation/devicetree/bindings/serial/mtk-uart.txt
+++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
@@ -8,6 +8,7 @@ Required properties:
* "mediatek,mt6582-uart" for MT6582 compatible UARTS
* "mediatek,mt6589-uart" for MT6589 compatible UARTS
* "mediatek,mt6755-uart" for MT6755 compatible UARTS
+ * "mediatek,mt6765-uart" for MT6765 compatible UARTS
* "mediatek,mt6795-uart" for MT6795 compatible UARTS
* "mediatek,mt6797-uart" for MT6797 compatible UARTS
* "mediatek,mt7622-uart" for MT7622 compatible UARTS
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3 3/4] dt-bindings: interrupt-controller: add binding for mt6765
From: Mars Cheng @ 2018-07-04 1:52 UTC (permalink / raw)
To: Matthias Brugger, Rob Herring, Marc Zyngier, Greg Kroah-Hartman
Cc: CC Hwang, Loda Chou, linux-kernel, linux-mediatek, devicetree,
wsd_upstream, linux-serial, linux-arm-kernel, Mars Cheng
In-Reply-To: <1530669174-17623-1-git-send-email-mars.cheng@mediatek.com>
Update the dt-binding documentation of sysirq for mt6765
Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
---
.../interrupt-controller/mediatek,sysirq.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/interrupt-controller/mediatek,sysirq.txt b/Documentation/devicetree/bindings/interrupt-controller/mediatek,sysirq.txt
index 07bf0b9..c8eda80 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/mediatek,sysirq.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/mediatek,sysirq.txt
@@ -11,6 +11,7 @@ Required properties:
"mediatek,mt7622-sysirq", "mediatek,mt6577-sysirq": for MT7622
"mediatek,mt6795-sysirq", "mediatek,mt6577-sysirq": for MT6795
"mediatek,mt6797-sysirq", "mediatek,mt6577-sysirq": for MT6797
+ "mediatek,mt6765-sysirq", "mediatek,mt6577-sysirq": for MT6765
"mediatek,mt6755-sysirq", "mediatek,mt6577-sysirq": for MT6755
"mediatek,mt6592-sysirq", "mediatek,mt6577-sysirq": for MT6592
"mediatek,mt6589-sysirq", "mediatek,mt6577-sysirq": for MT6589
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3 4/4] arm64: dts: mediatek: add mt6765 support
From: Mars Cheng @ 2018-07-04 1:52 UTC (permalink / raw)
To: Matthias Brugger, Rob Herring, Marc Zyngier, Greg Kroah-Hartman
Cc: CC Hwang, Loda Chou, linux-kernel, linux-mediatek, devicetree,
wsd_upstream, linux-serial, linux-arm-kernel, Mars Cheng
In-Reply-To: <1530669174-17623-1-git-send-email-mars.cheng@mediatek.com>
This adds basic chip support for MT6765 SoC.
Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
---
arch/arm64/boot/dts/mediatek/Makefile | 1 +
arch/arm64/boot/dts/mediatek/mt6765-evb.dts | 33 ++++++
arch/arm64/boot/dts/mediatek/mt6765.dtsi | 155 +++++++++++++++++++++++++++
3 files changed, 189 insertions(+)
create mode 100644 arch/arm64/boot/dts/mediatek/mt6765-evb.dts
create mode 100644 arch/arm64/boot/dts/mediatek/mt6765.dtsi
diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
index ac17f60..7506b0d 100644
--- a/arch/arm64/boot/dts/mediatek/Makefile
+++ b/arch/arm64/boot/dts/mediatek/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
dtb-$(CONFIG_ARCH_MEDIATEK) += mt2712-evb.dtb
dtb-$(CONFIG_ARCH_MEDIATEK) += mt6755-evb.dtb
+dtb-$(CONFIG_ARCH_MEDIATEK) += mt6765-evb.dtb
dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-evb.dtb
dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
diff --git a/arch/arm64/boot/dts/mediatek/mt6765-evb.dts b/arch/arm64/boot/dts/mediatek/mt6765-evb.dts
new file mode 100644
index 0000000..36dddff2
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt6765-evb.dts
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * dts file for Mediatek MT6765
+ *
+ * (C) Copyright 2018. Mediatek, Inc.
+ *
+ * Mars Cheng <mars.cheng@mediatek.com>
+ */
+
+/dts-v1/;
+#include "mt6765.dtsi"
+
+/ {
+ model = "MediaTek MT6765 EVB";
+ compatible = "mediatek,mt6765-evb", "mediatek,mt6765";
+
+ aliases {
+ serial0 = &uart0;
+ };
+
+ memory@40000000 {
+ device_type = "memory";
+ reg = <0 0x40000000 0 0x1e800000>;
+ };
+
+ chosen {
+ stdout-path = "serial0:921600n8";
+ };
+};
+
+&uart0 {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/mediatek/mt6765.dtsi b/arch/arm64/boot/dts/mediatek/mt6765.dtsi
new file mode 100644
index 0000000..051545b
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt6765.dtsi
@@ -0,0 +1,155 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * dts file for Mediatek MT6765
+ *
+ * (C) Copyright 2018. Mediatek, Inc.
+ *
+ * Mars Cheng <mars.cheng@mediatek.com>
+ */
+
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/ {
+ compatible = "mediatek,mt6765";
+ interrupt-parent = <&sysirq>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ psci {
+ compatible = "arm,psci-0.2";
+ method = "smc";
+ };
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpu@0 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ enable-method = "psci";
+ reg = <0x000>;
+ };
+
+ cpu@1 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ enable-method = "psci";
+ reg = <0x001>;
+ };
+
+ cpu@2 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ enable-method = "psci";
+ reg = <0x002>;
+ };
+
+ cpu@3 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ enable-method = "psci";
+ reg = <0x003>;
+ };
+
+ cpu@100 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ enable-method = "psci";
+ reg = <0x100>;
+ };
+
+ cpu@101 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ enable-method = "psci";
+ reg = <0x101>;
+ };
+
+ cpu@102 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ enable-method = "psci";
+ reg = <0x102>;
+ };
+
+ cpu@103 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ enable-method = "psci";
+ reg = <0x103>;
+ };
+ };
+
+ baud_clk: dummy26m {
+ compatible = "fixed-clock";
+ clock-frequency = <26000000>;
+ #clock-cells = <0>;
+ };
+
+ sys_clk: dummyclk {
+ compatible = "fixed-clock";
+ clock-frequency = <26000000>;
+ #clock-cells = <0>;
+ };
+
+ timer {
+ compatible = "arm,armv8-timer";
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
+ };
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ compatible = "simple-bus";
+ ranges;
+
+ sysirq: interrupt-controller@10200a80 {
+ compatible = "mediatek,mt6765-sysirq",
+ "mediatek,mt6577-sysirq";
+ interrupt-controller;
+ #interrupt-cells = <3>;
+ interrupt-parent = <&gic>;
+ reg = <0 0x10200a80 0 0x50>;
+ };
+
+ gic: interrupt-controller@c000000 {
+ compatible = "arm,gic-v3";
+ #interrupt-cells = <3>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+ #redistributor-regions = <1>;
+ interrupt-parent = <&gic>;
+ interrupt-controller;
+ reg = <0 0x0c000000 0 0x40000>, // distributor
+ <0 0x0c100000 0 0x200000>, // redistributor
+ <0 0x0c400000 0 0x40000>; // gicc
+ interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ uart0: serial@11002000 {
+ compatible = "mediatek,mt6765-uart",
+ "mediatek,mt6577-uart";
+ reg = <0 0x11002000 0 0x400>;
+ interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&baud_clk>, <&sys_clk>;
+ clock-names = "baud", "bus";
+ status = "disabled";
+ };
+
+ uart1: serial@11003000 {
+ compatible = "mediatek,mt6765-uart",
+ "mediatek,mt6577-uart";
+ reg = <0 0x11003000 0 0x400>;
+ interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&baud_clk>, <&sys_clk>;
+ clock-names = "baud", "bus";
+ status = "disabled";
+ };
+ }; /* end of soc */
+};
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v10 2/6] dt-bindings: add binding for atmel-usart in SPI mode
From: Lee Jones @ 2018-07-04 6:23 UTC (permalink / raw)
To: Radu Pirea
Cc: broonie, nicolas.ferre, alexandre.belloni, richard.genoud,
robh+dt, mark.rutland, gregkh, linux-spi, linux-arm-kernel,
linux-kernel, devicetree, linux-serial
In-Reply-To: <20180625172230.29686-3-radu.pirea@microchip.com>
On Mon, 25 Jun 2018, Radu Pirea wrote:
> This patch moves the bindings for serial from serial/atmel-usart.txt to
> mfd/atmel-usart.txt and adds bindings for USART in SPI mode.
>
> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
I already Acked this in V4.
> ---
> .../bindings/{serial => mfd}/atmel-usart.txt | 25 +++++++++++++++++--
> include/dt-bindings/mfd/at91-usart.h | 17 +++++++++++++
> 2 files changed, 40 insertions(+), 2 deletions(-)
> rename Documentation/devicetree/bindings/{serial => mfd}/atmel-usart.txt (76%)
> create mode 100644 include/dt-bindings/mfd/at91-usart.h
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v10 1/6] MAINTAINERS: add at91 usart mfd driver
From: Lee Jones @ 2018-07-04 6:23 UTC (permalink / raw)
To: Radu Pirea
Cc: broonie, nicolas.ferre, alexandre.belloni, richard.genoud,
robh+dt, mark.rutland, gregkh, linux-spi, linux-arm-kernel,
linux-kernel, devicetree, linux-serial
In-Reply-To: <20180625172230.29686-2-radu.pirea@microchip.com>
On Mon, 25 Jun 2018, Radu Pirea wrote:
> Added entry for at91 usart mfd driver.
>
> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> ---
> MAINTAINERS | 9 +++++++++
> 1 file changed, 9 insertions(+)
For my own reference:
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v3 4/4] arm64: dts: mediatek: add mt6765 support
From: Marc Zyngier @ 2018-07-04 7:35 UTC (permalink / raw)
To: Mars Cheng, Matthias Brugger, Rob Herring, Greg Kroah-Hartman
Cc: CC Hwang, Loda Chou, linux-kernel, linux-mediatek, devicetree,
wsd_upstream, linux-serial, linux-arm-kernel
In-Reply-To: <1530669174-17623-5-git-send-email-mars.cheng@mediatek.com>
On 04/07/18 02:52, Mars Cheng wrote:
> This adds basic chip support for MT6765 SoC.
>
> Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> ---
> arch/arm64/boot/dts/mediatek/Makefile | 1 +
> arch/arm64/boot/dts/mediatek/mt6765-evb.dts | 33 ++++++
> arch/arm64/boot/dts/mediatek/mt6765.dtsi | 155 +++++++++++++++++++++++++++
> 3 files changed, 189 insertions(+)
> create mode 100644 arch/arm64/boot/dts/mediatek/mt6765-evb.dts
> create mode 100644 arch/arm64/boot/dts/mediatek/mt6765.dtsi
>
[...]
> +
> + gic: interrupt-controller@c000000 {
> + compatible = "arm,gic-v3";
> + #interrupt-cells = <3>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> + #redistributor-regions = <1>;
> + interrupt-parent = <&gic>;
> + interrupt-controller;
> + reg = <0 0x0c000000 0 0x40000>, // distributor
> + <0 0x0c100000 0 0x200000>, // redistributor
> + <0 0x0c400000 0 0x40000>; // gicc
For the second time: please add *all* the GIC CPU interface regions,
described in the Cortex-A53 TRM[1] (GICC, GICH, and GICV).
Thanks,
M.
[1]
http://infocenter.arm.com/help/topic/com.arm.doc.ddi0500g/ch09s02s01.html
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* Re: [PATCH v3 4/4] arm64: dts: mediatek: add mt6765 support
From: Mars Cheng @ 2018-07-04 7:47 UTC (permalink / raw)
To: Marc Zyngier
Cc: Matthias Brugger, Rob Herring, Greg Kroah-Hartman, CC Hwang,
Loda Chou, linux-kernel, linux-mediatek, devicetree, wsd_upstream,
linux-serial, linux-arm-kernel
In-Reply-To: <f482708c-ea5e-5d81-a176-d69be9ccdf63@arm.com>
Hi Marc
On Wed, 2018-07-04 at 08:35 +0100, Marc Zyngier wrote:
> On 04/07/18 02:52, Mars Cheng wrote:
> > This adds basic chip support for MT6765 SoC.
> >
> > Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> > ---
> > arch/arm64/boot/dts/mediatek/Makefile | 1 +
> > arch/arm64/boot/dts/mediatek/mt6765-evb.dts | 33 ++++++
> > arch/arm64/boot/dts/mediatek/mt6765.dtsi | 155 +++++++++++++++++++++++++++
> > 3 files changed, 189 insertions(+)
> > create mode 100644 arch/arm64/boot/dts/mediatek/mt6765-evb.dts
> > create mode 100644 arch/arm64/boot/dts/mediatek/mt6765.dtsi
> >
>
> [...]
>
> > +
> > + gic: interrupt-controller@c000000 {
> > + compatible = "arm,gic-v3";
> > + #interrupt-cells = <3>;
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + #redistributor-regions = <1>;
> > + interrupt-parent = <&gic>;
> > + interrupt-controller;
> > + reg = <0 0x0c000000 0 0x40000>, // distributor
> > + <0 0x0c100000 0 0x200000>, // redistributor
> > + <0 0x0c400000 0 0x40000>; // gicc
>
> For the second time: please add *all* the GIC CPU interface regions,
> described in the Cortex-A53 TRM[1] (GICC, GICH, and GICV).
>
MT6765 has no GICH/GICV/ITS in mediatek design. Have confirmed with our
designer.
MT6797 had similar question from you. Sorry for not mentioned it first.
http://lists.infradead.org/pipermail/linux-mediatek/2017-February/008074.html
Thanks.
> Thanks,
>
> M.
>
> [1]
> https://urldefense.proofpoint.com/v2/url?u=http-3A__infocenter.arm.com_help_topic_com.arm.doc.ddi0500g_ch09s02s01.html&d=DwICaQ&c=X9NHckmGz7LNQmqtvpDCYVnn6eFXNivfZeknqiAo-n0&r=Ph_SbcClVGRWmGxVhfr-5CZF9ffiUOE7TZ47ns4ROh4&m=9L01qJc7apuzwLobX_nhN0ik8IFdu_X7hJ139x5dNNw&s=0zeZXtWPeITLj01RSxAQ6NfNkTUu9Il0Dddgk07-6QA&e=
^ permalink raw reply
* Re: [PATCH v3 4/4] arm64: dts: mediatek: add mt6765 support
From: Marc Zyngier @ 2018-07-04 7:59 UTC (permalink / raw)
To: Mars Cheng
Cc: Matthias Brugger, Rob Herring, Greg Kroah-Hartman, CC Hwang,
Loda Chou, linux-kernel, linux-mediatek, devicetree, wsd_upstream,
linux-serial, linux-arm-kernel
In-Reply-To: <1530690473.16183.3.camel@mtkswgap22>
On 04/07/18 08:47, Mars Cheng wrote:
> Hi Marc
>
> On Wed, 2018-07-04 at 08:35 +0100, Marc Zyngier wrote:
>> On 04/07/18 02:52, Mars Cheng wrote:
>>> This adds basic chip support for MT6765 SoC.
>>>
>>> Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
>>> ---
>>> arch/arm64/boot/dts/mediatek/Makefile | 1 +
>>> arch/arm64/boot/dts/mediatek/mt6765-evb.dts | 33 ++++++
>>> arch/arm64/boot/dts/mediatek/mt6765.dtsi | 155 +++++++++++++++++++++++++++
>>> 3 files changed, 189 insertions(+)
>>> create mode 100644 arch/arm64/boot/dts/mediatek/mt6765-evb.dts
>>> create mode 100644 arch/arm64/boot/dts/mediatek/mt6765.dtsi
>>>
>>
>> [...]
>>
>>> +
>>> + gic: interrupt-controller@c000000 {
>>> + compatible = "arm,gic-v3";
>>> + #interrupt-cells = <3>;
>>> + #address-cells = <2>;
>>> + #size-cells = <2>;
>>> + #redistributor-regions = <1>;
>>> + interrupt-parent = <&gic>;
>>> + interrupt-controller;
>>> + reg = <0 0x0c000000 0 0x40000>, // distributor
>>> + <0 0x0c100000 0 0x200000>, // redistributor
>>> + <0 0x0c400000 0 0x40000>; // gicc
>>
>> For the second time: please add *all* the GIC CPU interface regions,
>> described in the Cortex-A53 TRM[1] (GICC, GICH, and GICV).
>>
>
> MT6765 has no GICH/GICV/ITS in mediatek design. Have confirmed with our
> designer.
The only way *not* to have GICH or GICV is to assert GICCDISABLE on the
CPU, in which case you don't have GICC either, nor any support for the
GICv3 at all. So either the designer is wrong or the documentation is
wrong. Which one is it, do you think?
As for the ITS, that's a perfectly optional part of the design, and not
part of the CPU.
> MT6797 had similar question from you. Sorry for not mentioned it first.
>
> http://lists.infradead.org/pipermail/linux-mediatek/2017-February/008074.html
Well, that's wrong too.
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH v2 0/3] serial: 8250_dw: add fractional divisor support
From: Jisheng Zhang @ 2018-07-04 8:59 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-kernel, linux-serial, linux-arm-kernel
For Synopsys DesignWare 8250 uart which version >= 4.00a, there's a
valid divisor latch fraction register. The fractional divisor width is
4bits ~ 6bits.
patch1 lets serial8250_get_divisor() get uart_port * as param
patch2 introduces necessary hooks to 8250 core.
patch3 implements the fractional divisor support for Synopsys DW 8250.
Since v1:
- add an extra patch to let serial8250_get_divisor() get uart_port *
as param
- take Andy's suggestions to "integrates hooks in the same way like
it's done for the rest of 8250 ones". Many thanks to Andy.
Jisheng Zhang (3):
serial: 8250: let serial8250_get_divisor() get uart_port * as param
serial: 8250: introduce get_divisor() and set_divisor() hook
serial: 8250_dw: add fractional divisor support
drivers/tty/serial/8250/8250_core.c | 4 +++
drivers/tty/serial/8250/8250_dw.c | 53 +++++++++++++++++++++++++++++
drivers/tty/serial/8250/8250_port.c | 33 ++++++++++++++----
include/linux/serial_core.h | 7 ++++
4 files changed, 90 insertions(+), 7 deletions(-)
--
2.18.0
^ permalink raw reply
* [PATCH v2 1/3] serial: 8250: let serial8250_get_divisor() get uart_port * as param
From: Jisheng Zhang @ 2018-07-04 9:00 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-kernel, linux-serial, linux-arm-kernel
In-Reply-To: <20180704165908.4bb8b090@xhacker.debian>
Align serial8250_get_divisor() with serial8250_set_divisor() to accept
uart_port pointer as the first parameter. No functionality changes.
Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
drivers/tty/serial/8250/8250_port.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index cf541aab2bd0..709fe6b4265c 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2498,11 +2498,11 @@ static unsigned int npcm_get_divisor(struct uart_8250_port *up,
return DIV_ROUND_CLOSEST(port->uartclk, 16 * baud + 2) - 2;
}
-static unsigned int serial8250_get_divisor(struct uart_8250_port *up,
+static unsigned int serial8250_get_divisor(struct uart_port *port,
unsigned int baud,
unsigned int *frac)
{
- struct uart_port *port = &up->port;
+ struct uart_8250_port *up = up_to_u8250p(port);
unsigned int quot;
/*
@@ -2636,7 +2636,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
cval = serial8250_compute_lcr(up, termios->c_cflag);
baud = serial8250_get_baud_rate(port, termios, old);
- quot = serial8250_get_divisor(up, baud, &frac);
+ quot = serial8250_get_divisor(port, baud, &frac);
/*
* Ok, we're now changing the port state. Do it with
@@ -3197,7 +3197,7 @@ static void serial8250_console_restore(struct uart_8250_port *up)
termios.c_cflag = port->state->port.tty->termios.c_cflag;
baud = serial8250_get_baud_rate(port, &termios, NULL);
- quot = serial8250_get_divisor(up, baud, &frac);
+ quot = serial8250_get_divisor(port, baud, &frac);
serial8250_set_divisor(port, baud, quot, frac);
serial_port_out(port, UART_LCR, up->lcr);
--
2.18.0
^ permalink raw reply related
* [PATCH v2 2/3] serial: 8250: introduce get_divisor() and set_divisor() hook
From: Jisheng Zhang @ 2018-07-04 9:02 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-kernel, linux-serial, linux-arm-kernel
In-Reply-To: <20180704165908.4bb8b090@xhacker.debian>
Add these two hooks so that they can be overridden with driver specific
implementations.
Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
drivers/tty/serial/8250/8250_core.c | 4 ++++
drivers/tty/serial/8250/8250_port.c | 27 +++++++++++++++++++++++----
include/linux/serial_core.h | 7 +++++++
3 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 9342fc2ee7df..a0bb77290747 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1023,6 +1023,10 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
uart->port.get_mctrl = up->port.get_mctrl;
if (up->port.set_mctrl)
uart->port.set_mctrl = up->port.set_mctrl;
+ if (up->port.get_divisor)
+ uart->port.get_divisor = up->port.get_divisor;
+ if (up->port.set_divisor)
+ uart->port.set_divisor = up->port.set_divisor;
if (up->port.startup)
uart->port.startup = up->port.startup;
if (up->port.shutdown)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 709fe6b4265c..ce0dc17f18ee 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2498,9 +2498,9 @@ static unsigned int npcm_get_divisor(struct uart_8250_port *up,
return DIV_ROUND_CLOSEST(port->uartclk, 16 * baud + 2) - 2;
}
-static unsigned int serial8250_get_divisor(struct uart_port *port,
- unsigned int baud,
- unsigned int *frac)
+static unsigned int serial8250_do_get_divisor(struct uart_port *port,
+ unsigned int baud,
+ unsigned int *frac)
{
struct uart_8250_port *up = up_to_u8250p(port);
unsigned int quot;
@@ -2532,6 +2532,16 @@ static unsigned int serial8250_get_divisor(struct uart_port *port,
return quot;
}
+static unsigned int serial8250_get_divisor(struct uart_port *port,
+ unsigned int baud,
+ unsigned int *frac)
+{
+ if (port->get_divisor)
+ return port->get_divisor(port, baud, frac);
+
+ return serial8250_do_get_divisor(port, baud, frac);
+}
+
static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
tcflag_t c_cflag)
{
@@ -2570,7 +2580,7 @@ static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
return cval;
}
-static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
+static void serial8250_do_set_divisor(struct uart_port *port, unsigned int baud,
unsigned int quot, unsigned int quot_frac)
{
struct uart_8250_port *up = up_to_u8250p(port);
@@ -2603,6 +2613,15 @@ static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
}
}
+static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
+ unsigned int quot, unsigned int quot_frac)
+{
+ if (port->set_divisor)
+ port->set_divisor(port, baud, quot, quot_frac);
+ else
+ serial8250_do_set_divisor(port, baud, quot, quot_frac);
+}
+
static unsigned int serial8250_get_baud_rate(struct uart_port *port,
struct ktermios *termios,
struct ktermios *old)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 06ea4eeb09ab..406edae44ca3 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -127,6 +127,13 @@ struct uart_port {
struct ktermios *);
unsigned int (*get_mctrl)(struct uart_port *);
void (*set_mctrl)(struct uart_port *, unsigned int);
+ unsigned int (*get_divisor)(struct uart_port *,
+ unsigned int baud,
+ unsigned int *frac);
+ void (*set_divisor)(struct uart_port *,
+ unsigned int baud,
+ unsigned int quot,
+ unsigned int quot_frac);
int (*startup)(struct uart_port *port);
void (*shutdown)(struct uart_port *port);
void (*throttle)(struct uart_port *port);
--
2.18.0
^ permalink raw reply related
* [PATCH v2 3/3] serial: 8250_dw: add fractional divisor support
From: Jisheng Zhang @ 2018-07-04 9:03 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-kernel, linux-serial, linux-arm-kernel
In-Reply-To: <20180704165908.4bb8b090@xhacker.debian>
For Synopsys DesignWare 8250 uart which version >= 4.00a, there's a
valid divisor latch fraction register. The fractional divisor width is
4bits ~ 6bits.
Now the preparation is done, it's easy to add the feature support.
This patch firstly checks the component version during probe, if
version >= 4.00a, then calculates the fractional divisor width, then
setups dw specific get_divisor() and set_divisor() hook.
Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
drivers/tty/serial/8250/8250_dw.c | 53 +++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index aff04f1de3a5..b9630f633388 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -31,9 +31,12 @@
/* Offsets for the DesignWare specific registers */
#define DW_UART_USR 0x1f /* UART Status Register */
+#define DW_UART_DLF 0xc0 /* Divisor Latch Fraction Register */
#define DW_UART_CPR 0xf4 /* Component Parameter Register */
#define DW_UART_UCV 0xf8 /* UART Component Version */
+#define DW_FRAC_MIN_VERS 0x3430302A
+
/* Component Parameter Register bits */
#define DW_UART_CPR_ABP_DATA_WIDTH (3 << 0)
#define DW_UART_CPR_AFCE_MODE (1 << 4)
@@ -65,6 +68,7 @@ struct dw8250_data {
unsigned int skip_autocfg:1;
unsigned int uart_16550_compatible:1;
+ unsigned int dlf_size:3;
};
static inline int dw8250_modify_msr(struct uart_port *p, int offset, int value)
@@ -351,6 +355,33 @@ static bool dw8250_idma_filter(struct dma_chan *chan, void *param)
return param == chan->device->dev->parent;
}
+/*
+ * For version >= 4.00a, the dw uarts have a valid divisor latch fraction
+ * register. Calculate divisor with extra 4bits ~ 6bits fractional portion.
+ */
+static unsigned int dw8250_get_divisor(struct uart_port *p,
+ unsigned int baud,
+ unsigned int *frac)
+{
+ unsigned int quot;
+ struct dw8250_data *d = p->private_data;
+
+ quot = DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4), baud);
+ *frac = quot & (~0U >> (32 - d->dlf_size));
+
+ return quot >> d->dlf_size;
+}
+
+static void dw8250_set_divisor(struct uart_port *p, unsigned int baud,
+ unsigned int quot, unsigned int quot_frac)
+{
+ struct uart_8250_port *up = up_to_u8250p(p);
+
+ serial_port_out(p, DW_UART_DLF >> p->regshift, quot_frac);
+ serial_port_out(p, UART_LCR, up->lcr | UART_LCR_DLAB);
+ serial_dl_write(up, quot);
+}
+
static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
{
if (p->dev->of_node) {
@@ -414,6 +445,28 @@ static void dw8250_setup_port(struct uart_port *p)
dev_dbg(p->dev, "Designware UART version %c.%c%c\n",
(reg >> 24) & 0xff, (reg >> 16) & 0xff, (reg >> 8) & 0xff);
+ /*
+ * For version >= 4.00a, the dw uarts have a valid divisor latch
+ * fraction register. Calculate the fractional divisor width.
+ */
+ if (reg >= DW_FRAC_MIN_VERS) {
+ struct dw8250_data *d = p->private_data;
+
+ if (p->iotype == UPIO_MEM32BE) {
+ iowrite32be(~0U, p->membase + DW_UART_DLF);
+ reg = ioread32be(p->membase + DW_UART_DLF);
+ } else {
+ writel(~0U, p->membase + DW_UART_DLF);
+ reg = readl(p->membase + DW_UART_DLF);
+ }
+ d->dlf_size = fls(reg);
+
+ if (d->dlf_size) {
+ p->get_divisor = dw8250_get_divisor;
+ p->set_divisor = dw8250_set_divisor;
+ }
+ }
+
if (p->iotype == UPIO_MEM32BE)
reg = ioread32be(p->membase + DW_UART_CPR);
else
--
2.18.0
^ permalink raw reply related
* Re: [PATCH v2 1/3] serial: 8250: let serial8250_get_divisor() get uart_port * as param
From: Andy Shevchenko @ 2018-07-04 10:00 UTC (permalink / raw)
To: Jisheng Zhang, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-kernel, linux-serial, linux-arm-kernel
In-Reply-To: <20180704170040.39ac9cf3@xhacker.debian>
On Wed, 2018-07-04 at 17:00 +0800, Jisheng Zhang wrote:
> Align serial8250_get_divisor() with serial8250_set_divisor() to accept
> uart_port pointer as the first parameter. No functionality changes.
>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> ---
> drivers/tty/serial/8250/8250_port.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_port.c
> b/drivers/tty/serial/8250/8250_port.c
> index cf541aab2bd0..709fe6b4265c 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -2498,11 +2498,11 @@ static unsigned int npcm_get_divisor(struct
> uart_8250_port *up,
> return DIV_ROUND_CLOSEST(port->uartclk, 16 * baud + 2) - 2;
> }
>
> -static unsigned int serial8250_get_divisor(struct uart_8250_port *up,
> +static unsigned int serial8250_get_divisor(struct uart_port *port,
> unsigned int baud,
> unsigned int *frac)
> {
> - struct uart_port *port = &up->port;
> + struct uart_8250_port *up = up_to_u8250p(port);
> unsigned int quot;
>
> /*
> @@ -2636,7 +2636,7 @@ serial8250_do_set_termios(struct uart_port
> *port, struct ktermios *termios,
> cval = serial8250_compute_lcr(up, termios->c_cflag);
>
> baud = serial8250_get_baud_rate(port, termios, old);
> - quot = serial8250_get_divisor(up, baud, &frac);
> + quot = serial8250_get_divisor(port, baud, &frac);
>
> /*
> * Ok, we're now changing the port state. Do it with
> @@ -3197,7 +3197,7 @@ static void serial8250_console_restore(struct
> uart_8250_port *up)
> termios.c_cflag = port->state->port.tty-
> >termios.c_cflag;
>
> baud = serial8250_get_baud_rate(port, &termios, NULL);
> - quot = serial8250_get_divisor(up, baud, &frac);
> + quot = serial8250_get_divisor(port, baud, &frac);
>
> serial8250_set_divisor(port, baud, quot, frac);
> serial_port_out(port, UART_LCR, up->lcr);
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH v2 2/3] serial: 8250: introduce get_divisor() and set_divisor() hook
From: Andy Shevchenko @ 2018-07-04 10:00 UTC (permalink / raw)
To: Jisheng Zhang, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-kernel, linux-serial, linux-arm-kernel
In-Reply-To: <20180704170217.190711ef@xhacker.debian>
On Wed, 2018-07-04 at 17:02 +0800, Jisheng Zhang wrote:
> Add these two hooks so that they can be overridden with driver
> specific
> implementations.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> ---
> drivers/tty/serial/8250/8250_core.c | 4 ++++
> drivers/tty/serial/8250/8250_port.c | 27 +++++++++++++++++++++++----
> include/linux/serial_core.h | 7 +++++++
> 3 files changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_core.c
> b/drivers/tty/serial/8250/8250_core.c
> index 9342fc2ee7df..a0bb77290747 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -1023,6 +1023,10 @@ int serial8250_register_8250_port(struct
> uart_8250_port *up)
> uart->port.get_mctrl = up->port.get_mctrl;
> if (up->port.set_mctrl)
> uart->port.set_mctrl = up->port.set_mctrl;
> + if (up->port.get_divisor)
> + uart->port.get_divisor = up-
> >port.get_divisor;
> + if (up->port.set_divisor)
> + uart->port.set_divisor = up-
> >port.set_divisor;
> if (up->port.startup)
> uart->port.startup = up->port.startup;
> if (up->port.shutdown)
> diff --git a/drivers/tty/serial/8250/8250_port.c
> b/drivers/tty/serial/8250/8250_port.c
> index 709fe6b4265c..ce0dc17f18ee 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -2498,9 +2498,9 @@ static unsigned int npcm_get_divisor(struct
> uart_8250_port *up,
> return DIV_ROUND_CLOSEST(port->uartclk, 16 * baud + 2) - 2;
> }
>
> -static unsigned int serial8250_get_divisor(struct uart_port *port,
> - unsigned int baud,
> - unsigned int *frac)
> +static unsigned int serial8250_do_get_divisor(struct uart_port *port,
> + unsigned int baud,
> + unsigned int *frac)
> {
> struct uart_8250_port *up = up_to_u8250p(port);
> unsigned int quot;
> @@ -2532,6 +2532,16 @@ static unsigned int
> serial8250_get_divisor(struct uart_port *port,
> return quot;
> }
>
> +static unsigned int serial8250_get_divisor(struct uart_port *port,
> + unsigned int baud,
> + unsigned int *frac)
> +{
> + if (port->get_divisor)
> + return port->get_divisor(port, baud, frac);
> +
> + return serial8250_do_get_divisor(port, baud, frac);
> +}
> +
> static unsigned char serial8250_compute_lcr(struct uart_8250_port
> *up,
> tcflag_t c_cflag)
> {
> @@ -2570,7 +2580,7 @@ static unsigned char
> serial8250_compute_lcr(struct uart_8250_port *up,
> return cval;
> }
>
> -static void serial8250_set_divisor(struct uart_port *port, unsigned
> int baud,
> +static void serial8250_do_set_divisor(struct uart_port *port,
> unsigned int baud,
> unsigned int quot, unsigned int
> quot_frac)
> {
> struct uart_8250_port *up = up_to_u8250p(port);
> @@ -2603,6 +2613,15 @@ static void serial8250_set_divisor(struct
> uart_port *port, unsigned int baud,
> }
> }
>
> +static void serial8250_set_divisor(struct uart_port *port, unsigned
> int baud,
> + unsigned int quot, unsigned int
> quot_frac)
> +{
> + if (port->set_divisor)
> + port->set_divisor(port, baud, quot, quot_frac);
> + else
> + serial8250_do_set_divisor(port, baud, quot,
> quot_frac);
> +}
> +
> static unsigned int serial8250_get_baud_rate(struct uart_port *port,
> struct ktermios
> *termios,
> struct ktermios *old)
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 06ea4eeb09ab..406edae44ca3 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -127,6 +127,13 @@ struct uart_port {
> struct ktermios *);
> unsigned int (*get_mctrl)(struct uart_port *);
> void (*set_mctrl)(struct uart_port *,
> unsigned int);
> + unsigned int (*get_divisor)(struct uart_port
> *,
> + unsigned int baud,
> + unsigned int *frac);
> + void (*set_divisor)(struct uart_port
> *,
> + unsigned int baud,
> + unsigned int quot,
> + unsigned int
> quot_frac);
> int (*startup)(struct uart_port
> *port);
> void (*shutdown)(struct uart_port
> *port);
> void (*throttle)(struct uart_port
> *port);
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH v3 4/4] arm64: dts: mediatek: add mt6765 support
From: Mars Cheng @ 2018-07-04 11:38 UTC (permalink / raw)
To: Marc Zyngier
Cc: Matthias Brugger, Rob Herring, Greg Kroah-Hartman, CC Hwang,
Loda Chou, linux-kernel, linux-mediatek, devicetree, wsd_upstream,
linux-serial, linux-arm-kernel
In-Reply-To: <304dc0a7-bcba-961c-40f4-b324ecc2d003@arm.com>
Hi Marc
On Wed, 2018-07-04 at 08:59 +0100, Marc Zyngier wrote:
> On 04/07/18 08:47, Mars Cheng wrote:
> > Hi Marc
> >
> > On Wed, 2018-07-04 at 08:35 +0100, Marc Zyngier wrote:
> >> On 04/07/18 02:52, Mars Cheng wrote:
> >>> This adds basic chip support for MT6765 SoC.
> >>>
> >>> Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> >>> ---
> >>> arch/arm64/boot/dts/mediatek/Makefile | 1 +
> >>> arch/arm64/boot/dts/mediatek/mt6765-evb.dts | 33 ++++++
> >>> arch/arm64/boot/dts/mediatek/mt6765.dtsi | 155 +++++++++++++++++++++++++++
> >>> 3 files changed, 189 insertions(+)
> >>> create mode 100644 arch/arm64/boot/dts/mediatek/mt6765-evb.dts
> >>> create mode 100644 arch/arm64/boot/dts/mediatek/mt6765.dtsi
> >>>
> >>
> >> [...]
> >>
> >>> +
> >>> + gic: interrupt-controller@c000000 {
> >>> + compatible = "arm,gic-v3";
> >>> + #interrupt-cells = <3>;
> >>> + #address-cells = <2>;
> >>> + #size-cells = <2>;
> >>> + #redistributor-regions = <1>;
> >>> + interrupt-parent = <&gic>;
> >>> + interrupt-controller;
> >>> + reg = <0 0x0c000000 0 0x40000>, // distributor
> >>> + <0 0x0c100000 0 0x200000>, // redistributor
> >>> + <0 0x0c400000 0 0x40000>; // gicc
> >>
> >> For the second time: please add *all* the GIC CPU interface regions,
> >> described in the Cortex-A53 TRM[1] (GICC, GICH, and GICV).
> >>
> >
> > MT6765 has no GICH/GICV/ITS in mediatek design. Have confirmed with our
> > designer.
>
> The only way *not* to have GICH or GICV is to assert GICCDISABLE on the
> CPU, in which case you don't have GICC either, nor any support for the
> GICv3 at all. So either the designer is wrong or the documentation is
> wrong. Which one is it, do you think?
>
> As for the ITS, that's a perfectly optional part of the design, and not
> part of the CPU.
>
Clarified with our designer. It is our misunderstanding for TRM.
GICV/GICH do exist. Will add them in v4 soon. And fix MT6797 in another
patch.
Thanks.
> > MT6797 had similar question from you. Sorry for not mentioned it first.
> >
> > https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.infradead.org_pipermail_linux-2Dmediatek_2017-2DFebruary_008074.html&d=DwICaQ&c=X9NHckmGz7LNQmqtvpDCYVnn6eFXNivfZeknqiAo-n0&r=Ph_SbcClVGRWmGxVhfr-5CZF9ffiUOE7TZ47ns4ROh4&m=iACLXUO5vXXZCPSvhbBKZFXy0bXdO8f4kbgy6RLi2QM&s=2N4qyy0aMytzNgObeyU4tvCDREX4U1x4oeNgvZwUxvM&e=
>
> Well, that's wrong too.
>
> M.
^ permalink raw reply
* Re: [PATCH v2 3/3] serial: 8250_dw: add fractional divisor support
From: Andy Shevchenko @ 2018-07-04 16:08 UTC (permalink / raw)
To: Jisheng Zhang, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-arm-kernel, linux-kernel, linux-serial
In-Reply-To: <20180704170310.56772d77@xhacker.debian>
On Wed, 2018-07-04 at 17:03 +0800, Jisheng Zhang wrote:
Thanks for an update, my comments below.
> For Synopsys DesignWare 8250 uart which version >= 4.00a, there's a
> valid divisor latch fraction register. The fractional divisor width is
> 4bits ~ 6bits.
I have read 4.00a spec a bit and didn't find this limitation.
The fractional divider can fit up to 32 bits.
> Now the preparation is done, it's easy to add the feature support.
> This patch firstly checks the component version during probe, if
> version >= 4.00a, then calculates the fractional divisor width, then
> setups dw specific get_divisor() and set_divisor() hook.
> +#define DW_FRAC_MIN_VERS 0x3430302A
Do we really need this?
My intuition (I, unfortunately, didn't find any evidence in Synopsys
specs for UART) tells me that when it's unimplemented we would read back
0's, which is fine.
I would test this a bit later.
>
> + unsigned int dlf_size:3;
These bit fields (besides the fact you need 5) more or less for software
quirk flags. In your case I would rather keep a u32 value of DFL mask as
done for msr slightly above in this structure.
> };
>
> +/*
> + * For version >= 4.00a, the dw uarts have a valid divisor latch
> fraction
> + * register. Calculate divisor with extra 4bits ~ 6bits fractional
> portion.
> + */
This comment kinda noise. Better to put actual formula from datasheet
how this fractional divider is involved into calculus.
> +static unsigned int dw8250_get_divisor(struct uart_port *p,
> + unsigned int baud,
> + unsigned int *frac)
> +{
> + unsigned int quot;
> + struct dw8250_data *d = p->private_data;
> +
> + quot = DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4),
> baud);
If we have clock rate like 100MHz and 10 bits of fractional divider it
would give an integer overflow.
4 here is a magic. Needs to be commented / described better.
> + *frac = quot & (~0U >> (32 - d->dlf_size));
> +
Operating on dfl_mask is simple as
u64 quot = p->uartclk * (p->dfl_mask + 1);
*frac = div64_u64(quot, baud * 16 * (p->dfl_mask + 1);
return quot;
(Perhaps some magic with types is needed, but you get the idea)
> + return quot >> d->dlf_size;
> +}
> +
> +static void dw8250_set_divisor(struct uart_port *p, unsigned int
> baud,
> + unsigned int quot, unsigned int
> quot_frac)
> +{
> + struct uart_8250_port *up = up_to_u8250p(p);
> +
> + serial_port_out(p, DW_UART_DLF >> p->regshift, quot_frac);
It should use the helper, not playing tricks with serial_port_out().
> + serial_port_out(p, UART_LCR, up->lcr | UART_LCR_DLAB);
> + serial_dl_write(up, quot);
At some point it would be a helper, I think. We can call
serial8250_do_set_divisor() here. So, perhaps we might export it.
> +}
> +
> static void dw8250_quirks(struct uart_port *p, struct dw8250_data
> *data)
> {
> if (p->dev->of_node) {
> @@ -414,6 +445,28 @@ static void dw8250_setup_port(struct uart_port
> *p)
> dev_dbg(p->dev, "Designware UART version %c.%c%c\n",
> (reg >> 24) & 0xff, (reg >> 16) & 0xff, (reg >> 8) &
> 0xff);
>
> + /*
> + * For version >= 4.00a, the dw uarts have a valid divisor
> latch
> + * fraction register. Calculate the fractional divisor width.
> + */
> + if (reg >= DW_FRAC_MIN_VERS) {
> + struct dw8250_data *d = p->private_data;
> +
> + if (p->iotype == UPIO_MEM32BE) {
> + iowrite32be(~0U, p->membase + DW_UART_DLF);
> + reg = ioread32be(p->membase + DW_UART_DLF);
> + } else {
> + writel(~0U, p->membase + DW_UART_DLF);
> + reg = readl(p->membase + DW_UART_DLF);
> + }
This should use some helpers. I'll prepare a patch soon and send it
here, you may include it in your series.
I think you need to clean up back them. So the flow like
1. Write all 1:s
2. Read back the value
3. Write all 0:s
> + d->dlf_size = fls(reg);
Just save value itself as dfl_mask.
> +
> + if (d->dlf_size) {
> + p->get_divisor = dw8250_get_divisor;
> + p->set_divisor = dw8250_set_divisor;
> + }
> + }
> +
> if (p->iotype == UPIO_MEM32BE)
> reg = ioread32be(p->membase + DW_UART_CPR);
> else
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH v2 3/3] serial: 8250_dw: add fractional divisor support
From: Jisheng Zhang @ 2018-07-05 6:39 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Greg Kroah-Hartman, Jiri Slaby, linux-arm-kernel, linux-kernel,
linux-serial
In-Reply-To: <f2733332a58e7f2d92948e2ff252581f93e5073a.camel@linux.intel.com>
Hi Andy,
On Wed, 04 Jul 2018 19:08:22 +0300 Andy Shevchenko wrote:
> On Wed, 2018-07-04 at 17:03 +0800, Jisheng Zhang wrote:
>
> Thanks for an update, my comments below.
>
> > For Synopsys DesignWare 8250 uart which version >= 4.00a, there's a
> > valid divisor latch fraction register. The fractional divisor width is
> > 4bits ~ 6bits.
>
> I have read 4.00a spec a bit and didn't find this limitation.
> The fractional divider can fit up to 32 bits.
the limitation isn't put in the register description, but in the description
about fractional divisor width configure parameters. Searching DLF_SIZE will
find it.
From another side, 32bits fractional div is a waste, for example, let's
assume the fract divisor = 0.9,
if fractional width is 4 bits, DLF reg will be set to UP(0.9*2^4) = 15, the
real frac divisor = 15/2^4 = 0.93;
if fractional width is 32 bits, DLF reg will be set to UP(0.9*2^32) = 3865470567
the real frac divisor = 3865470567/2^32 = 0.90;
>
> > Now the preparation is done, it's easy to add the feature support.
> > This patch firstly checks the component version during probe, if
> > version >= 4.00a, then calculates the fractional divisor width, then
> > setups dw specific get_divisor() and set_divisor() hook.
>
> > +#define DW_FRAC_MIN_VERS 0x3430302A
>
> Do we really need this?
>
> My intuition (I, unfortunately, didn't find any evidence in Synopsys
> specs for UART) tells me that when it's unimplemented we would read back
> 0's, which is fine.
yeah, I agree with you. I will remove this version check in the new version
>
> I would test this a bit later.
>
> >
>
> > + unsigned int dlf_size:3;
>
> These bit fields (besides the fact you need 5) more or less for software
> quirk flags. In your case I would rather keep a u32 value of DFL mask as
> done for msr slightly above in this structure.
OK. When setting the frac div, we use DLF size rather than mask, so could
I only calculate the DLF size once then use an u8 value to hold the
calculated DLF size rather than calculating every time?
>
> > };
> >
> > +/*
> > + * For version >= 4.00a, the dw uarts have a valid divisor latch
> > fraction
> > + * register. Calculate divisor with extra 4bits ~ 6bits fractional
> > portion.
> > + */
>
> This comment kinda noise. Better to put actual formula from datasheet
> how this fractional divider is involved into calculus.
yeah. Will do.
>
> > +static unsigned int dw8250_get_divisor(struct uart_port *p,
> > + unsigned int baud,
> > + unsigned int *frac)
> > +{
> > + unsigned int quot;
> > + struct dw8250_data *d = p->private_data;
> > +
>
> > + quot = DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4),
> > baud);
>
> If we have clock rate like 100MHz and 10 bits of fractional divider it
> would give an integer overflow.
This depends on the fraction divider width. If it's only 4~6 bits,
then we are fine.
>
> 4 here is a magic. Needs to be commented / described better.
Yes. divisor = clk/(16*baud) = BRD(I) + BRD(F)
"I" means integer, "F" means fractional
2^dlf_size*clk/(16*baud) = 2^dlf_size*(BRD(I) + BRD(F))
clk*2^(dlf_size - 4)/baud = 2^dlf_size*((BRD(I) + BRD(F))
the left part is "DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4), baud)",
let's assume it equals quot.
then, BRD(I) = quot / 2^dlf_size = quot >> dlf_size, this is where
"quot >> d->dlf_size" below from.
BRD(F) = quot & dlf_mask = quot & (~0U >> (32 - dlf_size))
>
> > + *frac = quot & (~0U >> (32 - d->dlf_size));
> > +
>
> Operating on dfl_mask is simple as
>
> u64 quot = p->uartclk * (p->dfl_mask + 1);
Since the dlf_mask is always 2^n - 1,
clk * (p->dlf_mask + 1) = clk << p->dlf_size, but the later
is simpler
>
> *frac = div64_u64(quot, baud * 16 * (p->dfl_mask + 1);
> return quot;
quot = DIV_ROUND_CLOSEST(p->uartclk << (d->dlf_size - 4), baud)
*frac = quot & (~0U >> (32 - d->dlf_size))
return quot >> d->dlf_size;
vs.
quot = p->uartclk * (p->dfl_mask + 1);
*frac = div64_u64(quot, baud * 16 * (p->dfl_mask + 1);
return quot;
shift vs mul.
If the dlf width is only 4~6 bits, the first calculation can avoid
64bit div. I prefer the first calculation.
>
> (Perhaps some magic with types is needed, but you get the idea)
>
> > + return quot >> d->dlf_size;
> > +}
> > +
> > +static void dw8250_set_divisor(struct uart_port *p, unsigned int
> > baud,
> > + unsigned int quot, unsigned int
> > quot_frac)
> > +{
> > + struct uart_8250_port *up = up_to_u8250p(p);
> > +
> > + serial_port_out(p, DW_UART_DLF >> p->regshift, quot_frac);
>
> It should use the helper, not playing tricks with serial_port_out().
I assume the helper here means the one you mentioned below, i.e in
if (p->iotype == UPIO_MEM32BE) {
...
} else {
...
}
>
> > + serial_port_out(p, UART_LCR, up->lcr | UART_LCR_DLAB);
> > + serial_dl_write(up, quot);
>
> At some point it would be a helper, I think. We can call
> serial8250_do_set_divisor() here. So, perhaps we might export it.
serial8250_do_set_divisor will drop the frac, that's not we want ;)
>
> > +}
> > +
> > static void dw8250_quirks(struct uart_port *p, struct dw8250_data
> > *data)
> > {
> > if (p->dev->of_node) {
> > @@ -414,6 +445,28 @@ static void dw8250_setup_port(struct uart_port
> > *p)
> > dev_dbg(p->dev, "Designware UART version %c.%c%c\n",
> > (reg >> 24) & 0xff, (reg >> 16) & 0xff, (reg >> 8) &
> > 0xff);
> >
> > + /*
> > + * For version >= 4.00a, the dw uarts have a valid divisor
> > latch
> > + * fraction register. Calculate the fractional divisor width.
> > + */
> > + if (reg >= DW_FRAC_MIN_VERS) {
> > + struct dw8250_data *d = p->private_data;
> > +
>
> > + if (p->iotype == UPIO_MEM32BE) {
> > + iowrite32be(~0U, p->membase + DW_UART_DLF);
> > + reg = ioread32be(p->membase + DW_UART_DLF);
> > + } else {
> > + writel(~0U, p->membase + DW_UART_DLF);
> > + reg = readl(p->membase + DW_UART_DLF);
> > + }
>
> This should use some helpers. I'll prepare a patch soon and send it
> here, you may include it in your series.
Nice. Thanks.
>
> I think you need to clean up back them. So the flow like
>
> 1. Write all 1:s
> 2. Read back the value
> 3. Write all 0:s
oh, yeah! will do
>
> > + d->dlf_size = fls(reg);
>
> Just save value itself as dfl_mask.
we use the dlf size during calculation, so it's better to hold the dlf_size
instead.
Thanks for the kind review,
Jisheng
^ permalink raw reply
* Re: [PATCH v2 3/3] serial: 8250_dw: add fractional divisor support
From: Jisheng Zhang @ 2018-07-05 6:54 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Greg Kroah-Hartman, Jiri Slaby, linux-arm-kernel, linux-kernel,
linux-serial
In-Reply-To: <20180705143921.6a8aeb50@xhacker.debian>
On Thu, 5 Jul 2018 14:39:21 +0800 Jisheng Zhang wrote:
<snip>
> >
> > > + serial_port_out(p, UART_LCR, up->lcr | UART_LCR_DLAB);
> > > + serial_dl_write(up, quot);
> >
> > At some point it would be a helper, I think. We can call
> > serial8250_do_set_divisor() here. So, perhaps we might export it.
>
> serial8250_do_set_divisor will drop the frac, that's not we want ;)
>
And most importantly, serial8250_do_set_divisor() will set a wrong BRD(I)
for fractional capable DW uarts. For example, clk = 25MHZ, baud = 115200.
In fractional capable DW uarts, we should set BRD(I) as
25000000/(16*115200) = 13
but serial8250_do_set_divisor() will set BRD(I) as
DIV_ROUND_CLOSEST(25*1000000, 16*115200)) = 14
Thanks
^ permalink raw reply
* Re: [PATCH v3 1/4] dt-bindings: mediatek: add support for mt6765 reference board
From: Rob Herring @ 2018-07-05 23:22 UTC (permalink / raw)
To: Mars Cheng
Cc: Matthias Brugger, Marc Zyngier, Greg Kroah-Hartman, CC Hwang,
Loda Chou, linux-kernel, linux-mediatek, devicetree, wsd_upstream,
linux-serial, linux-arm-kernel
In-Reply-To: <1530669174-17623-2-git-send-email-mars.cheng@mediatek.com>
On Wed, Jul 04, 2018 at 09:52:51AM +0800, Mars Cheng wrote:
> Update binding document for mt6765 reference board
>
> Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> ---
> Documentation/devicetree/bindings/arm/mediatek.txt | 4 ++++
> 1 file changed, 4 insertions(+)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 2/4] dt-bindings: mtk-uart: add mt6765 uart bindings
From: Rob Herring @ 2018-07-05 23:22 UTC (permalink / raw)
To: Mars Cheng
Cc: Matthias Brugger, Marc Zyngier, Greg Kroah-Hartman, CC Hwang,
Loda Chou, linux-kernel, linux-mediatek, devicetree, wsd_upstream,
linux-serial, linux-arm-kernel
In-Reply-To: <1530669174-17623-3-git-send-email-mars.cheng@mediatek.com>
On Wed, Jul 04, 2018 at 09:52:52AM +0800, Mars Cheng wrote:
> Add documentation for mt6765 uart dt-bindings
>
> Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> ---
> .../devicetree/bindings/serial/mtk-uart.txt | 1 +
> 1 file changed, 1 insertion(+)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 3/4] dt-bindings: interrupt-controller: add binding for mt6765
From: Rob Herring @ 2018-07-05 23:23 UTC (permalink / raw)
To: Mars Cheng
Cc: Matthias Brugger, Marc Zyngier, Greg Kroah-Hartman, CC Hwang,
Loda Chou, linux-kernel, linux-mediatek, devicetree, wsd_upstream,
linux-serial, linux-arm-kernel
In-Reply-To: <1530669174-17623-4-git-send-email-mars.cheng@mediatek.com>
On Wed, Jul 04, 2018 at 09:52:53AM +0800, Mars Cheng wrote:
> Update the dt-binding documentation of sysirq for mt6765
>
> Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> ---
> .../interrupt-controller/mediatek,sysirq.txt | 1 +
> 1 file changed, 1 insertion(+)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v2] uart: fix race between uart_put_char() and uart_shutdown()
From: Greg Kroah-Hartman @ 2018-07-06 14:39 UTC (permalink / raw)
To: Tycho Andersen; +Cc: Jiri Slaby, linux-serial, linux-kernel, Serge E . Hallyn
In-Reply-To: <20180629164330.GL27027@cisco.cisco.com>
On Fri, Jun 29, 2018 at 10:43:30AM -0600, Tycho Andersen wrote:
> On Fri, Jun 29, 2018 at 04:24:46AM -0600, Tycho Andersen wrote:
> > v2: switch to locking uport->lock on allocation/deallocation instead of
> > locking the per-port mutex in uart_put_char. Note that since
> > uport->lock is a spin lock, we have to switch the allocation to
> > GFP_ATOMIC.
>
> Serge pointed out off-list that we may want to do the allocation
> before the lock so that it's more likely to be successful. I'm happy
> to send that change to this if it's what we want to do, I don't have a
> strong preference.
That sounds like a much better thing to do.
thanks,
greg k-h
^ permalink raw reply
* [PATCH v3] uart: fix race between uart_put_char() and uart_shutdown()
From: Tycho Andersen @ 2018-07-06 16:24 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: linux-serial, linux-kernel, Serge E . Hallyn, Tycho Andersen
In-Reply-To: <20180706143919.GA2344@kroah.com>
We have reports of the following crash:
PID: 7 TASK: ffff88085c6d61c0 CPU: 1 COMMAND: "kworker/u25:0"
#0 [ffff88085c6db710] machine_kexec at ffffffff81046239
#1 [ffff88085c6db760] crash_kexec at ffffffff810fc248
#2 [ffff88085c6db830] oops_end at ffffffff81008ae7
#3 [ffff88085c6db860] no_context at ffffffff81050b8f
#4 [ffff88085c6db8b0] __bad_area_nosemaphore at ffffffff81050d75
#5 [ffff88085c6db900] bad_area_nosemaphore at ffffffff81050e83
#6 [ffff88085c6db910] __do_page_fault at ffffffff8105132e
#7 [ffff88085c6db9b0] do_page_fault at ffffffff8105152c
#8 [ffff88085c6db9c0] page_fault at ffffffff81a3f122
[exception RIP: uart_put_char+149]
RIP: ffffffff814b67b5 RSP: ffff88085c6dba78 RFLAGS: 00010006
RAX: 0000000000000292 RBX: ffffffff827c5120 RCX: 0000000000000081
RDX: 0000000000000000 RSI: 000000000000005f RDI: ffffffff827c5120
RBP: ffff88085c6dba98 R8: 000000000000012c R9: ffffffff822ea320
R10: ffff88085fe4db04 R11: 0000000000000001 R12: ffff881059f9c000
R13: 0000000000000001 R14: 000000000000005f R15: 0000000000000fba
ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018
#9 [ffff88085c6dbaa0] tty_put_char at ffffffff81497544
#10 [ffff88085c6dbac0] do_output_char at ffffffff8149c91c
#11 [ffff88085c6dbae0] __process_echoes at ffffffff8149cb8b
#12 [ffff88085c6dbb30] commit_echoes at ffffffff8149cdc2
#13 [ffff88085c6dbb60] n_tty_receive_buf_fast at ffffffff8149e49b
#14 [ffff88085c6dbbc0] __receive_buf at ffffffff8149ef5a
#15 [ffff88085c6dbc20] n_tty_receive_buf_common at ffffffff8149f016
#16 [ffff88085c6dbca0] n_tty_receive_buf2 at ffffffff8149f194
#17 [ffff88085c6dbcb0] flush_to_ldisc at ffffffff814a238a
#18 [ffff88085c6dbd50] process_one_work at ffffffff81090be2
#19 [ffff88085c6dbe20] worker_thread at ffffffff81091b4d
#20 [ffff88085c6dbeb0] kthread at ffffffff81096384
#21 [ffff88085c6dbf50] ret_from_fork at ffffffff81a3d69f
after slogging through some dissasembly:
ffffffff814b6720 <uart_put_char>:
ffffffff814b6720: 55 push %rbp
ffffffff814b6721: 48 89 e5 mov %rsp,%rbp
ffffffff814b6724: 48 83 ec 20 sub $0x20,%rsp
ffffffff814b6728: 48 89 1c 24 mov %rbx,(%rsp)
ffffffff814b672c: 4c 89 64 24 08 mov %r12,0x8(%rsp)
ffffffff814b6731: 4c 89 6c 24 10 mov %r13,0x10(%rsp)
ffffffff814b6736: 4c 89 74 24 18 mov %r14,0x18(%rsp)
ffffffff814b673b: e8 b0 8e 58 00 callq ffffffff81a3f5f0 <mcount>
ffffffff814b6740: 4c 8b a7 88 02 00 00 mov 0x288(%rdi),%r12
ffffffff814b6747: 45 31 ed xor %r13d,%r13d
ffffffff814b674a: 41 89 f6 mov %esi,%r14d
ffffffff814b674d: 49 83 bc 24 70 01 00 cmpq $0x0,0x170(%r12)
ffffffff814b6754: 00 00
ffffffff814b6756: 49 8b 9c 24 80 01 00 mov 0x180(%r12),%rbx
ffffffff814b675d: 00
ffffffff814b675e: 74 2f je ffffffff814b678f <uart_put_char+0x6f>
ffffffff814b6760: 48 89 df mov %rbx,%rdi
ffffffff814b6763: e8 a8 67 58 00 callq ffffffff81a3cf10 <_raw_spin_lock_irqsave>
ffffffff814b6768: 41 8b 8c 24 78 01 00 mov 0x178(%r12),%ecx
ffffffff814b676f: 00
ffffffff814b6770: 89 ca mov %ecx,%edx
ffffffff814b6772: f7 d2 not %edx
ffffffff814b6774: 41 03 94 24 7c 01 00 add 0x17c(%r12),%edx
ffffffff814b677b: 00
ffffffff814b677c: 81 e2 ff 0f 00 00 and $0xfff,%edx
ffffffff814b6782: 75 23 jne ffffffff814b67a7 <uart_put_char+0x87>
ffffffff814b6784: 48 89 c6 mov %rax,%rsi
ffffffff814b6787: 48 89 df mov %rbx,%rdi
ffffffff814b678a: e8 e1 64 58 00 callq ffffffff81a3cc70 <_raw_spin_unlock_irqrestore>
ffffffff814b678f: 44 89 e8 mov %r13d,%eax
ffffffff814b6792: 48 8b 1c 24 mov (%rsp),%rbx
ffffffff814b6796: 4c 8b 64 24 08 mov 0x8(%rsp),%r12
ffffffff814b679b: 4c 8b 6c 24 10 mov 0x10(%rsp),%r13
ffffffff814b67a0: 4c 8b 74 24 18 mov 0x18(%rsp),%r14
ffffffff814b67a5: c9 leaveq
ffffffff814b67a6: c3 retq
ffffffff814b67a7: 49 8b 94 24 70 01 00 mov 0x170(%r12),%rdx
ffffffff814b67ae: 00
ffffffff814b67af: 48 63 c9 movslq %ecx,%rcx
ffffffff814b67b2: 41 b5 01 mov $0x1,%r13b
ffffffff814b67b5: 44 88 34 0a mov %r14b,(%rdx,%rcx,1)
ffffffff814b67b9: 41 8b 94 24 78 01 00 mov 0x178(%r12),%edx
ffffffff814b67c0: 00
ffffffff814b67c1: 83 c2 01 add $0x1,%edx
ffffffff814b67c4: 81 e2 ff 0f 00 00 and $0xfff,%edx
ffffffff814b67ca: 41 89 94 24 78 01 00 mov %edx,0x178(%r12)
ffffffff814b67d1: 00
ffffffff814b67d2: eb b0 jmp ffffffff814b6784 <uart_put_char+0x64>
ffffffff814b67d4: 66 66 66 2e 0f 1f 84 data32 data32 nopw %cs:0x0(%rax,%rax,1)
ffffffff814b67db: 00 00 00 00 00
for our build, this is crashing at:
circ->buf[circ->head] = c;
Looking in uart_port_startup(), it seems that circ->buf (state->xmit.buf)
protected by the "per-port mutex", which based on uart_port_check() is
state->port.mutex. Indeed, the lock acquired in uart_put_char() is
uport->lock, i.e. not the same lock.
Anyway, since the lock is not acquired, if uart_shutdown() is called, the
last chunk of that function may release state->xmit.buf before its assigned
to null, and cause the race above.
To fix it, let's lock uport->lock when allocating/deallocating
state->xmit.buf in addition to the per-port mutex.
v2: switch to locking uport->lock on allocation/deallocation instead of
locking the per-port mutex in uart_put_char. Note that since
uport->lock is a spin lock, we have to switch the allocation to
GFP_ATOMIC.
v3: move the allocation outside the lock, so we can switch back to
GFP_KERNEL
Signed-off-by: Tycho Andersen <tycho@tycho.ws>
---
drivers/tty/serial/serial_core.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 9c14a453f73c..46bc97121e49 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -181,7 +181,7 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
int init_hw)
{
struct uart_port *uport = uart_port_check(state);
- unsigned long page;
+ unsigned long page, flags = 0;
int retval = 0;
if (uport->type == PORT_UNKNOWN)
@@ -196,15 +196,18 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
* Initialise and allocate the transmit and temporary
* buffer.
*/
- if (!state->xmit.buf) {
- /* This is protected by the per port mutex */
- page = get_zeroed_page(GFP_KERNEL);
- if (!page)
- return -ENOMEM;
+ page = get_zeroed_page(GFP_KERNEL);
+ if (!page)
+ return -ENOMEM;
+ uart_port_lock(state, flags);
+ if (!state->xmit.buf) {
state->xmit.buf = (unsigned char *) page;
uart_circ_clear(&state->xmit);
+ } else {
+ free_page(page);
}
+ uart_port_unlock(uport, flags);
retval = uport->ops->startup(uport);
if (retval == 0) {
@@ -263,6 +266,7 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
{
struct uart_port *uport = uart_port_check(state);
struct tty_port *port = &state->port;
+ unsigned long flags = 0;
/*
* Set the TTY IO error marker
@@ -295,10 +299,12 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
/*
* Free the transmit buffer page.
*/
+ uart_port_lock(state, flags);
if (state->xmit.buf) {
free_page((unsigned long)state->xmit.buf);
state->xmit.buf = NULL;
}
+ uart_port_unlock(uport, flags);
}
/**
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v3] uart: fix race between uart_put_char() and uart_shutdown()
From: Andy Shevchenko @ 2018-07-06 16:49 UTC (permalink / raw)
To: Tycho Andersen
Cc: Greg Kroah-Hartman, Jiri Slaby, open list:SERIAL DRIVERS,
Linux Kernel Mailing List, Serge E . Hallyn
In-Reply-To: <20180706162457.20489-1-tycho@tycho.ws>
On Fri, Jul 6, 2018 at 7:24 PM, Tycho Andersen <tycho@tycho.ws> wrote:
> Looking in uart_port_startup(), it seems that circ->buf (state->xmit.buf)
> protected by the "per-port mutex", which based on uart_port_check() is
> state->port.mutex. Indeed, the lock acquired in uart_put_char() is
> uport->lock, i.e. not the same lock.
>
> Anyway, since the lock is not acquired, if uart_shutdown() is called, the
> last chunk of that function may release state->xmit.buf before its assigned
> to null, and cause the race above.
>
> To fix it, let's lock uport->lock when allocating/deallocating
> state->xmit.buf in addition to the per-port mutex.
Thanks for fixing this!
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Some nitpicks though.
> + unsigned long page, flags = 0;
I would rather put on separate lines and btw assignment is not needed.
It all goes through macros.
> - if (!state->xmit.buf) {
> - /* This is protected by the per port mutex */
> - page = get_zeroed_page(GFP_KERNEL);
> - if (!page)
> - return -ENOMEM;
> + page = get_zeroed_page(GFP_KERNEL);
> + if (!page)
> + return -ENOMEM;
> + if (!state->xmit.buf) {
> state->xmit.buf = (unsigned char *) page;
> uart_circ_clear(&state->xmit);
> + } else {
> + free_page(page);
> }
I see original code, but since you are adding else, does it make sense
to switch to positive condition?
> + unsigned long flags = 0;
Ditto about assignment.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox