serial: 8250_early: Add earlycon for BCM2835 aux uart

Commit Message

Matthias Brugger Jan. 26, 2020, 12:33 p.m. UTC

From: Matthias Brugger <[e-mail]>

Define the OF early console for BCM2835 aux UART, which can be enabled
by passing "earlycon" on the boot command line. This UART is found on
BCM283x and BCM27xx SoCs, a.k.a. Raspberry Pi in its variants.

Signed-off-by: Matthias Brugger <[e-mail]>

---

 drivers/tty/serial/8250/8250_bcm2835aux.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Matthias Brugger Jan. 26, 2020, 8:20 p.m. UTC | #1

On 26/01/2020 14:12, Lukas Wunner wrote:
> On Sun, Jan 26, 2020 at 01:33:14PM +0100, [e-mail] wrote:
>> +#ifdef CONFIG_SERIAL_8250_CONSOLE
>> +
>> +static int __init early_bcm2835aux_setup(struct earlycon_device *device,
>> +					const char *options)
>> +{
>> +	if (!device->port.membase)
>> +		return -ENODEV;
>> +
>> +	device->port.iotype = UPIO_MEM32;
>> +	device->port.regshift = 2;
>> +
>> +	return early_serial8250_setup(device, NULL);
>> +}
>> +
>> +OF_EARLYCON_DECLARE(bcm2835aux, "brcm,bcm2835-aux-uart",
>> +		    early_bcm2835aux_setup);
>> +#endif
> 
> Does this really work?  I also tried to get it working recently and
> the system just hung on boot.  Looking at it with a JTAG debugger
> showed that the bcm2835aux registers were inaccessible because
> the mini UART wasn't enabled in the AUXENB register.
> 
> Maybe if you use OF_EARLYCON_DECLARE, the firmware recognizes that
> serial1 is set as stdout-path and performs enablement of the mini UART?
> Or are you using U-Boot which perhaps does the enablement?

Yes I'm using U-Boot which enables the console for me. My understanding is that
the early console is thought as a re-use of the console the boot FW used for
logging. AFAIK for example it does not enable any needed clocks but expects
these to be enabled already.

Looking on the source code of U-Boot [1] I don't see that the AUXENB is written
somewhere, so I suppose that the FW should already has enabled the aux-uart.

I any case if it's just to set one bit, I think we can do that in
early_bcm2835aux_setup().

[1]
https://gitlab.denx.de/u-boot/u-boot/blob/master/drivers/serial/serial_bcm283x_mu.c

> 
> I also saw in the JTAG debugger that the uartclk member contained
> an incorrect value, so I'd expect that it has to be set as well in
> early_bcm2835aux_setup().

In my case the clock was set by U-Boot already.

Regards,
Matthias

> 
> I'll see to it that I give this patch a whirl when I return to the
> office next week.
> 
> Thanks,
> 
> Lukas
>

Nicolas Saenz Julienne Jan. 28, 2020, 12:42 p.m. UTC | #2

On Sun Jan 26, 2020 at 9:20 PM, Matthias Brugger wrote:
>
>
> On 26/01/2020 14:12, Lukas Wunner wrote:
> > On Sun, Jan 26, 2020 at 01:33:14PM +0100, [e-mail] wrote:
> >> +#ifdef CONFIG_SERIAL_8250_CONSOLE
> >> +
> >> +static int __init early_bcm2835aux_setup(struct earlycon_device *device,
> >> +					const char *options)
> >> +{
> >> +	if (!device->port.membase)
> >> +		return -ENODEV;
> >> +
> >> +	device->port.iotype = UPIO_MEM32;
> >> +	device->port.regshift = 2;
> >> +
> >> +	return early_serial8250_setup(device, NULL);
> >> +}
> >> +
> >> +OF_EARLYCON_DECLARE(bcm2835aux, "brcm,bcm2835-aux-uart",
> >> +		    early_bcm2835aux_setup);
> >> +#endif
> > 
> > Does this really work?  I also tried to get it working recently and
> > the system just hung on boot.  Looking at it with a JTAG debugger
> > showed that the bcm2835aux registers were inaccessible because
> > the mini UART wasn't enabled in the AUXENB register.
> > 
> > Maybe if you use OF_EARLYCON_DECLARE, the firmware recognizes that
> > serial1 is set as stdout-path and performs enablement of the mini UART?
> > Or are you using U-Boot which perhaps does the enablement?
>
> Yes I'm using U-Boot which enables the console for me. My understanding
> is that
> the early console is thought as a re-use of the console the boot FW used
> for
> logging. AFAIK for example it does not enable any needed clocks but
> expects
> these to be enabled already.
>
> Looking on the source code of U-Boot [1] I don't see that the AUXENB is
> written
> somewhere, so I suppose that the FW should already has enabled the
> aux-uart.
>
> I any case if it's just to set one bit, I think we can do that in
> early_bcm2835aux_setup().
>
> [1]
> https://gitlab.denx.de/u-boot/u-boot/blob/master/drivers/serial/serial_bcm283x_mu.c
>
> > 
> > I also saw in the JTAG debugger that the uartclk member contained
> > an incorrect value, so I'd expect that it has to be set as well in
> > early_bcm2835aux_setup().
>
> In my case the clock was set by U-Boot already.

I'm testing this by booting directly from RPi4's bootloader. And it
works as long as I add this to config.txt:

enable_uart=1
gpu_freq=500

Which AFAIK blocks frequency scalin on the GPU and fixes the clock to a
point where the serial is set at 115200 bauds.

Ideally it'd be nice to be able to query the clock frequency, and
recalculate the divisors based on that. But I don't know if it's
feasible at that point in the boot process.

We could maybe leave a comment explaining what is expected from the
bootloader, given the different options around.

Regards,
Nicolas

Patch

diff --git a/drivers/tty/serial/8250/8250_bcm2835aux.c b/drivers/tty/serial/8250/8250_bcm2835aux.c
index 8ce700c1a7fc..6769cea2964a 100644
--- a/drivers/tty/serial/8250/8250_bcm2835aux.c
+++ b/drivers/tty/serial/8250/8250_bcm2835aux.c
@@ -135,6 +135,24 @@  static struct platform_driver bcm2835aux_serial_driver = {
 };
 module_platform_driver(bcm2835aux_serial_driver);
 
+#ifdef CONFIG_SERIAL_8250_CONSOLE
+
+static int __init early_bcm2835aux_setup(struct earlycon_device *device,
+					const char *options)
+{
+	if (!device->port.membase)
+		return -ENODEV;
+
+	device->port.iotype = UPIO_MEM32;
+	device->port.regshift = 2;
+
+	return early_serial8250_setup(device, NULL);
+}
+
+OF_EARLYCON_DECLARE(bcm2835aux, "brcm,bcm2835-aux-uart",
+		    early_bcm2835aux_setup);
+#endif
+
 MODULE_DESCRIPTION("BCM2835 auxiliar UART driver");
 MODULE_AUTHOR("Martin Sperl <[e-mail]>");
 MODULE_LICENSE("GPL v2");