* [PATCH] mpic: make sparse happy
@ 2008-02-20 11:31 Johannes Berg
2008-02-21 5:50 ` Milton Miller
0 siblings, 1 reply; 4+ messages in thread
From: Johannes Berg @ 2008-02-20 11:31 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev list
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;
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] mpic: make sparse happy
2008-02-20 11:31 [PATCH] mpic: make sparse happy Johannes Berg
@ 2008-02-21 5:50 ` Milton Miller
2008-02-21 9:23 ` Johannes Berg
2008-02-21 9:39 ` [PATCH v2] " Johannes Berg
0 siblings, 2 replies; 4+ messages in thread
From: Milton Miller @ 2008-02-21 5:50 UTC (permalink / raw)
To: Johannes Berg; +Cc: ppcdev
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!
> --- 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.
...
> @@ -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.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] mpic: make sparse happy
2008-02-21 5:50 ` Milton Miller
@ 2008-02-21 9:23 ` Johannes Berg
2008-02-21 9:39 ` [PATCH v2] " Johannes Berg
1 sibling, 0 replies; 4+ messages in thread
From: Johannes Berg @ 2008-02-21 9:23 UTC (permalink / raw)
To: Milton Miller; +Cc: ppcdev
[-- Attachment #1: Type: text/plain, Size: 468 bytes --]
On Wed, 2008-02-20 at 23:50 -0600, Milton Miller wrote:
> 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!
> You now write to the register with dcr, big, and little endian variants!
Ouch, you're right, doh.
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] mpic: make sparse happy
2008-02-21 5:50 ` Milton Miller
2008-02-21 9:23 ` Johannes Berg
@ 2008-02-21 9:39 ` Johannes Berg
1 sibling, 0 replies; 4+ messages in thread
From: Johannes Berg @ 2008-02-21 9:39 UTC (permalink / raw)
To: Milton Miller; +Cc: ppcdev
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 <miltonm@bga.com>
---
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;
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2008-02-21 9:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-02-20 11:31 [PATCH] mpic: make sparse happy Johannes Berg
2008-02-21 5:50 ` Milton Miller
2008-02-21 9:23 ` Johannes Berg
2008-02-21 9:39 ` [PATCH v2] " Johannes Berg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).