Patches for mpc52xx_spi

Patches for mpc52xx_spi

[Luotao Fu]

Hi Grant,

here are several patches for the dedicated spi controller on mpc5200 SOC. The
patchset contains two fixes and an enhancement. I noticed that your original V4
Patch is still pending for mainline. So I made the patches against the latest
version in your -next tree. Tested on a mpc5200b machine with 7 spi devices,
(cs lines are conntected to gpios) on 2.6.31, should work with latest kernel
also. Please review.

cheers
Luotao Fu

[PATCH 1/3] mpc52xx_spi: fix clearing status register

[Luotao Fu]

Before reading status register to check MODF failure, we have to clear it
first since the MODF flag will be set after initializing the spi master,
if the hardware comes up with a low SS. The processor datasheet reads:
Mode Fault flag -- bit sets if SS input goes low while SPI is configured as a
master. Flag is cleared automatically by an SPI status register read (with MODF
set) followed by a SPI control register 1 write.
Hence simply rereading the register is not sufficient to clear the flag. We
redo the write also to make sure to clear the flag.

Signed-off-by: Luotao Fu <l.fu-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/spi/mpc52xx_spi.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/spi/mpc52xx_spi.c b/drivers/spi/mpc52xx_spi.c
index ef8379b..5b036f2 100644
--- a/drivers/spi/mpc52xx_spi.c
+++ b/drivers/spi/mpc52xx_spi.c
@@ -391,6 +391,7 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
 	struct mpc52xx_spi *ms;
 	void __iomem *regs;
 	int rc;
+	int ctrl1;
 
 	/* MMIO registers */
 	dev_dbg(&op->dev, "probing mpc5200 SPI device\n");
@@ -399,7 +400,8 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
 		return -ENODEV;
 
 	/* initialize the device */
-	out_8(regs+SPI_CTRL1, SPI_CTRL1_SPIE | SPI_CTRL1_SPE | SPI_CTRL1_MSTR);
+	ctrl1 = SPI_CTRL1_SPIE | SPI_CTRL1_SPE | SPI_CTRL1_MSTR;
+	out_8(regs + SPI_CTRL1, ctrl1);
 	out_8(regs + SPI_CTRL2, 0x0);
 	out_8(regs + SPI_DATADIR, 0xe);	/* Set output pins */
 	out_8(regs + SPI_PORTDATA, 0x8);	/* Deassert /SS signal */
@@ -409,6 +411,8 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
 	 * on the SPI bus.  This fault will also occur if the SPI signals
 	 * are not connected to any pins (port_config setting) */
 	in_8(regs + SPI_STATUS);
+	out_8(regs + SPI_CTRL1, ctrl1);
+
 	in_8(regs + SPI_DATA);
 	if (in_8(regs + SPI_STATUS) & SPI_STATUS_MODF) {
 		dev_err(&op->dev, "mode fault; is port_config correct?\n");
-- 
1.6.5.2


------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july

[PATCH 2/3] mpc52xx_spi: add missing mode_bits definition

[Luotao Fu]

Signed-off-by: Luotao Fu <l.fu@pengutronix.de>
---
 drivers/spi/mpc52xx_spi.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/spi/mpc52xx_spi.c b/drivers/spi/mpc52xx_spi.c
index 5b036f2..79ba678 100644
--- a/drivers/spi/mpc52xx_spi.c
+++ b/drivers/spi/mpc52xx_spi.c
@@ -430,6 +430,8 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
 	master->num_chipselect = 1;
 	master->setup = mpc52xx_spi_setup;
 	master->transfer = mpc52xx_spi_transfer;
+	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LSB_FIRST;
+
 	dev_set_drvdata(&op->dev, master);
 
 	ms = spi_master_get_devdata(master);
-- 
1.6.5.2

[PATCH 3/3] mpc52xx_spi: add gpio chipselect

[Luotao Fu]

This one enables the mpc52xx_spi driver for usage of user defined gpio lines
as chipselect. This way we can control some more spi devices than only one

Signed-off-by: Luotao Fu <l.fu-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/spi/mpc52xx_spi.c |   57 +++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/mpc52xx_spi.c b/drivers/spi/mpc52xx_spi.c
index 79ba678..58c2ce5 100644
--- a/drivers/spi/mpc52xx_spi.c
+++ b/drivers/spi/mpc52xx_spi.c
@@ -21,6 +21,7 @@
 #include <linux/spi/mpc52xx_spi.h>
 #include <linux/of_spi.h>
 #include <linux/io.h>
+#include <linux/of_gpio.h>
 #include <asm/time.h>
 #include <asm/mpc52xx.h>
 
@@ -79,7 +80,6 @@ struct mpc52xx_spi {
 	spinlock_t lock;
 	struct work_struct work;
 
-
 	/* Details of current transfer (length, and buffer pointers) */
 	struct spi_message *message;	/* current message */
 	struct spi_transfer *transfer;	/* current transfer */
@@ -89,6 +89,8 @@ struct mpc52xx_spi {
 	u8 *rx_buf;
 	const u8 *tx_buf;
 	int cs_change;
+	int gpio_cs_count;
+	unsigned int *gpio_cs;
 };
 
 /*
@@ -96,7 +98,13 @@ struct mpc52xx_spi {
  */
 static void mpc52xx_spi_chipsel(struct mpc52xx_spi *ms, int value)
 {
-	out_8(ms->regs + SPI_PORTDATA, value ? 0 : 0x08);
+	int cs;
+
+	if (ms->gpio_cs_count > 0) {
+		cs = ms->message->spi->chip_select;
+		gpio_direction_output(ms->gpio_cs[cs], value);
+	} else
+		out_8(ms->regs + SPI_PORTDATA, value ? 0 : 0x08);
 }
 
 /*
@@ -390,8 +398,9 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
 	struct spi_master *master;
 	struct mpc52xx_spi *ms;
 	void __iomem *regs;
-	int rc;
 	int ctrl1;
+	int rc, i = 0;
+	int gpio_cs;
 
 	/* MMIO registers */
 	dev_dbg(&op->dev, "probing mpc5200 SPI device\n");
@@ -426,8 +435,8 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
 		rc = -ENOMEM;
 		goto err_alloc;
 	}
+
 	master->bus_num = -1;
-	master->num_chipselect = 1;
 	master->setup = mpc52xx_spi_setup;
 	master->transfer = mpc52xx_spi_transfer;
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LSB_FIRST;
@@ -441,6 +450,39 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
 	ms->irq1 = irq_of_parse_and_map(op->node, 1);
 	ms->state = mpc52xx_spi_fsmstate_idle;
 	ms->ipb_freq = mpc5xxx_get_bus_frequency(op->node);
+	ms->gpio_cs_count = of_gpio_count(op->node);
+	if (ms->gpio_cs_count > 0) {
+		master->num_chipselect = ms->gpio_cs_count;
+		ms->gpio_cs = kmalloc(ms->gpio_cs_count * sizeof(unsigned int),
+				GFP_KERNEL);
+		if (!ms->gpio_cs) {
+			rc = -ENOMEM;
+			goto err_alloc;
+		}
+
+		for (i = 0; i < ms->gpio_cs_count; i++) {
+			gpio_cs = of_get_gpio(op->node, i);
+			if (gpio_cs < 0) {
+				dev_err(&op->dev,
+					"could not parse the gpio field "
+					"in oftree\n");
+				rc = -ENODEV;
+				goto err_alloc;
+			}
+
+			ms->gpio_cs[i] = gpio_cs;
+			rc = gpio_request(ms->gpio_cs[i], dev_name(&op->dev));
+			if (rc) {
+				dev_err(&op->dev,
+					"can't request spi cs gpio #%d "
+					"on gpio line %d\n",
+					i, gpio_cs);
+				goto err_gpio;
+			}
+		}
+	} else
+		master->num_chipselect = 1;
+
 	spin_lock_init(&ms->lock);
 	INIT_LIST_HEAD(&ms->queue);
 	INIT_WORK(&ms->work, mpc52xx_spi_wq);
@@ -477,6 +519,9 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
  err_register:
 	dev_err(&ms->master->dev, "initialization failed\n");
 	spi_master_put(master);
+ err_gpio:
+	while (i-- > 0)
+		gpio_free(ms->gpio_cs[i]);
  err_alloc:
  err_init:
 	iounmap(regs);
@@ -487,10 +532,14 @@ static int __devexit mpc52xx_spi_remove(struct of_device *op)
 {
 	struct spi_master *master = dev_get_drvdata(&op->dev);
 	struct mpc52xx_spi *ms = spi_master_get_devdata(master);
+	int i;
 
 	free_irq(ms->irq0, ms);
 	free_irq(ms->irq1, ms);
 
+	for (i = 0; i < ms->gpio_cs_count; i++)
+		gpio_free(ms->gpio_cs[i]);
+
 	spi_unregister_master(master);
 	spi_master_put(master);
 	iounmap(ms->regs);
-- 
1.6.5.2


------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july

Re: [PATCH 1/3] mpc52xx_spi: fix clearing status register

[Wolfram Sang]

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

On Tue, Nov 10, 2009 at 10:12:07AM +0100, Luotao Fu wrote:
> Before reading status register to check MODF failure, we have to clear it
> first since the MODF flag will be set after initializing the spi master,
> if the hardware comes up with a low SS. The processor datasheet reads:
> Mode Fault flag -- bit sets if SS input goes low while SPI is configured as a
> master. Flag is cleared automatically by an SPI status register read (with MODF
> set) followed by a SPI control register 1 write.
> Hence simply rereading the register is not sufficient to clear the flag. We
> redo the write also to make sure to clear the flag.
> 
> Signed-off-by: Luotao Fu <l.fu@pengutronix.de>

Nit below, otherwise:

Acked-by: Wolfram Sang <w.sang@pengutronix.de>

> ---
>  drivers/spi/mpc52xx_spi.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/spi/mpc52xx_spi.c b/drivers/spi/mpc52xx_spi.c
> index ef8379b..5b036f2 100644
> --- a/drivers/spi/mpc52xx_spi.c
> +++ b/drivers/spi/mpc52xx_spi.c
> @@ -391,6 +391,7 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
>  	struct mpc52xx_spi *ms;
>  	void __iomem *regs;
>  	int rc;
> +	int ctrl1;

u8?

>  
>  	/* MMIO registers */
>  	dev_dbg(&op->dev, "probing mpc5200 SPI device\n");
> @@ -399,7 +400,8 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
>  		return -ENODEV;
>  

Sidenote to all: It was tested that simply moving the read here will not suffice.

>  	/* initialize the device */
> -	out_8(regs+SPI_CTRL1, SPI_CTRL1_SPIE | SPI_CTRL1_SPE | SPI_CTRL1_MSTR);
> +	ctrl1 = SPI_CTRL1_SPIE | SPI_CTRL1_SPE | SPI_CTRL1_MSTR;
> +	out_8(regs + SPI_CTRL1, ctrl1);
>  	out_8(regs + SPI_CTRL2, 0x0);
>  	out_8(regs + SPI_DATADIR, 0xe);	/* Set output pins */
>  	out_8(regs + SPI_PORTDATA, 0x8);	/* Deassert /SS signal */
> @@ -409,6 +411,8 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
>  	 * on the SPI bus.  This fault will also occur if the SPI signals
>  	 * are not connected to any pins (port_config setting) */
>  	in_8(regs + SPI_STATUS);
> +	out_8(regs + SPI_CTRL1, ctrl1);
> +
>  	in_8(regs + SPI_DATA);
>  	if (in_8(regs + SPI_STATUS) & SPI_STATUS_MODF) {
>  		dev_err(&op->dev, "mode fault; is port_config correct?\n");
> -- 
> 1.6.5.2
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 2/3] mpc52xx_spi: add missing mode_bits definition

[Wolfram Sang]

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

On Tue, Nov 10, 2009 at 10:12:08AM +0100, Luotao Fu wrote:
> Signed-off-by: Luotao Fu <l.fu@pengutronix.de>

SPI_CS_HIGH should be dropped, otherwise:

Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>

> ---
>  drivers/spi/mpc52xx_spi.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/spi/mpc52xx_spi.c b/drivers/spi/mpc52xx_spi.c
> index 5b036f2..79ba678 100644
> --- a/drivers/spi/mpc52xx_spi.c
> +++ b/drivers/spi/mpc52xx_spi.c
> @@ -430,6 +430,8 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
>  	master->num_chipselect = 1;
>  	master->setup = mpc52xx_spi_setup;
>  	master->transfer = mpc52xx_spi_transfer;
> +	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LSB_FIRST;
> +
>  	dev_set_drvdata(&op->dev, master);
>  
>  	ms = spi_master_get_devdata(master);
> -- 
> 1.6.5.2
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [spi-devel-general] [PATCH 3/3] mpc52xx_spi: add gpio chipselect

[Wolfram Sang]

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

On Tue, Nov 10, 2009 at 10:12:09AM +0100, Luotao Fu wrote:
> This one enables the mpc52xx_spi driver for usage of user defined gpio lines
> as chipselect. This way we can control some more spi devices than only one
> 
> Signed-off-by: Luotao Fu <l.fu@pengutronix.de>
> ---
>  drivers/spi/mpc52xx_spi.c |   57 +++++++++++++++++++++++++++++++++++++++++---
>  1 files changed, 53 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/mpc52xx_spi.c b/drivers/spi/mpc52xx_spi.c
> index 79ba678..58c2ce5 100644
> --- a/drivers/spi/mpc52xx_spi.c
> +++ b/drivers/spi/mpc52xx_spi.c
> @@ -21,6 +21,7 @@
>  #include <linux/spi/mpc52xx_spi.h>
>  #include <linux/of_spi.h>
>  #include <linux/io.h>
> +#include <linux/of_gpio.h>
>  #include <asm/time.h>
>  #include <asm/mpc52xx.h>
>  
> @@ -79,7 +80,6 @@ struct mpc52xx_spi {
>  	spinlock_t lock;
>  	struct work_struct work;
>  
> -
>  	/* Details of current transfer (length, and buffer pointers) */
>  	struct spi_message *message;	/* current message */
>  	struct spi_transfer *transfer;	/* current transfer */
> @@ -89,6 +89,8 @@ struct mpc52xx_spi {
>  	u8 *rx_buf;
>  	const u8 *tx_buf;
>  	int cs_change;
> +	int gpio_cs_count;
> +	unsigned int *gpio_cs;
>  };
>  
>  /*
> @@ -96,7 +98,13 @@ struct mpc52xx_spi {
>   */
>  static void mpc52xx_spi_chipsel(struct mpc52xx_spi *ms, int value)
>  {
> -	out_8(ms->regs + SPI_PORTDATA, value ? 0 : 0x08);
> +	int cs;
> +
> +	if (ms->gpio_cs_count > 0) {
> +		cs = ms->message->spi->chip_select;
> +		gpio_direction_output(ms->gpio_cs[cs], value);

Use gpio_set_value() in combination with...

> +	} else
> +		out_8(ms->regs + SPI_PORTDATA, value ? 0 : 0x08);
>  }
>  
>  /*
> @@ -390,8 +398,9 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
>  	struct spi_master *master;
>  	struct mpc52xx_spi *ms;
>  	void __iomem *regs;
> -	int rc;
>  	int ctrl1;
> +	int rc, i = 0;
> +	int gpio_cs;
>  
>  	/* MMIO registers */
>  	dev_dbg(&op->dev, "probing mpc5200 SPI device\n");
> @@ -426,8 +435,8 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
>  		rc = -ENOMEM;
>  		goto err_alloc;
>  	}
> +
>  	master->bus_num = -1;
> -	master->num_chipselect = 1;
>  	master->setup = mpc52xx_spi_setup;
>  	master->transfer = mpc52xx_spi_transfer;
>  	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LSB_FIRST;
> @@ -441,6 +450,39 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
>  	ms->irq1 = irq_of_parse_and_map(op->node, 1);
>  	ms->state = mpc52xx_spi_fsmstate_idle;
>  	ms->ipb_freq = mpc5xxx_get_bus_frequency(op->node);
> +	ms->gpio_cs_count = of_gpio_count(op->node);
> +	if (ms->gpio_cs_count > 0) {
> +		master->num_chipselect = ms->gpio_cs_count;
> +		ms->gpio_cs = kmalloc(ms->gpio_cs_count * sizeof(unsigned int),
> +				GFP_KERNEL);
> +		if (!ms->gpio_cs) {
> +			rc = -ENOMEM;
> +			goto err_alloc;
> +		}
> +
> +		for (i = 0; i < ms->gpio_cs_count; i++) {
> +			gpio_cs = of_get_gpio(op->node, i);

... with of_get_gpio_flags() to avoid initialisation jitter?

> +			if (gpio_cs < 0) {
> +				dev_err(&op->dev,
> +					"could not parse the gpio field "
> +					"in oftree\n");
> +				rc = -ENODEV;
> +				goto err_alloc;

That should go to err_gpio and unregister the previous allocated ones.

> +			}
> +
> +			ms->gpio_cs[i] = gpio_cs;
> +			rc = gpio_request(ms->gpio_cs[i], dev_name(&op->dev));
> +			if (rc) {
> +				dev_err(&op->dev,
> +					"can't request spi cs gpio #%d "
> +					"on gpio line %d\n",
> +					i, gpio_cs);

Last two lines could become one.

> +				goto err_gpio;
> +			}
> +		}
> +	} else
> +		master->num_chipselect = 1;
> +
>  	spin_lock_init(&ms->lock);
>  	INIT_LIST_HEAD(&ms->queue);
>  	INIT_WORK(&ms->work, mpc52xx_spi_wq);
> @@ -477,6 +519,9 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
>   err_register:
>  	dev_err(&ms->master->dev, "initialization failed\n");
>  	spi_master_put(master);
> + err_gpio:
> +	while (i-- > 0)
> +		gpio_free(ms->gpio_cs[i]);
>   err_alloc:
>   err_init:
>  	iounmap(regs);
> @@ -487,10 +532,14 @@ static int __devexit mpc52xx_spi_remove(struct of_device *op)
>  {
>  	struct spi_master *master = dev_get_drvdata(&op->dev);
>  	struct mpc52xx_spi *ms = spi_master_get_devdata(master);
> +	int i;
>  
>  	free_irq(ms->irq0, ms);
>  	free_irq(ms->irq1, ms);
>  
> +	for (i = 0; i < ms->gpio_cs_count; i++)
> +		gpio_free(ms->gpio_cs[i]);
> +
>  	spi_unregister_master(master);
>  	spi_master_put(master);
>  	iounmap(ms->regs);
> -- 
> 1.6.5.2
> 
> 
> ------------------------------------------------------------------------------
> Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
> trial. Simplify your report design, integration and deployment - and focus on 
> what you do best, core application coding. Discover what's new with
> Crystal Reports now.  http://p.sf.net/sfu/bobj-july
> _______________________________________________
> spi-devel-general mailing list
> spi-devel-general@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/spi-devel-general

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[V2] Patches for mpc52xx_spi

[Luotao Fu]

Hi Grand,

here are the V2 of my patchset for mpc52xx_spi. Besides Wolframs suggestions I
also added some more fixes. Details of changes can be checked in the patch
header. Please do consider to apply the patches. The patch 1/2 e.g. fix bugs,
which prevent the driver to work in some cases, as seen on my board.

cheers
Luotao Fu

[PATCH 1/3] [V2] mpc52xx_spi: fix clearing status register

[Luotao Fu]

Before reading status register to check MODF failure, we have to clear it
first since the MODF flag will be set after initializing the spi master,
if the hardware comes up with a low SS. The processor datasheet reads:
Mode Fault flag -- bit sets if SS input goes low while SPI is configured as a
master. Flag is cleared automatically by an SPI status register read (with MODF
set) followed by a SPI control register 1 write.
Hence simply rereading the register is not sufficient to clear the flag. We
redo the write also to make sure to clear the flag.

V2 Changes:
* change variable type from int to u8

Signed-off-by: Luotao Fu <l.fu-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Acked-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/spi/mpc52xx_spi.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/spi/mpc52xx_spi.c b/drivers/spi/mpc52xx_spi.c
index ef8379b..2322250 100644
--- a/drivers/spi/mpc52xx_spi.c
+++ b/drivers/spi/mpc52xx_spi.c
@@ -391,6 +391,7 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
 	struct mpc52xx_spi *ms;
 	void __iomem *regs;
 	int rc;
+	u8 ctrl1;
 
 	/* MMIO registers */
 	dev_dbg(&op->dev, "probing mpc5200 SPI device\n");
@@ -399,7 +400,8 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
 		return -ENODEV;
 
 	/* initialize the device */
-	out_8(regs+SPI_CTRL1, SPI_CTRL1_SPIE | SPI_CTRL1_SPE | SPI_CTRL1_MSTR);
+	ctrl1 = SPI_CTRL1_SPIE | SPI_CTRL1_SPE | SPI_CTRL1_MSTR;
+	out_8(regs + SPI_CTRL1, ctrl1);
 	out_8(regs + SPI_CTRL2, 0x0);
 	out_8(regs + SPI_DATADIR, 0xe);	/* Set output pins */
 	out_8(regs + SPI_PORTDATA, 0x8);	/* Deassert /SS signal */
@@ -409,6 +411,8 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
 	 * on the SPI bus.  This fault will also occur if the SPI signals
 	 * are not connected to any pins (port_config setting) */
 	in_8(regs + SPI_STATUS);
+	out_8(regs + SPI_CTRL1, ctrl1);
+
 	in_8(regs + SPI_DATA);
 	if (in_8(regs + SPI_STATUS) & SPI_STATUS_MODF) {
 		dev_err(&op->dev, "mode fault; is port_config correct?\n");
-- 
1.6.5.2


------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july

[PATCH 2/3] [V2] mpc52xx_spi: add missing mode_bits definition

[Luotao Fu]

V2 changes:
* remove CS_HIGH mode

Signed-off-by: Luotao Fu <l.fu@pengutronix.de>
Acked-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/spi/mpc52xx_spi.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/spi/mpc52xx_spi.c b/drivers/spi/mpc52xx_spi.c
index 2322250..64862cf 100644
--- a/drivers/spi/mpc52xx_spi.c
+++ b/drivers/spi/mpc52xx_spi.c
@@ -430,6 +430,8 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
 	master->num_chipselect = 1;
 	master->setup = mpc52xx_spi_setup;
 	master->transfer = mpc52xx_spi_transfer;
+	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LSB_FIRST;
+
 	dev_set_drvdata(&op->dev, master);
 
 	ms = spi_master_get_devdata(master);
-- 
1.6.5.2

[PATCH 3/3] [V2] mpc52xx_spi: add gpio chipselect

[Luotao Fu]

This one enables the mpc52xx_spi driver for usage of user defined gpio lines
as chipselect. This way we can control some more spi devices than only one

V2 Changes:
* preinitialize the gpio as output in probe function and call gpio_set_value in
  the chip select function instead of calling direction_output every time.
* initialize the gpio line with output high, since we don't support CS_HIGH
  in the driver currently any way. change gpio value setting to default active
  low in chip select call.
* free the gpio array while error or removing.

Signed-off-by: Luotao Fu <l.fu-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/spi/mpc52xx_spi.c |   64 ++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/mpc52xx_spi.c b/drivers/spi/mpc52xx_spi.c
index 64862cf..97beba2 100644
--- a/drivers/spi/mpc52xx_spi.c
+++ b/drivers/spi/mpc52xx_spi.c
@@ -21,6 +21,7 @@
 #include <linux/spi/mpc52xx_spi.h>
 #include <linux/of_spi.h>
 #include <linux/io.h>
+#include <linux/of_gpio.h>
 #include <asm/time.h>
 #include <asm/mpc52xx.h>
 
@@ -79,7 +80,6 @@ struct mpc52xx_spi {
 	spinlock_t lock;
 	struct work_struct work;
 
-
 	/* Details of current transfer (length, and buffer pointers) */
 	struct spi_message *message;	/* current message */
 	struct spi_transfer *transfer;	/* current transfer */
@@ -89,6 +89,8 @@ struct mpc52xx_spi {
 	u8 *rx_buf;
 	const u8 *tx_buf;
 	int cs_change;
+	int gpio_cs_count;
+	unsigned int *gpio_cs;
 };
 
 /*
@@ -96,7 +98,13 @@ struct mpc52xx_spi {
  */
 static void mpc52xx_spi_chipsel(struct mpc52xx_spi *ms, int value)
 {
-	out_8(ms->regs + SPI_PORTDATA, value ? 0 : 0x08);
+	int cs;
+
+	if (ms->gpio_cs_count > 0) {
+		cs = ms->message->spi->chip_select;
+		gpio_set_value(ms->gpio_cs[cs], value ? 0 : 1);
+	} else
+		out_8(ms->regs + SPI_PORTDATA, value ? 0 : 0x08);
 }
 
 /*
@@ -390,8 +398,9 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
 	struct spi_master *master;
 	struct mpc52xx_spi *ms;
 	void __iomem *regs;
-	int rc;
 	u8 ctrl1;
+	int rc, i = 0;
+	int gpio_cs;
 
 	/* MMIO registers */
 	dev_dbg(&op->dev, "probing mpc5200 SPI device\n");
@@ -426,8 +435,8 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
 		rc = -ENOMEM;
 		goto err_alloc;
 	}
+
 	master->bus_num = -1;
-	master->num_chipselect = 1;
 	master->setup = mpc52xx_spi_setup;
 	master->transfer = mpc52xx_spi_transfer;
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LSB_FIRST;
@@ -441,6 +450,40 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
 	ms->irq1 = irq_of_parse_and_map(op->node, 1);
 	ms->state = mpc52xx_spi_fsmstate_idle;
 	ms->ipb_freq = mpc5xxx_get_bus_frequency(op->node);
+	ms->gpio_cs_count = of_gpio_count(op->node);
+	if (ms->gpio_cs_count > 0) {
+		master->num_chipselect = ms->gpio_cs_count;
+		ms->gpio_cs = kmalloc(ms->gpio_cs_count * sizeof(unsigned int),
+				GFP_KERNEL);
+		if (!ms->gpio_cs) {
+			rc = -ENOMEM;
+			goto err_alloc;
+		}
+
+		for (i = 0; i < ms->gpio_cs_count; i++) {
+			gpio_cs = of_get_gpio(op->node, i);
+			if (gpio_cs < 0) {
+				dev_err(&op->dev,
+					"could not parse the gpio field "
+					"in oftree\n");
+				rc = -ENODEV;
+				goto err_gpio;
+			}
+
+			rc = gpio_request(gpio_cs, dev_name(&op->dev));
+			if (rc) {
+				dev_err(&op->dev,
+					"can't request spi cs gpio #%d "
+					"on gpio line %d\n", i, gpio_cs);
+				goto err_gpio;
+			}
+
+			gpio_direction_output(gpio_cs, 1);
+			ms->gpio_cs[i] = gpio_cs;
+		}
+	} else
+		master->num_chipselect = 1;
+
 	spin_lock_init(&ms->lock);
 	INIT_LIST_HEAD(&ms->queue);
 	INIT_WORK(&ms->work, mpc52xx_spi_wq);
@@ -477,6 +520,12 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
  err_register:
 	dev_err(&ms->master->dev, "initialization failed\n");
 	spi_master_put(master);
+ err_gpio:
+	while (i-- > 0)
+		gpio_free(ms->gpio_cs[i]);
+
+	if (ms->gpio_cs != NULL)
+		kfree(ms->gpio_cs);
  err_alloc:
  err_init:
 	iounmap(regs);
@@ -487,10 +536,17 @@ static int __devexit mpc52xx_spi_remove(struct of_device *op)
 {
 	struct spi_master *master = dev_get_drvdata(&op->dev);
 	struct mpc52xx_spi *ms = spi_master_get_devdata(master);
+	int i;
 
 	free_irq(ms->irq0, ms);
 	free_irq(ms->irq1, ms);
 
+	for (i = 0; i < ms->gpio_cs_count; i++)
+		gpio_free(ms->gpio_cs[i]);
+
+	if (ms->gpio_cs != NULL)
+		kfree(ms->gpio_cs);
+
 	spi_unregister_master(master);
 	spi_master_put(master);
 	iounmap(ms->regs);
-- 
1.6.5.2


------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july

Re: [PATCH 3/3] [V2] mpc52xx_spi: add gpio chipselect

[Wolfram Sang]

[-- Attachment #1.1: Type: text/plain, Size: 5437 bytes --]

On Fri, Nov 13, 2009 at 11:41:17AM +0100, Luotao Fu wrote:
> This one enables the mpc52xx_spi driver for usage of user defined gpio lines
> as chipselect. This way we can control some more spi devices than only one
> 
> V2 Changes:
> * preinitialize the gpio as output in probe function and call gpio_set_value in
>   the chip select function instead of calling direction_output every time.
> * initialize the gpio line with output high, since we don't support CS_HIGH
>   in the driver currently any way. change gpio value setting to default active
>   low in chip select call.
> * free the gpio array while error or removing.
> 
> Signed-off-by: Luotao Fu <l.fu@pengutronix.de>

Just the kfree-comment, otherwise:

Acked-by: Wolfram Sang <w.sang@pengutronix.de>

> ---
>  drivers/spi/mpc52xx_spi.c |   64 ++++++++++++++++++++++++++++++++++++++++++---
>  1 files changed, 60 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/mpc52xx_spi.c b/drivers/spi/mpc52xx_spi.c
> index 64862cf..97beba2 100644
> --- a/drivers/spi/mpc52xx_spi.c
> +++ b/drivers/spi/mpc52xx_spi.c
> @@ -21,6 +21,7 @@
>  #include <linux/spi/mpc52xx_spi.h>
>  #include <linux/of_spi.h>
>  #include <linux/io.h>
> +#include <linux/of_gpio.h>
>  #include <asm/time.h>
>  #include <asm/mpc52xx.h>
>  
> @@ -79,7 +80,6 @@ struct mpc52xx_spi {
>  	spinlock_t lock;
>  	struct work_struct work;
>  
> -
>  	/* Details of current transfer (length, and buffer pointers) */
>  	struct spi_message *message;	/* current message */
>  	struct spi_transfer *transfer;	/* current transfer */
> @@ -89,6 +89,8 @@ struct mpc52xx_spi {
>  	u8 *rx_buf;
>  	const u8 *tx_buf;
>  	int cs_change;
> +	int gpio_cs_count;
> +	unsigned int *gpio_cs;
>  };
>  
>  /*
> @@ -96,7 +98,13 @@ struct mpc52xx_spi {
>   */
>  static void mpc52xx_spi_chipsel(struct mpc52xx_spi *ms, int value)
>  {
> -	out_8(ms->regs + SPI_PORTDATA, value ? 0 : 0x08);
> +	int cs;
> +
> +	if (ms->gpio_cs_count > 0) {
> +		cs = ms->message->spi->chip_select;
> +		gpio_set_value(ms->gpio_cs[cs], value ? 0 : 1);
> +	} else
> +		out_8(ms->regs + SPI_PORTDATA, value ? 0 : 0x08);
>  }
>  
>  /*
> @@ -390,8 +398,9 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
>  	struct spi_master *master;
>  	struct mpc52xx_spi *ms;
>  	void __iomem *regs;
> -	int rc;
>  	u8 ctrl1;
> +	int rc, i = 0;
> +	int gpio_cs;
>  
>  	/* MMIO registers */
>  	dev_dbg(&op->dev, "probing mpc5200 SPI device\n");
> @@ -426,8 +435,8 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
>  		rc = -ENOMEM;
>  		goto err_alloc;
>  	}
> +
>  	master->bus_num = -1;
> -	master->num_chipselect = 1;
>  	master->setup = mpc52xx_spi_setup;
>  	master->transfer = mpc52xx_spi_transfer;
>  	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LSB_FIRST;
> @@ -441,6 +450,40 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
>  	ms->irq1 = irq_of_parse_and_map(op->node, 1);
>  	ms->state = mpc52xx_spi_fsmstate_idle;
>  	ms->ipb_freq = mpc5xxx_get_bus_frequency(op->node);
> +	ms->gpio_cs_count = of_gpio_count(op->node);
> +	if (ms->gpio_cs_count > 0) {
> +		master->num_chipselect = ms->gpio_cs_count;
> +		ms->gpio_cs = kmalloc(ms->gpio_cs_count * sizeof(unsigned int),
> +				GFP_KERNEL);
> +		if (!ms->gpio_cs) {
> +			rc = -ENOMEM;
> +			goto err_alloc;
> +		}
> +
> +		for (i = 0; i < ms->gpio_cs_count; i++) {
> +			gpio_cs = of_get_gpio(op->node, i);
> +			if (gpio_cs < 0) {
> +				dev_err(&op->dev,
> +					"could not parse the gpio field "
> +					"in oftree\n");
> +				rc = -ENODEV;
> +				goto err_gpio;
> +			}
> +
> +			rc = gpio_request(gpio_cs, dev_name(&op->dev));
> +			if (rc) {
> +				dev_err(&op->dev,
> +					"can't request spi cs gpio #%d "
> +					"on gpio line %d\n", i, gpio_cs);
> +				goto err_gpio;
> +			}
> +
> +			gpio_direction_output(gpio_cs, 1);
> +			ms->gpio_cs[i] = gpio_cs;
> +		}
> +	} else
> +		master->num_chipselect = 1;
> +
>  	spin_lock_init(&ms->lock);
>  	INIT_LIST_HEAD(&ms->queue);
>  	INIT_WORK(&ms->work, mpc52xx_spi_wq);
> @@ -477,6 +520,12 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
>   err_register:
>  	dev_err(&ms->master->dev, "initialization failed\n");
>  	spi_master_put(master);
> + err_gpio:
> +	while (i-- > 0)
> +		gpio_free(ms->gpio_cs[i]);
> +
> +	if (ms->gpio_cs != NULL)
> +		kfree(ms->gpio_cs);

kfree() is NULL aware, so no if needed.

>   err_alloc:
>   err_init:
>  	iounmap(regs);
> @@ -487,10 +536,17 @@ static int __devexit mpc52xx_spi_remove(struct of_device *op)
>  {
>  	struct spi_master *master = dev_get_drvdata(&op->dev);
>  	struct mpc52xx_spi *ms = spi_master_get_devdata(master);
> +	int i;
>  
>  	free_irq(ms->irq0, ms);
>  	free_irq(ms->irq1, ms);
>  
> +	for (i = 0; i < ms->gpio_cs_count; i++)
> +		gpio_free(ms->gpio_cs[i]);
> +
> +	if (ms->gpio_cs != NULL)
> +		kfree(ms->gpio_cs);

ditto.

> +
>  	spi_unregister_master(master);
>  	spi_master_put(master);
>  	iounmap(ms->regs);
> -- 
> 1.6.5.2
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 3/3] [V2] mpc52xx_spi: add gpio chipselect

[Grant Likely]

On Fri, Nov 13, 2009 at 4:10 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> On Fri, Nov 13, 2009 at 11:41:17AM +0100, Luotao Fu wrote:
>> This one enables the mpc52xx_spi driver for usage of user defined gpio lines
>> as chipselect. This way we can control some more spi devices than only one
>>
>> V2 Changes:
>> * preinitialize the gpio as output in probe function and call gpio_set_value in
>>   the chip select function instead of calling direction_output every time.
>> * initialize the gpio line with output high, since we don't support CS_HIGH
>>   in the driver currently any way. change gpio value setting to default active
>>   low in chip select call.
>> * free the gpio array while error or removing.
>>
>> Signed-off-by: Luotao Fu <l.fu@pengutronix.de>

>> @@ -477,6 +520,12 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
>>   err_register:
>>       dev_err(&ms->master->dev, "initialization failed\n");
>>       spi_master_put(master);
>> + err_gpio:
>> +     while (i-- > 0)
>> +             gpio_free(ms->gpio_cs[i]);
>> +
>> +     if (ms->gpio_cs != NULL)
>> +             kfree(ms->gpio_cs);
>
> kfree() is NULL aware, so no if needed.

Not dangerous though.  No need to respin just for this.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [V2] Patches for mpc52xx_spi

[Grant Likely]

On Fri, Nov 13, 2009 at 3:41 AM, Luotao Fu <l.fu@pengutronix.de> wrote:
>
> Hi Grand,
>
> here are the V2 of my patchset for mpc52xx_spi. Besides Wolframs suggestions I
> also added some more fixes. Details of changes can be checked in the patch
> header. Please do consider to apply the patches. The patch 1/2 e.g. fix bugs,
> which prevent the driver to work in some cases, as seen on my board.

Applied to my 'test' branch on git.secretlab.ca

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.