2010-01-19 23:54:32

by Roel Kluin

[permalink] [raw]
Subject: [PATCH] MIPS: cleanup switches with cases that can be merged

I did a search for switch statements with cases that can be merged, but maybe
some were not intended?
---------------->8------------------------------------------8<-----------------
In these cases the same code was executed.

Signed-off-by: Roel Kluin <[email protected]>
---
arch/mips/include/asm/octeon/octeon-feature.h | 8 ++------
arch/mips/kernel/cpu-probe.c | 3 ---
arch/mips/math-emu/ieee754dp.c | 1 -
arch/mips/math-emu/ieee754sp.c | 1 -
arch/mips/pci/pci-octeon.c | 6 ++----
arch/mips/powertv/asic/asic_devices.c | 4 ----
arch/mips/sgi-ip32/ip32-irq.c | 9 +--------
7 files changed, 5 insertions(+), 27 deletions(-)

diff --git a/arch/mips/include/asm/octeon/octeon-feature.h b/arch/mips/include/asm/octeon/octeon-feature.h
index ef24a7b..cba6fbe 100644
--- a/arch/mips/include/asm/octeon/octeon-feature.h
+++ b/arch/mips/include/asm/octeon/octeon-feature.h
@@ -99,6 +99,8 @@ static inline int octeon_has_feature(enum octeon_feature feature)
return !cvmx_fuse_read(90);

case OCTEON_FEATURE_PCIE:
+ case OCTEON_FEATURE_MGMT_PORT:
+ case OCTEON_FEATURE_RAID:
return OCTEON_IS_MODEL(OCTEON_CN56XX)
|| OCTEON_IS_MODEL(OCTEON_CN52XX);

@@ -110,12 +112,6 @@ static inline int octeon_has_feature(enum octeon_feature feature)
case OCTEON_FEATURE_TRA:
return !(OCTEON_IS_MODEL(OCTEON_CN30XX)
|| OCTEON_IS_MODEL(OCTEON_CN50XX));
- case OCTEON_FEATURE_MGMT_PORT:
- return OCTEON_IS_MODEL(OCTEON_CN56XX)
- || OCTEON_IS_MODEL(OCTEON_CN52XX);
- case OCTEON_FEATURE_RAID:
- return OCTEON_IS_MODEL(OCTEON_CN56XX)
- || OCTEON_IS_MODEL(OCTEON_CN52XX);
case OCTEON_FEATURE_USB:
return !(OCTEON_IS_MODEL(OCTEON_CN38XX)
|| OCTEON_IS_MODEL(OCTEON_CN58XX));
diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index 80e202e..603e3bd 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -722,9 +722,6 @@ static inline void cpu_probe_mips(struct cpuinfo_mips *c, unsigned int cpu)
__cpu_name[cpu] = "MIPS 4Kc";
break;
case PRID_IMP_4KEC:
- c->cputype = CPU_4KEC;
- __cpu_name[cpu] = "MIPS 4KEc";
- break;
case PRID_IMP_4KECR2:
c->cputype = CPU_4KEC;
__cpu_name[cpu] = "MIPS 4KEc";
diff --git a/arch/mips/math-emu/ieee754dp.c b/arch/mips/math-emu/ieee754dp.c
index 6d2d89f..2f22fd7 100644
--- a/arch/mips/math-emu/ieee754dp.c
+++ b/arch/mips/math-emu/ieee754dp.c
@@ -148,7 +148,6 @@ ieee754dp ieee754dp_format(int sn, int xe, u64 xm)

switch(ieee754_csr.rm) {
case IEEE754_RN:
- return ieee754dp_zero(sn);
case IEEE754_RZ:
return ieee754dp_zero(sn);
case IEEE754_RU: /* toward +Infinity */
diff --git a/arch/mips/math-emu/ieee754sp.c b/arch/mips/math-emu/ieee754sp.c
index 4635340..a19b721 100644
--- a/arch/mips/math-emu/ieee754sp.c
+++ b/arch/mips/math-emu/ieee754sp.c
@@ -149,7 +149,6 @@ ieee754sp ieee754sp_format(int sn, int xe, unsigned xm)

switch(ieee754_csr.rm) {
case IEEE754_RN:
- return ieee754sp_zero(sn);
case IEEE754_RZ:
return ieee754sp_zero(sn);
case IEEE754_RU: /* toward +Infinity */
diff --git a/arch/mips/pci/pci-octeon.c b/arch/mips/pci/pci-octeon.c
index 9cb0c80..d248b70 100644
--- a/arch/mips/pci/pci-octeon.c
+++ b/arch/mips/pci/pci-octeon.c
@@ -209,16 +209,14 @@ const char *octeon_get_pci_interrupts(void)
case CVMX_BOARD_TYPE_NAO38:
/* This is really the NAC38 */
return "AAAAADABAAAAAAAAAAAAAAAAAAAAAAAA";
- case CVMX_BOARD_TYPE_THUNDER:
- return "";
- case CVMX_BOARD_TYPE_EBH3000:
- return "";
case CVMX_BOARD_TYPE_EBH3100:
case CVMX_BOARD_TYPE_CN3010_EVB_HS5:
case CVMX_BOARD_TYPE_CN3005_EVB_HS5:
return "AAABAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
case CVMX_BOARD_TYPE_BBGW_REF:
return "AABCD";
+ case CVMX_BOARD_TYPE_THUNDER:
+ case CVMX_BOARD_TYPE_EBH3000:
default:
return "";
}
diff --git a/arch/mips/powertv/asic/asic_devices.c b/arch/mips/powertv/asic/asic_devices.c
index bae8288..36dbcad 100644
--- a/arch/mips/powertv/asic/asic_devices.c
+++ b/arch/mips/powertv/asic/asic_devices.c
@@ -340,10 +340,6 @@ static void __init platform_configure_usb(void)

switch (asic) {
case ASIC_ZEUS:
- fs_update(0x0000, 0x11, 0x02, 0);
- bcm1_usb2_ctl = 0x803;
- break;
-
case ASIC_CRONUS:
case ASIC_CRONUSLITE:
fs_update(0x0000, 0x11, 0x02, 0);
diff --git a/arch/mips/sgi-ip32/ip32-irq.c b/arch/mips/sgi-ip32/ip32-irq.c
index 5c2bf11..d8b6520 100644
--- a/arch/mips/sgi-ip32/ip32-irq.c
+++ b/arch/mips/sgi-ip32/ip32-irq.c
@@ -512,10 +512,6 @@ void __init arch_init_irq(void)
"level");
break;

- case CRIME_GBE0_IRQ ... CRIME_GBE3_IRQ:
- set_irq_chip_and_handler_name(irq,
- &crime_edge_interrupt, handle_edge_irq, "edge");
- break;
case CRIME_CPUERR_IRQ:
case CRIME_MEMERR_IRQ:
set_irq_chip_and_handler_name(irq,
@@ -523,12 +519,9 @@ void __init arch_init_irq(void)
"level");
break;

+ case CRIME_GBE0_IRQ ... CRIME_GBE3_IRQ:
case CRIME_RE_EMPTY_E_IRQ ... CRIME_RE_IDLE_E_IRQ:
case CRIME_SOFT0_IRQ ... CRIME_SOFT2_IRQ:
- set_irq_chip_and_handler_name(irq,
- &crime_edge_interrupt, handle_edge_irq, "edge");
- break;
-
case CRIME_VICE_IRQ:
set_irq_chip_and_handler_name(irq,
&crime_edge_interrupt, handle_edge_irq, "edge");


2010-01-20 02:03:16

by David Daney

[permalink] [raw]
Subject: Re: [PATCH] MIPS: cleanup switches with cases that can be merged

Roel Kluin wrote:
> I did a search for switch statements with cases that can be merged, but maybe
> some were not intended?
> ---------------->8------------------------------------------8<-----------------
> In these cases the same code was executed.
>
> Signed-off-by: Roel Kluin <[email protected]>
> ---
> arch/mips/include/asm/octeon/octeon-feature.h | 8 ++------
> arch/mips/kernel/cpu-probe.c | 3 ---
> arch/mips/math-emu/ieee754dp.c | 1 -
> arch/mips/math-emu/ieee754sp.c | 1 -
> arch/mips/pci/pci-octeon.c | 6 ++----
> arch/mips/powertv/asic/asic_devices.c | 4 ----
> arch/mips/sgi-ip32/ip32-irq.c | 9 +--------
> 7 files changed, 5 insertions(+), 27 deletions(-)

This patch should be split up.

Octeon, PowerTV, and IP32 are all different architectures. They should
be in their own patches.

The two math-emu parts could probably go together.

cpu-probe seems like its own thing.

This brings us to the larger question: This is just code churn. Is it
even worthwhile?


David Daney


2010-01-20 12:10:17

by Alexander Clouter

[permalink] [raw]
Subject: Re: [PATCH] MIPS: cleanup switches with cases that can be merged

In gmane.linux.ports.mips.general David Daney <[email protected]> wrote:
>
> [snipped]
>
> This brings us to the larger question: This is just code churn. Is it
> even worthwhile?
>
Anything which reduces the line count and remove duplication whilst
sticking to CodingStyle I would always argue for, but "who am I" :)

It at a glance, this type of code churn, shows there are no differences
between two classes of <whatever> and the effect is it makes the chunk
of code a mental NOOP for the person reading the code. :)

To me this is a natural extension of running with Chapter 14 of
CodingStyle where you kmalloc using 'sizeof(*p)' rather than
'sizeof(struct *foo)' so reducing the chance of errors later on.

Just my thoughts.

Cheers

--
Alexander Clouter
.sigmonster says: Snoopy: No problem is so big that it can't be run away from.

2010-01-23 16:32:15

by Ralf Baechle

[permalink] [raw]
Subject: Re: [PATCH] MIPS: cleanup switches with cases that can be merged

On Tue, Jan 19, 2010 at 06:02:06PM -0800, David Daney wrote:

> This patch should be split up.
>
> Octeon, PowerTV, and IP32 are all different architectures. They
> should be in their own patches.
>
> The two math-emu parts could probably go together.
>
> cpu-probe seems like its own thing.

It's conceptually the same change that's being applied everywhere and the
total size is modest so I'm happy to apply it as just a single patch as is.

> This brings us to the larger question: This is just code churn. Is
> it even worthwhile?

This has been discussed many times over and we maintainers have not come to
a final conclusion on this type of patches. I tend to apply this sort of
patches anyway but treat them as low priority.

Ralf

2010-01-24 00:23:57

by Ralf Baechle

[permalink] [raw]
Subject: Re: [PATCH] MIPS: cleanup switches with cases that can be merged

On Wed, Jan 20, 2010 at 12:59:27AM +0100, Roel Kluin wrote:

> I did a search for switch statements with cases that can be merged, but maybe
> some were not intended?
> ---------------->8------------------------------------------8<-----------------
> In these cases the same code was executed.

Queued for 2.6.34.

Ralf