From ac01e97d644da8e947ffa1bde5083290fe2e36e7 Mon Sep 17 00:00:00 2001 From: Daniel Mack Date: Wed, 25 Mar 2009 00:18:35 +0000 Subject: [PATCH] spi/bfin_spi: fix resources leakage Re-order setup() a bit so we don't leak memory/dma/gpio resources upon errors. Also make sure we don't call kfree() twice on the same object. Signed-off-by: Daniel Mack Signed-off-by: Bryan Wu Signed-off-by: Yi Li Signed-off-by: Mike Frysinger --- drivers/spi/spi_bfin5xx.c | 120 ++++++++++++++++++++++++-------------- 1 file changed, 75 insertions(+), 45 deletions(-) diff --git a/drivers/spi/spi_bfin5xx.c b/drivers/spi/spi_bfin5xx.c index 10a6dc3d37ac..4f20b923a95c 100644 --- a/drivers/spi/spi_bfin5xx.c +++ b/drivers/spi/spi_bfin5xx.c @@ -1006,20 +1006,24 @@ static u16 ssel[][MAX_SPI_SSEL] = { /* first setup for new devices */ static int bfin_spi_setup(struct spi_device *spi) { - struct bfin5xx_spi_chip *chip_info = NULL; - struct chip_data *chip; + struct bfin5xx_spi_chip *chip_info; + struct chip_data *chip = NULL; struct driver_data *drv_data = spi_master_get_devdata(spi->master); - int ret; + int ret = -EINVAL; if (spi->bits_per_word != 8 && spi->bits_per_word != 16) - return -EINVAL; + goto error; /* Only alloc (or use chip_info) on first setup */ + chip_info = NULL; chip = spi_get_ctldata(spi); if (chip == NULL) { - chip = kzalloc(sizeof(struct chip_data), GFP_KERNEL); - if (!chip) - return -ENOMEM; + chip = kzalloc(sizeof(*chip), GFP_KERNEL); + if (!chip) { + dev_err(&spi->dev, "cannot allocate chip data\n"); + ret = -ENOMEM; + goto error; + } chip->enable_dma = 0; chip_info = spi->controller_data; @@ -1036,7 +1040,7 @@ static int bfin_spi_setup(struct spi_device *spi) if (chip_info->ctl_reg & (SPE|MSTR|CPOL|CPHA|LSBF|SIZE)) { dev_err(&spi->dev, "do not set bits in ctl_reg " "that the SPI framework manages\n"); - return -EINVAL; + goto error; } chip->enable_dma = chip_info->enable_dma != 0 @@ -1059,26 +1063,6 @@ static int bfin_spi_setup(struct spi_device *spi) /* we dont support running in slave mode (yet?) */ chip->ctl_reg |= MSTR; - /* - * if any one SPI chip is registered and wants DMA, request the - * DMA channel for it - */ - if (chip->enable_dma && !drv_data->dma_requested) { - /* register dma irq handler */ - if (request_dma(drv_data->dma_channel, "BFIN_SPI_DMA") < 0) { - dev_dbg(&spi->dev, - "Unable to request BlackFin SPI DMA channel\n"); - return -ENODEV; - } - if (set_dma_callback(drv_data->dma_channel, - bfin_spi_dma_irq_handler, drv_data) < 0) { - dev_dbg(&spi->dev, "Unable to set dma callback\n"); - return -EPERM; - } - dma_disable_irq(drv_data->dma_channel); - drv_data->dma_requested = 1; - } - /* * Notice: for blackfin, the speed_hz is the value of register * SPI_BAUD, not the real baudrate @@ -1087,16 +1071,6 @@ static int bfin_spi_setup(struct spi_device *spi) chip->flag = 1 << (spi->chip_select); chip->chip_select_num = spi->chip_select; - if (chip->chip_select_num == 0) { - ret = gpio_request(chip->cs_gpio, spi->modalias); - if (ret) { - if (drv_data->dma_requested) - free_dma(drv_data->dma_channel); - return ret; - } - gpio_direction_output(chip->cs_gpio, 1); - } - switch (chip->bits_per_word) { case 8: chip->n_bytes = 1; @@ -1123,9 +1097,39 @@ static int bfin_spi_setup(struct spi_device *spi) default: dev_err(&spi->dev, "%d bits_per_word is not supported\n", chip->bits_per_word); - if (chip_info) - kfree(chip); - return -ENODEV; + goto error; + } + + /* + * if any one SPI chip is registered and wants DMA, request the + * DMA channel for it + */ + if (chip->enable_dma && !drv_data->dma_requested) { + /* register dma irq handler */ + ret = request_dma(drv_data->dma_channel, "BFIN_SPI_DMA"); + if (ret) { + dev_err(&spi->dev, + "Unable to request BlackFin SPI DMA channel\n"); + goto error; + } + drv_data->dma_requested = 1; + + ret = set_dma_callback(drv_data->dma_channel, + bfin_spi_dma_irq_handler, drv_data); + if (ret) { + dev_err(&spi->dev, "Unable to set dma callback\n"); + goto error; + } + dma_disable_irq(drv_data->dma_channel); + } + + if (chip->chip_select_num == 0) { + ret = gpio_request(chip->cs_gpio, spi->modalias); + if (ret) { + dev_err(&spi->dev, "gpio_request() error\n"); + goto pin_error; + } + gpio_direction_output(chip->cs_gpio, 1); } dev_dbg(&spi->dev, "setup spi chip %s, width is %d, dma is %d\n", @@ -1136,14 +1140,38 @@ static int bfin_spi_setup(struct spi_device *spi) spi_set_ctldata(spi, chip); dev_dbg(&spi->dev, "chip select number is %d\n", chip->chip_select_num); - if ((chip->chip_select_num > 0) - && (chip->chip_select_num <= spi->master->num_chipselect)) - peripheral_request(ssel[spi->master->bus_num] - [chip->chip_select_num-1], spi->modalias); + if (chip->chip_select_num > 0 && + chip->chip_select_num <= spi->master->num_chipselect) { + ret = peripheral_request(ssel[spi->master->bus_num] + [chip->chip_select_num-1], spi->modalias); + if (ret) { + dev_err(&spi->dev, "peripheral_request() error\n"); + goto pin_error; + } + } bfin_spi_cs_deactive(drv_data, chip); return 0; + + pin_error: + if (chip->chip_select_num == 0) + gpio_free(chip->cs_gpio); + else + peripheral_free(ssel[spi->master->bus_num] + [chip->chip_select_num - 1]); + error: + if (chip) { + if (drv_data->dma_requested) + free_dma(drv_data->dma_channel); + drv_data->dma_requested = 0; + + kfree(chip); + /* prevent free 'chip' twice */ + spi_set_ctldata(spi, NULL); + } + + return ret; } /* @@ -1166,6 +1194,8 @@ static void bfin_spi_cleanup(struct spi_device *spi) gpio_free(chip->cs_gpio); kfree(chip); + /* prevent free 'chip' twice */ + spi_set_ctldata(spi, NULL); } static inline int bfin_spi_init_queue(struct driver_data *drv_data) -- 2.20.1