Re: [PATCH v12 1/4] battery: Add the battery hooking API

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

 



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.

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

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel




[Index of Archives]     [Linux ACPI]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Photo]     [Yosemite Photos]     [Yosemite Advice]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux