On 2012-03-29 18:26, H Hartley Sweeten wrote: >> +/************************************************************************* >> + * EP93xx IDE >> + *************************************************************************/ >> +static struct resource ep93xx_ide_resources[] = { >> + [0] = { >> + .start = EP93XX_IDE_PHYS_BASE, >> + .end = EP93XX_IDE_PHYS_BASE + 0xffff, >> + .flags = IORESOURCE_MEM, >> + }, >> + [1] = { >> + .start = IRQ_EP93XX_EXT3, >> + .end = IRQ_EP93XX_EXT3, >> + .flags = IORESOURCE_IRQ, >> + }, >> +}; > > Please use the DEFINE_RES_* macros. > Also, the last register in the IDE interface is at offset 0x34, so something like this: > > static struct resource ep93xx_ide_resources[] = { > DEFINE_RES_MEM(EP93XX_IDE_PHYS_BASE, 0x38), > DEFINE_RES_IRQ(IRQ_EP93XX_EXT3), > }; > Ok. Will do. >> +void __init ep93xx_register_ide(void) >> +{ >> + /* GPIO ports E, G and H used by IDE */ >> + ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_EONIDE | >> + EP93XX_SYSCON_DEVCFG_GONIDE | >> + EP93XX_SYSCON_DEVCFG_HONIDE); >> + platform_device_register(&ep93xx_ide_device); >> +} > > Please create ep93xx_ide_acquire_gpio and ep93xx_ide_acquire_gpio functions > to set/clear the necessary SYSCON bits during the probe/remove of the ide driver. > > This should also address your concern mentioned in PATCH 0/3 about making sure > the pin muxing is correct when the driver is loaded. > > The *_acquire_gpio function should also gpio_request the pins before setting the > SYSCON bits. That way if any of the pins are unavailable the driver will not load. Make > sure to gpio_free the pins in the *_release_gpio function. Look at how the keypad > is handled for an example. Yes, that makes sense. I will add necessary functions and modify the driver accordingly. Thanks for comments. Regards, RP -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html