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

[Sebastian Capella <sebastian.capella@linaro.org>]

Quoting Rafael J. Wysocki (2014-02-04 13:36:29)
> Well, this isn't a trivial patch.

I'll remove the trivial, thanks!

Quoting Rafael J. Wysocki (2014-02-04 13:36:29)
> On Tuesday, February 04, 2014 12:43:50 PM Sebastian Capella wrote:
> > +     while (1)
> > +             ;
> Please remove this change from the patch.  I don't care about checkpatch
> complaining here.
> > +     while (1)
> > +             ;
> Same here.

Will do, thanks!

> > @@ -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.

> 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.

> > -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?
> > -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)

> >  static int __init resumedelay_setup(char *str)
> >  {
> > -     resume_delay = simple_strtoul(str, NULL, 0);
> > +     int ret = kstrtoint(str, 0, &resume_delay);
> > +     /* mask must_check warn; on failure, leaves resume_delay unchanged */
> > +     (void)ret;
> 
> And that's not a trivial change surely?

I'll include this and the msleep as a separate, non-trivial checkpatch
cleanup patch if the changes remain after this discussion.

> 
> And why didn't you do (void)kstrtoint(str, 0, &resume_delay); instead?

Better thanks!

Sebastian

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>