[PATCH] Adapt spi_mpc83xx SPI driver for 832x

[PATCH] Adapt spi_mpc83xx SPI driver for 832x

[Joakim Tjernlund]

[-- Attachment #1: Type: text/plain, Size: 290 bytes --]

The MPC 832x has a different SPI controller i/f, probably due to
its QUICC engine support. This patch adapts the spi_mpx83xx driver to
be usable on QE based 83xx cpus.

 Jocke

PS.
   Sorry for the attatcment, I have yet to figure out how to tell
   evolution not to mangle inline patches.

[-- Attachment #2: spi2.diff --]
[-- Type: text/x-patch, Size: 2451 bytes --]

diff --git a/drivers/spi/spi_mpc83xx.c b/drivers/spi/spi_mpc83xx.c
index ff0b048..18f094e 100644
--- a/drivers/spi/spi_mpc83xx.c
+++ b/drivers/spi/spi_mpc83xx.c
@@ -47,13 +47,19 @@ struct mpc83xx_spi_reg {
 #define	SPMODE_ENABLE		(1 << 24)
 #define	SPMODE_LEN(x)		((x) << 20)
 #define	SPMODE_PM(x)		((x) << 16)
+#define	SPMODE_OP		(1 << 14)
 
 /*
  * Default for SPI Mode:
  * 	SPI MODE 0 (inactive low, phase middle, MSB, 8-bit length, slow clk
  */
-#define	SPMODE_INIT_VAL (SPMODE_CI_INACTIVEHIGH | SPMODE_DIV16 | SPMODE_REV | \
-			 SPMODE_MS | SPMODE_LEN(7) | SPMODE_PM(0xf))
+#define SPMODE_COMMON_INIT SPMODE_CI_INACTIVEHIGH | SPMODE_DIV16 | SPMODE_REV | \
+			   SPMODE_MS | SPMODE_LEN(7) | SPMODE_PM(0xf)
+#ifdef CONFIG_QUICC_ENGINE
+  #define SPMODE_INIT_VAL (SPMODE_COMMON_INIT | SPMODE_OP)
+#else
+  #define SPMODE_INIT_VAL (SPMODE_COMMON_INIT)
+#endif
 
 /* SPIE register values */
 #define	SPIE_NE		0x00000200	/* Not empty */
@@ -99,31 +105,39 @@ static inline u32 mpc83xx_spi_read_reg(_
 	return in_be32(reg);
 }
 
-#define MPC83XX_SPI_RX_BUF(type) 					  \
+#define MPC83XX_SPI_RX_BUF(type, shift)					  \
 void mpc83xx_spi_rx_buf_##type(u32 data, struct mpc83xx_spi *mpc83xx_spi) \
 {									  \
 	type * rx = mpc83xx_spi->rx;					  \
-	*rx++ = (type)data;						  \
+	*rx++ = (type)(data >> shift);					  \
 	mpc83xx_spi->rx = rx;						  \
 }
 
-#define MPC83XX_SPI_TX_BUF(type)				\
+#define MPC83XX_SPI_TX_BUF(type, shift)				\
 u32 mpc83xx_spi_tx_buf_##type(struct mpc83xx_spi *mpc83xx_spi)	\
 {								\
 	u32 data;						\
 	const type * tx = mpc83xx_spi->tx;			\
-	data = *tx++;						\
+	data = *tx++ << shift;					\
 	mpc83xx_spi->tx = tx;					\
 	return data;						\
 }
-
-MPC83XX_SPI_RX_BUF(u8)
-MPC83XX_SPI_RX_BUF(u16)
-MPC83XX_SPI_RX_BUF(u32)
-MPC83XX_SPI_TX_BUF(u8)
-MPC83XX_SPI_TX_BUF(u16)
-MPC83XX_SPI_TX_BUF(u32)
-
+#ifdef CONFIG_QUICC_ENGINE
+/* This assumes SPMODE_REV is set */
+MPC83XX_SPI_RX_BUF(u8, 16)
+MPC83XX_SPI_RX_BUF(u16, 16)
+MPC83XX_SPI_RX_BUF(u32, 0)
+MPC83XX_SPI_TX_BUF(u8, 24)
+MPC83XX_SPI_TX_BUF(u16, 16)
+MPC83XX_SPI_TX_BUF(u32, 0)
+#else
+MPC83XX_SPI_RX_BUF(u8, 0)
+MPC83XX_SPI_RX_BUF(u16, 0)
+MPC83XX_SPI_RX_BUF(u32, 0)
+MPC83XX_SPI_TX_BUF(u8, 0)
+MPC83XX_SPI_TX_BUF(u16, 0)
+MPC83XX_SPI_TX_BUF(u32, 0)
+#endif
 static void mpc83xx_spi_chipselect(struct spi_device *spi, int value)
 {
 	struct mpc83xx_spi *mpc83xx_spi;

Re: [PATCH] Adapt spi_mpc83xx SPI driver for 832x

[Vitaly Wool]

[-- Attachment #1: Type: text/plain, Size: 705 bytes --]

On 12/13/06, Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote:
>
> The MPC 832x has a different SPI controller i/f, probably due to
> its QUICC engine support. This patch adapts the spi_mpx83xx driver to
> be usable on QE based 83xx cpus.
>

First, I'm pretty much against sending such patches to arch lists and not to
subsystem lists.

@@ -99,31 +105,39 @@ static inline u32 mpc83xx_spi_read_reg(_
        return in_be32(reg);
 }

Then,  this
+       *rx++ = (type)(data >> shift);                                    \
and this
+       data = *tx++ << shift;                                  \

pieces of code are potentially dangerous (what if 'shift' is something like
'x & 0xf3 << 4' ?)

Vitaly

[-- Attachment #2: Type: text/html, Size: 1495 bytes --]

RE: [PATCH] Adapt spi_mpc83xx SPI driver for 832x

[Li Yang-r58472]

The data shifts for receive register are truly different.
Please also send such patch to SPI list:
spi-devel-general@lists.sourceforge.net

- Leo

> -----Original Message-----
> From: linuxppc-dev-bounces+leoli=3Dfreescale.com@ozlabs.org
> [mailto:linuxppc-dev-bounces+leoli=3Dfreescale.com@ozlabs.org] On =
Behalf
Of Joakim
> Tjernlund
> Sent: Wednesday, December 13, 2006 5:22 PM
> To: linuxppc-dev@ozlabs.org
> Subject: [PATCH] Adapt spi_mpc83xx SPI driver for 832x
>=20
> The MPC 832x has a different SPI controller i/f, probably due to
> its QUICC engine support. This patch adapts the spi_mpx83xx driver to
> be usable on QE based 83xx cpus.
>=20
>  Jocke
>=20
> PS.
>    Sorry for the attatcment, I have yet to figure out how to tell
>    evolution not to mangle inline patches.

Re: [PATCH] Adapt spi_mpc83xx SPI driver for 832x

[Kumar Gala]

On Dec 13, 2006, at 3:22 AM, Joakim Tjernlund wrote:

> The MPC 832x has a different SPI controller i/f, probably due to
> its QUICC engine support. This patch adapts the spi_mpx83xx driver to
> be usable on QE based 83xx cpus.
>
>  Jocke

One problem I have with this patch is the fact that it assumes the  
current driver for (mpc834x) and your mods to support mpc832x/QE are  
mutually exclusive.

We need to handle the case of having driver support for both the QE  
and MPC834x style in the same kernel.

- k

>
> PS.
>    Sorry for the attatcment, I have yet to figure out how to tell
>    evolution not to mangle inline patches.
> <spi2.diff>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

RE: [PATCH] Adapt spi_mpc83xx SPI driver for 832x

[Joakim Tjernlund]

> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org] 
> Sent: den 13 december 2006 15:46
> To: joakim.tjernlund@transmode.se
> Cc: linuxppc-dev Development; spi-devel-general@lists.sourceforge.net
> Subject: Re: [PATCH] Adapt spi_mpc83xx SPI driver for 832x
> 
> 
> On Dec 13, 2006, at 3:22 AM, Joakim Tjernlund wrote:
> 
> > The MPC 832x has a different SPI controller i/f, probably due to
> > its QUICC engine support. This patch adapts the spi_mpx83xx 
> driver to
> > be usable on QE based 83xx cpus.
> >
> >  Jocke
> 
> One problem I have with this patch is the fact that it assumes the  
> current driver for (mpc834x) and your mods to support mpc832x/QE are  
> mutually exclusive.

I don't see that as a problem ATM. If that is added it should be optional.

> 
> We need to handle the case of having driver support for both the QE  
> and MPC834x style in the same kernel.

Adding that will double the number RX_BUF/TX_BUF functions from 6 to 12
(possibly this can be reduced by adding more logic to the tx_buf/rx_buf functions)
not to mention what will happen when support for reversed bit order is added.

I would argue that the kernel lacks the possibility to remove complexity
I don't need. Example in this driver is that there is no way to remove
support for 16 and 32 bit SPI character sizes. The same goes for a lot of
the probing code in fsl-soc.c.

It would be nice if a board port could add a custom header file that
gets included by all .c automatically. Then one could add knobs
(read #defines) there to futher tune such things as SPI char size.
That way one don't have add Kconfig entries for all those small
tuning knobs.

 Jocke

Re: [spi-devel-general] [PATCH] Adapt spi_mpc83xx SPI driver for 832x

[David Brownell]

On Wednesday 13 December 2006 7:36 am, Joakim Tjernlund wrote:

> > One problem I have with this patch is the fact that it assumes the  
> > current driver for (mpc834x) and your mods to support mpc832x/QE are  
> > mutually exclusive.
> 
> I don't see that as a problem ATM. If that is added it should be optional.

If PPC supports multi-CPU/board kernels, PPC drivers should too.  I know that
some ARMs don't support them (PXA) while some do (OMAP); so PPC could go either
way.  (Overall it's a win, since it's easier to uncover build breakage if one
kernel can support lots of configurations and drivers.)

The problem I have with this patch is that it has too much #ifdeffery.  If
the PPC code expects that, it should be easy to put infrastructure in place
so that it can be eliminated.  That is, if the PPC powers-that-be allow it!


> > We need to handle the case of having driver support for both the QE  
> > and MPC834x style in the same kernel.
> 
> Adding that will double the number RX_BUF/TX_BUF functions from 6 to 12

Your patch already does this, just they're #ifdeffed so that only one of
the sets will build at a time.


> (possibly this can be reduced by adding more logic to the tx_buf/rx_buf functions)
> not to mention what will happen when support for reversed bit order is added.

Sure enough, sounds ugly.  But cpu_is_xxx() macros, combined with GCC dead
code elimination will strip out functions that are unused, so that e.g.

	if (cpu_is_mpc834x())
		fn = mpc834x_spi_tx_buf_u16;
	else if (cpu_is_mpc832x())
		fn = mpc832x_spi_tx_buf_u16;

would only link one of them unless the kernel supports both SOCs.  And
without in-driver #ifdeffery.

 
> I would argue that the kernel lacks the possibility to remove complexity
> I don't need. Example in this driver is that there is no way to remove
> support for 16 and 32 bit SPI character sizes. The same goes for a lot of
> the probing code in fsl-soc.c.

You could certainly add Kconfig options to help shift some complexity
from runtime to compile time.  But if you do that, remember that it's
not always a win in terms of the overall system.  Every configuration
needs to be tested for correctness, after all.


> It would be nice if a board port could add a custom header file that
> gets included by all .c automatically. Then one could add knobs
> (read #defines) there to futher tune such things as SPI char size.
> That way one don't have add Kconfig entries for all those small
> tuning knobs.

Sounds error prone to me.  What's the point ... removing maybe 100
bytes of code in this one driver?  You could save more than that
just by switching over to platform_device_probe() instead of using
platform_device_register().  And that'd be without adding failure
modes like "oops, this board stack really _does_ use 12 bit words
for the ADCs".

- Dave

RE: [spi-devel-general] [PATCH] Adapt spi_mpc83xx SPI driver for 832x

[Joakim Tjernlund]

> -----Original Message-----
> From: David Brownell [mailto:david-b@pacbell.net] 
> Sent: den 13 december 2006 21:31
> To: spi-devel-general@lists.sourceforge.net
> Cc: Joakim Tjernlund; 'Kumar Gala'; 'linuxppc-dev Development'
> Subject: Re: [spi-devel-general] [PATCH] Adapt spi_mpc83xx 
> SPI driver for 832x
> 
> 
> On Wednesday 13 December 2006 7:36 am, Joakim Tjernlund wrote:
> 
> > > One problem I have with this patch is the fact that it 
> assumes the  
> > > current driver for (mpc834x) and your mods to support 
> mpc832x/QE are  
> > > mutually exclusive.
> > 
> > I don't see that as a problem ATM. If that is added it 
> should be optional.
> 
> If PPC supports multi-CPU/board kernels, PPC drivers should 
> too.  I know that
> some ARMs don't support them (PXA) while some do (OMAP); so 
> PPC could go either
> way.  (Overall it's a win, since it's easier to uncover build 
> breakage if one
> kernel can support lots of configurations and drivers.)
> 
> The problem I have with this patch is that it has too much 
> #ifdeffery.  If

Too much? There is only 2 of them.

> the PPC code expects that, it should be easy to put 
> infrastructure in place
> so that it can be eliminated.  That is, if the PPC 
> powers-that-be allow it!
> 
> 
> > > We need to handle the case of having driver support for 
> both the QE  
> > > and MPC834x style in the same kernel.
> > 
> > Adding that will double the number RX_BUF/TX_BUF functions 
> from 6 to 12
> 
> Your patch already does this, just they're #ifdeffed so that 
> only one of
> the sets will build at a time.

Yes, that's my point

> 
> 
> > (possibly this can be reduced by adding more logic to the 
> tx_buf/rx_buf functions)
> > not to mention what will happen when support for reversed 
> bit order is added.
> 
> Sure enough, sounds ugly.  But cpu_is_xxx() macros, combined 
> with GCC dead
> code elimination will strip out functions that are unused, so 
> that e.g.
> 
> 	if (cpu_is_mpc834x())
> 		fn = mpc834x_spi_tx_buf_u16;
> 	else if (cpu_is_mpc832x())
> 		fn = mpc832x_spi_tx_buf_u16;
> 
> would only link one of them unless the kernel supports both SOCs.  And
> without in-driver #ifdeffery.

Hmm, I can't find any cpu_is_xxx macros/functions. I guess the
infrastructure isn't there yet?

> 
>  
> > I would argue that the kernel lacks the possibility to 
> remove complexity
> > I don't need. Example in this driver is that there is no 
> way to remove
> > support for 16 and 32 bit SPI character sizes. The same 
> goes for a lot of
> > the probing code in fsl-soc.c.
> 
> You could certainly add Kconfig options to help shift some complexity
> from runtime to compile time.  But if you do that, remember that it's
> not always a win in terms of the overall system.  Every configuration
> needs to be tested for correctness, after all.

Well, so does the dynamic kernel too. I don't know if the 16/32 bit
char size works, I don't have such HW.

> 
> 
> > It would be nice if a board port could add a custom header file that
> > gets included by all .c automatically. Then one could add knobs
> > (read #defines) there to futher tune such things as SPI char size.
> > That way one don't have add Kconfig entries for all those small
> > tuning knobs.
> 
> Sounds error prone to me.  What's the point ... removing maybe 100
> bytes of code in this one driver?  You could save more than that
> just by switching over to platform_device_probe() instead of using
> platform_device_register().  And that'd be without adding failure
> modes like "oops, this board stack really _does_ use 12 bit words
> for the ADCs".

One size reduction in another area doesn't justify bloat in
another. People are switching to kzmalloc() to save space(not
the only reason though).

The point is not only this driver. I add small tweaks here and
there and it would be great if I could put function prototypes,
my own #defines there too. This would help me not to clutter pristine
kernel code too much, makes maintenace easier.

 Jocke
 

RE: [spi-devel-general] [PATCH] Adapt spi_mpc83xx SPI driver for 832x

[Kumar Gala]

What about something like the following were we pass in if we are in a 
quicc engine to the driver via the platform device.

Joakim, I haven't tested this so would like to know if this works for you 
on your 832x.  I was also wondering if there was a reason you wanted to 
use the QE SPI in CPU mode and not QE mode, lack of driver, something 
else?

Also, if this patch looks good to David, I'll clean it up and send it 
upstream.  I want to rename the driver and cleanup the Kconfig to make it 
a bit more generic to cover more than just mpc83xx. (The same driver 
should work on the MPC8568 so I was going to rename the driver 
spi_mpc8xxx.c).

- k


diff --git a/drivers/spi/spi_mpc83xx.c b/drivers/spi/spi_mpc83xx.c
index ff0b048..9666440 100644
--- a/drivers/spi/spi_mpc83xx.c
+++ b/drivers/spi/spi_mpc83xx.c
@@ -47,6 +47,7 @@ struct mpc83xx_spi_reg {
  #define	SPMODE_ENABLE		(1 << 24)
  #define	SPMODE_LEN(x)		((x) << 20)
  #define	SPMODE_PM(x)		((x) << 16)
+#define	SPMODE_OP		(1 << 14)

  /*
   * Default for SPI Mode:
@@ -85,6 +86,10 @@ struct mpc83xx_spi {
  	unsigned nsecs;		/* (clock cycle time)/2 */

  	u32 sysclk;
+	u32 shift;		/* amount to adjust data regs if in qe mode */
+
+	bool qe_mode;
+
  	void (*activate_cs) (u8 cs, u8 polarity);
  	void (*deactivate_cs) (u8 cs, u8 polarity);
  };
@@ -103,7 +108,7 @@ static inline u32 mpc83xx_spi_read_reg(__be32 __iomem * reg)
  void mpc83xx_spi_rx_buf_##type(u32 data, struct mpc83xx_spi *mpc83xx_spi) \
  {									  \
  	type * rx = mpc83xx_spi->rx;					  \
-	*rx++ = (type)data;						  \
+	*rx++ = (type)(data >> mpc83xx_spi->shift);			  \
  	mpc83xx_spi->rx = rx;						  \
  }

@@ -112,7 +117,7 @@ u32 mpc83xx_spi_tx_buf_##type(struct mpc83xx_spi *mpc83xx_spi)	\
  {								\
  	u32 data;						\
  	const type * tx = mpc83xx_spi->tx;			\
-	data = *tx++;						\
+	data = *tx++ << mpc83xx_spi->shift;			\
  	mpc83xx_spi->tx = tx;					\
  	return data;						\
  }
@@ -198,12 +203,18 @@ int mpc83xx_spi_setup_transfer(struct spi_device *spi, struct spi_transfer *t)
  	if (bits_per_word <= 8) {
  		mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u8;
  		mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u8;
+		if (mpc83xx_spi->qe_mode)
+			mpc83xx_spi->shift = 24;
  	} else if (bits_per_word <= 16) {
  		mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u16;
  		mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u16;
+		if (mpc83xx_spi->qe_mode)
+			mpc83xx_spi->shift = 16;
  	} else if (bits_per_word <= 32) {
  		mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u32;
  		mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u32;
+		if (mpc83xx_spi->qe_mode)
+			mpc83xx_spi->shift = 0;
  	} else
  		return -EINVAL;

@@ -378,9 +389,15 @@ static int __init mpc83xx_spi_probe(struct platform_device *dev)
  	mpc83xx_spi->sysclk = pdata->sysclk;
  	mpc83xx_spi->activate_cs = pdata->activate_cs;
  	mpc83xx_spi->deactivate_cs = pdata->deactivate_cs;
+	mpc83xx_spi->qe_mode = pdata->qe_mode;
  	mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u8;
  	mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u8;

+	if (mpc83xx_spi->qe_mode)
+		mpc83xx_spi->shift = 24;
+	else
+		mpc83xx_spi->shift = 0;
+
  	mpc83xx_spi->bitbang.master->setup = mpc83xx_spi_setup;
  	init_completion(&mpc83xx_spi->done);

@@ -415,6 +432,8 @@ static int __init mpc83xx_spi_probe(struct platform_device *dev)

  	/* Enable SPI interface */
  	regval = pdata->initial_spmode | SPMODE_INIT_VAL | SPMODE_ENABLE;
+	if (pdata->qe_mode)
+		regval |= SPMODE_OP;
  	mpc83xx_spi_write_reg(&mpc83xx_spi->base->mode, regval);

  	ret = spi_bitbang_start(&mpc83xx_spi->bitbang);
diff --git a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h
index abb64c4..a5bf40f 100644
--- a/include/linux/fsl_devices.h
+++ b/include/linux/fsl_devices.h
@@ -111,6 +111,7 @@ struct fsl_usb2_platform_data {

  struct fsl_spi_platform_data {
  	u32 	initial_spmode;	/* initial SPMODE value */
+	bool	qe_mode;
  	u16	bus_num;

  	/* board specific information */

RE: [spi-devel-general] [PATCH] Adapt spi_mpc83xx SPI driver for 832x

[Joakim Tjernlund]

On Wed, 2006-12-13 at 23:54 -0600, Kumar Gala wrote:
> What about something like the following were we pass in if we are in a 
> quicc engine to the driver via the platform device.
> 
> Joakim, I haven't tested this so would like to know if this works for you 
> on your 832x.  I was also wondering if there was a reason you wanted to 
> use the QE SPI in CPU mode and not QE mode, lack of driver, something 
> else?

Just lack of driver. The amont of data is small in my case so it does
not matter much.

> 
> Also, if this patch looks good to David, I'll clean it up and send it 
> upstream.  I want to rename the driver and cleanup the Kconfig to make it 
> a bit more generic to cover more than just mpc83xx. (The same driver 
> should work on the MPC8568 so I was going to rename the driver 
> spi_mpc8xxx.c).
> 
> - k

patch needs a litte work, the shift is diffrent for tx resp. rx so I 
adjusted that and added a temp hack(still in patch below) to force qe_mode.
The name qe_mode is misleading. This i a QE enabled CPU, but SPI is running in cpu mode. qe_mode implies that SPI is runing in native QUICC
mode.

Adjusted patch(lets hope I didn't mangle this one:):

diff --git a/drivers/spi/spi_mpc83xx.c b/drivers/spi/spi_mpc83xx.c
index ff0b048..0c31878 100644
--- a/drivers/spi/spi_mpc83xx.c
+++ b/drivers/spi/spi_mpc83xx.c
@@ -47,6 +47,7 @@ struct mpc83xx_spi_reg {
 #define	SPMODE_ENABLE		(1 << 24)
 #define	SPMODE_LEN(x)		((x) << 20)
 #define	SPMODE_PM(x)		((x) << 16)
+#define	SPMODE_OP		(1 << 14)
 
 /*
  * Default for SPI Mode:
@@ -85,6 +86,11 @@ struct mpc83xx_spi {
 	unsigned nsecs;		/* (clock cycle time)/2 */
 
 	u32 sysclk;
+	u32 rx_shift;		/* amount to adjust RX data regs if in qe mode */
+	u32 tx_shift;		/* amount to adjust TX data regs if in qe mode */
+
+	bool qe_mode;
+
 	void (*activate_cs) (u8 cs, u8 polarity);
 	void (*deactivate_cs) (u8 cs, u8 polarity);
 };
@@ -103,7 +109,7 @@ static inline u32 mpc83xx_spi_read_reg(_
 void mpc83xx_spi_rx_buf_##type(u32 data, struct mpc83xx_spi *mpc83xx_spi) \
 {									  \
 	type * rx = mpc83xx_spi->rx;					  \
-	*rx++ = (type)data;						  \
+	*rx++ = (type)(data >> mpc83xx_spi->rx_shift);			  \
 	mpc83xx_spi->rx = rx;						  \
 }
 
@@ -112,7 +118,7 @@ u32 mpc83xx_spi_tx_buf_##type(struct mpc
 {								\
 	u32 data;						\
 	const type * tx = mpc83xx_spi->tx;			\
-	data = *tx++;						\
+	data = *tx++ << mpc83xx_spi->tx_shift;			\
 	mpc83xx_spi->tx = tx;					\
 	return data;						\
 }
@@ -195,12 +201,22 @@ int mpc83xx_spi_setup_transfer(struct sp
 	    || ((bits_per_word > 16) && (bits_per_word != 32)))
 		return -EINVAL;
 
+	mpc83xx_spi->rx_shift = 0;
+	mpc83xx_spi->tx_shift = 0;
 	if (bits_per_word <= 8) {
 		mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u8;
 		mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u8;
+		if (mpc83xx_spi->qe_mode) {
+			mpc83xx_spi->rx_shift = 16;
+			mpc83xx_spi->tx_shift = 24;
+		}
 	} else if (bits_per_word <= 16) {
 		mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u16;
 		mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u16;
+		if (mpc83xx_spi->qe_mode) {
+			mpc83xx_spi->rx_shift = 16;
+			mpc83xx_spi->tx_shift = 16;
+		}
 	} else if (bits_per_word <= 32) {
 		mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u32;
 		mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u32;
@@ -369,8 +385,8 @@ static int __init mpc83xx_spi_probe(stru
 		ret = -ENODEV;
 		goto free_master;
 	}
-
 	mpc83xx_spi = spi_master_get_devdata(master);
+	pdata->qe_mode = 1; // temp hack to force 832x mode
 	mpc83xx_spi->bitbang.master = spi_master_get(master);
 	mpc83xx_spi->bitbang.chipselect = mpc83xx_spi_chipselect;
 	mpc83xx_spi->bitbang.setup_transfer = mpc83xx_spi_setup_transfer;
@@ -378,9 +394,17 @@ static int __init mpc83xx_spi_probe(stru
 	mpc83xx_spi->sysclk = pdata->sysclk;
 	mpc83xx_spi->activate_cs = pdata->activate_cs;
 	mpc83xx_spi->deactivate_cs = pdata->deactivate_cs;
+	mpc83xx_spi->qe_mode = pdata->qe_mode;
 	mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u8;
 	mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u8;
 
+	mpc83xx_spi->rx_shift = 0;
+	mpc83xx_spi->tx_shift = 0;
+	if (mpc83xx_spi->qe_mode) {
+		mpc83xx_spi->rx_shift = 16;
+		mpc83xx_spi->tx_shift = 24;
+	}
+
 	mpc83xx_spi->bitbang.master->setup = mpc83xx_spi_setup;
 	init_completion(&mpc83xx_spi->done);
 
@@ -415,6 +439,9 @@ static int __init mpc83xx_spi_probe(stru
 
 	/* Enable SPI interface */
 	regval = pdata->initial_spmode | SPMODE_INIT_VAL | SPMODE_ENABLE;
+	if (pdata->qe_mode)
+		regval |= SPMODE_OP;
+
 	mpc83xx_spi_write_reg(&mpc83xx_spi->base->mode, regval);
 
 	ret = spi_bitbang_start(&mpc83xx_spi->bitbang);

Re: [spi-devel-general] [PATCH] Adapt spi_mpc83xx SPI driver for 832x

[Reeve Yang]

Hello, I'm working on the same thing but our board is using MPC8343E
processor. Should the same patch be applied? If so could someone post
a complete and clean patch? Though our kernel is 2.6.13, I back ported
spi_mpc83xx but it's not working (the fsl controller cannot be
detected). I'm hoping this patch could help on that ...

- Reeve

On 12/14/06, Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote:
> On Wed, 2006-12-13 at 23:54 -0600, Kumar Gala wrote:
> > What about something like the following were we pass in if we are in a
> > quicc engine to the driver via the platform device.
> >
> > Joakim, I haven't tested this so would like to know if this works for you
> > on your 832x.  I was also wondering if there was a reason you wanted to
> > use the QE SPI in CPU mode and not QE mode, lack of driver, something
> > else?
>
> Just lack of driver. The amont of data is small in my case so it does
> not matter much.
>
> >
> > Also, if this patch looks good to David, I'll clean it up and send it
> > upstream.  I want to rename the driver and cleanup the Kconfig to make it
> > a bit more generic to cover more than just mpc83xx. (The same driver
> > should work on the MPC8568 so I was going to rename the driver
> > spi_mpc8xxx.c).
> >
> > - k
>
> patch needs a litte work, the shift is diffrent for tx resp. rx so I
> adjusted that and added a temp hack(still in patch below) to force qe_mode.
> The name qe_mode is misleading. This i a QE enabled CPU, but SPI is running in cpu mode. qe_mode implies that SPI is runing in native QUICC
> mode.
>
> Adjusted patch(lets hope I didn't mangle this one:):
>
> diff --git a/drivers/spi/spi_mpc83xx.c b/drivers/spi/spi_mpc83xx.c
> index ff0b048..0c31878 100644
> --- a/drivers/spi/spi_mpc83xx.c
> +++ b/drivers/spi/spi_mpc83xx.c
> @@ -47,6 +47,7 @@ struct mpc83xx_spi_reg {
>  #define        SPMODE_ENABLE           (1 << 24)
>  #define        SPMODE_LEN(x)           ((x) << 20)
>  #define        SPMODE_PM(x)            ((x) << 16)
> +#define        SPMODE_OP               (1 << 14)
>
>  /*
>   * Default for SPI Mode:
> @@ -85,6 +86,11 @@ struct mpc83xx_spi {
>         unsigned nsecs;         /* (clock cycle time)/2 */
>
>         u32 sysclk;
> +       u32 rx_shift;           /* amount to adjust RX data regs if in qe mode */
> +       u32 tx_shift;           /* amount to adjust TX data regs if in qe mode */
> +
> +       bool qe_mode;
> +
>         void (*activate_cs) (u8 cs, u8 polarity);
>         void (*deactivate_cs) (u8 cs, u8 polarity);
>  };
> @@ -103,7 +109,7 @@ static inline u32 mpc83xx_spi_read_reg(_
>  void mpc83xx_spi_rx_buf_##type(u32 data, struct mpc83xx_spi *mpc83xx_spi) \
>  {                                                                        \
>         type * rx = mpc83xx_spi->rx;                                      \
> -       *rx++ = (type)data;                                               \
> +       *rx++ = (type)(data >> mpc83xx_spi->rx_shift);                    \
>         mpc83xx_spi->rx = rx;                                             \
>  }
>
> @@ -112,7 +118,7 @@ u32 mpc83xx_spi_tx_buf_##type(struct mpc
>  {                                                              \
>         u32 data;                                               \
>         const type * tx = mpc83xx_spi->tx;                      \
> -       data = *tx++;                                           \
> +       data = *tx++ << mpc83xx_spi->tx_shift;                  \
>         mpc83xx_spi->tx = tx;                                   \
>         return data;                                            \
>  }
> @@ -195,12 +201,22 @@ int mpc83xx_spi_setup_transfer(struct sp
>             || ((bits_per_word > 16) && (bits_per_word != 32)))
>                 return -EINVAL;
>
> +       mpc83xx_spi->rx_shift = 0;
> +       mpc83xx_spi->tx_shift = 0;
>         if (bits_per_word <= 8) {
>                 mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u8;
>                 mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u8;
> +               if (mpc83xx_spi->qe_mode) {
> +                       mpc83xx_spi->rx_shift = 16;
> +                       mpc83xx_spi->tx_shift = 24;
> +               }
>         } else if (bits_per_word <= 16) {
>                 mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u16;
>                 mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u16;
> +               if (mpc83xx_spi->qe_mode) {
> +                       mpc83xx_spi->rx_shift = 16;
> +                       mpc83xx_spi->tx_shift = 16;
> +               }
>         } else if (bits_per_word <= 32) {
>                 mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u32;
>                 mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u32;
> @@ -369,8 +385,8 @@ static int __init mpc83xx_spi_probe(stru
>                 ret = -ENODEV;
>                 goto free_master;
>         }
> -
>         mpc83xx_spi = spi_master_get_devdata(master);
> +       pdata->qe_mode = 1; // temp hack to force 832x mode
>         mpc83xx_spi->bitbang.master = spi_master_get(master);
>         mpc83xx_spi->bitbang.chipselect = mpc83xx_spi_chipselect;
>         mpc83xx_spi->bitbang.setup_transfer = mpc83xx_spi_setup_transfer;
> @@ -378,9 +394,17 @@ static int __init mpc83xx_spi_probe(stru
>         mpc83xx_spi->sysclk = pdata->sysclk;
>         mpc83xx_spi->activate_cs = pdata->activate_cs;
>         mpc83xx_spi->deactivate_cs = pdata->deactivate_cs;
> +       mpc83xx_spi->qe_mode = pdata->qe_mode;
>         mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u8;
>         mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u8;
>
> +       mpc83xx_spi->rx_shift = 0;
> +       mpc83xx_spi->tx_shift = 0;
> +       if (mpc83xx_spi->qe_mode) {
> +               mpc83xx_spi->rx_shift = 16;
> +               mpc83xx_spi->tx_shift = 24;
> +       }
> +
>         mpc83xx_spi->bitbang.master->setup = mpc83xx_spi_setup;
>         init_completion(&mpc83xx_spi->done);
>
> @@ -415,6 +439,9 @@ static int __init mpc83xx_spi_probe(stru
>
>         /* Enable SPI interface */
>         regval = pdata->initial_spmode | SPMODE_INIT_VAL | SPMODE_ENABLE;
> +       if (pdata->qe_mode)
> +               regval |= SPMODE_OP;
> +
>         mpc83xx_spi_write_reg(&mpc83xx_spi->base->mode, regval);
>
>         ret = spi_bitbang_start(&mpc83xx_spi->bitbang);
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>

Re: [spi-devel-general] [PATCH] Adapt spi_mpc83xx SPI driver for 832x

[Ben Warren]

Reeve,

--- Reeve Yang <yang.reeve@gmail.com> wrote:

> Hello, I'm working on the same thing but our board
> is using MPC8343E
> processor. Should the same patch be applied? If so
> could someone post
> a complete and clean patch? Though our kernel is
> 2.6.13, I back ported
> spi_mpc83xx but it's not working (the fsl controller
> cannot be
> detected). I'm hoping this patch could help on that
> ...
> 

I had the mpc83xx driver working on an MPC8349
processor under 2.6.14 a while ago.  The driver's very
small but requires a fair amount of board specific
initialization code.  Are you sure you added the
proper platform resources, etc?

regards,
Ben

Re: [spi-devel-general] [PATCH] Adapt spi_mpc83xx SPI driver for 832x

[Reeve Yang]

Hi ben, what you have done is exactly I'm trying to do. (I'm using
2.6.14 too, it's a typo in previous email). Could you elighten me what
board specific things need to be done? Following are the things I have
done.

********************************
<<< file 1: /vobs/kernel/linux-2.6.14/arch/ppc/syslib/mpc83xx_devices.c@@/main/linux-2.6.14_int/ryang_ATS89_1.0.0_dev_2/0
>>> file 2: mpc83xx_devices.c
********************************
-----[after 66 inserted 67-69]-----
> /* init this in board specific code */
> static struct fsl_spi_platform_data mpc83xx_spi_pdata = { 0 };
>
====> [reeve] pstruct platform_device ppc_sys_platform_devices[] = {

-----[after 217 inserted 221-238]-----
> 				.flags	= IORESOURCE_IRQ,
> 			},
> 		},
> 	},
> 	[MPC83xx_SPI] = {
> 		.name = "fsl-spi",
> 		.id	= 1,
>         .dev.platform_data = &mpc83xx_spi_pdata,
> 		.num_resources	 = 2,
> 		.resource = (struct resource[]) {
> 			{
> 				.start	= 0x7000,
> 				.end	= 0x7fff,
> 				.flags	= IORESOURCE_MEM,
> 			},
> 			{
> 				.start	= MPC83xx_IRQ_SPI,
> 				.end	= MPC83xx_IRQ_SPI,

In function mpc834x_sys_setup_arch, I add setup for spi platform data:

-----[after 175 inserted 203-212]-----
>     /* setup board specific spi information */
>     spi_pdata = (struct fsl_spi_platform_data *)ppc_sys_get_pdata(MPC83xx_SPI);
>     if (spi_pdata) {
>         spi_pdata->initial_spmode = 0;
>         spi_pdata->bus_num = 1;
>         spi_pdata->max_chipselect = 1;
>         spi_pdata->activate_cs = at10408_spi_activate_cs;
>         spi_pdata->deactivate_cs = at10408_spi_deactivate_cs;

We're doing chip select through CPLD gpio pins.

That's all I could image for board specific for fsl spi controller.
Did I miss anything?

- Reeve
On 12/14/06, Ben Warren <bwarren@qstreams.com> wrote:
> Reeve,
>
> --- Reeve Yang <yang.reeve@gmail.com> wrote:
>
> > Hello, I'm working on the same thing but our board
> > is using MPC8343E
> > processor. Should the same patch be applied? If so
> > could someone post
> > a complete and clean patch? Though our kernel is
> > 2.6.13, I back ported
> > spi_mpc83xx but it's not working (the fsl controller
> > cannot be
> > detected). I'm hoping this patch could help on that
> > ...
> >
>
> I had the mpc83xx driver working on an MPC8349
> processor under 2.6.14 a while ago.  The driver's very
> small but requires a fair amount of board specific
> initialization code.  Are you sure you added the
> proper platform resources, etc?
>
> regards,
> Ben
>

Re: [spi-devel-general] [PATCH] Adapt spi_mpc83xx SPI driver for 832x

[Ben Warren]

Reeve,

--- Reeve Yang <yang.reeve@gmail.com> wrote:

> Hi ben, what you have done is exactly I'm trying to
> do. (I'm using
> 2.6.14 too, it's a typo in previous email). Could
> you elighten me what
> board specific things need to be done? Following are
> the things I have
> done.

I don't have ready access to my code right now but
will be in touch with you tomorrow.  I propose to take
this sub-thread private, since I dought Joakim's patch
has anything to do with your problem and there are a
lot of recipient's here who don't really care.

cheers,
Ben

Re: [spi-devel-general] [PATCH] Adapt spi_mpc83xx SPI driver for 832x

[David Brownell]

On Thursday 14 December 2006 2:02 am, Joakim Tjernlund wrote:

> Adjusted patch(lets hope I didn't mangle this one:):

Assuming this is correct, I don't have a problem with this one.
Feel free to wrap it up with a signed-off-by and I'll forward it.


I have a question for someone who knows about this controller though...

When issuing an RX-only transfer, the SPI framework currently specifies
that spi_transfer.tx_buf will be null, and that the data shifted out
on the MOSI pin is "undefined".  Looking at the spi_mpc83xx driver in
the current tree, it looked to me as if it wouldn't actually accept
a null tx_buf ... did I miss something, or would the tx_buf##type macros
expand to a null pointer exception?  If I did miss something, what data
would be shifted out in that case?  (IRQ vectors from page zero?)

There's a proposal afoot to change how that's specified:  rather than
shifting out "undefined" data, define it as all zeroes.  It was easy
to see the impact of that on all the other SPI controller drivers now
upstream; all but one send zeroes already.  But not this one...

- Dave


> diff --git a/drivers/spi/spi_mpc83xx.c b/drivers/spi/spi_mpc83xx.c
> index ff0b048..0c31878 100644
> --- a/drivers/spi/spi_mpc83xx.c
> +++ b/drivers/spi/spi_mpc83xx.c
> @@ -47,6 +47,7 @@ struct mpc83xx_spi_reg {
>  #define	SPMODE_ENABLE		(1 << 24)
>  #define	SPMODE_LEN(x)		((x) << 20)
>  #define	SPMODE_PM(x)		((x) << 16)
> +#define	SPMODE_OP		(1 << 14)
>  
>  /*
>   * Default for SPI Mode:
> @@ -85,6 +86,11 @@ struct mpc83xx_spi {
>  	unsigned nsecs;		/* (clock cycle time)/2 */
>  
>  	u32 sysclk;
> +	u32 rx_shift;		/* amount to adjust RX data regs if in qe mode */
> +	u32 tx_shift;		/* amount to adjust TX data regs if in qe mode */
> +
> +	bool qe_mode;
> +
>  	void (*activate_cs) (u8 cs, u8 polarity);
>  	void (*deactivate_cs) (u8 cs, u8 polarity);
>  };
> @@ -103,7 +109,7 @@ static inline u32 mpc83xx_spi_read_reg(_
>  void mpc83xx_spi_rx_buf_##type(u32 data, struct mpc83xx_spi *mpc83xx_spi) \
>  {									  \
>  	type * rx = mpc83xx_spi->rx;					  \
> -	*rx++ = (type)data;						  \
> +	*rx++ = (type)(data >> mpc83xx_spi->rx_shift);			  \
>  	mpc83xx_spi->rx = rx;						  \
>  }
>  
> @@ -112,7 +118,7 @@ u32 mpc83xx_spi_tx_buf_##type(struct mpc
>  {								\
>  	u32 data;						\
>  	const type * tx = mpc83xx_spi->tx;			\
> -	data = *tx++;						\
> +	data = *tx++ << mpc83xx_spi->tx_shift;			\
>  	mpc83xx_spi->tx = tx;					\
>  	return data;						\
>  }
> @@ -195,12 +201,22 @@ int mpc83xx_spi_setup_transfer(struct sp
>  	    || ((bits_per_word > 16) && (bits_per_word != 32)))
>  		return -EINVAL;
>  
> +	mpc83xx_spi->rx_shift = 0;
> +	mpc83xx_spi->tx_shift = 0;
>  	if (bits_per_word <= 8) {
>  		mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u8;
>  		mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u8;
> +		if (mpc83xx_spi->qe_mode) {
> +			mpc83xx_spi->rx_shift = 16;
> +			mpc83xx_spi->tx_shift = 24;
> +		}
>  	} else if (bits_per_word <= 16) {
>  		mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u16;
>  		mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u16;
> +		if (mpc83xx_spi->qe_mode) {
> +			mpc83xx_spi->rx_shift = 16;
> +			mpc83xx_spi->tx_shift = 16;
> +		}
>  	} else if (bits_per_word <= 32) {
>  		mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u32;
>  		mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u32;
> @@ -369,8 +385,8 @@ static int __init mpc83xx_spi_probe(stru
>  		ret = -ENODEV;
>  		goto free_master;
>  	}
> -
>  	mpc83xx_spi = spi_master_get_devdata(master);
> +	pdata->qe_mode = 1; // temp hack to force 832x mode
>  	mpc83xx_spi->bitbang.master = spi_master_get(master);
>  	mpc83xx_spi->bitbang.chipselect = mpc83xx_spi_chipselect;
>  	mpc83xx_spi->bitbang.setup_transfer = mpc83xx_spi_setup_transfer;
> @@ -378,9 +394,17 @@ static int __init mpc83xx_spi_probe(stru
>  	mpc83xx_spi->sysclk = pdata->sysclk;
>  	mpc83xx_spi->activate_cs = pdata->activate_cs;
>  	mpc83xx_spi->deactivate_cs = pdata->deactivate_cs;
> +	mpc83xx_spi->qe_mode = pdata->qe_mode;
>  	mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u8;
>  	mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u8;
>  
> +	mpc83xx_spi->rx_shift = 0;
> +	mpc83xx_spi->tx_shift = 0;
> +	if (mpc83xx_spi->qe_mode) {
> +		mpc83xx_spi->rx_shift = 16;
> +		mpc83xx_spi->tx_shift = 24;
> +	}
> +
>  	mpc83xx_spi->bitbang.master->setup = mpc83xx_spi_setup;
>  	init_completion(&mpc83xx_spi->done);
>  
> @@ -415,6 +439,9 @@ static int __init mpc83xx_spi_probe(stru
>  
>  	/* Enable SPI interface */
>  	regval = pdata->initial_spmode | SPMODE_INIT_VAL | SPMODE_ENABLE;
> +	if (pdata->qe_mode)
> +		regval |= SPMODE_OP;
> +
>  	mpc83xx_spi_write_reg(&mpc83xx_spi->base->mode, regval);
>  
>  	ret = spi_bitbang_start(&mpc83xx_spi->bitbang);
> 
> 
> 

Re: [spi-devel-general] [PATCH] Adapt spi_mpc83xx SPI driver for 832x

[Kumar Gala]

On Dec 22, 2006, at 7:09 PM, David Brownell wrote:

> On Thursday 14 December 2006 2:02 am, Joakim Tjernlund wrote:
>
>> Adjusted patch(lets hope I didn't mangle this one:):
>
> Assuming this is correct, I don't have a problem with this one.
> Feel free to wrap it up with a signed-off-by and I'll forward it.

I'll cleanup the patches and send something to you.

> I have a question for someone who knows about this controller  
> though...
>
> When issuing an RX-only transfer, the SPI framework currently  
> specifies
> that spi_transfer.tx_buf will be null, and that the data shifted out
> on the MOSI pin is "undefined".  Looking at the spi_mpc83xx driver in
> the current tree, it looked to me as if it wouldn't actually accept
> a null tx_buf ... did I miss something, or would the tx_buf##type  
> macros
> expand to a null pointer exception?  If I did miss something, what  
> data
> would be shifted out in that case?  (IRQ vectors from page zero?)

Yeah this is a bug, your not missing anything.  Would be good if this  
fix could go into 2.6.20.

> There's a proposal afoot to change how that's specified:  rather than
> shifting out "undefined" data, define it as all zeroes.  It was easy
> to see the impact of that on all the other SPI controller drivers now
> upstream; all but one send zeroes already.  But not this one...
>
> - Dave

[snip]

Re: [spi-devel-general] [PATCH] Adapt spi_mpc83xx SPI driver for 832x

[David Brownell]

On Thursday 14 December 2006 2:02 am, Joakim Tjernlund wrote:

> patch needs a litte work, the shift is diffrent for tx resp. rx so I 
> adjusted that and added a temp hack(still in patch below) to force qe_mode.
> The name qe_mode is misleading. This i a QE enabled CPU, but SPI is running
> in cpu mode. qe_mode implies that SPI is runing in native QUICC 
> mode.
> 
> Adjusted patch(lets hope I didn't mangle this one:):

What's the story on this?  I see that no followup patch has been submitted,
so this 832x support is still not merged ...

I'm deleting all this 83xx history from my "pending" mbox, so if anyone
wants this support in the mainstream kernel, this is the only reminder
you'll get from me.

- Dave

 
> diff --git a/drivers/spi/spi_mpc83xx.c b/drivers/spi/spi_mpc83xx.c
> index ff0b048..0c31878 100644
> --- a/drivers/spi/spi_mpc83xx.c
> +++ b/drivers/spi/spi_mpc83xx.c
> @@ -47,6 +47,7 @@ struct mpc83xx_spi_reg {
>  #define	SPMODE_ENABLE		(1 << 24)
>  #define	SPMODE_LEN(x)		((x) << 20)
>  #define	SPMODE_PM(x)		((x) << 16)
> +#define	SPMODE_OP		(1 << 14)
>  
>  /*
>   * Default for SPI Mode:
> @@ -85,6 +86,11 @@ struct mpc83xx_spi {
>  	unsigned nsecs;		/* (clock cycle time)/2 */
>  
>  	u32 sysclk;
> +	u32 rx_shift;		/* amount to adjust RX data regs if in qe mode */
> +	u32 tx_shift;		/* amount to adjust TX data regs if in qe mode */
> +
> +	bool qe_mode;
> +
>  	void (*activate_cs) (u8 cs, u8 polarity);
>  	void (*deactivate_cs) (u8 cs, u8 polarity);
>  };
> @@ -103,7 +109,7 @@ static inline u32 mpc83xx_spi_read_reg(_
>  void mpc83xx_spi_rx_buf_##type(u32 data, struct mpc83xx_spi *mpc83xx_spi) \
>  {									  \
>  	type * rx = mpc83xx_spi->rx;					  \
> -	*rx++ = (type)data;						  \
> +	*rx++ = (type)(data >> mpc83xx_spi->rx_shift);			  \
>  	mpc83xx_spi->rx = rx;						  \
>  }
>  
> @@ -112,7 +118,7 @@ u32 mpc83xx_spi_tx_buf_##type(struct mpc
>  {								\
>  	u32 data;						\
>  	const type * tx = mpc83xx_spi->tx;			\
> -	data = *tx++;						\
> +	data = *tx++ << mpc83xx_spi->tx_shift;			\
>  	mpc83xx_spi->tx = tx;					\
>  	return data;						\
>  }
> @@ -195,12 +201,22 @@ int mpc83xx_spi_setup_transfer(struct sp
>  	    || ((bits_per_word > 16) && (bits_per_word != 32)))
>  		return -EINVAL;
>  
> +	mpc83xx_spi->rx_shift = 0;
> +	mpc83xx_spi->tx_shift = 0;
>  	if (bits_per_word <= 8) {
>  		mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u8;
>  		mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u8;
> +		if (mpc83xx_spi->qe_mode) {
> +			mpc83xx_spi->rx_shift = 16;
> +			mpc83xx_spi->tx_shift = 24;
> +		}
>  	} else if (bits_per_word <= 16) {
>  		mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u16;
>  		mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u16;
> +		if (mpc83xx_spi->qe_mode) {
> +			mpc83xx_spi->rx_shift = 16;
> +			mpc83xx_spi->tx_shift = 16;
> +		}
>  	} else if (bits_per_word <= 32) {
>  		mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u32;
>  		mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u32;
> @@ -369,8 +385,8 @@ static int __init mpc83xx_spi_probe(stru
>  		ret = -ENODEV;
>  		goto free_master;
>  	}
> -
>  	mpc83xx_spi = spi_master_get_devdata(master);
> +	pdata->qe_mode = 1; // temp hack to force 832x mode
>  	mpc83xx_spi->bitbang.master = spi_master_get(master);
>  	mpc83xx_spi->bitbang.chipselect = mpc83xx_spi_chipselect;
>  	mpc83xx_spi->bitbang.setup_transfer = mpc83xx_spi_setup_transfer;
> @@ -378,9 +394,17 @@ static int __init mpc83xx_spi_probe(stru
>  	mpc83xx_spi->sysclk = pdata->sysclk;
>  	mpc83xx_spi->activate_cs = pdata->activate_cs;
>  	mpc83xx_spi->deactivate_cs = pdata->deactivate_cs;
> +	mpc83xx_spi->qe_mode = pdata->qe_mode;
>  	mpc83xx_spi->get_rx = mpc83xx_spi_rx_buf_u8;
>  	mpc83xx_spi->get_tx = mpc83xx_spi_tx_buf_u8;
>  
> +	mpc83xx_spi->rx_shift = 0;
> +	mpc83xx_spi->tx_shift = 0;
> +	if (mpc83xx_spi->qe_mode) {
> +		mpc83xx_spi->rx_shift = 16;
> +		mpc83xx_spi->tx_shift = 24;
> +	}
> +
>  	mpc83xx_spi->bitbang.master->setup = mpc83xx_spi_setup;
>  	init_completion(&mpc83xx_spi->done);
>  
> @@ -415,6 +439,9 @@ static int __init mpc83xx_spi_probe(stru
>  
>  	/* Enable SPI interface */
>  	regval = pdata->initial_spmode | SPMODE_INIT_VAL | SPMODE_ENABLE;
> +	if (pdata->qe_mode)
> +		regval |= SPMODE_OP;
> +
>  	mpc83xx_spi_write_reg(&mpc83xx_spi->base->mode, regval);
>  
>  	ret = spi_bitbang_start(&mpc83xx_spi->bitbang);
> 

RE: [spi-devel-general] [PATCH] Adapt spi_mpc83xx SPI driver for 832x

[Joakim Tjernlund]

> -----Original Message-----
> From: David Brownell [mailto:david-b@pacbell.net] 
> Sent: den 17 februari 2007 03:18
> To: joakim.tjernlund@transmode.se
> Cc: Kumar Gala; spi-devel-general@lists.sourceforge.net; 
> 'linuxppc-dev Development'
> Subject: Re: [spi-devel-general] [PATCH] Adapt spi_mpc83xx 
> SPI driver for 832x
> 
> On Thursday 14 December 2006 2:02 am, Joakim Tjernlund wrote:
> 
> > patch needs a litte work, the shift is diffrent for tx 
> resp. rx so I 
> > adjusted that and added a temp hack(still in patch below) 
> to force qe_mode.
> > The name qe_mode is misleading. This i a QE enabled CPU, 
> but SPI is running
> > in cpu mode. qe_mode implies that SPI is runing in native QUICC 
> > mode.
> > 
> > Adjusted patch(lets hope I didn't mangle this one:):
> 
> What's the story on this?  I see that no followup patch has 
> been submitted,
> so this 832x support is still not merged ...
> 
> I'm deleting all this 83xx history from my "pending" mbox, so 
> if anyone
> wants this support in the mainstream kernel, this is the only reminder
> you'll get from me.
> 
> - Dave

[snip code]

I think Kumar wanted to clean it up before submission.
Something needs to be added to the device tree I think.

 Jocke

Re: [spi-devel-general] [PATCH] Adapt spi_mpc83xx SPI driver for 832x

[David Brownell]

On Wednesday 13 December 2006 4:13 pm, Joakim Tjernlund wrote:
>
> > The problem I have with this patch is that it has too much 
> > #ifdeffery.  If
> 
> Too much? There is only 2 of them.

But it's the wrong kind of  #ifdeffery, and that's avoidable.


> > > (possibly this can be reduced by adding more logic to the 
> > > tx_buf/rx_buf functions)
> > > not to mention what will happen when support for reversed 
> > > bit order is added.
> > 
> > Sure enough, sounds ugly.  But cpu_is_xxx() macros, combined 
> > with GCC dead
> > code elimination will strip out functions that are unused, so 
> > that e.g.
> > 
> > 	if (cpu_is_mpc834x())
> > 		fn = mpc834x_spi_tx_buf_u16;
> > 	else if (cpu_is_mpc832x())
> > 		fn = mpc832x_spi_tx_buf_u16;
> > 
> > would only link one of them unless the kernel supports both SOCs.  And
> > without in-driver #ifdeffery.
> 
> Hmm, I can't find any cpu_is_xxx macros/functions. I guess the
> infrastructure isn't there yet?

So it would seem.    At least for PPC.  I noticed the patch from
Kumar Gala, which takes an alternate approach based on parameters
passed through platform_data ... that works too, in terms of
code being cleaner.

- Dave