[PATCH 0/4] Add TX FIFO Threshold for Altera 16550-FIFOxx

[PATCH 0/4] Add TX FIFO Threshold for Altera 16550-FIFOxx

[tthayer]

From: Thor Thayer <tthayer@opensource.altera.com>

Some variants of the 16550 have a programmable TX FIFO threshold that
will trigger an IRQ when the FIFO drops below the threshold. The Altera
16550 compatible soft IP supports programmable TX FIFO thresholds.

This patch series adds the tx_loadsz to the device tree. The tx_loadsz
variable is amount of data that can be loaded into the FIFO on an IRQ.

This patch series also adds support for the TX FIFO threshold registers
in the Altera 16550-FIFOxx soft IP.

Thor Thayer (4):
  Documentation: dt: serial: Add TX FIFO load size
  serial: 8250: of: Load TX FIFO Load Size from DT
  serial: 8250: Set Altera 16550 TX FIFO Threshold
  nios2: dts: 10m50: Add tx-loadsz parameter

 Documentation/devicetree/bindings/serial/8250.txt |    1 +
 arch/nios2/boot/dts/10m50_devboard.dts            |    1 +
 drivers/tty/serial/8250/8250_of.c                 |    6 ++++
 drivers/tty/serial/8250/8250_port.c               |   36 +++++++++++++++++++++
 include/uapi/linux/serial_reg.h                   |    8 +++++
 5 files changed, 52 insertions(+)

-- 
1.7.9.5

[PATCH 1/4] Documentation: dt: serial: Add TX FIFO load size

[tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx]

From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>

Add the device tree bindings needed to support the TX FIFO
load size.

Signed-off-by: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
---
 Documentation/devicetree/bindings/serial/8250.txt |    1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
index f5561ac..93de5ef 100644
--- a/Documentation/devicetree/bindings/serial/8250.txt
+++ b/Documentation/devicetree/bindings/serial/8250.txt
@@ -45,6 +45,7 @@ Optional properties:
 - {rts,cts,dtr,dsr,rng,dcd}-gpios: specify a GPIO for RTS/CTS/DTR/DSR/RI/DCD
   line respectively. It will use specified GPIO instead of the peripheral
   function pin for the UART feature. If unsure, don't specify this property.
+- tx-loadsz: the number of bytes that can be loaded into the TX FIFO.
 
 Note:
 * fsl,ns16550:
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] Documentation: dt: serial: Add TX FIFO load size

[Rob Herring]

On Thu, Sep 08, 2016 at 11:12:18AM -0500, tthayer@opensource.altera.com wrote:
> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> Add the device tree bindings needed to support the TX FIFO
> load size.
> 
> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> ---
>  Documentation/devicetree/bindings/serial/8250.txt |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
> index f5561ac..93de5ef 100644
> --- a/Documentation/devicetree/bindings/serial/8250.txt
> +++ b/Documentation/devicetree/bindings/serial/8250.txt
> @@ -45,6 +45,7 @@ Optional properties:
>  - {rts,cts,dtr,dsr,rng,dcd}-gpios: specify a GPIO for RTS/CTS/DTR/DSR/RI/DCD
>    line respectively. It will use specified GPIO instead of the peripheral
>    function pin for the UART feature. If unsure, don't specify this property.
> +- tx-loadsz: the number of bytes that can be loaded into the TX FIFO.

It seems you don't really need to know the loadsz, but just need the 
threshold trigger level. So just use that directly. And please check if 
any other UARTs have threshold properties. i.MX has programmable 
thresholds.

Rob

Re: [PATCH 1/4] Documentation: dt: serial: Add TX FIFO load size

[Thor Thayer]

Hi Rob,

On 09/16/2016 02:19 PM, Rob Herring wrote:
> On Thu, Sep 08, 2016 at 11:12:18AM -0500, tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org wrote:
>> From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
>>
>> Add the device tree bindings needed to support the TX FIFO
>> load size.
>>
>> Signed-off-by: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
>> ---
>>  Documentation/devicetree/bindings/serial/8250.txt |    1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
>> index f5561ac..93de5ef 100644
>> --- a/Documentation/devicetree/bindings/serial/8250.txt
>> +++ b/Documentation/devicetree/bindings/serial/8250.txt
>> @@ -45,6 +45,7 @@ Optional properties:
>>  - {rts,cts,dtr,dsr,rng,dcd}-gpios: specify a GPIO for RTS/CTS/DTR/DSR/RI/DCD
>>    line respectively. It will use specified GPIO instead of the peripheral
>>    function pin for the UART feature. If unsure, don't specify this property.
>> +- tx-loadsz: the number of bytes that can be loaded into the TX FIFO.
>
> It seems you don't really need to know the loadsz, but just need the
> threshold trigger level. So just use that directly. And please check if
> any other UARTs have threshold properties. i.MX has programmable
> thresholds.
>
> Rob
>

Yes, it is correct that I only really need the loadsz, but there is a 
tx-loadsz parameter in the uart_8250_port structure. Since there 
currently isn't a way to update the default value of tx-loadsz from 
device tree, I thought this would be useful for other 8250 UARTs as well.

Yes, there are a couple of 8250 compatible UARTs that have programmable 
thresholds. The SC16654 and SC16C650 are 2 examples. However, I didn't 
see the programmable TX threshold implemented.

I looked at the i.MX UARTs but I don't see a programmable threshold 
parameter in the bindings( 
Documentation/devicetree/bindings/serial/fsl-imx-uart.txt).

I'm assuming you're saying that I should use the same binding names for 
this transmit threshold if they are available?

Thank you for reviewing and for the comments.

Thor


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] serial: 8250: of: Load TX FIFO Load Size from DT

[tthayer]

From: Thor Thayer <tthayer@opensource.altera.com>

Initialize the tx_loadsz parameter if it is defined in the
device tree.

Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
---
 drivers/tty/serial/8250/8250_of.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
index 38963d7..3e4be2f 100644
--- a/drivers/tty/serial/8250/8250_of.c
+++ b/drivers/tty/serial/8250/8250_of.c
@@ -195,6 +195,7 @@ static int of_platform_serial_probe(struct platform_device *ofdev)
 	switch (port_type) {
 	case PORT_8250 ... PORT_MAX_8250:
 	{
+		u32 prop;
 		struct uart_8250_port port8250;
 		memset(&port8250, 0, sizeof(port8250));
 		port8250.port = port;
@@ -202,6 +203,11 @@ static int of_platform_serial_probe(struct platform_device *ofdev)
 		if (port.fifosize)
 			port8250.capabilities = UART_CAP_FIFO;
 
+		/* Check for TX fifo load size */
+		if (of_property_read_u32(ofdev->dev.of_node,
+					 "tx-loadsz", &prop) == 0)
+			port8250.tx_loadsz = prop;
+
 		if (of_property_read_bool(ofdev->dev.of_node,
 					  "auto-flow-control"))
 			port8250.capabilities |= UART_CAP_AFE;
-- 
1.7.9.5

Re: [PATCH 2/4] serial: 8250: of: Load TX FIFO Load Size from DT

[Rob Herring]

On Thu, Sep 08, 2016 at 11:12:19AM -0500, tthayer@opensource.altera.com wrote:
> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> Initialize the tx_loadsz parameter if it is defined in the
> device tree.
> 
> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> ---
>  drivers/tty/serial/8250/8250_of.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
> index 38963d7..3e4be2f 100644
> --- a/drivers/tty/serial/8250/8250_of.c
> +++ b/drivers/tty/serial/8250/8250_of.c
> @@ -195,6 +195,7 @@ static int of_platform_serial_probe(struct platform_device *ofdev)
>  	switch (port_type) {
>  	case PORT_8250 ... PORT_MAX_8250:
>  	{
> +		u32 prop;
>  		struct uart_8250_port port8250;
>  		memset(&port8250, 0, sizeof(port8250));
>  		port8250.port = port;
> @@ -202,6 +203,11 @@ static int of_platform_serial_probe(struct platform_device *ofdev)
>  		if (port.fifosize)
>  			port8250.capabilities = UART_CAP_FIFO;
>  
> +		/* Check for TX fifo load size */
> +		if (of_property_read_u32(ofdev->dev.of_node,
> +					 "tx-loadsz", &prop) == 0)
> +			port8250.tx_loadsz = prop;

This can be simplified to:

of_property_read_u32(ofdev->dev.of_node, "tx-loadsz", &port8250.tx_loadsz);


> +
>  		if (of_property_read_bool(ofdev->dev.of_node,
>  					  "auto-flow-control"))
>  			port8250.capabilities |= UART_CAP_AFE;
> -- 
> 1.7.9.5
> 

Re: [PATCH 2/4] serial: 8250: of: Load TX FIFO Load Size from DT

[Thor Thayer]

Hi Rob,

On 09/16/2016 02:20 PM, Rob Herring wrote:
> On Thu, Sep 08, 2016 at 11:12:19AM -0500, tthayer@opensource.altera.com wrote:
>> From: Thor Thayer <tthayer@opensource.altera.com>
>>
>> Initialize the tx_loadsz parameter if it is defined in the
>> device tree.
>>
>> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
>> ---
>>  drivers/tty/serial/8250/8250_of.c |    6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
>> index 38963d7..3e4be2f 100644
>> --- a/drivers/tty/serial/8250/8250_of.c
>> +++ b/drivers/tty/serial/8250/8250_of.c
>> @@ -195,6 +195,7 @@ static int of_platform_serial_probe(struct platform_device *ofdev)
>>  	switch (port_type) {
>>  	case PORT_8250 ... PORT_MAX_8250:
>>  	{
>> +		u32 prop;
>>  		struct uart_8250_port port8250;
>>  		memset(&port8250, 0, sizeof(port8250));
>>  		port8250.port = port;
>> @@ -202,6 +203,11 @@ static int of_platform_serial_probe(struct platform_device *ofdev)
>>  		if (port.fifosize)
>>  			port8250.capabilities = UART_CAP_FIFO;
>>
>> +		/* Check for TX fifo load size */
>> +		if (of_property_read_u32(ofdev->dev.of_node,
>> +					 "tx-loadsz", &prop) == 0)
>> +			port8250.tx_loadsz = prop;
>
> This can be simplified to:
>
> of_property_read_u32(ofdev->dev.of_node, "tx-loadsz", &port8250.tx_loadsz);
>
>
Yes, I originally had it implemented using that function but then I 
realized the port8250.tx_loadsz is an unsigned int and therefore can 
change size on different platforms.

The assignment handles that. In retrospect, I should probably change 
this to a of_property_read_u8() anyway so that it won't cause compile 
warnings in the smaller architectures.

Thank you for reviewing and the comments.

Thor

>> +
>>  		if (of_property_read_bool(ofdev->dev.of_node,
>>  					  "auto-flow-control"))
>>  			port8250.capabilities |= UART_CAP_AFE;
>> --
>> 1.7.9.5
>>

Re: [PATCH 2/4] serial: 8250: of: Load TX FIFO Load Size from DT

[Arnd Bergmann]

On Friday, September 16, 2016 3:52:55 PM CEST Thor Thayer wrote:
> >
> >
> Yes, I originally had it implemented using that function but then I 
> realized the port8250.tx_loadsz is an unsigned int and therefore can 
> change size on different platforms.
> 
> The assignment handles that. In retrospect, I should probably change 
> this to a of_property_read_u8() anyway so that it won't cause compile 
> warnings in the smaller architectures.
> 
> Thank you for reviewing and the comments.

We don't support architectures with 'int' types other than 32 bits,
so the version that Rob suggested is guaranteed to work everywhere.

Using u8 properties seems like a bad idea in general, better stay
with 32-bit units for consistency.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/4] serial: 8250: Set Altera 16550 TX FIFO Threshold

[tthayer]

From: Thor Thayer <tthayer@opensource.altera.com>

The Altera 16550 soft IP UART requires 2 additional registers for
TX FIFO threshold support. These 2 registers enable the TX FIFO
Low Watermark and set the TX FIFO Low Watermark.
These registers are initialized in serial8350_do_startup(). Set
the TX FIFO threshold to the FIFO size - tx_loadsz.

Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
---
 drivers/tty/serial/8250/8250_port.c |   36 +++++++++++++++++++++++++++++++++++
 include/uapi/linux/serial_reg.h     |    8 ++++++++
 2 files changed, 44 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 7481b95..35a3e2c 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1880,6 +1880,30 @@ static int exar_handle_irq(struct uart_port *port)
 	return ret;
 }
 
+/*
+ * Newer 16550 compatible parts such as the SC16C650 & Altera 16550 Soft IP
+ * have a programmable TX threshold that triggers the THRE interrupt in
+ * the IIR register. In this case, the THRE interrupt indicates the FIFO
+ * has space available. Load it up with tx_loadsz bytes.
+ */
+static int serial8250_tx_threshold_handle_irq(struct uart_port *port)
+{
+	unsigned long flags;
+	unsigned int iir = serial_port_in(port, UART_IIR);
+
+	/* TX Threshold IRQ triggered so load up FIFO */
+	if ((iir & UART_IIR_ID) == UART_IIR_THRI) {
+		struct uart_8250_port *up = up_to_u8250p(port);
+
+		spin_lock_irqsave(&port->lock, flags);
+		serial8250_tx_chars(up);
+		spin_unlock_irqrestore(&port->lock, flags);
+	}
+
+	iir = serial_port_in(port, UART_IIR);
+	return serial8250_handle_irq(port, iir);
+}
+
 static unsigned int serial8250_tx_empty(struct uart_port *port)
 {
 	struct uart_8250_port *up = up_to_u8250p(port);
@@ -2169,6 +2193,18 @@ int serial8250_do_startup(struct uart_port *port)
 		serial_port_out(port, UART_LCR, 0);
 	}
 
+	/*
+	 * For the Altera 16550 variants, set TX threshold trigger level.
+	 */
+	if (((port->type == PORT_ALTR_16550_F32) ||
+	     (port->type == PORT_ALTR_16550_F64) ||
+	     (port->type == PORT_ALTR_16550_F128)) && (port->fifosize > 1)) {
+		serial_port_out(port, UART_ALTR_AFR, UART_ALTR_EN_TXFIFO_LW);
+		serial_port_out(port, UART_ALTR_TX_LOW,
+				port->fifosize - up->tx_loadsz);
+		port->handle_irq = serial8250_tx_threshold_handle_irq;
+	}
+
 	if (port->irq) {
 		unsigned char iir1;
 		/*
diff --git a/include/uapi/linux/serial_reg.h b/include/uapi/linux/serial_reg.h
index 1e5ac4e7..b4c0484 100644
--- a/include/uapi/linux/serial_reg.h
+++ b/include/uapi/linux/serial_reg.h
@@ -376,5 +376,13 @@
 #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 Altera ALTR_16550_F32/F64/F128
+ * Normalized from 0x100 to 0x40 because of shift by 2 (32 bit regs).
+ */
+#define UART_ALTR_AFR		0x40	/* Additional Features Register */
+#define UART_ALTR_EN_TXFIFO_LW	0x01	/* Enable the TX FIFO Low Watermark */
+#define UART_ALTR_TX_LOW	0x41	/* Tx FIFO Low Watermark */
+
 #endif /* _LINUX_SERIAL_REG_H */
 
-- 
1.7.9.5

[PATCH 4/4] nios2: dts: 10m50: Add tx-loadsz parameter

[tthayer]

From: Thor Thayer <tthayer@opensource.altera.com>

The tx-loadsz parameter sets the TX FIFO threshold level of
the Altera 16550-FIFO32 soft IP.

Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
---
 arch/nios2/boot/dts/10m50_devboard.dts |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/nios2/boot/dts/10m50_devboard.dts b/arch/nios2/boot/dts/10m50_devboard.dts
index 3e411c6..6be1502 100755
--- a/arch/nios2/boot/dts/10m50_devboard.dts
+++ b/arch/nios2/boot/dts/10m50_devboard.dts
@@ -83,6 +83,7 @@
 			fifo-size = <32>;
 			reg-io-width = <4>;
 			reg-shift = <2>;
+			tx-loadsz = <16>;
 		};
 
 		sysid: sysid@18001528 {
-- 
1.7.9.5