[RFC PATCH] tty: serial: Add 8250-core based omap driver

[RFC PATCH] tty: serial: Add 8250-core based omap driver

[Felipe Balbi]

Hi,

+linux-omap, lakml

On Wed, Jul 02, 2014 at 06:00:09PM +0200, Sebastian Andrzej Siewior wrote:
> This patch provides a 8250-core based UART driver for the internal OMAP
> UART. The longterm goal is to provide the same functionality as the
> current OMAP uart driver and hopefully DMA support which could borrowed
> from the 8250-core.
> 
> The whole PM-Runtime part is currently missing.

AWESOME!!!! I guess PM can be added after we know this i working fine..
Note that there are many workarounds which have been implemented one way
or another in the OMAP serial driver. Some of them might not even apply
to 8250, though.

> It has been only tested as console UART.
> The tty name is ttyS based instead of ttyO. How big is the pain here,
> what could be the easiest way to provide compatibility?

have been considering that myself for months. You could pass an optional
argument to serial8250_register_8250_port() but that only solves part of
the problem :-(

all in all, patch looks good to me.

> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  drivers/tty/serial/8250/8250_core.c |   7 ++
>  drivers/tty/serial/8250/8250_omap.c | 224 ++++++++++++++++++++++++++++++++++++
>  drivers/tty/serial/8250/Kconfig     |   9 ++
>  drivers/tty/serial/8250/Makefile    |   1 +
>  include/uapi/linux/serial_core.h    |   3 +-
>  5 files changed, 243 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/tty/serial/8250/8250_omap.c
> 
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index 7a91c6d..f47e876 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -260,6 +260,13 @@ static const struct serial8250_config uart_config[] = {
>  		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
>  		.flags		= UART_CAP_FIFO | UART_CAP_AFE,
>  	},
> +	[PORT_OMAP_16750] = {
> +		.name		= "OMAP",
> +		.fifo_size	= 64,
> +		.tx_loadsz	= 64,
> +		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_01,
> +		.flags		= UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP,
> +	},
>  	[PORT_TEGRA] = {
>  		.name		= "Tegra",
>  		.fifo_size	= 32,
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> new file mode 100644
> index 0000000..a23013f
> --- /dev/null
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -0,0 +1,224 @@
> +/*
> + * 8250-core based driver for the OMAP internal UART
> + *
> + *  Copyright (C) 2014 Sebastian Andrzej Siewior
> + *
> + */
> +
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/serial_8250.h>
> +#include <linux/serial_core.h>
> +#include <linux/serial_reg.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +
> +#include "8250.h"
> +
> +#define UART_DLL_EM 9
> +#define UART_DLM_EM 10
> +
> +#define DEFAULT_CLK_SPEED 48000000 /* 48Mhz*/
> +
> +#define UART_ERRATA_i202_MDR1_ACCESS	(1 << 0)
> +#define OMAP_UART_WER_HAS_TX_WAKEUP	(1 << 1)
> +
> +/* SCR register bitmasks */
> +#define OMAP_UART_SCR_RX_TRIG_GRANU1_MASK	(1 << 7)
> +#define OMAP_UART_SCR_TX_TRIG_GRANU1_MASK	(1 << 6)
> +#define OMAP_UART_SCR_TX_EMPTY			(1 << 3)
> +
> +/* MVR register bitmasks */
> +#define OMAP_UART_MVR_SCHEME_SHIFT	30
> +#define OMAP_UART_LEGACY_MVR_MAJ_MASK	0xf0
> +#define OMAP_UART_LEGACY_MVR_MAJ_SHIFT	4
> +#define OMAP_UART_LEGACY_MVR_MIN_MASK	0x0f
> +#define OMAP_UART_MVR_MAJ_MASK		0x700
> +#define OMAP_UART_MVR_MAJ_SHIFT		8
> +#define OMAP_UART_MVR_MIN_MASK		0x3f
> +
> +#define UART_BUILD_REVISION(x, y)	(((x) << 8) | (y))
> +
> +#define OMAP_UART_REV_46 0x0406
> +#define OMAP_UART_REV_52 0x0502
> +#define OMAP_UART_REV_63 0x0603
> +
> +struct serial8250_omap_priv {
> +	int line;
> +	u32 habit;
> +};
> +
> +static u32 uart_read(struct uart_8250_port *up, u32 reg)
> +{
> +	return readl(up->port.membase + (reg << up->port.regshift));
> +}
> +
> +static void uart_write(struct uart_8250_port *up, u32 reg, u32 val)
> +{
> +	writel(val, up->port.membase + (reg << up->port.regshift));
> +}
> +
> +static void omap_serial_fill_features_erratas(struct uart_8250_port *up,
> +		struct serial8250_omap_priv *priv)
> +{
> +	u32 mvr, scheme;
> +	u16 revision, major, minor;
> +
> +	mvr = uart_read(up, UART_OMAP_MVER);
> +
> +	/* Check revision register scheme */
> +	scheme = mvr >> OMAP_UART_MVR_SCHEME_SHIFT;
> +
> +	switch (scheme) {
> +	case 0: /* Legacy Scheme: OMAP2/3 */
> +		/* MINOR_REV[0:4], MAJOR_REV[4:7] */
> +		major = (mvr & OMAP_UART_LEGACY_MVR_MAJ_MASK) >>
> +			OMAP_UART_LEGACY_MVR_MAJ_SHIFT;
> +		minor = (mvr & OMAP_UART_LEGACY_MVR_MIN_MASK);
> +		break;
> +	case 1:
> +		/* New Scheme: OMAP4+ */
> +		/* MINOR_REV[0:5], MAJOR_REV[8:10] */
> +		major = (mvr & OMAP_UART_MVR_MAJ_MASK) >>
> +			OMAP_UART_MVR_MAJ_SHIFT;
> +		minor = (mvr & OMAP_UART_MVR_MIN_MASK);
> +		break;
> +	default:
> +		dev_warn(up->port.dev,
> +				"Unknown revision, defaulting to highest\n");
> +		/* highest possible revision */
> +		major = 0xff;
> +		minor = 0xff;
> +	}
> +	/* normalize revision for the driver */
> +	revision = UART_BUILD_REVISION(major, minor);
> +
> +	switch (revision) {
> +	case OMAP_UART_REV_46:
> +		priv->habit = UART_ERRATA_i202_MDR1_ACCESS;
> +		break;
> +	case OMAP_UART_REV_52:
> +		priv->habit = UART_ERRATA_i202_MDR1_ACCESS |
> +				OMAP_UART_WER_HAS_TX_WAKEUP;
> +		break;
> +	case OMAP_UART_REV_63:
> +		priv->habit = UART_ERRATA_i202_MDR1_ACCESS |
> +			OMAP_UART_WER_HAS_TX_WAKEUP;
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
> +static int serial8250_omap_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 serial8250_omap_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) {
> +		dev_err(&pdev->dev, "unable to allocate private data\n");
> +		return -ENOMEM;
> +	}
> +	membase = devm_ioremap_nocache(&pdev->dev, regs->start,
> +			resource_size(regs));
> +	if (!membase)
> +		return -ENODEV;
> +
> +	memset(&up, 0, sizeof(up));
> +	up.port.dev = &pdev->dev;
> +	up.port.mapbase = regs->start;
> +	up.port.membase = membase;
> +	up.port.irq = irq->start;
> +	/*
> +	 * It claims to be 16C750 compatible however it is a little different.
> +	 * It has EFR and has no FCR7_64byte bit. The AFE which it claims to is
> +	 * enabled via EFR instead of MCR.
> +	 */
> +	up.port.type = PORT_OMAP_16750;
> +	up.port.iotype = UPIO_MEM32;
> +	up.port.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UPF_FIXED_TYPE;
> +	up.port.private_data = priv;
> +
> +	up.port.regshift = 2;
> +	up.port.fifosize = 64;
> +
> +	if (pdev->dev.of_node) {
> +		up.port.line = of_alias_get_id(pdev->dev.of_node, "serial");
> +		of_property_read_u32(pdev->dev.of_node, "clock-frequency",
> +				&up.port.uartclk);
> +	} else {
> +		up.port.line = pdev->id;
> +	}
> +
> +	if (up.port.line < 0) {
> +		dev_err(&pdev->dev, "failed to get alias/pdev id, errno %d\n",
> +				up.port.line);
> +		ret = -ENODEV;
> +		/*XXX*/
> +		BUG();
> +	}
> +	if (!up.port.uartclk) {
> +		up.port.uartclk = DEFAULT_CLK_SPEED;
> +		dev_warn(&pdev->dev,
> +				"No clock speed specified: using default: %d\n",
> +				DEFAULT_CLK_SPEED);
> +	}
> +
> +	omap_serial_fill_features_erratas(&up, priv);
> +
> +	uart_write(&up, UART_OMAP_SCR, OMAP_UART_SCR_TX_EMPTY |
> +			OMAP_UART_SCR_RX_TRIG_GRANU1_MASK);
> +
> +	ret = serial8250_register_8250_port(&up);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "unable to register 8250 port\n");
> +		return ret;
> +	}
> +	priv->line = ret;
> +	platform_set_drvdata(pdev, priv);
> +	return 0;
> +}
> +
> +static int serial8250_omap_remove(struct platform_device *pdev)
> +{
> +	struct serial8250_omap_priv *priv = platform_get_drvdata(pdev);
> +
> +	serial8250_unregister_port(priv->line);
> +	return 0;
> +}
> +
> +static const struct of_device_id serial8250_omap_dt_ids[] = {
> +	{ .compatible = "ti,omap2-uart" },
> +	{ .compatible = "ti,omap3-uart" },
> +	{ .compatible = "ti,omap4-uart" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, serial8250_omap_dt_ids);
> +
> +static struct platform_driver serial8250_omap_platform_driver = {
> +	.driver = {
> +		.name		= "serial8250-omap",
> +		.of_match_table = serial8250_omap_dt_ids,
> +		.owner		= THIS_MODULE,
> +	},
> +	.probe			= serial8250_omap_probe,
> +	.remove			= serial8250_omap_remove,
> +};
> +
> +module_platform_driver(serial8250_omap_platform_driver);
> +
> +MODULE_AUTHOR("Sebastian Andrzej Siewior");
> +MODULE_DESCRIPTION("OMAP 8250 Driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index 349ee59..7a5073b 100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -298,3 +298,12 @@ 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_OMAP
> +	tristate "Support for OMAP internal UART (8250 based driver)"
> +	depends on SERIAL_8250 && ARCH_OMAP2PLUS
> +	help
> +	  If you have a machine based on an Texas Instruments OMAP CPU you
> +	  can enable its onboard serial ports by enabling this option.
> +
> +	  This driver is in early stage and uses ttyS instead of ttyO.
> diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
> index 36d68d0..4bac392 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_OMAP)		+= 8250_omap.o
> diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
> index 5820269..74f9b11 100644
> --- a/include/uapi/linux/serial_core.h
> +++ b/include/uapi/linux/serial_core.h
> @@ -54,7 +54,8 @@
>  #define PORT_ALTR_16550_F32 26	/* Altera 16550 UART with 32 FIFOs */
>  #define PORT_ALTR_16550_F64 27	/* Altera 16550 UART with 64 FIFOs */
>  #define PORT_ALTR_16550_F128 28 /* Altera 16550 UART with 128 FIFOs */
> -#define PORT_MAX_8250	28	/* max port ID */
> +#define PORT_OMAP_16750	29	/* TI's OMAP internal 16C750 compatible UART */
> +#define PORT_MAX_8250	29	/* max port ID */
>  
>  /*
>   * ARM specific type numbers.  These are not currently guaranteed
> -- 
> 2.0.0
> 

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140702/f0750271/attachment-0001.sig>

[RFC PATCH] tty: serial: Add 8250-core based omap driver

[Aaro Koskinen]

Hi,

On Wed, Jul 02, 2014 at 11:09:32AM -0500, Felipe Balbi wrote:
> > It has been only tested as console UART.
> > The tty name is ttyS based instead of ttyO. How big is the pain here,
> > what could be the easiest way to provide compatibility?
> 
> have been considering that myself for months. You could pass an optional
> argument to serial8250_register_8250_port() but that only solves part of
> the problem :-(

When ttyS -> ttyO change was done on OMAP, compatibility was not an issue.
Why should we care about it now?

A.

[RFC PATCH] tty: serial: Add 8250-core based omap driver

[Robert Nelson]

On Wed, Jul 2, 2014 at 2:09 PM, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> Hi,
>
> On Wed, Jul 02, 2014 at 11:09:32AM -0500, Felipe Balbi wrote:
>> > It has been only tested as console UART.
>> > The tty name is ttyS based instead of ttyO. How big is the pain here,
>> > what could be the easiest way to provide compatibility?
>>
>> have been considering that myself for months. You could pass an optional
>> argument to serial8250_register_8250_port() but that only solves part of
>> the problem :-(
>
> When ttyS -> ttyO change was done on OMAP, compatibility was not an issue.
> Why should we care about it now?

It would be a good opportunity to force everyone to update their bootloader. ;)

Besides the BeagleBoard forum is quiet now, no one is complaining
about that old (ttyS -> ttyO) transition anymore..

I'll just end up carrying a patch like, to support bb.org users over
the transition..

https://github.com/RobertCNelson/stable-kernel/blob/v3.7.x/patches/omap_beagle/0004-zeroMAP-Open-your-eyes.patch

Regards,

-- 
Robert Nelson
http://www.rcn-ee.com/

[RFC PATCH] tty: serial: Add 8250-core based omap driver

[Tony Lindgren]

* Robert Nelson <robertcnelson@gmail.com> [140702 12:27]:
> On Wed, Jul 2, 2014 at 2:09 PM, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> > Hi,
> >
> > On Wed, Jul 02, 2014 at 11:09:32AM -0500, Felipe Balbi wrote:
> >> > It has been only tested as console UART.
> >> > The tty name is ttyS based instead of ttyO. How big is the pain here,
> >> > what could be the easiest way to provide compatibility?
> >>
> >> have been considering that myself for months. You could pass an optional
> >> argument to serial8250_register_8250_port() but that only solves part of
> >> the problem :-(

Some kind of compability layer sure would be nice.

> > When ttyS -> ttyO change was done on OMAP, compatibility was not an issue.
> > Why should we care about it now?
> 
> It would be a good opportunity to force everyone to update their bootloader. ;)
> 
> Besides the BeagleBoard forum is quiet now, no one is complaining
> about that old (ttyS -> ttyO) transition anymore..

How about a Kconfig option to provide ttyO by default? The not even
do that if kernel has cmdline option nottyomap.
 
> I'll just end up carrying a patch like, to support bb.org users over
> the transition..
> 
> https://github.com/RobertCNelson/stable-kernel/blob/v3.7.x/patches/omap_beagle/0004-zeroMAP-Open-your-eyes.patch

Heh. Just to summarize the reason ttyO needs to be a separate name
and device entry from ttyS is because we also have external 8250
devices on GPMC and hotplug busses.

Regards,

Tony

[RFC PATCH] tty: serial: Add 8250-core based omap driver

[Felipe Balbi]

On Thu, Jul 03, 2014 at 12:34:11AM -0700, Tony Lindgren wrote:
> * Robert Nelson <robertcnelson@gmail.com> [140702 12:27]:
> > On Wed, Jul 2, 2014 at 2:09 PM, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> > > Hi,
> > >
> > > On Wed, Jul 02, 2014 at 11:09:32AM -0500, Felipe Balbi wrote:
> > >> > It has been only tested as console UART.
> > >> > The tty name is ttyS based instead of ttyO. How big is the pain here,
> > >> > what could be the easiest way to provide compatibility?
> > >>
> > >> have been considering that myself for months. You could pass an optional
> > >> argument to serial8250_register_8250_port() but that only solves part of
> > >> the problem :-(
> 
> Some kind of compability layer sure would be nice.
> 
> > > When ttyS -> ttyO change was done on OMAP, compatibility was not an issue.
> > > Why should we care about it now?
> > 
> > It would be a good opportunity to force everyone to update their bootloader. ;)
> > 
> > Besides the BeagleBoard forum is quiet now, no one is complaining
> > about that old (ttyS -> ttyO) transition anymore..
> 
> How about a Kconfig option to provide ttyO by default? The not even
> do that if kernel has cmdline option nottyomap.

what about single zImage ? I don't want to use ttyO on my
Allwinner/Exynos/Snapdragon/whatever SoC just because OMAP is in the
same image ;-)

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140703/4400eab2/attachment.sig>

[RFC PATCH] tty: serial: Add 8250-core based omap driver

[Robert Nelson]

On Thu, Jul 3, 2014 at 8:25 AM, Felipe Balbi <balbi@ti.com> wrote:
> On Thu, Jul 03, 2014 at 12:34:11AM -0700, Tony Lindgren wrote:
>> * Robert Nelson <robertcnelson@gmail.com> [140702 12:27]:
>> > On Wed, Jul 2, 2014 at 2:09 PM, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
>> > > Hi,
>> > >
>> > > On Wed, Jul 02, 2014 at 11:09:32AM -0500, Felipe Balbi wrote:
>> > >> > It has been only tested as console UART.
>> > >> > The tty name is ttyS based instead of ttyO. How big is the pain here,
>> > >> > what could be the easiest way to provide compatibility?
>> > >>
>> > >> have been considering that myself for months. You could pass an optional
>> > >> argument to serial8250_register_8250_port() but that only solves part of
>> > >> the problem :-(
>>
>> Some kind of compability layer sure would be nice.
>>
>> > > When ttyS -> ttyO change was done on OMAP, compatibility was not an issue.
>> > > Why should we care about it now?
>> >
>> > It would be a good opportunity to force everyone to update their bootloader. ;)
>> >
>> > Besides the BeagleBoard forum is quiet now, no one is complaining
>> > about that old (ttyS -> ttyO) transition anymore..
>>
>> How about a Kconfig option to provide ttyO by default? The not even
>> do that if kernel has cmdline option nottyomap.
>
> what about single zImage ? I don't want to use ttyO on my
> Allwinner/Exynos/Snapdragon/whatever SoC just because OMAP is in the
> same image ;-)

What if we just kept it simple, leave the ttyO driver enabled and add
a warning (pr_info) that it's deprecated.  It's not like it's broken,
it just won't get later features or devices support added.

Regards,

-- 
Robert Nelson
http://www.rcn-ee.com/

[RFC PATCH] tty: serial: Add 8250-core based omap driver

[Felipe Balbi]

On Thu, Jul 03, 2014 at 08:34:44AM -0500, Robert Nelson wrote:
> On Thu, Jul 3, 2014 at 8:25 AM, Felipe Balbi <balbi@ti.com> wrote:
> > On Thu, Jul 03, 2014 at 12:34:11AM -0700, Tony Lindgren wrote:
> >> * Robert Nelson <robertcnelson@gmail.com> [140702 12:27]:
> >> > On Wed, Jul 2, 2014 at 2:09 PM, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> >> > > Hi,
> >> > >
> >> > > On Wed, Jul 02, 2014 at 11:09:32AM -0500, Felipe Balbi wrote:
> >> > >> > It has been only tested as console UART.
> >> > >> > The tty name is ttyS based instead of ttyO. How big is the pain here,
> >> > >> > what could be the easiest way to provide compatibility?
> >> > >>
> >> > >> have been considering that myself for months. You could pass an optional
> >> > >> argument to serial8250_register_8250_port() but that only solves part of
> >> > >> the problem :-(
> >>
> >> Some kind of compability layer sure would be nice.
> >>
> >> > > When ttyS -> ttyO change was done on OMAP, compatibility was not an issue.
> >> > > Why should we care about it now?
> >> >
> >> > It would be a good opportunity to force everyone to update their bootloader. ;)
> >> >
> >> > Besides the BeagleBoard forum is quiet now, no one is complaining
> >> > about that old (ttyS -> ttyO) transition anymore..
> >>
> >> How about a Kconfig option to provide ttyO by default? The not even
> >> do that if kernel has cmdline option nottyomap.
> >
> > what about single zImage ? I don't want to use ttyO on my
> > Allwinner/Exynos/Snapdragon/whatever SoC just because OMAP is in the
> > same image ;-)
> 
> What if we just kept it simple, leave the ttyO driver enabled and add
> a warning (pr_info) that it's deprecated.  It's not like it's broken,
> it just won't get later features or devices support added.

Fine by me, I'd switch to 8250 as soon as it's merged though :-) would
be nice to get an example DTS change just so I can start testing on the
boards I have around.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140703/0ad8123b/attachment-0001.sig>

[RFC PATCH] tty: serial: Add 8250-core based omap driver

[Sebastian Reichel]

Hi,

On Thu, Jul 03, 2014 at 09:07:36AM -0500, Felipe Balbi wrote:
> On Thu, Jul 03, 2014 at 08:34:44AM -0500, Robert Nelson wrote:
> > On Thu, Jul 3, 2014 at 8:25 AM, Felipe Balbi <balbi@ti.com> wrote:
> > > On Thu, Jul 03, 2014 at 12:34:11AM -0700, Tony Lindgren wrote:
> > >> * Robert Nelson <robertcnelson@gmail.com> [140702 12:27]:
> > >> > On Wed, Jul 2, 2014 at 2:09 PM, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> > >> > > Hi,
> > >> > >
> > >> > > On Wed, Jul 02, 2014 at 11:09:32AM -0500, Felipe Balbi wrote:
> > >> > >> > It has been only tested as console UART.
> > >> > >> > The tty name is ttyS based instead of ttyO. How big is the pain here,
> > >> > >> > what could be the easiest way to provide compatibility?
> > >> > >>
> > >> > >> have been considering that myself for months. You could pass an optional
> > >> > >> argument to serial8250_register_8250_port() but that only solves part of
> > >> > >> the problem :-(
> > >>
> > >> Some kind of compability layer sure would be nice.
> > >>
> > >> > > When ttyS -> ttyO change was done on OMAP, compatibility was not an issue.
> > >> > > Why should we care about it now?
> > >> >
> > >> > It would be a good opportunity to force everyone to update their bootloader. ;)
> > >> >
> > >> > Besides the BeagleBoard forum is quiet now, no one is complaining
> > >> > about that old (ttyS -> ttyO) transition anymore..
> > >>
> > >> How about a Kconfig option to provide ttyO by default? The not even
> > >> do that if kernel has cmdline option nottyomap.
> > >
> > > what about single zImage ? I don't want to use ttyO on my
> > > Allwinner/Exynos/Snapdragon/whatever SoC just because OMAP is in the
> > > same image ;-)
> > 
> > What if we just kept it simple, leave the ttyO driver enabled and add
> > a warning (pr_info) that it's deprecated.  It's not like it's broken,
> > it just won't get later features or devices support added.
> 
> Fine by me, I'd switch to 8250 as soon as it's merged though :-) would
> be nice to get an example DTS change just so I can start testing on the
> boards I have around.

DT is supposed to contain information about the hardware, so it
should stay the same? I think there is no non-hackish way to decide
at runtime which driver should be loaded.

One possible solution is:
 * Keep both drivers for a couple of kernel releases
 * Add the deprecation warning to the older one
 * Add a conflict between both drivers in Kconfig

Thus its decided at build-time, which driver should be used. This
would keep existing .config files working for a couple of releases.

-- Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140703/b3a4290e/attachment.sig>

[RFC PATCH] tty: serial: Add 8250-core based omap driver

[Felipe Balbi]

On Thu, Jul 03, 2014 at 05:44:26PM +0200, Sebastian Reichel wrote:
> Hi,
> 
> On Thu, Jul 03, 2014 at 09:07:36AM -0500, Felipe Balbi wrote:
> > On Thu, Jul 03, 2014 at 08:34:44AM -0500, Robert Nelson wrote:
> > > On Thu, Jul 3, 2014 at 8:25 AM, Felipe Balbi <balbi@ti.com> wrote:
> > > > On Thu, Jul 03, 2014 at 12:34:11AM -0700, Tony Lindgren wrote:
> > > >> * Robert Nelson <robertcnelson@gmail.com> [140702 12:27]:
> > > >> > On Wed, Jul 2, 2014 at 2:09 PM, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> > > >> > > Hi,
> > > >> > >
> > > >> > > On Wed, Jul 02, 2014 at 11:09:32AM -0500, Felipe Balbi wrote:
> > > >> > >> > It has been only tested as console UART.
> > > >> > >> > The tty name is ttyS based instead of ttyO. How big is the pain here,
> > > >> > >> > what could be the easiest way to provide compatibility?
> > > >> > >>
> > > >> > >> have been considering that myself for months. You could pass an optional
> > > >> > >> argument to serial8250_register_8250_port() but that only solves part of
> > > >> > >> the problem :-(
> > > >>
> > > >> Some kind of compability layer sure would be nice.
> > > >>
> > > >> > > When ttyS -> ttyO change was done on OMAP, compatibility was not an issue.
> > > >> > > Why should we care about it now?
> > > >> >
> > > >> > It would be a good opportunity to force everyone to update their bootloader. ;)
> > > >> >
> > > >> > Besides the BeagleBoard forum is quiet now, no one is complaining
> > > >> > about that old (ttyS -> ttyO) transition anymore..
> > > >>
> > > >> How about a Kconfig option to provide ttyO by default? The not even
> > > >> do that if kernel has cmdline option nottyomap.
> > > >
> > > > what about single zImage ? I don't want to use ttyO on my
> > > > Allwinner/Exynos/Snapdragon/whatever SoC just because OMAP is in the
> > > > same image ;-)
> > > 
> > > What if we just kept it simple, leave the ttyO driver enabled and add
> > > a warning (pr_info) that it's deprecated.  It's not like it's broken,
> > > it just won't get later features or devices support added.
> > 
> > Fine by me, I'd switch to 8250 as soon as it's merged though :-) would
> > be nice to get an example DTS change just so I can start testing on the
> > boards I have around.
> 
> DT is supposed to contain information about the hardware, so it
> should stay the same? I think there is no non-hackish way to decide

compatible would change, at a minimum.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140703/41a0687f/attachment.sig>

[RFC PATCH] tty: serial: Add 8250-core based omap driver

[Sebastian Reichel]

Hi,

On Thu, Jul 03, 2014 at 10:52:40AM -0500, Felipe Balbi wrote:
> > DT is supposed to contain information about the hardware, so it
> > should stay the same? I think there is no non-hackish way to decide
> 
> compatible would change, at a minimum.

why? I would expect it to stay the same (and the current patch uses
the same compatible strings).

-- Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140703/0243fe41/attachment.sig>

[RFC PATCH] tty: serial: Add 8250-core based omap driver

[Javier Martinez Canillas]

On Thu, Jul 3, 2014 at 6:06 PM, Sebastian Reichel <sre@kernel.org> wrote:
> Hi,
>
> On Thu, Jul 03, 2014 at 10:52:40AM -0500, Felipe Balbi wrote:
>> > DT is supposed to contain information about the hardware, so it
>> > should stay the same? I think there is no non-hackish way to decide
>>
>> compatible would change, at a minimum.
>
> why? I would expect it to stay the same (and the current patch uses
> the same compatible strings).
>

Exactly, the new driver must support all the compatible strings
defined in Documentation/devicetree/bindings/serial/omap_serial.tx
(which already does as you pointed out).

Otherwise the current drivers/tty/serial/omap-serial.c could never be
removed since that would mean breaking DT backward compatibility.

> -- Sebastian

Best regards,
Javier

[RFC PATCH] tty: serial: Add 8250-core based omap driver

[Felipe Balbi]

On Thu, Jul 03, 2014 at 06:19:47PM +0200, Javier Martinez Canillas wrote:
> On Thu, Jul 3, 2014 at 6:06 PM, Sebastian Reichel <sre@kernel.org> wrote:
> > Hi,
> >
> > On Thu, Jul 03, 2014 at 10:52:40AM -0500, Felipe Balbi wrote:
> >> > DT is supposed to contain information about the hardware, so it
> >> > should stay the same? I think there is no non-hackish way to decide
> >>
> >> compatible would change, at a minimum.
> >
> > why? I would expect it to stay the same (and the current patch uses
> > the same compatible strings).
> >
> 
> Exactly, the new driver must support all the compatible strings
> defined in Documentation/devicetree/bindings/serial/omap_serial.tx
> (which already does as you pointed out).
> 
> Otherwise the current drivers/tty/serial/omap-serial.c could never be
> removed since that would mean breaking DT backward compatibility.

that settles it then... good. I'll start testing this next week.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140703/6befb010/attachment.sig>

[RFC PATCH] tty: serial: Add 8250-core based omap driver

[Tony Lindgren]

* Felipe Balbi <balbi@ti.com> [140703 10:10]:
> On Thu, Jul 03, 2014 at 06:19:47PM +0200, Javier Martinez Canillas wrote:
> > On Thu, Jul 3, 2014 at 6:06 PM, Sebastian Reichel <sre@kernel.org> wrote:
> > > Hi,
> > >
> > > On Thu, Jul 03, 2014 at 10:52:40AM -0500, Felipe Balbi wrote:
> > >> > DT is supposed to contain information about the hardware, so it
> > >> > should stay the same? I think there is no non-hackish way to decide
> > >>
> > >> compatible would change, at a minimum.
> > >
> > > why? I would expect it to stay the same (and the current patch uses
> > > the same compatible strings).
> > >
> > 
> > Exactly, the new driver must support all the compatible strings
> > defined in Documentation/devicetree/bindings/serial/omap_serial.tx
> > (which already does as you pointed out).
> > 
> > Otherwise the current drivers/tty/serial/omap-serial.c could never be
> > removed since that would mean breaking DT backward compatibility.
> 
> that settles it then... good. I'll start testing this next week.

Yeah full compability is the way to go here.

FYI we cannot switch over completely until runtime PM works as it
will block the deeper idle states for omaps and make it impossible
to debug PM features.

Regards,

Tony

[RFC PATCH] tty: serial: Add 8250-core based omap driver

[Sebastian Andrzej Siewior]

On 07/03/2014 09:34 AM, Tony Lindgren wrote:
> Heh. Just to summarize the reason ttyO needs to be a separate name
> and device entry from ttyS is because we also have external 8250
> devices on GPMC and hotplug busses.

So the GPMC devices will first get a higher minor/device number. The
internal serial ports should show up first. I don't see the problem
(yet).

If you need a separate major number (and name) like we do have it now
(between ttySx and ttyOx) then one of requirements would be to tell
Kconfig that one driver can be active at a time.

For me the ttyS vs ttyO thing is purely cosmetic. I personally don't
like that it is different on different platforms like ttymxc, ttySC,
ttyTHS, ?

I understand that it is easier to have a specific name for it so one
does not need to find out which node is which (but then there is udev
which could create unique device names based on attributes). And then
you need enter this thingy to securetty & inittab.

That said I am easy on it. Whatever the majority here decides to be a
sane solution I will try to make it happen.

> 
> Regards,
> 
> Tony
> 

Sebastian

[RFC PATCH] tty: serial: Add 8250-core based omap driver

[Robert Nelson]

On Fri, Jul 4, 2014 at 11:51 AM, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
> On 07/03/2014 09:34 AM, Tony Lindgren wrote:
>> Heh. Just to summarize the reason ttyO needs to be a separate name
>> and device entry from ttyS is because we also have external 8250
>> devices on GPMC and hotplug busses.
>
> So the GPMC devices will first get a higher minor/device number. The
> internal serial ports should show up first. I don't see the problem
> (yet).
>
> If you need a separate major number (and name) like we do have it now
> (between ttySx and ttyOx) then one of requirements would be to tell
> Kconfig that one driver can be active at a time.
>
> For me the ttyS vs ttyO thing is purely cosmetic. I personally don't
> like that it is different on different platforms like ttymxc, ttySC,
> ttyTHS, ?

Maybe it's time to migrate them all to one name? (ttyS)  Does the end
user really care if they have a soc with an omap/imx serial port
driver? Or do they just want to access /dev/ttySx and connect to their
device?

Regards,

-- 
Robert Nelson
http://www.rcn-ee.com/

[RFC PATCH] tty: serial: Add 8250-core based omap driver

[Tony Lindgren]

* Robert Nelson <robertcnelson@gmail.com> [140704 09:59]:
> On Fri, Jul 4, 2014 at 11:51 AM, Sebastian Andrzej Siewior
> <bigeasy@linutronix.de> wrote:
> > On 07/03/2014 09:34 AM, Tony Lindgren wrote:
> >> Heh. Just to summarize the reason ttyO needs to be a separate name
> >> and device entry from ttyS is because we also have external 8250
> >> devices on GPMC and hotplug busses.
> >
> > So the GPMC devices will first get a higher minor/device number. The
> > internal serial ports should show up first. I don't see the problem
> > (yet).
> >
> > If you need a separate major number (and name) like we do have it now
> > (between ttySx and ttyOx) then one of requirements would be to tell
> > Kconfig that one driver can be active at a time.
> >
> > For me the ttyS vs ttyO thing is purely cosmetic. I personally don't
> > like that it is different on different platforms like ttymxc, ttySC,
> > ttyTHS, ?

The dev entry had to be different with serial-omap as it was not
using 8250 code. With the code moving back to using 8250 code
there are no dev conflicts so ttyS is just fine as the 8250 code
handles allocating the devices.
 
> Maybe it's time to migrate them all to one name? (ttyS)  Does the end
> user really care if they have a soc with an omap/imx serial port
> driver? Or do they just want to access /dev/ttySx and connect to their
> device?

Yes the 8250 code should always use ttyS. Then maybe we could
have also ttyO dev entries created by the driver for compability.

Earlier serial-omap could not provide the ttyS compability easily
because they were separate drivers.

Regards,

Tony

[RFC PATCH] tty: serial: Add 8250-core based omap driver

[One Thousand Gnomes]

On Fri, 4 Jul 2014 11:57:40 -0500
Robert Nelson <robertcnelson@gmail.com> wrote:

> On Fri, Jul 4, 2014 at 11:51 AM, Sebastian Andrzej Siewior
> <bigeasy@linutronix.de> wrote:
> > On 07/03/2014 09:34 AM, Tony Lindgren wrote:
> >> Heh. Just to summarize the reason ttyO needs to be a separate name
> >> and device entry from ttyS is because we also have external 8250
> >> devices on GPMC and hotplug busses.
> >
> > So the GPMC devices will first get a higher minor/device number. The
> > internal serial ports should show up first. I don't see the problem
> > (yet).
> >
> > If you need a separate major number (and name) like we do have it now
> > (between ttySx and ttyOx) then one of requirements would be to tell
> > Kconfig that one driver can be active at a time.
> >
> > For me the ttyS vs ttyO thing is purely cosmetic. I personally don't
> > like that it is different on different platforms like ttymxc, ttySC,
> > ttyTHS, ?
> 
> Maybe it's time to migrate them all to one name? (ttyS)  Does the end
> user really care if they have a soc with an omap/imx serial port
> driver? Or do they just want to access /dev/ttySx and connect to their
> device?

Userspace question. Actual userspace visible device naming policy belongs
to udev.

Alan

[RFC PATCH] tty: serial: Add 8250-core based omap driver

[Peter Maydell]

On 2 July 2014 20:09, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> When ttyS -> ttyO change was done on OMAP, compatibility was not an issue.
> Why should we care about it now?

You should have cared about it back then as well -- it was
really annoying and required everybody running an OMAP
based board to suddenly fix up their userland and command
line configs when they got broken by a kernel upgrade.

thanks
-- PMM