HI Arvind, Thank you for the patch.
On Tuesday 20 Sep 2016 18:09:19 Arvind Yadav wrote: > From: Arvind Yadav <arvind.yadav...@gmail.com> > > Free memory and memory mapping , if cpg_mstp_clocks_init is not successful. > > Signed-off-by: Arvind Yadav <arvind.yadav...@gmail.com> > --- > drivers/clk/renesas/clk-mstp.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/renesas/clk-mstp.c b/drivers/clk/renesas/clk-mstp.c > index 5093a25..19e73c8 100644 > --- a/drivers/clk/renesas/clk-mstp.c > +++ b/drivers/clk/renesas/clk-mstp.c > @@ -179,14 +179,20 @@ static void __init cpg_mstp_clocks_init(struct > device_node *np) group->data.clks = clks; > > group->smstpcr = of_iomap(np, 0); > - group->mstpsr = of_iomap(np, 1); > - > if (group->smstpcr == NULL) { > pr_err("%s: failed to remap SMSTPCR\n", __func__); > kfree(group); > kfree(clks); > return; > } > + group->mstpsr = of_iomap(np, 1); > + if (group->mstpsr == NULL) { > + pr_err("%s: failed to remap MSTPCR\n", __func__); > + iounmap(group->smstpcr); > + kfree(group); > + kfree(clks); > + return; If you really want to do this (and I don't think that's worth it, an error here will render the system completely unbootable anyway, I'd rather remove the kfree() calls), you can test both group->smstpcr and group->mstpsr in a single go, with a single error path. You can then call iounmap() on both variables in the error path, either unconditionally if iounmap() is safe to be called on NULL pointers (to be checked, I haven't verified that personally), or conditionally based on the pointer value. > + } > > for (i = 0; i < MSTP_MAX_CLOCKS; ++i) > clks[i] = ERR_PTR(-ENOENT); > @@ -236,6 +242,10 @@ static void __init cpg_mstp_clocks_init(struct > device_node *np) } else { > pr_err("%s: failed to register %s %s clock (%ld)\n", > __func__, np->name, name, PTR_ERR(clks[clkidx])); > + iounmap(group->smstpcr); > + iounmap(group->mstpsr); > + kfree(group); > + kfree(clks); This is wrong, a single clock failing to be registered is not a fatal error, and should not free resources used by all other clocks in the group. > } > } -- Regards, Laurent Pinchart