Hi Fabio, On Tue, 29 May 2018 at 17:20, Fabio Estevam <feste...@gmail.com> wrote:
> On Tue, May 29, 2018 at 12:04 PM, Clément Péron <peron.c...@gmail.com> wrote: > > +static int __init epit_timer_init(struct device_node *np) > > +{ > > + struct epit_timer *epittm; > > + struct clk *clk_ipg; > > + int ret; > > + > > + epittm = kzalloc(sizeof(*epittm), GFP_KERNEL); > > + if (!epittm) > > + return -ENOMEM; > > + > > + epittm->base = of_iomap(np, 0); > > + if (!epittm->base) { > > + ret = -ENXIO; > > + goto out_kfree; > > + } > > + > > + epittm->irq = irq_of_parse_and_map(np, 0); > > + if (!epittm->irq) { > > + ret = -EINVAL; > > + goto out_kfree; > You should jump to out_iounmap and call of_iounmap(). > > + } > > + > > + clk_ipg = of_clk_get_by_name(np, "ipg"); > > + if (IS_ERR(clk_ipg)) { > > + pr_err("i.MX EPIT: unable to get clk_ipg\n"); > > + ret = PTR_ERR(clk_ipg); > > + goto out_kfree; > > + } > > + clk_prepare_enable(clk_ipg); > clk_prepare_enable() may fail, so better check its return value and > propagate it in the case of error. > > + > > + epittm->clk_per = of_clk_get_by_name(np, "per"); > > + if (IS_ERR(epittm->clk_per)) { > > + pr_err("i.MX EPIT: unable to get clk_per\n"); > > + ret = PTR_ERR(epittm->clk_per); > > + goto out_clk_ipg_disable; > > + } > > + clk_prepare_enable(epittm->clk_per); > Ditto. > > + > > + /* > > + * Initialise to a known state (all timers off, and timing reset) > > + */ > For a single line of comment you could simply write: > /* bla bla */ Thanks for the review, i will send a new version Clement