Skip to contentSkip to conversation

sparc: leon: Fix a retry loop in leon_init_timers()

PATCH Applied Dan Carpenter 3 participants 3 messages Nov 25, 2016 – Dec 12, 2016

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 sparc: leon: Fix a retry loop in leon_init_timers()

The original code causes a static checker warning because it has a continue inside a do { } while (0); loop. In that context, a continue and a break are equivalent. The intent was to go back to the start of the loop so the continue was a bug.

I've added a retry label at the start and changed the continue to a goto retry. Then I removed the do { } while (0) loop and pulled the code in one indent level.

Fixes 2791c1a43900 ("SPARC/LEON: added support for selecting Timer Core and Timer within core")

---
Not tested.

arch/sparc/kernel/leon_kernel.c +28 -28
index 33cd171..afcdd5e 100644
@@ -349,37 +349,37 @@ void __init leon_init_timers(void)
 
 	/* Find GPTIMER Timer Registers base address otherwise bail out. */
 	nnp = rootnp;
-	do {
-		np = of_find_node_by_name(nnp, "GAISLER_GPTIMER");
-		if (!np) {
-			np = of_find_node_by_name(nnp, "01_011");
-			if (!np)
-				goto bad;
-		}
 
-		ampopts = 0;
-		pp = of_find_property(np, "ampopts", &len);
-		if (pp) {
-			ampopts = *(int *)pp->value;
-			if (ampopts = 0) {
-				/* Skip this instance, resource already
-				 * allocated by other OS */
-				nnp = np;
-				continue;
-			}
+retry:
+	np = of_find_node_by_name(nnp, "GAISLER_GPTIMER");
+	if (!np) {
+		np = of_find_node_by_name(nnp, "01_011");
+		if (!np)
+			goto bad;
+	}
+
+	ampopts = 0;
+	pp = of_find_property(np, "ampopts", &len);
+	if (pp) {
+		ampopts = *(int *)pp->value;
+		if (ampopts = 0) {
+			/* Skip this instance, resource already
+			 * allocated by other OS */
+			nnp = np;
+			goto retry;
 		}
+	}
+
+	/* Select Timer-Instance on Timer Core. Default is zero */
+	leon3_gptimer_idx = ampopts & 0x7;
 
-		/* Select Timer-Instance on Timer Core. Default is zero */
-		leon3_gptimer_idx = ampopts & 0x7;
-
-		pp = of_find_property(np, "reg", &len);
-		if (pp)
-			leon3_gptimer_regs = *(struct leon3_gptimer_regs_map **)
-						pp->value;
-		pp = of_find_property(np, "interrupts", &len);
-		if (pp)
-			leon3_gptimer_irq = *(unsigned int *)pp->value;
-	} while (0);
+	pp = of_find_property(np, "reg", &len);
+	if (pp)
+		leon3_gptimer_regs = *(struct leon3_gptimer_regs_map **)
+					pp->value;
+	pp = of_find_property(np, "interrupts", &len);
+	if (pp)
+		leon3_gptimer_irq = *(unsigned int *)pp->value;
 
 	if (!(leon3_gptimer_regs && leon3_irqctrl_regs && leon3_gptimer_irq))
 		goto bad;
#1 lore raw

It looks good.

Thanks!
/Daniel

Quoted 84 lines · Dan Carpenter

The original code causes a static checker warning because it has a continue inside a do { } while (0); loop. In that context, a continue and a break are equivalent. The intent was to go back to the start of the loop so the continue was a bug.

I've added a retry label at the start and changed the continue to a goto retry. Then I removed the do { } while (0) loop and pulled the code in one indent level.

Fixes 2791c1a43900 ("SPARC/LEON: added support for selecting Timer Core and Timer within core")

---
Not tested.

arch/sparc/kernel/leon_kernel.c +28 -28
index 33cd171..afcdd5e 100644
@@ -349,37 +349,37 @@ void __init leon_init_timers(void)
 
  	/* Find GPTIMER Timer Registers base address otherwise bail out. */
  	nnp = rootnp;
-	do {
-		np = of_find_node_by_name(nnp, "GAISLER_GPTIMER");
-		if (!np) {
-			np = of_find_node_by_name(nnp, "01_011");
-			if (!np)
-				goto bad;
-		}
 
-		ampopts = 0;
-		pp = of_find_property(np, "ampopts", &len);
-		if (pp) {
-			ampopts = *(int *)pp->value;
-			if (ampopts = 0) {
-				/* Skip this instance, resource already
-				 * allocated by other OS */
-				nnp = np;
-				continue;
-			}
+retry:
+	np = of_find_node_by_name(nnp, "GAISLER_GPTIMER");
+	if (!np) {
+		np = of_find_node_by_name(nnp, "01_011");
+		if (!np)
+			goto bad;
+	}
+
+	ampopts = 0;
+	pp = of_find_property(np, "ampopts", &len);
+	if (pp) {
+		ampopts = *(int *)pp->value;
+		if (ampopts = 0) {
+			/* Skip this instance, resource already
+			 * allocated by other OS */
+			nnp = np;
+			goto retry;
  		}
+	}
+
+	/* Select Timer-Instance on Timer Core. Default is zero */
+	leon3_gptimer_idx = ampopts & 0x7;
 
-		/* Select Timer-Instance on Timer Core. Default is zero */
-		leon3_gptimer_idx = ampopts & 0x7;
-
-		pp = of_find_property(np, "reg", &len);
-		if (pp)
-			leon3_gptimer_regs = *(struct leon3_gptimer_regs_map **)
-						pp->value;
-		pp = of_find_property(np, "interrupts", &len);
-		if (pp)
-			leon3_gptimer_irq = *(unsigned int *)pp->value;
-	} while (0);
+	pp = of_find_property(np, "reg", &len);
+	if (pp)
+		leon3_gptimer_regs = *(struct leon3_gptimer_regs_map **)
+					pp->value;
+	pp = of_find_property(np, "interrupts", &len);
+	if (pp)
+		leon3_gptimer_irq = *(unsigned int *)pp->value;
 
  	if (!(leon3_gptimer_regs && leon3_irqctrl_regs && leon3_gptimer_irq))
  		goto bad;
maintainer Applied
#2 lore raw
In reply to Dan Carpenter

From: Dan Carpenter <[email protected]>
Date: Fri, 25 Nov 2016 14:25:54 +0300

Quoted 11 lines

The original code causes a static checker warning because it has a continue inside a do { } while (0); loop. In that context, a continue and a break are equivalent. The intent was to go back to the start of the loop so the continue was a bug.

I've added a retry label at the start and changed the continue to a goto retry. Then I removed the do { } while (0) loop and pulled the code in one indent level.

Fixes 2791c1a43900 ("SPARC/LEON: added support for selecting Timer Core and Timer within core")

Applied.