Re: [PATCH 05/24] C6X: early boot code

[Arnd Bergmann <arnd@arndb.de>]

On Monday 08 August 2011, Mark Salter wrote:

> +static const char *cpu_name, *cpu_voltage, *mmu, *fpu;
> +static char *soc_rev;
> +static char __cpu_rev[5], *cpu_rev;
> +static size_t initrd_size = CONFIG_BLK_DEV_RAM_SIZE*1024;
> +static unsigned int core_id;

Better make a data structure for the elements that you have per CPU,
that will help you when you get to SMP.

> +static void __init probe_machine(void)
> +{
> +	/*
> +	 * Iterate all c6x_md structures until we find the proper
> +	 * one for the current machine type
> +	 */
> +	for (machine_id = &__machine_desc_start;
> +	     machine_id < &__machine_desc_end;
> +	     machine_id++) {
> +		memcpy(&c6x_md, machine_id, sizeof(struct machdep_calls));
> +		if (c6x_md.probe())
> +			break;
> +	}
> +	/* What can we do if we didn't find ? */
> +	if (machine_id >= &__machine_desc_end) {
> +		DBG("No suitable machine found !\n");
> +		for (;;)
> +			;
> +	}
> +
> +	printk(KERN_INFO "Using %s machine description\n", c6x_md.name);
> +}

Why do you need separate machine descriptors? Since you can do all probing
through the device tree, you should not need to copy the complexity of
architectures that don't use it everywhere yet.

> +
> +static int show_cpuinfo(struct seq_file *m, void *v)
> +{
> +	seq_printf(m,
> +		   "CPU:\t\t%s\n"
> +		   "Core revision:\t%s\n"
> +		   "Core voltage:\t%s\n"
> +		   "Core id:\t%d\n"
> +		   "SoC cores:\t%d\n"
> +		   "MMU:\t\t%s\n"
> +		   "FPU:\t\t%s\n"
> +		   "Silicon rev:\t%s\n"
> +		   "Clocking:\t%uMHz\n"
> +		   "BogoMips:\t%lu.%02lu\n"
> +		   "Calibration:\t%lu loops\n",
> +		   cpu_name, cpu_rev, cpu_voltage,
> +		   core_id, c6x_num_cores, mmu, fpu,
> +		   soc_rev, (c6x_core_freq + 500000) / 1000000,
> +		   (loops_per_jiffy/(500000/HZ)),
> +		   (loops_per_jiffy/(5000/HZ))%100,
> +		   loops_per_jiffy);
> +
> +	return 0;
> +}

The fields are commonly all lowercase. Start with a line 'processor:\t0' to make it
easier to extend for multiple CPUs.

Out of "Clocking", "BogoMips" and "Calibration", you probably should have
only one, since they are all interdependent.

	Arnd