2015-05-15 12:27:47

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH] serial: 8250_uniphier: add UniPhier serial driver

Add the driver for on-chip UART used on UniPhier SoCs.
The register map of this hardware is similar to that of 8250
(but the address for FCR, LCR is different), so it should go
into drivers/tty/serial/8250 directory.

Signed-off-by: Masahiro Yamada <[email protected]>
---

drivers/tty/serial/8250/8250_uniphier.c | 243 ++++++++++++++++++++++++++++++++
drivers/tty/serial/8250/Kconfig | 7 +
drivers/tty/serial/8250/Makefile | 1 +
3 files changed, 251 insertions(+)
create mode 100644 drivers/tty/serial/8250/8250_uniphier.c

diff --git a/drivers/tty/serial/8250/8250_uniphier.c b/drivers/tty/serial/8250/8250_uniphier.c
new file mode 100644
index 0000000..f108d81
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_uniphier.c
@@ -0,0 +1,243 @@
+/*
+ * Copyright (C) 2015 Masahiro Yamada <[email protected]>
+ *
+ * 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/init.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/io.h>
+#include <linux/serial_core.h>
+#include <linux/serial_reg.h>
+#include <linux/serial_8250.h>
+#include "8250.h"
+
+/*
+ * Most (but not all) of UniPhier UART devices have 64-depth FIFO.
+ * Set this value if "fifo-size" property is missing from the device tree node.
+ */
+#define UNIPHIER_UART_DEFAULT_FIFO_SIZE 64
+
+#define UNIPHIER_UART_CHAR_FCR 3 /* Character / FIFO Control Register */
+#define UNIPHIER_UART_LCR_MCR 4 /* Line/Modem Control Register */
+#define UNIPHIER_UART_DLR 9 /* Divisor Latch Register */
+
+/*
+ * The register map is slightly different from that of 8250.
+ * IO callbacks must be overridden for correct access to FCR, LCR, and MCR.
+ */
+static unsigned int uniphier_serial_in(struct uart_port *p, int offset)
+{
+ int valshift = 0;
+
+ switch (offset) {
+ case UART_LCR:
+ valshift = 8;
+ case UART_MCR:
+ offset = UNIPHIER_UART_LCR_MCR;
+ break;
+ default:
+ break;
+ }
+
+ offset <<= p->regshift;
+
+ /*
+ * The return value must be masked with 0xff because LCR and MCR reside
+ * in the same register that must be accessed by 32-bit write/read.
+ * 8 or 16 bit access to this hardware result in unexpected behavior.
+ */
+ return (readl(p->membase + offset) >> valshift) & 0xff;
+}
+
+static void uniphier_serial_out(struct uart_port *p, int offset, int value)
+{
+ int valshift = 0;
+ bool normal = false;
+
+ switch (offset) {
+ case UART_FCR:
+ offset = UNIPHIER_UART_CHAR_FCR;
+ break;
+ case UART_LCR:
+ valshift = 8;
+ /* Divisor latch access bit does not exist. */
+ value &= ~(UART_LCR_DLAB << valshift);
+ case UART_MCR:
+ offset = UNIPHIER_UART_LCR_MCR;
+ break;
+ default:
+ normal = true;
+ break;
+ }
+
+ offset <<= p->regshift;
+
+ if (normal) {
+ writel(value, p->membase + offset);
+ } else {
+ /* special case: two registers share the same address. */
+ u32 tmp = readl(p->membase + offset);
+
+ tmp &= ~(0xff << valshift);
+ tmp |= value << valshift;
+ writel(tmp, p->membase + offset);
+ }
+}
+
+/*
+ * This hardware does not have the divisor latch access bit.
+ * The divisor latch register exists in different address.
+ * Override dl_read/write callbacks.
+ */
+static int uniphier_serial_dl_read(struct uart_8250_port *up)
+{
+ return readl(up->port.membase + UNIPHIER_UART_DLR);
+}
+
+static void uniphier_serial_dl_write(struct uart_8250_port *up, int value)
+{
+ writel(value, up->port.membase + UNIPHIER_UART_DLR);
+}
+
+struct uniphier8250_priv {
+ int line;
+ struct clk *clk;
+};
+
+static int uniphier_of_serial_setup(struct platform_device *pdev,
+ struct uart_port *port,
+ struct uniphier8250_priv *priv)
+{
+ int ret;
+ u32 prop;
+ struct device_node *np = pdev->dev.of_node;
+
+ ret = of_alias_get_id(np, "serial");
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to get alias id\n");
+ return ret;
+ }
+ port->line = priv->line = ret;
+
+ /* Get clk rate through clk driver */
+ priv->clk = of_clk_get(np, 0);
+ if (IS_ERR(priv->clk)) {
+ dev_err(&pdev->dev, "failed to get clock\n");
+ return PTR_ERR(priv->clk);
+ }
+
+ ret = clk_prepare_enable(priv->clk);
+ if (ret < 0)
+ return ret;
+
+ port->uartclk = clk_get_rate(priv->clk);
+
+ /* Check for fifo size */
+ if (of_property_read_u32(np, "fifo-size", &prop) == 0)
+ port->fifosize = prop;
+ else
+ port->fifosize = UNIPHIER_UART_DEFAULT_FIFO_SIZE;
+
+ return 0;
+}
+
+static int uniphier_uart_probe(struct platform_device *pdev)
+{
+ struct resource *regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ struct resource *irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ struct uniphier8250_priv *priv;
+ struct uart_8250_port up;
+ int ret;
+ void __iomem *membase;
+
+ if (!regs || !irq) {
+ dev_err(&pdev->dev, "missing registers or irq\n");
+ return -EINVAL;
+ }
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ membase = devm_ioremap(&pdev->dev, regs->start, resource_size(regs));
+ if (!membase)
+ return -ENOMEM;
+
+ memset(&up, 0, sizeof(up));
+
+ ret = uniphier_of_serial_setup(pdev, &up.port, priv);
+ if (ret < 0)
+ return ret;
+
+ up.port.dev = &pdev->dev;
+ up.port.private_data = priv;
+ up.port.mapbase = regs->start;
+ up.port.membase = membase;
+ up.port.irq = irq->start;
+
+ up.port.type = PORT_16550A;
+ up.port.iotype = UPIO_MEM32;
+ up.port.regshift = 2;
+ up.port.flags = UPF_FIXED_PORT | UPF_FIXED_TYPE;
+ up.capabilities = UART_CAP_FIFO;
+
+ up.port.serial_in = uniphier_serial_in;
+ up.port.serial_out = uniphier_serial_out;
+ up.dl_read = uniphier_serial_dl_read;
+ up.dl_write = uniphier_serial_dl_write;
+
+ ret = serial8250_register_8250_port(&up);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to register 8250 port\n");
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, priv);
+
+ return 0;
+}
+
+static int uniphier_uart_remove(struct platform_device *pdev)
+{
+ struct uniphier8250_priv *priv = platform_get_drvdata(pdev);
+
+ serial8250_unregister_port(priv->line);
+ if (!IS_ERR_OR_NULL(priv->clk)) {
+ clk_disable_unprepare(priv->clk);
+ clk_put(priv->clk);
+ }
+
+ return 0;
+}
+
+static const struct of_device_id uniphier_uart_match[] = {
+ { .compatible = "socionext,uniphier-uart" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, uniphier_uart_match);
+
+static struct platform_driver uniphier_uart_platform_driver = {
+ .probe = uniphier_uart_probe,
+ .remove = uniphier_uart_remove,
+ .driver = {
+ .name = "uniphier-uart",
+ .of_match_table = uniphier_uart_match,
+ },
+};
+module_platform_driver(uniphier_uart_platform_driver);
+
+MODULE_AUTHOR("Masahiro Yamada <[email protected]>");
+MODULE_DESCRIPTION("UniPhier UART driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index c350703..3e1ae446 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -342,3 +342,10 @@ config SERIAL_8250_MT6577
help
If you have a Mediatek based board and want to use the
serial port, say Y to this option. If unsure, say N.
+
+config SERIAL_8250_UNIPHIER
+ tristate "Support for UniPhier on-chip UART"
+ depends on SERIAL_8250 && ARCH_UNIPHIER
+ help
+ If you have a UniPhier based board and want to use the on-chip
+ serial ports, say Y to this option. If unsure, say N.
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
index 31e7cdc..f7ca8c3 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -23,3 +23,4 @@ obj-$(CONFIG_SERIAL_8250_EM) += 8250_em.o
obj-$(CONFIG_SERIAL_8250_OMAP) += 8250_omap.o
obj-$(CONFIG_SERIAL_8250_FINTEK) += 8250_fintek.o
obj-$(CONFIG_SERIAL_8250_MT6577) += 8250_mtk.o
+obj-$(CONFIG_SERIAL_8250_UNIPHIER) += 8250_uniphier.o
--
1.9.1


2015-05-15 22:28:51

by Joachim Eastwood

[permalink] [raw]
Subject: Re: [PATCH] serial: 8250_uniphier: add UniPhier serial driver

On 15 May 2015 at 14:26, Masahiro Yamada <[email protected]> wrote:
> Add the driver for on-chip UART used on UniPhier SoCs.
> The register map of this hardware is similar to that of 8250
> (but the address for FCR, LCR is different), so it should go
> into drivers/tty/serial/8250 directory.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> drivers/tty/serial/8250/8250_uniphier.c | 243 ++++++++++++++++++++++++++++++++
> drivers/tty/serial/8250/Kconfig | 7 +
> drivers/tty/serial/8250/Makefile | 1 +
> 3 files changed, 251 insertions(+)
> create mode 100644 drivers/tty/serial/8250/8250_uniphier.c
>
> diff --git a/drivers/tty/serial/8250/8250_uniphier.c b/drivers/tty/serial/8250/8250_uniphier.c
> new file mode 100644
> index 0000000..f108d81
> --- /dev/null
> +++ b/drivers/tty/serial/8250/8250_uniphier.c
> @@ -0,0 +1,243 @@
> +/*
> + * Copyright (C) 2015 Masahiro Yamada <[email protected]>
> + *
> + * 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/init.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/of.h>
> +#include <linux/io.h>
> +#include <linux/serial_core.h>
> +#include <linux/serial_reg.h>
> +#include <linux/serial_8250.h>

Please put the includes in alphabetic order.

Do you really need init.h?

> +#include "8250.h"
> +
> +/*
> + * Most (but not all) of UniPhier UART devices have 64-depth FIFO.
> + * Set this value if "fifo-size" property is missing from the device tree node.
> + */
> +#define UNIPHIER_UART_DEFAULT_FIFO_SIZE 64
> +
> +#define UNIPHIER_UART_CHAR_FCR 3 /* Character / FIFO Control Register */
> +#define UNIPHIER_UART_LCR_MCR 4 /* Line/Modem Control Register */
> +#define UNIPHIER_UART_DLR 9 /* Divisor Latch Register */
> +
> +/*
> + * The register map is slightly different from that of 8250.
> + * IO callbacks must be overridden for correct access to FCR, LCR, and MCR.
> + */

<snip>

> +static int uniphier_of_serial_setup(struct platform_device *pdev,
> + struct uart_port *port,
> + struct uniphier8250_priv *priv)
> +{
> + int ret;
> + u32 prop;
> + struct device_node *np = pdev->dev.of_node;
> +
> + ret = of_alias_get_id(np, "serial");
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed to get alias id\n");
> + return ret;
> + }
> + port->line = priv->line = ret;
> +
> + /* Get clk rate through clk driver */
> + priv->clk = of_clk_get(np, 0);
> + if (IS_ERR(priv->clk)) {
> + dev_err(&pdev->dev, "failed to get clock\n");
> + return PTR_ERR(priv->clk);
> + }

Use devm_clk_get() if possible.

> +
> + ret = clk_prepare_enable(priv->clk);
> + if (ret < 0)
> + return ret;
> +
> + port->uartclk = clk_get_rate(priv->clk);
> +
> + /* Check for fifo size */
> + if (of_property_read_u32(np, "fifo-size", &prop) == 0)
> + port->fifosize = prop;
> + else
> + port->fifosize = UNIPHIER_UART_DEFAULT_FIFO_SIZE;
> +
> + return 0;
> +}

<snip>

> +static int uniphier_uart_remove(struct platform_device *pdev)
> +{
> + struct uniphier8250_priv *priv = platform_get_drvdata(pdev);
> +
> + serial8250_unregister_port(priv->line);
> + if (!IS_ERR_OR_NULL(priv->clk)) {
> + clk_disable_unprepare(priv->clk);
> + clk_put(priv->clk);
> + }

If you use devm_clk_get() in uniphier_of_serial_setup() you only need
to call clk_disable_unprepare() here.

Calling clk_disable_unprepare() with NULL is allowed and you already
check for IS_ERR in your setup function.

> + return 0;
> +}


regards,
Joachim Eastwood

2015-05-16 09:17:38

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH] serial: 8250_uniphier: add UniPhier serial driver

2015-05-15 14:26 GMT+02:00 Masahiro Yamada <[email protected]>:
> Add the driver for on-chip UART used on UniPhier SoCs.
> The register map of this hardware is similar to that of 8250
> (but the address for FCR, LCR is different), so it should go
> into drivers/tty/serial/8250 directory.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> drivers/tty/serial/8250/8250_uniphier.c | 243 ++++++++++++++++++++++++++++++++
> drivers/tty/serial/8250/Kconfig | 7 +
> drivers/tty/serial/8250/Makefile | 1 +
> 3 files changed, 251 insertions(+)
> create mode 100644 drivers/tty/serial/8250/8250_uniphier.c
>
> diff --git a/drivers/tty/serial/8250/8250_uniphier.c b/drivers/tty/serial/8250/8250_uniphier.c
> new file mode 100644
> index 0000000..f108d81
> --- /dev/null
> +++ b/drivers/tty/serial/8250/8250_uniphier.c
> @@ -0,0 +1,243 @@
> +/*
> + * Copyright (C) 2015 Masahiro Yamada <[email protected]>
> + *
> + * 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/init.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/of.h>
> +#include <linux/io.h>
> +#include <linux/serial_core.h>
> +#include <linux/serial_reg.h>
> +#include <linux/serial_8250.h>
> +#include "8250.h"
> +
> +/*
> + * Most (but not all) of UniPhier UART devices have 64-depth FIFO.
> + * Set this value if "fifo-size" property is missing from the device tree node.
> + */
> +#define UNIPHIER_UART_DEFAULT_FIFO_SIZE 64
> +
> +#define UNIPHIER_UART_CHAR_FCR 3 /* Character / FIFO Control Register */
> +#define UNIPHIER_UART_LCR_MCR 4 /* Line/Modem Control Register */
> +#define UNIPHIER_UART_DLR 9 /* Divisor Latch Register */
> +
> +/*
> + * The register map is slightly different from that of 8250.
> + * IO callbacks must be overridden for correct access to FCR, LCR, and MCR.
> + */
> +static unsigned int uniphier_serial_in(struct uart_port *p, int offset)
> +{
> + int valshift = 0;
> +
> + switch (offset) {
> + case UART_LCR:
> + valshift = 8;

Please use a define for the value. Something like UNIPHIER_UART_LCR_SHIFT maybe.

Cheers,
Matthias



--
motzblog.wordpress.com

2015-05-18 03:23:50

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] serial: 8250_uniphier: add UniPhier serial driver

Hi Joachim,




2015-05-16 7:28 GMT+09:00 Joachim Eastwood <[email protected]>:

>> +
>> +#include <linux/init.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/clk.h>
>> +#include <linux/of.h>
>> +#include <linux/io.h>
>> +#include <linux/serial_core.h>
>> +#include <linux/serial_reg.h>
>> +#include <linux/serial_8250.h>
>
> Please put the includes in alphabetic order.

OK.



> Do you really need init.h?

Not really. Will remove it.


>> +static int uniphier_of_serial_setup(struct platform_device *pdev,
>> + struct uart_port *port,
>> + struct uniphier8250_priv *priv)
>> +{
>> + int ret;
>> + u32 prop;
>> + struct device_node *np = pdev->dev.of_node;
>> +
>> + ret = of_alias_get_id(np, "serial");
>> + if (ret < 0) {
>> + dev_err(&pdev->dev, "failed to get alias id\n");
>> + return ret;
>> + }
>> + port->line = priv->line = ret;
>> +
>> + /* Get clk rate through clk driver */
>> + priv->clk = of_clk_get(np, 0);
>> + if (IS_ERR(priv->clk)) {
>> + dev_err(&pdev->dev, "failed to get clock\n");
>> + return PTR_ERR(priv->clk);
>> + }
>
> Use devm_clk_get() if possible.

Sure.



>
>> +static int uniphier_uart_remove(struct platform_device *pdev)
>> +{
>> + struct uniphier8250_priv *priv = platform_get_drvdata(pdev);
>> +
>> + serial8250_unregister_port(priv->line);
>> + if (!IS_ERR_OR_NULL(priv->clk)) {
>> + clk_disable_unprepare(priv->clk);
>> + clk_put(priv->clk);
>> + }
>
> If you use devm_clk_get() in uniphier_of_serial_setup() you only need
> to call clk_disable_unprepare() here.
>
> Calling clk_disable_unprepare() with NULL is allowed and you already
> check for IS_ERR in your setup function.



Yes.

Thank you for your review!



--
Best Regards
Masahiro Yamada

2015-05-18 03:24:36

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] serial: 8250_uniphier: add UniPhier serial driver

Hi Matthias,


2015-05-16 18:17 GMT+09:00 Matthias Brugger <[email protected]>:

>> +/*
>> + * The register map is slightly different from that of 8250.
>> + * IO callbacks must be overridden for correct access to FCR, LCR, and MCR.
>> + */
>> +static unsigned int uniphier_serial_in(struct uart_port *p, int offset)
>> +{
>> + int valshift = 0;
>> +
>> + switch (offset) {
>> + case UART_LCR:
>> + valshift = 8;
>
> Please use a define for the value. Something like UNIPHIER_UART_LCR_SHIFT maybe.
>


Will do in v2.

Thank you!


--
Best Regards
Masahiro Yamada