i.MX SDMA patches

The following series has some i.MX SDMA fixes. The first two
should go into -rc, the third either in -rc or next. It fixes
that the driver blocks in probe() when the driver is compiled
into the kernel but the firmware is not. From this point of
view it should go in -rc, but on the other hand it's no straight
forward fix.

Sascha

Sascha Hauer (3):
      dmaengine i.MX SDMA: lock channel 0
      dmaengine i.MX SDMA: set firmware scripts addresses to negative value initially
      dmaengine i.MX SDMA: use request_firmware_nowait

 drivers/dma/imx-sdma.c |   47 ++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 38 insertions(+), 9 deletions(-)

channel0 of the sdma engine is the configuration channel. It
is a shared resource and thus must be protected by a mutex.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/dma/imx-sdma.c |   18 ++++++++++++++++--
 1 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 7bd7e98..f50c87c 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c

@@ -318,6 +318,7 @@ struct sdma_engine {
 	dma_addr_t			context_phys;
 	struct dma_device		dma_device;
 	struct clk			*clk;
+	struct mutex			channel_0_lock;
 	struct sdma_script_start_addrs	*script_addrs;
 };
 

@@ -415,11 +416,15 @@ static int sdma_load_script(struct sdma_engine *sdma, void *buf, int size,
 	dma_addr_t buf_phys;
 	int ret;
 
+	mutex_lock(&sdma->channel_0_lock);
+
 	buf_virt = dma_alloc_coherent(NULL,
 			size,
 			&buf_phys, GFP_KERNEL);
-	if (!buf_virt)
-		return -ENOMEM;
+	if (!buf_virt) {
+		ret = -ENOMEM;
+		goto err_out;
+	}
 
 	bd0->mode.command = C0_SETPM;
 	bd0->mode.status = BD_DONE | BD_INTR | BD_WRAP | BD_EXTD;

@@ -433,6 +438,9 @@ static int sdma_load_script(struct sdma_engine *sdma, void *buf, int size,
 
 	dma_free_coherent(NULL, size, buf_virt, buf_phys);
 
+err_out:
+	mutex_unlock(&sdma->channel_0_lock);
+
 	return ret;
 }
 

@@ -656,6 +664,8 @@ static int sdma_load_context(struct sdma_channel *sdmac)
 	dev_dbg(sdma->dev, "event_mask0 = 0x%08x\n", sdmac->event_mask0);
 	dev_dbg(sdma->dev, "event_mask1 = 0x%08x\n", sdmac->event_mask1);
 
+	mutex_lock(&sdma->channel_0_lock);
+
 	memset(context, 0, sizeof(*context));
 	context->channel_state.pc = load_address;
 

@@ -676,6 +686,8 @@ static int sdma_load_context(struct sdma_channel *sdmac)
 
 	ret = sdma_run_channel(&sdma->channel[0]);
 
+	mutex_unlock(&sdma->channel_0_lock);
+
 	return ret;
 }
 

@@ -1274,6 +1286,8 @@ static int __init sdma_probe(struct platform_device *pdev)
 	if (!sdma)
 		return -ENOMEM;
 
+	mutex_init(&sdma->channel_0_lock);
+
 	sdma->dev = &pdev->dev;
 
 	iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);

-- 
1.7.5.4

If we do not have a firmare script for a given transfer,
the setup of this channel must fail. For this the script
addresses have to be < 0 initially, not 0.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/dma/imx-sdma.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index f50c87c..8abf8c1 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c

@@ -1281,6 +1281,7 @@ static int __init sdma_probe(struct platform_device *pdev)
 	struct sdma_platform_data *pdata = pdev->dev.platform_data;
 	int i;
 	struct sdma_engine *sdma;
+	s32 *saddr_arr;
 
 	sdma = kzalloc(sizeof(*sdma), GFP_KERNEL);
 	if (!sdma)

@@ -1324,6 +1325,11 @@ static int __init sdma_probe(struct platform_device *pdev)
 		goto err_alloc;
 	}
 
+	/* initially no scripts available */
+	saddr_arr = (s32 *)sdma->script_addrs;
+	for (i = 0; i < SDMA_SCRIPT_ADDRS_ARRAY_SIZE_V1; i++)
+		saddr_arr[i] = -EINVAL;
+
 	if (of_id)
 		pdev->id_entry = of_id->data;
 	sdma->devtype = pdev->id_entry->driver_data;

-- 
1.7.5.4

The firmware blob may not be available when the driver
probes. Instead of blocking the whole kernel use
request_firmware_nowait() and continue without firmware.
The ROM scripts can already be used then if available.
For the devicetree case the ROM scripts are not available,
still the probe function should not block. The driver
will be unusable in this case, but we have no way of
detecting this properly. The configuration of the dma
channels will fail, so nothing bad should happen.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/dma/imx-sdma.c |   23 ++++++++++++++++-------
 1 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 8abf8c1..b5cc27d 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c

@@ -1143,18 +1143,17 @@ static void sdma_add_scripts(struct sdma_engine *sdma,
 			saddr_arr[i] = addr_arr[i];
 }
 
-static int __init sdma_get_firmware(struct sdma_engine *sdma,
-		const char *fw_name)
+static void sdma_load_firmware(const struct firmware *fw, void *context)
 {
-	const struct firmware *fw;
+	struct sdma_engine *sdma = context;
 	const struct sdma_firmware_header *header;
-	int ret;
 	const struct sdma_script_start_addrs *addr;
 	unsigned short *ram_code;
 
-	ret = request_firmware(&fw, fw_name, sdma->dev);
-	if (ret)
-		return ret;
+	if (!fw) {
+		dev_err(sdma->dev, "firmware not found\n");
+		return;
+	}
 
 	if (fw->size < sizeof(*header))
 		goto err_firmware;

@@ -1184,6 +1183,16 @@ static int __init sdma_get_firmware(struct sdma_engine *sdma,
 
 err_firmware:
 	release_firmware(fw);
+}
+
+static int __init sdma_get_firmware(struct sdma_engine *sdma,
+		const char *fw_name)
+{
+	int ret;
+
+	ret = request_firmware_nowait(THIS_MODULE,
+			FW_ACTION_HOTPLUG, fw_name, sdma->dev,
+			GFP_KERNEL, sdma, sdma_load_firmware);
 
 	return ret;
 }

-- 
1.7.5.4

On Thu, 2011-08-25 at 11:03 +0200, Sascha Hauer wrote:

The firmware blob may not be available when the driver
probes. Instead of blocking the whole kernel use
request_firmware_nowait() and continue without firmware.
The ROM scripts can already be used then if available.
For the devicetree case the ROM scripts are not available,
still the probe function should not block. The driver
will be unusable in this case, but we have no way of
detecting this properly. The configuration of the dma
channels will fail, so nothing bad should happen.

Shouldn't you track if firmware is loaded properly and in the case it is
not return error in your prepare functions?

--
~Vinod

On Thu, Aug 25, 2011 at 11:31:57PM +0530, Koul, Vinod wrote:

On Thu, 2011-08-25 at 11:03 +0200, Sascha Hauer wrote:

quoted

The firmware blob may not be available when the driver
probes. Instead of blocking the whole kernel use
request_firmware_nowait() and continue without firmware.
The ROM scripts can already be used then if available.
For the devicetree case the ROM scripts are not available,
still the probe function should not block. The driver
will be unusable in this case, but we have no way of
detecting this properly. The configuration of the dma
channels will fail, so nothing bad should happen.

Shouldn't you track if firmware is loaded properly and in the case it is
not return error in your prepare functions?

Parts of the firmware is in ROM, enough to handle many transfer
types. The RAM firmware only offers additional transfer types.
Which transfer types are available is tracked in sdma->script_addrs.
The firmware loader just adds values here. The DMA_SLAVE_CONFIG
call will fail if a needed piece of firmware is missing.

Sascha

--
~Vinod

quoted

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/dma/imx-sdma.c |   23 ++++++++++++++++-------
 1 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 8abf8c1..b5cc27d 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c

@@ -1143,18 +1143,17 @@ static void sdma_add_scripts(struct sdma_engine *sdma,
 			saddr_arr[i] = addr_arr[i];
 }
 
-static int __init sdma_get_firmware(struct sdma_engine *sdma,
-		const char *fw_name)
+static void sdma_load_firmware(const struct firmware *fw, void *context)
 {
-	const struct firmware *fw;
+	struct sdma_engine *sdma = context;
 	const struct sdma_firmware_header *header;
-	int ret;
 	const struct sdma_script_start_addrs *addr;
 	unsigned short *ram_code;
 
-	ret = request_firmware(&fw, fw_name, sdma->dev);
-	if (ret)
-		return ret;
+	if (!fw) {
+		dev_err(sdma->dev, "firmware not found\n");
+		return;
+	}
 
 	if (fw->size < sizeof(*header))
 		goto err_firmware;

@@ -1184,6 +1183,16 @@ static int __init sdma_get_firmware(struct sdma_engine *sdma,
 
 err_firmware:
 	release_firmware(fw);
+}
+
+static int __init sdma_get_firmware(struct sdma_engine *sdma,
+		const char *fw_name)
+{
+	int ret;
+
+	ret = request_firmware_nowait(THIS_MODULE,
+			FW_ACTION_HOTPLUG, fw_name, sdma->dev,
+			GFP_KERNEL, sdma, sdma_load_firmware);
 
 	return ret;
 }

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

On Thu, 2011-08-25 at 11:03 +0200, Sascha Hauer wrote:

The following series has some i.MX SDMA fixes. The first two
should go into -rc, the third either in -rc or next. It fixes
that the driver blocks in probe() when the driver is compiled
into the kernel but the firmware is not. From this point of
view it should go in -rc, but on the other hand it's no straight
forward fix.

Sascha

Sascha Hauer (3):
      dmaengine i.MX SDMA: lock channel 0
      dmaengine i.MX SDMA: set firmware scripts addresses to negative value initially
      dmaengine i.MX SDMA: use request_firmware_nowait

 drivers/dma/imx-sdma.c |   47 ++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 38 insertions(+), 9 deletions(-)

Applied, Thanks

-- 
~Vinod