diff mbox series

[v4,1/2] i2c: imx: notify about real errors on dma i2c_imx_dma_request

Message ID 20190107084738.4130-2-o.rempel@pengutronix.de
State Superseded
Headers show
Series i2c: imx: rework imx error handling. | expand

Commit Message

Oleksij Rempel Jan. 7, 2019, 8:47 a.m. UTC 
Current implementation will print "can't use DMA, using PIO instead",
which is confusing and mostly interpreted as error. At least on i.MX5x,
DMA event for I2C is muxed with SDHC. So it is project specific
configuration. On other hand, real error have debug level.

With this patch real error should be printed and configuration specific
state will become silent.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/i2c/busses/i2c-imx.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

Comments

Uwe Kleine-König Jan. 7, 2019, 8:27 p.m. UTC | #1 
Hello,

On Mon, Jan 07, 2019 at 09:47:37AM +0100, Oleksij Rempel wrote:
> Current implementation will print "can't use DMA, using PIO instead",
> which is confusing and mostly interpreted as error. At least on i.MX5x,
> DMA event for I2C is muxed with SDHC. So it is project specific
> configuration. On other hand, real error have debug level.
> 
> With this patch real error should be printed and configuration specific
> state will become silent.

I'd write:

At least on i.MX5x, the DMA events for I2C and SDHC use the same channel
and there can only be a single user. So in this case there should be no
message emitted that looks like an error if the I2C device doesn't have
an assigned DMA channel. In contrast real problems that were only
emitted at debug level before should be described at a higher level
to be better visible and so understandable.

> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

The patch is fine and can have my 

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks
Uwe
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index fa9ad53845d9..e28ef494dac8 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -285,9 +285,11 @@  static void i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
 	if (!dma)
 		return;
 
-	dma->chan_tx = dma_request_slave_channel(dev, "tx");
-	if (!dma->chan_tx) {
-		dev_dbg(dev, "can't request DMA tx channel\n");
+	dma->chan_tx = dma_request_chan(dev, "tx");
+	if (IS_ERR(dma->chan_tx)) {
+		ret = PTR_ERR(dma->chan_rx);
+		if (ret != -ENODEV && ret != -EPROBE_DEFER)
+			dev_err(dev, "can't request DMA tx channel (%d)\n", ret);
 		goto fail_al;
 	}
 
@@ -298,13 +300,15 @@  static void i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
 	dma_sconfig.direction = DMA_MEM_TO_DEV;
 	ret = dmaengine_slave_config(dma->chan_tx, &dma_sconfig);
 	if (ret < 0) {
-		dev_dbg(dev, "can't configure tx channel\n");
+		dev_err(dev, "can't configure tx channel (%d)\n", ret);
 		goto fail_tx;
 	}
 
-	dma->chan_rx = dma_request_slave_channel(dev, "rx");
-	if (!dma->chan_rx) {
-		dev_dbg(dev, "can't request DMA rx channel\n");
+	dma->chan_rx = dma_request_chan(dev, "rx");
+	if (IS_ERR(dma->chan_rx)) {
+		ret = PTR_ERR(dma->chan_rx);
+		if (ret != -ENODEV && ret != -EPROBE_DEFER)
+			dev_err(dev, "can't request DMA rx channel (%d)\n", ret);
 		goto fail_tx;
 	}
 
@@ -315,7 +319,7 @@  static void i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
 	dma_sconfig.direction = DMA_DEV_TO_MEM;
 	ret = dmaengine_slave_config(dma->chan_rx, &dma_sconfig);
 	if (ret < 0) {
-		dev_dbg(dev, "can't configure rx channel\n");
+		dev_err(dev, "can't configure rx channel (%d)\n", ret);
 		goto fail_rx;
 	}
 
@@ -332,7 +336,6 @@  static void i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
 	dma_release_channel(dma->chan_tx);
 fail_al:
 	devm_kfree(dev, dma);
-	dev_info(dev, "can't use DMA, using PIO instead.\n");
 }
 
 static void i2c_imx_dma_callback(void *arg)