[PATCH 0/2] i2c: sh_mobile: handle deferred probe properly when requesting DMA channels

[PATCH 0/2] i2c: sh_mobile: handle deferred probe properly when requesting DMA channels

[Wolfram Sang]

Here is the resend of my RFC series. Only patch description updates and
whitespace fixes since then. But it seems we agreed on this approach based on
the discussions so far: Geert's discovery [1], the RFC [2]. See there for a
complete description.


[1] http://thread.gmane.org/gmane.linux.drivers.i2c/20565/focus=41758
[2] http://thread.gmane.org/gmane.linux.ports.sh.devel/41806

Wolfram Sang (2):
  i2c: sh_mobile: refactor DMA setup
  i2c: sh_mobile: rework deferred probing

 drivers/i2c/busses/i2c-sh_mobile.c | 111 ++++++++++++++++++-------------------
 1 file changed, 53 insertions(+), 58 deletions(-)

-- 
2.1.3

[PATCH 1/2] i2c: sh_mobile: refactor DMA setup

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/@public.gmane.org>

Refactor DMA setup to keep the errno so we can implement better
deferred probe support in the next step.

Signed-off-by: Wolfram Sang <wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/@public.gmane.org>
---
 drivers/i2c/busses/i2c-sh_mobile.c | 35 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/drivers/i2c/busses/i2c-sh_mobile.c b/drivers/i2c/busses/i2c-sh_mobile.c
index b9c8cf2aac2c..2eccbb55dab2 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -552,7 +552,7 @@ static void sh_mobile_i2c_xfer_dma(struct sh_mobile_i2c_data *pd)
 	dma_addr_t dma_addr;
 	dma_cookie_t cookie;
 
-	if (!chan)
+	if (IS_ERR(chan))
 		return;
 
 	dma_addr = dma_map_single(chan->device->dev, pd->msg->buf, pd->msg->len, dir);
@@ -749,21 +749,18 @@ static const struct of_device_id sh_mobile_i2c_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, sh_mobile_i2c_dt_ids);
 
-static int sh_mobile_i2c_request_dma_chan(struct device *dev, enum dma_transfer_direction dir,
-					  dma_addr_t port_addr, struct dma_chan **chan_ptr)
+static struct dma_chan *sh_mobile_i2c_request_dma_chan(struct device *dev,
+				enum dma_transfer_direction dir, dma_addr_t port_addr)
 {
 	struct dma_chan *chan;
 	struct dma_slave_config cfg;
 	char *chan_name = dir == DMA_MEM_TO_DEV ? "tx" : "rx";
 	int ret;
 
-	*chan_ptr = NULL;
-
 	chan = dma_request_slave_channel_reason(dev, chan_name);
 	if (IS_ERR(chan)) {
-		ret = PTR_ERR(chan);
 		dev_dbg(dev, "request_channel failed for %s (%d)\n", chan_name, ret);
-		return ret;
+		return chan;
 	}
 
 	memset(&cfg, 0, sizeof(cfg));
@@ -780,25 +777,23 @@ static int sh_mobile_i2c_request_dma_chan(struct device *dev, enum dma_transfer_
 	if (ret) {
 		dev_dbg(dev, "slave_config failed for %s (%d)\n", chan_name, ret);
 		dma_release_channel(chan);
-		return ret;
+		return ERR_PTR(ret);
 	}
 
-	*chan_ptr = chan;
-
 	dev_dbg(dev, "got DMA channel for %s\n", chan_name);
-	return 0;
+	return chan;
 }
 
 static void sh_mobile_i2c_release_dma(struct sh_mobile_i2c_data *pd)
 {
-	if (pd->dma_tx) {
+	if (!IS_ERR(pd->dma_tx)) {
 		dma_release_channel(pd->dma_tx);
-		pd->dma_tx = NULL;
+		pd->dma_tx = ERR_PTR(-EPROBE_DEFER);
 	}
 
-	if (pd->dma_rx) {
+	if (!IS_ERR(pd->dma_rx)) {
 		dma_release_channel(pd->dma_rx);
-		pd->dma_rx = NULL;
+		pd->dma_rx = ERR_PTR(-EPROBE_DEFER);
 	}
 }
 
@@ -891,13 +886,15 @@ static int sh_mobile_i2c_probe(struct platform_device *dev)
 	/* Init DMA */
 	sg_init_table(&pd->sg, 1);
 	pd->dma_direction = DMA_NONE;
-	ret = sh_mobile_i2c_request_dma_chan(pd->dev, DMA_DEV_TO_MEM,
-					     res->start + ICDR, &pd->dma_rx);
+	pd->dma_rx = sh_mobile_i2c_request_dma_chan(pd->dev, DMA_DEV_TO_MEM,
+						    res->start + ICDR);
+	ret = PTR_ERR(pd->dma_rx);
 	if (ret == -EPROBE_DEFER)
 		return ret;
 
-	ret = sh_mobile_i2c_request_dma_chan(pd->dev, DMA_MEM_TO_DEV,
-					     res->start + ICDR, &pd->dma_tx);
+	pd->dma_tx = sh_mobile_i2c_request_dma_chan(pd->dev, DMA_MEM_TO_DEV,
+						    res->start + ICDR);
+	ret = PTR_ERR(pd->dma_tx);
 	if (ret == -EPROBE_DEFER) {
 		sh_mobile_i2c_release_dma(pd);
 		return ret;
-- 
2.1.3

Re: [PATCH 1/2] i2c: sh_mobile: refactor DMA setup

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 342 bytes --]

On Tue, Dec 16, 2014 at 01:31:25PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Refactor DMA setup to keep the errno so we can implement better
> deferred probe support in the next step.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Applied to for-current, thanks!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 2/2] i2c: sh_mobile: rework deferred probing

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/@public.gmane.org>

DMA is opt-in for this driver. So, we can't use deferred probing for
requesting DMA channels in probe, because our driver would get endlessly
deferred if DMA support is compiled in AND the DMA driver is missing.
Because we can't know when the DMA driver might show up, we always try
again when a DMA transfer would be possible. The downside is that there
is more overhead for setting up PIO transfers under the above scenario.
But well, having DMA enabled and the proper DMA driver missing looks
like a broken or test config anyhow.

Reported-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
Signed-off-by: Wolfram Sang <wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/@public.gmane.org>
---
 drivers/i2c/busses/i2c-sh_mobile.c | 98 +++++++++++++++++++-------------------
 1 file changed, 48 insertions(+), 50 deletions(-)

diff --git a/drivers/i2c/busses/i2c-sh_mobile.c b/drivers/i2c/busses/i2c-sh_mobile.c
index 2eccbb55dab2..1b7903f5f4b3 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -144,6 +144,7 @@ struct sh_mobile_i2c_data {
 	int sr;
 	bool send_stop;
 
+	struct resource *res;
 	struct dma_chan *dma_tx;
 	struct dma_chan *dma_rx;
 	struct scatterlist sg;
@@ -543,6 +544,41 @@ static void sh_mobile_i2c_dma_callback(void *data)
 	iic_set_clr(pd, ICIC, 0, ICIC_TDMAE | ICIC_RDMAE);
 }
 
+static struct dma_chan *sh_mobile_i2c_request_dma_chan(struct device *dev,
+				enum dma_transfer_direction dir, dma_addr_t port_addr)
+{
+	struct dma_chan *chan;
+	struct dma_slave_config cfg;
+	char *chan_name = dir == DMA_MEM_TO_DEV ? "tx" : "rx";
+	int ret;
+
+	chan = dma_request_slave_channel_reason(dev, chan_name);
+	if (IS_ERR(chan)) {
+		dev_dbg(dev, "request_channel failed for %s (%d)\n", chan_name, ret);
+		return chan;
+	}
+
+	memset(&cfg, 0, sizeof(cfg));
+	cfg.direction = dir;
+	if (dir == DMA_MEM_TO_DEV) {
+		cfg.dst_addr = port_addr;
+		cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	} else {
+		cfg.src_addr = port_addr;
+		cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	}
+
+	ret = dmaengine_slave_config(chan, &cfg);
+	if (ret) {
+		dev_dbg(dev, "slave_config failed for %s (%d)\n", chan_name, ret);
+		dma_release_channel(chan);
+		return ERR_PTR(ret);
+	}
+
+	dev_dbg(dev, "got DMA channel for %s\n", chan_name);
+	return chan;
+}
+
 static void sh_mobile_i2c_xfer_dma(struct sh_mobile_i2c_data *pd)
 {
 	bool read = pd->msg->flags & I2C_M_RD;
@@ -552,6 +588,15 @@ static void sh_mobile_i2c_xfer_dma(struct sh_mobile_i2c_data *pd)
 	dma_addr_t dma_addr;
 	dma_cookie_t cookie;
 
+	if (PTR_ERR(chan) == -EPROBE_DEFER) {
+		if (read)
+			chan = pd->dma_rx = sh_mobile_i2c_request_dma_chan(pd->dev, DMA_DEV_TO_MEM,
+									   pd->res->start + ICDR);
+		else
+			chan = pd->dma_tx = sh_mobile_i2c_request_dma_chan(pd->dev, DMA_MEM_TO_DEV,
+									   pd->res->start + ICDR);
+	}
+
 	if (IS_ERR(chan))
 		return;
 
@@ -749,41 +794,6 @@ static const struct of_device_id sh_mobile_i2c_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, sh_mobile_i2c_dt_ids);
 
-static struct dma_chan *sh_mobile_i2c_request_dma_chan(struct device *dev,
-				enum dma_transfer_direction dir, dma_addr_t port_addr)
-{
-	struct dma_chan *chan;
-	struct dma_slave_config cfg;
-	char *chan_name = dir == DMA_MEM_TO_DEV ? "tx" : "rx";
-	int ret;
-
-	chan = dma_request_slave_channel_reason(dev, chan_name);
-	if (IS_ERR(chan)) {
-		dev_dbg(dev, "request_channel failed for %s (%d)\n", chan_name, ret);
-		return chan;
-	}
-
-	memset(&cfg, 0, sizeof(cfg));
-	cfg.direction = dir;
-	if (dir == DMA_MEM_TO_DEV) {
-		cfg.dst_addr = port_addr;
-		cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
-	} else {
-		cfg.src_addr = port_addr;
-		cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
-	}
-
-	ret = dmaengine_slave_config(chan, &cfg);
-	if (ret) {
-		dev_dbg(dev, "slave_config failed for %s (%d)\n", chan_name, ret);
-		dma_release_channel(chan);
-		return ERR_PTR(ret);
-	}
-
-	dev_dbg(dev, "got DMA channel for %s\n", chan_name);
-	return chan;
-}
-
 static void sh_mobile_i2c_release_dma(struct sh_mobile_i2c_data *pd)
 {
 	if (!IS_ERR(pd->dma_tx)) {
@@ -846,6 +856,7 @@ static int sh_mobile_i2c_probe(struct platform_device *dev)
 
 	res = platform_get_resource(dev, IORESOURCE_MEM, 0);
 
+	pd->res = res;
 	pd->reg = devm_ioremap_resource(&dev->dev, res);
 	if (IS_ERR(pd->reg))
 		return PTR_ERR(pd->reg);
@@ -886,19 +897,7 @@ static int sh_mobile_i2c_probe(struct platform_device *dev)
 	/* Init DMA */
 	sg_init_table(&pd->sg, 1);
 	pd->dma_direction = DMA_NONE;
-	pd->dma_rx = sh_mobile_i2c_request_dma_chan(pd->dev, DMA_DEV_TO_MEM,
-						    res->start + ICDR);
-	ret = PTR_ERR(pd->dma_rx);
-	if (ret == -EPROBE_DEFER)
-		return ret;
-
-	pd->dma_tx = sh_mobile_i2c_request_dma_chan(pd->dev, DMA_MEM_TO_DEV,
-						    res->start + ICDR);
-	ret = PTR_ERR(pd->dma_tx);
-	if (ret == -EPROBE_DEFER) {
-		sh_mobile_i2c_release_dma(pd);
-		return ret;
-	}
+	pd->dma_rx = pd->dma_tx = ERR_PTR(-EPROBE_DEFER);
 
 	/* Enable Runtime PM for this device.
 	 *
@@ -936,8 +935,7 @@ static int sh_mobile_i2c_probe(struct platform_device *dev)
 		return ret;
 	}
 
-	dev_info(&dev->dev, "I2C adapter %d, bus speed %lu Hz, DMA=%c\n",
-		 adap->nr, pd->bus_speed, (pd->dma_rx || pd->dma_tx) ? 'y' : 'n');
+	dev_info(&dev->dev, "I2C adapter %d, bus speed %lu Hz\n", adap->nr, pd->bus_speed);
 
 	return 0;
 }
-- 
2.1.3

Re: [PATCH 2/2] i2c: sh_mobile: rework deferred probing

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 849 bytes --]

On Tue, Dec 16, 2014 at 01:31:26PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> DMA is opt-in for this driver. So, we can't use deferred probing for
> requesting DMA channels in probe, because our driver would get endlessly
> deferred if DMA support is compiled in AND the DMA driver is missing.
> Because we can't know when the DMA driver might show up, we always try
> again when a DMA transfer would be possible. The downside is that there
> is more overhead for setting up PIO transfers under the above scenario.
> But well, having DMA enabled and the proper DMA driver missing looks
> like a broken or test config anyhow.
> 
> Reported-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Applied to for-current, thanks!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]