[PATCH 01/17] ARM: pxa/raumfeld: add basic structure for devices

[daniel@caiaq.de (Daniel Mack)]

Hi Mike,

On Wed, Nov 25, 2009 at 05:26:32PM +0200, Mike Rapoport wrote:
> Below are my comments to the patches. Some of the comments apply to several
> patches, but I'm too lazy to copy them into relevant threads :)

Ok, I squashed most of the commits together for the next round so make
hat easier :)

> Daniel Mack wrote:
> > +static void __init raumfeld_common_init(void)
> > +{
> > +	enable_irq_wake(IRQ_WAKEUP0);
> > +	pxa_set_ffuart_info(NULL);
> > +
> > +	gpio_request(mfp_to_gpio(GPIO_W2W_RESET), "Wi2Wi reset");
> 
> gpio_request may fail, thought it's unlikely to happen. Anyway, adding check for
> it's return value seems to be a good practice.

Ok, I put BUG_ON() around them. I see no reason to care for proper error
handling as this is not a driver. If any of those functions fail in the
low-level code, there is something seriously broken.

> > +static void __init raumfeld_speaker_init(void)
> > +{
> > +	raumfeld_common_init();
> > +}
> 
> I failed to follow what peripherals are common to what boards, but after
> applying all the patches it seems that all three _init functions are really
> similar. Have you considered having one _init function for all three machines
> and calling machine-specific init based on machine_is_X?

Yes, I did consider that. But eventually, they are a) all not big and b)
too different. Doing the way you suggest make it look more messy. Have a
look at the second round and tell me if you stick to your opponion :)

Thanks for your input,
Daniel