[PATCH] mpic: make sparse happy

STALE6739d

4 messages, 2 authors, 2008-02-21 · open the first message on its own page

[PATCH] mpic: make sparse happy

From: Johannes Berg <johannes@sipsolutions.net>
Date: 2008-02-20 18:32:36

I was running sparse on something else and noticed sparse warnings
and especially the bogus code that is fixed by the first hunk of
this patch, so I fixed them all while at it.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 arch/powerpc/sysdev/mpic.c |   21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

--- everything.orig/arch/powerpc/sysdev/mpic.c	2008-02-20 12:25:41.000000000 +0100
+++ everything/arch/powerpc/sysdev/mpic.c	2008-02-20 12:28:37.000000000 +0100

@@ -175,13 +175,13 @@ static inline void _mpic_write(enum mpic
 	switch(type) {
 #ifdef CONFIG_PPC_DCR
 	case mpic_access_dcr:
-		return dcr_write(rb->dhost, reg, value);
+		dcr_write(rb->dhost, reg, value);
 #endif
 	case mpic_access_mmio_be:
-		return out_be32(rb->base + (reg >> 2), value);
+		out_be32(rb->base + (reg >> 2), value);
 	case mpic_access_mmio_le:
 	default:
-		return out_le32(rb->base + (reg >> 2), value);
+		out_le32(rb->base + (reg >> 2), value);
 	}
 }

@@ -1064,7 +1064,8 @@ struct mpic * __init mpic_alloc(struct d
 
 	/* Look for protected sources */
 	if (node) {
-		unsigned int psize, bits, mapsize;
+		int psize;
+		unsigned int bits, mapsize;
 		const u32 *psrc =
 			of_get_property(node, "protected-sources", &psize);
 		if (psrc) {

@@ -1107,10 +1108,10 @@ struct mpic * __init mpic_alloc(struct d
 	 * in, try to obtain one
 	 */
 	if (paddr == 0 && !(mpic->flags & MPIC_USES_DCR)) {
-		const u32 *reg;
-		reg = of_get_property(node, "reg", NULL);
-		BUG_ON(reg == NULL);
-		paddr = of_translate_address(node, reg);
+		const u32 *regprop;
+		regprop = of_get_property(node, "reg", NULL);
+		BUG_ON(regprop == NULL);
+		paddr = of_translate_address(node, regprop);
 		BUG_ON(paddr == OF_BAD_ADDR);
 	}

@@ -1321,7 +1322,7 @@ void __init mpic_set_serial_int(struct m
 
 void mpic_irq_set_priority(unsigned int irq, unsigned int pri)
 {
-	int is_ipi;
+	unsigned int is_ipi;
 	struct mpic *mpic = mpic_find(irq, &is_ipi);
 	unsigned int src = mpic_irq_to_hw(irq);
 	unsigned long flags;

@@ -1344,7 +1345,7 @@ void mpic_irq_set_priority(unsigned int 
 
 unsigned int mpic_irq_get_priority(unsigned int irq)
 {
-	int is_ipi;
+	unsigned int is_ipi;
 	struct mpic *mpic = mpic_find(irq, &is_ipi);
 	unsigned int src = mpic_irq_to_hw(irq);
 	unsigned long flags;

Re: [PATCH] mpic: make sparse happy

From: Milton Miller <hidden>
Date: 2008-02-21 05:51:06

At Wed Feb 20 22:31:44 EST 2008, Johannes Berg wrote:

I was running sparse on something else and noticed sparse warnings
and especially the bogus code that is fixed by the first hunk of
this patch, so I fixed them all while at it.

But your change is not equivalent!

quoted hunk

--- everything.orig/arch/powerpc/sysdev/mpic.c  2008-02-20

12:25:41.000000000 +0100

+++ everything/arch/powerpc/sysdev/mpic.c       2008-02-20

12:28:37.000000000 +0100

@@ -175,13 +175,13 @@ static inline void _mpic_write(enum mpic
        switch(type) {
 #ifdef CONFIG_PPC_DCR
        case mpic_access_dcr:
-               return dcr_write(rb->dhost, reg, value);
+               dcr_write(rb->dhost, reg, value);
 #endif
        case mpic_access_mmio_be:
-               return out_be32(rb->base + (reg >> 2), value);
+               out_be32(rb->base + (reg >> 2), value);
        case mpic_access_mmio_le:
        default:
-               return out_le32(rb->base + (reg >> 2), value);
+               out_le32(rb->base + (reg >> 2), value);
        }
 }

You now write to the register with dcr, big, and little endian variants!

Either put a return or break after the calls to the void functions so 
you don't fall through.

...

quoted hunk

@@ -1107,10 +1108,10 @@ struct mpic * __init mpic_alloc(struct d
         * in, try to obtain one
         */
        if (paddr == 0 && !(mpic->flags & MPIC_USES_DCR)) {
-               const u32 *reg;
-               reg = of_get_property(node, "reg", NULL);
-               BUG_ON(reg == NULL);
-               paddr = of_translate_address(node, reg);
+               const u32 *regprop;
+               regprop = of_get_property(node, "reg", NULL);
+               BUG_ON(regprop == NULL);
+               paddr = of_translate_address(node, regprop);
                BUG_ON(paddr == OF_BAD_ADDR);
        }

This is reg variable is shadowed ... ok, although i might have renamed 
the outer one features or greg_feature.  For that matter, I would have 
initialized this reg/regprop on definition.

Re: [PATCH] mpic: make sparse happy

From: Johannes Berg <johannes@sipsolutions.net>
Date: 2008-02-21 09:24:48

On Wed, 2008-02-20 at 23:50 -0600, Milton Miller wrote:

At Wed Feb 20 22:31:44 EST 2008, Johannes Berg wrote:

quoted

I was running sparse on something else and noticed sparse warnings
and especially the bogus code that is fixed by the first hunk of
this patch, so I fixed them all while at it.

But your change is not equivalent!

You now write to the register with dcr, big, and little endian variants!

Ouch, you're right, doh.

johannes

[PATCH v2] mpic: make sparse happy

From: Johannes Berg <johannes@sipsolutions.net>
Date: 2008-02-21 09:39:12

I was running sparse on something else and noticed sparse warnings
and especially the bogus code that is fixed by the first hunk of
this patch, so I fixed them all while at it.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Milton Miller <redacted>
---
 arch/powerpc/sysdev/mpic.c |   32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

--- everything.orig/arch/powerpc/sysdev/mpic.c	2008-02-20 23:22:52.000000000 +0100
+++ everything/arch/powerpc/sysdev/mpic.c	2008-02-21 10:25:53.000000000 +0100

@@ -175,13 +175,16 @@ static inline void _mpic_write(enum mpic
 	switch(type) {
 #ifdef CONFIG_PPC_DCR
 	case mpic_access_dcr:
-		return dcr_write(rb->dhost, reg, value);
+		dcr_write(rb->dhost, reg, value);
+		break;
 #endif
 	case mpic_access_mmio_be:
-		return out_be32(rb->base + (reg >> 2), value);
+		out_be32(rb->base + (reg >> 2), value);
+		break;
 	case mpic_access_mmio_le:
 	default:
-		return out_le32(rb->base + (reg >> 2), value);
+		out_le32(rb->base + (reg >> 2), value);
+		break;
 	}
 }

@@ -1000,7 +1003,7 @@ struct mpic * __init mpic_alloc(struct d
 				const char *name)
 {
 	struct mpic	*mpic;
-	u32		reg;
+	u32		greg_feature;
 	const char	*vers;
 	int		i;
 	int		intvec_top;

@@ -1064,7 +1067,8 @@ struct mpic * __init mpic_alloc(struct d
 
 	/* Look for protected sources */
 	if (node) {
-		unsigned int psize, bits, mapsize;
+		int psize;
+		unsigned int bits, mapsize;
 		const u32 *psrc =
 			of_get_property(node, "protected-sources", &psize);
 		if (psrc) {

@@ -1107,8 +1111,7 @@ struct mpic * __init mpic_alloc(struct d
 	 * in, try to obtain one
 	 */
 	if (paddr == 0 && !(mpic->flags & MPIC_USES_DCR)) {
-		const u32 *reg;
-		reg = of_get_property(node, "reg", NULL);
+		const u32 *reg = of_get_property(node, "reg", NULL);
 		BUG_ON(reg == NULL);
 		paddr = of_translate_address(node, reg);
 		BUG_ON(paddr == OF_BAD_ADDR);

@@ -1137,12 +1140,13 @@ struct mpic * __init mpic_alloc(struct d
 	 * MPICs, num sources as well. On ISU MPICs, sources are counted
 	 * as ISUs are added
 	 */
-	reg = mpic_read(mpic->gregs, MPIC_INFO(GREG_FEATURE_0));
-	mpic->num_cpus = ((reg & MPIC_GREG_FEATURE_LAST_CPU_MASK)
+	greg_feature = mpic_read(mpic->gregs, MPIC_INFO(GREG_FEATURE_0));
+	mpic->num_cpus = ((greg_feature & MPIC_GREG_FEATURE_LAST_CPU_MASK)
 			  >> MPIC_GREG_FEATURE_LAST_CPU_SHIFT) + 1;
 	if (isu_size == 0)
-		mpic->num_sources = ((reg & MPIC_GREG_FEATURE_LAST_SRC_MASK)
-				     >> MPIC_GREG_FEATURE_LAST_SRC_SHIFT) + 1;
+		mpic->num_sources =
+			((greg_feature & MPIC_GREG_FEATURE_LAST_SRC_MASK)
+			 >> MPIC_GREG_FEATURE_LAST_SRC_SHIFT) + 1;
 
 	/* Map the per-CPU registers */
 	for (i = 0; i < mpic->num_cpus; i++) {

@@ -1161,7 +1165,7 @@ struct mpic * __init mpic_alloc(struct d
 	mpic->isu_mask = (1 << mpic->isu_shift) - 1;
 
 	/* Display version */
-	switch (reg & MPIC_GREG_FEATURE_VERSION_MASK) {
+	switch (greg_feature & MPIC_GREG_FEATURE_VERSION_MASK) {
 	case 1:
 		vers = "1.0";
 		break;

@@ -1321,7 +1325,7 @@ void __init mpic_set_serial_int(struct m
 
 void mpic_irq_set_priority(unsigned int irq, unsigned int pri)
 {
-	int is_ipi;
+	unsigned int is_ipi;
 	struct mpic *mpic = mpic_find(irq, &is_ipi);
 	unsigned int src = mpic_irq_to_hw(irq);
 	unsigned long flags;

@@ -1344,7 +1348,7 @@ void mpic_irq_set_priority(unsigned int 
 
 unsigned int mpic_irq_get_priority(unsigned int irq)
 {
-	int is_ipi;
+	unsigned int is_ipi;
 	struct mpic *mpic = mpic_find(irq, &is_ipi);
 	unsigned int src = mpic_irq_to_hw(irq);
 	unsigned long flags;

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help