Re: [PATCH 1/9] GTA02/FreeRunner: Add machine definition

[Mark Brown]

On Tue, Mar 31, 2009 at 01:06:34PM -0500, Nelson Castillo wrote:
> This patch introduces the Openmoko GTA02 machine definition.

This should come after any other patches it depends on (the CPU support
looks like the most obvious one).  A couple of comments, mostly about
stuff that is relevant to me.

> index b289d19..570cb81 100644
> --- a/arch/arm/mach-s3c2442/Kconfig
> +++ b/arch/arm/mach-s3c2442/Kconfig
> @@ -11,6 +11,7 @@ config CPU_S3C2442
>       select S3C2410_CLOCK
>       select S3C2410_GPIO
>       select S3C2410_PM if PM
> +     select S3C2440_DMA if S3C2410_DMA
>       select CPU_S3C244X
>       select CPU_LLSERIAL_S3C2440
>       help

This looks like it's in the wrong patch (it perhaps wants to be a patch
by itself)?

> +++ b/arch/arm/mach-s3c2442/mach-gta02.c

> +     .reg_init_data = {
> +             [PCF50633_REGULATOR_AUTO] = {
> +                     .constraints = {
> +                             .min_uV = 3300000,
> +                             .max_uV = 3300000,
> +                             .valid_modes_mask = REGULATOR_MODE_NORMAL,
> +                             .boot_on = 1,
> +                             .apply_uV = 1,
> +                             .state_mem = {
> +                                     .enabled = 1,
> +                             },
> +                     },
> +                     .num_consumer_supplies = 0,

For future compatiblity you should change boot_on to always_on for
regulators that have no consumers.  This tells the regulator core that
the regulator can't be powered off.  There's no need to set zero
consumers explicitly, either.

> +static struct i2c_board_info gta02_i2c_devs[] __initdata = {
> +     {
> +             I2C_BOARD_INFO("pcf50633", 0x73),
> +             .irq = GTA02_IRQ_PCF50633,
> +             .platform_data = &gta02_pcf_pdata,
> +     },
> +};

You should register the WM8753 here too, this is now required in
mainline.