Hi, Arnd,
On Tue, Jul 6, 2021 at 6:16 PM Arnd Bergmann <arnd@xxxxxxxx> wrote:
>
> On Tue, Jul 6, 2021 at 6:18 AM Huacai Chen <chenhuacai@xxxxxxxxxxx> 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.
We use grub to boot the Linux kernel. We found X86 uses private data
structures (seems not UEFI-specific) to pass initrd information from
grub to kernel. Some archs use fdt, and other archs use cmdline with
"initrd=start,size" (init/do_mounts_initrd.c). So, I think use cmdline
is not unacceptable, but we do can remove the the rd_start/rd_size
parsing code here.
>
> > +
> > +#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?
OK, we will remove this.
>
> > +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?
cpu_clock_freq is a const which records the "standard frequency", it
is just used to display the frequency now. In future when cpufreq
added, cpu_clock_freq will be used to calculate the target frequency.
Huacai
>
> > +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
