Hi

On Mon, 16 Aug 2010 09:36:41 -0500
Robert Nelson <robertcnel...@gmail.com> 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;
> +

Sorry, I didn't notice this earlier: you should free already allocated
gpios if the next one fails.

<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
</minor>

Otherwise the set looks goot to me.


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

Reply via email to