On Tue, Aug 17, 2010 at 12:48 AM, Jarkko Nikula <jhnikula@xxxxxxxxx> wrote: > Hi > > On Mon, 16 Aug 2010 09:36:41 -0500 > Robert Nelson <robertcnelson@xxxxxxxxx> wrote: > >> +u8 get_omap3_beagle_rev(void) >> +{ >> + return omap3_beagle_version; >> +} >> + >> +static void __init omap3_beagle_get_revision(void) >> +{ >> + int ret; >> + u16 beagle_rev = 0; >> + >> + omap_mux_init_gpio(171, OMAP_PIN_INPUT_PULLUP); >> + omap_mux_init_gpio(172, OMAP_PIN_INPUT_PULLUP); >> + omap_mux_init_gpio(173, OMAP_PIN_INPUT_PULLUP); >> + >> + ret = gpio_request(171, "rev_id_0"); >> + if (ret < 0) >> + goto fail; >> + >> + ret = gpio_request(172, "rev_id_1"); >> + if (ret < 0) >> + goto fail; >> + >> + ret = gpio_request(173, "rev_id_2"); >> + if (ret < 0) >> + goto fail; >> + > Thanks Jarkko > Sorry, I didn't notice this earlier: you should free already allocated > gpios if the next one fails. Sure, i'll add something like: fail3: gpio_free(173); fail2: gpio_free(172); fail1: gpio_free(171); Do you have any preference if the gpio allocation fails? Right now it'll just halts, we could just return as Cx based board.. Which would be the route the current kernel would take without this patch... > <minor> > I was thinking would it make a sense to rename funtions below. I.e. to > indicate that only one of them is for runtime revision detection and > another is for revision initialization only. What do you think? > > get_omap3_beagle_rev -> omap3_beagle_get_rev > omap3_beagle_get_revision -> omap3_beagle_init_rev That does help clear up the two functions a little. I didn't give much thought to the names, they just came from the evm board revision example: s/evm/beagle/g Regards, -- Robert Nelson http://www.rcn-ee.com/ -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html