diff options
author | Andy Walls <awalls@md.metrocast.net> | 2011-01-15 20:02:05 -0500 |
---|---|---|
committer | Mauro Carvalho Chehab <mchehab@redhat.com> | 2011-01-19 08:46:07 -0500 |
commit | b757730b022b4b1367d0435fcaa1b0a01e8aef42 (patch) | |
tree | 50caa93ce85cd02a084d8008ed23a10338f1bdd7 | |
parent | a68a9b73fbb05144a71b573f262fbc8ed8f71179 (diff) |
[media] lirc_zilog: Update IR Rx polling kthread start/stop and some printks
The IR Rx polling thread was originally a kernel_thread long ago,
and had only been minimally converted to a kthread. This patch
finishes that conversion by
- cleaning up all the unneeded completions
- destroying the kthread properly by calling kthread_stop()
- changing lirc_thread() to test kthread_should_stop() just before
every point where it may sleep
- reorganizing the lirc_thread() function so it uses fewer lines
- modifying the name of the kthread from "lirc_zilog" to
"zilog-rx-i2c-N", so ps will show which kthread polls
which Zilog Z8 IR unit.
Also some minor tweaks were made to logging emitted by the
ir_probe() function.
Signed-off-by: Andy Walls <awalls@md.metrocast.net>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
-rw-r--r-- | drivers/staging/lirc/lirc_zilog.c | 113 |
1 files changed, 49 insertions, 64 deletions
diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c index f2e8c63fa5bd..f7aa5e4e98b3 100644 --- a/drivers/staging/lirc/lirc_zilog.c +++ b/drivers/staging/lirc/lirc_zilog.c | |||
@@ -69,9 +69,6 @@ struct IR_rx { | |||
69 | struct mutex buf_lock; | 69 | struct mutex buf_lock; |
70 | 70 | ||
71 | /* RX polling thread data */ | 71 | /* RX polling thread data */ |
72 | struct completion *t_notify; | ||
73 | struct completion *t_notify2; | ||
74 | int shutdown; | ||
75 | struct task_struct *task; | 72 | struct task_struct *task; |
76 | 73 | ||
77 | /* RX read data */ | 74 | /* RX read data */ |
@@ -171,12 +168,20 @@ static int add_to_buf(struct IR *ir) | |||
171 | * data and we have space | 168 | * data and we have space |
172 | */ | 169 | */ |
173 | do { | 170 | do { |
171 | if (kthread_should_stop()) | ||
172 | return -ENODATA; | ||
173 | |||
174 | /* | 174 | /* |
175 | * Lock i2c bus for the duration. RX/TX chips interfere so | 175 | * Lock i2c bus for the duration. RX/TX chips interfere so |
176 | * this is worth it | 176 | * this is worth it |
177 | */ | 177 | */ |
178 | mutex_lock(&ir->ir_lock); | 178 | mutex_lock(&ir->ir_lock); |
179 | 179 | ||
180 | if (kthread_should_stop()) { | ||
181 | mutex_unlock(&ir->ir_lock); | ||
182 | return -ENODATA; | ||
183 | } | ||
184 | |||
180 | /* | 185 | /* |
181 | * Send random "poll command" (?) Windows driver does this | 186 | * Send random "poll command" (?) Windows driver does this |
182 | * and it is a good point to detect chip failure. | 187 | * and it is a good point to detect chip failure. |
@@ -196,6 +201,10 @@ static int add_to_buf(struct IR *ir) | |||
196 | "trying reset\n"); | 201 | "trying reset\n"); |
197 | 202 | ||
198 | set_current_state(TASK_UNINTERRUPTIBLE); | 203 | set_current_state(TASK_UNINTERRUPTIBLE); |
204 | if (kthread_should_stop()) { | ||
205 | mutex_unlock(&ir->ir_lock); | ||
206 | return -ENODATA; | ||
207 | } | ||
199 | schedule_timeout((100 * HZ + 999) / 1000); | 208 | schedule_timeout((100 * HZ + 999) / 1000); |
200 | if (ir->tx != NULL) | 209 | if (ir->tx != NULL) |
201 | ir->tx->need_boot = 1; | 210 | ir->tx->need_boot = 1; |
@@ -205,6 +214,10 @@ static int add_to_buf(struct IR *ir) | |||
205 | continue; | 214 | continue; |
206 | } | 215 | } |
207 | 216 | ||
217 | if (kthread_should_stop()) { | ||
218 | mutex_unlock(&ir->ir_lock); | ||
219 | return -ENODATA; | ||
220 | } | ||
208 | ret = i2c_master_recv(rx->c, keybuf, sizeof(keybuf)); | 221 | ret = i2c_master_recv(rx->c, keybuf, sizeof(keybuf)); |
209 | mutex_unlock(&ir->ir_lock); | 222 | mutex_unlock(&ir->ir_lock); |
210 | if (ret != sizeof(keybuf)) { | 223 | if (ret != sizeof(keybuf)) { |
@@ -255,48 +268,35 @@ static int lirc_thread(void *arg) | |||
255 | struct IR *ir = arg; | 268 | struct IR *ir = arg; |
256 | struct IR_rx *rx = ir->rx; | 269 | struct IR_rx *rx = ir->rx; |
257 | 270 | ||
258 | if (rx->t_notify != NULL) | ||
259 | complete(rx->t_notify); | ||
260 | |||
261 | dprintk("poll thread started\n"); | 271 | dprintk("poll thread started\n"); |
262 | 272 | ||
263 | do { | 273 | while (!kthread_should_stop()) { |
264 | if (ir->open) { | 274 | set_current_state(TASK_INTERRUPTIBLE); |
265 | set_current_state(TASK_INTERRUPTIBLE); | ||
266 | 275 | ||
267 | /* | 276 | /* if device not opened, we can sleep half a second */ |
268 | * This is ~113*2 + 24 + jitter (2*repeat gap + | 277 | if (!ir->open) { |
269 | * code length). We use this interval as the chip | ||
270 | * resets every time you poll it (bad!). This is | ||
271 | * therefore just sufficient to catch all of the | ||
272 | * button presses. It makes the remote much more | ||
273 | * responsive. You can see the difference by | ||
274 | * running irw and holding down a button. With | ||
275 | * 100ms, the old polling interval, you'll notice | ||
276 | * breaks in the repeat sequence corresponding to | ||
277 | * lost keypresses. | ||
278 | */ | ||
279 | schedule_timeout((260 * HZ) / 1000); | ||
280 | if (rx->shutdown) | ||
281 | break; | ||
282 | if (!add_to_buf(ir)) | ||
283 | wake_up_interruptible(&rx->buf.wait_poll); | ||
284 | } else { | ||
285 | /* if device not opened so we can sleep half a second */ | ||
286 | set_current_state(TASK_INTERRUPTIBLE); | ||
287 | schedule_timeout(HZ/2); | 278 | schedule_timeout(HZ/2); |
279 | continue; | ||
288 | } | 280 | } |
289 | } while (!rx->shutdown); | ||
290 | 281 | ||
291 | if (rx->t_notify2 != NULL) | 282 | /* |
292 | wait_for_completion(rx->t_notify2); | 283 | * This is ~113*2 + 24 + jitter (2*repeat gap + code length). |
293 | 284 | * We use this interval as the chip resets every time you poll | |
294 | rx->task = NULL; | 285 | * it (bad!). This is therefore just sufficient to catch all |
295 | if (rx->t_notify != NULL) | 286 | * of the button presses. It makes the remote much more |
296 | complete(rx->t_notify); | 287 | * responsive. You can see the difference by running irw and |
288 | * holding down a button. With 100ms, the old polling | ||
289 | * interval, you'll notice breaks in the repeat sequence | ||
290 | * corresponding to lost keypresses. | ||
291 | */ | ||
292 | schedule_timeout((260 * HZ) / 1000); | ||
293 | if (kthread_should_stop()) | ||
294 | break; | ||
295 | if (!add_to_buf(ir)) | ||
296 | wake_up_interruptible(&rx->buf.wait_poll); | ||
297 | } | ||
297 | 298 | ||
298 | dprintk("poll thread ended\n"); | 299 | dprintk("poll thread ended\n"); |
299 | /* FIXME - investigate if this is the proper way to shutdown a kthread*/ | ||
300 | return 0; | 300 | return 0; |
301 | } | 301 | } |
302 | 302 | ||
@@ -1169,25 +1169,12 @@ static const struct file_operations lirc_fops = { | |||
1169 | .release = close | 1169 | .release = close |
1170 | }; | 1170 | }; |
1171 | 1171 | ||
1172 | /* FIXME - investigate if this is the proper way to shutdown a kthread */ | ||
1173 | static void destroy_rx_kthread(struct IR_rx *rx) | 1172 | static void destroy_rx_kthread(struct IR_rx *rx) |
1174 | { | 1173 | { |
1175 | DECLARE_COMPLETION(tn); | ||
1176 | DECLARE_COMPLETION(tn2); | ||
1177 | |||
1178 | if (rx == NULL) | ||
1179 | return; | ||
1180 | |||
1181 | /* end up polling thread */ | 1174 | /* end up polling thread */ |
1182 | if (rx->task && !IS_ERR(rx->task)) { | 1175 | if (rx != NULL && !IS_ERR_OR_NULL(rx->task)) { |
1183 | rx->t_notify = &tn; | 1176 | kthread_stop(rx->task); |
1184 | rx->t_notify2 = &tn2; | 1177 | rx->task = NULL; |
1185 | rx->shutdown = 1; | ||
1186 | wake_up_process(rx->task); | ||
1187 | complete(&tn2); | ||
1188 | wait_for_completion(&tn); | ||
1189 | rx->t_notify = NULL; | ||
1190 | rx->t_notify2 = NULL; | ||
1191 | } | 1178 | } |
1192 | } | 1179 | } |
1193 | 1180 | ||
@@ -1290,8 +1277,8 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id) | |||
1290 | else if (tx_only) /* module option */ | 1277 | else if (tx_only) /* module option */ |
1291 | return -ENXIO; | 1278 | return -ENXIO; |
1292 | 1279 | ||
1293 | zilog_info("%s: probing IR %s on %s (i2c-%d)\n", | 1280 | zilog_info("probing IR %s on %s (i2c-%d)\n", |
1294 | __func__, tx_probe ? "Tx" : "Rx", adap->name, adap->nr); | 1281 | tx_probe ? "Tx" : "Rx", adap->name, adap->nr); |
1295 | 1282 | ||
1296 | mutex_lock(&ir_devices_lock); | 1283 | mutex_lock(&ir_devices_lock); |
1297 | 1284 | ||
@@ -1360,27 +1347,23 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id) | |||
1360 | /* Proceed only if we have the required Tx and Rx clients ready to go */ | 1347 | /* Proceed only if we have the required Tx and Rx clients ready to go */ |
1361 | if (ir->tx == NULL || | 1348 | if (ir->tx == NULL || |
1362 | (ir->rx == NULL && !tx_only)) { | 1349 | (ir->rx == NULL && !tx_only)) { |
1363 | zilog_info("%s: probe of IR %s on %s (i2c-%d) done, waiting on " | 1350 | zilog_info("probe of IR %s on %s (i2c-%d) done. Waiting on " |
1364 | "IR %s\n", __func__, tx_probe ? "Tx" : "Rx", | 1351 | "IR %s.\n", tx_probe ? "Tx" : "Rx", adap->name, |
1365 | adap->name, adap->nr, tx_probe ? "Rx" : "Tx"); | 1352 | adap->nr, tx_probe ? "Rx" : "Tx"); |
1366 | goto out_ok; | 1353 | goto out_ok; |
1367 | } | 1354 | } |
1368 | 1355 | ||
1369 | /* initialise RX device */ | 1356 | /* initialise RX device */ |
1370 | if (ir->rx != NULL) { | 1357 | if (ir->rx != NULL) { |
1371 | DECLARE_COMPLETION(tn); | ||
1372 | |||
1373 | /* try to fire up polling thread */ | 1358 | /* try to fire up polling thread */ |
1374 | ir->rx->t_notify = &tn; | 1359 | ir->rx->task = kthread_run(lirc_thread, ir, |
1375 | ir->rx->task = kthread_run(lirc_thread, ir, "lirc_zilog"); | 1360 | "zilog-rx-i2c-%d", adap->nr); |
1376 | if (IS_ERR(ir->rx->task)) { | 1361 | if (IS_ERR(ir->rx->task)) { |
1377 | ret = PTR_ERR(ir->rx->task); | 1362 | ret = PTR_ERR(ir->rx->task); |
1378 | zilog_error("%s: could not start IR Rx polling thread" | 1363 | zilog_error("%s: could not start IR Rx polling thread" |
1379 | "\n", __func__); | 1364 | "\n", __func__); |
1380 | goto out_free_xx; | 1365 | goto out_free_xx; |
1381 | } | 1366 | } |
1382 | wait_for_completion(&tn); | ||
1383 | ir->rx->t_notify = NULL; | ||
1384 | } | 1367 | } |
1385 | 1368 | ||
1386 | /* register with lirc */ | 1369 | /* register with lirc */ |
@@ -1404,6 +1387,8 @@ static int ir_probe(struct i2c_client *client, const struct i2c_device_id *id) | |||
1404 | goto out_unregister; | 1387 | goto out_unregister; |
1405 | } | 1388 | } |
1406 | 1389 | ||
1390 | zilog_info("probe of IR %s on %s (i2c-%d) done. IR unit ready.\n", | ||
1391 | tx_probe ? "Tx" : "Rx", adap->name, adap->nr); | ||
1407 | out_ok: | 1392 | out_ok: |
1408 | mutex_unlock(&ir_devices_lock); | 1393 | mutex_unlock(&ir_devices_lock); |
1409 | return 0; | 1394 | return 0; |