diff mbox series

serial/sifive: select SERIAL_EARLYCON

Message ID 20190910055923.28384-1-hch@lst.de (mailing list archive)
State New, archived
Headers show
Series serial/sifive: select SERIAL_EARLYCON | expand

Commit Message

Christoph Hellwig Sept. 10, 2019, 5:59 a.m. UTC
The sifive serial driver implements earlycon support, but unless
another driver is built in that supports earlycon support it won't
be usable.  Explicitly select SERIAL_EARLYCON instead.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/tty/serial/Kconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

Andreas Schwab Sept. 10, 2019, 6:57 a.m. UTC | #1
On Sep 10 2019, Christoph Hellwig <hch@lst.de> wrote:

> The sifive serial driver implements earlycon support,

It should probably be documented in admin-guide/kernel-parameters.txt.

Andreas.
Christoph Hellwig Sept. 10, 2019, 7:05 a.m. UTC | #2
On Tue, Sep 10, 2019 at 08:57:37AM +0200, Andreas Schwab wrote:
> On Sep 10 2019, Christoph Hellwig <hch@lst.de> wrote:
> 
> > The sifive serial driver implements earlycon support,
> 
> It should probably be documented in admin-guide/kernel-parameters.txt.

How so?  Wіth OF and a stdout path you just set earlycon on the
command line without any arguments and it will be found.
Andreas Schwab Sept. 10, 2019, 8:18 a.m. UTC | #3
On Sep 10 2019, Christoph Hellwig <hch@lst.de> wrote:

> On Tue, Sep 10, 2019 at 08:57:37AM +0200, Andreas Schwab wrote:
>> On Sep 10 2019, Christoph Hellwig <hch@lst.de> wrote:
>> 
>> > The sifive serial driver implements earlycon support,
>> 
>> It should probably be documented in admin-guide/kernel-parameters.txt.
>
> How so?  Wіth OF and a stdout path you just set earlycon on the
> command line without any arguments and it will be found.

Doesn't work for me.

[    0.000000] Malformed early option 'earlycon'

Andreas.
Christoph Hellwig Sept. 10, 2019, 2:36 p.m. UTC | #4
On Tue, Sep 10, 2019 at 10:18:15AM +0200, Andreas Schwab wrote:
> > How so?  Wіth OF and a stdout path you just set earlycon on the
> > command line without any arguments and it will be found.
> 
> Doesn't work for me.
> 
> [    0.000000] Malformed early option 'earlycon'

That functionality is implemented by param_setup_earlycon and
early_init_dt_scan_chosen_stdout.  Check why those aren't built into
your kernel.
Andreas Schwab Sept. 10, 2019, 2:42 p.m. UTC | #5
On Sep 10 2019, Christoph Hellwig <hch@lst.de> wrote:

> On Tue, Sep 10, 2019 at 10:18:15AM +0200, Andreas Schwab wrote:
>> > How so?  Wіth OF and a stdout path you just set earlycon on the
>> > command line without any arguments and it will be found.
>> 
>> Doesn't work for me.
>> 
>> [    0.000000] Malformed early option 'earlycon'
>
> That functionality is implemented by param_setup_earlycon and
> early_init_dt_scan_chosen_stdout.  Check why those aren't built into
> your kernel.

They are.

Andreas.
Palmer Dabbelt Sept. 13, 2019, 8:40 p.m. UTC | #6
On Tue, 10 Sep 2019 07:36:30 PDT (-0700), Christoph Hellwig wrote:
> On Tue, Sep 10, 2019 at 10:18:15AM +0200, Andreas Schwab wrote:
>> > How so?  Wіth OF and a stdout path you just set earlycon on the
>> > command line without any arguments and it will be found.
>> 
>> Doesn't work for me.
>> 
>> [    0.000000] Malformed early option 'earlycon'
>
> That functionality is implemented by param_setup_earlycon and
> early_init_dt_scan_chosen_stdout.  Check why those aren't built into
> your kernel.

OpenEmbedded passes "earlycon=sbi", which I can find in the doumentation.  I 
can't find anything about just "earlycon".  I've sent a patch adding sbi to the 
list of earlycon arguments.
Christoph Hellwig Sept. 16, 2019, 6:42 a.m. UTC | #7
On Fri, Sep 13, 2019 at 01:40:27PM -0700, Palmer Dabbelt wrote:
> OpenEmbedded passes "earlycon=sbi", which I can find in the doumentation.  
> I can't find anything about just "earlycon".  I've sent a patch adding sbi 
> to the list of earlycon arguments.

earlycon without arguments is documented, although just for ARM64.
I can send a patch to update it to properly cover all DT platforms
in addition.
Paul Walmsley Sept. 16, 2019, 10:21 a.m. UTC | #8
On Tue, 10 Sep 2019, Christoph Hellwig wrote:

> The sifive serial driver implements earlycon support, but unless
> another driver is built in that supports earlycon support it won't
> be usable.  Explicitly select SERIAL_EARLYCON instead.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Paul Walmsley <paul.walmsley@sifive.com>

- Paul
Paul Walmsley Sept. 16, 2019, 11:57 a.m. UTC | #9
On Tue, 10 Sep 2019, Andreas Schwab wrote:

> On Sep 10 2019, Christoph Hellwig <hch@lst.de> wrote:
> 
> > On Tue, Sep 10, 2019 at 08:57:37AM +0200, Andreas Schwab wrote:
> >> On Sep 10 2019, Christoph Hellwig <hch@lst.de> wrote:
> >> 
> >> > The sifive serial driver implements earlycon support,
> >> 
> >> It should probably be documented in admin-guide/kernel-parameters.txt.
> >
> > How so?  Wіth OF and a stdout path you just set earlycon on the
> > command line without any arguments and it will be found.
> 
> Doesn't work for me.
> 
> [    0.000000] Malformed early option 'earlycon'

Try:

    earlycon=sifive,0x10010000

If it works, you should see this message on the kernel console very early 
in the boot:

[    0.000000] earlycon: sifive0 at MMIO 0x0000000010010000 (options '')


- Paul
Andreas Schwab Sept. 16, 2019, noon UTC | #10
On Sep 16 2019, Paul Walmsley <paul.walmsley@sifive.com> wrote:

> On Tue, 10 Sep 2019, Andreas Schwab wrote:
>
>> On Sep 10 2019, Christoph Hellwig <hch@lst.de> wrote:
>> 
>> > On Tue, Sep 10, 2019 at 08:57:37AM +0200, Andreas Schwab wrote:
>> >> On Sep 10 2019, Christoph Hellwig <hch@lst.de> wrote:
>> >> 
>> >> > The sifive serial driver implements earlycon support,
>> >> 
>> >> It should probably be documented in admin-guide/kernel-parameters.txt.
>> >
>> > How so?  Wіth OF and a stdout path you just set earlycon on the
>> > command line without any arguments and it will be found.
>> 
>> Doesn't work for me.
>> 
>> [    0.000000] Malformed early option 'earlycon'
>
> Try:
>
>     earlycon=sifive,0x10010000

That's not what Christoph wants.

Andreas.
Christoph Hellwig Sept. 16, 2019, 12:15 p.m. UTC | #11
On Mon, Sep 16, 2019 at 02:00:39PM +0200, Andreas Schwab wrote:
> > Try:
> >
> >     earlycon=sifive,0x10010000
> 
> That's not what Christoph wants.

Well, what I want is auto selection if the device tree uses the
stdout-path.  My Kendryte DT uses that and it works like a charm.  If
it doesn't work for you on the U54 board chances are that your DT
doesn't have the right settings and you have to use the line that
Paul posted.
Paul Walmsley Sept. 16, 2019, 12:18 p.m. UTC | #12
On Mon, 16 Sep 2019, Andreas Schwab wrote:

> On Sep 16 2019, Paul Walmsley <paul.walmsley@sifive.com> wrote:
> 
> > On Tue, 10 Sep 2019, Andreas Schwab wrote:
> >
> >> On Sep 10 2019, Christoph Hellwig <hch@lst.de> wrote:
> >> 
> >> > On Tue, Sep 10, 2019 at 08:57:37AM +0200, Andreas Schwab wrote:
> >> >> On Sep 10 2019, Christoph Hellwig <hch@lst.de> wrote:
> >> >> 
> >> >> > The sifive serial driver implements earlycon support,
> >> >> 
> >> >> It should probably be documented in admin-guide/kernel-parameters.txt.
> >> >
> >> > How so?  Wіth OF and a stdout path you just set earlycon on the
> >> > command line without any arguments and it will be found.
> >> 
> >> Doesn't work for me.
> >> 
> >> [    0.000000] Malformed early option 'earlycon'
> >
> > Try:
> >
> >     earlycon=sifive,0x10010000
> 
> That's not what Christoph wants.

I support Christoph's plan to add generic implicit earlycon support.

In the meantime, once v5.4-rc1 is released, I'll send a patch to add a 
sifive driver section to the earlycon documentation in 
admin-guide/kernel-parameters.txt.  Thanks for the (earlier) suggestion.


- Paul
Christoph Hellwig Sept. 16, 2019, 12:22 p.m. UTC | #13
On Mon, Sep 16, 2019 at 05:18:57AM -0700, Paul Walmsley wrote:
> I support Christoph's plan to add generic implicit earlycon support.

I'm not planning to add it, it exist already.  You just need a DT that
sets the stdout node up properly, and a earlycon driver using
OF_EARLYCON_DECLARE like the sifive uart driver.

The DT here for example works perfectly fine:

http://git.infradead.org/users/hch/riscv.git/commitdiff/f10e64873eafc68516b8884c06b9290b9887633b
Palmer Dabbelt Sept. 16, 2019, 4:12 p.m. UTC | #14
On Sun, 15 Sep 2019 23:42:53 PDT (-0700), Christoph Hellwig wrote:
> On Fri, Sep 13, 2019 at 01:40:27PM -0700, Palmer Dabbelt wrote:
>> OpenEmbedded passes "earlycon=sbi", which I can find in the doumentation.
>> I can't find anything about just "earlycon".  I've sent a patch adding sbi
>> to the list of earlycon arguments.
>
> earlycon without arguments is documented, although just for ARM64.
> I can send a patch to update it to properly cover all DT platforms
> in addition.

Thanks.  I've kind of lost track of the thread, but assuming that does the 
"automatically pick an earlycon" stuff then that's probably what we should be 
using in the distros.
Andreas Schwab Sept. 16, 2019, 7:40 p.m. UTC | #15
On Sep 16 2019, Palmer Dabbelt <palmer@sifive.com> wrote:

> On Sun, 15 Sep 2019 23:42:53 PDT (-0700), Christoph Hellwig wrote:
>> On Fri, Sep 13, 2019 at 01:40:27PM -0700, Palmer Dabbelt wrote:
>>> OpenEmbedded passes "earlycon=sbi", which I can find in the doumentation.
>>> I can't find anything about just "earlycon".  I've sent a patch adding sbi
>>> to the list of earlycon arguments.
>>
>> earlycon without arguments is documented, although just for ARM64.
>> I can send a patch to update it to properly cover all DT platforms
>> in addition.
>
> Thanks.  I've kind of lost track of the thread, but assuming that does the
> "automatically pick an earlycon" stuff then that's probably what we should
> be using in the distros.

Except that it doesn't work.

Andreas.
Palmer Dabbelt Sept. 16, 2019, 9:38 p.m. UTC | #16
On Mon, 16 Sep 2019 12:40:10 PDT (-0700), schwab@suse.de wrote:
> On Sep 16 2019, Palmer Dabbelt <palmer@sifive.com> wrote:
>
>> On Sun, 15 Sep 2019 23:42:53 PDT (-0700), Christoph Hellwig wrote:
>>> On Fri, Sep 13, 2019 at 01:40:27PM -0700, Palmer Dabbelt wrote:
>>>> OpenEmbedded passes "earlycon=sbi", which I can find in the doumentation.
>>>> I can't find anything about just "earlycon".  I've sent a patch adding sbi
>>>> to the list of earlycon arguments.
>>>
>>> earlycon without arguments is documented, although just for ARM64.
>>> I can send a patch to update it to properly cover all DT platforms
>>> in addition.
>>
>> Thanks.  I've kind of lost track of the thread, but assuming that does the
>> "automatically pick an earlycon" stuff then that's probably what we should
>> be using in the distros.
>
> Except that it doesn't work.

Sorry, once again I've lost track of the thread.

The code looks generic.  The device tree in arch/riscv for the HiFive Unleashed 
doesn't have a stdout-path set, which if I understand correctly is used by the 
automatic earlycon stuff to pick a console.  I gave this a quick test on QEMU, 
which finds a 16550 earlycon for me.  I use openembedded's qemuriscv64 target, 
the following diff to make sure I'm getting an earlycon

diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index 5cd8c36c8fcc..61290714bbcb 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -106,6 +106,7 @@ static void early_serial8250_write(struct console *console,
        struct earlycon_device *device = console->data;
        struct uart_port *port = &device->port;

+       uart_console_write(port, "_e_", 3, serial_putc);
        uart_console_write(port, s, count, serial_putc);
 }

and run this command line

    /home/palmer/work/linux/openembedded-riscv64/build/tmp-glibc/work/x86_64-linux/qemu-helper-native/1.0-r1/recipe-sysroot-native/usr/bin/qemu-system-riscv64 -device virtio-net-device,netdev=net0,mac=52:54:00:12:35:02 -netdev user,id=net0,hostfwd=tcp::2222-:22,hostfwd=tcp::2323-:23,tftp=/home/palmer/work/linux/openembedded-riscv64/build/tmp-glibc/deploy/images/qemuriscv64 -drive id=disk0,file=/home/palmer/work/linux/openembedded-riscv64/build/tmp-glibc/deploy/images/qemuriscv64/core-image-full-cmdline-qemuriscv64-20190711162644.rootfs.ext4,if=none,format=raw -device virtio-blk-device,drive=disk0 -object rng-random,filename=/dev/urandom,id=rng0 -device virtio-rng-device,rng=rng0 -show-cursor -monitor null -device loader,file=/home/palmer/work/linux/linux/arch/riscv/boot/Image,addr=0x80200000  -nographic -machine virt  -m 512 -serial mon:stdio -serial null -kernel /home/palmer/work/linux/openembedded-riscv64/build/tmp-glibc/deploy/images/qemuriscv64/fw_jump.elf -append 'root=/dev/vda 
 rw highres=off  console=ttyS0 mem=512M ip=dhcp earlycon '

which gives me some early stuff and then some non-early stuff

_e_[    0.407579] printk: console [ttyS0] disabled
_e_[    0.409205] 10000000.uart: ttyS0 at MMIO 0x10000000 (irq = 10, base_baud = 230400) is a 16550A
[    0.410720] printk: console [ttyS0] enabled
_e_[    0.410720] printk: console [ttyS0] enabled
[    0.411391] printk: bootconsole [ns16550a0] disabled
_e_[    0.411391] printk: bootconsole [ns16550a0] disabled
[    0.420664] [drm] radeon kernel modesetting enabled.
[    0.428086] random: fast init done
[    0.429331] random: crng init done
[    0.440678] loop: module loaded
[    0.447607] virtio_blk virtio1: [vda] 262830 512-byte logical blocks (135 MB/128 MiB)
[    0.469483] libphy: Fixed MDIO Bus: probed

If you don't have something like "/chosen/stdout-path = &uart0;" in your device 
tree, then that's probably the issue.  Here's where it's set in Christoph's 
k210:

    http://git.infradead.org/users/hch/riscv.git/blob/f10e64873eafc68516b8884c06b9290b9887633b:/arch/riscv/boot/dts/kendryte/kd210.dts#l20

but we don't set it for the HiFive Unleashed.  I'd call that a bug, something 
like this

diff --git a/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
index 93d68cbd64fe..6d0ec76d93fe 100644
--- a/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
+++ b/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
@@ -13,6 +13,7 @@
        compatible = "sifive,hifive-unleashed-a00", "sifive,fu540-c000";
 
        chosen {
+               stdout-path = &uart0;
        };
 
        cpus {

should fix it.  LMK if I've misunderstood something.
Andreas Schwab Sept. 17, 2019, 7:43 a.m. UTC | #17
On Sep 16 2019, Palmer Dabbelt <palmer@sifive.com> wrote:

> but we don't set it for the HiFive Unleashed.  I'd call that a bug,
> something like this
>
> diff --git a/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
> index 93d68cbd64fe..6d0ec76d93fe 100644
> --- a/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
> +++ b/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
> @@ -13,6 +13,7 @@
>        compatible = "sifive,hifive-unleashed-a00", "sifive,fu540-c000";
>
>        chosen {
> +               stdout-path = &uart0;
>        };
>
>        cpus {
>
> should fix it.  LMK if I've misunderstood something.

If that's how it's supposed to work, ok.  Other platforms add it in
u-boot.

Andreas.
Atish Patra Oct. 4, 2019, 9:45 p.m. UTC | #18
On Mon, 2019-09-16 at 14:38 -0700, Palmer Dabbelt wrote:
> On Mon, 16 Sep 2019 12:40:10 PDT (-0700), schwab@suse.de wrote:
> > On Sep 16 2019, Palmer Dabbelt <palmer@sifive.com> wrote:
> > 
> > > On Sun, 15 Sep 2019 23:42:53 PDT (-0700), Christoph Hellwig
> > > wrote:
> > > > On Fri, Sep 13, 2019 at 01:40:27PM -0700, Palmer Dabbelt wrote:
> > > > > OpenEmbedded passes "earlycon=sbi", which I can find in the
> > > > > doumentation.
> > > > > I can't find anything about just "earlycon".  I've sent a
> > > > > patch adding sbi
> > > > > to the list of earlycon arguments.
> > > > 
> > > > earlycon without arguments is documented, although just for
> > > > ARM64.
> > > > I can send a patch to update it to properly cover all DT
> > > > platforms
> > > > in addition.
> > > 
> > > Thanks.  I've kind of lost track of the thread, but assuming that
> > > does the
> > > "automatically pick an earlycon" stuff then that's probably what
> > > we should
> > > be using in the distros.
> > 
> > Except that it doesn't work.
> 
> Sorry, once again I've lost track of the thread.
> 
> The code looks generic.  The device tree in arch/riscv for the HiFive
> Unleashed 
> doesn't have a stdout-path set, which if I understand correctly is
> used by the 
> automatic earlycon stuff to pick a console.  I gave this a quick test
> on QEMU, 
> which finds a 16550 earlycon for me.  I use openembedded's
> qemuriscv64 target, 
> the following diff to make sure I'm getting an earlycon
> 
> diff --git a/drivers/tty/serial/8250/8250_early.c
> b/drivers/tty/serial/8250/8250_early.c
> index 5cd8c36c8fcc..61290714bbcb 100644
> --- a/drivers/tty/serial/8250/8250_early.c
> +++ b/drivers/tty/serial/8250/8250_early.c
> @@ -106,6 +106,7 @@ static void early_serial8250_write(struct console
> *console,
>         struct earlycon_device *device = console->data;
>         struct uart_port *port = &device->port;
> 
> +       uart_console_write(port, "_e_", 3, serial_putc);
>         uart_console_write(port, s, count, serial_putc);
>  }
> 
> and run this command line
> 
>     /home/palmer/work/linux/openembedded-riscv64/build/tmp-
> glibc/work/x86_64-linux/qemu-helper-native/1.0-r1/recipe-sysroot-
> native/usr/bin/qemu-system-riscv64 -device virtio-net-
> device,netdev=net0,mac=52:54:00:12:35:02 -netdev
> user,id=net0,hostfwd=tcp::2222-:22,hostfwd=tcp::2323-
> :23,tftp=/home/palmer/work/linux/openembedded-riscv64/build/tmp-
> glibc/deploy/images/qemuriscv64 -drive
> id=disk0,file=/home/palmer/work/linux/openembedded-riscv64/build/tmp-
> glibc/deploy/images/qemuriscv64/core-image-full-cmdline-qemuriscv64-
> 20190711162644.rootfs.ext4,if=none,format=raw -device virtio-blk-
> device,drive=disk0 -object rng-random,filename=/dev/urandom,id=rng0
> -device virtio-rng-device,rng=rng0 -show-cursor -monitor null -device
> loader,file=/home/palmer/work/linux/linux/arch/riscv/boot/Image,addr=
> 0x80200000  -nographic -machine virt  -m 512 -serial mon:stdio
> -serial null -kernel /home/palmer/work/linux/openembedded-
> riscv64/build/tmp-glibc/deploy/images/qemuriscv64/fw_jump.elf -append
> 'root=/dev/vda rw highres=off  console=ttyS0 mem=512M ip=dhcp
> earlycon '
> 
> which gives me some early stuff and then some non-early stuff
> 
> _e_[    0.407579] printk: console [ttyS0] disabled
> _e_[    0.409205] 10000000.uart: ttyS0 at MMIO 0x10000000 (irq = 10,
> base_baud = 230400) is a 16550A
> [    0.410720] printk: console [ttyS0] enabled
> _e_[    0.410720] printk: console [ttyS0] enabled
> [    0.411391] printk: bootconsole [ns16550a0] disabled
> _e_[    0.411391] printk: bootconsole [ns16550a0] disabled
> [    0.420664] [drm] radeon kernel modesetting enabled.
> [    0.428086] random: fast init done
> [    0.429331] random: crng init done
> [    0.440678] loop: module loaded
> [    0.447607] virtio_blk virtio1: [vda] 262830 512-byte logical
> blocks (135 MB/128 MiB)
> [    0.469483] libphy: Fixed MDIO Bus: probed
> 
> If you don't have something like "/chosen/stdout-path = &uart0;" in
> your device 
> tree, then that's probably the issue.  Here's where it's set in
> Christoph's 
> k210:
> 
>     
> http://git.infradead.org/users/hch/riscv.git/blob/f10e64873eafc68516b8884c06b9290b9887633b:/arch/riscv/boot/dts/kendryte/kd210.dts#l20
> 
> but we don't set it for the HiFive Unleashed.  I'd call that a bug,
> something 
> like this
> 
> diff --git a/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
> b/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
> index 93d68cbd64fe..6d0ec76d93fe 100644
> --- a/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
> +++ b/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
> @@ -13,6 +13,7 @@
>         compatible = "sifive,hifive-unleashed-a00", "sifive,fu540-
> c000";
>  
>         chosen {
> +               stdout-path = &uart0;
>         };
>  
>         cpus {
> 
> should fix it.  LMK if I've misunderstood something.
> 

OpenSBI already adds this node to DT for U-Boot serial console.
I have tested that just adding "earlycon" to the commandline works as
long as you are using OpenSBI and SERIAL_EARLYCON is enabled.

If this entry can be added to dt residing in kernel, we can remove the
code that adds the "stdout-path" from OpenSBI.

> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
diff mbox series

Patch

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 530cb966092f..6b77a72278e3 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1075,6 +1075,7 @@  config SERIAL_SIFIVE_CONSOLE
 	bool "Console on SiFive UART"
 	depends on SERIAL_SIFIVE=y
 	select SERIAL_CORE_CONSOLE
+	select SERIAL_EARLYCON
 	help
 	  Select this option if you would like to use a SiFive UART as the
 	  system console.