diff options
author | Lukas Wunner <lukas@wunner.de> | 2021-05-27 23:10:56 +0200 |
---|---|---|
committer | Mark Brown <broonie@kernel.org> | 2021-06-01 14:03:12 +0100 |
commit | 2ec6f20b33eb4f62ab90bdcd620436c883ec3af6 (patch) | |
tree | 141e2723fcc72a66f68bf8bfabdd89a459a14cfe /drivers/spi/spi-bitbang.c | |
parent | 13817d466eb8713a1ffd254f537402f091d48444 (diff) | |
download | lwn-2ec6f20b33eb4f62ab90bdcd620436c883ec3af6.tar.gz lwn-2ec6f20b33eb4f62ab90bdcd620436c883ec3af6.zip |
spi: Cleanup on failure of initial setup
Commit c7299fea6769 ("spi: Fix spi device unregister flow") changed the
SPI core's behavior if the ->setup() hook returns an error upon adding
an spi_device: Before, the ->cleanup() hook was invoked to free any
allocations that were made by ->setup(). With the commit, that's no
longer the case, so the ->setup() hook is expected to free the
allocations itself.
I've identified 5 drivers which depend on the old behavior and am fixing
them up hereinafter: spi-bitbang.c spi-fsl-spi.c spi-omap-uwire.c
spi-omap2-mcspi.c spi-pxa2xx.c
Importantly, ->setup() is not only invoked on spi_device *addition*:
It may subsequently be called to *change* SPI parameters. If changing
these SPI parameters fails, freeing memory allocations would be wrong.
That should only be done if the spi_device is finally destroyed.
I am therefore using a bool "initial_setup" in 4 of the affected drivers
to differentiate between the invocation on *adding* the spi_device and
any subsequent invocations: spi-bitbang.c spi-fsl-spi.c spi-omap-uwire.c
spi-omap2-mcspi.c
In spi-pxa2xx.c, it seems the ->setup() hook can only fail on spi_device
addition, not any subsequent calls. It therefore doesn't need the bool.
It's worth noting that 5 other drivers already perform a cleanup if the
->setup() hook fails. Before c7299fea6769, they caused a double-free
if ->setup() failed on spi_device addition. Since the commit, they're
fine. These drivers are: spi-mpc512x-psc.c spi-pl022.c spi-s3c64xx.c
spi-st-ssc4.c spi-tegra114.c
(spi-pxa2xx.c also already performs a cleanup, but only in one of
several error paths.)
Fixes: c7299fea6769 ("spi: Fix spi device unregister flow")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: Saravana Kannan <saravanak@google.com>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> # pxa2xx
Link: https://lore.kernel.org/r/f76a0599469f265b69c371538794101fa37b5536.1622149321.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@kernel.org>
Diffstat (limited to 'drivers/spi/spi-bitbang.c')
-rw-r--r-- | drivers/spi/spi-bitbang.c | 18 |
1 files changed, 14 insertions, 4 deletions
diff --git a/drivers/spi/spi-bitbang.c b/drivers/spi/spi-bitbang.c index 6a6af85aebfd..27d0087f8688 100644 --- a/drivers/spi/spi-bitbang.c +++ b/drivers/spi/spi-bitbang.c @@ -184,6 +184,8 @@ int spi_bitbang_setup(struct spi_device *spi) { struct spi_bitbang_cs *cs = spi->controller_state; struct spi_bitbang *bitbang; + bool initial_setup = false; + int retval; bitbang = spi_master_get_devdata(spi->master); @@ -192,22 +194,30 @@ int spi_bitbang_setup(struct spi_device *spi) if (!cs) return -ENOMEM; spi->controller_state = cs; + initial_setup = true; } /* per-word shift register access, in hardware or bitbanging */ cs->txrx_word = bitbang->txrx_word[spi->mode & (SPI_CPOL|SPI_CPHA)]; - if (!cs->txrx_word) - return -EINVAL; + if (!cs->txrx_word) { + retval = -EINVAL; + goto err_free; + } if (bitbang->setup_transfer) { - int retval = bitbang->setup_transfer(spi, NULL); + retval = bitbang->setup_transfer(spi, NULL); if (retval < 0) - return retval; + goto err_free; } dev_dbg(&spi->dev, "%s, %u nsec/bit\n", __func__, 2 * cs->nsecs); return 0; + +err_free: + if (initial_setup) + kfree(cs); + return retval; } EXPORT_SYMBOL_GPL(spi_bitbang_setup); |