| Message ID | 1458746964-9335-1-git-send-email-jarkko.nikula@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Jarkko Nikula <jarkko.nikula@linux.intel.com> writes: > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > If by some reason pxa2xx_spi_dma_prepare() fails we have not to ignore its > error. In such case we abort the transfer and return the error to upper > level. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> > --- > Reposting Andy's fix from December that got buried to list during > holidays. > --- > drivers/spi/spi-pxa2xx.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c > index 85e59a406a4c..47bdbd350a24 100644 > --- a/drivers/spi/spi-pxa2xx.c > +++ b/drivers/spi/spi-pxa2xx.c > @@ -928,6 +928,7 @@ static void pump_transfers(unsigned long data) > u32 dma_thresh = drv_data->cur_chip->dma_threshold; > u32 dma_burst = drv_data->cur_chip->dma_burst_size; > u32 change_mask = pxa2xx_spi_get_ssrc1_change_mask(drv_data); > + int err; > > /* Get current state information */ > message = drv_data->cur_msg; > @@ -1047,7 +1048,12 @@ static void pump_transfers(unsigned long data) > /* Ensure we have the correct interrupt handler */ > drv_data->transfer_handler = pxa2xx_spi_dma_transfer; > > - pxa2xx_spi_dma_prepare(drv_data, dma_burst); > + err = pxa2xx_spi_dma_prepare(drv_data, dma_burst); > + if (err) { > + message->status = err; > + giveback(drv_data); > + return; > + } Hi Jarrko, Even if this patch is perfectly fine, isn't pxa2xx_spi_dma_prepare() leaking tx_desc if its rx_desc allocation failed ? If so, wouldn't it make sense to add it to this patch ? Cheers.
On 03/23/2016 08:15 PM, Robert Jarzmik wrote: > Jarkko Nikula <jarkko.nikula@linux.intel.com> writes: > >> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> >> If by some reason pxa2xx_spi_dma_prepare() fails we have not to ignore its >> error. In such case we abort the transfer and return the error to upper >> level. >> >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> >> --- >> Reposting Andy's fix from December that got buried to list during >> holidays. >> --- >> drivers/spi/spi-pxa2xx.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c >> index 85e59a406a4c..47bdbd350a24 100644 >> --- a/drivers/spi/spi-pxa2xx.c >> +++ b/drivers/spi/spi-pxa2xx.c >> @@ -928,6 +928,7 @@ static void pump_transfers(unsigned long data) >> u32 dma_thresh = drv_data->cur_chip->dma_threshold; >> u32 dma_burst = drv_data->cur_chip->dma_burst_size; >> u32 change_mask = pxa2xx_spi_get_ssrc1_change_mask(drv_data); >> + int err; >> >> /* Get current state information */ >> message = drv_data->cur_msg; >> @@ -1047,7 +1048,12 @@ static void pump_transfers(unsigned long data) >> /* Ensure we have the correct interrupt handler */ >> drv_data->transfer_handler = pxa2xx_spi_dma_transfer; >> >> - pxa2xx_spi_dma_prepare(drv_data, dma_burst); >> + err = pxa2xx_spi_dma_prepare(drv_data, dma_burst); >> + if (err) { >> + message->status = err; >> + giveback(drv_data); >> + return; >> + } > Hi Jarrko, > > Even if this patch is perfectly fine, isn't pxa2xx_spi_dma_prepare() leaking > tx_desc if its rx_desc allocation failed ? If so, wouldn't it make sense to add > it to this patch ? > Good catch, it indeeds leak. Also unmapping needs to be done too. Will fix and resend.
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c index 85e59a406a4c..47bdbd350a24 100644 --- a/drivers/spi/spi-pxa2xx.c +++ b/drivers/spi/spi-pxa2xx.c @@ -928,6 +928,7 @@ static void pump_transfers(unsigned long data) u32 dma_thresh = drv_data->cur_chip->dma_threshold; u32 dma_burst = drv_data->cur_chip->dma_burst_size; u32 change_mask = pxa2xx_spi_get_ssrc1_change_mask(drv_data); + int err; /* Get current state information */ message = drv_data->cur_msg; @@ -1047,7 +1048,12 @@ static void pump_transfers(unsigned long data) /* Ensure we have the correct interrupt handler */ drv_data->transfer_handler = pxa2xx_spi_dma_transfer; - pxa2xx_spi_dma_prepare(drv_data, dma_burst); + err = pxa2xx_spi_dma_prepare(drv_data, dma_burst); + if (err) { + message->status = err; + giveback(drv_data); + return; + } /* Clear status and start DMA engine */ cr1 = chip->cr1 | dma_thresh | drv_data->dma_cr1;