[PATCH 1/8] at91: introduce commom AT91_BASE_SYS

[Arnd Bergmann]

On Friday 01 July 2011, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On all at91 except rm9200 and x40 have the System Controller starts
> at address 0xffffc000 and has a size of 16KiB.
> 
> On rm9200 it's start at 0xfffe4000 of 111KiB with non reserved data starting
> at 0xfffff000
> 
> This patch removes the individual definitions of AT91_BASE_SYS and
> replaces them with a common version at base 0xfffffc000 and size 16KiB
> and map the same memory space

Hi Jean-Christophe,

I have mixed feelings about this series. A lot of the work you do here
looks like great cleanups and simplifications, some other IMHO parts
are just moving code from one place to another or look like they
are instead going in the wrong direction.

As a general comment, please start with a cover-letter (the PATCH 0/8)
email that describes the overall intention of the series and holds
the combined diffstat.

> +void __init at91_map_io(void)
> +{
> +	/* Map peripherals */
> +	iotable_init(&at91_io_desc, 1);
> +
> +	if (cpu_is_at91cap9())
> +		at91_boot_soc = at91cap9_soc;
> +	else if (cpu_is_at91rm9200())
> +		at91_boot_soc = at91rm9200_soc;
> +	else if (cpu_is_at91sam9260())
> +		at91_boot_soc = at91sam9260_soc;
> +	else if (cpu_is_at91sam9261())
> +		at91_boot_soc = at91sam9261_soc;
> +	else if (cpu_is_at91sam9263())
> +		at91_boot_soc = at91sam9263_soc;
> +	else if (cpu_is_at91sam9g10())
> +		at91_boot_soc = at91sam9261_soc;
> +	else if (cpu_is_at91sam9g20())
> +		at91_boot_soc = at91sam9260_soc;
> +	else if (cpu_is_at91sam9g45())
> +		at91_boot_soc = at91sam9g45_soc;
> +	else if (cpu_is_at91sam9rl())
> +		at91_boot_soc = at91sam9rl_soc;
> +	else if (cpu_is_at91sam9x5())
> +		at91_boot_soc = at91sam9x5_soc;
> +	else
> +		panic("Impossible to detect the SOC type");
> +
> +	if (at91_boot_soc.map_io)
> +		at91_boot_soc.map_io();
> +}

I think it would be easier to turn this around and have a very short
function here:

void __init at91_map_io(void)
{
	/* Map peripherals */
	iotable_init(&at91_io_desc, 1);
}

and then change each of the soc specific functions into


-void __init at91cap9_map_io(void)
+static void __init at91cap9_map_io(void)
 {
-       /* Map peripherals */
-       iotable_init(at91cap9_io_desc, ARRAY_SIZE(at91cap9_io_desc));
+	at91_boot_soc = at91cap9_soc;
+	at91_map_io();
+       iotable_init(at91cap9_sram_desc, ARRAY_SIZE(at91cap9_sram_desc));
 }

This would keep the per-soc code local and still consolidate the
common code without introducing (IMHO ugly) cpu_is_at91...() checks.

	Arnd