All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Hilman <khilman@deeprootsystems.com>
To: Hemanth V <hemanthv@ti.com>
Cc: Aaro Koskinen <Aaro.Koskinen@nokia.com>, linux-omap@vger.kernel.org
Subject: Re: [PATCH PM 1/2] OMAP McSPI: Fix context save/restore
Date: Thu, 05 Feb 2009 06:29:05 -0800	[thread overview]
Message-ID: <87ocxh6jpq.fsf@deeprootsystems.com> (raw)
In-Reply-To: <56834.10.24.255.18.1233834716.squirrel@dbdmail.itg.ti.com> (Hemanth V.'s message of "Thu\, 5 Feb 2009 17\:21\:56 +0530 \(IST\)")

"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

  reply	other threads:[~2009-02-05 14:29 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2009-02-05 14:39       ` Hemanth V
2009-02-05 14:50         ` Högander Jouni
2009-02-05 14:52         ` Kevin Hilman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ocxh6jpq.fsf@deeprootsystems.com \
    --to=khilman@deeprootsystems.com \
    --cc=Aaro.Koskinen@nokia.com \
    --cc=hemanthv@ti.com \
    --cc=linux-omap@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.