Re: [PATCH] hid: Initialize battery_no to -1 & correct its format string

[Benjamin Tissoires]

On Tue, Jan 6, 2015 at 7:50 PM, Ping Cheng <pinglinux@xxxxxxxxx> wrote:
> Hi Benjamin,
>
> The relevant code was introduced by
> d70420b914c98a3758674c6e9858810e0ab4ea30. Can you take a look and let
> us know if Aniroop's patch fits your original thought or not?

Hehe, indeed, that's my code.

I would agree with Jiri, I really do not care. This code is called
only once at initialization and I am pretty sure it will not change
the time to initialize the device (I am confident the compiler will do
something better than blindly following the code).

So either way it is good. But I have a slight preference to initialize
the variable to 0 and keep the unsigned in the name.

Cheers,
Benjamin

>
> Thanks,
>
> Ping
>
> On Tue, Jan 6, 2015 at 6:32 AM, Aniroop Mathur <aniroop.mathur@xxxxxxxxx> wrote:
>> Dear Mr. Jason and Mr. Ping,
>>
>> Please update about below patch.
>> Except avoiding subtraction, there is also a need to avoid negative
>> battery name which may come due to %ld, so need to change to %lu.
>>
>> Thanks,
>> Aniroop
>>
>> On Mon, Dec 29, 2014 at 5:33 PM, Jiri Kosina <jkosina@xxxxxxx> wrote:
>>> On Sun, 28 Dec 2014, Aniroop Mathur wrote:
>>>
>>>> From: Aniroop Mathur <a.mathur@xxxxxxxxxxx>
>>>>
>>>> This patch initializes battery_no to -1 to avoid extra subtraction
>>>> operation performed everytime wacom battery is initialized
>>>
>>> This is so femto-optimization, that I don't really care deeply. Adding
>>> Jason and Ping to CC. If they want this, I'll apply the patch.
>>>
>>>> and correct format string of unsigned long from %ld to %lu.
>>>>
>>>> Signed-off-by: Aniroop Mathur <a.mathur@xxxxxxxxxxx>
>>>> ---
>>>>  drivers/hid/wacom_sys.c | 8 ++++----
>>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
>>>> index 129fd33..4b5ff84 100644
>>>> --- a/drivers/hid/wacom_sys.c
>>>> +++ b/drivers/hid/wacom_sys.c
>>>> @@ -919,17 +919,17 @@ static int wacom_ac_get_property(struct power_supply *psy,
>>>>
>>>>  static int wacom_initialize_battery(struct wacom *wacom)
>>>>  {
>>>> -     static atomic_t battery_no = ATOMIC_INIT(0);
>>>> +     static atomic_t battery_no = ATOMIC_INIT(-1);
>>>>       int error;
>>>>       unsigned long n;
>>>>
>>>>       if (wacom->wacom_wac.features.quirks & WACOM_QUIRK_BATTERY) {
>>>> -             n = atomic_inc_return(&battery_no) - 1;
>>>> +             n = atomic_inc_return(&battery_no);
>>>>
>>>>               wacom->battery.properties = wacom_battery_props;
>>>>               wacom->battery.num_properties = ARRAY_SIZE(wacom_battery_props);
>>>>               wacom->battery.get_property = wacom_battery_get_property;
>>>> -             sprintf(wacom->wacom_wac.bat_name, "wacom_battery_%ld", n);
>>>> +             sprintf(wacom->wacom_wac.bat_name, "wacom_battery_%lu", n);
>>>>               wacom->battery.name = wacom->wacom_wac.bat_name;
>>>>               wacom->battery.type = POWER_SUPPLY_TYPE_BATTERY;
>>>>               wacom->battery.use_for_apm = 0;
>>>> @@ -937,7 +937,7 @@ static int wacom_initialize_battery(struct wacom *wacom)
>>>>               wacom->ac.properties = wacom_ac_props;
>>>>               wacom->ac.num_properties = ARRAY_SIZE(wacom_ac_props);
>>>>               wacom->ac.get_property = wacom_ac_get_property;
>>>> -             sprintf(wacom->wacom_wac.ac_name, "wacom_ac_%ld", n);
>>>> +             sprintf(wacom->wacom_wac.ac_name, "wacom_ac_%lu", n);
>>>>               wacom->ac.name = wacom->wacom_wac.ac_name;
>>>>               wacom->ac.type = POWER_SUPPLY_TYPE_MAINS;
>>>>               wacom->ac.use_for_apm = 0;
>>>> --
>>>> 1.9.1
>>>>
>>>
>>> --
>>> Jiri Kosina
>>> SUSE Labs
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-input" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/