1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
|
From 1775e41d085b24a672dc271d08bfc83401288f0b Mon Sep 17 00:00:00 2001
From: Jae Hyun Yoo <jae.hyun.yoo@intel.com>
Date: Fri, 22 Mar 2019 16:34:54 -0700
Subject: [PATCH] media: platform: Fix a kernel warning on clk control
Video engine clock control functions in the Aspeed video engine driver are
being called from multiple context without any protection so video clocks
can be disabled twice and eventually it causes a kernel warning with stack
dump printing out like below:
[ 120.034729] WARNING: CPU: 0 PID: 1334 at drivers/clk/clk.c:684 clk_core_unprepare+0x13c/0x170
[ 120.043252] eclk-gate already unprepared
[ 120.047283] CPU: 0 PID: 1334 Comm: obmc-ikvm Tainted: G W 5.0.3-b94b74e8b52db91fe4e99e0bb481ec8bf2b5b47c #1
[ 120.058417] Hardware name: Generic DT based system
[ 120.063219] Backtrace:
[ 120.065787] [<80107cdc>] (dump_backtrace) from [<80107f10>] (show_stack+0x20/0x24)
[ 120.073371] r7:803a4ff0 r6:00000009 r5:00000000 r4:96197e1c
[ 120.079152] [<80107ef0>] (show_stack) from [<8068f7d8>] (dump_stack+0x20/0x28)
[ 120.086479] [<8068f7b8>] (dump_stack) from [<8011604c>] (__warn.part.3+0xb4/0xdc)
[ 120.094068] [<80115f98>] (__warn.part.3) from [<801160e0>] (warn_slowpath_fmt+0x6c/0x90)
[ 120.102164] r6:000002ac r5:8080c0b8 r4:80a07008
[ 120.106893] [<80116078>] (warn_slowpath_fmt) from [<803a4ff0>] (clk_core_unprepare+0x13c/0x170)
[ 120.115686] r3:8080cf8c r2:8080c17c
[ 120.119276] r7:97d68e58 r6:9df23200 r5:9668c260 r4:96459260
[ 120.125046] [<803a4eb4>] (clk_core_unprepare) from [<803a707c>] (clk_unprepare+0x34/0x3c)
[ 120.133226] r5:9668c260 r4:96459260
[ 120.136932] [<803a7048>] (clk_unprepare) from [<804f34bc>] (aspeed_video_off+0x44/0x48)
[ 120.145031] r5:9668c260 r4:9668cbc0
[ 120.148647] [<804f3478>] (aspeed_video_off) from [<804f3fd0>] (aspeed_video_release+0x94/0x118)
[ 120.157435] r5:966a0cb8 r4:966a0800
[ 120.161049] [<804f3f3c>] (aspeed_video_release) from [<804d2c58>] (v4l2_release+0xd4/0xe8)
[ 120.169404] r7:97d68e58 r6:9d087810 r5:9df23200 r4:966a0b20
[ 120.175168] [<804d2b84>] (v4l2_release) from [<80236224>] (__fput+0x98/0x1c4)
[ 120.182316] r5:96698e78 r4:9df23200
[ 120.185994] [<8023618c>] (__fput) from [<802363b8>] (____fput+0x18/0x1c)
[ 120.192712] r9:80a0700c r8:801011e4 r7:00000000 r6:80a64bbc r5:961dd560 r4:961dd89c
[ 120.200562] [<802363a0>] (____fput) from [<80131c08>] (task_work_run+0x7c/0xa4)
[ 120.207994] [<80131b8c>] (task_work_run) from [<80106884>] (do_work_pending+0x4a8/0x578)
[ 120.216163] r7:801011e4 r6:80a07008 r5:96197fb0 r4:ffffe000
[ 120.221856] [<801063dc>] (do_work_pending) from [<8010106c>] (slow_work_pending+0xc/0x20)
[ 120.230116] Exception stack(0x96197fb0 to 0x96197ff8)
[ 120.235254] 7fa0: 00000000 76ccf094 00000000 00000000
[ 120.243438] 7fc0: 00000008 00a11978 7eab3c30 00000006 00000000 00000000 475b0fa4 00000000
[ 120.251692] 7fe0: 00000002 7eab3a40 00000000 47720e38 80000010 00000008
[ 120.258396] r10:00000000 r9:96196000 r8:801011e4 r7:00000006 r6:7eab3c30 r5:00a11978
[ 120.266291] r4:00000008
To prevent this issue, this commit adds spinlock protection and clock
status checking logic into the Aspeed video engine driver.
Fixes: d2b4387f3bdf ("media: platform: Add Aspeed Video Engine driver")
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@intel.com>
Cc: Eddie James <eajames@linux.ibm.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Joel Stanley <joel@jms.id.au>
Cc: Andrew Jeffery <andrew@aj.id.au>
---
drivers/media/platform/aspeed-video.c | 32 +++++++++++++++++++++++++++++---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/aspeed-video.c b/drivers/media/platform/aspeed-video.c
index 8144fe36ad48..e70be8fdbde5 100644
--- a/drivers/media/platform/aspeed-video.c
+++ b/drivers/media/platform/aspeed-video.c
@@ -187,6 +187,7 @@ enum {
VIDEO_STREAMING,
VIDEO_FRAME_INPRG,
VIDEO_STOPPED,
+ VIDEO_CLOCKS_ON,
};
struct aspeed_video_addr {
@@ -483,19 +484,29 @@ static void aspeed_video_enable_mode_detect(struct aspeed_video *video)
static void aspeed_video_off(struct aspeed_video *video)
{
+ if (!test_bit(VIDEO_CLOCKS_ON, &video->flags))
+ return;
+
/* Disable interrupts */
aspeed_video_write(video, VE_INTERRUPT_CTRL, 0);
/* Turn off the relevant clocks */
clk_disable_unprepare(video->vclk);
clk_disable_unprepare(video->eclk);
+
+ clear_bit(VIDEO_CLOCKS_ON, &video->flags);
}
static void aspeed_video_on(struct aspeed_video *video)
{
+ if (test_bit(VIDEO_CLOCKS_ON, &video->flags))
+ return;
+
/* Turn on the relevant clocks */
clk_prepare_enable(video->eclk);
clk_prepare_enable(video->vclk);
+
+ set_bit(VIDEO_CLOCKS_ON, &video->flags);
}
static void aspeed_video_bufs_done(struct aspeed_video *video,
@@ -513,12 +524,14 @@ static void aspeed_video_bufs_done(struct aspeed_video *video,
static void aspeed_video_irq_res_change(struct aspeed_video *video)
{
+ spin_lock(&video->lock);
dev_dbg(video->dev, "Resolution changed; resetting\n");
set_bit(VIDEO_RES_CHANGE, &video->flags);
clear_bit(VIDEO_FRAME_INPRG, &video->flags);
aspeed_video_off(video);
+ spin_unlock(&video->lock);
aspeed_video_bufs_done(video, VB2_BUF_STATE_ERROR);
schedule_delayed_work(&video->res_work, RESOLUTION_CHANGE_DELAY);
@@ -938,9 +951,13 @@ static void aspeed_video_init_regs(struct aspeed_video *video)
static void aspeed_video_start(struct aspeed_video *video)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&video->lock, flags);
aspeed_video_on(video);
aspeed_video_init_regs(video);
+ spin_unlock_irqrestore(&video->lock, flags);
/* Resolution set to 640x480 if no signal found */
aspeed_video_get_resolution(video);
@@ -956,6 +973,9 @@ static void aspeed_video_start(struct aspeed_video *video)
static void aspeed_video_stop(struct aspeed_video *video)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&video->lock, flags);
set_bit(VIDEO_STOPPED, &video->flags);
cancel_delayed_work_sync(&video->res_work);
@@ -969,6 +989,7 @@ static void aspeed_video_stop(struct aspeed_video *video)
video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
video->flags = 0;
+ spin_unlock_irqrestore(&video->lock, flags);
}
static int aspeed_video_querycap(struct file *file, void *fh,
@@ -1306,16 +1327,21 @@ static void aspeed_video_resolution_work(struct work_struct *work)
struct delayed_work *dwork = to_delayed_work(work);
struct aspeed_video *video = container_of(dwork, struct aspeed_video,
res_work);
- u32 input_status = video->v4l2_input_status;
+ unsigned long flags;
+ u32 input_status;
+ spin_lock_irqsave(&video->lock, flags);
+ input_status = video->v4l2_input_status;
aspeed_video_on(video);
/* Exit early in case no clients remain */
- if (test_bit(VIDEO_STOPPED, &video->flags))
+ if (test_bit(VIDEO_STOPPED, &video->flags)) {
+ spin_unlock_irqrestore(&video->lock, flags);
goto done;
+ }
aspeed_video_init_regs(video);
-
+ spin_unlock_irqrestore(&video->lock, flags);
aspeed_video_get_resolution(video);
if (video->detected_timings.width != video->active_timings.width ||
--
2.7.4
|
