On Fri, Jan 19, 2018 at 5:34 PM, Greentime Hu [off-list ref] wrote:

Hi, Arnd:

2018-01-18 18:11 GMT+08:00 Arnd Bergmann [off-list ref]:

quoted

On Mon, Jan 15, 2018 at 6:53 AM, Greentime Hu [off-list ref] wrote:

I had not looked at this patch in enough detail earlier, sorry about
that. It should be
easy enough to fix though.

quoted

+#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.

quoted

quoted

+
+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.

quoted

quoted

+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