linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] tty: serial: Add mediatek UART driver
@ 2014-08-05 10:54 Matthias Brugger
  2014-08-05 10:54 ` [PATCH 1/3] tty: serial: 8250: Add new capability for highspeed register Matthias Brugger
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Matthias Brugger @ 2014-08-05 10:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	rdunlap, gregkh, jslaby, grant.likely, matthias.bgg,
	heikki.krogerus, alan, paul.gortmaker, asierra, mwelling,
	dianders, m-karicheri2, jschultz, mingo, balbi, heiko,
	devicetree, linux-doc, linux-serial, linux-api

This patch set adds support for the UART found in Mediatek SoCs.
The chip is a changed version of a 8250 controller.
Especially it introduces a new register called highspeed. The value
in this register has to be set depending on the baudrate. The value
in the register influences the way the divisor has to be calculated.

The patch series is build against v3.16-rc1 and tested on mt6589. Should
work as well on mt6577 and mt6582.

Thanks,
Matthias

Matthias Brugger (3):
  tty: serial: 8250: Add new capability for highspeed register
  tty: serial: 8250: Add Mediatek UART driver
  DTS: serial: Add bindings documention for the Mediatek UARTs

 .../devicetree/bindings/serial/mtk-uart.txt        |   19 ++
 drivers/tty/serial/8250/8250.h                     |    1 +
 drivers/tty/serial/8250/8250_core.c                |   47 +++++
 drivers/tty/serial/8250/8250_mtk.c                 |  211 ++++++++++++++++++++
 drivers/tty/serial/8250/Kconfig                    |    7 +
 drivers/tty/serial/8250/Makefile                   |    1 +
 include/uapi/linux/serial_reg.h                    |    6 +
 7 files changed, 292 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/mtk-uart.txt
 create mode 100644 drivers/tty/serial/8250/8250_mtk.c

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] tty: serial: 8250: Add new capability for highspeed register
  2014-08-05 10:54 [PATCH 0/3] tty: serial: Add mediatek UART driver Matthias Brugger
@ 2014-08-05 10:54 ` Matthias Brugger
  2014-08-05 11:43   ` One Thousand Gnomes
  2014-08-05 10:54 ` [PATCH 2/3] tty: serial: 8250: Add Mediatek UART driver Matthias Brugger
  2014-08-05 10:54 ` [PATCH 3/3] DTS: serial: Add bindings documention for the Mediatek UARTs Matthias Brugger
  2 siblings, 1 reply; 9+ messages in thread
From: Matthias Brugger @ 2014-08-05 10:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	rdunlap, gregkh, jslaby, grant.likely, matthias.bgg,
	heikki.krogerus, alan, paul.gortmaker, asierra, mwelling,
	dianders, m-karicheri2, jschultz, mingo, balbi, heiko,
	devicetree, linux-doc, linux-serial, linux-api

This patch adds a new capability to the 8250 driver framework. The Mediatek UART
port has a highspeed register to set the baudrate the port will work with.
This influences the calculation of the divisor.

Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
---
 drivers/tty/serial/8250/8250.h      |    1 +
 drivers/tty/serial/8250/8250_core.c |   47 +++++++++++++++++++++++++++++++++++
 include/uapi/linux/serial_reg.h     |    6 +++++
 3 files changed, 54 insertions(+)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 1ebf853..2b17655 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -70,6 +70,7 @@ struct serial8250_config {
 #define UART_CAP_UUE	(1 << 12)	/* UART needs IER bit 6 set (Xscale) */
 #define UART_CAP_RTOIE	(1 << 13)	/* UART needs IER bit 4 set (Xscale, Tegra) */
 #define UART_CAP_HFIFO	(1 << 14)	/* UART has a "hidden" FIFO */
+#define UART_CAP_HIGHS	(1 << 15)	/* UART has a highspeed register (Mediatek) */
 
 #define UART_BUG_QUOT	(1 << 0)	/* UART has buggy quot LSB */
 #define UART_BUG_TXEN	(1 << 1)	/* UART has buggy TX IIR status */
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 27f7ad6..a0c531b 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -2434,6 +2434,53 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
 	serial_dl_write(up, quot);
 
 	/*
+	 * Mediatek UARTs use an extra highspeed register (UART_MTK_HIGHS)
+	 *
+	 * We need to recalcualte the quot register, as the claculation depends
+	 * on the vaule in the highspeed register.
+	 *
+	 * Some baudrates are not supported by the chip, so we use the next
+	 * lower rate supported.
+	 *
+	 * If highspeed register is set to 3, we need to specify sample count
+	 * and sample point to increase accuracy. If not, we reset the
+	 * registers to their default values.
+	 */
+	if (up->port.flags & UART_CAP_HIGHS) {
+		if (baud <= 115200) {
+			serial_port_out(port, UART_MTK_HIGHS, 0x0);
+		} else if (baud <= 576000) {
+			serial_port_out(port, UART_MTK_HIGHS, 0x2);
+
+			/* Set to next lower baudrate supported */
+			if ((baud == 500000) || (baud == 576000))
+				baud = 460800;
+			quot = DIV_ROUND_CLOSEST(port->uartclk, 4 * baud);
+		} else {
+			serial_port_out(port, UART_MTK_HIGHS, 0x3);
+
+			/* Set to highest baudrate supported */
+			if (baud >= 1152000)
+				baud = 1000000;
+			quot = DIV_ROUND_CLOSEST(port->uartclk, 256 * baud);
+		}
+
+		serial_dl_write(up, quot);
+
+		if (baud > 460800) {
+			unsigned int tmp;
+
+			tmp = DIV_ROUND_CLOSEST(port->uartclk, quot * baud);
+			serial_port_out(port, UART_MTK_SAMPLE_COUNT, tmp - 1);
+			serial_port_out(port, UART_MTK_SAMPLE_POINT,
+						(tmp - 2) >> 1);
+		} else {
+			serial_port_out(port, UART_MTK_SAMPLE_COUNT, 0x00);
+			serial_port_out(port, UART_MTK_SAMPLE_POINT, 0xff);
+		}
+	}
+
+	/*
 	 * XR17V35x UARTs have an extra fractional divisor register (DLD)
 	 *
 	 * We need to recalculate all of the registers, because DLM and DLL
diff --git a/include/uapi/linux/serial_reg.h b/include/uapi/linux/serial_reg.h
index 99b4705..f6dc97c 100644
--- a/include/uapi/linux/serial_reg.h
+++ b/include/uapi/linux/serial_reg.h
@@ -385,5 +385,11 @@
 #define UART_EXAR_TXTRG		0x0a	/* Tx FIFO trigger level write-only */
 #define UART_EXAR_RXTRG		0x0b	/* Rx FIFO trigger level write-only */
 
+/*
+ * These are definitions for the Mediatek UART
+ */
+#define UART_MTK_HIGHS		0x09	/* Highspeed register */
+#define UART_MTK_SAMPLE_COUNT	0x0a	/* Sample count register */
+#define UART_MTK_SAMPLE_POINT	0x0b	/* Sample point register */
 #endif /* _LINUX_SERIAL_REG_H */
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] tty: serial: 8250: Add Mediatek UART driver
  2014-08-05 10:54 [PATCH 0/3] tty: serial: Add mediatek UART driver Matthias Brugger
  2014-08-05 10:54 ` [PATCH 1/3] tty: serial: 8250: Add new capability for highspeed register Matthias Brugger
@ 2014-08-05 10:54 ` Matthias Brugger
  2014-08-05 11:55   ` Varka Bhadram
  2014-08-05 10:54 ` [PATCH 3/3] DTS: serial: Add bindings documention for the Mediatek UARTs Matthias Brugger
  2 siblings, 1 reply; 9+ messages in thread
From: Matthias Brugger @ 2014-08-05 10:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	rdunlap, gregkh, jslaby, grant.likely, matthias.bgg,
	heikki.krogerus, alan, paul.gortmaker, asierra, mwelling,
	dianders, m-karicheri2, jschultz, mingo, balbi, heiko,
	devicetree, linux-doc, linux-serial, linux-api

This patch adds support for the UART block found on Mediatek SoCs.
The driver uses the highspeed capability of the 8250_core to set the
highspeed register and calculate the divisor for it.

Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
---
 drivers/tty/serial/8250/8250_mtk.c |  211 ++++++++++++++++++++++++++++++++++++
 drivers/tty/serial/8250/Kconfig    |    7 ++
 drivers/tty/serial/8250/Makefile   |    1 +
 3 files changed, 219 insertions(+)
 create mode 100644 drivers/tty/serial/8250/8250_mtk.c

diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
new file mode 100644
index 0000000..96c72b4
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -0,0 +1,211 @@
+/*
+ * Mediatek 8250 driver.
+ *
+ * Copyright (c) 2014 MundoReader S.L.
+ * Author: Matthias Brugger <matthias.bgg@gmail.com>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/serial_8250.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/pm_runtime.h>
+#include "8250.h"
+
+#define MTK_UART_RATE_FIX 0x0D /* UART Rate Fix Register */
+
+struct mtk8250_data {
+	int			line;
+	struct clk		*clk;
+};
+
+static void
+mtk8250_do_pm(struct uart_port *port, unsigned int state, unsigned int old)
+{
+	if (!state)
+		pm_runtime_get_sync(port->dev);
+
+	serial8250_do_pm(port, state, old);
+
+	if (state)
+		pm_runtime_put_sync_suspend(port->dev);
+}
+
+static int mtk8250_probe_of(struct uart_port *p,
+			   struct mtk8250_data *data)
+{
+	int err;
+	struct device_node	*np = p->dev->of_node;
+
+	data->clk = of_clk_get(np, 0);
+	if (IS_ERR(data->clk)) {
+		pr_warn("Can't get timer clock");
+		return PTR_ERR(data->clk);
+	}
+
+	err = clk_prepare_enable(data->clk);
+	if (err) {
+		pr_warn("Can't prepare clock");
+		clk_put(data->clk);
+		return err;
+	}
+	p->uartclk = clk_get_rate(data->clk);
+
+	return 0;
+}
+
+static int mtk8250_probe(struct platform_device *pdev)
+{
+	struct uart_8250_port uart = {};
+	struct resource *regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	struct resource *irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	struct mtk8250_data *data;
+	int err;
+
+	if (!regs || !irq) {
+		dev_err(&pdev->dev, "no registers/irq defined\n");
+		return -EINVAL;
+	}
+
+	spin_lock_init(&uart.port.lock);
+	uart.port.mapbase = regs->start;
+	uart.port.irq = irq->start;
+	uart.port.pm = mtk8250_do_pm;
+	uart.port.type = PORT_16550;
+	uart.port.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UART_CAP_HIGHS;
+	uart.port.dev = &pdev->dev;
+
+	uart.port.membase = devm_ioremap(&pdev->dev, regs->start,
+					 resource_size(regs));
+	if (!uart.port.membase)
+		return -ENOMEM;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	uart.port.iotype = UPIO_MEM32;
+	uart.port.regshift = 2;
+	uart.port.private_data = data;
+
+	if (pdev->dev.of_node) {
+		err = mtk8250_probe_of(&uart.port, data);
+		if (err)
+			return err;
+	} else
+		return -ENODEV;
+
+	/* Disable Rate Fix function */
+	writel(0x0, uart.port.membase +
+			(MTK_UART_RATE_FIX << uart.port.regshift));
+
+	data->line = serial8250_register_8250_port(&uart);
+	if (data->line < 0)
+		return data->line;
+
+	platform_set_drvdata(pdev, data);
+
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+
+	return 0;
+}
+
+static int mtk8250_remove(struct platform_device *pdev)
+{
+	struct mtk8250_data *data = platform_get_drvdata(pdev);
+
+	pm_runtime_get_sync(&pdev->dev);
+
+	serial8250_unregister_port(data->line);
+	if (!IS_ERR(data->clk)) {
+		clk_disable_unprepare(data->clk);
+		clk_put(data->clk);
+	}
+
+	pm_runtime_disable(&pdev->dev);
+	pm_runtime_put_noidle(&pdev->dev);
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int mtk8250_suspend(struct device *dev)
+{
+	struct mtk8250_data *data = dev_get_drvdata(dev);
+
+	serial8250_suspend_port(data->line);
+
+	return 0;
+}
+
+static int mtk8250_resume(struct device *dev)
+{
+	struct mtk8250_data *data = dev_get_drvdata(dev);
+
+	serial8250_resume_port(data->line);
+
+	return 0;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+#ifdef CONFIG_PM_RUNTIME
+static int mtk8250_runtime_suspend(struct device *dev)
+{
+	struct mtk8250_data *data = dev_get_drvdata(dev);
+
+	if (!IS_ERR(data->clk))
+		clk_disable_unprepare(data->clk);
+
+	return 0;
+}
+
+static int mtk8250_runtime_resume(struct device *dev)
+{
+	struct mtk8250_data *data = dev_get_drvdata(dev);
+
+	if (!IS_ERR(data->clk))
+		clk_prepare_enable(data->clk);
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops mtk8250_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(mtk8250_suspend, mtk8250_resume)
+	SET_RUNTIME_PM_OPS(mtk8250_runtime_suspend, mtk8250_runtime_resume,
+				NULL)
+};
+
+static const struct of_device_id mtk8250_of_match[] = {
+	{ .compatible = "mediatek,mt6577-uart" },
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mtk8250_of_match);
+
+static struct platform_driver mtk8250_platform_driver = {
+	.driver = {
+		.name		= "mt6577-uart",
+		.owner		= THIS_MODULE,
+		.pm		= &mtk8250_pm_ops,
+		.of_match_table	= mtk8250_of_match,
+	},
+	.probe			= mtk8250_probe,
+	.remove			= mtk8250_remove,
+};
+module_platform_driver(mtk8250_platform_driver);
+
+MODULE_AUTHOR("Matthias Brugger");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Mediatek 8250 serial port driver");
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index 349ee59..fac34aa 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -298,3 +298,10 @@ config SERIAL_8250_RT288X
 	  If you have a Ralink RT288x/RT305x SoC based board and want to use the
 	  serial port, say Y to this option. The driver can handle up to 2 serial
 	  ports. If unsure, say N.
+
+config SERIAL_8250_MT6577
+	bool "Mediatek serial port support"
+	depends on SERIAL_8250 && ARCH_MEDIATEK
+	help
+	  If you have a Mediatek based board and want to use the
+	  serial port, say Y to this option. If unsure, say N.
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
index 36d68d0..6dcb46b 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_SERIAL_8250_HUB6)		+= 8250_hub6.o
 obj-$(CONFIG_SERIAL_8250_FSL)		+= 8250_fsl.o
 obj-$(CONFIG_SERIAL_8250_DW)		+= 8250_dw.o
 obj-$(CONFIG_SERIAL_8250_EM)		+= 8250_em.o
+obj-$(CONFIG_SERIAL_8250_MT6577)	+= 8250_mtk.o
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] DTS: serial: Add bindings documention for the Mediatek UARTs
  2014-08-05 10:54 [PATCH 0/3] tty: serial: Add mediatek UART driver Matthias Brugger
  2014-08-05 10:54 ` [PATCH 1/3] tty: serial: 8250: Add new capability for highspeed register Matthias Brugger
  2014-08-05 10:54 ` [PATCH 2/3] tty: serial: 8250: Add Mediatek UART driver Matthias Brugger
@ 2014-08-05 10:54 ` Matthias Brugger
  2 siblings, 0 replies; 9+ messages in thread
From: Matthias Brugger @ 2014-08-05 10:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	rdunlap, gregkh, jslaby, grant.likely, matthias.bgg,
	heikki.krogerus, alan, paul.gortmaker, asierra, mwelling,
	dianders, m-karicheri2, jschultz, mingo, balbi, heiko,
	devicetree, linux-doc, linux-serial, linux-api

This patch adds the devicetree documentation for the Mediatek UART.

Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
---
 .../devicetree/bindings/serial/mtk-uart.txt        |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/mtk-uart.txt

diff --git a/Documentation/devicetree/bindings/serial/mtk-uart.txt b/Documentation/devicetree/bindings/serial/mtk-uart.txt
new file mode 100644
index 0000000..2bf571e
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/mtk-uart.txt
@@ -0,0 +1,19 @@
+* Mediatek Universal Asynchronous Receiver/Transmitter (UART)
+
+- compatible: "mediatek,mt6577-uart"
+  Compatibility with mt6577, mt6589, mt6582
+
+- reg: The base address of the UART register bank.
+
+- interrupts: A single interrupt specifier.
+
+- clocks: Clock driving the hardware.
+
+Example:
+
+	uart0: serial@11006000 {
+		compatible = "mediatek,mt6577-uart";
+		reg = <0x11006000 0x400>;
+		interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_LOW>;
+		clocks = <&uart_clk>;
+	};
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] tty: serial: 8250: Add new capability for highspeed register
  2014-08-05 10:54 ` [PATCH 1/3] tty: serial: 8250: Add new capability for highspeed register Matthias Brugger
@ 2014-08-05 11:43   ` One Thousand Gnomes
  0 siblings, 0 replies; 9+ messages in thread
From: One Thousand Gnomes @ 2014-08-05 11:43 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: linux-kernel, robh+dt, pawel.moll, mark.rutland, ijc+devicetree,
	galak, rdunlap, gregkh, jslaby, grant.likely, heikki.krogerus,
	alan, paul.gortmaker, asierra, mwelling, dianders, m-karicheri2,
	jschultz, mingo, balbi, heiko, devicetree, linux-doc,
	linux-serial, linux-api

>  
>  	/*
> +	 * Mediatek UARTs use an extra highspeed register (UART_MTK_HIGHS)
> +	 *
> +	 * We need to recalcualte the quot register, as the claculation depends
> +	 * on the vaule in the highspeed register.
> +	 *
> +	 * Some baudrates are not supported by the chip, so we use the next
> +	 * lower rate supported.
> +	 *
> +	 * If highspeed register is set to 3, we need to specify sample count
> +	 * and sample point to increase accuracy. If not, we reset the
> +	 * registers to their default values.
> +	 */

Don't put stuff in the core driver core for your chip specific weirdness,
wrap the termios method with your own in your driver code as a fair
number of other 8250ish drivers do.

If you do that then you don't need to waste a UART CAPS flag and you
don't need to put anything in the core driver code.

Alan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] tty: serial: 8250: Add Mediatek UART driver
  2014-08-05 10:54 ` [PATCH 2/3] tty: serial: 8250: Add Mediatek UART driver Matthias Brugger
@ 2014-08-05 11:55   ` Varka Bhadram
  2014-08-05 12:02     ` Alan Cox
  0 siblings, 1 reply; 9+ messages in thread
From: Varka Bhadram @ 2014-08-05 11:55 UTC (permalink / raw)
  To: Matthias Brugger, linux-kernel
  Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	rdunlap, gregkh, jslaby, grant.likely, heikki.krogerus, alan,
	paul.gortmaker, asierra, mwelling, dianders, m-karicheri2,
	jschultz, mingo, balbi, heiko, devicetree, linux-doc,
	linux-serial, linux-api

On 08/05/2014 04:24 PM, Matthias Brugger wrote:

(...)

> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/serial_8250.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/pm_runtime.h>
> +#include "8250.h"
> +

Better if we have includes in alphabetical order..

> +#define MTK_UART_RATE_FIX 0x0D /* UART Rate Fix Register */
> +
> +struct mtk8250_data {
> +	int			line;
> +	struct clk		*clk;
> +};
> +
> +static void
> +mtk8250_do_pm(struct uart_port *port, unsigned int state, unsigned int old)
> +{
> +	if (!state)
> +		pm_runtime_get_sync(port->dev);
> +
> +	serial8250_do_pm(port, state, old);
> +
> +	if (state)
> +		pm_runtime_put_sync_suspend(port->dev);
> +}
> +
> +static int mtk8250_probe_of(struct uart_port *p,
> +			   struct mtk8250_data *data)

static int mtk8250_probe_of(struct uart_port *p,
			    struct mtk8250_data *data)

> +{
> +	int err;
> +	struct device_node	*np = p->dev->of_node;
> +
> +	data->clk = of_clk_get(np, 0);
> +	if (IS_ERR(data->clk)) {
> +		pr_warn("Can't get timer clock");

missed terminating new line...

> +		return PTR_ERR(data->clk);
> +	}
> +
> +	err = clk_prepare_enable(data->clk);
> +	if (err) {
> +		pr_warn("Can't prepare clock");

same...

> +		clk_put(data->clk);
> +		return err;
> +	}
> +	p->uartclk = clk_get_rate(data->clk);
> +
> +	return 0;
> +}

(...)

> +static struct platform_driver mtk8250_platform_driver = {
> +	.driver = {
> +		.name		= "mt6577-uart",
> +		.owner		= THIS_MODULE,

No need to update this field...

> +		.pm		= &mtk8250_pm_ops,
> +		.of_match_table	= mtk8250_of_match,
> +	},
> +	.probe			= mtk8250_probe,
> +	.remove			= mtk8250_remove,
> +};
> +module_platform_driver(mtk8250_platform_driver);
>
-- 
Regards,
Varka Bhadram.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] tty: serial: 8250: Add Mediatek UART driver
  2014-08-05 11:55   ` Varka Bhadram
@ 2014-08-05 12:02     ` Alan Cox
  2014-08-05 12:04       ` Varka Bhadram
  0 siblings, 1 reply; 9+ messages in thread
From: Alan Cox @ 2014-08-05 12:02 UTC (permalink / raw)
  To: Varka Bhadram
  Cc: Matthias Brugger, linux-kernel, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, rdunlap, gregkh, jslaby,
	grant.likely, heikki.krogerus, paul.gortmaker, asierra, mwelling,
	dianders, m-karicheri2, jschultz, mingo, balbi, heiko,
	devicetree, linux-doc, linux-serial, linux-api

On Tue, 2014-08-05 at 17:25 +0530, Varka Bhadram wrote:
> On 08/05/2014 04:24 PM, Matthias Brugger wrote:
> 
> (...)
> 
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/serial_8250.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/clk.h>
> > +#include <linux/pm_runtime.h>
> > +#include "8250.h"
> > +
> 
> Better if we have includes in alphabetical order..


So 8250.h would be first and it wouldn't compile ???

Can we stick to serious critiques ?

Alan



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] tty: serial: 8250: Add Mediatek UART driver
  2014-08-05 12:02     ` Alan Cox
@ 2014-08-05 12:04       ` Varka Bhadram
  2014-08-05 13:38         ` Alan Cox
  0 siblings, 1 reply; 9+ messages in thread
From: Varka Bhadram @ 2014-08-05 12:04 UTC (permalink / raw)
  To: Alan Cox
  Cc: Matthias Brugger, linux-kernel, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, rdunlap, gregkh, jslaby,
	grant.likely, heikki.krogerus, paul.gortmaker, asierra, mwelling,
	dianders, m-karicheri2, jschultz, mingo, balbi, heiko,
	devicetree, linux-doc, linux-serial, linux-api

On 08/05/2014 05:32 PM, Alan Cox wrote:
> On Tue, 2014-08-05 at 17:25 +0530, Varka Bhadram wrote:
>> On 08/05/2014 04:24 PM, Matthias Brugger wrote:
>>
>> (...)
>>
>>> +#include <linux/io.h>
>>> +#include <linux/module.h>
>>> +#include <linux/serial_8250.h>
>>> +#include <linux/of_irq.h>
>>> +#include <linux/of_platform.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/pm_runtime.h>
>>> +#include "8250.h"
>>> +
>> Better if we have includes in alphabetical order..
>
> So 8250.h would be first and it wouldn't compile ???
>
> Can we stick to serious critiques ?

The local headers should be at the end of all includes..?


-- 
Regards,
Varka Bhadram.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] tty: serial: 8250: Add Mediatek UART driver
  2014-08-05 12:04       ` Varka Bhadram
@ 2014-08-05 13:38         ` Alan Cox
  0 siblings, 0 replies; 9+ messages in thread
From: Alan Cox @ 2014-08-05 13:38 UTC (permalink / raw)
  To: Varka Bhadram
  Cc: Matthias Brugger, linux-kernel, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, rdunlap, gregkh, jslaby,
	grant.likely, heikki.krogerus, paul.gortmaker, asierra, mwelling,
	dianders, m-karicheri2, jschultz, mingo, balbi, heiko,
	devicetree, linux-doc, linux-serial, linux-api

On Tue, 2014-08-05 at 17:34 +0530, Varka Bhadram wrote:
> On 08/05/2014 05:32 PM, Alan Cox wrote:
> > On Tue, 2014-08-05 at 17:25 +0530, Varka Bhadram wrote:
> >> On 08/05/2014 04:24 PM, Matthias Brugger wrote:
> >>
> >> (...)
> >>
> >>> +#include <linux/io.h>
> >>> +#include <linux/module.h>
> >>> +#include <linux/serial_8250.h>
> >>> +#include <linux/of_irq.h>
> >>> +#include <linux/of_platform.h>
> >>> +#include <linux/platform_device.h>
> >>> +#include <linux/clk.h>
> >>> +#include <linux/pm_runtime.h>
> >>> +#include "8250.h"
> >>> +
> >> Better if we have includes in alphabetical order..
> >
> > So 8250.h would be first and it wouldn't compile ???
> >
> > Can we stick to serious critiques ?
> 
> The local headers should be at the end of all includes

Which is not alaphetical order

The include ordering does not matter, please stick to actual things that
matter.


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-08-05 13:38 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-05 10:54 [PATCH 0/3] tty: serial: Add mediatek UART driver Matthias Brugger
2014-08-05 10:54 ` [PATCH 1/3] tty: serial: 8250: Add new capability for highspeed register Matthias Brugger
2014-08-05 11:43   ` One Thousand Gnomes
2014-08-05 10:54 ` [PATCH 2/3] tty: serial: 8250: Add Mediatek UART driver Matthias Brugger
2014-08-05 11:55   ` Varka Bhadram
2014-08-05 12:02     ` Alan Cox
2014-08-05 12:04       ` Varka Bhadram
2014-08-05 13:38         ` Alan Cox
2014-08-05 10:54 ` [PATCH 3/3] DTS: serial: Add bindings documention for the Mediatek UARTs Matthias Brugger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).