Re: [PATCH] x86: Coding style fixes for arch/x86/kernel/cpu/centaur.c

["Paolo Ciarrocchi" <paolo.ciarrocchi@gmail.com>]

On Feb 18, 2008 3:56 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com> wrote:
>
> > [PATCH] x86: Coding style fixes for arch/x86/kernel/cpu/centaur.c
> >
> > Kills more than 150 errors/warnings
>
> thanks Paolo, applied. This file was _really_ unclean.

You are welcome. Kudos to your quality script ;-)

> btw., there were more style inconsistencies in this file, some of which
> were reported by checkpatch.pl - the patch below (ontop of yours) fixes
> them. Feel free to do such type of changes in future cleanup patches.
> Some of them are real CodingStyle problems, some of them are just
> arbitrary taste details. Now the file looks very clean in all of its
> details, and is completely checkpatch.pl --file clean as well. (If any
> of the changes i did is unclear, please ask.)

>  /*
> - *     Set up an actual MCR
> + * Set up an actual MCR
>   */

This was not highlighted by checkpatch.pl, right?
The same applies for a few other "fix" you did on some comments.

[...]
>  {
> -       u32 mem = ramtop();
>         u32 root = power2(mem);
> +       u32 mem = ramtop();

Can you please explain the rationale behind this change?

[...]
>
> +enum {
> +               ECX8            = 1<<1,
> +               EIERRINT        = 1<<2,
> +               DPM             = 1<<3,
> +               DMCE            = 1<<4,
> +               DSTPCLK         = 1<<5,
> +               ELINEAR         = 1<<6,
> +               DSMC            = 1<<7,
> +               DTLOCK          = 1<<8,
> +               EDCTLB          = 1<<8,
> +               EMMX            = 1<<9,
> +               DPDC            = 1<<11,
> +               EBRPRED         = 1<<12,
> +               DIC             = 1<<13,
> +               DDC             = 1<<14,
> +               DNA             = 1<<15,
> +               ERETSTK         = 1<<16,
> +               E2MMX           = 1<<19,
> +               EAMD3D          = 1<<20,
> +};
> +
>  static void __cpuinit init_centaur(struct cpuinfo_x86 *c)
>  {
> -       enum {
> -               ECX8 = 1<<1,
> -               EIERRINT = 1<<2,
> -               DPM = 1<<3,
> -               DMCE = 1<<4,
> -               DSTPCLK = 1<<5,
> -               ELINEAR = 1<<6,
> -               DSMC = 1<<7,
> -               DTLOCK = 1<<8,
> -               EDCTLB = 1<<8,
> -               EMMX = 1<<9,
> -               DPDC = 1<<11,
> -               EBRPRED = 1<<12,
> -               DIC = 1<<13,
> -               DDC = 1<<14,
> -               DNA = 1<<15,
> -               ERETSTK = 1<<16,
> -               E2MMX = 1<<19,
> -               EAMD3D = 1<<20,
> -       };

I see why you did the cleanup but I think even this part was not
catched by checkpatch.pl

>

Ok, I got all your cleanup. I just need to compare your work with the relative
checkpatch.pl output, in the past it happened that the error/warning
messages were not
very clear so I agreed with Andy about a couple of changes.

Thanks.

Ciao,
-- 
Paolo
http://paolo.ciarrocchi.googlepages.com/