[PATCH v2 16/23] at91: Make LCD controller device common

[Ryan Mallon]

On 21/04/11 19:02, Uwe Kleine-König wrote:
> Hello Ryan,
> 
> On Thu, Apr 21, 2011 at 05:42:08PM +1200, Ryan Mallon wrote:
>> Replace the individual LCD controller device code for each at91 variant with a
>> single implementation in devices.c
>>
>> Signed-off-by: Ryan Mallon <ryan at bluewatersys.com>

<snip>

>> +static struct platform_device at91_lcdc_device = {
>> +	.name		= "atmel_lcdfb",
>> +	.id		= 0,
>> +	.dev		= {
>> +				.dma_mask		= &lcdc_dmamask,
>> +				.coherent_dma_mask	= DMA_BIT_MASK(32),
>> +				.platform_data		= &lcdc_data,
>> +	},
>> +	.resource	= lcdc_resources,
>> +	.num_resources	= ARRAY_SIZE(lcdc_resources),
>> +};
> One difference to my approach (which you can like or not) is that mxc
> uses dynamic allocation of the platform_devices. A struct
> platform_device has a size of 320 bytes. So instead of using
> at91_lcdc_device you could add to at91_add_device_lcdc:
> 
> 	struct platform_device *pdev = platform_device_register_resndata(
> 		NULL, "atmel_lcdfb", 0, &lcdc_resources,
> 		ARRAY_SIZE(lcdc_resources), *data, sizeof(*data));
> 
> with the added benefit that lcdc_resources and *data can be __initdata
> (or __initconst) to save a few more bytes.
> 
> The obvious downside is that dynamic allocation probably takes more time
> than using static data and so increases the boottime. I didn't measure
> it, but I think the difference is small enough to see over it.

Having the resources and *data also be __initdata would be good. One
reason I have taken the approach I have, rather than what you suggest,
is that my patches introduce minimal change. To create the common
devices.c I have cut and pasted one of the implementations and only made
necessary changes to make it generic to all of the variants rather than
do a complete rewrite. I think that this approach makes the patch series
easier to review and more likely to be correct (less new bugs introduced).

~Ryan