diff mbox series

[v2] i2c: imx: don't print error if DMA is not configured

Message ID 20181112085408.6585-1-o.rempel@pengutronix.de
State Changes Requested
Headers show
Series [v2] i2c: imx: don't print error if DMA is not configured | expand

Commit Message

Oleksij Rempel Nov. 12, 2018, 8:54 a.m. UTC 
From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

At least on i.MX5x variants is DMA event for I2C muxed with SDHC. So
it is project specific configuration and not an error.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
[ore: make it actually compile, fix a corner case]
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes:
20181112 v2:
 - drop "[PATCH v1 2/3] i2c: imx: probe dma only only on i.MX50 and
   later" and rebase without this patch.
 - Set proper initial author

 drivers/i2c/busses/i2c-imx.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

Comments

Wolfram Sang Nov. 27, 2018, 11:31 a.m. UTC | #1 
Hi,

> At least on i.MX5x variants is DMA event for I2C muxed with SDHC. So
> it is project specific configuration and not an error.

The patch itself looks okay from a glimpse, but IMHO $subject and
$commit message don't really cut it. This is not so much about not
printing something but refactoring the complete error handling, or? E.g.
you handle -EPROBE_DEFER now, which wasn't the case before. Also, some
dev_dbg are turned to dev_err which is probably okay but confuses a
little reading $subject.

Thanks,

   Wolfram
Oleksij Rempel Nov. 29, 2018, 5:49 a.m. UTC | #2 
Hi Wolfram,

On 27.11.18 12:31, Wolfram Sang wrote:
> Hi,
> 
>> At least on i.MX5x variants is DMA event for I2C muxed with SDHC. So
>> it is project specific configuration and not an error.
> 
> The patch itself looks okay from a glimpse, but IMHO $subject and
> $commit message don't really cut it. This is not so much about not
> printing something but refactoring the complete error handling, or? E.g.
> you handle -EPROBE_DEFER now, which wasn't the case before. Also, some
> dev_dbg are turned to dev_err which is probably okay but confuses a
> little reading $subject.

Yes you right. Should I update the subject and resend this patch?
Wolfram Sang Nov. 29, 2018, 10:26 a.m. UTC | #3 
> > The patch itself looks okay from a glimpse, but IMHO $subject and
> > $commit message don't really cut it. This is not so much about not
> > printing something but refactoring the complete error handling, or? E.g.
> > you handle -EPROBE_DEFER now, which wasn't the case before. Also, some
> > dev_dbg are turned to dev_err which is probably okay but confuses a
> > little reading $subject.
> 
> Yes you right. Should I update the subject and resend this patch?

Yes, please.
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index c406700789e1..3c12b174bbb1 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -273,7 +273,7 @@  static inline unsigned char imx_i2c_read_reg(struct imx_i2c_struct *i2c_imx,
 }
 
 /* Functions for DMA support */
-static void i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
+static int i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
 						dma_addr_t phy_addr)
 {
 	struct imx_i2c_dma *dma;
@@ -283,11 +283,13 @@  static void i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
 
 	dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL);
 	if (!dma)
-		return;
+		return -ENOMEM;
 
-	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;
 	}
 
@@ -324,15 +328,15 @@  static void i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
 	dev_info(dev, "using %s (tx) and %s (rx) for DMA transfers\n",
 		dma_chan_name(dma->chan_tx), dma_chan_name(dma->chan_rx));
 
-	return;
+	return 0;
 
 fail_rx:
 	dma_release_channel(dma->chan_rx);
 fail_tx:
 	dma_release_channel(dma->chan_tx);
 fail_al:
-	devm_kfree(dev, dma);
-	dev_info(dev, "can't use DMA, using PIO instead.\n");
+	/* return successfully if there is no dma support */
+	return ret == -ENODEV ? 0 : ret;
 }
 
 static void i2c_imx_dma_callback(void *arg)
@@ -1159,11 +1163,13 @@  static int i2c_imx_probe(struct platform_device *pdev)
 	dev_dbg(&i2c_imx->adapter.dev, "device resources: %pR\n", res);
 	dev_dbg(&i2c_imx->adapter.dev, "adapter name: \"%s\"\n",
 		i2c_imx->adapter.name);
-	dev_info(&i2c_imx->adapter.dev, "IMX I2C adapter registered\n");
 
 	/* Init DMA config if supported */
-	i2c_imx_dma_request(i2c_imx, phy_addr);
+	ret = i2c_imx_dma_request(i2c_imx, phy_addr);
+	if (ret < 0)
+		goto clk_notifier_unregister;
 
+	dev_info(&i2c_imx->adapter.dev, "IMX I2C adapter registered\n");
 	return 0;   /* Return OK */
 
 clk_notifier_unregister: