[CFT 07/11] spi: omap2-mcspi: add DMA engine support

[Russell King - ARM Linux]

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 <shubhrajyoti at ti.com>
> > > Acked-by: Grant Likely <grant.likely at secretlab.ca>
> > > Signed-off-by: Russell King <rmk+kernel at arm.linux.org.uk>
> > 
> > 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 <shubhrajyoti at ti.com>
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 <shubhrajyoti at ti.com>

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.