Re: [PATCH V2] [POWERPC] Add common clock setting routine mpc52xx_psc_set_clkdiv()

[Stephen Rothwell <sfr@canb.auug.org.au>]

[-- Attachment #1: Type: text/plain, Size: 1780 bytes --]

Hi Grant,

Just a couple of trivial things.

On Tue, 08 Jan 2008 22:09:50 -0700 Grant Likely <grant.likely@secretlab.ca> wrote:
>
> +int mpc52xx_set_psc_clkdiv(int psc_id, int clkdiv)

> +	switch (psc_id) {
> +	  case 1: reg = &mpc52xx_cdm->mclken_div_psc1; mask = 0x20; break;
> +	  case 2: reg = &mpc52xx_cdm->mclken_div_psc2; mask = 0x40; break;
> +	  case 3: reg = &mpc52xx_cdm->mclken_div_psc3; mask = 0x80; break;
> +	  case 6: reg = &mpc52xx_cdm->mclken_div_psc6; mask = 0x10; break;
> +	  default:
> +		return -ENODEV;
> +	}

We line up case labels with the switch statement itself.

> +int mpc52xx_set_psc_clkdiv(int psc_id, int clkdiv)

> +	switch (psc_id) {
> +	  case 1: reg = &cdm->mclken_div_psc1; mask = 0x20; break;
> +	  case 2: reg = &cdm->mclken_div_psc2; mask = 0x40; break;
> +	  case 3: reg = &cdm->mclken_div_psc3; mask = 0x80; break;
> +	  case 6: reg = &cdm->mclken_div_psc6; mask = 0x10; break;
> +	  default:
> +	  	return -ENODEV;
> +	}

Here as well.

> +++ b/include/asm-powerpc/mpc52xx.h
> -extern void mpc5200_setup_xlb_arbiter(void);
> -extern void mpc52xx_declare_of_platform_devices(void);
> +extern void __init mpc5200_setup_xlb_arbiter(void);
> +extern void __init mpc52xx_declare_of_platform_devices(void);
> +extern void __init mpc52xx_map_common_devices(void);

We don't mark things __init in header files.  It is unnecessary and can
get out of sync - which causes unexpected results.

> +/* mpc52xx_pci.c */
>  #ifdef CONFIG_PCI
>  extern int __init mpc52xx_add_bridge(struct device_node *node);
>  extern void __init mpc52xx_setup_pci(void);

I see where yours above came from ...

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]