public inbox for linux-omap@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH PM 0/2] OMAP McSPI: Fixes to save/restore
@ 2009-02-05 11:05 Aaro Koskinen
  2009-02-05 11:07 ` [PATCH PM 1/2] OMAP McSPI: Fix context save/restore Aaro Koskinen
  0 siblings, 1 reply; 8+ messages in thread
From: Aaro Koskinen @ 2009-02-05 11:05 UTC (permalink / raw)
  To: linux-omap@vger.kernel.org

Hello,

Here's two fixes for the recent save/restore change (OMAP3 McSPI: Adds 
context save/restore), which does not seem to work with chip select. 
Jouni Hogander already sent one patch, but it does not solve all the 
problems. This patch updates the shadow value on every write, and uses 
it instead of reading the register.

A.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH PM 1/2] OMAP McSPI: Fix context save/restore
  2009-02-05 11:05 [PATCH PM 0/2] OMAP McSPI: Fixes to save/restore Aaro Koskinen
@ 2009-02-05 11:07 ` Aaro Koskinen
  2009-02-05 11:07   ` [PATCH PM 2/2] OMAP McSPI: spin_unlock_irq() missing Aaro Koskinen
  2009-02-05 11:51   ` [PATCH PM 1/2] OMAP McSPI: Fix context save/restore Hemanth V
  0 siblings, 2 replies; 8+ messages in thread
From: Aaro Koskinen @ 2009-02-05 11:07 UTC (permalink / raw)
  To: linux-omap

Fix context save/restore to work with chip select. Also update the shadow
CHCONF0 register on every write and eliminate register reads.

Signed-off-by: Aaro Koskinen <Aaro.Koskinen@nokia.com>
---
 drivers/spi/omap2_mcspi.c |   51 +++++++++++++++++++++++++-------------------
 1 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
index a7ee3b7..4d09777 100644
--- a/drivers/spi/omap2_mcspi.c
+++ b/drivers/spi/omap2_mcspi.c
@@ -134,6 +134,7 @@ struct omap2_mcspi_cs {
 	void __iomem		*base;
 	unsigned long		phys;
 	int			word_len;
+	u32			chconf0;
 };
 
 /* used for context save and restore, structure members to be updated whenever
@@ -142,7 +143,6 @@ struct omap2_mcspi_cs {
 struct omap2_mcspi_regs {
 	u32 sysconfig;
 	u32 modulctrl;
-	u32 chconf0;
 	u32 wakeupenable;
 };
 
@@ -187,12 +187,27 @@ static inline u32 mcspi_read_cs_reg(const struct spi_device *spi, int idx)
 	return __raw_readl(cs->base + idx);
 }
 
+static inline u32 mcspi_cached_chconf0(const struct spi_device *spi)
+{
+	struct omap2_mcspi_cs *cs = spi->controller_state;
+
+	return cs->chconf0;
+}
+
+static inline void mcspi_write_chconf0(const struct spi_device *spi, u32 val)
+{
+	struct omap2_mcspi_cs *cs = spi->controller_state;
+
+	cs->chconf0 = val;
+	mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, val);
+}
+
 static void omap2_mcspi_set_dma_req(const struct spi_device *spi,
 		int is_read, int enable)
 {
 	u32 l, rw;
 
-	l = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
+	l = mcspi_cached_chconf0(spi);
 
 	if (is_read) /* 1 is read, 0 write */
 		rw = OMAP2_MCSPI_CHCONF_DMAR;
@@ -200,7 +215,7 @@ static void omap2_mcspi_set_dma_req(const struct spi_device *spi,
 		rw = OMAP2_MCSPI_CHCONF_DMAW;
 
 	MOD_REG_BIT(l, rw, enable);
-	mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, l);
+	mcspi_write_chconf0(spi, l);
 }
 
 static void omap2_mcspi_set_enable(const struct spi_device *spi, int enable)
@@ -215,9 +230,9 @@ static void omap2_mcspi_force_cs(struct spi_device *spi, int cs_active)
 {
 	u32 l;
 
-	l = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
+	l = mcspi_cached_chconf0(spi);
 	MOD_REG_BIT(l, OMAP2_MCSPI_CHCONF_FORCE, cs_active);
-	mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, l);
+	mcspi_write_chconf0(spi, l);
 }
 
 static void omap2_mcspi_set_master_mode(struct spi_master *master)
@@ -248,10 +263,6 @@ static void omap2_mcspi_restore_ctx(struct omap2_mcspi *mcspi)
 	mcspi_write_reg(spi_cntrl, OMAP2_MCSPI_SYSCONFIG,
 			omap2_mcspi_ctx[spi_cntrl->bus_num - 1].sysconfig);
 
-	mcspi_write_reg(spi_cntrl, OMAP2_MCSPI_CHCONF0,
-			omap2_mcspi_ctx[spi_cntrl->bus_num - 1].chconf0);
-
-
 	mcspi_write_reg(spi_cntrl, OMAP2_MCSPI_WAKEUPENABLE,
 			omap2_mcspi_ctx[spi_cntrl->bus_num - 1].wakeupenable);
 }
@@ -392,7 +403,7 @@ omap2_mcspi_txrx_pio(struct spi_device *spi, struct spi_transfer *xfer)
 	c = count;
 	word_len = cs->word_len;
 
-	l = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
+	l = mcspi_cached_chconf0(spi);
 	l &= ~OMAP2_MCSPI_CHCONF_TRM_MASK;
 
 	/* We store the pre-calculated register addresses on stack to speed
@@ -432,8 +443,7 @@ omap2_mcspi_txrx_pio(struct spi_device *spi, struct spi_transfer *xfer)
 				 * more word i/o: switch to rx+tx
 				 */
 				if (c == 0 && tx == NULL)
-					mcspi_write_cs_reg(spi,
-							OMAP2_MCSPI_CHCONF0, l);
+					mcspi_write_chconf0(spi, l);
 				*rx++ = __raw_readl(rx_reg);
 #ifdef VERBOSE
 				dev_dbg(&spi->dev, "read-%d %02x\n",
@@ -471,8 +481,7 @@ omap2_mcspi_txrx_pio(struct spi_device *spi, struct spi_transfer *xfer)
 				 * more word i/o: switch to rx+tx
 				 */
 				if (c == 0 && tx == NULL)
-					mcspi_write_cs_reg(spi,
-							OMAP2_MCSPI_CHCONF0, l);
+					mcspi_write_chconf0(spi, l);
 				*rx++ = __raw_readl(rx_reg);
 #ifdef VERBOSE
 				dev_dbg(&spi->dev, "read-%d %04x\n",
@@ -510,8 +519,7 @@ omap2_mcspi_txrx_pio(struct spi_device *spi, struct spi_transfer *xfer)
 				 * more word i/o: switch to rx+tx
 				 */
 				if (c == 0 && tx == NULL)
-					mcspi_write_cs_reg(spi,
-							OMAP2_MCSPI_CHCONF0, l);
+					mcspi_write_chconf0(spi, l);
 				*rx++ = __raw_readl(rx_reg);
 #ifdef VERBOSE
 				dev_dbg(&spi->dev, "read-%d %04x\n",
@@ -559,7 +567,7 @@ static int omap2_mcspi_setup_transfer(struct spi_device *spi,
 	} else
 		div = 15;
 
-	l = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
+	l = mcspi_cached_chconf0(spi);
 
 	/* standard 4-wire master mode:  SCK, MOSI/out, MISO/in, nCS
 	 * REVISIT: this controller could support SPI_3WIRE mode.
@@ -591,9 +599,7 @@ static int omap2_mcspi_setup_transfer(struct spi_device *spi,
 	else
 		l &= ~OMAP2_MCSPI_CHCONF_PHA;
 
-	mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, l);
-
-	omap2_mcspi_ctx[spi_cntrl->bus_num - 1].chconf0 = l;
+	mcspi_write_chconf0(spi, l);
 
 	dev_dbg(&spi->dev, "setup: speed %d, sample %s edge, clk %s\n",
 			OMAP2_MCSPI_MAX_FREQ / (1 << div),
@@ -697,6 +703,7 @@ static int omap2_mcspi_setup(struct spi_device *spi)
 			return -ENOMEM;
 		cs->base = mcspi->base + spi->chip_select * 0x14;
 		cs->phys = mcspi->phys + spi->chip_select * 0x14;
+		cs->chconf0 = 0;
 		spi->controller_state = cs;
 	}
 
@@ -791,13 +798,13 @@ static void omap2_mcspi_work(struct work_struct *work)
 				cs_active = 1;
 			}
 
-			chconf = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
+			chconf = mcspi_cached_chconf0(spi);
 			chconf &= ~OMAP2_MCSPI_CHCONF_TRM_MASK;
 			if (t->tx_buf == NULL)
 				chconf |= OMAP2_MCSPI_CHCONF_TRM_RX_ONLY;
 			else if (t->rx_buf == NULL)
 				chconf |= OMAP2_MCSPI_CHCONF_TRM_TX_ONLY;
-			mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, chconf);
+			mcspi_write_chconf0(spi, chconf);
 
 			if (t->len) {
 				unsigned	count;
-- 
1.5.4.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH PM 2/2] OMAP McSPI: spin_unlock_irq() missing
  2009-02-05 11:07 ` [PATCH PM 1/2] OMAP McSPI: Fix context save/restore Aaro Koskinen
@ 2009-02-05 11:07   ` Aaro Koskinen
  2009-02-05 11:51   ` [PATCH PM 1/2] OMAP McSPI: Fix context save/restore Hemanth V
  1 sibling, 0 replies; 8+ messages in thread
From: Aaro Koskinen @ 2009-02-05 11:07 UTC (permalink / raw)
  To: linux-omap

spin_unlock_irq() not called on failure path

Signed-off-by: Aaro Koskinen <Aaro.Koskinen@nokia.com>
---
 drivers/spi/omap2_mcspi.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
index 4d09777..56e91a4 100644
--- a/drivers/spi/omap2_mcspi.c
+++ b/drivers/spi/omap2_mcspi.c
@@ -751,7 +751,7 @@ static void omap2_mcspi_work(struct work_struct *work)
 	spin_lock_irq(&mcspi->lock);
 
 	if (omap2_mcspi_enable_clocks(mcspi))
-		return;
+		goto out;
 
 	/* We only enable one channel at a time -- the one whose message is
 	 * at the head of the queue -- although this controller would gladly
@@ -855,6 +855,7 @@ static void omap2_mcspi_work(struct work_struct *work)
 
 	omap2_mcspi_disable_clocks(mcspi);
 
+out:
 	spin_unlock_irq(&mcspi->lock);
 }
 
-- 
1.5.4.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH PM 1/2] OMAP McSPI: Fix context save/restore
  2009-02-05 11:07 ` [PATCH PM 1/2] OMAP McSPI: Fix context save/restore Aaro Koskinen
  2009-02-05 11:07   ` [PATCH PM 2/2] OMAP McSPI: spin_unlock_irq() missing Aaro Koskinen
@ 2009-02-05 11:51   ` Hemanth V
  2009-02-05 14:29     ` Kevin Hilman
  1 sibling, 1 reply; 8+ messages in thread
From: Hemanth V @ 2009-02-05 11:51 UTC (permalink / raw)
  To: Aaro Koskinen; +Cc: linux-omap

> Fix context save/restore to work with chip select. Also update the shadow
> CHCONF0 register on every write and eliminate register reads.

I believe the shadow CHCONF register need to have only the default
configuration as initialized by omap2_mcspi_setup_transfer(), since other other
bits are initialized as and when required by omap2_mcspi_work().

The original problem you saw might be because of not masking
OMAP2_MCSPI_CHCONF_TRM_MASK and OMAP2_MCSPI_CHCONF_FORCE bits while storing to
shadow register.

>
> Signed-off-by: Aaro Koskinen <Aaro.Koskinen@nokia.com>
> ---
>  drivers/spi/omap2_mcspi.c |   51 +++++++++++++++++++++++++-------------------
>  1 files changed, 29 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
> index a7ee3b7..4d09777 100644
> --- a/drivers/spi/omap2_mcspi.c
> +++ b/drivers/spi/omap2_mcspi.c
> @@ -134,6 +134,7 @@ struct omap2_mcspi_cs {
>  	void __iomem		*base;
>  	unsigned long		phys;
>  	int			word_len;
> +	u32			chconf0;
>  };
>
>  /* used for context save and restore, structure members to be updated whenever
> @@ -142,7 +143,6 @@ struct omap2_mcspi_cs {
>  struct omap2_mcspi_regs {
>  	u32 sysconfig;
>  	u32 modulctrl;
> -	u32 chconf0;
>  	u32 wakeupenable;
>  };
>
> @@ -187,12 +187,27 @@ static inline u32 mcspi_read_cs_reg(const struct
> spi_device *spi, int idx)
>  	return __raw_readl(cs->base + idx);
>  }
>
> +static inline u32 mcspi_cached_chconf0(const struct spi_device *spi)
> +{
> +	struct omap2_mcspi_cs *cs = spi->controller_state;
> +
> +	return cs->chconf0;
> +}
> +
> +static inline void mcspi_write_chconf0(const struct spi_device *spi, u32 val)
> +{
> +	struct omap2_mcspi_cs *cs = spi->controller_state;
> +
> +	cs->chconf0 = val;
> +	mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, val);
> +}
> +
>  static void omap2_mcspi_set_dma_req(const struct spi_device *spi,
>  		int is_read, int enable)
>  {
>  	u32 l, rw;
>
> -	l = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
> +	l = mcspi_cached_chconf0(spi);
>
>  	if (is_read) /* 1 is read, 0 write */
>  		rw = OMAP2_MCSPI_CHCONF_DMAR;
> @@ -200,7 +215,7 @@ static void omap2_mcspi_set_dma_req(const struct spi_device
> *spi,
>  		rw = OMAP2_MCSPI_CHCONF_DMAW;
>
>  	MOD_REG_BIT(l, rw, enable);
> -	mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, l);
> +	mcspi_write_chconf0(spi, l);
>  }
>
>  static void omap2_mcspi_set_enable(const struct spi_device *spi, int enable)
> @@ -215,9 +230,9 @@ static void omap2_mcspi_force_cs(struct spi_device *spi,
> int cs_active)
>  {
>  	u32 l;
>
> -	l = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
> +	l = mcspi_cached_chconf0(spi);
>  	MOD_REG_BIT(l, OMAP2_MCSPI_CHCONF_FORCE, cs_active);
> -	mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, l);
> +	mcspi_write_chconf0(spi, l);
>  }
>
>  static void omap2_mcspi_set_master_mode(struct spi_master *master)
> @@ -248,10 +263,6 @@ static void omap2_mcspi_restore_ctx(struct omap2_mcspi
> *mcspi)
>  	mcspi_write_reg(spi_cntrl, OMAP2_MCSPI_SYSCONFIG,
>  			omap2_mcspi_ctx[spi_cntrl->bus_num - 1].sysconfig);
>
> -	mcspi_write_reg(spi_cntrl, OMAP2_MCSPI_CHCONF0,
> -			omap2_mcspi_ctx[spi_cntrl->bus_num - 1].chconf0);
> -
> -
>  	mcspi_write_reg(spi_cntrl, OMAP2_MCSPI_WAKEUPENABLE,
>  			omap2_mcspi_ctx[spi_cntrl->bus_num - 1].wakeupenable);
>  }
> @@ -392,7 +403,7 @@ omap2_mcspi_txrx_pio(struct spi_device *spi, struct
> spi_transfer *xfer)
>  	c = count;
>  	word_len = cs->word_len;
>
> -	l = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
> +	l = mcspi_cached_chconf0(spi);
>  	l &= ~OMAP2_MCSPI_CHCONF_TRM_MASK;
>
>  	/* We store the pre-calculated register addresses on stack to speed
> @@ -432,8 +443,7 @@ omap2_mcspi_txrx_pio(struct spi_device *spi, struct
> spi_transfer *xfer)
>  				 * more word i/o: switch to rx+tx
>  				 */
>  				if (c == 0 && tx == NULL)
> -					mcspi_write_cs_reg(spi,
> -							OMAP2_MCSPI_CHCONF0, l);
> +					mcspi_write_chconf0(spi, l);
>  				*rx++ = __raw_readl(rx_reg);
>  #ifdef VERBOSE
>  				dev_dbg(&spi->dev, "read-%d %02x\n",
> @@ -471,8 +481,7 @@ omap2_mcspi_txrx_pio(struct spi_device *spi, struct
> spi_transfer *xfer)
>  				 * more word i/o: switch to rx+tx
>  				 */
>  				if (c == 0 && tx == NULL)
> -					mcspi_write_cs_reg(spi,
> -							OMAP2_MCSPI_CHCONF0, l);
> +					mcspi_write_chconf0(spi, l);
>  				*rx++ = __raw_readl(rx_reg);
>  #ifdef VERBOSE
>  				dev_dbg(&spi->dev, "read-%d %04x\n",
> @@ -510,8 +519,7 @@ omap2_mcspi_txrx_pio(struct spi_device *spi, struct
> spi_transfer *xfer)
>  				 * more word i/o: switch to rx+tx
>  				 */
>  				if (c == 0 && tx == NULL)
> -					mcspi_write_cs_reg(spi,
> -							OMAP2_MCSPI_CHCONF0, l);
> +					mcspi_write_chconf0(spi, l);
>  				*rx++ = __raw_readl(rx_reg);
>  #ifdef VERBOSE
>  				dev_dbg(&spi->dev, "read-%d %04x\n",
> @@ -559,7 +567,7 @@ static int omap2_mcspi_setup_transfer(struct spi_device
> *spi,
>  	} else
>  		div = 15;
>
> -	l = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
> +	l = mcspi_cached_chconf0(spi);
>
>  	/* standard 4-wire master mode:  SCK, MOSI/out, MISO/in, nCS
>  	 * REVISIT: this controller could support SPI_3WIRE mode.
> @@ -591,9 +599,7 @@ static int omap2_mcspi_setup_transfer(struct spi_device
> *spi,
>  	else
>  		l &= ~OMAP2_MCSPI_CHCONF_PHA;
>
> -	mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, l);
> -
> -	omap2_mcspi_ctx[spi_cntrl->bus_num - 1].chconf0 = l;
> +	mcspi_write_chconf0(spi, l);
>
>  	dev_dbg(&spi->dev, "setup: speed %d, sample %s edge, clk %s\n",
>  			OMAP2_MCSPI_MAX_FREQ / (1 << div),
> @@ -697,6 +703,7 @@ static int omap2_mcspi_setup(struct spi_device *spi)
>  			return -ENOMEM;
>  		cs->base = mcspi->base + spi->chip_select * 0x14;
>  		cs->phys = mcspi->phys + spi->chip_select * 0x14;
> +		cs->chconf0 = 0;
>  		spi->controller_state = cs;
>  	}
>
> @@ -791,13 +798,13 @@ static void omap2_mcspi_work(struct work_struct *work)
>  				cs_active = 1;
>  			}
>
> -			chconf = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
> +			chconf = mcspi_cached_chconf0(spi);
>  			chconf &= ~OMAP2_MCSPI_CHCONF_TRM_MASK;
>  			if (t->tx_buf == NULL)
>  				chconf |= OMAP2_MCSPI_CHCONF_TRM_RX_ONLY;
>  			else if (t->rx_buf == NULL)
>  				chconf |= OMAP2_MCSPI_CHCONF_TRM_TX_ONLY;
> -			mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, chconf);
> +			mcspi_write_chconf0(spi, chconf);
>
>  			if (t->len) {
>  				unsigned	count;
> --
> 1.5.4.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH PM 1/2] OMAP McSPI: Fix context save/restore
  2009-02-05 11:51   ` [PATCH PM 1/2] OMAP McSPI: Fix context save/restore Hemanth V
@ 2009-02-05 14:29     ` Kevin Hilman
  2009-02-05 14:39       ` Hemanth V
  0 siblings, 1 reply; 8+ messages in thread
From: Kevin Hilman @ 2009-02-05 14:29 UTC (permalink / raw)
  To: Hemanth V; +Cc: Aaro Koskinen, linux-omap

"Hemanth V" <hemanthv@ti.com> writes:

>> Fix context save/restore to work with chip select. Also update the shadow
>> CHCONF0 register on every write and eliminate register reads.
>
> I believe the shadow CHCONF register need to have only the default
> configuration as initialized by omap2_mcspi_setup_transfer(), since other other
> bits are initialized as and when required by omap2_mcspi_work().
>
> The original problem you saw might be because of not masking
> OMAP2_MCSPI_CHCONF_TRM_MASK and OMAP2_MCSPI_CHCONF_FORCE bits while storing to
> shadow register.

Hemanth,

Can you address this problem and post a single patch with the full
save/restore.

I have reverted the original patch in the pm branch and would like
to see a single patch including the fixes from Jouni and Aaro.

Kevin


>>
>> Signed-off-by: Aaro Koskinen <Aaro.Koskinen@nokia.com>
>> ---
>>  drivers/spi/omap2_mcspi.c |   51 +++++++++++++++++++++++++-------------------
>>  1 files changed, 29 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
>> index a7ee3b7..4d09777 100644
>> --- a/drivers/spi/omap2_mcspi.c
>> +++ b/drivers/spi/omap2_mcspi.c
>> @@ -134,6 +134,7 @@ struct omap2_mcspi_cs {
>>  	void __iomem		*base;
>>  	unsigned long		phys;
>>  	int			word_len;
>> +	u32			chconf0;
>>  };
>>
>>  /* used for context save and restore, structure members to be updated whenever
>> @@ -142,7 +143,6 @@ struct omap2_mcspi_cs {
>>  struct omap2_mcspi_regs {
>>  	u32 sysconfig;
>>  	u32 modulctrl;
>> -	u32 chconf0;
>>  	u32 wakeupenable;
>>  };
>>
>> @@ -187,12 +187,27 @@ static inline u32 mcspi_read_cs_reg(const struct
>> spi_device *spi, int idx)
>>  	return __raw_readl(cs->base + idx);
>>  }
>>
>> +static inline u32 mcspi_cached_chconf0(const struct spi_device *spi)
>> +{
>> +	struct omap2_mcspi_cs *cs = spi->controller_state;
>> +
>> +	return cs->chconf0;
>> +}
>> +
>> +static inline void mcspi_write_chconf0(const struct spi_device *spi, u32 val)
>> +{
>> +	struct omap2_mcspi_cs *cs = spi->controller_state;
>> +
>> +	cs->chconf0 = val;
>> +	mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, val);
>> +}
>> +
>>  static void omap2_mcspi_set_dma_req(const struct spi_device *spi,
>>  		int is_read, int enable)
>>  {
>>  	u32 l, rw;
>>
>> -	l = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
>> +	l = mcspi_cached_chconf0(spi);
>>
>>  	if (is_read) /* 1 is read, 0 write */
>>  		rw = OMAP2_MCSPI_CHCONF_DMAR;
>> @@ -200,7 +215,7 @@ static void omap2_mcspi_set_dma_req(const struct spi_device
>> *spi,
>>  		rw = OMAP2_MCSPI_CHCONF_DMAW;
>>
>>  	MOD_REG_BIT(l, rw, enable);
>> -	mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, l);
>> +	mcspi_write_chconf0(spi, l);
>>  }
>>
>>  static void omap2_mcspi_set_enable(const struct spi_device *spi, int enable)
>> @@ -215,9 +230,9 @@ static void omap2_mcspi_force_cs(struct spi_device *spi,
>> int cs_active)
>>  {
>>  	u32 l;
>>
>> -	l = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
>> +	l = mcspi_cached_chconf0(spi);
>>  	MOD_REG_BIT(l, OMAP2_MCSPI_CHCONF_FORCE, cs_active);
>> -	mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, l);
>> +	mcspi_write_chconf0(spi, l);
>>  }
>>
>>  static void omap2_mcspi_set_master_mode(struct spi_master *master)
>> @@ -248,10 +263,6 @@ static void omap2_mcspi_restore_ctx(struct omap2_mcspi
>> *mcspi)
>>  	mcspi_write_reg(spi_cntrl, OMAP2_MCSPI_SYSCONFIG,
>>  			omap2_mcspi_ctx[spi_cntrl->bus_num - 1].sysconfig);
>>
>> -	mcspi_write_reg(spi_cntrl, OMAP2_MCSPI_CHCONF0,
>> -			omap2_mcspi_ctx[spi_cntrl->bus_num - 1].chconf0);
>> -
>> -
>>  	mcspi_write_reg(spi_cntrl, OMAP2_MCSPI_WAKEUPENABLE,
>>  			omap2_mcspi_ctx[spi_cntrl->bus_num - 1].wakeupenable);
>>  }
>> @@ -392,7 +403,7 @@ omap2_mcspi_txrx_pio(struct spi_device *spi, struct
>> spi_transfer *xfer)
>>  	c = count;
>>  	word_len = cs->word_len;
>>
>> -	l = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
>> +	l = mcspi_cached_chconf0(spi);
>>  	l &= ~OMAP2_MCSPI_CHCONF_TRM_MASK;
>>
>>  	/* We store the pre-calculated register addresses on stack to speed
>> @@ -432,8 +443,7 @@ omap2_mcspi_txrx_pio(struct spi_device *spi, struct
>> spi_transfer *xfer)
>>  				 * more word i/o: switch to rx+tx
>>  				 */
>>  				if (c == 0 && tx == NULL)
>> -					mcspi_write_cs_reg(spi,
>> -							OMAP2_MCSPI_CHCONF0, l);
>> +					mcspi_write_chconf0(spi, l);
>>  				*rx++ = __raw_readl(rx_reg);
>>  #ifdef VERBOSE
>>  				dev_dbg(&spi->dev, "read-%d %02x\n",
>> @@ -471,8 +481,7 @@ omap2_mcspi_txrx_pio(struct spi_device *spi, struct
>> spi_transfer *xfer)
>>  				 * more word i/o: switch to rx+tx
>>  				 */
>>  				if (c == 0 && tx == NULL)
>> -					mcspi_write_cs_reg(spi,
>> -							OMAP2_MCSPI_CHCONF0, l);
>> +					mcspi_write_chconf0(spi, l);
>>  				*rx++ = __raw_readl(rx_reg);
>>  #ifdef VERBOSE
>>  				dev_dbg(&spi->dev, "read-%d %04x\n",
>> @@ -510,8 +519,7 @@ omap2_mcspi_txrx_pio(struct spi_device *spi, struct
>> spi_transfer *xfer)
>>  				 * more word i/o: switch to rx+tx
>>  				 */
>>  				if (c == 0 && tx == NULL)
>> -					mcspi_write_cs_reg(spi,
>> -							OMAP2_MCSPI_CHCONF0, l);
>> +					mcspi_write_chconf0(spi, l);
>>  				*rx++ = __raw_readl(rx_reg);
>>  #ifdef VERBOSE
>>  				dev_dbg(&spi->dev, "read-%d %04x\n",
>> @@ -559,7 +567,7 @@ static int omap2_mcspi_setup_transfer(struct spi_device
>> *spi,
>>  	} else
>>  		div = 15;
>>
>> -	l = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
>> +	l = mcspi_cached_chconf0(spi);
>>
>>  	/* standard 4-wire master mode:  SCK, MOSI/out, MISO/in, nCS
>>  	 * REVISIT: this controller could support SPI_3WIRE mode.
>> @@ -591,9 +599,7 @@ static int omap2_mcspi_setup_transfer(struct spi_device
>> *spi,
>>  	else
>>  		l &= ~OMAP2_MCSPI_CHCONF_PHA;
>>
>> -	mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, l);
>> -
>> -	omap2_mcspi_ctx[spi_cntrl->bus_num - 1].chconf0 = l;
>> +	mcspi_write_chconf0(spi, l);
>>
>>  	dev_dbg(&spi->dev, "setup: speed %d, sample %s edge, clk %s\n",
>>  			OMAP2_MCSPI_MAX_FREQ / (1 << div),
>> @@ -697,6 +703,7 @@ static int omap2_mcspi_setup(struct spi_device *spi)
>>  			return -ENOMEM;
>>  		cs->base = mcspi->base + spi->chip_select * 0x14;
>>  		cs->phys = mcspi->phys + spi->chip_select * 0x14;
>> +		cs->chconf0 = 0;
>>  		spi->controller_state = cs;
>>  	}
>>
>> @@ -791,13 +798,13 @@ static void omap2_mcspi_work(struct work_struct *work)
>>  				cs_active = 1;
>>  			}
>>
>> -			chconf = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
>> +			chconf = mcspi_cached_chconf0(spi);
>>  			chconf &= ~OMAP2_MCSPI_CHCONF_TRM_MASK;
>>  			if (t->tx_buf == NULL)
>>  				chconf |= OMAP2_MCSPI_CHCONF_TRM_RX_ONLY;
>>  			else if (t->rx_buf == NULL)
>>  				chconf |= OMAP2_MCSPI_CHCONF_TRM_TX_ONLY;
>> -			mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, chconf);
>> +			mcspi_write_chconf0(spi, chconf);
>>
>>  			if (t->len) {
>>  				unsigned	count;
>> --
>> 1.5.4.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH PM 1/2] OMAP McSPI: Fix context save/restore
  2009-02-05 14:29     ` Kevin Hilman
@ 2009-02-05 14:39       ` Hemanth V
  2009-02-05 14:50         ` Högander Jouni
  2009-02-05 14:52         ` Kevin Hilman
  0 siblings, 2 replies; 8+ messages in thread
From: Hemanth V @ 2009-02-05 14:39 UTC (permalink / raw)
  To: Kevin Hilman; +Cc: Aaro Koskinen, linux-omap

----- Original Message ----- 
From: "Kevin Hilman" <khilman@deeprootsystems.com>
To: "Hemanth V" <hemanthv@ti.com>
Cc: "Aaro Koskinen" <Aaro.Koskinen@nokia.com>; <linux-omap@vger.kernel.org>
Sent: Thursday, February 05, 2009 7:59 PM
Subject: Re: [PATCH PM 1/2] OMAP McSPI: Fix context save/restore


> "Hemanth V" <hemanthv@ti.com> writes:
>
>>> Fix context save/restore to work with chip select. Also update the 
>>> shadow
>>> CHCONF0 register on every write and eliminate register reads.
>>
>> I believe the shadow CHCONF register need to have only the default
>> configuration as initialized by omap2_mcspi_setup_transfer(), since other 
>> other
>> bits are initialized as and when required by omap2_mcspi_work().
>>
>> The original problem you saw might be because of not masking
>> OMAP2_MCSPI_CHCONF_TRM_MASK and OMAP2_MCSPI_CHCONF_FORCE bits while 
>> storing to
>> shadow register.
>
> Hemanth,
>
> Can you address this problem and post a single patch with the full
> save/restore.
>
> I have reverted the original patch in the pm branch and would like
> to see a single patch including the fixes from Jouni and Aaro.

Kevin,

Both fixes cannot be taken since they are conflicting, I would like
to go with Jouni's fix.

Hemanth

>
> Kevin
>
>
>>>
>>> Signed-off-by: Aaro Koskinen <Aaro.Koskinen@nokia.com>
>>> ---
>>>  drivers/spi/omap2_mcspi.c |   51 
>>> +++++++++++++++++++++++++-------------------
>>>  1 files changed, 29 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
>>> index a7ee3b7..4d09777 100644
>>> --- a/drivers/spi/omap2_mcspi.c
>>> +++ b/drivers/spi/omap2_mcspi.c
>>> @@ -134,6 +134,7 @@ struct omap2_mcspi_cs {
>>>  void __iomem *base;
>>>  unsigned long phys;
>>>  int word_len;
>>> + u32 chconf0;
>>>  };
>>>
>>>  /* used for context save and restore, structure members to be updated 
>>> whenever
>>> @@ -142,7 +143,6 @@ struct omap2_mcspi_cs {
>>>  struct omap2_mcspi_regs {
>>>  u32 sysconfig;
>>>  u32 modulctrl;
>>> - u32 chconf0;
>>>  u32 wakeupenable;
>>>  };
>>>
>>> @@ -187,12 +187,27 @@ static inline u32 mcspi_read_cs_reg(const struct
>>> spi_device *spi, int idx)
>>>  return __raw_readl(cs->base + idx);
>>>  }
>>>
>>> +static inline u32 mcspi_cached_chconf0(const struct spi_device *spi)
>>> +{
>>> + struct omap2_mcspi_cs *cs = spi->controller_state;
>>> +
>>> + return cs->chconf0;
>>> +}
>>> +
>>> +static inline void mcspi_write_chconf0(const struct spi_device *spi, 
>>> u32 val)
>>> +{
>>> + struct omap2_mcspi_cs *cs = spi->controller_state;
>>> +
>>> + cs->chconf0 = val;
>>> + mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, val);
>>> +}
>>> +
>>>  static void omap2_mcspi_set_dma_req(const struct spi_device *spi,
>>>  int is_read, int enable)
>>>  {
>>>  u32 l, rw;
>>>
>>> - l = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
>>> + l = mcspi_cached_chconf0(spi);
>>>
>>>  if (is_read) /* 1 is read, 0 write */
>>>  rw = OMAP2_MCSPI_CHCONF_DMAR;
>>> @@ -200,7 +215,7 @@ static void omap2_mcspi_set_dma_req(const struct 
>>> spi_device
>>> *spi,
>>>  rw = OMAP2_MCSPI_CHCONF_DMAW;
>>>
>>>  MOD_REG_BIT(l, rw, enable);
>>> - mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, l);
>>> + mcspi_write_chconf0(spi, l);
>>>  }
>>>
>>>  static void omap2_mcspi_set_enable(const struct spi_device *spi, int 
>>> enable)
>>> @@ -215,9 +230,9 @@ static void omap2_mcspi_force_cs(struct spi_device 
>>> *spi,
>>> int cs_active)
>>>  {
>>>  u32 l;
>>>
>>> - l = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
>>> + l = mcspi_cached_chconf0(spi);
>>>  MOD_REG_BIT(l, OMAP2_MCSPI_CHCONF_FORCE, cs_active);
>>> - mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, l);
>>> + mcspi_write_chconf0(spi, l);
>>>  }
>>>
>>>  static void omap2_mcspi_set_master_mode(struct spi_master *master)
>>> @@ -248,10 +263,6 @@ static void omap2_mcspi_restore_ctx(struct 
>>> omap2_mcspi
>>> *mcspi)
>>>  mcspi_write_reg(spi_cntrl, OMAP2_MCSPI_SYSCONFIG,
>>>  omap2_mcspi_ctx[spi_cntrl->bus_num - 1].sysconfig);
>>>
>>> - mcspi_write_reg(spi_cntrl, OMAP2_MCSPI_CHCONF0,
>>> - omap2_mcspi_ctx[spi_cntrl->bus_num - 1].chconf0);
>>> -
>>> -
>>>  mcspi_write_reg(spi_cntrl, OMAP2_MCSPI_WAKEUPENABLE,
>>>  omap2_mcspi_ctx[spi_cntrl->bus_num - 1].wakeupenable);
>>>  }
>>> @@ -392,7 +403,7 @@ omap2_mcspi_txrx_pio(struct spi_device *spi, struct
>>> spi_transfer *xfer)
>>>  c = count;
>>>  word_len = cs->word_len;
>>>
>>> - l = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
>>> + l = mcspi_cached_chconf0(spi);
>>>  l &= ~OMAP2_MCSPI_CHCONF_TRM_MASK;
>>>
>>>  /* We store the pre-calculated register addresses on stack to speed
>>> @@ -432,8 +443,7 @@ omap2_mcspi_txrx_pio(struct spi_device *spi, struct
>>> spi_transfer *xfer)
>>>  * more word i/o: switch to rx+tx
>>>  */
>>>  if (c == 0 && tx == NULL)
>>> - mcspi_write_cs_reg(spi,
>>> - OMAP2_MCSPI_CHCONF0, l);
>>> + mcspi_write_chconf0(spi, l);
>>>  *rx++ = __raw_readl(rx_reg);
>>>  #ifdef VERBOSE
>>>  dev_dbg(&spi->dev, "read-%d %02x\n",
>>> @@ -471,8 +481,7 @@ omap2_mcspi_txrx_pio(struct spi_device *spi, struct
>>> spi_transfer *xfer)
>>>  * more word i/o: switch to rx+tx
>>>  */
>>>  if (c == 0 && tx == NULL)
>>> - mcspi_write_cs_reg(spi,
>>> - OMAP2_MCSPI_CHCONF0, l);
>>> + mcspi_write_chconf0(spi, l);
>>>  *rx++ = __raw_readl(rx_reg);
>>>  #ifdef VERBOSE
>>>  dev_dbg(&spi->dev, "read-%d %04x\n",
>>> @@ -510,8 +519,7 @@ omap2_mcspi_txrx_pio(struct spi_device *spi, struct
>>> spi_transfer *xfer)
>>>  * more word i/o: switch to rx+tx
>>>  */
>>>  if (c == 0 && tx == NULL)
>>> - mcspi_write_cs_reg(spi,
>>> - OMAP2_MCSPI_CHCONF0, l);
>>> + mcspi_write_chconf0(spi, l);
>>>  *rx++ = __raw_readl(rx_reg);
>>>  #ifdef VERBOSE
>>>  dev_dbg(&spi->dev, "read-%d %04x\n",
>>> @@ -559,7 +567,7 @@ static int omap2_mcspi_setup_transfer(struct 
>>> spi_device
>>> *spi,
>>>  } else
>>>  div = 15;
>>>
>>> - l = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
>>> + l = mcspi_cached_chconf0(spi);
>>>
>>>  /* standard 4-wire master mode:  SCK, MOSI/out, MISO/in, nCS
>>>  * REVISIT: this controller could support SPI_3WIRE mode.
>>> @@ -591,9 +599,7 @@ static int omap2_mcspi_setup_transfer(struct 
>>> spi_device
>>> *spi,
>>>  else
>>>  l &= ~OMAP2_MCSPI_CHCONF_PHA;
>>>
>>> - mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, l);
>>> -
>>> - omap2_mcspi_ctx[spi_cntrl->bus_num - 1].chconf0 = l;
>>> + mcspi_write_chconf0(spi, l);
>>>
>>>  dev_dbg(&spi->dev, "setup: speed %d, sample %s edge, clk %s\n",
>>>  OMAP2_MCSPI_MAX_FREQ / (1 << div),
>>> @@ -697,6 +703,7 @@ static int omap2_mcspi_setup(struct spi_device *spi)
>>>  return -ENOMEM;
>>>  cs->base = mcspi->base + spi->chip_select * 0x14;
>>>  cs->phys = mcspi->phys + spi->chip_select * 0x14;
>>> + cs->chconf0 = 0;
>>>  spi->controller_state = cs;
>>>  }
>>>
>>> @@ -791,13 +798,13 @@ static void omap2_mcspi_work(struct work_struct 
>>> *work)
>>>  cs_active = 1;
>>>  }
>>>
>>> - chconf = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
>>> + chconf = mcspi_cached_chconf0(spi);
>>>  chconf &= ~OMAP2_MCSPI_CHCONF_TRM_MASK;
>>>  if (t->tx_buf == NULL)
>>>  chconf |= OMAP2_MCSPI_CHCONF_TRM_RX_ONLY;
>>>  else if (t->rx_buf == NULL)
>>>  chconf |= OMAP2_MCSPI_CHCONF_TRM_TX_ONLY;
>>> - mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, chconf);
>>> + mcspi_write_chconf0(spi, chconf);
>>>
>>>  if (t->len) {
>>>  unsigned count;
>>> --
>>> 1.5.4.3
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>>
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH PM 1/2] OMAP McSPI: Fix context save/restore
  2009-02-05 14:39       ` Hemanth V
@ 2009-02-05 14:50         ` Högander Jouni
  2009-02-05 14:52         ` Kevin Hilman
  1 sibling, 0 replies; 8+ messages in thread
From: Högander Jouni @ 2009-02-05 14:50 UTC (permalink / raw)
  To: ext Hemanth V
  Cc: Kevin Hilman, Koskinen Aaro (Nokia-D/Helsinki),
	linux-omap@vger.kernel.org

ext Hemanth V <hemanthv@ti.com> writes:

> ----- Original Message -----
> From: "Kevin Hilman" <khilman@deeprootsystems.com>
> To: "Hemanth V" <hemanthv@ti.com>
> Cc: "Aaro Koskinen" <Aaro.Koskinen@nokia.com>; <linux-omap@vger.kernel.org>
> Sent: Thursday, February 05, 2009 7:59 PM
> Subject: Re: [PATCH PM 1/2] OMAP McSPI: Fix context save/restore
>
>
>> "Hemanth V" <hemanthv@ti.com> writes:
>>
>>>> Fix context save/restore to work with chip select. Also update the
>>>> shadow
>>>> CHCONF0 register on every write and eliminate register reads.
>>>
>>> I believe the shadow CHCONF register need to have only the default
>>> configuration as initialized by omap2_mcspi_setup_transfer(), since other
>>> other
>>> bits are initialized as and when required by omap2_mcspi_work().
>>>
>>> The original problem you saw might be because of not masking
>>> OMAP2_MCSPI_CHCONF_TRM_MASK and OMAP2_MCSPI_CHCONF_FORCE bits while
>>> storing to
>>> shadow register.
>>
>> Hemanth,
>>
>> Can you address this problem and post a single patch with the full
>> save/restore.
>>
>> I have reverted the original patch in the pm branch and would like
>> to see a single patch including the fixes from Jouni and Aaro.
>
> Kevin,
>
> Both fixes cannot be taken since they are conflicting, I would like
> to go with Jouni's fix.

Bad thing in that one is that it doesn't work:)

Kevin, to my understanding patches from Aaro Koskinen are working. If
they are ok to you, please push them. So the patches are:

[PATCH PM 0/2] OMAP McSPI: Fixes to save/restore
[PATCH PM 1/2] OMAP McSPI: Fix context save/restore
[PATCH PM 2/2] OMAP McSPI: spin_unlock_irq() missing

and the sender is Koskinen Aaro. Those patches are working alone.

-- 
Jouni Högander

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH PM 1/2] OMAP McSPI: Fix context save/restore
  2009-02-05 14:39       ` Hemanth V
  2009-02-05 14:50         ` Högander Jouni
@ 2009-02-05 14:52         ` Kevin Hilman
  1 sibling, 0 replies; 8+ messages in thread
From: Kevin Hilman @ 2009-02-05 14:52 UTC (permalink / raw)
  To: Hemanth V; +Cc: Aaro Koskinen, linux-omap

"Hemanth V" <hemanthv@ti.com> writes:

> ----- Original Message ----- 
> From: "Kevin Hilman" <khilman@deeprootsystems.com>
> To: "Hemanth V" <hemanthv@ti.com>
> Cc: "Aaro Koskinen" <Aaro.Koskinen@nokia.com>; <linux-omap@vger.kernel.org>
> Sent: Thursday, February 05, 2009 7:59 PM
> Subject: Re: [PATCH PM 1/2] OMAP McSPI: Fix context save/restore
>
>
>> "Hemanth V" <hemanthv@ti.com> writes:
>>
>>>> Fix context save/restore to work with chip select. Also update the
>>>> shadow
>>>> CHCONF0 register on every write and eliminate register reads.
>>>
>>> I believe the shadow CHCONF register need to have only the default
>>> configuration as initialized by omap2_mcspi_setup_transfer(), since
>>> other other
>>> bits are initialized as and when required by omap2_mcspi_work().
>>>
>>> The original problem you saw might be because of not masking
>>> OMAP2_MCSPI_CHCONF_TRM_MASK and OMAP2_MCSPI_CHCONF_FORCE bits while
>>> storing to
>>> shadow register.
>>
>> Hemanth,
>>
>> Can you address this problem and post a single patch with the full
>> save/restore.
>>
>> I have reverted the original patch in the pm branch and would like
>> to see a single patch including the fixes from Jouni and Aaro.
>
> Kevin,
>
> Both fixes cannot be taken since they are conflicting, I would like
> to go with Jouni's fix.
>

Actually, I'd prefer to see Aaro's fix as it goes a little further to
optimize the save/restore path.  He also fixed a locking bug.

Kevin

>
>>
>> Kevin
>>
>>
>>>>
>>>> Signed-off-by: Aaro Koskinen <Aaro.Koskinen@nokia.com>
>>>> ---
>>>>  drivers/spi/omap2_mcspi.c |   51
>>>> +++++++++++++++++++++++++-------------------
>>>>  1 files changed, 29 insertions(+), 22 deletions(-)
>>>>
>>>> diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
>>>> index a7ee3b7..4d09777 100644
>>>> --- a/drivers/spi/omap2_mcspi.c
>>>> +++ b/drivers/spi/omap2_mcspi.c
>>>> @@ -134,6 +134,7 @@ struct omap2_mcspi_cs {
>>>>  void __iomem *base;
>>>>  unsigned long phys;
>>>>  int word_len;
>>>> + u32 chconf0;
>>>>  };
>>>>
>>>>  /* used for context save and restore, structure members to be
>>>> updated whenever
>>>> @@ -142,7 +143,6 @@ struct omap2_mcspi_cs {
>>>>  struct omap2_mcspi_regs {
>>>>  u32 sysconfig;
>>>>  u32 modulctrl;
>>>> - u32 chconf0;
>>>>  u32 wakeupenable;
>>>>  };
>>>>
>>>> @@ -187,12 +187,27 @@ static inline u32 mcspi_read_cs_reg(const struct
>>>> spi_device *spi, int idx)
>>>>  return __raw_readl(cs->base + idx);
>>>>  }
>>>>
>>>> +static inline u32 mcspi_cached_chconf0(const struct spi_device *spi)
>>>> +{
>>>> + struct omap2_mcspi_cs *cs = spi->controller_state;
>>>> +
>>>> + return cs->chconf0;
>>>> +}
>>>> +
>>>> +static inline void mcspi_write_chconf0(const struct spi_device
>>>> *spi, u32 val)
>>>> +{
>>>> + struct omap2_mcspi_cs *cs = spi->controller_state;
>>>> +
>>>> + cs->chconf0 = val;
>>>> + mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, val);
>>>> +}
>>>> +
>>>>  static void omap2_mcspi_set_dma_req(const struct spi_device *spi,
>>>>  int is_read, int enable)
>>>>  {
>>>>  u32 l, rw;
>>>>
>>>> - l = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
>>>> + l = mcspi_cached_chconf0(spi);
>>>>
>>>>  if (is_read) /* 1 is read, 0 write */
>>>>  rw = OMAP2_MCSPI_CHCONF_DMAR;
>>>> @@ -200,7 +215,7 @@ static void omap2_mcspi_set_dma_req(const
>>>> struct spi_device
>>>> *spi,
>>>>  rw = OMAP2_MCSPI_CHCONF_DMAW;
>>>>
>>>>  MOD_REG_BIT(l, rw, enable);
>>>> - mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, l);
>>>> + mcspi_write_chconf0(spi, l);
>>>>  }
>>>>
>>>>  static void omap2_mcspi_set_enable(const struct spi_device *spi,
>>>> int enable)
>>>> @@ -215,9 +230,9 @@ static void omap2_mcspi_force_cs(struct
>>>> spi_device *spi,
>>>> int cs_active)
>>>>  {
>>>>  u32 l;
>>>>
>>>> - l = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
>>>> + l = mcspi_cached_chconf0(spi);
>>>>  MOD_REG_BIT(l, OMAP2_MCSPI_CHCONF_FORCE, cs_active);
>>>> - mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, l);
>>>> + mcspi_write_chconf0(spi, l);
>>>>  }
>>>>
>>>>  static void omap2_mcspi_set_master_mode(struct spi_master *master)
>>>> @@ -248,10 +263,6 @@ static void omap2_mcspi_restore_ctx(struct
>>>> omap2_mcspi
>>>> *mcspi)
>>>>  mcspi_write_reg(spi_cntrl, OMAP2_MCSPI_SYSCONFIG,
>>>>  omap2_mcspi_ctx[spi_cntrl->bus_num - 1].sysconfig);
>>>>
>>>> - mcspi_write_reg(spi_cntrl, OMAP2_MCSPI_CHCONF0,
>>>> - omap2_mcspi_ctx[spi_cntrl->bus_num - 1].chconf0);
>>>> -
>>>> -
>>>>  mcspi_write_reg(spi_cntrl, OMAP2_MCSPI_WAKEUPENABLE,
>>>>  omap2_mcspi_ctx[spi_cntrl->bus_num - 1].wakeupenable);
>>>>  }
>>>> @@ -392,7 +403,7 @@ omap2_mcspi_txrx_pio(struct spi_device *spi, struct
>>>> spi_transfer *xfer)
>>>>  c = count;
>>>>  word_len = cs->word_len;
>>>>
>>>> - l = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
>>>> + l = mcspi_cached_chconf0(spi);
>>>>  l &= ~OMAP2_MCSPI_CHCONF_TRM_MASK;
>>>>
>>>>  /* We store the pre-calculated register addresses on stack to speed
>>>> @@ -432,8 +443,7 @@ omap2_mcspi_txrx_pio(struct spi_device *spi, struct
>>>> spi_transfer *xfer)
>>>>  * more word i/o: switch to rx+tx
>>>>  */
>>>>  if (c == 0 && tx == NULL)
>>>> - mcspi_write_cs_reg(spi,
>>>> - OMAP2_MCSPI_CHCONF0, l);
>>>> + mcspi_write_chconf0(spi, l);
>>>>  *rx++ = __raw_readl(rx_reg);
>>>>  #ifdef VERBOSE
>>>>  dev_dbg(&spi->dev, "read-%d %02x\n",
>>>> @@ -471,8 +481,7 @@ omap2_mcspi_txrx_pio(struct spi_device *spi, struct
>>>> spi_transfer *xfer)
>>>>  * more word i/o: switch to rx+tx
>>>>  */
>>>>  if (c == 0 && tx == NULL)
>>>> - mcspi_write_cs_reg(spi,
>>>> - OMAP2_MCSPI_CHCONF0, l);
>>>> + mcspi_write_chconf0(spi, l);
>>>>  *rx++ = __raw_readl(rx_reg);
>>>>  #ifdef VERBOSE
>>>>  dev_dbg(&spi->dev, "read-%d %04x\n",
>>>> @@ -510,8 +519,7 @@ omap2_mcspi_txrx_pio(struct spi_device *spi, struct
>>>> spi_transfer *xfer)
>>>>  * more word i/o: switch to rx+tx
>>>>  */
>>>>  if (c == 0 && tx == NULL)
>>>> - mcspi_write_cs_reg(spi,
>>>> - OMAP2_MCSPI_CHCONF0, l);
>>>> + mcspi_write_chconf0(spi, l);
>>>>  *rx++ = __raw_readl(rx_reg);
>>>>  #ifdef VERBOSE
>>>>  dev_dbg(&spi->dev, "read-%d %04x\n",
>>>> @@ -559,7 +567,7 @@ static int omap2_mcspi_setup_transfer(struct
>>>> spi_device
>>>> *spi,
>>>>  } else
>>>>  div = 15;
>>>>
>>>> - l = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
>>>> + l = mcspi_cached_chconf0(spi);
>>>>
>>>>  /* standard 4-wire master mode:  SCK, MOSI/out, MISO/in, nCS
>>>>  * REVISIT: this controller could support SPI_3WIRE mode.
>>>> @@ -591,9 +599,7 @@ static int omap2_mcspi_setup_transfer(struct
>>>> spi_device
>>>> *spi,
>>>>  else
>>>>  l &= ~OMAP2_MCSPI_CHCONF_PHA;
>>>>
>>>> - mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, l);
>>>> -
>>>> - omap2_mcspi_ctx[spi_cntrl->bus_num - 1].chconf0 = l;
>>>> + mcspi_write_chconf0(spi, l);
>>>>
>>>>  dev_dbg(&spi->dev, "setup: speed %d, sample %s edge, clk %s\n",
>>>>  OMAP2_MCSPI_MAX_FREQ / (1 << div),
>>>> @@ -697,6 +703,7 @@ static int omap2_mcspi_setup(struct spi_device *spi)
>>>>  return -ENOMEM;
>>>>  cs->base = mcspi->base + spi->chip_select * 0x14;
>>>>  cs->phys = mcspi->phys + spi->chip_select * 0x14;
>>>> + cs->chconf0 = 0;
>>>>  spi->controller_state = cs;
>>>>  }
>>>>
>>>> @@ -791,13 +798,13 @@ static void omap2_mcspi_work(struct
>>>> work_struct *work)
>>>>  cs_active = 1;
>>>>  }
>>>>
>>>> - chconf = mcspi_read_cs_reg(spi, OMAP2_MCSPI_CHCONF0);
>>>> + chconf = mcspi_cached_chconf0(spi);
>>>>  chconf &= ~OMAP2_MCSPI_CHCONF_TRM_MASK;
>>>>  if (t->tx_buf == NULL)
>>>>  chconf |= OMAP2_MCSPI_CHCONF_TRM_RX_ONLY;
>>>>  else if (t->rx_buf == NULL)
>>>>  chconf |= OMAP2_MCSPI_CHCONF_TRM_TX_ONLY;
>>>> - mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, chconf);
>>>> + mcspi_write_chconf0(spi, chconf);
>>>>
>>>>  if (t->len) {
>>>>  unsigned count;
>>>> --
>>>> 1.5.4.3
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>
>>>>
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2009-02-05 14:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-02-05 11:05 [PATCH PM 0/2] OMAP McSPI: Fixes to save/restore Aaro Koskinen
2009-02-05 11:07 ` [PATCH PM 1/2] OMAP McSPI: Fix context save/restore Aaro Koskinen
2009-02-05 11:07   ` [PATCH PM 2/2] OMAP McSPI: spin_unlock_irq() missing Aaro Koskinen
2009-02-05 11:51   ` [PATCH PM 1/2] OMAP McSPI: Fix context save/restore Hemanth V
2009-02-05 14:29     ` Kevin Hilman
2009-02-05 14:39       ` Hemanth V
2009-02-05 14:50         ` Högander Jouni
2009-02-05 14:52         ` Kevin Hilman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox