On Fri, Jan 19, 2018 at 5:34 PM, Greentime Hu <green.hu@xxxxxxxxx> wrote: > Hi, Arnd: > > 2018-01-18 18:11 GMT+08:00 Arnd Bergmann <arnd@xxxxxxxx>: >> On Mon, Jan 15, 2018 at 6:53 AM, Greentime Hu <green.hu@xxxxxxxxx> wrote: >> >> I had not looked at this patch in enough detail earlier, sorry about >> that. It should be >> easy enough to fix though. >> >>> +#ifdef CONFIG_VGA_CONSOLE >>> +struct screen_info screen_info; >>> +#endif >> >> I would assume that you can't ever have a VGA console. Just drop all >> the references >> here and instead send a patch to the fbdev maintainer to add the dependency >> at CONFIG_VGA_CONSOLE to prevent selecting it with nds32. > > I found it can be built pass for now because we disable it in defconfig. > Should I send the patch in v7 series? yes, I think that would be best. >>> + >>> +extern void __init early_init_devtree(void *params); >>> +extern void __init early_trap_init(void); >> >> similarly, these are declared in include/linux/of_fdt.h >> > > early_trap_init is a nds32 function. I will move it to nds32.h Right, makes sense. >>> +void calibrate_delay(void) >>> +{ >>> + const int *val; >>> + struct device_node *cpu = NULL; >>> + cpu = of_find_compatible_node(NULL, NULL, "andestech,nds32v3"); >>> + val = of_get_property(cpu, "clock-frequency", NULL); >>> + if (!val || !*val) >>> + panic("no cpu 'clock-frequency' parameter in device tree"); >>> + loops_per_jiffy = be32_to_cpup(val) / HZ; >>> + pr_cont("%lu.%02lu BogoMIPS (lpj=%lu)\n", >>> + loops_per_jiffy / (500000 / HZ), >>> + (loops_per_jiffy / (5000 / HZ)) % 100, loops_per_jiffy); >>> +} >> >> This seems very odd to me: The 'clock-frequency' property in the >> cpu node should refer to the actual frequency it is running at, but that >> tends to be different from the bogomips as needed by the ndelay() >> function. Can you explain what is going on here? >> > > This implementation is referenced from openrisc. > https://lkml.org/lkml/2017/11/17/228 It's correct on openrisc, because that has a reliable cycle counter, and that gets used in its delay function: void __delay(unsigned long cycles) { cycles_t start = get_cycles(); while ((get_cycles() - start) < cycles) cpu_relax(); } In my review comment that you cited, I assumed that nds32 had similar hardware. However, as you explained earlier, the nds32 architecture does not provide a cycle counter and the clocksource resolution is not high enough to be a good replacement, so you have to use the traditional delay calibration. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html