[U-Boot] [PATCH] spi: mxc_spi: Set master mode for all channels
Dirk Behme
dirk.behme at gmail.com
Sat Apr 6 15:42:32 CEST 2013
Am 06.04.2013 15:15, schrieb Fabio Estevam:
> 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.
Ok, thanks, I wasn't aware of this.
> Signed-off-by: Fabio Estevam <fabio.estevam at freescale.com>
> ---
> drivers/spi/mxc_spi.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c
> index 4c19e0b..9eb2bce 100644
> --- a/drivers/spi/mxc_spi.c
> +++ b/drivers/spi/mxc_spi.c
> @@ -137,9 +137,14 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs,
> return -1;
> }
>
> - /* Reset spi */
> - reg_write(®s->ctrl, 0);
> - reg_write(®s->ctrl, MXC_CSPICTRL_EN);
> + /*
> + * Reset SPI and set all CSs to master mode,
I haven't checked the rest of the driver, but do we touch the CSs bits
anywhere else, too? Could that parts dropped with this patch, then.
I.e. never touch the bits regs->ctrl[7-4] again?
> if toggling
> + * between slave and master mode we might see a glitch
> + * on the clock line
> + */
> + reg_write(®s->ctrl, 0x000000F0);
Do we have a macro for the 0xF?
> + reg_ctrl = reg_read(®s->ctrl);
Do we expect reg_ctrl to be 0x000000F0 then? If yes, do we have to
really read it here? If we read it here, should we drop the read
below, then?
> + reg_write(®s->ctrl, reg_ctrl | MXC_CSPICTRL_EN);
>
> reg_ctrl = reg_read(®s->ctrl);
So, as mentioned by Stefano before, I'd propose to either
a) do the reg_read(®s->ctrl); only _once_. E.g.
/*
* 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_write(®s->ctrl, 0x000000F0); /* FIXME: use a macro here*/
reg_ctrl = reg_read(®s->ctrl) | MXC_CSPICTRL_EN; /* Reading back
the register content is done because we don't get back the value
written above */
reg_write(®s->ctrl, reg_ctrl);
=> no additional read of reg_ctrl needed here as reg_ctrl contains
already the recent register content <=
or
b) _never_ read back the register. E.g.
/*
* 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 = 0x000000F0; /* FIXME: use a macro here*/
reg_write(®s->ctrl, reg_ctrl);
reg_ctrl |= MXC_CSPICTRL_EN;
reg_write(®s->ctrl, reg_ctrl);
=> no additional read of reg_ctrl needed here as reg_ctrl contains
already the recent register content <=
Would one of these options work?
Best regards
Dirk
More information about the U-Boot
mailing list