From mboxrd@z Thu Jan 1 00:00:00 1970 From: Russell King - ARM Linux Subject: Re: [CFT 07/11] spi: omap2-mcspi: add DMA engine support Date: Thu, 14 Jun 2012 13:50:13 +0100 Message-ID: <20120614125012.GG31187@n2100.arm.linux.org.uk> References: <20120607110610.GB15973@n2100.arm.linux.org.uk> <20120614115335.GE31187@n2100.arm.linux.org.uk> <20120614120842.GF31187@n2100.arm.linux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Grant Likely , spi-devel-general@lists.sourceforge.net To: linux-arm-kernel@lists.infradead.org, linux-omap@vger.kernel.org Return-path: Content-Disposition: inline In-Reply-To: <20120614120842.GF31187@n2100.arm.linux.org.uk> Sender: linux-omap-owner@vger.kernel.org List-Id: linux-spi.vger.kernel.org On Thu, Jun 14, 2012 at 01:08:43PM +0100, Russell King - ARM Linux wrote: > On Thu, Jun 14, 2012 at 12:53:35PM +0100, Russell King - ARM Linux wrote: > > On Thu, Jun 07, 2012 at 12:08:35PM +0100, Russell King wrote: > > > Add DMA engine support to the OMAP SPI driver. This supplements the > > > private DMA API implementation contained within this driver, and the > > > driver can be independently switched at build time between using DMA > > > engine and the private DMA API for the transmit and receive sides. > > > > > > Tested-by: Shubhrajyoti > > > Acked-by: Grant Likely > > > Signed-off-by: Russell King > > > > Right, now that we have working OMAP in mainline again... > > Another warning: > > ------------[ cut here ]------------ > WARNING: at /home/rmk/git/linux-rmk/drivers/base/dd.c:257 driver_probe_device+0x78/0x21c() > Modules linked in: > Backtrace: > [] (dump_backtrace+0x0/0x10c) from [] (dump_stack+0x18/0x1c) r7:00000000 r6:c01ff28c r5:c040050c r4:00000101 > [] (dump_stack+0x0/0x1c) from [] (warn_slowpath_common+0x58/0x70) > [] (warn_slowpath_common+0x0/0x70) from [] (warn_slowpath_null+0x24/0x2c) > r8:c04aa4d8 r7:c04aa63c r6:de70ce00 r5:de70ce34 r4:de70ce00 > [] (warn_slowpath_null+0x0/0x2c) from [] (driver_probe_device+0x78/0x21c) > [] (driver_probe_device+0x0/0x21c) from [] (__driver_attach+0x6c/0x90) > r7:de443ec8 r6:c04aa63c r5:de70ce34 r4:de70ce00 > [] (__driver_attach+0x0/0x90) from [] (bus_for_each_dev+0x58/0x98) > r6:c04aa63c r5:c01ff430 r4:00000000 > [] (bus_for_each_dev+0x0/0x98) from [] (driver_attach+0x20/0x28) > r7:de6c9e80 r6:c04aa63c r5:c04aa63c r4:c0465b80 > [] (driver_attach+0x0/0x28) from [] (bus_add_driver+0xb4/0x230) > [] (bus_add_driver+0x0/0x230) from [] (driver_register+0xac/0x138) > [] (driver_register+0x0/0x138) from [] (spi_register_driver+0x4c/0x60) > r8:0000005b r7:c045f848 r6:00000006 r5:00000018 r4:c0465b80 > [] (spi_register_driver+0x0/0x60) from [] (ks8851_init+0x14/0x1c) > [] (ks8851_init+0x0/0x1c) from [] (do_one_initcall+0x9c/0x164) > [] (do_one_initcall+0x0/0x164) from [] (kernel_init+0x128/0x210) > [] (kernel_init+0x0/0x210) from [] (do_exit+0x0/0x72c) > ---[ end trace 4dcda79f5e89dd84 ]--- > ks8851 spi1.0: message enable is 0 > ks8851 spi1.0: eth0: revision 0, MAC 08:00:28:01:4d:c6, IRQ 194, has EEPROM > > The relevant line: > > WARN_ON(!list_empty(&dev->devres_head)); > > Which suggests that someone is using devres against the struct device for > the KS8851 device before the driver is bound. That's a bug, plain and > simple. I've not yet been able to track down what it is or where it's > being done, but it is something that has been introduced during the last > merge window. > > devm_* APIs should only be used by _drivers_ against the struct device > that they are driving - because the lifetime of these things is bounded > by the point at which the driver is bound to that struct device, to the > point that it is unbound from that struct device (and at that point, > all devm_* stuff against the struct device gets destroyed.) This commit introduced the bug: commit 1a77b127ae147f5827043a9896d7f4cb248b402e Author: Shubhrajyoti D Date: Sat Mar 17 12:44:01 2012 +0530 OMAP : SPI : use devm_* functions The various devm_* functions allocate memory that is released when a driver detaches. This patch uses devm_request_and_ioremap to request memory in probe function. Since the freeing is not needed the calls are deleted from remove function.Also use use devm_kzalloc for the cs memory allocation. Signed-off-by: Shubhrajyoti D and sure enough, reverting this makes the warning go away. Specifically, it is this part which is the culpret: diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c index 7745f91..cb2c0e3 100644 --- a/drivers/spi/spi-omap2-mcspi.c +++ b/drivers/spi/spi-omap2-mcspi.c @@ -789,7 +789,7 @@ static int omap2_mcspi_setup(struct spi_device *spi) mcspi_dma = &mcspi->dma_channels[spi->chip_select]; if (!cs) { - cs = kzalloc(sizeof *cs, GFP_KERNEL); + cs = devm_kzalloc(&spi->dev , sizeof *cs, GFP_KERNEL); if (!cs) return -ENOMEM; cs->base = mcspi->base + spi->chip_select * 0x14; @@ -831,7 +831,6 @@ static void omap2_mcspi_cleanup(struct spi_device *spi) cs = spi->controller_state; list_del(&cs->node); - kfree(spi->controller_state); } if (spi->chip_select < spi->master->num_chipselect) { because, at the time when omap2_mcspi_setup() is called, spi->dev is not bound, and so is outside of the devres valid lifetime of that struct device.