Re: [RFC v2 2/2] watchdog: pseries-wdt: initial support for PAPR virtual watchdog timers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/10/22 09:15, Scott Cheloha wrote:


On May 10, 2022, at 11:04 AM, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:

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");

The line is still over 100 columns if I do this.


You might consider using shorter strings, such as "poweroff", "restart",
"dump". The meaning needs to be documented anyway. You could also drop
"or" because it doesn't add value in this context, and the '"' don't really
add value either.

On a side note, personally I don't like textual module parameters because
they make the command line extremely long and add complexity to decoding
it, but that is your call and responsibility (and you are not the only one
using strings).

Guenter

I can shrink the line by removing the valid inputs from the string if 100 columns
is a hard rule.

If so, where should I document the valid inputs instead?

Is Documentation/watchdog/watchdog-parameters.rst a better place for them?





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux