Skip to contentSkip to conversation

serial: pl01x: Add support for devices with the rate pre-configured.

PATCH Superseded Eric Anholt 5 participants 9 messages Feb 5, 2016 – Mar 8, 2016 Broadcom BCM2711/BCM2835 Arm architecture

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Eric Anholt

author
#0 lore raw

PATCH 1/1 serial: pl01x: Add support for devices with the rate pre-configured.

For Raspberry Pi, we had the input clock rate to the pl011 fixed in the rpi.c file, but it may be changed by firmware due to user changes to config.txt. Since the firmware always sets up the uart (default 115200 output unless the user changes it), we can just skip our own uart init to simplify the boot process and more reliably get serial output.

Signed-off-by Eric Anholt <[email protected]>
---
 board/raspberrypi/rpi/rpi.c             | 3 +--
 drivers/serial/serial_pl01x.c           | 4 ++++
 include/dm/platform_data/serial_pl01x.h | 1 +
 3 files changed, 6 insertions(+), 2 deletions(-)
board/raspberrypi/rpi/rpi.c +1 -2
index 4b80d7b..1fd3f97 100644
@@ -35,8 +35,7 @@ static const struct pl01x_serial_platdata serial_platdata = {
 #else
 	.base = 0x20201000,
 #endif
-	.type = TYPE_PL011,
-	.clock = 3000000,
+	.type = TYPE_PL01X_PRECONFIGURED,
 };
 
 U_BOOT_DEVICE(bcm2835_serials) = {
drivers/serial/serial_pl01x.c +4 -0
index 552c945..7d4d214 100644
@@ -83,6 +83,8 @@ static int pl01x_generic_serial_init(struct pl01x_regs *regs,
 		/* disable everything */
 		writel(0, &regs->pl011_cr);
 		break;
+	case TYPE_PL01X_PRECONFIGURED:
+		break;
 	default:
 		return -EINVAL;
 	}

@@ -172,6 +174,8 @@ static int pl01x_generic_setbrg(struct pl01x_regs *regs, enum pl01x_type type,
 		writel(UART_PL011_CR_UARTEN | UART_PL011_CR_TXE |
 		       UART_PL011_CR_RXE | UART_PL011_CR_RTS, &regs->pl011_cr);
 		break;
+	case TYPE_PL01X_PRECONFIGURED:
+		break;
 	}
 	default:
 		return -EINVAL;
include/dm/platform_data/serial_pl01x.h +1 -0
index 5e068f3..ae1bf31 100644
@@ -9,6 +9,7 @@
 enum pl01x_type {
 	TYPE_PL010,
 	TYPE_PL011,
+	TYPE_PL01X_PRECONFIGURED,
 };
 
 /*
Signature · 1 line
2.7.0
#1 lore raw

PATCH 1/1 serial: pl01x: Add support for devices with the rate pre-configured.

Quoted 6 lines · Eric Anholt

For Raspberry Pi, we had the input clock rate to the pl011 fixed in the rpi.c file, but it may be changed by firmware due to user changes to config.txt. Since the firmware always sets up the uart (default 115200 output unless the user changes it), we can just skip our own uart init to simplify the boot process and more reliably get serial output.

Tested-by Stephen Warren <[email protected]>

(With a 6MHz init_uart_clock and with/without init_uart_baud=9600)

#2 lore raw

PATCH 1/1 serial: pl01x: Add support for devices with the rate pre-configured.

Quoted 10 lines · Stephen Warren
Quoted 6 lines · Eric Anholt

For Raspberry Pi, we had the input clock rate to the pl011 fixed in the rpi.c file, but it may be changed by firmware due to user changes to config.txt. Since the firmware always sets up the uart (default 115200 output unless the user changes it), we can just skip our own uart init to simplify the boot process and more reliably get serial output.

Tested-by Stephen Warren <[email protected]>

(With a 6MHz init_uart_clock and with/without init_uart_baud=9600)

Simon/Tom, did this slip through the cracks, or are you deferring it until the next release?

#3 lore raw

PATCH 1/1 serial: pl01x: Add support for devices with the rate pre-configured.

Hi Stephen,

Quoted 13 lines · Stephen Warren
Quoted 11 lines · Stephen Warren
Quoted 7 lines · Eric Anholt

For Raspberry Pi, we had the input clock rate to the pl011 fixed in the rpi.c file, but it may be changed by firmware due to user changes to config.txt. Since the firmware always sets up the uart (default 115200 output unless the user changes it), we can just skip our own uart init to simplify the boot process and more reliably get serial output.

Quoted 5 lines
Quoted 2 lines
Tested-by Stephen Warren <[email protected]>

(With a 6MHz init_uart_clock and with/without init_uart_baud=9600)

Simon/Tom, did this slip through the cracks, or are you deferring it until the next release?

We haven't heard back on the review comments.

I don't really mind if we go the DT approach or this one. But this patch needs a little rework I think.

Regards,
Simon

#4 lore raw

PATCH 1/1 serial: pl01x: Add support for devices with the rate pre-configured.

Quoted 20 lines · Simon Glass

Hi Stephen,

Quoted 17 lines · Stephen Warren
Quoted 13 lines · Stephen Warren
Quoted 7 lines · Eric Anholt

For Raspberry Pi, we had the input clock rate to the pl011 fixed in the rpi.c file, but it may be changed by firmware due to user changes to config.txt. Since the firmware always sets up the uart (default 115200 output unless the user changes it), we can just skip our own uart init to simplify the boot process and more reliably get serial output.

Tested-by Stephen Warren <[email protected]>

(With a 6MHz init_uart_clock and with/without init_uart_baud=9600)

Quoted 6 lines
Quoted 1 line

Simon/Tom, did this slip through the cracks, or are you deferring it until the next release?

We haven't heard back on the review comments.

I don't really mind if we go the DT approach or this one. But this patch needs a little rework I think.

I thought the main review comment was "didn't we try and do this with DT?" ? I'm feeling like we might want to skip doing anything DT related for this part...

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160307/0a942334/attachment.sig>
#5 lore raw

PATCH 1/1 serial: pl01x: Add support for devices with the rate pre-configured.

Hi Tom,

Quoted 23 lines · Tom Rini
Quoted 22 lines · Simon Glass

Hi Stephen,

Quoted 18 lines · Stephen Warren
Quoted 11 lines · Stephen Warren
Quoted 7 lines · Eric Anholt

For Raspberry Pi, we had the input clock rate to the pl011 fixed in the rpi.c file, but it may be changed by firmware due to user changes to config.txt. Since the firmware always sets up the uart (default 115200 output unless the user changes it), we can just skip our own uart init to simplify the boot process and more reliably get serial output.

Quoted 2 lines
Tested-by Stephen Warren <[email protected]>

(With a 6MHz init_uart_clock and with/without init_uart_baud=9600)

Simon/Tom, did this slip through the cracks, or are you deferring it until the next release?

Quoted 8 lines
Quoted 4 lines

We haven't heard back on the review comments.

I don't really mind if we go the DT approach or this one. But this patch needs a little rework I think.

I thought the main review comment was "didn't we try and do this with DT?" ? I'm feeling like we might want to skip doing anything DT related for this part...

My comment was on 6 Feb:

Quoted 1 line

+ TYPE_PL01X_PRECONFIGURED,

This seems odd. You are not adding a new UART type.

How about a separate bool flag?

Regards,
Simon

#6 lore raw

PATCH 1/1 serial: pl01x: Add support for devices with the rate pre-configured.

In reply to Eric Anholt

Hi Eric,

Quoted 55 lines · Eric Anholt

For Raspberry Pi, we had the input clock rate to the pl011 fixed in the rpi.c file, but it may be changed by firmware due to user changes to config.txt. Since the firmware always sets up the uart (default 115200 output unless the user changes it), we can just skip our own uart init to simplify the boot process and more reliably get serial output.

Signed-off-by Eric Anholt <[email protected]>
---
 board/raspberrypi/rpi/rpi.c             | 3 +--
 drivers/serial/serial_pl01x.c           | 4 ++++
 include/dm/platform_data/serial_pl01x.h | 1 +
 3 files changed, 6 insertions(+), 2 deletions(-)
board/raspberrypi/rpi/rpi.c +1 -2
index 4b80d7b..1fd3f97 100644
@@ -35,8 +35,7 @@ static const struct pl01x_serial_platdata serial_platdata = {
 #else
        .base = 0x20201000,
 #endif
-       .type = TYPE_PL011,
-       .clock = 3000000,
+       .type = TYPE_PL01X_PRECONFIGURED,
 };
 
 U_BOOT_DEVICE(bcm2835_serials) = {
drivers/serial/serial_pl01x.c +4 -0
index 552c945..7d4d214 100644
@@ -83,6 +83,8 @@ static int pl01x_generic_serial_init(struct pl01x_regs *regs,
                /* disable everything */
                writel(0, &regs->pl011_cr);
                break;
+       case TYPE_PL01X_PRECONFIGURED:
+               break;
        default:
                return -EINVAL;
        }

@@ -172,6 +174,8 @@ static int pl01x_generic_setbrg(struct pl01x_regs *regs, enum pl01x_type type,
                writel(UART_PL011_CR_UARTEN | UART_PL011_CR_TXE |
                       UART_PL011_CR_RXE | UART_PL011_CR_RTS, &regs->pl011_cr);
                break;
+       case TYPE_PL01X_PRECONFIGURED:
+               break;
        }
        default:
                return -EINVAL;
include/dm/platform_data/serial_pl01x.h +0 -0
index 5e068f3..ae1bf31 100644

include/dm/platform_data/serial_pl01x.h line 12

Quoted 4 lines
include/dm/platform_data/serial_pl01x.h +1 -0
@@ -9,6 +9,7 @@
 enum pl01x_type {
        TYPE_PL010,
        TYPE_PL011,
+       TYPE_PL01X_PRECONFIGURED,

This seems odd. You are not adding a new UART type.

How about a separate bool flag?

Regards,
Simon

#7 lore raw

PATCH 1/1 serial: pl01x: Add support for devices with the rate pre-configured.

Quoted 65 lines · Simon Glass

Hi Eric,

Quoted 55 lines · Eric Anholt

For Raspberry Pi, we had the input clock rate to the pl011 fixed in the rpi.c file, but it may be changed by firmware due to user changes to config.txt. Since the firmware always sets up the uart (default 115200 output unless the user changes it), we can just skip our own uart init to simplify the boot process and more reliably get serial output.

Signed-off-by Eric Anholt <[email protected]>
---
 board/raspberrypi/rpi/rpi.c             | 3 +--
 drivers/serial/serial_pl01x.c           | 4 ++++
 include/dm/platform_data/serial_pl01x.h | 1 +
 3 files changed, 6 insertions(+), 2 deletions(-)
board/raspberrypi/rpi/rpi.c +1 -2
index 4b80d7b..1fd3f97 100644
@@ -35,8 +35,7 @@ static const struct pl01x_serial_platdata serial_platdata = {
 #else
        .base = 0x20201000,
 #endif
-       .type = TYPE_PL011,
-       .clock = 3000000,
+       .type = TYPE_PL01X_PRECONFIGURED,
 };
 
 U_BOOT_DEVICE(bcm2835_serials) = {
drivers/serial/serial_pl01x.c +4 -0
index 552c945..7d4d214 100644
@@ -83,6 +83,8 @@ static int pl01x_generic_serial_init(struct pl01x_regs *regs,
                /* disable everything */
                writel(0, &regs->pl011_cr);
                break;
+       case TYPE_PL01X_PRECONFIGURED:
+               break;
        default:
                return -EINVAL;
        }

@@ -172,6 +174,8 @@ static int pl01x_generic_setbrg(struct pl01x_regs *regs, enum pl01x_type type,
                writel(UART_PL011_CR_UARTEN | UART_PL011_CR_TXE |
                       UART_PL011_CR_RXE | UART_PL011_CR_RTS, &regs->pl011_cr);
                break;
+       case TYPE_PL01X_PRECONFIGURED:
+               break;
        }
        default:
                return -EINVAL;
include/dm/platform_data/serial_pl01x.h +0 -0
index 5e068f3..ae1bf31 100644

include/dm/platform_data/serial_pl01x.h line 12

Quoted 4 lines
include/dm/platform_data/serial_pl01x.h +1 -0
@@ -9,6 +9,7 @@
 enum pl01x_type {
        TYPE_PL010,
        TYPE_PL011,
+       TYPE_PL01X_PRECONFIGURED,

This seems odd. You are not adding a new UART type.

Quoted 1 line

How about a separate bool flag?

I posted a patch with the same purpose a while ago
http://lists.denx.de/pipermail/u-boot/2015-October/230079.html

Regards,
Sergey

Quoted 7 lines

Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#8 lore raw

PATCH 1/1 serial: pl01x: Add support for devices with the rate pre-configured.

Quoted 66 lines · Sergei Temerkhanov
Quoted 65 lines · Simon Glass

Hi Eric,

Quoted 55 lines · Eric Anholt

For Raspberry Pi, we had the input clock rate to the pl011 fixed in the rpi.c file, but it may be changed by firmware due to user changes to config.txt. Since the firmware always sets up the uart (default 115200 output unless the user changes it), we can just skip our own uart init to simplify the boot process and more reliably get serial output.

Signed-off-by Eric Anholt <[email protected]>
---
 board/raspberrypi/rpi/rpi.c             | 3 +--
 drivers/serial/serial_pl01x.c           | 4 ++++
 include/dm/platform_data/serial_pl01x.h | 1 +
 3 files changed, 6 insertions(+), 2 deletions(-)
board/raspberrypi/rpi/rpi.c +1 -2
index 4b80d7b..1fd3f97 100644
@@ -35,8 +35,7 @@ static const struct pl01x_serial_platdata serial_platdata = {
 #else
        .base = 0x20201000,
 #endif
-       .type = TYPE_PL011,
-       .clock = 3000000,
+       .type = TYPE_PL01X_PRECONFIGURED,
 };
 
 U_BOOT_DEVICE(bcm2835_serials) = {
drivers/serial/serial_pl01x.c +4 -0
index 552c945..7d4d214 100644
@@ -83,6 +83,8 @@ static int pl01x_generic_serial_init(struct pl01x_regs *regs,
                /* disable everything */
                writel(0, &regs->pl011_cr);
                break;
+       case TYPE_PL01X_PRECONFIGURED:
+               break;
        default:
                return -EINVAL;
        }

@@ -172,6 +174,8 @@ static int pl01x_generic_setbrg(struct pl01x_regs *regs, enum pl01x_type type,
                writel(UART_PL011_CR_UARTEN | UART_PL011_CR_TXE |
                       UART_PL011_CR_RXE | UART_PL011_CR_RTS, &regs->pl011_cr);
                break;
+       case TYPE_PL01X_PRECONFIGURED:
+               break;
        }
        default:
                return -EINVAL;
include/dm/platform_data/serial_pl01x.h +0 -0
index 5e068f3..ae1bf31 100644

include/dm/platform_data/serial_pl01x.h line 12

Quoted 4 lines
include/dm/platform_data/serial_pl01x.h +1 -0
@@ -9,6 +9,7 @@
 enum pl01x_type {
        TYPE_PL010,
        TYPE_PL011,
+       TYPE_PL01X_PRECONFIGURED,

This seems odd. You are not adding a new UART type.

Quoted 4 lines
Quoted 1 line

How about a separate bool flag?

I posted a patch with the same purpose a while ago
http://lists.denx.de/pipermail/u-boot/2015-October/230079.html

And then we got bogged down in "make a DT binding that's accepted" hell.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160208/e3c87ccd/attachment.sig>