> -----Original Message----- > From: linux-omap-owner@xxxxxxxxxxxxxxx > [mailto:linux-omap-owner@xxxxxxxxxxxxxxx] On Behalf Of G, > Manjunath Kondaiah > Sent: Thursday, May 06, 2010 8:45 AM > To: Arce, Abraham; Tony Lindgren > Cc: linux-omap@xxxxxxxxxxxxxxx; > spi-devel-general@xxxxxxxxxxxxxxxxxxxxx > Subject: RE: [PATCH v1 2/3] OMAP4: Ethernet: KS8851 Board Support > > > > > -----Original Message----- > > From: Arce, Abraham > > Sent: Thursday, May 06, 2010 6:18 AM > > To: Tony Lindgren > > Cc: G, Manjunath Kondaiah; linux-omap@xxxxxxxxxxxxxxx; > > spi-devel-general@xxxxxxxxxxxxxxxxxxxxx > > Subject: RE: [PATCH v1 2/3] OMAP4: Ethernet: KS8851 Board Support > > > > > > > > + > > > > > > +static void omap_ethernet_init(void) > > > > > > +{ > > > > > > + gpio_request(ETHERNET_KS8851_POWER_ENABLE, "ethernet"); > > > > > > + gpio_direction_output(ETHERNET_KS8851_POWER_ENABLE, 1); > > > > > > + gpio_request(ETHERNET_KS8851_QUART, "quart"); > > > > > > + gpio_direction_output(ETHERNET_KS8851_QUART, 1); > > > > > > + gpio_request(ETHERNET_KS8851_IRQ, "ks8851"); > > > > > > > > > > Any reason for not checking return value of gpio_request()? > > > > > > > > > > > > > Thought initially about them as dedicated lines for > > ethernet avoiding > > > > both reasons to check for a gpio's request: > > > > > > > > 1. invalid gpio > > > > 2. already claimed gpio > > > > > > You still need to check for the result. > > > > > > > Is the below approach ok? Using goto would make it better? > > > > int status; > > > > status = gpio_request(ETHERNET_KS8851_POWER_ENABLE, > > "ethernet"); > > if (status < 0) > > You need to have warning message about this failure. > > > return status; > > > > gpio_request(ETHERNET_KS8851_QUART, "quart"); > > if (status < 0) { > > -Ditto- > > > gpio_free(ETHERNET_KS8851_POWER_ENABLE); > > return status; > > Return to where? This function seems to be void, change signature > of this API. > > > } > > > > gpio_request(ETHERNET_KS8851_IRQ, "ks8851"); > > if (status < 0) { > > Bug! Checking previous status. > > > gpio_free(ETHERNET_KS8851_POWER_ENABLE); > > gpio_free(ETHERNET_KS8851_QUART); > > return status; > > } > > > > gpio_direction_output(ETHERNET_KS8851_POWER_ENABLE, 1); > > gpio_direction_output(ETHERNET_KS8851_QUART, 1); > > gpio_direction_input(ETHERNET_KS8851_IRQ); > > Goto will be better approach compared to above logic. > > if(gpio_request(x) > goto err1; > > if(gpio_request(y) > goto err2; > > if(gpio_request(z) > goto err3; > ... > return 0; > > err3: > free(x); > err2: > free(y); > err1: > free(z); > > return status; Minor changes: if (gpio_request(x)) return status; if (gpio_request(y)) goto err1; if (gpio_request(z)) goto err2; ... return 0; err2: free(y); err1: free(x); return status; -Manjunath -- 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
