[v2] clocksource: sh_cmt: Use devm_* managed helpers

Commit Message

Laurent Pinchart June 18, 2013, 12:36 p.m. UTC

This simplifies the main error path by getting rid of it.

Signed-off-by: Laurent Pinchart <[e-mail]>
---
 drivers/clocksource/sh_cmt.c | 44 +++++++++++++++-----------------------------
 1 file changed, 15 insertions(+), 29 deletions(-)

Changes since v1:

- Check the return value of devm_ioremap_resource() with IS_ERR()
- Don't print an error message when devm_ioremap_resource() fails

Comments

Magnus Damm June 18, 2013, 1:29 p.m. UTC | #1

On Tue, Jun 18, 2013 at 9:36 PM, Laurent Pinchart
<[e-mail]> wrote:
> This simplifies the main error path by getting rid of it.
>
> Signed-off-by: Laurent Pinchart <[e-mail]>

Looking good, thanks!

Acked-by: Magnus Damm <[e-mail]>
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Simon Horman June 27, 2013, 7:31 a.m. UTC | #2

On Tue, Jun 18, 2013 at 10:29:38PM +0900, Magnus Damm wrote:
> On Tue, Jun 18, 2013 at 9:36 PM, Laurent Pinchart
> <[e-mail]> wrote:
> > This simplifies the main error path by getting rid of it.
> >
> > Signed-off-by: Laurent Pinchart <[e-mail]>
> 
> Looking good, thanks!
> 
> Acked-by: Magnus Damm <[e-mail]>

Thanks, queued up for v3.12 in the clocksource branch.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Simon Horman July 1, 2013, 9:26 a.m. UTC | #3

On Thu, Jun 27, 2013 at 04:31:57PM +0900, Simon Horman wrote:
> On Tue, Jun 18, 2013 at 10:29:38PM +0900, Magnus Damm wrote:
> > On Tue, Jun 18, 2013 at 9:36 PM, Laurent Pinchart
> > <[e-mail]> wrote:
> > > This simplifies the main error path by getting rid of it.
> > >
> > > Signed-off-by: Laurent Pinchart <[e-mail]>
> > 
> > Looking good, thanks!
> > 
> > Acked-by: Magnus Damm <[e-mail]>
> 
> Thanks, queued up for v3.12 in the clocksource branch.

I have dropped this patch as it seems to cause the armadillo800eva
to fail to boot:


Booting Linux on physical CPU 0x0
Linux version 3.10.0-rc2-00001-g4903eae ([e-mail]) (gcc version 4.4.5 (Debian 4.4.5-8) ) #7390 Mon Jul 1 18:11:53 JST 2013
CPU: ARMv7 Processor [412fc093] revision 3 (ARMv7), cr=10c53c7d
CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
Machine: armadillo800eva, model: armadillo 800 eva
debug: ignoring loglevel setting.
Memory policy: ECC disabled, Data cache writeback
On node 0 totalpages: 131072
free_area_init_node: node 0, pgdat c046854c, node_mem_map c0694000
  Normal zone: 1024 pages used for memmap
  Normal zone: 0 pages reserved
  Normal zone: 131072 pages, LIFO batch:31
CPU: All CPU(s) started in SVC mode.
bootconsole [early_ttySC1] enabled
pcpu-alloc: s0 r0 d32768 u32768 alloc=1*32768
pcpu-alloc: [0] 0 
Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 130048
Kernel command line: console=tty0 console=ttySC1,115200 earlyprintk=sh-sci.1,115200 ignore_loglevel root=/dev/nfs ip=dhcp nfsroot=,rsize=4096,wsize=4096 rw
PID hash table entries: 2048 (order: 1, 8192 bytes)
Dentry cache hash table entries: 65536 (order: 6, 262144 bytes)
Inode-cache hash table entries: 32768 (order: 5, 131072 bytes)
Memory: 512MB = 512MB total
Memory: 515056k/515056k available, 9232k reserved, 0K highmem
Virtual kernel memory layout:
    vector  : 0xffff0000 - 0xffff1000   (   4 kB)
    fixmap  : 0xfff00000 - 0xfffe0000   ( 896 kB)
    vmalloc : 0xe0800000 - 0xff000000   ( 488 MB)
    lowmem  : 0xc0000000 - 0xe0000000   ( 512 MB)
    modules : 0xbf000000 - 0xc0000000   (  16 MB)
      .text : 0xc0008000 - 0xc0414034   (4145 kB)
      .init : 0xc0415000 - 0xc0438578   ( 142 kB)
      .data : 0xc043a000 - 0xc0468d80   ( 188 kB)
       .bss : 0xc0468d80 - 0xc04924a0   ( 166 kB)
NR_IRQS:16 nr_irqs:16 16
GIC CPU mask not found - kernel will fail to boot.
GIC CPU mask not found - kernel will fail to boot.
sched_clock: 32 bits at 128 Hz, resolution 7812500ns, wraps every 3489660920ms
Console: colour dummy device 80x30
console [tty0] enabled
 sh_cmt.10: used for clock events
 sh_cmt.10: used for periodic clock events
 sh_cmt.10: used as clock source
Unable to handle kernel paging request at virtual address fffffffc
pgd = c0004000
[fffffffc] *pgd=5fffe821, *pte=00000000, *ppte=00000000
Internal error: Oops: 17 [#1] ARM
Modules linked in:
CPU: 0 PID: 0 Comm: swapper Not tainted 3.10.0-rc2-00001-g4903eae #7390
task: c04441b8 ti: c043a000 task.ti: c043a000
PC is at strcmp+0x10/0x40
LR is at platform_match+0x84/0x90
pc : [<c0167a08>]    lr : [<c01bdcc0>]    psr: 60000153
sp : c043bf30  ip : c043bf40  fp : c043bf3c
r10: 00000000  r9 : 00000001  r8 : 00000000
r7 : de0030e0  r6 : de0030f0  r5 : c0462798  r4 : 00000000
r3 : 00000000  r2 : 0000002d  r1 : c03c0126  r0 : fffffffc
Flags: nZCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
Control: 10c53c7d  Table: 40004059  DAC: 00000015
Process swapper (pid: 0, stack limit = 0xc043a230)
Stack: (0xc043bf30 to 0xc043c000)
bf20:                                     c043bf5c c043bf40 c01bdcc0 c0167a04
bf40: de0030e0 c043716c c0436640 0000000b c043bfa4 c043bf60 c042687c c01bdc48
bf60: 412fc093 00000000 00000001 00000001 00000002 c03c004c 00000000 c03c004c
bf80: c0468d80 c0431bb4 7fffffff 40004059 412fc093 00000000 c043bfbc c043bfa8
bfa0: c041a7b0 c0426800 ffffffff c0468d80 c043bff4 c043bfc0 c0415834 c041a794
bfc0: ffffffff ffffffff c0415458 00000000 00000000 c0431bb4 10c53c7d c0442028
bfe0: c0431bb0 c0444f34 00000000 c043bff8 40008070 c041564c 00000000 00000000
Backtrace: 
[<c01679f8>] (strcmp+0x0/0x40) from [<c01bdcc0>] (platform_match+0x84/0x90)
[<c01bdc3c>] (platform_match+0x0/0x90) from [<c042687c>] (early_platform_driver_probe+0x88/0x254)
 r7:0000000b r6:c0436640 r5:c043716c r4:de0030e0
[<c04267f4>] (early_platform_driver_probe+0x0/0x254) from [<c041a7b0>] (shmobile_late_time_init+0x28/0x30)
[<c041a788>] (shmobile_late_time_init+0x0/0x30) from [<c0415834>] (start_kernel+0x1f4/0x284)
 r5:c0468d80 r4:ffffffff
[<c0415640>] (start_kernel+0x0/0x284) from [<40008070>] (0x40008070)
 r7:c0444f34 r6:c0431bb0 r5:c0442028 r4:10c53c7d
Code: e1a0c00d e92dd800 e24cb004 e3a03000 (e7d02003) 
---[ end trace 1b75b31a2719ed1c ]---
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/clocksource/sh_cmt.c b/drivers/clocksource/sh_cmt.c
index 0965e98..72e7a46 100644
--- a/drivers/clocksource/sh_cmt.c
+++ b/drivers/clocksource/sh_cmt.c
@@ -681,20 +681,18 @@  static int sh_cmt_setup(struct sh_cmt_priv *p, struct platform_device *pdev)
 	struct sh_timer_config *cfg = pdev->dev.platform_data;
 	struct resource *res, *res2;
 	int irq, ret;
-	ret = -ENXIO;
 
-	memset(p, 0, sizeof(*p));
 	p->pdev = pdev;
 
 	if (!cfg) {
 		dev_err(&p->pdev->dev, "missing platform data\n");
-		goto err0;
+		return -ENXIO;
 	}
 
 	res = platform_get_resource(p->pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		dev_err(&p->pdev->dev, "failed to get I/O memory\n");
-		goto err0;
+		return -ENXIO;
 	}
 
 	/* optional resource for the shared timer start/stop register */
@@ -703,23 +701,21 @@  static int sh_cmt_setup(struct sh_cmt_priv *p, struct platform_device *pdev)
 	irq = platform_get_irq(p->pdev, 0);
 	if (irq < 0) {
 		dev_err(&p->pdev->dev, "failed to get irq\n");
-		goto err0;
+		return -ENXIO;
 	}
 
 	/* map memory, let mapbase point to our channel */
-	p->mapbase = ioremap_nocache(res->start, resource_size(res));
-	if (p->mapbase == NULL) {
-		dev_err(&p->pdev->dev, "failed to remap I/O memory\n");
-		goto err0;
-	}
+	p->mapbase = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(p->mapbase))
+		return PTR_ERR(p->mapbase);
 
 	/* map second resource for CMSTR */
-	p->mapbase_str = ioremap_nocache(res2 ? res2->start :
-					 res->start - cfg->channel_offset,
-					 res2 ? resource_size(res2) : 2);
+	p->mapbase_str = devm_ioremap_nocache(&pdev->dev, res2 ? res2->start :
+					      res->start - cfg->channel_offset,
+					      res2 ? resource_size(res2) : 2);
 	if (p->mapbase_str == NULL) {
 		dev_err(&p->pdev->dev, "failed to remap I/O second memory\n");
-		goto err1;
+		return -ENXIO;
 	}
 
 	/* request irq using setup_irq() (too early for request_irq()) */
@@ -730,11 +726,10 @@  static int sh_cmt_setup(struct sh_cmt_priv *p, struct platform_device *pdev)
 			     IRQF_IRQPOLL  | IRQF_NOBALANCING;
 
 	/* get hold of clock */
-	p->clk = clk_get(&p->pdev->dev, "cmt_fck");
+	p->clk = devm_clk_get(&p->pdev->dev, "cmt_fck");
 	if (IS_ERR(p->clk)) {
 		dev_err(&p->pdev->dev, "cannot get clock\n");
-		ret = PTR_ERR(p->clk);
-		goto err2;
+		return PTR_ERR(p->clk);
 	}
 
 	if (res2 && (resource_size(res2) == 4)) {
@@ -773,27 +768,19 @@  static int sh_cmt_setup(struct sh_cmt_priv *p, struct platform_device *pdev)
 			      cfg->clocksource_rating);
 	if (ret) {
 		dev_err(&p->pdev->dev, "registration failed\n");
-		goto err3;
+		return ret;
 	}
 	p->cs_enabled = false;
 
 	ret = setup_irq(irq, &p->irqaction);
 	if (ret) {
 		dev_err(&p->pdev->dev, "failed to request irq %d\n", irq);
-		goto err3;
+		return ret;
 	}
 
 	platform_set_drvdata(pdev, p);
 
 	return 0;
-err3:
-	clk_put(p->clk);
-err2:
-	iounmap(p->mapbase_str);
-err1:
-	iounmap(p->mapbase);
-err0:
-	return ret;
 }
 
 static int sh_cmt_probe(struct platform_device *pdev)
@@ -812,7 +799,7 @@  static int sh_cmt_probe(struct platform_device *pdev)
 		goto out;
 	}
 
-	p = kmalloc(sizeof(*p), GFP_KERNEL);
+	p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
 	if (p == NULL) {
 		dev_err(&pdev->dev, "failed to allocate driver data\n");
 		return -ENOMEM;
@@ -820,7 +807,6 @@  static int sh_cmt_probe(struct platform_device *pdev)
 
 	ret = sh_cmt_setup(p, pdev);
 	if (ret) {
-		kfree(p);
 		pm_runtime_idle(&pdev->dev);
 		return ret;
 	}