[RFC 2/2] mpc52xx_psc_spi device driver must not touch port_config and cdm

[Dragos Carp]

Acked-by: Dragos Carp <dragos.carp at toptica.com>

On Mon, 2008-01-07 at 12:07 -0700, Grant Likely wrote:
> From: Grant Likely <grant.likely at secretlab.ca>
> 
> It is dangerous for an mpc52xx device driver to modify the port_config
> register.  If the driver is probed incorrectly, it will change the pin
> IO configuration in ways which may not be compatible with the board.
> port_config should be set up by the bootloader, or failing that, in
> the platform setup code in arch/powerpc/platforms/52xx.
> 
> Also, modifying CDM registers directly can cause a race condition with
> other drivers.  Instead call a common routine to modify CDM settings.
> 
> Signed-off-by: Grant Likely <grant.likely at secretlab.ca>
> ---
> 
>  drivers/spi/mpc52xx_psc_spi.c |   77 +----------------------------------------
>  1 files changed, 2 insertions(+), 75 deletions(-)
> 
> diff --git a/drivers/spi/mpc52xx_psc_spi.c b/drivers/spi/mpc52xx_psc_spi.c
> index a3ebc63..253ed56 100644
> --- a/drivers/spi/mpc52xx_psc_spi.c
> +++ b/drivers/spi/mpc52xx_psc_spi.c
> @@ -330,80 +330,13 @@ static void mpc52xx_psc_spi_cleanup(struct spi_device *spi)
>  
>  static int mpc52xx_psc_spi_port_config(int psc_id, struct mpc52xx_psc_spi *mps)
>  {
> -	struct device_node *np;
> -	struct mpc52xx_cdm __iomem *cdm;
> -	struct mpc52xx_gpio __iomem *gpio;
>  	struct mpc52xx_psc __iomem *psc = mps->psc;
> -	u32 ul;
>  	u32 mclken_div;
>  	int ret = 0;
>  
> -#if defined(CONFIG_PPC_MERGE)
> -	np = of_find_compatible_node(NULL, NULL, "mpc5200-cdm");
> -	cdm = of_iomap(np, 0);
> -	of_node_put(np);
> -	np = of_find_compatible_node(NULL, NULL, "mpc5200-gpio");
> -	gpio = of_iomap(np, 0);
> -	of_node_put(np);
> -#else
> -	cdm = ioremap(MPC52xx_PA(MPC52xx_CDM_OFFSET), MPC52xx_CDM_SIZE);
> -	gpio = ioremap(MPC52xx_PA(MPC52xx_GPIO_OFFSET), MPC52xx_GPIO_SIZE);
> -#endif
> -	if (!cdm || !gpio) {
> -		printk(KERN_ERR "Error mapping CDM/GPIO\n");
> -		ret = -EFAULT;
> -		goto unmap_regs;
> -	}
> -
>  	/* default sysclk is 512MHz */
> -	mclken_div = 0x8000 |
> -		(((mps->sysclk ? mps->sysclk : 512000000) / MCLK) & 0x1FF);
> -
> -	switch (psc_id) {
> -	case 1:
> -		ul = in_be32(&gpio->port_config);
> -		ul &= 0xFFFFFFF8;
> -		ul |= 0x00000006;
> -		out_be32(&gpio->port_config, ul);
> -		out_be16(&cdm->mclken_div_psc1, mclken_div);
> -		ul = in_be32(&cdm->clk_enables);
> -		ul |= 0x00000020;
> -		out_be32(&cdm->clk_enables, ul);
> -		break;
> -	case 2:
> -		ul = in_be32(&gpio->port_config);
> -		ul &= 0xFFFFFF8F;
> -		ul |= 0x00000060;
> -		out_be32(&gpio->port_config, ul);
> -		out_be16(&cdm->mclken_div_psc2, mclken_div);
> -		ul = in_be32(&cdm->clk_enables);
> -		ul |= 0x00000040;
> -		out_be32(&cdm->clk_enables, ul);
> -		break;
> -	case 3:
> -		ul = in_be32(&gpio->port_config);
> -		ul &= 0xFFFFF0FF;
> -		ul |= 0x00000600;
> -		out_be32(&gpio->port_config, ul);
> -		out_be16(&cdm->mclken_div_psc3, mclken_div);
> -		ul = in_be32(&cdm->clk_enables);
> -		ul |= 0x00000080;
> -		out_be32(&cdm->clk_enables, ul);
> -		break;
> -	case 6:
> -		ul = in_be32(&gpio->port_config);
> -		ul &= 0xFF8FFFFF;
> -		ul |= 0x00700000;
> -		out_be32(&gpio->port_config, ul);
> -		out_be16(&cdm->mclken_div_psc6, mclken_div);
> -		ul = in_be32(&cdm->clk_enables);
> -		ul |= 0x00000010;
> -		out_be32(&cdm->clk_enables, ul);
> -		break;
> -	default:
> -		ret = -EINVAL;
> -		goto unmap_regs;
> -	}
> +	mclken_div = (mps->sysclk ? mps->sysclk : 512000000) / MCLK;
> +	mpc52xx_set_psc_clkdiv(psc_id, mclken_div);
>  
>  	/* Reset the PSC into a known state */
>  	out_8(&psc->command, MPC52xx_PSC_RST_RX);
> @@ -427,12 +360,6 @@ static int mpc52xx_psc_spi_port_config(int psc_id, struct mpc52xx_psc_spi *mps)
>  
>  	mps->bits_per_word = 8;
>  
> -unmap_regs:
> -	if (cdm)
> -		iounmap(cdm);
> -	if (gpio)
> -		iounmap(gpio);
> -
>  	return ret;
>  }
>  
> 
>