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