On Thursday, May 07, 2009 3:51 PM, Ryan Mallon wrote: >>> You can (and should) use platform_driver_probe and still have >>> the module be loadable. For the gpio settings, I think that >>> probably the best approach is to use gpio_request for each of >>> the gpio pins so that gpio_lib knows that they are in use and >>> sysfs/debugfs will correctly show they are assigned to ide, >>> and then call ep93xx_ide_on_gpio, so you will have something >>> like in your init code: >> >> Hmmm... >> >> How about doing all the request/free's in the ep93xx_ide_on_gpio() >> function? That way the platform code can initially call it and set >> all the pins into gpio mode (freeing them). Then later when the >> IDE driver is loaded it can call the function to try and put the >> pins into IDE mode (requesting them). If any of the pins are >> unavailable the ep93xx_ide_on_gpio() function can return the >> necessary error code preventing the IDE driver from loading. When >> the driver is unloaded it just needs to call the core to free >> the gpio's. >> >> Something like: >> >> int ep93xx_ide_on_gpio(int enable) >> { >> u32 reg; >> int err; >> int i; >> >> reg = __raw_read(EP93XX_SYSCON_DEVICE_CONFIG); >> >> if (enable) { >> for (i = 0; i < 8; i++) { >> err = gpio_request(EP93XX_GPIO_LINE_E(i), "ep93xx_ide"); >> if (err) >> goto fail_gpio_e; >> err = gpio_request(EP93XX_GPIO_LINE_F(i), "ep93xx_ide"); >> if (err) >> goto fail_gpio_f; >> err = gpio_request(EP93XX_GPIO_LINE_G(i), "ep93xx_ide"); >> if (err) >> goto fail_gpio_g; >> } >> reg &= ~(EP93XX_SYSCON_DEVICE_CONFIG_EONIDE | >> EP93XX_SYSCON_DEVICE_CONFIG_GONIDE | >> EP93XX_SYSCON_DEVICE_CONFIG_HONIDE); >> } else { >> for (i = 0; i < 8; i++) { >> gpio_free(EP93XX_GPIO_LINE_E(i)); >> gpio_free(EP93XX_GPIO_LINE_F(i)); >> gpio_free(EP93XX_GPIO_LINE_G(i)); >> } >> reg |= (EP93XX_SYSCON_DEVICE_CONFIG_EONIDE | >> EP93XX_SYSCON_DEVICE_CONFIG_GONIDE | >> EP93XX_SYSCON_DEVICE_CONFIG_HONIDE); >> } >> >> __raw_writel(0xaa, EP93XX_SYSCON_SWLOCK); >> __raw_writel(reg, EP93XX_SYSCON_DEVICE_CONFIG); > > Is this going to be modified later to use the ep93xx_syscon_ functions > once they are merged? Yes, hopefully that occurs sometime soon. Still no responses on it. >> return 0; >> >> fail_gpio_g: >> free_gpio(EP93XX_GPIO_LINE_F(i); >> fail_gpio_f: >> free_gpio(EP93XX_GPIO_LINE_E(i); >> fail_gpio_e: >> for ( ; i >= 0; --i) { >> free_gpio(EP93XX_GPIO_LINE_E(i); >> free_gpio(EP93XX_GPIO_LINE_F(i); >> free_gpio(EP93XX_GPIO_LINE_G(i); >> } >> return err; >> } > > Yeah, that makes sense. Keeps the driver nice and clean that way too. > The ep93xx_ide_on_gpio function for core.c should be posted as a > separate patch though. > I agree. Actually that patch needs to go in now so that the port E/F/G pins will work for normal GPIO. The syscon register defaults at reset with the bits cleared so the boot state has them set for IDE mode. That drove me nuts for quite a while... If wanted I will put together the necessary patch for this. This function will hopefully get a bit cleaner later. I've been messing with an addition to the GPIOLIB API to add support for "ports". When/if that goes in all the for() loops go away and you just need to do a gpio_port_request(...) to grab the pins and gpio_port_free(...) to put them back. Ahh.. If only the days were longer.... Regards, Hartley -- 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