dma: mv_xor: error handling fixes

dma: mv_xor: error handling fixes

[Thomas Petazzoni]

Jason, Andrew, Gr?gory,

Here are two patches fixing error handling code in the mv_xor
initialization code. When the initialization of one particular XOR
channel fails, we have some cleanup code that isn't correct, and
triggers a kernel warning (fixed by the first patch) and then a kernel
crash (fixed by the second patch).

The original bug has been introduced in the big XOR series pushed for
3.8, so ideally, those patches should be pushed for 3.8 as well (but
they can wait after 3.8-rc1 if needed).

If you want a formal pull request, please let me know, but I guess for
two patches, it is just as easy for you to apply it to some of your
branch.

Thanks!

Thomas

[PATCH 1/2] dma: mv_xor: fix error handling of mv_xor_channel_add()

[Thomas Petazzoni]

When mv_xor_channel_add() fails for one XOR channel, we jump to the
err_channel_add label to clean up all previous channels that had been
initialized correctly. Unfortunately, while handling this error
condition, we were disposing the IRQ mapping before calling
mv_xor_channel_remove() (which does the free_irq()), which is
incorrect.

Instead, do things properly in the reverse order of the
initialization: first remove the XOR channel (so that free_irq() is
done), and then dispose the IRQ mapping.

This avoids ugly warnings when for some reason one of the XOR channel
fails to initialize.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
This is a bug fix for a change introduced in 3.8, so it should be
applied to 3.8.
---
 drivers/dma/mv_xor.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
index 9659e58..79fde35 100644
--- a/drivers/dma/mv_xor.c
+++ b/drivers/dma/mv_xor.c
@@ -1361,9 +1361,9 @@ static int __devinit mv_xor_probe(struct platform_device *pdev)
 err_channel_add:
 	for (i = 0; i < MV_XOR_MAX_CHANNELS; i++)
 		if (xordev->channels[i]) {
+			mv_xor_channel_remove(xordev->channels[i]);
 			if (pdev->dev.of_node)
 				irq_dispose_mapping(xordev->channels[i]->irq);
-			mv_xor_channel_remove(xordev->channels[i]);
 		}
 
 	clk_disable_unprepare(xordev->clk);
-- 
1.7.9.5

[PATCH 2/2] dma: mv_xor: fix error handling for clocks

[Thomas Petazzoni]

When a channel fails to initialize, we release all ressources,
including clocks. However, a XOR unit is not necessarily associated to
a clock (some variants of Marvell SoCs have a clock for XOR units,
some don't), so we shouldn't unconditionally be releasing the clock.

Instead, just like we do in the mv_xor_remove() function, we should
check if one clock was found before releasing it.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
This is a bug fix for a change introduced in 3.8, so it should be
applied to 3.8.
---
 drivers/dma/mv_xor.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
index 79fde35..11f1a65 100644
--- a/drivers/dma/mv_xor.c
+++ b/drivers/dma/mv_xor.c
@@ -1366,8 +1366,11 @@ err_channel_add:
 				irq_dispose_mapping(xordev->channels[i]->irq);
 		}
 
-	clk_disable_unprepare(xordev->clk);
-	clk_put(xordev->clk);
+	if (!IS_ERR(xordev->clk)) {
+		clk_disable_unprepare(xordev->clk);
+		clk_put(xordev->clk);
+	}
+
 	return ret;
 }
 
-- 
1.7.9.5

dma: mv_xor: error handling fixes

[Thomas Petazzoni]

Jason, Andrew, Gr?gory,

On Thu, 13 Dec 2012 14:20:41 +0100, Thomas Petazzoni wrote:
> Jason, Andrew, Gr?gory,
> 
> Here are two patches fixing error handling code in the mv_xor
> initialization code. When the initialization of one particular XOR
> channel fails, we have some cleanup code that isn't correct, and
> triggers a kernel warning (fixed by the first patch) and then a kernel
> crash (fixed by the second patch).
> 
> The original bug has been introduced in the big XOR series pushed for
> 3.8, so ideally, those patches should be pushed for 3.8 as well (but
> they can wait after 3.8-rc1 if needed).

Any issue with those patches? Any reason for them not being pushed for
3.8-rc?

Thanks!

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com