| Message ID | 1381291240-23727-1-git-send-email-sangjung.woo@samsung.com |
|---|---|
| State | Superseded |
| Headers | show |
On 10/09/2013 01:07 PM, Joe Perches wrote: > On Wed, 2013-10-09 at 13:00 +0900, Sangjung Woo wrote: >> In order to be free automatically and make the cleanup paths more >> simple, use devm_kzalloc() instead of kzalloc(). > [] >> diff --git a/drivers/rtc/rtc-pl030.c b/drivers/rtc/rtc-pl030.c > [] >> @@ -106,7 +106,7 @@ static int pl030_probe(struct amba_device *dev, const struct amba_id *id) >> if (ret) >> goto err_req; >> >> - rtc = kmalloc(sizeof(*rtc), GFP_KERNEL); >> + rtc = devm_kzalloc(&dev->dev, sizeof(*rtc), GFP_KERNEL); First of all, thanks for your review. > You're not deleting a memset and you're > converting a kmalloc. You are right. > > Why do you need the zalloc version? > The key point of this patch is resource-managed memory allocation. As you already know, memory space that is allocated by devm_kzalloc() function is automatically freed on driver detach. That makes the code tidy and reduces human's mistakes not to kfree().
On Wed, 2013-10-09 at 13:36 +0900, sangjung.woo wrote: > On 10/09/2013 01:07 PM, Joe Perches wrote: > > On Wed, 2013-10-09 at 13:00 +0900, Sangjung Woo wrote: > >> In order to be free automatically and make the cleanup paths more > >> simple, use devm_kzalloc() instead of kzalloc(). > > [] > >> diff --git a/drivers/rtc/rtc-pl030.c b/drivers/rtc/rtc-pl030.c > > [] > >> @@ -106,7 +106,7 @@ static int pl030_probe(struct amba_device *dev, const struct amba_id *id) > >> if (ret) > >> goto err_req; > >> > >> - rtc = kmalloc(sizeof(*rtc), GFP_KERNEL); > >> + rtc = devm_kzalloc(&dev->dev, sizeof(*rtc), GFP_KERNEL); [] > > You're not deleting a memset and you're > > converting a kmalloc. > You are right. > > > > Why do you need the zalloc version? > > > The key point of this patch is resource-managed memory allocation. The commit message doesn't match the patch subject (shows kzalloc) I was a bit surprised to find there isn't a devm_kmalloc. This seems fine otherwise.
On 10/09/2013 01:59 PM, Joe Perches wrote: > The commit message doesn't match the patch subject > (shows kzalloc) > > I was a bit surprised to find there isn't a devm_kmalloc. > > This seems fine otherwise. I just sent the second patch file after modifying the commit message. Thank you for your opinion. Cheers, Sangjung Woo >
diff --git a/drivers/rtc/rtc-pl030.c b/drivers/rtc/rtc-pl030.c index 22bacdb..ecd57c6 100644 --- a/drivers/rtc/rtc-pl030.c +++ b/drivers/rtc/rtc-pl030.c @@ -106,7 +106,7 @@ static int pl030_probe(struct amba_device *dev, const struct amba_id *id) if (ret) goto err_req; - rtc = kmalloc(sizeof(*rtc), GFP_KERNEL); + rtc = devm_kzalloc(&dev->dev, sizeof(*rtc), GFP_KERNEL); if (!rtc) { ret = -ENOMEM; goto err_rtc; @@ -115,7 +115,7 @@ static int pl030_probe(struct amba_device *dev, const struct amba_id *id) rtc->base = ioremap(dev->res.start, resource_size(&dev->res)); if (!rtc->base) { ret = -ENOMEM; - goto err_map; + goto err_rtc; } __raw_writel(0, rtc->base + RTC_CR); @@ -141,8 +141,6 @@ static int pl030_probe(struct amba_device *dev, const struct amba_id *id) free_irq(dev->irq[0], rtc); err_irq: iounmap(rtc->base); - err_map: - kfree(rtc); err_rtc: amba_release_regions(dev); err_req: @@ -160,7 +158,6 @@ static int pl030_remove(struct amba_device *dev) free_irq(dev->irq[0], rtc); rtc_device_unregister(rtc->rtc); iounmap(rtc->base); - kfree(rtc); amba_release_regions(dev); return 0;
In order to be free automatically and make the cleanup paths more simple, use devm_kzalloc() instead of kzalloc(). Signed-off-by: Sangjung Woo <sangjung.woo@samsung.com> --- drivers/rtc/rtc-pl030.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)