RFC: delete UART current-speed from 4xx DTS?

DISCUSSION Paul Gortmaker 3 participants 11 messages Sep 15, 2009 – Sep 23, 2009

Share & tools

Paul Gortmaker

author

2009-09-15 14:31 UTC #0 lore raw

One of the guys here was getting a messed up console on a bamboo board (on linux boot), which he traced to the fact that the default dts has a 9600 baudrate coded into it (board was running 115k2, not 9600). Either deleting the line, or replacing the 9600 with zero fixed the problem.

Looking at the Fsl boards, it seems that 99% of them don't list any current-speed at all. I'm willing to whip up a patch to delete them from the various boards, but I thought I'd check 1st to see if there was some other reasoning/input. I suppose one could argue that u-boot should be replacing the 9600 with 115k2 on the fly, but if the explicit specification isn't helping anyone, then it probably just makes sense to delete them anyway, I figured.

Here is the subset of boards I was considering deleting the lines from:

bamboo.dts ebony.dts hcu4.dts holly.dts katmai.dts sam440ep.dts sequoia.dts taishan.dts walnut.dts yosemite.dts

Thanks,
Paul.

Josh Boyer

2009-09-15 15:32 UTC #1 lore raw

Quoted 4 lines · Paul Gortmaker

One of the guys here was getting a messed up console on a bamboo board (on linux boot), which he traced to the fact that the default dts has a 9600 baudrate coded into it (board was running 115k2, not 9600). Either deleting the line, or replacing the 9600 with zero fixed the problem.

Once booted, was there a valid current-speed property in /proc/device-tree for the serial node? I'm curious if U-Boot created it, or if the kernel just used whatever baud was present already.

When I did the bamboo port a while ago, I recall having issues with either a missing clock-frequency or current-speed (or both perhaps) and the bootloader on the board was the original PIBS. It might have been an issue with PIBS but I'm guessing the rest of the 4xx boards copied from either Ebony or Bamboo in their ports and hence contain that property.

Quoted 2 lines

Looking at the Fsl boards, it seems that 99% of them don't list any current-speed at all. I'm willing to whip up a patch to delete them

I think 99% of them use U-Boot, which should fix things up either way.

Quoted 5 lines

from the various boards, but I thought I'd check 1st to see if there was some other reasoning/input. I suppose one could argue that u-boot should be replacing the 9600 with 115k2 on the fly, but if the explicit specification isn't helping anyone, then it probably just makes sense to delete them anyway, I figured.

Yeah, that's probably valid. I'd be happy to test on the set of boards I have.

Quoted 4 lines

Here is the subset of boards I was considering deleting the lines from:
bamboo.dts ebony.dts hcu4.dts holly.dts katmai.dts sam440ep.dts sequoia.dts taishan.dts walnut.dts yosemite.dts

I can test bamboo with PIBS, ebony, holly (which isn't 4xx even though it's a tree name) with PIBS, sequoia, taishan, yosemite, and walnut. Perhaps a few of the 405 boards I have as well.

It's easy enough for me to whip up a patch, and since I'll be testing either way I'd be happy to do that if you'd like.

josh

Paul Gortmaker

author

2009-09-15 19:32 UTC #2 lore raw

Quoted 2 lines · [Re: RFC: delete UART current-speed from 4xx DTS?] On 15/09/2009 (Tue 11:32)…

Quoted 1 line · Paul Gortmaker
One of the guys here was getting a messed up console on a bamboo board

I meant to say Yosemite board (and hence u-boot), sorry for that. It gives garbage up until the udbg -> ttyS0 handover, at which point the console=ttyS0,115200 fixes things.

Quoted 7 lines

Quoted 3 lines
(on linux boot), which he traced to the fact that the default dts has a 9600 baudrate coded into it (board was running 115k2, not 9600). Either deleting the line, or replacing the 9600 with zero fixed the problem.
Once booted, was there a valid current-speed property in /proc/device-tree for the serial node? I'm curious if U-Boot created it, or if the kernel just used whatever baud was present already.

Had to go back to get this info; it turns out there is a valid prop in all the serial nodes (2.6.31-rc7), and hexdumping it gives 0x2580 (9600).

Quoted 6 lines

When I did the bamboo port a while ago, I recall having issues with either a missing clock-frequency or current-speed (or both perhaps) and the bootloader on the board was the original PIBS. It might have been an issue with PIBS but I'm guessing the rest of the 4xx boards copied from either Ebony or Bamboo in their ports and hence contain that property.

Right - so there could still perhaps be the same issue with PIBS/bamboo present that you saw earlier (given my inability to keep board names straight)

Quoted 5 lines

Quoted 3 lines
Looking at the Fsl boards, it seems that 99% of them don't list any current-speed at all. I'm willing to whip up a patch to delete them
I think 99% of them use U-Boot, which should fix things up either way.

This is the interesting part -- being a yosemite (u-boot), I would have thought so as well. I've not looked at the u-boot code, but it may only add a current-speed if there isn't one yet. At least that is what the behaviour tends to indicate. Board is running u-boot 1.3.3 so what we are seeing here may not reflect what current u-boot code is doing anyway...

Quoted 15 lines

Quoted 6 lines
from the various boards, but I thought I'd check 1st to see if there was some other reasoning/input. I suppose one could argue that u-boot should be replacing the 9600 with 115k2 on the fly, but if the explicit specification isn't helping anyone, then it probably just makes sense to delete them anyway, I figured.
Yeah, that's probably valid. I'd be happy to test on the set of boards I have.
Quoted 4 lines
Here is the subset of boards I was considering deleting the lines from:
bamboo.dts ebony.dts hcu4.dts holly.dts katmai.dts sam440ep.dts sequoia.dts taishan.dts walnut.dts yosemite.dts

Quoted 6 lines

I can test bamboo with PIBS, ebony, holly (which isn't 4xx even though it's a tree name) with PIBS, sequoia, taishan, yosemite, and walnut. Perhaps a few of the 405 boards I have as well.
It's easy enough for me to whip up a patch, and since I'll be testing either way I'd be happy to do that if you'd like.

Sure -- the testing effort will be greater than the time to make the patch, so you doing coverage on all those would be great. I think I've probably only got easy immediate access to a PIBS/bamboo at the moment. We already know the yosemite is OK with the change, so that is one less to test.

Thanks,
Paul.

Quoted 2 lines

josh

Josh Boyer

2009-09-15 20:02 UTC #3 lore raw

Quoted 8 lines · Paul Gortmaker

Quoted 2 lines · [Re: RFC: delete UART current-speed from 4xx DTS?] On 15/09/2009 (Tue 11:32)…
Quoted 1 line · Paul Gortmaker
One of the guys here was getting a messed up console on a bamboo board
I meant to say Yosemite board (and hence u-boot), sorry for that. It gives garbage up until the udbg -> ttyS0 handover, at which point the console=ttyS0,115200 fixes things.

Ok.

Quoted 10 lines

Quoted 7 lines
Quoted 3 lines
(on linux boot), which he traced to the fact that the default dts has a 9600 baudrate coded into it (board was running 115k2, not 9600). Either deleting the line, or replacing the 9600 with zero fixed the problem.
Once booted, was there a valid current-speed property in /proc/device-tree for the serial node? I'm curious if U-Boot created it, or if the kernel just used whatever baud was present already.
Had to go back to get this info; it turns out there is a valid prop in all the serial nodes (2.6.31-rc7), and hexdumping it gives 0x2580 (9600).

Sorry, that was after you removed the property in the DTS entirely, or setting it to 0, or just using the existing DTS as-is? I should have phrased my question better, but I think I answered it myself already.

In my brief test with Sequoia and Bamboo, I removed the current-speed property entirely and once booted there was no property in the serial node, which is what I would expect for the old version of U-Boot on these boards. The good news is that it seems to work fine :).

Quoted 9 lines

Quoted 5 lines
When I did the bamboo port a while ago, I recall having issues with either a missing clock-frequency or current-speed (or both perhaps) and the bootloader on the board was the original PIBS. It might have been an issue with PIBS but I'm guessing the rest of the 4xx boards copied from either Ebony or Bamboo in their ports and hence contain that property.
Right - so there could still perhaps be the same issue with PIBS/bamboo present that you saw earlier (given my inability to keep board names straight)

OK. I'll be sure to test PIBS.

Quoted 5 lines

Quoted 5 lines
Quoted 4 lines
Looking at the Fsl boards, it seems that 99% of them don't list any current-speed at all. I'm willing to whip up a patch to delete them

Quoted 8 lines

Quoted 1 line
I think 99% of them use U-Boot, which should fix things up either way.
This is the interesting part -- being a yosemite (u-boot), I would have thought so as well. I've not looked at the u-boot code, but it may only add a current-speed if there isn't one yet. At least that is what the behaviour tends to indicate. Board is running u-boot 1.3.3 so what we are seeing here may not reflect what current u-boot code is doing anyway...

Yeah, 1.3.3 is pretty old. However, I _think_ the kernel will just leave the baud rate alone entirely without a current-speed property and without console=ttyS0,<baud rate>. Example:

linux:/proc/device-tree/plb/opb/serial@ef600300 # ls

clock-frequency  device_type       interrupts  reg                              
compatible       interrupt-parent  name        virtual-reg

linux:/proc/device-tree/plb/opb/serial@ef600300 # cat /proc/cmdline rootdelay=15 root=/dev/sda1 rw ip=off linux:/proc/device-tree/plb/opb/serial@ef600300 # uname -a Linux linux 2.6.31 #4 Tue Sep 15 16:57:49 UTC 2009 ppc ppc ppc GNU/Linux linux:/proc/device-tree/plb/opb/serial@ef600300 # cat /proc/cpuinfo
processor : 0
cpu : 440EP Rev. B
clock : 399.999996MHz
revision : 24.211 (pvr 4222 18d3)
bogomips : 799.99
timebase : 399999996
platform : PowerPC 44x Platform
model : amcc,bamboo
Memory : 191 MB
linux:/proc/device-tree/plb/opb/serial@ef600300 #

Quoted 4 lines

Quoted 4 lines
I can test bamboo with PIBS, ebony, holly (which isn't 4xx even though it's a tree name) with PIBS, sequoia, taishan, yosemite, and walnut. Perhaps a few of the 405 boards I have as well.

Quoted 8 lines

Quoted 2 lines
It's easy enough for me to whip up a patch, and since I'll be testing either way I'd be happy to do that if you'd like.
Sure -- the testing effort will be greater than the time to make the patch, so you doing coverage on all those would be great. I think I've probably only got easy immediate access to a PIBS/bamboo at the moment. We already know the yosemite is OK with the change, so that is one less to test.

OK, sounds good. I'll do some more testing over the next few days and post a patch. Thanks for bringing this to my attention.

josh

Paul Gortmaker

author

2009-09-15 20:44 UTC #4 lore raw

Quoted 23 lines · [Re: RFC: delete UART current-speed from 4xx DTS?] On 15/09/2009 (Tue 16:02)…

Quoted 8 lines · Paul Gortmaker
Quoted 2 lines · [Re: RFC: delete UART current-speed from 4xx DTS?] On 15/09/2009 (Tue 11:32)…
Quoted 1 line · Paul Gortmaker
One of the guys here was getting a messed up console on a bamboo board
I meant to say Yosemite board (and hence u-boot), sorry for that. It gives garbage up until the udbg -> ttyS0 handover, at which point the console=ttyS0,115200 fixes things.
Ok.
Quoted 10 lines
Quoted 7 lines
Quoted 3 lines
(on linux boot), which he traced to the fact that the default dts has a 9600 baudrate coded into it (board was running 115k2, not 9600). Either deleting the line, or replacing the 9600 with zero fixed the problem.
Once booted, was there a valid current-speed property in /proc/device-tree for the serial node? I'm curious if U-Boot created it, or if the kernel just used whatever baud was present already.
Had to go back to get this info; it turns out there is a valid prop in all the serial nodes (2.6.31-rc7), and hexdumping it gives 0x2580 (9600).

Quoted 8 lines

Sorry, that was after you removed the property in the DTS entirely, or setting it to 0, or just using the existing DTS as-is? I should have phrased my question better, but I think I answered it myself already.
In my brief test with Sequoia and Bamboo, I removed the current-speed property entirely and once booted there was no property in the serial node, which is what I would expect for the old version of U-Boot on these boards. The good news is that it seems to work fine :).

Yep - and if there is a node with a current-speed that differs from what u-boot (1.3.3) is actively using, that current-speed setting will take precedence for early printk and also that baud will appear in the /proc/device-tree (just to clarify on what I'd tried to convey earlier).

[...]

Quoted 11 lines

Quoted 8 lines
Quoted 2 lines
It's easy enough for me to whip up a patch, and since I'll be testing either way I'd be happy to do that if you'd like.
Sure -- the testing effort will be greater than the time to make the patch, so you doing coverage on all those would be great. I think I've probably only got easy immediate access to a PIBS/bamboo at the moment. We already know the yosemite is OK with the change, so that is one less to test.
OK, sounds good. I'll do some more testing over the next few days and post a patch. Thanks for bringing this to my attention.

No problem; thanks for the quick response and volunteering to test on the boards you have.

Paul.

Quoted 2 lines

josh

Benjamin Herrenschmidt

2009-09-15 21:44 UTC #5 lore raw

In reply to Josh Boyer

Quoted 10 lines · Josh Boyer

Quoted 4 lines · Paul Gortmaker
One of the guys here was getting a messed up console on a bamboo board (on linux boot), which he traced to the fact that the default dts has a 9600 baudrate coded into it (board was running 115k2, not 9600). Either deleting the line, or replacing the 9600 with zero fixed the problem.
Once booted, was there a valid current-speed property in /proc/device-tree for the serial node? I'm curious if U-Boot created it, or if the kernel just used whatever baud was present already.

Quoted 5 lines

When I did the bamboo port a while ago, I recall having issues with either a missing clock-frequency or current-speed (or both perhaps) and the bootloader on the board was the original PIBS. It might have been an issue with PIBS but I'm guessing the rest of the 4xx boards copied from either Ebony or Bamboo in their ports and hence contain that property.

I think I recently added code to legacy_serial probe the speed from the HW if the property is absent, which should help.

Cheers,
Ben.

Josh Boyer

2009-09-16 13:19 UTC #6 lore raw

Quoted 11 lines · Benjamin Herrenschmidt

Quoted 10 lines · Josh Boyer
Quoted 4 lines · Paul Gortmaker
One of the guys here was getting a messed up console on a bamboo board (on linux boot), which he traced to the fact that the default dts has a 9600 baudrate coded into it (board was running 115k2, not 9600). Either deleting the line, or replacing the 9600 with zero fixed the problem.
Once booted, was there a valid current-speed property in /proc/device-tree for the serial node? I'm curious if U-Boot created it, or if the kernel just used whatever baud was present already.

Quoted 8 lines

Quoted 5 lines
When I did the bamboo port a while ago, I recall having issues with either a missing clock-frequency or current-speed (or both perhaps) and the bootloader on the board was the original PIBS. It might have been an issue with PIBS but I'm guessing the rest of the 4xx boards copied from either Ebony or Bamboo in their ports and hence contain that property.
I think I recently added code to legacy_serial probe the speed from the HW if the property is absent, which should help.

Ok, so I think that is related to what I originally hit.

I played around with removing the current-speed property on canyonlands today, and noticed that I would get no console output at all unless I specified a baudrate with console=ttyS0,115200. That was sort of contrary to what I found with bamboo, so I diffed the configs to see why. Bamboo has udbg enabled and hence has legacy_serial builtin, whereas canyonlands just has of_serial.

So on boards where of_serial is the only serial driver, we need either an accurate current-speed property, or a specific baudrate on the command line. That makes a bit more tenuous to remove the properties entirely, because if people disable udbg and are relying on that behavior they get no more console output. Need to think on that a bit I guess.

Alternatively, we could try patching of_serial.c to do the baudrate probe as well.

josh

Paul Gortmaker

author

2009-09-16 14:57 UTC #7 lore raw

Quoted 12 lines · [Re: RFC: delete UART current-speed from 4xx DTS?] On 16/09/2009 (Wed 09:19)…

Quoted 10 lines · Benjamin Herrenschmidt
Quoted 6 lines · Josh Boyer
When I did the bamboo port a while ago, I recall having issues with either a missing clock-frequency or current-speed (or both perhaps) and the bootloader on the board was the original PIBS. It might have been an issue with PIBS but I'm guessing the rest of the 4xx boards copied from either Ebony or Bamboo in their ports and hence contain that property.
I think I recently added code to legacy_serial probe the speed from the HW if the property is absent, which should help.

Quoted 7 lines

Ok, so I think that is related to what I originally hit.
I played around with removing the current-speed property on canyonlands today, and noticed that I would get no console output at all unless I specified a baudrate with console=ttyS0,115200. That was sort of contrary to what I found with bamboo, so I diffed the configs to see why. Bamboo has udbg enabled and hence has legacy_serial builtin, whereas canyonlands just has of_serial.

Makes sense - udbg kind of bridges the information gap in getting the baudrate that the bootloader was using carried forward. I'd have never noticed that because I've always been in the habit of recommending that people always use an explicit console=ttySn,rate for all non PC like hardware.

I just saw Documentation/serial-console.txt (also somewhat PC-centric) claims that "If no console device is specified, the first device found capable of acting as a system console will be used." But I've never relied on that, or even tested if it is really a valid claim once you stray outside the realm of common PC hardware, and end user chosen baudrates. I'm pretty sure that on sbc8349, if I omit the console= and I don't have udbg enabled, I won't see anything either (and I've always implicitly filed that under "user configuration error").

Quoted 3 lines

So on boards where of_serial is the only serial driver, we need either an accurate current-speed property, or a specific baudrate on the command line.

I just took a rummage around in u-boot, and at the moment there isn't really many boards poking around with current-speed. Just for board/muas3001, and any older mpc85xx CPUs with CPM UARTs. There doesn't appear to be any real global trend there at all. (That is not to say that it couldn't be added though...) But in any case, I still don't think having something as end-user configurable as a baud rate should be hard coded in a dtb (esp. if the firmware isn't updating it on the fly...)

Quoted 3 lines

That makes a bit more tenuous to remove the properties entirely, because if people disable udbg and are relying on that behavior they get no more console output. Need to think on that a bit I guess.

Would be interesting to get other input from people on how big they think that user group who would be relying on this are. I'm thinking a lot of other boards have already tossed out any such guarantee. I'll have to do some experiments to confirm that though.

Quoted 2 lines

Alternatively, we could try patching of_serial.c to do the baudrate probe as well.

Improving it to "do the right thing" independent of the current behaviour of other boards is probably worthwhile in terms of user friendly improvements. It always sucks to launch into a kernel and see nothing but another case of "silent boot death".

Paul.

Quoted 2 lines

josh

Benjamin Herrenschmidt

2009-09-16 21:31 UTC #8 lore raw

In reply to Josh Boyer

Quoted 9 lines

Ok, so I think that is related to what I originally hit.
I played around with removing the current-speed property on canyonlands today, and noticed that I would get no console output at all unless I specified a baudrate with console=ttyS0,115200. That was sort of contrary to what I found with bamboo, so I diffed the configs to see why. Bamboo has udbg enabled and hence has legacy_serial builtin, whereas canyonlands just has of_serial.

Quoted 8 lines

So on boards where of_serial is the only serial driver, we need either an accurate current-speed property, or a specific baudrate on the command line. That makes a bit more tenuous to remove the properties entirely, because if people disable udbg and are relying on that behavior they get no more console output. Need to think on that a bit I guess.
Alternatively, we could try patching of_serial.c to do the baudrate probe as well.

Well, I've always wondered why we just don't put the probe in the 8250 driver... I proposed it on the list a while back and there was no serious objection, but then forgot about it :-)

We should do it, and check if the result is sane (looks like a standard speed). If it is, we should then just used it.

Ben.

Josh Boyer

2009-09-17 01:44 UTC #9 lore raw

Quoted 15 lines · Benjamin Herrenschmidt

Quoted 15 lines
Ok, so I think that is related to what I originally hit.
I played around with removing the current-speed property on canyonlands today, and noticed that I would get no console output at all unless I specified a baudrate with console=ttyS0,115200. That was sort of contrary to what I found with bamboo, so I diffed the configs to see why. Bamboo has udbg enabled and hence has legacy_serial builtin, whereas canyonlands just has of_serial.
So on boards where of_serial is the only serial driver, we need either an accurate current-speed property, or a specific baudrate on the command line. That makes a bit more tenuous to remove the properties entirely, because if people disable udbg and are relying on that behavior they get no more console output. Need to think on that a bit I guess.

Quoted 5 lines

Quoted 2 lines
Alternatively, we could try patching of_serial.c to do the baudrate probe as well.
Well, I've always wondered why we just don't put the probe in the 8250 driver... I proposed it on the list a while back and there was no

Probably because anything to do with the serial drivers or tty layer scares the living hell out of people other than Alan and Andrew. And I sometimes think even Andrew is faking...

;)

Quoted 4 lines

serious objection, but then forgot about it :-)
We should do it, and check if the result is sane (looks like a standard speed). If it is, we should then just used it.

Sounds sane enough. I'm hoping to get some time in the next few days to poke at this.

josh

Josh Boyer

2009-09-23 18:15 UTC #10 lore raw

In reply to Josh Boyer

Quoted 9 lines · Josh Boyer

Quoted 1 line · Benjamin Herrenschmidt
On Tue, 2009-09-15 at 11:32 -0400, Josh Boyer wrote:
Ok, so I think that is related to what I originally hit.
I played around with removing the current-speed property on canyonlands today, and noticed that I would get no console output at all unless I specified a baudrate with console=ttyS0,115200. That was sort of contrary to what I found with bamboo, so I diffed the configs to see why. Bamboo has udbg enabled and hence has legacy_serial builtin, whereas canyonlands just has of_serial.

Correcting myself again, there really isn't any difference from a driver perspective at a build level. Both Cayonlands and Bamboo have udbg selected. The difference is that Bamboo has linux,stdout-path specified in the choosen node and Caynonlands does not. Udbg looks for that, etc etc.

Quoted 2 lines

Alternatively, we could try patching of_serial.c to do the baudrate probe as well.

I looked at this. It's rather ugly, because of_serial.c doesn't really do anything from a hardware perspective. It simply binds to the device nodes and lets the underlying serial driver do all the hardware handling. We'd need to have it ioremap, probe, iounmap and that all seems rather silly.

Looking at the 8250 driver, there is a place we could autoprobe it. Something like the ugly patch below. It would probably need to be wrapped in a CONFIG option, but you get the idea.

Overall, I'm not sure having either is a requirement for removing the current-speed properties from the DTS files. The only thing I could see needing is telling of_serial.c to ignore a current-speed=0 property, as that seems to be pretty invalid, but what some versions of U-Boot do.

josh

---

drivers/serial/8250.c +21 -1

index 2209620..0cced56 100644
@@ -2778,7 +2778,7 @@ serial8250_console_write(struct console *co, const char *s, unsigned int count)
 static int __init serial8250_console_setup(struct console *co, char *options)
 {
 	struct uart_port *port;
-	int baud = 9600;
+	int baud = 0;
 	int bits = 8;
 	int parity = 'n';
 	int flow = 'n';

@@ -2797,6 +2797,26 @@ static int __init serial8250_console_setup(struct console *co, char *options)
 	if (options)
 		uart_parse_options(options, &baud, &parity, &bits, &flow);
 
+	if (baud == 0) {
+		unsigned char old_lcr, div, pre;
+		struct uart_8250_port *up = (struct uart_8250_port *) port;
+
+		old_lcr = serial_inp(up, UART_LCR);
+		serial_outp(up, UART_LCR, UART_LCR_DLAB);
+		div = serial_dl_read(up);
+		if (serial_inp(up, UART_MCR) & 0x80)
+			pre = 4;
+		else
+			pre = 1;
+
+		serial_outp(up, UART_LCR, old_lcr);
+
+		baud = (port->uartclk / pre) / (div * 16);
+
+		if (baud > (port->uartclk / 16))
+			baud = 9600;
+	}
+
 	return uart_set_options(port, co, baud, parity, bits, flow);
 }