2023-04-03 18:01:59

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

This baud rate is set for the device by mainline u-boot and is also what
is set in the Pinebook Pro Device Tree, which is a device similar to the
PinePhone Pro but with a different form factor.

Otherwise, the baud rate of the firmware and Linux don't match by default
and a 'console=ttyS2,1500000n8' kernel command line parameter is required
to have proper output for both.

Signed-off-by: Javier Martinez Canillas <[email protected]>
---

I tried to instead get rid of the baud rate altogether, as suggested by
Peter Robinson. AFAIU that should just pick whatever bad rate has been
using by the early console.

But neither using 'stdout-path = "serial2" nor 'stdout-path = &uart2'
worked for me.

In both cases I didn't have any output unless setting a baud rate using
the 'console='param.

arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
index a0795a2b1cb1..6bbe65bd5bd4 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
@@ -26,7 +26,7 @@ aliases {
};

chosen {
- stdout-path = "serial2:115200n8";
+ stdout-path = "serial2:1500000n8";
};

gpio-keys {

base-commit: 3adf89324a2b2a9dbc2c12d8895021e7e34e3346
--
2.40.0


2023-04-04 07:51:53

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

Hi,

Am Montag, 3. April 2023, 19:59:37 CEST schrieb Javier Martinez Canillas:
> This baud rate is set for the device by mainline u-boot and is also what
> is set in the Pinebook Pro Device Tree, which is a device similar to the
> PinePhone Pro but with a different form factor.
>
> Otherwise, the baud rate of the firmware and Linux don't match by default
> and a 'console=ttyS2,1500000n8' kernel command line parameter is required
> to have proper output for both.

The interesting question is always if this will break someone else's setup.
I've never really understood the strange setting of 1.5MBps, but on the
other hand it _is_ a reality on most boards.

Personally I don't care that much either way, but would like a comment
from the other people working on that device - if possible.

I guess if we don't hear anything, I'll apply it nevertheless at some point


Heiko


> Signed-off-by: Javier Martinez Canillas <[email protected]>
> ---
>
> I tried to instead get rid of the baud rate altogether, as suggested by
> Peter Robinson. AFAIU that should just pick whatever bad rate has been
> using by the early console.
>
> But neither using 'stdout-path = "serial2" nor 'stdout-path = &uart2'
> worked for me.
>
> In both cases I didn't have any output unless setting a baud rate using
> the 'console='param.
>
> arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> index a0795a2b1cb1..6bbe65bd5bd4 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> @@ -26,7 +26,7 @@ aliases {
> };
>
> chosen {
> - stdout-path = "serial2:115200n8";
> + stdout-path = "serial2:1500000n8";
> };
>
> gpio-keys {
>
> base-commit: 3adf89324a2b2a9dbc2c12d8895021e7e34e3346
>




2023-04-04 08:12:12

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

Heiko Stübner <[email protected]> writes:

Hello Heiko,

Thanks for your feedback.

> Hi,
>
> Am Montag, 3. April 2023, 19:59:37 CEST schrieb Javier Martinez Canillas:
>> This baud rate is set for the device by mainline u-boot and is also what
>> is set in the Pinebook Pro Device Tree, which is a device similar to the
>> PinePhone Pro but with a different form factor.
>>
>> Otherwise, the baud rate of the firmware and Linux don't match by default
>> and a 'console=ttyS2,1500000n8' kernel command line parameter is required
>> to have proper output for both.
>
> The interesting question is always if this will break someone else's setup.

Indeed.

> I've never really understood the strange setting of 1.5MBps, but on the
> other hand it _is_ a reality on most boards.
>

As far as I understand, it is just to get a faster data transmission but I
have my doubts that it is worth to divert from the more common 115200 rate
just for this.

As you said though, it is a reality and also what mainline u-boot uses for
this device.

> Personally I don't care that much either way, but would like a comment
> from the other people working on that device - if possible.
>

Same, I don't care either but just that would be good to make Linux and
u-boot to match. If this change will break other people setups, maybe I
can convince u-boot to sync with Linux and also use 115200 for the phone.

> I guess if we don't hear anything, I'll apply it nevertheless at some point
>
>
> Heiko
>

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

2023-04-04 08:15:48

by Jarrah

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

Hi,

On 4/4/23 17:51, Heiko Stübner wrote:
> The interesting question is always if this will break someone else's setup.
> I've never really understood the strange setting of 1.5MBps, but on the
> other hand it _is_ a reality on most boards.
>
> Personally I don't care that much either way, but would like a comment
> from the other people working on that device - if possible.


I don't have a strong opinion either way, but I would note that
Tow-Boot[0] which later models of this device ship with uses 115200,
meaning that this would put the device out of sync with the default
u-boot implementation from the factory.

At least for me it's been convenient to have both the PP and PPP use the
same settings while debugging.


Thanks,

Jarrah.


[0] https://tow-boot.org/

2023-04-04 08:20:08

by Peter Robinson

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

On Tue, Apr 4, 2023 at 9:13 AM Jarrah <[email protected]> wrote:
>
> Hi,
>
> On 4/4/23 17:51, Heiko Stübner wrote:
> > The interesting question is always if this will break someone else's setup.
> > I've never really understood the strange setting of 1.5MBps, but on the
> > other hand it _is_ a reality on most boards.
> >
> > Personally I don't care that much either way, but would like a comment
> > from the other people working on that device - if possible.
>
>
> I don't have a strong opinion either way, but I would note that
> Tow-Boot[0] which later models of this device ship with uses 115200,
> meaning that this would put the device out of sync with the default
> u-boot implementation from the factory.

Upstream U-Boot support uses the 1.5m that all the other rockchip devices use.

> At least for me it's been convenient to have both the PP and PPP use the
> same settings while debugging.

Pinebook Pro and numerous other devices use 1.5m so I don't think what
other devices use make are ultimately particularly relevant.

2023-04-04 08:26:46

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

Jarrah <[email protected]> writes:

> Hi,
>
> On 4/4/23 17:51, Heiko Stübner wrote:
>> The interesting question is always if this will break someone else's setup.
>> I've never really understood the strange setting of 1.5MBps, but on the
>> other hand it _is_ a reality on most boards.
>>
>> Personally I don't care that much either way, but would like a comment
>> from the other people working on that device - if possible.
>
>
> I don't have a strong opinion either way, but I would note that
> Tow-Boot[0] which later models of this device ship with uses 115200,
> meaning that this would put the device out of sync with the default
> u-boot implementation from the factory.
>

Probably we can't change then because it would break the setup of people
using Tow-Boot as a bootloader.

> At least for me it's been convenient to have both the PP and PPP use the
> same settings while debugging.
>

Agreed, all my other boards use 115200 as well, the only exceptions are
the Rockpro64 and PinePhone Pro for me.

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

2023-04-04 08:41:30

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

Peter Robinson <[email protected]> writes:

> On Tue, Apr 4, 2023 at 9:13 AM Jarrah <[email protected]> wrote:
>>
>> Hi,
>>
>> On 4/4/23 17:51, Heiko Stübner wrote:
>> > The interesting question is always if this will break someone else's setup.
>> > I've never really understood the strange setting of 1.5MBps, but on the
>> > other hand it _is_ a reality on most boards.
>> >
>> > Personally I don't care that much either way, but would like a comment
>> > from the other people working on that device - if possible.
>>
>>
>> I don't have a strong opinion either way, but I would note that
>> Tow-Boot[0] which later models of this device ship with uses 115200,
>> meaning that this would put the device out of sync with the default
>> u-boot implementation from the factory.
>
> Upstream U-Boot support uses the 1.5m that all the other rockchip devices use.
>

That's true too. In fact, the Pinebook Pro also uses 1.5m so people using
Tow-Boot already will have a mismatch when using that device. The PPP is
the device that is not consistent with all the other rk3399 boards.

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

2023-04-04 08:42:39

by Peter Robinson

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

On Tue, Apr 4, 2023 at 9:24 AM Javier Martinez Canillas
<[email protected]> wrote:
>
> Jarrah <[email protected]> writes:
>
> > Hi,
> >
> > On 4/4/23 17:51, Heiko Stübner wrote:
> >> The interesting question is always if this will break someone else's setup.
> >> I've never really understood the strange setting of 1.5MBps, but on the
> >> other hand it _is_ a reality on most boards.
> >>
> >> Personally I don't care that much either way, but would like a comment
> >> from the other people working on that device - if possible.
> >
> >
> > I don't have a strong opinion either way, but I would note that
> > Tow-Boot[0] which later models of this device ship with uses 115200,
> > meaning that this would put the device out of sync with the default
> > u-boot implementation from the factory.
> >
>
> Probably we can't change then because it would break the setup of people
> using Tow-Boot as a bootloader.
>
> > At least for me it's been convenient to have both the PP and PPP use the
> > same settings while debugging.
> >
>
> Agreed, all my other boards use 115200 as well, the only exceptions are
> the Rockpro64 and PinePhone Pro for me.

All Rockchips devices use 1.5m except a chromebook and the Puma, and
this device.

2023-04-04 08:48:49

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

Peter Robinson <[email protected]> writes:

> On Tue, Apr 4, 2023 at 9:24 AM Javier Martinez Canillas
> <[email protected]> wrote:

[...]

>> > At least for me it's been convenient to have both the PP and PPP use the
>> > same settings while debugging.
>> >
>>
>> Agreed, all my other boards use 115200 as well, the only exceptions are
>> the Rockpro64 and PinePhone Pro for me.
>
> All Rockchips devices use 1.5m except a chromebook and the Puma, and
> this device.
>

Yes, I meant all the other non-rockchip boards I have. So I understood the
appeal of what Jarrah mentioned about using 115200 for everything.

Having said that, my vote would be to change the PinePhone Pro to 1.5 MB
given that it is what all the rockchip (but two) boards use in mainline.

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

2023-04-04 11:39:08

by Martijn Braam

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB


On 4/4/23 09:51, Heiko Stübner wrote:
> Hi,
>
> Am Montag, 3. April 2023, 19:59:37 CEST schrieb Javier Martinez Canillas:
>> This baud rate is set for the device by mainline u-boot and is also what
>> is set in the Pinebook Pro Device Tree, which is a device similar to the
>> PinePhone Pro but with a different form factor.
>>
>> Otherwise, the baud rate of the firmware and Linux don't match by default
>> and a 'console=ttyS2,1500000n8' kernel command line parameter is required
>> to have proper output for both.
> The interesting question is always if this will break someone else's setup.
> I've never really understood the strange setting of 1.5MBps, but on the
> other hand it _is_ a reality on most boards.

It breaks my device test setup at least. The extra speed isn't worth the
hassle
of having a few devices at weird baudrates and the bootloader already
starts outputting debug logs at 115200 baud.

>
> Personally I don't care that much either way, but would like a comment
> from the other people working on that device - if possible.
>
> I guess if we don't hear anything, I'll apply it nevertheless at some point
>
>
> Heiko
>
>
>> Signed-off-by: Javier Martinez Canillas <[email protected]>
>> ---
>>
>> I tried to instead get rid of the baud rate altogether, as suggested by
>> Peter Robinson. AFAIU that should just pick whatever bad rate has been
>> using by the early console.
>>
>> But neither using 'stdout-path = "serial2" nor 'stdout-path = &uart2'
>> worked for me.
>>
>> In both cases I didn't have any output unless setting a baud rate using
>> the 'console='param.
>>
>> arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
>> index a0795a2b1cb1..6bbe65bd5bd4 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
>> @@ -26,7 +26,7 @@ aliases {
>> };
>>
>> chosen {
>> - stdout-path = "serial2:115200n8";
>> + stdout-path = "serial2:1500000n8";
>> };
>>
>> gpio-keys {
>>
>> base-commit: 3adf89324a2b2a9dbc2c12d8895021e7e34e3346
>>
>
>
>

2023-04-04 11:44:08

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

Martijn Braam <[email protected]> writes:

> On 4/4/23 09:51, Heiko Stübner wrote:
>> Hi,
>>
>> Am Montag, 3. April 2023, 19:59:37 CEST schrieb Javier Martinez Canillas:
>>> This baud rate is set for the device by mainline u-boot and is also what
>>> is set in the Pinebook Pro Device Tree, which is a device similar to the
>>> PinePhone Pro but with a different form factor.
>>>
>>> Otherwise, the baud rate of the firmware and Linux don't match by default
>>> and a 'console=ttyS2,1500000n8' kernel command line parameter is required
>>> to have proper output for both.
>> The interesting question is always if this will break someone else's setup.
>> I've never really understood the strange setting of 1.5MBps, but on the
>> other hand it _is_ a reality on most boards.
>
> It breaks my device test setup at least. The extra speed isn't worth the
> hassle

More than the extra speed is to have consistency accross all the rockchip
devices in upstream and also sync with mainline u-boot.

> of having a few devices at weird baudrates and the bootloader already
> starts outputting debug logs at 115200 baud.
>

And mine starts outputting at 1.5MBps :) I guess that there isn't a one
size fits all, so the question is whether the bikeshed color is what was
painted in all other rockchip boards or the one that Tow-Boot has chosen.

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

2023-04-04 12:13:48

by Ondřej Jirman

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

Hello,

On Tue, Apr 04, 2023 at 01:42:36PM +0200, Javier Martinez Canillas wrote:
> Martijn Braam <[email protected]> writes:
>
> > On 4/4/23 09:51, Heiko St?bner wrote:
> >> Hi,
> >>
> >> Am Montag, 3. April 2023, 19:59:37 CEST schrieb Javier Martinez Canillas:
> >>> This baud rate is set for the device by mainline u-boot and is also what
> >>> is set in the Pinebook Pro Device Tree, which is a device similar to the
> >>> PinePhone Pro but with a different form factor.
> >>>
> >>> Otherwise, the baud rate of the firmware and Linux don't match by default
> >>> and a 'console=ttyS2,1500000n8' kernel command line parameter is required
> >>> to have proper output for both.
> >> The interesting question is always if this will break someone else's setup.
> >> I've never really understood the strange setting of 1.5MBps, but on the
> >> other hand it _is_ a reality on most boards.
> >
> > It breaks my device test setup at least. The extra speed isn't worth the
> > hassle
>
> More than the extra speed is to have consistency accross all the rockchip
> devices in upstream and also sync with mainline u-boot.
>
> > of having a few devices at weird baudrates and the bootloader already
> > starts outputting debug logs at 115200 baud.
> >
>
> And mine starts outputting at 1.5MBps :) I guess that there isn't a one
> size fits all, so the question is whether the bikeshed color is what was
> painted in all other rockchip boards or the one that Tow-Boot has chosen.

For what it's worth, levinboot also defaults to 1.5 Mbaud.

https://gitlab.com/DeltaGem/levinboot/-/blob/release/rk3399/entry.S#L65
https://gitlab.com/DeltaGem/levinboot/-/blob/release/configure.py#L67

And it's very nice anything above >115200 is not broken by bad HW design,
like on original Pinephone, so this higher speed actually works. ;)

kind regards,
o.

> --
> Best regards,
>
> Javier Martinez Canillas
> Core Platforms
> Red Hat
>

2023-04-04 12:49:33

by Ondřej Jirman

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

On Tue, Apr 04, 2023 at 09:51:11AM +0200, Heiko St?bner wrote:
> Hi,
>
> Am Montag, 3. April 2023, 19:59:37 CEST schrieb Javier Martinez Canillas:
> > This baud rate is set for the device by mainline u-boot and is also what
> > is set in the Pinebook Pro Device Tree, which is a device similar to the
> > PinePhone Pro but with a different form factor.
> >
> > Otherwise, the baud rate of the firmware and Linux don't match by default
> > and a 'console=ttyS2,1500000n8' kernel command line parameter is required
> > to have proper output for both.
>
> The interesting question is always if this will break someone else's setup.
> I've never really understood the strange setting of 1.5MBps, but on the
> other hand it _is_ a reality on most boards.

Normal users of the phone probably run with UART console disabled, because
UART is muxed with audio jack output and to enable it they have to add
console=ttyS2 to the kernel command line and flip a physical switch inside
the phone.

Fortunately, not sepcifying stdout-path baud rate in the options part
of the string, will make the serial driver probe for the baud rate from
the previous boot stage and make the user happy by keeping whatever was
already set in the bootloader.

https://elixir.bootlin.com/linux/latest/source/drivers/tty/serial/8250/8250_port.c#L3496

So we can make the kernel just keep the baudrate setup from the previous
boot stage by:

stdout-path = "serial2";

regards,
o.

> Personally I don't care that much either way, but would like a comment
> from the other people working on that device - if possible.
>
> I guess if we don't hear anything, I'll apply it nevertheless at some point
>
>
> Heiko
>
>
> > Signed-off-by: Javier Martinez Canillas <[email protected]>
> > ---
> >
> > I tried to instead get rid of the baud rate altogether, as suggested by
> > Peter Robinson. AFAIU that should just pick whatever bad rate has been
> > using by the early console.
> >
> > But neither using 'stdout-path = "serial2" nor 'stdout-path = &uart2'
> > worked for me.
> >
> > In both cases I didn't have any output unless setting a baud rate using
> > the 'console='param.
> >
> > arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > index a0795a2b1cb1..6bbe65bd5bd4 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > @@ -26,7 +26,7 @@ aliases {
> > };
> >
> > chosen {
> > - stdout-path = "serial2:115200n8";
> > + stdout-path = "serial2:1500000n8";
> > };
> >
> > gpio-keys {
> >
> > base-commit: 3adf89324a2b2a9dbc2c12d8895021e7e34e3346
> >
>
>
>
>

2023-04-04 13:09:08

by Peter Geis

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

On Tue, Apr 4, 2023 at 3:55 AM Heiko Stübner <[email protected]> wrote:
>
> Hi,
>
> Am Montag, 3. April 2023, 19:59:37 CEST schrieb Javier Martinez Canillas:
> > This baud rate is set for the device by mainline u-boot and is also what
> > is set in the Pinebook Pro Device Tree, which is a device similar to the
> > PinePhone Pro but with a different form factor.
> >
> > Otherwise, the baud rate of the firmware and Linux don't match by default
> > and a 'console=ttyS2,1500000n8' kernel command line parameter is required
> > to have proper output for both.
>
> The interesting question is always if this will break someone else's setup.
> I've never really understood the strange setting of 1.5MBps, but on the
> other hand it _is_ a reality on most boards.

Good Morning Heiko,

The 1.5M baud is default because the clock structure on rockchip
devices does not allow a clean 115200 baud. By attempting to force
115200, it will always be slightly off (either low or high depending
on how the driver decided to round). If this actually causes any
problems is the subject of much debate.

Very Respectfully,
Peter Geis

>
> Personally I don't care that much either way, but would like a comment
> from the other people working on that device - if possible.
>
> I guess if we don't hear anything, I'll apply it nevertheless at some point
>
>
> Heiko
>
>
> > Signed-off-by: Javier Martinez Canillas <[email protected]>
> > ---
> >
> > I tried to instead get rid of the baud rate altogether, as suggested by
> > Peter Robinson. AFAIU that should just pick whatever bad rate has been
> > using by the early console.
> >
> > But neither using 'stdout-path = "serial2" nor 'stdout-path = &uart2'
> > worked for me.
> >
> > In both cases I didn't have any output unless setting a baud rate using
> > the 'console='param.
> >
> > arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > index a0795a2b1cb1..6bbe65bd5bd4 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > @@ -26,7 +26,7 @@ aliases {
> > };
> >
> > chosen {
> > - stdout-path = "serial2:115200n8";
> > + stdout-path = "serial2:1500000n8";
> > };
> >
> > gpio-keys {
> >
> > base-commit: 3adf89324a2b2a9dbc2c12d8895021e7e34e3346
> >
>
>
>
>
>
> _______________________________________________
> Linux-rockchip mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

2023-04-04 14:17:28

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

Ondřej Jirman <[email protected]> writes:

> On Tue, Apr 04, 2023 at 09:51:11AM +0200, Heiko Stübner wrote:
>> Hi,
>>
>> Am Montag, 3. April 2023, 19:59:37 CEST schrieb Javier Martinez Canillas:
>> > This baud rate is set for the device by mainline u-boot and is also what
>> > is set in the Pinebook Pro Device Tree, which is a device similar to the
>> > PinePhone Pro but with a different form factor.
>> >
>> > Otherwise, the baud rate of the firmware and Linux don't match by default
>> > and a 'console=ttyS2,1500000n8' kernel command line parameter is required
>> > to have proper output for both.
>>
>> The interesting question is always if this will break someone else's setup.
>> I've never really understood the strange setting of 1.5MBps, but on the
>> other hand it _is_ a reality on most boards.
>
> Normal users of the phone probably run with UART console disabled, because
> UART is muxed with audio jack output and to enable it they have to add
> console=ttyS2 to the kernel command line and flip a physical switch inside
> the phone.
>
> Fortunately, not sepcifying stdout-path baud rate in the options part
> of the string, will make the serial driver probe for the baud rate from
> the previous boot stage and make the user happy by keeping whatever was
> already set in the bootloader.
>

As mentioned in the first email of this thread, I tried that but it didn't
work for me. Either using stdout-path = "serial2"; or stdout-path = &uart2;
gives me no serial output with console=ttyS2, I needed to specify the baud
rate explicitly (i.e: console=ttyS2,1500000n8).

> https://elixir.bootlin.com/linux/latest/source/drivers/tty/serial/8250/8250_port.c#L3496
>

Is that helper really used by the serial driver in the PinePhone Pro?
(drivers/tty/serial/8250/8250_dw.c), I was meaning to dig why just ttyS2
was not working but decided that could be a follow-up patch.

Since chaging to 1.5 MBps seemed to have merits on its own, decided to
post this patch anyways in the meantime.

> So we can make the kernel just keep the baudrate setup from the previous
> boot stage by:
>
> stdout-path = "serial2";
>

Did it work for you? Maybe I'm doing something silly but as mentioned it
didn't work for me with upstream u-boot.

> regards,
> o.
>

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

2023-04-04 15:42:14

by Ondřej Jirman

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

On Tue, Apr 04, 2023 at 04:04:53PM +0200, Javier Martinez Canillas wrote:
> Ondřej Jirman <[email protected]> writes:
>
> > On Tue, Apr 04, 2023 at 09:51:11AM +0200, Heiko Stübner wrote:
> >> Hi,
> >>
> >> Am Montag, 3. April 2023, 19:59:37 CEST schrieb Javier Martinez Canillas:
> >> > This baud rate is set for the device by mainline u-boot and is also what
> >> > is set in the Pinebook Pro Device Tree, which is a device similar to the
> >> > PinePhone Pro but with a different form factor.
> >> >
> >> > Otherwise, the baud rate of the firmware and Linux don't match by default
> >> > and a 'console=ttyS2,1500000n8' kernel command line parameter is required
> >> > to have proper output for both.
> >>
> >> The interesting question is always if this will break someone else's setup.
> >> I've never really understood the strange setting of 1.5MBps, but on the
> >> other hand it _is_ a reality on most boards.
> >
> > Normal users of the phone probably run with UART console disabled, because
> > UART is muxed with audio jack output and to enable it they have to add
> > console=ttyS2 to the kernel command line and flip a physical switch inside
> > the phone.
> >
> > Fortunately, not sepcifying stdout-path baud rate in the options part
> > of the string, will make the serial driver probe for the baud rate from
> > the previous boot stage and make the user happy by keeping whatever was
> > already set in the bootloader.
> >
>
> As mentioned in the first email of this thread, I tried that but it didn't
> work for me. Either using stdout-path = "serial2"; or stdout-path = &uart2;
> gives me no serial output with console=ttyS2, I needed to specify the baud
> rate explicitly (i.e: console=ttyS2,1500000n8).
>
> > https://elixir.bootlin.com/linux/latest/source/drivers/tty/serial/8250/8250_port.c#L3496
> >
>
> Is that helper really used by the serial driver in the PinePhone Pro?
> (drivers/tty/serial/8250/8250_dw.c), I was meaning to dig why just ttyS2
> was not working but decided that could be a follow-up patch.

It's called by univ8250_console_setup in 8250_core.c But looking at it again,
this may only be used to match console=uart.

https://elixir.bootlin.com/linux/latest/source/drivers/tty/serial/8250/8250_core.c#L638

Bummer.

> Since chaging to 1.5 MBps seemed to have merits on its own, decided to
> post this patch anyways in the meantime.
>
> > So we can make the kernel just keep the baudrate setup from the previous
> > boot stage by:
> >
> > stdout-path = "serial2";
> >
>
> Did it work for you? Maybe I'm doing something silly but as mentioned it
> didn't work for me with upstream u-boot.

I was just eyeballing the code. I didn't test it.

kind regards,
o.

> > regards,
> > o.
> >
>
> --
> Best regards,
>
> Javier Martinez Canillas
> Core Platforms
> Red Hat
>

2023-04-04 16:57:00

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

Hi Peter,

Am Dienstag, 4. April 2023, 14:52:02 CEST schrieb Peter Geis:
> On Tue, Apr 4, 2023 at 3:55 AM Heiko Stübner <[email protected]> wrote:
> >
> > Hi,
> >
> > Am Montag, 3. April 2023, 19:59:37 CEST schrieb Javier Martinez Canillas:
> > > This baud rate is set for the device by mainline u-boot and is also what
> > > is set in the Pinebook Pro Device Tree, which is a device similar to the
> > > PinePhone Pro but with a different form factor.
> > >
> > > Otherwise, the baud rate of the firmware and Linux don't match by default
> > > and a 'console=ttyS2,1500000n8' kernel command line parameter is required
> > > to have proper output for both.
> >
> > The interesting question is always if this will break someone else's setup.
> > I've never really understood the strange setting of 1.5MBps, but on the
> > other hand it _is_ a reality on most boards.

> The 1.5M baud is default because the clock structure on rockchip
> devices does not allow a clean 115200 baud. By attempting to force
> 115200, it will always be slightly off (either low or high depending
> on how the driver decided to round). If this actually causes any
> problems is the subject of much debate.

thanks so much for this piece of clock-detail. As I wrote, I never really
understood the why _before_ but also never cared that much to dive
into it and find out.

So your explanation closes one knowledge gap in my head.

Thanks a lot :-)
Heiko



2023-04-18 12:15:18

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

Heiko Stübner <[email protected]> writes:

Hello Heiko,

> Hi Peter,
>
> Am Dienstag, 4. April 2023, 14:52:02 CEST schrieb Peter Geis:
>> On Tue, Apr 4, 2023 at 3:55 AM Heiko Stübner <[email protected]> wrote:
>> >
>> > Hi,
>> >
>> > Am Montag, 3. April 2023, 19:59:37 CEST schrieb Javier Martinez Canillas:
>> > > This baud rate is set for the device by mainline u-boot and is also what
>> > > is set in the Pinebook Pro Device Tree, which is a device similar to the
>> > > PinePhone Pro but with a different form factor.
>> > >
>> > > Otherwise, the baud rate of the firmware and Linux don't match by default
>> > > and a 'console=ttyS2,1500000n8' kernel command line parameter is required
>> > > to have proper output for both.
>> >
>> > The interesting question is always if this will break someone else's setup.
>> > I've never really understood the strange setting of 1.5MBps, but on the
>> > other hand it _is_ a reality on most boards.
>
>> The 1.5M baud is default because the clock structure on rockchip
>> devices does not allow a clean 115200 baud. By attempting to force
>> 115200, it will always be slightly off (either low or high depending
>> on how the driver decided to round). If this actually causes any
>> problems is the subject of much debate.
>
> thanks so much for this piece of clock-detail. As I wrote, I never really
> understood the why _before_ but also never cared that much to dive
> into it and find out.
>
> So your explanation closes one knowledge gap in my head.
>
> Thanks a lot :-)

Did you make a decision about this? I guess the clock explanation is yet
another argument in favour of switching the PPP to a 1.5 Mbps baud rate ?

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

2023-04-18 14:38:04

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

Hi,

Am Dienstag, 18. April 2023, 14:11:53 CEST schrieb Javier Martinez Canillas:
> Heiko Stübner <[email protected]> writes:
>
> Hello Heiko,
>
> > Hi Peter,
> >
> > Am Dienstag, 4. April 2023, 14:52:02 CEST schrieb Peter Geis:
> >> On Tue, Apr 4, 2023 at 3:55 AM Heiko Stübner <[email protected]> wrote:
> >> >
> >> > Hi,
> >> >
> >> > Am Montag, 3. April 2023, 19:59:37 CEST schrieb Javier Martinez Canillas:
> >> > > This baud rate is set for the device by mainline u-boot and is also what
> >> > > is set in the Pinebook Pro Device Tree, which is a device similar to the
> >> > > PinePhone Pro but with a different form factor.
> >> > >
> >> > > Otherwise, the baud rate of the firmware and Linux don't match by default
> >> > > and a 'console=ttyS2,1500000n8' kernel command line parameter is required
> >> > > to have proper output for both.
> >> >
> >> > The interesting question is always if this will break someone else's setup.
> >> > I've never really understood the strange setting of 1.5MBps, but on the
> >> > other hand it _is_ a reality on most boards.
> >
> >> The 1.5M baud is default because the clock structure on rockchip
> >> devices does not allow a clean 115200 baud. By attempting to force
> >> 115200, it will always be slightly off (either low or high depending
> >> on how the driver decided to round). If this actually causes any
> >> problems is the subject of much debate.
> >
> > thanks so much for this piece of clock-detail. As I wrote, I never really
> > understood the why _before_ but also never cared that much to dive
> > into it and find out.
> >
> > So your explanation closes one knowledge gap in my head.
> >
> > Thanks a lot :-)
>
> Did you make a decision about this? I guess the clock explanation is yet
> another argument in favour of switching the PPP to a 1.5 Mbps baud rate ?

Sorry, but no decision made here. Either way it's breaking for someone,
which makes this quite hard.

The rate accuracy is the one side, the two-boot issue is the other side.
And mainline u-boot (and levinboot - whatever that is) provides a 3rd side.

People starting with the phone probably won't replace the bootloader
in a first step but instead might play with a system image or newer kernel.
So if the uart will break for everyone using the default bootloader from
the factory that is somewhat bad.

I don't have a Pinephone Pro myself, so I really hoped for some Acks
or similar to appear in the meantime.

Do we have someone with an actual Pine64 affiliation in this loop?


Heiko


2023-07-21 23:57:18

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

Heiko Stübner <[email protected]> writes:

Hello Heiko,

> Hi,
>
> Am Dienstag, 18. April 2023, 14:11:53 CEST schrieb Javier Martinez Canillas:
>> Heiko Stübner <[email protected]> writes:
>>
>> Hello Heiko,
>>
>> > Hi Peter,
>> >
>> > Am Dienstag, 4. April 2023, 14:52:02 CEST schrieb Peter Geis:
>> >> On Tue, Apr 4, 2023 at 3:55 AM Heiko Stübner <[email protected]> wrote:
>> >> >
>> >> > Hi,
>> >> >
>> >> > Am Montag, 3. April 2023, 19:59:37 CEST schrieb Javier Martinez Canillas:
>> >> > > This baud rate is set for the device by mainline u-boot and is also what
>> >> > > is set in the Pinebook Pro Device Tree, which is a device similar to the
>> >> > > PinePhone Pro but with a different form factor.
>> >> > >
>> >> > > Otherwise, the baud rate of the firmware and Linux don't match by default
>> >> > > and a 'console=ttyS2,1500000n8' kernel command line parameter is required
>> >> > > to have proper output for both.
>> >> >
>> >> > The interesting question is always if this will break someone else's setup.
>> >> > I've never really understood the strange setting of 1.5MBps, but on the
>> >> > other hand it _is_ a reality on most boards.
>> >
>> >> The 1.5M baud is default because the clock structure on rockchip
>> >> devices does not allow a clean 115200 baud. By attempting to force
>> >> 115200, it will always be slightly off (either low or high depending
>> >> on how the driver decided to round). If this actually causes any
>> >> problems is the subject of much debate.
>> >
>> > thanks so much for this piece of clock-detail. As I wrote, I never really
>> > understood the why _before_ but also never cared that much to dive
>> > into it and find out.
>> >
>> > So your explanation closes one knowledge gap in my head.
>> >
>> > Thanks a lot :-)
>>
>> Did you make a decision about this? I guess the clock explanation is yet
>> another argument in favour of switching the PPP to a 1.5 Mbps baud rate ?
>
> Sorry, but no decision made here. Either way it's breaking for someone,
> which makes this quite hard.
>

Another ping on this patch.

> The rate accuracy is the one side, the two-boot issue is the other side.
> And mainline u-boot (and levinboot - whatever that is) provides a 3rd side.
>
> People starting with the phone probably won't replace the bootloader
> in a first step but instead might play with a system image or newer kernel.
> So if the uart will break for everyone using the default bootloader from
> the factory that is somewhat bad.
>

Probably won't replace the DTB shipped with the firmware either? If one is
replacing the firmware provided DTB witch the one in the mainline kernel,
probably such person is also using mainline u-boot?

> I don't have a Pinephone Pro myself, so I really hoped for some Acks
> or similar to appear in the meantime.
>

For someone like me who is only using mainline u-boot, linux, etc then
having a consistent uart baud rate across all components is really useful.

Otherwise I either have serial console for u-boot or the kernel, but can't
have both working so is annoying.

It would be good to have a definite answer on this. Since every time that
I try to hack on my PPP, I end changing my DTS and remember this patch :)

> Do we have someone with an actual Pine64 affiliation in this loop?
>

I don't think pine64 cares that much about the software. They just build
the hardware and expect the community to add the support I believe.

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


2023-07-28 19:57:42

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

Hi Javier,

Am Samstag, 22. Juli 2023, 01:06:54 CEST schrieb Javier Martinez Canillas:
> Heiko Stübner <[email protected]> writes:
> > Am Dienstag, 18. April 2023, 14:11:53 CEST schrieb Javier Martinez Canillas:
> >> Heiko Stübner <[email protected]> writes:
> >> > Am Dienstag, 4. April 2023, 14:52:02 CEST schrieb Peter Geis:
> >> >> On Tue, Apr 4, 2023 at 3:55 AM Heiko Stübner <[email protected]> wrote:
> >> >> > Am Montag, 3. April 2023, 19:59:37 CEST schrieb Javier Martinez Canillas:
> >> >> > > This baud rate is set for the device by mainline u-boot and is also what
> >> >> > > is set in the Pinebook Pro Device Tree, which is a device similar to the
> >> >> > > PinePhone Pro but with a different form factor.
> >> >> > >
> >> >> > > Otherwise, the baud rate of the firmware and Linux don't match by default
> >> >> > > and a 'console=ttyS2,1500000n8' kernel command line parameter is required
> >> >> > > to have proper output for both.
> >> >> >
> >> >> > The interesting question is always if this will break someone else's setup.
> >> >> > I've never really understood the strange setting of 1.5MBps, but on the
> >> >> > other hand it _is_ a reality on most boards.
> >> >
> >> >> The 1.5M baud is default because the clock structure on rockchip
> >> >> devices does not allow a clean 115200 baud. By attempting to force
> >> >> 115200, it will always be slightly off (either low or high depending
> >> >> on how the driver decided to round). If this actually causes any
> >> >> problems is the subject of much debate.
> >> >
> >> > thanks so much for this piece of clock-detail. As I wrote, I never really
> >> > understood the why _before_ but also never cared that much to dive
> >> > into it and find out.
> >> >
> >> > So your explanation closes one knowledge gap in my head.
> >> >
> >> > Thanks a lot :-)
> >>
> >> Did you make a decision about this? I guess the clock explanation is yet
> >> another argument in favour of switching the PPP to a 1.5 Mbps baud rate ?
> >
> > Sorry, but no decision made here. Either way it's breaking for someone,
> > which makes this quite hard.
> >
>
> Another ping on this patch.
>
> > The rate accuracy is the one side, the two-boot issue is the other side.
> > And mainline u-boot (and levinboot - whatever that is) provides a 3rd side.
> >
> > People starting with the phone probably won't replace the bootloader
> > in a first step but instead might play with a system image or newer kernel.
> > So if the uart will break for everyone using the default bootloader from
> > the factory that is somewhat bad.
> >
>
> Probably won't replace the DTB shipped with the firmware either? If one is
> replacing the firmware provided DTB witch the one in the mainline kernel,
> probably such person is also using mainline u-boot?

Not necessarily.

I.e. putting an extlinux.conf on an sd-card with a kernel-image and dtb
is not rocket science ;-)


> > I don't have a Pinephone Pro myself, so I really hoped for some Acks
> > or similar to appear in the meantime.
> >
>
> For someone like me who is only using mainline u-boot, linux, etc then
> having a consistent uart baud rate across all components is really useful.
>
> Otherwise I either have serial console for u-boot or the kernel, but can't
> have both working so is annoying.
>
> It would be good to have a definite answer on this. Since every time that
> I try to hack on my PPP, I end changing my DTS and remember this patch :)

So far people only reported "breaks my setup". I'm in a pickle here ;-) .
Without anybody saying "I want to also move into this direction" I really
feel I should not merge a patch that breaks other peoples setups.


Heiko



2023-07-28 20:20:07

by Maya Matuszczyk

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

Hi Heiko

pt., 28 lip 2023 o 21:00 Heiko Stuebner <[email protected]> napisał(a):
>
> Hi Javier,
>
> Am Samstag, 22. Juli 2023, 01:06:54 CEST schrieb Javier Martinez Canillas:
> > Heiko Stübner <[email protected]> writes:
> > > Am Dienstag, 18. April 2023, 14:11:53 CEST schrieb Javier Martinez Canillas:
> > >> Heiko Stübner <[email protected]> writes:
> > >> > Am Dienstag, 4. April 2023, 14:52:02 CEST schrieb Peter Geis:
> > >> >> On Tue, Apr 4, 2023 at 3:55 AM Heiko Stübner <[email protected]> wrote:
> > >> >> > Am Montag, 3. April 2023, 19:59:37 CEST schrieb Javier Martinez Canillas:
> > >> >> > > This baud rate is set for the device by mainline u-boot and is also what
> > >> >> > > is set in the Pinebook Pro Device Tree, which is a device similar to the
> > >> >> > > PinePhone Pro but with a different form factor.
> > >> >> > >
> > >> >> > > Otherwise, the baud rate of the firmware and Linux don't match by default
> > >> >> > > and a 'console=ttyS2,1500000n8' kernel command line parameter is required
> > >> >> > > to have proper output for both.
> > >> >> >
> > >> >> > The interesting question is always if this will break someone else's setup.
> > >> >> > I've never really understood the strange setting of 1.5MBps, but on the
> > >> >> > other hand it _is_ a reality on most boards.
> > >> >
> > >> >> The 1.5M baud is default because the clock structure on rockchip
> > >> >> devices does not allow a clean 115200 baud. By attempting to force
> > >> >> 115200, it will always be slightly off (either low or high depending
> > >> >> on how the driver decided to round). If this actually causes any
> > >> >> problems is the subject of much debate.
> > >> >
> > >> > thanks so much for this piece of clock-detail. As I wrote, I never really
> > >> > understood the why _before_ but also never cared that much to dive
> > >> > into it and find out.
> > >> >
> > >> > So your explanation closes one knowledge gap in my head.
> > >> >
> > >> > Thanks a lot :-)
> > >>
> > >> Did you make a decision about this? I guess the clock explanation is yet
> > >> another argument in favour of switching the PPP to a 1.5 Mbps baud rate ?
> > >
> > > Sorry, but no decision made here. Either way it's breaking for someone,
> > > which makes this quite hard.
> > >
> >
> > Another ping on this patch.
> >
> > > The rate accuracy is the one side, the two-boot issue is the other side.
> > > And mainline u-boot (and levinboot - whatever that is) provides a 3rd side.
> > >
> > > People starting with the phone probably won't replace the bootloader
> > > in a first step but instead might play with a system image or newer kernel.
> > > So if the uart will break for everyone using the default bootloader from
> > > the factory that is somewhat bad.
> > >
> >
> > Probably won't replace the DTB shipped with the firmware either? If one is
> > replacing the firmware provided DTB witch the one in the mainline kernel,
> > probably such person is also using mainline u-boot?
>
> Not necessarily.
>
> I.e. putting an extlinux.conf on an sd-card with a kernel-image and dtb
> is not rocket science ;-)
>
>
> > > I don't have a Pinephone Pro myself, so I really hoped for some Acks
> > > or similar to appear in the meantime.
> > >
> >
> > For someone like me who is only using mainline u-boot, linux, etc then
> > having a consistent uart baud rate across all components is really useful.
> >
> > Otherwise I either have serial console for u-boot or the kernel, but can't
> > have both working so is annoying.
> >
> > It would be good to have a definite answer on this. Since every time that
> > I try to hack on my PPP, I end changing my DTS and remember this patch :)
>
> So far people only reported "breaks my setup". I'm in a pickle here ;-) .
> Without anybody saying "I want to also move into this direction" I really
> feel I should not merge a patch that breaks other peoples setups.

Well, I'd prefer 1.5M baud rate as it is more consistent with other Rockchip
boards and it makes for a much more usable terminal experience when
logged in, it also doesn't affect boot times when serial is enabled with a
high loglevel and console on serial as 115200 does.

Though I'm just fine with using kernel's cmdline to set a baud rate.

*shrugs*

Best Regards,
Maya Matuszczyk

pt., 28 lip 2023 o 21:00 Heiko Stuebner <[email protected]> napisał(a):
>
> Hi Javier,
>
> Am Samstag, 22. Juli 2023, 01:06:54 CEST schrieb Javier Martinez Canillas:
> > Heiko Stübner <[email protected]> writes:
> > > Am Dienstag, 18. April 2023, 14:11:53 CEST schrieb Javier Martinez Canillas:
> > >> Heiko Stübner <[email protected]> writes:
> > >> > Am Dienstag, 4. April 2023, 14:52:02 CEST schrieb Peter Geis:
> > >> >> On Tue, Apr 4, 2023 at 3:55 AM Heiko Stübner <[email protected]> wrote:
> > >> >> > Am Montag, 3. April 2023, 19:59:37 CEST schrieb Javier Martinez Canillas:
> > >> >> > > This baud rate is set for the device by mainline u-boot and is also what
> > >> >> > > is set in the Pinebook Pro Device Tree, which is a device similar to the
> > >> >> > > PinePhone Pro but with a different form factor.
> > >> >> > >
> > >> >> > > Otherwise, the baud rate of the firmware and Linux don't match by default
> > >> >> > > and a 'console=ttyS2,1500000n8' kernel command line parameter is required
> > >> >> > > to have proper output for both.
> > >> >> >
> > >> >> > The interesting question is always if this will break someone else's setup.
> > >> >> > I've never really understood the strange setting of 1.5MBps, but on the
> > >> >> > other hand it _is_ a reality on most boards.
> > >> >
> > >> >> The 1.5M baud is default because the clock structure on rockchip
> > >> >> devices does not allow a clean 115200 baud. By attempting to force
> > >> >> 115200, it will always be slightly off (either low or high depending
> > >> >> on how the driver decided to round). If this actually causes any
> > >> >> problems is the subject of much debate.
> > >> >
> > >> > thanks so much for this piece of clock-detail. As I wrote, I never really
> > >> > understood the why _before_ but also never cared that much to dive
> > >> > into it and find out.
> > >> >
> > >> > So your explanation closes one knowledge gap in my head.
> > >> >
> > >> > Thanks a lot :-)
> > >>
> > >> Did you make a decision about this? I guess the clock explanation is yet
> > >> another argument in favour of switching the PPP to a 1.5 Mbps baud rate ?
> > >
> > > Sorry, but no decision made here. Either way it's breaking for someone,
> > > which makes this quite hard.
> > >
> >
> > Another ping on this patch.
> >
> > > The rate accuracy is the one side, the two-boot issue is the other side.
> > > And mainline u-boot (and levinboot - whatever that is) provides a 3rd side.
> > >
> > > People starting with the phone probably won't replace the bootloader
> > > in a first step but instead might play with a system image or newer kernel.
> > > So if the uart will break for everyone using the default bootloader from
> > > the factory that is somewhat bad.
> > >
> >
> > Probably won't replace the DTB shipped with the firmware either? If one is
> > replacing the firmware provided DTB witch the one in the mainline kernel,
> > probably such person is also using mainline u-boot?
>
> Not necessarily.
>
> I.e. putting an extlinux.conf on an sd-card with a kernel-image and dtb
> is not rocket science ;-)
>
>
> > > I don't have a Pinephone Pro myself, so I really hoped for some Acks
> > > or similar to appear in the meantime.
> > >
> >
> > For someone like me who is only using mainline u-boot, linux, etc then
> > having a consistent uart baud rate across all components is really useful.
> >
> > Otherwise I either have serial console for u-boot or the kernel, but can't
> > have both working so is annoying.
> >
> > It would be good to have a definite answer on this. Since every time that
> > I try to hack on my PPP, I end changing my DTS and remember this patch :)
>
> So far people only reported "breaks my setup". I'm in a pickle here ;-) .
> Without anybody saying "I want to also move into this direction" I really
> feel I should not merge a patch that breaks other peoples setups.
>
>
> Heiko
>
>
>
> _______________________________________________
> Linux-rockchip mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

2023-07-28 22:28:22

by Ondřej Jirman

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

Hi Heiko and Javier,

On Fri, Jul 28, 2023 at 08:59:50PM +0200, Heiko Stuebner wrote:
>
> Hi Javier,
>
> > Probably won't replace the DTB shipped with the firmware either? If one is
> > replacing the firmware provided DTB witch the one in the mainline kernel,
> > probably such person is also using mainline u-boot?
>
> Not necessarily.
>
> I.e. putting an extlinux.conf on an sd-card with a kernel-image and dtb
> is not rocket science ;-)
>
> > For someone like me who is only using mainline u-boot, linux, etc then
> > having a consistent uart baud rate across all components is really useful.
> >
> > Otherwise I either have serial console for u-boot or the kernel, but can't
> > have both working so is annoying.
> >
> > It would be good to have a definite answer on this. Since every time that
> > I try to hack on my PPP, I end changing my DTS and remember this patch :)
>
> So far people only reported "breaks my setup". I'm in a pickle here ;-) .
> Without anybody saying "I want to also move into this direction" I really
> feel I should not merge a patch that breaks other peoples setups.

Even if this is not changed in kernel, bootloader should fix this situation by
patching the kernel DTB before booting the kernel. U-Boot already patches the
DTB in several ways, so I guess it's possible to just change stdout-path either
in generic code, or in per-board ft_board_setup() for Pinephone Pro to something
that matches whatever the user configured via Kconfig:

https://elixir.bootlin.com/u-boot/latest/source/configs/pinephone-pro-rk3399_defconfig#L79

I mean that if U-Boot allows the user to configure arbitrary baudrate via
KConfig, then it *should* also update the kernel DTB to match, especially if the
choice is different from DTB that it actually embeds:

https://elixir.bootlin.com/u-boot/latest/source/arch/arm/dts/rk3399-pinephone-pro.dts#L29

No static baudrate choice in DT will save anyone anyway, with the provided flexibility
of configuration.

Maybe this would be a nice location to fix this:

https://elixir.bootlin.com/u-boot/v2023.07.02/source/common/fdt_support.c#L131

kind regards,
o.

>
> Heiko
>
>

2023-07-29 00:27:36

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB

Maya Matuszczyk <[email protected]> writes:

> Hi Heiko
>
> pt., 28 lip 2023 o 21:00 Heiko Stuebner <[email protected]> napisał(a):
>>

[...]

>> So far people only reported "breaks my setup". I'm in a pickle here ;-) .
>> Without anybody saying "I want to also move into this direction" I really
>> feel I should not merge a patch that breaks other peoples setups.
>
> Well, I'd prefer 1.5M baud rate as it is more consistent with other Rockchip
> boards and it makes for a much more usable terminal experience when
> logged in, it also doesn't affect boot times when serial is enabled with a
> high loglevel and console on serial as 115200 does.
>
> Though I'm just fine with using kernel's cmdline to set a baud rate.
>

Same, but also what Peter mentioned in this thread:

Peter Geis <[email protected]> writes:
>
> Good Morning Heiko,
>
> The 1.5M baud is default because the clock structure on rockchip
> devices does not allow a clean 115200 baud. By attempting to force
> 115200, it will always be slightly off (either low or high depending
> on how the driver decided to round). If this actually causes any
> problems is the subject of much debate.
>
> Very Respectfully,
> Peter Geis
>

So that's another argument for setting it to 1.5M. Anyways, I'll just stop
asking for this and set my cmdline to a non-default baud rate and move on.

I was just asking in case there was a decision made on this topic.

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat