On Wed, 21 Jan 2015, Markos Chandras wrote: > >> diff --git a/arch/mips/kernel/proc.c b/arch/mips/kernel/proc.c > >> index 097fc8d14e42..a8fdf9685cad 100644 > >> --- a/arch/mips/kernel/proc.c > >> +++ b/arch/mips/kernel/proc.c > >> @@ -82,7 +82,9 @@ static int show_cpuinfo(struct seq_file *m, void *v) > >> seq_printf(m, "]\n"); > >> } > >> > >> - seq_printf(m, "isa\t\t\t: mips1"); > >> + seq_printf(m, "isa\t\t\t:"); > >> + if (!cpu_has_mips_r6) > >> + seq_printf(m, " mips1"); > > > > I think define `cpu_has_mips_r1' instead and use it here. It may turn > > out needed elsewhere too. We probably don't need a new `MIPS_CPU_ISA_I' > > bit at this stage so this could be: Typo here, I meant `cpu_has_mips_1' actually, sorry about that. > the change is simple enough and I see no reason to define the > cpu_has_mips_r1 at the moment. If we ever need to explicitly handle r1, > we can reconsider that. It's a matter of code clarity, good code is self-explanatory. Here the intent is to print `mips1' if it is supported. By avoiding the extra definition you're detaching the intent from what code says. Someone reading this code (who may not necessarily know the architecture documents by heart) has to scratch their head thinking: "why isn't `mips1' printed for R6, what the former has to do with the latter, and why is this case different to `mips2' and other ones that follow?" Whereas the intent is clear with this: #define cpu_has_mips_1 (!cpu_has_mips_r6) // Aha, `mips1' is there if no R6! if (cpu_has_mips_1) seq_printf(m, " mips1"); // Well, this is obvious... Do you see what I mean? Do you agree now? Maciej
