Re: [PATCH PM 1/2] OMAP McSPI: Fix context save/restore

[Kevin Hilman <khilman@deeprootsystems.com>]

"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