* [PATCH 0/3] minor omap dma fixes
@ 2013-06-13 13:09 Rajendra Nayak
2013-06-13 13:09 ` [PATCH 1/3] ARM: OMAP: dma: Remove the wrong dev_id check Rajendra Nayak
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Rajendra Nayak @ 2013-06-13 13:09 UTC (permalink / raw)
To: tony; +Cc: linux-arm-kernel, linux-omap, Rajendra Nayak
Hi,
Here are some minor omap dma fixes.
R Sricharan (2):
ARM: OMAP: dma: Remove the wrong dev_id check
ARM: OMAP: dma: Fix the dma_chan_link_map init order
Rajendra Nayak (1):
ARM: OMAP: dma: Fix the kfree ordering
arch/arm/plat-omap/dma.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] ARM: OMAP: dma: Remove the wrong dev_id check
2013-06-13 13:09 [PATCH 0/3] minor omap dma fixes Rajendra Nayak
@ 2013-06-13 13:09 ` Rajendra Nayak
2013-06-13 13:09 ` [PATCH 2/3] ARM: OMAP: dma: Fix the dma_chan_link_map init order Rajendra Nayak
2013-06-13 13:09 ` [PATCH 3/3] ARM: OMAP: dma: Fix the kfree ordering Rajendra Nayak
2 siblings, 0 replies; 6+ messages in thread
From: Rajendra Nayak @ 2013-06-13 13:09 UTC (permalink / raw)
To: tony; +Cc: linux-arm-kernel, linux-omap, R Sricharan, Rajendra Nayak
From: R Sricharan <r.sricharan@ti.com>
Once a free channel is found, the check for dev_id == 0 does
not make any sense. Get rid of it.
Signed-off-by: R Sricharan <r.sricharan@ti.com>
Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
arch/arm/plat-omap/dma.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
index e06c34b..2f99331 100644
--- a/arch/arm/plat-omap/dma.c
+++ b/arch/arm/plat-omap/dma.c
@@ -701,8 +701,8 @@ int omap_request_dma(int dev_id, const char *dev_name,
for (ch = 0; ch < dma_chan_count; ch++) {
if (free_ch == -1 && dma_chan[ch].dev_id == -1) {
free_ch = ch;
- if (dev_id == 0)
- break;
+ /* Exit after first free channel found */
+ break;
}
}
if (free_ch == -1) {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] ARM: OMAP: dma: Fix the dma_chan_link_map init order
2013-06-13 13:09 [PATCH 0/3] minor omap dma fixes Rajendra Nayak
2013-06-13 13:09 ` [PATCH 1/3] ARM: OMAP: dma: Remove the wrong dev_id check Rajendra Nayak
@ 2013-06-13 13:09 ` Rajendra Nayak
2013-06-13 13:09 ` [PATCH 3/3] ARM: OMAP: dma: Fix the kfree ordering Rajendra Nayak
2 siblings, 0 replies; 6+ messages in thread
From: Rajendra Nayak @ 2013-06-13 13:09 UTC (permalink / raw)
To: tony; +Cc: linux-arm-kernel, linux-omap, R Sricharan, Rajendra Nayak
From: R Sricharan <r.sricharan@ti.com>
Init dma_chan_link_map[lch] *after* its memset to 0.
Signed-off-by: R Sricharan <r.sricharan@ti.com>
Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
arch/arm/plat-omap/dma.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
index 2f99331..8a71f75 100644
--- a/arch/arm/plat-omap/dma.c
+++ b/arch/arm/plat-omap/dma.c
@@ -894,11 +894,12 @@ void omap_start_dma(int lch)
int next_lch, cur_lch;
char dma_chan_link_map[MAX_LOGICAL_DMA_CH_COUNT];
- dma_chan_link_map[lch] = 1;
/* Set the link register of the first channel */
enable_lnk(lch);
memset(dma_chan_link_map, 0, sizeof(dma_chan_link_map));
+ dma_chan_link_map[lch] = 1;
+
cur_lch = dma_chan[lch].next_lch;
do {
next_lch = dma_chan[cur_lch].next_lch;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] ARM: OMAP: dma: Fix the kfree ordering
2013-06-13 13:09 [PATCH 0/3] minor omap dma fixes Rajendra Nayak
2013-06-13 13:09 ` [PATCH 1/3] ARM: OMAP: dma: Remove the wrong dev_id check Rajendra Nayak
2013-06-13 13:09 ` [PATCH 2/3] ARM: OMAP: dma: Fix the dma_chan_link_map init order Rajendra Nayak
@ 2013-06-13 13:09 ` Rajendra Nayak
2013-06-13 13:16 ` Russell King - ARM Linux
2 siblings, 1 reply; 6+ messages in thread
From: Rajendra Nayak @ 2013-06-13 13:09 UTC (permalink / raw)
To: tony; +Cc: linux-arm-kernel, linux-omap, Rajendra Nayak
kfree of 'p' followed by kfree of 'd' where d = p->dma_attr
certainly seems wrong.
Results in a Oops when probe fails and ends up at exit_dma_lch_fail:
[ 0.612579] Unable to handle kernel NULL pointer dereference at virtual address 00000048
[ 0.620971] pgd = c0004000
[ 0.623840] [00000048] *pgd=00000000
[ 0.627593] Internal error: Oops: 5 [#1] SMP ARM
[ 0.632415] Modules linked in:
[ 0.635650] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 3.10.0-rc5-00003-g435cd37-dirty #7
[ 0.644042] task: ed8613c0 ti: ed862000 task.ti: ed862000
[ 0.649688] PC is at kfree+0x64/0x178
[ 0.653533] LR is at kfree+0x34/0x178
[ 0.657379] pc : [<c0114e64>] lr : [<c0114e34>] psr: 40000193
[ 0.657379] sp : ed863e50 ip : 00080000 fp : ed9e7000
[ 0.669342] r10: c07cb990 r9 : 00000001 r8 : c0d95840
[ 0.674804] r7 : a0000113 r6 : 00000000 r5 : c0802240 r4 : 00000802
[ 0.681579] r3 : c0dc0000 r2 : 80000000 r1 : 80802240 r0 : c0802240
[ 0.688385] Flags: nZcv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel
[ 0.696075] Control: 10c53c7d Table: 8000404a DAC: 00000017
[ 0.702056] Process swapper/0 (pid: 1, stack limit = 0xed862240)
[ 0.708312] Stack: (0xed863e50 to 0xed864000)
[ 0.712860] 3e40: c0857a68 00000000 fffffffa 80000000
[ 0.721343] 3e60: ffffffff 00000001 20000113 c0040ee8 ed9e98b0 00000030 ed9e7044 ed9e7010
[ 0.729858] 3e80: c0da9434 ed9e7044 00000000 c081acd4 00000000 c079e460 00000000 c035ba38
[ 0.738342] 3ea0: c035ba20 c035a594 ed9e7010 c081acd4 ed9e7044 00000000 ed862000 c035a7bc
[ 0.746826] 3ec0: c081acd4 c035a728 ed863ed0 c0358dac ed88cea8 ed965110 ed8613c0 c081acd4
[ 0.755310] 3ee0: c0836c00 ed9ebec0 00000000 c03596f0 c06933dc c0856440 c0856440 c081acd4
[ 0.763793] 3f00: c0856440 c075c3d4 ed862000 c035ad90 c0856440 c07728e8 c0856440 c075c3d4
[ 0.772308] 3f20: ed862000 c0008744 00000000 c072eaac c072eaac 00000000 00000001 60000113
[ 0.780792] 3f40: c06f2418 00000000 00000003 00000003 60000113 c079e44c 00000004 c0856440
[ 0.789276] 3f60: c075c3d4 000000a5 c07ba740 c079e460 00000000 c075c308 00000003 00000003
[ 0.797760] 3f80: c075c3d4 00000000 00000000 c0545568 00000000 00000000 00000000 00000000
[ 0.806243] 3fa0: 00000000 c0545570 00000000 c00143a8 00000000 00000000 00000000 00000000
[ 0.814727] 3fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 0.823211] 3fe0: 00000000 00000000 00000000 00000000 00000013 00000000 a2ae33a6 b733c363
[ 0.831726] [<c0114e64>] (kfree+0x64/0x178) from [<c0040ee8>] (omap_system_dma_probe+0x208/0x294)
[ 0.840942] [<c0040ee8>] (omap_system_dma_probe+0x208/0x294) from [<c035ba38>] (platform_drv_probe+0x18/0x1c)
[ 0.851226] [<c035ba38>] (platform_drv_probe+0x18/0x1c) from [<c035a594>] (driver_probe_device+0x90/0x224)
[ 0.861236] [<c035a594>] (driver_probe_device+0x90/0x224) from [<c035a7bc>] (__driver_attach+0x94/0x98)
[ 0.870971] [<c035a7bc>] (__driver_attach+0x94/0x98) from [<c0358dac>] (bus_for_each_dev+0x68/0x8c)
[ 0.880340] [<c0358dac>] (bus_for_each_dev+0x68/0x8c) from [<c03596f0>] (bus_add_driver+0x208/0x2a4)
[ 0.889831] [<c03596f0>] (bus_add_driver+0x208/0x2a4) from [<c035ad90>] (driver_register+0x78/0x194)
[ 0.899291] [<c035ad90>] (driver_register+0x78/0x194) from [<c0008744>] (do_one_initcall+0x30/0x168)
[ 0.908782] [<c0008744>] (do_one_initcall+0x30/0x168) from [<c075c308>] (kernel_init_freeable+0xf4/0x1c0)
[ 0.918701] [<c075c308>] (kernel_init_freeable+0xf4/0x1c0) from [<c0545570>] (kernel_init+0x8/0xe4)
[ 0.928100] [<c0545570>] (kernel_init+0x8/0xe4) from [<c00143a8>] (ret_from_fork+0x14/0x2c)
[ 0.936767] Code: e3160902 1590001c e590601c e1a00005 (e5961048)
[ 0.943145] ---[ end trace f2e8a388a9e63b21 ]---
Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
arch/arm/plat-omap/dma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
index 8a71f75..8e16503 100644
--- a/arch/arm/plat-omap/dma.c
+++ b/arch/arm/plat-omap/dma.c
@@ -2111,8 +2111,8 @@ exit_dma_irq_fail:
}
exit_dma_lch_fail:
- kfree(p);
kfree(d);
+ kfree(p);
kfree(dma_chan);
return ret;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] ARM: OMAP: dma: Fix the kfree ordering
2013-06-13 13:09 ` [PATCH 3/3] ARM: OMAP: dma: Fix the kfree ordering Rajendra Nayak
@ 2013-06-13 13:16 ` Russell King - ARM Linux
2013-06-13 13:48 ` Rajendra Nayak
0 siblings, 1 reply; 6+ messages in thread
From: Russell King - ARM Linux @ 2013-06-13 13:16 UTC (permalink / raw)
To: Rajendra Nayak; +Cc: tony, linux-omap, linux-arm-kernel
On Thu, Jun 13, 2013 at 06:39:20PM +0530, Rajendra Nayak wrote:
> diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
> index 8a71f75..8e16503 100644
> --- a/arch/arm/plat-omap/dma.c
> +++ b/arch/arm/plat-omap/dma.c
> @@ -2111,8 +2111,8 @@ exit_dma_irq_fail:
> }
>
> exit_dma_lch_fail:
> - kfree(p);
> kfree(d);
> + kfree(p);
Err.
p = pdev->dev.platform_data;
d = p->dma_attr;
Why is it kfree'ing platform data in the first place? This means that
a failed bind can't be reattempted later. It also means that an unbind
plus rebind in userspace will free the platform data leaving stale
pointers behind.
This is totally nonsense. Don't kfree() data in your driver which you
haven't allocated yourself!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] ARM: OMAP: dma: Fix the kfree ordering
2013-06-13 13:16 ` Russell King - ARM Linux
@ 2013-06-13 13:48 ` Rajendra Nayak
0 siblings, 0 replies; 6+ messages in thread
From: Rajendra Nayak @ 2013-06-13 13:48 UTC (permalink / raw)
To: Russell King - ARM Linux; +Cc: tony, linux-omap, linux-arm-kernel
On Thursday 13 June 2013 06:46 PM, Russell King - ARM Linux wrote:
> On Thu, Jun 13, 2013 at 06:39:20PM +0530, Rajendra Nayak wrote:
>> diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
>> index 8a71f75..8e16503 100644
>> --- a/arch/arm/plat-omap/dma.c
>> +++ b/arch/arm/plat-omap/dma.c
>> @@ -2111,8 +2111,8 @@ exit_dma_irq_fail:
>> }
>>
>> exit_dma_lch_fail:
>> - kfree(p);
>> kfree(d);
>> + kfree(p);
>
> Err.
>
> p = pdev->dev.platform_data;
> d = p->dma_attr;
>
> Why is it kfree'ing platform data in the first place? This means that
> a failed bind can't be reattempted later. It also means that an unbind
> plus rebind in userspace will free the platform data leaving stale
> pointers behind.
Right, I just seemed to have overlooked the fact that p was pointing to
platform data. Will remove all the kfree(p) and kfree(d) across the
driver (I just realized this is also the case in .remove)
>
> This is totally nonsense. Don't kfree() data in your driver which you
> haven't allocated yourself!
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-06-13 13:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-13 13:09 [PATCH 0/3] minor omap dma fixes Rajendra Nayak
2013-06-13 13:09 ` [PATCH 1/3] ARM: OMAP: dma: Remove the wrong dev_id check Rajendra Nayak
2013-06-13 13:09 ` [PATCH 2/3] ARM: OMAP: dma: Fix the dma_chan_link_map init order Rajendra Nayak
2013-06-13 13:09 ` [PATCH 3/3] ARM: OMAP: dma: Fix the kfree ordering Rajendra Nayak
2013-06-13 13:16 ` Russell King - ARM Linux
2013-06-13 13:48 ` Rajendra Nayak
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).