Hello Fabio, On Mon, Feb 29, 2016 at 08:54:19PM -0300, Fabio Estevam wrote: > On Mon, Feb 29, 2016 at 6:38 PM, Uwe Kleine-König > <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > > On Mon, Feb 29, 2016 at 06:16:50PM -0300, Fabio Estevam wrote: > >> On Mon, Feb 29, 2016 at 12:52 PM, Steffen Trumtrar > >> <s.trumtrar@xxxxxxxxxxxxxx> wrote: > >> > >> > + ret = clk_prepare_enable(rngc->clk); > >> > + if (ret) > >> > + return ret; > >> > + > >> > + rngc->irq = platform_get_irq(pdev, 0); > >> > + if (!rngc->irq) { > >> > + dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n"); > >> > + clk_disable_unprepare(rngc->clk); > >> > + > >> > + return ret; > >> > >> You are returning the wrong error code here: > >> > >> Better do like this: > >> > >> rngc->irq = platform_get_irq(pdev, 0); > >> if (rngc->irq < 0) { > > > > rngc->irq is unsigned, so this is never true. > > > >> dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n"); > >> clk_disable_unprepare(rngc->clk); > >> return rngc->irq; > >> } > > > > So here comes my better approach: > > As irq is only used inside probe it can be removed from struct mxc_rngc. Good idea. > Or maybe like this: > > ret = platform_get_irq(pdev, 0); > if (ret < 0) { > dev_err(&pdev->dev, "FSL RNGC couldn't get irq\n"); > clk_disable_unprepare(rngc->clk); > return ret; > } Some people think platform_get_irq returning 0 should be handled as error. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html