Am 06.04.2013 15:15, schrieb Fabio Estevam:

From: Fabio Estevam fabio.estevam@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@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(&regs->ctrl, 0);
• reg_write(&regs->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(&regs->ctrl, 0x000000F0);

Do we have a macro for the 0xF?

• reg_ctrl = reg_read(&regs->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(&regs->ctrl, reg_ctrl | MXC_CSPICTRL_EN);

  reg_ctrl = reg_read(&regs->ctrl);

So, as mentioned by Stefano before, I'd propose to either

a) do the reg_read(&regs->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(&regs->ctrl, 0x000000F0); /* FIXME: use a macro here*/ reg_ctrl = reg_read(&regs->ctrl) | MXC_CSPICTRL_EN; /* Reading back the register content is done because we don't get back the value written above */ reg_write(&regs->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(&regs->ctrl, reg_ctrl); reg_ctrl |= MXC_CSPICTRL_EN; reg_write(&regs->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