Re: [PATCH 05/19] LoongArch: Add boot and setup routines

[Arnd Bergmann <arnd@kernel.org>]

On Tue, Jul 6, 2021 at 6:18 AM Huacai Chen <chenhuacai@loongson.cn> wrote:

> +
> +#ifdef CONFIG_64BIT
> +       /* Guess if the sign extension was forgotten by bootloader */
> +       if (start < CAC_BASE)
> +               start = (int)start;
> +#endif
> +       initrd_start = start;
> +       initrd_end += start;
> +       return 0;
> +}
> +early_param("rd_start", rd_start_early);
> +
> +static int __init rd_size_early(char *p)
> +{
> +       initrd_end += memparse(p, &p);
> +       return 0;
> +}
> +early_param("rd_size", rd_size_early);

The early parameters should not be used for this, I'm fairly sure the UEFI
boot protocol already has ways to communicate all necessary information.

> +
> +#ifdef CONFIG_ACPI
> +       init_initrd();
> +#endif

Why is the initrd support tied to ACPI? Can you actually boot without ACPI?

> +#if defined(CONFIG_VT)
> +#if defined(CONFIG_VGA_CONSOLE)
> +       conswitchp = &vga_con;
> +#elif defined(CONFIG_DUMMY_CONSOLE)
> +       conswitchp = &dummy_con;
> +#endif
> +#endif

The VGA console seems rather outdated. If you have UEFI, why not use
the provided framebuffer for the console?

> +u64 cpu_clock_freq;
> +EXPORT_SYMBOL(cpu_clock_freq);
> +u64 const_clock_freq;
> +EXPORT_SYMBOL(const_clock_freq);

You should generally not rely on the CPU clock frequency being fixed
like this, as this breaks down as soon as you add a drivers/cpufreq/ driver.

What code uses these?

> +void __init time_init(void)
> +{
> +       if (!cpu_has_cpucfg)
> +               const_clock_freq = cpu_clock_freq;
> +       else
> +               const_clock_freq = calc_const_freq();
> +
> +       init_timeval = drdtime() - csr_readq(LOONGARCH_CSR_CNTC);
> +
> +       constant_clockevent_init();
> +       constant_clocksource_init();
> +}

Clocksource and clockevents drivers should be located in drivers/clocksource
and reviewed by its maintainers.

         Arnd