diff mbox series

[v3,2/3] serial: samsung: Add missing checks for dma_map_single failure

Message ID 1491200468-28463-2-git-send-email-m.szyprowski@samsung.com
State Accepted
Commit 500fcc08a32bfd54f11951ba81530775df15c474
Headers show
Series None | expand

Commit Message

Marek Szyprowski April 3, 2017, 6:21 a.m. UTC 
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(-)

-- 
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

Comments

Shuah Khan April 3, 2017, 1:50 p.m. UTC | #1 
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
diff mbox series

Patch

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)