* [PATCH] mmc: moxart: fix probe logic
@ 2015-01-29 16:15 ` Arnd Bergmann
0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-01-29 16:15 UTC (permalink / raw)
To: linux-mmc; +Cc: Ulf Hansson, Jonas Jensen, linux-arm-kernel
Jonas Jensen wanted to submit a patch for these, but apparently
forgot about it. I stumbled over this symptom first:
drivers/built-in.o: In function `moxart_probe':
:(.text+0x2af128): undefined reference to `of_dma_request_slave_channel'
This is because of_dma_request_slave_channel is an internal helper
and not exported to loadable module. I'm changing the driver to
use dma_request_slave_channel_reason() instead.
Further problems from inspection:
* The remove function must not call kfree on the host pointer,
because it is allocated together with the mmc_host.
* The clock is never released
* The dma_cap_mask_t is completely unused and can be removed
* deferred probing does not work if the dma driver is loaded
after the mmc driver.
This patch should fix all of the above.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Jonas, can you check this patch and provide an Ack if it's ok?
diff --git a/drivers/mmc/host/moxart-mmc.c b/drivers/mmc/host/moxart-mmc.c
index d2a1ef61ec97..d9401a1cc589 100644
--- a/drivers/mmc/host/moxart-mmc.c
+++ b/drivers/mmc/host/moxart-mmc.c
@@ -17,6 +17,7 @@
#include <linux/init.h>
#include <linux/platform_device.h>
#include <linux/delay.h>
+#include <linux/errno.h>
#include <linux/interrupt.h>
#include <linux/blkdev.h>
#include <linux/dma-mapping.h>
@@ -131,6 +132,7 @@ struct moxart_host {
struct mmc_host *mmc;
struct mmc_request *mrq;
struct scatterlist *cur_sg;
+ struct clk *clk;
struct completion dma_complete;
struct completion pio_complete;
@@ -560,9 +562,7 @@ static int moxart_probe(struct platform_device *pdev)
struct mmc_host *mmc;
struct moxart_host *host = NULL;
struct dma_slave_config cfg;
- struct clk *clk;
void __iomem *reg_mmc;
- dma_cap_mask_t mask;
int irq, ret;
u32 i;
@@ -586,10 +586,10 @@ static int moxart_probe(struct platform_device *pdev)
goto out;
}
- clk = of_clk_get(node, 0);
- if (IS_ERR(clk)) {
+ host->clk = of_clk_get(node, 0);
+ if (IS_ERR(host->clk)) {
dev_err(dev, "of_clk_get failed\n");
- ret = PTR_ERR(clk);
+ ret = PTR_ERR(host->clk);
goto out;
}
@@ -603,18 +603,15 @@ static int moxart_probe(struct platform_device *pdev)
if (ret)
goto out;
- dma_cap_zero(mask);
- dma_cap_set(DMA_SLAVE, mask);
-
host = mmc_priv(mmc);
host->mmc = mmc;
host->base = reg_mmc;
host->reg_phys = res_mmc.start;
host->timeout = msecs_to_jiffies(1000);
- host->sysclk = clk_get_rate(clk);
+ host->sysclk = clk_get_rate(host->clk);
host->fifo_width = readl(host->base + REG_FEATURE) << 2;
- host->dma_chan_tx = of_dma_request_slave_channel(node, "tx");
- host->dma_chan_rx = of_dma_request_slave_channel(node, "rx");
+ host->dma_chan_tx = dma_request_slave_channel_reason(dev, "tx");
+ host->dma_chan_rx = dma_request_slave_channel_reason(dev, "rx");
spin_lock_init(&host->lock);
@@ -624,6 +621,11 @@ static int moxart_probe(struct platform_device *pdev)
mmc->ocr_avail = 0xffff00; /* Support 2.0v - 3.6v power. */
if (IS_ERR(host->dma_chan_tx) || IS_ERR(host->dma_chan_rx)) {
+ if (PTR_ERR(host->dma_chan_tx) == -EPROBE_DEFER ||
+ PTR_ERR(host->dma_chan_rx) == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto out;
+ }
dev_dbg(dev, "PIO mode transfer enabled\n");
host->have_dma = false;
} else {
@@ -677,6 +679,8 @@ static int moxart_probe(struct platform_device *pdev)
return 0;
out:
+ if (host->clk)
+ clk_put(host->clk);
if (mmc)
mmc_free_host(mmc);
return ret;
@@ -694,6 +698,7 @@ static int moxart_remove(struct platform_device *pdev)
dma_release_channel(host->dma_chan_tx);
if (!IS_ERR(host->dma_chan_rx))
dma_release_channel(host->dma_chan_rx);
+ clk_put(host->clk);
mmc_remove_host(mmc);
mmc_free_host(mmc);
@@ -702,9 +707,6 @@ static int moxart_remove(struct platform_device *pdev)
writel(readl(host->base + REG_CLOCK_CONTROL) | CLK_OFF,
host->base + REG_CLOCK_CONTROL);
}
-
- kfree(host);
-
return 0;
}
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH] mmc: moxart: fix probe logic
@ 2015-01-29 16:15 ` Arnd Bergmann
0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-01-29 16:15 UTC (permalink / raw)
To: linux-arm-kernel
Jonas Jensen wanted to submit a patch for these, but apparently
forgot about it. I stumbled over this symptom first:
drivers/built-in.o: In function `moxart_probe':
:(.text+0x2af128): undefined reference to `of_dma_request_slave_channel'
This is because of_dma_request_slave_channel is an internal helper
and not exported to loadable module. I'm changing the driver to
use dma_request_slave_channel_reason() instead.
Further problems from inspection:
* The remove function must not call kfree on the host pointer,
because it is allocated together with the mmc_host.
* The clock is never released
* The dma_cap_mask_t is completely unused and can be removed
* deferred probing does not work if the dma driver is loaded
after the mmc driver.
This patch should fix all of the above.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Jonas, can you check this patch and provide an Ack if it's ok?
diff --git a/drivers/mmc/host/moxart-mmc.c b/drivers/mmc/host/moxart-mmc.c
index d2a1ef61ec97..d9401a1cc589 100644
--- a/drivers/mmc/host/moxart-mmc.c
+++ b/drivers/mmc/host/moxart-mmc.c
@@ -17,6 +17,7 @@
#include <linux/init.h>
#include <linux/platform_device.h>
#include <linux/delay.h>
+#include <linux/errno.h>
#include <linux/interrupt.h>
#include <linux/blkdev.h>
#include <linux/dma-mapping.h>
@@ -131,6 +132,7 @@ struct moxart_host {
struct mmc_host *mmc;
struct mmc_request *mrq;
struct scatterlist *cur_sg;
+ struct clk *clk;
struct completion dma_complete;
struct completion pio_complete;
@@ -560,9 +562,7 @@ static int moxart_probe(struct platform_device *pdev)
struct mmc_host *mmc;
struct moxart_host *host = NULL;
struct dma_slave_config cfg;
- struct clk *clk;
void __iomem *reg_mmc;
- dma_cap_mask_t mask;
int irq, ret;
u32 i;
@@ -586,10 +586,10 @@ static int moxart_probe(struct platform_device *pdev)
goto out;
}
- clk = of_clk_get(node, 0);
- if (IS_ERR(clk)) {
+ host->clk = of_clk_get(node, 0);
+ if (IS_ERR(host->clk)) {
dev_err(dev, "of_clk_get failed\n");
- ret = PTR_ERR(clk);
+ ret = PTR_ERR(host->clk);
goto out;
}
@@ -603,18 +603,15 @@ static int moxart_probe(struct platform_device *pdev)
if (ret)
goto out;
- dma_cap_zero(mask);
- dma_cap_set(DMA_SLAVE, mask);
-
host = mmc_priv(mmc);
host->mmc = mmc;
host->base = reg_mmc;
host->reg_phys = res_mmc.start;
host->timeout = msecs_to_jiffies(1000);
- host->sysclk = clk_get_rate(clk);
+ host->sysclk = clk_get_rate(host->clk);
host->fifo_width = readl(host->base + REG_FEATURE) << 2;
- host->dma_chan_tx = of_dma_request_slave_channel(node, "tx");
- host->dma_chan_rx = of_dma_request_slave_channel(node, "rx");
+ host->dma_chan_tx = dma_request_slave_channel_reason(dev, "tx");
+ host->dma_chan_rx = dma_request_slave_channel_reason(dev, "rx");
spin_lock_init(&host->lock);
@@ -624,6 +621,11 @@ static int moxart_probe(struct platform_device *pdev)
mmc->ocr_avail = 0xffff00; /* Support 2.0v - 3.6v power. */
if (IS_ERR(host->dma_chan_tx) || IS_ERR(host->dma_chan_rx)) {
+ if (PTR_ERR(host->dma_chan_tx) == -EPROBE_DEFER ||
+ PTR_ERR(host->dma_chan_rx) == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto out;
+ }
dev_dbg(dev, "PIO mode transfer enabled\n");
host->have_dma = false;
} else {
@@ -677,6 +679,8 @@ static int moxart_probe(struct platform_device *pdev)
return 0;
out:
+ if (host->clk)
+ clk_put(host->clk);
if (mmc)
mmc_free_host(mmc);
return ret;
@@ -694,6 +698,7 @@ static int moxart_remove(struct platform_device *pdev)
dma_release_channel(host->dma_chan_tx);
if (!IS_ERR(host->dma_chan_rx))
dma_release_channel(host->dma_chan_rx);
+ clk_put(host->clk);
mmc_remove_host(mmc);
mmc_free_host(mmc);
@@ -702,9 +707,6 @@ static int moxart_remove(struct platform_device *pdev)
writel(readl(host->base + REG_CLOCK_CONTROL) | CLK_OFF,
host->base + REG_CLOCK_CONTROL);
}
-
- kfree(host);
-
return 0;
}
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] mmc: moxart: fix probe logic
2015-01-29 16:15 ` Arnd Bergmann
@ 2015-01-29 17:01 ` Russell King - ARM Linux
-1 siblings, 0 replies; 24+ messages in thread
From: Russell King - ARM Linux @ 2015-01-29 17:01 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linux-mmc, Ulf Hansson, linux-arm-kernel, Jonas Jensen
On Thu, Jan 29, 2015 at 05:15:31PM +0100, Arnd Bergmann wrote:
> @@ -586,10 +586,10 @@ static int moxart_probe(struct platform_device *pdev)
> goto out;
> }
>
> - clk = of_clk_get(node, 0);
> - if (IS_ERR(clk)) {
> + host->clk = of_clk_get(node, 0);
> + if (IS_ERR(host->clk)) {
> dev_err(dev, "of_clk_get failed\n");
> - ret = PTR_ERR(clk);
> + ret = PTR_ERR(host->clk);
> goto out;
> }
>
...
> @@ -677,6 +679,8 @@ static int moxart_probe(struct platform_device *pdev)
> return 0;
>
> out:
> + if (host->clk)
> + clk_put(host->clk);
Are you sure?
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] mmc: moxart: fix probe logic
@ 2015-01-29 17:01 ` Russell King - ARM Linux
0 siblings, 0 replies; 24+ messages in thread
From: Russell King - ARM Linux @ 2015-01-29 17:01 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jan 29, 2015 at 05:15:31PM +0100, Arnd Bergmann wrote:
> @@ -586,10 +586,10 @@ static int moxart_probe(struct platform_device *pdev)
> goto out;
> }
>
> - clk = of_clk_get(node, 0);
> - if (IS_ERR(clk)) {
> + host->clk = of_clk_get(node, 0);
> + if (IS_ERR(host->clk)) {
> dev_err(dev, "of_clk_get failed\n");
> - ret = PTR_ERR(clk);
> + ret = PTR_ERR(host->clk);
> goto out;
> }
>
...
> @@ -677,6 +679,8 @@ static int moxart_probe(struct platform_device *pdev)
> return 0;
>
> out:
> + if (host->clk)
> + clk_put(host->clk);
Are you sure?
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] mmc: moxart: fix probe logic
2015-01-29 17:01 ` Russell King - ARM Linux
@ 2015-01-29 22:04 ` Arnd Bergmann
-1 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-01-29 22:04 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: linux-mmc, Ulf Hansson, linux-arm-kernel, Jonas Jensen
On Thursday 29 January 2015 17:01:04 Russell King - ARM Linux wrote:
> On Thu, Jan 29, 2015 at 05:15:31PM +0100, Arnd Bergmann wrote:
> > @@ -586,10 +586,10 @@ static int moxart_probe(struct platform_device *pdev)
> > goto out;
> > }
> >
> > - clk = of_clk_get(node, 0);
> > - if (IS_ERR(clk)) {
> > + host->clk = of_clk_get(node, 0);
> > + if (IS_ERR(host->clk)) {
> > dev_err(dev, "of_clk_get failed\n");
> > - ret = PTR_ERR(clk);
> > + ret = PTR_ERR(host->clk);
> > goto out;
> > }
> >
> ...
> > @@ -677,6 +679,8 @@ static int moxart_probe(struct platform_device *pdev)
> > return 0;
> >
> > out:
> > + if (host->clk)
> > + clk_put(host->clk);
>
> Are you sure?
>
Oops, sorry about that.
Thanks for the review! Fixed patch follows.
Arnd
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] mmc: moxart: fix probe logic
@ 2015-01-29 22:04 ` Arnd Bergmann
0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-01-29 22:04 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 29 January 2015 17:01:04 Russell King - ARM Linux wrote:
> On Thu, Jan 29, 2015 at 05:15:31PM +0100, Arnd Bergmann wrote:
> > @@ -586,10 +586,10 @@ static int moxart_probe(struct platform_device *pdev)
> > goto out;
> > }
> >
> > - clk = of_clk_get(node, 0);
> > - if (IS_ERR(clk)) {
> > + host->clk = of_clk_get(node, 0);
> > + if (IS_ERR(host->clk)) {
> > dev_err(dev, "of_clk_get failed\n");
> > - ret = PTR_ERR(clk);
> > + ret = PTR_ERR(host->clk);
> > goto out;
> > }
> >
> ...
> > @@ -677,6 +679,8 @@ static int moxart_probe(struct platform_device *pdev)
> > return 0;
> >
> > out:
> > + if (host->clk)
> > + clk_put(host->clk);
>
> Are you sure?
>
Oops, sorry about that.
Thanks for the review! Fixed patch follows.
Arnd
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2] mmc: moxart: fix probe logic
2015-01-29 17:01 ` Russell King - ARM Linux
@ 2015-01-29 22:06 ` Arnd Bergmann
-1 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-01-29 22:06 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: linux-mmc, Ulf Hansson, linux-arm-kernel, Jonas Jensen
Jonas Jensen wanted to submit a patch for these, but apparently
forgot about it. I stumbled over this symptom first:
drivers/built-in.o: In function `moxart_probe':
:(.text+0x2af128): undefined reference to `of_dma_request_slave_channel'
This is because of_dma_request_slave_channel is an internal helper
and not exported to loadable module. I'm changing the driver to
use dma_request_slave_channel_reason() instead.
Further problems from inspection:
* The remove function must not call kfree on the host pointer,
because it is allocated together with the mmc_host.
* The clock is never released
* The dma_cap_mask_t is completely unused and can be removed
* deferred probing does not work if the dma driver is loaded
after the mmc driver.
This patch should fix all of the above.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: do not call clk_put() on an error pointer
diff --git a/drivers/mmc/host/moxart-mmc.c b/drivers/mmc/host/moxart-mmc.c
index d2a1ef61ec97..66af7ae1f926 100644
--- a/drivers/mmc/host/moxart-mmc.c
+++ b/drivers/mmc/host/moxart-mmc.c
@@ -17,6 +17,7 @@
#include <linux/init.h>
#include <linux/platform_device.h>
#include <linux/delay.h>
+#include <linux/errno.h>
#include <linux/interrupt.h>
#include <linux/blkdev.h>
#include <linux/dma-mapping.h>
@@ -131,6 +132,7 @@ struct moxart_host {
struct mmc_host *mmc;
struct mmc_request *mrq;
struct scatterlist *cur_sg;
+ struct clk *clk;
struct completion dma_complete;
struct completion pio_complete;
@@ -560,9 +562,7 @@ static int moxart_probe(struct platform_device *pdev)
struct mmc_host *mmc;
struct moxart_host *host = NULL;
struct dma_slave_config cfg;
- struct clk *clk;
void __iomem *reg_mmc;
- dma_cap_mask_t mask;
int irq, ret;
u32 i;
@@ -586,10 +586,11 @@ static int moxart_probe(struct platform_device *pdev)
goto out;
}
- clk = of_clk_get(node, 0);
- if (IS_ERR(clk)) {
+ host->clk = of_clk_get(node, 0);
+ if (IS_ERR(host->clk)) {
dev_err(dev, "of_clk_get failed\n");
- ret = PTR_ERR(clk);
+ ret = PTR_ERR(host->clk);
+ host->clk = NULL;
goto out;
}
@@ -603,18 +604,15 @@ static int moxart_probe(struct platform_device *pdev)
if (ret)
goto out;
- dma_cap_zero(mask);
- dma_cap_set(DMA_SLAVE, mask);
-
host = mmc_priv(mmc);
host->mmc = mmc;
host->base = reg_mmc;
host->reg_phys = res_mmc.start;
host->timeout = msecs_to_jiffies(1000);
- host->sysclk = clk_get_rate(clk);
+ host->sysclk = clk_get_rate(host->clk);
host->fifo_width = readl(host->base + REG_FEATURE) << 2;
- host->dma_chan_tx = of_dma_request_slave_channel(node, "tx");
- host->dma_chan_rx = of_dma_request_slave_channel(node, "rx");
+ host->dma_chan_tx = dma_request_slave_channel_reason(dev, "tx");
+ host->dma_chan_rx = dma_request_slave_channel_reason(dev, "rx");
spin_lock_init(&host->lock);
@@ -624,6 +622,11 @@ static int moxart_probe(struct platform_device *pdev)
mmc->ocr_avail = 0xffff00; /* Support 2.0v - 3.6v power. */
if (IS_ERR(host->dma_chan_tx) || IS_ERR(host->dma_chan_rx)) {
+ if (PTR_ERR(host->dma_chan_tx) == -EPROBE_DEFER ||
+ PTR_ERR(host->dma_chan_rx) == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto out;
+ }
dev_dbg(dev, "PIO mode transfer enabled\n");
host->have_dma = false;
} else {
@@ -677,6 +680,8 @@ static int moxart_probe(struct platform_device *pdev)
return 0;
out:
+ if (host->clk)
+ clk_put(host->clk);
if (mmc)
mmc_free_host(mmc);
return ret;
@@ -694,6 +699,7 @@ static int moxart_remove(struct platform_device *pdev)
dma_release_channel(host->dma_chan_tx);
if (!IS_ERR(host->dma_chan_rx))
dma_release_channel(host->dma_chan_rx);
+ clk_put(host->clk);
mmc_remove_host(mmc);
mmc_free_host(mmc);
@@ -702,9 +708,6 @@ static int moxart_remove(struct platform_device *pdev)
writel(readl(host->base + REG_CLOCK_CONTROL) | CLK_OFF,
host->base + REG_CLOCK_CONTROL);
}
-
- kfree(host);
-
return 0;
}
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2] mmc: moxart: fix probe logic
@ 2015-01-29 22:06 ` Arnd Bergmann
0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-01-29 22:06 UTC (permalink / raw)
To: linux-arm-kernel
Jonas Jensen wanted to submit a patch for these, but apparently
forgot about it. I stumbled over this symptom first:
drivers/built-in.o: In function `moxart_probe':
:(.text+0x2af128): undefined reference to `of_dma_request_slave_channel'
This is because of_dma_request_slave_channel is an internal helper
and not exported to loadable module. I'm changing the driver to
use dma_request_slave_channel_reason() instead.
Further problems from inspection:
* The remove function must not call kfree on the host pointer,
because it is allocated together with the mmc_host.
* The clock is never released
* The dma_cap_mask_t is completely unused and can be removed
* deferred probing does not work if the dma driver is loaded
after the mmc driver.
This patch should fix all of the above.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: do not call clk_put() on an error pointer
diff --git a/drivers/mmc/host/moxart-mmc.c b/drivers/mmc/host/moxart-mmc.c
index d2a1ef61ec97..66af7ae1f926 100644
--- a/drivers/mmc/host/moxart-mmc.c
+++ b/drivers/mmc/host/moxart-mmc.c
@@ -17,6 +17,7 @@
#include <linux/init.h>
#include <linux/platform_device.h>
#include <linux/delay.h>
+#include <linux/errno.h>
#include <linux/interrupt.h>
#include <linux/blkdev.h>
#include <linux/dma-mapping.h>
@@ -131,6 +132,7 @@ struct moxart_host {
struct mmc_host *mmc;
struct mmc_request *mrq;
struct scatterlist *cur_sg;
+ struct clk *clk;
struct completion dma_complete;
struct completion pio_complete;
@@ -560,9 +562,7 @@ static int moxart_probe(struct platform_device *pdev)
struct mmc_host *mmc;
struct moxart_host *host = NULL;
struct dma_slave_config cfg;
- struct clk *clk;
void __iomem *reg_mmc;
- dma_cap_mask_t mask;
int irq, ret;
u32 i;
@@ -586,10 +586,11 @@ static int moxart_probe(struct platform_device *pdev)
goto out;
}
- clk = of_clk_get(node, 0);
- if (IS_ERR(clk)) {
+ host->clk = of_clk_get(node, 0);
+ if (IS_ERR(host->clk)) {
dev_err(dev, "of_clk_get failed\n");
- ret = PTR_ERR(clk);
+ ret = PTR_ERR(host->clk);
+ host->clk = NULL;
goto out;
}
@@ -603,18 +604,15 @@ static int moxart_probe(struct platform_device *pdev)
if (ret)
goto out;
- dma_cap_zero(mask);
- dma_cap_set(DMA_SLAVE, mask);
-
host = mmc_priv(mmc);
host->mmc = mmc;
host->base = reg_mmc;
host->reg_phys = res_mmc.start;
host->timeout = msecs_to_jiffies(1000);
- host->sysclk = clk_get_rate(clk);
+ host->sysclk = clk_get_rate(host->clk);
host->fifo_width = readl(host->base + REG_FEATURE) << 2;
- host->dma_chan_tx = of_dma_request_slave_channel(node, "tx");
- host->dma_chan_rx = of_dma_request_slave_channel(node, "rx");
+ host->dma_chan_tx = dma_request_slave_channel_reason(dev, "tx");
+ host->dma_chan_rx = dma_request_slave_channel_reason(dev, "rx");
spin_lock_init(&host->lock);
@@ -624,6 +622,11 @@ static int moxart_probe(struct platform_device *pdev)
mmc->ocr_avail = 0xffff00; /* Support 2.0v - 3.6v power. */
if (IS_ERR(host->dma_chan_tx) || IS_ERR(host->dma_chan_rx)) {
+ if (PTR_ERR(host->dma_chan_tx) == -EPROBE_DEFER ||
+ PTR_ERR(host->dma_chan_rx) == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto out;
+ }
dev_dbg(dev, "PIO mode transfer enabled\n");
host->have_dma = false;
} else {
@@ -677,6 +680,8 @@ static int moxart_probe(struct platform_device *pdev)
return 0;
out:
+ if (host->clk)
+ clk_put(host->clk);
if (mmc)
mmc_free_host(mmc);
return ret;
@@ -694,6 +699,7 @@ static int moxart_remove(struct platform_device *pdev)
dma_release_channel(host->dma_chan_tx);
if (!IS_ERR(host->dma_chan_rx))
dma_release_channel(host->dma_chan_rx);
+ clk_put(host->clk);
mmc_remove_host(mmc);
mmc_free_host(mmc);
@@ -702,9 +708,6 @@ static int moxart_remove(struct platform_device *pdev)
writel(readl(host->base + REG_CLOCK_CONTROL) | CLK_OFF,
host->base + REG_CLOCK_CONTROL);
}
-
- kfree(host);
-
return 0;
}
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2] mmc: moxart: fix probe logic
2015-01-29 22:06 ` Arnd Bergmann
@ 2015-02-02 10:52 ` Jonas Jensen
-1 siblings, 0 replies; 24+ messages in thread
From: Jonas Jensen @ 2015-02-02 10:52 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Russell King - ARM Linux, linux-mmc, Ulf Hansson, linux-arm-kernel
Thanks for taking the time.
On 29 January 2015 at 23:06, Arnd Bergmann <arnd@arndb.de> wrote:
> Jonas Jensen wanted to submit a patch for these, but apparently
> forgot about it. I stumbled over this symptom first:
Sorry about this, I remember thinking about the changes but only made
a mental note (which was lost over time). I was sidetracked with other
changes and work.
> @@ -586,10 +586,10 @@ static int moxart_probe(struct platform_device *pdev)
> goto out;
> }
>
> - clk = of_clk_get(node, 0);
> - if (IS_ERR(clk)) {
> + host->clk = of_clk_get(node, 0);
host->clk is a NULL dereference at this point in probe() (log below).
I moved the single line "host = mmc_priv(mmc);" to happen just before,
and everything is working normally again.
Uncompressing Linux... done, booting the kernel.
[ 0.000000] Booting Linux on physical CPU 0x0
[ 0.000000] Linux version 3.17.0-00628-g572d48d-dirty (i@Ildjarn)
(gcc version 4.9.1 (crosstool-NG 1.20.0) ) #3509 PREEMPT Mon Feb 2
10:57:21 CET 2015
[ 0.000000] CPU: FA526 [66015261] revision 1 (ARMv4), cr=0000397f
[ 0.000000] CPU: VIVT data cache, VIVT instruction cache
[ 0.000000] Machine model: MOXA UC-7112-LX
[ 0.000000] bootconsole [earlycon0] enabled
[ 0.000000] Memory policy: Data cache writeback
[ 0.000000] On node 0 totalpages: 8192
[ 0.000000] free_area_init_node: node 0, pgdat c03633c4,
node_mem_map c1fba000
[ 0.000000] Normal zone: 64 pages used for memmap
[ 0.000000] Normal zone: 0 pages reserved
[ 0.000000] Normal zone: 8192 pages, LIFO batch:0
[ 0.000000] pcpu-alloc: s0 r0 d32768 u32768 alloc=1*32768
[ 0.000000] pcpu-alloc: [0] 0
[ 0.000000] Built 1 zonelists in Zone order, mobility grouping on.
Total pages: 8128
[ 0.000000] Kernel command line: console=ttyS0,115200n8 earlyprintk
root=/dev/mmcblk0p1 rw rootwait debug
[ 0.000000] PID hash table entries: 128 (order: -3, 512 bytes)
[ 0.000000] Dentry cache hash table entries: 4096 (order: 2, 16384 bytes)
[ 0.000000] Inode-cache hash table entries: 2048 (order: 1, 8192 bytes)
[ 0.000000] Memory: 28824K/32768K available (2710K kernel code, 94K
rwdata, 508K rodata, 116K init, 125K bss, 3944K reserved)
[ 0.000000] Virtual kernel memory layout:
[ 0.000000] vector : 0xffff0000 - 0xffff1000 ( 4 kB)
[ 0.000000] fixmap : 0xffc00000 - 0xffe00000 (2048 kB)
[ 0.000000] vmalloc : 0xc2800000 - 0xff000000 ( 968 MB)
[ 0.000000] lowmem : 0xc0000000 - 0xc2000000 ( 32 MB)
[ 0.000000] .text : 0xc0008000 - 0xc032ccd0 (3220 kB)
[ 0.000000] .init : 0xc032d000 - 0xc034a390 ( 117 kB)
[ 0.000000] .data : 0xc034c000 - 0xc0363b20 ( 95 kB)
[ 0.000000] .bss : 0xc0363b20 - 0xc03831c8 ( 126 kB)
[ 0.000000] SLUB: HWalign=32, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
[ 0.000000] Preemptible hierarchical RCU implementation.
[ 0.000000] NR_IRQS:16 nr_irqs:16 16
[ 0.000000] sched_clock: 32 bits at 100 Hz, resolution 10000000ns,
wraps every 21474836480000000ns
[ 0.010000] Calibrating delay loop... 146.84 BogoMIPS (lpj=734208)
[ 0.100000] pid_max: default: 4096 minimum: 301
[ 0.100000] Mount-cache hash table entries: 1024 (order: 0, 4096 bytes)
[ 0.110000] Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes)
[ 0.120000] CPU: Testing write buffer coherency: ok
[ 0.130000] Setting up static identity map for 0x2955c8 - 0x295610
[ 0.150000] devtmpfs: initialized
[ 0.170000] NET: Registered protocol family 16
[ 0.180000] DMA: preallocated 256 KiB pool for atomic coherent allocations
[ 0.280000] Switched to clocksource moxart_timer
[ 0.290000] NET: Registered protocol family 2
[ 0.300000] TCP established hash table entries: 1024 (order: 0, 4096 bytes)
[ 0.310000] TCP bind hash table entries: 1024 (order: 0, 4096 bytes)
[ 0.320000] TCP: Hash tables configured (established 1024 bind 1024)
[ 0.330000] TCP: reno registered
[ 0.330000] UDP hash table entries: 256 (order: 0, 4096 bytes)
[ 0.340000] UDP-Lite hash table entries: 256 (order: 0, 4096 bytes)
[ 0.350000] NET: Registered protocol family 1
[ 0.360000] futex hash table entries: 16 (order: -5, 192 bytes)
[ 0.410000] jffs2: version 2.2. (NAND) © 2001-2006 Red Hat, Inc.
[ 0.420000] msgmni has been set to 56
[ 0.430000] io scheduler noop registered
[ 0.430000] io scheduler cfq registered (default)
[ 0.440000] gpiochip_find_base: found new base at 224
[ 0.440000] gpiochip_add: registered GPIOs 224 to 255 on device: moxart-gpio
[ 0.450000] Serial: 8250/16550 driver, 1 ports, IRQ sharing enabled
[ 0.470000] console [ttyS0] disabled
[ 0.470000] 98200000.uart0: ttyS0 at MMIO 0x98200000 (irq = 21,
base_baud = 921600) is a 16550A
[ 0.480000] console [ttyS0] enabled
[ 0.480000] console [ttyS0] enabled
[ 0.490000] bootconsole [earlycon0] disabled
[ 0.490000] bootconsole [earlycon0] disabled
[ 0.500000] MOXA Smartio/Industio family driver version 2.0.5
[ 0.510000] mxser: found MOXA UC-7112-LX board (CAP=0x0)
[ 0.510000] ttyMX0 at I/O 0xf9820040 (irq = 21, base_baud = 0) is a 16550A
[ 0.520000] ttyMX1 at I/O 0xf9820060 (irq = 21, base_baud = 0) is a 16550A
[ 0.530000] mxser: mxser_initbrd success IRQ=21 max baud=921600 bps
[ 0.570000] loop: module loaded
[ 0.600000] 80000000.flash: Found 1 x16 devices at 0x0 in 16-bit
bank. Manufacturer ID 0x000089 Chip ID 0x000018
[ 0.610000] Intel/Sharp Extended Query Table at 0x0031
[ 0.620000] Intel/Sharp Extended Query Table at 0x0031
[ 0.620000] Using buffer write method
[ 0.620000] cfi_cmdset_0001: Erase suspend on write enabled
[ 0.630000] erase region 0: offset=0x0,size=0x20000,blocks=128
[ 0.640000] 4 ofpart partitions found on MTD device 80000000.flash
[ 0.640000] Creating 4 MTD partitions on "80000000.flash":
[ 0.650000] 0x000000000000-0x000000040000 : "bootloader"
[ 0.660000] 0x000000040000-0x000000200000 : "linux kernel"
[ 0.680000] 0x000000200000-0x000000a00000 : "root filesystem"
[ 0.690000] 0x000000a00000-0x000001000000 : "user filesystem"
[ 1.300000] libphy: MOXA ART Ethernet MII: probed
[ 1.920000] libphy: MOXA ART Ethernet MII: probed
[ 1.960000] of_get_named_gpiod_flags: parsed 'gpios' property of
node '/gpio_keys_polled/button@25[0]' - status (0)
[ 1.970000] gpiod_request: __gpiod_request() status=0
[ 1.970000] input: gpio_keys_polled as /devices/gpio_keys_polled/input/input0
[ 1.980000] evbug: Connected device: input0 (gpio_keys_polled at
gpio-keys-polled/input0)
[ 1.990000] of_get_named_gpiod_flags: parsed 'gpio-rtc-data'
property of node '/soc/rtc[0]' - status (0)
[ 2.000000] of_get_named_gpiod_flags: parsed 'gpio-rtc-sclk'
property of node '/soc/rtc[0]' - status (0)
[ 2.010000] of_get_named_gpiod_flags: parsed 'gpio-rtc-reset'
property of node '/soc/rtc[0]' - status (0)
[ 2.020000] gpiod_request: __gpiod_request() status=0
[ 2.030000] gpiod_request: __gpiod_request() status=0
[ 2.030000] gpiod_request: __gpiod_request() status=0
[ 2.040000] moxart-rtc 90000000.soc:rtc: rtc core: registered
90000000.soc:rtc as rtc0
[ 2.060000] Unable to handle kernel NULL pointer dereference at
virtual address 00000020
[ 2.070000] pgd = c0004000
[ 2.070000] [00000020] *pgd=00000000
[ 2.070000] Internal error: Oops: 805 [#1] PREEMPT ARM
[ 2.070000] CPU: 0 PID: 1 Comm: swapper Not tainted
3.17.0-00628-g572d48d-dirty #3509
[ 2.070000] task: c1828000 ti: c182a000 task.ti: c182a000
[ 2.070000] PC is at moxart_probe+0xa0/0x378
[ 2.070000] LR is at of_clk_get_by_clkspec+0x38/0x48
[ 2.070000] pc : [<c01a241c>] lr : [<c01b2de4>] psr: 80000013
[ 2.070000] sp : c182be10 ip : c182a000 fp : 00000000
[ 2.070000] r10: c0363b20 r9 : 00000012 r8 : c1862100
[ 2.070000] r7 : c1862110 r6 : c1fb831c r5 : 00000000 r4 : c18d5400
[ 2.070000] r3 : 00000000 r2 : 00000001 r1 : c18090c0 r0 : c18090c0
[ 2.070000] Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM
Segment kernel
[ 2.070000] Control: 0000397f Table: 00004000 DAC: 00000017
[ 2.070000] Process swapper (pid: 1, stack limit = 0xc182a1c0)
[ 2.070000] Stack: (0xc182be10 to 0xc182c000)
[ 2.070000] be00: c198a860
c198a860 c03795d0 00000004
[ 2.070000] be20: 98e00000 98e00fff c1fb8374 00000200 00000000
00000000 00000000 c00da538
[ 2.070000] be40: c1991870 c1991910 00000001 c1991910 c02fe5f3
c1861820 c1991870 00000001
[ 2.070000] be60: c03466bc c1862110 c035c5f4 c035c5f4 c037d5f0
00000000 c03466bc c0363b20
[ 2.070000] be80: 00000000 c016e940 c016e914 c1862110 c1862144
c016d9ac c1862110 c1862144
[ 2.070000] bea0: c035c5f4 c035a648 c03466bc c016db9c 00000000
c035c5f4 c016db34 c016c4bc
[ 2.070000] bec0: c1807f4c c18565d0 c035c5f4 00000000 c197ea80
c016cc00 c030404e c0304050
[ 2.070000] bee0: 00000000 c035c5f4 c033c6d8 c182a008 00000000
c016e0c8 00000000 c198a800
[ 2.070000] bf00: c033c6d8 c032dc5c c1809f60 c1856000 c1856000
c0296be0 c0375a00 00000000
[ 2.070000] bf20: 00000000 c00d2320 c02eedd6 c1809f60 c0314584
00000000 c1ffce23 c0027ab0
[ 2.070000] bf40: c0314584 0000003d 00000006 00000006 00000000
00000006 0000003d 00000006
[ 2.070000] bf60: 0000003d c0346ee8 c034a29c c0363b20 c0346ef0
c032de08 00000006 00000006
[ 2.070000] bf80: c032d50c c182a000 00000000 c028eee8 00000000
00000000 00000000 00000000
[ 2.070000] bfa0: 00000000 c028eef0 00000000 c0009230 00000000
00000000 00000000 00000000
[ 2.070000] bfc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[ 2.070000] bfe0: 00000000 00000000 00000000 00000000 00000013
00000000 ffffffff ffffffff
[ 2.070000] [<c01a241c>] (moxart_probe) from [<c016e940>]
(platform_drv_probe+0x2c/0x60)
[ 2.070000] [<c016e940>] (platform_drv_probe) from [<c016d9ac>]
(driver_probe_device+0xa8/0x1e8)
[ 2.070000] [<c016d9ac>] (driver_probe_device) from [<c016db9c>]
(__driver_attach+0x68/0x88)
[ 2.070000] [<c016db9c>] (__driver_attach) from [<c016c4bc>]
(bus_for_each_dev+0x70/0x94)
[ 2.070000] [<c016c4bc>] (bus_for_each_dev) from [<c016cc00>]
(bus_add_driver+0xd0/0x1b8)
[ 2.070000] [<c016cc00>] (bus_add_driver) from [<c016e0c8>]
(driver_register+0x9c/0xe0)
[ 2.070000] [<c016e0c8>] (driver_register) from [<c032dc5c>]
(do_one_initcall+0x108/0x1bc)
[ 2.070000] [<c032dc5c>] (do_one_initcall) from [<c032de08>]
(kernel_init_freeable+0xf8/0x1b0)
[ 2.070000] [<c032de08>] (kernel_init_freeable) from [<c028eef0>]
(kernel_init+0x8/0xe0)
[ 2.070000] [<c028eef0>] (kernel_init) from [<c0009230>]
(ret_from_fork+0x14/0x24)
[ 2.070000] Code: e1a00006 e1a01005 eb004276 e3700a01 (e5850020)
[ 2.080000] ---[ end trace b4cd6d53de4c770d ]---
[ 2.090000] Kernel panic - not syncing: Fatal exception
[ 2.090000] Rebooting in 10 seconds..
Tested-by: Jonas Jensen <jonas.jensen@gmail.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2] mmc: moxart: fix probe logic
@ 2015-02-02 10:52 ` Jonas Jensen
0 siblings, 0 replies; 24+ messages in thread
From: Jonas Jensen @ 2015-02-02 10:52 UTC (permalink / raw)
To: linux-arm-kernel
Thanks for taking the time.
On 29 January 2015 at 23:06, Arnd Bergmann <arnd@arndb.de> wrote:
> Jonas Jensen wanted to submit a patch for these, but apparently
> forgot about it. I stumbled over this symptom first:
Sorry about this, I remember thinking about the changes but only made
a mental note (which was lost over time). I was sidetracked with other
changes and work.
> @@ -586,10 +586,10 @@ static int moxart_probe(struct platform_device *pdev)
> goto out;
> }
>
> - clk = of_clk_get(node, 0);
> - if (IS_ERR(clk)) {
> + host->clk = of_clk_get(node, 0);
host->clk is a NULL dereference at this point in probe() (log below).
I moved the single line "host = mmc_priv(mmc);" to happen just before,
and everything is working normally again.
Uncompressing Linux... done, booting the kernel.
[ 0.000000] Booting Linux on physical CPU 0x0
[ 0.000000] Linux version 3.17.0-00628-g572d48d-dirty (i at Ildjarn)
(gcc version 4.9.1 (crosstool-NG 1.20.0) ) #3509 PREEMPT Mon Feb 2
10:57:21 CET 2015
[ 0.000000] CPU: FA526 [66015261] revision 1 (ARMv4), cr=0000397f
[ 0.000000] CPU: VIVT data cache, VIVT instruction cache
[ 0.000000] Machine model: MOXA UC-7112-LX
[ 0.000000] bootconsole [earlycon0] enabled
[ 0.000000] Memory policy: Data cache writeback
[ 0.000000] On node 0 totalpages: 8192
[ 0.000000] free_area_init_node: node 0, pgdat c03633c4,
node_mem_map c1fba000
[ 0.000000] Normal zone: 64 pages used for memmap
[ 0.000000] Normal zone: 0 pages reserved
[ 0.000000] Normal zone: 8192 pages, LIFO batch:0
[ 0.000000] pcpu-alloc: s0 r0 d32768 u32768 alloc=1*32768
[ 0.000000] pcpu-alloc: [0] 0
[ 0.000000] Built 1 zonelists in Zone order, mobility grouping on.
Total pages: 8128
[ 0.000000] Kernel command line: console=ttyS0,115200n8 earlyprintk
root=/dev/mmcblk0p1 rw rootwait debug
[ 0.000000] PID hash table entries: 128 (order: -3, 512 bytes)
[ 0.000000] Dentry cache hash table entries: 4096 (order: 2, 16384 bytes)
[ 0.000000] Inode-cache hash table entries: 2048 (order: 1, 8192 bytes)
[ 0.000000] Memory: 28824K/32768K available (2710K kernel code, 94K
rwdata, 508K rodata, 116K init, 125K bss, 3944K reserved)
[ 0.000000] Virtual kernel memory layout:
[ 0.000000] vector : 0xffff0000 - 0xffff1000 ( 4 kB)
[ 0.000000] fixmap : 0xffc00000 - 0xffe00000 (2048 kB)
[ 0.000000] vmalloc : 0xc2800000 - 0xff000000 ( 968 MB)
[ 0.000000] lowmem : 0xc0000000 - 0xc2000000 ( 32 MB)
[ 0.000000] .text : 0xc0008000 - 0xc032ccd0 (3220 kB)
[ 0.000000] .init : 0xc032d000 - 0xc034a390 ( 117 kB)
[ 0.000000] .data : 0xc034c000 - 0xc0363b20 ( 95 kB)
[ 0.000000] .bss : 0xc0363b20 - 0xc03831c8 ( 126 kB)
[ 0.000000] SLUB: HWalign=32, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
[ 0.000000] Preemptible hierarchical RCU implementation.
[ 0.000000] NR_IRQS:16 nr_irqs:16 16
[ 0.000000] sched_clock: 32 bits at 100 Hz, resolution 10000000ns,
wraps every 21474836480000000ns
[ 0.010000] Calibrating delay loop... 146.84 BogoMIPS (lpj=734208)
[ 0.100000] pid_max: default: 4096 minimum: 301
[ 0.100000] Mount-cache hash table entries: 1024 (order: 0, 4096 bytes)
[ 0.110000] Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes)
[ 0.120000] CPU: Testing write buffer coherency: ok
[ 0.130000] Setting up static identity map for 0x2955c8 - 0x295610
[ 0.150000] devtmpfs: initialized
[ 0.170000] NET: Registered protocol family 16
[ 0.180000] DMA: preallocated 256 KiB pool for atomic coherent allocations
[ 0.280000] Switched to clocksource moxart_timer
[ 0.290000] NET: Registered protocol family 2
[ 0.300000] TCP established hash table entries: 1024 (order: 0, 4096 bytes)
[ 0.310000] TCP bind hash table entries: 1024 (order: 0, 4096 bytes)
[ 0.320000] TCP: Hash tables configured (established 1024 bind 1024)
[ 0.330000] TCP: reno registered
[ 0.330000] UDP hash table entries: 256 (order: 0, 4096 bytes)
[ 0.340000] UDP-Lite hash table entries: 256 (order: 0, 4096 bytes)
[ 0.350000] NET: Registered protocol family 1
[ 0.360000] futex hash table entries: 16 (order: -5, 192 bytes)
[ 0.410000] jffs2: version 2.2. (NAND) ?? 2001-2006 Red Hat, Inc.
[ 0.420000] msgmni has been set to 56
[ 0.430000] io scheduler noop registered
[ 0.430000] io scheduler cfq registered (default)
[ 0.440000] gpiochip_find_base: found new base at 224
[ 0.440000] gpiochip_add: registered GPIOs 224 to 255 on device: moxart-gpio
[ 0.450000] Serial: 8250/16550 driver, 1 ports, IRQ sharing enabled
[ 0.470000] console [ttyS0] disabled
[ 0.470000] 98200000.uart0: ttyS0 at MMIO 0x98200000 (irq = 21,
base_baud = 921600) is a 16550A
[ 0.480000] console [ttyS0] enabled
[ 0.480000] console [ttyS0] enabled
[ 0.490000] bootconsole [earlycon0] disabled
[ 0.490000] bootconsole [earlycon0] disabled
[ 0.500000] MOXA Smartio/Industio family driver version 2.0.5
[ 0.510000] mxser: found MOXA UC-7112-LX board (CAP=0x0)
[ 0.510000] ttyMX0 at I/O 0xf9820040 (irq = 21, base_baud = 0) is a 16550A
[ 0.520000] ttyMX1 at I/O 0xf9820060 (irq = 21, base_baud = 0) is a 16550A
[ 0.530000] mxser: mxser_initbrd success IRQ=21 max baud=921600 bps
[ 0.570000] loop: module loaded
[ 0.600000] 80000000.flash: Found 1 x16 devices at 0x0 in 16-bit
bank. Manufacturer ID 0x000089 Chip ID 0x000018
[ 0.610000] Intel/Sharp Extended Query Table at 0x0031
[ 0.620000] Intel/Sharp Extended Query Table at 0x0031
[ 0.620000] Using buffer write method
[ 0.620000] cfi_cmdset_0001: Erase suspend on write enabled
[ 0.630000] erase region 0: offset=0x0,size=0x20000,blocks=128
[ 0.640000] 4 ofpart partitions found on MTD device 80000000.flash
[ 0.640000] Creating 4 MTD partitions on "80000000.flash":
[ 0.650000] 0x000000000000-0x000000040000 : "bootloader"
[ 0.660000] 0x000000040000-0x000000200000 : "linux kernel"
[ 0.680000] 0x000000200000-0x000000a00000 : "root filesystem"
[ 0.690000] 0x000000a00000-0x000001000000 : "user filesystem"
[ 1.300000] libphy: MOXA ART Ethernet MII: probed
[ 1.920000] libphy: MOXA ART Ethernet MII: probed
[ 1.960000] of_get_named_gpiod_flags: parsed 'gpios' property of
node '/gpio_keys_polled/button at 25[0]' - status (0)
[ 1.970000] gpiod_request: __gpiod_request() status=0
[ 1.970000] input: gpio_keys_polled as /devices/gpio_keys_polled/input/input0
[ 1.980000] evbug: Connected device: input0 (gpio_keys_polled at
gpio-keys-polled/input0)
[ 1.990000] of_get_named_gpiod_flags: parsed 'gpio-rtc-data'
property of node '/soc/rtc[0]' - status (0)
[ 2.000000] of_get_named_gpiod_flags: parsed 'gpio-rtc-sclk'
property of node '/soc/rtc[0]' - status (0)
[ 2.010000] of_get_named_gpiod_flags: parsed 'gpio-rtc-reset'
property of node '/soc/rtc[0]' - status (0)
[ 2.020000] gpiod_request: __gpiod_request() status=0
[ 2.030000] gpiod_request: __gpiod_request() status=0
[ 2.030000] gpiod_request: __gpiod_request() status=0
[ 2.040000] moxart-rtc 90000000.soc:rtc: rtc core: registered
90000000.soc:rtc as rtc0
[ 2.060000] Unable to handle kernel NULL pointer dereference at
virtual address 00000020
[ 2.070000] pgd = c0004000
[ 2.070000] [00000020] *pgd=00000000
[ 2.070000] Internal error: Oops: 805 [#1] PREEMPT ARM
[ 2.070000] CPU: 0 PID: 1 Comm: swapper Not tainted
3.17.0-00628-g572d48d-dirty #3509
[ 2.070000] task: c1828000 ti: c182a000 task.ti: c182a000
[ 2.070000] PC is at moxart_probe+0xa0/0x378
[ 2.070000] LR is at of_clk_get_by_clkspec+0x38/0x48
[ 2.070000] pc : [<c01a241c>] lr : [<c01b2de4>] psr: 80000013
[ 2.070000] sp : c182be10 ip : c182a000 fp : 00000000
[ 2.070000] r10: c0363b20 r9 : 00000012 r8 : c1862100
[ 2.070000] r7 : c1862110 r6 : c1fb831c r5 : 00000000 r4 : c18d5400
[ 2.070000] r3 : 00000000 r2 : 00000001 r1 : c18090c0 r0 : c18090c0
[ 2.070000] Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM
Segment kernel
[ 2.070000] Control: 0000397f Table: 00004000 DAC: 00000017
[ 2.070000] Process swapper (pid: 1, stack limit = 0xc182a1c0)
[ 2.070000] Stack: (0xc182be10 to 0xc182c000)
[ 2.070000] be00: c198a860
c198a860 c03795d0 00000004
[ 2.070000] be20: 98e00000 98e00fff c1fb8374 00000200 00000000
00000000 00000000 c00da538
[ 2.070000] be40: c1991870 c1991910 00000001 c1991910 c02fe5f3
c1861820 c1991870 00000001
[ 2.070000] be60: c03466bc c1862110 c035c5f4 c035c5f4 c037d5f0
00000000 c03466bc c0363b20
[ 2.070000] be80: 00000000 c016e940 c016e914 c1862110 c1862144
c016d9ac c1862110 c1862144
[ 2.070000] bea0: c035c5f4 c035a648 c03466bc c016db9c 00000000
c035c5f4 c016db34 c016c4bc
[ 2.070000] bec0: c1807f4c c18565d0 c035c5f4 00000000 c197ea80
c016cc00 c030404e c0304050
[ 2.070000] bee0: 00000000 c035c5f4 c033c6d8 c182a008 00000000
c016e0c8 00000000 c198a800
[ 2.070000] bf00: c033c6d8 c032dc5c c1809f60 c1856000 c1856000
c0296be0 c0375a00 00000000
[ 2.070000] bf20: 00000000 c00d2320 c02eedd6 c1809f60 c0314584
00000000 c1ffce23 c0027ab0
[ 2.070000] bf40: c0314584 0000003d 00000006 00000006 00000000
00000006 0000003d 00000006
[ 2.070000] bf60: 0000003d c0346ee8 c034a29c c0363b20 c0346ef0
c032de08 00000006 00000006
[ 2.070000] bf80: c032d50c c182a000 00000000 c028eee8 00000000
00000000 00000000 00000000
[ 2.070000] bfa0: 00000000 c028eef0 00000000 c0009230 00000000
00000000 00000000 00000000
[ 2.070000] bfc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[ 2.070000] bfe0: 00000000 00000000 00000000 00000000 00000013
00000000 ffffffff ffffffff
[ 2.070000] [<c01a241c>] (moxart_probe) from [<c016e940>]
(platform_drv_probe+0x2c/0x60)
[ 2.070000] [<c016e940>] (platform_drv_probe) from [<c016d9ac>]
(driver_probe_device+0xa8/0x1e8)
[ 2.070000] [<c016d9ac>] (driver_probe_device) from [<c016db9c>]
(__driver_attach+0x68/0x88)
[ 2.070000] [<c016db9c>] (__driver_attach) from [<c016c4bc>]
(bus_for_each_dev+0x70/0x94)
[ 2.070000] [<c016c4bc>] (bus_for_each_dev) from [<c016cc00>]
(bus_add_driver+0xd0/0x1b8)
[ 2.070000] [<c016cc00>] (bus_add_driver) from [<c016e0c8>]
(driver_register+0x9c/0xe0)
[ 2.070000] [<c016e0c8>] (driver_register) from [<c032dc5c>]
(do_one_initcall+0x108/0x1bc)
[ 2.070000] [<c032dc5c>] (do_one_initcall) from [<c032de08>]
(kernel_init_freeable+0xf8/0x1b0)
[ 2.070000] [<c032de08>] (kernel_init_freeable) from [<c028eef0>]
(kernel_init+0x8/0xe0)
[ 2.070000] [<c028eef0>] (kernel_init) from [<c0009230>]
(ret_from_fork+0x14/0x24)
[ 2.070000] Code: e1a00006 e1a01005 eb004276 e3700a01 (e5850020)
[ 2.080000] ---[ end trace b4cd6d53de4c770d ]---
[ 2.090000] Kernel panic - not syncing: Fatal exception
[ 2.090000] Rebooting in 10 seconds..
Tested-by: Jonas Jensen <jonas.jensen@gmail.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] mmc: moxart: fix probe logic
2015-02-02 10:52 ` Jonas Jensen
@ 2015-02-02 14:58 ` Arnd Bergmann
-1 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-02-02 14:58 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Jonas Jensen, linux-mmc, Ulf Hansson, Russell King - ARM Linux
On Monday 02 February 2015 11:52:44 Jonas Jensen wrote:
>
> On 29 January 2015 at 23:06, Arnd Bergmann <arnd@arndb.de> wrote:
> > Jonas Jensen wanted to submit a patch for these, but apparently
> > forgot about it. I stumbled over this symptom first:
>
> Sorry about this, I remember thinking about the changes but only made
> a mental note (which was lost over time). I was sidetracked with other
> changes and work.
>
>
> > @@ -586,10 +586,10 @@ static int moxart_probe(struct platform_device *pdev)
> > goto out;
> > }
> >
> > - clk = of_clk_get(node, 0);
> > - if (IS_ERR(clk)) {
> > + host->clk = of_clk_get(node, 0);
>
> host->clk is a NULL dereference at this point in probe() (log below).
>
> I moved the single line "host = mmc_priv(mmc);" to happen just before,
> and everything is working normally again.
Thanks a lot for testing, and finding the second embarrassing bug
in my patch!
Can you forward the patch you successfully tested with your
Signed-off-by?
Arnd
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2] mmc: moxart: fix probe logic
@ 2015-02-02 14:58 ` Arnd Bergmann
0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-02-02 14:58 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 02 February 2015 11:52:44 Jonas Jensen wrote:
>
> On 29 January 2015 at 23:06, Arnd Bergmann <arnd@arndb.de> wrote:
> > Jonas Jensen wanted to submit a patch for these, but apparently
> > forgot about it. I stumbled over this symptom first:
>
> Sorry about this, I remember thinking about the changes but only made
> a mental note (which was lost over time). I was sidetracked with other
> changes and work.
>
>
> > @@ -586,10 +586,10 @@ static int moxart_probe(struct platform_device *pdev)
> > goto out;
> > }
> >
> > - clk = of_clk_get(node, 0);
> > - if (IS_ERR(clk)) {
> > + host->clk = of_clk_get(node, 0);
>
> host->clk is a NULL dereference at this point in probe() (log below).
>
> I moved the single line "host = mmc_priv(mmc);" to happen just before,
> and everything is working normally again.
Thanks a lot for testing, and finding the second embarrassing bug
in my patch!
Can you forward the patch you successfully tested with your
Signed-off-by?
Arnd
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3] mmc: moxart: fix probe logic
2015-02-02 14:58 ` Arnd Bergmann
@ 2015-02-02 15:43 ` Jonas Jensen
-1 siblings, 0 replies; 24+ messages in thread
From: Jonas Jensen @ 2015-02-02 15:43 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-arm-kernel, arnd, ulf.hansson, linux, Jonas Jensen
Jonas Jensen wanted to submit a patch for these, but apparently
forgot about it. I stumbled over this symptom first:
drivers/built-in.o: In function `moxart_probe':
:(.text+0x2af128): undefined reference to `of_dma_request_slave_channel'
This is because of_dma_request_slave_channel is an internal helper
and not exported to loadable module. I'm changing the driver to
use dma_request_slave_channel_reason() instead.
Further problems from inspection:
* The remove function must not call kfree on the host pointer,
because it is allocated together with the mmc_host.
* The clock is never released
* The dma_cap_mask_t is completely unused and can be removed
* deferred probing does not work if the dma driver is loaded
after the mmc driver.
This patch should fix all of the above.
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
This is a forward (with a single line fix) of Arnd's v2 patch.
v2: do not call clk_put() on an error pointer
v3: fix NULL dereference on host->clk
Applies to next-20150113
drivers/mmc/host/moxart-mmc.c | 34 +++++++++++++++++++---------------
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/mmc/host/moxart-mmc.c b/drivers/mmc/host/moxart-mmc.c
index d2a1ef6..6ab57d1e 100644
--- a/drivers/mmc/host/moxart-mmc.c
+++ b/drivers/mmc/host/moxart-mmc.c
@@ -17,6 +17,7 @@
#include <linux/init.h>
#include <linux/platform_device.h>
#include <linux/delay.h>
+#include <linux/errno.h>
#include <linux/interrupt.h>
#include <linux/blkdev.h>
#include <linux/dma-mapping.h>
@@ -131,6 +132,7 @@ struct moxart_host {
struct mmc_host *mmc;
struct mmc_request *mrq;
struct scatterlist *cur_sg;
+ struct clk *clk;
struct completion dma_complete;
struct completion pio_complete;
@@ -560,9 +562,7 @@ static int moxart_probe(struct platform_device *pdev)
struct mmc_host *mmc;
struct moxart_host *host = NULL;
struct dma_slave_config cfg;
- struct clk *clk;
void __iomem *reg_mmc;
- dma_cap_mask_t mask;
int irq, ret;
u32 i;
@@ -573,6 +573,8 @@ static int moxart_probe(struct platform_device *pdev)
goto out;
}
+ host = mmc_priv(mmc);
+
ret = of_address_to_resource(node, 0, &res_mmc);
if (ret) {
dev_err(dev, "of_address_to_resource failed\n");
@@ -586,10 +588,11 @@ static int moxart_probe(struct platform_device *pdev)
goto out;
}
- clk = of_clk_get(node, 0);
- if (IS_ERR(clk)) {
+ host->clk = of_clk_get(node, 0);
+ if (IS_ERR(host->clk)) {
dev_err(dev, "of_clk_get failed\n");
- ret = PTR_ERR(clk);
+ ret = PTR_ERR(host->clk);
+ host->clk = NULL;
goto out;
}
@@ -603,18 +606,14 @@ static int moxart_probe(struct platform_device *pdev)
if (ret)
goto out;
- dma_cap_zero(mask);
- dma_cap_set(DMA_SLAVE, mask);
-
- host = mmc_priv(mmc);
host->mmc = mmc;
host->base = reg_mmc;
host->reg_phys = res_mmc.start;
host->timeout = msecs_to_jiffies(1000);
- host->sysclk = clk_get_rate(clk);
+ host->sysclk = clk_get_rate(host->clk);
host->fifo_width = readl(host->base + REG_FEATURE) << 2;
- host->dma_chan_tx = of_dma_request_slave_channel(node, "tx");
- host->dma_chan_rx = of_dma_request_slave_channel(node, "rx");
+ host->dma_chan_tx = dma_request_slave_channel_reason(dev, "tx");
+ host->dma_chan_rx = dma_request_slave_channel_reason(dev, "rx");
spin_lock_init(&host->lock);
@@ -624,6 +623,11 @@ static int moxart_probe(struct platform_device *pdev)
mmc->ocr_avail = 0xffff00; /* Support 2.0v - 3.6v power. */
if (IS_ERR(host->dma_chan_tx) || IS_ERR(host->dma_chan_rx)) {
+ if (PTR_ERR(host->dma_chan_tx) == -EPROBE_DEFER ||
+ PTR_ERR(host->dma_chan_rx) == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto out;
+ }
dev_dbg(dev, "PIO mode transfer enabled\n");
host->have_dma = false;
} else {
@@ -677,6 +681,8 @@ static int moxart_probe(struct platform_device *pdev)
return 0;
out:
+ if (host->clk)
+ clk_put(host->clk);
if (mmc)
mmc_free_host(mmc);
return ret;
@@ -694,6 +700,7 @@ static int moxart_remove(struct platform_device *pdev)
dma_release_channel(host->dma_chan_tx);
if (!IS_ERR(host->dma_chan_rx))
dma_release_channel(host->dma_chan_rx);
+ clk_put(host->clk);
mmc_remove_host(mmc);
mmc_free_host(mmc);
@@ -702,9 +709,6 @@ static int moxart_remove(struct platform_device *pdev)
writel(readl(host->base + REG_CLOCK_CONTROL) | CLK_OFF,
host->base + REG_CLOCK_CONTROL);
}
-
- kfree(host);
-
return 0;
}
--
1.8.2.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3] mmc: moxart: fix probe logic
@ 2015-02-02 15:43 ` Jonas Jensen
0 siblings, 0 replies; 24+ messages in thread
From: Jonas Jensen @ 2015-02-02 15:43 UTC (permalink / raw)
To: linux-arm-kernel
Jonas Jensen wanted to submit a patch for these, but apparently
forgot about it. I stumbled over this symptom first:
drivers/built-in.o: In function `moxart_probe':
:(.text+0x2af128): undefined reference to `of_dma_request_slave_channel'
This is because of_dma_request_slave_channel is an internal helper
and not exported to loadable module. I'm changing the driver to
use dma_request_slave_channel_reason() instead.
Further problems from inspection:
* The remove function must not call kfree on the host pointer,
because it is allocated together with the mmc_host.
* The clock is never released
* The dma_cap_mask_t is completely unused and can be removed
* deferred probing does not work if the dma driver is loaded
after the mmc driver.
This patch should fix all of the above.
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
This is a forward (with a single line fix) of Arnd's v2 patch.
v2: do not call clk_put() on an error pointer
v3: fix NULL dereference on host->clk
Applies to next-20150113
drivers/mmc/host/moxart-mmc.c | 34 +++++++++++++++++++---------------
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/mmc/host/moxart-mmc.c b/drivers/mmc/host/moxart-mmc.c
index d2a1ef6..6ab57d1e 100644
--- a/drivers/mmc/host/moxart-mmc.c
+++ b/drivers/mmc/host/moxart-mmc.c
@@ -17,6 +17,7 @@
#include <linux/init.h>
#include <linux/platform_device.h>
#include <linux/delay.h>
+#include <linux/errno.h>
#include <linux/interrupt.h>
#include <linux/blkdev.h>
#include <linux/dma-mapping.h>
@@ -131,6 +132,7 @@ struct moxart_host {
struct mmc_host *mmc;
struct mmc_request *mrq;
struct scatterlist *cur_sg;
+ struct clk *clk;
struct completion dma_complete;
struct completion pio_complete;
@@ -560,9 +562,7 @@ static int moxart_probe(struct platform_device *pdev)
struct mmc_host *mmc;
struct moxart_host *host = NULL;
struct dma_slave_config cfg;
- struct clk *clk;
void __iomem *reg_mmc;
- dma_cap_mask_t mask;
int irq, ret;
u32 i;
@@ -573,6 +573,8 @@ static int moxart_probe(struct platform_device *pdev)
goto out;
}
+ host = mmc_priv(mmc);
+
ret = of_address_to_resource(node, 0, &res_mmc);
if (ret) {
dev_err(dev, "of_address_to_resource failed\n");
@@ -586,10 +588,11 @@ static int moxart_probe(struct platform_device *pdev)
goto out;
}
- clk = of_clk_get(node, 0);
- if (IS_ERR(clk)) {
+ host->clk = of_clk_get(node, 0);
+ if (IS_ERR(host->clk)) {
dev_err(dev, "of_clk_get failed\n");
- ret = PTR_ERR(clk);
+ ret = PTR_ERR(host->clk);
+ host->clk = NULL;
goto out;
}
@@ -603,18 +606,14 @@ static int moxart_probe(struct platform_device *pdev)
if (ret)
goto out;
- dma_cap_zero(mask);
- dma_cap_set(DMA_SLAVE, mask);
-
- host = mmc_priv(mmc);
host->mmc = mmc;
host->base = reg_mmc;
host->reg_phys = res_mmc.start;
host->timeout = msecs_to_jiffies(1000);
- host->sysclk = clk_get_rate(clk);
+ host->sysclk = clk_get_rate(host->clk);
host->fifo_width = readl(host->base + REG_FEATURE) << 2;
- host->dma_chan_tx = of_dma_request_slave_channel(node, "tx");
- host->dma_chan_rx = of_dma_request_slave_channel(node, "rx");
+ host->dma_chan_tx = dma_request_slave_channel_reason(dev, "tx");
+ host->dma_chan_rx = dma_request_slave_channel_reason(dev, "rx");
spin_lock_init(&host->lock);
@@ -624,6 +623,11 @@ static int moxart_probe(struct platform_device *pdev)
mmc->ocr_avail = 0xffff00; /* Support 2.0v - 3.6v power. */
if (IS_ERR(host->dma_chan_tx) || IS_ERR(host->dma_chan_rx)) {
+ if (PTR_ERR(host->dma_chan_tx) == -EPROBE_DEFER ||
+ PTR_ERR(host->dma_chan_rx) == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto out;
+ }
dev_dbg(dev, "PIO mode transfer enabled\n");
host->have_dma = false;
} else {
@@ -677,6 +681,8 @@ static int moxart_probe(struct platform_device *pdev)
return 0;
out:
+ if (host->clk)
+ clk_put(host->clk);
if (mmc)
mmc_free_host(mmc);
return ret;
@@ -694,6 +700,7 @@ static int moxart_remove(struct platform_device *pdev)
dma_release_channel(host->dma_chan_tx);
if (!IS_ERR(host->dma_chan_rx))
dma_release_channel(host->dma_chan_rx);
+ clk_put(host->clk);
mmc_remove_host(mmc);
mmc_free_host(mmc);
@@ -702,9 +709,6 @@ static int moxart_remove(struct platform_device *pdev)
writel(readl(host->base + REG_CLOCK_CONTROL) | CLK_OFF,
host->base + REG_CLOCK_CONTROL);
}
-
- kfree(host);
-
return 0;
}
--
1.8.2.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v3] mmc: moxart: fix probe logic
2015-02-02 15:43 ` Jonas Jensen
@ 2015-02-02 20:03 ` Arnd Bergmann
-1 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-02-02 20:03 UTC (permalink / raw)
To: Jonas Jensen; +Cc: linux-mmc, linux-arm-kernel, ulf.hansson, linux
On Monday 02 February 2015 16:43:06 Jonas Jensen wrote:
>
> Notes:
> This is a forward (with a single line fix) of Arnd's v2 patch.
>
> v2: do not call clk_put() on an error pointer
> v3: fix NULL dereference on host->clk
>
> Applies to next-20150113
Thanks for forwarding the fixed version. I just realized that you
probably haven't forwarded a lot of patches from other people, so you
missed two things:
- to preserve patch ownership, the first line (before the rest of the
description) should be 'From: Arnd Bergmann <arnd@arndb.de>'
- It is common to include the entire signoff chain, which in this case
is my Signed-off-by on one line, followed by yours on the next line
at the end of the description.
Ulf can probably fix this up when he applies the patch, or you can
send it again to be sure.
Arnd
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3] mmc: moxart: fix probe logic
@ 2015-02-02 20:03 ` Arnd Bergmann
0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-02-02 20:03 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 02 February 2015 16:43:06 Jonas Jensen wrote:
>
> Notes:
> This is a forward (with a single line fix) of Arnd's v2 patch.
>
> v2: do not call clk_put() on an error pointer
> v3: fix NULL dereference on host->clk
>
> Applies to next-20150113
Thanks for forwarding the fixed version. I just realized that you
probably haven't forwarded a lot of patches from other people, so you
missed two things:
- to preserve patch ownership, the first line (before the rest of the
description) should be 'From: Arnd Bergmann <arnd@arndb.de>'
- It is common to include the entire signoff chain, which in this case
is my Signed-off-by on one line, followed by yours on the next line
at the end of the description.
Ulf can probably fix this up when he applies the patch, or you can
send it again to be sure.
Arnd
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3] mmc: moxart: fix probe logic
2015-02-02 15:43 ` Jonas Jensen
@ 2015-02-03 8:02 ` Ulf Hansson
-1 siblings, 0 replies; 24+ messages in thread
From: Ulf Hansson @ 2015-02-03 8:02 UTC (permalink / raw)
To: Jonas Jensen, Arnd Bergmann
Cc: linux-mmc, linux-arm-kernel, Russell King - ARM Linux
On 2 February 2015 at 16:43, Jonas Jensen <jonas.jensen@gmail.com> wrote:
> Jonas Jensen wanted to submit a patch for these, but apparently
> forgot about it. I stumbled over this symptom first:
>
> drivers/built-in.o: In function `moxart_probe':
> :(.text+0x2af128): undefined reference to `of_dma_request_slave_channel'
>
> This is because of_dma_request_slave_channel is an internal helper
> and not exported to loadable module. I'm changing the driver to
> use dma_request_slave_channel_reason() instead.
>
> Further problems from inspection:
>
> * The remove function must not call kfree on the host pointer,
> because it is allocated together with the mmc_host.
>
> * The clock is never released
>
> * The dma_cap_mask_t is completely unused and can be removed
>
> * deferred probing does not work if the dma driver is loaded
> after the mmc driver.
>
> This patch should fix all of the above.
>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
> ---
>
> Notes:
> This is a forward (with a single line fix) of Arnd's v2 patch.
>
> v2: do not call clk_put() on an error pointer
> v3: fix NULL dereference on host->clk
>
> Applies to next-20150113
>
> drivers/mmc/host/moxart-mmc.c | 34 +++++++++++++++++++---------------
> 1 file changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/mmc/host/moxart-mmc.c b/drivers/mmc/host/moxart-mmc.c
> index d2a1ef6..6ab57d1e 100644
> --- a/drivers/mmc/host/moxart-mmc.c
> +++ b/drivers/mmc/host/moxart-mmc.c
> @@ -17,6 +17,7 @@
> #include <linux/init.h>
> #include <linux/platform_device.h>
> #include <linux/delay.h>
> +#include <linux/errno.h>
> #include <linux/interrupt.h>
> #include <linux/blkdev.h>
> #include <linux/dma-mapping.h>
> @@ -131,6 +132,7 @@ struct moxart_host {
> struct mmc_host *mmc;
> struct mmc_request *mrq;
> struct scatterlist *cur_sg;
> + struct clk *clk;
> struct completion dma_complete;
> struct completion pio_complete;
>
> @@ -560,9 +562,7 @@ static int moxart_probe(struct platform_device *pdev)
> struct mmc_host *mmc;
> struct moxart_host *host = NULL;
> struct dma_slave_config cfg;
> - struct clk *clk;
> void __iomem *reg_mmc;
> - dma_cap_mask_t mask;
> int irq, ret;
> u32 i;
>
> @@ -573,6 +573,8 @@ static int moxart_probe(struct platform_device *pdev)
> goto out;
> }
>
> + host = mmc_priv(mmc);
> +
> ret = of_address_to_resource(node, 0, &res_mmc);
> if (ret) {
> dev_err(dev, "of_address_to_resource failed\n");
> @@ -586,10 +588,11 @@ static int moxart_probe(struct platform_device *pdev)
> goto out;
> }
>
> - clk = of_clk_get(node, 0);
This code would be simpler using devm_clk_get(), since then error
wouldn't be needed.
Any reason to why you don't want to use that?
> - if (IS_ERR(clk)) {
> + host->clk = of_clk_get(node, 0);
> + if (IS_ERR(host->clk)) {
> dev_err(dev, "of_clk_get failed\n");
> - ret = PTR_ERR(clk);
> + ret = PTR_ERR(host->clk);
> + host->clk = NULL;
> goto out;
> }
>
> @@ -603,18 +606,14 @@ static int moxart_probe(struct platform_device *pdev)
> if (ret)
> goto out;
>
> - dma_cap_zero(mask);
> - dma_cap_set(DMA_SLAVE, mask);
> -
> - host = mmc_priv(mmc);
> host->mmc = mmc;
> host->base = reg_mmc;
> host->reg_phys = res_mmc.start;
> host->timeout = msecs_to_jiffies(1000);
> - host->sysclk = clk_get_rate(clk);
> + host->sysclk = clk_get_rate(host->clk);
> host->fifo_width = readl(host->base + REG_FEATURE) << 2;
> - host->dma_chan_tx = of_dma_request_slave_channel(node, "tx");
> - host->dma_chan_rx = of_dma_request_slave_channel(node, "rx");
> + host->dma_chan_tx = dma_request_slave_channel_reason(dev, "tx");
> + host->dma_chan_rx = dma_request_slave_channel_reason(dev, "rx");
>
> spin_lock_init(&host->lock);
>
> @@ -624,6 +623,11 @@ static int moxart_probe(struct platform_device *pdev)
> mmc->ocr_avail = 0xffff00; /* Support 2.0v - 3.6v power. */
>
> if (IS_ERR(host->dma_chan_tx) || IS_ERR(host->dma_chan_rx)) {
> + if (PTR_ERR(host->dma_chan_tx) == -EPROBE_DEFER ||
> + PTR_ERR(host->dma_chan_rx) == -EPROBE_DEFER) {
> + ret = -EPROBE_DEFER;
> + goto out;
> + }
> dev_dbg(dev, "PIO mode transfer enabled\n");
> host->have_dma = false;
> } else {
> @@ -677,6 +681,8 @@ static int moxart_probe(struct platform_device *pdev)
> return 0;
>
> out:
> + if (host->clk)
> + clk_put(host->clk);
> if (mmc)
> mmc_free_host(mmc);
> return ret;
> @@ -694,6 +700,7 @@ static int moxart_remove(struct platform_device *pdev)
> dma_release_channel(host->dma_chan_tx);
> if (!IS_ERR(host->dma_chan_rx))
> dma_release_channel(host->dma_chan_rx);
> + clk_put(host->clk);
> mmc_remove_host(mmc);
> mmc_free_host(mmc);
>
> @@ -702,9 +709,6 @@ static int moxart_remove(struct platform_device *pdev)
> writel(readl(host->base + REG_CLOCK_CONTROL) | CLK_OFF,
> host->base + REG_CLOCK_CONTROL);
> }
> -
> - kfree(host);
> -
> return 0;
> }
>
> --
> 1.8.2.1
>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3] mmc: moxart: fix probe logic
@ 2015-02-03 8:02 ` Ulf Hansson
0 siblings, 0 replies; 24+ messages in thread
From: Ulf Hansson @ 2015-02-03 8:02 UTC (permalink / raw)
To: linux-arm-kernel
On 2 February 2015 at 16:43, Jonas Jensen <jonas.jensen@gmail.com> wrote:
> Jonas Jensen wanted to submit a patch for these, but apparently
> forgot about it. I stumbled over this symptom first:
>
> drivers/built-in.o: In function `moxart_probe':
> :(.text+0x2af128): undefined reference to `of_dma_request_slave_channel'
>
> This is because of_dma_request_slave_channel is an internal helper
> and not exported to loadable module. I'm changing the driver to
> use dma_request_slave_channel_reason() instead.
>
> Further problems from inspection:
>
> * The remove function must not call kfree on the host pointer,
> because it is allocated together with the mmc_host.
>
> * The clock is never released
>
> * The dma_cap_mask_t is completely unused and can be removed
>
> * deferred probing does not work if the dma driver is loaded
> after the mmc driver.
>
> This patch should fix all of the above.
>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
> ---
>
> Notes:
> This is a forward (with a single line fix) of Arnd's v2 patch.
>
> v2: do not call clk_put() on an error pointer
> v3: fix NULL dereference on host->clk
>
> Applies to next-20150113
>
> drivers/mmc/host/moxart-mmc.c | 34 +++++++++++++++++++---------------
> 1 file changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/mmc/host/moxart-mmc.c b/drivers/mmc/host/moxart-mmc.c
> index d2a1ef6..6ab57d1e 100644
> --- a/drivers/mmc/host/moxart-mmc.c
> +++ b/drivers/mmc/host/moxart-mmc.c
> @@ -17,6 +17,7 @@
> #include <linux/init.h>
> #include <linux/platform_device.h>
> #include <linux/delay.h>
> +#include <linux/errno.h>
> #include <linux/interrupt.h>
> #include <linux/blkdev.h>
> #include <linux/dma-mapping.h>
> @@ -131,6 +132,7 @@ struct moxart_host {
> struct mmc_host *mmc;
> struct mmc_request *mrq;
> struct scatterlist *cur_sg;
> + struct clk *clk;
> struct completion dma_complete;
> struct completion pio_complete;
>
> @@ -560,9 +562,7 @@ static int moxart_probe(struct platform_device *pdev)
> struct mmc_host *mmc;
> struct moxart_host *host = NULL;
> struct dma_slave_config cfg;
> - struct clk *clk;
> void __iomem *reg_mmc;
> - dma_cap_mask_t mask;
> int irq, ret;
> u32 i;
>
> @@ -573,6 +573,8 @@ static int moxart_probe(struct platform_device *pdev)
> goto out;
> }
>
> + host = mmc_priv(mmc);
> +
> ret = of_address_to_resource(node, 0, &res_mmc);
> if (ret) {
> dev_err(dev, "of_address_to_resource failed\n");
> @@ -586,10 +588,11 @@ static int moxart_probe(struct platform_device *pdev)
> goto out;
> }
>
> - clk = of_clk_get(node, 0);
This code would be simpler using devm_clk_get(), since then error
wouldn't be needed.
Any reason to why you don't want to use that?
> - if (IS_ERR(clk)) {
> + host->clk = of_clk_get(node, 0);
> + if (IS_ERR(host->clk)) {
> dev_err(dev, "of_clk_get failed\n");
> - ret = PTR_ERR(clk);
> + ret = PTR_ERR(host->clk);
> + host->clk = NULL;
> goto out;
> }
>
> @@ -603,18 +606,14 @@ static int moxart_probe(struct platform_device *pdev)
> if (ret)
> goto out;
>
> - dma_cap_zero(mask);
> - dma_cap_set(DMA_SLAVE, mask);
> -
> - host = mmc_priv(mmc);
> host->mmc = mmc;
> host->base = reg_mmc;
> host->reg_phys = res_mmc.start;
> host->timeout = msecs_to_jiffies(1000);
> - host->sysclk = clk_get_rate(clk);
> + host->sysclk = clk_get_rate(host->clk);
> host->fifo_width = readl(host->base + REG_FEATURE) << 2;
> - host->dma_chan_tx = of_dma_request_slave_channel(node, "tx");
> - host->dma_chan_rx = of_dma_request_slave_channel(node, "rx");
> + host->dma_chan_tx = dma_request_slave_channel_reason(dev, "tx");
> + host->dma_chan_rx = dma_request_slave_channel_reason(dev, "rx");
>
> spin_lock_init(&host->lock);
>
> @@ -624,6 +623,11 @@ static int moxart_probe(struct platform_device *pdev)
> mmc->ocr_avail = 0xffff00; /* Support 2.0v - 3.6v power. */
>
> if (IS_ERR(host->dma_chan_tx) || IS_ERR(host->dma_chan_rx)) {
> + if (PTR_ERR(host->dma_chan_tx) == -EPROBE_DEFER ||
> + PTR_ERR(host->dma_chan_rx) == -EPROBE_DEFER) {
> + ret = -EPROBE_DEFER;
> + goto out;
> + }
> dev_dbg(dev, "PIO mode transfer enabled\n");
> host->have_dma = false;
> } else {
> @@ -677,6 +681,8 @@ static int moxart_probe(struct platform_device *pdev)
> return 0;
>
> out:
> + if (host->clk)
> + clk_put(host->clk);
> if (mmc)
> mmc_free_host(mmc);
> return ret;
> @@ -694,6 +700,7 @@ static int moxart_remove(struct platform_device *pdev)
> dma_release_channel(host->dma_chan_tx);
> if (!IS_ERR(host->dma_chan_rx))
> dma_release_channel(host->dma_chan_rx);
> + clk_put(host->clk);
> mmc_remove_host(mmc);
> mmc_free_host(mmc);
>
> @@ -702,9 +709,6 @@ static int moxart_remove(struct platform_device *pdev)
> writel(readl(host->base + REG_CLOCK_CONTROL) | CLK_OFF,
> host->base + REG_CLOCK_CONTROL);
> }
> -
> - kfree(host);
> -
> return 0;
> }
>
> --
> 1.8.2.1
>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3] mmc: moxart: fix probe logic
2015-02-03 8:02 ` Ulf Hansson
@ 2015-02-03 9:06 ` Arnd Bergmann
-1 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-02-03 9:06 UTC (permalink / raw)
To: Ulf Hansson
Cc: Jonas Jensen, linux-mmc, linux-arm-kernel, Russell King - ARM Linux
On Tuesday 03 February 2015 09:02:22 Ulf Hansson wrote:
> > @@ -586,10 +588,11 @@ static int moxart_probe(struct platform_device *pdev)
> > goto out;
> > }
> >
> > - clk = of_clk_get(node, 0);
>
> This code would be simpler using devm_clk_get(), since then error
> wouldn't be needed.
>
> Any reason to why you don't want to use that?
You are absolutely right, and that would have avoided both of the bugs
I introduced.
Arnd
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3] mmc: moxart: fix probe logic
@ 2015-02-03 9:06 ` Arnd Bergmann
0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-02-03 9:06 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday 03 February 2015 09:02:22 Ulf Hansson wrote:
> > @@ -586,10 +588,11 @@ static int moxart_probe(struct platform_device *pdev)
> > goto out;
> > }
> >
> > - clk = of_clk_get(node, 0);
>
> This code would be simpler using devm_clk_get(), since then error
> wouldn't be needed.
>
> Any reason to why you don't want to use that?
You are absolutely right, and that would have avoided both of the bugs
I introduced.
Arnd
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v4] mmc: moxart: fix probe logic
2015-02-02 20:03 ` Arnd Bergmann
@ 2015-02-03 15:55 ` Jonas Jensen
-1 siblings, 0 replies; 24+ messages in thread
From: Jonas Jensen @ 2015-02-03 15:55 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-arm-kernel, arnd, ulf.hansson, linux, Jonas Jensen
From: Arnd Bergmann <arnd@arndb.de>
Jonas Jensen wanted to submit a patch for these, but apparently
forgot about it. I stumbled over this symptom first:
drivers/built-in.o: In function `moxart_probe':
:(.text+0x2af128): undefined reference to `of_dma_request_slave_channel'
This is because of_dma_request_slave_channel is an internal helper
and not exported to loadable module. I'm changing the driver to
use dma_request_slave_channel_reason() instead.
Further problems from inspection:
* The remove function must not call kfree on the host pointer,
because it is allocated together with the mmc_host.
* The clock is never released
* The dma_cap_mask_t is completely unused and can be removed
* deferred probing does not work if the dma driver is loaded
after the mmc driver.
This patch should fix all of the above.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
This is a forward of Arnd's v2 patch.
v2: do not call clk_put() on an error pointer
v3: fix NULL dereference on host->clk
v4: rework to use devm_clk_get()
Applies to next-20150113
drivers/mmc/host/moxart-mmc.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/mmc/host/moxart-mmc.c b/drivers/mmc/host/moxart-mmc.c
index d2a1ef6..6cb4053 100644
--- a/drivers/mmc/host/moxart-mmc.c
+++ b/drivers/mmc/host/moxart-mmc.c
@@ -17,6 +17,7 @@
#include <linux/init.h>
#include <linux/platform_device.h>
#include <linux/delay.h>
+#include <linux/errno.h>
#include <linux/interrupt.h>
#include <linux/blkdev.h>
#include <linux/dma-mapping.h>
@@ -562,7 +563,6 @@ static int moxart_probe(struct platform_device *pdev)
struct dma_slave_config cfg;
struct clk *clk;
void __iomem *reg_mmc;
- dma_cap_mask_t mask;
int irq, ret;
u32 i;
@@ -586,9 +586,9 @@ static int moxart_probe(struct platform_device *pdev)
goto out;
}
- clk = of_clk_get(node, 0);
+ clk = devm_clk_get(dev, 0);
if (IS_ERR(clk)) {
- dev_err(dev, "of_clk_get failed\n");
+ dev_err(dev, "devm_clk_get failed\n");
ret = PTR_ERR(clk);
goto out;
}
@@ -603,9 +603,6 @@ static int moxart_probe(struct platform_device *pdev)
if (ret)
goto out;
- dma_cap_zero(mask);
- dma_cap_set(DMA_SLAVE, mask);
-
host = mmc_priv(mmc);
host->mmc = mmc;
host->base = reg_mmc;
@@ -613,8 +610,8 @@ static int moxart_probe(struct platform_device *pdev)
host->timeout = msecs_to_jiffies(1000);
host->sysclk = clk_get_rate(clk);
host->fifo_width = readl(host->base + REG_FEATURE) << 2;
- host->dma_chan_tx = of_dma_request_slave_channel(node, "tx");
- host->dma_chan_rx = of_dma_request_slave_channel(node, "rx");
+ host->dma_chan_tx = dma_request_slave_channel_reason(dev, "tx");
+ host->dma_chan_rx = dma_request_slave_channel_reason(dev, "rx");
spin_lock_init(&host->lock);
@@ -624,6 +621,11 @@ static int moxart_probe(struct platform_device *pdev)
mmc->ocr_avail = 0xffff00; /* Support 2.0v - 3.6v power. */
if (IS_ERR(host->dma_chan_tx) || IS_ERR(host->dma_chan_rx)) {
+ if (PTR_ERR(host->dma_chan_tx) == -EPROBE_DEFER ||
+ PTR_ERR(host->dma_chan_rx) == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto out;
+ }
dev_dbg(dev, "PIO mode transfer enabled\n");
host->have_dma = false;
} else {
@@ -702,9 +704,6 @@ static int moxart_remove(struct platform_device *pdev)
writel(readl(host->base + REG_CLOCK_CONTROL) | CLK_OFF,
host->base + REG_CLOCK_CONTROL);
}
-
- kfree(host);
-
return 0;
}
--
1.8.2.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v4] mmc: moxart: fix probe logic
@ 2015-02-03 15:55 ` Jonas Jensen
0 siblings, 0 replies; 24+ messages in thread
From: Jonas Jensen @ 2015-02-03 15:55 UTC (permalink / raw)
To: linux-arm-kernel
From: Arnd Bergmann <arnd@arndb.de>
Jonas Jensen wanted to submit a patch for these, but apparently
forgot about it. I stumbled over this symptom first:
drivers/built-in.o: In function `moxart_probe':
:(.text+0x2af128): undefined reference to `of_dma_request_slave_channel'
This is because of_dma_request_slave_channel is an internal helper
and not exported to loadable module. I'm changing the driver to
use dma_request_slave_channel_reason() instead.
Further problems from inspection:
* The remove function must not call kfree on the host pointer,
because it is allocated together with the mmc_host.
* The clock is never released
* The dma_cap_mask_t is completely unused and can be removed
* deferred probing does not work if the dma driver is loaded
after the mmc driver.
This patch should fix all of the above.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
This is a forward of Arnd's v2 patch.
v2: do not call clk_put() on an error pointer
v3: fix NULL dereference on host->clk
v4: rework to use devm_clk_get()
Applies to next-20150113
drivers/mmc/host/moxart-mmc.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/mmc/host/moxart-mmc.c b/drivers/mmc/host/moxart-mmc.c
index d2a1ef6..6cb4053 100644
--- a/drivers/mmc/host/moxart-mmc.c
+++ b/drivers/mmc/host/moxart-mmc.c
@@ -17,6 +17,7 @@
#include <linux/init.h>
#include <linux/platform_device.h>
#include <linux/delay.h>
+#include <linux/errno.h>
#include <linux/interrupt.h>
#include <linux/blkdev.h>
#include <linux/dma-mapping.h>
@@ -562,7 +563,6 @@ static int moxart_probe(struct platform_device *pdev)
struct dma_slave_config cfg;
struct clk *clk;
void __iomem *reg_mmc;
- dma_cap_mask_t mask;
int irq, ret;
u32 i;
@@ -586,9 +586,9 @@ static int moxart_probe(struct platform_device *pdev)
goto out;
}
- clk = of_clk_get(node, 0);
+ clk = devm_clk_get(dev, 0);
if (IS_ERR(clk)) {
- dev_err(dev, "of_clk_get failed\n");
+ dev_err(dev, "devm_clk_get failed\n");
ret = PTR_ERR(clk);
goto out;
}
@@ -603,9 +603,6 @@ static int moxart_probe(struct platform_device *pdev)
if (ret)
goto out;
- dma_cap_zero(mask);
- dma_cap_set(DMA_SLAVE, mask);
-
host = mmc_priv(mmc);
host->mmc = mmc;
host->base = reg_mmc;
@@ -613,8 +610,8 @@ static int moxart_probe(struct platform_device *pdev)
host->timeout = msecs_to_jiffies(1000);
host->sysclk = clk_get_rate(clk);
host->fifo_width = readl(host->base + REG_FEATURE) << 2;
- host->dma_chan_tx = of_dma_request_slave_channel(node, "tx");
- host->dma_chan_rx = of_dma_request_slave_channel(node, "rx");
+ host->dma_chan_tx = dma_request_slave_channel_reason(dev, "tx");
+ host->dma_chan_rx = dma_request_slave_channel_reason(dev, "rx");
spin_lock_init(&host->lock);
@@ -624,6 +621,11 @@ static int moxart_probe(struct platform_device *pdev)
mmc->ocr_avail = 0xffff00; /* Support 2.0v - 3.6v power. */
if (IS_ERR(host->dma_chan_tx) || IS_ERR(host->dma_chan_rx)) {
+ if (PTR_ERR(host->dma_chan_tx) == -EPROBE_DEFER ||
+ PTR_ERR(host->dma_chan_rx) == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto out;
+ }
dev_dbg(dev, "PIO mode transfer enabled\n");
host->have_dma = false;
} else {
@@ -702,9 +704,6 @@ static int moxart_remove(struct platform_device *pdev)
writel(readl(host->base + REG_CLOCK_CONTROL) | CLK_OFF,
host->base + REG_CLOCK_CONTROL);
}
-
- kfree(host);
-
return 0;
}
--
1.8.2.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v4] mmc: moxart: fix probe logic
2015-02-03 15:55 ` Jonas Jensen
@ 2015-02-04 8:40 ` Ulf Hansson
-1 siblings, 0 replies; 24+ messages in thread
From: Ulf Hansson @ 2015-02-04 8:40 UTC (permalink / raw)
To: Jonas Jensen, Arnd Bergmann
Cc: linux-mmc, linux-arm-kernel, Russell King - ARM Linux
On 3 February 2015 at 16:55, Jonas Jensen <jonas.jensen@gmail.com> wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Jonas Jensen wanted to submit a patch for these, but apparently
> forgot about it. I stumbled over this symptom first:
>
> drivers/built-in.o: In function `moxart_probe':
> :(.text+0x2af128): undefined reference to `of_dma_request_slave_channel'
>
> This is because of_dma_request_slave_channel is an internal helper
> and not exported to loadable module. I'm changing the driver to
> use dma_request_slave_channel_reason() instead.
>
> Further problems from inspection:
>
> * The remove function must not call kfree on the host pointer,
> because it is allocated together with the mmc_host.
>
> * The clock is never released
>
> * The dma_cap_mask_t is completely unused and can be removed
>
> * deferred probing does not work if the dma driver is loaded
> after the mmc driver.
>
> This patch should fix all of the above.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
Thanks! Applied for next, with minor fixes - see below.
Kind regards
Uffe
> ---
>
> Notes:
> This is a forward of Arnd's v2 patch.
>
> v2: do not call clk_put() on an error pointer
> v3: fix NULL dereference on host->clk
> v4: rework to use devm_clk_get()
>
> Applies to next-20150113
>
> drivers/mmc/host/moxart-mmc.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mmc/host/moxart-mmc.c b/drivers/mmc/host/moxart-mmc.c
> index d2a1ef6..6cb4053 100644
> --- a/drivers/mmc/host/moxart-mmc.c
> +++ b/drivers/mmc/host/moxart-mmc.c
> @@ -17,6 +17,7 @@
> #include <linux/init.h>
> #include <linux/platform_device.h>
> #include <linux/delay.h>
> +#include <linux/errno.h>
> #include <linux/interrupt.h>
> #include <linux/blkdev.h>
> #include <linux/dma-mapping.h>
> @@ -562,7 +563,6 @@ static int moxart_probe(struct platform_device *pdev)
> struct dma_slave_config cfg;
> struct clk *clk;
> void __iomem *reg_mmc;
> - dma_cap_mask_t mask;
> int irq, ret;
> u32 i;
>
> @@ -586,9 +586,9 @@ static int moxart_probe(struct platform_device *pdev)
> goto out;
> }
>
> - clk = of_clk_get(node, 0);
> + clk = devm_clk_get(dev, 0);
Changed to:
clk = devm_clk_get(dev, NULL);
> if (IS_ERR(clk)) {
> - dev_err(dev, "of_clk_get failed\n");
> + dev_err(dev, "devm_clk_get failed\n");
No need to print this, handled by clock framework.
> ret = PTR_ERR(clk);
> goto out;
> }
> @@ -603,9 +603,6 @@ static int moxart_probe(struct platform_device *pdev)
> if (ret)
> goto out;
>
> - dma_cap_zero(mask);
> - dma_cap_set(DMA_SLAVE, mask);
> -
> host = mmc_priv(mmc);
> host->mmc = mmc;
> host->base = reg_mmc;
> @@ -613,8 +610,8 @@ static int moxart_probe(struct platform_device *pdev)
> host->timeout = msecs_to_jiffies(1000);
> host->sysclk = clk_get_rate(clk);
> host->fifo_width = readl(host->base + REG_FEATURE) << 2;
> - host->dma_chan_tx = of_dma_request_slave_channel(node, "tx");
> - host->dma_chan_rx = of_dma_request_slave_channel(node, "rx");
> + host->dma_chan_tx = dma_request_slave_channel_reason(dev, "tx");
> + host->dma_chan_rx = dma_request_slave_channel_reason(dev, "rx");
>
> spin_lock_init(&host->lock);
>
> @@ -624,6 +621,11 @@ static int moxart_probe(struct platform_device *pdev)
> mmc->ocr_avail = 0xffff00; /* Support 2.0v - 3.6v power. */
>
> if (IS_ERR(host->dma_chan_tx) || IS_ERR(host->dma_chan_rx)) {
> + if (PTR_ERR(host->dma_chan_tx) == -EPROBE_DEFER ||
> + PTR_ERR(host->dma_chan_rx) == -EPROBE_DEFER) {
> + ret = -EPROBE_DEFER;
> + goto out;
> + }
> dev_dbg(dev, "PIO mode transfer enabled\n");
> host->have_dma = false;
> } else {
> @@ -702,9 +704,6 @@ static int moxart_remove(struct platform_device *pdev)
> writel(readl(host->base + REG_CLOCK_CONTROL) | CLK_OFF,
> host->base + REG_CLOCK_CONTROL);
> }
> -
> - kfree(host);
> -
> return 0;
> }
>
> --
> 1.8.2.1
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v4] mmc: moxart: fix probe logic
@ 2015-02-04 8:40 ` Ulf Hansson
0 siblings, 0 replies; 24+ messages in thread
From: Ulf Hansson @ 2015-02-04 8:40 UTC (permalink / raw)
To: linux-arm-kernel
On 3 February 2015 at 16:55, Jonas Jensen <jonas.jensen@gmail.com> wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Jonas Jensen wanted to submit a patch for these, but apparently
> forgot about it. I stumbled over this symptom first:
>
> drivers/built-in.o: In function `moxart_probe':
> :(.text+0x2af128): undefined reference to `of_dma_request_slave_channel'
>
> This is because of_dma_request_slave_channel is an internal helper
> and not exported to loadable module. I'm changing the driver to
> use dma_request_slave_channel_reason() instead.
>
> Further problems from inspection:
>
> * The remove function must not call kfree on the host pointer,
> because it is allocated together with the mmc_host.
>
> * The clock is never released
>
> * The dma_cap_mask_t is completely unused and can be removed
>
> * deferred probing does not work if the dma driver is loaded
> after the mmc driver.
>
> This patch should fix all of the above.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
Thanks! Applied for next, with minor fixes - see below.
Kind regards
Uffe
> ---
>
> Notes:
> This is a forward of Arnd's v2 patch.
>
> v2: do not call clk_put() on an error pointer
> v3: fix NULL dereference on host->clk
> v4: rework to use devm_clk_get()
>
> Applies to next-20150113
>
> drivers/mmc/host/moxart-mmc.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mmc/host/moxart-mmc.c b/drivers/mmc/host/moxart-mmc.c
> index d2a1ef6..6cb4053 100644
> --- a/drivers/mmc/host/moxart-mmc.c
> +++ b/drivers/mmc/host/moxart-mmc.c
> @@ -17,6 +17,7 @@
> #include <linux/init.h>
> #include <linux/platform_device.h>
> #include <linux/delay.h>
> +#include <linux/errno.h>
> #include <linux/interrupt.h>
> #include <linux/blkdev.h>
> #include <linux/dma-mapping.h>
> @@ -562,7 +563,6 @@ static int moxart_probe(struct platform_device *pdev)
> struct dma_slave_config cfg;
> struct clk *clk;
> void __iomem *reg_mmc;
> - dma_cap_mask_t mask;
> int irq, ret;
> u32 i;
>
> @@ -586,9 +586,9 @@ static int moxart_probe(struct platform_device *pdev)
> goto out;
> }
>
> - clk = of_clk_get(node, 0);
> + clk = devm_clk_get(dev, 0);
Changed to:
clk = devm_clk_get(dev, NULL);
> if (IS_ERR(clk)) {
> - dev_err(dev, "of_clk_get failed\n");
> + dev_err(dev, "devm_clk_get failed\n");
No need to print this, handled by clock framework.
> ret = PTR_ERR(clk);
> goto out;
> }
> @@ -603,9 +603,6 @@ static int moxart_probe(struct platform_device *pdev)
> if (ret)
> goto out;
>
> - dma_cap_zero(mask);
> - dma_cap_set(DMA_SLAVE, mask);
> -
> host = mmc_priv(mmc);
> host->mmc = mmc;
> host->base = reg_mmc;
> @@ -613,8 +610,8 @@ static int moxart_probe(struct platform_device *pdev)
> host->timeout = msecs_to_jiffies(1000);
> host->sysclk = clk_get_rate(clk);
> host->fifo_width = readl(host->base + REG_FEATURE) << 2;
> - host->dma_chan_tx = of_dma_request_slave_channel(node, "tx");
> - host->dma_chan_rx = of_dma_request_slave_channel(node, "rx");
> + host->dma_chan_tx = dma_request_slave_channel_reason(dev, "tx");
> + host->dma_chan_rx = dma_request_slave_channel_reason(dev, "rx");
>
> spin_lock_init(&host->lock);
>
> @@ -624,6 +621,11 @@ static int moxart_probe(struct platform_device *pdev)
> mmc->ocr_avail = 0xffff00; /* Support 2.0v - 3.6v power. */
>
> if (IS_ERR(host->dma_chan_tx) || IS_ERR(host->dma_chan_rx)) {
> + if (PTR_ERR(host->dma_chan_tx) == -EPROBE_DEFER ||
> + PTR_ERR(host->dma_chan_rx) == -EPROBE_DEFER) {
> + ret = -EPROBE_DEFER;
> + goto out;
> + }
> dev_dbg(dev, "PIO mode transfer enabled\n");
> host->have_dma = false;
> } else {
> @@ -702,9 +704,6 @@ static int moxart_remove(struct platform_device *pdev)
> writel(readl(host->base + REG_CLOCK_CONTROL) | CLK_OFF,
> host->base + REG_CLOCK_CONTROL);
> }
> -
> - kfree(host);
> -
> return 0;
> }
>
> --
> 1.8.2.1
>
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2015-02-04 8:40 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-29 16:15 [PATCH] mmc: moxart: fix probe logic Arnd Bergmann
2015-01-29 16:15 ` Arnd Bergmann
2015-01-29 17:01 ` Russell King - ARM Linux
2015-01-29 17:01 ` Russell King - ARM Linux
2015-01-29 22:04 ` Arnd Bergmann
2015-01-29 22:04 ` Arnd Bergmann
2015-01-29 22:06 ` [PATCH v2] " Arnd Bergmann
2015-01-29 22:06 ` Arnd Bergmann
2015-02-02 10:52 ` Jonas Jensen
2015-02-02 10:52 ` Jonas Jensen
2015-02-02 14:58 ` Arnd Bergmann
2015-02-02 14:58 ` Arnd Bergmann
2015-02-02 15:43 ` [PATCH v3] " Jonas Jensen
2015-02-02 15:43 ` Jonas Jensen
2015-02-02 20:03 ` Arnd Bergmann
2015-02-02 20:03 ` Arnd Bergmann
2015-02-03 15:55 ` [PATCH v4] " Jonas Jensen
2015-02-03 15:55 ` Jonas Jensen
2015-02-04 8:40 ` Ulf Hansson
2015-02-04 8:40 ` Ulf Hansson
2015-02-03 8:02 ` [PATCH v3] " Ulf Hansson
2015-02-03 8:02 ` Ulf Hansson
2015-02-03 9:06 ` Arnd Bergmann
2015-02-03 9:06 ` Arnd Bergmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.