On Wed, Jan 03, 2018 at 06:40:12PM +0200, Andy Shevchenko wrote:
> On Wed, Jan 3, 2018 at 4:53 PM, Ognjen Galić <smclt30p@xxxxxxxxx> wrote:
> > On Wed, Jan 03, 2018 at 04:25:42PM +0200, Andy Shevchenko wrote:
> >> On Wed, Jan 3, 2018 at 1:58 PM, Ognjen Galic <smclt30p@xxxxxxxxx> wrote:
>
> >> Thanks for an update. I have couple of minors. Otherwise look pretty much good!
> >>
> >> > drivers/acpi/battery.h | 11 ----
> >> > include/acpi/battery.h | 21 +++++++
> >>
> >> There are -M and -C command line parameters to git format-patch.
>
> >> They can take an optional argument (percentage) of threshold.
> >>
> >> Playing with those numbers you can achieve
>
> ^^^^ Pay attention to the above
>
> >>
> >> rename ...
> >>
> >> line and see actual diff.
> >>
> >> No need to resend because of this. Just an explanation for the future Git work.
> >
> > I did use thos options. I used the following command:
> >
> > git format-patch -M -C --notes -v12 -o ~/patches/. @^^^^
> >
> > I really don't know what you are targeting. :)
>
> Please, read what I wrote above and the manual of git-format-patch.
>
> >> > +void __battery_hook_unregister(struct acpi_battery_hook *hook, int lock)
> >> > +{
> >> > + struct list_head *position;
> >> > + struct acpi_battery *battery;
> >>
> >> Missed empty line?
> >
> > checkpatch.pl complains if there are NOT empty lines between
> > declarations and statements.
>
> checkpatch some times on one hand complains about something which it
> should not, on the other didn't take into consideration cases like
> this one.
>
> Your statement started with comment, btw.
>
> >> > + /*
> >> > + * In order to remove a hook, we first need to
> >> > + * de-register all the batteries that are registered.
> >> > + */
> >> > + if (lock)
> >> > + mutex_lock(&hook_mutex);
>
> > I mean, it's not game-breaking, its just minor style stuff. I won't be
> > sending more revisions because of these small issues, as I think its
> > uneccessary to flood both Rafael and the mailing lists with patch
> > revisions that remove or add a few spaces. No offence, it just got old.
>
> Yes, his call anyway to apply or ask you for amendments. I'm just
> helping with review.
Rafael, what do you think? Do you want these style/syntax issues fixed
or is it good to go?
Thanks for the time to review Andy! You are a champ! :)
>
> >> > +static void __exit battery_hook_exit(void)
> >> > +{
> >> > + struct acpi_battery_hook *hook;
> >> > + struct acpi_battery_hook *ptr;
> >>
> >> Missed empty line?
> >
> > See above about the checkpatch.pl stuff.
>
> See above the answer.
>
>
> --
> With Best Regards,
> Andy Shevchenko
