Message ID | 20230403175937.2842085-1-javierm@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | arm64: dts: rockchip: Change serial baud rate for Pinephone Pro to 1.5 MB | expand |
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 <javierm@redhat.com> > --- > > 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 >
Heiko Stübner <heiko@sntech.de> 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 >
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/
On Tue, Apr 4, 2023 at 9:13 AM Jarrah <kernel@undef.tools> 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.
Jarrah <kernel@undef.tools> 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.
Peter Robinson <pbrobinson@gmail.com> writes: > On Tue, Apr 4, 2023 at 9:13 AM Jarrah <kernel@undef.tools> 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.
On Tue, Apr 4, 2023 at 9:24 AM Javier Martinez Canillas <javierm@redhat.com> wrote: > > Jarrah <kernel@undef.tools> 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.
Peter Robinson <pbrobinson@gmail.com> writes: > On Tue, Apr 4, 2023 at 9:24 AM Javier Martinez Canillas > <javierm@redhat.com> 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.
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 <javierm@redhat.com> >> --- >> >> 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 >> > > >
Martijn Braam <martijn@brixit.nl> 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.
Hello, On Tue, Apr 04, 2023 at 01:42:36PM +0200, Javier Martinez Canillas wrote: > Martijn Braam <martijn@brixit.nl> 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 >
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 <javierm@redhat.com> > > --- > > > > 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 > > > > > >
On Tue, Apr 4, 2023 at 3:55 AM Heiko Stübner <heiko@sntech.de> 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 <javierm@redhat.com> > > --- > > > > 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 > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip
Ondřej Jirman <megi@xff.cz> 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. >
On Tue, Apr 04, 2023 at 04:04:53PM +0200, Javier Martinez Canillas wrote: > Ondřej Jirman <megi@xff.cz> 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 >
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 <heiko@sntech.de> 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
Heiko Stübner <heiko@sntech.de> 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 <heiko@sntech.de> 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 ?
Hi, Am Dienstag, 18. April 2023, 14:11:53 CEST schrieb Javier Martinez Canillas: > Heiko Stübner <heiko@sntech.de> 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 <heiko@sntech.de> 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
Heiko Stübner <heiko@sntech.de> writes: Hello Heiko, > Hi, > > Am Dienstag, 18. April 2023, 14:11:53 CEST schrieb Javier Martinez Canillas: >> Heiko Stübner <heiko@sntech.de> 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 <heiko@sntech.de> 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.
Hi Javier, Am Samstag, 22. Juli 2023, 01:06:54 CEST schrieb Javier Martinez Canillas: > Heiko Stübner <heiko@sntech.de> writes: > > Am Dienstag, 18. April 2023, 14:11:53 CEST schrieb Javier Martinez Canillas: > >> Heiko Stübner <heiko@sntech.de> 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 <heiko@sntech.de> 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
Hi Heiko pt., 28 lip 2023 o 21:00 Heiko Stuebner <heiko@sntech.de> napisał(a): > > Hi Javier, > > Am Samstag, 22. Juli 2023, 01:06:54 CEST schrieb Javier Martinez Canillas: > > Heiko Stübner <heiko@sntech.de> writes: > > > Am Dienstag, 18. April 2023, 14:11:53 CEST schrieb Javier Martinez Canillas: > > >> Heiko Stübner <heiko@sntech.de> 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 <heiko@sntech.de> 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 <heiko@sntech.de> napisał(a): > > Hi Javier, > > Am Samstag, 22. Juli 2023, 01:06:54 CEST schrieb Javier Martinez Canillas: > > Heiko Stübner <heiko@sntech.de> writes: > > > Am Dienstag, 18. April 2023, 14:11:53 CEST schrieb Javier Martinez Canillas: > > >> Heiko Stübner <heiko@sntech.de> 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 <heiko@sntech.de> 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 > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip
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 > >
Maya Matuszczyk <maccraft123mc@gmail.com> writes: > Hi Heiko > > pt., 28 lip 2023 o 21:00 Heiko Stuebner <heiko@sntech.de> 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 <pgwipeout@gmail.com> 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.
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 {
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 <javierm@redhat.com> --- 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(-) base-commit: 3adf89324a2b2a9dbc2c12d8895021e7e34e3346