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 <[email protected]>
> > > Acked-by: Grant Likely <[email protected]>
> > > Signed-off-by: Russell King <[email protected]>
> > 
> > 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:
> [<c0017d0c>] (dump_backtrace+0x0/0x10c) from [<c033e208>] 
> (dump_stack+0x18/0x1c) r7:00000000 r6:c01ff28c r5:c040050c r4:00000101
> [<c033e1f0>] (dump_stack+0x0/0x1c) from [<c00337ec>] 
> (warn_slowpath_common+0x58/0x70)
> [<c0033794>] (warn_slowpath_common+0x0/0x70) from [<c0033828>] 
> (warn_slowpath_null+0x24/0x2c)
>  r8:c04aa4d8 r7:c04aa63c r6:de70ce00 r5:de70ce34 r4:de70ce00
> [<c0033804>] (warn_slowpath_null+0x0/0x2c) from [<c01ff28c>] 
> (driver_probe_device+0x78/0x21c)
> [<c01ff214>] (driver_probe_device+0x0/0x21c) from [<c01ff49c>] 
> (__driver_attach+0x6c/0x90)
>  r7:de443ec8 r6:c04aa63c r5:de70ce34 r4:de70ce00
> [<c01ff430>] (__driver_attach+0x0/0x90) from [<c01fda70>] 
> (bus_for_each_dev+0x58/0x98)
>  r6:c04aa63c r5:c01ff430 r4:00000000
> [<c01fda18>] (bus_for_each_dev+0x0/0x98) from [<c01ff0f4>] 
> (driver_attach+0x20/0x28)
>  r7:de6c9e80 r6:c04aa63c r5:c04aa63c r4:c0465b80
> [<c01ff0d4>] (driver_attach+0x0/0x28) from [<c01fe2f4>] 
> (bus_add_driver+0xb4/0x230)
> [<c01fe240>] (bus_add_driver+0x0/0x230) from [<c01ffb24>] 
> (driver_register+0xac/0x138)
> [<c01ffa78>] (driver_register+0x0/0x138) from [<c0215d4c>] 
> (spi_register_driver+0x4c/0x60)
>  r8:0000005b r7:c045f848 r6:00000006 r5:00000018 r4:c0465b80
> [<c0215d00>] (spi_register_driver+0x0/0x60) from [<c045414c>] 
> (ks8851_init+0x14/0x1c)
> [<c0454138>] (ks8851_init+0x0/0x1c) from [<c0008770>] 
> (do_one_initcall+0x9c/0x164)
> [<c00086d4>] (do_one_initcall+0x0/0x164) from [<c0436410>] 
> (kernel_init+0x128/0x210)
> [<c04362e8>] (kernel_init+0x0/0x210) from [<c0038754>] (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 <[email protected]>
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 <[email protected]>

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.

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
spi-devel-general mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

Reply via email to