dmaengine: pl330: fix bug that cause start the same descs in cyclic
This bug will cause NULL pointer after commit dfac17, and cause wrong package in I2S DMA transfer before commit dfac17. Tested on RK3288-pinky2 board. Detail: I2S DMA transfer(sound/core/pcm_dmaengine.c): dmaengine_pcm_prepare_and_submit --> dmaengine_prep_dma_cyclic --> pl330_prep_dma_cyclic --> the case: 1. pl330_submit_req(desc0): thrd->req[0].desc = desc0, thrd->lstenq = 0 2. pl330_submit_req(desc1): thrd->req[1].desc = desc1, thrd->lstenq = 1 3. _start(desc0) by submit_req: thrd->req_running = 0 because: idx = 1 - thrd->lstenq = 0 4. pl330_update(desc0 OK): thrd->req[0].desc = NULL, desc0 to req_done list because: idx = active = thrd->req_running = 0 5. _start(desc1) by pl330_update: thrd->req_running = 1 because: idx = 1 - thrd->lstenq = 0, but thrd->req[0].desc == NULL, so: idx = thrd->lstenq = 1 6. pl330_submit_req(desc2): thrd->req[0].desc = desc2, thrd->lstenq = 0 7. _start(desc1) by submit_req: thrd->req_running = 1 because: idx = 1 - thrd->lstenq = 1 Note: _start started the same descs _start should start desc2 here, NOT desc1 8. pl330_update(desc1 OK): thrd->req[1].desc = NULL, desc1 to req_done list because: idx = active = thrd->req_running = 1 9. _start(desc2) by pl330_update : thrd->req_running = 0 because: idx = 1 - thrd->lstenq = 0 10.pl330_update(desc1 OK, NOT desc2): thrd->req[0].desc = NULL, desc2 to req_done list because: idx = active = thrd->req_running = 0 11.pl330_submit_req(desc3): thrd->req[0].desc = desc3, thrd->lstenq = 0 12.pl330_submit_req(desc4): thrd->req[1].desc = desc4, thrd->lstenq = 1 13._start(desc3) by submit_req: thrd->req_running = 0 because: idx = 1 - thrd->lstenq = 0 14.pl330_update(desc2 OK NOT desc3): thrd->req[0].desc = NULL desc3 to req_done list because: idx = active = thrd->req_running = 0 15._start(desc4) by pl330_update: thrd->req_running = 1 because: idx = 1 - thrd->lstenq = 0, but thrd->req[0].desc == NULL, so: idx = thrd->lstenq = 1 16.pl330_submit_req(desc5): thrd->req[0].desc = desc5, thrd->lstenq = 0 17._start(desc4) by submit_req: thrd->req_running = 1 because: idx = 1 - thrd->lstenq = 1 18.pl330_update(desc3 OK NOT desc4): thrd->req[1].desc = NULL desc4 to req_done list because: idx = active = thrd->req_running = 1 19._start(desc4) by pl330_update: thrd->req_running = 0 because: idx = 1 - thrd->lstenq = 1, but thrd->req[1].desc == NULL, so: idx = thrd->lstenq = 0 20.pl330_update(desc4 OK): thrd->req[0].desc = NULL, desc5 to req_done list because: idx = active = thrd->req_running = 0 21.pl330_update(desc4 OK): 1) before commit dfac17(set req_running -1 in pl330_update/mark_free()): because: active = -1, abort result: desc0-desc5's callback are all called, but step 10 and step 18 go wrong. 2) before commit dfac17: idx = active = thrd->req_runnig = 0 --> descdone = thrd->req[0] = NULL --> list_add_tail(&descdone->rqd, &pl330->req_done); --> got NULL pointer!!! Signed-off-by: Addy Ke <addy.ke@rock-chips.com> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Showing
Please register or sign in to comment