On 5/10/22 08:48, Scott Cheloha wrote:
On May 9, 2022, at 9:35 PM, Tzung-Bi Shih <tzungbi@xxxxxxxxxx> wrote:
On Mon, May 09, 2022 at 12:43:57PM -0500, Scott Cheloha wrote:
+#define SETFIELD(_v, _b, _e) \
+ (((unsigned long)(_v) << PPC_BITLSHIFT(_e)) & PPC_BITMASK((_b), (_e)))
+#define GETFIELD(_v, _b, _e) \
+ (((unsigned long)(_v) & PPC_BITMASK((_b), (_e))) >> PPC_BITLSHIFT(_e))
From `./scripts/checkpatch.pl --strict`:
WARNING: please, no spaces at the start of a line
+#define PSERIES_WDTQL_MUST_STOP 1
From `./scripts/checkpatch.pl --strict`:
WARNING: please, no space before tabs
+static const struct kernel_param_ops action_ops = { .set = action_set };
+module_param_cb(action, &action_ops, NULL, S_IRUGO);
From `./scripts/checkpatch.pl --strict`:
WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using
octal permissions '0444'.
+MODULE_PARM_DESC(action, "Action taken when watchdog expires: \"hard-poweroff\", \"hard-restart\", or \"dump-restart\" (default=\"hard-restart\")");
The line exceeds 100 columns.
I was under the impression that strings were an exception to the
line-length rule. Is that not the case?
You can use
MODULE_PARM_DESC(action,
"Some text");
`scripts/checkpatch.pl --strict` complains if I break the string up:
WARNING: quoted string split across lines
#162: FILE: drivers/watchdog/pseries-wdt.c:162:
+MODULE_PARM_DESC(action, "Action taken when watchdog expires: "
+ "\"hard-poweroff\", \"hard-restart\", or \"dump-restart\" "
WARNING: quoted string split across lines
#163: FILE: drivers/watchdog/pseries-wdt.c:163:
+ "\"hard-poweroff\", \"hard-restart\", or \"dump-restart\" "
+ "(default=\"hard-restart\")");
+static bool nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, bool, S_IRUGO);
From `./scripts/checkpatch.pl --strict`:
WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using
octal permissions '0444'.
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
From `./scripts/checkpatch.pl --strict`, the line exceeds 100 columns.
+#define WATCHDOG_TIMEOUT 60
+static unsigned int timeout = WATCHDOG_TIMEOUT;
+module_param(timeout, uint, S_IRUGO);
From `./scripts/checkpatch.pl --strict`:
WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using
octal permissions '0444'.
+MODULE_PARM_DESC(timeout, "Initial watchdog timeout in seconds (default=" __MODULE_STRING(WATCHDOG_TIMEOUT) ")");
From `./scripts/checkpatch.pl --strict`, the line exceeds 100 columns.
+struct pseries_wdt {
+ struct watchdog_device wd;
+ unsigned long num; /* NB: Watchdog numbers are 1-based */
What does NB stand for? Could it be removed from the comment?
Sure, removed.
Does `timer_id` or some other equivalent names make more sense for the
variable?
The hardware documentation calls the parameter "watchdogNumber", so
I think "num" is better than "id" in this case.
