Re: [PATCH v6] hid-logitech-hidpp: read battery voltage from newer devices

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

 



On Sun, Oct 27, 2019 at 12:26 AM Pedro Vanzella <pedro@xxxxxxxxxxxxxxxxx> 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 charge status. The device will respond with three bytes: the first
> two are its voltage, and the last one is a bitset, reporting if the
> battery is charging, fast or slow, in critical level or discharging.
>
> 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 mV.
> We store that as-is, but when queried, we report it in uV as
> expected by sysfs.
>
> 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@xxxxxxxxxxxxxxxxx>
> ---

Applied to for-5.6/logitech, sorry for the delay.

Cheers,
Benjamin

>  drivers/hid/hid-logitech-hidpp.c | 172 ++++++++++++++++++++++++++++++-
>  1 file changed, 168 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 0179f7ed77e5..959586dab535 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,15 @@ 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;
> +       int charge_type;
>         bool online;
>  };
>
> @@ -1219,6 +1223,144 @@ 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 *level, int *charge_type)
> +{
> +       int status;
> +
> +       long charge_sts = (long)data[2];
> +
> +       *level = POWER_SUPPLY_CAPACITY_LEVEL_UNKNOWN;
> +       switch (data[2] & 0xe0) {
> +       case 0x00:
> +               status = POWER_SUPPLY_STATUS_CHARGING;
> +               break;
> +       case 0x20:
> +               status = POWER_SUPPLY_STATUS_FULL;
> +               *level = POWER_SUPPLY_CAPACITY_LEVEL_FULL;
> +               break;
> +       case 0x40:
> +               status = POWER_SUPPLY_STATUS_DISCHARGING;
> +               break;
> +       case 0xe0:
> +               status = POWER_SUPPLY_STATUS_NOT_CHARGING;
> +               break;
> +       default:
> +               status = POWER_SUPPLY_STATUS_UNKNOWN;
> +       }
> +
> +       *charge_type = POWER_SUPPLY_CHARGE_TYPE_STANDARD;
> +       if (test_bit(3, &charge_sts)) {
> +               *charge_type = POWER_SUPPLY_CHARGE_TYPE_FAST;
> +       }
> +       if (test_bit(4, &charge_sts)) {
> +               *charge_type = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
> +       }
> +
> +       if (test_bit(5, &charge_sts)) {
> +               *level = POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL;
> +       }
> +
> +       *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,
> +                                                int *level, int *charge_type)
> +{
> +       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_BATTERY_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,
> +                                                    level, charge_type);
> +
> +       return 0;
> +}
> +
> +static int hidpp20_query_battery_voltage_info(struct hidpp_device *hidpp)
> +{
> +       u8 feature_type;
> +       int ret;
> +       int status, voltage, level, charge_type;
> +
> +       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, &level, &charge_type);
> +
> +       if (ret)
> +               return ret;
> +
> +       hidpp->battery.status = status;
> +       hidpp->battery.voltage = voltage;
> +       hidpp->battery.level = level;
> +       hidpp->battery.charge_type = charge_type;
> +       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, level, charge_type;
> +
> +       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,
> +                                                   &level, &charge_type);
> +
> +       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;
> +               hidpp->battery.level = level;
> +               hidpp->battery.charge_type = charge_type;
> +               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 +1370,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 +1408,13 @@ 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:
> +                       /* hardware reports voltage in in mV. sysfs expects uV */
> +                       val->intval = hidpp->battery.voltage * 1000;
> +                       break;
> +               case POWER_SUPPLY_PROP_CHARGE_TYPE:
> +                       val->intval = hidpp->battery.charge_type;
> +                       break;
>                 default:
>                         ret = -EINVAL;
>                         break;
> @@ -3083,6 +3233,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 +3357,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 +3391,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 +3401,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 +3568,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);
> --
> 2.23.0
>




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux