2009-11-10 09:12:28

by Luotao Fu

[permalink] [raw]
Subject: Patches for mpc52xx_spi


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


2009-11-10 09:12:58

by Luotao Fu

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

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 <[email protected]>
---
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

2009-11-10 09:12:38

by Luotao Fu

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

Signed-off-by: Luotao Fu <[email protected]>
---
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

2009-11-10 09:13:01

by Luotao Fu

[permalink] [raw]
Subject: [PATCH 3/3] mpc52xx_spi: add gpio chipselect

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 <[email protected]>
---
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

2009-11-11 09:48:51

by Wolfram Sang

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

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 <[email protected]>

Nit below, otherwise:

Acked-by: Wolfram Sang <[email protected]>

> ---
> 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
> [email protected]
> https://lists.ozlabs.org/listinfo/linuxppc-dev

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


Attachments:
(No filename) (2.53 kB)
signature.asc (197.00 B)
Digital signature
Download all attachments

2009-11-11 10:39:04

by Wolfram Sang

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

On Tue, Nov 10, 2009 at 10:12:08AM +0100, Luotao Fu wrote:
> Signed-off-by: Luotao Fu <[email protected]>

SPI_CS_HIGH should be dropped, otherwise:

Reviewed-by: Wolfram Sang <[email protected]>

> ---
> 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
> [email protected]
> https://lists.ozlabs.org/listinfo/linuxppc-dev

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


Attachments:
(No filename) (1.14 kB)
signature.asc (197.00 B)
Digital signature
Download all attachments

2009-11-11 10:50:50

by Wolfram Sang

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

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 <[email protected]>
> ---
> 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
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/spi-devel-general

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


Attachments:
(No filename) (5.07 kB)
signature.asc (197.00 B)
Digital signature
Download all attachments

2009-11-13 10:42:59

by Luotao Fu

[permalink] [raw]
Subject: [V2] Patches for mpc52xx_spi


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

2009-11-13 10:42:31

by Luotao Fu

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

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 <[email protected]>
Acked-by: Wolfram Sang <[email protected]>
---
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

2009-11-13 10:42:24

by Luotao Fu

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

V2 changes:
* remove CS_HIGH mode

Signed-off-by: Luotao Fu <[email protected]>
Acked-by: Wolfram Sang <[email protected]>
---
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

2009-11-13 10:42:44

by Luotao Fu

[permalink] [raw]
Subject: [PATCH 3/3] [V2] mpc52xx_spi: add gpio chipselect

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 <[email protected]>
---
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

2009-11-13 11:10:37

by Wolfram Sang

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

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 <[email protected]>

Just the kfree-comment, otherwise:

Acked-by: Wolfram Sang <[email protected]>

> ---
> 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
> [email protected]
> https://lists.ozlabs.org/listinfo/linuxppc-dev

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


Attachments:
(No filename) (5.14 kB)
signature.asc (197.00 B)
Digital signature
Download all attachments

2009-11-13 18:22:52

by Grant Likely

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

On Fri, Nov 13, 2009 at 4:10 AM, Wolfram Sang <[email protected]> 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 <[email protected]>

>> @@ -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.

2009-11-13 18:53:43

by Grant Likely

[permalink] [raw]
Subject: Re: [V2] Patches for mpc52xx_spi

On Fri, Nov 13, 2009 at 3:41 AM, Luotao Fu <[email protected]> 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.