[patch 2.6.25-git] spi_mpc83xx much improved driver

David Brownell

author

2008-04-30 22:37 UTC #0 lore raw

PATCH 1/1 spi_mpc83xx much improved driver

From: Joakim Tjernlund <[email protected]>
Date: Fri, 11 Apr 2008 18:57:21 +0200
Subject: [PATCH] Much improved mpc83xx SPI driver.

The current driver may cause glitches on SPI CLK line since one must disable the SPI controller before changing any HW settings. Fix this by implementing a local spi_transfer function that won't change speed and/or word size while CS is active.

While doing that heavy lifting a few other issues were addressed too:
- Make word size 16 and 32 work too.
- Honor bits_per_word and speed_hz in spi transaction.
- Optimize the common path.

This also stops using the "bitbang" framework (except for a few constants).

Signed-off-by Joakim Tjernlund <[email protected]>

[ Roel Kluin <[email protected]>: "irq" needs to be signed ]

Signed-off-by David Brownell <[email protected]>

---
 drivers/spi/Kconfig       |    1 
 drivers/spi/spi_mpc83xx.c |  409 +++++++++++++++++++++++++++++++---------------
 2 files changed, 281 insertions(+), 129 deletions(-)

drivers/spi/Kconfig +0 -1

@@ -126,7 +126,6 @@ config SPI_MPC52xx_PSC
 config SPI_MPC83xx
 	tristate "Freescale MPC83xx/QUICC Engine SPI controller"
 	depends on SPI_MASTER && (PPC_83xx || QUICC_ENGINE) && EXPERIMENTAL
-	select SPI_BITBANG
 	help
 	  This enables using the Freescale MPC83xx and QUICC Engine SPI
 	  controllers in master mode.

drivers/spi/spi_mpc83xx.c +281 ~~-128~~

560 lines not on this page · raw message

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Andrew Morton

2008-04-30 22:46 UTC #2 lore raw

Quoted 19 lines · On Wed, 30 Apr 2008 15:37:07 -0700 David Brownell…

From: Joakim Tjernlund <[email protected]>
Date: Fri, 11 Apr 2008 18:57:21 +0200
Subject: [PATCH] Much improved mpc83xx SPI driver.
The current driver may cause glitches on SPI CLK line since one must disable the SPI controller before changing any HW settings. Fix this by implementing a local spi_transfer function that won't change speed and/or word size while CS is active.
While doing that heavy lifting a few other issues were addressed too:
- Make word size 16 and 32 work too.
- Honor bits_per_word and speed_hz in spi transaction.
- Optimize the common path.
This also stops using the "bitbang" framework (except for a few constants).
...

drivers/spi/spi_mpc83xx.c int mpc83xx_spi_setup_transfer(struct sp · line 342

Quoted 6 lines

drivers/spi/spi_mpc83xx.c +6 -0

+static void mpc83xx_spi_work(struct work_struct *work)
+{
+	struct mpc83xx_spi *mpc83xx_spi =
+		container_of(work, struct mpc83xx_spi, work);
+
+	spin_lock_irq(&mpc83xx_spi->lock);

irq-safe.

drivers/spi/spi_mpc83xx.c static int mpc83xx_spi_setup(struct spi_ · line 469

Quoted 3 lines

...
drivers/spi/spi_mpc83xx.c +1 -0
+	spin_lock(&mpc83xx_spi->lock);

not irq-safe.

Deliberate?

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

David Brownell

author

2008-04-30 22:55 UTC #4 lore raw

Quoted 11 lines · Andrew Morton

Quoted 1 line
+ spin_lock_irq(&mpc83xx_spi->lock);
irq-safe.
Quoted 3 lines
...
+ spin_lock(&mpc83xx_spi->lock);
not irq-safe.
Deliberate?

That latter one is inside an #if 0/#endif block, so it won't matter. The #if 0 block bothered me. Maybe Joakim can remove it.

(By the way, I'd feel safer about this patch if there were an ack or two from more PPC folk...)

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Joakim Tjernlund

2008-05-01 10:42 UTC #5 lore raw

Quoted 15 lines

-----Original Message-----
From: David Brownell [mailto:[email protected]]
Sent: den 1 maj 2008 00:56
To: Andrew Morton
Cc: [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [patch 2.6.25-git] spi_mpc83xx much improved driver
Quoted 6 lines · Andrew Morton
Quoted 1 line
+ spin_lock_irq(&mpc83xx_spi->lock);
irq-safe.
Quoted 2 lines
...

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
+ spin_lock(&mpc83xx_spi->lock);
not irq-safe.
Deliberate?
That latter one is inside an #if 0/#endif block, so it won't matter. The #if 0 block bothered me. Maybe Joakim can remove it.

The non irq-safe spin_lock was there before my patch and now it is removed with the #if 0/#endif block. I don't need this code but I left it there disabled to make it clear for everyone else that it was removed. I can remove it but I rather leave it there for one release so everyone will have a chance to see it and object if they feel differently.

Quoted 3 lines

(By the way, I'd feel safer about this patch if there were an ack or two from more PPC folk...)

Yeah, that would be good.

Jocke

Joakim Tjernlund

2008-05-01 10:42 UTC #7 lore raw

Quoted 15 lines

-----Original Message-----
From: David Brownell [mailto:david-b-yBeKhBN/[email protected]]
Sent: den 1 maj 2008 00:56
To: Andrew Morton
Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; [email protected]; [email protected]; [email protected]
Subject: Re: [patch 2.6.25-git] spi_mpc83xx much improved driver
Quoted 6 lines · Andrew Morton
Quoted 1 line
+ spin_lock_irq(&mpc83xx_spi->lock);
irq-safe.
Quoted 2 lines
...

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
+ spin_lock(&mpc83xx_spi->lock);
not irq-safe.
Deliberate?
That latter one is inside an #if 0/#endif block, so it won't matter. The #if 0 block bothered me. Maybe Joakim can remove it.

The non irq-safe spin_lock was there before my patch and now it is removed with the #if 0/#endif block. I don't need this code but I left it there disabled to make it clear for everyone else that it was removed. I can remove it but I rather leave it there for one release so everyone will have a chance to see it and object if they feel differently.

Quoted 3 lines

(By the way, I'd feel safer about this patch if there were an ack or two from more PPC folk...)

Yeah, that would be good.

Jocke

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Joakim Tjernlund

2008-05-01 10:42 UTC #8 lore raw

In reply to David Brownell

Quoted 5 lines

-----Original Message-----
From: David Brownell [mailto:[email protected]]
Sent: den 1 maj 2008 00:56
To: Andrew Morton
Cc: [email protected]; =

[email protected]; [email protected];

Quoted 10 lines

[email protected]
Subject: Re: [patch 2.6.25-git] spi_mpc83xx much improved driver =20
Quoted 6 lines · Andrew Morton
Quoted 1 line
+=A0=A0=A0=A0=A0spin_lock_irq(&mpc83xx_spi->lock);
irq-safe.
Quoted 2 lines
...

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
+=A0=A0=A0=A0=A0spin_lock(&mpc83xx_spi->lock);
not irq-safe.
Deliberate?
=20
That latter one is inside an #if 0/#endif block, so it won't matter. The #if 0 block bothered me. Maybe Joakim can remove it.

The non irq-safe spin_lock was there before my patch and now it is removed with the #if 0/#endif block. I don't need this code but I left it there disabled to make it clear for everyone else that it was removed. I can remove it but I rather leave it there for one release so everyone will have a chance to see it and object if they feel differently.

Quoted 3 lines

=20
(By the way, I'd feel safer about this patch if there were an ack or two from more PPC folk...)

Yeah, that would be good.

Jocke