lkml.org 
[lkml]   [2007]   [Oct]   [24]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
    /
    DateWed, 24 Oct 2007 07:01:00 +0200
    FromDomen Puncer <>
    SubjectRe: [PATCH] mpc5200: psc-spi driver must not touch port_config or cdm registers
    On 23/10/07 21:39 -0600, Grant Likely wrote:
    > From: Grant Likely <grant.likely@secretlab.ca>
    > 
    > port_config and the cdm are the responsibility of firmware; and if
    > firmware doesn't set it up correctly, it should be fixed up by
    > platform code on a per-board basis because it's a property of the
    > board.
    > 
    > Drivers should never touch these registers.  They are completely
    > unprotected and the platform may do odd things with them that the
    > driver must not interfere with.
    > 
    > Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
    > ---
    > 
    > Domen, Dragos; can you please check your usage of this driver.  None of
    > the in-tree board ports use it as-is, but they are of course just a
    > device tree change away from being usable on the lite5200b.
    
    My test cases were just a jumper on RX-TX line.
    
    It's not even allowed to set mclken_div_pscX?
    There can be different speed devices on same SPI
    (spi_board_info->max_speed_hz).
    
    > 
    > Board ports that use it need to be modified to make the cdm/port_config
    > changes in firmware (prefered) or in arch/powerpc/platforms/52xx/<board>.c
    > 
    > Cheers,
    > g.
    > 
    >  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 7051e6c..ec3a618 100644
    > --- a/drivers/spi/mpc52xx_psc_spi.c
    > +++ b/drivers/spi/mpc52xx_psc_spi.c
    > @@ -328,78 +328,15 @@ static void mpc52xx_psc_spi_cleanup(struct spi_device *spi)
    >  	kfree(spi->controller_state);
    >  }
    > 
    > -static int mpc52xx_psc_spi_port_config(int psc_id, struct mpc52xx_psc_spi *mps)
    > +static void mpc52xx_psc_spi_config(int psc_id, struct mpc52xx_psc_spi *mps)
    >  {
    > -	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)
    > -	cdm = mpc52xx_find_and_map("mpc5200-cdm");
    > -	gpio = mpc52xx_find_and_map("mpc5200-gpio");
    > -#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);
    
    It's actually 528e6, and mclken_div doesn't seem to be used anywhere anymore.
    
    > 
    > -	switch (psc_id) {
    > -	case 1:
    ...
    
    
    	Domen
    -
    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-10-24 07:27    [from the cache]
    ©2003-2008