* [PATCH] drivers/rtc/rtc-mrst.c: Use release_mem_region after request_mem_region @ 2011-03-14 20:58 Julia Lawall 2011-04-07 23:01 ` [PATCH] drivers/rtc/rtc-mrst.c: Use release_mem_region after Andrew Morton 0 siblings, 1 reply; 3+ messages in thread From: Julia Lawall @ 2011-03-14 20:58 UTC (permalink / raw) To: Alessandro Zummo; +Cc: kernel-janitors, rtc-linux, linux-kernel The memory allocated using request_mem_region should be released using release_mem_region, not release_region. The semantic patch that fixes part of this problem is as follows: (http://coccinelle.lip6.fr/) // <smpl> @@ expression E1,E2,E3; @@ request_mem_region(E1,E2,E3) ... ?- release_region(E1,E2) + release_mem_region(E1,E2) // </smpl> Signed-off-by: Julia Lawall <julia@diku.dk> --- drivers/rtc/rtc-mrst.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/rtc/rtc-mrst.c b/drivers/rtc/rtc-mrst.c index b86bc32..558dd7d 100644 --- a/drivers/rtc/rtc-mrst.c +++ b/drivers/rtc/rtc-mrst.c @@ -379,7 +379,7 @@ cleanup1: mrst_rtc.dev = NULL; rtc_device_unregister(mrst_rtc.rtc); cleanup0: - release_region(iomem->start, iomem->end + 1 - iomem->start); + release_mem_region(iomem->start, iomem->end + 1 - iomem->start); dev_err(dev, "rtc-mrst: unable to initialise\n"); return retval; } @@ -405,7 +405,7 @@ static void __exit rtc_mrst_do_remove(struct device *dev) mrst->rtc = NULL; iomem = mrst->iomem; - release_region(iomem->start, iomem->end + 1 - iomem->start); + release_mem_region(iomem->start, iomem->end + 1 - iomem->start); mrst->iomem = NULL; mrst->dev = NULL; ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drivers/rtc/rtc-mrst.c: Use release_mem_region after 2011-03-14 20:58 [PATCH] drivers/rtc/rtc-mrst.c: Use release_mem_region after request_mem_region Julia Lawall @ 2011-04-07 23:01 ` Andrew Morton 2011-04-08 5:14 ` Julia Lawall 0 siblings, 1 reply; 3+ messages in thread From: Andrew Morton @ 2011-04-07 23:01 UTC (permalink / raw) To: Julia Lawall; +Cc: Alessandro Zummo, kernel-janitors, rtc-linux, linux-kernel On Mon, 14 Mar 2011 22:00:25 +0100 Julia Lawall <julia@diku.dk> wrote: > The memory allocated using request_mem_region should be released using > release_mem_region, not release_region. > > The semantic patch that fixes part of this problem is as follows: > (http://coccinelle.lip6.fr/) > > // <smpl> > @@ > expression E1,E2,E3; > @@ > > request_mem_region(E1,E2,E3) > ... > ?- release_region(E1,E2) > + release_mem_region(E1,E2) > // </smpl> > > Signed-off-by: Julia Lawall <julia@diku.dk> > > --- > drivers/rtc/rtc-mrst.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/rtc/rtc-mrst.c b/drivers/rtc/rtc-mrst.c > index b86bc32..558dd7d 100644 > --- a/drivers/rtc/rtc-mrst.c > +++ b/drivers/rtc/rtc-mrst.c > @@ -379,7 +379,7 @@ cleanup1: > mrst_rtc.dev = NULL; > rtc_device_unregister(mrst_rtc.rtc); > cleanup0: > - release_region(iomem->start, iomem->end + 1 - iomem->start); > + release_mem_region(iomem->start, iomem->end + 1 - iomem->start); > dev_err(dev, "rtc-mrst: unable to initialise\n"); > return retval; > } > @@ -405,7 +405,7 @@ static void __exit rtc_mrst_do_remove(struct device *dev) > mrst->rtc = NULL; > > iomem = mrst->iomem; > - release_region(iomem->start, iomem->end + 1 - iomem->start); > + release_mem_region(iomem->start, iomem->end + 1 - iomem->start); > mrst->iomem = NULL; > > mrst->dev = NULL; We may as well clean this up too. --- a/drivers/rtc/rtc-mrst.c~drivers-rtc-rtc-mrstc-use-release_mem_region-after-request_mem_region-fix +++ a/drivers/rtc/rtc-mrst.c @@ -380,7 +380,7 @@ cleanup1: cleanup0: dev_set_drvdata(dev, NULL); mrst_rtc.dev = NULL; - release_mem_region(iomem->start, iomem->end + 1 - iomem->start); + release_mem_region(iomem->start, resource_size(iomem)); dev_err(dev, "rtc-mrst: unable to initialise\n"); return retval; } @@ -406,7 +406,7 @@ static void __devexit rtc_mrst_do_remove mrst->rtc = NULL; iomem = mrst->iomem; - release_mem_region(iomem->start, iomem->end + 1 - iomem->start); + release_mem_region(iomem->start, resource_size(iomem)); mrst->iomem = NULL; mrst->dev = NULL; _ which makes one wonder why we don't have void release_resource_mem(struct resource *r) { release_mem_region(r->start, resource_size(r)); } ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drivers/rtc/rtc-mrst.c: Use release_mem_region after 2011-04-07 23:01 ` [PATCH] drivers/rtc/rtc-mrst.c: Use release_mem_region after Andrew Morton @ 2011-04-08 5:14 ` Julia Lawall 0 siblings, 0 replies; 3+ messages in thread From: Julia Lawall @ 2011-04-08 5:14 UTC (permalink / raw) To: Andrew Morton; +Cc: Alessandro Zummo, kernel-janitors, rtc-linux, linux-kernel On Thu, 7 Apr 2011, Andrew Morton wrote: > On Mon, 14 Mar 2011 22:00:25 +0100 > Julia Lawall <julia@diku.dk> wrote: > > > The memory allocated using request_mem_region should be released using > > release_mem_region, not release_region. > > > > The semantic patch that fixes part of this problem is as follows: > > (http://coccinelle.lip6.fr/) > > > > // <smpl> > > @@ > > expression E1,E2,E3; > > @@ > > > > request_mem_region(E1,E2,E3) > > ... > > ?- release_region(E1,E2) > > + release_mem_region(E1,E2) > > // </smpl> > > > > Signed-off-by: Julia Lawall <julia@diku.dk> > > > > --- > > drivers/rtc/rtc-mrst.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/rtc/rtc-mrst.c b/drivers/rtc/rtc-mrst.c > > index b86bc32..558dd7d 100644 > > --- a/drivers/rtc/rtc-mrst.c > > +++ b/drivers/rtc/rtc-mrst.c > > @@ -379,7 +379,7 @@ cleanup1: > > mrst_rtc.dev = NULL; > > rtc_device_unregister(mrst_rtc.rtc); > > cleanup0: > > - release_region(iomem->start, iomem->end + 1 - iomem->start); > > + release_mem_region(iomem->start, iomem->end + 1 - iomem->start); > > dev_err(dev, "rtc-mrst: unable to initialise\n"); > > return retval; > > } > > @@ -405,7 +405,7 @@ static void __exit rtc_mrst_do_remove(struct device *dev) > > mrst->rtc = NULL; > > > > iomem = mrst->iomem; > > - release_region(iomem->start, iomem->end + 1 - iomem->start); > > + release_mem_region(iomem->start, iomem->end + 1 - iomem->start); > > mrst->iomem = NULL; > > > > mrst->dev = NULL; > > We may as well clean this up too. > > --- a/drivers/rtc/rtc-mrst.c~drivers-rtc-rtc-mrstc-use-release_mem_region-after-request_mem_region-fix > +++ a/drivers/rtc/rtc-mrst.c > @@ -380,7 +380,7 @@ cleanup1: > cleanup0: > dev_set_drvdata(dev, NULL); > mrst_rtc.dev = NULL; > - release_mem_region(iomem->start, iomem->end + 1 - iomem->start); > + release_mem_region(iomem->start, resource_size(iomem)); > dev_err(dev, "rtc-mrst: unable to initialise\n"); > return retval; > } > @@ -406,7 +406,7 @@ static void __devexit rtc_mrst_do_remove > mrst->rtc = NULL; > > iomem = mrst->iomem; > - release_mem_region(iomem->start, iomem->end + 1 - iomem->start); > + release_mem_region(iomem->start, resource_size(iomem)); > mrst->iomem = NULL; > > mrst->dev = NULL; > _ > > which makes one wonder why we don't have > > void release_resource_mem(struct resource *r) > { > release_mem_region(r->start, resource_size(r)); > } Perhaps the name is not quite good, because release resource is something different from: void release_resource(struct resource *r) { release_region(r->start, resource_size(r)); } It would be really nice to have such a wrapper for both of them, since the code is so common, and the size computation is so error prone. julia ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-04-08 5:14 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-03-14 20:58 [PATCH] drivers/rtc/rtc-mrst.c: Use release_mem_region after request_mem_region Julia Lawall 2011-04-07 23:01 ` [PATCH] drivers/rtc/rtc-mrst.c: Use release_mem_region after Andrew Morton 2011-04-08 5:14 ` Julia Lawall
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox