2013-04-15 05:45:29

by Jongsung Kim

[permalink] [raw]
Subject: [RESEND][PATCH] ARM: PL011: add support for extended FIFO-size of PL011-r1p5

The latest r1p5-revision of the ARM PL011 UART has 32-byte FIFOs,
while all earlier ones have 16-byte FIFOs. This patch suggests
a way to set the FIFO-size correctly & flexibly by using a member
function named get_fifosize, rather than using the fifosize member
variable. The function takes the UARTPeriphID, and returns the
correct FIFO size.

Signed-off-by: Jongsung Kim <[email protected]>
---
drivers/tty/serial/amba-pl011.c | 20 ++++++++++++++++----
1 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 3ea5408..22af0c8 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -72,32 +72,44 @@
/* There is by now at least one vendor with differing details, so handle it */
struct vendor_data {
unsigned int ifls;
- unsigned int fifosize;
unsigned int lcrh_tx;
unsigned int lcrh_rx;
bool oversampling;
bool dma_threshold;
bool cts_event_workaround;
+
+ unsigned int (*get_fifosize)(unsigned int periphid);
};

+static unsigned int get_fifosize_arm(unsigned int periphid)
+{
+ unsigned int rev = (periphid >> 20) & 0xf;
+ return rev < 3 ? 16 : 32;
+}
+
static struct vendor_data vendor_arm = {
.ifls = UART011_IFLS_RX4_8|UART011_IFLS_TX4_8,
- .fifosize = 16,
.lcrh_tx = UART011_LCRH,
.lcrh_rx = UART011_LCRH,
.oversampling = false,
.dma_threshold = false,
.cts_event_workaround = false,
+ .get_fifosize = get_fifosize_arm,
};

+static unsigned int get_fifosize_st(unsigned int periphid)
+{
+ return 64;
+}
+
static struct vendor_data vendor_st = {
.ifls = UART011_IFLS_RX_HALF|UART011_IFLS_TX_HALF,
- .fifosize = 64,
.lcrh_tx = ST_UART011_LCRH_TX,
.lcrh_rx = ST_UART011_LCRH_RX,
.oversampling = true,
.dma_threshold = true,
.cts_event_workaround = true,
+ .get_fifosize = get_fifosize_st,
};

static struct uart_amba_port *amba_ports[UART_NR];
@@ -2010,7 +2022,7 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
uap->lcrh_rx = vendor->lcrh_rx;
uap->lcrh_tx = vendor->lcrh_tx;
uap->old_cr = 0;
- uap->fifosize = vendor->fifosize;
+ uap->fifosize = vendor->get_fifosize(dev->periphid);
uap->port.dev = &dev->dev;
uap->port.mapbase = dev->res.start;
uap->port.membase = base;


2013-04-19 14:18:42

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [RESEND][PATCH] ARM: PL011: add support for extended FIFO-size of PL011-r1p5

On Mon, Apr 15, 2013 at 02:45:25PM +0900, Jongsung Kim wrote:
> The latest r1p5-revision of the ARM PL011 UART has 32-byte FIFOs,
> while all earlier ones have 16-byte FIFOs. This patch suggests
> a way to set the FIFO-size correctly & flexibly by using a member
> function named get_fifosize, rather than using the fifosize member
> variable. The function takes the UARTPeriphID, and returns the
> correct FIFO size.

Same comments as previous version.

2013-04-20 13:31:41

by Linus Walleij

[permalink] [raw]
Subject: Re: [RESEND][PATCH] ARM: PL011: add support for extended FIFO-size of PL011-r1p5

On Mon, Apr 15, 2013 at 7:45 AM, Jongsung Kim <[email protected]> wrote:

At the risk to re-stating what Russell has already said...

> +
> + unsigned int (*get_fifosize)(unsigned int periphid);
> };
>
> +static unsigned int get_fifosize_arm(unsigned int periphid)
> +{
> + unsigned int rev = (periphid >> 20) & 0xf;
> + return rev < 3 ? 16 : 32;
> +}

This is a very complicated way to to something very simple.

Keep the .fifosize as part of struct vendor_data, define a
new vendor data cunk for your variant, use .id and .mask
to select your variant.

static struct vendor_data vendor_arm_deepfifo = {
.fifosize = 32,
(... the rest is the same)
}

(...)

static struct amba_id pl011_ids[] = {
{
.id = 0x00341011,
.mask = 0x00ffffff,
.data = &vendor_arm_deepfifo,
},
(...)
};

As you can see in amba_lookup() in
drivers/amba/bus.c the table is traversed from the top,
so for the new "3" variant this entry will match, while the
next entry will match all older versions.

Yours,
Linus Walleij

2013-04-21 08:55:39

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [RESEND][PATCH] ARM: PL011: add support for extended FIFO-size of PL011-r1p5

On Sat, Apr 20, 2013 at 03:31:39PM +0200, Linus Walleij wrote:
> static struct amba_id pl011_ids[] = {
> {
> .id = 0x00341011,
> .mask = 0x00ffffff,
> .data = &vendor_arm_deepfifo,
> },
> (...)
> };
>
> As you can see in amba_lookup() in
> drivers/amba/bus.c the table is traversed from the top,
> so for the new "3" variant this entry will match, while the
> next entry will match all older versions.

This isn't equivalent - this will only match variant 3, not any later
ones. The original patch uses the deep fifo for variant 3 or larger.

2013-04-22 02:24:27

by Jongsung Kim

[permalink] [raw]
Subject: RE: [RESEND][PATCH] ARM: PL011: add support for extended FIFO-size of PL011-r1p5

Thank you for your comments, Russell. I'll happily apply your recommendation
just after it come up to the merge window. Or, do you want me to send it by now?

-----Original Message-----
From: Russell King - ARM Linux [mailto:[email protected]]
Sent: Friday, April 19, 2013 11:18 PM
To: Jongsung Kim
Cc: [email protected]; [email protected]; [email protected];
[email protected]
Subject: Re: [RESEND][PATCH] ARM: PL011: add support for extended FIFO-size of
PL011-r1p5

On Mon, Apr 15, 2013 at 02:45:25PM +0900, Jongsung Kim wrote:
> The latest r1p5-revision of the ARM PL011 UART has 32-byte FIFOs,
> while all earlier ones have 16-byte FIFOs. This patch suggests
> a way to set the FIFO-size correctly & flexibly by using a member
> function named get_fifosize, rather than using the fifosize member
> variable. The function takes the UARTPeriphID, and returns the
> correct FIFO size.

Same comments as previous version.

2013-04-22 17:37:03

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [RESEND][PATCH] ARM: PL011: add support for extended FIFO-size of PL011-r1p5

On Mon, Apr 22, 2013 at 11:24:18AM +0900, Jongsung Kim wrote:
> Thank you for your comments, Russell. I'll happily apply your recommendation
> just after it come up to the merge window. Or, do you want me to send it by
> now?

Sorry, don't understand what you're asking, could you rephrase please?