Re: [PATCH 3/3] [V2] mpc52xx_spi: add gpio chipselect
On Fri, Nov 13, 2009 at 4:10 AM, Wolfram Sang 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 >> @@ -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. ___ 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
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 Just the kfree-comment, otherwise: Acked-by: Wolfram Sang > --- > 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 > #include > #include > +#include > #include > #include > > @@ -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 aw
[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 --- 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 #include #include +#include #include #include @@ -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); +