Message ID | 1432120864-23916-1-git-send-email-jonathanh@nvidia.com |
---|---|
State | Accepted, archived |
Headers | show |
On 20/05/15 12:21, Jon Hunter wrote: > If the call to dmaengine_slave_config() fails, then the DMA buffer will > not be freed/unmapped. Fix this by moving the code that stores the > address of the buffer in the tegra_uart_port structure to before the > call to dmaengine_slave_config(). By the way, just to be clear, I did try to fix this before [1], but failed :-( Thanks to Alex for pointing this out. Cheers Jon [1] https://lkml.org/lkml/2015/5/5/802 > Reported-by: Alexandre Courbot <acourbot@nvidia.com> > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > --- > drivers/tty/serial/serial-tegra.c | 32 +++++++++++--------------------- > 1 file changed, 11 insertions(+), 21 deletions(-) > > diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c > index 3b63f103f0c9..cf0133ae762d 100644 > --- a/drivers/tty/serial/serial-tegra.c > +++ b/drivers/tty/serial/serial-tegra.c > @@ -999,6 +999,12 @@ static int tegra_uart_dma_channel_allocate(struct tegra_uart_port *tup, > dma_release_channel(dma_chan); > return -ENOMEM; > } > + dma_sconfig.src_addr = tup->uport.mapbase; > + dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE; > + dma_sconfig.src_maxburst = 4; > + tup->rx_dma_chan = dma_chan; > + tup->rx_dma_buf_virt = dma_buf; > + tup->rx_dma_buf_phys = dma_phys; > } else { > dma_phys = dma_map_single(tup->uport.dev, > tup->uport.state->xmit.buf, UART_XMIT_SIZE, > @@ -1009,39 +1015,23 @@ static int tegra_uart_dma_channel_allocate(struct tegra_uart_port *tup, > return -ENOMEM; > } > dma_buf = tup->uport.state->xmit.buf; > - } > - > - if (dma_to_memory) { > - dma_sconfig.src_addr = tup->uport.mapbase; > - dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE; > - dma_sconfig.src_maxburst = 4; > - } else { > dma_sconfig.dst_addr = tup->uport.mapbase; > dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE; > dma_sconfig.dst_maxburst = 16; > + tup->tx_dma_chan = dma_chan; > + tup->tx_dma_buf_virt = dma_buf; > + tup->tx_dma_buf_phys = dma_phys; > } > > ret = dmaengine_slave_config(dma_chan, &dma_sconfig); > if (ret < 0) { > dev_err(tup->uport.dev, > "Dma slave config failed, err = %d\n", ret); > - goto scrub; > + tegra_uart_dma_channel_free(tup, dma_to_memory); > + return ret; > } > > - if (dma_to_memory) { > - tup->rx_dma_chan = dma_chan; > - tup->rx_dma_buf_virt = dma_buf; > - tup->rx_dma_buf_phys = dma_phys; > - } else { > - tup->tx_dma_chan = dma_chan; > - tup->tx_dma_buf_virt = dma_buf; > - tup->tx_dma_buf_phys = dma_phys; > - } > return 0; > - > -scrub: > - tegra_uart_dma_channel_free(tup, dma_to_memory); > - return ret; > } > > static int tegra_uart_startup(struct uart_port *u) > -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 20/05/15 12:25, Jon Hunter wrote: > > On 20/05/15 12:21, Jon Hunter wrote: >> If the call to dmaengine_slave_config() fails, then the DMA buffer will >> not be freed/unmapped. Fix this by moving the code that stores the >> address of the buffer in the tegra_uart_port structure to before the >> call to dmaengine_slave_config(). > > By the way, just to be clear, I did try to fix this before [1], but > failed :-( To be doubly clear, this is targeted to be applied on top of the previous patch [1] which is now in linux-next. Jon > [1] https://lkml.org/lkml/2015/5/5/802 -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, May 20, 2015 at 8:21 PM, Jon Hunter <jonathanh@nvidia.com> wrote: > If the call to dmaengine_slave_config() fails, then the DMA buffer will > not be freed/unmapped. Fix this by moving the code that stores the > address of the buffer in the tegra_uart_port structure to before the > call to dmaengine_slave_config(). > > Reported-by: Alexandre Courbot <acourbot@nvidia.com> > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> Looks good, we had the same if/else condition appearing three times in this function for no real reason anyway. This considerably simplifies the code. > drivers/tty/serial/serial-tegra.c | 32 +++++++++++--------------------- > 1 file changed, 11 insertions(+), 21 deletions(-) Negative number of lines, another good point for this patch! Reviewed-by: Alexandre Courbot <acourbot@nvidia.com> -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c index 3b63f103f0c9..cf0133ae762d 100644 --- a/drivers/tty/serial/serial-tegra.c +++ b/drivers/tty/serial/serial-tegra.c @@ -999,6 +999,12 @@ static int tegra_uart_dma_channel_allocate(struct tegra_uart_port *tup, dma_release_channel(dma_chan); return -ENOMEM; } + dma_sconfig.src_addr = tup->uport.mapbase; + dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE; + dma_sconfig.src_maxburst = 4; + tup->rx_dma_chan = dma_chan; + tup->rx_dma_buf_virt = dma_buf; + tup->rx_dma_buf_phys = dma_phys; } else { dma_phys = dma_map_single(tup->uport.dev, tup->uport.state->xmit.buf, UART_XMIT_SIZE, @@ -1009,39 +1015,23 @@ static int tegra_uart_dma_channel_allocate(struct tegra_uart_port *tup, return -ENOMEM; } dma_buf = tup->uport.state->xmit.buf; - } - - if (dma_to_memory) { - dma_sconfig.src_addr = tup->uport.mapbase; - dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE; - dma_sconfig.src_maxburst = 4; - } else { dma_sconfig.dst_addr = tup->uport.mapbase; dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE; dma_sconfig.dst_maxburst = 16; + tup->tx_dma_chan = dma_chan; + tup->tx_dma_buf_virt = dma_buf; + tup->tx_dma_buf_phys = dma_phys; } ret = dmaengine_slave_config(dma_chan, &dma_sconfig); if (ret < 0) { dev_err(tup->uport.dev, "Dma slave config failed, err = %d\n", ret); - goto scrub; + tegra_uart_dma_channel_free(tup, dma_to_memory); + return ret; } - if (dma_to_memory) { - tup->rx_dma_chan = dma_chan; - tup->rx_dma_buf_virt = dma_buf; - tup->rx_dma_buf_phys = dma_phys; - } else { - tup->tx_dma_chan = dma_chan; - tup->tx_dma_buf_virt = dma_buf; - tup->tx_dma_buf_phys = dma_phys; - } return 0; - -scrub: - tegra_uart_dma_channel_free(tup, dma_to_memory); - return ret; } static int tegra_uart_startup(struct uart_port *u)
If the call to dmaengine_slave_config() fails, then the DMA buffer will not be freed/unmapped. Fix this by moving the code that stores the address of the buffer in the tegra_uart_port structure to before the call to dmaengine_slave_config(). Reported-by: Alexandre Courbot <acourbot@nvidia.com> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> --- drivers/tty/serial/serial-tegra.c | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-)