linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mmc: omap: Fix error introduced by fix to release_mem_region() path
@ 2013-12-02 19:34 Tony Lindgren
  2013-12-03 18:16 ` Jarkko Nikula
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Tony Lindgren @ 2013-12-02 19:34 UTC (permalink / raw)
  To: Chris Ball
  Cc: linux-mmc, linux-omap, Dan Carpenter, Jarkko Nikula,
	Aaro Koskinen, Jarkko Lavinen

Commit 31ee9181eb92: (mmc: omap: Fix DMA configuration to not rely
on device id) fixed getting of the DMA resources when booted with
device tree. This patch however changed the handling of the
free_mem_region() error path by reusing the struct resource *res
for the DMA resources.

Fix the error the same way omap_hsmmc.c driver handles it, which
is to restore the resource before using the values to call
release_mem_region().

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>

---

Chris, looks like the patch introducing this error is queued in
arm-soc fixes branch.. Care to ack this one too and I'll merge it
via arm-soc as well?

After that, no need for me to patch this driver for my mach-omap2
device tree conversion I hope :)

--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -1465,7 +1465,10 @@ err_free_mmc_host:
 err_ioremap:
 	kfree(host);
 err_free_mem_region:
-	release_mem_region(res->start, resource_size(res));
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (res)
+		release_mem_region(res->start, resource_size(res));
+
 	return ret;
 }
 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] mmc: omap: Fix error introduced by fix to release_mem_region() path
  2013-12-02 19:34 [PATCH] mmc: omap: Fix error introduced by fix to release_mem_region() path Tony Lindgren
@ 2013-12-03 18:16 ` Jarkko Nikula
  2013-12-04 18:00   ` Tony Lindgren
  2013-12-04 18:01 ` Tony Lindgren
  2013-12-04 21:14 ` [PATCH 0/7] mmc: omap: Fixes, cleanup and add ERASE capability Jarkko Nikula
  2 siblings, 1 reply; 15+ messages in thread
From: Jarkko Nikula @ 2013-12-03 18:16 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Chris Ball, linux-mmc, linux-omap, Dan Carpenter, Aaro Koskinen,
	Jarkko Lavinen

On 12/02/2013 07:34 PM, Tony Lindgren wrote:
> Commit 31ee9181eb92: (mmc: omap: Fix DMA configuration to not rely
> on device id) fixed getting of the DMA resources when booted with
> device tree. This patch however changed the handling of the
> free_mem_region() error path by reusing the struct resource *res
> for the DMA resources.
> 
> Fix the error the same way omap_hsmmc.c driver handles it, which
> is to restore the resource before using the values to call
> release_mem_region().
> 
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> 
I guess my devm_ioremap conversion would have fixed this?

http://www.spinics.net/lists/linux-mmc/msg23317.html

Does not help now but I'm thinking if queuing some of those my patches
(3-5) on top of yours as a fix? Especially fix to NULL pointer dereference:

http://www.spinics.net/lists/linux-mmc/msg23320.html

-- 
Jarkko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] mmc: omap: Fix error introduced by fix to release_mem_region() path
  2013-12-03 18:16 ` Jarkko Nikula
@ 2013-12-04 18:00   ` Tony Lindgren
  0 siblings, 0 replies; 15+ messages in thread
From: Tony Lindgren @ 2013-12-04 18:00 UTC (permalink / raw)
  To: Jarkko Nikula
  Cc: Chris Ball, linux-mmc, linux-omap, Dan Carpenter, Aaro Koskinen,
	Jarkko Lavinen

* Jarkko Nikula <jarkko.nikula@bitmer.com> [131203 08:18]:
> On 12/02/2013 07:34 PM, Tony Lindgren wrote:
> > Commit 31ee9181eb92: (mmc: omap: Fix DMA configuration to not rely
> > on device id) fixed getting of the DMA resources when booted with
> > device tree. This patch however changed the handling of the
> > free_mem_region() error path by reusing the struct resource *res
> > for the DMA resources.
> > 
> > Fix the error the same way omap_hsmmc.c driver handles it, which
> > is to restore the resource before using the values to call
> > release_mem_region().
> > 
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Signed-off-by: Tony Lindgren <tony@atomide.com>
> > 
> I guess my devm_ioremap conversion would have fixed this?
> 
> http://www.spinics.net/lists/linux-mmc/msg23317.html

Probably..
 
> Does not help now but I'm thinking if queuing some of those my patches
> (3-5) on top of yours as a fix? Especially fix to NULL pointer dereference:
> 
> http://www.spinics.net/lists/linux-mmc/msg23320.html

Yeah please do, how about also adding the error log to the
description?

Regards,

Tony

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] mmc: omap: Fix error introduced by fix to release_mem_region() path
  2013-12-02 19:34 [PATCH] mmc: omap: Fix error introduced by fix to release_mem_region() path Tony Lindgren
  2013-12-03 18:16 ` Jarkko Nikula
@ 2013-12-04 18:01 ` Tony Lindgren
  2013-12-04 21:14 ` [PATCH 0/7] mmc: omap: Fixes, cleanup and add ERASE capability Jarkko Nikula
  2 siblings, 0 replies; 15+ messages in thread
From: Tony Lindgren @ 2013-12-04 18:01 UTC (permalink / raw)
  To: Chris Ball
  Cc: linux-mmc, linux-omap, Dan Carpenter, Jarkko Nikula,
	Aaro Koskinen, Jarkko Lavinen

* Tony Lindgren <tony@atomide.com> [131202 11:36]:
> Commit 31ee9181eb92: (mmc: omap: Fix DMA configuration to not rely
> on device id) fixed getting of the DMA resources when booted with
> device tree. This patch however changed the handling of the
> free_mem_region() error path by reusing the struct resource *res
> for the DMA resources.
> 
> Fix the error the same way omap_hsmmc.c driver handles it, which
> is to restore the resource before using the values to call
> release_mem_region().
> 
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> 
> ---
> 
> Chris, looks like the patch introducing this error is queued in
> arm-soc fixes branch.. Care to ack this one too and I'll merge it
> via arm-soc as well?
> 
> After that, no need for me to patch this driver for my mach-omap2
> device tree conversion I hope :)

OK the previous fixes just hit the mainline, no need for me to
queue this any longer. Please feel free to pick this one up for
the -rc cycle.

Regards,

Tony
 
> --- a/drivers/mmc/host/omap.c
> +++ b/drivers/mmc/host/omap.c
> @@ -1465,7 +1465,10 @@ err_free_mmc_host:
>  err_ioremap:
>  	kfree(host);
>  err_free_mem_region:
> -	release_mem_region(res->start, resource_size(res));
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (res)
> +		release_mem_region(res->start, resource_size(res));
> +
>  	return ret;
>  }
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 0/7] mmc: omap: Fixes, cleanup and add ERASE capability
  2013-12-02 19:34 [PATCH] mmc: omap: Fix error introduced by fix to release_mem_region() path Tony Lindgren
  2013-12-03 18:16 ` Jarkko Nikula
  2013-12-04 18:01 ` Tony Lindgren
@ 2013-12-04 21:14 ` Jarkko Nikula
  2013-12-04 21:14   ` [PATCH 1/7] mmc: omap: Fix NULL pointer dereference due uninitialized cover_tasklet Jarkko Nikula
                     ` (8 more replies)
  2 siblings, 9 replies; 15+ messages in thread
From: Jarkko Nikula @ 2013-12-04 21:14 UTC (permalink / raw)
  To: Chris Ball
  Cc: linux-mmc, linux-omap, Tony Lindgren, Dan Carpenter,
	Aaro Koskinen, Jarkko Lavinen, Jarkko Nikula

Hi

I've sent these patches earlier to linux-mmc list. This time set is one
patch smaller since commit 31ee9181eb92
("mmc: omap: Fix DMA configuration to not rely on device id") does
practically same what one of my patch did.

Patch 1 is an obvious fix. 2-4 are preparing for 5 which also should fix
the error introduced by 31ee9181eb92:
http://www.spinics.net/lists/linux-omap/msg100790.html

Patch 6 continue cleanup and 7 adds the ERASE capability to the driver so
one could utilize discards with mkfs, mount option or with the fstrim.

Set goes on top of v3.13-rc2-208-g8ecffd791448.

Jarkko Nikula (7):
  mmc: omap: Fix NULL pointer dereference due uninitialized
    cover_tasklet
  mmc: omap: Convert to devm_kzalloc
  mmc: omap: Remove duplicate host->irq assignment
  mmc: omap: Remove mem_res field from struct mmc_omap_host
  mmc: omap: Convert to devm_ioremap_resource
  mmc: omap: Remove always set use_dma flag from struct mmc_omap_host
  mmc: omap: Add erase capability

 drivers/mmc/host/omap.c | 93 +++++++++++++++++++------------------------------
 1 file changed, 36 insertions(+), 57 deletions(-)

-- 
1.8.4.3


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/7] mmc: omap: Fix NULL pointer dereference due uninitialized cover_tasklet
  2013-12-04 21:14 ` [PATCH 0/7] mmc: omap: Fixes, cleanup and add ERASE capability Jarkko Nikula
@ 2013-12-04 21:14   ` Jarkko Nikula
  2013-12-04 21:14   ` [PATCH 2/7] mmc: omap: Convert to devm_kzalloc Jarkko Nikula
                     ` (7 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Jarkko Nikula @ 2013-12-04 21:14 UTC (permalink / raw)
  To: Chris Ball
  Cc: linux-mmc, linux-omap, Tony Lindgren, Dan Carpenter,
	Aaro Koskinen, Jarkko Lavinen, Jarkko Nikula

Omap MMC driver initialization can cause a NULL pointer dereference in
tasklet_hi_action on Nokia N810 if its miniSD cover is open during driver
initialization.

[    1.070000] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[    1.080000] pgd = c0004000
[    1.080000] [00000000] *pgd=00000000
[    1.080000] Internal error: Oops: 80000005 [#1] PREEMPT ARM
[    1.080000] Modules linked in:
[    1.080000] CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted 3.13.0-rc2+ #95
[    1.080000] Workqueue: events menelaus_work
[    1.080000] task: c7863340 ti: c7878000 task.ti: c7878000
[    1.080000] PC is at 0x0
[    1.080000] LR is at tasklet_hi_action+0x68/0xa4
...
[    1.080000] [<c003543c>] (tasklet_hi_action+0x68/0xa4) from [<c0034dd0>] (__do_softirq+0xbc/0x208)
[    1.080000] [<c0034dd0>] (__do_softirq+0xbc/0x208) from [<c003521c>] (irq_exit+0x84/0xac)
[    1.080000] [<c003521c>] (irq_exit+0x84/0xac) from [<c00135cc>] (handle_IRQ+0x64/0x84)
[    1.080000] [<c00135cc>] (handle_IRQ+0x64/0x84) from [<c000859c>] (omap2_intc_handle_irq+0x54/0x68)
[    1.080000] [<c000859c>] (omap2_intc_handle_irq+0x54/0x68) from [<c0015be0>] (__irq_svc+0x40/0x74)
[    1.080000] Exception stack(0xc7879d70 to 0xc7879db8)
[    1.080000] 9d60:                                     000003f1 0000000a 00000009 0000001c
[    1.080000] 9d80: c7879e70 c780bc10 c780bc10 00000000 00000001 00008603 c780bc78 c7879e4e
[    1.080000] 9da0: 00000002 c7879db8 c00343b0 c0160c9c 20000113 ffffffff
[    1.080000] [<c0015be0>] (__irq_svc+0x40/0x74) from [<c0160c9c>] (__aeabi_uidiv+0x20/0x9c)
[    1.080000] [<c0160c9c>] (__aeabi_uidiv+0x20/0x9c) from [<c00343b0>] (msecs_to_jiffies+0x18/0x24)
[    1.080000] [<c00343b0>] (msecs_to_jiffies+0x18/0x24) from [<c01ec3ec>] (omap_i2c_xfer+0x30c/0x458)
[    1.080000] [<c01ec3ec>] (omap_i2c_xfer+0x30c/0x458) from [<c01e9724>] (__i2c_transfer+0x3c/0x74)
[    1.080000] [<c01e9724>] (__i2c_transfer+0x3c/0x74) from [<c01eac4c>] (i2c_transfer+0x78/0x94)
[    1.080000] [<c01eac4c>] (i2c_transfer+0x78/0x94) from [<c01eb0bc>] (i2c_smbus_xfer+0x3c0/0x4f8)
[    1.080000] [<c01eb0bc>] (i2c_smbus_xfer+0x3c0/0x4f8) from [<c01eb414>] (i2c_smbus_write_byte_data+0x34/0x3c)
[    1.080000] [<c01eb414>] (i2c_smbus_write_byte_data+0x34/0x3c) from [<c01bb308>] (menelaus_write_reg+0x1c/0x40)
[    1.080000] [<c01bb308>] (menelaus_write_reg+0x1c/0x40) from [<c01bb904>] (menelaus_work+0xa0/0xc4)
[    1.080000] [<c01bb904>] (menelaus_work+0xa0/0xc4) from [<c00439c4>] (process_one_work+0x1fc/0x334)
[    1.080000] [<c00439c4>] (process_one_work+0x1fc/0x334) from [<c0043d6c>] (worker_thread+0x244/0x380)
[    1.080000] [<c0043d6c>] (worker_thread+0x244/0x380) from [<c0049d04>] (kthread+0xc0/0xd4)
[    1.080000] [<c0049d04>] (kthread+0xc0/0xd4) from [<c0012758>] (ret_from_fork+0x14/0x3c)
[    1.080000] Code: bad PC value
[    1.090000] ---[ end trace 7bc2fc7cd14f1d95 ]---
[    1.100000] Kernel panic - not syncing: Fatal exception in interrupt

Reason for this is that omap_notify_cover_event which calls
tasklet_hi_schedule gets called before struct cover_tasklet is initialized.

Call to omap_notify_cover_event on Nokia N810 happens from menelaus.c PMIC
driver via board-n8x0.c during execution of mmc_add_host in case of open
miniSD cover.

Fix this by moving cover_timer and cover_tasklet initialization before
mmc_add_host call in mmc_omap_new_slot.

Signed-off-by: Jarkko Nikula <jarkko.nikula@bitmer.com>
---
v2: a snip of stack dump added to commit log
---
 drivers/mmc/host/omap.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c
index 98b6b6e..42b665d 100644
--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -1262,6 +1262,13 @@ static int mmc_omap_new_slot(struct mmc_omap_host *host, int id)
 	mmc->max_req_size = mmc->max_blk_size * mmc->max_blk_count;
 	mmc->max_seg_size = mmc->max_req_size;
 
+	if (slot->pdata->get_cover_state != NULL) {
+		setup_timer(&slot->cover_timer, mmc_omap_cover_timer,
+			    (unsigned long)slot);
+		tasklet_init(&slot->cover_tasklet, mmc_omap_cover_handler,
+			     (unsigned long)slot);
+	}
+
 	r = mmc_add_host(mmc);
 	if (r < 0)
 		goto err_remove_host;
@@ -1278,11 +1285,6 @@ static int mmc_omap_new_slot(struct mmc_omap_host *host, int id)
 					&dev_attr_cover_switch);
 		if (r < 0)
 			goto err_remove_slot_name;
-
-		setup_timer(&slot->cover_timer, mmc_omap_cover_timer,
-			    (unsigned long)slot);
-		tasklet_init(&slot->cover_tasklet, mmc_omap_cover_handler,
-			     (unsigned long)slot);
 		tasklet_schedule(&slot->cover_tasklet);
 	}
 
-- 
1.8.4.3


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/7] mmc: omap: Convert to devm_kzalloc
  2013-12-04 21:14 ` [PATCH 0/7] mmc: omap: Fixes, cleanup and add ERASE capability Jarkko Nikula
  2013-12-04 21:14   ` [PATCH 1/7] mmc: omap: Fix NULL pointer dereference due uninitialized cover_tasklet Jarkko Nikula
@ 2013-12-04 21:14   ` Jarkko Nikula
  2013-12-04 21:14   ` [PATCH 3/7] mmc: omap: Remove duplicate host->irq assignment Jarkko Nikula
                     ` (6 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Jarkko Nikula @ 2013-12-04 21:14 UTC (permalink / raw)
  To: Chris Ball
  Cc: linux-mmc, linux-omap, Tony Lindgren, Dan Carpenter,
	Aaro Koskinen, Jarkko Lavinen, Jarkko Nikula

Signed-off-by: Jarkko Nikula <jarkko.nikula@bitmer.com>
---
 drivers/mmc/host/omap.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c
index 42b665d..927ed24 100644
--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -1345,7 +1345,8 @@ static int mmc_omap_probe(struct platform_device *pdev)
 	if (res == NULL)
 		return -EBUSY;
 
-	host = kzalloc(sizeof(struct mmc_omap_host), GFP_KERNEL);
+	host = devm_kzalloc(&pdev->dev, sizeof(struct mmc_omap_host),
+			    GFP_KERNEL);
 	if (host == NULL) {
 		ret = -ENOMEM;
 		goto err_free_mem_region;
@@ -1465,7 +1466,6 @@ err_free_iclk:
 err_free_mmc_host:
 	iounmap(host->virt_base);
 err_ioremap:
-	kfree(host);
 err_free_mem_region:
 	release_mem_region(res->start, resource_size(res));
 	return ret;
@@ -1500,8 +1500,6 @@ static int mmc_omap_remove(struct platform_device *pdev)
 			   pdev->resource[0].end - pdev->resource[0].start + 1);
 	destroy_workqueue(host->mmc_omap_wq);
 
-	kfree(host);
-
 	return 0;
 }
 
-- 
1.8.4.3


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/7] mmc: omap: Remove duplicate host->irq assignment
  2013-12-04 21:14 ` [PATCH 0/7] mmc: omap: Fixes, cleanup and add ERASE capability Jarkko Nikula
  2013-12-04 21:14   ` [PATCH 1/7] mmc: omap: Fix NULL pointer dereference due uninitialized cover_tasklet Jarkko Nikula
  2013-12-04 21:14   ` [PATCH 2/7] mmc: omap: Convert to devm_kzalloc Jarkko Nikula
@ 2013-12-04 21:14   ` Jarkko Nikula
  2013-12-04 21:14   ` [PATCH 4/7] mmc: omap: Remove mem_res field from struct mmc_omap_host Jarkko Nikula
                     ` (5 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Jarkko Nikula @ 2013-12-04 21:14 UTC (permalink / raw)
  To: Chris Ball
  Cc: linux-mmc, linux-omap, Tony Lindgren, Dan Carpenter,
	Aaro Koskinen, Jarkko Lavinen, Jarkko Nikula

host-irq is set twice so remove needless one.

Signed-off-by: Jarkko Nikula <jarkko.nikula@bitmer.com>
---
 drivers/mmc/host/omap.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c
index 927ed24..b438f0a 100644
--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -1375,7 +1375,6 @@ static int mmc_omap_probe(struct platform_device *pdev)
 	host->mem_res = res;
 	host->irq = irq;
 	host->use_dma = 1;
-	host->irq = irq;
 	host->phys_base = host->mem_res->start;
 	host->virt_base = ioremap(res->start, resource_size(res));
 	if (!host->virt_base)
-- 
1.8.4.3


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/7] mmc: omap: Remove mem_res field from struct mmc_omap_host
  2013-12-04 21:14 ` [PATCH 0/7] mmc: omap: Fixes, cleanup and add ERASE capability Jarkko Nikula
                     ` (2 preceding siblings ...)
  2013-12-04 21:14   ` [PATCH 3/7] mmc: omap: Remove duplicate host->irq assignment Jarkko Nikula
@ 2013-12-04 21:14   ` Jarkko Nikula
  2013-12-04 21:14   ` [PATCH 5/7] mmc: omap: Convert to devm_ioremap_resource Jarkko Nikula
                     ` (4 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Jarkko Nikula @ 2013-12-04 21:14 UTC (permalink / raw)
  To: Chris Ball
  Cc: linux-mmc, linux-omap, Tony Lindgren, Dan Carpenter,
	Aaro Koskinen, Jarkko Lavinen, Jarkko Nikula

Field mem_res in struct mmc_omap_host is used only once in mmc_omap_probe
when setting the phys_base field so we may just se the phys_base straight
and remove needless mem_res.

Signed-off-by: Jarkko Nikula <jarkko.nikula@bitmer.com>
---
 drivers/mmc/host/omap.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c
index b438f0a..0d669cf 100644
--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -130,7 +130,6 @@ struct mmc_omap_host {
 	u32			dma_rx_burst;
 	struct dma_chan		*dma_tx;
 	u32			dma_tx_burst;
-	struct resource		*mem_res;
 	void __iomem		*virt_base;
 	unsigned int		phys_base;
 	int			irq;
@@ -1372,10 +1371,9 @@ static int mmc_omap_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, host);
 
 	host->id = pdev->id;
-	host->mem_res = res;
 	host->irq = irq;
 	host->use_dma = 1;
-	host->phys_base = host->mem_res->start;
+	host->phys_base = res->start;
 	host->virt_base = ioremap(res->start, resource_size(res));
 	if (!host->virt_base)
 		goto err_ioremap;
-- 
1.8.4.3


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/7] mmc: omap: Convert to devm_ioremap_resource
  2013-12-04 21:14 ` [PATCH 0/7] mmc: omap: Fixes, cleanup and add ERASE capability Jarkko Nikula
                     ` (3 preceding siblings ...)
  2013-12-04 21:14   ` [PATCH 4/7] mmc: omap: Remove mem_res field from struct mmc_omap_host Jarkko Nikula
@ 2013-12-04 21:14   ` Jarkko Nikula
  2013-12-04 21:14   ` [PATCH 6/7] mmc: omap: Remove always set use_dma flag from struct mmc_omap_host Jarkko Nikula
                     ` (3 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Jarkko Nikula @ 2013-12-04 21:14 UTC (permalink / raw)
  To: Chris Ball
  Cc: linux-mmc, linux-omap, Tony Lindgren, Dan Carpenter,
	Aaro Koskinen, Jarkko Lavinen, Jarkko Nikula

Simplify probe and cleanup code by using devm_ioremap_resource. This also
makes probe code to follow more common allocate private struct followed by
other initialization style.

Signed-off-by: Jarkko Nikula <jarkko.nikula@bitmer.com>
---
 drivers/mmc/host/omap.c | 41 ++++++++++++-----------------------------
 1 file changed, 12 insertions(+), 29 deletions(-)

diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c
index 0d669cf..e4c7a03 100644
--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -1334,22 +1334,19 @@ static int mmc_omap_probe(struct platform_device *pdev)
 		return -EPROBE_DEFER;
 	}
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	host = devm_kzalloc(&pdev->dev, sizeof(struct mmc_omap_host),
+			    GFP_KERNEL);
+	if (host == NULL)
+		return -ENOMEM;
+
 	irq = platform_get_irq(pdev, 0);
-	if (res == NULL || irq < 0)
+	if (irq < 0)
 		return -ENXIO;
 
-	res = request_mem_region(res->start, resource_size(res),
-				 pdev->name);
-	if (res == NULL)
-		return -EBUSY;
-
-	host = devm_kzalloc(&pdev->dev, sizeof(struct mmc_omap_host),
-			    GFP_KERNEL);
-	if (host == NULL) {
-		ret = -ENOMEM;
-		goto err_free_mem_region;
-	}
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	host->virt_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(host->virt_base))
+		return PTR_ERR(host->virt_base);
 
 	INIT_WORK(&host->slot_release_work, mmc_omap_slot_release_work);
 	INIT_WORK(&host->send_stop_work, mmc_omap_send_stop_work);
@@ -1374,15 +1371,9 @@ static int mmc_omap_probe(struct platform_device *pdev)
 	host->irq = irq;
 	host->use_dma = 1;
 	host->phys_base = res->start;
-	host->virt_base = ioremap(res->start, resource_size(res));
-	if (!host->virt_base)
-		goto err_ioremap;
-
 	host->iclk = clk_get(&pdev->dev, "ick");
-	if (IS_ERR(host->iclk)) {
-		ret = PTR_ERR(host->iclk);
-		goto err_free_mmc_host;
-	}
+	if (IS_ERR(host->iclk))
+		return PTR_ERR(host->iclk);
 	clk_enable(host->iclk);
 
 	host->fclk = clk_get(&pdev->dev, "fck");
@@ -1460,11 +1451,6 @@ err_free_dma:
 err_free_iclk:
 	clk_disable(host->iclk);
 	clk_put(host->iclk);
-err_free_mmc_host:
-	iounmap(host->virt_base);
-err_ioremap:
-err_free_mem_region:
-	release_mem_region(res->start, resource_size(res));
 	return ret;
 }
 
@@ -1492,9 +1478,6 @@ static int mmc_omap_remove(struct platform_device *pdev)
 	if (host->dma_rx)
 		dma_release_channel(host->dma_rx);
 
-	iounmap(host->virt_base);
-	release_mem_region(pdev->resource[0].start,
-			   pdev->resource[0].end - pdev->resource[0].start + 1);
 	destroy_workqueue(host->mmc_omap_wq);
 
 	return 0;
-- 
1.8.4.3


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 6/7] mmc: omap: Remove always set use_dma flag from struct mmc_omap_host
  2013-12-04 21:14 ` [PATCH 0/7] mmc: omap: Fixes, cleanup and add ERASE capability Jarkko Nikula
                     ` (4 preceding siblings ...)
  2013-12-04 21:14   ` [PATCH 5/7] mmc: omap: Convert to devm_ioremap_resource Jarkko Nikula
@ 2013-12-04 21:14   ` Jarkko Nikula
  2013-12-04 21:14   ` [PATCH 7/7] mmc: omap: Add erase capability Jarkko Nikula
                     ` (2 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Jarkko Nikula @ 2013-12-04 21:14 UTC (permalink / raw)
  To: Chris Ball
  Cc: linux-mmc, linux-omap, Tony Lindgren, Dan Carpenter,
	Aaro Koskinen, Jarkko Lavinen, Jarkko Nikula

Because use_dma is set only in mmc_omap_probe and unset nowhere there is no
need to carry that flag in struct mmc_omap_host for mmc_omap_prepare_data
function.

Signed-off-by: Jarkko Nikula <jarkko.nikula@bitmer.com>
---
 drivers/mmc/host/omap.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c
index e4c7a03..42175cd 100644
--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -152,7 +152,6 @@ struct mmc_omap_host {
 	u32			total_bytes_left;
 
 	unsigned		features;
-	unsigned		use_dma:1;
 	unsigned		brs_received:1, dma_done:1;
 	unsigned		dma_in_use:1;
 	spinlock_t		dma_lock;
@@ -944,7 +943,7 @@ static void
 mmc_omap_prepare_data(struct mmc_omap_host *host, struct mmc_request *req)
 {
 	struct mmc_data *data = req->data;
-	int i, use_dma, block_size;
+	int i, use_dma = 1, block_size;
 	unsigned sg_len;
 
 	host->data = data;
@@ -969,13 +968,10 @@ mmc_omap_prepare_data(struct mmc_omap_host *host, struct mmc_request *req)
 	sg_len = (data->blocks == 1) ? 1 : data->sg_len;
 
 	/* Only do DMA for entire blocks */
-	use_dma = host->use_dma;
-	if (use_dma) {
-		for (i = 0; i < sg_len; i++) {
-			if ((data->sg[i].length % block_size) != 0) {
-				use_dma = 0;
-				break;
-			}
+	for (i = 0; i < sg_len; i++) {
+		if ((data->sg[i].length % block_size) != 0) {
+			use_dma = 0;
+			break;
 		}
 	}
 
@@ -1369,7 +1365,6 @@ static int mmc_omap_probe(struct platform_device *pdev)
 
 	host->id = pdev->id;
 	host->irq = irq;
-	host->use_dma = 1;
 	host->phys_base = res->start;
 	host->iclk = clk_get(&pdev->dev, "ick");
 	if (IS_ERR(host->iclk))
-- 
1.8.4.3


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 7/7] mmc: omap: Add erase capability
  2013-12-04 21:14 ` [PATCH 0/7] mmc: omap: Fixes, cleanup and add ERASE capability Jarkko Nikula
                     ` (5 preceding siblings ...)
  2013-12-04 21:14   ` [PATCH 6/7] mmc: omap: Remove always set use_dma flag from struct mmc_omap_host Jarkko Nikula
@ 2013-12-04 21:14   ` Jarkko Nikula
  2013-12-05 18:18   ` [PATCH 0/7] mmc: omap: Fixes, cleanup and add ERASE capability Tony Lindgren
  2013-12-06 13:41   ` Aaro Koskinen
  8 siblings, 0 replies; 15+ messages in thread
From: Jarkko Nikula @ 2013-12-04 21:14 UTC (permalink / raw)
  To: Chris Ball
  Cc: linux-mmc, linux-omap, Tony Lindgren, Dan Carpenter,
	Aaro Koskinen, Jarkko Lavinen, Jarkko Nikula

This patch adds the erase capability to OMAP1/OMAP2420 MMC driver. Idea is
the same than in commit 93caf8e ("omap_hsmmc: add erase capability") that we
disable the data timeout interrupt for erases.

Signed-off-by: Jarkko Nikula <jarkko.nikula@bitmer.com>
---
 drivers/mmc/host/omap.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c
index 42175cd..5c2e58b 100644
--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -26,6 +26,7 @@
 #include <linux/omap-dma.h>
 #include <linux/mmc/host.h>
 #include <linux/mmc/card.h>
+#include <linux/mmc/mmc.h>
 #include <linux/clk.h>
 #include <linux/scatterlist.h>
 #include <linux/slab.h>
@@ -336,6 +337,7 @@ mmc_omap_start_command(struct mmc_omap_host *host, struct mmc_command *cmd)
 	u32 cmdreg;
 	u32 resptype;
 	u32 cmdtype;
+	u16 irq_mask;
 
 	host->cmd = cmd;
 
@@ -388,12 +390,14 @@ mmc_omap_start_command(struct mmc_omap_host *host, struct mmc_command *cmd)
 	OMAP_MMC_WRITE(host, CTO, 200);
 	OMAP_MMC_WRITE(host, ARGL, cmd->arg & 0xffff);
 	OMAP_MMC_WRITE(host, ARGH, cmd->arg >> 16);
-	OMAP_MMC_WRITE(host, IE,
-		       OMAP_MMC_STAT_A_EMPTY    | OMAP_MMC_STAT_A_FULL    |
-		       OMAP_MMC_STAT_CMD_CRC    | OMAP_MMC_STAT_CMD_TOUT  |
-		       OMAP_MMC_STAT_DATA_CRC   | OMAP_MMC_STAT_DATA_TOUT |
-		       OMAP_MMC_STAT_END_OF_CMD | OMAP_MMC_STAT_CARD_ERR  |
-		       OMAP_MMC_STAT_END_OF_DATA);
+	irq_mask = OMAP_MMC_STAT_A_EMPTY    | OMAP_MMC_STAT_A_FULL    |
+		   OMAP_MMC_STAT_CMD_CRC    | OMAP_MMC_STAT_CMD_TOUT  |
+		   OMAP_MMC_STAT_DATA_CRC   | OMAP_MMC_STAT_DATA_TOUT |
+		   OMAP_MMC_STAT_END_OF_CMD | OMAP_MMC_STAT_CARD_ERR  |
+		   OMAP_MMC_STAT_END_OF_DATA;
+	if (cmd->opcode == MMC_ERASE)
+		irq_mask &= ~OMAP_MMC_STAT_DATA_TOUT;
+	OMAP_MMC_WRITE(host, IE, irq_mask);
 	OMAP_MMC_WRITE(host, CMD, cmdreg);
 }
 
@@ -1234,7 +1238,7 @@ static int mmc_omap_new_slot(struct mmc_omap_host *host, int id)
 
 	mmc->caps = 0;
 	if (host->pdata->slots[id].wires >= 4)
-		mmc->caps |= MMC_CAP_4_BIT_DATA;
+		mmc->caps |= MMC_CAP_4_BIT_DATA | MMC_CAP_ERASE;
 
 	mmc->ops = &mmc_omap_ops;
 	mmc->f_min = 400000;
-- 
1.8.4.3


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/7] mmc: omap: Fixes, cleanup and add ERASE capability
  2013-12-04 21:14 ` [PATCH 0/7] mmc: omap: Fixes, cleanup and add ERASE capability Jarkko Nikula
                     ` (6 preceding siblings ...)
  2013-12-04 21:14   ` [PATCH 7/7] mmc: omap: Add erase capability Jarkko Nikula
@ 2013-12-05 18:18   ` Tony Lindgren
  2013-12-06 13:41   ` Aaro Koskinen
  8 siblings, 0 replies; 15+ messages in thread
From: Tony Lindgren @ 2013-12-05 18:18 UTC (permalink / raw)
  To: Jarkko Nikula
  Cc: Chris Ball, linux-mmc, linux-omap, Dan Carpenter, Aaro Koskinen,
	Jarkko Lavinen

* Jarkko Nikula <jarkko.nikula@bitmer.com> [131204 11:15]:
> Hi
> 
> I've sent these patches earlier to linux-mmc list. This time set is one
> patch smaller since commit 31ee9181eb92
> ("mmc: omap: Fix DMA configuration to not rely on device id") does
> practically same what one of my patch did.
> 
> Patch 1 is an obvious fix. 2-4 are preparing for 5 which also should fix
> the error introduced by 31ee9181eb92:
> http://www.spinics.net/lists/linux-omap/msg100790.html
> 
> Patch 6 continue cleanup and 7 adds the ERASE capability to the driver so
> one could utilize discards with mkfs, mount option or with the fstrim.
> 
> Set goes on top of v3.13-rc2-208-g8ecffd791448.

Great they look good to me:

Acked-by: Tony Lindgren <tony@atomide.com>
 
> Jarkko Nikula (7):
>   mmc: omap: Fix NULL pointer dereference due uninitialized
>     cover_tasklet
>   mmc: omap: Convert to devm_kzalloc
>   mmc: omap: Remove duplicate host->irq assignment
>   mmc: omap: Remove mem_res field from struct mmc_omap_host
>   mmc: omap: Convert to devm_ioremap_resource
>   mmc: omap: Remove always set use_dma flag from struct mmc_omap_host
>   mmc: omap: Add erase capability
> 
>  drivers/mmc/host/omap.c | 93 +++++++++++++++++++------------------------------
>  1 file changed, 36 insertions(+), 57 deletions(-)
> 
> -- 
> 1.8.4.3
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/7] mmc: omap: Fixes, cleanup and add ERASE capability
  2013-12-04 21:14 ` [PATCH 0/7] mmc: omap: Fixes, cleanup and add ERASE capability Jarkko Nikula
                     ` (7 preceding siblings ...)
  2013-12-05 18:18   ` [PATCH 0/7] mmc: omap: Fixes, cleanup and add ERASE capability Tony Lindgren
@ 2013-12-06 13:41   ` Aaro Koskinen
  2013-12-06 19:54     ` Jarkko Nikula
  8 siblings, 1 reply; 15+ messages in thread
From: Aaro Koskinen @ 2013-12-06 13:41 UTC (permalink / raw)
  To: Jarkko Nikula
  Cc: Chris Ball, linux-mmc, linux-omap, Tony Lindgren, Dan Carpenter,
	Jarkko Lavinen

Hi,

On Wed, Dec 04, 2013 at 09:14:07PM +0000, Jarkko Nikula wrote:
> I've sent these patches earlier to linux-mmc list. This time set is one
> patch smaller since commit 31ee9181eb92
> ("mmc: omap: Fix DMA configuration to not rely on device id") does
> practically same what one of my patch did.
> 
> Patch 1 is an obvious fix. 2-4 are preparing for 5 which also should fix
> the error introduced by 31ee9181eb92:
> http://www.spinics.net/lists/linux-omap/msg100790.html
> 
> Patch 6 continue cleanup and 7 adds the ERASE capability to the driver so
> one could utilize discards with mkfs, mount option or with the fstrim.
> 
> Set goes on top of v3.13-rc2-208-g8ecffd791448.

Patch 5 had some fuzz with plain 3.13-rc2. Otherwise these look fine.
I tested these on 770 and N800, so for all patches:

Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi>

A.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/7] mmc: omap: Fixes, cleanup and add ERASE capability
  2013-12-06 13:41   ` Aaro Koskinen
@ 2013-12-06 19:54     ` Jarkko Nikula
  0 siblings, 0 replies; 15+ messages in thread
From: Jarkko Nikula @ 2013-12-06 19:54 UTC (permalink / raw)
  To: Aaro Koskinen
  Cc: Chris Ball, linux-mmc, linux-omap, Tony Lindgren, Dan Carpenter,
	Jarkko Lavinen

On 12/06/2013 01:41 PM, Aaro Koskinen wrote:
>> Set goes on top of v3.13-rc2-208-g8ecffd791448.
> 
> Patch 5 had some fuzz with plain 3.13-rc2. Otherwise these look fine.
> I tested these on 770 and N800, so for all patches:
> 
> Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi>
> 
Yes, reason for your fuzz observation is that 3.13-rc2 don't have those
two patches from Tony but 8ecffd791448 had.

-- 
Jarkko

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2013-12-06 19:54 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-02 19:34 [PATCH] mmc: omap: Fix error introduced by fix to release_mem_region() path Tony Lindgren
2013-12-03 18:16 ` Jarkko Nikula
2013-12-04 18:00   ` Tony Lindgren
2013-12-04 18:01 ` Tony Lindgren
2013-12-04 21:14 ` [PATCH 0/7] mmc: omap: Fixes, cleanup and add ERASE capability Jarkko Nikula
2013-12-04 21:14   ` [PATCH 1/7] mmc: omap: Fix NULL pointer dereference due uninitialized cover_tasklet Jarkko Nikula
2013-12-04 21:14   ` [PATCH 2/7] mmc: omap: Convert to devm_kzalloc Jarkko Nikula
2013-12-04 21:14   ` [PATCH 3/7] mmc: omap: Remove duplicate host->irq assignment Jarkko Nikula
2013-12-04 21:14   ` [PATCH 4/7] mmc: omap: Remove mem_res field from struct mmc_omap_host Jarkko Nikula
2013-12-04 21:14   ` [PATCH 5/7] mmc: omap: Convert to devm_ioremap_resource Jarkko Nikula
2013-12-04 21:14   ` [PATCH 6/7] mmc: omap: Remove always set use_dma flag from struct mmc_omap_host Jarkko Nikula
2013-12-04 21:14   ` [PATCH 7/7] mmc: omap: Add erase capability Jarkko Nikula
2013-12-05 18:18   ` [PATCH 0/7] mmc: omap: Fixes, cleanup and add ERASE capability Tony Lindgren
2013-12-06 13:41   ` Aaro Koskinen
2013-12-06 19:54     ` Jarkko Nikula

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).