"Hemanth V" <hemanthv@xxxxxx> writes:
> ----- Original Message -----
> From: "Kevin Hilman" <khilman@xxxxxxxxxxxxxxxxxxx>
> To: "Hemanth V" <hemanthv@xxxxxx>
> Cc: "Aaro Koskinen" <Aaro.Koskinen@xxxxxxxxx>; <linux-omap@xxxxxxxxxxxxxxx>
> Sent: Thursday, February 05, 2009 7:59 PM
> Subject: Re: [PATCH PM 1/2] OMAP McSPI: Fix context save/restore
>
>
>> "Hemanth V" <hemanthv@xxxxxx> 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@xxxxxxxxx>
>>>> ---
>>>> 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@xxxxxxxxxxxxxxx
>>>> 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@xxxxxxxxxxxxxxx
>>> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
