[PATCH] tty/serial: Add support for Altera serial port

[PATCH] tty/serial: Add support for Altera serial port

[Ley Foon Tan]

Add support for Altera 8250/16550 compatible serial port.

Signed-off-by: Ley Foon Tan <lftan@altera.com>
---
 .../devicetree/bindings/tty/serial/of-serial.txt   |    3 ++
 drivers/tty/serial/8250/8250.c                     |   23 +++++++++++++++++++-
 drivers/tty/serial/of_serial.c                     |    6 +++++
 include/uapi/linux/serial_core.h                   |    5 +++-
 4 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/tty/serial/of-serial.txt b/Documentation/devicetree/bindings/tty/serial/of-serial.txt
index 1e1145c..8f01cb1 100644
--- a/Documentation/devicetree/bindings/tty/serial/of-serial.txt
+++ b/Documentation/devicetree/bindings/tty/serial/of-serial.txt
@@ -11,6 +11,9 @@ Required properties:
 	- "nvidia,tegra20-uart"
 	- "nxp,lpc3220-uart"
 	- "ibm,qpace-nwp-serial"
+	- "altr,16550-FIFO32"
+	- "altr,16550-FIFO64"
+	- "altr,16550-FIFO128"
 	- "serial" if the port type is unknown.
 - reg : offset and length of the register set for the device.
 - interrupts : should contain uart interrupt.
diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index 0efc815..661096d 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -301,7 +301,28 @@ static const struct serial8250_config uart_config[] = {
 	},
 	[PORT_8250_CIR] = {
 		.name		= "CIR port"
-	}
+	},
+	[PORT_ALTR_16550_F32] = {
+		.name		= "Altera 16550 FIFO32",
+		.fifo_size	= 32,
+		.tx_loadsz	= 32,
+		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
+		.flags		= UART_CAP_FIFO | UART_CAP_AFE,
+	},
+	[PORT_ALTR_16550_F64] = {
+		.name		= "Altera 16550 FIFO64",
+		.fifo_size	= 64,
+		.tx_loadsz	= 64,
+		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
+		.flags		= UART_CAP_FIFO | UART_CAP_AFE,
+	},
+	[PORT_ALTR_16550_F128] = {
+		.name		= "Altera 16550 FIFO128",
+		.fifo_size	= 128,
+		.tx_loadsz	= 128,
+		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
+		.flags		= UART_CAP_FIFO | UART_CAP_AFE,
+	},
 };
 
 /* Uart divisor latch read */
diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c
index d587460..b025d54 100644
--- a/drivers/tty/serial/of_serial.c
+++ b/drivers/tty/serial/of_serial.c
@@ -241,6 +241,12 @@ static struct of_device_id of_platform_serial_table[] = {
 	{ .compatible = "ns16850",  .data = (void *)PORT_16850, },
 	{ .compatible = "nvidia,tegra20-uart", .data = (void *)PORT_TEGRA, },
 	{ .compatible = "nxp,lpc3220-uart", .data = (void *)PORT_LPC3220, },
+	{ .compatible = "altr,16550-FIFO32",
+		.data = (void *)PORT_ALTR_16550_F32, },
+	{ .compatible = "altr,16550-FIFO64",
+		.data = (void *)PORT_ALTR_16550_F64, },
+	{ .compatible = "altr,16550-FIFO128",
+		.data = (void *)PORT_ALTR_16550_F128, },
 #ifdef CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL
 	{ .compatible = "ibm,qpace-nwp-serial",
 		.data = (void *)PORT_NWPSERIAL, },
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index b6a23a4..74c2bf7 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -51,7 +51,10 @@
 #define PORT_8250_CIR	23	/* CIR infrared port, has its own driver */
 #define PORT_XR17V35X	24	/* Exar XR17V35x UARTs */
 #define PORT_BRCM_TRUMANAGE	25
-#define PORT_MAX_8250	25	/* max port ID */
+#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 */
 
 /*
  * ARM specific type numbers.  These are not currently guaranteed
-- 
1.7.7.4

Re: [PATCH] tty/serial: Add support for Altera serial port

[Heikki Krogerus]

Hi,

On Thu, Mar 07, 2013 at 10:28:37AM +0800, Ley Foon Tan wrote:
> diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
> index 0efc815..661096d 100644
> --- a/drivers/tty/serial/8250/8250.c
> +++ b/drivers/tty/serial/8250/8250.c
> @@ -301,7 +301,28 @@ static const struct serial8250_config uart_config[] = {
>  	},
>  	[PORT_8250_CIR] = {
>  		.name		= "CIR port"
> -	}
> +	},
> +	[PORT_ALTR_16550_F32] = {
> +		.name		= "Altera 16550 FIFO32",
> +		.fifo_size	= 32,
> +		.tx_loadsz	= 32,
> +		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
> +		.flags		= UART_CAP_FIFO | UART_CAP_AFE,
> +	},
> +	[PORT_ALTR_16550_F64] = {
> +		.name		= "Altera 16550 FIFO64",
> +		.fifo_size	= 64,
> +		.tx_loadsz	= 64,
> +		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
> +		.flags		= UART_CAP_FIFO | UART_CAP_AFE,
> +	},
> +	[PORT_ALTR_16550_F128] = {
> +		.name		= "Altera 16550 FIFO128",
> +		.fifo_size	= 128,
> +		.tx_loadsz	= 128,
> +		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
> +		.flags		= UART_CAP_FIFO | UART_CAP_AFE,
> +	},

Adding three new types for what are basically the same UART, just with
different fifosizes, feels a bit silly to me, so in the future the
fifosize should be delivered as DT property. The following patches
fill uart_port.fifosize with the value from the property. I can see
the fifosize property is already used with some uarts, at least under
arch/arm/boot/dts/, but I guess not for of_serial.c driver.

Maybe there should also be DT property for auto flow control. It seems
to be quite common feature. Then in this case the type could just be
PORT_16550A.

-- 
heikki

[PATCH 1/2] serial: 8250: Allow probe drivers to ignore tx_loadsz

[Heikki Krogerus]

In most cases the tx_loadsz is the same as fifosize. This
will store the fifosize in it if it was not separately
delivered from the driver.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/tty/serial/8250/8250.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index cf6a538..19ebbdf 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -3247,6 +3247,10 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
 		uart->tx_loadsz		= up->tx_loadsz;
 		uart->capabilities	= up->capabilities;
 
+		/* Take tx_loadsz from fifosize if it wasn't set separately */
+		if (uart->port.fifosize && !uart->tx_loadsz)
+			uart->tx_loadsz = uart->port.fifosize;
+
 		if (up->port.dev)
 			uart->port.dev = up->port.dev;
 
-- 
1.7.10.4

[PATCH 2/2] serial: of_serial: Handle fifosize property

[Heikki Krogerus]

fifosize property is already used with a number of serial
devices. This should reduce the need for extra types in
8250.c just in case the fifosize differs from the standard.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 Documentation/devicetree/bindings/tty/serial/of-serial.txt |    1 +
 drivers/tty/serial/of_serial.c                             |    4 ++++
 2 files changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/tty/serial/of-serial.txt b/Documentation/devicetree/bindings/tty/serial/of-serial.txt
index 8f01cb1..55fe4e7 100644
--- a/Documentation/devicetree/bindings/tty/serial/of-serial.txt
+++ b/Documentation/devicetree/bindings/tty/serial/of-serial.txt
@@ -33,6 +33,7 @@ Optional properties:
   RTAS and should not be registered.
 - no-loopback-test: set to indicate that the port does not implements loopback
   test mode
+- fifosize: the fifo size of the UART.
 
 Example:
 
diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c
index b025d54..4f89761 100644
--- a/drivers/tty/serial/of_serial.c
+++ b/drivers/tty/serial/of_serial.c
@@ -97,6 +97,10 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
 	if (of_property_read_u32(np, "reg-shift", &prop) == 0)
 		port->regshift = prop;
 
+	/* Check for fifo size */
+	if (of_property_read_u32(np, "fifosize", &prop) == 0)
+		port->fifosize = prop;
+
 	port->irq = irq_of_parse_and_map(np, 0);
 	port->iotype = UPIO_MEM;
 	if (of_property_read_u32(np, "reg-io-width", &prop) == 0) {
-- 
1.7.10.4

Re: [PATCH 2/2] serial: of_serial: Handle fifosize property

[Ley Foon Tan]

On Thu, 2013-03-21 at 12:48 +0200, Heikki Krogerus wrote:
> fifosize property is already used with a number of serial
> devices. This should reduce the need for extra types in
> 8250.c just in case the fifosize differs from the standard.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  Documentation/devicetree/bindings/tty/serial/of-serial.txt |    1 +
>  drivers/tty/serial/of_serial.c                             |    4 ++++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/tty/serial/of-serial.txt b/Documentation/devicetree/bindings/tty/serial/of-serial.txt
> index 8f01cb1..55fe4e7 100644
> --- a/Documentation/devicetree/bindings/tty/serial/of-serial.txt
> +++ b/Documentation/devicetree/bindings/tty/serial/of-serial.txt
> @@ -33,6 +33,7 @@ Optional properties:
>    RTAS and should not be registered.
>  - no-loopback-test: set to indicate that the port does not implements loopback
>    test mode
> +- fifosize: the fifo size of the UART.
>  
>  Example:
>  
> diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c
> index b025d54..4f89761 100644
> --- a/drivers/tty/serial/of_serial.c
> +++ b/drivers/tty/serial/of_serial.c
> @@ -97,6 +97,10 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
>  	if (of_property_read_u32(np, "reg-shift", &prop) == 0)
>  		port->regshift = prop;
>  
> +	/* Check for fifo size */
> +	if (of_property_read_u32(np, "fifosize", &prop) == 0)
> +		port->fifosize = prop;
> +
Suggest to use "fifo-size" for the device tree property, to align with
other DT properties.

>  	port->irq = irq_of_parse_and_map(np, 0);
>  	port->iotype = UPIO_MEM;
>  	if (of_property_read_u32(np, "reg-io-width", &prop) == 0) {

I think you need to remove the UPF_FIXED_TYPE from port-flags as well to
use the fifo size from device tree. Otherwise, it will get from the
static array in 8250.c.

LFTan

Re: [PATCH 2/2] serial: of_serial: Handle fifosize property

[Heikki Krogerus]

Hi,

On Thu, Mar 21, 2013 at 07:41:39PM +0800, Ley Foon Tan wrote:
> On Thu, 2013-03-21 at 12:48 +0200, Heikki Krogerus wrote:
> > +	/* Check for fifo size */
> > +	if (of_property_read_u32(np, "fifosize", &prop) == 0)
> > +		port->fifosize = prop;
> > +
> Suggest to use "fifo-size" for the device tree property, to align with
> other DT properties.

I was going to, but then I noticed that in some .dtsi files "fifosize"
is used with uarts. Should I still change it?

> >  	port->irq = irq_of_parse_and_map(np, 0);
> >  	port->iotype = UPIO_MEM;
> >  	if (of_property_read_u32(np, "reg-io-width", &prop) == 0) {
> 
> I think you need to remove the UPF_FIXED_TYPE from port-flags as well to
> use the fifo size from device tree. Otherwise, it will get from the
> static array in 8250.c.

No, it's the other way around. It is picked from the array
conditionally, only in case it was not already set. However, if
UPF_FIXED_TYPE is removed then autoconfig() will override it.

Thanks,

-- 
heikki

Re: [PATCH 2/2] serial: of_serial: Handle fifosize property

[Ley Foon Tan]

On Thu, 2013-03-21 at 15:24 +0200, Heikki Krogerus wrote:
> Hi,
> 
> On Thu, Mar 21, 2013 at 07:41:39PM +0800, Ley Foon Tan wrote:
> > On Thu, 2013-03-21 at 12:48 +0200, Heikki Krogerus wrote:
> > > +	/* Check for fifo size */
> > > +	if (of_property_read_u32(np, "fifosize", &prop) == 0)
> > > +		port->fifosize = prop;
> > > +
> > Suggest to use "fifo-size" for the device tree property, to align with
> > other DT properties.
> 
> I was going to, but then I noticed that in some .dtsi files "fifosize"
> is used with uarts. Should I still change it?
I just make the grep for "fifosize" in arch/. It is used by other serial
drivers (not of_serial.c). So, you are safe to change it to "fifo-size".

> 
> > >  	port->irq = irq_of_parse_and_map(np, 0);
> > >  	port->iotype = UPIO_MEM;
> > >  	if (of_property_read_u32(np, "reg-io-width", &prop) == 0) {
> > 
> > I think you need to remove the UPF_FIXED_TYPE from port-flags as well to
> > use the fifo size from device tree. Otherwise, it will get from the
> > static array in 8250.c.
> 
> No, it's the other way around. It is picked from the array
> conditionally, only in case it was not already set. However, if
> UPF_FIXED_TYPE is removed then autoconfig() will override it.
> 
> Thanks,
> 
Okay, I got what you means now. I think someone updated the 8250.c
recently, previously it is always take from static array.
Thanks.

Re: [PATCH 2/2] serial: of_serial: Handle fifosize property

[Heikki Krogerus]

On Fri, Mar 22, 2013 at 11:32:18AM +0800, Ley Foon Tan wrote:
> On Thu, 2013-03-21 at 15:24 +0200, Heikki Krogerus wrote:
> > On Thu, Mar 21, 2013 at 07:41:39PM +0800, Ley Foon Tan wrote:
> > > Suggest to use "fifo-size" for the device tree property, to align with
> > > other DT properties.
> > 
> > I was going to, but then I noticed that in some .dtsi files "fifosize"
> > is used with uarts. Should I still change it?
> I just make the grep for "fifosize" in arch/. It is used by other serial
> drivers (not of_serial.c). So, you are safe to change it to "fifo-size".

OK, I'll change it.

Thanks!

-- 
heikki

[PATCHv2 2/2] serial: of_serial: Handle fifo-size property

[Heikki Krogerus]

This will reduce the need for extra types in 8250.c just
in case the fifo size differs from the standard.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 Documentation/devicetree/bindings/tty/serial/of-serial.txt |    1 +
 drivers/tty/serial/of_serial.c                             |    4 ++++
 2 files changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/tty/serial/of-serial.txt b/Documentation/devicetree/bindings/tty/serial/of-serial.txt
index 8f01cb1..c13f0ce 100644
--- a/Documentation/devicetree/bindings/tty/serial/of-serial.txt
+++ b/Documentation/devicetree/bindings/tty/serial/of-serial.txt
@@ -33,6 +33,7 @@ Optional properties:
   RTAS and should not be registered.
 - no-loopback-test: set to indicate that the port does not implements loopback
   test mode
+- fifo-size: the fifo size of the UART.
 
 Example:
 
diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c
index b025d54..267711b 100644
--- a/drivers/tty/serial/of_serial.c
+++ b/drivers/tty/serial/of_serial.c
@@ -97,6 +97,10 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
 	if (of_property_read_u32(np, "reg-shift", &prop) == 0)
 		port->regshift = prop;
 
+	/* Check for fifo size */
+	if (of_property_read_u32(np, "fifo-size", &prop) == 0)
+		port->fifosize = prop;
+
 	port->irq = irq_of_parse_and_map(np, 0);
 	port->iotype = UPIO_MEM;
 	if (of_property_read_u32(np, "reg-io-width", &prop) == 0) {
-- 
1.7.10.4

Re: [PATCHv2 2/2] serial: of_serial: Handle fifo-size property

[Ley Foon Tan]

On Fri, 2013-03-22 at 10:05 +0200, Heikki Krogerus wrote:
> This will reduce the need for extra types in 8250.c just
> in case the fifo size differs from the standard.
Besides the fifo size, we need to have hardware flow control setting
from device tree as well.
Thanks.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  Documentation/devicetree/bindings/tty/serial/of-serial.txt |    1 +
>  drivers/tty/serial/of_serial.c                             |    4 ++++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/tty/serial/of-serial.txt b/Documentation/devicetree/bindings/tty/serial/of-serial.txt
> index 8f01cb1..c13f0ce 100644
> --- a/Documentation/devicetree/bindings/tty/serial/of-serial.txt
> +++ b/Documentation/devicetree/bindings/tty/serial/of-serial.txt
> @@ -33,6 +33,7 @@ Optional properties:
>    RTAS and should not be registered.
>  - no-loopback-test: set to indicate that the port does not implements loopback
>    test mode
> +- fifo-size: the fifo size of the UART.
>  

Re: [PATCHv2 2/2] serial: of_serial: Handle fifo-size property

[Heikki Krogerus]

Hi,

On Fri, Mar 22, 2013 at 04:16:48PM +0800, Ley Foon Tan wrote:
> On Fri, 2013-03-22 at 10:05 +0200, Heikki Krogerus wrote:
> > This will reduce the need for extra types in 8250.c just
> > in case the fifo size differs from the standard.
> Besides the fifo size, we need to have hardware flow control setting
> from device tree as well.

This is a bit problematic. We can set the uart_8250_port.capabilities
flag UART_CAP_AFE based on "hw-flow-control" property, but then the
properties from the type will not be used, just like in case of
fifosize. We would need to know all the flags of the type that are
normally taken from the uart_config[] array.

The UART_CAP_FIFO can be set by checking if we have set the
port.fifosize or not, so if the bellow is acceptable then I can make a
patch. There is no risk of overriding the capabilities for the
existing uarts, as we only touch the uart_8250_port.capabilities if
the new properties are used. So is it OK?

diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c
index 267711b..cc5135d 100644
--- a/drivers/tty/serial/of_serial.c
+++ b/drivers/tty/serial/of_serial.c
@@ -171,11 +171,16 @@ static int of_platform_serial_probe(struct platform_device *ofdev)
 #ifdef CONFIG_SERIAL_8250
        case PORT_8250 ... PORT_MAX_8250:
        {
-               /* For now the of bindings don't support the extra
-                  8250 specific bits */
                struct uart_8250_port port8250;
                memset(&port8250, 0, sizeof(port8250));
                port8250.port = port;
+
+               if (port.fifosize)
+                       port8250.capabilities = UART_CAP_FIFO;
+
+               if (of_property_read_u32(np, "hw-flow-control", &prop) == 0)
+                       port8250.capabilities |= UART_CAP_AFE;
+
                ret = serial8250_register_8250_port(&port8250);
                break;
        }

-- 
heikki

Re: [PATCHv2 2/2] serial: of_serial: Handle fifo-size property

[Heikki Krogerus]

Hi,

On Fri, Mar 22, 2013 at 11:24:21AM +0200, Heikki Krogerus wrote:
> On Fri, Mar 22, 2013 at 04:16:48PM +0800, Ley Foon Tan wrote:
> > Besides the fifo size, we need to have hardware flow control setting
> > from device tree as well.
> 
> This is a bit problematic. We can set the uart_8250_port.capabilities
> flag UART_CAP_AFE based on "hw-flow-control" property, but then the
> properties from the type will not be used, just like in case of
> fifosize. We would need to know all the flags of the type that are
> normally taken from the uart_config[] array.
> 
> The UART_CAP_FIFO can be set by checking if we have set the
> port.fifosize or not, so if the bellow is acceptable then I can make a
> patch. There is no risk of overriding the capabilities for the
> existing uarts, as we only touch the uart_8250_port.capabilities if
> the new properties are used. So is it OK?

I'll resend these two patches and add the support for hardware flow
control.

-- 
heikki