[PATCH dev-5.0 4/4] media: aspeed: clear interrupt status flags immediately
Jae Hyun Yoo
jae.hyun.yoo at linux.intel.com
Wed May 1 03:35:56 AEST 2019
On 4/29/2019 8:04 PM, Joel Stanley wrote:
> On Mon, 29 Apr 2019 at 23:38, Jae Hyun Yoo <jae.hyun.yoo at linux.intel.com> wrote:
>>
>> On 4/29/2019 3:29 PM, Eddie James wrote:
>>>
>>> On 4/25/19 5:20 PM, Jae Hyun Yoo wrote:
>>>> Interrupt status flags should be cleared immediately otherwise
>>>> interrupt handler will be called again and again until the flag
>>>> is cleared, but this driver clears some flags through a 500ms
>>>> delayed work which is a bad idea in interrupt handling, so this
>>>> commit makes the interrupt handler clear the status flags
>>>> immediately.
>>>>
>>>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo at linux.intel.com>
>>>> ---
>>>> drivers/media/platform/aspeed-video.c | 12 +++++++-----
>>>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/aspeed-video.c
>>>> b/drivers/media/platform/aspeed-video.c
>>>> index 77c209a472ca..e218f375b9f5 100644
>>>> --- a/drivers/media/platform/aspeed-video.c
>>>> +++ b/drivers/media/platform/aspeed-video.c
>>>> @@ -546,17 +546,18 @@ static irqreturn_t aspeed_video_irq(int irq,
>>>> void *arg)
>>>> * re-initialize
>>>> */
>>>> if (sts & VE_INTERRUPT_MODE_DETECT_WD) {
>>>> + aspeed_video_write(video, VE_INTERRUPT_STATUS,
>>>> + VE_INTERRUPT_MODE_DETECT_WD);
>>>
>>>
>>> aspeed_video_irq_res_change disables all IRQs and turns off the clocks.
>>> This shouldn't be necessary.
>>
>> In fact, this patch fixes a watch dog reset with printing out a stack
>> trace like below. This happens very rarely but it's critical because it
>> causes a BMC reset. In my experiments, interrupt flags should be cleared
>> even with the aspeed_video_write(video, VE_INTERRUPT_CTRL, 0) in
>> aspeed_video_off(), or we should add
>> apeed_video_write(video, VE_INTERRUPT_STATUS, 0xffffffff)
>> before the disabling interrupt. I think the way in this patch is better.
>
> In general, a driver should certainly be clearing (acking) the
> interrupt bits in the interrupt handler before returning.
>
> Jae, it would be interesting to know the value of VE_INTERRUPT_STATUS
> in the soft lockup situation.
>
> I took a closer look at this function, and it should probably not
> return IRQ_HANDLED at the bottom, as it may have fallen through all of
> the if statements and not have handled any interrupt. Jae, can you
> take a look at this and send another patch if you think that is
> correct.
You are right. It should return IRQ_NONE when there is any unhanded bit.
I'll look at the interrupt handler again. Thanks for your pointing it
out.
Thanks,
Jae
> Cheers,
>
> Joel
>
>>
>> After applying this patch, I've not seen the watch dog reset yet for
>> over a week.
>>
>> Thanks,
>>
>> Jae
>>
>> [ 2174.199114] sched: RT throttling activated
>> [ 2200.009118] watchdog: BUG: soft lockup - CPU#0 stuck for 22s!
>> [irq/23-aspeed-v:609]
>> [ 2200.016802] CPU: 0 PID: 609 Comm: irq/23-aspeed-v Not tainted
>> 5.0.7-e124b50aeacb66baa42541ebc6c3544350f75a79 #1
>> [ 2200.026884] Hardware name: Generic DT based system
>> [ 2200.031705] PC is at irq_finalize_oneshot.part.0+0x6c/0x11c
>> [ 2200.037284] LR is at unmask_irq.part.4+0x30/0x44
>> [ 2200.041902] pc : [<8014c550>] lr : [<8014ecb4>] psr: a0000013
>> [ 2200.048159] sp : 9e56bef0 ip : 00000000 fp : 9e56bf0c
>> [ 2200.053377] r10: 80a07008 r9 : 00000000 r8 : 8014c6b0
>> [ 2200.058595] r7 : 00000001 r6 : 00000001 r5 : 9d11ed90 r4 : 9d11ed80
>> [ 2200.065112] r3 : 02400200 r2 : 9d11ed80 r1 : 00000080 r0 : 9d002940
>> [ 2200.071630] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM
>> Segment none
>> [ 2200.078754] Control: 00c5387d Table: 9d98c008 DAC: 00000051
>> [ 2200.084496] CPU: 0 PID: 609 Comm: irq/23-aspeed-v Not tainted
>> 5.0.7-e124b50aeacb66baa42541ebc6c3544350f75a79 #1
>> [ 2200.094567] Hardware name: Generic DT based system
>> [ 2200.099352] Backtrace:
>> [ 2200.101844] [<80107cdc>] (dump_backtrace) from [<80107f10>]
>> (show_stack+0x20/0x24)
>> [ 2200.109415] r7:00000000 r6:9e56a000 r5:80a070d8 r4:80a1ba08
>> [ 2200.115092] [<80107ef0>] (show_stack) from [<80691844>]
>> (dump_stack+0x20/0x28)
>> [ 2200.122320] [<80691824>] (dump_stack) from [<80103dd8>]
>> (show_regs+0x1c/0x20)
>> [ 2200.129467] [<80103dbc>] (show_regs) from [<8017a940>]
>> (watchdog_timer_fn+0x1c4/0x21c)
>> [ 2200.137393] [<8017a77c>] (watchdog_timer_fn) from [<80159ce0>]
>> (__hrtimer_run_queues.constprop.3+0x14c/0x280)
>> [ 2200.147306] r10:8017a77c r9:80a18cb0 r8:00000006 r7:20000193
>> r6:80a18b80 r5:80a18bc0
>> [ 2200.155121] r4:80a1ba40
>> [ 2200.157667] [<80159b94>] (__hrtimer_run_queues.constprop.3) from
>> [<8015aac0>] (hrtimer_interrupt+0xf4/0x258)
>> [ 2200.167491] r10:80a18d00 r9:80a18cb0 r8:ffffffff r7:7fffffff
>> r6:00000003 r5:20000193
>> [ 2200.175305] r4:80a18b80
>> [ 2200.177867] [<8015a9cc>] (hrtimer_interrupt) from [<8050d51c>]
>> (fttmr010_timer_interrupt+0x20/0x28)
>> [ 2200.186915] r10:9e56bdf8 r9:00000000 r8:9d013600 r7:00000011
>> r6:9d01fd80 r5:80a4af84
>> [ 2200.194727] r4:9d001b80
>> [ 2200.197292] [<8050d4fc>] (fttmr010_timer_interrupt) from [<8014be68>]
>> (__handle_irq_event_percpu+0x48/0x1c4)
>> [ 2200.207125] [<8014be20>] (__handle_irq_event_percpu) from
>> [<8014c01c>] (handle_irq_event_percpu+0x38/0x90)
>> [ 2200.216778] r10:80a07008 r9:9e56a000 r8:9d013600 r7:00000000
>> r6:9d01fd80 r5:80a4af84
>> [ 2200.224594] r4:80a07008
>> [ 2200.227139] [<8014bfe4>] (handle_irq_event_percpu) from [<8014c0ac>]
>> (handle_irq_event+0x38/0x4c)
>> [ 2200.236003] r6:00000001 r5:80a4af84 r4:9d01fd80
>> [ 2200.240623] [<8014c074>] (handle_irq_event) from [<8014fa84>]
>> (handle_edge_irq+0xb0/0x1cc)
>> [ 2200.248881] r5:80a4af84 r4:9d01fd80
>> [ 2200.252462] [<8014f9d4>] (handle_edge_irq) from [<8014b6a4>]
>> (generic_handle_irq+0x30/0x44)
>> [ 2200.260805] r5:80a4af84 r4:00000011
>> [ 2200.264387] [<8014b674>] (generic_handle_irq) from [<8014b710>]
>> (__handle_domain_irq+0x58/0xb8)
>> [ 2200.273091] [<8014b6b8>] (__handle_domain_irq) from [<80102164>]
>> (avic_handle_irq+0x68/0x70)
>> [ 2200.281525] r9:9e56a000 r8:8014c6b0 r7:9e56bed4 r6:ffffffff
>> r5:9e56bea0 r4:9d002940
>> [ 2200.289267] [<801020fc>] (avic_handle_irq) from [<801019ec>]
>> (__irq_svc+0x6c/0x90)
>> [ 2200.296824] Exception stack(0x9e56bea0 to 0x9e56bee8)
>> [ 2200.301879] bea0: 9d002940 00000080 9d11ed80 02400200 9d11ed80
>> 9d11ed90 00000001 00000001
>> [ 2200.310060] bec0: 8014c6b0 00000000 80a07008 9e56bf0c 00000000
>> 9e56bef0 8014ecb4 8014c550
>> [ 2200.318222] bee0: a0000013 ffffffff
>> [ 2200.321708] r5:a0000013 r4:8014c550
>> [ 2200.325290] [<8014c4e4>] (irq_finalize_oneshot.part.0) from
>> [<8014c72c>] (irq_thread_fn+0x7c/0x88)
>> [ 2200.334238] r5:9d11ed80 r4:9e539540
>> [ 2200.337821] [<8014c6b0>] (irq_thread_fn) from [<8014c974>]
>> (irq_thread+0x104/0x1e0)
>> [ 2200.345473] r7:00000001 r6:9d11ed80 r5:ffffe000 r4:9e539540
>> [ 2200.351153] [<8014c870>] (irq_thread) from [<80133490>]
>> (kthread+0x14c/0x164)
>> [ 2200.358287] r10:9d0a3c00 r9:8014c870 r8:9e539540 r7:9e56a000
>> r6:00000000 r5:9e542060
>> [ 2200.366104] r4:9e539b00
>> [ 2200.368650] [<80133344>] (kthread) from [<801010e8>]
>> (ret_from_fork+0x14/0x2c)
>> [ 2200.375865] Exception stack(0x9e56bfb0 to 0x9e56bff8)
>> [ 2200.380911] bfa0: 00000000
>> 00000000 00000000 00000000
>> [ 2200.389079] bfc0: 00000000 00000000 00000000 00000000 00000000
>> 00000000 00000000 00000000
>> [ 2200.397248] bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
>> [ 2200.403863] r10:00000000 r9:00000000 r8:00000000 r7:00000000
>> r6:00000000 r5:80133344
>> [ 2200.411681] r4:9e542060
>> [ 2228.009114] watchdog: BUG: soft lockup - CPU#0 stuck for 22s!
>> [irq/23-aspeed-v:609]
>> [ 2228.016798] CPU: 0 PID: 609 Comm: irq/23-aspeed-v Tainted: G
>> L 5.0.7-e124b50aeacb66baa42541ebc6c3544350f75a79 #1
>> [ 2228.028268] Hardware name: Generic DT based system
>> [ 2228.033085] PC is at irq_finalize_oneshot.part.0+0x6c/0x11c
>> [ 2228.038670] LR is at unmask_irq.part.4+0x30/0x44
>> [ 2228.043288] pc : [<8014c550>] lr : [<8014ecb4>] psr: a0000013
>> [ 2228.049545] sp : 9e56bef0 ip : 00000000 fp : 9e56bf0c
>> [ 2228.054762] r10: 80a07008 r9 : 00000000 r8 : 8014c6b0
>> [ 2228.059980] r7 : 00000001 r6 : 00000001 r5 : 9d11ed90 r4 : 9d11ed80
>> [ 2228.066498] r3 : 02400200 r2 : 9d11ed80 r1 : 00000080 r0 : 9d002940
>> [ 2228.073014] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM
>> Segment none
>> [ 2228.080137] Control: 00c5387d Table: 9d98c008 DAC: 00000051
>> [ 2228.085881] CPU: 0 PID: 609 Comm: irq/23-aspeed-v Tainted: G
>> L 5.0.7-e124b50aeacb66baa42541ebc6c3544350f75a79 #1
>> [ 2228.097338] Hardware name: Generic DT based system
>> [ 2228.102123] Backtrace:
>> [ 2228.104609] [<80107cdc>] (dump_backtrace) from [<80107f10>]
>> (show_stack+0x20/0x24)
>> [ 2228.112176] r7:00000000 r6:9e56a000 r5:80a070d8 r4:80a1ba08
>> [ 2228.117852] [<80107ef0>] (show_stack) from [<80691844>]
>> (dump_stack+0x20/0x28)
>> [ 2228.125080] [<80691824>] (dump_stack) from [<80103dd8>]
>> (show_regs+0x1c/0x20)
>> [ 2228.132234] [<80103dbc>] (show_regs) from [<8017a940>]
>> (watchdog_timer_fn+0x1c4/0x21c)
>> [ 2228.140164] [<8017a77c>] (watchdog_timer_fn) from [<80159ce0>]
>> (__hrtimer_run_queues.constprop.3+0x14c/0x280)
>> [ 2228.150078] r10:8017a77c r9:80a18cb0 r8:00000006 r7:20000193
>> r6:80a18b80 r5:80a18bc0
>> [ 2228.157891] r4:80a1ba40
>> [ 2228.160438] [<80159b94>] (__hrtimer_run_queues.constprop.3) from
>> [<8015aac0>] (hrtimer_interrupt+0xf4/0x258)
>> [ 2228.170262] r10:80a18d00 r9:80a18cb0 r8:ffffffff r7:7fffffff
>> r6:00000003 r5:20000193
>> [ 2228.178077] r4:80a18b80
>> [ 2228.180637] [<8015a9cc>] (hrtimer_interrupt) from [<8050d51c>]
>> (fttmr010_timer_interrupt+0x20/0x28)
>> [ 2228.189674] r10:9e56bdf8 r9:00000000 r8:9d013600 r7:00000011
>> r6:9d01fd80 r5:80a4af84
>> [ 2228.197491] r4:9d001b80
>> [ 2228.200052] [<8050d4fc>] (fttmr010_timer_interrupt) from [<8014be68>]
>> (__handle_irq_event_percpu+0x48/0x1c4)
>> [ 2228.209884] [<8014be20>] (__handle_irq_event_percpu) from
>> [<8014c01c>] (handle_irq_event_percpu+0x38/0x90)
>> [ 2228.219530] r10:80a07008 r9:9e56a000 r8:9d013600 r7:00000000
>> r6:9d01fd80 r5:80a4af84
>> [ 2228.227348] r4:80a07008
>> [ 2228.229892] [<8014bfe4>] (handle_irq_event_percpu) from [<8014c0ac>]
>> (handle_irq_event+0x38/0x4c)
>> [ 2228.238758] r6:00000001 r5:80a4af84 r4:9d01fd80
>> [ 2228.243388] [<8014c074>] (handle_irq_event) from [<8014fa84>]
>> (handle_edge_irq+0xb0/0x1cc)
>> [ 2228.251643] r5:80a4af84 r4:9d01fd80
>> [ 2228.255224] [<8014f9d4>] (handle_edge_irq) from [<8014b6a4>]
>> (generic_handle_irq+0x30/0x44)
>> [ 2228.263558] r5:80a4af84 r4:00000011
>> [ 2228.267142] [<8014b674>] (generic_handle_irq) from [<8014b710>]
>> (__handle_domain_irq+0x58/0xb8)
>> [ 2228.275844] [<8014b6b8>] (__handle_domain_irq) from [<80102164>]
>> (avic_handle_irq+0x68/0x70)
>> [ 2228.284279] r9:9e56a000 r8:8014c6b0 r7:9e56bed4 r6:ffffffff
>> r5:9e56bea0 r4:9d002940
>> [ 2228.292019] [<801020fc>] (avic_handle_irq) from [<801019ec>]
>> (__irq_svc+0x6c/0x90)
>> [ 2228.299577] Exception stack(0x9e56bea0 to 0x9e56bee8)
>> [ 2228.304630] bea0: 9d002940 00000080 9d11ed80 02400200 9d11ed80
>> 9d11ed90 00000001 00000001
>> [ 2228.312803] bec0: 8014c6b0 00000000 80a07008 9e56bf0c 00000000
>> 9e56bef0 8014ecb4 8014c550
>> [ 2228.320967] bee0: a0000013 ffffffff
>> [ 2228.324451] r5:a0000013 r4:8014c550
>> [ 2228.328038] [<8014c4e4>] (irq_finalize_oneshot.part.0) from
>> [<8014c72c>] (irq_thread_fn+0x7c/0x88)
>> [ 2228.336983] r5:9d11ed80 r4:9e539540
>> [ 2228.340566] [<8014c6b0>] (irq_thread_fn) from [<8014c974>]
>> (irq_thread+0x104/0x1e0)
>> [ 2228.348219] r7:00000001 r6:9d11ed80 r5:ffffe000 r4:9e539540
>> [ 2228.353898] [<8014c870>] (irq_thread) from [<80133490>]
>> (kthread+0x14c/0x164)
>> [ 2228.361031] r10:9d0a3c00 r9:8014c870 r8:9e539540 r7:9e56a000
>> r6:00000000 r5:9e542060
>> [ 2228.368848] r4:9e539b00
>> [ 2228.371394] [<80133344>] (kthread) from [<801010e8>]
>> (ret_from_fork+0x14/0x2c)
>> [ 2228.378609] Exception stack(0x9e56bfb0 to 0x9e56bff8)
>> [ 2228.383656] bfa0: 00000000
>> 00000000 00000000 00000000
>> [ 2228.391833] bfc0: 00000000 00000000 00000000 00000000 00000000
>> 00000000 00000000 00000000
>> [ 2228.400004] bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
>> [ 2228.406617] r10:00000000 r9:00000000 r8:00000000 r7:00000000
>> r6:00000000 r5:80133344
>> [ 2228.414436] r4:9e542060
>>
>>>
>>>
>>> The rest looks fine.
>>>
>>> Thanks,
>>>
>>> Eddie
>>>
>>>
>>>> aspeed_video_irq_res_change(video);
>>>> return IRQ_HANDLED;
>>>> }
>>>> if (sts & VE_INTERRUPT_MODE_DETECT) {
>>>> + aspeed_video_write(video, VE_INTERRUPT_STATUS,
>>>> + VE_INTERRUPT_MODE_DETECT);
>>>> if (test_bit(VIDEO_RES_DETECT, &video->flags)) {
>>>> aspeed_video_update(video, VE_INTERRUPT_CTRL,
>>>> VE_INTERRUPT_MODE_DETECT, 0);
>>>> - aspeed_video_write(video, VE_INTERRUPT_STATUS,
>>>> - VE_INTERRUPT_MODE_DETECT);
>>>> -
>>>> set_bit(VIDEO_MODE_DETECT_DONE, &video->flags);
>>>> wake_up_interruptible_all(&video->wait);
>>>> } else {
>>>> @@ -574,6 +575,9 @@ static irqreturn_t aspeed_video_irq(int irq, void
>>>> *arg)
>>>> u32 frame_size = aspeed_video_read(video,
>>>> VE_OFFSET_COMP_STREAM);
>>>> + aspeed_video_write(video, VE_INTERRUPT_STATUS,
>>>> + VE_INTERRUPT_COMP_COMPLETE);
>>>> +
>>>> spin_lock(&video->lock);
>>>> clear_bit(VIDEO_FRAME_INPRG, &video->flags);
>>>> buf = list_first_entry_or_null(&video->buffers,
>>>> @@ -599,8 +603,6 @@ static irqreturn_t aspeed_video_irq(int irq, void
>>>> *arg)
>>>> VE_SEQ_CTRL_TRIG_COMP, 0);
>>>> aspeed_video_update(video, VE_INTERRUPT_CTRL,
>>>> VE_INTERRUPT_COMP_COMPLETE, 0);
>>>> - aspeed_video_write(video, VE_INTERRUPT_STATUS,
>>>> - VE_INTERRUPT_COMP_COMPLETE);
>>>> if (test_bit(VIDEO_STREAMING, &video->flags) && buf)
>>>> aspeed_video_start_frame(video);
>>>
More information about the openbmc
mailing list