| Message ID | 1334238054-31600-1-git-send-email-hannuxx@iki.fi |
|---|---|
| State | Superseded |
| Headers | show |
> + if (info->tegra_rtc_irq <= 0) > + return -EBUSY; Interrupt support is needed only for wake up or timer support. Apart from them RTC can be used for reference time keeping. So, is it not good to print a warn message as "wake up with alarm is not supported" and proceed further instead of returning failure from the driver itself?
On 13/04/12 11:05 +0530, Venu Byravarasu wrote: > > + if (info->tegra_rtc_irq <= 0) > > + return -EBUSY; > Interrupt support is needed only for wake up or timer support. > Apart from them RTC can be used for reference time keeping. > > So, is it not good to print a warn message as "wake up > with alarm is not supported" and proceed further instead of > returning failure from the driver itself? I propose that this patch would go in as is, as this is mainly for adjusting probe/remove routines for using devm_* funcs, and to leave that above issue you mentioned to another patch, as it would be a change to current behavior. br, Hannu
On 04/12/2012 07:40 AM, Hannu Heikkinen wrote: > From: Hannu Heikkinen <ext-hannu.m.heikkinen@nokia.com> > > Use the devres managed resource functions in the probe routine. > Affects also in remove routine where previously used free and release > functions are not needed. > > The devm_* functions eliminate the need for manual resource releasing > and simplify error handling. Resources allocated by devm_* are freed > automatically on driver detach. > > Signed-off-by: Hannu Heikkinen <ext-hannu.m.heikkinen@nokia.com> Acked-by: Stephen Warren <swarren@wwwdotorg.org>
Should be fine. > -----Original Message----- > From: Hannu Heikkinen [mailto:hannuxx@iki.fi] > Sent: Friday, April 13, 2012 6:24 PM > To: Venu Byravarasu > Cc: rtc-linux@googlegroups.com; linux-kernel@vger.kernel.org; > a.zummo@towertech.it > Subject: Re: [PATCH] rtc: tegra: cleanup probe/remove routines > > On 13/04/12 11:05 +0530, Venu Byravarasu wrote: > > > + if (info->tegra_rtc_irq <= 0) > > > + return -EBUSY; > > Interrupt support is needed only for wake up or timer support. > > Apart from them RTC can be used for reference time keeping. > > > > So, is it not good to print a warn message as "wake up > > with alarm is not supported" and proceed further instead of > > returning failure from the driver itself? > > I propose that this patch would go in as is, as this is mainly for adjusting > probe/remove routines for using devm_* funcs, and to leave that above issue > you mentioned to another patch, as it would be a change to current behavior. > > > br, > Hannu
Sorry for top posting... any progress (applied etc) on this? Patch supposed to be OK as ti is... br, Hannu On Thu, Apr 12, 2012 at 4:40 PM, Hannu Heikkinen <hannuxx@iki.fi> wrote: > From: Hannu Heikkinen <ext-hannu.m.heikkinen@nokia.com> > > Use the devres managed resource functions in the probe routine. > Affects also in remove routine where previously used free and release > functions are not needed. > > The devm_* functions eliminate the need for manual resource releasing > and simplify error handling. Resources allocated by devm_* are freed > automatically on driver detach. > > Signed-off-by: Hannu Heikkinen <ext-hannu.m.heikkinen@nokia.com> > --- > drivers/rtc/rtc-tegra.c | 50 > ++++++++++++----------------------------------- > 1 file changed, 13 insertions(+), 37 deletions(-) > > diff --git a/drivers/rtc/rtc-tegra.c b/drivers/rtc/rtc-tegra.c > index 75259fe..c006025 100644 > --- a/drivers/rtc/rtc-tegra.c > +++ b/drivers/rtc/rtc-tegra.c > @@ -309,7 +309,8 @@ static int __devinit tegra_rtc_probe(struct > platform_device *pdev) > struct resource *res; > int ret; > > - info = kzalloc(sizeof(struct tegra_rtc_info), GFP_KERNEL); > + info = devm_kzalloc(&pdev->dev, sizeof(struct tegra_rtc_info), > + GFP_KERNEL); > if (!info) > return -ENOMEM; > > @@ -317,29 +318,18 @@ static int __devinit tegra_rtc_probe(struct > platform_device *pdev) > if (!res) { > dev_err(&pdev->dev, > "Unable to allocate resources for device.\n"); > - ret = -EBUSY; > - goto err_free_info; > + return -EBUSY; > } > > - if (!request_mem_region(res->start, resource_size(res), > pdev->name)) { > - dev_err(&pdev->dev, > - "Unable to request mem region for device.\n"); > - ret = -EBUSY; > - goto err_free_info; > + info->rtc_base = devm_request_and_ioremap(&pdev->dev, res); > + if (!info->rtc_base) { > + dev_err(&pdev->dev, "Unable to request mem region and grab > IOs for device.\n"); > + return -EBUSY; > } > > info->tegra_rtc_irq = platform_get_irq(pdev, 0); > - if (info->tegra_rtc_irq <= 0) { > - ret = -EBUSY; > - goto err_release_mem_region; > - } > - > - info->rtc_base = ioremap_nocache(res->start, resource_size(res)); > - if (!info->rtc_base) { > - dev_err(&pdev->dev, "Unable to grab IOs for device.\n"); > - ret = -EBUSY; > - goto err_release_mem_region; > - } > + if (info->tegra_rtc_irq <= 0) > + return -EBUSY; > > /* set context info. */ > info->pdev = pdev; > @@ -362,11 +352,12 @@ static int __devinit tegra_rtc_probe(struct > platform_device *pdev) > dev_err(&pdev->dev, > "Unable to register device (err=%d).\n", > ret); > - goto err_iounmap; > + return ret; > } > > - ret = request_irq(info->tegra_rtc_irq, tegra_rtc_irq_handler, > - IRQF_TRIGGER_HIGH, "rtc alarm", &pdev->dev); > + ret = devm_request_irq(&pdev->dev, info->tegra_rtc_irq, > + tegra_rtc_irq_handler, IRQF_TRIGGER_HIGH, > + "rtc alarm", &pdev->dev); > if (ret) { > dev_err(&pdev->dev, > "Unable to request interrupt for device > (err=%d).\n", > @@ -380,12 +371,6 @@ static int __devinit tegra_rtc_probe(struct > platform_device *pdev) > > err_dev_unreg: > rtc_device_unregister(info->rtc_dev); > -err_iounmap: > - iounmap(info->rtc_base); > -err_release_mem_region: > - release_mem_region(res->start, resource_size(res)); > -err_free_info: > - kfree(info); > > return ret; > } > @@ -393,17 +378,8 @@ err_free_info: > static int __devexit tegra_rtc_remove(struct platform_device *pdev) > { > struct tegra_rtc_info *info = platform_get_drvdata(pdev); > - struct resource *res; > - > - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > - if (!res) > - return -EBUSY; > > - free_irq(info->tegra_rtc_irq, &pdev->dev); > rtc_device_unregister(info->rtc_dev); > - iounmap(info->rtc_base); > - release_mem_region(res->start, resource_size(res)); > - kfree(info); > > platform_set_drvdata(pdev, NULL); > > -- > 1.7.9.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >
diff --git a/drivers/rtc/rtc-tegra.c b/drivers/rtc/rtc-tegra.c index 75259fe..c006025 100644 --- a/drivers/rtc/rtc-tegra.c +++ b/drivers/rtc/rtc-tegra.c @@ -309,7 +309,8 @@ static int __devinit tegra_rtc_probe(struct platform_device *pdev) struct resource *res; int ret; - info = kzalloc(sizeof(struct tegra_rtc_info), GFP_KERNEL); + info = devm_kzalloc(&pdev->dev, sizeof(struct tegra_rtc_info), + GFP_KERNEL); if (!info) return -ENOMEM; @@ -317,29 +318,18 @@ static int __devinit tegra_rtc_probe(struct platform_device *pdev) if (!res) { dev_err(&pdev->dev, "Unable to allocate resources for device.\n"); - ret = -EBUSY; - goto err_free_info; + return -EBUSY; } - if (!request_mem_region(res->start, resource_size(res), pdev->name)) { - dev_err(&pdev->dev, - "Unable to request mem region for device.\n"); - ret = -EBUSY; - goto err_free_info; + info->rtc_base = devm_request_and_ioremap(&pdev->dev, res); + if (!info->rtc_base) { + dev_err(&pdev->dev, "Unable to request mem region and grab IOs for device.\n"); + return -EBUSY; } info->tegra_rtc_irq = platform_get_irq(pdev, 0); - if (info->tegra_rtc_irq <= 0) { - ret = -EBUSY; - goto err_release_mem_region; - } - - info->rtc_base = ioremap_nocache(res->start, resource_size(res)); - if (!info->rtc_base) { - dev_err(&pdev->dev, "Unable to grab IOs for device.\n"); - ret = -EBUSY; - goto err_release_mem_region; - } + if (info->tegra_rtc_irq <= 0) + return -EBUSY; /* set context info. */ info->pdev = pdev; @@ -362,11 +352,12 @@ static int __devinit tegra_rtc_probe(struct platform_device *pdev) dev_err(&pdev->dev, "Unable to register device (err=%d).\n", ret); - goto err_iounmap; + return ret; } - ret = request_irq(info->tegra_rtc_irq, tegra_rtc_irq_handler, - IRQF_TRIGGER_HIGH, "rtc alarm", &pdev->dev); + ret = devm_request_irq(&pdev->dev, info->tegra_rtc_irq, + tegra_rtc_irq_handler, IRQF_TRIGGER_HIGH, + "rtc alarm", &pdev->dev); if (ret) { dev_err(&pdev->dev, "Unable to request interrupt for device (err=%d).\n", @@ -380,12 +371,6 @@ static int __devinit tegra_rtc_probe(struct platform_device *pdev) err_dev_unreg: rtc_device_unregister(info->rtc_dev); -err_iounmap: - iounmap(info->rtc_base); -err_release_mem_region: - release_mem_region(res->start, resource_size(res)); -err_free_info: - kfree(info); return ret; } @@ -393,17 +378,8 @@ err_free_info: static int __devexit tegra_rtc_remove(struct platform_device *pdev) { struct tegra_rtc_info *info = platform_get_drvdata(pdev); - struct resource *res; - - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!res) - return -EBUSY; - free_irq(info->tegra_rtc_irq, &pdev->dev); rtc_device_unregister(info->rtc_dev); - iounmap(info->rtc_base); - release_mem_region(res->start, resource_size(res)); - kfree(info); platform_set_drvdata(pdev, NULL);