| Message ID | 20091028132137.GA17813@sig21.net |
|---|---|
| State | Accepted |
| Commit | 61c3506c2cabe58bcdfe438d1e57b62994db1616 |
| Headers | show |
On Wed, 2009-10-28 at 14:21 +0100, Johannes Stezenbach wrote: > spi_write() requires the buffer to be DMA-safe, kmalloc() > it seperately to ensure this. > > Signed-off-by: Johannes Stezenbach <js@sig21.net> > > > diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c > index 4c19269..21dd4d9 100644 > --- a/drivers/mtd/devices/m25p80.c > +++ b/drivers/mtd/devices/m25p80.c > @@ -79,7 +79,7 @@ struct m25p { > struct mtd_info mtd; > unsigned partitioned:1; > u8 erase_opcode; > - u8 command[CMD_SIZE + FAST_READ_DUMMY_BYTE]; > + u8 *command; > }; > > static inline struct m25p *mtd_to_m25p(struct mtd_info *mtd) > @@ -770,6 +770,11 @@ static int __devinit m25p_probe(struct spi_device *spi) > flash = kzalloc(sizeof *flash, GFP_KERNEL); > if (!flash) > return -ENOMEM; > + flash->command = kmalloc(CMD_SIZE + FAST_READ_DUMMY_BYTE, GFP_KERNEL); > + if (!flash->command) { > + kfree(flash); > + return -ENOMEM; > + } Even though it is just 4 or 5 bytes it can do DMA? Does not sound too sane to use DMA in that case. Does this patch fix a real error? I do not know much about SPI, but for me it sounds like there should be a method to ask SPI to avoid using DMA, and you should use that method.
On Tue, Nov 03, 2009 at 08:36:05AM +0200, Artem Bityutskiy wrote: > On Wed, 2009-10-28 at 14:21 +0100, Johannes Stezenbach wrote: > > spi_write() requires the buffer to be DMA-safe, kmalloc() > > it seperately to ensure this. > > Even though it is just 4 or 5 bytes it can do DMA? Does not sound too > sane to use DMA in that case. Does this patch fix a real error? > > I do not know much about SPI, but for me it sounds like there should be > a method to ask SPI to avoid using DMA, and you should use that method. It fixes a real error -- with an out-of-tree driver and ancient kernel. For the flash read/write case it is used with list-DMA (e.g. write 4 bytes, read 64KB). There are extensive comments in include/linux/spi/spi.h which document the DMA-safe requirement of the buffers. Thanks Johannes
On Tue, 2009-11-03 at 11:00 +0100, Johannes Stezenbach wrote: > On Tue, Nov 03, 2009 at 08:36:05AM +0200, Artem Bityutskiy wrote: > > On Wed, 2009-10-28 at 14:21 +0100, Johannes Stezenbach wrote: > > > spi_write() requires the buffer to be DMA-safe, kmalloc() > > > it seperately to ensure this. > > > > Even though it is just 4 or 5 bytes it can do DMA? Does not sound too > > sane to use DMA in that case. Does this patch fix a real error? > > > > I do not know much about SPI, but for me it sounds like there should be > > a method to ask SPI to avoid using DMA, and you should use that method. > > It fixes a real error -- with an out-of-tree driver and ancient kernel. > For the flash read/write case it is used with list-DMA (e.g. write 4 bytes, > read 64KB). There are extensive comments in include/linux/spi/spi.h > which document the DMA-safe requirement of the buffers. Pushed to my l2-mtd-2.6 tree, thanks.
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c index 4c19269..21dd4d9 100644 --- a/drivers/mtd/devices/m25p80.c +++ b/drivers/mtd/devices/m25p80.c @@ -79,7 +79,7 @@ struct m25p { struct mtd_info mtd; unsigned partitioned:1; u8 erase_opcode; - u8 command[CMD_SIZE + FAST_READ_DUMMY_BYTE]; + u8 *command; }; static inline struct m25p *mtd_to_m25p(struct mtd_info *mtd) @@ -770,6 +770,11 @@ static int __devinit m25p_probe(struct spi_device *spi) flash = kzalloc(sizeof *flash, GFP_KERNEL); if (!flash) return -ENOMEM; + flash->command = kmalloc(CMD_SIZE + FAST_READ_DUMMY_BYTE, GFP_KERNEL); + if (!flash->command) { + kfree(flash); + return -ENOMEM; + } flash->spi = spi; mutex_init(&flash->lock); @@ -888,8 +893,10 @@ static int __devexit m25p_remove(struct spi_device *spi) status = del_mtd_partitions(&flash->mtd); else status = del_mtd_device(&flash->mtd); - if (status == 0) + if (status == 0) { + kfree(flash->command); kfree(flash); + } return 0; }
spi_write() requires the buffer to be DMA-safe, kmalloc() it seperately to ensure this. Signed-off-by: Johannes Stezenbach <js@sig21.net>