Commit a134b825608df6382dbcf4fe2c54232ba8f7355f

Authored by Nat Gurumoorthy
Committed by Wim Van Sebroeck
1 parent 02f8c6aee8

watchdog: Use "request_muxed_region" in it87 watchdog drivers

Changes the it87 watchdog drivers to use "request_muxed_region".
Serialize access to the hardware by using "request_muxed_region" macro defined
by Alan Cox. Call to this macro will hold off the requestor if the resource is
currently busy.

The use of the above macro makes it possible to get rid of
spinlocks in it8712f_wdt.c and it87_wdt.c watchdog drivers.
This also greatly simplifies the implementation of it87_wdt.c driver.

 "superio_enter" will return an error if call to "request_muxed_region" fails.
Rest of the code change is to ripple an error return from superio_enter to
the top level.

Signed-off-by: Nat Gurumoorthy <natg@google.com>
Signed-off-by: Wim Van Sebroeck <wim@iguana.be>

Showing 2 changed files with 134 additions and 95 deletions Side-by-side Diff

drivers/watchdog/it8712f_wdt.c
... ... @@ -51,7 +51,6 @@
51 51  
52 52 static unsigned long wdt_open;
53 53 static unsigned expect_close;
54   -static spinlock_t io_lock;
55 54 static unsigned char revision;
56 55  
57 56 /* Dog Food address - We use the game port address */
58 57  
59 58  
60 59  
... ... @@ -121,20 +120,26 @@
121 120 outb(ldn, VAL);
122 121 }
123 122  
124   -static inline void superio_enter(void)
  123 +static inline int superio_enter(void)
125 124 {
126   - spin_lock(&io_lock);
  125 + /*
  126 + * Try to reserve REG and REG + 1 for exclusive access.
  127 + */
  128 + if (!request_muxed_region(REG, 2, NAME))
  129 + return -EBUSY;
  130 +
127 131 outb(0x87, REG);
128 132 outb(0x01, REG);
129 133 outb(0x55, REG);
130 134 outb(0x55, REG);
  135 + return 0;
131 136 }
132 137  
133 138 static inline void superio_exit(void)
134 139 {
135 140 outb(0x02, REG);
136 141 outb(0x02, VAL);
137   - spin_unlock(&io_lock);
  142 + release_region(REG, 2);
138 143 }
139 144  
140 145 static inline void it8712f_wdt_ping(void)
141 146  
142 147  
... ... @@ -173,10 +178,13 @@
173 178 return 0;
174 179 }
175 180  
176   -static void it8712f_wdt_enable(void)
  181 +static int it8712f_wdt_enable(void)
177 182 {
  183 + int ret = superio_enter();
  184 + if (ret)
  185 + return ret;
  186 +
178 187 printk(KERN_DEBUG NAME ": enabling watchdog timer\n");
179   - superio_enter();
180 188 superio_select(LDN_GPIO);
181 189  
182 190 superio_outb(wdt_control_reg, WDT_CONTROL);
183 191  
184 192  
185 193  
... ... @@ -186,13 +194,17 @@
186 194 superio_exit();
187 195  
188 196 it8712f_wdt_ping();
  197 +
  198 + return 0;
189 199 }
190 200  
191   -static void it8712f_wdt_disable(void)
  201 +static int it8712f_wdt_disable(void)
192 202 {
193   - printk(KERN_DEBUG NAME ": disabling watchdog timer\n");
  203 + int ret = superio_enter();
  204 + if (ret)
  205 + return ret;
194 206  
195   - superio_enter();
  207 + printk(KERN_DEBUG NAME ": disabling watchdog timer\n");
196 208 superio_select(LDN_GPIO);
197 209  
198 210 superio_outb(0, WDT_CONFIG);
... ... @@ -202,6 +214,7 @@
202 214 superio_outb(0, WDT_TIMEOUT);
203 215  
204 216 superio_exit();
  217 + return 0;
205 218 }
206 219  
207 220 static int it8712f_wdt_notify(struct notifier_block *this,
... ... @@ -252,6 +265,7 @@
252 265 WDIOF_MAGICCLOSE,
253 266 };
254 267 int value;
  268 + int ret;
255 269  
256 270 switch (cmd) {
257 271 case WDIOC_GETSUPPORT:
... ... @@ -259,7 +273,9 @@
259 273 return -EFAULT;
260 274 return 0;
261 275 case WDIOC_GETSTATUS:
262   - superio_enter();
  276 + ret = superio_enter();
  277 + if (ret)
  278 + return ret;
263 279 superio_select(LDN_GPIO);
264 280  
265 281 value = it8712f_wdt_get_status();
... ... @@ -280,7 +296,9 @@
280 296 if (value > (max_units * 60))
281 297 return -EINVAL;
282 298 margin = value;
283   - superio_enter();
  299 + ret = superio_enter();
  300 + if (ret)
  301 + return ret;
284 302 superio_select(LDN_GPIO);
285 303  
286 304 it8712f_wdt_update_margin();
287 305  
... ... @@ -299,10 +317,14 @@
299 317  
300 318 static int it8712f_wdt_open(struct inode *inode, struct file *file)
301 319 {
  320 + int ret;
302 321 /* only allow one at a time */
303 322 if (test_and_set_bit(0, &wdt_open))
304 323 return -EBUSY;
305   - it8712f_wdt_enable();
  324 +
  325 + ret = it8712f_wdt_enable();
  326 + if (ret)
  327 + return ret;
306 328 return nonseekable_open(inode, file);
307 329 }
308 330  
... ... @@ -313,7 +335,8 @@
313 335 ": watchdog device closed unexpectedly, will not"
314 336 " disable the watchdog timer\n");
315 337 } else if (!nowayout) {
316   - it8712f_wdt_disable();
  338 + if (it8712f_wdt_disable())
  339 + printk(KERN_WARNING NAME "Watchdog disable failed\n");
317 340 }
318 341 expect_close = 0;
319 342 clear_bit(0, &wdt_open);
320 343  
... ... @@ -340,8 +363,10 @@
340 363 {
341 364 int err = -ENODEV;
342 365 int chip_type;
  366 + int ret = superio_enter();
  367 + if (ret)
  368 + return ret;
343 369  
344   - superio_enter();
345 370 chip_type = superio_inw(DEVID);
346 371 if (chip_type != IT8712F_DEVID)
347 372 goto exit;
... ... @@ -382,8 +407,6 @@
382 407 {
383 408 int err = 0;
384 409  
385   - spin_lock_init(&io_lock);
386   -
387 410 if (it8712f_wdt_find(&address))
388 411 return -ENODEV;
389 412  
... ... @@ -392,7 +415,11 @@
392 415 return -EBUSY;
393 416 }
394 417  
395   - it8712f_wdt_disable();
  418 + err = it8712f_wdt_disable();
  419 + if (err) {
  420 + printk(KERN_ERR NAME ": unable to disable watchdog timer.\n");
  421 + goto out;
  422 + }
396 423  
397 424 err = register_reboot_notifier(&it8712f_wdt_notifier);
398 425 if (err) {
drivers/watchdog/it87_wdt.c
... ... @@ -137,7 +137,6 @@
137 137  
138 138 static unsigned int base, gpact, ciract, max_units, chip_type;
139 139 static unsigned long wdt_status;
140   -static DEFINE_SPINLOCK(spinlock);
141 140  
142 141 static int nogameport = DEFAULT_NOGAMEPORT;
143 142 static int exclusive = DEFAULT_EXCLUSIVE;
144 143  
145 144  
146 145  
... ... @@ -163,18 +162,26 @@
163 162  
164 163 /* Superio Chip */
165 164  
166   -static inline void superio_enter(void)
  165 +static inline int superio_enter(void)
167 166 {
  167 + /*
  168 + * Try to reserve REG and REG + 1 for exclusive access.
  169 + */
  170 + if (!request_muxed_region(REG, 2, WATCHDOG_NAME))
  171 + return -EBUSY;
  172 +
168 173 outb(0x87, REG);
169 174 outb(0x01, REG);
170 175 outb(0x55, REG);
171 176 outb(0x55, REG);
  177 + return 0;
172 178 }
173 179  
174 180 static inline void superio_exit(void)
175 181 {
176 182 outb(0x02, REG);
177 183 outb(0x02, VAL);
  184 + release_region(REG, 2);
178 185 }
179 186  
180 187 static inline void superio_select(int ldn)
181 188  
182 189  
... ... @@ -255,13 +262,12 @@
255 262 set_bit(WDTS_KEEPALIVE, &wdt_status);
256 263 }
257 264  
258   -static void wdt_start(void)
  265 +static int wdt_start(void)
259 266 {
260   - unsigned long flags;
  267 + int ret = superio_enter();
  268 + if (ret)
  269 + return ret;
261 270  
262   - spin_lock_irqsave(&spinlock, flags);
263   - superio_enter();
264   -
265 271 superio_select(GPIO);
266 272 if (test_bit(WDTS_USE_GP, &wdt_status))
267 273 superio_outb(WDT_GAMEPORT, WDTCTRL);
268 274  
269 275  
270 276  
... ... @@ -270,16 +276,16 @@
270 276 wdt_update_timeout();
271 277  
272 278 superio_exit();
273   - spin_unlock_irqrestore(&spinlock, flags);
  279 +
  280 + return 0;
274 281 }
275 282  
276   -static void wdt_stop(void)
  283 +static int wdt_stop(void)
277 284 {
278   - unsigned long flags;
  285 + int ret = superio_enter();
  286 + if (ret)
  287 + return ret;
279 288  
280   - spin_lock_irqsave(&spinlock, flags);
281   - superio_enter();
282   -
283 289 superio_select(GPIO);
284 290 superio_outb(0x00, WDTCTRL);
285 291 superio_outb(WDT_TOV1, WDTCFG);
... ... @@ -288,7 +294,7 @@
288 294 superio_outb(0x00, WDTVALMSB);
289 295  
290 296 superio_exit();
291   - spin_unlock_irqrestore(&spinlock, flags);
  297 + return 0;
292 298 }
293 299  
294 300 /**
... ... @@ -303,8 +309,6 @@
303 309  
304 310 static int wdt_set_timeout(int t)
305 311 {
306   - unsigned long flags;
307   -
308 312 if (t < 1 || t > max_units * 60)
309 313 return -EINVAL;
310 314  
311 315  
312 316  
... ... @@ -313,14 +317,15 @@
313 317 else
314 318 timeout = t;
315 319  
316   - spin_lock_irqsave(&spinlock, flags);
317 320 if (test_bit(WDTS_TIMER_RUN, &wdt_status)) {
318   - superio_enter();
  321 + int ret = superio_enter();
  322 + if (ret)
  323 + return ret;
  324 +
319 325 superio_select(GPIO);
320 326 wdt_update_timeout();
321 327 superio_exit();
322 328 }
323   - spin_unlock_irqrestore(&spinlock, flags);
324 329 return 0;
325 330 }
326 331  
327 332  
... ... @@ -339,12 +344,12 @@
339 344  
340 345 static int wdt_get_status(int *status)
341 346 {
342   - unsigned long flags;
343   -
344 347 *status = 0;
345 348 if (testmode) {
346   - spin_lock_irqsave(&spinlock, flags);
347   - superio_enter();
  349 + int ret = superio_enter();
  350 + if (ret)
  351 + return ret;
  352 +
348 353 superio_select(GPIO);
349 354 if (superio_inb(WDTCTRL) & WDT_ZERO) {
350 355 superio_outb(0x00, WDTCTRL);
... ... @@ -353,7 +358,6 @@
353 358 }
354 359  
355 360 superio_exit();
356   - spin_unlock_irqrestore(&spinlock, flags);
357 361 }
358 362 if (test_and_clear_bit(WDTS_KEEPALIVE, &wdt_status))
359 363 *status |= WDIOF_KEEPALIVEPING;
360 364  
... ... @@ -379,9 +383,17 @@
379 383 if (exclusive && test_and_set_bit(WDTS_DEV_OPEN, &wdt_status))
380 384 return -EBUSY;
381 385 if (!test_and_set_bit(WDTS_TIMER_RUN, &wdt_status)) {
  386 + int ret;
382 387 if (nowayout && !test_and_set_bit(WDTS_LOCKED, &wdt_status))
383 388 __module_get(THIS_MODULE);
384   - wdt_start();
  389 +
  390 + ret = wdt_start();
  391 + if (ret) {
  392 + clear_bit(WDTS_LOCKED, &wdt_status);
  393 + clear_bit(WDTS_TIMER_RUN, &wdt_status);
  394 + clear_bit(WDTS_DEV_OPEN, &wdt_status);
  395 + return ret;
  396 + }
385 397 }
386 398 return nonseekable_open(inode, file);
387 399 }
... ... @@ -403,7 +415,16 @@
403 415 {
404 416 if (test_bit(WDTS_TIMER_RUN, &wdt_status)) {
405 417 if (test_and_clear_bit(WDTS_EXPECTED, &wdt_status)) {
406   - wdt_stop();
  418 + int ret = wdt_stop();
  419 + if (ret) {
  420 + /*
  421 + * Stop failed. Just keep the watchdog alive
  422 + * and hope nothing bad happens.
  423 + */
  424 + set_bit(WDTS_EXPECTED, &wdt_status);
  425 + wdt_keepalive();
  426 + return ret;
  427 + }
407 428 clear_bit(WDTS_TIMER_RUN, &wdt_status);
408 429 } else {
409 430 wdt_keepalive();
... ... @@ -484,7 +505,9 @@
484 505 &ident, sizeof(ident)) ? -EFAULT : 0;
485 506  
486 507 case WDIOC_GETSTATUS:
487   - wdt_get_status(&status);
  508 + rc = wdt_get_status(&status);
  509 + if (rc)
  510 + return rc;
488 511 return put_user(status, uarg.i);
489 512  
490 513 case WDIOC_GETBOOTSTATUS:
491 514  
... ... @@ -500,14 +523,22 @@
500 523  
501 524 switch (new_options) {
502 525 case WDIOS_DISABLECARD:
503   - if (test_bit(WDTS_TIMER_RUN, &wdt_status))
504   - wdt_stop();
  526 + if (test_bit(WDTS_TIMER_RUN, &wdt_status)) {
  527 + rc = wdt_stop();
  528 + if (rc)
  529 + return rc;
  530 + }
505 531 clear_bit(WDTS_TIMER_RUN, &wdt_status);
506 532 return 0;
507 533  
508 534 case WDIOS_ENABLECARD:
509   - if (!test_and_set_bit(WDTS_TIMER_RUN, &wdt_status))
510   - wdt_start();
  535 + if (!test_and_set_bit(WDTS_TIMER_RUN, &wdt_status)) {
  536 + rc = wdt_start();
  537 + if (rc) {
  538 + clear_bit(WDTS_TIMER_RUN, &wdt_status);
  539 + return rc;
  540 + }
  541 + }
511 542 return 0;
512 543  
513 544 default:
514 545  
515 546  
... ... @@ -560,16 +591,17 @@
560 591 int rc = 0;
561 592 int try_gameport = !nogameport;
562 593 u8 chip_rev;
563   - unsigned long flags;
  594 + int gp_rreq_fail = 0;
564 595  
565 596 wdt_status = 0;
566 597  
567   - spin_lock_irqsave(&spinlock, flags);
568   - superio_enter();
  598 + rc = superio_enter();
  599 + if (rc)
  600 + return rc;
  601 +
569 602 chip_type = superio_inw(CHIPID);
570 603 chip_rev = superio_inb(CHIPREV) & 0x0f;
571 604 superio_exit();
572   - spin_unlock_irqrestore(&spinlock, flags);
573 605  
574 606 switch (chip_type) {
575 607 case IT8702_ID:
... ... @@ -603,8 +635,9 @@
603 635 return -ENODEV;
604 636 }
605 637  
606   - spin_lock_irqsave(&spinlock, flags);
607   - superio_enter();
  638 + rc = superio_enter();
  639 + if (rc)
  640 + return rc;
608 641  
609 642 superio_select(GPIO);
610 643 superio_outb(WDT_TOV1, WDTCFG);
611 644  
612 645  
... ... @@ -620,21 +653,16 @@
620 653 }
621 654 gpact = superio_inb(ACTREG);
622 655 superio_outb(0x01, ACTREG);
623   - superio_exit();
624   - spin_unlock_irqrestore(&spinlock, flags);
625 656 if (request_region(base, 1, WATCHDOG_NAME))
626 657 set_bit(WDTS_USE_GP, &wdt_status);
627 658 else
628   - rc = -EIO;
629   - } else {
630   - superio_exit();
631   - spin_unlock_irqrestore(&spinlock, flags);
  659 + gp_rreq_fail = 1;
632 660 }
633 661  
634 662 /* If we haven't Gameport support, try to get CIR support */
635 663 if (!test_bit(WDTS_USE_GP, &wdt_status)) {
636 664 if (!request_region(CIR_BASE, 8, WATCHDOG_NAME)) {
637   - if (rc == -EIO)
  665 + if (gp_rreq_fail)
638 666 printk(KERN_ERR PFX
639 667 "I/O Address 0x%04x and 0x%04x"
640 668 " already in use\n", base, CIR_BASE);
641 669  
642 670  
... ... @@ -646,21 +674,16 @@
646 674 goto err_out;
647 675 }
648 676 base = CIR_BASE;
649   - spin_lock_irqsave(&spinlock, flags);
650   - superio_enter();
651 677  
652 678 superio_select(CIR);
653 679 superio_outw(base, BASEREG);
654 680 superio_outb(0x00, CIR_ILS);
655 681 ciract = superio_inb(ACTREG);
656 682 superio_outb(0x01, ACTREG);
657   - if (rc == -EIO) {
  683 + if (gp_rreq_fail) {
658 684 superio_select(GAMEPORT);
659 685 superio_outb(gpact, ACTREG);
660 686 }
661   -
662   - superio_exit();
663   - spin_unlock_irqrestore(&spinlock, flags);
664 687 }
665 688  
666 689 if (timeout < 1 || timeout > max_units * 60) {
... ... @@ -704,6 +727,7 @@
704 727 "nogameport=%d)\n", chip_type, chip_rev, timeout,
705 728 nowayout, testmode, exclusive, nogameport);
706 729  
  730 + superio_exit();
707 731 return 0;
708 732  
709 733 err_out_reboot:
710 734  
711 735  
712 736  
713 737  
714 738  
715 739  
... ... @@ -711,49 +735,37 @@
711 735 err_out_region:
712 736 release_region(base, test_bit(WDTS_USE_GP, &wdt_status) ? 1 : 8);
713 737 if (!test_bit(WDTS_USE_GP, &wdt_status)) {
714   - spin_lock_irqsave(&spinlock, flags);
715   - superio_enter();
716 738 superio_select(CIR);
717 739 superio_outb(ciract, ACTREG);
718   - superio_exit();
719   - spin_unlock_irqrestore(&spinlock, flags);
720 740 }
721 741 err_out:
722 742 if (try_gameport) {
723   - spin_lock_irqsave(&spinlock, flags);
724   - superio_enter();
725 743 superio_select(GAMEPORT);
726 744 superio_outb(gpact, ACTREG);
727   - superio_exit();
728   - spin_unlock_irqrestore(&spinlock, flags);
729 745 }
730 746  
  747 + superio_exit();
731 748 return rc;
732 749 }
733 750  
734 751 static void __exit it87_wdt_exit(void)
735 752 {
736   - unsigned long flags;
737   - int nolock;
738   -
739   - nolock = !spin_trylock_irqsave(&spinlock, flags);
740   - superio_enter();
741   - superio_select(GPIO);
742   - superio_outb(0x00, WDTCTRL);
743   - superio_outb(0x00, WDTCFG);
744   - superio_outb(0x00, WDTVALLSB);
745   - if (max_units > 255)
746   - superio_outb(0x00, WDTVALMSB);
747   - if (test_bit(WDTS_USE_GP, &wdt_status)) {
748   - superio_select(GAMEPORT);
749   - superio_outb(gpact, ACTREG);
750   - } else {
751   - superio_select(CIR);
752   - superio_outb(ciract, ACTREG);
  753 + if (superio_enter() == 0) {
  754 + superio_select(GPIO);
  755 + superio_outb(0x00, WDTCTRL);
  756 + superio_outb(0x00, WDTCFG);
  757 + superio_outb(0x00, WDTVALLSB);
  758 + if (max_units > 255)
  759 + superio_outb(0x00, WDTVALMSB);
  760 + if (test_bit(WDTS_USE_GP, &wdt_status)) {
  761 + superio_select(GAMEPORT);
  762 + superio_outb(gpact, ACTREG);
  763 + } else {
  764 + superio_select(CIR);
  765 + superio_outb(ciract, ACTREG);
  766 + }
  767 + superio_exit();
753 768 }
754   - superio_exit();
755   - if (!nolock)
756   - spin_unlock_irqrestore(&spinlock, flags);
757 769  
758 770 misc_deregister(&wdt_miscdev);
759 771 unregister_reboot_notifier(&wdt_notifier);