All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2] spi: mxc_spi: Set master mode for all channels
@ 2013-04-09 23:06 Fabio Estevam
  2013-04-10  6:10 ` Stefano Babic
  2013-04-14  7:08 ` Dirk Behme
  0 siblings, 2 replies; 6+ messages in thread
From: Fabio Estevam @ 2013-04-09 23:06 UTC (permalink / raw)
  To: u-boot

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.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
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);
 
 	/*
 	 * The following computation is taken directly from Freescale's code.
@@ -174,9 +178,6 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs,
 	reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_POSTDIV(0x0F)) |
 		MXC_CSPICTRL_POSTDIV(post_div);
 
-	/* always set to master mode */
-	reg_ctrl |= 1 << (cs + 4);
-
 	/* We need to disable SPI before changing registers */
 	reg_ctrl &= ~MXC_CSPICTRL_EN;
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2] spi: mxc_spi: Set master mode for all channels
  2013-04-09 23:06 [U-Boot] [PATCH v2] spi: mxc_spi: Set master mode for all channels Fabio Estevam
@ 2013-04-10  6:10 ` Stefano Babic
  2013-04-10 12:01   ` Fabio Estevam
  2013-04-14  7:08 ` Dirk Behme
  1 sibling, 1 reply; 6+ messages in thread
From: Stefano Babic @ 2013-04-10  6:10 UTC (permalink / raw)
  To: u-boot

On 10/04/2013 01:06, Fabio Estevam wrote:
> 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.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@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
=====================================================================

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2] spi: mxc_spi: Set master mode for all channels
  2013-04-10  6:10 ` Stefano Babic
@ 2013-04-10 12:01   ` Fabio Estevam
  2013-04-13 15:50     ` Stefano Babic
  0 siblings, 1 reply; 6+ messages in thread
From: Fabio Estevam @ 2013-04-10 12:01 UTC (permalink / raw)
  To: u-boot

Hi Stefano,

On Wed, Apr 10, 2013 at 3:10 AM, Stefano Babic <sbabic@denx.de> wrote:

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

It will not break other platforms because this code is protected with
a "#ifdef MXC_ECSPI".

And MXC_ECSPI is only defined on mx5/mx6:

arch/arm/include/asm/arch-mx5/imx-regs.h:#define MXC_ECSPI
arch/arm/include/asm/arch-mx6/imx-regs.h:#define MXC_ECSPI

,so we are safe here.

Regards,

Fabio Estevam

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2] spi: mxc_spi: Set master mode for all channels
  2013-04-10 12:01   ` Fabio Estevam
@ 2013-04-13 15:50     ` Stefano Babic
  0 siblings, 0 replies; 6+ messages in thread
From: Stefano Babic @ 2013-04-13 15:50 UTC (permalink / raw)
  To: u-boot

On 10/04/2013 14:01, Fabio Estevam wrote:
> Hi Stefano,
> 
> On Wed, Apr 10, 2013 at 3:10 AM, Stefano Babic <sbabic@denx.de> wrote:
> 
>> I am afraid you are breaking MX3x / MX25 because you add
>> MXC_CSPICTRL_MODE_MASK only to MX5 / MX6.
> 
> It will not break other platforms because this code is protected with
> a "#ifdef MXC_ECSPI".
> 
> And MXC_ECSPI is only defined on mx5/mx6:
> 
> arch/arm/include/asm/arch-mx5/imx-regs.h:#define MXC_ECSPI
> arch/arm/include/asm/arch-mx6/imx-regs.h:#define MXC_ECSPI
> 
> ,so we are safe here.

Thanks, you're right.

Applied to u-boot-imx, thanks.

Best regards,
Stefano Babic


-- 
=====================================================================
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
=====================================================================

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2] spi: mxc_spi: Set master mode for all channels
  2013-04-09 23:06 [U-Boot] [PATCH v2] spi: mxc_spi: Set master mode for all channels Fabio Estevam
  2013-04-10  6:10 ` Stefano Babic
@ 2013-04-14  7:08 ` Dirk Behme
  2013-05-01  5:38   ` Dirk Behme
  1 sibling, 1 reply; 6+ messages in thread
From: Dirk Behme @ 2013-04-14  7:08 UTC (permalink / raw)
  To: u-boot

Am 10.04.2013 01:06, 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.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> 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;

I was offline some days, giving me some time to think about this ;)

Most probably it does no harm, but I somehow feel uncomfortable with 
setting *all* CSs to master mode. Just because there might be already 
(one, several?) CS at master mode and switching it back to the (reset 
default) slave will give a glitch on the clock line.

Wouldn't it be cleaner to keep the master mode only for the CS which 
are already in master mode before?

E.g. instead of

reg_ctrl = MXC_CSPICTRL_MODE_MASK;

from above something like

reg_ctrl = reg_read(&regs->ctrl) & MXC_CSPICTRL_MODE_MASK;

?

And then ...

> +	reg_write(&regs->ctrl, reg_ctrl);
> +	reg_ctrl |=  MXC_CSPICTRL_EN;
> +	reg_write(&regs->ctrl, reg_ctrl);
>
>   	/*
>   	 * The following computation is taken directly from Freescale's code.
> @@ -174,9 +178,6 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs,
>   	reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_POSTDIV(0x0F)) |
>   		MXC_CSPICTRL_POSTDIV(post_div);
>
> -	/* always set to master mode */
> -	reg_ctrl |= 1 << (cs + 4);

... keeping thins line?

Best regards

Dirk

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v2] spi: mxc_spi: Set master mode for all channels
  2013-04-14  7:08 ` Dirk Behme
@ 2013-05-01  5:38   ` Dirk Behme
  0 siblings, 0 replies; 6+ messages in thread
From: Dirk Behme @ 2013-05-01  5:38 UTC (permalink / raw)
  To: u-boot

Am 14.04.2013 09:08, schrieb Dirk Behme:
> Am 10.04.2013 01:06, 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.
>>
>> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
>> ---
>> 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;
>
> I was offline some days, giving me some time to think about this ;)
>
> Most probably it does no harm, but I somehow feel uncomfortable with
> setting *all* CSs to master mode. Just because there might be already
> (one, several?) CS at master mode and switching it back to the (reset
> default) slave will give a glitch on the clock line.
>
> Wouldn't it be cleaner to keep the master mode only for the CS which
> are already in master mode before?
>
> E.g. instead of
>
> reg_ctrl = MXC_CSPICTRL_MODE_MASK;
>
> from above something like
>
> reg_ctrl = reg_read(&regs->ctrl) & MXC_CSPICTRL_MODE_MASK;
>
> ?
>
> And then ...
>
>> +    reg_write(&regs->ctrl, reg_ctrl);
>> +    reg_ctrl |=  MXC_CSPICTRL_EN;
>> +    reg_write(&regs->ctrl, reg_ctrl);
>>
>>       /*
>>        * The following computation is taken directly from
>> Freescale's code.
>> @@ -174,9 +178,6 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave
>> *mxcs, unsigned int cs,
>>       reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_POSTDIV(0x0F)) |
>>           MXC_CSPICTRL_POSTDIV(post_div);
>>
>> -    /* always set to master mode */
>> -    reg_ctrl |= 1 << (cs + 4);
>
> ... keeping thins line?

What's about anything like below?

Best regards

Dirk

From: Dirk Behme <dirk.behme@gmail.com>
Date: Wed, 1 May 2013 07:28:38 +0200
Subject: [PATCH] spi: mxc_spi: Keep master mode only for configured 
channels

To avoid a glitch on the clock line while resetting the SPI controller,
the commit 0f1411bc8d (spi: mxc_spi: Set master mode for all channels)
enables the master mode for all channels.

Instead of enabling the master mode for all channels, keep only the
channels which are already configured in master mode in this mode.

To be able to switch additional channels to master mode, re-introduce
the master mode enable which was removed by above commit, then.

Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
---
  drivers/spi/mxc_spi.c |   11 +++++++----
  1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c
index 5bed858..843a1f2 100644
--- a/drivers/spi/mxc_spi.c
+++ b/drivers/spi/mxc_spi.c
@@ -138,11 +138,11 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave 
*mxcs, unsigned int cs,
  	}

  	/*
-	 * 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
+	 * Reset SPI and the CONREG, but keep the CSs which are already
+	 * in 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_ctrl = reg_read(&regs->ctrl) & MXC_CSPICTRL_MODE_MASK;
  	reg_write(&regs->ctrl, reg_ctrl);
  	reg_ctrl |=  MXC_CSPICTRL_EN;
  	reg_write(&regs->ctrl, reg_ctrl);
@@ -178,6 +178,9 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, 
unsigned int cs,
  	reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_POSTDIV(0x0F)) |
  		MXC_CSPICTRL_POSTDIV(post_div);

+	/* always set to master mode */
+	reg_ctrl |= 1 << (cs + 4);
+
  	/* We need to disable SPI before changing registers */
  	reg_ctrl &= ~MXC_CSPICTRL_EN;

-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-05-01  5:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-09 23:06 [U-Boot] [PATCH v2] spi: mxc_spi: Set master mode for all channels Fabio Estevam
2013-04-10  6:10 ` Stefano Babic
2013-04-10 12:01   ` Fabio Estevam
2013-04-13 15:50     ` Stefano Babic
2013-04-14  7:08 ` Dirk Behme
2013-05-01  5:38   ` Dirk Behme

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.