[U-Boot] [PATCH v2] spi: mxc_spi: Set master mode for all channels

[Stefano Babic]

On 10/04/2013 01:06, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam at freescale.com>
> 
> The glitch in the SPI clock line, which commit 3cea335c34 (spi: mxc_spi: Fix spi
> clock glitch durant reset) solved, is back now and itwas re-introduced by 
> commit d36b39bf0d (spi: mxc_spi: Fix ECSPI reset handling).
> 
> Actually the glitch is happening due to always toggling between slave mode
> and master mode by configuring the CHANNEL_MODE bits in this reset function.
> 
> Since the spi driver only supports master mode, set the mode for all channels 
> always to master mode in order to have a stable, "glitch-free" SPI clock line.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam at freescale.com>
> ---

Hi Fabio,


> Changes since v1:
> - Introduce MXC_CSPICTRL_MODE_MASK definition
> - Remove additional read of reg_ctrl
> 
>  arch/arm/include/asm/arch-mx5/imx-regs.h |    1 +
>  arch/arm/include/asm/arch-mx6/imx-regs.h |    1 +
>  drivers/spi/mxc_spi.c                    |   17 +++++++++--------
>  3 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-mx5/imx-regs.h b/arch/arm/include/asm/arch-mx5/imx-regs.h
> index 249d15a..a71cc13 100644
> --- a/arch/arm/include/asm/arch-mx5/imx-regs.h
> +++ b/arch/arm/include/asm/arch-mx5/imx-regs.h
> @@ -230,6 +230,7 @@
>  #define MXC_CSPICTRL_EN		(1 << 0)
>  #define MXC_CSPICTRL_MODE	(1 << 1)
>  #define MXC_CSPICTRL_XCH	(1 << 2)
> +#define MXC_CSPICTRL_MODE_MASK	(0xf << 4)
>  #define MXC_CSPICTRL_CHIPSELECT(x)	(((x) & 0x3) << 12)
>  #define MXC_CSPICTRL_BITCOUNT(x)	(((x) & 0xfff) << 20)
>  #define MXC_CSPICTRL_PREDIV(x)	(((x) & 0xF) << 12)
> diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h b/arch/arm/include/asm/arch-mx6/imx-regs.h
> index eaa7439..d79ab2f 100644
> --- a/arch/arm/include/asm/arch-mx6/imx-regs.h
> +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h
> @@ -346,6 +346,7 @@ struct cspi_regs {
>  #define MXC_CSPICTRL_EN		(1 << 0)
>  #define MXC_CSPICTRL_MODE	(1 << 1)
>  #define MXC_CSPICTRL_XCH	(1 << 2)
> +#define MXC_CSPICTRL_MODE_MASK (0xf << 4)
>  #define MXC_CSPICTRL_CHIPSELECT(x)	(((x) & 0x3) << 12)
>  #define MXC_CSPICTRL_BITCOUNT(x)	(((x) & 0xfff) << 20)
>  #define MXC_CSPICTRL_PREDIV(x)	(((x) & 0xF) << 12)
> diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c
> index 4c19e0b..20419e6 100644
> --- a/drivers/spi/mxc_spi.c
> +++ b/drivers/spi/mxc_spi.c
> @@ -137,11 +137,15 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs,
>  		return -1;
>  	}
>  
> -	/* Reset spi */
> -	reg_write(&regs->ctrl, 0);
> -	reg_write(&regs->ctrl, MXC_CSPICTRL_EN);
> -
> -	reg_ctrl = reg_read(&regs->ctrl);
> +	/*
> +	 * Reset SPI and set all CSs to master mode, if toggling
> +	 * between slave and master mode we might see a glitch
> +	 * on the clock line
> +	 */
> +	reg_ctrl = MXC_CSPICTRL_MODE_MASK;
> +	reg_write(&regs->ctrl, reg_ctrl);
> +	reg_ctrl |=  MXC_CSPICTRL_EN;
> +	reg_write(&regs->ctrl, reg_ctrl);

I am afraid you are breaking MX3x / MX25 because you add
MXC_CSPICTRL_MODE_MASK only to MX5 / MX6.

Best regards,
Stefano


-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================