* [PATCH v5 2/2] arm64: dts: Add Mediatek SoC MT2712 and evaluation board dts and Makefile
From: YT Shen @ 2017-07-28 11:37 UTC (permalink / raw)
To: Rob Herring, Matthias Brugger
Cc: Mark Rutland, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Greg Kroah-Hartman, Catalin Marinas, Will Deacon, Mars Cheng,
YT Shen, devicetree, linux-kernel, linux-serial, linux-arm-kernel,
linux-mediatek, srv_heupstream
In-Reply-To: <1501241835-47849-1-git-send-email-yt.shen@mediatek.com>
This adds basic chip support for Mediatek 2712
Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
arch/arm64/boot/dts/mediatek/Makefile | 1 +
arch/arm64/boot/dts/mediatek/mt2712-evb.dts | 32 +++++
arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 178 ++++++++++++++++++++++++++++
3 files changed, 211 insertions(+)
create mode 100644 arch/arm64/boot/dts/mediatek/mt2712-evb.dts
create mode 100644 arch/arm64/boot/dts/mediatek/mt2712e.dtsi
diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
index 015eb07..f323936 100644
--- a/arch/arm64/boot/dts/mediatek/Makefile
+++ b/arch/arm64/boot/dts/mediatek/Makefile
@@ -1,3 +1,4 @@
+dtb-$(CONFIG_ARCH_MEDIATEK) += mt2712-evb.dtb
dtb-$(CONFIG_ARCH_MEDIATEK) += mt6755-evb.dtb
dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-evb.dtb
dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
diff --git a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
new file mode 100644
index 0000000..8c804df
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
@@ -0,0 +1,32 @@
+/*
+ * Copyright (c) 2017 MediaTek Inc.
+ * Author: YT Shen <yt.shen@mediatek.com>
+ *
+ * SPDX-License-Identifier: (GPL-2.0 OR MIT)
+ */
+
+/dts-v1/;
+#include "mt2712e.dtsi"
+
+/ {
+ model = "MediaTek MT2712 evaluation board";
+ compatible = "mediatek,mt2712-evb", "mediatek,mt2712";
+
+ aliases {
+ serial0 = &uart0;
+ };
+
+ memory@40000000 {
+ device_type = "memory";
+ reg = <0 0x40000000 0 0x80000000>;
+ };
+
+ chosen {
+ stdout-path = "serial0:921600n8";
+ };
+};
+
+&uart0 {
+ status = "okay";
+};
+
diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
new file mode 100644
index 0000000..1e135af
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
@@ -0,0 +1,178 @@
+/*
+ * Copyright (c) 2017 MediaTek Inc.
+ * Author: YT Shen <yt.shen@mediatek.com>
+ *
+ * SPDX-License-Identifier: (GPL-2.0 OR MIT)
+ */
+
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/ {
+ compatible = "mediatek,mt2712";
+ interrupt-parent = <&sysirq>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpu-map {
+ cluster0 {
+ core0 {
+ cpu = <&cpu0>;
+ };
+ core1 {
+ cpu = <&cpu1>;
+ };
+ };
+
+ cluster1 {
+ core0 {
+ cpu = <&cpu2>;
+ };
+ };
+ };
+
+ cpu0: cpu@0 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a35";
+ reg = <0x000>;
+ };
+
+ cpu1: cpu@1 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a35";
+ reg = <0x001>;
+ enable-method = "psci";
+ };
+
+ cpu2: cpu@200 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <0x200>;
+ enable-method = "psci";
+ };
+ };
+
+ psci {
+ compatible = "arm,psci-0.2";
+ method = "smc";
+ };
+
+ baud_clk: dummy26m {
+ compatible = "fixed-clock";
+ clock-frequency = <26000000>;
+ #clock-cells = <0>;
+ };
+
+ sys_clk: dummyclk {
+ compatible = "fixed-clock";
+ clock-frequency = <26000000>;
+ #clock-cells = <0>;
+ };
+
+ timer {
+ compatible = "arm,armv8-timer";
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_PPI 13
+ (GIC_CPU_MASK_RAW(0x13) | IRQ_TYPE_LEVEL_LOW)>,
+ <GIC_PPI 14
+ (GIC_CPU_MASK_RAW(0x13) | IRQ_TYPE_LEVEL_LOW)>,
+ <GIC_PPI 11
+ (GIC_CPU_MASK_RAW(0x13) | IRQ_TYPE_LEVEL_LOW)>,
+ <GIC_PPI 10
+ (GIC_CPU_MASK_RAW(0x13) | IRQ_TYPE_LEVEL_LOW)>;
+ };
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ compatible = "simple-bus";
+ ranges;
+
+ uart5: serial@1000f000 {
+ compatible = "mediatek,mt2712-uart",
+ "mediatek,mt6577-uart";
+ reg = <0 0x1000f000 0 0x400>;
+ interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&baud_clk>, <&sys_clk>;
+ clock-names = "baud", "bus";
+ status = "disabled";
+ };
+
+ sysirq: interrupt-controller@10220a80 {
+ compatible = "mediatek,mt2712-sysirq",
+ "mediatek,mt6577-sysirq";
+ interrupt-controller;
+ #interrupt-cells = <3>;
+ interrupt-parent = <&gic>;
+ reg = <0 0x10220a80 0 0x40>;
+ };
+
+ gic: interrupt-controller@10510000 {
+ compatible = "arm,gic-400";
+ #interrupt-cells = <3>;
+ interrupt-parent = <&gic>;
+ interrupt-controller;
+ reg = <0 0x10510000 0 0x10000>,
+ <0 0x10520000 0 0x20000>,
+ <0 0x10540000 0 0x20000>,
+ <0 0x10560000 0 0x20000>;
+ interrupts = <GIC_PPI 9
+ (GIC_CPU_MASK_RAW(0x13) | IRQ_TYPE_LEVEL_HIGH)>;
+ };
+
+ uart0: serial@11002000 {
+ compatible = "mediatek,mt2712-uart",
+ "mediatek,mt6577-uart";
+ reg = <0 0x11002000 0 0x400>;
+ interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&baud_clk>, <&sys_clk>;
+ clock-names = "baud", "bus";
+ status = "disabled";
+ };
+
+ uart1: serial@11003000 {
+ compatible = "mediatek,mt2712-uart",
+ "mediatek,mt6577-uart";
+ reg = <0 0x11003000 0 0x400>;
+ interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&baud_clk>, <&sys_clk>;
+ clock-names = "baud", "bus";
+ status = "disabled";
+ };
+
+ uart2: serial@11004000 {
+ compatible = "mediatek,mt2712-uart",
+ "mediatek,mt6577-uart";
+ reg = <0 0x11004000 0 0x400>;
+ interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&baud_clk>, <&sys_clk>;
+ clock-names = "baud", "bus";
+ status = "disabled";
+ };
+
+ uart3: serial@11005000 {
+ compatible = "mediatek,mt2712-uart",
+ "mediatek,mt6577-uart";
+ reg = <0 0x11005000 0 0x400>;
+ interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&baud_clk>, <&sys_clk>;
+ clock-names = "baud", "bus";
+ status = "disabled";
+ };
+
+ uart4: serial@11019000 {
+ compatible = "mediatek,mt2712-uart",
+ "mediatek,mt6577-uart";
+ reg = <0 0x11019000 0 0x400>;
+ interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&baud_clk>, <&sys_clk>;
+ clock-names = "baud", "bus";
+ status = "disabled";
+ };
+ };
+};
+
--
1.9.1
^ permalink raw reply related
* Re: [RFC PATCH 0/4] serial: uartps: Dynamic allocation
From: Alan Cox @ 2017-07-28 18:39 UTC (permalink / raw)
To: Michal Simek
Cc: monstr, Greg Kroah-Hartman, linux-kernel, linux-arm-kernel,
linux-serial, Jiri Slaby, Sören Brinkmann
In-Reply-To: <cover.1500629544.git.michal.simek@xilinx.com>
On Fri, 21 Jul 2017 11:32:23 +0200
Michal Simek <michal.simek@xilinx.com> wrote:
> Hi Alan,
>
> this is the initial version before next step which is move
> uart_register_driver to probe function.
> I was able to get rid of static array with uart_port structures.
> It was wired with console which is also fixed.
> And the next step is the most complicated one handle .nr in uart_driver
> structure in more generic way.
>
> Thanks,
> Michal
Sorry for the delay been on jury service
Series
Reviewed-by: Alan Cox <alan@linux.intel.com>
^ permalink raw reply
* [PATCH] drivers/serial: Do not leave sysfs group in case of error in aspeed_vuart_probe()
From: Alexey Khoroshilov @ 2017-07-28 18:54 UTC (permalink / raw)
To: Jeremy Kerr, Joel Stanley
Cc: Alexey Khoroshilov, Rob Herring, Benjamin Herrenschmidt,
Andy Shevchenko, Greg Kroah-Hartman, linux-serial, linux-kernel,
ldv-project
There are several error handling paths in aspeed_vuart_probe(),
where sysfs group is left unremoved. The patch fixes them.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/tty/serial/8250/8250_aspeed_vuart.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
index 822be4906763..33a801353114 100644
--- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -223,12 +223,13 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
if (IS_ERR(vuart->clk)) {
dev_warn(&pdev->dev,
"clk or clock-frequency not defined\n");
- return PTR_ERR(vuart->clk);
+ rc = PTR_ERR(vuart->clk);
+ goto err_sysfs_remove;
}
rc = clk_prepare_enable(vuart->clk);
if (rc < 0)
- return rc;
+ goto err_sysfs_remove;
clk = clk_get_rate(vuart->clk);
}
@@ -286,6 +287,8 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
err_clk_disable:
clk_disable_unprepare(vuart->clk);
irq_dispose_mapping(port.port.irq);
+err_sysfs_remove:
+ sysfs_remove_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
return rc;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 0/5] earlycon hang under some conditions
From: Andy Shevchenko @ 2017-07-30 14:31 UTC (permalink / raw)
To: Jeffy Chen
Cc: linux-kernel@vger.kernel.org, Greg Kroah-Hartman, Brian Norris,
Douglas Anderson, Peter Hurley, Sören Brinkmann,
Michal Simek, Marc Gonzalez, Jiri Slaby, Vineet Gupta,
linux-serial@vger.kernel.org, linux-snps-arc,
linux-arm Mailing List
In-Reply-To: <1500357778-23169-1-git-send-email-jeffy.chen@rock-chips.com>
On Tue, Jul 18, 2017 at 9:02 AM, Jeffy Chen <jeffy.chen@rock-chips.com> wrote:
> I was testing earlycon with 8250 dw serial console. And it hangs in
> these cases:
> 1/ kernel hang when calling early write function after free_initmem:
> a) the earlycon not disabled after the init code(due to keep_bootcon or
> not specify a real console to switch to)
> b) the early write func is marked as __init, for example 8250_early.
> This serial fix 1/ case only.
Here is another opinion.
I can tell that conditions to get into the trouble are quite unusual or rare.
Any rationale behind why one needs such a combination of parameters?
P.S. For doulble output ask Sergey and Petr (they are quite concerned
about it). Actually strange I didn't see their names in Cc list for
console related stuff,
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 4/9] serial: stm32: fix pio transmit timeout
From: Greg Kroah-Hartman @ 2017-07-30 14:32 UTC (permalink / raw)
To: Bich HEMON
Cc: Rob Herring, Mark Rutland, Maxime Coquelin, Alexandre TORGUE,
Jiri Slaby, linux-serial@vger.kernel.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1499958494-19354-5-git-send-email-bich.hemon@st.com>
On Thu, Jul 13, 2017 at 03:08:28PM +0000, Bich HEMON wrote:
> From: Gerald Baeza <gerald.baeza@st.com>
>
> 100µs was too short for low speed transmission
> (9600bps)
>
> Signed-off-by: Gerald Baeza <gerald.baeza@st.com>
> Signed-off-by: Bich Hemon <bich.hemon@st.com>
> ---
> drivers/tty/serial/stm32-usart.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
This patch did not apply :(
^ permalink raw reply
* Re: [PATCH v4 3/9] serial: Add common rs485 device tree parsing function
From: Greg KH @ 2017-07-30 14:49 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sascha Hauer,
devicetree-u79uwXL29TY76Z2rM5mHXA, Nicolas Ferre,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <20170718105948.21986-4-u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On Tue, Jul 18, 2017 at 12:59:42PM +0200, Uwe Kleine-König wrote:
> From: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>
> Several drivers have the same device tree parsing code. Create
> a common helper function for it.
>
> Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> [ukl: implement default <0 0> for rts-delay, unset unspecified flags]
> Acked-by: Nicolas Ferre <nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> drivers/tty/serial/Kconfig | 4 ++++
> drivers/tty/serial/Makefile | 2 ++
> drivers/tty/serial/of.c | 47 +++++++++++++++++++++++++++++++++++++++++++++
Why is this a separate file, with a Kconfig option that is always
enabled? Why not just add the single function to the serial core?
> +/**
> + * of_get_rs485_mode() - Implement parsing rs485 properties
> + * @np: uart node
> + * @rs485conf: output parameter
> + *
> + * This function implements the device tree binding described in
> + * Documentation/devicetree/bindings/serial/rs485.txt.
> + *
> + * Return: 0 on success, 1 if the node doesn't contain rs485 stuff.
1 is not an error code :(
Return a proper error please.
> + */
> +int of_get_rs485_mode(struct device_node *np, struct serial_rs485 *rs485conf)
> +{
> + u32 rs485_delay[2];
> + int ret;
> +
> + if (!IS_ENABLED(CONFIG_OF) || !np)
> + return 1;
> +
> + ret = of_property_read_u32_array(np, "rs485-rts-delay", rs485_delay, 2);
> + if (!ret) {
> + rs485conf->delay_rts_before_send = rs485_delay[0];
> + rs485conf->delay_rts_after_send = rs485_delay[1];
> + } else {
> + rs485conf->delay_rts_before_send = 0;
> + rs485conf->delay_rts_after_send = 0;
> + }
> +
> + /*
> + * clear full-duplex and enabled flags to get to a defined state with
> + * the two following properties.
> + */
> + rs485conf->flags &= ~(SER_RS485_RX_DURING_TX | SER_RS485_ENABLED);
> +
> + if (of_property_read_bool(np, "rs485-rx-during-tx"))
> + rs485conf->flags |= SER_RS485_RX_DURING_TX;
> +
> + if (of_property_read_bool(np, "linux,rs485-enabled-at-boot-time"))
> + rs485conf->flags |= SER_RS485_ENABLED;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_get_rs485_mode);
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 1775500294bb..3a89e8925722 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -501,4 +501,17 @@ static inline int uart_handle_break(struct uart_port *port)
> (cflag) & CRTSCTS || \
> !((cflag) & CLOCAL))
>
> +/*
> + * Common device tree parsing helpers
> + */
> +#ifdef CONFIG_OF_SERIAL
> +int of_get_rs485_mode(struct device_node *np, struct serial_rs485 *rs485conf);
> +#else
> +static inline int of_get_rs485_mode(struct device_node *np,
> + struct serial_rs485 *rs485conf)
> +{
> + return 1;
Again, a real error.
And why not have a "generic" firmware function for this that defaults to
the of_ for that platform type if present? What's the odds that acpi
will need/want this soon anyway?
thanks,
greg k-h
--
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 4/9] serial: atmel: Use common rs485 device tree parsing function
From: Greg KH @ 2017-07-30 14:50 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sascha Hauer,
Richard Genoud, devicetree-u79uwXL29TY76Z2rM5mHXA,
Alexandre Belloni, kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <20170718105948.21986-5-u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On Tue, Jul 18, 2017 at 12:59:43PM +0200, Uwe Kleine-König wrote:
> From: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>
> We just got a common helper for parsing the rs485 specific
> device tree properties. Use it and drop the open coded parser.
>
> Note that there is a small difference between the removed and the now
> used implementation: The former cleared flags to 0 if rs485-rts-delay
> was given, the common helper clears SER_RS485_RX_DURING_TX and
> SER_RS485_ENABLED only but always which makes more sense.
>
> Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> [ukleinek: point out semantic change in commit log]
> Acked-by: Nicolas Ferre <nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
> Acked-by: Richard Genoud <richard.genoud-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> drivers/tty/serial/atmel_serial.c | 25 +------------------------
> 1 file changed, 1 insertion(+), 24 deletions(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 7551cab438ff..04e55f010894 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -1667,29 +1667,6 @@ static void atmel_init_property(struct atmel_uart_port *atmel_port,
> }
> }
>
> -static void atmel_init_rs485(struct uart_port *port,
> - struct platform_device *pdev)
> -{
> - struct device_node *np = pdev->dev.of_node;
> -
> - struct serial_rs485 *rs485conf = &port->rs485;
> - u32 rs485_delay[2];
> -
> - /* rs485 properties */
> - if (of_property_read_u32_array(np, "rs485-rts-delay",
> - rs485_delay, 2) == 0) {
> - rs485conf->delay_rts_before_send = rs485_delay[0];
> - rs485conf->delay_rts_after_send = rs485_delay[1];
> - rs485conf->flags = 0;
> - }
> -
> - if (of_get_property(np, "rs485-rx-during-tx", NULL))
> - rs485conf->flags |= SER_RS485_RX_DURING_TX;
> -
> - if (of_get_property(np, "linux,rs485-enabled-at-boot-time", NULL))
> - rs485conf->flags |= SER_RS485_ENABLED;
> -}
> -
> static void atmel_set_ops(struct uart_port *port)
> {
> struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> @@ -2373,7 +2350,7 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
> atmel_init_property(atmel_port, pdev);
> atmel_set_ops(port);
>
> - atmel_init_rs485(port, pdev);
> + of_get_rs485_mode(pdev->dev.of_node, &port->rs485);
No error checking?
--
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 5/9] serial: fsl_lpuart: Use common rs485 device tree parsing function
From: Greg KH @ 2017-07-30 14:50 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sascha Hauer,
Fabio Estevam, devicetree-u79uwXL29TY76Z2rM5mHXA, Shawn Guo,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <20170718105948.21986-6-u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On Tue, Jul 18, 2017 at 12:59:44PM +0200, Uwe Kleine-König wrote:
> From: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>
> We just got a common helper for parsing the rs485 specific
> device tree properties. Use it and drop the open coded parser.
>
> As a side effect this adds support for parsing rs485-rts-delay and
> rs485-rx-during-tx. As the driver doesn't support this though, probing
> fails if these are defined.
>
> Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> drivers/tty/serial/fsl_lpuart.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index 4becfbf35c4c..cac81b143abe 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -2161,6 +2161,24 @@ static int lpuart_probe(struct platform_device *pdev)
> return ret;
> }
>
> + of_get_rs485_mode(np, &sport->port.rs485);
error checking?
Elsewhere in this series as well.
Also, why did you not cc: me on this series so I am sure to see it?
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2 0/3] tty: serial: 8250 introduce mctrl_gpio helpers
From: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg @ 2017-07-31 7:13 UTC (permalink / raw)
To: linux-serial-u79uwXL29TY76Z2rM5mHXA
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA, jslaby-IBi9RG/b67k,
tony-4v6yS6AI5VpBDgjK7y7TUQ, robh-DgEjT+Ai2ygdnm+yROfE0A,
mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, julia.lawall-L2FTfq7BK8M,
peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8, Yegor Yefremov
From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
This patch series aims to reintroduce the mctrl_gpio helpers for 8250
UARTs.
There are some UARTs that use GPIO signals as a wakeup-sourse.
The first patch addresses this issue and tries to destinguish GPIO usage
via searching for "wakeup-sourse" property. Though it must be decided whether
this property is secure to use for this purpose.
Changes v1 -> v2:
- unlock mutex, if mctrl_gpio_init fails (suggested by Julia Lawall)
- describe -ENOSYS return value choice (suggested by Andy Shevchenko)
Yegor Yefremov (3):
serial: mctrl_gpio: restrict MCTRL initialization
tty/serial/8250: use mctrl_gpio helpers
tty/serial/8250: add DT bindings description for mctrl_gpio helpers
Documentation/devicetree/bindings/serial/8250.txt | 26 +++++++++++++++++
drivers/tty/serial/8250/8250.h | 35 ++++++++++++++++++++++-
drivers/tty/serial/8250/8250_core.c | 12 ++++++++
drivers/tty/serial/8250/8250_omap.c | 31 +++++++++++---------
drivers/tty/serial/8250/8250_port.c | 7 ++++-
drivers/tty/serial/8250/Kconfig | 1 +
drivers/tty/serial/serial_mctrl_gpio.c | 4 +++
include/linux/serial_8250.h | 1 +
8 files changed, 102 insertions(+), 15 deletions(-)
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2 1/3] serial: mctrl_gpio: restrict MCTRL initialization
From: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg @ 2017-07-31 7:13 UTC (permalink / raw)
To: linux-serial-u79uwXL29TY76Z2rM5mHXA
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA, jslaby-IBi9RG/b67k,
tony-4v6yS6AI5VpBDgjK7y7TUQ, robh-DgEjT+Ai2ygdnm+yROfE0A,
mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, julia.lawall-L2FTfq7BK8M,
peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8, Yegor Yefremov
In-Reply-To: <1501485203-13185-1-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
GPIOs specified for serial port can be used either as GPIO driven MCTRL
or as a wakeup-source. Use the latter property to abort the MCTRL
initialization.
Return -ENOSYS as this would just skip mctrl initialization in the
case of wakeup source usage and not disable the port completely due to
failed serial8250_register_8250_port().
Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
drivers/tty/serial/serial_mctrl_gpio.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index d2da6aa..dce661c 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -19,6 +19,7 @@
#include <linux/irq.h>
#include <linux/gpio/consumer.h>
#include <linux/termios.h>
+#include <linux/property.h>
#include <linux/serial_core.h>
#include <linux/module.h>
@@ -118,6 +119,9 @@ struct mctrl_gpios *mctrl_gpio_init_noauto(struct device *dev, unsigned int idx)
struct mctrl_gpios *gpios;
enum mctrl_gpio_idx i;
+ if (device_property_present(dev, "wakeup-source"))
+ return ERR_PTR(-ENOSYS);
+
gpios = devm_kzalloc(dev, sizeof(*gpios), GFP_KERNEL);
if (!gpios)
return ERR_PTR(-ENOMEM);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v2 2/3] tty/serial/8250: use mctrl_gpio helpers
From: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg @ 2017-07-31 7:13 UTC (permalink / raw)
To: linux-serial-u79uwXL29TY76Z2rM5mHXA
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA, jslaby-IBi9RG/b67k,
tony-4v6yS6AI5VpBDgjK7y7TUQ, robh-DgEjT+Ai2ygdnm+yROfE0A,
mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, julia.lawall-L2FTfq7BK8M,
peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8, Yegor Yefremov
In-Reply-To: <1501485203-13185-1-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
This patch permits the usage for GPIOs to control
the CTS/RTS/DTR/DSR/DCD/RI signals.
Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
drivers/tty/serial/8250/8250.h | 35 ++++++++++++++++++++++++++++++++++-
drivers/tty/serial/8250/8250_core.c | 12 ++++++++++++
drivers/tty/serial/8250/8250_omap.c | 31 ++++++++++++++++++-------------
drivers/tty/serial/8250/8250_port.c | 7 ++++++-
drivers/tty/serial/8250/Kconfig | 1 +
include/linux/serial_8250.h | 1 +
6 files changed, 72 insertions(+), 15 deletions(-)
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index b2bdc35..c47ef69 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -15,6 +15,8 @@
#include <linux/serial_reg.h>
#include <linux/dmaengine.h>
+#include "../serial_mctrl_gpio.h"
+
struct uart_8250_dma {
int (*tx_dma)(struct uart_8250_port *p);
int (*rx_dma)(struct uart_8250_port *p);
@@ -145,12 +147,43 @@ void serial8250_em485_destroy(struct uart_8250_port *p);
static inline void serial8250_out_MCR(struct uart_8250_port *up, int value)
{
+ int mctrl_gpio = 0;
+
serial_out(up, UART_MCR, value);
+
+ if (value & UART_MCR_RTS)
+ mctrl_gpio |= TIOCM_RTS;
+ if (value & UART_MCR_DTR)
+ mctrl_gpio |= TIOCM_DTR;
+
+ mctrl_gpio_set(up->gpios, mctrl_gpio);
}
static inline int serial8250_in_MCR(struct uart_8250_port *up)
{
- return serial_in(up, UART_MCR);
+ int mctrl, mctrl_gpio = 0;
+
+ mctrl = serial_in(up, UART_MCR);
+
+ /* save current MCR values */
+ if (mctrl & UART_MCR_RTS)
+ mctrl_gpio |= TIOCM_RTS;
+ if (mctrl & UART_MCR_DTR)
+ mctrl_gpio |= TIOCM_DTR;
+
+ mctrl_gpio = mctrl_gpio_get_outputs(up->gpios, &mctrl_gpio);
+
+ if (mctrl_gpio & TIOCM_RTS)
+ mctrl |= UART_MCR_RTS;
+ else
+ mctrl &= ~UART_MCR_RTS;
+
+ if (mctrl_gpio & TIOCM_DTR)
+ mctrl |= UART_MCR_DTR;
+ else
+ mctrl &= ~UART_MCR_DTR;
+
+ return mctrl;
}
#if defined(__alpha__) && !defined(CONFIG_PCI)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index b5def35..3371c5c 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -975,6 +975,8 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
uart = serial8250_find_match_or_unused(&up->port);
if (uart && uart->port.type != PORT_8250_CIR) {
+ struct mctrl_gpios *gpios;
+
if (uart->port.dev)
uart_remove_one_port(&serial8250_reg, &uart->port);
@@ -1012,6 +1014,15 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
if (up->port.flags & UPF_FIXED_TYPE)
uart->port.type = up->port.type;
+ gpios = mctrl_gpio_init(&uart->port, 0);
+ if (IS_ERR(gpios)) {
+ if (PTR_ERR(gpios) != -ENOSYS) {
+ ret = PTR_ERR(gpios);
+ goto out;
+ }
+ } else
+ uart->gpios = gpios;
+
serial8250_set_defaults(uart);
/* Possibly override default I/O functions. */
@@ -1051,6 +1062,7 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
if (ret == 0)
ret = uart->port.line;
}
+out:
mutex_unlock(&serial_mutex);
return ret;
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 833771b..93fe5b9 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -134,18 +134,21 @@ static void omap8250_set_mctrl(struct uart_port *port, unsigned int mctrl)
serial8250_do_set_mctrl(port, mctrl);
- /*
- * Turn off autoRTS if RTS is lowered and restore autoRTS setting
- * if RTS is raised
- */
- lcr = serial_in(up, UART_LCR);
- serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
- if ((mctrl & TIOCM_RTS) && (port->status & UPSTAT_AUTORTS))
- priv->efr |= UART_EFR_RTS;
- else
- priv->efr &= ~UART_EFR_RTS;
- serial_out(up, UART_EFR, priv->efr);
- serial_out(up, UART_LCR, lcr);
+ if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(up->gpios,
+ UART_GPIO_RTS))) {
+ /*
+ * Turn off autoRTS if RTS is lowered and restore autoRTS
+ * setting if RTS is raised
+ */
+ lcr = serial_in(up, UART_LCR);
+ serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+ if ((mctrl & TIOCM_RTS) && (port->status & UPSTAT_AUTORTS))
+ priv->efr |= UART_EFR_RTS;
+ else
+ priv->efr &= ~UART_EFR_RTS;
+ serial_out(up, UART_EFR, priv->efr);
+ serial_out(up, UART_LCR, lcr);
+ }
}
/*
@@ -446,7 +449,9 @@ static void omap_8250_set_termios(struct uart_port *port,
priv->efr = 0;
up->port.status &= ~(UPSTAT_AUTOCTS | UPSTAT_AUTORTS | UPSTAT_AUTOXOFF);
- if (termios->c_cflag & CRTSCTS && up->port.flags & UPF_HARD_FLOW) {
+ if (termios->c_cflag & CRTSCTS && up->port.flags & UPF_HARD_FLOW
+ && IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(up->gpios,
+ UART_GPIO_RTS))) {
/* Enable AUTOCTS (autoRTS is enabled when RTS is raised) */
up->port.status |= UPSTAT_AUTOCTS | UPSTAT_AUTORTS;
priv->efr |= UART_EFR_CTS;
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index a5fe0e6..f173cda 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1631,6 +1631,8 @@ static void serial8250_disable_ms(struct uart_port *port)
if (up->bugs & UART_BUG_NOMSR)
return;
+ mctrl_gpio_disable_ms(up->gpios);
+
up->ier &= ~UART_IER_MSI;
serial_port_out(port, UART_IER, up->ier);
}
@@ -1643,6 +1645,8 @@ static void serial8250_enable_ms(struct uart_port *port)
if (up->bugs & UART_BUG_NOMSR)
return;
+ mctrl_gpio_enable_ms(up->gpios);
+
up->ier |= UART_IER_MSI;
serial8250_rpm_get(up);
@@ -1946,7 +1950,8 @@ unsigned int serial8250_do_get_mctrl(struct uart_port *port)
ret |= TIOCM_DSR;
if (status & UART_MSR_CTS)
ret |= TIOCM_CTS;
- return ret;
+
+ return mctrl_gpio_get(up->gpios, &ret);
}
EXPORT_SYMBOL_GPL(serial8250_do_get_mctrl);
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index a1161ec..c0e51b0 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -6,6 +6,7 @@
config SERIAL_8250
tristate "8250/16550 and compatible serial support"
select SERIAL_CORE
+ select SERIAL_MCTRL_GPIO if GPIOLIB
---help---
This selects whether you want to include the driver for the standard
serial ports. The standard answer is Y. People who might say N
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index 61fbb44..23b6ab1 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -113,6 +113,7 @@ struct uart_8250_port {
* if no_console_suspend
*/
unsigned char probe;
+ struct mctrl_gpios *gpios;
#define UART_PROBE_RSA (1 << 0)
/*
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v2 3/3] tty/serial/8250: add DT bindings description for mctrl_gpio helpers
From: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg @ 2017-07-31 7:13 UTC (permalink / raw)
To: linux-serial-u79uwXL29TY76Z2rM5mHXA
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA, jslaby-IBi9RG/b67k,
tony-4v6yS6AI5VpBDgjK7y7TUQ, robh-DgEjT+Ai2ygdnm+yROfE0A,
mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, julia.lawall-L2FTfq7BK8M,
peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8, Yegor Yefremov
In-Reply-To: <1501485203-13185-1-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Describe {rts,cts,dtr,dsr,rng,dcd}-gpios usage.
Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
Documentation/devicetree/bindings/serial/8250.txt | 26 +++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
index 419ff6c..1c09db2 100644
--- a/Documentation/devicetree/bindings/serial/8250.txt
+++ b/Documentation/devicetree/bindings/serial/8250.txt
@@ -48,6 +48,16 @@ Optional properties:
- tx-threshold: Specify the TX FIFO low water indication for parts with
programmable TX FIFO thresholds.
- resets : phandle + reset specifier pairs
+- rts-gpios:
+- cts-gpios:
+- dtr-gpios:
+- dsr-gpios:
+- rng-gpios:
+- dcd-gpios: specify a GPIO for RTS/CTS/DTR/DSR/RI/DCD
+ line respectively. It will use specified GPIO instead of the peripheral
+ function pin for the UART feature. If unsure, don't specify this property.
+ If "wakeup-source" property is specified for the device this GPIO
+ initialization will be aborted.
Note:
* fsl,ns16550:
@@ -69,3 +79,19 @@ Example:
interrupts = <10>;
reg-shift = <2>;
};
+
+Example for OMAP UART using GPIO-based modem control signals:
+
+ uart4: serial@49042000 {
+ compatible = "ti,omap3-uart";
+ reg = <0x49042000 0x400>;
+ interrupts = <80>;
+ ti,hwmods = "uart4";
+ clock-frequency = <48000000>;
+ cts-gpios = <&gpio3 5 GPIO_ACTIVE_LOW>;
+ rts-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
+ dtr-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
+ dsr-gpios = <&gpio1 13 GPIO_ACTIVE_LOW>;
+ dcd-gpios = <&gpio1 14 GPIO_ACTIVE_LOW>;
+ rng-gpios = <&gpio1 15 GPIO_ACTIVE_LOW>;
+ };
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [RFC PATCH 1/4] serial: uartps: Remove console_initcall from the driver
From: Michal Simek @ 2017-07-31 7:37 UTC (permalink / raw)
To: Sören Brinkmann, Michal Simek
Cc: linux-kernel, monstr, Alan Cox, Jiri Slaby, linux-serial,
Greg Kroah-Hartman, linux-arm-kernel
In-Reply-To: <20170721154720.ifjsn3og6apqi7mv@xsjsorenbubuntu.xilinx.com>
On 21.7.2017 17:47, Sören Brinkmann wrote:
> On Fri, 2017-07-21 at 11:32:24 +0200, Michal Simek wrote:
>> register_console() is called from
>> uart_add_one_port()->uart_configure_port()
>> that's why register_console() is called twice.
>>
>> This patch remove console_initcall to call register_console() only from
>> one location.
>>
>> Also based on my tests cdns_uart_console_setup() is not called
>> from the first register_console() call.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> I am not quite sure about this because console_initcall is called
>> early but I can see any difference in usage.
>> pl011 is not calling this but others are doing it.
>
> Doesn't this break early console/printk? I would expect that the UART
> initialization may happen later than console init.
as I said. I can't see any issue with it. Definitely please try.
Thanks,
Michal
^ permalink raw reply
* Re: [RFC PATCH 0/4] serial: uartps: Dynamic allocation
From: Michal Simek @ 2017-07-31 7:42 UTC (permalink / raw)
To: Alan Cox, Michal Simek
Cc: linux-kernel, monstr, Sören Brinkmann, Jiri Slaby,
linux-serial, Greg Kroah-Hartman, linux-arm-kernel
In-Reply-To: <20170728193952.1f85073f@alans-desktop>
On 28.7.2017 20:39, Alan Cox wrote:
> On Fri, 21 Jul 2017 11:32:23 +0200
> Michal Simek <michal.simek@xilinx.com> wrote:
>
>> Hi Alan,
>>
>> this is the initial version before next step which is move
>> uart_register_driver to probe function.
>> I was able to get rid of static array with uart_port structures.
>> It was wired with console which is also fixed.
>> And the next step is the most complicated one handle .nr in uart_driver
>> structure in more generic way.
>>
>> Thanks,
>> Michal
>
> Sorry for the delay been on jury service
np at all.
Do you have any suggestion how to do the last step?
>
> Series
>
> Reviewed-by: Alan Cox <alan@linux.intel.com>
>
Thanks,
Michal
^ permalink raw reply
* Re: [PATCH v4 3/9] serial: Add common rs485 device tree parsing function
From: Uwe Kleine-König @ 2017-07-31 7:42 UTC (permalink / raw)
To: Greg KH
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA, Sascha Hauer,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20170730144947.GA18707-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
On Sun, Jul 30, 2017 at 07:49:47AM -0700, Greg KH wrote:
> On Tue, Jul 18, 2017 at 12:59:42PM +0200, Uwe Kleine-König wrote:
> > From: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> >
> > Several drivers have the same device tree parsing code. Create
> > a common helper function for it.
> >
> > Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > [ukl: implement default <0 0> for rts-delay, unset unspecified flags]
> > Acked-by: Nicolas Ferre <nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > ---
> > drivers/tty/serial/Kconfig | 4 ++++
> > drivers/tty/serial/Makefile | 2 ++
> > drivers/tty/serial/of.c | 47 +++++++++++++++++++++++++++++++++++++++++++++
>
> Why is this a separate file, with a Kconfig option that is always
> enabled? Why not just add the single function to the serial core?
Probably just a matter of taste. If you prefer I can put it into
drivers/tty/serial/serial_core.c
> > +/**
> > + * of_get_rs485_mode() - Implement parsing rs485 properties
> > + * @np: uart node
> > + * @rs485conf: output parameter
> > + *
> > + * This function implements the device tree binding described in
> > + * Documentation/devicetree/bindings/serial/rs485.txt.
> > + *
> > + * Return: 0 on success, 1 if the node doesn't contain rs485 stuff.
>
> 1 is not an error code :(
>
> Return a proper error please.
Well, it's not an error. These properties are optional and if I return
(say) -ENOENT in this case, a caller looks as follows:
ret = of_get_rs485_mode(...);
if (ret == -ENOENT) {
/*
* hmm, does this mean there are no rs485 properties, or
* is this a different error that I should propagate
* because there is a problem with the device tree?
* I'm optimistic and assume the former.
* As of_get_rs485_mode already did everything
* necessary, there is no need to do anything else.
*/
} else if (ret < 0) {
return ret;
}
With the semantic I chose it is just:
ret = of_get_rs485_mode(...);
if (ret < 0)
return ret;
without any guessing.
> > + */
> > +int of_get_rs485_mode(struct device_node *np, struct serial_rs485 *rs485conf)
> > +{
> > + u32 rs485_delay[2];
> > + int ret;
> > +
> > + if (!IS_ENABLED(CONFIG_OF) || !np)
> > + return 1;
> > +
> > + ret = of_property_read_u32_array(np, "rs485-rts-delay", rs485_delay, 2);
> > + if (!ret) {
> > + rs485conf->delay_rts_before_send = rs485_delay[0];
> > + rs485conf->delay_rts_after_send = rs485_delay[1];
> > + } else {
> > + rs485conf->delay_rts_before_send = 0;
> > + rs485conf->delay_rts_after_send = 0;
> > + }
> > +
> > + /*
> > + * clear full-duplex and enabled flags to get to a defined state with
> > + * the two following properties.
> > + */
> > + rs485conf->flags &= ~(SER_RS485_RX_DURING_TX | SER_RS485_ENABLED);
> > +
> > + if (of_property_read_bool(np, "rs485-rx-during-tx"))
> > + rs485conf->flags |= SER_RS485_RX_DURING_TX;
> > +
> > + if (of_property_read_bool(np, "linux,rs485-enabled-at-boot-time"))
> > + rs485conf->flags |= SER_RS485_ENABLED;
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(of_get_rs485_mode);
> > diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> > index 1775500294bb..3a89e8925722 100644
> > --- a/include/linux/serial_core.h
> > +++ b/include/linux/serial_core.h
> > @@ -501,4 +501,17 @@ static inline int uart_handle_break(struct uart_port *port)
> > (cflag) & CRTSCTS || \
> > !((cflag) & CLOCAL))
> >
> > +/*
> > + * Common device tree parsing helpers
> > + */
> > +#ifdef CONFIG_OF_SERIAL
> > +int of_get_rs485_mode(struct device_node *np, struct serial_rs485 *rs485conf);
> > +#else
> > +static inline int of_get_rs485_mode(struct device_node *np,
> > + struct serial_rs485 *rs485conf)
> > +{
> > + return 1;
>
> Again, a real error.
OK, I could agree to return -ENOSYS here. I predict I get complaints
about this though and people will do:
ret = of_get_rs485_mode(...);
if (ret == -ENOSYS) {
/* ignore */
} else if (ret < 0) {
return ret;
}
which one might consider troublesome or even wrong. (See the same
discussions about 22c403676dbbb7c6f186099527af7f065498ef45 where I
wanted to keep -ENOSYS.)
> And why not have a "generic" firmware function for this that defaults to
> the of_ for that platform type if present? What's the odds that acpi
> will need/want this soon anyway?
I don't know about ACPI and all systems I care about are using dt. So
I'd suggest we stick to dt and switch to "generic" if and when the need
arises?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
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] powerpc/asm/cacheflush: Cleanup cacheflush function params
From: Christophe LEROY @ 2017-07-31 8:41 UTC (permalink / raw)
To: Matt Brown, linuxppc-dev, linux-ide, dri-devel,
adi-buildroot-devel, linux-mmc, linux-mtd, linux-arm-kernel,
linux-spi, linux-serial, linux-usb
In-Reply-To: <20170720062850.2195-1-matthew.brown.dev@gmail.com>
Le 20/07/2017 à 08:28, Matt Brown a écrit :
> The cacheflush prototypes currently use start and stop values and each
> call requires typecasting the address to an unsigned long.
> This patch changes the cacheflush prototypes to follow the x86 style of
> using a base and size values, with base being a void pointer.
>
> All callers of the cacheflush functions, including drivers, have been
> modified to conform to the new prototypes.
>
> The 64 bit cacheflush functions which were implemented in assembly code
> (flush_dcache_range, flush_inval_dcache_range) have been translated into
> C for readability and coherence.
>
> Signed-off-by: Matt Brown <matthew.brown.dev@gmail.com>
> ---
> arch/powerpc/include/asm/cacheflush.h | 47 +++++++++++++++++--------
> arch/powerpc/kernel/misc_64.S | 52 ----------------------------
> arch/powerpc/mm/dma-noncoherent.c | 15 ++++----
> arch/powerpc/platforms/512x/mpc512x_shared.c | 10 +++---
> arch/powerpc/platforms/85xx/smp.c | 6 ++--
> arch/powerpc/sysdev/dart_iommu.c | 5 +--
> drivers/ata/pata_bf54x.c | 3 +-
> drivers/char/agp/uninorth-agp.c | 6 ++--
> drivers/gpu/drm/drm_cache.c | 3 +-
> drivers/macintosh/smu.c | 15 ++++----
> drivers/mmc/host/bfin_sdh.c | 3 +-
> drivers/mtd/nand/bf5xx_nand.c | 6 ++--
> drivers/soc/fsl/qbman/dpaa_sys.h | 2 +-
> drivers/soc/fsl/qbman/qman_ccsr.c | 3 +-
> drivers/spi/spi-bfin5xx.c | 10 +++---
> drivers/tty/serial/mpsc.c | 46 ++++++++----------------
> drivers/usb/musb/blackfin.c | 6 ++--
> 17 files changed, 86 insertions(+), 152 deletions(-)
>
[...]
> diff --git a/arch/powerpc/mm/dma-noncoherent.c b/arch/powerpc/mm/dma-noncoherent.c
> index 3825284..5fd3171 100644
> --- a/arch/powerpc/mm/dma-noncoherent.c
> +++ b/arch/powerpc/mm/dma-noncoherent.c
> @@ -204,9 +204,9 @@ __dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t *handle, gfp_t
> * kernel direct-mapped region for device DMA.
> */
> {
> - unsigned long kaddr = (unsigned long)page_address(page);
> + void *kaddr = page_address(page);
> memset(page_address(page), 0, size);
> - flush_dcache_range(kaddr, kaddr + size);
> + flush_dcache_range(kaddr, size);
> }
>
> /*
> @@ -316,9 +316,6 @@ EXPORT_SYMBOL(__dma_free_coherent);
> */
> void __dma_sync(void *vaddr, size_t size, int direction)
> {
> - unsigned long start = (unsigned long)vaddr;
> - unsigned long end = start + size;
> -
> switch (direction) {
> case DMA_NONE:
> BUG();
> @@ -328,15 +325,15 @@ void __dma_sync(void *vaddr, size_t size, int direction)
> * the potential for discarding uncommitted data from the cache
> */
> if ((start | end) & (L1_CACHE_BYTES - 1))
How can the above compile when 'start' and 'end' are removed ?
Shouldn't it be replaced by
if ((vaddr | size) & (L1_CACHE_BYTES - 1))
> - flush_dcache_range(start, end);
> + flush_dcache_range(vaddr, size);
> else
> - invalidate_dcache_range(start, end);
> + invalidate_dcache_range(vaddr, size);
> break;
> case DMA_TO_DEVICE: /* writeback only */
> - clean_dcache_range(start, end);
> + clean_dcache_range(vaddr, size);
> break;
> case DMA_BIDIRECTIONAL: /* writeback and invalidate */
> - flush_dcache_range(start, end);
> + flush_dcache_range(vaddr, size);
> break;
> }
> }
[...]
Christophe
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC v2 1/6] mux: include compiler.h from mux/consumer.h
From: Peter Rosin @ 2017-07-31 9:02 UTC (permalink / raw)
To: Ulrich Hecht, linux-serial
Cc: linux-renesas-soc, magnus.damm, laurent.pinchart, wsa, robh,
geert, linux-i2c
In-Reply-To: <1500305076-15570-2-git-send-email-ulrich.hecht+renesas@gmail.com>
Hi Ulrich,
This patch looks good to me. Let me know if I should feed this
through the mux subsystem or if it will take some other route.
In case someone else ends up taking it:
Acked-by: Peter Rosin <peda@axentia.se>
Cheers,
Peter
On 2017-07-17 17:24, Ulrich Hecht wrote:
> Required for __must_check.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
> include/linux/mux/consumer.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
> index 5577e1b..ea96d4c 100644
> --- a/include/linux/mux/consumer.h
> +++ b/include/linux/mux/consumer.h
> @@ -13,6 +13,8 @@
> #ifndef _LINUX_MUX_CONSUMER_H
> #define _LINUX_MUX_CONSUMER_H
>
> +#include <linux/compiler.h>
> +
> struct device;
> struct mux_control;
>
>
^ permalink raw reply
* Re: [PATCH] drivers/serial: Do not leave sysfs group in case of error in aspeed_vuart_probe()
From: Joel Stanley @ 2017-07-31 9:23 UTC (permalink / raw)
To: Alexey Khoroshilov
Cc: Jeremy Kerr, Rob Herring, Benjamin Herrenschmidt, Andy Shevchenko,
Greg Kroah-Hartman, linux-serial, Linux Kernel Mailing List,
ldv-project
In-Reply-To: <1501268043-20687-1-git-send-email-khoroshilov@ispras.ru>
Hello Alexey,
On Sat, Jul 29, 2017 at 4:24 AM, Alexey Khoroshilov
<khoroshilov@ispras.ru> wrote:
> There are several error handling paths in aspeed_vuart_probe(),
> where sysfs group is left unremoved. The patch fixes them.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Thanks for the patch.
Acked-by: Joel Stanley <joel@jms.id.au>
Cheers,
Joel
^ permalink raw reply
* Re: [PATCH 4/9] serial: stm32: fix pio transmit timeout
From: Bich HEMON @ 2017-07-31 9:30 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Mark Rutland, Maxime Coquelin, Alexandre TORGUE,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Rob Herring, linux-serial@vger.kernel.org, Jiri Slaby,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20170730143256.GA30584@kroah.com>
Hi Greg,
On 07/30/2017 04:32 PM, Greg Kroah-Hartman wrote:
> On Thu, Jul 13, 2017 at 03:08:28PM +0000, Bich HEMON wrote:
>> From: Gerald Baeza <gerald.baeza@st.com>
>>
>> 100µs was too short for low speed transmission
>> (9600bps)
>>
>> Signed-off-by: Gerald Baeza <gerald.baeza@st.com>
>> Signed-off-by: Bich Hemon <bich.hemon@st.com>
>> ---
>> drivers/tty/serial/stm32-usart.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> This patch did not apply :(
>
I have rebased the patch on top of your tty-next branch . Please drop
this patch as I will send you a new version.
Thx,
Bich
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] serial: stm32: fix pio transmit timeout
From: Bich HEMON @ 2017-07-31 9:31 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rob Herring, Mark Rutland, Maxime Coquelin,
Alexandre TORGUE, Jiri Slaby, linux-serial@vger.kernel.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Cc: Bich HEMON
From: Gerald Baeza <gerald.baeza@st.com>
100µs was too short for low speed transmission
(9600bps)
Signed-off-by: Gerald Baeza <gerald.baeza@st.com>
Signed-off-by: Bich Hemon <bich.hemon@st.com>
---
drivers/tty/serial/stm32-usart.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index 72c0ec1..b16e7e7 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -203,7 +203,7 @@ static void stm32_transmit_chars_pio(struct uart_port *port)
ret = readl_relaxed_poll_timeout_atomic(port->membase + ofs->isr,
isr,
(isr & USART_SR_TXE),
- 10, 100);
+ 10, 100000);
if (ret)
dev_err(port->dev, "tx empty not set\n");
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [RFC v2 1/6] mux: include compiler.h from mux/consumer.h
From: Ulrich Hecht @ 2017-07-31 9:56 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-serial, Linux-Renesas, Magnus Damm, Laurent, Wolfram Sang,
Rob Herring, Geert Uytterhoeven, linux-i2c
In-Reply-To: <1a1233da-2f39-7777-1149-7a0f32a4b1d5@axentia.se>
On Mon, Jul 31, 2017 at 11:02 AM, Peter Rosin <peda@axentia.se> wrote:
> Hi Ulrich,
>
> This patch looks good to me. Let me know if I should feed this
> through the mux subsystem or if it will take some other route.
Please pick it up, thanks.
> In case someone else ends up taking it:
> Acked-by: Peter Rosin <peda@axentia.se>
CU
Uli
^ permalink raw reply
* Re: [RFC v2 2/6] serdev: add method to set parity
From: Laurent Pinchart @ 2017-07-31 10:55 UTC (permalink / raw)
To: Ulrich Hecht
Cc: linux-serial, linux-renesas-soc, magnus.damm, wsa, robh, peda,
geert, linux-i2c
In-Reply-To: <1500305076-15570-3-git-send-email-ulrich.hecht+renesas@gmail.com>
Hi Ulrich,
Thank you for the patch.
On Monday 17 Jul 2017 17:24:32 Ulrich Hecht wrote:
> Adds serdev_device_set_parity() and an implementation for ttyport.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
> drivers/tty/serdev/core.c | 12 ++++++++++++
> drivers/tty/serdev/serdev-ttyport.c | 17 +++++++++++++++++
> include/linux/serdev.h | 4 ++++
> 3 files changed, 33 insertions(+)
>
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index f71b473..1fbaa4c 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -242,6 +242,18 @@ int serdev_device_set_tiocm(struct serdev_device
> *serdev, int set, int clear) }
> EXPORT_SYMBOL_GPL(serdev_device_set_tiocm);
>
> +int serdev_device_set_parity(struct serdev_device *serdev,
> + bool enable, bool odd)
Code calling this function will be difficult to read:
serdev_device_set_partity(dev, true, false);
Without looking up the function definition, the last two arguments are not
very explicit. How about replacing them with a disabled, odd, even enum ?
> +{
> + struct serdev_controller *ctrl = serdev->ctrl;
> +
> + if (!ctrl || !ctrl->ops->set_parity)
> + return -ENOTSUPP;
> +
> + return ctrl->ops->set_parity(ctrl, enable, odd);
> +}
> +EXPORT_SYMBOL_GPL(serdev_device_set_parity);
> +
> static int serdev_drv_probe(struct device *dev)
> {
> const struct serdev_device_driver *sdrv =
> to_serdev_device_driver(dev->driver); diff --git
> a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
> index 302018d..9114956 100644
> --- a/drivers/tty/serdev/serdev-ttyport.c
> +++ b/drivers/tty/serdev/serdev-ttyport.c
> @@ -195,6 +195,22 @@ static int ttyport_set_tiocm(struct serdev_controller
> *ctrl, unsigned int set, u return tty->driver->ops->tiocmset(tty, set,
> clear);
> }
>
> +static int ttyport_set_parity(struct serdev_controller *ctrl,
> + bool enable, bool odd)
> +{
> + struct serport *serport = serdev_controller_get_drvdata(ctrl);
> + struct tty_struct *tty = serport->tty;
> + struct ktermios ktermios = tty->termios;
> +
> + ktermios.c_cflag &= ~(PARENB | PARODD);
> + if (enable)
> + ktermios.c_cflag |= PARENB;
> + if (odd)
> + ktermios.c_cflag |= PARODD;
> +
> + return tty_set_termios(tty, &ktermios);
> +}
> +
> static const struct serdev_controller_ops ctrl_ops = {
> .write_buf = ttyport_write_buf,
> .write_flush = ttyport_write_flush,
> @@ -206,6 +222,7 @@ static const struct serdev_controller_ops ctrl_ops = {
> .wait_until_sent = ttyport_wait_until_sent,
> .get_tiocm = ttyport_get_tiocm,
> .set_tiocm = ttyport_set_tiocm,
> + .set_parity = ttyport_set_parity,
> };
>
> struct device *serdev_tty_port_register(struct tty_port *port,
> diff --git a/include/linux/serdev.h b/include/linux/serdev.h
> index e69402d..8b67fcd 100644
> --- a/include/linux/serdev.h
> +++ b/include/linux/serdev.h
> @@ -90,6 +90,7 @@ struct serdev_controller_ops {
> void (*wait_until_sent)(struct serdev_controller *, long);
> int (*get_tiocm)(struct serdev_controller *);
> int (*set_tiocm)(struct serdev_controller *, unsigned int, unsigned
int);
> + int (*set_parity)(struct serdev_controller *, bool, bool);
> };
>
> /**
> @@ -298,6 +299,9 @@ static inline int serdev_device_set_rts(struct
> serdev_device *serdev, bool enabl return serdev_device_set_tiocm(serdev, 0,
> TIOCM_RTS);
> }
>
> +int serdev_device_set_parity(struct serdev_device *serdev,
> + bool enable, bool odd);
> +
> /*
> * serdev hooks into TTY core
> */
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [RFC v2 5/6] max9260: add driver for i2c over GMSL passthrough
From: Laurent Pinchart @ 2017-07-31 11:13 UTC (permalink / raw)
To: Ulrich Hecht
Cc: linux-serial, linux-renesas-soc, magnus.damm, wsa, robh, peda,
geert, linux-i2c
In-Reply-To: <1500305076-15570-6-git-send-email-ulrich.hecht+renesas@gmail.com>
Hi Ulrich,
Thank you for the patch.
On Monday 17 Jul 2017 17:24:35 Ulrich Hecht wrote:
> This driver implements tunnelling of i2c requests over GMSL via a
> MAX9260 deserializer. It provides an i2c adapter that can be used
> to reach devices on the far side of the link.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
> drivers/media/i2c/Kconfig | 6 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/max9260.c | 288 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 295 insertions(+)
> create mode 100644 drivers/media/i2c/max9260.c
[snip]
> diff --git a/drivers/media/i2c/max9260.c b/drivers/media/i2c/max9260.c
> new file mode 100644
> index 0000000..ca34e67
> --- /dev/null
> +++ b/drivers/media/i2c/max9260.c
> @@ -0,0 +1,288 @@
> +/*
> + * Maxim MAX9260 GMSL Deserializer Driver
> + *
> + * Copyright (C) 2017 Ulrich Hecht
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by
> the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
> +#include <linux/serdev.h>
> +#include <linux/slab.h>
> +#include <linux/tty.h>
> +
> +#define SYNC 0x79
> +#define ACK 0xc3
> +
> +#define RX_FINISHED 0
> +#define RX_FRAME_ERROR 1
> +#define RX_EXPECT_ACK 2
> +#define RX_EXPECT_ACK_DATA 3
> +#define RX_EXPECT_DATA 4
Should you make this an enum and update the type of max9260_device::rx_state
accordingly ?
> +
> +struct max9260_device {
> + struct serdev_device *serdev;
> + u8 *rx_buf;
> + int rx_len;
rx_len can't be negative, you can make it an unsigned int.
> + int rx_state;
> + wait_queue_head_t rx_wq;
> + struct i2c_adapter adap;
> +};
> +
> +static void wait_for_transaction(struct max9260_device *dev)
> +{
> + wait_event_interruptible_timeout(dev->rx_wq,
> + dev->rx_state <= RX_FRAME_ERROR,
> + HZ/2);
> +}
> +
> +static void max9260_transact(struct max9260_device *dev,
> + int expect,
> + u8 *request, int len)
> +{
> + serdev_device_mux_select(dev->serdev);
> +
> + serdev_device_set_baudrate(dev->serdev, 115200);
> + serdev_device_set_parity(dev->serdev, 1, 0);
> +
> + dev->rx_state = expect;
> + serdev_device_write_buf(dev->serdev, request, len);
> +
> + wait_for_transaction(dev);
> +
> + serdev_device_mux_deselect(dev->serdev);
You will end up selecting and deselecting the multiplexer around every I2C
transaction, which can be quite inefficient when initializing devices on the
other side of the link that require hundreds of I2C writes.
Fixing this could be considered as a future optimization, but it will likely
involve changes in the serdev mux API, so I think it should be considered now
already, even if not fully implemented in this driver.
> +}
> +
> +static int max9260_read_reg(struct max9260_device *dev, int reg)
> +{
> + u8 request[] = { 0x79, 0x91, reg, 1 };
> + u8 rx;
> +
> + dev->rx_len = 1;
> + dev->rx_buf = ℞
> +
> + max9260_transact(dev, RX_EXPECT_ACK_DATA, request, 4);
s/4/ARRAY_SIZE(request)/
Passing part of the transaction through function arguments and part through
the max9260_device structure seems weird. I think you should pass both the
output and input buffers and lengths as arguments, and store the information
you need in max9260_device within max9260_transact().
> + if (dev->rx_state == RX_FINISHED)
> + return rx;
> +
> + return -EIO;
> +}
> +
> +static int max9260_setup(struct max9260_device *dev)
> +{
> + int ret;
> +
> + ret = max9260_read_reg(dev, 0x1e);
No magic values, please use macros.
> +
> + if (ret != 0x02) {
Ditto here and in multiple locations below.
> + dev_err(&dev->serdev->dev,
> + "device does not identify as MAX9260\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static void max9260_uart_write_wakeup(struct serdev_device *serdev)
> +{
> +}
> +
> +static int max9260_uart_receive_buf(struct serdev_device *serdev,
> + const u8 *data, size_t count)
> +{
> + struct max9260_device *dev = serdev_device_get_drvdata(serdev);
> + int accepted;
This can't be negative, you can make it an unsigned int.
> +
> + switch (dev->rx_state) {
> + case RX_FINISHED:
> + dev_dbg(&dev->serdev->dev, "excess data ignored\n");
> + return count;
> +
> + case RX_EXPECT_ACK:
> + case RX_EXPECT_ACK_DATA:
> + if (data[0] != ACK) {
> + dev_dbg(&dev->serdev->dev, "frame error");
> + dev->rx_state = RX_FRAME_ERROR;
> + wake_up_interruptible(&dev->rx_wq);
> + return 1;
> + }
> +
> + if (dev->rx_state == RX_EXPECT_ACK_DATA) {
> + dev->rx_state = RX_EXPECT_DATA;
> + } else {
> + dev->rx_state = RX_FINISHED;
> + wake_up_interruptible(&dev->rx_wq);
> + }
> + return 1;
> +
> + case RX_EXPECT_DATA:
> + accepted = dev->rx_len < count ? dev->rx_len : count;
accepted = min(dev->rx_len, count);
> +
> + memcpy(dev->rx_buf, data, accepted);
> +
> + dev->rx_len -= accepted;
> + dev->rx_buf += accepted;
> +
> + if (!dev->rx_len) {
> + dev->rx_state = RX_FINISHED;
> + wake_up_interruptible(&dev->rx_wq);
> + }
> +
> + return accepted;
> +
> + case RX_FRAME_ERROR:
> + dev_dbg(&dev->serdev->dev, "%d bytes ignored\n", count);
> + return count;
> +
> + }
> + return 0;
> +}
> +
> +struct serdev_device_ops max9260_serdev_client_ops = {
static const ?
> + .receive_buf = max9260_uart_receive_buf,
> + .write_wakeup = max9260_uart_write_wakeup,
> +};
> +
> +static u32 max9260_i2c_func(struct i2c_adapter *adapter)
> +{
> + return I2C_FUNC_SMBUS_BYTE|I2C_FUNC_SMBUS_BYTE_DATA;
> +}
> +
> +static s32 max9260_smbus_xfer(struct i2c_adapter *adap, u16 addr,
> + unsigned short flags, char read_write, u8 command, int size,
> + union i2c_smbus_data *data)
> +{
> + u8 request[] = { SYNC,
> + (addr << 1) + (read_write == I2C_SMBUS_READ),
> + command, 0, 0 };
> + struct max9260_device *dev = i2c_get_adapdata(adap);
> +
> + switch (size) {
> + case I2C_SMBUS_BYTE:
> + if (read_write == I2C_SMBUS_WRITE) {
> + max9260_transact(dev, RX_EXPECT_ACK, request, 4);
> + dev_dbg(&adap->dev,
> + "smbus byte - addr 0x%02x, wrote 0x%02x.\n",
> + addr, command);
> + } else {
> + /* TBD */
> + return -EOPNOTSUPP;
> + }
> + break;
> +
> + case I2C_SMBUS_BYTE_DATA:
> + request[3] = 1;
> + if (read_write == I2C_SMBUS_WRITE) {
> + request[4] = data->byte;
> + max9260_transact(dev, RX_EXPECT_ACK, request, 5);
> + dev_dbg(&adap->dev,
> + "smbus byte data - addr 0x%02x, wrote 0x%02x
at 0x%02x.\n",
> + addr, data->byte, command);
> + } else {
> + dev->rx_len = 1;
> + dev->rx_buf = &data->byte;
> + max9260_transact(dev, RX_EXPECT_ACK_DATA, request, 4);
> + dev_dbg(&adap->dev,
> + "smbus byte data - addr 0x%02x, read 0x%02x
at 0x%02x.\n",
> + addr, data->byte, command);
> + }
> + break;
Missing blank line.
> + default:
> + dev_dbg(&adap->dev,
> + "Unsupported I2C/SMBus command %d\n", size);
> + return -EOPNOTSUPP;
> + }
> +
> + if (dev->rx_state != RX_FINISHED) {
> + dev_dbg(&adap->dev, "xfer timed out\n");
> + return -EIO;
The comment states timed out but the return value corresponds to an I/O error.
Which one should it be ?
> + }
> +
> + return 0;
> +}
> +
> +static const struct i2c_algorithm max9260_i2c_algorithm = {
> + .functionality = max9260_i2c_func,
> + .smbus_xfer = max9260_smbus_xfer,
> +};
> +
> +static int max9260_probe(struct serdev_device *serdev)
> +{
> + struct max9260_device *dev;
> + struct i2c_adapter *adap;
> + int ret;
> +
> + dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> + if (!dev)
> + return -ENOMEM;
> +
> + init_waitqueue_head(&dev->rx_wq);
> +
> + dev->serdev = serdev;
> + serdev_device_open(serdev);
> + serdev_device_set_drvdata(serdev, dev);
> +
> + serdev_device_set_client_ops(serdev, &max9260_serdev_client_ops);
> +
> + ret = max9260_setup(dev);
> + if (ret < 0)
> + goto err_free;
> +
> + adap = &dev->adap;
> + i2c_set_adapdata(adap, dev);
> +
> + adap->owner = THIS_MODULE;
> + adap->algo = &max9260_i2c_algorithm;
> + adap->dev.parent = &serdev->dev;
> + adap->retries = 5;
> + strlcpy(adap->name, dev_name(&serdev->dev), sizeof(adap->name));
> +
> + ret = i2c_add_adapter(adap);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +
> +err_free:
No need for a serdev_device_close() ?
> + kfree(dev);
> + return ret;
> +}
> +
> +static void max9260_remove(struct serdev_device *serdev)
> +{
> + struct max9260_device *dev = serdev_device_get_drvdata(serdev);
> +
> + serdev_device_close(dev->serdev);
> +
> + kfree(dev);
> +}
> +
> +static const struct of_device_id max9260_dt_ids[] = {
> + { .compatible = "maxim,max9260" },
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(of, max9260_dt_ids);
> +
> +static struct serdev_device_driver max9260_driver = {
> + .probe = max9260_probe,
> + .remove = max9260_remove,
> + .driver = {
> + .name = "max9260",
> + .of_match_table = of_match_ptr(max9260_dt_ids),
> + },
> +};
> +
> +module_serdev_device_driver(max9260_driver);
> +
> +MODULE_DESCRIPTION("Maxim MAX9260 GMSL Deserializer Driver");
> +MODULE_AUTHOR("Ulrich Hecht");
> +MODULE_LICENSE("GPL");
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [RFC v2 6/6] ARM: dts: blanche: add SCIF1 and MAX9260 deserializer
From: Laurent Pinchart @ 2017-07-31 11:20 UTC (permalink / raw)
To: Ulrich Hecht
Cc: linux-serial, linux-renesas-soc, magnus.damm, wsa, robh, peda,
geert, linux-i2c
In-Reply-To: <1500305076-15570-7-git-send-email-ulrich.hecht+renesas@gmail.com>
Hi Ulrich,
Thank you for the patch.
On Monday 17 Jul 2017 17:24:36 Ulrich Hecht wrote:
> Adds serial port SCIF1 and the MAX9260 deserializers connected to it.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
> arch/arm/boot/dts/r8a7792-blanche.dts | 52 +++++++++++++++++++++++++++++++
You're probably aware of this already, but you need to document the DT
bindings :-)
> 1 file changed, 52 insertions(+)
>
> diff --git a/arch/arm/boot/dts/r8a7792-blanche.dts
> b/arch/arm/boot/dts/r8a7792-blanche.dts index 9b67dca..2ae9a87 100644
> --- a/arch/arm/boot/dts/r8a7792-blanche.dts
> +++ b/arch/arm/boot/dts/r8a7792-blanche.dts
> @@ -21,6 +21,7 @@
> aliases {
> serial0 = &scif0;
> serial1 = &scif3;
> + serial2 = &scif1;
> };
>
> chosen {
> @@ -186,6 +187,16 @@
> gpio = <&gpio11 12 GPIO_ACTIVE_HIGH>;
> enable-active-high;
> };
> +
> + mux: mux-controller {
> + compatible = "gpio-mux";
> + #mux-control-cells = <0>;
> +
> + mux-gpios = <&gpio5 12 GPIO_ACTIVE_HIGH>,
> + <&gpio5 13 GPIO_ACTIVE_HIGH>,
> + <&gpio5 14 GPIO_ACTIVE_HIGH>,
> + <&gpio5 15 GPIO_ACTIVE_HIGH>;
> + };
> };
>
> &extal_clk {
> @@ -202,6 +213,11 @@
> function = "scif0";
> };
>
> + scif1_pins: scif1 {
> + groups = "scif1_data";
> + function = "scif1";
> + };
> +
> scif3_pins: scif3 {
> groups = "scif3_data";
> function = "scif3";
> @@ -246,6 +262,42 @@
> status = "okay";
> };
>
> +&scif1 {
> + pinctrl-0 = <&scif1_pins>;
> + pinctrl-names = "default";
> +
> + status = "okay";
> +
> + mux-controls = <&mux>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> + gmsl-deserializer@0 {
> + compatible = "maxim,max9260";
> + reg = <0x8>;
> + };
> + gmsl-deserializer@1 {
> + compatible = "maxim,max9260";
> + reg = <0x9>;
> + };
> + gmsl-deserializer@2 {
> + compatible = "maxim,max9260";
> + reg = <0xa>;
> + };
> + gmsl-deserializer@3 {
> + compatible = "maxim,max9260";
> + reg = <0xb>;
> + };
> + gmsl-deserializer@4 {
> + compatible = "maxim,max9260";
> + reg = <0x4>;
> + };
> + gmsl-deserializer@5 {
> + compatible = "maxim,max9260";
> + reg = <0x5>;
> + };
> +};
> +
> &scif3 {
> pinctrl-0 = <&scif3_pins>;
> pinctrl-names = "default";
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [RFC v2 1/6] mux: include compiler.h from mux/consumer.h
From: Laurent Pinchart @ 2017-07-31 11:22 UTC (permalink / raw)
To: Ulrich Hecht
Cc: linux-serial, linux-renesas-soc, magnus.damm, wsa, robh, peda,
geert, linux-i2c
In-Reply-To: <1500305076-15570-2-git-send-email-ulrich.hecht+renesas@gmail.com>
Hi Ulrich,
Thank you for the patch.
On Monday 17 Jul 2017 17:24:31 Ulrich Hecht wrote:
> Required for __must_check.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> include/linux/mux/consumer.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
> index 5577e1b..ea96d4c 100644
> --- a/include/linux/mux/consumer.h
> +++ b/include/linux/mux/consumer.h
> @@ -13,6 +13,8 @@
> #ifndef _LINUX_MUX_CONSUMER_H
> #define _LINUX_MUX_CONSUMER_H
>
> +#include <linux/compiler.h>
> +
> struct device;
> struct mux_control;
--
Regards,
Laurent Pinchart
^ 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