* [PATCH] watchdog: davinci_wdt: update to devm_* API
@ 2013-02-07 3:32 Kumar, Anil
2013-02-07 11:21 ` Sergei Shtylyov
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Kumar, Anil @ 2013-02-07 3:32 UTC (permalink / raw)
To: linux-watchdog, linux-kernel, davinci-linux-open-source
Cc: wim, nsekhar, anilkumar.v
Update the code to use devm_* API so that driver
core will manage resources.
Signed-off-by: Kumar, Anil <anilkumar.v@ti.com>
---
This patch applies on top of v3.8-rc6.
Tested on da850 EVM.
:100644 100644 e8e8724... 6ad76a3... M drivers/watchdog/davinci_wdt.c
drivers/watchdog/davinci_wdt.c | 34 +++++++++-------------------------
1 files changed, 9 insertions(+), 25 deletions(-)
diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
index e8e8724..6ad76a3 100644
--- a/drivers/watchdog/davinci_wdt.c
+++ b/drivers/watchdog/davinci_wdt.c
@@ -69,7 +69,6 @@ static unsigned long wdt_status;
#define WDT_REGION_INITED 2
#define WDT_DEVICE_INITED 3
-static struct resource *wdt_mem;
static void __iomem *wdt_base;
struct clk *wdt_clk;
@@ -201,10 +200,10 @@ static struct miscdevice davinci_wdt_miscdev = {
static int davinci_wdt_probe(struct platform_device *pdev)
{
- int ret = 0, size;
- struct device *dev = &pdev->dev;
+ int ret = 0;
+ static struct resource *wdt_mem;
- wdt_clk = clk_get(dev, NULL);
+ wdt_clk = clk_get(&pdev->dev, NULL);
if (WARN_ON(IS_ERR(wdt_clk)))
return PTR_ERR(wdt_clk);
@@ -213,49 +212,34 @@ static int davinci_wdt_probe(struct platform_device *pdev)
if (heartbeat < 1 || heartbeat > MAX_HEARTBEAT)
heartbeat = DEFAULT_HEARTBEAT;
- dev_info(dev, "heartbeat %d sec\n", heartbeat);
+ dev_info(&pdev->dev, "heartbeat %d sec\n", heartbeat);
wdt_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (wdt_mem == NULL) {
- dev_err(dev, "failed to get memory region resource\n");
+ dev_err(&pdev->dev, "failed to get memory region resource\n");
return -ENOENT;
}
- size = resource_size(wdt_mem);
- if (!request_mem_region(wdt_mem->start, size, pdev->name)) {
- dev_err(dev, "failed to get memory region\n");
- return -ENOENT;
- }
-
- wdt_base = ioremap(wdt_mem->start, size);
+ wdt_base = devm_request_and_ioremap(&pdev->dev, wdt_mem);
if (!wdt_base) {
- dev_err(dev, "failed to map memory region\n");
- release_mem_region(wdt_mem->start, size);
- wdt_mem = NULL;
+ dev_err(&pdev->dev, "ioremap failed\n");
return -ENOMEM;
}
ret = misc_register(&davinci_wdt_miscdev);
if (ret < 0) {
- dev_err(dev, "cannot register misc device\n");
- release_mem_region(wdt_mem->start, size);
- wdt_mem = NULL;
+ dev_err(&pdev->dev, "cannot register misc device\n");
} else {
set_bit(WDT_DEVICE_INITED, &wdt_status);
+ return ret;
}
- iounmap(wdt_base);
return ret;
}
static int davinci_wdt_remove(struct platform_device *pdev)
{
misc_deregister(&davinci_wdt_miscdev);
- if (wdt_mem) {
- release_mem_region(wdt_mem->start, resource_size(wdt_mem));
- wdt_mem = NULL;
- }
-
clk_disable_unprepare(wdt_clk);
clk_put(wdt_clk);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] watchdog: davinci_wdt: update to devm_* API
2013-02-07 3:32 [PATCH] watchdog: davinci_wdt: update to devm_* API Kumar, Anil
@ 2013-02-07 11:21 ` Sergei Shtylyov
2013-02-08 2:19 ` Kumar, Anil
2013-02-07 17:50 ` Sekhar Nori
2013-02-08 5:37 ` Kumar, Anil
2 siblings, 1 reply; 13+ messages in thread
From: Sergei Shtylyov @ 2013-02-07 11:21 UTC (permalink / raw)
To: Kumar, Anil; +Cc: linux-watchdog, linux-kernel, davinci-linux-open-source, wim
Hello.
On 07-02-2013 7:32, Kumar, Anil wrote:
> Update the code to use devm_* API so that driver
> core will manage resources.
> Signed-off-by: Kumar, Anil <anilkumar.v@ti.com>
> ---
> This patch applies on top of v3.8-rc6.
> Tested on da850 EVM.
> :100644 100644 e8e8724... 6ad76a3... M drivers/watchdog/davinci_wdt.c
> drivers/watchdog/davinci_wdt.c | 34 +++++++++-------------------------
> 1 files changed, 9 insertions(+), 25 deletions(-)
> diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
> index e8e8724..6ad76a3 100644
> --- a/drivers/watchdog/davinci_wdt.c
> +++ b/drivers/watchdog/davinci_wdt.c
[...]
> @@ -213,49 +212,34 @@ static int davinci_wdt_probe(struct platform_device *pdev)
[...]
> - size = resource_size(wdt_mem);
> - if (!request_mem_region(wdt_mem->start, size, pdev->name)) {
> - dev_err(dev, "failed to get memory region\n");
> - return -ENOENT;
> - }
> -
> - wdt_base = ioremap(wdt_mem->start, size);
> + wdt_base = devm_request_and_ioremap(&pdev->dev, wdt_mem);
> if (!wdt_base) {
> - dev_err(dev, "failed to map memory region\n");
> - release_mem_region(wdt_mem->start, size);
> - wdt_mem = NULL;
> + dev_err(&pdev->dev, "ioremap failed\n");
> return -ENOMEM;
Comment to devm_request_and_ioremap() suggest returning -EADDRNOTAVAIL
instead.
WBR, Sergei
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] watchdog: davinci_wdt: update to devm_* API
2013-02-07 3:32 [PATCH] watchdog: davinci_wdt: update to devm_* API Kumar, Anil
2013-02-07 11:21 ` Sergei Shtylyov
@ 2013-02-07 17:50 ` Sekhar Nori
2013-02-08 2:35 ` Kumar, Anil
2013-02-08 5:37 ` Kumar, Anil
2 siblings, 1 reply; 13+ messages in thread
From: Sekhar Nori @ 2013-02-07 17:50 UTC (permalink / raw)
To: Kumar, Anil; +Cc: linux-watchdog, linux-kernel, davinci-linux-open-source, wim
On 2/7/2013 9:02 AM, Kumar, Anil wrote:
> Update the code to use devm_* API so that driver
> core will manage resources.
>
> Signed-off-by: Kumar, Anil <anilkumar.v@ti.com>
> ---
> This patch applies on top of v3.8-rc6.
>
> Tested on da850 EVM.
>
> :100644 100644 e8e8724... 6ad76a3... M drivers/watchdog/davinci_wdt.c
> drivers/watchdog/davinci_wdt.c | 34 +++++++++-------------------------
> 1 files changed, 9 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
> index e8e8724..6ad76a3 100644
> --- a/drivers/watchdog/davinci_wdt.c
> +++ b/drivers/watchdog/davinci_wdt.c
> @@ -69,7 +69,6 @@ static unsigned long wdt_status;
> #define WDT_REGION_INITED 2
> #define WDT_DEVICE_INITED 3
>
> -static struct resource *wdt_mem;
> static void __iomem *wdt_base;
> struct clk *wdt_clk;
>
> @@ -201,10 +200,10 @@ static struct miscdevice davinci_wdt_miscdev = {
>
> static int davinci_wdt_probe(struct platform_device *pdev)
> {
> - int ret = 0, size;
> - struct device *dev = &pdev->dev;
Its not clear why you had to drop use of this variable?
> + int ret = 0;
> + static struct resource *wdt_mem;
>
> - wdt_clk = clk_get(dev, NULL);
> + wdt_clk = clk_get(&pdev->dev, NULL);
When you are converting to use devres, why not convert this to
devm_clk_get() as well?
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] watchdog: davinci_wdt: update to devm_* API
2013-02-07 11:21 ` Sergei Shtylyov
@ 2013-02-08 2:19 ` Kumar, Anil
0 siblings, 0 replies; 13+ messages in thread
From: Kumar, Anil @ 2013-02-08 2:19 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: linux-watchdog, linux-kernel, davinci-linux-open-source, wim
Hi Sergei
On Thu, Feb 07, 2013 at 16:51:37, Sergei Shtylyov wrote:
> Hello.
>
> On 07-02-2013 7:32, Kumar, Anil wrote:
>
> > Update the code to use devm_* API so that driver
> > core will manage resources.
>
> > Signed-off-by: Kumar, Anil <anilkumar.v@ti.com>
> > ---
> > This patch applies on top of v3.8-rc6.
>
> > Tested on da850 EVM.
>
> > :100644 100644 e8e8724... 6ad76a3... M drivers/watchdog/davinci_wdt.c
> > drivers/watchdog/davinci_wdt.c | 34 +++++++++-------------------------
> > 1 files changed, 9 insertions(+), 25 deletions(-)
>
> > diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
> > index e8e8724..6ad76a3 100644
> > --- a/drivers/watchdog/davinci_wdt.c
> > +++ b/drivers/watchdog/davinci_wdt.c
> [...]
> > @@ -213,49 +212,34 @@ static int davinci_wdt_probe(struct platform_device *pdev)
> [...]
> > - size = resource_size(wdt_mem);
> > - if (!request_mem_region(wdt_mem->start, size, pdev->name)) {
> > - dev_err(dev, "failed to get memory region\n");
> > - return -ENOENT;
> > - }
> > -
> > - wdt_base = ioremap(wdt_mem->start, size);
> > + wdt_base = devm_request_and_ioremap(&pdev->dev, wdt_mem);
> > if (!wdt_base) {
> > - dev_err(dev, "failed to map memory region\n");
> > - release_mem_region(wdt_mem->start, size);
> > - wdt_mem = NULL;
> > + dev_err(&pdev->dev, "ioremap failed\n");
> > return -ENOMEM;
>
> Comment to devm_request_and_ioremap() suggest returning -EADDRNOTAVAIL
> instead.
Right.
Thanks,
Anil
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] watchdog: davinci_wdt: update to devm_* API
@ 2013-02-08 2:19 ` Kumar, Anil
0 siblings, 0 replies; 13+ messages in thread
From: Kumar, Anil @ 2013-02-08 2:19 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: linux-watchdog, linux-kernel, davinci-linux-open-source, wim
Hi Sergei
On Thu, Feb 07, 2013 at 16:51:37, Sergei Shtylyov wrote:
> Hello.
>
> On 07-02-2013 7:32, Kumar, Anil wrote:
>
> > Update the code to use devm_* API so that driver
> > core will manage resources.
>
> > Signed-off-by: Kumar, Anil <anilkumar.v@ti.com>
> > ---
> > This patch applies on top of v3.8-rc6.
>
> > Tested on da850 EVM.
>
> > :100644 100644 e8e8724... 6ad76a3... M drivers/watchdog/davinci_wdt.c
> > drivers/watchdog/davinci_wdt.c | 34 +++++++++-------------------------
> > 1 files changed, 9 insertions(+), 25 deletions(-)
>
> > diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
> > index e8e8724..6ad76a3 100644
> > --- a/drivers/watchdog/davinci_wdt.c
> > +++ b/drivers/watchdog/davinci_wdt.c
> [...]
> > @@ -213,49 +212,34 @@ static int davinci_wdt_probe(struct platform_device *pdev)
> [...]
> > - size = resource_size(wdt_mem);
> > - if (!request_mem_region(wdt_mem->start, size, pdev->name)) {
> > - dev_err(dev, "failed to get memory region\n");
> > - return -ENOENT;
> > - }
> > -
> > - wdt_base = ioremap(wdt_mem->start, size);
> > + wdt_base = devm_request_and_ioremap(&pdev->dev, wdt_mem);
> > if (!wdt_base) {
> > - dev_err(dev, "failed to map memory region\n");
> > - release_mem_region(wdt_mem->start, size);
> > - wdt_mem = NULL;
> > + dev_err(&pdev->dev, "ioremap failed\n");
> > return -ENOMEM;
>
> Comment to devm_request_and_ioremap() suggest returning -EADDRNOTAVAIL
> instead.
Right.
Thanks,
Anil
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] watchdog: davinci_wdt: update to devm_* API
2013-02-07 17:50 ` Sekhar Nori
@ 2013-02-08 2:35 ` Kumar, Anil
0 siblings, 0 replies; 13+ messages in thread
From: Kumar, Anil @ 2013-02-08 2:35 UTC (permalink / raw)
To: Nori, Sekhar; +Cc: linux-watchdog, linux-kernel, davinci-linux-open-source, wim
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1854 bytes --]
Hi Sekhar,
Thanks for the review.
On Thu, Feb 07, 2013 at 23:20:48, Nori, Sekhar wrote:
>
> On 2/7/2013 9:02 AM, Kumar, Anil wrote:
> > Update the code to use devm_* API so that driver
> > core will manage resources.
> >
> > Signed-off-by: Kumar, Anil <anilkumar.v@ti.com>
> > ---
> > This patch applies on top of v3.8-rc6.
> >
> > Tested on da850 EVM.
> >
> > :100644 100644 e8e8724... 6ad76a3... M drivers/watchdog/davinci_wdt.c
> > drivers/watchdog/davinci_wdt.c | 34 +++++++++-------------------------
> > 1 files changed, 9 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
> > index e8e8724..6ad76a3 100644
> > --- a/drivers/watchdog/davinci_wdt.c
> > +++ b/drivers/watchdog/davinci_wdt.c
> > @@ -69,7 +69,6 @@ static unsigned long wdt_status;
> > #define WDT_REGION_INITED 2
> > #define WDT_DEVICE_INITED 3
> >
> > -static struct resource *wdt_mem;
> > static void __iomem *wdt_base;
> > struct clk *wdt_clk;
> >
> > @@ -201,10 +200,10 @@ static struct miscdevice davinci_wdt_miscdev = {
> >
> > static int davinci_wdt_probe(struct platform_device *pdev)
> > {
> > - int ret = 0, size;
> > - struct device *dev = &pdev->dev;
>
> Its not clear why you had to drop use of this variable?
Actually, I have not found any particular need to take pointer
into dev and then use in the code. Rather we can directly use.
>
> > + int ret = 0;
> > + static struct resource *wdt_mem;
> >
> > - wdt_clk = clk_get(dev, NULL);
> > + wdt_clk = clk_get(&pdev->dev, NULL);
>
> When you are converting to use devres, why not convert this to
> devm_clk_get() as well?
>
Right. I will do it.
Thanks,
Anil
ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] watchdog: davinci_wdt: update to devm_* API
@ 2013-02-08 2:35 ` Kumar, Anil
0 siblings, 0 replies; 13+ messages in thread
From: Kumar, Anil @ 2013-02-08 2:35 UTC (permalink / raw)
To: Nori, Sekhar; +Cc: linux-watchdog, linux-kernel, davinci-linux-open-source, wim
Hi Sekhar,
Thanks for the review.
On Thu, Feb 07, 2013 at 23:20:48, Nori, Sekhar wrote:
>
> On 2/7/2013 9:02 AM, Kumar, Anil wrote:
> > Update the code to use devm_* API so that driver
> > core will manage resources.
> >
> > Signed-off-by: Kumar, Anil <anilkumar.v@ti.com>
> > ---
> > This patch applies on top of v3.8-rc6.
> >
> > Tested on da850 EVM.
> >
> > :100644 100644 e8e8724... 6ad76a3... M drivers/watchdog/davinci_wdt.c
> > drivers/watchdog/davinci_wdt.c | 34 +++++++++-------------------------
> > 1 files changed, 9 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
> > index e8e8724..6ad76a3 100644
> > --- a/drivers/watchdog/davinci_wdt.c
> > +++ b/drivers/watchdog/davinci_wdt.c
> > @@ -69,7 +69,6 @@ static unsigned long wdt_status;
> > #define WDT_REGION_INITED 2
> > #define WDT_DEVICE_INITED 3
> >
> > -static struct resource *wdt_mem;
> > static void __iomem *wdt_base;
> > struct clk *wdt_clk;
> >
> > @@ -201,10 +200,10 @@ static struct miscdevice davinci_wdt_miscdev = {
> >
> > static int davinci_wdt_probe(struct platform_device *pdev)
> > {
> > - int ret = 0, size;
> > - struct device *dev = &pdev->dev;
>
> Its not clear why you had to drop use of this variable?
Actually, I have not found any particular need to take pointer
into dev and then use in the code. Rather we can directly use.
>
> > + int ret = 0;
> > + static struct resource *wdt_mem;
> >
> > - wdt_clk = clk_get(dev, NULL);
> > + wdt_clk = clk_get(&pdev->dev, NULL);
>
> When you are converting to use devres, why not convert this to
> devm_clk_get() as well?
>
Right. I will do it.
Thanks,
Anil
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] watchdog: davinci_wdt: update to devm_* API
2013-02-08 2:35 ` Kumar, Anil
@ 2013-02-08 4:19 ` Sekhar Nori
-1 siblings, 0 replies; 13+ messages in thread
From: Sekhar Nori @ 2013-02-08 4:19 UTC (permalink / raw)
To: Kumar, Anil; +Cc: linux-watchdog, linux-kernel, davinci-linux-open-source, wim
On 2/8/2013 8:05 AM, Kumar, Anil wrote:
> On Thu, Feb 07, 2013 at 23:20:48, Nori, Sekhar wrote:
>> On 2/7/2013 9:02 AM, Kumar, Anil wrote:
>>> Update the code to use devm_* API so that driver
>>> core will manage resources.
>>> Signed-off-by: Kumar, Anil <anilkumar.v@ti.com>
>>> diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
>>> @@ -201,10 +200,10 @@ static struct miscdevice davinci_wdt_miscdev = {
>>>
>>> static int davinci_wdt_probe(struct platform_device *pdev)
>>> {
>>> - int ret = 0, size;
>>> - struct device *dev = &pdev->dev;
>>
>> Its not clear why you had to drop use of this variable?
>
> Actually, I have not found any particular need to take pointer
> into dev and then use in the code. Rather we can directly use.
No, it is good enough as-is. It will help rid your patch of unnecessary
changes and its not really convenient to to keep reading &pdev->dev all
the time.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] watchdog: davinci_wdt: update to devm_* API
@ 2013-02-08 4:19 ` Sekhar Nori
0 siblings, 0 replies; 13+ messages in thread
From: Sekhar Nori @ 2013-02-08 4:19 UTC (permalink / raw)
To: Kumar, Anil; +Cc: linux-watchdog, linux-kernel, davinci-linux-open-source, wim
On 2/8/2013 8:05 AM, Kumar, Anil wrote:
> On Thu, Feb 07, 2013 at 23:20:48, Nori, Sekhar wrote:
>> On 2/7/2013 9:02 AM, Kumar, Anil wrote:
>>> Update the code to use devm_* API so that driver
>>> core will manage resources.
>>> Signed-off-by: Kumar, Anil <anilkumar.v@ti.com>
>>> diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
>>> @@ -201,10 +200,10 @@ static struct miscdevice davinci_wdt_miscdev = {
>>>
>>> static int davinci_wdt_probe(struct platform_device *pdev)
>>> {
>>> - int ret = 0, size;
>>> - struct device *dev = &pdev->dev;
>>
>> Its not clear why you had to drop use of this variable?
>
> Actually, I have not found any particular need to take pointer
> into dev and then use in the code. Rather we can directly use.
No, it is good enough as-is. It will help rid your patch of unnecessary
changes and its not really convenient to to keep reading &pdev->dev all
the time.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] watchdog: davinci_wdt: update to devm_* API
2013-02-08 4:19 ` Sekhar Nori
@ 2013-02-08 4:21 ` Kumar, Anil
-1 siblings, 0 replies; 13+ messages in thread
From: Kumar, Anil @ 2013-02-08 4:21 UTC (permalink / raw)
To: Nori, Sekhar; +Cc: linux-watchdog, linux-kernel, davinci-linux-open-source, wim
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1217 bytes --]
On Fri, Feb 08, 2013 at 09:49:27, Nori, Sekhar wrote:
> On 2/8/2013 8:05 AM, Kumar, Anil wrote:
> > On Thu, Feb 07, 2013 at 23:20:48, Nori, Sekhar wrote:
> >> On 2/7/2013 9:02 AM, Kumar, Anil wrote:
>
> >>> Update the code to use devm_* API so that driver
> >>> core will manage resources.
>
> >>> Signed-off-by: Kumar, Anil <anilkumar.v@ti.com>
>
> >>> diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
>
> >>> @@ -201,10 +200,10 @@ static struct miscdevice davinci_wdt_miscdev = {
> >>>
> >>> static int davinci_wdt_probe(struct platform_device *pdev)
> >>> {
> >>> - int ret = 0, size;
> >>> - struct device *dev = &pdev->dev;
> >>
> >> Its not clear why you had to drop use of this variable?
> >
> > Actually, I have not found any particular need to take pointer
> > into dev and then use in the code. Rather we can directly use.
>
> No, it is good enough as-is. It will help rid your patch of unnecessary
> changes and its not really convenient to to keep reading &pdev->dev all
> the time.
>
Ok
Thanks,
Anil
ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] watchdog: davinci_wdt: update to devm_* API
@ 2013-02-08 4:21 ` Kumar, Anil
0 siblings, 0 replies; 13+ messages in thread
From: Kumar, Anil @ 2013-02-08 4:21 UTC (permalink / raw)
To: Nori, Sekhar; +Cc: linux-watchdog, linux-kernel, davinci-linux-open-source, wim
On Fri, Feb 08, 2013 at 09:49:27, Nori, Sekhar wrote:
> On 2/8/2013 8:05 AM, Kumar, Anil wrote:
> > On Thu, Feb 07, 2013 at 23:20:48, Nori, Sekhar wrote:
> >> On 2/7/2013 9:02 AM, Kumar, Anil wrote:
>
> >>> Update the code to use devm_* API so that driver
> >>> core will manage resources.
>
> >>> Signed-off-by: Kumar, Anil <anilkumar.v@ti.com>
>
> >>> diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
>
> >>> @@ -201,10 +200,10 @@ static struct miscdevice davinci_wdt_miscdev = {
> >>>
> >>> static int davinci_wdt_probe(struct platform_device *pdev)
> >>> {
> >>> - int ret = 0, size;
> >>> - struct device *dev = &pdev->dev;
> >>
> >> Its not clear why you had to drop use of this variable?
> >
> > Actually, I have not found any particular need to take pointer
> > into dev and then use in the code. Rather we can directly use.
>
> No, it is good enough as-is. It will help rid your patch of unnecessary
> changes and its not really convenient to to keep reading &pdev->dev all
> the time.
>
Ok
Thanks,
Anil
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] watchdog: davinci_wdt: update to devm_* API
2013-02-07 3:32 [PATCH] watchdog: davinci_wdt: update to devm_* API Kumar, Anil
@ 2013-02-08 5:37 ` Kumar, Anil
2013-02-07 17:50 ` Sekhar Nori
2013-02-08 5:37 ` Kumar, Anil
2 siblings, 0 replies; 13+ messages in thread
From: Kumar, Anil @ 2013-02-08 5:37 UTC (permalink / raw)
To: linux-watchdog, linux-kernel, davinci-linux-open-source
Cc: wim, Nori, Sekhar, Kumar, Anil
On Thu, Feb 07, 2013 at 09:02:15, Kumar, Anil wrote:
> Update the code to use devm_* API so that driver
> core will manage resources.
>
> Signed-off-by: Kumar, Anil <anilkumar.v@ti.com>
> ---
> This patch applies on top of v3.8-rc6.
>
> Tested on da850 EVM.
>
> :100644 100644 e8e8724... 6ad76a3... M drivers/watchdog/davinci_wdt.c
> drivers/watchdog/davinci_wdt.c | 34 +++++++++-------------------------
> 1 files changed, 9 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
> index e8e8724..6ad76a3 100644
> --- a/drivers/watchdog/davinci_wdt.c
> +++ b/drivers/watchdog/davinci_wdt.c
> @@ -69,7 +69,6 @@ static unsigned long wdt_status;
> #define WDT_REGION_INITED 2
> #define WDT_DEVICE_INITED 3
>
> -static struct resource *wdt_mem;
> static void __iomem *wdt_base;
> struct clk *wdt_clk;
>
> @@ -201,10 +200,10 @@ static struct miscdevice davinci_wdt_miscdev = {
>
> static int davinci_wdt_probe(struct platform_device *pdev)
> {
> - int ret = 0, size;
> - struct device *dev = &pdev->dev;
> + int ret = 0;
> + static struct resource *wdt_mem;
>
> - wdt_clk = clk_get(dev, NULL);
> + wdt_clk = clk_get(&pdev->dev, NULL);
> if (WARN_ON(IS_ERR(wdt_clk)))
> return PTR_ERR(wdt_clk);
>
> @@ -213,49 +212,34 @@ static int davinci_wdt_probe(struct platform_device *pdev)
> if (heartbeat < 1 || heartbeat > MAX_HEARTBEAT)
> heartbeat = DEFAULT_HEARTBEAT;
>
> - dev_info(dev, "heartbeat %d sec\n", heartbeat);
> + dev_info(&pdev->dev, "heartbeat %d sec\n", heartbeat);
>
> wdt_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (wdt_mem == NULL) {
> - dev_err(dev, "failed to get memory region resource\n");
> + dev_err(&pdev->dev, "failed to get memory region resource\n");
> return -ENOENT;
> }
>
> - size = resource_size(wdt_mem);
> - if (!request_mem_region(wdt_mem->start, size, pdev->name)) {
> - dev_err(dev, "failed to get memory region\n");
> - return -ENOENT;
> - }
> -
> - wdt_base = ioremap(wdt_mem->start, size);
> + wdt_base = devm_request_and_ioremap(&pdev->dev, wdt_mem);
> if (!wdt_base) {
> - dev_err(dev, "failed to map memory region\n");
> - release_mem_region(wdt_mem->start, size);
> - wdt_mem = NULL;
> + dev_err(&pdev->dev, "ioremap failed\n");
> return -ENOMEM;
> }
>
> ret = misc_register(&davinci_wdt_miscdev);
> if (ret < 0) {
> - dev_err(dev, "cannot register misc device\n");
> - release_mem_region(wdt_mem->start, size);
> - wdt_mem = NULL;
> + dev_err(&pdev->dev, "cannot register misc device\n");
> } else {
> set_bit(WDT_DEVICE_INITED, &wdt_status);
> + return ret;
No need of return "ret" as it is retuning "ret" just below.
I will fix it in V2.
> }
>
> - iounmap(wdt_base);
> return ret;
> }
>
> static int davinci_wdt_remove(struct platform_device *pdev)
> {
> misc_deregister(&davinci_wdt_miscdev);
> - if (wdt_mem) {
> - release_mem_region(wdt_mem->start, resource_size(wdt_mem));
> - wdt_mem = NULL;
> - }
> -
> clk_disable_unprepare(wdt_clk);
> clk_put(wdt_clk);
>
> --
> 1.7.4.1
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] watchdog: davinci_wdt: update to devm_* API
@ 2013-02-08 5:37 ` Kumar, Anil
0 siblings, 0 replies; 13+ messages in thread
From: Kumar, Anil @ 2013-02-08 5:37 UTC (permalink / raw)
To: linux-watchdog, linux-kernel, davinci-linux-open-source
Cc: wim, Nori, Sekhar, Kumar, Anil
On Thu, Feb 07, 2013 at 09:02:15, Kumar, Anil wrote:
> Update the code to use devm_* API so that driver
> core will manage resources.
>
> Signed-off-by: Kumar, Anil <anilkumar.v@ti.com>
> ---
> This patch applies on top of v3.8-rc6.
>
> Tested on da850 EVM.
>
> :100644 100644 e8e8724... 6ad76a3... M drivers/watchdog/davinci_wdt.c
> drivers/watchdog/davinci_wdt.c | 34 +++++++++-------------------------
> 1 files changed, 9 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
> index e8e8724..6ad76a3 100644
> --- a/drivers/watchdog/davinci_wdt.c
> +++ b/drivers/watchdog/davinci_wdt.c
> @@ -69,7 +69,6 @@ static unsigned long wdt_status;
> #define WDT_REGION_INITED 2
> #define WDT_DEVICE_INITED 3
>
> -static struct resource *wdt_mem;
> static void __iomem *wdt_base;
> struct clk *wdt_clk;
>
> @@ -201,10 +200,10 @@ static struct miscdevice davinci_wdt_miscdev = {
>
> static int davinci_wdt_probe(struct platform_device *pdev)
> {
> - int ret = 0, size;
> - struct device *dev = &pdev->dev;
> + int ret = 0;
> + static struct resource *wdt_mem;
>
> - wdt_clk = clk_get(dev, NULL);
> + wdt_clk = clk_get(&pdev->dev, NULL);
> if (WARN_ON(IS_ERR(wdt_clk)))
> return PTR_ERR(wdt_clk);
>
> @@ -213,49 +212,34 @@ static int davinci_wdt_probe(struct platform_device *pdev)
> if (heartbeat < 1 || heartbeat > MAX_HEARTBEAT)
> heartbeat = DEFAULT_HEARTBEAT;
>
> - dev_info(dev, "heartbeat %d sec\n", heartbeat);
> + dev_info(&pdev->dev, "heartbeat %d sec\n", heartbeat);
>
> wdt_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (wdt_mem == NULL) {
> - dev_err(dev, "failed to get memory region resource\n");
> + dev_err(&pdev->dev, "failed to get memory region resource\n");
> return -ENOENT;
> }
>
> - size = resource_size(wdt_mem);
> - if (!request_mem_region(wdt_mem->start, size, pdev->name)) {
> - dev_err(dev, "failed to get memory region\n");
> - return -ENOENT;
> - }
> -
> - wdt_base = ioremap(wdt_mem->start, size);
> + wdt_base = devm_request_and_ioremap(&pdev->dev, wdt_mem);
> if (!wdt_base) {
> - dev_err(dev, "failed to map memory region\n");
> - release_mem_region(wdt_mem->start, size);
> - wdt_mem = NULL;
> + dev_err(&pdev->dev, "ioremap failed\n");
> return -ENOMEM;
> }
>
> ret = misc_register(&davinci_wdt_miscdev);
> if (ret < 0) {
> - dev_err(dev, "cannot register misc device\n");
> - release_mem_region(wdt_mem->start, size);
> - wdt_mem = NULL;
> + dev_err(&pdev->dev, "cannot register misc device\n");
> } else {
> set_bit(WDT_DEVICE_INITED, &wdt_status);
> + return ret;
No need of return "ret" as it is retuning "ret" just below.
I will fix it in V2.
> }
>
> - iounmap(wdt_base);
> return ret;
> }
>
> static int davinci_wdt_remove(struct platform_device *pdev)
> {
> misc_deregister(&davinci_wdt_miscdev);
> - if (wdt_mem) {
> - release_mem_region(wdt_mem->start, resource_size(wdt_mem));
> - wdt_mem = NULL;
> - }
> -
> clk_disable_unprepare(wdt_clk);
> clk_put(wdt_clk);
>
> --
> 1.7.4.1
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2013-02-08 5:38 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-07 3:32 [PATCH] watchdog: davinci_wdt: update to devm_* API Kumar, Anil
2013-02-07 11:21 ` Sergei Shtylyov
2013-02-08 2:19 ` Kumar, Anil
2013-02-08 2:19 ` Kumar, Anil
2013-02-07 17:50 ` Sekhar Nori
2013-02-08 2:35 ` Kumar, Anil
2013-02-08 2:35 ` Kumar, Anil
2013-02-08 4:19 ` Sekhar Nori
2013-02-08 4:19 ` Sekhar Nori
2013-02-08 4:21 ` Kumar, Anil
2013-02-08 4:21 ` Kumar, Anil
2013-02-08 5:37 ` Kumar, Anil
2013-02-08 5:37 ` Kumar, Anil
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.