Re: [PATCH v7 2/3] trivial: PM / Hibernate: clean up checkpatch in hibernate.c

On Tuesday, February 04, 2014 02:37:33 PM Sebastian Capella wrote:

Quoting Rafael J. Wysocki (2014-02-04 13:36:29)

quoted

Well, this isn't a trivial patch.

I'll remove the trivial, thanks!

Quoting Rafael J. Wysocki (2014-02-04 13:36:29)

quoted

On Tuesday, February 04, 2014 12:43:50 PM Sebastian Capella wrote:

quoted

+     while (1)
+             ;

Please remove this change from the patch.  I don't care about checkpatch
complaining here.

quoted

+     while (1)
+             ;

Same here.

Will do, thanks!

quoted

quoted

@@ -765,7 +762,7 @@ static int software_resume(void)
      if (isdigit(resume_file[0]) && resume_wait) {
              int partno;
              while (!get_gendisk(swsusp_resume_device, &partno))
-                     msleep(10);
+                     msleep(20);

That's the reason why it is not trivial.

First, the change being made doesn't belong in this patch.

Thanks I'll separate it if it remains.

quoted

Second, what's the problem with the original value?

The warning from checkpatch implies that it's misleading to
msleep < 20ms since msleep is using msec_to_jiffies + 1 for
the duration.  In any case, this is polling for devices discovery to
complete.  It is used when resumewait is specified on the command
line telling hibernate to wait for the resume device to appear.

What checkpatch is saying is about *new* code, not the existing one.

You need to have a *reason* to change the way the existing code works
and the above explanation doesn't sound like a good one to me in this
particular case.

quoted

quoted

-static ssize_t image_size_show(struct kobject *kobj, struct kobj_attribute *attr,
+static ssize_t image_size_show(struct kobject *kobj,
+                            struct kobj_attribute *attr,

Why can't you leave the code as is here?

quoted

-static ssize_t image_size_store(struct kobject *kobj, struct kobj_attribute *attr,
+static ssize_t image_size_store(struct kobject *kobj,
+                             struct kobj_attribute *attr,

And here?

Purely long line cleanup. (>80 colunms)

Please don't do any >80 columns cleanups in any patches you want me to apply.
Seriously.  This is irritating and unuseful.

And if you don't want checkpatch to complain about that, please send a patch
to modify checkpatch accordingly.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.