lkml.org 
[lkml]   [2007]   [May]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 1/2] [POWERPC] 8xx: mpc885ads pcmcia support
From
Hi,

Some minor nitpicks below. Overall it looks good.


-Olof

On Sun, May 06, 2007 at 05:26:33AM +0400, Vitaly Bordug wrote:
> @@ -322,6 +334,70 @@ void init_smc_ioports(struct fs_uart_platform_info *data)
> }
> }
>
> +#ifdef CONFIG_PCMCIA_M8XX
> +static void pcmcia_hw_setup(int slot, int enable)
> +{
> + unsigned *bcsr_io;
> +
> + bcsr_io = ioremap(BCSR1, sizeof(unsigned long));
> + if (enable)
> + clrbits32(bcsr_io, BCSR1_PCCEN);
> + else
> + setbits32(bcsr_io, BCSR1_PCCEN);
> +
> + iounmap(bcsr_io);
> +}

If you move this (and next) function up, you don't have to do the
prototypes in the beginning of the file. One less #ifdef that way.

> +
> +static int pcmcia_set_voltage(int slot, int vcc, int vpp)
> +{
> + u32 reg = 0;
> + unsigned *bcsr_io;
> +
> + bcsr_io = ioremap(BCSR1, sizeof(unsigned long));
> +
> + switch(vcc) {
> + case 0:
> + break;
> + case 33:
> + reg |= BCSR1_PCCVCC0;
> + break;
> + case 50:
> + reg |= BCSR1_PCCVCC1;
> + break;
> + default:
> + return 1;
> + }
> +
> + switch(vpp) {
> + case 0:
> + break;
> + case 33:
> + case 50:
> + if(vcc == vpp)
> + reg |= BCSR1_PCCVPP1;
> + else
> + return 1;
> + break;
> + case 120:
> + if ((vcc == 33) || (vcc == 50))
> + reg |= BCSR1_PCCVPP0;
> + else
> + return 1;
> + default:
> + return 1;
> + }
> +
> + /* first, turn off all power */
> + clrbits32(bcsr_io, 0x00610000);
> +
> + /* enable new powersettings */
> + setbits32(bcsr_io, reg);
> +
> + iounmap(bcsr_io);
> + return 0;
> +}
> +#endif
> +
> int platform_device_skip(const char *model, int id)
> {
> #ifdef CONFIG_MPC8xx_SECOND_ETH_SCC3
> diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
> index 8a123c7..880e45f 100644
> --- a/arch/powerpc/sysdev/fsl_soc.c
> +++ b/arch/powerpc/sysdev/fsl_soc.c
> @@ -1028,6 +1028,19 @@ err:
>
> arch_initcall(fs_enet_of_init);
>
> +static int __init fsl_pcmcia_of_init(void)
> +{
> + struct device_node *np = NULL;
> + /*
> + * Register all the devices which type is "pcmcia"
> + */
> + while ((np = of_find_compatible_node(np,
> + "pcmcia", "fsl,pq-pcmcia")) != NULL)
> + of_platform_device_create(np, "m8xx-pcmcia", NULL);

Do you really need this now that you have the device table in the driver?

> + return 0;
> +}
> +
> +arch_initcall(fsl_pcmcia_of_init);
>
> static const char *smc_regs = "regs";
> static const char *smc_pram = "pram";
> diff --git a/arch/powerpc/sysdev/mpc8xx_pic.h b/arch/powerpc/sysdev/mpc8xx_pic.h
> index afa2ee6..07be061 100644
> --- a/arch/powerpc/sysdev/mpc8xx_pic.h
> +++ b/arch/powerpc/sysdev/mpc8xx_pic.h
> @@ -4,7 +4,7 @@
> #include <linux/irq.h>
> #include <linux/interrupt.h>
>
> -extern struct hw_interrupt_type mpc8xx_pic;
> +/*extern struct hw_interrupt_type mpc8xx_pic;*/

Take it out or leave it in, but don't comment it out, please

>
> int mpc8xx_pic_init(void);
> unsigned int mpc8xx_get_irq(void);
> diff --git a/drivers/pcmcia/Kconfig b/drivers/pcmcia/Kconfig
> index 35f8864..c3fd55d 100644
> --- a/drivers/pcmcia/Kconfig
> +++ b/drivers/pcmcia/Kconfig
> @@ -183,6 +183,7 @@ config PCMCIA_M8XX
> tristate "MPC8xx PCMCIA support"
> depends on PCMCIA && PPC && 8xx
> select PCCARD_IODYN
> + select PCCARD_NONSTATIC

I was going to say "whitespace!", but seems like this added line is the only one
that's actually using tabs. :-) (Yes, I saw 2/2 that fixes this).

> help
> Say Y here to include support for PowerPC 8xx series PCMCIA
> controller.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-05-08 09:01    [W:0.121 / U:0.156 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site