diff mbox series

tty: hvc: riscv_sbi: instantiate the legcay console earlier

Message ID 20241014000857.3032-1-jszhang@kernel.org (mailing list archive)
State New
Headers show
Series tty: hvc: riscv_sbi: instantiate the legcay console earlier | expand

Checks

Context Check Description
conchuod/vmtest-for-next-PR success PR summary
conchuod/patch-1-test-1 success .github/scripts/patches/tests/build_rv32_defconfig.sh took 139.60s
conchuod/patch-1-test-2 success .github/scripts/patches/tests/build_rv64_clang_allmodconfig.sh took 1271.52s
conchuod/patch-1-test-3 success .github/scripts/patches/tests/build_rv64_gcc_allmodconfig.sh took 1484.34s
conchuod/patch-1-test-4 success .github/scripts/patches/tests/build_rv64_nommu_k210_defconfig.sh took 20.80s
conchuod/patch-1-test-5 success .github/scripts/patches/tests/build_rv64_nommu_virt_defconfig.sh took 23.03s
conchuod/patch-1-test-6 success .github/scripts/patches/tests/checkpatch.sh took 0.46s
conchuod/patch-1-test-7 success .github/scripts/patches/tests/dtb_warn_rv64.sh took 43.91s
conchuod/patch-1-test-8 success .github/scripts/patches/tests/header_inline.sh took 0.00s
conchuod/patch-1-test-9 success .github/scripts/patches/tests/kdoc.sh took 0.56s
conchuod/patch-1-test-10 success .github/scripts/patches/tests/module_param.sh took 0.01s
conchuod/patch-1-test-11 success .github/scripts/patches/tests/verify_fixes.sh took 0.00s
conchuod/patch-1-test-12 success .github/scripts/patches/tests/verify_signedoff.sh took 0.03s

Commit Message

Jisheng Zhang Oct. 14, 2024, 12:08 a.m. UTC
The hvc_instantiate() is an early console discovery mechanism, it is
usually called before allocating hvc terminal devices. In fact, if
we check hvc_riscv_sbi's hvc_instantiate() return value, we'll find
that it's -1. So the calling hvc_instantiate() is too late.

We can remove the hvc_instantiate() to only rely on the hvc_alloc() to
register the kernel console. We can also move its calling earlier so
the kernel console is registered earlier, so that we can get kernel
console msg earlier. We take the 2nd choice in this patch.

Before the patch:
[    0.367440] printk: legacy console [hvc0] enabled
[    0.401397] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled

After the patch:

[    0.004665] printk: legacy console [hvc0] enabled
[    0.050183] Calibrating delay loop (skipped), value calculated using timer frequency.. 20.00 BogoMIPS (lpj=100000)

As can be seen, now the kernel console is registered much earlier before
the BogoMIPS calibrating.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 drivers/tty/hvc/hvc_riscv_sbi.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Comments

Greg KH Oct. 14, 2024, 6:10 a.m. UTC | #1
On Mon, Oct 14, 2024 at 08:08:57AM +0800, Jisheng Zhang wrote:
> The hvc_instantiate() is an early console discovery mechanism, it is
> usually called before allocating hvc terminal devices. In fact, if
> we check hvc_riscv_sbi's hvc_instantiate() return value, we'll find
> that it's -1. So the calling hvc_instantiate() is too late.
> 
> We can remove the hvc_instantiate() to only rely on the hvc_alloc() to
> register the kernel console. We can also move its calling earlier so
> the kernel console is registered earlier, so that we can get kernel
> console msg earlier. We take the 2nd choice in this patch.
> 
> Before the patch:
> [    0.367440] printk: legacy console [hvc0] enabled
> [    0.401397] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
> 
> After the patch:
> 
> [    0.004665] printk: legacy console [hvc0] enabled
> [    0.050183] Calibrating delay loop (skipped), value calculated using timer frequency.. 20.00 BogoMIPS (lpj=100000)
> 
> As can be seen, now the kernel console is registered much earlier before
> the BogoMIPS calibrating.
> 
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>

What commit id does this fix?

> ---
>  drivers/tty/hvc/hvc_riscv_sbi.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/hvc/hvc_riscv_sbi.c b/drivers/tty/hvc/hvc_riscv_sbi.c
> index cede8a572594..d2ecfbf7c84a 100644
> --- a/drivers/tty/hvc/hvc_riscv_sbi.c
> +++ b/drivers/tty/hvc/hvc_riscv_sbi.c
> @@ -68,12 +68,10 @@ static int __init hvc_sbi_init(void)
>  		err = PTR_ERR_OR_ZERO(hvc_alloc(0, 0, &hvc_sbi_dbcn_ops, 256));
>  		if (err)
>  			return err;
> -		hvc_instantiate(0, 0, &hvc_sbi_dbcn_ops);
>  	} else if (IS_ENABLED(CONFIG_RISCV_SBI_V01)) {
>  		err = PTR_ERR_OR_ZERO(hvc_alloc(0, 0, &hvc_sbi_v01_ops, 256));
>  		if (err)
>  			return err;
> -		hvc_instantiate(0, 0, &hvc_sbi_v01_ops);
>  	} else {
>  		return -ENODEV;
>  	}
> @@ -81,3 +79,18 @@ static int __init hvc_sbi_init(void)
>  	return 0;
>  }
>  device_initcall(hvc_sbi_init);
> +
> +static int __init hvc_sbi_console_init(void)
> +{
> +	int err;
> +
> +	if (sbi_debug_console_available)
> +		err = hvc_instantiate(0, 0, &hvc_sbi_dbcn_ops);
> +	else if (IS_ENABLED(CONFIG_RISCV_SBI_V01))
> +		err = hvc_instantiate(0, 0, &hvc_sbi_v01_ops);
> +	else
> +		return -ENODEV;
> +
> +	return err < 0 ? -ENODEV : 0;

Please spell out a ? : line, it's not required here.

> +}
> +console_initcall(hvc_sbi_console_init);

Are you sure this will always work properly?  For some reason the
original code did not do this, you might want to check the
lore.kernel.org archives to find out why.

thanks,

greg k-h
diff mbox series

Patch

diff --git a/drivers/tty/hvc/hvc_riscv_sbi.c b/drivers/tty/hvc/hvc_riscv_sbi.c
index cede8a572594..d2ecfbf7c84a 100644
--- a/drivers/tty/hvc/hvc_riscv_sbi.c
+++ b/drivers/tty/hvc/hvc_riscv_sbi.c
@@ -68,12 +68,10 @@  static int __init hvc_sbi_init(void)
 		err = PTR_ERR_OR_ZERO(hvc_alloc(0, 0, &hvc_sbi_dbcn_ops, 256));
 		if (err)
 			return err;
-		hvc_instantiate(0, 0, &hvc_sbi_dbcn_ops);
 	} else if (IS_ENABLED(CONFIG_RISCV_SBI_V01)) {
 		err = PTR_ERR_OR_ZERO(hvc_alloc(0, 0, &hvc_sbi_v01_ops, 256));
 		if (err)
 			return err;
-		hvc_instantiate(0, 0, &hvc_sbi_v01_ops);
 	} else {
 		return -ENODEV;
 	}
@@ -81,3 +79,18 @@  static int __init hvc_sbi_init(void)
 	return 0;
 }
 device_initcall(hvc_sbi_init);
+
+static int __init hvc_sbi_console_init(void)
+{
+	int err;
+
+	if (sbi_debug_console_available)
+		err = hvc_instantiate(0, 0, &hvc_sbi_dbcn_ops);
+	else if (IS_ENABLED(CONFIG_RISCV_SBI_V01))
+		err = hvc_instantiate(0, 0, &hvc_sbi_v01_ops);
+	else
+		return -ENODEV;
+
+	return err < 0 ? -ENODEV : 0;
+}
+console_initcall(hvc_sbi_console_init);