[U-Boot] [PATCH] serial: pl01x: Add support for devices with the rate pre-configured.

[U-Boot] [PATCH] serial: pl01x: Add support for devices with the rate pre-configured.

[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 <eric@anholt.net>
---
 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(-)

diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
index 4b80d7b..1fd3f97 100644
--- a/board/raspberrypi/rpi/rpi.c
+++ b/board/raspberrypi/rpi/rpi.c
@@ -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) = {
diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
index 552c945..7d4d214 100644
--- a/drivers/serial/serial_pl01x.c
+++ b/drivers/serial/serial_pl01x.c
@@ -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;
diff --git a/include/dm/platform_data/serial_pl01x.h b/include/dm/platform_data/serial_pl01x.h
index 5e068f3..ae1bf31 100644
--- a/include/dm/platform_data/serial_pl01x.h
+++ b/include/dm/platform_data/serial_pl01x.h
@@ -9,6 +9,7 @@
 enum pl01x_type {
 	TYPE_PL010,
 	TYPE_PL011,
+	TYPE_PL01X_PRECONFIGURED,
 };
 
 /*
-- 
2.7.0

[U-Boot] [PATCH] serial: pl01x: Add support for devices with the rate pre-configured.

[Stephen Warren]

On 02/05/2016 02:43 PM, Eric Anholt wrote:
> 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 <swarren@wwwdotorg.org>
(With a 6MHz init_uart_clock and with/without init_uart_baud=9600)

[U-Boot] [PATCH] serial: pl01x: Add support for devices with the rate pre-configured.

[Simon Glass]

Hi Eric,

On 5 February 2016 at 14:43, Eric Anholt <eric@anholt.net> wrote:
> 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 <eric@anholt.net>
> ---
>  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(-)
>
> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
> index 4b80d7b..1fd3f97 100644
> --- a/board/raspberrypi/rpi/rpi.c
> +++ b/board/raspberrypi/rpi/rpi.c
> @@ -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) = {
> diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
> index 552c945..7d4d214 100644
> --- a/drivers/serial/serial_pl01x.c
> +++ b/drivers/serial/serial_pl01x.c
> @@ -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;
> diff --git a/include/dm/platform_data/serial_pl01x.h b/include/dm/platform_data/serial_pl01x.h
> index 5e068f3..ae1bf31 100644
> --- a/include/dm/platform_data/serial_pl01x.h
> +++ b/include/dm/platform_data/serial_pl01x.h
> @@ -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

[U-Boot] [PATCH] serial: pl01x: Add support for devices with the rate pre-configured.

[Sergei Temerkhanov]

On Sat, Feb 6, 2016 at 12:44 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Eric,
>
> On 5 February 2016 at 14:43, Eric Anholt <eric@anholt.net> wrote:
>> 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 <eric@anholt.net>
>> ---
>>  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(-)
>>
>> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
>> index 4b80d7b..1fd3f97 100644
>> --- a/board/raspberrypi/rpi/rpi.c
>> +++ b/board/raspberrypi/rpi/rpi.c
>> @@ -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) = {
>> diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
>> index 552c945..7d4d214 100644
>> --- a/drivers/serial/serial_pl01x.c
>> +++ b/drivers/serial/serial_pl01x.c
>> @@ -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;
>> diff --git a/include/dm/platform_data/serial_pl01x.h b/include/dm/platform_data/serial_pl01x.h
>> index 5e068f3..ae1bf31 100644
>> --- a/include/dm/platform_data/serial_pl01x.h
>> +++ b/include/dm/platform_data/serial_pl01x.h
>> @@ -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?

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

Regards,
Sergey

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

[U-Boot] [PATCH] serial: pl01x: Add support for devices with the rate pre-configured.

[Tom Rini]

On Mon, Feb 08, 2016 at 11:26:04AM -0800, Sergei Temerkhanov wrote:
> On Sat, Feb 6, 2016 at 12:44 PM, Simon Glass <sjg@chromium.org> wrote:
> > Hi Eric,
> >
> > On 5 February 2016 at 14:43, Eric Anholt <eric@anholt.net> wrote:
> >> 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 <eric@anholt.net>
> >> ---
> >>  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(-)
> >>
> >> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
> >> index 4b80d7b..1fd3f97 100644
> >> --- a/board/raspberrypi/rpi/rpi.c
> >> +++ b/board/raspberrypi/rpi/rpi.c
> >> @@ -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) = {
> >> diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
> >> index 552c945..7d4d214 100644
> >> --- a/drivers/serial/serial_pl01x.c
> >> +++ b/drivers/serial/serial_pl01x.c
> >> @@ -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;
> >> diff --git a/include/dm/platform_data/serial_pl01x.h b/include/dm/platform_data/serial_pl01x.h
> >> index 5e068f3..ae1bf31 100644
> >> --- a/include/dm/platform_data/serial_pl01x.h
> >> +++ b/include/dm/platform_data/serial_pl01x.h
> >> @@ -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?
> 
> 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.

-- 
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>

[U-Boot] [PATCH] serial: pl01x: Add support for devices with the rate pre-configured.

[Stephen Warren]

On 02/05/2016 09:19 PM, Stephen Warren wrote:
> On 02/05/2016 02:43 PM, Eric Anholt wrote:
>> 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 <swarren@wwwdotorg.org>
> (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?

[U-Boot] [PATCH] serial: pl01x: Add support for devices with the rate pre-configured.

[Simon Glass]

Hi Stephen,

On 5 March 2016 at 23:26, Stephen Warren <swarren@wwwdotorg.org> wrote:
>
> On 02/05/2016 09:19 PM, Stephen Warren wrote:
>>
>> On 02/05/2016 02:43 PM, Eric Anholt wrote:
>>>
>>> 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 <swarren@wwwdotorg.org>
>> (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

[U-Boot] [PATCH] serial: pl01x: Add support for devices with the rate pre-configured.

[Tom Rini]

On Sun, Mar 06, 2016 at 07:38:47PM -0700, Simon Glass wrote:
> Hi Stephen,
> 
> On 5 March 2016 at 23:26, Stephen Warren <swarren@wwwdotorg.org> wrote:
> >
> > On 02/05/2016 09:19 PM, Stephen Warren wrote:
> >>
> >> On 02/05/2016 02:43 PM, Eric Anholt wrote:
> >>>
> >>> 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 <swarren@wwwdotorg.org>
> >> (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.

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...

-- 
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>

[U-Boot] [PATCH] serial: pl01x: Add support for devices with the rate pre-configured.

[Simon Glass]

Hi Tom,

On 7 March 2016 at 16:37, Tom Rini <trini@konsulko.com> wrote:
> On Sun, Mar 06, 2016 at 07:38:47PM -0700, Simon Glass wrote:
>> Hi Stephen,
>>
>> On 5 March 2016 at 23:26, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> >
>> > On 02/05/2016 09:19 PM, Stephen Warren wrote:
>> >>
>> >> On 02/05/2016 02:43 PM, Eric Anholt wrote:
>> >>>
>> >>> 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 <swarren@wwwdotorg.org>
>> >> (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.
>
> 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:

> +       TYPE_PL01X_PRECONFIGURED,

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

How about a separate bool flag?

Regards,
Simon