"Hemanth V" <hemanthv@xxxxxx> writes: > From: Hemanth V <hemanthv@xxxxxx> > > This patch adds context save/restore feature to McSPI driver. This has been > tested by instrumenting the driver code i.e by adding a McSPI softreset > in omap2_mcspi_disable_clocks function. > > This patch includes review comment fixes > > Signed-off-by: Hemanth V <hemanthv@xxxxxx> Thanks, this looks much better. Could you add a comment/description just before the definition of omap2_mcspi_regs which describes how the save/restore works, and that any modifying of these registers must also update the shadow regs. Also, one very minor comment cleanup below. After this, I'll apply it to the pm branch. Thanks, Kevin > --- > drivers/spi/omap2_mcspi.c | 94 ++++++++++++++++++++++++++++++++++++---------- > 1 files changed, 74 insertions(+), 20 deletions(-) > > Index: linux-omap-2.6/drivers/spi/omap2_mcspi.c > =================================================================== > --- linux-omap-2.6.orig/drivers/spi/omap2_mcspi.c > +++ linux-omap-2.6/drivers/spi/omap2_mcspi.c > @@ -41,6 +41,9 @@ > > #define OMAP2_MCSPI_MAX_FREQ 48000000 > > +/* OMAP2 has 3 SPI controllers, while OMAP3 has 4 */ > +#define OMAP2_MCSPI_MAX_CTRL 4 > + > #define OMAP2_MCSPI_REVISION 0x00 > #define OMAP2_MCSPI_SYSCONFIG 0x10 > #define OMAP2_MCSPI_SYSSTATUS 0x14 > @@ -133,6 +136,15 @@ > int word_len; > }; > > +struct omap2_mcspi_regs { > + u32 sysconfig; > + u32 modulctrl; > + u32 chconf0; > + u32 wakeupenable; > +}; > + > +static struct omap2_mcspi_regs omap2_mcspi_ctx[OMAP2_MCSPI_MAX_CTRL]; > + > static struct workqueue_struct *omap2_mcspi_wq; > > #define MOD_REG_BIT(val, mask, set) do { \ > @@ -217,6 +229,45 @@ > MOD_REG_BIT(l, OMAP2_MCSPI_MODULCTRL_MS, 0); > MOD_REG_BIT(l, OMAP2_MCSPI_MODULCTRL_SINGLE, 1); > mcspi_write_reg(master, OMAP2_MCSPI_MODULCTRL, l); > + > + omap2_mcspi_ctx[master->bus_num - 1].modulctrl = l; > +} > + > +static void omap2_mcspi_restore_ctx(struct omap2_mcspi *mcspi) > +{ > + struct spi_master *spi_cntrl; > + spi_cntrl = mcspi->master; > + > + /*McSPI : context restore */ add space after /* > + mcspi_write_reg(spi_cntrl, OMAP2_MCSPI_MODULCTRL, > + omap2_mcspi_ctx[spi_cntrl->bus_num - 1].modulctrl); > + > + 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); > +} > +static void omap2_mcspi_disable_clocks(struct omap2_mcspi *mcspi) > +{ > + clk_disable(mcspi->ick); > + clk_disable(mcspi->fck); > +} > + > +static int omap2_mcspi_enable_clocks(struct omap2_mcspi *mcspi) > +{ > + if (clk_enable(mcspi->ick)) > + return -ENODEV; > + if (clk_enable(mcspi->fck)) > + return -ENODEV; > + > + omap2_mcspi_restore_ctx(mcspi); > + > + return 0; > } > > static unsigned > @@ -486,10 +537,12 @@ > { > struct omap2_mcspi_cs *cs = spi->controller_state; > struct omap2_mcspi *mcspi; > + struct spi_master *spi_cntrl; > u32 l = 0, div = 0; > u8 word_len = spi->bits_per_word; > > mcspi = spi_master_get_devdata(spi->master); > + spi_cntrl = mcspi->master; > > if (t != NULL && t->bits_per_word) > word_len = t->bits_per_word; > @@ -537,6 +590,8 @@ > > mcspi_write_cs_reg(spi, OMAP2_MCSPI_CHCONF0, l); > > + omap2_mcspi_ctx[spi_cntrl->bus_num - 1].chconf0 = l; > + > dev_dbg(&spi->dev, "setup: speed %d, sample %s edge, clk %s\n", > OMAP2_MCSPI_MAX_FREQ / (1 << div), > (spi->mode & SPI_CPHA) ? "trailing" : "leading", > @@ -649,11 +704,11 @@ > return ret; > } > > - clk_enable(mcspi->ick); > - clk_enable(mcspi->fck); > + if (omap2_mcspi_enable_clocks(mcspi)) > + return -ENODEV; > + > ret = omap2_mcspi_setup_transfer(spi, NULL); > - clk_disable(mcspi->fck); > - clk_disable(mcspi->ick); > + omap2_mcspi_disable_clocks(mcspi); > > return ret; > } > @@ -685,8 +740,8 @@ > mcspi = container_of(work, struct omap2_mcspi, work); > spin_lock_irq(&mcspi->lock); > > - clk_enable(mcspi->ick); > - clk_enable(mcspi->fck); > + if (omap2_mcspi_enable_clocks(mcspi)) > + return; > > /* We only enable one channel at a time -- the one whose message is > * at the head of the queue -- although this controller would gladly > @@ -788,8 +843,7 @@ > spin_lock_irq(&mcspi->lock); > } > > - clk_disable(mcspi->fck); > - clk_disable(mcspi->ick); > + omap2_mcspi_disable_clocks(mcspi); > > spin_unlock_irq(&mcspi->lock); > } > @@ -877,8 +931,8 @@ > struct spi_master *master = mcspi->master; > u32 tmp; > > - clk_enable(mcspi->ick); > - clk_enable(mcspi->fck); > + if (omap2_mcspi_enable_clocks(mcspi)) > + return -1; > > mcspi_write_reg(master, OMAP2_MCSPI_SYSCONFIG, > OMAP2_MCSPI_SYSCONFIG_SOFTRESET); > @@ -886,18 +940,18 @@ > tmp = mcspi_read_reg(master, OMAP2_MCSPI_SYSSTATUS); > } while (!(tmp & OMAP2_MCSPI_SYSSTATUS_RESETDONE)); > > - mcspi_write_reg(master, OMAP2_MCSPI_SYSCONFIG, > - OMAP2_MCSPI_SYSCONFIG_AUTOIDLE | > - OMAP2_MCSPI_SYSCONFIG_ENAWAKEUP | > - OMAP2_MCSPI_SYSCONFIG_SMARTIDLE); > - > - mcspi_write_reg(master, OMAP2_MCSPI_WAKEUPENABLE, > - OMAP2_MCSPI_WAKEUPENABLE_WKEN); > + tmp = OMAP2_MCSPI_SYSCONFIG_AUTOIDLE | > + OMAP2_MCSPI_SYSCONFIG_ENAWAKEUP | > + OMAP2_MCSPI_SYSCONFIG_SMARTIDLE; > + mcspi_write_reg(master, OMAP2_MCSPI_SYSCONFIG, tmp); > + omap2_mcspi_ctx[master->bus_num - 1].sysconfig = tmp; > + > + tmp = OMAP2_MCSPI_WAKEUPENABLE_WKEN; > + mcspi_write_reg(master, OMAP2_MCSPI_WAKEUPENABLE, tmp); > + omap2_mcspi_ctx[master->bus_num - 1].wakeupenable = tmp; > > omap2_mcspi_set_master_mode(master); > - > - clk_disable(mcspi->fck); > - clk_disable(mcspi->ick); > + omap2_mcspi_disable_clocks(mcspi); > return 0; > } -- 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