Messages in this thread | | | From | Clément Péron <> | Date | Tue, 29 May 2018 17:59:51 +0200 | Subject | Re: [PATCH v2 4/5] clocksource: add driver for i.MX EPIT timer |
| |
Hi Fabio, On Tue, 29 May 2018 at 17:20, Fabio Estevam <festevam@gmail.com> wrote:
> On Tue, May 29, 2018 at 12:04 PM, Clément Péron <peron.clem@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
| |