Wolfram Sang | 74cd4c6 | 2011-09-26 15:40:13 +0200 | [diff] [blame] | 1 | Converting old watchdog drivers to the watchdog framework |
| 2 | by Wolfram Sang <w.sang@pengutronix.de> |
| 3 | ========================================================= |
| 4 | |
| 5 | Before the watchdog framework came into the kernel, every driver had to |
| 6 | implement the API on its own. Now, as the framework factored out the common |
| 7 | components, those drivers can be lightened making it a user of the framework. |
| 8 | This document shall guide you for this task. The necessary steps are described |
| 9 | as well as things to look out for. |
| 10 | |
| 11 | |
| 12 | Remove the file_operations struct |
| 13 | --------------------------------- |
| 14 | |
| 15 | Old drivers define their own file_operations for actions like open(), write(), |
| 16 | etc... These are now handled by the framework and just call the driver when |
| 17 | needed. So, in general, the 'file_operations' struct and assorted functions can |
| 18 | go. Only very few driver-specific details have to be moved to other functions. |
| 19 | Here is a overview of the functions and probably needed actions: |
| 20 | |
| 21 | - open: Everything dealing with resource management (file-open checks, magic |
| 22 | close preparations) can simply go. Device specific stuff needs to go to the |
| 23 | driver specific start-function. Note that for some drivers, the start-function |
| 24 | also serves as the ping-function. If that is the case and you need start/stop |
| 25 | to be balanced (clocks!), you are better off refactoring a separate start-function. |
| 26 | |
| 27 | - close: Same hints as for open apply. |
| 28 | |
| 29 | - write: Can simply go, all defined behaviour is taken care of by the framework, |
| 30 | i.e. ping on write and magic char ('V') handling. |
| 31 | |
| 32 | - ioctl: While the driver is allowed to have extensions to the IOCTL interface, |
| 33 | the most common ones are handled by the framework, supported by some assistance |
| 34 | from the driver: |
| 35 | |
| 36 | WDIOC_GETSUPPORT: |
| 37 | Returns the mandatory watchdog_info struct from the driver |
| 38 | |
| 39 | WDIOC_GETSTATUS: |
| 40 | Needs the status-callback defined, otherwise returns 0 |
| 41 | |
| 42 | WDIOC_GETBOOTSTATUS: |
| 43 | Needs the bootstatus member properly set. Make sure it is 0 if you |
| 44 | don't have further support! |
| 45 | |
| 46 | WDIOC_SETOPTIONS: |
| 47 | No preparations needed |
| 48 | |
| 49 | WDIOC_KEEPALIVE: |
| 50 | If wanted, options in watchdog_info need to have WDIOF_KEEPALIVEPING |
| 51 | set |
| 52 | |
| 53 | WDIOC_SETTIMEOUT: |
| 54 | Options in watchdog_info need to have WDIOF_SETTIMEOUT set |
| 55 | and a set_timeout-callback has to be defined. The core will also |
| 56 | do limit-checking, if min_timeout and max_timeout in the watchdog |
| 57 | device are set. All is optional. |
| 58 | |
| 59 | WDIOC_GETTIMEOUT: |
| 60 | No preparations needed |
| 61 | |
| 62 | Other IOCTLs can be served using the ioctl-callback. Note that this is mainly |
| 63 | intended for porting old drivers; new drivers should not invent private IOCTLs. |
| 64 | Private IOCTLs are processed first. When the callback returns with |
| 65 | -ENOIOCTLCMD, the IOCTLs of the framework will be tried, too. Any other error |
| 66 | is directly given to the user. |
| 67 | |
| 68 | Example conversion: |
| 69 | |
| 70 | -static const struct file_operations s3c2410wdt_fops = { |
| 71 | - .owner = THIS_MODULE, |
| 72 | - .llseek = no_llseek, |
| 73 | - .write = s3c2410wdt_write, |
| 74 | - .unlocked_ioctl = s3c2410wdt_ioctl, |
| 75 | - .open = s3c2410wdt_open, |
| 76 | - .release = s3c2410wdt_release, |
| 77 | -}; |
| 78 | |
| 79 | Check the functions for device-specific stuff and keep it for later |
| 80 | refactoring. The rest can go. |
| 81 | |
| 82 | |
| 83 | Remove the miscdevice |
| 84 | --------------------- |
| 85 | |
| 86 | Since the file_operations are gone now, you can also remove the 'struct |
| 87 | miscdevice'. The framework will create it on watchdog_dev_register() called by |
| 88 | watchdog_register_device(). |
| 89 | |
| 90 | -static struct miscdevice s3c2410wdt_miscdev = { |
| 91 | - .minor = WATCHDOG_MINOR, |
| 92 | - .name = "watchdog", |
| 93 | - .fops = &s3c2410wdt_fops, |
| 94 | -}; |
| 95 | |
| 96 | |
| 97 | Remove obsolete includes and defines |
| 98 | ------------------------------------ |
| 99 | |
| 100 | Because of the simplifications, a few defines are probably unused now. Remove |
| 101 | them. Includes can be removed, too. For example: |
| 102 | |
| 103 | - #include <linux/fs.h> |
| 104 | - #include <linux/miscdevice.h> (if MODULE_ALIAS_MISCDEV is not used) |
| 105 | - #include <linux/uaccess.h> (if no custom IOCTLs are used) |
| 106 | |
| 107 | |
| 108 | Add the watchdog operations |
| 109 | --------------------------- |
| 110 | |
| 111 | All possible callbacks are defined in 'struct watchdog_ops'. You can find it |
| 112 | explained in 'watchdog-kernel-api.txt' in this directory. start(), stop() and |
| 113 | owner must be set, the rest are optional. You will easily find corresponding |
| 114 | functions in the old driver. Note that you will now get a pointer to the |
| 115 | watchdog_device as a parameter to these functions, so you probably have to |
| 116 | change the function header. Other changes are most likely not needed, because |
| 117 | here simply happens the direct hardware access. If you have device-specific |
| 118 | code left from the above steps, it should be refactored into these callbacks. |
| 119 | |
| 120 | Here is a simple example: |
| 121 | |
| 122 | +static struct watchdog_ops s3c2410wdt_ops = { |
| 123 | + .owner = THIS_MODULE, |
| 124 | + .start = s3c2410wdt_start, |
| 125 | + .stop = s3c2410wdt_stop, |
| 126 | + .ping = s3c2410wdt_keepalive, |
| 127 | + .set_timeout = s3c2410wdt_set_heartbeat, |
| 128 | +}; |
| 129 | |
| 130 | A typical function-header change looks like: |
| 131 | |
| 132 | -static void s3c2410wdt_keepalive(void) |
| 133 | +static int s3c2410wdt_keepalive(struct watchdog_device *wdd) |
| 134 | { |
| 135 | ... |
| 136 | + |
| 137 | + return 0; |
| 138 | } |
| 139 | |
| 140 | ... |
| 141 | |
| 142 | - s3c2410wdt_keepalive(); |
| 143 | + s3c2410wdt_keepalive(&s3c2410_wdd); |
| 144 | |
| 145 | |
| 146 | Add the watchdog device |
| 147 | ----------------------- |
| 148 | |
| 149 | Now we need to create a 'struct watchdog_device' and populate it with the |
| 150 | necessary information for the framework. The struct is also explained in detail |
| 151 | in 'watchdog-kernel-api.txt' in this directory. We pass it the mandatory |
| 152 | watchdog_info struct and the newly created watchdog_ops. Often, old drivers |
| 153 | have their own record-keeping for things like bootstatus and timeout using |
| 154 | static variables. Those have to be converted to use the members in |
| 155 | watchdog_device. Note that the timeout values are unsigned int. Some drivers |
| 156 | use signed int, so this has to be converted, too. |
| 157 | |
| 158 | Here is a simple example for a watchdog device: |
| 159 | |
| 160 | +static struct watchdog_device s3c2410_wdd = { |
| 161 | + .info = &s3c2410_wdt_ident, |
| 162 | + .ops = &s3c2410wdt_ops, |
| 163 | +}; |
| 164 | |
| 165 | |
Wolfram Sang | 02861cc | 2011-12-02 00:43:11 +0100 | [diff] [blame] | 166 | Handle the 'nowayout' feature |
| 167 | ----------------------------- |
| 168 | |
| 169 | A few drivers use nowayout statically, i.e. there is no module parameter for it |
| 170 | and only CONFIG_WATCHDOG_NOWAYOUT determines if the feature is going to be |
| 171 | used. This needs to be converted by initializing the status variable of the |
| 172 | watchdog_device like this: |
| 173 | |
| 174 | .status = WATCHDOG_NOWAYOUT_INIT_STATUS, |
| 175 | |
| 176 | Most drivers, however, also allow runtime configuration of nowayout, usually |
| 177 | by adding a module parameter. The conversion for this would be something like: |
| 178 | |
| 179 | watchdog_set_nowayout(&s3c2410_wdd, nowayout); |
| 180 | |
| 181 | The module parameter itself needs to stay, everything else related to nowayout |
| 182 | can go, though. This will likely be some code in open(), close() or write(). |
| 183 | |
| 184 | |
Wolfram Sang | 74cd4c6 | 2011-09-26 15:40:13 +0200 | [diff] [blame] | 185 | Register the watchdog device |
| 186 | ---------------------------- |
| 187 | |
| 188 | Replace misc_register(&miscdev) with watchdog_register_device(&watchdog_dev). |
| 189 | Make sure the return value gets checked and the error message, if present, |
| 190 | still fits. Also convert the unregister case. |
| 191 | |
| 192 | - ret = misc_register(&s3c2410wdt_miscdev); |
| 193 | + ret = watchdog_register_device(&s3c2410_wdd); |
| 194 | |
| 195 | ... |
| 196 | |
| 197 | - misc_deregister(&s3c2410wdt_miscdev); |
| 198 | + watchdog_unregister_device(&s3c2410_wdd); |
| 199 | |
| 200 | |
| 201 | Update the Kconfig-entry |
| 202 | ------------------------ |
| 203 | |
| 204 | The entry for the driver now needs to select WATCHDOG_CORE: |
| 205 | |
| 206 | + select WATCHDOG_CORE |
| 207 | |
| 208 | |
| 209 | Create a patch and send it to upstream |
| 210 | -------------------------------------- |
| 211 | |
| 212 | Make sure you understood Documentation/SubmittingPatches and send your patch to |
| 213 | linux-watchdog@vger.kernel.org. We are looking forward to it :) |
| 214 | |