[2/2] video:da8xx-fb: Convert to devm_* api

Commit Message

Lad, Prabhakar May 16, 2013, 7:10 a.m. UTC

From: Lad, Prabhakar <[e-mail]>

Use devm_ioremap_resource instead of reques_mem_region()/ioremap() and
devm_request_irq() instead of request_irq().

This ensures more consistent error values and simplifies error paths.

Signed-off-by: Lad, Prabhakar <[e-mail]>
---
 drivers/video/da8xx-fb.c |   39 +++++++--------------------------------
 1 files changed, 7 insertions(+), 32 deletions(-)

Comments

Lad, Prabhakar June 26, 2013, 11:20 a.m. UTC | #1

Hi Tomi/ Florian

On Thu, May 16, 2013 at 12:40 PM, Lad Prabhakar
<[e-mail]> wrote:
> From: Lad, Prabhakar <[e-mail]>
>
> Use devm_ioremap_resource instead of reques_mem_region()/ioremap() and
> devm_request_irq() instead of request_irq().
>
> This ensures more consistent error values and simplifies error paths.
>
> Signed-off-by: Lad, Prabhakar <[e-mail]>
> ---

Can you pick up this patch and drop 1/2.

Regards,
--Prabhakar Lad

Tomi Valkeinen June 26, 2013, 11:39 a.m. UTC | #2

On 16/05/13 10:10, Lad Prabhakar wrote:
> From: Lad, Prabhakar <[e-mail]>
> 
> Use devm_ioremap_resource instead of reques_mem_region()/ioremap() and
> devm_request_irq() instead of request_irq().
> 
> This ensures more consistent error values and simplifies error paths.
> 
> Signed-off-by: Lad, Prabhakar <[e-mail]>
> ---
>  drivers/video/da8xx-fb.c |   39 +++++++--------------------------------
>  1 files changed, 7 insertions(+), 32 deletions(-)

There's something similar in Darren's da8xx-fb series. Can you check if
there are differences?

 Tomi

Lad, Prabhakar June 26, 2013, 11:56 a.m. UTC | #3

Hi Tomi,

On Wed, Jun 26, 2013 at 5:09 PM, Tomi Valkeinen <[e-mail]> wrote:
> On 16/05/13 10:10, Lad Prabhakar wrote:
>> From: Lad, Prabhakar <[e-mail]>
>>
>> Use devm_ioremap_resource instead of reques_mem_region()/ioremap() and
>> devm_request_irq() instead of request_irq().
>>
>> This ensures more consistent error values and simplifies error paths.
>>
>> Signed-off-by: Lad, Prabhakar <[e-mail]>
>> ---
>>  drivers/video/da8xx-fb.c |   39 +++++++--------------------------------
>>  1 files changed, 7 insertions(+), 32 deletions(-)
>
> There's something similar in Darren's da8xx-fb series. Can you check if
> there are differences?
>
I don't see similar changes in his patch series.

Darren while you are at it you can take this patch along you series.
let me know how you plan to.

Regards,
--Prabhakar Lad

Tomi Valkeinen June 26, 2013, noon UTC | #4

On 26/06/13 14:56, Prabhakar Lad wrote:
> Hi Tomi,
> 
> On Wed, Jun 26, 2013 at 5:09 PM, Tomi Valkeinen <[e-mail]> wrote:
>> On 16/05/13 10:10, Lad Prabhakar wrote:
>>> From: Lad, Prabhakar <[e-mail]>
>>>
>>> Use devm_ioremap_resource instead of reques_mem_region()/ioremap() and
>>> devm_request_irq() instead of request_irq().
>>>
>>> This ensures more consistent error values and simplifies error paths.
>>>
>>> Signed-off-by: Lad, Prabhakar <[e-mail]>
>>> ---
>>>  drivers/video/da8xx-fb.c |   39 +++++++--------------------------------
>>>  1 files changed, 7 insertions(+), 32 deletions(-)
>>
>> There's something similar in Darren's da8xx-fb series. Can you check if
>> there are differences?
>>
> I don't see similar changes in his patch series.

[PATCH 14/23] video: da8xx-fb: use devres

 Tomi

Lad, Prabhakar June 26, 2013, 12:06 p.m. UTC | #5

Hi Tomi,

On Wed, Jun 26, 2013 at 5:30 PM, Tomi Valkeinen <[e-mail]> wrote:
> On 26/06/13 14:56, Prabhakar Lad wrote:
>> Hi Tomi,
>>
>> On Wed, Jun 26, 2013 at 5:09 PM, Tomi Valkeinen <[e-mail]> wrote:
>>> On 16/05/13 10:10, Lad Prabhakar wrote:
>>>> From: Lad, Prabhakar <[e-mail]>
>>>>
>>>> Use devm_ioremap_resource instead of reques_mem_region()/ioremap() and
>>>> devm_request_irq() instead of request_irq().
>>>>
>>>> This ensures more consistent error values and simplifies error paths.
>>>>
>>>> Signed-off-by: Lad, Prabhakar <[e-mail]>
>>>> ---
>>>>  drivers/video/da8xx-fb.c |   39 +++++++--------------------------------
>>>>  1 files changed, 7 insertions(+), 32 deletions(-)
>>>
>>> There's something similar in Darren's da8xx-fb series. Can you check if
>>> there are differences?
>>>
>> I don't see similar changes in his patch series.
>
> [PATCH 14/23] video: da8xx-fb: use devres
>
Oops missed it out :), patch is almost similar except
for following block,

-       fb_clk = clk_get(&device->dev, "fck");
+       fb_clk = devm_clk_get(&device->dev, "fck");
        if (IS_ERR(fb_clk)) {
                dev_err(&device->dev, "Can not get device clock\n");
-               ret = -ENODEV;
-               goto err_ioremap;
+               return -ENODEV;
        }

How do you suggest to handle it ?

Regards,
--Prabhakar Lad

Tomi Valkeinen June 26, 2013, 12:11 p.m. UTC | #6

On 26/06/13 15:06, Prabhakar Lad wrote:
> Hi Tomi,
> 
> On Wed, Jun 26, 2013 at 5:30 PM, Tomi Valkeinen <[e-mail]> wrote:
>> On 26/06/13 14:56, Prabhakar Lad wrote:
>>> Hi Tomi,
>>>
>>> On Wed, Jun 26, 2013 at 5:09 PM, Tomi Valkeinen <[e-mail]> wrote:
>>>> On 16/05/13 10:10, Lad Prabhakar wrote:
>>>>> From: Lad, Prabhakar <[e-mail]>
>>>>>
>>>>> Use devm_ioremap_resource instead of reques_mem_region()/ioremap() and
>>>>> devm_request_irq() instead of request_irq().
>>>>>
>>>>> This ensures more consistent error values and simplifies error paths.
>>>>>
>>>>> Signed-off-by: Lad, Prabhakar <[e-mail]>
>>>>> ---
>>>>>  drivers/video/da8xx-fb.c |   39 +++++++--------------------------------
>>>>>  1 files changed, 7 insertions(+), 32 deletions(-)
>>>>
>>>> There's something similar in Darren's da8xx-fb series. Can you check if
>>>> there are differences?
>>>>
>>> I don't see similar changes in his patch series.
>>
>> [PATCH 14/23] video: da8xx-fb: use devres
>>
> Oops missed it out :), patch is almost similar except
> for following block,
> 
> -       fb_clk = clk_get(&device->dev, "fck");
> +       fb_clk = devm_clk_get(&device->dev, "fck");
>         if (IS_ERR(fb_clk)) {
>                 dev_err(&device->dev, "Can not get device clock\n");
> -               ret = -ENODEV;
> -               goto err_ioremap;
> +               return -ENODEV;
>         }
> 
> How do you suggest to handle it ?

I think it's easiest if Darren handles the da8xx-fb series for 3.11. So
if there's something missing from Darren's series, please discuss with him.

 Tomi

Lad, Prabhakar July 2, 2013, 9:15 a.m. UTC | #7

Hi Darren,

On Wed, Jun 26, 2013 at 5:41 PM, Tomi Valkeinen <[e-mail]> wrote:
> On 26/06/13 15:06, Prabhakar Lad wrote:
>> Hi Tomi,
>>
>> On Wed, Jun 26, 2013 at 5:30 PM, Tomi Valkeinen <[e-mail]> wrote:
>>> On 26/06/13 14:56, Prabhakar Lad wrote:
>>>> Hi Tomi,
>>>>
>>>> On Wed, Jun 26, 2013 at 5:09 PM, Tomi Valkeinen <[e-mail]> wrote:
>>>>> On 16/05/13 10:10, Lad Prabhakar wrote:
>>>>>> From: Lad, Prabhakar <[e-mail]>
>>>>>>
>>>>>> Use devm_ioremap_resource instead of reques_mem_region()/ioremap() and
>>>>>> devm_request_irq() instead of request_irq().
>>>>>>
>>>>>> This ensures more consistent error values and simplifies error paths.
>>>>>>
>>>>>> Signed-off-by: Lad, Prabhakar <[e-mail]>
>>>>>> ---
>>>>>>  drivers/video/da8xx-fb.c |   39 +++++++--------------------------------
>>>>>>  1 files changed, 7 insertions(+), 32 deletions(-)
>>>>>
>>>>> There's something similar in Darren's da8xx-fb series. Can you check if
>>>>> there are differences?
>>>>>
>>>> I don't see similar changes in his patch series.
>>>
>>> [PATCH 14/23] video: da8xx-fb: use devres
>>>
>> Oops missed it out :), patch is almost similar except
>> for following block,
>>
>> -       fb_clk = clk_get(&device->dev, "fck");
>> +       fb_clk = devm_clk_get(&device->dev, "fck");
>>         if (IS_ERR(fb_clk)) {
>>                 dev_err(&device->dev, "Can not get device clock\n");
>> -               ret = -ENODEV;
>> -               goto err_ioremap;
>> +               return -ENODEV;
>>         }
>>
>> How do you suggest to handle it ?
>
> I think it's easiest if Darren handles the da8xx-fb series for 3.11. So
> if there's something missing from Darren's series, please discuss with him.
>
Either you merge this patch with yours or rebase your patch on top of my patch.
Let me know how you would like to handle it ?

Regards,
--Prabhakar Lad

Etheridge, Darren July 2, 2013, 10:31 p.m. UTC | #8

> >> How do you suggest to handle it ?
> >
> > I think it's easiest if Darren handles the da8xx-fb series for 3.11.
> > So if there's something missing from Darren's series, please discuss
> with him.
> >
> Either you merge this patch with yours or rebase your patch on top of
> my patch.
> Let me know how you would like to handle it ?
Prabhakar,

I will include your change with the next version of this patch series that will address Tomi's review comments.  

Darren

Patch

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index aafe8b9..d35ea1d 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -134,7 +134,6 @@ 
 #define LOWER_MARGIN	32
 
 static void __iomem *da8xx_fb_reg_base;
-static struct resource *lcdc_regs;
 static unsigned int lcd_revision;
 static irq_handler_t lcdc_irq_handler;
 static wait_queue_head_t frame_done_wq;
@@ -1015,12 +1014,9 @@  static int fb_remove(struct platform_device *dev)
 				  par->p_palette_base);
 		dma_free_coherent(NULL, par->vram_size, par->vram_virt,
 				  par->vram_phys);
-		free_irq(par->irq, par);
 		pm_runtime_put_sync(&dev->dev);
 		pm_runtime_disable(&dev->dev);
 		framebuffer_release(info);
-		iounmap(da8xx_fb_reg_base);
-		release_mem_region(lcdc_regs->start, resource_size(lcdc_regs));
 
 	}
 	return 0;
@@ -1212,12 +1208,12 @@  static int fb_probe(struct platform_device *device)
 {
 	struct da8xx_lcdc_platform_data *fb_pdata =
 						device->dev.platform_data;
+	static struct resource *lcdc_regs;
 	struct lcd_ctrl_config *lcd_cfg;
 	struct fb_videomode *lcdc_info;
 	struct fb_info *da8xx_fb_info;
 	struct clk *fb_clk = NULL;
 	struct da8xx_fb_par *par;
-	resource_size_t len;
 	int ret, i;
 	unsigned long ulcm;
 
@@ -1227,29 +1223,14 @@  static int fb_probe(struct platform_device *device)
 	}
 
 	lcdc_regs = platform_get_resource(device, IORESOURCE_MEM, 0);
-	if (!lcdc_regs) {
-		dev_err(&device->dev,
-			"Can not get memory resource for LCD controller\n");
-		return -ENOENT;
-	}
-
-	len = resource_size(lcdc_regs);
-
-	lcdc_regs = request_mem_region(lcdc_regs->start, len, lcdc_regs->name);
-	if (!lcdc_regs)
-		return -EBUSY;
-
-	da8xx_fb_reg_base = ioremap(lcdc_regs->start, len);
-	if (!da8xx_fb_reg_base) {
-		ret = -EBUSY;
-		goto err_request_mem;
-	}
+	da8xx_fb_reg_base = devm_ioremap_resource(&device->dev, lcdc_regs);
+	if (IS_ERR(da8xx_fb_reg_base))
+		return PTR_ERR(da8xx_fb_reg_base);
 
 	fb_clk = clk_get(&device->dev, "fck");
 	if (IS_ERR(fb_clk)) {
 		dev_err(&device->dev, "Can not get device clock\n");
-		ret = -ENODEV;
-		goto err_ioremap;
+		return -ENODEV;
 	}
 
 	pm_runtime_enable(&device->dev);
@@ -1430,8 +1411,8 @@  static int fb_probe(struct platform_device *device)
 		lcdc_irq_handler = lcdc_irq_handler_rev02;
 	}
 
-	ret = request_irq(par->irq, lcdc_irq_handler, 0,
-			DRIVER_NAME, par);
+	ret = devm_request_irq(&device->dev, par->irq, lcdc_irq_handler, 0,
+			       DRIVER_NAME, par);
 	if (ret)
 		goto irq_freq;
 	return 0;
@@ -1460,12 +1441,6 @@  err_pm_runtime_disable:
 	pm_runtime_put_sync(&device->dev);
 	pm_runtime_disable(&device->dev);
 
-err_ioremap:
-	iounmap(da8xx_fb_reg_base);
-
-err_request_mem:
-	release_mem_region(lcdc_regs->start, len);
-
 	return ret;
 }