Patchwork [7/8] spi_mpc8xxx: Turn qe_mode into flags

login
register
mail settings
Submitter Anton Vorontsov
Date Oct. 12, 2009, 4:49 p.m.
Message ID <20091012164925.GG4578@oksana.dev.rtsoft.ru>
Download mbox | patch
Permalink /patch/35807/
State Accepted, archived
Delegated to: Kumar Gala
Headers show

Comments

Anton Vorontsov - Oct. 12, 2009, 4:49 p.m.
Soon there will be more flags introduced in subsequent patches, so
let's turn qe_mode into flags.

Also introduce mpc8xxx_spi_strmode() and print current SPI mode.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Acked-by: David Brownell <dbrownell@users.sourceforge.net>
---
 drivers/spi/spi_mpc8xxx.c   |   30 +++++++++++++++++++-----------
 include/linux/fsl_devices.h |    2 +-
 2 files changed, 20 insertions(+), 12 deletions(-)
Kumar Gala - Nov. 5, 2009, 1:47 p.m.
On Oct 12, 2009, at 11:49 AM, Anton Vorontsov wrote:

> Soon there will be more flags introduced in subsequent patches, so
> let's turn qe_mode into flags.
>
> Also introduce mpc8xxx_spi_strmode() and print current SPI mode.
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> Acked-by: David Brownell <dbrownell@users.sourceforge.net>
> ---
> drivers/spi/spi_mpc8xxx.c   |   30 +++++++++++++++++++-----------
> include/linux/fsl_devices.h |    2 +-
> 2 files changed, 20 insertions(+), 12 deletions(-)

applied to next

- k
Benjamin Herrenschmidt - Nov. 24, 2009, 5:03 a.m.
On Thu, 2009-11-05 at 07:47 -0600, Kumar Gala wrote:
> On Oct 12, 2009, at 11:49 AM, Anton Vorontsov wrote:
> 
> > Soon there will be more flags introduced in subsequent patches, so
> > let's turn qe_mode into flags.
> >
> > Also introduce mpc8xxx_spi_strmode() and print current SPI mode.
> >
> > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> > Acked-by: David Brownell <dbrownell@users.sourceforge.net>
> > ---
> > drivers/spi/spi_mpc8xxx.c   |   30 +++++++++++++++++++-----------
> > include/linux/fsl_devices.h |    2 +-
> > 2 files changed, 20 insertions(+), 12 deletions(-)
> 
> applied to next

This patch breaks my 6xx config:

/home/benh/linux-powerpc-test/arch/powerpc/platforms/83xx/mpc832x_rdb.c: In function ‘of_fsl_spi_probe’:
/home/benh/linux-powerpc-test/arch/powerpc/platforms/83xx/mpc832x_rdb.c:77: error: ‘struct fsl_spi_platform_data’ has no member named ‘qe_

The reason is that the mpc832x_rdb.c code still uses the legacy probing
method. The fix is not totally trivial as the new flags are defined inside
spi_mpc8xxx.c so either the flags need to be moved to fsl_devices.h or
mpc832x_rdb.c needs to be converted to new style stuff.

I'll commit (will be up later today in my next branch) a fix moving the
flags over for now so the tree doesn't break building.

If you are going to keep the flags in the .c file you probably also want
to remove the platform device definition from fsl_devices.h anyways as
there's no point exposing to the world a structure with a "flags" member
if the definition of those flags isn't also exposed.

Cheers,
Ben.
Anton Vorontsov - Nov. 24, 2009, 3:41 p.m.
On Tue, Nov 24, 2009 at 04:03:36PM +1100, Benjamin Herrenschmidt wrote:
[...]
> This patch breaks my 6xx config:
> 
> /home/benh/linux-powerpc-test/arch/powerpc/platforms/83xx/mpc832x_rdb.c: In function ‘of_fsl_spi_probe’:
> /home/benh/linux-powerpc-test/arch/powerpc/platforms/83xx/mpc832x_rdb.c:77: error: ‘struct fsl_spi_platform_data’ has no member named ‘qe_
> 
> The reason is that the mpc832x_rdb.c code still uses the legacy probing
> method. The fix is not totally trivial as the new flags are defined inside
> spi_mpc8xxx.c

Thanks for noticing. I indeed forgot about the legacy stuff. :-/

> If you are going to keep the flags in the .c file you probably also want
> to remove the platform device definition from fsl_devices.h anyways as
> there's no point exposing to the world a structure with a "flags" member
> if the definition of those flags isn't also exposed.

Yep. Though, IIRC Joakim asked to keep the legacy bindings for some
time (until we implement SPI chip-select framework?)... so for now
we'll have to leave we the exposed flags.

Thanks again,

Patch

diff --git a/drivers/spi/spi_mpc8xxx.c b/drivers/spi/spi_mpc8xxx.c
index 4b119ea..80374df 100644
--- a/drivers/spi/spi_mpc8xxx.c
+++ b/drivers/spi/spi_mpc8xxx.c
@@ -96,7 +96,8 @@  struct mpc8xxx_spi {
 	u32 rx_shift;		/* RX data reg shift when in qe mode */
 	u32 tx_shift;		/* TX data reg shift when in qe mode */
 
-	bool qe_mode;
+	unsigned int flags;
+#define SPI_QE_CPU_MODE		(1 << 0) /* QE CPU ("PIO") mode */
 
 	struct workqueue_struct *workqueue;
 	struct work_struct work;
@@ -235,14 +236,14 @@  int mpc8xxx_spi_setup_transfer(struct spi_device *spi, struct spi_transfer *t)
 	if (bits_per_word <= 8) {
 		cs->get_rx = mpc8xxx_spi_rx_buf_u8;
 		cs->get_tx = mpc8xxx_spi_tx_buf_u8;
-		if (mpc8xxx_spi->qe_mode) {
+		if (mpc8xxx_spi->flags & SPI_QE_CPU_MODE) {
 			cs->rx_shift = 16;
 			cs->tx_shift = 24;
 		}
 	} else if (bits_per_word <= 16) {
 		cs->get_rx = mpc8xxx_spi_rx_buf_u16;
 		cs->get_tx = mpc8xxx_spi_tx_buf_u16;
-		if (mpc8xxx_spi->qe_mode) {
+		if (mpc8xxx_spi->flags & SPI_QE_CPU_MODE) {
 			cs->rx_shift = 16;
 			cs->tx_shift = 16;
 		}
@@ -252,7 +253,8 @@  int mpc8xxx_spi_setup_transfer(struct spi_device *spi, struct spi_transfer *t)
 	} else
 		return -EINVAL;
 
-	if (mpc8xxx_spi->qe_mode && spi->mode & SPI_LSB_FIRST) {
+	if (mpc8xxx_spi->flags & SPI_QE_CPU_MODE &&
+			spi->mode & SPI_LSB_FIRST) {
 		cs->tx_shift = 0;
 		if (bits_per_word <= 8)
 			cs->rx_shift = 8;
@@ -518,6 +520,13 @@  static void mpc8xxx_spi_cleanup(struct spi_device *spi)
 	kfree(spi->controller_state);
 }
 
+static const char *mpc8xxx_spi_strmode(unsigned int flags)
+{
+	if (flags & SPI_QE_CPU_MODE)
+		return "QE CPU";
+	return "CPU";
+}
+
 static struct spi_master * __devinit
 mpc8xxx_spi_probe(struct device *dev, struct resource *mem, unsigned int irq)
 {
@@ -544,14 +553,14 @@  mpc8xxx_spi_probe(struct device *dev, struct resource *mem, unsigned int irq)
 	master->cleanup = mpc8xxx_spi_cleanup;
 
 	mpc8xxx_spi = spi_master_get_devdata(master);
-	mpc8xxx_spi->qe_mode = pdata->qe_mode;
 	mpc8xxx_spi->get_rx = mpc8xxx_spi_rx_buf_u8;
 	mpc8xxx_spi->get_tx = mpc8xxx_spi_tx_buf_u8;
+	mpc8xxx_spi->flags = pdata->flags;
 	mpc8xxx_spi->spibrg = pdata->sysclk;
 
 	mpc8xxx_spi->rx_shift = 0;
 	mpc8xxx_spi->tx_shift = 0;
-	if (mpc8xxx_spi->qe_mode) {
+	if (mpc8xxx_spi->flags & SPI_QE_CPU_MODE) {
 		mpc8xxx_spi->rx_shift = 16;
 		mpc8xxx_spi->tx_shift = 24;
 	}
@@ -584,7 +593,7 @@  mpc8xxx_spi_probe(struct device *dev, struct resource *mem, unsigned int irq)
 
 	/* Enable SPI interface */
 	regval = pdata->initial_spmode | SPMODE_INIT_VAL | SPMODE_ENABLE;
-	if (pdata->qe_mode)
+	if (mpc8xxx_spi->flags & SPI_QE_CPU_MODE)
 		regval |= SPMODE_OP;
 
 	mpc8xxx_spi_write_reg(&mpc8xxx_spi->base->mode, regval);
@@ -604,9 +613,8 @@  mpc8xxx_spi_probe(struct device *dev, struct resource *mem, unsigned int irq)
 	if (ret < 0)
 		goto unreg_master;
 
-	printk(KERN_INFO
-	       "%s: MPC8xxx SPI Controller driver at 0x%p (irq = %d)\n",
-	       dev_name(dev), mpc8xxx_spi->base, mpc8xxx_spi->irq);
+	dev_info(dev, "at 0x%p (irq = %d), %s mode\n", mpc8xxx_spi->base,
+		 mpc8xxx_spi->irq, mpc8xxx_spi_strmode(mpc8xxx_spi->flags));
 
 	return master;
 
@@ -797,7 +805,7 @@  static int __devinit of_mpc8xxx_spi_probe(struct of_device *ofdev,
 
 	prop = of_get_property(np, "mode", NULL);
 	if (prop && !strcmp(prop, "cpu-qe"))
-		pdata->qe_mode = 1;
+		pdata->flags = SPI_QE_CPU_MODE;
 
 	ret = of_mpc8xxx_spi_get_chipselects(dev);
 	if (ret)
diff --git a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h
index 43fc95d..39fd946 100644
--- a/include/linux/fsl_devices.h
+++ b/include/linux/fsl_devices.h
@@ -74,7 +74,7 @@  struct spi_device;
 struct fsl_spi_platform_data {
 	u32 	initial_spmode;	/* initial SPMODE value */
 	s16	bus_num;
-	bool	qe_mode;
+	unsigned int flags;
 	/* board specific information */
 	u16	max_chipselect;
 	void	(*cs_control)(struct spi_device *spi, bool on);