linux-kernel - Re: [PATCH 1/2] HID: logitech-hidpp: add battery support for HID++ 2.0 devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160711105456.GA4663@mail.corp.redhat.com>
Date:	Mon, 11 Jul 2016 12:54:56 +0200
From:	Benjamin Tissoires <benjamin.tissoires@...hat.com>
To:	Bastien Nocera <hadess@...ess.net>
Cc:	Peter Hutterer <peter.hutterer@...-t.net>,
	Jiri Kosina <jikos@...nel.org>, linux-input@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Nestor Lopez Casado <nlopezcasad@...itech.com>
Subject: Re: [PATCH 1/2] HID: logitech-hidpp: add battery support for HID++
 2.0 devices

On Jul 08 2016 or thereabouts, Bastien Nocera wrote:
> On Wed, 2016-06-29 at 19:28 +1000, Peter Hutterer wrote:
> > +static int hidpp_battery_get_property(struct power_supply *psy,
> > +                                     enum power_supply_property psp,
> > +                                     union power_supply_propval
> > *val)
> > +{
> > +       struct hidpp_device *hidpp = power_supply_get_drvdata(psy);
> > +       int ret = 0;
> > +
> > +       switch(psp) {
> > +               case POWER_SUPPLY_PROP_STATUS:
> > +                       val->intval = hidpp->battery.status;
> > +                       break;
> > +               case POWER_SUPPLY_PROP_CAPACITY:
> > +                       val->intval = hidpp->battery.level;
> > +                       break;
> > +               default:
> 
> You forgot to handle POWER_SUPPLY_PROP_SCOPE. This means that UPower
> thinks it's supplying power to the computer to which it is connected.
> 
> Should be set to "POWER_SUPPLY_SCOPE_DEVICE". This should fix it.
> 
> From 8fbfcfd411a4b2c55ec24adc8b8ecc0bca2db5e3 Mon Sep 17 00:00:00 2001
> From: Bastien Nocera <hadess@...ess.net>
> Date: Fri, 8 Jul 2016 16:34:18 +0200
> Subject: [PATCH] HID: logitech-hidpp: Add scope to battery
> 
> Without a scope defined, UPower assumes that the battery is provide
> power to the computer it's connected to, like a laptop battery or a UPS.
> 
> Signed-off-by: Bastien Nocera <hadess@...ess.net>

FWIW, the following patch is:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@...hat.com>

> ---
>  drivers/hid/hid-logitech-hidpp.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 4eeb550..4aaf237 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -761,6 +761,7 @@ static int hidpp20_battery_event(struct hidpp_device *hidpp,
>  static enum power_supply_property hidpp_battery_props[] = {
>         POWER_SUPPLY_PROP_STATUS,
>         POWER_SUPPLY_PROP_CAPACITY,
> +       POWER_SUPPLY_PROP_SCOPE,
>  };
>  
>  static int hidpp_battery_get_property(struct power_supply *psy,
> @@ -777,6 +778,9 @@ static int hidpp_battery_get_property(struct power_supply *psy,
>                 case POWER_SUPPLY_PROP_CAPACITY:
>                         val->intval = hidpp->battery.level;
>                         break;
> +               case POWER_SUPPLY_PROP_SCOPE:
> +                       val->intval = POWER_SUPPLY_SCOPE_DEVICE;
> +                       break;
>                 default:
>                         ret = -EINVAL;
>                         break;
> -- 
> 2.7.4
>