| Message ID | 20210522180611.314300-1-jose.exposito89@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/5] HID: magicmouse: register power supply | expand |
On Sat, 22 May 2021, José Expósito wrote: > Unlike the Apple Magic Mouse 1 and the Apple Magic Trackpad 1, the > second generation of the devices don't report their battery status > automatically. > > This patchset adds support for reporting the battery capacity and > charging status for the Apple Magic Mouse 2 and Apple Magic Trackpad > 2 both over bluetooth and USB. > > This patch: > > Register the required power supply structs for the Apple Magic Mouse 2 > and the Apple Magic Trackpad 2 to be able to report battery capacity > and status in future patches. > > Signed-off-by: José Expósito <jose.exposito89@gmail.com> > > --- > > v2: Add depends on USB_HID to Kconfig Hmm, why is this dependency needed in the first place, please? I think trying to keep the drivers independent on transport drivers (especially in cases like this, where more variants of physical transports actually really do exist) is worth trying. Thanks, -- Jiri Kosina SUSE Labs
Hi Jiri, First of all, thank you for taking the time to review my patches. On Thu, Jun 24, 2021 at 03:33:39PM +0200, Jiri Kosina wrote: > On Sat, 22 May 2021, José Expósito wrote: > > > [...] > > v2: Add depends on USB_HID to Kconfig > > Hmm, why is this dependency needed in the first place, please? I think > trying to keep the drivers independent on transport drivers (especially in > cases like this, where more variants of physical transports actually > really do exist) is worth trying. Sorry, that's something I should have explained in the changelog. Intel's test bot reported compilation errors on the first version of the patch when USB support wasn't configured: https://lore.kernel.org/patchwork/patch/1425313/ I was kindly pointed to a similar error and its fix, but, maybe in this case this is not the right fix? Maybe there is a macro that I can use to wrap the USB related code in an #ifdef? Thanks, Jose
On Fri, 25 Jun 2021, José Expósito wrote: > > > [...] > > > v2: Add depends on USB_HID to Kconfig > > > > Hmm, why is this dependency needed in the first place, please? I think > > trying to keep the drivers independent on transport drivers (especially in > > cases like this, where more variants of physical transports actually > > really do exist) is worth trying. > > Sorry, that's something I should have explained in the changelog. > > Intel's test bot reported compilation errors on the first version of the patch > when USB support wasn't configured: > https://lore.kernel.org/patchwork/patch/1425313/ > > I was kindly pointed to a similar error and its fix, but, maybe in this case this > is not the right fix? > Maybe there is a macro that I can use to wrap the USB related code in an #ifdef? It can certainly be wrapped, but looking into the code now, it probably wouldn't really bring more clarity. I will apply the series with adding the USB_HID dependency for now. Thanks, -- Jiri Kosina SUSE Labs
On Wed, Jul 28, 2021 at 11:35:20AM +0200, Jiri Kosina wrote: > On Fri, 25 Jun 2021, José Expósito wrote: > > > > > [...] > > > > v2: Add depends on USB_HID to Kconfig > > > > > > Hmm, why is this dependency needed in the first place, please? I think > > > trying to keep the drivers independent on transport drivers (especially in > > > cases like this, where more variants of physical transports actually > > > really do exist) is worth trying. > > > > Sorry, that's something I should have explained in the changelog. > > > > Intel's test bot reported compilation errors on the first version of the patch > > when USB support wasn't configured: > > https://lore.kernel.org/patchwork/patch/1425313/ > > > > I was kindly pointed to a similar error and its fix, but, maybe in this case this > > is not the right fix? > > Maybe there is a macro that I can use to wrap the USB related code in an #ifdef? > > It can certainly be wrapped, but looking into the code now, it probably > wouldn't really bring more clarity. I will apply the series with adding > the USB_HID dependency for now. Hi Jiri, I've been investigating a bit about this issue and I think this might not be the righ solution for the problem. John Chen's patch (9de07a4e8d4cb269f9876b2ffa282b5ffd09e05b): https://lore.kernel.org/lkml/20210327130508.24849-5-johnchen902@gmail.com/ Already adds battery reporting over bluetooth, so my patch is redundant... And worse than his, I should add. I was investigating how to do something similar over USB, but I couldn't finish a patch yet. So, if you don't mind, I'd prefer not to apply this patchset yet until I figure out a better solution on v3. Thanks, Jose
On Wed, 28 Jul 2021, José Expósito wrote: > So, if you don't mind, I'd prefer not to apply this patchset yet until I > figure out a better solution on v3. Thanks for the heads up. I am dropping it for now. -- Jiri Kosina SUSE Labs
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c index 2bb473d8c424..0f766bce4537 100644 --- a/drivers/hid/hid-magicmouse.c +++ b/drivers/hid/hid-magicmouse.c @@ -112,6 +112,9 @@ MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state fie * @scroll_jiffies: Time of last scroll motion. * @touches: Most recent data for a touch, indexed by tracking ID. * @tracking_ids: Mapping of current touch input data to @touches. + * @battery: Required data to report the battery status of the Apple Magic + * Mouse 2 and Apple Magic Trackpad 2. Battery is reported automatically on the + * first generation of the devices. */ struct magicmouse_sc { struct input_dev *input; @@ -132,8 +135,89 @@ struct magicmouse_sc { struct hid_device *hdev; struct delayed_work work; + + struct { + struct power_supply *ps; + struct power_supply_desc ps_desc; + } battery; +}; + +static enum power_supply_property magicmouse_ps_props[] = { + POWER_SUPPLY_PROP_PRESENT, + POWER_SUPPLY_PROP_SCOPE, }; +static bool magicmouse_can_report_battery(struct magicmouse_sc *msc) +{ + return (msc->input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) || + (msc->input->id.product == USB_DEVICE_ID_APPLE_MAGICMOUSE2); +} + +static int magicmouse_battery_get_property(struct power_supply *psy, + enum power_supply_property psp, + union power_supply_propval *val) +{ + struct magicmouse_sc *msc = power_supply_get_drvdata(psy); + int ret = 0; + + if (!magicmouse_can_report_battery(msc)) + return -EINVAL; + + switch (psp) { + case POWER_SUPPLY_PROP_PRESENT: + val->intval = 1; + break; + case POWER_SUPPLY_PROP_SCOPE: + val->intval = POWER_SUPPLY_SCOPE_DEVICE; + break; + default: + ret = -EINVAL; + break; + } + + return ret; +} + +static int magicmouse_battery_probe(struct hid_device *hdev) +{ + struct magicmouse_sc *msc = hid_get_drvdata(hdev); + struct power_supply *ps = NULL; + struct power_supply_config ps_cfg = { .drv_data = msc }; + int ret; + + if (!magicmouse_can_report_battery(msc)) + return 0; + + msc->battery.ps_desc.type = POWER_SUPPLY_TYPE_BATTERY; + msc->battery.ps_desc.properties = magicmouse_ps_props; + msc->battery.ps_desc.num_properties = ARRAY_SIZE(magicmouse_ps_props); + msc->battery.ps_desc.get_property = magicmouse_battery_get_property; + msc->battery.ps_desc.name = kasprintf(GFP_KERNEL, "magic_trackpad_2_%s", + msc->input->uniq); + if (!msc->battery.ps_desc.name) { + hid_err(hdev, "unable to register ps_desc name, ENOMEM\n"); + return -ENOMEM; + } + + ps = devm_power_supply_register(&hdev->dev, &msc->battery.ps_desc, + &ps_cfg); + if (IS_ERR(ps)) { + ret = PTR_ERR(ps); + hid_err(hdev, "unable to register battery device: %d\n", ret); + return ret; + } + + msc->battery.ps = ps; + + ret = power_supply_powers(msc->battery.ps, &hdev->dev); + if (ret) { + hid_err(hdev, "unable to activate battery device: %d\n", ret); + return ret; + } + + return 0; +} + static int magicmouse_firm_touch(struct magicmouse_sc *msc) { int touch = -1; @@ -726,6 +810,12 @@ static int magicmouse_probe(struct hid_device *hdev, goto err_stop_hw; } + ret = magicmouse_battery_probe(hdev); + if (ret) { + hid_err(hdev, "battery not registered\n"); + goto err_stop_hw; + } + if (id->product == USB_DEVICE_ID_APPLE_MAGICMOUSE) report = hid_register_report(hdev, HID_INPUT_REPORT, MOUSE_REPORT_ID, 0);
Unlike the Apple Magic Mouse 1 and the Apple Magic Trackpad 1, the second generation of the devices don't report their battery status automatically. This patchset adds support for reporting the battery capacity and charging status for the Apple Magic Mouse 2 and Apple Magic Trackpad 2 both over bluetooth and USB. This patch: Register the required power supply structs for the Apple Magic Mouse 2 and the Apple Magic Trackpad 2 to be able to report battery capacity and status in future patches. Signed-off-by: José Expósito <jose.exposito89@gmail.com> --- v2: Add depends on USB_HID to Kconfig --- drivers/hid/hid-magicmouse.c | 90 ++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+)