lkml.org 
[lkml]   [2014]   [Feb]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v7 2/3] trivial: PM / Hibernate: clean up checkpatch in hibernate.c
Date
On Tuesday, February 04, 2014 02:37:33 PM Sebastian Capella wrote:
> 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.

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.

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

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.


\
 
 \ /
  Last update: 2014-02-05 01:41    [W:0.123 / U:0.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site