diff options
author | Alan Cox <alan@lxorguk.ukuu.org.uk> | 2008-08-04 12:54:46 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2008-08-04 20:12:07 -0400 |
commit | 41dc8b72e37c514f7332cbc3f3dd864910c2a1fa (patch) | |
tree | fbdb145a083f03a0e6c24a6fc527477b773c5099 /drivers/watchdog | |
parent | d6547378df1c11bc6790b87abedb3526ded40ef9 (diff) |
s3c2410_wdt watchdog driver: Locking and coding style
Kill off use of semaphores.
Fix ioctl races and locking holes.
From: Alan Cox <alan@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Alan Cox <alan@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'drivers/watchdog')
-rw-r--r-- | drivers/watchdog/s3c2410_wdt.c | 148 |
1 files changed, 80 insertions, 68 deletions
diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c index 98532c0e0689..97b4a2e8eb09 100644 --- a/drivers/watchdog/s3c2410_wdt.c +++ b/drivers/watchdog/s3c2410_wdt.c | |||
@@ -46,9 +46,8 @@ | |||
46 | #include <linux/platform_device.h> | 46 | #include <linux/platform_device.h> |
47 | #include <linux/interrupt.h> | 47 | #include <linux/interrupt.h> |
48 | #include <linux/clk.h> | 48 | #include <linux/clk.h> |
49 | 49 | #include <linux/uaccess.h> | |
50 | #include <asm/uaccess.h> | 50 | #include <linux/io.h> |
51 | #include <asm/io.h> | ||
52 | 51 | ||
53 | #include <asm/arch/map.h> | 52 | #include <asm/arch/map.h> |
54 | 53 | ||
@@ -65,8 +64,8 @@ | |||
65 | static int nowayout = WATCHDOG_NOWAYOUT; | 64 | static int nowayout = WATCHDOG_NOWAYOUT; |
66 | static int tmr_margin = CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME; | 65 | static int tmr_margin = CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME; |
67 | static int tmr_atboot = CONFIG_S3C2410_WATCHDOG_ATBOOT; | 66 | static int tmr_atboot = CONFIG_S3C2410_WATCHDOG_ATBOOT; |
68 | static int soft_noboot = 0; | 67 | static int soft_noboot; |
69 | static int debug = 0; | 68 | static int debug; |
70 | 69 | ||
71 | module_param(tmr_margin, int, 0); | 70 | module_param(tmr_margin, int, 0); |
72 | module_param(tmr_atboot, int, 0); | 71 | module_param(tmr_atboot, int, 0); |
@@ -74,24 +73,23 @@ module_param(nowayout, int, 0); | |||
74 | module_param(soft_noboot, int, 0); | 73 | module_param(soft_noboot, int, 0); |
75 | module_param(debug, int, 0); | 74 | module_param(debug, int, 0); |
76 | 75 | ||
77 | MODULE_PARM_DESC(tmr_margin, "Watchdog tmr_margin in seconds. default=" __MODULE_STRING(CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME) ")"); | 76 | MODULE_PARM_DESC(tmr_margin, "Watchdog tmr_margin in seconds. default=" |
78 | 77 | __MODULE_STRING(CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME) ")"); | |
79 | MODULE_PARM_DESC(tmr_atboot, "Watchdog is started at boot time if set to 1, default=" __MODULE_STRING(CONFIG_S3C2410_WATCHDOG_ATBOOT)); | 78 | MODULE_PARM_DESC(tmr_atboot, |
80 | 79 | "Watchdog is started at boot time if set to 1, default=" | |
81 | MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); | 80 | __MODULE_STRING(CONFIG_S3C2410_WATCHDOG_ATBOOT)); |
82 | 81 | MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" | |
82 | __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); | ||
83 | MODULE_PARM_DESC(soft_noboot, "Watchdog action, set to 1 to ignore reboots, 0 to reboot (default depends on ONLY_TESTING)"); | 83 | MODULE_PARM_DESC(soft_noboot, "Watchdog action, set to 1 to ignore reboots, 0 to reboot (default depends on ONLY_TESTING)"); |
84 | |||
85 | MODULE_PARM_DESC(debug, "Watchdog debug, set to >1 for debug, (default 0)"); | 84 | MODULE_PARM_DESC(debug, "Watchdog debug, set to >1 for debug, (default 0)"); |
86 | 85 | ||
87 | 86 | ||
88 | typedef enum close_state { | 87 | typedef enum close_state { |
89 | CLOSE_STATE_NOT, | 88 | CLOSE_STATE_NOT, |
90 | CLOSE_STATE_ALLOW=0x4021 | 89 | CLOSE_STATE_ALLOW = 0x4021 |
91 | } close_state_t; | 90 | } close_state_t; |
92 | 91 | ||
93 | static DECLARE_MUTEX(open_lock); | 92 | static unsigned long open_lock; |
94 | |||
95 | static struct device *wdt_dev; /* platform device attached to */ | 93 | static struct device *wdt_dev; /* platform device attached to */ |
96 | static struct resource *wdt_mem; | 94 | static struct resource *wdt_mem; |
97 | static struct resource *wdt_irq; | 95 | static struct resource *wdt_irq; |
@@ -99,38 +97,58 @@ static struct clk *wdt_clock; | |||
99 | static void __iomem *wdt_base; | 97 | static void __iomem *wdt_base; |
100 | static unsigned int wdt_count; | 98 | static unsigned int wdt_count; |
101 | static close_state_t allow_close; | 99 | static close_state_t allow_close; |
100 | static DEFINE_SPINLOCK(wdt_lock); | ||
102 | 101 | ||
103 | /* watchdog control routines */ | 102 | /* watchdog control routines */ |
104 | 103 | ||
105 | #define DBG(msg...) do { \ | 104 | #define DBG(msg...) do { \ |
106 | if (debug) \ | 105 | if (debug) \ |
107 | printk(KERN_INFO msg); \ | 106 | printk(KERN_INFO msg); \ |
108 | } while(0) | 107 | } while (0) |
109 | 108 | ||
110 | /* functions */ | 109 | /* functions */ |
111 | 110 | ||
112 | static int s3c2410wdt_keepalive(void) | 111 | static void s3c2410wdt_keepalive(void) |
113 | { | 112 | { |
113 | spin_lock(&wdt_lock); | ||
114 | writel(wdt_count, wdt_base + S3C2410_WTCNT); | 114 | writel(wdt_count, wdt_base + S3C2410_WTCNT); |
115 | return 0; | 115 | spin_unlock(&wdt_lock); |
116 | } | 116 | } |
117 | 117 | ||
118 | static int s3c2410wdt_stop(void) | 118 | static void __s3c2410wdt_stop(void) |
119 | { | 119 | { |
120 | unsigned long wtcon; | 120 | unsigned long wtcon; |
121 | 121 | ||
122 | spin_lock(&wdt_lock); | ||
122 | wtcon = readl(wdt_base + S3C2410_WTCON); | 123 | wtcon = readl(wdt_base + S3C2410_WTCON); |
123 | wtcon &= ~(S3C2410_WTCON_ENABLE | S3C2410_WTCON_RSTEN); | 124 | wtcon &= ~(S3C2410_WTCON_ENABLE | S3C2410_WTCON_RSTEN); |
124 | writel(wtcon, wdt_base + S3C2410_WTCON); | 125 | writel(wtcon, wdt_base + S3C2410_WTCON); |
126 | spin_unlock(&wdt_lock); | ||
127 | } | ||
125 | 128 | ||
126 | return 0; | 129 | static void __s3c2410wdt_stop(void) |
130 | { | ||
131 | unsigned long wtcon; | ||
132 | |||
133 | wtcon = readl(wdt_base + S3C2410_WTCON); | ||
134 | wtcon &= ~(S3C2410_WTCON_ENABLE | S3C2410_WTCON_RSTEN); | ||
135 | writel(wtcon, wdt_base + S3C2410_WTCON); | ||
136 | } | ||
137 | |||
138 | static void s3c2410wdt_stop(void) | ||
139 | { | ||
140 | spin_lock(&wdt_lock); | ||
141 | __s3c2410wdt_stop(); | ||
142 | spin_unlock(&wdt_lock); | ||
127 | } | 143 | } |
128 | 144 | ||
129 | static int s3c2410wdt_start(void) | 145 | static void s3c2410wdt_start(void) |
130 | { | 146 | { |
131 | unsigned long wtcon; | 147 | unsigned long wtcon; |
132 | 148 | ||
133 | s3c2410wdt_stop(); | 149 | spin_lock(&wdt_lock); |
150 | |||
151 | __s3c2410wdt_stop(); | ||
134 | 152 | ||
135 | wtcon = readl(wdt_base + S3C2410_WTCON); | 153 | wtcon = readl(wdt_base + S3C2410_WTCON); |
136 | wtcon |= S3C2410_WTCON_ENABLE | S3C2410_WTCON_DIV128; | 154 | wtcon |= S3C2410_WTCON_ENABLE | S3C2410_WTCON_DIV128; |
@@ -149,6 +167,7 @@ static int s3c2410wdt_start(void) | |||
149 | writel(wdt_count, wdt_base + S3C2410_WTDAT); | 167 | writel(wdt_count, wdt_base + S3C2410_WTDAT); |
150 | writel(wdt_count, wdt_base + S3C2410_WTCNT); | 168 | writel(wdt_count, wdt_base + S3C2410_WTCNT); |
151 | writel(wtcon, wdt_base + S3C2410_WTCON); | 169 | writel(wtcon, wdt_base + S3C2410_WTCON); |
170 | spin_unlock(&wdt_lock); | ||
152 | 171 | ||
153 | return 0; | 172 | return 0; |
154 | } | 173 | } |
@@ -211,7 +230,7 @@ static int s3c2410wdt_set_heartbeat(int timeout) | |||
211 | 230 | ||
212 | static int s3c2410wdt_open(struct inode *inode, struct file *file) | 231 | static int s3c2410wdt_open(struct inode *inode, struct file *file) |
213 | { | 232 | { |
214 | if(down_trylock(&open_lock)) | 233 | if (test_and_set_bit(0, &open_lock)) |
215 | return -EBUSY; | 234 | return -EBUSY; |
216 | 235 | ||
217 | if (nowayout) | 236 | if (nowayout) |
@@ -231,15 +250,14 @@ static int s3c2410wdt_release(struct inode *inode, struct file *file) | |||
231 | * Lock it in if it's a module and we set nowayout | 250 | * Lock it in if it's a module and we set nowayout |
232 | */ | 251 | */ |
233 | 252 | ||
234 | if (allow_close == CLOSE_STATE_ALLOW) { | 253 | if (allow_close == CLOSE_STATE_ALLOW) |
235 | s3c2410wdt_stop(); | 254 | s3c2410wdt_stop(); |
236 | } else { | 255 | else { |
237 | dev_err(wdt_dev, "Unexpected close, not stopping watchdog\n"); | 256 | dev_err(wdt_dev, "Unexpected close, not stopping watchdog\n"); |
238 | s3c2410wdt_keepalive(); | 257 | s3c2410wdt_keepalive(); |
239 | } | 258 | } |
240 | |||
241 | allow_close = CLOSE_STATE_NOT; | 259 | allow_close = CLOSE_STATE_NOT; |
242 | up(&open_lock); | 260 | clear_bit(0, &open_lock); |
243 | return 0; | 261 | return 0; |
244 | } | 262 | } |
245 | 263 | ||
@@ -249,7 +267,7 @@ static ssize_t s3c2410wdt_write(struct file *file, const char __user *data, | |||
249 | /* | 267 | /* |
250 | * Refresh the timer. | 268 | * Refresh the timer. |
251 | */ | 269 | */ |
252 | if(len) { | 270 | if (len) { |
253 | if (!nowayout) { | 271 | if (!nowayout) { |
254 | size_t i; | 272 | size_t i; |
255 | 273 | ||
@@ -265,7 +283,6 @@ static ssize_t s3c2410wdt_write(struct file *file, const char __user *data, | |||
265 | allow_close = CLOSE_STATE_ALLOW; | 283 | allow_close = CLOSE_STATE_ALLOW; |
266 | } | 284 | } |
267 | } | 285 | } |
268 | |||
269 | s3c2410wdt_keepalive(); | 286 | s3c2410wdt_keepalive(); |
270 | } | 287 | } |
271 | return len; | 288 | return len; |
@@ -273,48 +290,41 @@ static ssize_t s3c2410wdt_write(struct file *file, const char __user *data, | |||
273 | 290 | ||
274 | #define OPTIONS WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE | 291 | #define OPTIONS WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE |
275 | 292 | ||
276 | static struct watchdog_info s3c2410_wdt_ident = { | 293 | static const struct watchdog_info s3c2410_wdt_ident = { |
277 | .options = OPTIONS, | 294 | .options = OPTIONS, |
278 | .firmware_version = 0, | 295 | .firmware_version = 0, |
279 | .identity = "S3C2410 Watchdog", | 296 | .identity = "S3C2410 Watchdog", |
280 | }; | 297 | }; |
281 | 298 | ||
282 | 299 | ||
283 | static int s3c2410wdt_ioctl(struct inode *inode, struct file *file, | 300 | static long s3c2410wdt_ioctl(struct file *file, unsigned int cmd, |
284 | unsigned int cmd, unsigned long arg) | 301 | unsigned long arg) |
285 | { | 302 | { |
286 | void __user *argp = (void __user *)arg; | 303 | void __user *argp = (void __user *)arg; |
287 | int __user *p = argp; | 304 | int __user *p = argp; |
288 | int new_margin; | 305 | int new_margin; |
289 | 306 | ||
290 | switch (cmd) { | 307 | switch (cmd) { |
291 | default: | 308 | default: |
292 | return -ENOTTY; | 309 | return -ENOTTY; |
293 | 310 | case WDIOC_GETSUPPORT: | |
294 | case WDIOC_GETSUPPORT: | 311 | return copy_to_user(argp, &s3c2410_wdt_ident, |
295 | return copy_to_user(argp, &s3c2410_wdt_ident, | 312 | sizeof(s3c2410_wdt_ident)) ? -EFAULT : 0; |
296 | sizeof(s3c2410_wdt_ident)) ? -EFAULT : 0; | 313 | case WDIOC_GETSTATUS: |
297 | 314 | case WDIOC_GETBOOTSTATUS: | |
298 | case WDIOC_GETSTATUS: | 315 | return put_user(0, p); |
299 | case WDIOC_GETBOOTSTATUS: | 316 | case WDIOC_KEEPALIVE: |
300 | return put_user(0, p); | 317 | s3c2410wdt_keepalive(); |
301 | 318 | return 0; | |
302 | case WDIOC_KEEPALIVE: | 319 | case WDIOC_SETTIMEOUT: |
303 | s3c2410wdt_keepalive(); | 320 | if (get_user(new_margin, p)) |
304 | return 0; | 321 | return -EFAULT; |
305 | 322 | if (s3c2410wdt_set_heartbeat(new_margin)) | |
306 | case WDIOC_SETTIMEOUT: | 323 | return -EINVAL; |
307 | if (get_user(new_margin, p)) | 324 | s3c2410wdt_keepalive(); |
308 | return -EFAULT; | 325 | return put_user(tmr_margin, p); |
309 | 326 | case WDIOC_GETTIMEOUT: | |
310 | if (s3c2410wdt_set_heartbeat(new_margin)) | 327 | return put_user(tmr_margin, p); |
311 | return -EINVAL; | ||
312 | |||
313 | s3c2410wdt_keepalive(); | ||
314 | return put_user(tmr_margin, p); | ||
315 | |||
316 | case WDIOC_GETTIMEOUT: | ||
317 | return put_user(tmr_margin, p); | ||
318 | } | 328 | } |
319 | } | 329 | } |
320 | 330 | ||
@@ -324,7 +334,7 @@ static const struct file_operations s3c2410wdt_fops = { | |||
324 | .owner = THIS_MODULE, | 334 | .owner = THIS_MODULE, |
325 | .llseek = no_llseek, | 335 | .llseek = no_llseek, |
326 | .write = s3c2410wdt_write, | 336 | .write = s3c2410wdt_write, |
327 | .ioctl = s3c2410wdt_ioctl, | 337 | .unlocked_ioctl = s3c2410wdt_ioctl, |
328 | .open = s3c2410wdt_open, | 338 | .open = s3c2410wdt_open, |
329 | .release = s3c2410wdt_release, | 339 | .release = s3c2410wdt_release, |
330 | }; | 340 | }; |
@@ -411,14 +421,15 @@ static int s3c2410wdt_probe(struct platform_device *pdev) | |||
411 | * not, try the default value */ | 421 | * not, try the default value */ |
412 | 422 | ||
413 | if (s3c2410wdt_set_heartbeat(tmr_margin)) { | 423 | if (s3c2410wdt_set_heartbeat(tmr_margin)) { |
414 | started = s3c2410wdt_set_heartbeat(CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME); | 424 | started = s3c2410wdt_set_heartbeat( |
425 | CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME); | ||
415 | 426 | ||
416 | if (started == 0) { | 427 | if (started == 0) |
417 | dev_info(dev,"tmr_margin value out of range, default %d used\n", | 428 | dev_info(dev, |
429 | "tmr_margin value out of range, default %d used\n", | ||
418 | CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME); | 430 | CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME); |
419 | } else { | 431 | else |
420 | dev_info(dev, "default timer value is out of range, cannot start\n"); | 432 | dev_info(dev, "default timer value is out of range, cannot start\n"); |
421 | } | ||
422 | } | 433 | } |
423 | 434 | ||
424 | ret = misc_register(&s3c2410wdt_miscdev); | 435 | ret = misc_register(&s3c2410wdt_miscdev); |
@@ -447,7 +458,7 @@ static int s3c2410wdt_probe(struct platform_device *pdev) | |||
447 | (wtcon & S3C2410_WTCON_ENABLE) ? "" : "in", | 458 | (wtcon & S3C2410_WTCON_ENABLE) ? "" : "in", |
448 | (wtcon & S3C2410_WTCON_RSTEN) ? "" : "dis", | 459 | (wtcon & S3C2410_WTCON_RSTEN) ? "" : "dis", |
449 | (wtcon & S3C2410_WTCON_INTEN) ? "" : "en"); | 460 | (wtcon & S3C2410_WTCON_INTEN) ? "" : "en"); |
450 | 461 | ||
451 | return 0; | 462 | return 0; |
452 | 463 | ||
453 | err_clk: | 464 | err_clk: |
@@ -487,7 +498,7 @@ static int s3c2410wdt_remove(struct platform_device *dev) | |||
487 | 498 | ||
488 | static void s3c2410wdt_shutdown(struct platform_device *dev) | 499 | static void s3c2410wdt_shutdown(struct platform_device *dev) |
489 | { | 500 | { |
490 | s3c2410wdt_stop(); | 501 | s3c2410wdt_stop(); |
491 | } | 502 | } |
492 | 503 | ||
493 | #ifdef CONFIG_PM | 504 | #ifdef CONFIG_PM |
@@ -540,7 +551,8 @@ static struct platform_driver s3c2410wdt_driver = { | |||
540 | }; | 551 | }; |
541 | 552 | ||
542 | 553 | ||
543 | static char banner[] __initdata = KERN_INFO "S3C2410 Watchdog Timer, (c) 2004 Simtec Electronics\n"; | 554 | static char banner[] __initdata = |
555 | KERN_INFO "S3C2410 Watchdog Timer, (c) 2004 Simtec Electronics\n"; | ||
544 | 556 | ||
545 | static int __init watchdog_init(void) | 557 | static int __init watchdog_init(void) |
546 | { | 558 | { |