On Mon, Sep 01, 2008 at 02:57:56PM -0700, sakoman@gmail.com wrote:
.size = 15 * NAND_BLOCK_SIZE,
Unnecessary additional spacing after '='.
+static struct omap_onenand_platform_data overo_onenand_data = {
- .parts = overo_onenand_partitions,
- .nr_parts = ARRAY_SIZE(overo_onenand_partitions),
- .dma_channel = -1, /* disable DMA in OMAP OneNAND driver */
Tabbing vs spacing indentation styles, please settle on one way to do it.
+void __init overo_flash_init(void)
Should be static?
+{
- u8 cs = 0;
- u8 onenandcs = GPMC_CS_NUM + 1;
Do these two tabs after 'u8' help? Everywhere else you use a single space.
- while (cs < GPMC_CS_NUM) {
u32 ret = 0;ret = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG7);if ((ret & 0x3F) == (ONENAND_MAP >> 24))onenandcs = cs;cs++;- }
- if (onenandcs > GPMC_CS_NUM) {
printk(KERN_ERR "OneNAND: Unable to find configuration"" in GPMC\n ");
Unnecessary additional space after '\n' (which will result in the next line output by the kernel having its level tag ignored.)
- if ((gpio_request(W2W_NRESET, "W2W_NRESET") == 0) &&
(gpio_direction_output(W2W_NRESET, 1) == 0)) {gpio_export(W2W_NRESET, 0);gpio_direction_output(W2W_NRESET, 0);
Since you've already just set W2W_NRESET as an output, shouldn't this be: gpio_set_value(W2W_NRESET, 0);
instead?
udelay(10);gpio_direction_output(W2W_NRESET, 1);
gpio_set_value(W2W_NRESET, 1);
? Ditto for the rest like this.
- } else
printk(KERN_ERR "could not obtain gpio for W2W_NRESET\n");- if ((gpio_request(BT_NRESET, "BT_NRESET") == 0) &&
(gpio_direction_output(BT_NRESET, 1) == 0)) {gpio_export(BT_NRESET, 0);gpio_direction_output(BT_NRESET, 0);mdelay(6);gpio_direction_output(BT_NRESET, 1);- } else
printk(KERN_ERR "could not obtain gpio for BT_NRESET\n");- if ((gpio_request(USBH_CPEN, "USBH_CPEN") == 0) &&
(gpio_direction_output(USBH_CPEN, 1) == 0))gpio_export(USBH_CPEN, 0);- else
printk(KERN_ERR "could not obtain gpio for USBH_CPEN\n");- if ((gpio_request(USBH_NRESET, "USBH_NRESET") == 0) &&
(gpio_direction_output(USBH_NRESET, 1) == 0))gpio_export(USBH_NRESET, 0);- else
printk(KERN_ERR "could not obtain gpio for USBH_NRESET\n");+}
+arch_initcall(overo_i2c_init);
Is there a reason why overo_i2c_init can't be called from your .init_machine function?
diff --git a/arch/arm/plat-omap/include/mach/hardware.h b/arch/arm/plat-omap/include/mach/hardware.h index 438de80..f859c00 100644 --- a/arch/arm/plat-omap/include/mach/hardware.h +++ b/arch/arm/plat-omap/include/mach/hardware.h @@ -374,6 +374,10 @@ #include "board-sx1.h" #endif
+#ifdef CONFIG_MACH_OVERO +#include "board-overo.h" +#endif
I do hate it when machine classes like to add all their platform specific header includes into another include, meaning that whenever you touch _any_ platform specific kernel, everything rebuilds whether it's affected or not.
With the driver model, it just isn't necessary anymore; I would much rather prefer to see "board-overo.h" included by files which need it, which to me looks like being board-overo.c.
+static int overo_panel_init(struct lcd_panel *panel,
struct omapfb_device *fbdev)+{
- if ((gpio_request(LCD_ENABLE, "LCD_ENABLE") == 0) &&
(gpio_direction_output(LCD_ENABLE, 1) == 0))gpio_export(LCD_ENABLE, 0);- else
printk(KERN_ERR "could not obtain gpio for LCD_ENABLE\n");- return 0;
+}
+static void overo_panel_cleanup(struct lcd_panel *panel) +{
No freeing of LCD_ENABLE?
+}
+static int overo_panel_enable(struct lcd_panel *panel) +{
- gpio_direction_output(LCD_ENABLE, 1);
gpio_set_value() ?
- return 0;
+}
+static void overo_panel_disable(struct lcd_panel *panel) +{
- gpio_direction_output(LCD_ENABLE, 0);
gpio_set_value() ?
+static int overo_panel_probe(struct platform_device *pdev) +{
- omapfb_register_panel(&overo_panel);
- return 0;
+}
+static int overo_panel_remove(struct platform_device *pdev) +{
No possible unregistering of the panel?
+static int overo_panel_suspend(struct platform_device *pdev,
pm_message_t mesg)+{
- return 0;
+}
+static int overo_panel_resume(struct platform_device *pdev) +{
- return 0;
+}
No suspend/resume? So these empty functions aren't required.
+struct platform_driver overo_panel_driver = {
Should be static.