Hi,

On Fri, Aug 15, 2014 at 07:42:33PM +0200, Sebastian Andrzej Siewior wrote:

I don't think you need this. Reasons below...

I don't see any reason why this could not be always cleared regardless
of the type:

        up->ier &= ~(UART_IER_RLSI | UART_IRE_RDI);

[cut]

+static int omap8250_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 omap8250_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
+	 * have is enabled via EFR instead of MCR.
+	 */
+	up.port.type = PORT_OMAP_16750;

Since you are not calling serial8250_do_set_termios, 8250_core.c newer
overrides the FCR set in this driver. However, if the FCR is a
problem, since Yoshihiro added the member for it to struct
uart_8250_port (commit aef9a7bd9), just make it possible for the probe
drivers to provide also it's value:

--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c

@@ -2829,7 +2829,9 @@ static void serial8250_config_port(struct uart_port *port, int flags)
                port->handle_irq = exar_handle_irq;
 
        register_dev_spec_attr_grp(up);
-       up->fcr = uart_config[up->port.type].fcr;
+
+       if (!up->fcr)
+               up->fcr = uart_config[up->port.type].fcr;
 }
 
 static int

So instead of using PORT_OMAP_16750:
        up.port.type = PORT_16750;
        up.capabilities = UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP;

and the fcr if needed.
        up.fcr = ...

+	up.port.iotype = UPIO_MEM32;
+	up.port.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UPF_FIXED_TYPE |
+		UPF_SOFT_FLOW | UPF_HARD_FLOW;
+	up.port.private_data = priv;
+
+	up.port.regshift = 2;
+	up.port.fifosize = 64;

You don't need to set the fifosize unless you want to replace the
value you get from uart_config array.

+	up.port.set_termios = omap_8250_set_termios;
+	up.port.pm = omap_8250_pm;
+	up.port.startup = omap_8250_startup;
+	up.port.shutdown = omap_8250_shutdown;
+	up.port.throttle = omap_8250_throttle;
+	up.port.unthrottle = omap_8250_unthrottle;
+
+	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);
+		priv->wakeirq = irq_of_parse_and_map(pdev->dev.of_node, 1);
+	} 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);
+		return -ENODEV;
+	}
+	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);
+	}
+
+#ifdef CONFIG_PM_RUNTIME
+	up.capabilities |= UART_CAP_RPM;
+#endif

By setting this here, you will not get the capabilities from the
uart_config[type].flags if runtime PM is enabled in any case, right?

[cut]

So in this case I see no reason for new type.

Cheers,

-- 
heikki