| 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
| ||
|
Message-ID: <9203df304b8ab11688ca15e56cc866982796894d.camel@archlinux.org>
Date: Mon, 02 Sep 2019 09:03:12 +0100
From: Filipe Laíns <lains@...hlinux.org>
To: Pedro Vanzella <pedro@...rovanzella.com>,
linux-input@...r.kernel.org
Cc: Jiri Kosina <jikos@...nel.org>,
Benjamin Tissoires <benjamin.tissoires@...hat.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5] hid-logitech-hidpp: read battery voltage from newer
devices
On Sat, 2019-08-31 at 13:56 -0400, Pedro Vanzella wrote:
> Newer Logitech mice report their battery voltage through feature
> 0x1001
> instead of the battery levels through feature 0x1000.
>
> When the device is brought up and we try to query the battery, figure
> out if it supports the old or the new feature. If it supports the new
> feature, record the feature index and read the battery voltage and
> its status.
>
> If everything went correctly, record the fact that we're capable
> of querying battery voltage.
>
> Note that the protocol only gives us the current voltage in
> millivolts.
>
> Like we do for other ways of interacting with the battery for other
> devices, refresh the battery status and notify the power supply
> subsystem of the changes in voltage and status.
>
> Since there are no known devices which implement both the old and the
> new battery feature, we make sure to try the newer feature first and
> only fall back to the old one if that fails.
>
> Signed-off-by: Pedro Vanzella <pedro@...rovanzella.com>
> ---
> drivers/hid/hid-logitech-hidpp.c | 141
> ++++++++++++++++++++++++++++++-
> 1 file changed, 137 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-
> logitech-hidpp.c
> index 0179f7ed77e5..2f215e5be001 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -87,6 +87,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
> #define HIDPP_CAPABILITY_HIDPP20_BATTERY BIT(1)
> #define HIDPP_CAPABILITY_BATTERY_MILEAGE BIT(2)
> #define HIDPP_CAPABILITY_BATTERY_LEVEL_STATUS BIT(3)
> +#define HIDPP_CAPABILITY_BATTERY_VOLTAGE BIT(4)
>
> /*
> * There are two hidpp protocols in use, the first version hidpp10
> is known
> @@ -135,12 +136,14 @@ struct hidpp_report {
> struct hidpp_battery {
> u8 feature_index;
> u8 solar_feature_index;
> + u8 voltage_feature_index;
> struct power_supply_desc desc;
> struct power_supply *ps;
> char name[64];
> int status;
> int capacity;
> int level;
> + int voltage; /* in millivolts */
> bool online;
> };
>
> @@ -1219,6 +1222,118 @@ static int hidpp20_battery_event(struct
> hidpp_device *hidpp,
> return 0;
> }
>
> +/* ---------------------------------------------------------------
> ----------- */
> +/* 0x1001: Battery
> voltage */
> +/* ---------------------------------------------------------------
> ----------- */
> +
> +#define HIDPP_PAGE_BATTERY_VOLTAGE 0x1001
> +
> +#define CMD_BATTERY_VOLTAGE_GET_BATTERY_VOLTAGE 0x00
> +
> +#define EVENT_BATTERY_VOLTAGE_STATUS_BROADCAST 0x00
> +
> +static int hidpp20_battery_map_status_voltage(u8 data[3], int
> *voltage)
> +{
> + int status;
> +
> + switch (data[2]) {
> + case 0x00: /* discharging */
> + status = POWER_SUPPLY_STATUS_DISCHARGING;
> + break;
> + case 0x10: /* wireless charging */
> + case 0x80: /* charging */
> + status = POWER_SUPPLY_STATUS_CHARGING;
> + break;
> + case 0x81: /* fully charged */
> + status = POWER_SUPPLY_STATUS_FULL;
> + break;
> + default:
> + status = POWER_SUPPLY_STATUS_NOT_CHARGING;
> + }
> +
> + *voltage = get_unaligned_be16(data);
> +
> + return status;
> +}
> +
> +static int hidpp20_battery_get_battery_voltage(struct hidpp_device
> *hidpp,
> + u8 feature_index,
> + int *status, int
> *voltage)
> +{
> + struct hidpp_report response;
> + int ret;
> + u8 *params = (u8 *)response.fap.params;
> +
> + ret = hidpp_send_fap_command_sync(hidpp, feature_index,
> + CMD_BATTERY_VOLTAGE_GET_BATTE
> RY_VOLTAGE,
> + NULL, 0, &response);
> +
> + if (ret > 0) {
> + hid_err(hidpp->hid_dev, "%s: received protocol error
> 0x%02x\n",
> + __func__, ret);
> + return -EPROTO;
> + }
> + if (ret)
> + return ret;
> +
> + hidpp->capabilities |= HIDPP_CAPABILITY_BATTERY_VOLTAGE;
> +
> + *status = hidpp20_battery_map_status_voltage(params, voltage);
> +
> + return 0;
> +}
> +
> +static int hidpp20_query_battery_voltage_info(struct hidpp_device
> *hidpp)
> +{
> + u8 feature_type;
> + int ret;
> + int status, voltage;
> +
> + if (hidpp->battery.voltage_feature_index == 0xff) {
> + ret = hidpp_root_get_feature(hidpp,
> HIDPP_PAGE_BATTERY_VOLTAGE,
> + &hidpp-
> >battery.voltage_feature_index,
> + &feature_type);
> + if (ret)
> + return ret;
> + }
> +
> + ret = hidpp20_battery_get_battery_voltage(hidpp,
> + hidpp-
> >battery.voltage_feature_index,
> + &status, &voltage);
> +
> + if (ret)
> + return ret;
> +
> + hidpp->battery.status = status;
> + hidpp->battery.voltage = voltage;
> + hidpp->battery.online = status !=
> POWER_SUPPLY_STATUS_NOT_CHARGING;
> +
> + return 0;
> +}
> +
> +static int hidpp20_battery_voltage_event(struct hidpp_device *hidpp,
> + u8 *data, int size)
> +{
> + struct hidpp_report *report = (struct hidpp_report *)data;
> + int status, voltage;
> +
> + if (report->fap.feature_index != hidpp-
> >battery.voltage_feature_index ||
> + report->fap.funcindex_clientid !=
> EVENT_BATTERY_VOLTAGE_STATUS_BROADCAST)
> + return 0;
> +
> + status = hidpp20_battery_map_status_voltage(report->fap.params,
> &voltage);
> +
> + hidpp->battery.online = status !=
> POWER_SUPPLY_STATUS_NOT_CHARGING;
> +
> + if (voltage != hidpp->battery.voltage || status != hidpp-
> >battery.status) {
> + hidpp->battery.voltage = voltage;
> + hidpp->battery.status = status;
> + if (hidpp->battery.ps)
> + power_supply_changed(hidpp->battery.ps);
> + }
> + return 0;
> +}
> +
> static enum power_supply_property hidpp_battery_props[] = {
> POWER_SUPPLY_PROP_ONLINE,
> POWER_SUPPLY_PROP_STATUS,
> @@ -1228,6 +1343,7 @@ static enum power_supply_property
> hidpp_battery_props[] = {
> POWER_SUPPLY_PROP_SERIAL_NUMBER,
> 0, /* placeholder for POWER_SUPPLY_PROP_CAPACITY, */
> 0, /* placeholder for POWER_SUPPLY_PROP_CAPACITY_LEVEL, */
> + 0, /* placeholder for POWER_SUPPLY_PROP_VOLTAGE_NOW, */
> };
>
> static int hidpp_battery_get_property(struct power_supply *psy,
> @@ -1265,6 +1381,9 @@ static int hidpp_battery_get_property(struct
> power_supply *psy,
> case POWER_SUPPLY_PROP_SERIAL_NUMBER:
> val->strval = hidpp->hid_dev->uniq;
> break;
> + case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> + val->intval = hidpp->battery.voltage;
> + break;
> default:
> ret = -EINVAL;
> break;
> @@ -3083,6 +3202,9 @@ static int hidpp_raw_hidpp_event(struct
> hidpp_device *hidpp, u8 *data,
> ret = hidpp_solar_battery_event(hidpp, data, size);
> if (ret != 0)
> return ret;
> + ret = hidpp20_battery_voltage_event(hidpp, data, size);
> + if (ret != 0)
> + return ret;
> }
>
> if (hidpp->capabilities & HIDPP_CAPABILITY_HIDPP10_BATTERY) {
> @@ -3204,12 +3326,16 @@ static int hidpp_initialize_battery(struct
> hidpp_device *hidpp)
>
> hidpp->battery.feature_index = 0xff;
> hidpp->battery.solar_feature_index = 0xff;
> + hidpp->battery.voltage_feature_index = 0xff;
>
> if (hidpp->protocol_major >= 2) {
> if (hidpp->quirks & HIDPP_QUIRK_CLASS_K750)
> ret = hidpp_solar_request_battery_event(hidpp);
> - else
> - ret = hidpp20_query_battery_info(hidpp);
> + else {
> + ret =
> hidpp20_query_battery_voltage_info(hidpp);
> + if (ret)
> + ret =
> hidpp20_query_battery_info(hidpp);
> + }
>
> if (ret)
> return ret;
> @@ -3234,7 +3360,7 @@ static int hidpp_initialize_battery(struct
> hidpp_device *hidpp)
> if (!battery_props)
> return -ENOMEM;
>
> - num_battery_props = ARRAY_SIZE(hidpp_battery_props) - 2;
> + num_battery_props = ARRAY_SIZE(hidpp_battery_props) - 3;
>
> if (hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_MILEAGE)
> battery_props[num_battery_props++] =
> @@ -3244,6 +3370,10 @@ static int hidpp_initialize_battery(struct
> hidpp_device *hidpp)
> battery_props[num_battery_props++] =
> POWER_SUPPLY_PROP_CAPACITY_LEVEL;
>
> + if (hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_VOLTAGE)
> + battery_props[num_battery_props++] =
> + POWER_SUPPLY_PROP_VOLTAGE_NOW;
> +
> battery = &hidpp->battery;
>
> n = atomic_inc_return(&battery_no) - 1;
> @@ -3407,7 +3537,10 @@ static void hidpp_connect_event(struct
> hidpp_device *hidpp)
> else
> hidpp10_query_battery_status(hidpp);
> } else if (hidpp->capabilities &
> HIDPP_CAPABILITY_HIDPP20_BATTERY) {
> - hidpp20_query_battery_info(hidpp);
> + if (hidpp->capabilities &
> HIDPP_CAPABILITY_BATTERY_VOLTAGE)
> + hidpp20_query_battery_voltage_info(hidpp);
> + else
> + hidpp20_query_battery_info(hidpp);
> }
> if (hidpp->battery.ps)
> power_supply_changed(hidpp->battery.ps);
Reviewed-by: Filipe Laíns <lains@...hlinux.org>
Thank you,
Filipe Laíns
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists