tty/serial: of_serial: add DT alias ID handling
PATCH 1/3 tty/serial: of_serial: add DT alias ID handling
Add support for alias parsing from the DT. This allows for consistent tty numbering.
--- drivers/tty/serial/of_serial.c | 4 ++++ 1 file changed, 4 insertions(+)
drivers/tty/serial/of_serial.c +4 -0
@@ -128,6 +128,10 @@ static int of_platform_serial_setup(struct platform_device *ofdev,if (of_find_property (np,"no-loopback-test" ,NULL )) port->flags |=UPF_SKIP_TEST ; + ret =of_alias_get_id (np,"serial" ); +if (ret >=0 ) + port->line = ret; + port->dev = &ofdev->dev;switch (type) {
Signature · 1 line
2.1.0
PATCH 2/3 tty/serial: of_serial: add support for PXA/MMP uarts
Add mrvl,pxa-uart and mrvl,mmp-uart compatible strings for the of_serial driver. These are 8250 variants which have a port type of PORT_XSCALE.
There is also the serial driver pxa.c with these compatible strings already. However, it can be replaced with the common 8250 driver. It has some issues like it cannot coexist with the 8250 driver due to tty name collision. That also means adding these compatible strings here should not case a problem.
--- drivers/tty/serial/of_serial.c | 4 ++++ 1 file changed, 4 insertions(+)
drivers/tty/serial/of_serial.c +4 -0
@@ -335,6 +335,10 @@ static struct of_device_id of_platform_serial_table[] = { .data = (void *)PORT_ALTR_16550_F64, }, { .compatible = "altr,16550-FIFO128", .data = (void *)PORT_ALTR_16550_F128, }, + { .compatible = "mrvl,mmp-uart", + .data = (void *)PORT_XSCALE, }, + { .compatible = "mrvl,pxa-uart", + .data = (void *)PORT_XSCALE, }, #ifdef CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL { .compatible = "ibm,qpace-nwp-serial", .data = (void *)PORT_NWPSERIAL, },
Signature · 1 line
2.1.0
PATCH 2/3 tty/serial: of_serial: add support for PXA/MMP uarts
Hi Rob,
Quoted 8 lines · Rob Herring
Add mrvl,pxa-uart and mrvl,mmp-uart compatible strings for the
of_serialdriver. These are 8250 variants which have a port type ofPORT_XSCALE.There is also the serial driver
pxa.cwith these compatible strings already. However, it can be replaced with the common 8250 driver. It has some issues like it cannot coexist with the 8250 driver due to tty name collision. That also means adding these compatible strings here should not case a problem.
So what determines which driver is controlling the port if both drivers are built-in?
Regards,
Peter Hurley
Quoted 24 lines
Signed-off-by Rob Herring <[email protected]>Cc Greg Kroah-Hartman <[email protected]>Cc Jiri Slaby <[email protected]>Cc linux-serial at vger.kernel.org--- drivers/tty/serial/of_serial.c | 4 ++++ 1 file changed, 4 insertions(+)drivers/tty/serial/of_serial.c +4
-0@@ -335,6 +335,10 @@ static struct of_device_id of_platform_serial_table[] = { .data = (void *)PORT_ALTR_16550_F64, }, { .compatible = "altr,16550-FIFO128", .data = (void *)PORT_ALTR_16550_F128, }, + { .compatible = "mrvl,mmp-uart", + .data = (void *)PORT_XSCALE, }, + { .compatible = "mrvl,pxa-uart", + .data = (void *)PORT_XSCALE, }, #ifdef CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL { .compatible = "ibm,qpace-nwp-serial", .data = (void *)PORT_NWPSERIAL, },
PATCH 2/3 tty/serial: of_serial: add support for PXA/MMP uarts
Quoted 6 lines · Peter Hurley
Hi Rob,
Quoted 3 lines · Rob Herring
Add mrvl,pxa-uart and mrvl,mmp-uart compatible strings for the
of_serialdriver. These are 8250 variants which have a port type ofPORT_XSCALE.
Quoted 8 lines
Quoted 5 lines
There is also the serial driver
pxa.cwith these compatible strings already. However, it can be replaced with the common 8250 driver. It has some issues like it cannot coexist with the 8250 driver due to tty name collision. That also means adding these compatible strings here should not case a problem.So what determines which driver is controlling the port if both drivers are built-in?
If both are built-in, whoever registers ttyS ports first wins. This will be the 8250 driver due to link order. So I guess PXA systems have avoided this by never building in 8250 driver.
Rob
Quoted 29 lines
Regards,
Peter HurleyQuoted 24 lines
Signed-off-by Rob Herring <[email protected]>Cc Greg Kroah-Hartman <[email protected]>Cc Jiri Slaby <[email protected]>Cc linux-serial at vger.kernel.org--- drivers/tty/serial/of_serial.c | 4 ++++ 1 file changed, 4 insertions(+)drivers/tty/serial/of_serial.c +4
-0@@ -335,6 +335,10 @@ static struct of_device_id of_platform_serial_table[] = { .data = (void *)PORT_ALTR_16550_F64, }, { .compatible = "altr,16550-FIFO128", .data = (void *)PORT_ALTR_16550_F128, }, + { .compatible = "mrvl,mmp-uart", + .data = (void *)PORT_XSCALE, }, + { .compatible = "mrvl,pxa-uart", + .data = (void *)PORT_XSCALE, }, #ifdef CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL { .compatible = "ibm,qpace-nwp-serial", .data = (void *)PORT_NWPSERIAL, },
PATCH 2/3 tty/serial: of_serial: add support for PXA/MMP uarts
Quoted 13 lines · Rob Herring
Quoted 12 lines · Peter Hurley
Hi Rob,
Quoted 8 lines · Rob Herring
Add mrvl,pxa-uart and mrvl,mmp-uart compatible strings for the
of_serialdriver. These are 8250 variants which have a port type ofPORT_XSCALE.There is also the serial driver
pxa.cwith these compatible strings already. However, it can be replaced with the common 8250 driver. It has some issues like it cannot coexist with the 8250 driver due to tty name collision. That also means adding these compatible strings here should not case a problem.
Quoted 5 lines
Quoted 2 lines
So what determines which driver is controlling the port if both drivers are built-in?
If both are built-in, whoever registers ttyS ports first wins. This will be the 8250 driver due to link order.
Ok, but then I think the commit log should reflect that this patch effectively replaces pxa2xx-uart driver with the 8250 driver for PXA/MMP uarts if both drivers are built-in or both drivers are modules.
Quoted 1 line
So I guess PXA systems have avoided this by never building in 8250 driver.
Platform devices are initialized first, so before this patch the pxa2xx-uart driver would have claimed the platform ports before the 8250 driver, if both were built-in (or both modules).
Maybe Kconfig should warn if they're both built-in or both modules?
Regards,
Peter Hurley
Quoted 25 lines
Quoted 25 lines
Quoted 24 lines
Signed-off-by Rob Herring <[email protected]>Cc Greg Kroah-Hartman <[email protected]>Cc Jiri Slaby <[email protected]>Cc linux-serial at vger.kernel.org--- drivers/tty/serial/of_serial.c | 4 ++++ 1 file changed, 4 insertions(+)drivers/tty/serial/of_serial.c +4
-0@@ -335,6 +335,10 @@ static struct of_device_id of_platform_serial_table[] = { .data = (void *)PORT_ALTR_16550_F64, }, { .compatible = "altr,16550-FIFO128", .data = (void *)PORT_ALTR_16550_F128, }, + { .compatible = "mrvl,mmp-uart", + .data = (void *)PORT_XSCALE, }, + { .compatible = "mrvl,pxa-uart", + .data = (void *)PORT_XSCALE, }, #ifdef CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL { .compatible = "ibm,qpace-nwp-serial", .data = (void *)PORT_NWPSERIAL, },
PATCH 2/3 tty/serial: of_serial: add support for PXA/MMP uarts
Quoted 17 lines · Peter Hurley
Quoted 16 lines · Rob Herring
Quoted 14 lines · Peter Hurley
Hi Rob,
Quoted 8 lines · Rob Herring
Add mrvl,pxa-uart and mrvl,mmp-uart compatible strings for the
of_serialdriver. These are 8250 variants which have a port type ofPORT_XSCALE.There is also the serial driver
pxa.cwith these compatible strings already. However, it can be replaced with the common 8250 driver. It has some issues like it cannot coexist with the 8250 driver due to tty name collision. That also means adding these compatible strings here should not case a problem.So what determines which driver is controlling the port if both drivers are built-in?
Quoted 6 lines
Quoted 2 lines
If both are built-in, whoever registers ttyS ports first wins. This will be the 8250 driver due to link order.
Ok, but then I think the commit log should reflect that this patch effectively replaces pxa2xx-uart driver with the 8250 driver for PXA/MMP uarts if both drivers are built-in or both drivers are modules.
It doesn't always. It is only on DT enabled platforms. But I can be more explicit about that.
There is another patch to remove the PXA driver entirely which I'm inquiring about why it was never merged [1]. This is why I went down the path of getting the 8250 driver to work rather than trying to fix the PXA driver.
Quoted 5 lines
Quoted 1 line
So I guess PXA systems have avoided this by never building in 8250 driver.
Platform devices are initialized first, so before this patch the pxa2xx-uart driver would have claimed the platform ports before the 8250 driver, if both were built-in (or both modules).
That's not exactly what I observed. The console is killed when the real driver initializes. I have no other debug output capability after that, so I'm not completely sure what was happening. Part of the problem is you need to move uart_register_driver to probe from the initcall as this is what other serial drivers sharing ttyS have done. Using 8250 earlycon with the PXA driver seems to cause problems, so I'd have to duplicate the earlycon support in the PXA driver.
Quoted 1 line
Maybe
Kconfigshould warn if they're both built-in or both modules?
Is there a way to do that?
Rob
PATCH 2/3 tty/serial: of_serial: add support for PXA/MMP uarts
Quoted 25 lines · Rob Herring
Quoted 17 lines · Peter Hurley
Quoted 16 lines · Rob Herring
Quoted 14 lines · Peter Hurley
Hi Rob,
Quoted 8 lines · Rob Herring
Add mrvl,pxa-uart and mrvl,mmp-uart compatible strings for the
of_serialdriver. These are 8250 variants which have a port type ofPORT_XSCALE.There is also the serial driver
pxa.cwith these compatible strings already. However, it can be replaced with the common 8250 driver. It has some issues like it cannot coexist with the 8250 driver due to tty name collision. That also means adding these compatible strings here should not case a problem.So what determines which driver is controlling the port if both drivers are built-in?
Quoted 6 lines
Quoted 2 lines
If both are built-in, whoever registers ttyS ports first wins. This will be the 8250 driver due to link order.
Ok, but then I think the commit log should reflect that this patch effectively replaces pxa2xx-uart driver with the 8250 driver for PXA/MMP uarts if both drivers are built-in or both drivers are modules.
Quoted 7 lines
It doesn't always. It is only on DT enabled platforms. But I can be more explicit about that.
There is another patch to remove the PXA driver entirely which I'm inquiring about why it was never merged [1]. This is why I went down the path of getting the 8250 driver to work rather than trying to fix the PXA driver.
Ah. I had seen your inquiry but didn't grasp the significance. Greg acked that patch over 8 mos ago; it's still stuck somewhere?
OTOH, I didn't realize that patch removed the PXA driver. That driver has errata workarounds which didn't get integrated into the 8250 core; it seems a shame to toss that away.
[ Sidebar: I did a trial split of 8250_core into a separate 8250 base module
and an 8250 universal driver. I keep having to rework the split but it's pretty close now; the 8250 base module might be just the thing for simplifying the pxa driver and getting console/earlycon/dma for free.
Let me see what I can do about getting that done.
]
Quoted 11 lines
Quoted 5 lines
Quoted 1 line
So I guess PXA systems have avoided this by never building in 8250 driver.
Platform devices are initialized first, so before this patch the pxa2xx-uart driver would have claimed the platform ports before the 8250 driver, if both were built-in (or both modules).
That's not exactly what I observed. The console is killed when the real driver initializes. I have no other debug output capability after that, so I'm not completely sure what was happening. Part of the problem is you need to move
uart_register_driverto probe from the initcall as this is what other serial drivers sharing ttyS have done.
Ah, yep. I missed that subtlety.
Quoted 6 lines
Using 8250 earlycon with the PXA driver seems to cause problems, so I'd have to duplicate the earlycon support in the PXA driver.
Quoted 1 line
Maybe
Kconfigshould warn if they're both built-in or both modules?Is there a way to do that?
I think so; conf warns if one setting overrides another, so I think there's probably a way to make that happen in the Kconfig. I'll mess with that later tonight or tomorrow, and get back to you.
Regards,
Peter Hurley
PATCH 2/3 tty/serial: of_serial: add support for PXA/MMP uarts
Quoted 29 lines · Peter Hurley
Quoted 28 lines · Rob Herring
Quoted 17 lines · Peter Hurley
Quoted 16 lines · Rob Herring
Quoted 14 lines · Peter Hurley
Hi Rob,
Quoted 8 lines · Rob Herring
Add mrvl,pxa-uart and mrvl,mmp-uart compatible strings for the
of_serialdriver. These are 8250 variants which have a port type ofPORT_XSCALE.There is also the serial driver
pxa.cwith these compatible strings already. However, it can be replaced with the common 8250 driver. It has some issues like it cannot coexist with the 8250 driver due to tty name collision. That also means adding these compatible strings here should not case a problem.So what determines which driver is controlling the port if both drivers are built-in?
Quoted 6 lines
Quoted 2 lines
If both are built-in, whoever registers ttyS ports first wins. This will be the 8250 driver due to link order.
Ok, but then I think the commit log should reflect that this patch effectively replaces pxa2xx-uart driver with the 8250 driver for PXA/MMP uarts if both drivers are built-in or both drivers are modules.
It doesn't always. It is only on DT enabled platforms. But I can be more explicit about that.
Quoted 7 lines
Quoted 4 lines
There is another patch to remove the PXA driver entirely which I'm inquiring about why it was never merged [1]. This is why I went down the path of getting the 8250 driver to work rather than trying to fix the PXA driver.
Ah. I had seen your inquiry but didn't grasp the significance. Greg acked that patch over 8 mos ago; it's still stuck somewhere?
It appears to me that this patch got added as a dependency to the rest of the series which never got merged. Guess they lost interest.
Quoted 3 lines
OTOH, I didn't realize that patch removed the PXA driver. That driver has errata workarounds which didn't get integrated into the 8250 core; it seems a shame to toss that away.
There was some discussion on errata in the prior versions. One of them was deemed not needed. Another one is needed, but that's on certain PXA2xx. So we'll need to pickup that work-around at some point, but it is not yet needed here because PXA2xx is not enabled for DT.
Quoted 7 lines
[ Sidebar: I did a trial split of 8250_core into a separate 8250 base module
and an 8250 universal driver. I keep having to rework the split but it's pretty close now; the 8250 base module might be just the thing for simplifying the pxa driver and getting console/earlycon/dma for free.Let me see what I can do about getting that done.
]
Yes, it would be nice to split the legacy ISA parts from the proper platform driver. There's way too many kconfig options for the 8250 as well IMO.
BTW, It appears that the main motivator for a separate driver was to add DMA support and that's what the out of tree driver adds. But now, we would never add the PXA DMA API support and the 8250 driver has learned DMA support.
Rob
PATCH 2/3 tty/serial: of_serial: add support for PXA/MMP uarts
Quoted 6 lines · Rob Herring
Quoted 5 lines · Peter Hurley
Quoted 4 lines · Rob Herring
Quoted 3 lines · Peter Hurley
Quoted 2 lines · Rob Herring
Quoted 1 line · Peter Hurley
On 01/26/2015 11:50 PM, Rob Herring wrote:
[...]
Quoted 8 lines
Quoted 3 lines
OTOH, I didn't realize that patch removed the PXA driver. That driver has errata workarounds which didn't get integrated into the 8250 core; it seems a shame to toss that away.
There was some discussion on errata in the prior versions. One of them was deemed not needed. Another one is needed, but that's on certain PXA2xx. So we'll need to pickup that work-around at some point, but it is not yet needed here because PXA2xx is not enabled for DT.
Do you know offhand which errata? If not, nbd; I can do a little digging.
Quoted 11 lines
Quoted 7 lines
[ Sidebar: I did a trial split of 8250_core into a separate 8250 base module
and an 8250 universal driver. I keep having to rework the split but it's pretty close now; the 8250 base module might be just the thing for simplifying the pxa driver and getting console/earlycon/dma for free.Let me see what I can do about getting that done.
]Yes, it would be nice to split the legacy ISA parts from the proper platform driver. There's way too many kconfig options for the 8250 as well IMO.
After I finish the split, I can look at making most/all of those options dependent on the universal 8250 driver only.
Quoted 4 lines
BTW, It appears that the main motivator for a separate driver was to add DMA support and that's what the out of tree driver adds. But now, we would never add the PXA DMA API support and the 8250 driver has learned DMA support.
Yeah, PXA should not have been forked. But I say that with the benefit of hindsight.
Regards,
Peter Hurley
PATCH 2/3 tty/serial: of_serial: add support for PXA/MMP uarts
In reply to Rob HerringQuoted 1 line · Rob Herring
On Tue, Jan 27, 2015 at 9:09 AM, Peter Hurley <[email protected]> wrote:
[...]
Quoted 3 lines
Quoted 1 line
Maybe
Kconfigshould warn if they're both built-in or both modules?Is there a way to do that?
Well, it's not what I had in mind originally, but the diff below generates a broken dependencies warning without causing build problems:
$ scripts/kconfig/mconf Kconfig warning: (SERIAL_PXA) selects TTYS_DRIVER_PXA which has unmet direct dependencies (TTY && HAS_IOMEM && !TTYS_DRIVER)
(My original idea was thwarted by the requirement that choice/endchoice requires prompts).
That said, for PXA, I think we agree splitting out a standalone 8250 platform driver is the solution.
Regards,
Peter Hurley
--
drivers/tty/serial/8250/Kconfig +1 -0
@@ -6,6 +6,7 @@ config SERIAL_8250 tristate "8250/16550 and compatible serial support" select SERIAL_CORE + select TTYS_DRIVER ---help--- This selects whether you want to include the driver for the standard serial ports. The standard answer is Y. People who might say N
drivers/tty/serial/Kconfig +8 -0
@@ -7,6 +7,13 @@ if TTY menu "Serial drivers" depends on HAS_IOMEM +config TTYS_DRIVER + tristate + +config TTYS_DRIVER_PXA + tristate + depends on !TTYS_DRIVER + config SERIAL_EARLYCON bool help
@@ -438,6 +445,7 @@ config SERIAL_PXA bool "PXA serial port support" depends on ARCH_PXA || ARCH_MMP select SERIAL_CORE + select TTYS_DRIVER_PXA help If you have a machine based on an Intel XScale PXA2xx CPU you can enable its onboard serial ports by enabling this option.
PATCH 2/3 tty/serial: of_serial: add support for PXA/MMP uarts
Quoted 9 lines · Peter Hurley
Quoted 1 line · Rob Herring
On Tue, Jan 27, 2015 at 9:09 AM, Peter Hurley <[email protected]> wrote:
[...]
Quoted 3 lines
Quoted 1 line
Maybe
Kconfigshould warn if they're both built-in or both modules?Is there a way to do that?
Quoted 7 lines
Well, it's not what I had in mind originally, but the diff below generates a broken dependencies warning without causing build problems:
$ scripts/kconfig/mconf Kconfig warning: (SERIAL_PXA) selects TTYS_DRIVER_PXA which has unmet direct dependencies (TTY && HAS_IOMEM && !TTYS_DRIVER)(My original idea was thwarted by the requirement that choice/endchoice requires prompts).
Okay, but this should be a separate patch. The problem exists with or without my patch.
Quoted 2 lines
That said, for PXA, I think we agree splitting out a standalone 8250 platform driver is the solution.
So after all the discussion, you are okay with the original patch?
Rob
Quoted 45 lines
Regards,
Peter Hurley--
drivers/tty/serial/8250/Kconfig +1
-0@@ -6,6 +6,7 @@ config SERIAL_8250 tristate "8250/16550 and compatible serial support" select SERIAL_CORE + select TTYS_DRIVER ---help--- This selects whether you want to include the driver for the standard serial ports. The standard answer is Y. People who might say Ndrivers/tty/serial/Kconfig +8
-0@@ -7,6 +7,13 @@ if TTY menu "Serial drivers" depends on HAS_IOMEM +config TTYS_DRIVER + tristate + +config TTYS_DRIVER_PXA + tristate + depends on !TTYS_DRIVER + config SERIAL_EARLYCON bool help@@ -438,6 +445,7 @@ config SERIAL_PXA bool "PXA serial port support" depends on ARCH_PXA || ARCH_MMP select SERIAL_CORE + select TTYS_DRIVER_PXA help If you have a machine based on an Intel XScale PXA2xx CPU you can enable its onboard serial ports by enabling this option.
PATCH 2/3 tty/serial: of_serial: add support for PXA/MMP uarts
Quoted 15 lines · Rob Herring
Quoted 14 lines · Peter Hurley
Quoted 1 line · Rob Herring
On Tue, Jan 27, 2015 at 9:09 AM, Peter Hurley <[email protected]> wrote:
[...]
Quoted 3 lines
Quoted 1 line
Maybe
Kconfigshould warn if they're both built-in or both modules?Is there a way to do that?
Well, it's not what I had in mind originally, but the diff below generates a broken dependencies warning without causing build problems:
$ scripts/kconfig/mconf Kconfig warning: (SERIAL_PXA) selects TTYS_DRIVER_PXA which has unmet direct dependencies (TTY && HAS_IOMEM && !TTYS_DRIVER)
Quoted 5 lines
Quoted 2 lines
(My original idea was thwarted by the requirement that choice/endchoice requires prompts).
Okay, but this should be a separate patch. The problem exists with or without my patch.
Yeah, don't worry about this. The solution of how to deal with multiple ttyS drivers needs to address this problem.
Quoted 4 lines
Quoted 2 lines
That said, for PXA, I think we agree splitting out a standalone 8250 platform driver is the solution.
So after all the discussion, you are okay with the original patch?
With a more specific commit log, yes. At a minimum, noting under what circumstances the 8250 driver replaces the pxa2xx-uart driver. (And maybe noting that this doesn't break hardware that needs those workarounds in pxa.c).
Regards,
Peter Hurley
PATCH 2/3 tty/serial: of_serial: add support for PXA/MMP uarts
Quoted 9 lines · Peter Hurley
Quoted 3 lines · Rob Herring
Quoted 1 line · Peter Hurley
On 01/27/2015 11:44 AM, Rob Herring wrote:
So after all the discussion, you are okay with the original patch?
With a more specific commit log, yes. At a minimum, noting under what circumstances the 8250 driver replaces the pxa2xx-uart driver. (And maybe noting that this doesn't break hardware that needs those workarounds in
pxa.c).
nvm; upstreamed.
PATCH 3/3 tty/serial: 8250_early: Add support for PXA UARTs
In reply to Rob HerringThe PXA variant of the 8250 UART adds a UART enable bit which must not be cleared. Make the earlycon support maintain this bit if it is set. This implies some initialization of the UART, but we cannot unconditionally set the bit as some other variants require this bit to be clear for other functions.
--- drivers/tty/serial/8250/8250_early.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
drivers/tty/serial/8250/8250_early.c +5 -3
@@ -93,10 +93,10 @@ static void __init early_serial8250_write(struct console *console,struct uart_port *port = &early_device->port;unsigned int ier; -/* Save the IER and disable interrupts */ +/* Save the IER and disable interrupts preserving the UUE bit */ ier =serial8250_early_in (port,UART_IER );if (ier) -serial8250_early_out (port,UART_IER ,0 ); +serial8250_early_out (port,UART_IER , ier &UART_IER_UUE );uart_console_write (port, s, count, serial_putc);
@@ -127,9 +127,11 @@ static void __init init_port(struct earlycon_device *device)struct uart_port *port = &device->port;unsigned int divisor;unsigned char c; +unsigned int ier;serial8250_early_out (port,UART_LCR ,0x3 );/* 8n1 */ -serial8250_early_out (port,UART_IER ,0 );/* no interrupt */ + ier =serial8250_early_in (port,UART_IER ); +serial8250_early_out (port,UART_IER , ier &UART_IER_UUE );/* no interrupt */ serial8250_early_out (port,UART_FCR ,0 );/* no fifo */ serial8250_early_out (port,UART_MCR ,0x3 );/* DTR + RTS */
Signature · 1 line
2.1.0
PATCH 3/3 tty/serial: 8250_early: Add support for PXA UARTs
Quoted 5 lines · Rob Herring
The PXA variant of the 8250 UART adds a UART enable bit which must not be cleared. Make the earlycon support maintain this bit if it is set. This implies some initialization of the UART, but we cannot unconditionally set the bit as some other variants require this bit to be clear for other functions.
So if the bootloader doesn't enable the uart, then earlycon doesn't work? Or is there some other piece that is enabling the uart after earlycon setup (besides the driver, of course)?
Quoted 26 lines
Signed-off-by Rob Herring <[email protected]>Cc Greg Kroah-Hartman <[email protected]>Cc Jiri Slaby <[email protected]>Cc linux-serial at vger.kernel.org--- drivers/tty/serial/8250/8250_early.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)drivers/tty/serial/8250/8250_early.c +2
-2@@ -93,10 +93,10 @@ static void __init early_serial8250_write(struct console *console,struct uart_port *port = &early_device->port;unsigned int ier; -/* Save the IER and disable interrupts */ +/* Save the IER and disable interrupts preserving the UUE bit */ ier =serial8250_early_in (port,UART_IER );if (ier) -serial8250_early_out (port,UART_IER ,0 ); +serial8250_early_out (port,UART_IER , ier &UART_IER_UUE );uart_console_write (port, s, count, serial_putc);
drivers/tty/serial/8250/8250_early.c static void __init init_port(struct earlycon_device *device) · line 134
Quoted 9 lines
drivers/tty/serial/8250/8250_early.c +3
-1@@ -127,9 +127,11 @@ static void __init init_port(struct earlycon_device *device)struct uart_port *port = &device->port;unsigned int divisor;unsigned char c; +unsigned int ier;serial8250_early_out (port,UART_LCR ,0x3 );/* 8n1 */ -serial8250_early_out (port,UART_IER ,0 );/* no interrupt */ + ier =serial8250_early_in (port,UART_IER ); +serial8250_early_out (port,UART_IER , ier &UART_IER_UUE );/* no interrupt */
The trailing comment seems no longer relevant; how about below?
/* Mask interrupts (preserve XScale's uart enable bit) */ serial8250_early_out(port, UART_IER, ier & UART_IER_UUE);
OTOH, if the uart is disabled, shouldn't earlycon be disabled?
Regards,
Peter Hurley
Quoted 4 lines
drivers/tty/serial/8250/8250_early.c +0
-0serial8250_early_out (port,UART_FCR ,0 );/* no fifo */ serial8250_early_out (port,UART_MCR ,0x3 );/* DTR + RTS */
PATCH 3/3 tty/serial: 8250_early: Add support for PXA UARTs
Quoted 8 lines · Peter Hurley
Quoted 5 lines · Rob Herring
The PXA variant of the 8250 UART adds a UART enable bit which must not be cleared. Make the earlycon support maintain this bit if it is set. This implies some initialization of the UART, but we cannot unconditionally set the bit as some other variants require this bit to be clear for other functions.
So if the bootloader doesn't enable the uart, then earlycon doesn't work?
Correct. Some earlycon drivers rely on this fact and don't do any initialization. There is a patch posted also to skip init for the 8250 earlycon. We could rely on that instead, but the 1st chunk is still needed.
The UART already being initialized should be pretty common for PXA based systems. More so than x86 UARTs. I think it is better to work under some conditions than none.
Quoted 34 lines
Or is there some other piece that is enabling the uart after earlycon setup (besides the driver, of course)?
Quoted 31 lines
Signed-off-by Rob Herring <[email protected]>Cc Greg Kroah-Hartman <[email protected]>Cc Jiri Slaby <[email protected]>Cc linux-serial at vger.kernel.org--- drivers/tty/serial/8250/8250_early.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)drivers/tty/serial/8250/8250_early.c +3
-2@@ -93,10 +93,10 @@ static void __init early_serial8250_write(struct console *console,struct uart_port *port = &early_device->port;unsigned int ier; -/* Save the IER and disable interrupts */ +/* Save the IER and disable interrupts preserving the UUE bit */ ier =serial8250_early_in (port,UART_IER );if (ier) -serial8250_early_out (port,UART_IER ,0 ); +serial8250_early_out (port,UART_IER , ier &UART_IER_UUE );uart_console_write (port, s, count, serial_putc);@@ -127,9 +127,11 @@ static void __init init_port(struct earlycon_device *device)struct uart_port *port = &device->port;unsigned int divisor;unsigned char c; +unsigned int ier;
Quoted 6 lines
drivers/tty/serial/8250/8250_early.c static void __init init_port(struct earlycon_device *device) · line 134
Quoted 4 lines
drivers/tty/serial/8250/8250_early.c +2
-1serial8250_early_out (port,UART_LCR ,0x3 );/* 8n1 */ -serial8250_early_out (port,UART_IER ,0 );/* no interrupt */ + ier =serial8250_early_in (port,UART_IER ); +serial8250_early_out (port,UART_IER , ier &UART_IER_UUE );/* no interrupt */ The trailing comment seems no longer relevant; how about below?
It is because we are still disabling interrupts.
Quoted 4 lines
/* Mask interrupts (preserve XScale's uart enable bit) */ serial8250_early_out(port, UART_IER, ier & UART_IER_UUE);OTOH, if the uart is disabled, shouldn't earlycon be disabled?
There's no way I can detect that.
Rob
PATCH 3/3 tty/serial: 8250_early: Add support for PXA UARTs
Quoted 10 lines · Rob Herring
Quoted 8 lines · Peter Hurley
Quoted 5 lines · Rob Herring
The PXA variant of the 8250 UART adds a UART enable bit which must not be cleared. Make the earlycon support maintain this bit if it is set. This implies some initialization of the UART, but we cannot unconditionally set the bit as some other variants require this bit to be clear for other functions.
So if the bootloader doesn't enable the uart, then earlycon doesn't work?
Quoted 8 lines
Correct. Some earlycon drivers rely on this fact and don't do any initialization. There is a patch posted also to skip init for the 8250 earlycon. We could rely on that instead, but the 1st chunk is still needed.
The UART already being initialized should be pretty common for PXA based systems. More so than x86 UARTs. I think it is better to work under some conditions than none.
Ok.
Quoted 34 lines
Quoted 34 lines
Or is there some other piece that is enabling the uart after earlycon setup (besides the driver, of course)?
Quoted 31 lines
Signed-off-by Rob Herring <[email protected]>Cc Greg Kroah-Hartman <[email protected]>Cc Jiri Slaby <[email protected]>Cc linux-serial at vger.kernel.org--- drivers/tty/serial/8250/8250_early.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)drivers/tty/serial/8250/8250_early.c +3
-2@@ -93,10 +93,10 @@ static void __init early_serial8250_write(struct console *console,struct uart_port *port = &early_device->port;unsigned int ier; -/* Save the IER and disable interrupts */ +/* Save the IER and disable interrupts preserving the UUE bit */ ier =serial8250_early_in (port,UART_IER );if (ier) -serial8250_early_out (port,UART_IER ,0 ); +serial8250_early_out (port,UART_IER , ier &UART_IER_UUE );uart_console_write (port, s, count, serial_putc);@@ -127,9 +127,11 @@ static void __init init_port(struct earlycon_device *device)struct uart_port *port = &device->port;unsigned int divisor;unsigned char c; +unsigned int ier;
Quoted 8 lines
Quoted 6 lines
drivers/tty/serial/8250/8250_early.c static void __init init_port(struct earlycon_device *device) · line 134
Quoted 4 lines
drivers/tty/serial/8250/8250_early.c +2
-1serial8250_early_out (port,UART_LCR ,0x3 );/* 8n1 */ -serial8250_early_out (port,UART_IER ,0 );/* no interrupt */ + ier =serial8250_early_in (port,UART_IER ); +serial8250_early_out (port,UART_IER , ier &UART_IER_UUE );/* no interrupt */ The trailing comment seems no longer relevant; how about below?
It is because we are still disabling interrupts.
I realize that, which is why my suggested replacement comment includes that information. It's not obvious what effect UART_IER_UUE has from just looking at the code.
Quoted 6 lines
Quoted 4 lines
/* Mask interrupts (preserve XScale's uart enable bit) */ serial8250_early_out(port, UART_IER, ier & UART_IER_UUE);OTOH, if the uart is disabled, shouldn't earlycon be disabled?
There's no way I can detect that.
Right, sorry.
[ I was about to write, "If UART_IER_UEE is not set, ..." which of course is true
for every other non-PXA 8250-base uart :)
]
Regards,
Peter Hurley