On Thu, Jul 31, 2008 at 11:46 AM, Krzysztof Helt [off-list ref] wrote:

On Wed, 30 Jul 2008 08:51:08 -0400
Jaya Kumar [off-list ref] wrote:

quoted

+
+static irqreturn_t am200_handle_irq(int irq, void *dev_id)
+{

You may pass metronomefb_par* here to avoid one reference.

Agreed.

quoted

+     ret = wait_event_interruptible_timeout(par->waitq,
+                                     gpio_get_value(RDY_GPIO_PIN), HZ);
+     return ret;
+}

You may drop the ret variable here.

Agreed.

quoted

+     am200_device = platform_device_alloc("metronomefb", -1);

The second parameter is instance id. Probably 0 is better here.

I would like to mark it as single instance only, -1. I see in platform.c:

      if (pdev->id != -1)
                snprintf(pdev->dev.bus_id, BUS_ID_SIZE, "%s.%d", pdev->name,
                         pdev->id);
        else
                strlcpy(pdev->dev.bus_id, pdev->name, BUS_ID_SIZE);

quoted

+     am200_presetup_fb();

Should the am200 be set up in case of error?

Agreed, will fix the error.

Thanks,
jaya

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/