| Message ID | 1491200468-28463-2-git-send-email-m.szyprowski@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
Hi Marek, On Mon, Apr 3, 2017 at 12:21 AM, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > This patch adds missing checks for dma_map_single() failure and proper error > reporting. Although this issue was harmless on ARM architecture, it is always > good to use the DMA mapping API in a proper way. This patch fixes the following > DMA API debug warning: > > WARNING: CPU: 1 PID: 3785 at lib/dma-debug.c:1171 check_unmap+0x8a0/0xf28 > dma-pl330 121a0000.pdma: DMA-API: device driver failed to check map error[device address=0x000000006e0f9000] [size=4096 bytes] [mapped as single] > Modules linked in: > CPU: 1 PID: 3785 Comm: (agetty) Tainted: G W 4.11.0-rc1-00137-g07ca963-dirty #59 > Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) > [<c011aaa4>] (unwind_backtrace) from [<c01127c0>] (show_stack+0x20/0x24) > [<c01127c0>] (show_stack) from [<c06ba5d8>] (dump_stack+0x84/0xa0) > [<c06ba5d8>] (dump_stack) from [<c0139528>] (__warn+0x14c/0x180) > [<c0139528>] (__warn) from [<c01395a4>] (warn_slowpath_fmt+0x48/0x50) > [<c01395a4>] (warn_slowpath_fmt) from [<c072a114>] (check_unmap+0x8a0/0xf28) > [<c072a114>] (check_unmap) from [<c072a834>] (debug_dma_unmap_page+0x98/0xc8) > [<c072a834>] (debug_dma_unmap_page) from [<c0803874>] (s3c24xx_serial_shutdown+0x314/0x52c) > [<c0803874>] (s3c24xx_serial_shutdown) from [<c07f5124>] (uart_port_shutdown+0x54/0x88) > [<c07f5124>] (uart_port_shutdown) from [<c07f522c>] (uart_shutdown+0xd4/0x110) > [<c07f522c>] (uart_shutdown) from [<c07f6a8c>] (uart_hangup+0x9c/0x208) > [<c07f6a8c>] (uart_hangup) from [<c07c426c>] (__tty_hangup+0x49c/0x634) > [<c07c426c>] (__tty_hangup) from [<c07c78ac>] (tty_ioctl+0xc88/0x16e4) > [<c07c78ac>] (tty_ioctl) from [<c03b5f2c>] (do_vfs_ioctl+0xc4/0xd10) > [<c03b5f2c>] (do_vfs_ioctl) from [<c03b6bf4>] (SyS_ioctl+0x7c/0x8c) > [<c03b6bf4>] (SyS_ioctl) from [<c010b4a0>] (ret_fast_syscall+0x0/0x3c) > > Reported-by: Seung-Woo Kim <sw0312.kim@samsung.com> > Fixes: 62c37eedb74c8 ("serial: samsung: add dma reqest/release functions") > CC: stable@vger.kernel.org # v4.10+ > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > --- Looks good. Reviewed-by: shuahkh@osg.samsung.com thanks, -- Shuah > This issue was there since adding DMA support, but this patch applies cleanly > only to v4.10+ kernels due to other changes in the surrounding code. > > v3: > - moved spinlock removal to separate patch, extended commit message > > v2: > - fixed commit id in 'fixes' tag, added 'reviewed-by' tag > --- > drivers/tty/serial/samsung.c | 31 ++++++++++++++++++++++++------- > 1 file changed, 24 insertions(+), 7 deletions(-) > > diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c > index 9f3759bdb44f..ca0bcd7fd61f 100644 > --- a/drivers/tty/serial/samsung.c > +++ b/drivers/tty/serial/samsung.c > @@ -859,7 +859,7 @@ static void s3c24xx_serial_break_ctl(struct uart_port *port, int break_state) > static int s3c24xx_serial_request_dma(struct s3c24xx_uart_port *p) > { > struct s3c24xx_uart_dma *dma = p->dma; > - unsigned long flags; > + int ret; > > /* Default slave configuration parameters */ > dma->rx_conf.direction = DMA_DEV_TO_MEM; > @@ -884,8 +884,8 @@ static int s3c24xx_serial_request_dma(struct s3c24xx_uart_port *p) > > dma->tx_chan = dma_request_chan(p->port.dev, "tx"); > if (IS_ERR(dma->tx_chan)) { > - dma_release_channel(dma->rx_chan); > - return PTR_ERR(dma->tx_chan); > + ret = PTR_ERR(dma->tx_chan); > + goto err_release_rx; > } > > dmaengine_slave_config(dma->tx_chan, &dma->tx_conf); > @@ -894,15 +894,17 @@ static int s3c24xx_serial_request_dma(struct s3c24xx_uart_port *p) > dma->rx_size = PAGE_SIZE; > > dma->rx_buf = kmalloc(dma->rx_size, GFP_KERNEL); > - > if (!dma->rx_buf) { > - dma_release_channel(dma->rx_chan); > - dma_release_channel(dma->tx_chan); > - return -ENOMEM; > + ret = -ENOMEM; > + goto err_release_tx; > } > > dma->rx_addr = dma_map_single(p->port.dev, dma->rx_buf, > dma->rx_size, DMA_FROM_DEVICE); > + if (dma_mapping_error(p->port.dev, dma->rx_addr)) { > + ret = -EIO; > + goto err_free_rx; > + } > > spin_lock_irqsave(&p->port.lock, flags); > > @@ -911,8 +913,23 @@ static int s3c24xx_serial_request_dma(struct s3c24xx_uart_port *p) > UART_XMIT_SIZE, DMA_TO_DEVICE); > > spin_unlock_irqrestore(&p->port.lock, flags); > + if (dma_mapping_error(p->port.dev, dma->tx_addr)) { > + ret = -EIO; > + goto err_unmap_rx; > + } > > return 0; > + > +err_unmap_rx: > + dma_unmap_single(p->port.dev, dma->rx_addr, dma->rx_size, > + DMA_FROM_DEVICE); > +err_free_rx: > + kfree(dma->rx_buf); > +err_release_tx: > + dma_release_channel(dma->tx_chan); > +err_release_rx: > + dma_release_channel(dma->rx_chan); > + return ret; > } > > static void s3c24xx_serial_release_dma(struct s3c24xx_uart_port *p) > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 03, 2017 at 08:21:00AM +0200, Marek Szyprowski wrote: > This patch adds missing checks for dma_map_single() failure and proper error > reporting. Although this issue was harmless on ARM architecture, it is always > good to use the DMA mapping API in a proper way. This patch fixes the following > DMA API debug warning: > > WARNING: CPU: 1 PID: 3785 at lib/dma-debug.c:1171 check_unmap+0x8a0/0xf28 > dma-pl330 121a0000.pdma: DMA-API: device driver failed to check map error[device address=0x000000006e0f9000] [size=4096 bytes] [mapped as single] > Modules linked in: > CPU: 1 PID: 3785 Comm: (agetty) Tainted: G W 4.11.0-rc1-00137-g07ca963-dirty #59 > Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) > [<c011aaa4>] (unwind_backtrace) from [<c01127c0>] (show_stack+0x20/0x24) > [<c01127c0>] (show_stack) from [<c06ba5d8>] (dump_stack+0x84/0xa0) > [<c06ba5d8>] (dump_stack) from [<c0139528>] (__warn+0x14c/0x180) > [<c0139528>] (__warn) from [<c01395a4>] (warn_slowpath_fmt+0x48/0x50) > [<c01395a4>] (warn_slowpath_fmt) from [<c072a114>] (check_unmap+0x8a0/0xf28) > [<c072a114>] (check_unmap) from [<c072a834>] (debug_dma_unmap_page+0x98/0xc8) > [<c072a834>] (debug_dma_unmap_page) from [<c0803874>] (s3c24xx_serial_shutdown+0x314/0x52c) > [<c0803874>] (s3c24xx_serial_shutdown) from [<c07f5124>] (uart_port_shutdown+0x54/0x88) > [<c07f5124>] (uart_port_shutdown) from [<c07f522c>] (uart_shutdown+0xd4/0x110) > [<c07f522c>] (uart_shutdown) from [<c07f6a8c>] (uart_hangup+0x9c/0x208) > [<c07f6a8c>] (uart_hangup) from [<c07c426c>] (__tty_hangup+0x49c/0x634) > [<c07c426c>] (__tty_hangup) from [<c07c78ac>] (tty_ioctl+0xc88/0x16e4) > [<c07c78ac>] (tty_ioctl) from [<c03b5f2c>] (do_vfs_ioctl+0xc4/0xd10) > [<c03b5f2c>] (do_vfs_ioctl) from [<c03b6bf4>] (SyS_ioctl+0x7c/0x8c) > [<c03b6bf4>] (SyS_ioctl) from [<c010b4a0>] (ret_fast_syscall+0x0/0x3c) > > Reported-by: Seung-Woo Kim <sw0312.kim@samsung.com> > Fixes: 62c37eedb74c8 ("serial: samsung: add dma reqest/release functions") > CC: stable@vger.kernel.org # v4.10+ > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > --- > This issue was there since adding DMA support, but this patch applies cleanly > only to v4.10+ kernels due to other changes in the surrounding code. > > v3: > - moved spinlock removal to separate patch, extended commit message Thanks for the changes, looks fine. Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" 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/samsung.c b/drivers/tty/serial/samsung.c index 9f3759bdb44f..ca0bcd7fd61f 100644 --- a/drivers/tty/serial/samsung.c +++ b/drivers/tty/serial/samsung.c @@ -859,7 +859,7 @@ static void s3c24xx_serial_break_ctl(struct uart_port *port, int break_state) static int s3c24xx_serial_request_dma(struct s3c24xx_uart_port *p) { struct s3c24xx_uart_dma *dma = p->dma; - unsigned long flags; + int ret; /* Default slave configuration parameters */ dma->rx_conf.direction = DMA_DEV_TO_MEM; @@ -884,8 +884,8 @@ static int s3c24xx_serial_request_dma(struct s3c24xx_uart_port *p) dma->tx_chan = dma_request_chan(p->port.dev, "tx"); if (IS_ERR(dma->tx_chan)) { - dma_release_channel(dma->rx_chan); - return PTR_ERR(dma->tx_chan); + ret = PTR_ERR(dma->tx_chan); + goto err_release_rx; } dmaengine_slave_config(dma->tx_chan, &dma->tx_conf); @@ -894,15 +894,17 @@ static int s3c24xx_serial_request_dma(struct s3c24xx_uart_port *p) dma->rx_size = PAGE_SIZE; dma->rx_buf = kmalloc(dma->rx_size, GFP_KERNEL); - if (!dma->rx_buf) { - dma_release_channel(dma->rx_chan); - dma_release_channel(dma->tx_chan); - return -ENOMEM; + ret = -ENOMEM; + goto err_release_tx; } dma->rx_addr = dma_map_single(p->port.dev, dma->rx_buf, dma->rx_size, DMA_FROM_DEVICE); + if (dma_mapping_error(p->port.dev, dma->rx_addr)) { + ret = -EIO; + goto err_free_rx; + } spin_lock_irqsave(&p->port.lock, flags); @@ -911,8 +913,23 @@ static int s3c24xx_serial_request_dma(struct s3c24xx_uart_port *p) UART_XMIT_SIZE, DMA_TO_DEVICE); spin_unlock_irqrestore(&p->port.lock, flags); + if (dma_mapping_error(p->port.dev, dma->tx_addr)) { + ret = -EIO; + goto err_unmap_rx; + } return 0; + +err_unmap_rx: + dma_unmap_single(p->port.dev, dma->rx_addr, dma->rx_size, + DMA_FROM_DEVICE); +err_free_rx: + kfree(dma->rx_buf); +err_release_tx: + dma_release_channel(dma->tx_chan); +err_release_rx: + dma_release_channel(dma->rx_chan); + return ret; } static void s3c24xx_serial_release_dma(struct s3c24xx_uart_port *p)
This patch adds missing checks for dma_map_single() failure and proper error reporting. Although this issue was harmless on ARM architecture, it is always good to use the DMA mapping API in a proper way. This patch fixes the following DMA API debug warning: WARNING: CPU: 1 PID: 3785 at lib/dma-debug.c:1171 check_unmap+0x8a0/0xf28 dma-pl330 121a0000.pdma: DMA-API: device driver failed to check map error[device address=0x000000006e0f9000] [size=4096 bytes] [mapped as single] Modules linked in: CPU: 1 PID: 3785 Comm: (agetty) Tainted: G W 4.11.0-rc1-00137-g07ca963-dirty #59 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) [<c011aaa4>] (unwind_backtrace) from [<c01127c0>] (show_stack+0x20/0x24) [<c01127c0>] (show_stack) from [<c06ba5d8>] (dump_stack+0x84/0xa0) [<c06ba5d8>] (dump_stack) from [<c0139528>] (__warn+0x14c/0x180) [<c0139528>] (__warn) from [<c01395a4>] (warn_slowpath_fmt+0x48/0x50) [<c01395a4>] (warn_slowpath_fmt) from [<c072a114>] (check_unmap+0x8a0/0xf28) [<c072a114>] (check_unmap) from [<c072a834>] (debug_dma_unmap_page+0x98/0xc8) [<c072a834>] (debug_dma_unmap_page) from [<c0803874>] (s3c24xx_serial_shutdown+0x314/0x52c) [<c0803874>] (s3c24xx_serial_shutdown) from [<c07f5124>] (uart_port_shutdown+0x54/0x88) [<c07f5124>] (uart_port_shutdown) from [<c07f522c>] (uart_shutdown+0xd4/0x110) [<c07f522c>] (uart_shutdown) from [<c07f6a8c>] (uart_hangup+0x9c/0x208) [<c07f6a8c>] (uart_hangup) from [<c07c426c>] (__tty_hangup+0x49c/0x634) [<c07c426c>] (__tty_hangup) from [<c07c78ac>] (tty_ioctl+0xc88/0x16e4) [<c07c78ac>] (tty_ioctl) from [<c03b5f2c>] (do_vfs_ioctl+0xc4/0xd10) [<c03b5f2c>] (do_vfs_ioctl) from [<c03b6bf4>] (SyS_ioctl+0x7c/0x8c) [<c03b6bf4>] (SyS_ioctl) from [<c010b4a0>] (ret_fast_syscall+0x0/0x3c) Reported-by: Seung-Woo Kim <sw0312.kim@samsung.com> Fixes: 62c37eedb74c8 ("serial: samsung: add dma reqest/release functions") CC: stable@vger.kernel.org # v4.10+ Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> --- This issue was there since adding DMA support, but this patch applies cleanly only to v4.10+ kernels due to other changes in the surrounding code. v3: - moved spinlock removal to separate patch, extended commit message v2: - fixed commit id in 'fixes' tag, added 'reviewed-by' tag --- drivers/tty/serial/samsung.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-)