Tony

>-----Original Message-----
>From: Tony Lindgren [mailto:[email protected]]
>Sent: Tuesday, May 19, 2009 11:30 AM
>To: Pandita, Vikram
>Cc: Christensen, Mikkel; [email protected]
>Subject: Re: [PATCH v3 1/3] OMAP3:zoom2: Add support for OMAP3 Zoom2 board
>
>* Pandita, Vikram <[email protected]> [090518 20:55]:
>> Tony
>>
>> >* Pandita, Vikram <[email protected]> [090518 14:31]:
>> >> Tony
>> >>
>> >> >>
>> >> >> +
>> >> >> +      zoom2_init_smsc911x();
>> >> >> +      zoom2_init_quaduart();
>> >> >> +      return platform_add_devices(zoom2_devices, 
>> >> >> ARRAY_SIZE(zoom2_devices));
>> >> >> +}
>> >> >> +arch_initcall(omap_zoom2_debugboard_init);
>> >> >
>> >> >Please just move all the related functions to board-zoom2.c. I don't see 
>> >> >any
>> >> >reason to have a separate file for the debugboard features. The runtime 
>> >> >detection
>> >> >should do the trick.
>> >>
>> >> Two reasons for keeping a separate debug board.
>> >> a) debug board is detachable in h/w and hence we thought its better to 
>> >> keep the s/w also that
>way
>> >> The quart chip on debug bard is a quard-uart and can support 4 different 
>> >> console outputs.
>> >> All those we thought of adding in future.
>> >
>> >Well those you can easily optimize out by not compiling in those devices.
>> >
>> >>
>> >> b) we took reference from board-rx51-peripherals.c wherein some parts of 
>> >> the board file can be
>> >moved out to a new file
>> >>
>> >> If you still think we need to merge, then let us know.
>> >> We are open to suggestions/changes.
>> >
>> >Well the reason for separate board-*-peripheral-whatever.c files is that
>> >they're intended to be shared with other board-*.c files. Currently n8x0
>> >and rx51 share devices. In general, we should try to do generic platform
>> >init files, like gpmc-onenand.c and gpmc-smc91x.c.
>> >
>> >Then I see need for gpmc-sdp-flash.c that's shared between 2430 and 3430 sdp
>> >at least.
>> >
>> >So yeah, for zoom2, I'd rather see just board-zoom2.c for now. But if you
>> >create some other board that uses the same debug board functions, then
>> >there could be a separate zoom-debugboard.c.
>>
>>
>> Yes. There is next generation of zoom boards that will re-use the Debug 
>> board.
>> That way the debug board file will get re-used.
>
>OK, how about rename it to something that makes sense as a name for
>multiple zoom like boards and resubmit.

I am planning on renaming debug board zoom2 file to board-zoom-debugboard.c
That way it can be used by different Zoom family of boards.

>
>Also the arch_initcall() issue below still needs fixing.

I have cleaned the issue by making the debug_board_init call as late_initcall().

There is a dependency on serial.c file and hence the need for 
arch_init/late_init calls.

platform_device_register() should happen for serial.c file (8250) for the omap 
uarts first and this should be followed by platform_device_register() for the 
quard uart on debug board.

So there is a dependency on order of registration of platform device for the 
same 8250 driver.

If you ack, then I can post the patches with the changes.

>
>Regards,
>
>Tony
>
>>
>> >
>> >Regards,
>> >
>> >Tony
>> >
>> >
>> >> >
>> >> >Also, the arch_initcall() above is wrong, it runs for all the boards and
>> >> >platforms compiled in even if they are not zoom2 boards.
>> >> >
>> >> >Other than, looks OK to me, so we might be able to get this into mainline
>> >> >this merge window.
>> >> >
>> >> >Regards,
>> >> >
>> >> >Tony
>> >>
>> >>
>> >>
>> >> >
>> >> >
>> >> >> diff --git a/arch/arm/mach-omap2/board-zoom2.c 
>> >> >> b/arch/arm/mach-omap2/board-zoom2.c
>> >> >> new file mode 100644
>> >> >> index 0000000..5a656b3
>> >> >> --- /dev/null
>> >> >> +++ b/arch/arm/mach-omap2/board-zoom2.c
>> >> >> @@ -0,0 +1,107 @@
>> >> >> +/*
>> >> >> + * Copyright (C) 2009 Texas Instruments Inc.
>> >> >> + * Mikkel Christensen <[email protected]>
>> >> >> + *
>> >> >> + * Modified from mach-omap2/board-ldp.c
>> >> >> + *
>> >> >> + * This program is free software; you can redistribute it and/or 
>> >> >> modify
>> >> >> + * it under the terms of the GNU General Public License version 2 as
>> >> >> + * published by the Free Software Foundation.
>> >> >> + */
>> >> >> +
>> >> >> +#include <linux/kernel.h>
>> >> >> +#include <linux/init.h>
>> >> >> +#include <linux/platform_device.h>
>> >> >> +#include <linux/i2c/twl4030.h>
>> >> >> +
>> >> >> +#include <asm/mach-types.h>
>> >> >> +#include <asm/mach/arch.h>
>> >> >> +
>> >> >> +#include <mach/gpio.h>
>> >> >> +#include <mach/common.h>
>> >> >> +#include <mach/usb.h>
>> >> >> +
>> >> >> +#include "mmc-twl4030.h"
>> >> >> +
>> >> >> +static void __init omap_zoom2_init_irq(void)
>> >> >> +{
>> >> >> +      omap2_init_common_hw(NULL);
>> >> >> +      omap_init_irq();
>> >> >> +      omap_gpio_init();
>> >> >> +}
>> >> >> +
>> >> >> +static struct omap_uart_config zoom2_uart_config __initdata = {
>> >> >> +      .enabled_uarts  = ((1 << 0) | (1 << 1) | (1 << 2)),
>> >> >> +};
>> >> >> +
>> >> >> +static struct omap_board_config_kernel zoom2_config[] __initdata = {
>> >> >> +      { OMAP_TAG_UART,        &zoom2_uart_config },
>> >> >> +};
>> >> >> +
>> >> >> +static struct twl4030_gpio_platform_data zoom2_gpio_data = {
>> >> >> +      .gpio_base      = OMAP_MAX_GPIO_LINES,
>> >> >> +      .irq_base       = TWL4030_GPIO_IRQ_BASE,
>> >> >> +      .irq_end        = TWL4030_GPIO_IRQ_END,
>> >> >> +};
>> >> >> +
>> >> >> +static struct twl4030_platform_data zoom2_twldata = {
>> >> >> +      .irq_base       = TWL4030_IRQ_BASE,
>> >> >> +      .irq_end        = TWL4030_IRQ_END,
>> >> >> +
>> >> >> +      /* platform_data for children goes here */
>> >> >> +      .gpio           = &zoom2_gpio_data,
>> >> >> +};
>> >> >> +
>> >> >> +static struct i2c_board_info __initdata zoom2_i2c_boardinfo[] = {
>> >> >> +      {
>> >> >> +              I2C_BOARD_INFO("twl4030", 0x48),
>> >> >> +              .flags = I2C_CLIENT_WAKE,
>> >> >> +              .irq = INT_34XX_SYS_NIRQ,
>> >> >> +              .platform_data = &zoom2_twldata,
>> >> >> +      },
>> >> >> +};
>> >> >> +
>> >> >> +static int __init omap_i2c_init(void)
>> >> >> +{
>> >> >> +      omap_register_i2c_bus(1, 2600, zoom2_i2c_boardinfo,
>> >> >> +                      ARRAY_SIZE(zoom2_i2c_boardinfo));
>> >> >> +      omap_register_i2c_bus(2, 400, NULL, 0);
>> >> >> +      omap_register_i2c_bus(3, 400, NULL, 0);
>> >> >> +      return 0;
>> >> >> +}
>> >> >> +
>> >> >> +static struct twl4030_hsmmc_info mmc[] __initdata = {
>> >> >> +      {
>> >> >> +              .mmc            = 1,
>> >> >> +              .wires          = 4,
>> >> >> +              .gpio_cd        = -EINVAL,
>> >> >> +              .gpio_wp        = -EINVAL,
>> >> >> +      },
>> >> >> +      {}      /* Terminator */
>> >> >> +};
>> >> >> +
>> >> >> +static void __init omap_zoom2_init(void)
>> >> >> +{
>> >> >> +      omap_i2c_init();
>> >> >> +      omap_board_config = zoom2_config;
>> >> >> +      omap_board_config_size = ARRAY_SIZE(zoom2_config);
>> >> >> +      omap_serial_init();
>> >> >> +      twl4030_mmc_init(mmc);
>> >> >> +      usb_musb_init();
>> >> >> +}
>> >> >> +
>> >> >> +static void __init omap_zoom2_map_io(void)
>> >> >> +{
>> >> >> +      omap2_set_globals_343x();
>> >> >> +      omap2_map_common_io();
>> >> >> +}
>> >> >> +
>> >> >> +MACHINE_START(OMAP_ZOOM2, "OMAP Zoom2 board")
>> >> >> +      .phys_io        = 0x48000000,
>> >> >> +      .io_pg_offst    = ((0xd8000000) >> 18) & 0xfffc,
>> >> >> +      .boot_params    = 0x80000100,
>> >> >> +      .map_io         = omap_zoom2_map_io,
>> >> >> +      .init_irq       = omap_zoom2_init_irq,
>> >> >> +      .init_machine   = omap_zoom2_init,
>> >> >> +      .timer          = &omap_timer,
>> >> >> +MACHINE_END
>> >> >> --
>> >> >> 1.5.4.3
>> >> >>
>> >> >> --
>> >> >> To unsubscribe from this list: send the line "unsubscribe linux-omap" 
>> >> >> in
>> >> >> the body of a message to [email protected]
>> >> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> >>
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to